[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
On Wed, May 24, 2023 at 08:01:18PM +, Parav Pandit wrote: > > > From: Michael S. Tsirkin > > Sent: Wednesday, May 24, 2023 3:58 PM > > > > > Hope this helps. > > > Definitely a good suggestion. > > > I will send PR for virtio-docs to maintain our TODO list. > > > > Hmm. This is just a dumping ground then. Fine by me, but what I had in > > mind is > > a common list of known problems to address by TC as a whole. > > If you want that we want patches on list and discussion, maybe even voting. > > > It is not a dumping ground, it is the list of things to track and document > without searching emails. virtio-doc is a dumping ground though :) > It is a patch on the list for the doc in virtio-docs area that if needed can > be voted. Let's start with nvidia-todo.md in virtio-doc indeed. People can at least read it, good starting point. Next I would like us to proceed to a todo list under virtio-spec, so that TC at least knows what are current problems before us. -- MST - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] RE: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
> From: Michael S. Tsirkin > Sent: Wednesday, May 24, 2023 3:58 PM > > > Hope this helps. > > Definitely a good suggestion. > > I will send PR for virtio-docs to maintain our TODO list. > > Hmm. This is just a dumping ground then. Fine by me, but what I had in mind > is > a common list of known problems to address by TC as a whole. > If you want that we want patches on list and discussion, maybe even voting. > It is not a dumping ground, it is the list of things to track and document without searching emails. It is a patch on the list for the doc in virtio-docs area that if needed can be voted. - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
On Wed, May 24, 2023 at 06:57:30PM +, Parav Pandit wrote: > > > From: Michael S. Tsirkin > > Sent: Wednesday, May 24, 2023 1:57 AM > > > > I am frankly lost at this point, this thread is too big. > > > > So where do you want to go for this project? I would like something > > specific, > > email threads are growing too big. > > I think the reason is that there is actually a host of small related > > subprojects. So > > when you propose just this small feature, everyone jumps in with their own > > pet > > project proposing addressing that too. > > > > For example: > > - SIOV guys were adding transport over VQ. It would seem that > > can include support for legacy. > Yes. SIOV can support legacy and modern emulation. > We need to have the SIOV devices without this emulation support so that they > natively good. > > > - For migration we need ability to report member events to owner. > Which events you have in mind? memory faults/dirtying for example. > The AQ is design to carry the migration of the VFs and would be controlled > via the AQ so it knows whats going on. > > > It would seem that can be used to support INT#x. > > - Ability to stick capabilities in extended config space > > is a good idea, for a variety of reasons. > > > > and so on. Understandably frustrating, and it is easy to go overboard with > > generality. > > > > If you feel your path for progress is clear, that's good. > The path seems clear to me for supporting legacy registers access for the VFs > and future SIOV. > Let's conclude it in other thread we are discussing of patch 1/2. > > > if not, here are ideas for getting this unstuck: > > - start a TODO.md file with a list of things we need to address, > > and for each one a list of options > > - everyone will add to this file then we get a higher level > > picture and can discuss "this will be addressed by A, that by B" > This is certainly present in my personal todo list to discuss, I am happy to > maintain in the public virtio doc at > https://github.com/oasis-tcs/virtio-docs/ > > > - have a chat at the upstream developers meeting (May 31) > > so far it's about vdpa blk > > - anyone coming to the kvm forum to have a chat there? > > > > Hope this helps. > Definitely a good suggestion. > I will send PR for virtio-docs to maintain our TODO list. Hmm. This is just a dumping ground then. Fine by me, but what I had in mind is a common list of known problems to address by TC as a whole. If you want that we want patches on list and discussion, maybe even voting. > Since some of the discussions are ongoing basis if we can meet monthly > on more practically on need basis, it will help to make progress > faster. That meeting is byweekly actually I think. -- MST - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] RE: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
> From: Michael S. Tsirkin > Sent: Wednesday, May 24, 2023 1:57 AM > I am frankly lost at this point, this thread is too big. > > So where do you want to go for this project? I would like something specific, > email threads are growing too big. > I think the reason is that there is actually a host of small related > subprojects. So > when you propose just this small feature, everyone jumps in with their own pet > project proposing addressing that too. > > For example: > - SIOV guys were adding transport over VQ. It would seem that > can include support for legacy. Yes. SIOV can support legacy and modern emulation. We need to have the SIOV devices without this emulation support so that they natively good. > - For migration we need ability to report member events to owner. Which events you have in mind? The AQ is design to carry the migration of the VFs and would be controlled via the AQ so it knows whats going on. > It would seem that can be used to support INT#x. > - Ability to stick capabilities in extended config space > is a good idea, for a variety of reasons. > > and so on. Understandably frustrating, and it is easy to go overboard with > generality. > > If you feel your path for progress is clear, that's good. The path seems clear to me for supporting legacy registers access for the VFs and future SIOV. Let's conclude it in other thread we are discussing of patch 1/2. > if not, here are ideas for getting this unstuck: > - start a TODO.md file with a list of things we need to address, > and for each one a list of options > - everyone will add to this file then we get a higher level > picture and can discuss "this will be addressed by A, that by B" This is certainly present in my personal todo list to discuss, I am happy to maintain in the public virtio doc at https://github.com/oasis-tcs/virtio-docs/ > - have a chat at the upstream developers meeting (May 31) > so far it's about vdpa blk > - anyone coming to the kvm forum to have a chat there? > > Hope this helps. Definitely a good suggestion. I will send PR for virtio-docs to maintain our TODO list. Since some of the discussions are ongoing basis if we can meet monthly on more practically on need basis, it will help to make progress faster. - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
On Tue, May 23, 2023 at 09:32:41PM +, Parav Pandit wrote: > > > > From: Michael S. Tsirkin > > Sent: Tuesday, May 23, 2023 2:17 PM > > > > On Mon, May 15, 2023 at 02:30:55PM +, Parav Pandit wrote: > > > > > > > From: Michael S. Tsirkin > > > > Sent: Monday, May 15, 2023 6:09 AM > > > > To be more specific, device config often has the same layout under > > > > legacy and modern. Thus if getting an offset within device config, > > > > same decoding logic should be reusable between legacy and modern. > > > Modern device registers are directly accessible from the guest driver > > > without > > mediation for VF,SF,SIOV. > > > Hence, there is no need to have commands for modern register access over > > some VQ. > > > > Yes, there's need, and some of it you were pushing yourself. > Yes, there is need without mediation of AQ. > It was discussed in depth cfgq idea with trade off. > > > AQ will allow reduced memory with SRIOV, and support for SIOV. > Already discussed, v0 and v1, mediation via AQ is not the way for SRIOV. > SIOV starting with single goal of only backward compatilbity hence, AQ is > also doesn't look future forward. > > And your summary email, we parked it aside for now. I am frankly lost at this point, this thread is too big. So where do you want to go for this project? I would like something specific, email threads are growing too big. I think the reason is that there is actually a host of small related subprojects. So when you propose just this small feature, everyone jumps in with their own pet project proposing addressing that too. For example: - SIOV guys were adding transport over VQ. It would seem that can include support for legacy. - For migration we need ability to report member events to owner. It would seem that can be used to support INT#x. - Ability to stick capabilities in extended config space is a good idea, for a variety of reasons. and so on. Understandably frustrating, and it is easy to go overboard with generality. If you feel your path for progress is clear, that's good. if not, here are ideas for getting this unstuck: - start a TODO.md file with a list of things we need to address, and for each one a list of options - everyone will add to this file then we get a higher level picture and can discuss "this will be addressed by A, that by B" - have a chat at the upstream developers meeting (May 31) so far it's about vdpa blk - anyone coming to the kvm forum to have a chat there? Hope this helps. -- MST - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] RE: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
> From: Michael S. Tsirkin > Sent: Tuesday, May 23, 2023 2:17 PM > > On Mon, May 15, 2023 at 02:30:55PM +, Parav Pandit wrote: > > > > > From: Michael S. Tsirkin > > > Sent: Monday, May 15, 2023 6:09 AM > > > To be more specific, device config often has the same layout under > > > legacy and modern. Thus if getting an offset within device config, > > > same decoding logic should be reusable between legacy and modern. > > Modern device registers are directly accessible from the guest driver > > without > mediation for VF,SF,SIOV. > > Hence, there is no need to have commands for modern register access over > some VQ. > > Yes, there's need, and some of it you were pushing yourself. Yes, there is need without mediation of AQ. It was discussed in depth cfgq idea with trade off. > AQ will allow reduced memory with SRIOV, and support for SIOV. Already discussed, v0 and v1, mediation via AQ is not the way for SRIOV. SIOV starting with single goal of only backward compatilbity hence, AQ is also doesn't look future forward. And your summary email, we parked it aside for now. - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
On Mon, May 15, 2023 at 02:30:55PM +, Parav Pandit wrote: > > > From: Michael S. Tsirkin > > Sent: Monday, May 15, 2023 6:09 AM > > To be more specific, device config often has the same layout under legacy > > and > > modern. Thus if getting an offset within device config, same decoding logic > > should be reusable between legacy and modern. > Modern device registers are directly accessible from the guest driver without > mediation for VF,SF,SIOV. > Hence, there is no need to have commands for modern register access over some > VQ. Yes, there's need, and some of it you were pushing yourself. AQ will allow reduced memory with SRIOV, and support for SIOV. > Will reply to rest of the Jason's comments in some time. - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] RE: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
> From: Michael S. Tsirkin > Sent: Tuesday, May 23, 2023 2:39 AM > but I would like us to better > document how and when does device switch to and from legacy mode, and for > the switch to be somehow reusable for solutions that are closer to vdpa. > To my knowledge vdpa do not have such switch, nor such switch in the spec, nor in the vdpa or PCI device. > Does enabling legacy commands cause the switch? Or should we add a > LEGACY_PCI command to enable/disable? I kind of like the second option ... Some devices may find it helpful to issue this command on the AQ, to say enable your legacy emulation and allocate needed resources. I guess before getting distracted on this side topic, lets close on the AQ or two register MMIO approach in other email thread.
[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
On Sat, May 06, 2023 at 03:01:33AM +0300, Parav Pandit wrote: > This short series introduces legacy registers access commands for the owner > group member PCI PF to access the legacy registers of the member VFs. > > If in future any SIOV devices to support legacy registers, they > can be easily supported using same commands by using the group > member identifiers of the future SIOV devices. > > More details as overview, motivation, use case are further described > below. > > Patch summary: > -- > patch-1 adds administrative virtuqueue commands > patch-2 adds its conformance section > > This short series is on top of latest work [1] from Michael. > It uses the newly introduced administrative virtqueue facility with 3 new > commands which uses the existing virtio_admin_cmd. > > [1] https://lists.oasis-open.org/archives/virtio-comment/202305/msg00112.html > > Usecase: > > 1. A hypervisor/system needs to provide transitional >virtio devices to the guest VM at scale of thousands, >typically, one to eight devices per VM. > > 2. A hypervisor/system needs to provide such devices using a >vendor agnostic driver in the hypervisor system. > > 3. A hypervisor system prefers to have single stack regardless of >virtio device type (net/blk) and be future compatible with a >single vfio stack using SR-IOV or other scalable device >virtualization technology to map PCI devices to the guest VM. >(as transitional or otherwise) > > Motivation/Background: > -- > The existing virtio transitional PCI device is missing support for > PCI SR-IOV based devices. Currently it does not work beyond > PCI PF, or as software emulated device in reality. Currently it > has below cited system level limitations: > > [a] PCIe spec citation: > VFs do not support I/O Space and thus VF BARs shall not indicate I/O Space. > > [b] cpu arch citiation: > Intel 64 and IA-32 Architectures Software Developer’s Manual: > The processor’s I/O address space is separate and distinct from > the physical-memory address space. The I/O address space consists > of 64K individually addressable 8-bit I/O ports, numbered 0 through H. > > [c] PCIe spec citation: > If a bridge implements an I/O address range,...I/O address range will be > aligned to a 4 KB boundary. > > Overview: > - > Above usecase requirements can be solved by PCI PF group owner accessing > its group member PCI VFs legacy registers using an admin virtqueue of > the group owner PCI PF. > > Two new admin virtqueue commands are added which read/write PCI VF > registers. > > The third command suggested by Jason queries the VF device's driver > notification region. > > Software usage example: > --- > One way to use and map to the guest VM is by using vfio driver > framework in Linux kernel. > > +--+ > |pci_dev_id = 0x100X | > +---|pci_rev_id = 0x0 |-+ > |vfio device|BAR0 = I/O region | | > | |Other attributes | | > | +--+ | > || > + +--+ +-+ | > | |I/O BAR to AQ | | Other vfio | | > | |rd/wr mapper | | functionalities | | > | +--+ +-+ | > || > +--+-+---+ >| | > +++ +++ > | +-+ | | PCI VF device A | > | | AQ |-+>+-+ | > | +-+ | | | | legacy regs | | > | PCI PF device | | | +-+ | > +-+ | +-+ > | > | +++ > | | PCI VF device N | > +>+-+ | > | | legacy regs | | > | +-+ | > +-+ > > 2. Virtio pci driver to bind to the listed device id and >use it as native device in the host. > > 3. Use it in a light weight hypervisor to run bare-metal OS. > > Please review. > > Fixes: https://github.com/oasis-tcs/virtio-spec/issues/167 > Signed-off-by: Parav Pandit Just thought of an issue: one use-case this can't address is SVQ. Specifically, legacy has: /* A 32-bit r/w PFN for the currently selected queue */ #define VIRTIO_PCI_QUEUE_PFN8 this can only address up to 40 bits which might be ok for legacy guests, but if instead we want the queue to live in host memory (this is what SVQ does) then this does not work as host is likely to have more than 2^40 byte memory. Further, one of advantages of modern is that used ring can live in host memory with aliasing when available is passed through directly
[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
On Tue, May 16, 2023 at 09:41:07PM +, Parav Pandit wrote: > > > From: Michael S. Tsirkin > > Sent: Tuesday, May 16, 2023 5:09 PM > > > > Possibly one can set. I don’t know if any actual device really supported > > endianness. > > > No users have asked for it, even asking explicitly to those > > > non_little_endian > > users. > > > > For sure, ARM BE does exist and Red Hat supports it because yes, it was > > requested. > Interesting. > Any vdpa device support this? How is the guest endianness conveyed to this > vdpa device? I don't think, so far. > And which barrier this ARM uses before issuing notification on legacy? Would be challenging with hardware, but can work for software virtio for nested virt I guess? > > You can not just go by what marketing tells you today we either try > > to build future proof interfaces or we don't bother at all - by the time > > these > > things are in the field everything shifted. > > > Huh. > One can add the BE support incrementally when the device plans to support it. > > 1.x is already future proof from endianness, so no need to bring endianness > for it. > Only legacy can work in the single platform with hw in wider deployment as > David acked. > So over engineering is avoided by not introducing BE support. > > > > So may be when there is/if a real user, it can be done in future. > > > > The concern is if you assume LE is default, no way to say "I do not support > > LE". > > Something like an explicit "SET LE" and "SET BE" commands will automatically > > allow discovering which endian-ness is supported. > > > This is good suggestion. > The LE for legacy is assumed because barrier etc cannot work. > So, avoiding theoretical spec commands here that may not find any user. maybe at least outlike what happens if some device needs to add BE and drop LE? > > > > > > > > > > > For any case, having a simple and deterministic device design is > > > > > > always better assuming we've agreed that mediation is a must for > > > > > > legacy drivers. Using dedicated commands can make sure the > > > > > > implementation won't need to go with corner cases of legacy. > > > > > > > > > > > Corner case handling just moved from the device to hypervisor. > > > > > > > > That's not safe, the device should be ready for malicious inputs. > > > > > > > With current offset, register, device will be just fine as most > > > implementations > > have to program control path etc on these registers write/read etc. > > > So, device will be able to handle them with plain 2 commands. > > > > Except legacy is broken broken broken. It just does not work for physical > > devices in a crazy number of ways. How about the weird 48 bit limitation on > > PAs? Because no one ever will need any more. > > > Legacy is broken and works only in small but widely used platform. > Hence, it attempts to support it. > > 48-bit PA limitation in virtio? For queue base, yes. One of the crazy things in virtio ... > > I have 0 sympathy to this idea of reviving all these bugs then circling > > back and > > coming up with weird work arounds. Please, just build a sane transport and > > have software deal with bugs as best it can. > > > When I proposed the register interface that adheres to the alignment > boundary, you suggested to follow legacy semantics. > Now when legacy semantics is followed, you go opposite direction of let sw > decide it. Sorry if I was unclear. I was making a distinction between device specific with arbitrary alignment, where we should just follow legacy semantics, and common config where I think sw should decide it. > Can we agree that alignment, width and offset to be decided by the sw? For device type specific we can't since spec said we can't :( For common sure. -- MST - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] RE: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
> From: Michael S. Tsirkin > Sent: Tuesday, May 16, 2023 5:09 PM > > Possibly one can set. I don’t know if any actual device really supported > endianness. > > No users have asked for it, even asking explicitly to those > > non_little_endian > users. > > For sure, ARM BE does exist and Red Hat supports it because yes, it was > requested. Interesting. Any vdpa device support this? How is the guest endianness conveyed to this vdpa device? And which barrier this ARM uses before issuing notification on legacy? > You can not just go by what marketing tells you today we either try > to build future proof interfaces or we don't bother at all - by the time these > things are in the field everything shifted. > Huh. One can add the BE support incrementally when the device plans to support it. 1.x is already future proof from endianness, so no need to bring endianness for it. Only legacy can work in the single platform with hw in wider deployment as David acked. So over engineering is avoided by not introducing BE support. > > So may be when there is/if a real user, it can be done in future. > > The concern is if you assume LE is default, no way to say "I do not support > LE". > Something like an explicit "SET LE" and "SET BE" commands will automatically > allow discovering which endian-ness is supported. > This is good suggestion. The LE for legacy is assumed because barrier etc cannot work. So, avoiding theoretical spec commands here that may not find any user. > > > > > > > > > For any case, having a simple and deterministic device design is > > > > > always better assuming we've agreed that mediation is a must for > > > > > legacy drivers. Using dedicated commands can make sure the > > > > > implementation won't need to go with corner cases of legacy. > > > > > > > > > Corner case handling just moved from the device to hypervisor. > > > > > > That's not safe, the device should be ready for malicious inputs. > > > > > With current offset, register, device will be just fine as most > > implementations > have to program control path etc on these registers write/read etc. > > So, device will be able to handle them with plain 2 commands. > > Except legacy is broken broken broken. It just does not work for physical > devices in a crazy number of ways. How about the weird 48 bit limitation on > PAs? Because no one ever will need any more. > Legacy is broken and works only in small but widely used platform. Hence, it attempts to support it. 48-bit PA limitation in virtio? > I have 0 sympathy to this idea of reviving all these bugs then circling back > and > coming up with weird work arounds. Please, just build a sane transport and > have software deal with bugs as best it can. > When I proposed the register interface that adheres to the alignment boundary, you suggested to follow legacy semantics. Now when legacy semantics is followed, you go opposite direction of let sw decide it. Can we agree that alignment, width and offset to be decided by the sw?
[virtio-dev] RE: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
> From: Michael S. Tsirkin > Sent: Tuesday, May 16, 2023 5:23 PM > > On Tue, May 16, 2023 at 09:19:02PM +, Parav Pandit wrote: > > > > > From: Michael S. Tsirkin > > > Sent: Tuesday, May 16, 2023 4:58 PM > > > > > > > Let me give you an example. I feel that device specific config > > > > > should allow arbitrary length accesses so that e.g. > > > > > mac can be read and written atomically. > > > > > > > > > > On the other hand, this is not the case for common config. > > > > > > > > > Why is this not the case with common config? > > > > Spec snippet: "When using the legacy interface the driver MAY > > > > access the device-specific configuration region using any width > > > > accesses" > > > > > > I mean, what you said above exactly? > > > device-specific configuration excludes common config. > > > > > > > > So, what prevents the device to fail AQ command with the error code when > alignment/width/size is not right? > > Nothing but you then need to specify what is and what isn't right. > It follows the existing legacy interface section, you asked to not duplicate it. > > > > > > I feel we do not need to allow e.g. access to both common config > > > > > and device specific one in a single operation, that is just messy. > > > > It is just an offset and value. > > > > What part bothers you? > > > > > > E.g. that it can cross the coundary between common and device config. > > > > > This is the legacy transport. > > > > > Look we didn't build modern because we wanted to make work, we did > > > because legacy is broken. So either let legacy die already or let's > > > build a sane interface to emulate it please. > > Almost everyone building virtio device would prefer for legacy to die. > > I am still trying to understand that why bisecting register definition of > > legacy > makes it sane. > > Because legacy common config is very evil, do not copy it. > Legacy device config is only bad in minor tolerable ways. > It is not used outside of the legacy interface. The point is guest driver will follow certain config write/read pattern. Hypervisor has no way to fail it. So what do you expect hypervisor to do on undesired sequence? Do you propose to correct it and send it to device? And if so, the intention is building sane device for legacy interface? If that is the intention, sound fine. - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
On Tue, May 16, 2023 at 09:19:02PM +, Parav Pandit wrote: > > > From: Michael S. Tsirkin > > Sent: Tuesday, May 16, 2023 4:58 PM > > > > > Let me give you an example. I feel that device specific config > > > > should allow arbitrary length accesses so that e.g. > > > > mac can be read and written atomically. > > > > > > > > On the other hand, this is not the case for common config. > > > > > > > Why is this not the case with common config? > > > Spec snippet: "When using the legacy interface the driver MAY access > > > the device-specific configuration region using any width accesses" > > > > I mean, what you said above exactly? > > device-specific configuration excludes common config. > > > > > So, what prevents the device to fail AQ command with the error code when > alignment/width/size is not right? Nothing but you then need to specify what is and what isn't right. > > > > I feel we do not need to allow e.g. access to both common config and > > > > device specific one in a single operation, that is just messy. > > > It is just an offset and value. > > > What part bothers you? > > > > E.g. that it can cross the coundary between common and device config. > > > This is the legacy transport. > > > Look we didn't build modern because we wanted to make work, we did because > > legacy is broken. So either let legacy die already or let's build a sane > > interface > > to emulate it please. > Almost everyone building virtio device would prefer for legacy to die. > I am still trying to understand that why bisecting register definition of > legacy makes it sane. Because legacy common config is very evil, do not copy it. Legacy device config is only bad in minor tolerable ways. > Device behaves based on the register offset, is fine for the legacy > transport, guest driver will make the register accesses anyway it prefers. - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] RE: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
> From: Michael S. Tsirkin > Sent: Tuesday, May 16, 2023 4:58 PM > > > Let me give you an example. I feel that device specific config > > > should allow arbitrary length accesses so that e.g. > > > mac can be read and written atomically. > > > > > > On the other hand, this is not the case for common config. > > > > > Why is this not the case with common config? > > Spec snippet: "When using the legacy interface the driver MAY access > > the device-specific configuration region using any width accesses" > > I mean, what you said above exactly? > device-specific configuration excludes common config. > > So, what prevents the device to fail AQ command with the error code when alignment/width/size is not right? > > > I feel we do not need to allow e.g. access to both common config and > > > device specific one in a single operation, that is just messy. > > It is just an offset and value. > > What part bothers you? > > E.g. that it can cross the coundary between common and device config. > This is the legacy transport. > Look we didn't build modern because we wanted to make work, we did because > legacy is broken. So either let legacy die already or let's build a sane > interface > to emulate it please. Almost everyone building virtio device would prefer for legacy to die. I am still trying to understand that why bisecting register definition of legacy makes it sane. Device behaves based on the register offset, is fine for the legacy transport, guest driver will make the register accesses anyway it prefers.
[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
On Tue, May 16, 2023 at 07:29:44PM +, Parav Pandit wrote: > > > From: Jason Wang > > Sent: Tuesday, May 16, 2023 12:08 AM > > > > For guest and by guest are two different things. > > > I am proposing that a cfgq/cmdq should be used directly by the guest > > > driver as > > base line like any other VQ. > > > > I'm fine with this, but if I understand correctly, but I don't see any > > proposal like > > this that is posted to the list? > > > Because it is orthogonal to the work here, it's hard to make progress by > mixing everything. > > > > And if there is a need for mediation, it should be possible. > > > > > > > I think we've discussed many times that legacy is tricky for hardware. > > > And we want to proceed for the described use case and requirements. > > > > > > > And your proposal proves this further by introducing modern alike > > > > notification areas. > > > Not really. A modern device is offering the notification area like legacy > > > to the > > legacy interface. > > > So this is just fine. > > > > I don't say it's not fine, it's pretty fine and transport virtqueue have > > similar > > commands. And this also answer the question that you think tunnel PCI over > > adminq is not scalable. We can keep this command in that case as well, or > > invent new capabilities. > > > Ok. so virtio_admin_q_cmd_lq_notify_result is fine. > > > > > > > > We probably need to deal with others like endianess. > > > > > > > No point in discussing this again. One size does not fit all. > > > It solves large part of the use cases so it is valuable and will be > > > supported. > > > > This is doable, we need to leave some space for future development. Or is it > > just complicated to have an endian command? > > > Possibly one can set. I don’t know if any actual device really supported > endianness. > No users have asked for it, even asking explicitly to those non_little_endian > users. For sure, ARM BE does exist and Red Hat supports it because yes, it was requested. You can not just go by what marketing tells you today we either try to build future proof interfaces or we don't bother at all - by the time these things are in the field everything shifted. > So may be when there is/if a real user, it can be done in future. The concern is if you assume LE is default, no way to say "I do not support LE". Something like an explicit "SET LE" and "SET BE" commands will automatically allow discovering which endian-ness is supported. > > > > > > > For any case, having a simple and deterministic device design is > > > > always better assuming we've agreed that mediation is a must for > > > > legacy drivers. Using dedicated commands can make sure the > > > > implementation won't need to go with corner cases of legacy. > > > > > > > Corner case handling just moved from the device to hypervisor. > > > > That's not safe, the device should be ready for malicious inputs. > > > With current offset, register, device will be just fine as most > implementations have to program control path etc on these registers > write/read etc. > So, device will be able to handle them with plain 2 commands. Except legacy is broken broken broken. It just does not work for physical devices in a crazy number of ways. How about the weird 48 bit limitation on PAs? Because no one ever will need any more. I have 0 sympathy to this idea of reviving all these bugs then circling back and coming up with weird work arounds. Please, just build a sane transport and have software deal with bugs as best it can. > > > For SIOV there is no backward compatibility requirement. > > > > It has several ways to support current virtio-pci drivers in the guest, and > > such > > compatibility is almost a must for the public cloud. > > We can't enforce the user to reinstall/recompile their kernels/application > > in > > order to work for SIOV. > Sure, an SIOV device emulating PCI device for guest is one requirement. > > And SIOV device working natively spawned from the parent PF and used without > guest is another requirement. > > And second can be done bit elegantly in self-contained way without depending > on the parent PF. > And requires more work unrelated to this one. > > > I think we are discussing what it can be, so see the above reply for the > > notification areas, we can stick virtio_admin_cmd_lq_notify_query_result > > command for PCI over adminq, or invent new capabilities to make it fully > > self > > contained. > > > As we both understand well that a queue is not going to transport a doorbell > and actual IMS interrupts, we will mostly have commands to service some needs > for backward compat. > It is a separate work when there are no PCI VF devices. > > > > > > > > > > > > > > > > > > You only want to exchange currently defined config registers, > > > > > interrupts and > > > > notification configuration using AQ. > > > > > > > > > > Transport VQ is NOT transporting actual data, it is not > > > > > transporting > > >
[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
On Tue, May 16, 2023 at 07:11:36PM +, Parav Pandit wrote: > > > From: Michael S. Tsirkin > > Sent: Tuesday, May 16, 2023 2:22 AM > > > > On Mon, May 15, 2023 at 03:59:41PM +, Parav Pandit wrote: > > > > > > > From: Jason Wang > > > > Sent: Monday, May 15, 2023 3:31 AM > > > > > > > > What do we gain from bisecting it? > > > > > > > > > > > > 1) No need to deal with the offset in the hardware > > > > > > > In context of legacy is doesn’t matter much given that its over AQ. > > > > Let me give you an example. I feel that device specific config should allow > > arbitrary length accesses so that e.g. > > mac can be read and written atomically. > > > > On the other hand, this is not the case for common config. > > > Why is this not the case with common config? > Spec snippet: "When using the legacy interface the driver MAY access the > device-specific configuration region using any > width accesses" I mean, what you said above exactly? device-specific configuration excludes common config. > > I feel we do not need to allow e.g. access to both common config and device > > specific one in a single operation, that is just messy. > It is just an offset and value. > What part bothers you? E.g. that it can cross the coundary between common and device config. > > Now sure, we can add text > > with an explanation, but if instead there's a flag saying "this is common > > cfg" > > or "this is device cfg" then it fall out automatically. > > > > Is this a deal breaker? No, but note how neither I nor Jason did not think > > about > > this ahead of the time and the correct usage just falls out of the interface > > automatically. This is a mark of a good interface design. Lots of corner > > cases > > that one has to be careful around is not. > It just moves corner case one layer above in multiple software hypervisors > and hypervisor is not failing it either on cfg read/writes as expected by the > guest. > So bisecting is not adding anything extra. Those failure checks are going to > be ignored in hypervisor anyway. Look we didn't build modern because we wanted to make work, we did because legacy is broken. So either let legacy die already or let's build a sane interface to emulate it please. -- MST - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] RE: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
> From: Jason Wang > Sent: Tuesday, May 16, 2023 12:08 AM > > For guest and by guest are two different things. > > I am proposing that a cfgq/cmdq should be used directly by the guest driver > > as > base line like any other VQ. > > I'm fine with this, but if I understand correctly, but I don't see any > proposal like > this that is posted to the list? > Because it is orthogonal to the work here, it's hard to make progress by mixing everything. > > And if there is a need for mediation, it should be possible. > > > > > I think we've discussed many times that legacy is tricky for hardware. > > And we want to proceed for the described use case and requirements. > > > > > And your proposal proves this further by introducing modern alike > > > notification areas. > > Not really. A modern device is offering the notification area like legacy > > to the > legacy interface. > > So this is just fine. > > I don't say it's not fine, it's pretty fine and transport virtqueue have > similar > commands. And this also answer the question that you think tunnel PCI over > adminq is not scalable. We can keep this command in that case as well, or > invent new capabilities. > Ok. so virtio_admin_q_cmd_lq_notify_result is fine. > > > > > We probably need to deal with others like endianess. > > > > > No point in discussing this again. One size does not fit all. > > It solves large part of the use cases so it is valuable and will be > > supported. > > This is doable, we need to leave some space for future development. Or is it > just complicated to have an endian command? > Possibly one can set. I don’t know if any actual device really supported endianness. No users have asked for it, even asking explicitly to those non_little_endian users. So may be when there is/if a real user, it can be done in future. > > > > > For any case, having a simple and deterministic device design is > > > always better assuming we've agreed that mediation is a must for > > > legacy drivers. Using dedicated commands can make sure the > > > implementation won't need to go with corner cases of legacy. > > > > > Corner case handling just moved from the device to hypervisor. > > That's not safe, the device should be ready for malicious inputs. > With current offset, register, device will be just fine as most implementations have to program control path etc on these registers write/read etc. So, device will be able to handle them with plain 2 commands. > > For SIOV there is no backward compatibility requirement. > > It has several ways to support current virtio-pci drivers in the guest, and > such > compatibility is almost a must for the public cloud. > We can't enforce the user to reinstall/recompile their kernels/application in > order to work for SIOV. Sure, an SIOV device emulating PCI device for guest is one requirement. And SIOV device working natively spawned from the parent PF and used without guest is another requirement. And second can be done bit elegantly in self-contained way without depending on the parent PF. And requires more work unrelated to this one. > I think we are discussing what it can be, so see the above reply for the > notification areas, we can stick virtio_admin_cmd_lq_notify_query_result > command for PCI over adminq, or invent new capabilities to make it fully self > contained. > As we both understand well that a queue is not going to transport a doorbell and actual IMS interrupts, we will mostly have commands to service some needs for backward compat. It is a separate work when there are no PCI VF devices. > > > > > > > > > > > > > You only want to exchange currently defined config registers, > > > > interrupts and > > > notification configuration using AQ. > > > > > > > > Transport VQ is NOT transporting actual data, it is not > > > > transporting > > > notifications etc. > > > > It is just a config channel. > > > > Hence, they are just commands not a "full transport". > > > > > > > > > For any case, a virtqueue could not be a full transport. It needs > > > bus facilities to do bootstrap/DMA/notifications at least. The idea > > > is to save for transport specific resources and use a more scalable > > > way to config devices. > > > > > Hence it needs only needs a cfgq, not the full transport. :) > > > > Well, not a native speaker but I'm fine if you want to call it configq. Michael called it admin vq, so will stick to it for now. :)
[virtio-dev] RE: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
> From: Michael S. Tsirkin > Sent: Tuesday, May 16, 2023 2:22 AM > > On Mon, May 15, 2023 at 03:59:41PM +, Parav Pandit wrote: > > > > > From: Jason Wang > > > Sent: Monday, May 15, 2023 3:31 AM > > > > > > What do we gain from bisecting it? > > > > > > > > > 1) No need to deal with the offset in the hardware > > > > > In context of legacy is doesn’t matter much given that its over AQ. > > Let me give you an example. I feel that device specific config should allow > arbitrary length accesses so that e.g. > mac can be read and written atomically. > > On the other hand, this is not the case for common config. > Why is this not the case with common config? Spec snippet: "When using the legacy interface the driver MAY access the device-specific configuration region using any width accesses" > I feel we do not need to allow e.g. access to both common config and device > specific one in a single operation, that is just messy. It is just an offset and value. What part bothers you? > Now sure, we can add text > with an explanation, but if instead there's a flag saying "this is common cfg" > or "this is device cfg" then it fall out automatically. > > Is this a deal breaker? No, but note how neither I nor Jason did not think > about > this ahead of the time and the correct usage just falls out of the interface > automatically. This is a mark of a good interface design. Lots of corner > cases > that one has to be careful around is not. It just moves corner case one layer above in multiple software hypervisors and hypervisor is not failing it either on cfg read/writes as expected by the guest. So bisecting is not adding anything extra. Those failure checks are going to be ignored in hypervisor anyway.
[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
On Mon, May 15, 2023 at 03:59:41PM +, Parav Pandit wrote: > > > From: Jason Wang > > Sent: Monday, May 15, 2023 3:31 AM > > > > What do we gain from bisecting it? > > > > > > 1) No need to deal with the offset in the hardware > > > In context of legacy is doesn’t matter much given that its over AQ. Let me give you an example. I feel that device specific config should allow arbitrary length accesses so that e.g. mac can be read and written atomically. On the other hand, this is not the case for common config. I feel we do not need to allow e.g. access to both common config and device specific one in a single operation, that is just messy. Now sure, we can add text with an explanation, but if instead there's a flag saying "this is common cfg" or "this is device cfg" then it fall out automatically. Is this a deal breaker? No, but note how neither I nor Jason did not think about this ahead of the time and the correct usage just falls out of the interface automatically. This is a mark of a good interface design. Lots of corner cases that one has to be careful around is not. -- MST - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
On Tue, May 16, 2023 at 11:43:09AM +0800, Jason Wang wrote: > On Tue, May 16, 2023 at 11:37 AM Jason Wang wrote: > > > > > > 在 2023/5/16 02:01, Michael S. Tsirkin 写道: > > > On Mon, May 15, 2023 at 06:00:02PM +, Parav Pandit wrote: > > >>> From: Michael S. Tsirkin > > >>> Sent: Monday, May 15, 2023 1:56 PM > > >>> > > >>> On Mon, May 15, 2023 at 05:51:05PM +, Parav Pandit wrote: > > > From: Michael S. Tsirkin > > > Sent: Monday, May 15, 2023 1:45 PM > > > > > > On Mon, May 15, 2023 at 03:49:44PM +, Parav Pandit wrote: > > >> All legacy interface via AQ. > > >> All modern interface access via PCI or its own transport between > > >> driver and > > > device. > > > > > > I am wondering however about the hypervisor notifications. > > > Generally these are the most problematic aspect here I feel. For > > > example, how does it interact with VIRTIO_F_NOTIF_CONFIG_DATA? And > > > generally, having both guest and host be allowed to access device's > > > BAR > > >>> seems problematic. > > > Can we reserve a PF BAR region for these things or is that too > > > expensive? > > VIRTIO_F_NOTIF_CONFIG_DATA is not present for the legacy. > > >>> it is not but it just might be required, or it won't be there. > > >>> > > >> How can it be required if it not part of it? > > >> I likely didn't follow your question/comment, can you please explain. > > > VIRTIO_F_NOTIF_CONFIG_DATA is there presumably for a reason. > > > > > For modern device, guest driver will access the device own BAR > > directly with > > >>> its own config_data anyway. > > Should we reserve a region in the PF BAR for SF/SIOV device?, should > > device > > >>> report which BAR/area to use for the given SF/SIOV device? > > May be yes, those are different discussions with tradeoff to consider > > during > > >>> SIOV discussions. It is not related to VFs. > > >>> > > >>> For SIOV it's a given. But we can do this for SRIOV: reserve a PF > > >>> region per VF. > > >> Each VF has its own BAR area for driver notifications. > > > But it is passed through to guest presumably. > > > > > > Probably not in the case of VIRTIO_F_NOTIF_CONFIG_DATA. Did you see any > > problem if we do mediation here except for some performance penalty? > > Ok, rethink of this, when using legacy, VIRTIO_F_NOTIF_CONFIG_DATA > should be assumed to be not negotiated. So I think I agree with Parav, > it should not be a problem. > > Or do you mean for the devices that have some mandated features? > > Thanks Exactly. VIRTIO_F_NOTIF_CONFIG_DATA if there is likely to be required for device to work as opposed to being an optimization. > > > > Thanks > > > > > > > - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
On Tue, May 16, 2023 at 11:28:04AM +0800, Jason Wang wrote: > > > > > Admin virtqueue doesn't preventing anything from letting IMS go directly > > > to the > > > device. > > But there is no aq/cfgq/cmdq defined for the guest, so with current > > proposal it is prevented. > > > It's a choice of the implementation but not spec. Spec doesn't say adminq > can't be used for guest. It actually kind of does. adminq controls group members through the group owner. owner is not itself a member. For a guest controlling a member to use adminq you have to also pass the owner to guest. Possible but not a standard architecture. -- MST - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
On Tue, May 16, 2023 at 11:45 AM Parav Pandit wrote: > > > > From: Jason Wang > > Sent: Monday, May 15, 2023 11:28 PM > > > > Hi Parav: > > > > 在 2023/5/15 23:49, Parav Pandit 写道: > > >> From: Jason Wang > > >> Sent: Monday, May 15, 2023 3:11 AM > > >>> It is just bunch of admin commands in the new SIOV or virtual virtio > > >>> device > > >> chapter. > > >>> It is not a transport chapter. > > >> > > >> Provisioning could be a part of the transport. For example SR-IOV > > >> allows static and dynamic provisioning. If a virtio device needs to > > >> be implemented in VF, it needs to be provisioned first. > > >> > > > Provisioning a device by admin != transport guest driver <-> device > > communication. > > > > > > Two very different things. > > > > > > When developing a new transport, we need to consider at least how to probe > > the device. Before probe the device, it needs to be provisioned. > Yes, and yes to both. I agree. > > > If we want a new transport, it could implement the provisioning especially > > if it > > want the design to be bus independent. For the proposal like transport > > virtqueue, it's also fine to use bus specific facility to provision and > > probe the > > device. > > > Provisioning is for defining the transport specific attributes. > Each transport has few command things and few unique things. > So provisioning commands can define transport specific attributes and few > generic things. > This is fine. > > > > > That is what I want to say. > > > > > > > > > > And the motivation is also clear is to provide composing a virtio > > > device for the > > guest VM for the backward compatibility for 1.x part of the > > specification. > > > It seems fine and indeed orthogonal to me that: it is for backward > > compatibility for already defined config fields for existing guest VM > > driver. > > > It does not conflict with the cfgq/cmdq idea whose main purpose is > > > for the > > new config fields, new use cases that doesn't require any mediation. > > > Such cfgq works across PF, VF, SF/SIOV devices in uniform way > > > without > > mediation. > > > > My understanding is that the cfgq/cmdq could be done on top, since > > the commands are part of transport unless I miss something. > > > > >>> On top of what? > > >>> cfgq/cmdq is just another queue like any other VQ. > > >>> it is orthogonal to accessing 1.x registers using group owner's queue. > > >> > > >> I think there hasn't been any proposal that call any virtqueue like > > >> cfgq/cmdq. So using that may cause a lot of misunderstanding. I think > > >> the context here is to provide or extend transport facilities via a > > >> virtqueue not something like the control virtqueue. > > > The context is to compose a PCI device for a guest which for the currently > > defined features. > > > Once a new attribute is done using cfgq/cmdq there is no problem of > > transporting it via its group member. > > > > > > And therefore, saying any new attribute also to ride on same tranportq/aq > > > via > > is not appropriate. > > > > > > The point is to try to find if there's a better way, more below. > > > > > > > > > >> Admin virtqueue doesn't preventing anything from letting IMS go > > >> directly to the device. > > > But there is no aq/cfgq/cmdq defined for the guest, so with current > > > proposal it > > is prevented. > > > > > > It's a choice of the implementation but not spec. Spec doesn't say adminq > > can't > > be used for guest. > > > For guest and by guest are two different things. > I am proposing that a cfgq/cmdq should be used directly by the guest driver > as base line like any other VQ. I'm fine with this, but if I understand correctly, but I don't see any proposal like this that is posted to the list? > And if there is a need for mediation, it should be possible. > > > I think we've discussed many times that legacy is tricky for hardware. > And we want to proceed for the described use case and requirements. > > > And your proposal proves this further by introducing modern alike > > notification areas. > Not really. A modern device is offering the notification area like legacy to > the legacy interface. > So this is just fine. I don't say it's not fine, it's pretty fine and transport virtqueue have similar commands. And this also answer the question that you think tunnel PCI over adminq is not scalable. We can keep this command in that case as well, or invent new capabilities. > > > We probably need to deal with others like endianess. > > > No point in discussing this again. One size does not fit all. > It solves large part of the use cases so it is valuable and will be supported. This is doable, we need to leave some space for future development. Or is it just complicated to have an endian command? > > > For any case, having a simple and deterministic device design is always > > better assuming we've agreed that mediation is a must for legacy
[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
在 2023/5/11 21:15, Parav Pandit 写道: From: Jason Wang Sent: Thursday, May 11, 2023 3:05 AM If we decide to use the admin vq, is there any good reason to tie it to PCI if we don't want to tunneling PCI over adminq? To truly tunnel PCI over adminq, please tunnel _all_ pci things over AQ, not just below 1 to 11. Just to clarify, below 1 to 11 is not tunnel PCI over adminq, it's the idea of transport virtqueue which is not specific to PCI. If we want to tunnel PCI over adminq, it needs be accessed via your BAR access commands. Doorbells, capabilities and more. Then it is qualifying as a transport. Otherwise, it is not a "virtio pci transport over aq". Yes, it's basically just have some extension on your proposal, adding a config space access method that should be fine. I neither see why one would tunnel whole PCI over some queue, which was not your intention either from the patches I see. VQ over fabrics is much cleaner design to transport VQ over non-PCI. That's a different topic, and we don't need to think of the legacy support there. People have tried to tunnel pci over some other transports and it only fine for non-production toys. I've replied this in another thread. Why not simply invent individual commands to access legacy facilities like commands to access like what transport virtqueue did for modern device?: I don’t see how this is being any different than register-offset interface. It bisects more things at hypervisor level that makes things hard to add #12th entry. 1) device features 2) driver features 3) queue address 4) queue size 5) queue select 6) queue notify 7) device status 8) ISR status 9) config msix 10) queue msix 11) device configuration space It focuses on the facilities instead of transport specific details like registers (we don't even need legacy registers in this case), I gives more deterministic behavior so we don't need to care about the cross registers read/write. 1.x has these registers at raw level and that seems fine. Note that 1.x has more, the above is dumped from the section of "Legacy Interfaces: A Note on PCI Device Layout". Thanks Anything new will come on the cfgq and hence no bisection or registers needed. - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] RE: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
> From: Jason Wang > Sent: Monday, May 15, 2023 11:28 PM > > Hi Parav: > > 在 2023/5/15 23:49, Parav Pandit 写道: > >> From: Jason Wang > >> Sent: Monday, May 15, 2023 3:11 AM > >>> It is just bunch of admin commands in the new SIOV or virtual virtio > >>> device > >> chapter. > >>> It is not a transport chapter. > >> > >> Provisioning could be a part of the transport. For example SR-IOV > >> allows static and dynamic provisioning. If a virtio device needs to > >> be implemented in VF, it needs to be provisioned first. > >> > > Provisioning a device by admin != transport guest driver <-> device > communication. > > > > Two very different things. > > > When developing a new transport, we need to consider at least how to probe > the device. Before probe the device, it needs to be provisioned. Yes, and yes to both. I agree. > If we want a new transport, it could implement the provisioning especially if > it > want the design to be bus independent. For the proposal like transport > virtqueue, it's also fine to use bus specific facility to provision and probe > the > device. > Provisioning is for defining the transport specific attributes. Each transport has few command things and few unique things. So provisioning commands can define transport specific attributes and few generic things. This is fine. > > That is what I want to say. > > > > > > And the motivation is also clear is to provide composing a virtio > > device for the > guest VM for the backward compatibility for 1.x part of the > specification. > > It seems fine and indeed orthogonal to me that: it is for backward > compatibility for already defined config fields for existing guest VM > driver. > > It does not conflict with the cfgq/cmdq idea whose main purpose is > > for the > new config fields, new use cases that doesn't require any mediation. > > Such cfgq works across PF, VF, SF/SIOV devices in uniform way > > without > mediation. > > My understanding is that the cfgq/cmdq could be done on top, since > the commands are part of transport unless I miss something. > > >>> On top of what? > >>> cfgq/cmdq is just another queue like any other VQ. > >>> it is orthogonal to accessing 1.x registers using group owner's queue. > >> > >> I think there hasn't been any proposal that call any virtqueue like > >> cfgq/cmdq. So using that may cause a lot of misunderstanding. I think > >> the context here is to provide or extend transport facilities via a > >> virtqueue not something like the control virtqueue. > > The context is to compose a PCI device for a guest which for the currently > defined features. > > Once a new attribute is done using cfgq/cmdq there is no problem of > transporting it via its group member. > > > > And therefore, saying any new attribute also to ride on same tranportq/aq > > via > is not appropriate. > > > The point is to try to find if there's a better way, more below. > > > > > >> Admin virtqueue doesn't preventing anything from letting IMS go > >> directly to the device. > > But there is no aq/cfgq/cmdq defined for the guest, so with current > > proposal it > is prevented. > > > It's a choice of the implementation but not spec. Spec doesn't say adminq > can't > be used for guest. > For guest and by guest are two different things. I am proposing that a cfgq/cmdq should be used directly by the guest driver as base line like any other VQ. And if there is a need for mediation, it should be possible. > I think we've discussed many times that legacy is tricky for hardware. And we want to proceed for the described use case and requirements. > And your proposal proves this further by introducing modern alike > notification areas. Not really. A modern device is offering the notification area like legacy to the legacy interface. So this is just fine. > We probably need to deal with others like endianess. > No point in discussing this again. One size does not fit all. It solves large part of the use cases so it is valuable and will be supported. > For any case, having a simple and deterministic device design is always > better assuming we've agreed that mediation is a must for legacy > drivers. Using dedicated commands can make sure the implementation won't > need to go with corner cases of legacy. > Corner case handling just moved from the device to hypervisor. > > > > >>> Its same thing using more opcodes. > >> > >> Do we really care about the number of opcodes? I think we've reserved > >> sufficient spaces. > >> > > I don’t see opcode as problem, we have plenty. > > And hence, when there is a device that _does not implement config space for > SIOV/SF, it should be introduced. > > It is the introduction and bisection at the hypervisor level unnecessary. > > > > And I don’t have object to it either, the main reason is: I don’t see it > > being > useful for 1.x access going forward. > > > As you said for future SIOV/SF? For
[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
On Tue, May 16, 2023 at 11:37 AM Jason Wang wrote: > > > 在 2023/5/16 02:01, Michael S. Tsirkin 写道: > > On Mon, May 15, 2023 at 06:00:02PM +, Parav Pandit wrote: > >>> From: Michael S. Tsirkin > >>> Sent: Monday, May 15, 2023 1:56 PM > >>> > >>> On Mon, May 15, 2023 at 05:51:05PM +, Parav Pandit wrote: > > From: Michael S. Tsirkin > > Sent: Monday, May 15, 2023 1:45 PM > > > > On Mon, May 15, 2023 at 03:49:44PM +, Parav Pandit wrote: > >> All legacy interface via AQ. > >> All modern interface access via PCI or its own transport between > >> driver and > > device. > > > > I am wondering however about the hypervisor notifications. > > Generally these are the most problematic aspect here I feel. For > > example, how does it interact with VIRTIO_F_NOTIF_CONFIG_DATA? And > > generally, having both guest and host be allowed to access device's BAR > >>> seems problematic. > > Can we reserve a PF BAR region for these things or is that too > > expensive? > VIRTIO_F_NOTIF_CONFIG_DATA is not present for the legacy. > >>> it is not but it just might be required, or it won't be there. > >>> > >> How can it be required if it not part of it? > >> I likely didn't follow your question/comment, can you please explain. > > VIRTIO_F_NOTIF_CONFIG_DATA is there presumably for a reason. > > > For modern device, guest driver will access the device own BAR directly > with > >>> its own config_data anyway. > Should we reserve a region in the PF BAR for SF/SIOV device?, should > device > >>> report which BAR/area to use for the given SF/SIOV device? > May be yes, those are different discussions with tradeoff to consider > during > >>> SIOV discussions. It is not related to VFs. > >>> > >>> For SIOV it's a given. But we can do this for SRIOV: reserve a PF region > >>> per VF. > >> Each VF has its own BAR area for driver notifications. > > But it is passed through to guest presumably. > > > Probably not in the case of VIRTIO_F_NOTIF_CONFIG_DATA. Did you see any > problem if we do mediation here except for some performance penalty? Ok, rethink of this, when using legacy, VIRTIO_F_NOTIF_CONFIG_DATA should be assumed to be not negotiated. So I think I agree with Parav, it should not be a problem. Or do you mean for the devices that have some mandated features? Thanks > > Thanks > > > > - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
在 2023/5/16 02:01, Michael S. Tsirkin 写道: On Mon, May 15, 2023 at 06:00:02PM +, Parav Pandit wrote: From: Michael S. Tsirkin Sent: Monday, May 15, 2023 1:56 PM On Mon, May 15, 2023 at 05:51:05PM +, Parav Pandit wrote: From: Michael S. Tsirkin Sent: Monday, May 15, 2023 1:45 PM On Mon, May 15, 2023 at 03:49:44PM +, Parav Pandit wrote: All legacy interface via AQ. All modern interface access via PCI or its own transport between driver and device. I am wondering however about the hypervisor notifications. Generally these are the most problematic aspect here I feel. For example, how does it interact with VIRTIO_F_NOTIF_CONFIG_DATA? And generally, having both guest and host be allowed to access device's BAR seems problematic. Can we reserve a PF BAR region for these things or is that too expensive? VIRTIO_F_NOTIF_CONFIG_DATA is not present for the legacy. it is not but it just might be required, or it won't be there. How can it be required if it not part of it? I likely didn't follow your question/comment, can you please explain. VIRTIO_F_NOTIF_CONFIG_DATA is there presumably for a reason. For modern device, guest driver will access the device own BAR directly with its own config_data anyway. Should we reserve a region in the PF BAR for SF/SIOV device?, should device report which BAR/area to use for the given SF/SIOV device? May be yes, those are different discussions with tradeoff to consider during SIOV discussions. It is not related to VFs. For SIOV it's a given. But we can do this for SRIOV: reserve a PF region per VF. Each VF has its own BAR area for driver notifications. But it is passed through to guest presumably. Probably not in the case of VIRTIO_F_NOTIF_CONFIG_DATA. Did you see any problem if we do mediation here except for some performance penalty? Thanks - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
Hi Parav: 在 2023/5/15 23:49, Parav Pandit 写道: From: Jason Wang Sent: Monday, May 15, 2023 3:11 AM It is just bunch of admin commands in the new SIOV or virtual virtio device chapter. It is not a transport chapter. Provisioning could be a part of the transport. For example SR-IOV allows static and dynamic provisioning. If a virtio device needs to be implemented in VF, it needs to be provisioned first. Provisioning a device by admin != transport guest driver <-> device communication. Two very different things. When developing a new transport, we need to consider at least how to probe the device. Before probe the device, it needs to be provisioned. If we want a new transport, it could implement the provisioning especially if it want the design to be bus independent. For the proposal like transport virtqueue, it's also fine to use bus specific facility to provision and probe the device. That is what I want to say. And the motivation is also clear is to provide composing a virtio device for the guest VM for the backward compatibility for 1.x part of the specification. It seems fine and indeed orthogonal to me that: it is for backward compatibility for already defined config fields for existing guest VM driver. It does not conflict with the cfgq/cmdq idea whose main purpose is for the new config fields, new use cases that doesn't require any mediation. Such cfgq works across PF, VF, SF/SIOV devices in uniform way without mediation. My understanding is that the cfgq/cmdq could be done on top, since the commands are part of transport unless I miss something. On top of what? cfgq/cmdq is just another queue like any other VQ. it is orthogonal to accessing 1.x registers using group owner's queue. I think there hasn't been any proposal that call any virtqueue like cfgq/cmdq. So using that may cause a lot of misunderstanding. I think the context here is to provide or extend transport facilities via a virtqueue not something like the control virtqueue. The context is to compose a PCI device for a guest which for the currently defined features. Once a new attribute is done using cfgq/cmdq there is no problem of transporting it via its group member. And therefore, saying any new attribute also to ride on same tranportq/aq via is not appropriate. The point is to try to find if there's a better way, more below. Admin virtqueue doesn't preventing anything from letting IMS go directly to the device. But there is no aq/cfgq/cmdq defined for the guest, so with current proposal it is prevented. It's a choice of the implementation but not spec. Spec doesn't say adminq can't be used for guest. Long discussion with Thomas on IMS topic. I will avoid diverting to unrelated discussion for now. Or proposed command in [1] should be tagged as siov_r1, then things will be cleaner. Maybe we can say, one of the implementations is siov_r1, since it can be used to implement devices in other transport. One of the main motivations for transport virtqueue is to reduce the transport specific resources to ease the virtio device implementation. Yes, the main motivation as for backward compatibility for the currently defined features. With that I don't see legacy 3 commands anyway conflict with [1]. It doesn't, but it's not elegant since: I don’t see how making 3 commands to 9 commands makes it elegant by doing bisecting of registers offset in hypervisor. Or it needs to be done by the hardware and cross register write/read needs to be handled there as well. That is the limitation of legacy and device can always _decide_ when to apply the parameter later when enough bits are written. Given its control path, it is not an overhead. I think we've discussed many times that legacy is tricky for hardware. And your proposal proves this further by introducing modern alike notification areas. We probably need to deal with others like endianess. For any case, having a simple and deterministic device design is always better assuming we've agreed that mediation is a must for legacy drivers. Using dedicated commands can make sure the implementation won't need to go with corner cases of legacy. Its same thing using more opcodes. Do we really care about the number of opcodes? I think we've reserved sufficient spaces. I don’t see opcode as problem, we have plenty. And hence, when there is a device that _does not implement config space for SIOV/SF, it should be introduced. It is the introduction and bisection at the hypervisor level unnecessary. And I don’t have object to it either, the main reason is: I don’t see it being useful for 1.x access going forward. As you said for future SIOV/SF? For example, it can have better scalability in IMS since it doesn't require any real capabilities or registers. 1) Modern transport, admin virtqueue with well defined transport commands 2) Legacy transport, works moe like a PCI transport on top of the
[virtio-dev] RE: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
> From: Michael S. Tsirkin > Sent: Monday, May 15, 2023 2:02 PM > > > > > How can it be required if it not part of it? > > I likely didn't follow your question/comment, can you please explain. > > VIRTIO_F_NOTIF_CONFIG_DATA is there presumably for a reason. The discussion is for the legacy interface for which CONFIG_DATA doesn't exist. > > > > > For modern device, guest driver will access the device own BAR > > > > directly with > > > its own config_data anyway. > > > > Should we reserve a region in the PF BAR for SF/SIOV device?, > > > > should device > > > report which BAR/area to use for the given SF/SIOV device? > > > > May be yes, those are different discussions with tradeoff to > > > > consider during > > > SIOV discussions. It is not related to VFs. > > > > > > For SIOV it's a given. But we can do this for SRIOV: reserve a PF region > > > per > VF. > > Each VF has its own BAR area for driver notifications. > > But it is passed through to guest presumably. Yes, so? - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
On Mon, May 15, 2023 at 06:00:02PM +, Parav Pandit wrote: > > From: Michael S. Tsirkin > > Sent: Monday, May 15, 2023 1:56 PM > > > > On Mon, May 15, 2023 at 05:51:05PM +, Parav Pandit wrote: > > > > > > > From: Michael S. Tsirkin > > > > Sent: Monday, May 15, 2023 1:45 PM > > > > > > > > On Mon, May 15, 2023 at 03:49:44PM +, Parav Pandit wrote: > > > > > All legacy interface via AQ. > > > > > All modern interface access via PCI or its own transport between > > > > > driver and > > > > device. > > > > > > > > I am wondering however about the hypervisor notifications. > > > > Generally these are the most problematic aspect here I feel. For > > > > example, how does it interact with VIRTIO_F_NOTIF_CONFIG_DATA? And > > > > generally, having both guest and host be allowed to access device's BAR > > seems problematic. > > > > Can we reserve a PF BAR region for these things or is that too > > > > expensive? > > > > > > VIRTIO_F_NOTIF_CONFIG_DATA is not present for the legacy. > > > > it is not but it just might be required, or it won't be there. > > > How can it be required if it not part of it? > I likely didn't follow your question/comment, can you please explain. VIRTIO_F_NOTIF_CONFIG_DATA is there presumably for a reason. > > > For modern device, guest driver will access the device own BAR directly > > > with > > its own config_data anyway. > > > Should we reserve a region in the PF BAR for SF/SIOV device?, should > > > device > > report which BAR/area to use for the given SF/SIOV device? > > > May be yes, those are different discussions with tradeoff to consider > > > during > > SIOV discussions. It is not related to VFs. > > > > For SIOV it's a given. But we can do this for SRIOV: reserve a PF region > > per VF. > Each VF has its own BAR area for driver notifications. But it is passed through to guest presumably. -- MST - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] RE: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
> From: Michael S. Tsirkin > Sent: Monday, May 15, 2023 1:56 PM > > On Mon, May 15, 2023 at 05:51:05PM +, Parav Pandit wrote: > > > > > From: Michael S. Tsirkin > > > Sent: Monday, May 15, 2023 1:45 PM > > > > > > On Mon, May 15, 2023 at 03:49:44PM +, Parav Pandit wrote: > > > > All legacy interface via AQ. > > > > All modern interface access via PCI or its own transport between > > > > driver and > > > device. > > > > > > I am wondering however about the hypervisor notifications. > > > Generally these are the most problematic aspect here I feel. For > > > example, how does it interact with VIRTIO_F_NOTIF_CONFIG_DATA? And > > > generally, having both guest and host be allowed to access device's BAR > seems problematic. > > > Can we reserve a PF BAR region for these things or is that too expensive? > > > > VIRTIO_F_NOTIF_CONFIG_DATA is not present for the legacy. > > it is not but it just might be required, or it won't be there. > How can it be required if it not part of it? I likely didn't follow your question/comment, can you please explain. > > For modern device, guest driver will access the device own BAR directly with > its own config_data anyway. > > Should we reserve a region in the PF BAR for SF/SIOV device?, should device > report which BAR/area to use for the given SF/SIOV device? > > May be yes, those are different discussions with tradeoff to consider during > SIOV discussions. It is not related to VFs. > > For SIOV it's a given. But we can do this for SRIOV: reserve a PF region per > VF. Each VF has its own BAR area for driver notifications. - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
On Mon, May 15, 2023 at 05:51:05PM +, Parav Pandit wrote: > > > From: Michael S. Tsirkin > > Sent: Monday, May 15, 2023 1:45 PM > > > > On Mon, May 15, 2023 at 03:49:44PM +, Parav Pandit wrote: > > > All legacy interface via AQ. > > > All modern interface access via PCI or its own transport between driver > > > and > > device. > > > > I am wondering however about the hypervisor notifications. Generally these > > are the most problematic aspect here I feel. For example, how does it > > interact > > with VIRTIO_F_NOTIF_CONFIG_DATA? And generally, having both guest and > > host be allowed to access device's BAR seems problematic. > > Can we reserve a PF BAR region for these things or is that too expensive? > > VIRTIO_F_NOTIF_CONFIG_DATA is not present for the legacy. it is not but it just might be required, or it won't be there. > For modern device, guest driver will access the device own BAR directly with > its own config_data anyway. > Should we reserve a region in the PF BAR for SF/SIOV device?, should device > report which BAR/area to use for the given SF/SIOV device? > May be yes, those are different discussions with tradeoff to consider during > SIOV discussions. It is not related to VFs. For SIOV it's a given. But we can do this for SRIOV: reserve a PF region per VF. - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] RE: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
> From: Michael S. Tsirkin > Sent: Monday, May 15, 2023 1:45 PM > > On Mon, May 15, 2023 at 03:49:44PM +, Parav Pandit wrote: > > All legacy interface via AQ. > > All modern interface access via PCI or its own transport between driver and > device. > > I am wondering however about the hypervisor notifications. Generally these > are the most problematic aspect here I feel. For example, how does it > interact > with VIRTIO_F_NOTIF_CONFIG_DATA? And generally, having both guest and > host be allowed to access device's BAR seems problematic. > Can we reserve a PF BAR region for these things or is that too expensive? VIRTIO_F_NOTIF_CONFIG_DATA is not present for the legacy. For modern device, guest driver will access the device own BAR directly with its own config_data anyway. Should we reserve a region in the PF BAR for SF/SIOV device?, should device report which BAR/area to use for the given SF/SIOV device? May be yes, those are different discussions with tradeoff to consider during SIOV discussions. It is not related to VFs. - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
On Mon, May 15, 2023 at 03:49:44PM +, Parav Pandit wrote: > All legacy interface via AQ. > All modern interface access via PCI or its own transport between driver and > device. I am wondering however about the hypervisor notifications. Generally these are the most problematic aspect here I feel. For example, how does it interact with VIRTIO_F_NOTIF_CONFIG_DATA? And generally, having both guest and host be allowed to access device's BAR seems problematic. Can we reserve a PF BAR region for these things or is that too expensive? -- MST - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] RE: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
Hi Michael, > From: virtio-comm...@lists.oasis-open.org open.org> On Behalf Of Parav Pandit > Sent: Thursday, May 11, 2023 5:03 PM > > > From: Michael S. Tsirkin > > Sent: Thursday, May 11, 2023 4:58 PM > > > > On Thu, May 11, 2023 at 08:47:04PM +, Parav Pandit wrote: > > > > From: Michael S. Tsirkin > > > > Sent: Thursday, May 11, 2023 9:39 AM > > > > > > > > On Thu, May 11, 2023 at 01:28:40PM +, Parav Pandit wrote: > > > > > This is not the objective to transport all PCI virtio fields over AQ. > > > > > Transport VQ commands proposal commands and commands in this > > > > > proposal > > > > are just fine the way there without quoting it as "transport". > > > > > > > > I mean, it's a transport in a sense of where it will reside in e.g. > > > > linux virtio stack - below virtio core. > > > > We don't save much code there by reusing the same register layout > > > > of legacy pci. > > > > But let's at least make this feature complete, and then we'll see > > > > whether it is cleaner to reuse legacy layout or build a new one. > > > > > > I don't follow your comment about "this feature complete". > > > When you say _this_, it means the feature proposed in this patch of > > supporting legacy over PCI VFs? > > > > > > If so, anything missing in this proposal? > > > > I think I commented at least on what I see as missing is first of all > > a need to support INT#x emulation since level interrupts are I think not > supported for VFs. > > right? > > I don't see it mandatory as I responded in [1]. > I didn't see your response on [1]. > > [1] https://lore.kernel.org/virtio-comment/71d65eb3-c025-9287-0157- > 81e1d0557...@nvidia.com/ Do you have any further suggestion, or we can differ the eventq for future when/if there is any real problem? - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] RE: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
> From: Jason Wang > Sent: Monday, May 15, 2023 3:31 AM > > What do we gain from bisecting it? > > > 1) No need to deal with the offset in the hardware > In context of legacy is doesn’t matter much given that its over AQ. > 2) transport independent > AQ commands are transport independent so, it is covered. > 3) avoid duplication with the (legacy support of) transport virtqueue > Modern interface does not need mediation; hence it is not duplication with legacy. > > > Every new additional needs involvement of the hypervisor as Michael noted > below on "effort" point. > > > I'm not sure I get your point. > > If additional effort is not wanted, we need a full PCI over adminq, > that's what I suggested, then we don't need any extension for any new > features probably. > When there is already a PCI VF, I don’t see why one would implement "PCI over adminq". It is impractical. And SF/SIOV is bit still far in future. When "PCI over adminq" happen, it is really a very different set of commands that emulated the "PCI virtio" device. I don’t know when/why. > If we choose to invent dedicated adminq commands: > > For legacy, we don't need any new additional effort since the above 11 > maps to the legacy virtio-pci facilities. > Only few commands are needed. Modern interface config doesn’t travel through mediation. > For modern, if the feature is something related to the transport, we > need new transport interface for sure. > Everything is transport as it flows to/from driver to device covered in the respective transport chapter. > > > > > The register offset and read/write is far simpler interface for hypervisor. > > > If this is true, why not go full PCI over adminq? Hypervisor in this > case even don't need to deal with config space. > I don’t see a use case now and its extremely heavy to do so and it doesn’t apply to PCI VFs at all. > > > > > Not to do cross register access is what we need to define in the spec for > > 1.x or > future things. > > > You can't assume the hypervisor behaviors, so the device or at least the > spec should be ready for that. > My above comment is for the driver to device interface to access registers on its natural boundary. Which part of the proposal assumes a hypervisor behavior? Nothing to do with hypervisor.
[virtio-dev] RE: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
> From: Jason Wang > Sent: Monday, May 15, 2023 3:11 AM > > It is just bunch of admin commands in the new SIOV or virtual virtio device > chapter. > > It is not a transport chapter. > > > Provisioning could be a part of the transport. For example SR-IOV allows > static > and dynamic provisioning. If a virtio device needs to be implemented in VF, it > needs to be provisioned first. > Provisioning a device by admin != transport guest driver <-> device communication. Two very different things. > > > > >>> And the motivation is also clear is to provide composing a virtio > >>> device for the > >> guest VM for the backward compatibility for 1.x part of the specification. > >>> It seems fine and indeed orthogonal to me that: it is for backward > >> compatibility for already defined config fields for existing guest VM > >> driver. > >>> It does not conflict with the cfgq/cmdq idea whose main purpose is > >>> for the > >> new config fields, new use cases that doesn't require any mediation. > >>> Such cfgq works across PF, VF, SF/SIOV devices in uniform way > >>> without > >> mediation. > >> > >> My understanding is that the cfgq/cmdq could be done on top, since > >> the commands are part of transport unless I miss something. > >> > > On top of what? > > cfgq/cmdq is just another queue like any other VQ. > > it is orthogonal to accessing 1.x registers using group owner's queue. > > > I think there hasn't been any proposal that call any virtqueue like > cfgq/cmdq. So > using that may cause a lot of misunderstanding. I think the context here is to > provide or extend transport facilities via a virtqueue not something like the > control virtqueue. The context is to compose a PCI device for a guest which for the currently defined features. Once a new attribute is done using cfgq/cmdq there is no problem of transporting it via its group member. And therefore, saying any new attribute also to ride on same tranportq/aq via is not appropriate. > > Admin virtqueue doesn't preventing anything from letting IMS go directly to > the > device. But there is no aq/cfgq/cmdq defined for the guest, so with current proposal it is prevented. > > > > Long discussion with Thomas on IMS topic. > > I will avoid diverting to unrelated discussion for now. > > > >>> Or proposed command in [1] should be tagged as siov_r1, then things will > be > >> cleaner. > >> > >> Maybe we can say, one of the implementations is siov_r1, since it can be > used > >> to implement devices in other transport. One of the main motivations for > >> transport virtqueue is to reduce the transport specific resources to ease > >> the > >> virtio device implementation. > >> > > Yes, the main motivation as for backward compatibility for the currently > defined features. > > > >>> With that I don't see legacy 3 commands anyway conflict with [1]. > >> It doesn't, but it's not elegant since: > >> > > I don’t see how making 3 commands to 9 commands makes it elegant by > doing bisecting of registers offset in hypervisor. > > > Or it needs to be done by the hardware and cross register write/read > needs to be handled there as well. > That is the limitation of legacy and device can always _decide_ when to apply the parameter later when enough bits are written. Given its control path, it is not an overhead. > > > Its same thing using more opcodes. > > > Do we really care about the number of opcodes? I think we've reserved > sufficient spaces. > I don’t see opcode as problem, we have plenty. And hence, when there is a device that _does not implement config space for SIOV/SF, it should be introduced. It is the introduction and bisection at the hypervisor level unnecessary. And I don’t have object to it either, the main reason is: I don’t see it being useful for 1.x access going forward. > > > > >> 1) Modern transport, admin virtqueue with well defined transport > commands > >> 2) Legacy transport, works moe like a PCI transport on top of the admin > >> virtqueue > >> > >> Transport virtqueue tend to be transport independent, but 2) tie it somehow > to > >> the PCI. That's why I suggest to > >> > > The 4 patches are not transporting all the PCI things over transport VQ. it > > is > not a transport. > > > It really depends on the different viewpoint. You wrote/asked for "PCI transport over admin q". That means PCI capabilities, PCI config registers and all things defined in the PCI transport section to go over the AQ. This is clearly not the objective of these 2 patches. Nor it is your proposal of transport VQ. You only want to exchange currently defined config registers, interrupts and notification configuration using AQ. Transport VQ is NOT transporting actual data, it is not transporting notifications etc. It is just a config channel. Hence, they are just commands not a "full transport". > To me, adminq is not > specific to PCI, so this proposal looks more like a partial transport > for legacy. It is not a partial transport
[virtio-dev] RE: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
> From: Jason Wang > Sent: Monday, May 15, 2023 1:20 AM > > Right, but as discussed, a possible user is the SIOV transitional device via > transport virtqueues. > > > > transport vq might, but I am worried we'll have to spend extra > > work clarifying it's a legacy interface. For example we will > > have to explain that only 32 bits of features must be used. > > Focusing more effort on transport vq is attractive though ... > > It is worth examining this option in more depth. > > Did you? > > > My understanding is, for any case, we need such clarifications. What > Parav proposed here is not the access to the actual registers but stick > to the semantic of those registers. I think we must say high 32bits of > the features must be assumed to be 0. > I am avoided duplicating anything that is already written for the PCI interface. In legacy interface section of PCI, we already have: "Note that only Feature Bits 0 to 31 are accessible through the Legacy Interface". If MMIO is missing such line in existing interface, it is better to be added there. It is not scope of this patch.
[virtio-dev] RE: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
> From: Michael S. Tsirkin > Sent: Monday, May 15, 2023 6:09 AM > To be more specific, device config often has the same layout under legacy and > modern. Thus if getting an offset within device config, same decoding logic > should be reusable between legacy and modern. Modern device registers are directly accessible from the guest driver without mediation for VF,SF,SIOV. Hence, there is no need to have commands for modern register access over some VQ. Will reply to rest of the Jason's comments in some time.
[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
On Mon, May 15, 2023 at 03:30:47PM +0800, Jason Wang wrote: > > 在 2023/5/11 21:02, Parav Pandit 写道: > > > > > From: Michael S. Tsirkin > > > Sent: Thursday, May 11, 2023 8:55 AM > > > > 1) device features > > > > 2) driver features > > > > 3) queue address > > > > 4) queue size > > > > 5) queue select > > > > 6) queue notify > > > > 7) device status > > > > 8) ISR status > > > > 9) config msix > > > > 10) queue msix > > > > 11) device configuration space > > > > > > #9 may not even go to the group owner device. > > > It the config_msix_vector register so I don't understand why it can't work > but can work with your proposal. > > > > What do we gain from bisecting it? > > > 1) No need to deal with the offset in the hardware > > 2) transport independent > > 3) avoid duplication with the (legacy support of) transport virtqueue To be more specific, device config often has the same layout under legacy and modern. Thus if getting an offset within device config, same decoding logic should be reusable between legacy and modern. -- MST - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
在 2023/5/11 21:02, Parav Pandit 写道: From: Michael S. Tsirkin Sent: Thursday, May 11, 2023 8:55 AM 1) device features 2) driver features 3) queue address 4) queue size 5) queue select 6) queue notify 7) device status 8) ISR status 9) config msix 10) queue msix 11) device configuration space #9 may not even go to the group owner device. It the config_msix_vector register so I don't understand why it can't work but can work with your proposal. What do we gain from bisecting it? 1) No need to deal with the offset in the hardware 2) transport independent 3) avoid duplication with the (legacy support of) transport virtqueue Every new additional needs involvement of the hypervisor as Michael noted below on "effort" point. I'm not sure I get your point. If additional effort is not wanted, we need a full PCI over adminq, that's what I suggested, then we don't need any extension for any new features probably. If we choose to invent dedicated adminq commands: For legacy, we don't need any new additional effort since the above 11 maps to the legacy virtio-pci facilities. For modern, if the feature is something related to the transport, we need new transport interface for sure. The register offset and read/write is far simpler interface for hypervisor. If this is true, why not go full PCI over adminq? Hypervisor in this case even don't need to deal with config space. Not to do cross register access is what we need to define in the spec for 1.x or future things. You can't assume the hypervisor behaviors, so the device or at least the spec should be ready for that. Thanks It focuses on the facilities instead of transport specific details like registers (we don't even need legacy registers in this case), I gives more deterministic behavior so we don't need to care about the cross registers read/write. This needs thought, it is definitely more work. Effort that could be maybe spent on new features. What is the motivation here? supporting legacy mmio guests? - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
在 2023/5/11 20:54, Michael S. Tsirkin 写道: On Thu, May 11, 2023 at 03:04:40PM +0800, Jason Wang wrote: On Wed, May 10, 2023 at 3:43 PM Michael S. Tsirkin wrote: On Wed, May 10, 2023 at 03:01:25PM +0800, Jason Wang wrote: On Wed, May 10, 2023 at 2:05 PM Michael S. Tsirkin wrote: On Mon, May 08, 2023 at 10:23:39AM +0800, Jason Wang wrote: I thought so too originally. Unfortunately I now think that no, legacy is not going to be a byproduct of transport virtqueue for modern - it is different enough that it needs dedicated commands. If you mean the transport virtqueue, I think some dedicated commands for legacy are needed. Then it would be a transport that supports transitional devices. It would be much better than having commands for a partial transport like this patch did. OK I am beginning to get what you are saying. So your criticism is this: what if device supports vq transport for modern, and we want to build a transitional device on top. how will that look. yes? Yes. I think it needs to be done through the transport virtqueue otherwise the transport is not self-contained. I mean, any feature can be done over transport vq. But there is value in adding legacy commands to an otherwise modern device without reworking it completely to switch to a different transport. There's probably no need for a rework since legacy is not complicated. More below. A reasonable thing to include at least in the commit log. Parav? You are also asking what if the device uses transport vq, and we want transitional on top of that. It's a fair question but I don't exactly get why would this legacy support feature be wanted for the vq transport and not for other transports. Not sure I get the question, but all the existing transport support legacy, if we want to have another, should the legacy support be a must or not? This specific proposal is for tunneling legacy over admin vq. It can live alongside a normal modern VF, with hypervisor combining these to create a transitional device. Exactly, but what I meant here is If we decide to use the admin vq, is there any good reason to tie it to PCI if we don't want to tunneling PCI over adminq? Why not simply invent individual commands to access legacy facilities like commands to access like what transport virtqueue did for modern device?: 1) device features 2) driver features 3) queue address 4) queue size 5) queue select 6) queue notify 7) device status 8) ISR status 9) config msix 10) queue msix 11) device configuration space It focuses on the facilities instead of transport specific details like registers (we don't even need legacy registers in this case), I gives more deterministic behavior so we don't need to care about the cross registers read/write. This needs thought, it is definitely more work. Effort that could be maybe spent on new features. What is the motivation here? supporting legacy mmio guests? Probably. It tries to make legacy transport independent, and it's the way that how transport virtqueue want to be. Thanks - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
在 2023/5/11 21:28, Parav Pandit 写道: From: Jason Wang Sent: Thursday, May 11, 2023 3:07 AM 4th point in cover letter is: "config virtqueues of the managed device". This work belongs to the driver -> device direct communication using a queue from driver to device. So, I imagine this work can be done using a queue by the guest driver and serviced by the device like how a guest driver configures the queue today without any mediation. For PCI, MMIO transport, surely this can be done by the PCI device directly being is PF, VF or SIOV. (Instead of config register, using a new queue interface). Looks fine to me. Great. Can this new cfg queue mediated like CVQ that is done in a sw? May be yes. Should it be always mediated when it is of VF, SIOV Device? Mostly no because it is yet another VQ for PF, VF, SIOV. Yes, the mediation is always conditional but not mandated when doing the design. I am yet to parse rest of the 4 patches, please give me some time to review it. I went over the past work of [1], [2]. [1] https://lore.kernel.org/all/20220826100034.200432-2-lingshan.zhu@intel .com/ [2] https://lists.oasis-open.org/archives/virtio-comment/202208/msg00141.h tml The "virtio q as transport" in [2] is bit misleading as its only role is to transport the _registers_ of the SIOV_R1 device through its parent PF. Rest of the work is the pure management work to manage the life cycle of SIOV devices (create/delete/configure/compose). I think this can be tweaked for static provisioning setups like SR-IOV. E.g group owner can choose to not advertise the life cycle management commands. Then the hypervisor may know the provisioning is transport specific. It is just bunch of admin commands in the new SIOV or virtual virtio device chapter. It is not a transport chapter. Provisioning could be a part of the transport. For example SR-IOV allows static and dynamic provisioning. If a virtio device needs to be implemented in VF, it needs to be provisioned first. And the motivation is also clear is to provide composing a virtio device for the guest VM for the backward compatibility for 1.x part of the specification. It seems fine and indeed orthogonal to me that: it is for backward compatibility for already defined config fields for existing guest VM driver. It does not conflict with the cfgq/cmdq idea whose main purpose is for the new config fields, new use cases that doesn't require any mediation. Such cfgq works across PF, VF, SF/SIOV devices in uniform way without mediation. My understanding is that the cfgq/cmdq could be done on top, since the commands are part of transport unless I miss something. On top of what? cfgq/cmdq is just another queue like any other VQ. it is orthogonal to accessing 1.x registers using group owner's queue. I think there hasn't been any proposal that call any virtqueue like cfgq/cmdq. So using that may cause a lot of misunderstanding. I think the context here is to provide or extend transport facilities via a virtqueue not something like the control virtqueue. It also scales device register memory also very well in predictable way. The registers and feature bits access described in [2], can certainly be done using new non_legacy new AQ commands. Both legacy and non-legacy command have different semantics as Michael mentioned. The AQ semantics that Michael did as opposed to "virtqueue as transport" fits well for the use case described in [1]. There are changes on going in MSI-X area and SIOV, so you might want to wait for it. I think you meant IMS, actually, it's not hard to add new commands to support a general IMS. We can start from a virtio specific one (as it looks a common requirement not only for PCI/SIOV but also for transport like MMIO) IMS configuration usually supposed to go directly to the device. Admin virtqueue doesn't preventing anything from letting IMS go directly to the device. Long discussion with Thomas on IMS topic. I will avoid diverting to unrelated discussion for now. Or proposed command in [1] should be tagged as siov_r1, then things will be cleaner. Maybe we can say, one of the implementations is siov_r1, since it can be used to implement devices in other transport. One of the main motivations for transport virtqueue is to reduce the transport specific resources to ease the virtio device implementation. Yes, the main motivation as for backward compatibility for the currently defined features. With that I don't see legacy 3 commands anyway conflict with [1]. It doesn't, but it's not elegant since: I don’t see how making 3 commands to 9 commands makes it elegant by doing bisecting of registers offset in hypervisor. Or it needs to be done by the hardware and cross register write/read needs to be handled there as well. Its same thing using more opcodes. Do we really care about the number of opcodes? I think we've reserved sufficient spaces. 1) Modern transport, admin
[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
在 2023/5/11 21:04, Michael S. Tsirkin 写道: On Thu, May 11, 2023 at 03:06:48PM +0800, Jason Wang wrote: With that I don't see legacy 3 commands anyway conflict with [1]. It doesn't, but it's not elegant since: 1) Modern transport, admin virtqueue with well defined transport commands 2) Legacy transport, works moe like a PCI transport on top of the admin virtqueue Transport virtqueue tend to be transport independent, but 2) tie it somehow to the PCI. This "somehow" matters though. The connection is really just the common config layout. That's why I suggest to 1) introduce legacy transport commands or 2) tweak your proposal to become a PCI over admin virtqueue So you are saying it's really not generally "legacy over admin queue" it is more like "legacy pci over admin queue"? Yes. Good point. And I feel it's a good point that this chooses legacy pci but it could really be mmio or ccw or the upcoming vq transport just as well. I suspect mmio or ccw won't work well for a physical pci card though. Right, but as discussed, a possible user is the SIOV transitional device via transport virtqueues. transport vq might, but I am worried we'll have to spend extra work clarifying it's a legacy interface. For example we will have to explain that only 32 bits of features must be used. Focusing more effort on transport vq is attractive though ... It is worth examining this option in more depth. Did you? My understanding is, for any case, we need such clarifications. What Parav proposed here is not the access to the actual registers but stick to the semantic of those registers. I think we must say high 32bits of the features must be assumed to be 0. Want to share how is transport vq closer than legacy pci to what we might want? I'm not sure I get the question, but in another thread I show 10+ commands to access the legacy facilities which is really very close to the transport virtqueue. Thanks Some commands functionality is common between [1] and this proposal. But that's how the legacy is. It is confined to legacy emulation. So [1] can be done as follow_on to AQ and these series. A small note about [2]. virtio_transportq_ctrl_dev_attribute should be detached from CREATE call and split to two commands. So that VF and SF/SIOV can both utilize it. SF/SIOV_R1 can use the creation and config part. VFs will use only the device features + config space. Good point. This could be fixed. Thanks - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
RE: [virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
> From: virtio-dev@lists.oasis-open.org On > Behalf Of Michael S. Tsirkin > Sent: Thursday, May 11, 2023 9:45 AM > > On Thu, May 11, 2023 at 01:15:15PM +, Parav Pandit wrote: > > > Why not simply invent individual commands to access legacy > > > facilities like commands to access like what transport virtqueue did > > > for modern > > > device?: > > > > > I don’t see how this is being any different than register-offset interface. > > For example legacy device config's offset moves depending on MSI enable. > Which is a missing piece in the current proposal btw as I keep saying. How is this missing? The offset is decided by the driver when msi enable. Device also know that msi is enable/disabled. So device responds based on what is done with msi, like how a device responds today on iobar. I responded in this previously, don’t have the link, not sure if you got chance to read through.
[virtio-dev] RE: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
> From: Michael S. Tsirkin > Sent: Thursday, May 11, 2023 4:58 PM > > On Thu, May 11, 2023 at 08:47:04PM +, Parav Pandit wrote: > > > From: Michael S. Tsirkin > > > Sent: Thursday, May 11, 2023 9:39 AM > > > > > > On Thu, May 11, 2023 at 01:28:40PM +, Parav Pandit wrote: > > > > This is not the objective to transport all PCI virtio fields over AQ. > > > > Transport VQ commands proposal commands and commands in this > > > > proposal > > > are just fine the way there without quoting it as "transport". > > > > > > I mean, it's a transport in a sense of where it will reside in e.g. > > > linux virtio stack - below virtio core. > > > We don't save much code there by reusing the same register layout of > > > legacy pci. > > > But let's at least make this feature complete, and then we'll see > > > whether it is cleaner to reuse legacy layout or build a new one. > > > > I don't follow your comment about "this feature complete". > > When you say _this_, it means the feature proposed in this patch of > supporting legacy over PCI VFs? > > > > If so, anything missing in this proposal? > > I think I commented at least on what I see as missing is first of all a need > to > support INT#x emulation since level interrupts are I think not supported for > VFs. > right? I don't see it mandatory as I responded in [1]. I didn't see your response on [1]. [1] https://lore.kernel.org/virtio-comment/71d65eb3-c025-9287-0157-81e1d0557...@nvidia.com/ - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
On Thu, May 11, 2023 at 08:47:04PM +, Parav Pandit wrote: > > From: Michael S. Tsirkin > > Sent: Thursday, May 11, 2023 9:39 AM > > > > On Thu, May 11, 2023 at 01:28:40PM +, Parav Pandit wrote: > > > This is not the objective to transport all PCI virtio fields over AQ. > > > Transport VQ commands proposal commands and commands in this proposal > > are just fine the way there without quoting it as "transport". > > > > I mean, it's a transport in a sense of where it will reside in e.g. > > linux virtio stack - below virtio core. > > We don't save much code there by reusing the same register layout of legacy > > pci. > > But let's at least make this feature complete, and then we'll see whether > > it is > > cleaner to reuse legacy layout or build a new one. > > I don't follow your comment about "this feature complete". > When you say _this_, it means the feature proposed in this patch of > supporting legacy over PCI VFs? > > If so, anything missing in this proposal? I think I commented at least on what I see as missing is first of all a need to support INT#x emulation since level interrupts are I think not supported for VFs. right? - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] RE: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
> From: Michael S. Tsirkin > Sent: Thursday, May 11, 2023 9:39 AM > > On Thu, May 11, 2023 at 01:28:40PM +, Parav Pandit wrote: > > This is not the objective to transport all PCI virtio fields over AQ. > > Transport VQ commands proposal commands and commands in this proposal > are just fine the way there without quoting it as "transport". > > I mean, it's a transport in a sense of where it will reside in e.g. > linux virtio stack - below virtio core. > We don't save much code there by reusing the same register layout of legacy > pci. > But let's at least make this feature complete, and then we'll see whether it > is > cleaner to reuse legacy layout or build a new one. I don't follow your comment about "this feature complete". When you say _this_, it means the feature proposed in this patch of supporting legacy over PCI VFs? If so, anything missing in this proposal? - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] RE: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
> From: Michael S. Tsirkin > Sent: Thursday, May 11, 2023 9:39 AM > > On Thu, May 11, 2023 at 01:28:40PM +, Parav Pandit wrote: > > This is not the objective to transport all PCI virtio fields over AQ. > > Transport VQ commands proposal commands and commands in this proposal > are just fine the way there without quoting it as "transport". > > I mean, it's a transport in a sense of where it will reside in e.g. > linux virtio stack - below virtio core. Not sure I understand, if you are asking for SF/SIOV virtio device, it should have its own communication path from driver to device without going through the PF AQ. Because it has no backward compatibility baggage. There is no need for PF based AQ for it. PF based AQ is only needed for exposing a backward compatible guest VM facing PCI or MMIO virtio device whose backend is either PCI VF or SF/SIOV virtio passthrough device. Such backward compatible composition requirement and above SIOV device are two very different requirements. > We don't save much code there by reusing the same register layout of legacy > pci. > But let's at least make this feature complete, and then we'll see whether it > is > cleaner to reuse legacy layout or build a new one. > > -- > MST - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] RE: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
> From: Jason Wang > Sent: Thursday, May 11, 2023 3:17 AM > > a configq/cmdq discussion is for new features for PF, VF, and SF/SIOV > > devices to work in _non_ backward compatible way, because for new > > features there is no such thing as backward compatibility between > > guest driver and the device. > > Ok, so what command did configq/cmdq have? > I thought it was similar to transport virtqueue, but it looks more like a > control virtqueue here. Yes, it is more like a cvq to do device attributes discovery and configuration. Just that its generic enough. For example creating VQ that spans multiple physical addresses. Discovery and enabling new device attributes which are rarely accessed or accessed during device init/cleanup time. The fundamental difference between cfgvq to tvq is: Cfgvq is between the member virtio device and its driver. Tvq=aq is on group owner of the virtio device. > > > > > > > > >> Compatibility is coming at a cost which is not scaling. > > >> If you attempt to scale, it breaks the future path forward to go without > mediation. > > >> > > > > So eve growing these fields and optionally placement on configq > > doesn't really help and device builder to build it efficiently > > (without much predictability). > > >>> > > >>> Config queue is not the only choice, we have a lot of other > > >>> choices (for example PASID may help to reduce the on-chip resources). > > >>> > > >> PASID is for process isolation security construct, it is not for > > >> transportation. > > > > > > I meant with PASID you don't even need a BAR. > > > > > Not sure I fully understand, but my guess is, this is coming from > > mediation thought process. > > Not actually. I meant, for example without PASID, for modern devices, the > common cfg structure must be placed on a BAR through MMIO. > > But with the help of PASID, it could stay in the memory and the device can > fetch > it via DMA. The help to reduce the resources for registers. > Yes, it can stay in memory and device can fetch via DMA using a VQ using its own q from guest driver. Why does it need PASID? > > > > >> Same, SIOV and VFs do not prefer mediation going forward in the use > cases we come across. > > > > > > Unless you don't want to provide VF compatibility for SIOV. > > I do not understand what compatibility is this between which two elements? > > VF has its identify currently and SIOV will have its own identity in future. > > Ok, so it really depends on how to implement the transport for SIOV. I guess > it > won't use registers/BAR any more. If this is true, if we want to run a > virtio-pci > driver, it needs some mediation. > > It works like what you proposed here. Someday, SR-IOV or BAR transport will > become "legacy", but we need a way to make the "legacy virtio-pci driver" > work. > > > > Live migration at VF level for the passthrough device can be just fine > > without any mediation as long as all the parameters on src and dst > > side match. > > We already discussed in v0, so I prefer to avoid this again and focus > > on the patch in discussion. > > I agree we are far from the topic of legacy support. But my point stands > still. > We can discuss when live migration patches were posted by somebody. > Ok. > > Yet we are repeating and diverging from the discussion, that we should > > not intermix: > > 1. How to access legacy registers of the VF > > With what you proposed, we don't even need legacy registers for VF. We just > need to keep its semantic via the admin virtqueue commands. Yes, we don’t have IOBAR registers for the PCI VF. It is the 1.x VF so there are no legacy registers on it. As proposed it is accessed are via aq commands of PF.
[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
On Thu, May 11, 2023 at 01:15:15PM +, Parav Pandit wrote: > > Why not simply invent individual commands to access legacy facilities like > > commands to access like what transport virtqueue did for modern > > device?: > > > I don’t see how this is being any different than register-offset interface. For example legacy device config's offset moves depending on MSI enable. Which is a missing piece in the current proposal btw as I keep saying. Setting a flag or using a separate command for device config does indeed sound cleaner. -- MST - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
On Thu, May 11, 2023 at 01:28:40PM +, Parav Pandit wrote: > This is not the objective to transport all PCI virtio fields over AQ. > Transport VQ commands proposal commands and commands in this proposal are > just fine the way there without quoting it as "transport". I mean, it's a transport in a sense of where it will reside in e.g. linux virtio stack - below virtio core. We don't save much code there by reusing the same register layout of legacy pci. But let's at least make this feature complete, and then we'll see whether it is cleaner to reuse legacy layout or build a new one. -- MST - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] RE: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
> From: Jason Wang > Sent: Thursday, May 11, 2023 3:07 AM > > > 4th point in cover letter is: "config virtqueues of the managed device". > > > > > > This work belongs to the driver -> device direct communication using > > > a queue from driver to device. > > > So, I imagine this work can be done using a queue by the guest > > > driver and serviced by the device like how a guest driver configures > > > the queue today without any mediation. > > > For PCI, MMIO transport, surely this can be done by the PCI device > > > directly being is PF, VF or SIOV. > > > (Instead of config register, using a new queue interface). Looks fine to > > > me. > > Great. > > > > > > > Can this new cfg queue mediated like CVQ that is done in a sw? May be yes. > > > Should it be always mediated when it is of VF, SIOV Device? Mostly > > > no because it is yet another VQ for PF, VF, SIOV. > > Yes, the mediation is always conditional but not mandated when doing the > design. > > > > > > > I am yet to parse rest of the 4 patches, please give me some time to > > > review > it. > > > > I went over the past work of [1], [2]. > > > > [1] > > https://lore.kernel.org/all/20220826100034.200432-2-lingshan.zhu@intel > > .com/ [2] > > https://lists.oasis-open.org/archives/virtio-comment/202208/msg00141.h > > tml > > > > The "virtio q as transport" in [2] is bit misleading as its only role is to > transport the _registers_ of the SIOV_R1 device through its parent PF. > > Rest of the work is the pure management work to manage the life cycle of > SIOV devices (create/delete/configure/compose). > > I think this can be tweaked for static provisioning setups like SR-IOV. E.g > group > owner can choose to not advertise the life cycle management commands. Then > the hypervisor may know the provisioning is transport specific. > It is just bunch of admin commands in the new SIOV or virtual virtio device chapter. It is not a transport chapter. > > > > And the motivation is also clear is to provide composing a virtio device > > for the > guest VM for the backward compatibility for 1.x part of the specification. > > > > It seems fine and indeed orthogonal to me that: it is for backward > compatibility for already defined config fields for existing guest VM driver. > > > > It does not conflict with the cfgq/cmdq idea whose main purpose is for the > new config fields, new use cases that doesn't require any mediation. > > Such cfgq works across PF, VF, SF/SIOV devices in uniform way without > mediation. > > My understanding is that the cfgq/cmdq could be done on top, since the > commands are part of transport unless I miss something. > On top of what? cfgq/cmdq is just another queue like any other VQ. it is orthogonal to accessing 1.x registers using group owner's queue. > > It also scales device register memory also very well in predictable way. > > > > The registers and feature bits access described in [2], can certainly be > > done > using new non_legacy new AQ commands. > > Both legacy and non-legacy command have different semantics as Michael > mentioned. > > > > The AQ semantics that Michael did as opposed to "virtqueue as transport" > > fits > well for the use case described in [1]. > > > > There are changes on going in MSI-X area and SIOV, so you might want to > wait for it. > > I think you meant IMS, actually, it's not hard to add new commands to support > a general IMS. We can start from a virtio specific one (as it looks a common > requirement not only for PCI/SIOV but also for transport like MMIO) > IMS configuration usually supposed to go directly to the device. Long discussion with Thomas on IMS topic. I will avoid diverting to unrelated discussion for now. > > Or proposed command in [1] should be tagged as siov_r1, then things will be > cleaner. > > Maybe we can say, one of the implementations is siov_r1, since it can be used > to implement devices in other transport. One of the main motivations for > transport virtqueue is to reduce the transport specific resources to ease the > virtio device implementation. > Yes, the main motivation as for backward compatibility for the currently defined features. > > > > With that I don't see legacy 3 commands anyway conflict with [1]. > > It doesn't, but it's not elegant since: > I don’t see how making 3 commands to 9 commands makes it elegant by doing bisecting of registers offset in hypervisor. Its same thing using more opcodes. > 1) Modern transport, admin virtqueue with well defined transport commands > 2) Legacy transport, works moe like a PCI transport on top of the admin > virtqueue > > Transport virtqueue tend to be transport independent, but 2) tie it somehow to > the PCI. That's why I suggest to > The 4 patches are not transporting all the PCI things over transport VQ. it is not a transport. It is mediation done for SIOV like how it's done for 2 patches. > 1) introduce legacy transport commands > > or > > 2) tweak your proposal to become a PCI over admin
[virtio-dev] RE: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
> From: Jason Wang > Sent: Thursday, May 11, 2023 3:05 AM > If we decide to use the admin vq, is there any good reason to tie it to PCI > if we > don't want to tunneling PCI over adminq? To truly tunnel PCI over adminq, please tunnel _all_ pci things over AQ, not just below 1 to 11. Doorbells, capabilities and more. Then it is qualifying as a transport. Otherwise, it is not a "virtio pci transport over aq". I neither see why one would tunnel whole PCI over some queue, which was not your intention either from the patches I see. VQ over fabrics is much cleaner design to transport VQ over non-PCI. People have tried to tunnel pci over some other transports and it only fine for non-production toys. > > Why not simply invent individual commands to access legacy facilities like > commands to access like what transport virtqueue did for modern > device?: > I don’t see how this is being any different than register-offset interface. It bisects more things at hypervisor level that makes things hard to add #12th entry. > 1) device features > 2) driver features > 3) queue address > 4) queue size > 5) queue select > 6) queue notify > 7) device status > 8) ISR status > 9) config msix > 10) queue msix > 11) device configuration space > > It focuses on the facilities instead of transport specific details like > registers (we > don't even need legacy registers in this case), I gives more deterministic > behavior so we don't need to care about the cross registers read/write. > 1.x has these registers at raw level and that seems fine. Anything new will come on the cfgq and hence no bisection or registers needed.
[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
On Thu, May 11, 2023 at 03:06:48PM +0800, Jason Wang wrote: > > With that I don't see legacy 3 commands anyway conflict with [1]. > > It doesn't, but it's not elegant since: > > 1) Modern transport, admin virtqueue with well defined transport commands > 2) Legacy transport, works moe like a PCI transport on top of the > admin virtqueue > > Transport virtqueue tend to be transport independent, but 2) tie it > somehow to the PCI. This "somehow" matters though. The connection is really just the common config layout. > That's why I suggest to > > 1) introduce legacy transport commands > > or > > 2) tweak your proposal to become a PCI over admin virtqueue So you are saying it's really not generally "legacy over admin queue" it is more like "legacy pci over admin queue"? Good point. And I feel it's a good point that this chooses legacy pci but it could really be mmio or ccw or the upcoming vq transport just as well. I suspect mmio or ccw won't work well for a physical pci card though. transport vq might, but I am worried we'll have to spend extra work clarifying it's a legacy interface. For example we will have to explain that only 32 bits of features must be used. Focusing more effort on transport vq is attractive though ... It is worth examining this option in more depth. Did you? Want to share how is transport vq closer than legacy pci to what we might want? > > Some commands functionality is common between [1] and this proposal. > > But that's how the legacy is. It is confined to legacy emulation. > > So [1] can be done as follow_on to AQ and these series. > > > > A small note about [2]. > > virtio_transportq_ctrl_dev_attribute should be detached from CREATE call > > and split to two commands. > > So that VF and SF/SIOV can both utilize it. > > SF/SIOV_R1 can use the creation and config part. > > VFs will use only the device features + config space. > > Good point. This could be fixed. > > Thanks > > > - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] RE: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
> From: Michael S. Tsirkin > Sent: Thursday, May 11, 2023 8:55 AM > > 1) device features > > 2) driver features > > 3) queue address > > 4) queue size > > 5) queue select > > 6) queue notify > > 7) device status > > 8) ISR status > > 9) config msix > > 10) queue msix > > 11) device configuration space > > #9 may not even go to the group owner device. What do we gain from bisecting it? Every new additional needs involvement of the hypervisor as Michael noted below on "effort" point. The register offset and read/write is far simpler interface for hypervisor. Not to do cross register access is what we need to define in the spec for 1.x or future things. > > It focuses on the facilities instead of transport specific details > > like registers (we don't even need legacy registers in this case), I > > gives more deterministic behavior so we don't need to care about the > > cross registers read/write. > > This needs thought, it is definitely more work. Effort that could be maybe > spent > on new features. What is the motivation here? supporting legacy mmio guests?
[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
On Thu, May 11, 2023 at 03:04:40PM +0800, Jason Wang wrote: > On Wed, May 10, 2023 at 3:43 PM Michael S. Tsirkin wrote: > > > > On Wed, May 10, 2023 at 03:01:25PM +0800, Jason Wang wrote: > > > On Wed, May 10, 2023 at 2:05 PM Michael S. Tsirkin > > > wrote: > > > > > > > > On Mon, May 08, 2023 at 10:23:39AM +0800, Jason Wang wrote: > > > > > > I thought so too originally. Unfortunately I now think that no, > > > > > > legacy is not > > > > > > going to be a byproduct of transport virtqueue for modern - > > > > > > it is different enough that it needs dedicated commands. > > > > > > > > > > If you mean the transport virtqueue, I think some dedicated commands > > > > > for legacy are needed. Then it would be a transport that supports > > > > > transitional devices. It would be much better than having commands for > > > > > a partial transport like this patch did. > > > > > > > > OK I am beginning to get what you are saying. So your criticism is > > > > this: what if device supports vq transport for modern, and we want to > > > > build a transitional device on top. how will that look. yes? > > > > > > Yes. I think it needs to be done through the transport virtqueue > > > otherwise the transport is not self-contained. > > > > I mean, any feature can be done over transport vq. > > > > But there is value in adding legacy commands to an otherwise > > modern device without reworking it completely to > > switch to a different transport. > > There's probably no need for a rework since legacy is not complicated. > More below. > > > > > > > > > A reasonable thing to include at least in the commit log. Parav? > > > > > > > > You are also asking what if the device uses transport vq, > > > > and we want transitional on top of that. > > > > It's a fair question but I don't exactly get why would > > > > this legacy support feature be wanted for the vq transport > > > > and not for other transports. > > > > > > Not sure I get the question, but all the existing transport support > > > legacy, if we want to have another, should the legacy support be a > > > must or not? > > > > This specific proposal is for tunneling legacy over admin vq. > > It can live alongside a normal modern VF, with hypervisor > > combining these to create a transitional device. > > Exactly, but what I meant here is > > If we decide to use the admin vq, is there any good reason to tie it > to PCI if we don't want to tunneling PCI over adminq? > > Why not simply invent individual commands to access legacy facilities > like commands to access like what transport virtqueue did for modern > device?: > > 1) device features > 2) driver features > 3) queue address > 4) queue size > 5) queue select > 6) queue notify > 7) device status > 8) ISR status > 9) config msix > 10) queue msix > 11) device configuration space > > It focuses on the facilities instead of transport specific details > like registers (we don't even need legacy registers in this case), I > gives more deterministic behavior so we don't need to care about the > cross registers read/write. This needs thought, it is definitely more work. Effort that could be maybe spent on new features. What is the motivation here? supporting legacy mmio guests? > > > > > > > > > > > > > > > > > > > > > > > > Consider simplest case, multibyte fields. Legacy needs multibyte > > > > > > write, > > > > > > modern does not even need multibyte read. > > > > > > > > > > I'm not sure I will get here, > > > > > > > > What does this mean? > > > > > > I meant I don't get what the issue if "modern does not even need > > > multibyte read". > > > > parse error again. reword? > > I meant we need two sets of command for legacy and modern. We can > choose not to expose multibyte reads for modern commands. > > > > > > > > > > > > since we can't expose admin vq to > > > > > guests, it means we need some software mediation. So if we just > > > > > implement what PCI allows us, then everything would be fine (even if > > > > > some method is not used). > > > > > > > > > > Thanks > > > > > > > > To repeat discussion on one of the previous versions, no it will not be > > > > fine because legacy virtio abuses pci in fundamentally broken ways. > > > > So yes you need a mediator on the host but even giving this > > > > mediator a chance to be robust on top of hardware > > > > means the hardware interface can not simply mirror legacy > > > > to hardware. > > > > > > > > For example, host mediator needs to trap writes into mac, > > > > buffer them and then send a 6 byte write. > > > > Now, pci actually does allow 6 byte writes, but legacy > > > > guests instead to 6 single byte writes for portability reasons. > > > > > > It's a known race condition, so PCI over adminq doesn't make it worse. > > > > it can however make it better - you can do a single 6 byte write command. > > > > It would be tricky to detect when a legacy guest has finished writing > to the mac. > > > > The mediator can just mirror what guests write
[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
On Thu, May 11, 2023 at 12:04 AM Parav Pandit wrote: > > > > On 5/9/2023 11:51 PM, Jason Wang wrote: > > On Tue, May 9, 2023 at 11:56 AM Parav Pandit wrote: > >> > >> SIOV does not require mediation. > >> Composition is optional like VFs. > > > > This is not what I read from SIOV spec. > > > SIOV R1 spec was defined by _a_ vendor and it is undergoing upgrade to > more practical use now. So we should let them finish the work. > That's fine. > It is hard to comment in this forum about it. Yes. > > >> > > Hence, the transport we built is to consider this in mind for the > coming future. > >>> > >>> For transport virtqueue, it's not specific to PCI. It could be used in a > >>> much > >>> broader use case. > >>> > >> May be. > >> More below. > >> > So if each VF has its own configq, or cmdq, it totally make sense to > me which is bootstrap interface to transport existing config space > interface. > The problem is: it is not backward compatible; Hence a device has no > way of when to support both or only new configq. > >>> > >>> Providing compatibility in software is much more simpler than inventing > >>> new > >>> hardware interfaces. Isn't it? (e.g if we want to provide compatibility > >>> for VF on > >>> a SIOV device). And inventing a new hardware interface for compatibility > >>> might > >>> not always work, it may break the advantages of the new hardware (like > >>> scalability). > >>> > >> What I proposed below is new hardware interface for new features. > >> Not for compatibility. > > > > I'm confused, the proposal is for legacy drives so it's for > > compatibility for sure. No? > This proposal is to get legacy devices to work over a PCI VF. > > a configq/cmdq discussion is for new features for PF, VF, and SF/SIOV > devices to work in _non_ backward compatible way, > because for new > features there is no such thing as backward compatibility between guest > driver and the device. Ok, so what command did configq/cmdq have? I thought it was similar to transport virtqueue, but it looks more like a control virtqueue here. > > > > >> Compatibility is coming at a cost which is not scaling. > >> If you attempt to scale, it breaks the future path forward to go without > >> mediation. > >> > > So eve growing these fields and optionally placement on configq > doesn't really help and device builder to build it efficiently > (without much predictability). > >>> > >>> Config queue is not the only choice, we have a lot of other choices (for > >>> example > >>> PASID may help to reduce the on-chip resources). > >>> > >> PASID is for process isolation security construct, it is not for > >> transportation. > > > > I meant with PASID you don't even need a BAR. > > > Not sure I fully understand, but my guess is, this is coming from > mediation thought process. Not actually. I meant, for example without PASID, for modern devices, the common cfg structure must be placed on a BAR through MMIO. But with the help of PASID, it could stay in the memory and the device can fetch it via DMA. The help to reduce the resources for registers. > > >> Same, SIOV and VFs do not prefer mediation going forward in the use cases > >> we come across. > > > > Unless you don't want to provide VF compatibility for SIOV. > I do not understand what compatibility is this between which two elements? > VF has its identify currently and SIOV will have its own identity in future. Ok, so it really depends on how to implement the transport for SIOV. I guess it won't use registers/BAR any more. If this is true, if we want to run a virtio-pci driver, it needs some mediation. It works like what you proposed here. Someday, SR-IOV or BAR transport will become "legacy", but we need a way to make the "legacy virtio-pci driver" work. > > >>> > >>> Just to be sure we're on the same page. The proposal of both you and mine > >>> are > >>> based on the adminq for PF not VF. The reason is obvious: > >>> adminq per VF won't work without PASID, since it would have security > >>> issues. > >>> > >> The current proposal for legacy should be seen as separate and not to > >> intermix with per VF based configuration queue. > >> > >> adminvq/config/control vq per VF and SIOV both devices will work without > >> PASID (unrelated to legacy). > >> Because they are like any other queue, such as txq, rxq, cvq. All of these > >> queues are non-mediated today for PF, VF devices. > >> (And so additional configq follows the natural model for config space > >> access). > > > > This is only true if: > > > > 1) you don't want to provide any backward compatibility for current > > PCI transport, > There is no concept of backward compatibility for new features. > There will be new driver anyway in the guest. > > yes, I don't see a point in mediating 1.x config space for existing > drivers as it is requires mediation. > > > this means you need to use new drivers in the guest > > 2) you don't want to do
[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
On Thu, May 11, 2023 at 5:08 AM Parav Pandit wrote: > > Hi Jason, Michel, > > > From: virtio-comm...@lists.oasis-open.org > open.org> On Behalf Of Parav Pandit > > Sent: Wednesday, May 10, 2023 1:34 PM > > > > > From: Michael S. Tsirkin > > > Sent: Wednesday, May 10, 2023 12:16 PM > > > > > > On Wed, May 10, 2023 at 12:11:50PM -0400, Parav Pandit wrote: > > > > > > > > > > > > On 5/10/2023 2:04 AM, Michael S. Tsirkin wrote: > > > > > On Mon, May 08, 2023 at 10:23:39AM +0800, Jason Wang wrote: > > > > > > > I thought so too originally. Unfortunately I now think that > > > > > > > no, legacy is not going to be a byproduct of transport > > > > > > > virtqueue for modern - it is different enough that it needs > > > > > > > dedicated > > commands. > > > > > > > > > > > > If you mean the transport virtqueue, I think some dedicated > > > > > > commands for legacy are needed. Then it would be a transport > > > > > > that supports transitional devices. It would be much better than > > > > > > having commands for a partial transport like this patch did. > > > > > > > > > > OK I am beginning to get what you are saying. So your criticism > > > > > is > > > > > this: what if device supports vq transport for modern, and we want > > > > > to build a transitional device on top. how will that look. yes? > > > > > A reasonable thing to include at least in the commit log. Parav? > > > > > > > > > I am still trying to understand what is "vq transport for modern"? > > > > Do you mean transporting currently defined config space access over vq? > > > > If so, is this VQ belong to the guest or hypervisor? > > > > > > https://lore.kernel.org/all/20220826100034.200432-2- > > > lingshan.zhu%40intel.com/t.mbox.gz > > > > The gz link is not accessible. > > But I got the right link [1]. > > > > [1] https://lore.kernel.org/all/20220826100034.200432-2- > > lingshan@intel.com/ > > > > 1. Above patch cover letter [1] is missing the basic objective/problem > > statement. > > i.e. why a transport virtqueue is needed? > > But I probably get the idea of [1] as we did the AQ. > > > > 2. Commit log says about > > a. querying resource of management device (aka group owner in AQ now) b. > > creating and destroying the managed device (aka group owner creating group > > member devices) c. configure the managed device (aka group owner > > configuring/composing group member devices such as VFs, SFs, SIOV). > > > > So, all above 2.a to 2.c belongs to the admin group owner and group > > management commands like how it is defined in the AQ proposal. Yes, the plan is to rebase on top of AQ. Lingshan may add more details here. > > > > So, 3 out of the 4 motivations are achieved by AQ proposal. > > This AQ belongs to the hypervisor. I am clear on this part. > > > > 4th point in cover letter is: "config virtqueues of the managed device". > > > > This work belongs to the driver -> device direct communication using a queue > > from driver to device. > > So, I imagine this work can be done using a queue by the guest driver and > > serviced by the device like how a guest driver configures the queue today > > without any mediation. > > For PCI, MMIO transport, surely this can be done by the PCI device directly > > being is PF, VF or SIOV. > > (Instead of config register, using a new queue interface). Looks fine to me. Great. > > > > Can this new cfg queue mediated like CVQ that is done in a sw? May be yes. > > Should it be always mediated when it is of VF, SIOV Device? Mostly no > > because > > it is yet another VQ for PF, VF, SIOV. Yes, the mediation is always conditional but not mandated when doing the design. > > > > I am yet to parse rest of the 4 patches, please give me some time to review > > it. > > I went over the past work of [1], [2]. > > [1] > https://lore.kernel.org/all/20220826100034.200432-2-lingshan@intel.com/ > [2] https://lists.oasis-open.org/archives/virtio-comment/202208/msg00141.html > > The "virtio q as transport" in [2] is bit misleading as its only role is to > transport the _registers_ of the SIOV_R1 device through its parent PF. > Rest of the work is the pure management work to manage the life cycle of SIOV > devices (create/delete/configure/compose). I think this can be tweaked for static provisioning setups like SR-IOV. E.g group owner can choose to not advertise the life cycle management commands. Then the hypervisor may know the provisioning is transport specific. > > And the motivation is also clear is to provide composing a virtio device for > the guest VM for the backward compatibility for 1.x part of the specification. > > It seems fine and indeed orthogonal to me that: it is for backward > compatibility for already defined config fields for existing guest VM driver. > > It does not conflict with the cfgq/cmdq idea whose main purpose is for the > new config fields, new use cases that doesn't require any mediation. > Such cfgq works across PF, VF, SF/SIOV devices in uniform way without > mediation.
[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
On Wed, May 10, 2023 at 3:43 PM Michael S. Tsirkin wrote: > > On Wed, May 10, 2023 at 03:01:25PM +0800, Jason Wang wrote: > > On Wed, May 10, 2023 at 2:05 PM Michael S. Tsirkin wrote: > > > > > > On Mon, May 08, 2023 at 10:23:39AM +0800, Jason Wang wrote: > > > > > I thought so too originally. Unfortunately I now think that no, > > > > > legacy is not > > > > > going to be a byproduct of transport virtqueue for modern - > > > > > it is different enough that it needs dedicated commands. > > > > > > > > If you mean the transport virtqueue, I think some dedicated commands > > > > for legacy are needed. Then it would be a transport that supports > > > > transitional devices. It would be much better than having commands for > > > > a partial transport like this patch did. > > > > > > OK I am beginning to get what you are saying. So your criticism is > > > this: what if device supports vq transport for modern, and we want to > > > build a transitional device on top. how will that look. yes? > > > > Yes. I think it needs to be done through the transport virtqueue > > otherwise the transport is not self-contained. > > I mean, any feature can be done over transport vq. > > But there is value in adding legacy commands to an otherwise > modern device without reworking it completely to > switch to a different transport. There's probably no need for a rework since legacy is not complicated. More below. > > > > > A reasonable thing to include at least in the commit log. Parav? > > > > > > You are also asking what if the device uses transport vq, > > > and we want transitional on top of that. > > > It's a fair question but I don't exactly get why would > > > this legacy support feature be wanted for the vq transport > > > and not for other transports. > > > > Not sure I get the question, but all the existing transport support > > legacy, if we want to have another, should the legacy support be a > > must or not? > > This specific proposal is for tunneling legacy over admin vq. > It can live alongside a normal modern VF, with hypervisor > combining these to create a transitional device. Exactly, but what I meant here is If we decide to use the admin vq, is there any good reason to tie it to PCI if we don't want to tunneling PCI over adminq? Why not simply invent individual commands to access legacy facilities like commands to access like what transport virtqueue did for modern device?: 1) device features 2) driver features 3) queue address 4) queue size 5) queue select 6) queue notify 7) device status 8) ISR status 9) config msix 10) queue msix 11) device configuration space It focuses on the facilities instead of transport specific details like registers (we don't even need legacy registers in this case), I gives more deterministic behavior so we don't need to care about the cross registers read/write. > > > > > > > > > > > > > > > > > > Consider simplest case, multibyte fields. Legacy needs multibyte > > > > > write, > > > > > modern does not even need multibyte read. > > > > > > > > I'm not sure I will get here, > > > > > > What does this mean? > > > > I meant I don't get what the issue if "modern does not even need > > multibyte read". > > parse error again. reword? I meant we need two sets of command for legacy and modern. We can choose not to expose multibyte reads for modern commands. > > > > > > > > since we can't expose admin vq to > > > > guests, it means we need some software mediation. So if we just > > > > implement what PCI allows us, then everything would be fine (even if > > > > some method is not used). > > > > > > > > Thanks > > > > > > To repeat discussion on one of the previous versions, no it will not be > > > fine because legacy virtio abuses pci in fundamentally broken ways. > > > So yes you need a mediator on the host but even giving this > > > mediator a chance to be robust on top of hardware > > > means the hardware interface can not simply mirror legacy > > > to hardware. > > > > > > For example, host mediator needs to trap writes into mac, > > > buffer them and then send a 6 byte write. > > > Now, pci actually does allow 6 byte writes, but legacy > > > guests instead to 6 single byte writes for portability reasons. > > > > It's a known race condition, so PCI over adminq doesn't make it worse. > > it can however make it better - you can do a single 6 byte write command. > It would be tricky to detect when a legacy guest has finished writing to the mac. > > The mediator can just mirror what guests write over the admin > > commands. > > and this "just" just isn't good enough, or we end up with hacks > in hardware. Yes but this "issue" exists in this proposal as well. Thanks > > > Thanks > > > > > -- > > > MSr > > > > - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] RE: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
> From: Michael S. Tsirkin > Sent: Wednesday, May 10, 2023 5:33 PM > > I think some kind of "enable" command for VFs might have value. No? Unlike the SIOV devices, VFs life cycle is under PF control, not sure how the enable command can help. After SR-IOV is enabled, VF is short of enabled, it may not be ready, but that is guarded by the device reset flow. - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
On Wed, May 10, 2023 at 09:08:44PM +, Parav Pandit wrote: > Hi Jason, Michel, > > > From: virtio-comm...@lists.oasis-open.org > open.org> On Behalf Of Parav Pandit > > Sent: Wednesday, May 10, 2023 1:34 PM > > > > > From: Michael S. Tsirkin > > > Sent: Wednesday, May 10, 2023 12:16 PM > > > > > > On Wed, May 10, 2023 at 12:11:50PM -0400, Parav Pandit wrote: > > > > > > > > > > > > On 5/10/2023 2:04 AM, Michael S. Tsirkin wrote: > > > > > On Mon, May 08, 2023 at 10:23:39AM +0800, Jason Wang wrote: > > > > > > > I thought so too originally. Unfortunately I now think that > > > > > > > no, legacy is not going to be a byproduct of transport > > > > > > > virtqueue for modern - it is different enough that it needs > > > > > > > dedicated > > commands. > > > > > > > > > > > > If you mean the transport virtqueue, I think some dedicated > > > > > > commands for legacy are needed. Then it would be a transport > > > > > > that supports transitional devices. It would be much better than > > > > > > having commands for a partial transport like this patch did. > > > > > > > > > > OK I am beginning to get what you are saying. So your criticism > > > > > is > > > > > this: what if device supports vq transport for modern, and we want > > > > > to build a transitional device on top. how will that look. yes? > > > > > A reasonable thing to include at least in the commit log. Parav? > > > > > > > > > I am still trying to understand what is "vq transport for modern"? > > > > Do you mean transporting currently defined config space access over vq? > > > > If so, is this VQ belong to the guest or hypervisor? > > > > > > https://lore.kernel.org/all/20220826100034.200432-2- > > > lingshan.zhu%40intel.com/t.mbox.gz > > > > The gz link is not accessible. > > But I got the right link [1]. > > > > [1] https://lore.kernel.org/all/20220826100034.200432-2- > > lingshan@intel.com/ > > > > 1. Above patch cover letter [1] is missing the basic objective/problem > > statement. > > i.e. why a transport virtqueue is needed? > > But I probably get the idea of [1] as we did the AQ. > > > > 2. Commit log says about > > a. querying resource of management device (aka group owner in AQ now) b. > > creating and destroying the managed device (aka group owner creating group > > member devices) c. configure the managed device (aka group owner > > configuring/composing group member devices such as VFs, SFs, SIOV). > > > > So, all above 2.a to 2.c belongs to the admin group owner and group > > management commands like how it is defined in the AQ proposal. > > > > So, 3 out of the 4 motivations are achieved by AQ proposal. > > This AQ belongs to the hypervisor. I am clear on this part. > > > > 4th point in cover letter is: "config virtqueues of the managed device". > > > > This work belongs to the driver -> device direct communication using a queue > > from driver to device. > > So, I imagine this work can be done using a queue by the guest driver and > > serviced by the device like how a guest driver configures the queue today > > without any mediation. > > For PCI, MMIO transport, surely this can be done by the PCI device directly > > being is PF, VF or SIOV. > > (Instead of config register, using a new queue interface). Looks fine to me. > > > > Can this new cfg queue mediated like CVQ that is done in a sw? May be yes. > > Should it be always mediated when it is of VF, SIOV Device? Mostly no > > because > > it is yet another VQ for PF, VF, SIOV. > > > > I am yet to parse rest of the 4 patches, please give me some time to review > > it. > > I went over the past work of [1], [2]. > > [1] > https://lore.kernel.org/all/20220826100034.200432-2-lingshan@intel.com/ > [2] https://lists.oasis-open.org/archives/virtio-comment/202208/msg00141.html > > The "virtio q as transport" in [2] is bit misleading as its only role is to > transport the _registers_ of the SIOV_R1 device through its parent PF. > Rest of the work is the pure management work to manage the life cycle of SIOV > devices (create/delete/configure/compose). > > And the motivation is also clear is to provide composing a virtio device for > the guest VM for the backward compatibility for 1.x part of the specification. > > It seems fine and indeed orthogonal to me that: it is for backward > compatibility for already defined config fields for existing guest VM driver. > > It does not conflict with the cfgq/cmdq idea whose main purpose is for the > new config fields, new use cases that doesn't require any mediation. > Such cfgq works across PF, VF, SF/SIOV devices in uniform way without > mediation. > It also scales device register memory also very well in predictable way. > > The registers and feature bits access described in [2], can certainly be done > using new non_legacy new AQ commands. > Both legacy and non-legacy command have different semantics as Michael > mentioned. > > The AQ semantics that Michael did as opposed to "virtqueue as transport" fits
[virtio-dev] RE: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
Hi Jason, Michel, > From: virtio-comm...@lists.oasis-open.org open.org> On Behalf Of Parav Pandit > Sent: Wednesday, May 10, 2023 1:34 PM > > > From: Michael S. Tsirkin > > Sent: Wednesday, May 10, 2023 12:16 PM > > > > On Wed, May 10, 2023 at 12:11:50PM -0400, Parav Pandit wrote: > > > > > > > > > On 5/10/2023 2:04 AM, Michael S. Tsirkin wrote: > > > > On Mon, May 08, 2023 at 10:23:39AM +0800, Jason Wang wrote: > > > > > > I thought so too originally. Unfortunately I now think that > > > > > > no, legacy is not going to be a byproduct of transport > > > > > > virtqueue for modern - it is different enough that it needs > > > > > > dedicated > commands. > > > > > > > > > > If you mean the transport virtqueue, I think some dedicated > > > > > commands for legacy are needed. Then it would be a transport > > > > > that supports transitional devices. It would be much better than > > > > > having commands for a partial transport like this patch did. > > > > > > > > OK I am beginning to get what you are saying. So your criticism > > > > is > > > > this: what if device supports vq transport for modern, and we want > > > > to build a transitional device on top. how will that look. yes? > > > > A reasonable thing to include at least in the commit log. Parav? > > > > > > > I am still trying to understand what is "vq transport for modern"? > > > Do you mean transporting currently defined config space access over vq? > > > If so, is this VQ belong to the guest or hypervisor? > > > > https://lore.kernel.org/all/20220826100034.200432-2- > > lingshan.zhu%40intel.com/t.mbox.gz > > The gz link is not accessible. > But I got the right link [1]. > > [1] https://lore.kernel.org/all/20220826100034.200432-2- > lingshan@intel.com/ > > 1. Above patch cover letter [1] is missing the basic objective/problem > statement. > i.e. why a transport virtqueue is needed? > But I probably get the idea of [1] as we did the AQ. > > 2. Commit log says about > a. querying resource of management device (aka group owner in AQ now) b. > creating and destroying the managed device (aka group owner creating group > member devices) c. configure the managed device (aka group owner > configuring/composing group member devices such as VFs, SFs, SIOV). > > So, all above 2.a to 2.c belongs to the admin group owner and group > management commands like how it is defined in the AQ proposal. > > So, 3 out of the 4 motivations are achieved by AQ proposal. > This AQ belongs to the hypervisor. I am clear on this part. > > 4th point in cover letter is: "config virtqueues of the managed device". > > This work belongs to the driver -> device direct communication using a queue > from driver to device. > So, I imagine this work can be done using a queue by the guest driver and > serviced by the device like how a guest driver configures the queue today > without any mediation. > For PCI, MMIO transport, surely this can be done by the PCI device directly > being is PF, VF or SIOV. > (Instead of config register, using a new queue interface). Looks fine to me. > > Can this new cfg queue mediated like CVQ that is done in a sw? May be yes. > Should it be always mediated when it is of VF, SIOV Device? Mostly no because > it is yet another VQ for PF, VF, SIOV. > > I am yet to parse rest of the 4 patches, please give me some time to review > it. I went over the past work of [1], [2]. [1] https://lore.kernel.org/all/20220826100034.200432-2-lingshan@intel.com/ [2] https://lists.oasis-open.org/archives/virtio-comment/202208/msg00141.html The "virtio q as transport" in [2] is bit misleading as its only role is to transport the _registers_ of the SIOV_R1 device through its parent PF. Rest of the work is the pure management work to manage the life cycle of SIOV devices (create/delete/configure/compose). And the motivation is also clear is to provide composing a virtio device for the guest VM for the backward compatibility for 1.x part of the specification. It seems fine and indeed orthogonal to me that: it is for backward compatibility for already defined config fields for existing guest VM driver. It does not conflict with the cfgq/cmdq idea whose main purpose is for the new config fields, new use cases that doesn't require any mediation. Such cfgq works across PF, VF, SF/SIOV devices in uniform way without mediation. It also scales device register memory also very well in predictable way. The registers and feature bits access described in [2], can certainly be done using new non_legacy new AQ commands. Both legacy and non-legacy command have different semantics as Michael mentioned. The AQ semantics that Michael did as opposed to "virtqueue as transport" fits well for the use case described in [1]. There are changes on going in MSI-X area and SIOV, so you might want to wait for it. Or proposed command in [1] should be tagged as siov_r1, then things will be cleaner. With that I don't see legacy 3 commands anyway conflict with [1]. Some
[virtio-dev] RE: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
> From: Michael S. Tsirkin > Sent: Wednesday, May 10, 2023 12:16 PM > > On Wed, May 10, 2023 at 12:11:50PM -0400, Parav Pandit wrote: > > > > > > On 5/10/2023 2:04 AM, Michael S. Tsirkin wrote: > > > On Mon, May 08, 2023 at 10:23:39AM +0800, Jason Wang wrote: > > > > > I thought so too originally. Unfortunately I now think that no, > > > > > legacy is not going to be a byproduct of transport virtqueue for > > > > > modern - it is different enough that it needs dedicated commands. > > > > > > > > If you mean the transport virtqueue, I think some dedicated > > > > commands for legacy are needed. Then it would be a transport that > > > > supports transitional devices. It would be much better than having > > > > commands for a partial transport like this patch did. > > > > > > OK I am beginning to get what you are saying. So your criticism is > > > this: what if device supports vq transport for modern, and we want > > > to build a transitional device on top. how will that look. yes? > > > A reasonable thing to include at least in the commit log. Parav? > > > > > I am still trying to understand what is "vq transport for modern"? > > Do you mean transporting currently defined config space access over vq? > > If so, is this VQ belong to the guest or hypervisor? > > https://lore.kernel.org/all/20220826100034.200432-2- > lingshan.zhu%40intel.com/t.mbox.gz The gz link is not accessible. But I got the right link [1]. [1] https://lore.kernel.org/all/20220826100034.200432-2-lingshan@intel.com/ 1. Above patch cover letter [1] is missing the basic objective/problem statement. i.e. why a transport virtqueue is needed? But I probably get the idea of [1] as we did the AQ. 2. Commit log says about a. querying resource of management device (aka group owner in AQ now) b. creating and destroying the managed device (aka group owner creating group member devices) c. configure the managed device (aka group owner configuring/composing group member devices such as VFs, SFs, SIOV). So, all above 2.a to 2.c belongs to the admin group owner and group management commands like how it is defined in the AQ proposal. So, 3 out of the 4 motivations are achieved by AQ proposal. This AQ belongs to the hypervisor. I am clear on this part. 4th point in cover letter is: "config virtqueues of the managed device". This work belongs to the driver -> device direct communication using a queue from driver to device. So, I imagine this work can be done using a queue by the guest driver and serviced by the device like how a guest driver configures the queue today without any mediation. For PCI, MMIO transport, surely this can be done by the PCI device directly being is PF, VF or SIOV. (Instead of config register, using a new queue interface). Looks fine to me. Can this new cfg queue mediated like CVQ that is done in a sw? May be yes. Should it be always mediated when it is of VF, SIOV Device? Mostly no because it is yet another VQ for PF, VF, SIOV. I am yet to parse rest of the 4 patches, please give me some time to review it. - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
On Wed, May 10, 2023 at 12:11:50PM -0400, Parav Pandit wrote: > > > On 5/10/2023 2:04 AM, Michael S. Tsirkin wrote: > > On Mon, May 08, 2023 at 10:23:39AM +0800, Jason Wang wrote: > > > > I thought so too originally. Unfortunately I now think that no, legacy > > > > is not > > > > going to be a byproduct of transport virtqueue for modern - > > > > it is different enough that it needs dedicated commands. > > > > > > If you mean the transport virtqueue, I think some dedicated commands > > > for legacy are needed. Then it would be a transport that supports > > > transitional devices. It would be much better than having commands for > > > a partial transport like this patch did. > > > > OK I am beginning to get what you are saying. So your criticism is > > this: what if device supports vq transport for modern, and we want to > > build a transitional device on top. how will that look. yes? > > A reasonable thing to include at least in the commit log. Parav? > > > I am still trying to understand what is "vq transport for modern"? > Do you mean transporting currently defined config space access over vq? > If so, is this VQ belong to the guest or hypervisor? https://lore.kernel.org/all/20220826100034.200432-2-lingshan.zhu%40intel.com/t.mbox.gz -- MST - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
On 5/10/2023 3:43 AM, Michael S. Tsirkin wrote: On Wed, May 10, 2023 at 03:01:25PM +0800, Jason Wang wrote: On Wed, May 10, 2023 at 2:05 PM Michael S. Tsirkin wrote: On Mon, May 08, 2023 at 10:23:39AM +0800, Jason Wang wrote: I thought so too originally. Unfortunately I now think that no, legacy is not going to be a byproduct of transport virtqueue for modern - it is different enough that it needs dedicated commands. If you mean the transport virtqueue, I think some dedicated commands for legacy are needed. Then it would be a transport that supports transitional devices. It would be much better than having commands for a partial transport like this patch did. OK I am beginning to get what you are saying. So your criticism is this: what if device supports vq transport for modern, and we want to build a transitional device on top. how will that look. yes? Yes. I think it needs to be done through the transport virtqueue otherwise the transport is not self-contained. I mean, any feature can be done over transport vq. But there is value in adding legacy commands to an otherwise modern device without reworking it completely to switch to a different transport. Yes. A reasonable thing to include at least in the commit log. Parav? You are also asking what if the device uses transport vq, and we want transitional on top of that. It's a fair question but I don't exactly get why would this legacy support feature be wanted for the vq transport and not for other transports. Not sure I get the question, but all the existing transport support legacy, if we want to have another, should the legacy support be a must or not? This specific proposal is for tunneling legacy over admin vq. It can live alongside a normal modern VF, with hypervisor combining these to create a transitional device. True. - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
On 5/10/2023 2:04 AM, Michael S. Tsirkin wrote: On Mon, May 08, 2023 at 10:23:39AM +0800, Jason Wang wrote: I thought so too originally. Unfortunately I now think that no, legacy is not going to be a byproduct of transport virtqueue for modern - it is different enough that it needs dedicated commands. If you mean the transport virtqueue, I think some dedicated commands for legacy are needed. Then it would be a transport that supports transitional devices. It would be much better than having commands for a partial transport like this patch did. OK I am beginning to get what you are saying. So your criticism is this: what if device supports vq transport for modern, and we want to build a transitional device on top. how will that look. yes? A reasonable thing to include at least in the commit log. Parav? I am still trying to understand what is "vq transport for modern"? Do you mean transporting currently defined config space access over vq? If so, is this VQ belong to the guest or hypervisor? - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
On 5/10/2023 12:22 AM, Jason Wang wrote: On Wed, May 10, 2023 at 11:51 AM Jason Wang wrote: Just to make sure we are at the same page. 1) if the hardware has configq and we need to make it work for current virtio-pci driver, hypervisor needs to trap guest PCI access and translate it to configq command. This means the hypervisor needs to hide the configq from guests. In this case the configq needs a dedicated DMA address which is what PASID can help. 2) if the hardware can report the device states, unless we want to migrate L2 guest, hypervisor should hide it from L1, so PASID is required to isolate the DMA for guest traffic and device state. A configq negotiates + discovers new fields for the PCI PF, VF, SF/SIOV devices over PCIe or other transports. So no need to hide/mediate for hw based devices, like cvq and like data vqs. For vdpa kind of use case it can work like cvq mediation, which I believe is happening without PASID today. - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
On Wed, May 10, 2023 at 03:01:25PM +0800, Jason Wang wrote: > On Wed, May 10, 2023 at 2:05 PM Michael S. Tsirkin wrote: > > > > On Mon, May 08, 2023 at 10:23:39AM +0800, Jason Wang wrote: > > > > I thought so too originally. Unfortunately I now think that no, legacy > > > > is not > > > > going to be a byproduct of transport virtqueue for modern - > > > > it is different enough that it needs dedicated commands. > > > > > > If you mean the transport virtqueue, I think some dedicated commands > > > for legacy are needed. Then it would be a transport that supports > > > transitional devices. It would be much better than having commands for > > > a partial transport like this patch did. > > > > OK I am beginning to get what you are saying. So your criticism is > > this: what if device supports vq transport for modern, and we want to > > build a transitional device on top. how will that look. yes? > > Yes. I think it needs to be done through the transport virtqueue > otherwise the transport is not self-contained. I mean, any feature can be done over transport vq. But there is value in adding legacy commands to an otherwise modern device without reworking it completely to switch to a different transport. > > A reasonable thing to include at least in the commit log. Parav? > > > > You are also asking what if the device uses transport vq, > > and we want transitional on top of that. > > It's a fair question but I don't exactly get why would > > this legacy support feature be wanted for the vq transport > > and not for other transports. > > Not sure I get the question, but all the existing transport support > legacy, if we want to have another, should the legacy support be a > must or not? This specific proposal is for tunneling legacy over admin vq. It can live alongside a normal modern VF, with hypervisor combining these to create a transitional device. > > > > > > > > > > > > Consider simplest case, multibyte fields. Legacy needs multibyte write, > > > > modern does not even need multibyte read. > > > > > > I'm not sure I will get here, > > > > What does this mean? > > I meant I don't get what the issue if "modern does not even need > multibyte read". parse error again. reword? > > > > > since we can't expose admin vq to > > > guests, it means we need some software mediation. So if we just > > > implement what PCI allows us, then everything would be fine (even if > > > some method is not used). > > > > > > Thanks > > > > To repeat discussion on one of the previous versions, no it will not be > > fine because legacy virtio abuses pci in fundamentally broken ways. > > So yes you need a mediator on the host but even giving this > > mediator a chance to be robust on top of hardware > > means the hardware interface can not simply mirror legacy > > to hardware. > > > > For example, host mediator needs to trap writes into mac, > > buffer them and then send a 6 byte write. > > Now, pci actually does allow 6 byte writes, but legacy > > guests instead to 6 single byte writes for portability reasons. > > It's a known race condition, so PCI over adminq doesn't make it worse. it can however make it better - you can do a single 6 byte write command. > The mediator can just mirror what guests write over the admin > commands. and this "just" just isn't good enough, or we end up with hacks in hardware. > Thanks > > > -- > > MSr > > - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
On Wed, May 10, 2023 at 2:05 PM Michael S. Tsirkin wrote: > > On Mon, May 08, 2023 at 10:23:39AM +0800, Jason Wang wrote: > > > I thought so too originally. Unfortunately I now think that no, legacy is > > > not > > > going to be a byproduct of transport virtqueue for modern - > > > it is different enough that it needs dedicated commands. > > > > If you mean the transport virtqueue, I think some dedicated commands > > for legacy are needed. Then it would be a transport that supports > > transitional devices. It would be much better than having commands for > > a partial transport like this patch did. > > OK I am beginning to get what you are saying. So your criticism is > this: what if device supports vq transport for modern, and we want to > build a transitional device on top. how will that look. yes? Yes. I think it needs to be done through the transport virtqueue otherwise the transport is not self-contained. > A reasonable thing to include at least in the commit log. Parav? > > You are also asking what if the device uses transport vq, > and we want transitional on top of that. > It's a fair question but I don't exactly get why would > this legacy support feature be wanted for the vq transport > and not for other transports. Not sure I get the question, but all the existing transport support legacy, if we want to have another, should the legacy support be a must or not? > > > > > > > Consider simplest case, multibyte fields. Legacy needs multibyte write, > > > modern does not even need multibyte read. > > > > I'm not sure I will get here, > > What does this mean? I meant I don't get what the issue if "modern does not even need multibyte read". > > > since we can't expose admin vq to > > guests, it means we need some software mediation. So if we just > > implement what PCI allows us, then everything would be fine (even if > > some method is not used). > > > > Thanks > > To repeat discussion on one of the previous versions, no it will not be > fine because legacy virtio abuses pci in fundamentally broken ways. > So yes you need a mediator on the host but even giving this > mediator a chance to be robust on top of hardware > means the hardware interface can not simply mirror legacy > to hardware. > > For example, host mediator needs to trap writes into mac, > buffer them and then send a 6 byte write. > Now, pci actually does allow 6 byte writes, but legacy > guests instead to 6 single byte writes for portability reasons. It's a known race condition, so PCI over adminq doesn't make it worse. The mediator can just mirror what guests write over the admin commands. Thanks > -- > MSr > - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
On Mon, May 08, 2023 at 10:23:39AM +0800, Jason Wang wrote: > > I thought so too originally. Unfortunately I now think that no, legacy is > > not > > going to be a byproduct of transport virtqueue for modern - > > it is different enough that it needs dedicated commands. > > If you mean the transport virtqueue, I think some dedicated commands > for legacy are needed. Then it would be a transport that supports > transitional devices. It would be much better than having commands for > a partial transport like this patch did. OK I am beginning to get what you are saying. So your criticism is this: what if device supports vq transport for modern, and we want to build a transitional device on top. how will that look. yes? A reasonable thing to include at least in the commit log. Parav? You are also asking what if the device uses transport vq, and we want transitional on top of that. It's a fair question but I don't exactly get why would this legacy support feature be wanted for the vq transport and not for other transports. > > Consider simplest case, multibyte fields. Legacy needs multibyte write, > > modern does not even need multibyte read. > > I'm not sure I will get here, What does this mean? > since we can't expose admin vq to > guests, it means we need some software mediation. So if we just > implement what PCI allows us, then everything would be fine (even if > some method is not used). > > Thanks To repeat discussion on one of the previous versions, no it will not be fine because legacy virtio abuses pci in fundamentally broken ways. So yes you need a mediator on the host but even giving this mediator a chance to be robust on top of hardware means the hardware interface can not simply mirror legacy to hardware. For example, host mediator needs to trap writes into mac, buffer them and then send a 6 byte write. Now, pci actually does allow 6 byte writes, but legacy guests instead to 6 single byte writes for portability reasons. -- MSr - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
On Wed, May 10, 2023 at 11:51 AM Jason Wang wrote: > > On Tue, May 9, 2023 at 11:56 AM Parav Pandit wrote: > > > > > From: Jason Wang > > > Sent: Monday, May 8, 2023 11:45 PM > > > > > > The fundamental reason for not accessing the 1.x VF and SIOV device > > > > registers, config space, feature bits through PF is: it requires PF > > > > device mediation. VF and SIOV devices are first class citizen in PCIe > > > > spec and deserve direct interaction with the device. > > > > > > Unless I miss something obvious, SIOV requires mediation (or > > > composition) for sure. Otherwise you break the compatibility. > > > > > SIOV does not require mediation. > > Composition is optional like VFs. > > This is not what I read from SIOV spec. > > > > > > > > > > > Hence, the transport we built is to consider this in mind for the > > > > coming future. > > > > > > For transport virtqueue, it's not specific to PCI. It could be used in a > > > much > > > broader use case. > > > > > May be. > > More below. > > > > > > So if each VF has its own configq, or cmdq, it totally make sense to > > > > me which is bootstrap interface to transport existing config space > > > > interface. > > > > The problem is: it is not backward compatible; Hence a device has no > > > > way of when to support both or only new configq. > > > > > > Providing compatibility in software is much more simpler than inventing > > > new > > > hardware interfaces. Isn't it? (e.g if we want to provide compatibility > > > for VF on > > > a SIOV device). And inventing a new hardware interface for compatibility > > > might > > > not always work, it may break the advantages of the new hardware (like > > > scalability). > > > > > What I proposed below is new hardware interface for new features. > > Not for compatibility. > > I'm confused, the proposal is for legacy drives so it's for > compatibility for sure. No? > > > Compatibility is coming at a cost which is not scaling. > > If you attempt to scale, it breaks the future path forward to go without > > mediation. > > > > > > > > > > So eve growing these fields and optionally placement on configq > > > > doesn't really help and device builder to build it efficiently > > > > (without much predictability). > > > > > > Config queue is not the only choice, we have a lot of other choices (for > > > example > > > PASID may help to reduce the on-chip resources). > > > > > PASID is for process isolation security construct, it is not for > > transportation. > > I meant with PASID you don't even need a BAR. > > > > > > > > > > > Instead of we say, that what exists today in config space stays in > > > > config space, anything additional on new q, than its deterministic > > > > behavior to size up the scale. > > > > > > Just to be clear, if we have PCI over adminq, VF's config space could be > > > the > > > minimal one for PCI spec complaint. The real config space is accessed via > > > the > > > admin virtqueue. > > > > > Yeah I understood what you are saying. We don’t see a path forward with > > such mediation. > > VF and PF to have same level of direct access to the device. > > > > > > > > > > For example, a PCI device who wants to support 100 VFs, can easily > > > > size its memory to 30 bytes * 100 reserved for supporting config space. > > > > > > Those capabilities (30 bytes) can be accessed via admin virtqueue. So we > > > don't > > > need to place them in the config space. > > > > > Same, SIOV and VFs do not prefer mediation going forward in the use cases > > we come across. > > Unless you don't want to provide VF compatibility for SIOV. > > > > > > > And new 40 bytes * 100 fields doesn't have to be in the resident memory. > > > > > > > > If we have optional configq/cmdq for transport, than 30*100 bytes are > > > > used (reserved) as 3000/(30+40) = 42 VFs. > > > > > > > > Only if some VFs use configq, more VFs can be deployed. > > > > > > I don't understand here. > > > > > Lets first clarify the non-mediated passthrough model of the VF first than > > come to above scale example. > > > > > > It is hard to > > > > build scale this way. Therefore suggestion is to place new attributes > > > > on new config/cmd/transport q, and old to stay as-is. > > > > > > Just to be sure we're on the same page. The proposal of both you and mine > > > are > > > based on the adminq for PF not VF. The reason is obvious: > > > adminq per VF won't work without PASID, since it would have security > > > issues. > > > > > The current proposal for legacy should be seen as separate and not to > > intermix with per VF based configuration queue. > > > > adminvq/config/control vq per VF and SIOV both devices will work without > > PASID (unrelated to legacy). > > Because they are like any other queue, such as txq, rxq, cvq. All of these > > queues are non-mediated today for PF, VF devices. > > (And so additional configq follows the natural model for config space > > access). > > This is only true if: > > 1) you don't want to provide
[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
On Tue, May 9, 2023 at 11:56 AM Parav Pandit wrote: > > > From: Jason Wang > > Sent: Monday, May 8, 2023 11:45 PM > > > > The fundamental reason for not accessing the 1.x VF and SIOV device > > > registers, config space, feature bits through PF is: it requires PF > > > device mediation. VF and SIOV devices are first class citizen in PCIe > > > spec and deserve direct interaction with the device. > > > > Unless I miss something obvious, SIOV requires mediation (or > > composition) for sure. Otherwise you break the compatibility. > > > SIOV does not require mediation. > Composition is optional like VFs. This is not what I read from SIOV spec. > > > > > > > Hence, the transport we built is to consider this in mind for the > > > coming future. > > > > For transport virtqueue, it's not specific to PCI. It could be used in a > > much > > broader use case. > > > May be. > More below. > > > > So if each VF has its own configq, or cmdq, it totally make sense to > > > me which is bootstrap interface to transport existing config space > > > interface. > > > The problem is: it is not backward compatible; Hence a device has no > > > way of when to support both or only new configq. > > > > Providing compatibility in software is much more simpler than inventing new > > hardware interfaces. Isn't it? (e.g if we want to provide compatibility for > > VF on > > a SIOV device). And inventing a new hardware interface for compatibility > > might > > not always work, it may break the advantages of the new hardware (like > > scalability). > > > What I proposed below is new hardware interface for new features. > Not for compatibility. I'm confused, the proposal is for legacy drives so it's for compatibility for sure. No? > Compatibility is coming at a cost which is not scaling. > If you attempt to scale, it breaks the future path forward to go without > mediation. > > > > > > > So eve growing these fields and optionally placement on configq > > > doesn't really help and device builder to build it efficiently > > > (without much predictability). > > > > Config queue is not the only choice, we have a lot of other choices (for > > example > > PASID may help to reduce the on-chip resources). > > > PASID is for process isolation security construct, it is not for > transportation. I meant with PASID you don't even need a BAR. > > > > > > > Instead of we say, that what exists today in config space stays in > > > config space, anything additional on new q, than its deterministic > > > behavior to size up the scale. > > > > Just to be clear, if we have PCI over adminq, VF's config space could be the > > minimal one for PCI spec complaint. The real config space is accessed via > > the > > admin virtqueue. > > > Yeah I understood what you are saying. We don’t see a path forward with such > mediation. > VF and PF to have same level of direct access to the device. > > > > > > > For example, a PCI device who wants to support 100 VFs, can easily > > > size its memory to 30 bytes * 100 reserved for supporting config space. > > > > Those capabilities (30 bytes) can be accessed via admin virtqueue. So we > > don't > > need to place them in the config space. > > > Same, SIOV and VFs do not prefer mediation going forward in the use cases we > come across. Unless you don't want to provide VF compatibility for SIOV. > > > > And new 40 bytes * 100 fields doesn't have to be in the resident memory. > > > > > > If we have optional configq/cmdq for transport, than 30*100 bytes are > > > used (reserved) as 3000/(30+40) = 42 VFs. > > > > > > Only if some VFs use configq, more VFs can be deployed. > > > > I don't understand here. > > > Lets first clarify the non-mediated passthrough model of the VF first than > come to above scale example. > > > > It is hard to > > > build scale this way. Therefore suggestion is to place new attributes > > > on new config/cmd/transport q, and old to stay as-is. > > > > Just to be sure we're on the same page. The proposal of both you and mine > > are > > based on the adminq for PF not VF. The reason is obvious: > > adminq per VF won't work without PASID, since it would have security issues. > > > The current proposal for legacy should be seen as separate and not to > intermix with per VF based configuration queue. > > adminvq/config/control vq per VF and SIOV both devices will work without > PASID (unrelated to legacy). > Because they are like any other queue, such as txq, rxq, cvq. All of these > queues are non-mediated today for PF, VF devices. > (And so additional configq follows the natural model for config space access). This is only true if: 1) you don't want to provide any backward compatibility for current PCI transport, this means you need to use new drivers in the guest 2) you don't want to do live migration If you need one of the above, PASID is a must. Thanks - To unsubscribe, e-mail:
[virtio-dev] RE: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
> From: Jason Wang > Sent: Monday, May 8, 2023 11:45 PM > > The fundamental reason for not accessing the 1.x VF and SIOV device > > registers, config space, feature bits through PF is: it requires PF > > device mediation. VF and SIOV devices are first class citizen in PCIe > > spec and deserve direct interaction with the device. > > Unless I miss something obvious, SIOV requires mediation (or > composition) for sure. Otherwise you break the compatibility. > SIOV does not require mediation. Composition is optional like VFs. > > > > Hence, the transport we built is to consider this in mind for the > > coming future. > > For transport virtqueue, it's not specific to PCI. It could be used in a much > broader use case. > May be. More below. > > So if each VF has its own configq, or cmdq, it totally make sense to > > me which is bootstrap interface to transport existing config space > > interface. > > The problem is: it is not backward compatible; Hence a device has no > > way of when to support both or only new configq. > > Providing compatibility in software is much more simpler than inventing new > hardware interfaces. Isn't it? (e.g if we want to provide compatibility for > VF on > a SIOV device). And inventing a new hardware interface for compatibility might > not always work, it may break the advantages of the new hardware (like > scalability). > What I proposed below is new hardware interface for new features. Not for compatibility. Compatibility is coming at a cost which is not scaling. If you attempt to scale, it breaks the future path forward to go without mediation. > > > > So eve growing these fields and optionally placement on configq > > doesn't really help and device builder to build it efficiently > > (without much predictability). > > Config queue is not the only choice, we have a lot of other choices (for > example > PASID may help to reduce the on-chip resources). > PASID is for process isolation security construct, it is not for transportation. > > > > Instead of we say, that what exists today in config space stays in > > config space, anything additional on new q, than its deterministic > > behavior to size up the scale. > > Just to be clear, if we have PCI over adminq, VF's config space could be the > minimal one for PCI spec complaint. The real config space is accessed via the > admin virtqueue. > Yeah I understood what you are saying. We don’t see a path forward with such mediation. VF and PF to have same level of direct access to the device. > > > > For example, a PCI device who wants to support 100 VFs, can easily > > size its memory to 30 bytes * 100 reserved for supporting config space. > > Those capabilities (30 bytes) can be accessed via admin virtqueue. So we don't > need to place them in the config space. > Same, SIOV and VFs do not prefer mediation going forward in the use cases we come across. > > And new 40 bytes * 100 fields doesn't have to be in the resident memory. > > > > If we have optional configq/cmdq for transport, than 30*100 bytes are > > used (reserved) as 3000/(30+40) = 42 VFs. > > > > Only if some VFs use configq, more VFs can be deployed. > > I don't understand here. > Lets first clarify the non-mediated passthrough model of the VF first than come to above scale example. > > It is hard to > > build scale this way. Therefore suggestion is to place new attributes > > on new config/cmd/transport q, and old to stay as-is. > > Just to be sure we're on the same page. The proposal of both you and mine are > based on the adminq for PF not VF. The reason is obvious: > adminq per VF won't work without PASID, since it would have security issues. > The current proposal for legacy should be seen as separate and not to intermix with per VF based configuration queue. adminvq/config/control vq per VF and SIOV both devices will work without PASID (unrelated to legacy). Because they are like any other queue, such as txq, rxq, cvq. All of these queues are non-mediated today for PF, VF devices. (And so additional configq follows the natural model for config space access).
[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
On Tue, May 9, 2023 at 1:08 AM Parav Pandit wrote: > > > > On 5/7/2023 10:23 PM, Jason Wang wrote: > > On Sun, May 7, 2023 at 9:44 PM Michael S. Tsirkin wrote: > >> > >> On Sat, May 06, 2023 at 10:31:30AM +0800, Jason Wang wrote: > >>> On Sat, May 6, 2023 at 8:02 AM Parav Pandit wrote: > > This short series introduces legacy registers access commands for the > owner > group member PCI PF to access the legacy registers of the member VFs. > > If in future any SIOV devices to support legacy registers, they > can be easily supported using same commands by using the group > member identifiers of the future SIOV devices. > > More details as overview, motivation, use case are further described > below. > > Patch summary: > -- > patch-1 adds administrative virtuqueue commands > patch-2 adds its conformance section > > This short series is on top of latest work [1] from Michael. > It uses the newly introduced administrative virtqueue facility with 3 new > commands which uses the existing virtio_admin_cmd. > > [1] > https://lists.oasis-open.org/archives/virtio-comment/202305/msg00112.html > > Usecase: > > 1. A hypervisor/system needs to provide transitional > virtio devices to the guest VM at scale of thousands, > typically, one to eight devices per VM. > > 2. A hypervisor/system needs to provide such devices using a > vendor agnostic driver in the hypervisor system. > > 3. A hypervisor system prefers to have single stack regardless of > virtio device type (net/blk) and be future compatible with a > single vfio stack using SR-IOV or other scalable device > virtualization technology to map PCI devices to the guest VM. > (as transitional or otherwise) > > Motivation/Background: > -- > The existing virtio transitional PCI device is missing support for > PCI SR-IOV based devices. Currently it does not work beyond > PCI PF, or as software emulated device in reality. Currently it > has below cited system level limitations: > > [a] PCIe spec citation: > VFs do not support I/O Space and thus VF BARs shall not indicate I/O > Space. > > [b] cpu arch citiation: > Intel 64 and IA-32 Architectures Software Developer’s Manual: > The processor’s I/O address space is separate and distinct from > the physical-memory address space. The I/O address space consists > of 64K individually addressable 8-bit I/O ports, numbered 0 through > H. > > [c] PCIe spec citation: > If a bridge implements an I/O address range,...I/O address range will be > aligned to a 4 KB boundary. > > Overview: > - > Above usecase requirements can be solved by PCI PF group owner accessing > its group member PCI VFs legacy registers using an admin virtqueue of > the group owner PCI PF. > > Two new admin virtqueue commands are added which read/write PCI VF > registers. > > The third command suggested by Jason queries the VF device's driver > notification region. > > Software usage example: > --- > One way to use and map to the guest VM is by using vfio driver > framework in Linux kernel. > > +--+ > |pci_dev_id = 0x100X | > +---|pci_rev_id = 0x0 |-+ > |vfio device|BAR0 = I/O region | | > | |Other attributes | | > | +--+ | > || > + +--+ +-+ | > | |I/O BAR to AQ | | Other vfio | | > | |rd/wr mapper | | functionalities | | > | +--+ +-+ | > || > +--+-+---+ > | | > +++ +++ > | +-+ | | PCI VF device A | > | | AQ |-+>+-+ | > | +-+ | | | | legacy regs | | > | PCI PF device | | | +-+ | > +-+ | +-+ > | > | +++ > | | PCI VF device N | > +>+-+ | > | | legacy regs | | > | +-+ | > +-+ > > 2. Virtio pci driver to bind to the listed device id and >
[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
On 5/7/2023 10:23 PM, Jason Wang wrote: On Sun, May 7, 2023 at 9:44 PM Michael S. Tsirkin wrote: On Sat, May 06, 2023 at 10:31:30AM +0800, Jason Wang wrote: On Sat, May 6, 2023 at 8:02 AM Parav Pandit wrote: This short series introduces legacy registers access commands for the owner group member PCI PF to access the legacy registers of the member VFs. If in future any SIOV devices to support legacy registers, they can be easily supported using same commands by using the group member identifiers of the future SIOV devices. More details as overview, motivation, use case are further described below. Patch summary: -- patch-1 adds administrative virtuqueue commands patch-2 adds its conformance section This short series is on top of latest work [1] from Michael. It uses the newly introduced administrative virtqueue facility with 3 new commands which uses the existing virtio_admin_cmd. [1] https://lists.oasis-open.org/archives/virtio-comment/202305/msg00112.html Usecase: 1. A hypervisor/system needs to provide transitional virtio devices to the guest VM at scale of thousands, typically, one to eight devices per VM. 2. A hypervisor/system needs to provide such devices using a vendor agnostic driver in the hypervisor system. 3. A hypervisor system prefers to have single stack regardless of virtio device type (net/blk) and be future compatible with a single vfio stack using SR-IOV or other scalable device virtualization technology to map PCI devices to the guest VM. (as transitional or otherwise) Motivation/Background: -- The existing virtio transitional PCI device is missing support for PCI SR-IOV based devices. Currently it does not work beyond PCI PF, or as software emulated device in reality. Currently it has below cited system level limitations: [a] PCIe spec citation: VFs do not support I/O Space and thus VF BARs shall not indicate I/O Space. [b] cpu arch citiation: Intel 64 and IA-32 Architectures Software Developer’s Manual: The processor’s I/O address space is separate and distinct from the physical-memory address space. The I/O address space consists of 64K individually addressable 8-bit I/O ports, numbered 0 through H. [c] PCIe spec citation: If a bridge implements an I/O address range,...I/O address range will be aligned to a 4 KB boundary. Overview: - Above usecase requirements can be solved by PCI PF group owner accessing its group member PCI VFs legacy registers using an admin virtqueue of the group owner PCI PF. Two new admin virtqueue commands are added which read/write PCI VF registers. The third command suggested by Jason queries the VF device's driver notification region. Software usage example: --- One way to use and map to the guest VM is by using vfio driver framework in Linux kernel. +--+ |pci_dev_id = 0x100X | +---|pci_rev_id = 0x0 |-+ |vfio device|BAR0 = I/O region | | | |Other attributes | | | +--+ | || + +--+ +-+ | | |I/O BAR to AQ | | Other vfio | | | |rd/wr mapper | | functionalities | | | +--+ +-+ | || +--+-+---+ | | +++ +++ | +-+ | | PCI VF device A | | | AQ |-+>+-+ | | +-+ | | | | legacy regs | | | PCI PF device | | | +-+ | +-+ | +-+ | | +++ | | PCI VF device N | +>+-+ | | | legacy regs | | | +-+ | +-+ 2. Virtio pci driver to bind to the listed device id and use it as native device in the host. 3. Use it in a light weight hypervisor to run bare-metal OS. Please review. Fixes: https://github.com/oasis-tcs/virtio-spec/issues/167 Signed-off-by: Parav Pandit --- changelog: v1->v2: - addressed comments from Michael - added theory of operation - grammar corrections - removed group fields description from individual commands as it is already present in generic section - added endianness normative for legacy device registers region - renamed the file to drop vf and add legacy prefix - added overview in commit log - renamed subsection to reflect command So as replied in V1, I think it's not a good idea to invent commands for a partial transport just for legacy devices. It's better either: 1) rebase or collaborate this work on top of the transport
[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
On 5/7/2023 5:04 AM, Michael S. Tsirkin wrote:
one things I still don't see addressed here is support for
legacy interrupts. legacy driver can disable msix and
interrupts will be then sent.
how about a special command that is used when device would
normally send INT#x? it can also return ISR to reduce latency.
I am not sure if this is a real issue. Because even the legacy guests
have msix enabled by default. In theory yes, it can fall back to intx.
There are few options.
1. A hypervisor driver can be conservative and steal an msix of the VF
for transporting intx.
Pros: Does not need special things in device
Cons:
a. Fairly intrusive in hypervisor vf driver.
b. May not be ever used as guest is unlikely to fail on msix
2. Since multiple VFs intx to be serviced, one command per VF in AQ is
too much overhead that device needs to map a request to,
A better way is to have an eventq of depth = num_vfs, like many other
virtio devices have it.
An eventq can hold per VF interrupt entry including the isr value that
you suggest above.
Something like,
union eventq_entry {
u8 raw_data[16];
struct intx_entry {
u8 event_opcode;
u8 group_type;
u8 reserved[6];
le64 group_identifier;
u8 isr_status;
};
};
This eventq resides on the owner parent PF.
isr_status is read on clear like today.
May be such eventq can be useful in future for wider case.
We may have to find a different name for it as other devices has device
specific eventq.
I am inclined to differ this to a later point if one can identify the
real failure with msix for the guest VM.
So far we don't see this ever happening.
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
On Sun, May 7, 2023 at 9:44 PM Michael S. Tsirkin wrote: > > On Sat, May 06, 2023 at 10:31:30AM +0800, Jason Wang wrote: > > On Sat, May 6, 2023 at 8:02 AM Parav Pandit wrote: > > > > > > This short series introduces legacy registers access commands for the > > > owner > > > group member PCI PF to access the legacy registers of the member VFs. > > > > > > If in future any SIOV devices to support legacy registers, they > > > can be easily supported using same commands by using the group > > > member identifiers of the future SIOV devices. > > > > > > More details as overview, motivation, use case are further described > > > below. > > > > > > Patch summary: > > > -- > > > patch-1 adds administrative virtuqueue commands > > > patch-2 adds its conformance section > > > > > > This short series is on top of latest work [1] from Michael. > > > It uses the newly introduced administrative virtqueue facility with 3 new > > > commands which uses the existing virtio_admin_cmd. > > > > > > [1] > > > https://lists.oasis-open.org/archives/virtio-comment/202305/msg00112.html > > > > > > Usecase: > > > > > > 1. A hypervisor/system needs to provide transitional > > >virtio devices to the guest VM at scale of thousands, > > >typically, one to eight devices per VM. > > > > > > 2. A hypervisor/system needs to provide such devices using a > > >vendor agnostic driver in the hypervisor system. > > > > > > 3. A hypervisor system prefers to have single stack regardless of > > >virtio device type (net/blk) and be future compatible with a > > >single vfio stack using SR-IOV or other scalable device > > >virtualization technology to map PCI devices to the guest VM. > > >(as transitional or otherwise) > > > > > > Motivation/Background: > > > -- > > > The existing virtio transitional PCI device is missing support for > > > PCI SR-IOV based devices. Currently it does not work beyond > > > PCI PF, or as software emulated device in reality. Currently it > > > has below cited system level limitations: > > > > > > [a] PCIe spec citation: > > > VFs do not support I/O Space and thus VF BARs shall not indicate I/O > > > Space. > > > > > > [b] cpu arch citiation: > > > Intel 64 and IA-32 Architectures Software Developer’s Manual: > > > The processor’s I/O address space is separate and distinct from > > > the physical-memory address space. The I/O address space consists > > > of 64K individually addressable 8-bit I/O ports, numbered 0 through H. > > > > > > [c] PCIe spec citation: > > > If a bridge implements an I/O address range,...I/O address range will be > > > aligned to a 4 KB boundary. > > > > > > Overview: > > > - > > > Above usecase requirements can be solved by PCI PF group owner accessing > > > its group member PCI VFs legacy registers using an admin virtqueue of > > > the group owner PCI PF. > > > > > > Two new admin virtqueue commands are added which read/write PCI VF > > > registers. > > > > > > The third command suggested by Jason queries the VF device's driver > > > notification region. > > > > > > Software usage example: > > > --- > > > One way to use and map to the guest VM is by using vfio driver > > > framework in Linux kernel. > > > > > > +--+ > > > |pci_dev_id = 0x100X | > > > +---|pci_rev_id = 0x0 |-+ > > > |vfio device|BAR0 = I/O region | | > > > | |Other attributes | | > > > | +--+ | > > > || > > > + +--+ +-+ | > > > | |I/O BAR to AQ | | Other vfio | | > > > | |rd/wr mapper | | functionalities | | > > > | +--+ +-+ | > > > || > > > +--+-+---+ > > >| | > > > +++ +++ > > > | +-+ | | PCI VF device A | > > > | | AQ |-+>+-+ | > > > | +-+ | | | | legacy regs | | > > > | PCI PF device | | | +-+ | > > > +-+ | +-+ > > > | > > > | +++ > > > | | PCI VF device N | > > > +>+-+ | > > > | | legacy regs | | > > > | +-+ | > > > +-+ > > > > > > 2. Virtio pci driver to bind to the listed device id and > > >use it as native device in the host. > > > > > > 3. Use it in a light weight hypervisor to run bare-metal OS. > > > > > > Please review. > > > > > > Fixes: https://github.com/oasis-tcs/virtio-spec/issues/167 > > > Signed-off-by: Parav Pandit >
[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
On Sat, May 06, 2023 at 10:31:30AM +0800, Jason Wang wrote: > On Sat, May 6, 2023 at 8:02 AM Parav Pandit wrote: > > > > This short series introduces legacy registers access commands for the owner > > group member PCI PF to access the legacy registers of the member VFs. > > > > If in future any SIOV devices to support legacy registers, they > > can be easily supported using same commands by using the group > > member identifiers of the future SIOV devices. > > > > More details as overview, motivation, use case are further described > > below. > > > > Patch summary: > > -- > > patch-1 adds administrative virtuqueue commands > > patch-2 adds its conformance section > > > > This short series is on top of latest work [1] from Michael. > > It uses the newly introduced administrative virtqueue facility with 3 new > > commands which uses the existing virtio_admin_cmd. > > > > [1] > > https://lists.oasis-open.org/archives/virtio-comment/202305/msg00112.html > > > > Usecase: > > > > 1. A hypervisor/system needs to provide transitional > >virtio devices to the guest VM at scale of thousands, > >typically, one to eight devices per VM. > > > > 2. A hypervisor/system needs to provide such devices using a > >vendor agnostic driver in the hypervisor system. > > > > 3. A hypervisor system prefers to have single stack regardless of > >virtio device type (net/blk) and be future compatible with a > >single vfio stack using SR-IOV or other scalable device > >virtualization technology to map PCI devices to the guest VM. > >(as transitional or otherwise) > > > > Motivation/Background: > > -- > > The existing virtio transitional PCI device is missing support for > > PCI SR-IOV based devices. Currently it does not work beyond > > PCI PF, or as software emulated device in reality. Currently it > > has below cited system level limitations: > > > > [a] PCIe spec citation: > > VFs do not support I/O Space and thus VF BARs shall not indicate I/O Space. > > > > [b] cpu arch citiation: > > Intel 64 and IA-32 Architectures Software Developer’s Manual: > > The processor’s I/O address space is separate and distinct from > > the physical-memory address space. The I/O address space consists > > of 64K individually addressable 8-bit I/O ports, numbered 0 through H. > > > > [c] PCIe spec citation: > > If a bridge implements an I/O address range,...I/O address range will be > > aligned to a 4 KB boundary. > > > > Overview: > > - > > Above usecase requirements can be solved by PCI PF group owner accessing > > its group member PCI VFs legacy registers using an admin virtqueue of > > the group owner PCI PF. > > > > Two new admin virtqueue commands are added which read/write PCI VF > > registers. > > > > The third command suggested by Jason queries the VF device's driver > > notification region. > > > > Software usage example: > > --- > > One way to use and map to the guest VM is by using vfio driver > > framework in Linux kernel. > > > > +--+ > > |pci_dev_id = 0x100X | > > +---|pci_rev_id = 0x0 |-+ > > |vfio device|BAR0 = I/O region | | > > | |Other attributes | | > > | +--+ | > > || > > + +--+ +-+ | > > | |I/O BAR to AQ | | Other vfio | | > > | |rd/wr mapper | | functionalities | | > > | +--+ +-+ | > > || > > +--+-+---+ > >| | > > +++ +++ > > | +-+ | | PCI VF device A | > > | | AQ |-+>+-+ | > > | +-+ | | | | legacy regs | | > > | PCI PF device | | | +-+ | > > +-+ | +-+ > > | > > | +++ > > | | PCI VF device N | > > +>+-+ | > > | | legacy regs | | > > | +-+ | > > +-+ > > > > 2. Virtio pci driver to bind to the listed device id and > >use it as native device in the host. > > > > 3. Use it in a light weight hypervisor to run bare-metal OS. > > > > Please review. > > > > Fixes: https://github.com/oasis-tcs/virtio-spec/issues/167 > > Signed-off-by: Parav Pandit > > > > --- > > changelog: > > v1->v2: > > - addressed comments from Michael > > - added theory of operation > > - grammar corrections > > - removed group fields description from individual commands as > > it is already present in generic section > > - added endianness normative for legacy device
[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
On Sat, May 06, 2023 at 03:01:33AM +0300, Parav Pandit wrote: > This short series introduces legacy registers access commands for the owner > group member PCI PF to access the legacy registers of the member VFs. > > If in future any SIOV devices to support legacy registers, they > can be easily supported using same commands by using the group > member identifiers of the future SIOV devices. > > More details as overview, motivation, use case are further described > below. > > Patch summary: > -- > patch-1 adds administrative virtuqueue commands > patch-2 adds its conformance section > > This short series is on top of latest work [1] from Michael. > It uses the newly introduced administrative virtqueue facility with 3 new > commands which uses the existing virtio_admin_cmd. > > [1] https://lists.oasis-open.org/archives/virtio-comment/202305/msg00112.html > > Usecase: > > 1. A hypervisor/system needs to provide transitional >virtio devices to the guest VM at scale of thousands, >typically, one to eight devices per VM. > > 2. A hypervisor/system needs to provide such devices using a >vendor agnostic driver in the hypervisor system. > > 3. A hypervisor system prefers to have single stack regardless of >virtio device type (net/blk) and be future compatible with a >single vfio stack using SR-IOV or other scalable device >virtualization technology to map PCI devices to the guest VM. >(as transitional or otherwise) > > Motivation/Background: > -- > The existing virtio transitional PCI device is missing support for > PCI SR-IOV based devices. Currently it does not work beyond > PCI PF, or as software emulated device in reality. Currently it > has below cited system level limitations: > > [a] PCIe spec citation: > VFs do not support I/O Space and thus VF BARs shall not indicate I/O Space. > > [b] cpu arch citiation: > Intel 64 and IA-32 Architectures Software Developer’s Manual: > The processor’s I/O address space is separate and distinct from > the physical-memory address space. The I/O address space consists > of 64K individually addressable 8-bit I/O ports, numbered 0 through H. > > [c] PCIe spec citation: > If a bridge implements an I/O address range,...I/O address range will be > aligned to a 4 KB boundary. > > Overview: > - > Above usecase requirements can be solved by PCI PF group owner accessing > its group member PCI VFs legacy registers using an admin virtqueue of > the group owner PCI PF. > > Two new admin virtqueue commands are added which read/write PCI VF > registers. > > The third command suggested by Jason queries the VF device's driver > notification region. > > Software usage example: > --- > One way to use and map to the guest VM is by using vfio driver > framework in Linux kernel. > > +--+ > |pci_dev_id = 0x100X | > +---|pci_rev_id = 0x0 |-+ > |vfio device|BAR0 = I/O region | | > | |Other attributes | | > | +--+ | > || > + +--+ +-+ | > | |I/O BAR to AQ | | Other vfio | | > | |rd/wr mapper | | functionalities | | > | +--+ +-+ | > || > +--+-+---+ >| | > +++ +++ > | +-+ | | PCI VF device A | > | | AQ |-+>+-+ | > | +-+ | | | | legacy regs | | > | PCI PF device | | | +-+ | > +-+ | +-+ > | > | +++ > | | PCI VF device N | > +>+-+ | > | | legacy regs | | > | +-+ | > +-+ > > 2. Virtio pci driver to bind to the listed device id and >use it as native device in the host. > > 3. Use it in a light weight hypervisor to run bare-metal OS. > > Please review. > > Fixes: https://github.com/oasis-tcs/virtio-spec/issues/167 > Signed-off-by: Parav Pandit > > --- > changelog: > v1->v2: > - addressed comments from Michael > - added theory of operation > - grammar corrections > - removed group fields description from individual commands as > it is already present in generic section > - added endianness normative for legacy device registers region > - renamed the file to drop vf and add legacy prefix > - added overview in commit log > - renamed subsection to reflect command one things I still don't see addressed here is support for legacy interrupts. legacy driver can disable msix and interrupts will be then
[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
On Sat, May 6, 2023 at 8:02 AM Parav Pandit wrote: > > This short series introduces legacy registers access commands for the owner > group member PCI PF to access the legacy registers of the member VFs. > > If in future any SIOV devices to support legacy registers, they > can be easily supported using same commands by using the group > member identifiers of the future SIOV devices. > > More details as overview, motivation, use case are further described > below. > > Patch summary: > -- > patch-1 adds administrative virtuqueue commands > patch-2 adds its conformance section > > This short series is on top of latest work [1] from Michael. > It uses the newly introduced administrative virtqueue facility with 3 new > commands which uses the existing virtio_admin_cmd. > > [1] https://lists.oasis-open.org/archives/virtio-comment/202305/msg00112.html > > Usecase: > > 1. A hypervisor/system needs to provide transitional >virtio devices to the guest VM at scale of thousands, >typically, one to eight devices per VM. > > 2. A hypervisor/system needs to provide such devices using a >vendor agnostic driver in the hypervisor system. > > 3. A hypervisor system prefers to have single stack regardless of >virtio device type (net/blk) and be future compatible with a >single vfio stack using SR-IOV or other scalable device >virtualization technology to map PCI devices to the guest VM. >(as transitional or otherwise) > > Motivation/Background: > -- > The existing virtio transitional PCI device is missing support for > PCI SR-IOV based devices. Currently it does not work beyond > PCI PF, or as software emulated device in reality. Currently it > has below cited system level limitations: > > [a] PCIe spec citation: > VFs do not support I/O Space and thus VF BARs shall not indicate I/O Space. > > [b] cpu arch citiation: > Intel 64 and IA-32 Architectures Software Developer’s Manual: > The processor’s I/O address space is separate and distinct from > the physical-memory address space. The I/O address space consists > of 64K individually addressable 8-bit I/O ports, numbered 0 through H. > > [c] PCIe spec citation: > If a bridge implements an I/O address range,...I/O address range will be > aligned to a 4 KB boundary. > > Overview: > - > Above usecase requirements can be solved by PCI PF group owner accessing > its group member PCI VFs legacy registers using an admin virtqueue of > the group owner PCI PF. > > Two new admin virtqueue commands are added which read/write PCI VF > registers. > > The third command suggested by Jason queries the VF device's driver > notification region. > > Software usage example: > --- > One way to use and map to the guest VM is by using vfio driver > framework in Linux kernel. > > +--+ > |pci_dev_id = 0x100X | > +---|pci_rev_id = 0x0 |-+ > |vfio device|BAR0 = I/O region | | > | |Other attributes | | > | +--+ | > || > + +--+ +-+ | > | |I/O BAR to AQ | | Other vfio | | > | |rd/wr mapper | | functionalities | | > | +--+ +-+ | > || > +--+-+---+ >| | > +++ +++ > | +-+ | | PCI VF device A | > | | AQ |-+>+-+ | > | +-+ | | | | legacy regs | | > | PCI PF device | | | +-+ | > +-+ | +-+ > | > | +++ > | | PCI VF device N | > +>+-+ | > | | legacy regs | | > | +-+ | > +-+ > > 2. Virtio pci driver to bind to the listed device id and >use it as native device in the host. > > 3. Use it in a light weight hypervisor to run bare-metal OS. > > Please review. > > Fixes: https://github.com/oasis-tcs/virtio-spec/issues/167 > Signed-off-by: Parav Pandit > > --- > changelog: > v1->v2: > - addressed comments from Michael > - added theory of operation > - grammar corrections > - removed group fields description from individual commands as > it is already present in generic section > - added endianness normative for legacy device registers region > - renamed the file to drop vf and add legacy prefix > - added overview in commit log > - renamed subsection to reflect command So as replied in V1, I think it's not a good idea to invent commands for a partial transport just for legacy devices. It's better either: 1) rebase or collaborate
