Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ

["Michael S. Tsirkin" <mst@redhat.com>]

On Sat, May 06, 2023 at 10:31:30AM +0800, Jason Wang wrote:
> On Sat, May 6, 2023 at 8:02âAM Parav Pandit <parav@nvidia.com> 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 FFFFH.
> >
> > [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 <parav@nvidia.com>
> >
> > ---
> > 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 virtqueue
> 
> or
> 
> 2) having a PCI over admin virtqueue transport, since this proposal
> has already had BAR access, we can add config space access then it is
> self-contained so we don't need to go through every corner case like
> inventing dedicated commands to accessing some function that is
> duplicated with capabilities. It will become yet another transport and
> legacy support is just a good byproduct.
> 
> Thanks


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.
Consider simplest case, multibyte fields. Legacy needs multibyte write,
modern does not even need multibyte read.

> >
> > v0->v1:
> > - addressed comments, suggesetions and ideas from Michael Tsirkin and Jason Wang
> > - far more simpler design than MMR access
> > - removed complexities of MMR device ids
> > - removed complexities of MMR registers and extended capabilities
> > - dropped adding new extended capabilities because if if they are
> >   added, a pci device still needs to have existing capabilities
> >   in the legacy configuration space and hypervisor driver do not
> >   need to access them
> >
> >
> > Parav Pandit (2):
> >   transport-pci: Introduce legacy registers access commands
> >   transport-pci: Add legacy register access conformance section
> >
> >  admin.tex                     |   5 +-
> >  conformance.tex               |   2 +
> >  transport-pci-legacy-regs.tex | 133 ++++++++++++++++++++++++++++++++++
> >  transport-pci.tex             |   2 +
> >  4 files changed, 141 insertions(+), 1 deletion(-)
> >  create mode 100644 transport-pci-legacy-regs.tex
> >
> > --
> > 2.26.2
> >