[virtio-dev] Re: [virtio-comment] [PATCH v9 0/4] admin: Access legacy registers using admin commands
On Thu, Jul 06, 2023 at 04:20:03AM +, Parav Pandit wrote: > > > > From: Michael S. Tsirkin > > Sent: Wednesday, July 5, 2023 7:09 AM > > > OK I sent a bunch of comments. Pls try to address, unfortunately I didn't > > go in > > depth over the last patch, but I feel it should be squashed into the generic > > legacy command sections avoiding need for indirection, so rewriting it now > > wouldn't be efficient. > > I address all your comments and of Cornelia. > PCI part I still prefer to be separate so kept it. > > Thanks a lot. I clarified some questions you asked about. Maybe v11 is justified in view of my answers. Let me know before I review again. -- 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: [virtio-comment] [PATCH v9 0/4] admin: Access legacy registers using admin commands
> From: Michael S. Tsirkin > Sent: Wednesday, July 5, 2023 7:09 AM > OK I sent a bunch of comments. Pls try to address, unfortunately I didn't go > in > depth over the last patch, but I feel it should be squashed into the generic > legacy command sections avoiding need for indirection, so rewriting it now > wouldn't be efficient. I address all your comments and of Cornelia. PCI part I still prefer to be separate so kept it. Thanks a lot.
[virtio-dev] RE: [virtio-comment] [PATCH v9 0/4] admin: Access legacy registers using admin commands
> From: Cornelia Huck > Sent: Wednesday, July 5, 2023 9:43 AM > > >> From: Michael S. Tsirkin > >> Sent: Wednesday, July 5, 2023 7:09 AM > >> > >> OK I sent a bunch of comments. Pls try to address, unfortunately I > >> didn't go in depth over the last patch, but I feel it should be > >> squashed into the generic legacy command sections avoiding need for > >> indirection, so rewriting it now wouldn't be efficient. > > > > I relooked at well yday. > > Transport has very small section. > > This aligns to how the existing spec is written, for example vq notify data > > is > explained in generic way and defined in pci way in PCI chapter. > > The indirection is fine and also self-contained similarly here. > > Would it make sense to do this the other way around (i.e. use something that > is > PCI-only from the start?) > > IIUC, the "legacy" problem as it stands is something specific to PCI, with > MMIO > using two distinct approaches for pre-1.0 and 1.0+, and CCW simply using > different commands (and no need for any kind of cross > access.) Not 100% sure about MMIO, but I don't think we need a version of this > for CCW. V4 or v5 was having everything in PCI and Michael asked to write as generic as possible. so I moved large part as generic. And kept pci part in pci. - 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: [virtio-comment] [PATCH v9 0/4] admin: Access legacy registers using admin commands
On Wed, Jul 05 2023, Parav Pandit wrote: >> From: Michael S. Tsirkin >> Sent: Wednesday, July 5, 2023 7:09 AM >> >> OK I sent a bunch of comments. Pls try to address, unfortunately I didn't go >> in >> depth over the last patch, but I feel it should be squashed into the generic >> legacy command sections avoiding need for indirection, so rewriting it now >> wouldn't be efficient. > > I relooked at well yday. > Transport has very small section. > This aligns to how the existing spec is written, for example vq notify data > is explained in generic way and defined in pci way in PCI chapter. > The indirection is fine and also self-contained similarly here. Would it make sense to do this the other way around (i.e. use something that is PCI-only from the start?) IIUC, the "legacy" problem as it stands is something specific to PCI, with MMIO using two distinct approaches for pre-1.0 and 1.0+, and CCW simply using different commands (and no need for any kind of cross access.) Not 100% sure about MMIO, but I don't think we need a version of this for CCW. - 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: [virtio-comment] [PATCH v9 0/4] admin: Access legacy registers using admin commands
> From: Michael S. Tsirkin > Sent: Wednesday, July 5, 2023 7:09 AM > > OK I sent a bunch of comments. Pls try to address, unfortunately I didn't go > in > depth over the last patch, but I feel it should be squashed into the generic > legacy command sections avoiding need for indirection, so rewriting it now > wouldn't be efficient. I relooked at well yday. Transport has very small section. This aligns to how the existing spec is written, for example vq notify data is explained in generic way and defined in pci way in PCI chapter. The indirection is fine and also self-contained similarly here.
[virtio-dev] Re: [virtio-comment] [PATCH v9 0/4] admin: Access legacy registers using admin commands
On Wed, Jul 05, 2023 at 02:37:20AM +0300, Parav Pandit wrote: > This short series introduces legacy registers access commands for the owner > group member access the legacy registers of the member VFs. > This short series introduces legacy region access commands by the group owner > device for its member devices. > Currently it is applicable to the PCI PF and VF devices. 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. OK I sent a bunch of comments. Pls try to address, unfortunately I didn't go in depth over the last patch, but I feel it should be squashed into the generic legacy command sections avoiding need for indirection, so rewriting it now wouldn't be efficient. Thanks! > Patch summary: > -- > patch-1 split rows of admin opcode tables by a line > patch-2 fix section numbering > patch-3 add generic legacy region access commands > patch-4 add pci specific definition > > It uses the newly introduced administration command facility with 4 new > commands and a new optional command to query the legacy notification region. > > 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 is 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. > > 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 | | > | +--+ +-+ | > || > +--+-+---+ >| | >Legacy regionDriver notification > access | >| | > +++ +++ > | +-+ | | 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 in the host. > > 3. Use it in a light weight hypervisor to run bare-metal OS. > > Please review. > > Alternatives considered: > > 1. Exposing BAR0 as MMIO BAR that follows legacy registers template > Pros: > a. Kind of works with legacy drivers as some of them have used API >which is agnostic to MMIO vs IOBAR. > b. Does not
