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:45:50PM +0000, Parav Pandit wrote:
> 
> 
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Thursday, July 6, 2023 12:39 PM
> 
> > > > +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.
> >
> Array and referring to owner/member is already present in this patch.


I don't see a statement that devices MAY support VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_QUERY
with entries returning just owner, just member, or both.

> Do you suggest to drop above statement?

> > 
> > > > +
> > > > +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?
> Yes, will change to the member device 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.
> >
> Will split the line 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}.