[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Klaus Jensen
Subject: Re: [PATCH 12/15] hw/nvme: Initialize capability structures for primary/secondary controllers
Date: Wed, 3 Nov 2021 13:07:31 +0100

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.

   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

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.

Attachment: signature.asc
Description: PGP signature

reply via email to

[Prev in Thread] Current Thread [Next in Thread]