Re: [PATCH 12/15] hw/nvme: Initialize capability structures for primary/secondary controllers

[Klaus Jensen]

On Oct  7 18:24, Lukasz Maniak wrote:
> From: Łukasz Gieryk <lukasz.gieryk@linux.intel.com>
> 
> With two new properties (sriov_max_vi_per_vf, sriov_max_vq_per_vf) one
> can configure the maximum number of virtual queues and interrupts
> assignable to a single virtual device. The primary and secondary
> controller capability structures are initialized accordingly.
> 
> Since the number of available queues (interrupts) now varies between
> VF/PF, BAR size calculation is also adjusted.
> 

While this patch allows configuring the VQFRSM and VIFRSM fields, it
implicitly sets VQFRT and VIFRT (i.e. by setting them to the product of
sriov_max_vi_pervf and max_vfs). Which is just setting it to an upper
bound and this removes a testable case for host software (e.g.
requesting more flexible resources than what is currently available).

This patch also requires that these parameters are set if sriov_max_vfs
is. I think we can provide better defaults.

How about,

1. if only sriov_max_vfs is set, then all VFs get private resources
   equal to max_ioqpairs. Like before this patch. This limits the number
   of parameters required to get a basic setup going.

2. if sriov_v{q,i}_private is set (I suggested this parameter in patch
   10), the difference between that and max_ioqpairs become flexible
   resources. Also, I'd be just fine with having sriov_v{q,i}_flexible
   instead and just make the difference become private resources.
   Potato/potato.

   a. in the absence of sriov_max_v{q,i}_per_vf, set them to the number
      of calculated flexible resources.

This probably smells a bit like bikeshedding, but I think this gives
more flexibility and better defaults, which helps with verifying host
software.

If we can't agree on this now, I suggest we could go ahead and merge the
base functionality (i.e. private resources only) and ruminate some more
about these parameters.

signature.asc
Description: PGP signature