Re: [virtio-comment] RE: [PATCH v2 1/2] transport-pci: Introduce legacy registers access commands

[Jason Wang <jasowang@redhat.com>]

Hi Parav:

On Wed, May 24, 2023 at 6:22âAM Parav Pandit <parav@nvidia.com> wrote:
>
>
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Tuesday, May 23, 2023 2:49 PM
> > > > iokit is from my previous work experience not virtio.
> > > > For virtio we had a lot of trouble with hacks like this on windows.
> > > I don't know why windows driver need to talk to PF driver until now when the
> > AQ doesn't exists.
> >
> > What is it you want to know?
> >
> Why a VF driver needs to talk to a PF driver in windows without AQ?
>
> > > > That's an important platform.
> > > >
> > > I am not sure how important is it.
> > > What I learnt that a decade has passed and even after that virtio drivers are
> > not part of the OS distro.
> >
> > Because with QEMU they are so easy to side-load. So we never bothered.
> >
> I don't understand your comment.
> I was saying to my knowledge a Windows OS do not have a virtio drivers in their distro which you said is important.
>
> > > It doesn't need to because guest is anyway going to do whatever it does.
> >
> > yes. But hypervisor can do work-arounds or detect broken functionality and
> > stop cleanly.
> >
> You mentioned this several times.
> Below requirement still holds fine.
>
> > > The clear requirement is: to not fix legacy but to support legacy.
> > > Fixing legacy is role of 1.x. Not the role of legacy interface itself using
> > AQ/MMIO.
> >
>  > And for purpose of BAR mapping, such BAR in the VF can be mapped too.
> > > You didn't answer why VF BAR is a problem and suggest PF BAR.
> >
> > VF BAR by itself is not a problem. Discovering VF BAR mapping offset through PF
> > is.
> >
> At the end of the email you right that "AQ is fine" but here you write discovering that through the PF AQ is.
> How to interpret your comment?
>
> > > If notification for VF is in VF -> hence other config registers also in VF, such
> > blanket statements does not help.
> >
> > not other config registers. what's needed in VF driver should be retrievable
> > through VF.
> >
> This applies to the modern device, for legacy emulation, I don't see this a mandatory.
>
> > > Your comments and suggestions are helpful.
> > > Some of them follow the loop that doesn't help.
> > > Again, thanks a lot for the inputs and reviews.
> > >
> > > The design discussion helped to produce two solutions here than what was
> > started in v0.
> >
> > OK good. Now I suggest you make an effort to come up with some compromise
> > that will make this thread stop. I really can't say I even remember what we said
> > in the beginning.
>
> > You had this idea of starting with a problem statement document how about
> > doing that? Because I feel every time people bring up some idea and a problem
> > this idea would solve, you go "this is out of scope". Let's decide what the scope
> > is?
> >
> The requirements are simple and straightforward at start of the cover letter.
> i.e. for quick reference in short: A PCI VF passthrough to guest VM to support legacy + 1.x.
>
> I propose:
> Method-1:
> 1. VF legacy registers access over AQ of PF
> 2. VF driver notifications using MMIO of VF
>
> Method-2:
> 1. legacy registers access using two MMIO registers (data and addr)
> 2. VF driver notifications using MMIO of VF
>

Based on your above summary (and a more detailed one in another
thread), it's not hard to see the tricky part is the try to
provide/mediate modern features for legacy. Assuming we all agree:

1) We want transitional devices but not legacy only devices
2) Mediation is a must for legacy

Then finding a way to reuse a modern interface is much better than
building a new path back to the old legacy maze. I really think we
should go with a way to meditate on top of a modern device. This
probably requires some new feature bit(s) like
VIRTIO_NET_F_LEGACY_HEADER but it has the lowest cost.

1) no new hardware interface like registers/capabilities etc
2) no datapath meditation, hypervisor will negotiate
VIRTIO_NET_F_LEGACY_HEADER for legacy guest drivers
3) endian is fine, hypervisor know it's modern so it's little
4) reset is fine, hypervisor know it's modern so it can do proper meditation
5) notification is fine, hypervisor know it's modern so it can use
modern notification areas
6) all the behaviour is deterministic, no undocumented or code defined
legacy behaviours

Anything that prevents you from adding things like VIRTIO_NET_F_LEGACY_HEADER?

You wrote the following in v1:

"""
There is virtio PCI VF.
The user who attaches this VF to the VM, it doesnât know if its guest
is to run legacy driver or 1.x
Hence hypervisor doesnât want to run special stack when guest may
(likely) be 1.x and device also supports 1.x.
"""

I don't understand this actually. For "special stack", are you
referring to the mediation path for legacy? I don't see how
VIRTIO_NET_F_LEGACY_HEADER differs from new hardware interfaces like
admin virtqueue in this regard. If management stack wants a
transitional device, Hypervisor just need

1) for modern BAR, assign it directly to the VM if it wishes, no
mediation at all
2) for legacy BAR, trap and mediate

The only difference happens in 2), that is which kind of interfaces
should we go, existing modern or new legacy like admin virtqueue.

Or as Michael said, it would be better to explain how the hypervisor
expects to work may help.

Thanks