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

Jason Wang
2023-05-11T07:17:20Z

On Thu, May 11, 2023 at 12:04 AM Parav Pandit <parav@nvidia.com> wrote:
>
>
>
> On 5/9/2023 11:51 PM, Jason Wang wrote:
> > On Tue, May 9, 2023 at 11:56 AM Parav Pandit <parav@nvidia.com> wrote:
> >>
> >> SIOV does not require mediation.
> >> Composition is optional like VFs.
> >
> > This is not what I read from SIOV spec.
> >
> SIOV R1 spec was defined by _a_ vendor and it is undergoing upgrade to
> more practical use now. So we should let them finish the work.
>

That's fine.

> It is hard to comment in this forum about it.

Yes.

>
> >>
> >>>>
> >>>> Hence, the transport we built is to consider this in mind for the
> >>>> coming future.
> >>>
> >>> For transport virtqueue, it's not specific to PCI. It could be used in a much
> >>> broader use case.
> >>>
> >> May be.
> >> More below.
> >>
> >>>> So if each VF has its own configq, or cmdq, it totally make sense to
> >>>> me which is bootstrap interface to transport existing config space interface.
> >>>> The problem is: it is not backward compatible; Hence a device has no
> >>>> way of when to support both or only new configq.
> >>>
> >>> Providing compatibility in software is much more simpler than inventing new
> >>> hardware interfaces. Isn't it? (e.g if we want to provide compatibility for VF on
> >>> a SIOV device). And inventing a new hardware interface for compatibility might
> >>> not always work, it may break the advantages of the new hardware (like
> >>> scalability).
> >>>
> >> What I proposed below is new hardware interface for new features.
> >> Not for compatibility.
> >
> > I'm confused, the proposal is for legacy drives so it's for
> > compatibility for sure. No?
> This proposal is to get legacy devices to work over a PCI VF.
>
> a configq/cmdq discussion is for new features for PF, VF, and SF/SIOV
> devices to work in _non_ backward compatible way,
> because for new
> features there is no such thing as backward compatibility between guest
> driver and the device.

Ok, so what command did configq/cmdq have? I thought it was similar to
transport virtqueue, but it looks more like a control virtqueue here.

>
> >
> >> Compatibility is coming at a cost which is not scaling.
> >> If you attempt to scale, it breaks the future path forward to go without mediation.
> >>
> >>>>
> >>>> So eve growing these fields and optionally placement on configq
> >>>> doesn't really help and device builder to build it efficiently
> >>>> (without much predictability).
> >>>
> >>> Config queue is not the only choice, we have a lot of other choices (for example
> >>> PASID may help to reduce the on-chip resources).
> >>>
> >> PASID is for process isolation security construct, it is not for transportation.
> >
> > I meant with PASID you don't even need a BAR.
> >
> Not sure I fully understand, but my guess is, this is coming from
> mediation thought process.

Not actually. I meant, for example without PASID, for modern devices,
the common cfg structure must be placed on a BAR through MMIO.

But with the help of PASID, it could stay in the memory and the device
can fetch it via DMA. The help to reduce the resources for registers.

>
> >> Same, SIOV and VFs do not prefer mediation going forward in the use cases we come across.
> >
> > Unless you don't want to provide VF compatibility for SIOV.
> I do not understand what compatibility is this between which two elements?
> VF has its identify currently and SIOV will have its own identity in future.

Ok, so it really depends on how to implement the transport for SIOV. I
guess it won't use registers/BAR any more. If this is true, if we want
to run a virtio-pci driver, it needs some mediation.

It works like what you proposed here. Someday, SR-IOV or BAR transport
will become "legacy", but we need a way to make the "legacy virtio-pci
driver" work.

>
> >>>
> >>> Just to be sure we're on the same page. The proposal of both you and mine are
> >>> based on the adminq for PF not VF. The reason is obvious:
> >>> adminq per VF won't work without PASID, since it would have security issues.
> >>>
> >> The current proposal for legacy should be seen as separate and not to intermix with per VF based configuration queue.
> >>
> >> adminvq/config/control vq per VF and SIOV both devices will work without PASID (unrelated to legacy).
> >> Because they are like any other queue, such as txq, rxq, cvq. All of these queues are non-mediated today for PF, VF devices.
> >> (And so additional configq follows the natural model for config space access).
> >
> > This is only true if:
> >
> > 1) you don't want to provide any backward compatibility for current
> > PCI transport,
> There is no concept of backward compatibility for new features.
> There will be new driver anyway in the guest.
>
> yes, I don't see a point in mediating 1.x config space for existing
> drivers as it is requires mediation.
>
> > this means you need to use new drivers in the guest
> > 2) you don't want to do live migration
> >
> Not sure how you assert this.
>
> Live migration at VF level for the passthrough device can be just fine
> without any mediation as long as all the parameters on src and dst side
> match.
> We already discussed in v0, so I prefer to avoid this again and focus on
> the patch in discussion.

I agree we are far from the topic of legacy support. But my point
stands still. We can discuss when live migration patches were posted
by somebody.

>
> > If you need one of the above, PASID is a must.
>
> A PCI VF passthrough device LM without mediation is achievable without
> PASID for all the existing defined features using config space.
>
> and also possible when new features are communicated over configq/cmdq.
>
> Transporting existing config space using some additional queue is _not_
> the objective, because building a scalable system where device has zero
> knowledge about needed backward compatibility is hard.
> I provided the example of 100 Vfs with 30 and (30+40) bytes config
> layout previously.
>
> Yet we are repeating and diverging from the discussion, that we should
> not intermix:
> 1. How to access legacy registers of the VF

With what you proposed, we don't even need legacy registers for VF. We
just need to keep its semantic via the admin virtqueue commands.

> vs
> 2. How to access existing or new configuration of the VF
>
> Because both have very different requirements.
>

Thanks