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

Michael S. Tsirkin
2023-05-10T06:05:36Z

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?

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.

> > 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?

> 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.
--
MSr