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

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

On Fri, May 19, 2023 at 04:37:16PM +0000, Parav Pandit wrote:
> 
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Friday, May 19, 2023 2:07 AM
> > 
> > On Thu, May 18, 2023 at 03:42:19PM -0400, Michael S. Tsirkin wrote:
> > > > > Does this sound like a reasonable
> > > > > compromize to you?
> > > >
> > > > Splitting proposed one command to two commands, 1. one for accessing
> > > > legacy common config 2. second for accessing legacy device specific
> > > > config
> > > >
> > > > seems fine to me as below.
> > > >
> > > > So we will have total 5 commands (instead of 3).
> > > >
> > > > 1. legacy common config read
> > > > 2. legacy common config write
> > > >
> > > > 3. legacy device config read
> > > > 4. legacy device config write
> > > > 5. query device notification area
> > > >
> > > > #1 and #3 same cmd signature but different opcode.
> > > > #2 and #4 same cmd signature but different opcode.
> > > >
> > >
> > > Sounds reasonable. Jason?
> > >
> > > notification thing needs more thought I feel though.
> > > It feels weirdly bolted on, but I can't put my finger on what's wrong
> > > exactly yet. Will think it over.
> > 
> > 
> > So with a fresh mind, at least three things:
> > 
> > 1. given driver attaches to the PF, it should be possible to forward
> >    notifications there, as opposed to individual VFs.  NumVFs is 16 bit so
> >    it will fit in a 32 bit write together with VQ index.
> >
> 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?

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.


> 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.



> > 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. 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? Let's do software with
maybe a couple of features such as VIRTIO_NET_F_LEGACY_HEADER.


-- 
MST