On Tue, Jan 25, 2022 at 6:59 PM Max Gurtovoy <mgurto...@nvidia.com> wrote: > > > On 1/25/2022 5:52 AM, Parav Pandit wrote: > > Hi Jason, > > > >> From: Jason Wang <jasow...@redhat.com> > >> Sent: Tuesday, January 25, 2022 8:59 AM > >> > >> 在 2022/1/19 下午12:48, Parav Pandit 写道: > >>>> From: Jason Wang <jasow...@redhat.com> > >>>> Sent: Wednesday, January 19, 2022 9:33 AM > >>>> > >>>> > >>>> It it means IMS, there's already a proposal[1] that introduce MSI > >>>> commands via the admin virtqueue. And we had similar requirement for > >>>> virtio-MMIO[2] and managed device or SF [3], so I would rather to > >>>> introduce IMS (need a better name though) as a basic facility instead > >>>> of tie it to any specific transport. > >>>> > >>> IMS of [1] is a interrupt configuration by the virtio driver for the > >>> device is it > >> driving, which needs a queue. > >>> So regardless of the device type as PCI PF/VF/SF/ADI, there is desire to > >>> have a > >> generic admin queue not attached to device type. > >>> And AQ in this proposal exactly serves this purpose. > >>> > >>> Device configuring its own IMS vector vs PCI PF configuring VF's MSI-X max > >> vector count are two different functionality. > >>> Both of these commands can ride on a generic queue. > >>> However the queue is not same, because PF owns its own admin queue > >>> (for vf msix config), VF or SF operates its own admin queue (for IMS > >>> config). > >> > >> So I think in the next version we need to clarify: > >> > >> 1) is there a single admin virtqueue shared by all the VFs and PF > >> > >> or > >> > >> 2) per VF/PF admin virtqueue, and how does the driver know how to find the > >> corresponding admin virtqueue > >> > > Admin queue is not per VF. > > Lets take concrete examples. > > 1. So for example, PCI PF can have one AQ. > > This AQ carries command to query/config MSI-X vector of VFs. > > > > 2. In second example, PCI PF is creating/destroying SFs. This is again done > > by using the AQ of the PCI PF. > > > > 3. A PCI VF has its own AQ to configure some of its own generic attribute, > > don't know which is that today. > > May be something that is extremely hard to do over features bit. > > Currently proposed v2 doesn't restrict admin queue to be within PCI PF or > > VF or that matter not limited to other transports. > > > >>> So a good example is, > >>> 1. PCI PF configures 8 MSI-X or 16 IMS vectors for the VF using PF_AQ in > >>> HV. > >>> 2. PCI VF when using IMS configures, IMS data, vector, mask etc using > >>> VF_AQ > >> in GVM. > >>> Both the functions will have AQ feature bit set. > >> > >> Where did the VF_AQ sit? I guess it belongs to the VF. But if this is > >> true, don't we need some kind of address isolation like PASID? > >> > > Above one for IMS is not a good example. I replied the reasoning last week > > for it. > > > >>> Fair enough, so we have more users of admin queue than just MSI-X config. > >> > >> Well, what I really meant is that we actually have more users of IMS. > >> That's is exactly what virito-mmio wants. In this case introducing admin > >> queue looks too heavyweight for that. > >> > > IMS config cannot be done over AQ as described in previous email in this > > thread. > > > >>>>> 2. AQ to follows IN_ORDER and INDIRECT_DESC negotiation like rest of > >>>>> the queues 3. Update commit log to describe why config space is not > >>>>> chosen (scale, on-die registers, uniform way to handle all aq cmds) > >>>> I fail to understand the scale/registeres issues. With the one of my > >>>> previous > >>>> proposal (device selector), technically we don't even need any config > >>>> space > >> or > >>>> BAR for VF or SF by multiplexing the registers for PF. > >>>> > >>> Scale issue is: when you want to create, query, manipulate hundreds of > >> objects, having shared MMIO register or configuration register, will be too > >> slow. > >> > >> > >> Ok, this need to be clarified in the commit log. And we need make sure > >> it's not an issue that is only happen for some specific vendor. > > It is present in the v2 commit log cover letter. > > Please let me know if you think it should be in the actual patch commit log. > > > > > >>> And additionally such register set doesn't scale to allow sharing large > >> number of bytes as DMA cannot be done. > >> > >> > >> That's true. > >> > >> > >>> From physical device perspective, it doesn’t scale because device needs > >>> to > >> have those resources ready to answer on MMIO reads and for hundreds to > >> thousand of devices it just cannot do it. > >>> This is one of the reason for birth of IMS. > >> > >> IMS allows the table to be stored in the memory and cached by the device > >> to have the best scalability. But I had other questions: > >> > >> 1) if we have a single admin virtqueue, there will still be contention > >> in the driver side > >> > > AQ inherently allows out of order commands execution. > > It shouldn't face contention. For example 1K depth AQ should be serving > > hundreds of descriptors commands in parallel for SF creation, VF MSI-X > > config and more. > > > > Which area/commands etc you think can lead to the contention? > > > >> 2) if we have per vf admin virtqueue, it still doesn't scale since it > >> occupies more hardware resources > >> > > That is too heavy, and doesn’t scale. Proposal is to not have per vf admin > > queue. > > Proposal is to have one admin queue in a virtio device. > > Right ? where did we mention something that can imply otherwise ?
Well, I don't know but probably this part, " PCI VF when using IMS configures, IMS data, vector, mask etc using VF_AQ ..." > > > > > >>>> I do see one advantage is that the admin virtqueue is transport > >> independent > >>>> (or it could be used as a transport). > >>>> > >>> I am yet to read the transport part from [1]. > >> > >> Yes, the main goal is to be compatible with SIOV. > >> > > Admin queue is a command interface transport where higher layer services > > can be buit. > > This includes SR-IOV config, SIOV config. > > And v2 enables SIOV commands implementation whenever they are ready. > > > >>>>> 4. Improve documentation around msix config to link to sriov section of > >> virtio > >>>> spec > >>>>> 5. Describe error that if VF is bound to the device, admin commands > >>>> targeting VF can fail, describe this error code > >>>>> Did I miss anything? > >>>>> > >>>>> Yet to receive your feedback on group, if/why is it needed and, why/if > >>>>> it > >> must > >>>> be in this proposal, what pieces prevents it do as follow-on. > >>>>> Cornelia, Jason, > >>>>> Can you please review current proposal as well before we revise v2? > >>>> If I understand correctly, most of the features (except for the admin > >>>> virtqueue in_order stuffs) are not specific to the admin virtqueue. As > >>>> discussed in the previous versions, I still think it's better: > >>>> > >>>> 1) adding sections in the basic device facility or data structure for > >>>> provisioning and MSI > >>>> 2) introduce admin virtqueue on top as an device interface for those > >>>> features > >>>> > >>> I didn't follow your suggestion. Can you please explain? > >>> Specifically "data structure for provisioning and MSI".. > >> > >> I meant: > >> > >> There's a chapter "Basic Facilities of a Virtio Device", we can > >> introduce the concepts there like: > >> > >> 1) Managed device and Management device (terminology proposed by > >> Michael), and can use PF and VF as a example > >> > >> 2) Managed device provisioning (the data structure to specify the > >> attributes of a managed device (VF)) > >> > >> 3) MSI > >> > > Above is good idea. Will revisit v2, if it is not arranged this way. > > Let me make sure I understand, you would like to see a new chapter under > "Basic Facilities of a Virtio Device" that is > > called "Device management" and this chapter will explain in few words > the concept Yes. > and it will point to another chapter under "Basic Facilities > of a Virtio Device" > > that was introduced here "Admin Virtqueues" ? So far as I see from the proposal, it needs belong to PCI transport part or a new transport. > > So you do agree that managing a managed (create/destroy/setup/etc...) > will be done using the AQ of the managing device ? I agree. Thanks > > > > >> And then we can introduced admin virtqueue in either > >> > >> 1) transport part > >> > >> or > >> > >> 2) PCI transport > >> > > It is not specific to PCI transport, and currently it is not a transport > > either. > > So admin queue will keep as general entity for admin work. > > > >> In the admin virtqueue, there will be commands to provision and > >> configure MSI. > >> > > Please review v2 if it is not arranged this way. > > > >>>> The leaves the chance for future extensions to allow those features to > >>>> be used by transport specific interface which will benefit for > >>>> > >>> AQ allows communication (command, response) between driver and device > >> in transport independent way. > >>> Sometimes it query/set transport specific fields like MSI-X vectors of VF. > >>> Sometimes device configure its on IMS interrupt. > >>> Something else in future. > >>> So it is really a generic request-response queue. > >> > >> I agree, but I think we can't mandate new features to a specific transport. > >> > > Certainly. Admin queue is transport independent. > > PCI MSI-X configuration is PCI transport specific command, so structures > > are defined it accordingly. > > It is similar to struct virtio_pci_cap, struct virtio_pci_common_cfg etc. > > > > Any other transport will have transport specific interrupt configuration. > > So it will be defined accordingly whenever that occurs. > > For example, IMS for VF or IMS for SF. > --------------------------------------------------------------------- To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
