On Fri, Nov 05, 2021 at 09:46:28AM +0100, Łukasz Gieryk wrote: > On Thu, Nov 04, 2021 at 04:48:43PM +0100, Łukasz Gieryk wrote: > > 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. > > > > I’ve just re-read through my email and decided that some bits need > clarification. > > This implementation supports the “Flexible”-resources-only flavor of > SR/IOV, while the “Private” also could be supported. Some effort is > required to support both, and I cannot afford that (at least I cannot > commit today, neither the other Lukasz). > > While I’m ready to rework the Flexible config and prepare it to be > extended later to handle the Private variant, the 2nd version of these > patches will still support the Flexible flavor only. > > I will include appropriate TODO/open in the next cover letter. >
The summary of my thoughts, so far: - I'm going to introduce sriov_v{q,i}_flexible and better defaults, according to your suggestion (as far as I understand your intentions, please correct me if I've missed something). - The Private SR/IOV flavor, if it's ever implemented, could introduce sriov_vq_private_per_vf. - The updated formulas are listed below. Flexible resources not supported (not implemented): - Private resources for PF = max_ioqpairs - Private resources per VF = sriov_vq_private_per_vf - (error if sriov_vq_flexible is set) With flexible resources: - VQPRT, private resources for PF = max_ioqpairs - sriov_vq_flexible - VQFRT, total flexible resources = sriov_vq_flexible (if set, or) VQPRT * num_vfs - VQFRSM, maximum assignable per VF = sriov_max_vq_per_vf (if set, or) VQPRT - VQGRAN, granularity = #define constant - (error if sriov_vq_private_per_vf is set) Is this version acceptable?