public inbox for virtio-dev@lists.linux.dev
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Jason Wang <jasowang@redhat.com>
Cc: Parav Pandit <parav@nvidia.com>,
	virtio-dev@lists.oasis-open.org, cohuck@redhat.com,
	david.edmondson@oracle.com, sburla@marvell.com,
	yishaih@nvidia.com, maorg@nvidia.com,
	virtio-comment@lists.oasis-open.org, shahafs@nvidia.com
Subject: [virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ
Date: Sun, 7 May 2023 09:44:14 -0400	[thread overview]
Message-ID: <20230507093959-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <CACGkMEt_2B68a+BZ4-vfeDf_eYj5ZhxrDNd6dxuiF85qr0yN2g@mail.gmail.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
> >


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


  reply	other threads:[~2023-05-07 13:44 UTC|newest]

Thread overview: 126+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-06  0:01 [virtio-dev] [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ Parav Pandit
2023-05-06  0:01 ` [virtio-dev] [PATCH v2 1/2] transport-pci: Introduce legacy registers access commands Parav Pandit
2023-05-17  5:44   ` [virtio-dev] " Michael S. Tsirkin
2023-05-17 19:32     ` [virtio-dev] " Parav Pandit
2023-05-18 19:42       ` [virtio-dev] Re: [virtio-comment] " Michael S. Tsirkin
2023-05-18 20:51         ` [virtio-dev] " Parav Pandit
2023-05-19  1:54         ` [virtio-dev] " Jason Wang
2023-05-19  2:04           ` [virtio-dev] " Parav Pandit
2023-05-19  6:06         ` [virtio-dev] " Michael S. Tsirkin
2023-05-19 16:37           ` [virtio-dev] " Parav Pandit
2023-05-21  9:16             ` [virtio-dev] " Michael S. Tsirkin
2023-05-21 13:21               ` [virtio-dev] " Parav Pandit
2023-05-21 14:33                 ` [virtio-dev] " Michael S. Tsirkin
2023-05-21 14:44                   ` [virtio-dev] " Parav Pandit
2023-05-22 20:07                     ` [virtio-dev] " Michael S. Tsirkin
2023-05-22 21:05                       ` [virtio-dev] " Parav Pandit
2023-05-22 21:34                         ` [virtio-dev] " Michael S. Tsirkin
2023-05-23 17:13                           ` [virtio-dev] " Parav Pandit
2023-05-23 18:48                             ` [virtio-dev] " Michael S. Tsirkin
2023-05-23 22:22                               ` [virtio-dev] " Parav Pandit
2023-05-24  1:17                                 ` [virtio-dev] " Jason Wang
2023-05-24 10:07                                 ` Michael S. Tsirkin
2023-05-24 19:18                                   ` [virtio-dev] " Parav Pandit
2023-05-24 20:12                                     ` [virtio-dev] " Michael S. Tsirkin
2023-05-24 21:02                                       ` [virtio-dev] " Parav Pandit
2023-05-22 21:42                         ` [virtio-dev] " Michael S. Tsirkin
2023-05-22  0:54                   ` Jason Wang
2023-05-22  2:46                     ` [virtio-dev] " Parav Pandit
2023-05-22 19:35                     ` [virtio-dev] " Michael S. Tsirkin
2023-05-06  0:01 ` [virtio-dev] [PATCH v2 2/2] transport-pci: Add legacy register access conformance section Parav Pandit
2023-05-06  2:31 ` [virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ Jason Wang
2023-05-07 13:44   ` Michael S. Tsirkin [this message]
2023-05-08  2:23     ` Jason Wang
2023-05-08 17:07       ` Parav Pandit
2023-05-09  3:44         ` Jason Wang
2023-05-09  3:56           ` [virtio-dev] " Parav Pandit
2023-05-10  3:51             ` [virtio-dev] " Jason Wang
2023-05-10  4:22               ` Jason Wang
2023-05-10 16:07                 ` Parav Pandit
2023-05-11  7:20                   ` [virtio-dev] Re: [virtio-comment] " Jason Wang
2023-05-11 11:35                     ` Michael S. Tsirkin
2023-05-15  5:08                       ` Jason Wang
2023-05-15 15:25                     ` [virtio-dev] " Parav Pandit
2023-05-10 16:04               ` [virtio-dev] " Parav Pandit
2023-05-11  7:17                 ` Jason Wang
2023-05-11 14:31                   ` [virtio-dev] " Parav Pandit
2023-05-15  5:12                     ` [virtio-dev] Re: [virtio-comment] " Jason Wang
2023-05-15 15:26                       ` [virtio-dev] " Parav Pandit
2023-05-10  6:04       ` [virtio-dev] " Michael S. Tsirkin
2023-05-10  7:01         ` Jason Wang
2023-05-10  7:43           ` Michael S. Tsirkin
2023-05-10 16:13             ` Parav Pandit
2023-05-11  7:04             ` Jason Wang
2023-05-11 12:54               ` Michael S. Tsirkin
2023-05-11 13:02                 ` [virtio-dev] " Parav Pandit
2023-05-15  7:30                   ` [virtio-dev] " Jason Wang
2023-05-15 10:08                     ` Michael S. Tsirkin
2023-05-15 14:30                       ` [virtio-dev] " Parav Pandit
2023-05-23 18:16                         ` [virtio-dev] " Michael S. Tsirkin
2023-05-23 21:32                           ` [virtio-dev] " Parav Pandit
2023-05-24  5:56                             ` [virtio-dev] " Michael S. Tsirkin
2023-05-24 18:57                               ` [virtio-dev] " Parav Pandit
2023-05-24 19:58                                 ` [virtio-dev] " Michael S. Tsirkin
2023-05-24 20:01                                   ` [virtio-dev] " Parav Pandit
2023-05-24 20:15                                     ` [virtio-dev] " Michael S. Tsirkin
2023-05-15 15:59                     ` [virtio-dev] " Parav Pandit
2023-05-16  6:21                       ` [virtio-dev] " Michael S. Tsirkin
2023-05-16 19:11                         ` [virtio-dev] " Parav Pandit
2023-05-16 20:58                           ` [virtio-dev] " Michael S. Tsirkin
2023-05-16 21:19                             ` [virtio-dev] " Parav Pandit
2023-05-16 21:23                               ` [virtio-dev] " Michael S. Tsirkin
2023-05-16 21:30                                 ` [virtio-dev] " Parav Pandit
2023-05-15  7:13                 ` [virtio-dev] " Jason Wang
2023-05-11 13:15               ` [virtio-dev] " Parav Pandit
2023-05-11 13:45                 ` [virtio-dev] " Michael S. Tsirkin
2023-05-12 14:03                   ` Parav Pandit
2023-05-16  3:54                 ` Jason Wang
2023-05-16 19:35                   ` [virtio-dev] RE: [virtio-comment] " Parav Pandit
2023-05-16 21:11                     ` [virtio-dev] " Michael S. Tsirkin
2023-05-16 21:49                       ` [virtio-dev] " Parav Pandit
2023-05-16 21:56                         ` [virtio-dev] " Michael S. Tsirkin
2023-05-10 16:11         ` [virtio-dev] " Parav Pandit
2023-05-10 16:16           ` Michael S. Tsirkin
2023-05-10 17:33             ` [virtio-dev] " Parav Pandit
2023-05-10 21:08               ` Parav Pandit
2023-05-10 21:33                 ` [virtio-dev] " Michael S. Tsirkin
2023-05-10 21:48                   ` [virtio-dev] " Parav Pandit
2023-05-11  7:06                 ` [virtio-dev] " Jason Wang
2023-05-11 13:04                   ` Michael S. Tsirkin
2023-05-15  5:19                     ` Jason Wang
2023-05-15 15:31                       ` [virtio-dev] " Parav Pandit
2023-05-11 13:28                   ` Parav Pandit
2023-05-11 13:38                     ` [virtio-dev] " Michael S. Tsirkin
2023-05-11 16:00                       ` [virtio-dev] " Parav Pandit
2023-05-11 20:47                       ` Parav Pandit
2023-05-11 20:58                         ` [virtio-dev] " Michael S. Tsirkin
2023-05-11 21:03                           ` [virtio-dev] " Parav Pandit
2023-05-15 16:55                             ` Parav Pandit
2023-05-15  7:10                     ` [virtio-dev] " Jason Wang
2023-05-15 15:49                       ` [virtio-dev] " Parav Pandit
2023-05-15 17:44                         ` [virtio-dev] " Michael S. Tsirkin
2023-05-15 17:51                           ` [virtio-dev] " Parav Pandit
2023-05-15 17:56                             ` [virtio-dev] " Michael S. Tsirkin
2023-05-15 18:00                               ` [virtio-dev] " Parav Pandit
2023-05-15 18:01                                 ` [virtio-dev] " Michael S. Tsirkin
2023-05-15 18:05                                   ` [virtio-dev] " Parav Pandit
2023-05-16  3:37                                   ` [virtio-dev] " Jason Wang
2023-05-16  3:43                                     ` Jason Wang
2023-05-16  5:38                                       ` Michael S. Tsirkin
2023-05-16  3:28                         ` Jason Wang
2023-05-16  3:45                           ` [virtio-dev] " Parav Pandit
2023-05-16  4:08                             ` [virtio-dev] " Jason Wang
2023-05-16 19:29                               ` [virtio-dev] " Parav Pandit
2023-05-16 21:09                                 ` [virtio-dev] " Michael S. Tsirkin
2023-05-16 21:41                                   ` [virtio-dev] " Parav Pandit
2023-05-16 21:54                                     ` [virtio-dev] " Michael S. Tsirkin
2023-05-16  4:18                           ` Michael S. Tsirkin
2023-05-07  9:04 ` Michael S. Tsirkin
2023-05-08 16:54   ` Parav Pandit
2023-05-15 20:29     ` [virtio-dev] Re: [virtio-comment] " Michael S. Tsirkin
2023-05-15 20:56       ` [virtio-dev] " Parav Pandit
2023-05-16  4:32         ` [virtio-dev] " Michael S. Tsirkin
2023-05-16 18:45           ` [virtio-dev] " Parav Pandit
2023-05-16 20:42             ` [virtio-dev] " Michael S. Tsirkin
2023-05-23  6:38 ` [virtio-dev] " Michael S. Tsirkin
2023-05-23 17:28   ` [virtio-dev] " Parav Pandit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230507093959-mutt-send-email-mst@kernel.org \
    --to=mst@redhat.com \
    --cc=cohuck@redhat.com \
    --cc=david.edmondson@oracle.com \
    --cc=jasowang@redhat.com \
    --cc=maorg@nvidia.com \
    --cc=parav@nvidia.com \
    --cc=sburla@marvell.com \
    --cc=shahafs@nvidia.com \
    --cc=virtio-comment@lists.oasis-open.org \
    --cc=virtio-dev@lists.oasis-open.org \
    --cc=yishaih@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox