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 04:33:01PM +0000, Parav Pandit wrote:
> 
> 
> > From: Cornelia Huck <cohuck@redhat.com>
> > Sent: Thursday, July 6, 2023 12:28 PM
> > 
> > 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..." ?
> >
> Fine too.
> Any thing wrong in having or as above, so I don't write it next time?
> Or that in current form reads better to me.
>  
> > > +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?
> Will change to "Any of the ".
> 
> > 
> > > +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/
> >
> Yes, already changed in my v11 where patch 3 and 4 content is merged to single generic section.
> 
>  
> > > +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"?
> >
> Notifications are generally more than one and spec has the section "driver notifications", so...
>  

We will need to detail how notifications work though. The reason
is that the legacy notifications are received by owner driver
(which is not legacy!) and forwarded to device.
We need to explain exactly how this is done for people who
don't read minds and did not follow 3 months of mailing list
debates, and explain how this has the same effect as
triggering driver notification through the legacy
interface.