On Wed, Nov 03, 2021 at 01:07:31PM +0100, Klaus Jensen wrote: > On Oct 7 18:24, Lukasz Maniak wrote: > > From: Łukasz Gieryk <lukasz.gie...@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. >
Originally I considered more params, but ended up coding the simplest, user-friendly solution, because I did not like the mess with so many parameters, and the flexibility wasn't needed for my use cases. But I do agree: others may need the flexibility. Case (FRT < max_vfs * FRSM) is valid and resembles an actual device. > 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. The problem is that the spec allows VFs to support either only private, or only flexible resources. At this point I have to admit, that since my use cases for QEMU/Nvme/SRIOV require flexible resources, I haven’t paid much attention to the case with VFs having private resources. So this SR/IOV implementation doesn’t even support such case (max_vX_per_vf != 0). Let me summarize the possible config space, and how the current parameters (could) map to these (interrupt-related ones omitted): Flexible resources not supported (not implemented): - Private resources for PF = max_ioqpairs - Private resources per VF = ? - (error if flexible resources are configured) With flexible resources: - VQPRT, private resources for PF = max_ioqpairs - VQFRT, total flexible resources = max_vq_per_vf * num_vfs - VQFRSM, maximum assignable per VF = max_vq_per_vf - VQGRAN, granularity = #define constant - (error if private resources per VF are configured) Since I don’t want to misunderstand your suggestion: could you provide a similar map with your parameters, formulas, and explain how to determine if flexible resources are active? I want to be sure we are on the same page. -- Regards, Łukasz