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

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

On Mon, Jun 05, 2023 at 01:27:04PM +0000, Parav Pandit wrote:
> 
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Monday, June 5, 2023 1:52 AM
> 
> [..]
> > > > E.g. with 1.x using VIRTIO_F_NOTIF_CONFIG_DATA - probably not.
> > > >
> > > Not really because legacy doesn't have that feature.
> > > Legacy notifications are subset of 1.x feature.
> > > This was also discussed.
> > 
> > So for legacy such hardware will likely not want to use the same BAR.
> > 
> I don't follow this comment in conjunction with your below comment about offset 0.
> If we consider offset 0, legacy will use the same notification area as 1.x, right?
> So I am confused on "hardware don't want to use the same "BAR".

Never mind,

> > 
> > > > Let's just say, we are not in the position where we dictate these
> > > > things to OS vendors yet.  One of motivations of the spec is making
> > > > writing drivers easier so let's add an option for that pls.
> > > >
> > > There is no need to add multiple options at this point when the hw vendors
> > are not going to implement.
> > > When we find another vendor to implement, we can always add the new
> > method.
> > > Marvell and Nvidia both are forwarding with AQ at present proposal.
> > >
> > > > > VF has BAR for driver notifications. This BAR is used for 1.x and legacy.
> > > >
> > > > But it's the same piece of silicon - be it in PF or in VF.
> > > > I really don't see why it matters.
> > > >
> > > In same piece of silicon one needs to open yet another PCI steering entry to
> > tell that treat this for VF.
> > > This double size programming is useless.
> > >
> > > Yet again we are discussing same point as before.
> > >
> > > > > There is no need to ask vendors to build new HW to put driver
> > > > > notifications
> > > > on the PF BAR for VFs, when it already exists on the VF.
> > > > I'm talking about giving vendors an option not asking them.
> > > >
> > > Ok. Understood.
> > > When a vendor prefers to implement such cross hw forwarding it can be done.
> > > There isn't a desire to burn and program extra hw for functionality that
> > already exists on the VF.
> > > Hence, VF's doorbell on PF can be parked for the future option when one
> > actually wants to implement it.
> > 
> > I reached out to windows team to know how big a problem this is. Let's see.
> > 
> > 
> Ok. Linux didn't had multi-function driver support and infra was added.
> So even if the facility may not be there, or if the current IPR facility they may not want to use,
> 
> In future OS vendor itself can provide the new APIs.
> I certainly don't see being blocked on this.
> 
> But good to get feedback on there current state of OS.
> 
> > I have another idea meanwhile: VF already has notification capability. The only
> > issue is offset has to be queried through modern BAR.  How about we agree that
> > for legacy, we just agree that offset 0 is used?  Does not sound like a big
> > sacrifice of memory to me, and we completely avoid the new command and the
> > need to tie PF and VF.
> 
> It will not work for the SIOV which shouldn't replicate this capability (and some other cap) so many times, are you ok with it?

Yes - SIOV has both command and BAR in the PF.

> Can you explain the motivation of : why querying notification offset via the group member PF is a problem, if there is."

I tried already, I can repeat if you like:

So, I am thinking of a model of using a tiny stub driver for the member.
Control path driver is bigger as it operates AQ command, that would be
the owner driver.  E.g. for PCI this driver needs to map BAR offsets.
Then it handles all data path for this member: driver and device
notifications.  It is clean and easy on all systems assuming you know
offsets at device probe.  But getting it in software from another driver
(owner driver) is AFAIK tricky on some systems.

Generally solutions can be found, they can be more or less robust, and
if systems designers have to overcome obstacles in implementing virtio
they will just go to a competing interface as opposed to jump through
hoops and work on extending either the OS or the virtio spec.

Yes I know your sales department tells you there's no demand for this
feature on anything that is not linux/x86 and maybe arm, and maybe this
means there won't ever be either, but let's just not raise this again
can we? E.g. there was some interest from microsoft at some point,
I think there was a reorg and that project died but hey you never know.
Anyway, you asked I am trying to answer.

The cleanest way is just doing things consistently through one device.
In this case for data path it has to be the VF, so let's just make this
part simple and self contained.


> I don't see a problem with AQ that covers VF,SF/SIOV for legacy.
> 
> Proposed AQ command is cheap and covers both VF, SF/SIOV.

Well for SIOV BAR has to be the owner BAR. This was exactly the reason I
asked for this. If it's not to be I think this is my second preference:
no new command at all, and SIOV will come up with its own.

-- 
MST