[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]