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

["Michael S. Tsirkin" <mst@redhat.com>]

On Mon, May 22, 2023 at 08:54:44AM +0800, Jason Wang wrote:
> 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

Yes this is what I suggested.

> >
> >
> >
> > --
> > MST
> >