[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/2] hw/nvme: Implement shadow doorbell buffer support
From: |
John Levon |
Subject: |
Re: [PATCH 1/2] hw/nvme: Implement shadow doorbell buffer support |
Date: |
Wed, 15 Jun 2022 12:45:56 +0100 |
On Wed, Jun 15, 2022 at 07:22:22PM +0800, Jinhao Fan wrote:
> >>> Isn't this racy against the driver? Compare
> >>> https://github.com/spdk/spdk/blob/master/lib/nvmf/vfio_user.c#L1317
> >>
> >> 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:
>
> I think we also have a check for concurrent update on the tail. After writing
> eventidx, we read the tail again. It is here:
>
> @@ -5854,6 +5943,11 @@ static void nvme_process_sq(void *opaque)
> req->status = status;
> nvme_enqueue_req_completion(cq, req);
> }
> +
> + if (n->dbbuf_enabled) {
> + nvme_update_sq_eventidx(sq);
> + nvme_update_sq_tail(sq);
> + }
Ah, and we go around the loop another time in this case.
> > 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
>
> Therefore, at this point, we read the tail of 5.
The driver could still update the tail after the nvme_update_sq_tail() above.
However, the driver ordering (read tail, then eventidx), does mean that it would
then do an mmio write, so yes, this looks safe, thank you.
regards
john
- Re: [PATCH 1/2] hw/nvme: Implement shadow doorbell buffer support, (continued)
- Re: [PATCH 1/2] hw/nvme: Implement shadow doorbell buffer support, Jinhao Fan, 2022/06/14
- Re: [PATCH 1/2] hw/nvme: Implement shadow doorbell buffer support, Keith Busch, 2022/06/14
- Re: [PATCH 1/2] hw/nvme: Implement shadow doorbell buffer support, Jinhao Fan, 2022/06/14
- Re: [PATCH 1/2] hw/nvme: Implement shadow doorbell buffer support, Klaus Jensen, 2022/06/15
- Re: [PATCH 1/2] hw/nvme: Implement shadow doorbell buffer support, Jinhao Fan, 2022/06/15
- Re: [PATCH 1/2] hw/nvme: Implement shadow doorbell buffer support, Klaus Jensen, 2022/06/15
- Re: [PATCH 1/2] hw/nvme: Implement shadow doorbell buffer support, John Levon, 2022/06/15
- Re: [PATCH 1/2] hw/nvme: Implement shadow doorbell buffer support, Klaus Jensen, 2022/06/15
- Re: [PATCH 1/2] hw/nvme: Implement shadow doorbell buffer support, John Levon, 2022/06/15
- Re: [PATCH 1/2] hw/nvme: Implement shadow doorbell buffer support, Jinhao Fan, 2022/06/15
- Re: [PATCH 1/2] hw/nvme: Implement shadow doorbell buffer support,
John Levon <=
[PATCH 2/2] hw/nvme: Add trace events for shadow doorbell buffer, Jinhao Fan, 2022/06/07