[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: Re: [virtio-comment] Re: [PATCH 09/11] transport-pci: Describe PCI MMR dev config registers
On Fri, Apr 7, 2023 at 4:55âPM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Fri, Mar 31, 2023 at 01:58:32AM +0300, Parav Pandit wrote:
> > Legacy virtio configuration registers and adjacent
> > device configuration registers are located somewhere
> > in a memory BAR.
> >
> > A new capability supplies the location of these registers
> > which a driver can use to map I/O access to legacy
> > memory mapped registers.
> >
> > This gives the ability to locate legacy registers in either
> > the existing memory BAR or as completely new BAR at BAR 0.
> >
> > A below example diagram attempts to depicts it in an existing
> > memory BAR.
> >
> > +------------------------------+
> > |Transitional |
> > |MMR SRIOV VF |
> > | |
> > ++---------------+ |
> > ||dev_id = | |
> > ||{0x10f9-0x10ff}| |
> > |+---------------+ |
> > | |
> > ++--------------------+ |
> > || PCIe ext cap = 0xB | |
> > || cfg_type = 10 | |
> > || offset = 0x1000 | |
> > || bar = A {0..5}| |
> > |+--|-----------------+ |
> > | | |
> > | | |
> > | | +-------------------+ |
> > | | | Memory BAR = A | |
> > | | | | |
> > | +------>+--------------+ | |
> > | | |legacy virtio | | |
> > | | |+ dev cfg | | |
> > | | |registers | | |
> > | | +--------------+ | |
> > | +-----------------+ | |
> > +------------------------------+
> >
> > Co-developed-by: Satananda Burla <sburla@marvell.com>
> > Signed-off-by: Parav Pandit <parav@nvidia.com>
>
>
> I am split about using the extended capability for this, since in
> practice this makes for more code in hypervisors. How about just using
> an existing capability as opposed to the extended capability? Less work
> for existing hypervisors, no? And let's begin to use the extended
> capability for something more important than legacy access.
>
>
>
>
> > ---
> > transport-pci.tex | 33 +++++++++++++++++++++++++++++++--
> > 1 file changed, 31 insertions(+), 2 deletions(-)
> >
> > diff --git a/transport-pci.tex b/transport-pci.tex
> > index aeda4a1..55a6aa0 100644
> > --- a/transport-pci.tex
> > +++ b/transport-pci.tex
> > @@ -168,6 +168,7 @@ \subsection{Virtio Structure PCI Capabilities}\label{sec:Virtio Transport Option
> > \item ISR Status
> > \item Device-specific configuration (optional)
> > \item PCI configuration access
> > +\item Legacy memory mapped configuration registers (optional)
> > \end{itemize}
> >
> > Each structure can be mapped by a Base Address register (BAR) belonging to
> > @@ -228,6 +229,8 @@ \subsection{Virtio Structure PCI Capabilities}\label{sec:Virtio Transport Option
> > #define VIRTIO_PCI_CAP_SHARED_MEMORY_CFG 8
> > /* Vendor-specific data */
> > #define VIRTIO_PCI_CAP_VENDOR_CFG 9
> > +/* Legacy configuration registers capability */
> > +#define VIRTIO_PCI_CAP_LEGACY_MMR_CFG 10
> > \end{lstlisting}
> >
> > Any other value is reserved for future use.
> > @@ -682,6 +685,18 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport
> > Configuration Space / Legacy Interface: Device Configuration
> > Space}~\nameref{sec:Basic Facilities of a Virtio Device / Device Configuration Space / Legacy Interface: Device Configuration Space} for workarounds.
> >
> > +\paragraph{Transitional MMR Interface: A Note on Configuration Registers}
> > +\label{sec:Virtio Transport Options / Virtio Over PCI Bus / Virtio Structure PCI Capabilities / Common configuration structure layout / Transitional MMR Interface: A Note on Configuration Registers}
> > +
> > +The transitional MMR device MUST present legacy virtio registers
> > +consisting of legacy common configuration registers followed by
> > +legacy device specific configuration registers described in section
> > +\ref{sec:Virtio Transport Options / Virtio Over PCI Bus / Virtio Structure PCI Capabilities / Common configuration structure layout / Legacy Interfaces: A Note on Configuration Registers}
> > +in a memory region PCI BAR.
> > +
> > +The transitional MMR device MUST provide the location of the
> > +legacy virtio configuration registers using a legacy memory mapped
> > +registers capability described in section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus / Virtio Structure PCI Capabilities / Transitional MMR Interface: Legacy Memory Mapped Configuration Registers Capability}.
> >
> > \subsubsection{Notification structure layout}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Notification capability}
> >
> > @@ -956,9 +971,23 @@ \subsubsection{PCI configuration access capability}\label{sec:Virtio Transport O
> > specified by some other Virtio Structure PCI Capability
> > of type other than \field{VIRTIO_PCI_CAP_PCI_CFG}.
> >
> > +\subsubsection{Transitional MMR Interface: Legacy Memory Mapped Configuration Registers Capability}
> > +\label{sec:Virtio Transport Options / Virtio Over PCI Bus / Virtio Structure PCI Capabilities / Transitional MMR Interface: Legacy Memory Mapped Configuration Registers Capability}
> > +
> > +The optional VIRTIO_PCI_CAP_LEGACY_MMR_CFG capability defines
> > +the location of the legacy virtio configuration registers
> > +followed by legacy device specific configuration registers in
> > +the memory region BAR for the transitional MMR device.
> > +
> > +The \field{cap.offset} MUST be 4-byte aligned.
> > +The \field{cap.offset} SHOULD be 4KBytes aligned and
>
> what's the point of this?
>
> > +\field{cap.length} SHOULD be 4KBytes.
>
> Why is length 4KBytes? Why not the actual length?
>
>
> > +
> > +The transitional MMR device MUST present a legacy configuration
> > +memory mapped registers capability using \field{virtio_pcie_ext_cap}.
> > +
> > \subsubsection{Legacy Interface: A Note on Feature Bits}
> > -\label{sec:Virtio Transport Options / Virtio Over PCI Bus /
> > -Virtio Structure PCI Capabilities / Legacy Interface: A Note on Feature Bits}
> > +\label{sec:Virtio Transport Options / Virtio Over PCI Bus / Virtio Structure PCI Capabilities / Legacy Interface: A Note on Feature Bits}
> >
>
>
> So it is not by chance that we abadoned the legacy interface,
> it had some fundamental issues.
> Let me list some off the top of my mind:
> - no atomicity across accesses, so if a register changes
> while it is read driver gets trash. solved using generation id
> - no defined endian-ness. solved using le
> - no way to reject driver configuration
I can think more:
- VIRTIO_F_ACCESS_PLATFORM
- VIRTIO_F_ORDER_PLATFORM
The above two are a must for hardware devices which are almost
impossible for legacy interfaces.
Hardware transitional devices are really tricky, ENI is one example
which can only work for x86.
config ALIBABA_ENI_VDPA
tristate "vDPA driver for Alibaba ENI"
select VIRTIO_PCI_LIB_LEGACY
depends on PCI_MSI && X86
It has an MMIO bar for legacy registers and it works by chance for
Linux drivers; DPDK requires patches to let it work.
This is fine for vDPA but not for virtio if the design can only work
for some specific setups (OSes/archs).
Thanks
>
> we just did a new thing instead, but I feel if you are reviving the
> legacy interface yet again, it is worth thinking about solving the worst
> of the warts. For example, I can see an endian-ness register that
> hypervisor can either read to check device is compatible with guest, or
> maybe even write to force device to specific endian-ness.
> A generation counter that hypervisor can check to
> verify value is consistent? Can work if hypervisor
> caches configuration.
> A bad state flag that device can set to make hypervisor
> stop guest? Better than corrupting it ...
>
>
>
>
> > Only Feature Bits 0 to 31 are accessible through the
> > Legacy Interface. When used through the Legacy Interface,
> > --
> > 2.26.2
>
>
> This publicly archived list offers a means to provide input to the
> OASIS Virtual I/O Device (VIRTIO) TC.
>
> In order to verify user consent to the Feedback License terms and
> to minimize spam in the list archive, subscription is required
> before posting.
>
> Subscribe: virtio-comment-subscribe@lists.oasis-open.org
> Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
> List help: virtio-comment-help@lists.oasis-open.org
> List archive: https://lists.oasis-open.org/archives/virtio-comment/
> Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
> List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
> Committee: https://www.oasis-open.org/committees/virtio/
> Join OASIS: https://www.oasis-open.org/join/
>
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]