Re: [PATCH] VIRTIO_F_USED_EVENT_AUTO_DISABLE: add new used buffer notification suppression mechanism

Jason Wang
2024-07-04T05:37:59Z

On Wed, Jul 3, 2024 at 5:09 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Wed, Jul 03, 2024 at 04:47:41PM +0800, Jason Wang wrote:
> > On Wed, Jul 3, 2024 at 4:36 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> > >
> > > On Wed, Jul 03, 2024 at 03:59:11PM +0800, Jason Wang wrote:
> > > > On Wed, Jul 3, 2024 at 3:37 PM Lege Wang <lege.wang@jaguarmicro.com> wrote:
> > > > >
> > > > > hi,
> > > > >
> > > > > > > >
> > > > > > > > 1) With the event index, as long as the used index doesn't pass used
> > > > > > > > events you don't need to fetch even index every time
> > > > > > > Yeah, I agree VIRTIO_F_EVENT_IDX could help here, but I think it's a relatively
> > > > > > > crude mechanism, I have two questions below:
> > > > > > > 1. Used event notification suppression structure is still located in
> > > > > > > host memory(in dpu case), I'm not sure whether used_event would
> > > > > > > be allowed to update in the running of one virtio device,
> > > > > >
> > > > > > What did you mean by "update" here?
> > > > > I mean "modify".
> > > > >
> > > > > >
> > > > > > > If it's allowed,
> > > > > > > seems devices still need to fetch newest used_event info timely.
> > > > > >
> > > > > > It depends on how you define "timely", I mean unless the used event is
> > > > > > not crossed, you don't need to fetch it from the main memory?
> > > > > Yes, I got your point here.
> > > > > >
> > > > > > But basically, I meant putting used_event in a cap/register other than
> > > > > > inventing something completely new.
> > > > > Sorry, I don't get your point here. What does " cap/register " mean, used_event
> > > > > Is located in main memory, right?
> > > >
> > > > I meant something like this.
> > > >
> > > > Introduce a capability to allow the driver to duplicate used_event in
> > > > the register. And say when the feature is negotiated, the driver MUST
> > > > update both used_event in the memory and the register.
> > > >
> > > > Not saying it can work, but we need to know why it can't work like this.
> > >
> > > Well I feel if you are proposing a mechanism it's up to you to
> > > explain how it works without races.
> >
> > I agree, that's why I'm saying "Not saying it can work". But what I
> > meant is really to find a way to reuse the event index instead of
> > introducing something completely new.
> >
> > > The current notification suppression works because the read
> > > of the notification by the device flushes out used buffer writes by
> > > the device.
> >
> > You meant read after write is ordered by PCI?
>
> pci read responses do not bypass writes, yes.
>
> > > If you move it to a separate domain (such as the pci bar of the device)
> > > this no longer holds.
> >
> > Would this be implementation specific details or could it be done by PCI?
>
> what do you want done by PCI? Generally if things are in one place
> they are easier to synchronize, if you spread them around you
> need to synchronize them.

I meant the synchronization looks more like an implementation detail
in the device. Synchronizing with device internal logic should be
simpler than with PCI/memory.

For example, did you mean the synchronization between driver write to
register (PCI write) and device read from that (internal logic). If
needed, an implementation needs to serialize those two, then we are
probably fine.

Thanks