Re: [virtio-comment] Re: [PATCH 09/11] transport-pci: Describe PCI MMR dev config registers

[Jason Wang <jasowang@redhat.com>]

On Wed, Apr 12, 2023 at 1:55âPM Parav Pandit <parav@nvidia.com> wrote:
>
>
>
> > From: Jason Wang <jasowang@redhat.com>
> > Sent: Wednesday, April 12, 2023 1:38 AM
>
> > > Modern device says FEAETURE_1 must be offered and must be negotiated by
> > driver.
> > > Legacy has Mac as RW area. (hypervisor can do it).
> > > Reset flow is difference between the legacy and modern.
> >
> > Just to make sure we're at the same page. We're talking in the context of
> > mediation. Without mediation, your proposal can't work.
> >
> Right.
>
> > So in this case, the guest driver is not talking with the device directly. Qemu
> > needs to traps whatever it wants to achieve the
> > mediation:
> >
> I prefer to avoid picking specific sw component here, but yes. QEMU can trap.
>
> > 1) It's perfectly fine that Qemu negotiated VERSION_1 but presented a
> > mediated legacy device to guests.
> Right but if VERSION_1 is negotiated, device will work as V_1 with 12B virtio_net_hdr.

Shadow virtqueue could be used here. And we have much more issues
without shadow virtqueue, more below.

>
> > 2) For MAC and Reset, Qemu can trap and do anything it wants.
> >
> The idea is not to poke in the fields even though such sw can.
> MAC is RW in legacy.
> Mac ia RO in 1.x.
>
> So QEMU cannot make RO register into RW.

It can be done via using the control vq. Trap the MAC write and
forward it via control virtqueue.

>
> The proposed solution in this series enables it and avoid per field sw interpretation and mediation in parsing values etc.

I don't think it's possible. See the discussion about ORDER_PLATFORM
and ACCESS_PLATFORM in previous threads.

>
> > > The PCI device exposed is transitional to the guest VM, so it can do legacy as
> > well as newer features.
> > > And if BAR 0 is hard coded, it may not be able to support features that may
> > need additional BAR.
> >
> > This part I don't understand, you can just put existing modern capabilities in
> > BAR0, then everything is fine.
> >
> Not sure I follow.
> May be a step back.
>
> What is proposed here, that
> a. legacy registers are emulated as MMIO in a BAR.
> b. This can be either be BAR0 or some other BAR
>
> Your question was why this flexibility?

Yes.

>
> The reason is:
> a. if device prefers to implement only two BARs, it can do so and have window for this 60+ config registers in an existing BAR.
> b. if device prefers to implement a new BAR dedicated for legacy registers emulation, it is fine too.
>
> A mediating sw will be able to forward them regardless.

I'm not sure I fully understand this. The only difference is that for
b, it can only use BAR0. Unless there's a new feature that mandates
BAR0 (which I think is impossible since all the features are
advertised via capabilities now). We're fine.

>
> > > Right, it doesnât. But spec shouldnât write BAR0 is only for legacy MMIO
> > emulation, that would prevent BAR0 usage.
> >
> > How can it be prevented? Can you give me an example?
>
> I mean to say, that say if we write a spec like below,
>
> A device exposes BAR 0 of size X bytes for supporting legacy configuration and device specific registers as memory mapped region.
>

Ok, it looks just a matter of how the spec is written. The problematic
part is that it tries to enforce a size which is suboptimal.

What's has been done is:

"
Transitional devices MUST expose the Legacy Interface in I/O space in BAR0.
"

Without mentioning the size.

Thanks


> Above line will prevent using BAR0 beyond legacy register emulation.
>