Re: [virtio-comment] RE: [PATCH v19] virtio-net: support inner header hash

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

On Wed, Jun 28, 2023 at 05:06:40PM +0000, Parav Pandit wrote:
> 
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Wednesday, June 28, 2023 12:46 PM
> 
> > Actually, my preblem is exactly if there's no one single interface that can do
> > everything.
> >
> Extended config space reading via single DMA interface is just enough.
> 
> > 
> > Two interfaces that do the same might seem redundant but there could be good
> > reasons for this, in this case legacy.
> >
> But legacy should not force it to newer interface. More below.
>  
> > See the difference?
> > 
> 
> > > I didn't propose two interfaces. Let me explain again.
> > >
> > > I proposed,
> > > a. all new fields to go via vq (single interface) b. all existing
> > > fields to stay in cfg space (single interface) c. if one wants to
> > > optimize optionally existing fields can utilize (same flexibility how you asked
> > for PF and VF notification).
> > >
> > >
> > > > We already need config space with provisioning.
> > > Provisioning does provision features, config space and control parameters,
> > msix vectors and more.
> > > Provisioning does _not_ need config space.
> > 
> > I mean it needs to provision it.
> > 
> No matter there is config space of dma, a device to be provisioned.
> 
> > > > In fact e.g. on Linux we want drivers to keep doing virtio_cread()
> > > > and all the DMA tricks should be hidden behind the scenes. You don't
> > > > like it that config space is on-chip? Build an interface for it not
> > > > to be on chip. There is simply no downside to that.
> > > >
> > > This is the ask to build software interface between driver and device to not
> > demand on-chip config space more importantly _predictably_.
> > 
> > Absolutely. So we would say that driver SHOULD use the DMA interface if it is
> > present.
> > 
> I am asking for driver MUST use the DMA interface for _extended_ config space.
> Why?
> For two reasons.
> 
> 1. Because than device is 100% sure that it does not fulfill MMIO needs
> 2. Single interface to access extended config space, which is what you asked for.
> 3. Single driver code because there is single way to get it.
> 

Right. But I think there's a better way.
Just say that any feature in MMIO MUST be also in DMA.
So any modern driver will have no reason at all to use MMIO - DMA
is a superset.


> > > > I came back after a small break and I still see this discussion that
> > > > keeps playing out as a big distraction.
> > > > In particular where are you doing to store the things that are for DMA?
> > > > In system RAM? We don't *have* anywhere in system RAM to store this,
> > > > we will need an interface to allocate system RAM and pass it to
> > > > device for this idea to work. So in 1.3 where we won't have this,
> > > > there is much less of an excuse to use a vq.
> > > To store what?
> > > CVQ is already there to do GET and SET operation.
> > 
> > To store whatever value CVQ is requesting. Where is it going to come from?
> >
> It is going to come from the device memory that does not have hard requirements of MMIO accessibility.
> For example slow flash memory.
>  
> > Asynchronous is just harder for software.  Look at the hoops virtio net has to
> > jump through to do control path over cvq and weep. Compare to single liner
> > virtio_cread.
> > 
> > I happen to maintain Linux drivers too, so I care.
> > 
> It is harder for first time if no such construct exists; and it is clearly not the case.
> This patch is already adding CVQ get and set command.

I expect get to mostly go unused.

> Async behavior already built into the virtio spec.
> Extending it for extended config space just make device more cheaper to build.
> Software cost is paid only once at device init time just like CVQ, AQ and more.
> 
> > > >
> > > > Please let people just focus on fixing config space instead of
> > > > temporary cvq hacks.
> > >
> > > The ask is to have predictable sizing for existing defined config space field and
> > not keep infinitely growing by two interfaces.
> > 
> > I don't know what "predictable sizing" is or why does it matter.
> > 
> Because when device has two ways to access config space, it always have to account and build the interface that, hey some driver will not use DMA.
> Hence have it always in the MMIO accessible area.

If we say that drivers should use DMA, they will.  If we additionally
explain that some features might not be in MMIO no sane driver will use
MMIO if it does not have to.
Or if it does then it has violated the spec and will get less features?




> > I get the value of reduced MMIO register map.
> > I get that the only way to get that appears to be to pass values around using
> > DMA.
> >
> Ok. great.
>  
> > I don't think we need to throw away config space - just use DMA to access it.
> > 
> > And "existing defined" here seems to be at a completely random point in time.
> > If someone wants to have fields in MMIO, I see no reason not to.
> 
> This demands two ways of access and that conflicts with your point of desire to do single way.
> I proposed single way, extended config space via DMA interface, that is really as simple as that.


My desire is to have a single way that works for everything.
We have legacy so we will have legacy ways too, these
might not work for everything.

-- 
MST