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

[Parav Pandit <parav@nvidia.com>]

> From: Jason Wang <jasowang@redhat.com>
> Sent: Wednesday, April 12, 2023 2:15 AM
> 
> 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.
>
This proposal Is not implementing about vdpa mediator that requires far higher understanding in hypervisor.
Such mediation works fine for vdpa and it is upto vdpa layer to do. Not relevant here.
 
> >
> > 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.
> 
I have read the previous thread.
Hypervisor will be limiting to those platforms where ORDER_PLATFORM is not needed.
And this is a pci transitional device that uses the standard platform dma anyway so ACCESS_PLATFORM is not related.

> >
> > 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. 
Why do say it can use only BAR 0?

For example, a device may have implemented say only BAR2, and small portion of the BAR2 is pointing to legacy MMIO config registers.
A mediator hypervisor sw will be able to read/write to it when BAR0 is exposed towards the guest VM as IOBAR 0.

> 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.
>
No new feature. Legacy BAR emulation is exposed via the extended capability we discussed providing the location.
 
> >
> > > > 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.

For new legacy MMIO registers can be implemented as BAR0 with same size. But better to not place such restriction like above wording.