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

[Jason Wang <jasowang@redhat.com>]

On Wed, May 10, 2023 at 3:43âPM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Wed, May 10, 2023 at 03:01:25PM +0800, Jason Wang wrote:
> > On Wed, May 10, 2023 at 2:05âPM Michael S. Tsirkin <mst@redhat.com> 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?
> >
> > Yes. I think it needs to be done through the transport virtqueue
> > otherwise the transport is not self-contained.
>
> I mean, any feature can be done over transport vq.
>
> But there is value in adding legacy commands to an otherwise
> modern device without reworking it completely to
> switch to a different transport.

There's probably no need for a rework since legacy is not complicated.
More below.

>
>
> > > A reasonable thing to include at least in the commit log. Parav?
> > >
> > > You are also asking what if the device uses transport vq,
> > > and we want transitional on top of that.
> > > It's a fair question but I don't exactly get why would
> > > this legacy support feature be wanted for the vq transport
> > > and not for other transports.
> >
> > Not sure I get the question, but all the existing transport support
> > legacy, if we want to have another, should the legacy support be a
> > must or not?
>
> This specific proposal is for tunneling legacy over admin vq.
> It can live alongside a normal modern VF, with hypervisor
> combining these to create a transitional device.

Exactly, but what I meant here is

If we decide to use the admin vq, is there any good reason to tie it
to PCI if we don't want to tunneling PCI over adminq?

Why not simply invent individual commands to access legacy facilities
like commands to access like what transport virtqueue did for modern
device?:

1) device features
2) driver features
3) queue address
4) queue size
5) queue select
6) queue notify
7) device status
8) ISR status
9) config msix
10) queue msix
11) device configuration space

It focuses on the facilities instead of transport specific details
like registers (we don't even need legacy registers in this case), I
gives more deterministic behavior so we don't need to care about the
cross registers read/write.

>
> > >
> > >
> > >
> > >
> > > > > Consider simplest case, multibyte fields. Legacy needs multibyte write,
> > > > > modern does not even need multibyte read.
> > > >
> > > > I'm not sure I will get here,
> > >
> > > What does this mean?
> >
> > I meant I don't get what the issue if "modern does not even need
> > multibyte read".
>
> parse error again. reword?

I meant we need two sets of command for legacy and modern. We can
choose not to expose multibyte reads for modern commands.

>
> > >
> > > > since we can't expose admin vq to
> > > > guests, it means we need some software mediation. So if we just
> > > > implement what PCI allows us, then everything would be fine (even if
> > > > some method is not used).
> > > >
> > > > Thanks
> > >
> > > To repeat discussion on one of the previous versions, no it will not be
> > > fine because legacy virtio abuses pci in fundamentally broken ways.
> > > So yes you need a mediator on the host but even giving this
> > > mediator a chance to be robust on top of hardware
> > > means the hardware interface can not simply mirror legacy
> > > to hardware.
> > >
> > > For example, host mediator needs to trap writes into mac,
> > > buffer them and then send a 6 byte write.
> > > Now, pci actually does allow 6 byte writes, but legacy
> > > guests instead to 6 single byte writes for portability reasons.
> >
> > It's a known race condition, so PCI over adminq doesn't make it worse.
>
> it can however make it better - you can do a single 6 byte write command.
>

It would be tricky to detect when a legacy guest has finished writing
to the mac.

> > The mediator can just mirror what guests write over the admin
> > commands.
>
> and this "just" just isn't good enough, or we end up with hacks
> in hardware.

Yes but this "issue" exists in this proposal as well.

Thanks

>
> > Thanks
> >
> > > --
> > > MSr
> > >
>