RE: [virtio-comment] RE: [PATCH v5 2/7] Introduce admin command set

[Parav Pandit <parav@nvidia.com>]

> From: virtio-comment@lists.oasis-open.org <virtio-comment@lists.oasis-
> open.org> On Behalf Of Michael S. Tsirkin
> 
> On Mon, Jun 20, 2022 at 11:06:07AM +0000, Parav Pandit wrote:
> >
> >
> > > From: Michael S. Tsirkin <mst@redhat.com>
> > > Sent: Monday, June 20, 2022 6:00 AM
> > >
> > > On Tue, May 31, 2022 at 08:39:24PM +0000, Parav Pandit wrote:
> > > > > And as I said, you will need much more spec work to reach the
> > > > > level to which features are specified - and note we are not yet
> > > > > happy with how features are
> > > > Can you be specific of the work that you are expecting in this v5
> version?
> > >
> > > I proposed getting rid of attrs_mask/device_admin_caps and instead
> > > specify that e.g. feature bits 64 to 95 are reserved for management
> purposes.
> > > Maybe just add a 32 bit register, or maybe "extended features"
> > > with a selector and length like mmio and ccw already do, up to you.
> > > I think Cornelia likes this suggestion too.
> > >
> > > This would replace functionality of the
> > >
> VIRTIO_ADMIN_DEVICE_CAPS_IDENTIFY/VIRTIO_ADMIN_DEVICE_CAPS_AC
> > > CEPT
> > > commands and have the advantage of being generally well specified
> > > and understood.
> > >
> > > Is this acceptable? Or is there a reason the new commands are
> preferable?
> >
> > We prefer the new commands for following reason.
> >
> > 1. proposed new command doesn't demand exposing registers in the PCI
> > memory mapped area 2. Today AQ is perceived in to be PF, but it doesn't
> have to be. For next 10 years spec may evolve to have AQ for some other
> purpose on VFs/SFs.
> > Such devices scale at much higher magnitude than PFs.
> > And exposing memory mapped registers for rare functionality is the last
> thing to do in my mind.
> > 3. Placements of such features bits in an AQ gives device lot more flexibility
> on _how_ to implement them. Some in sw, fw, hw, memory die etc.
> > Placement of them in PCI register space reduces these options.
> >
> > So exposing them something in PCI register space has to have strong
> technical reason than just simplicity of access.
> 
> I ansolutely get this argument. Please do get mine which is avoiding
> duplicating very similar functionality is subtly differing ways.
Yes, because it is anticipated that AQ will be more widely used that adds more features/functionality.

> 
> Your above arguments apply equally to most other registers we already
> have.  Wouldn't you then agree we need to address this more drastically by
> defining an alternative transport such as virtio over virtqueue then?

We cannot change what already exists. But we can better define new additions.
If we would be adding AQ only for the purpose of querying feature bits of a device under operation that would be a significant overhead.
But that is not done. An AQ is reused for multiple tasks.

I see feature bits is something that is very essential for device initialization sequence; without negotiating them it is very hard to operate the device.
Implementing them as essential pci registers feature bits makes a total sense like done today.

For AQ features proposed that has no hard requirements of it with device initialization, it is better to query and negotiate run time.