Re: [PATCH v10 4/4] transport-pci: Introduce group legacy group member config region access

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

On Thu, Jul 06, 2023 at 06:28:21PM +0200, Cornelia Huck wrote:
> On Thu, Jul 06 2023, Parav Pandit <parav@nvidia.com> wrote:
> 
> > diff --git a/transport-pci-legacy-regs.tex b/transport-pci-legacy-regs.tex
> > new file mode 100644
> > index 0000000..ceea28c
> > --- /dev/null
> > +++ b/transport-pci-legacy-regs.tex
> > @@ -0,0 +1,42 @@
> > +\subsection{Legacy Interface: Group member device Configuration Region Access}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / Legacy Interface: Group Member Device Configuration Region Access}
> > +
> > +The PCI owner device or the member device or both support driver notifications using
> 
> What about
> 
> "The PCI owner device, the member device, or both can choose to
> support..." ?
> 
> > +a notification region defined in \field{struct virtio_pci_legacy_notify_region}.
> > +
> > +In \field{struct virtio_virtio_admin_cmd_legacy_notify_query_entry},
> > +\field{region_data} is defined as following:
> > +
> > +\begin{lstlisting}
> > +struct virtio_pci_legacy_notify_region {
> > +        u8 owner;  /* When set to 1, notification region is of the owner device */
> > +        u8 bar;    /* BAR of the member or owner device */
> > +        u8 padding[6];
> > +        le64 offset; /* Offset within bar. */
> > +};
> > +\end{lstlisting}
> > +
> > +The group owner device hardwires VF BAR0 to zero in the SR-IOV Extended
> > +capability.
> > +
> > +The group member device does not use PCI BAR0 in all the Virtio PCI capabilities
> 
> "in [or for?] any of the...", I guess?
> 
> > +listed in section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus / Virtio Structure PCI Capabilities}.
> > +
> > +\devicenormative{\subsubsection}{Legacy Interface: Group Member Device Legacy Configuration Region Access}{Virtio Transport Options / Virtio Over PCI Bus / Legacy Interface: Group Member Device Configuration Region Access}
> > +
> > +When a PCI SR-IOV group owner device supports
> > +VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_READ,
> > +VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_WRITE, VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_READ,
> > +VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_WRITE commands, the group owner device MUST
> > +hardwire VF BAR0 to zero in the SR-IOV Extended capability and the group memberi
> 
> s/memberi/member/
> 
> > +device MUST NOT use BAR0 in any of the Virtio Structure PCI Capabilities.
> > +
> > +The group owner device or the group member device or both MAY support driver
> > +notifications region.
> 
> Make this "a driver notification region"?

I think in fact for conformance we can just refer to supporting the command.
The command can return an array of structures, each referring to
the memory of the owner device or the member device.


> > +
> > +For the SR-IOV group type, the owner device supporting
> > +VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_READ,
> > +VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_WRITE, VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_READ,
> > +VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_WRITE and VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_QUERY
> > +commands and its member device SHOULD follow the rules for the PCI Device ID,

what does "its" mean here? Of the group?
Members of the SR-IOV group type are VFs. They can not follow
the rules for the Device ID: the spec says:
This field in all VFs returns FFFFh when read.

Even if you somehow refer to the software it's extraneous anyway
since the spec proceeds: VI software should return the Vendor ID value
from the associated PF as the Vendor ID value for the VF.

We'll need a separate statement for Device ID.

> > +Revision ID and Subsystem Device ID of the non-transitional devices documented in
> > +section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Discovery}.