[virtio-dev] RE: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ

Parav Pandit
2023-05-10T17:33:51Z

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Wednesday, May 10, 2023 12:16 PM
>
> On Wed, May 10, 2023 at 12:11:50PM -0400, Parav Pandit wrote:
> >
> >
> > On 5/10/2023 2:04 AM, Michael S. Tsirkin wrote:
> > > On Mon, May 08, 2023 at 10:23:39AM +0800, Jason Wang wrote:
> > > > > I thought so too originally. Unfortunately I now think that no,
> > > > > legacy is not going to be a byproduct of transport virtqueue for
> > > > > modern - it is different enough that it needs dedicated commands.
> > > >
> > > > If you mean the transport virtqueue, I think some dedicated
> > > > commands for legacy are needed. Then it would be a transport that
> > > > supports transitional devices. It would be much better than having
> > > > commands for a partial transport like this patch did.
> > >
> > > OK I am beginning to get what you are saying. So your criticism is
> > > this: what if device supports vq transport for modern, and we want
> > > to build a transitional device on top. how will that look. yes?
> > > A reasonable thing to include at least in the commit log. Parav?
> > >
> > I am still trying to understand what is "vq transport for modern"?
> > Do you mean transporting currently defined config space access over vq?
> > If so, is this VQ belong to the guest or hypervisor?
>
> https://lore.kernel.org/all/20220826100034.200432-2-
> lingshan.zhu%40intel.com/t.mbox.gz

The gz link is not accessible.
But I got the right link [1].

[1] https://lore.kernel.org/all/20220826100034.200432-2-lingshan.zhu@intel.com/

1. Above patch cover letter [1] is missing the basic objective/problem statement.
i.e. why a transport virtqueue is needed?
But I probably get the idea of [1] as we did the AQ.

2. Commit log says about
a. querying resource of management device (aka group owner in AQ now)
b. creating and destroying the managed device (aka group owner creating group member devices)
c. configure the managed device (aka group owner configuring/composing group member devices such as VFs, SFs, SIOV).

So, all above 2.a to 2.c belongs to the admin group owner and group management commands like how it is defined in the AQ proposal.

So, 3 out of the 4 motivations are achieved by AQ proposal.
This AQ belongs to the hypervisor. I am clear on this part.

4th point in cover letter is: "config virtqueues of the managed device".

This work belongs to the driver -> device direct communication using a queue from driver to device.
So, I imagine this work can be done using a queue by the guest driver and serviced by the device like how a guest driver configures the queue today without any mediation.
For PCI, MMIO transport, surely this can be done by the PCI device directly being is PF, VF or SIOV.
(Instead of config register, using a new queue interface). Looks fine to me.

Can this new cfg queue mediated like CVQ that is done in a sw? May be yes.
Should it be always mediated when it is of VF, SIOV Device? Mostly no because it is yet another VQ for PF, VF, SIOV.

I am yet to parse rest of the 4 patches, please give me some time to review it.