Re: [PATCH v3 0/3] transport-pci: Introduce legacy registers access using AQ

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

On Sun, Jun 04, 2023 at 02:10:16PM +0000, Parav Pandit wrote:
> 
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Sunday, June 4, 2023 9:56 AM
> > 
> > On Sun, Jun 04, 2023 at 01:41:54PM +0000, Parav Pandit wrote:
> > >
> > >
> > > > From: Michael S. Tsirkin <mst@redhat.com>
> > > > Sent: Sunday, June 4, 2023 9:34 AM
> > > >
> > > > On Fri, Jun 02, 2023 at 11:36:01PM +0300, Parav Pandit wrote:
> > > > > This short series introduces legacy registers access commands for
> > > > > the owner group member PCI PF to access the legacy registers of the
> > member VFs.
> > > >
> > > > Note that some work will be needed here to fix up grammar and
> > > > spelling mistakes.
> > > >
> > > I already verified using codespell but I guess it missed few.
> > > If you have specific already identified, let me know.
> > 
> > I remember seeing something about a drier somewhere :)
> >
> Yes. I will fix it.
>  
> > > If not I will run a different checker.
> > 
> > Yea, pls check grammar too.
> > 
> Ok.
> 
> > > > > If in future any SIOV devices to support legacy registers, they
> > > > > can be easily supported using same commands by using the group
> > > > > member identifiers of the future SIOV devices.
> > > >
> > > > Yes, with the exception of
> > > > VIRTIO_ADMIN_CMD_LQ_NOTIFY_QUERY - currently refers to VF BAR,
> > > > subfunctions do not have it.
> > > A subfunction will also have its own BAR carved out from the PF BAR.
> > > A subfunction definition will be as contains as possible.
> > >
> > > > Can we find a way to have it in the PF BAR instead?
> > > At subfution level also it will be a BAR number, which will map to the PF BAR.
> > 
> > exactly. why not support this for PF too?
> > 
> > > > E.g. the notification can include VF# + VQ#?
> > > > At least as an option?
> > > No. we discussed this before to have each device on its own BAR. Hence no
> > VF# in the doorbell.

doorbell == available notification? yes in fact it's not
strictly needed since PF can use a different address for
each VF.


> > 
> > you said that you want it but without much in the way of explanation. I'm still
> > not convinced it's a workable interface. 
> You said "AQ is just fine" in v2, this is contradictory to what you write above.
> We cannot progress with dual comments like this.

I am talking about the idea of exposing legacy VQ notifications in the
PF BAR. To me it looks nicely symmetrical, all legacy emulation goes
through PF, and I think it simplifies a bunch of driver code moving some
complexity into hardware which is what you seem to be intent on?

> > Will it help if I get some feedback from
> > windows driver team on the design?
> >
> May be. I am not sure, given that these drivers are not inbox, they are not in purview of the OS vendor.

Exactly a reason we might not see new APIs?

> And OS vendor improves the OS APIs in the future for new use cases.
> 
> Even the SR-IOV model in Windows is not elaborate as Linux AFAIK.
> So I am not sure how much value add we will have.
> But sure, better to have their view.
> Will you please add them to the discussion here or I should check?

I don't think they are subscribed to the list, so I can't.
Will talk to them.

> Did anyone use vdpa acceleration on Windows?
> Do you have data points that someone wants VF acceleration on Windows hypervisor?
> If you bring the use case, I would like to see there commitment to develop the code also and it will be great to have virtio there.

We are talking about an interface that will be used tens of years from
now. If the interface is awkward to use on some platforms let's make it
easier not hide behind excuses of no demand to fix it as of last Friday.
Device vendors can choose not to support some platforms, that is their
decision. As a compromise, consider device exposing this both in a PF
and in a VF and let's allow driver to use what's most convenient?

-- 
MST