Re: [PATCH 1/2] hw/nvme: Implement shadow doorbell buffer support

[John Levon <levon@movementarian.org>]

On Wed, Jun 15, 2022 at 11:33:02AM +0200, Klaus Jensen wrote:

> > BTW I'm surprised that this patch has just this:
> > 
> > +static void nvme_update_sq_eventidx(const NvmeSQueue *sq)
> > +{
> > +    pci_dma_write(&sq->ctrl->parent_obj, sq->ei_addr, &sq->tail,
> > +                  sizeof(sq->tail));
> > +}
> > 
> > Isn't this racy against the driver? Compare
> > https://github.com/spdk/spdk/blob/master/lib/nvmf/vfio_user.c#L1317
> > 
> > thanks
> > john
> 
> QEMU has full memory barriers on dma read/write, so I believe this is
> safe?

But don't you need to re-read the tail still, for example:


driver 			device

			eventidx is 3

write 4 to tail
			read tail of 4
write 5 to tail
read eventidx of 3
nvme_dbbuf_need_event (1)

			set eventidx to 4
			go to sleep

at (1), our tail update of 4->5 doesn't straddle the eventidx, so we don't send
any MMIO, and the device won't wake up. This is why the above code checks the
tail twice for any concurrent update.

regards
john