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

[Jason Wang <jasowang@redhat.com>]

On Sun, May 21, 2023 at 10:33âPM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Sun, May 21, 2023 at 01:21:17PM +0000, Parav Pandit wrote:
> >
> > > From: virtio-comment@lists.oasis-open.org <virtio-comment@lists.oasis-
> > > open.org> On Behalf Of Michael S. Tsirkin
> > > Sent: Sunday, May 21, 2023 5:17 AM
> >
> > > > The Notification of the VFs are on the VF BAR for modern or legacy.
> > > > One needs to build additional cross forwarding hardware from PF to VF for the
> > > doorbells.
> > >
> > > I think doorbells are driver notifications (linux driver calls them kicks)?
> > > I don't understand what you are saying above really.
> > > what can and what can't be done?
> > >
> > VF has the notification BAR region.
> > All 1.x and legacy such notification lands on the VF BAR.
> >
> > > Again all this idea (as opposed to Jason's transport vq) is to have a simple
> > > software model. Attaching a driver to two devices at the same time is hard to
> > > achive e.g. under windows.
> > >
> > Yet you initiate same discussion point that we already discussed again after summarizing.
> > A driver is not attached to two devices.
> > A driver is attached to a single device.
>
> And that device is the owner no? to send commands?
>
> > A device uses a parent/owner group device to access legacy.
> >
> > Software model may evolve over time.
> >
> > >
> > > > And it cannot utilize what already exists for 1.x VF.
> > > >
> > > >  > 2. It should be possible to send notifications through an admin
> > > > command too,
> > > > >    otherwise admin commands are an incomplete set of functionality.
> > > > >
> > > > Yes. it is only for the functionality. As we discussed in past already, this will not
> > > get any performance.
> > >
> > > Performance might be ok if hardware disables kicks most of the time.
> > >
> > Even for the first kick it is order of magnitude higher.
> > Kicks is the natural tool for the low latency.
> >
> > >
> > > > > 3. I feel using a capability to describe legacy notification
> > > > >    area would be better just because we already have a
> > > > >    structure for this. make it an express capability if you like.
> > > > The AQ command interface is far more programable object than PCI capability
> > > to return this admin info.
> > > > Hence I prefer AQ cmd.
> > >
> > > I feel your preferences for 1 and 3 conflict. If you really insist on kicks being on
> > > VFs then at least let us make VF driver in the host simple.
> > It is straight forward.
> >
> > If you prefer the "offset" example of past,
> >
> > If (legacy_offset == queue_notify_offset)
> >    *db = guest_supplied_q_notify_content;
> > else
> >     virtio_net_send_aq_cmd();
> >
> > "simple" is really a subjective term in this context.
>
> yes this is qemu. sure.
>
> So we have legacy emulation send commands to VF or to PF.  Okay. But let
> us avoid the need for VF driver to send commands to PF to initialize.
> Just get all information it needs from VF itself.
>
>
> Maybe it's a good idea to reuse existing notification capability,
> or maybe a new one, but let's avoid making VF driver depend on PF
> commands.
>
>
>
> > > If it has to talk to PF
> > > driver things are really complex. At this point we are very very far from VFIO
> > > model, and then what exactly have we gained by implementing legacy control
> > > path in hardware?
> > It is in the VFIO model, legacy largely falls in the special path for backward compat for the hw devices.
> >
> > > Let's do software with maybe a couple of features such as
> > > VIRTIO_NET_F_LEGACY_HEADER.
> > >
> > We have already these design choices and tradeoff in v0 and v1, it doesn't fit the requirements.
>
> BTW not for this project but generally this is why I thought it is not
> at all a bad idea to have a requirements text document e.g. under
> todo/
> discuss it as normally maybe even vote on including features in a
> plan for a release.
>
> > We cannot iterate exact 100% same points again in this summary discussion.
> >
> > If you have any different comments from v0 and v1, please share, otherwise these commands should proceed.
> >
> > What you hint is saying hey, "transport everything through the PF, then why do you have a VF?"
> > We again past that discussion.
> >
> > This is different requirement, than
> > "There is a VF passthrough accessible directly from the guest without a VI, some of them need to have legacy support also".
> > Hence how do one make that legacy guest utilize it the VF in passthrough manner for 1.x and 0.9.5.
> > With this proposal 0.9.5 slow registers are accessed over the owner group admin PF keeping the whole model and functionality intact.
> >
> > This is it.
>
>
> So, I am saying one model is small driver for VF and a big one for PF.
> And to keep the VF driver simple, it should get information simply from
> config space capability.

Think in a different way, if we don't care about the speed of those
slow registers. For the method that accesses the configuration
structure, could we simply extend VIRTIO_PCI_CAP_PCI_CFG capability?

Thanks

>
>
>
> --
> MST
>