Re: [PATCH v12 03/10] admin: introduce group administration commands

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

On Tue, Apr 25, 2023 at 01:25:24PM +0000, Parav Pandit wrote:
> 
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Tuesday, April 25, 2023 2:20 AM
> 
> > > Below paragraph is no longer applies as we already discussed more use
> > > cases of the AQ such as accessing the PCI VF's legacy config space registers.
> > > Please drop below paragraph.
> > 
> > Look - at one point you want commit log to record design process, an another
> > you turn around and ask me to drop it.
> > I feel like keeping this around frankly. Maybe I will add a sentence or two along
> > the lines of "Note that nothing limits us from extending this down the road
> > though - but let's focus on what we already know for now".
> > 
> I am requesting to drop the commit point that limits the usage of the AQ.
> Do you agree that AQ can be extended for use beyond VF and SIOV group management commands?
> Or one should create a new VQ type?
> If it is the later, I don't see the need of multiple AQ.

not management. that is old term and indeed too narrow.
administration as in hypervisor (admin) access through PF while 
guest has access through VF.

legacy access you link to below seems to fall within scope:
we access VF through a PF.

So what is the problem?

maybe we'll change the meaning down the road. I do not
see the immediate need currently though.


> > This extreme focus on commit log is poinless anyway - it makes much more
> > sense for code since commit log describes the change in english. Here the
> > whole change is english anyway.
> > 
> > > > Without this the admin vq would become a "whatever" vq which does
> > > > not do anything specific at all, just a general transport like thing.
> > > > I feel going this way opens the design space to the point where we
> > > > no longer know what belongs in e.g. config space what in the control
> > > > q and what in the admin q.
> > > > As it is, whatever deals with groups is in the admin q; other things
> > > > not in the admin q.
> > > >
> > >
> > > A PF can use same or different AQ with a new struct
> > > virtio_legacy_register_access with a different opcode range.
> > 
> > We'll get to that bridge and we'll cross it, the proposal is not on list even yet. 
> It is at [1].
> [1] https://lists.oasis-open.org/archives/virtio-dev/202304/msg00511.html
> I cannot draft the legacy command using AQ because AQ patch-5 in v12 fails to merge.
> 
> Before I draft it, I sent [1] on the design way forward.
> 
> > I
> > actually think that yes, you need it in a separate function. If PF is passed
> > through to guest then PF can not also safely DMA into host memory.
> That is fine. Only some user tests would pass the PF and VF to the VMs.
> One can create N combinations to make it not work.
> In all practical purposes, PF is owned by the hypervisor, trusted sw as listed in the PCI spec.
> 
> > Alternatively, we could use an MMIO based mechanism for admin commands.
> > And maybe that would be a good fit.
> We discussed that MMIO is not a good fit for the VFs at scale as listed in [1].
> 
> [1] https://lists.oasis-open.org/archives/virtio-dev/202304/msg00511.html

Then I don't see a problem. It's just commit log, not spec. Let it
slide is my suggestion, this is spec text not code.

-- 
MST