[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/1] hw/nvme: fix missing cq eventidx update
From: |
Klaus Jensen |
Subject: |
Re: [PATCH 1/1] hw/nvme: fix missing cq eventidx update |
Date: |
Thu, 8 Dec 2022 11:20:06 +0100 |
On Dec 8 11:14, Philippe Mathieu-Daudé wrote:
> On 8/12/22 09:29, Klaus Jensen wrote:
> > From: Klaus Jensen <k.jensen@samsung.com>
> >
> > Prior to reading the shadow doorbell cq head, we have to update the
> > eventidx. Otherwise, we risk that the driver will skip an mmio doorbell
> > write. This happens on riscv64, as reported by Guenter.
> >
> > Adding the missing update to the cq eventidx fixes the issue.
> >
> > Fixes: 3f7fe8de3d49 ("hw/nvme: Implement shadow doorbell buffer support")
> > Reported-by: Guenter Roeck <linux@roeck-us.net>
> > Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> > ---
> > hw/nvme/ctrl.c | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
> > index e54276dc1dc7..1192919b4869 100644
> > --- a/hw/nvme/ctrl.c
> > +++ b/hw/nvme/ctrl.c
> > @@ -1331,6 +1331,13 @@ static inline void nvme_blk_write(BlockBackend *blk,
> > int64_t offset,
> > }
> > }
> > +static void nvme_update_cq_eventidx(const NvmeCQueue *cq)
> > +{
> > + pci_dma_write(&cq->ctrl->parent_obj, cq->ei_addr, &cq->head,
>
> 'parent_obj' is a private field. Better to use the QOM accessor:
>
> pci_dma_write(PCI_DEVICE(&cq->ctrl), cq->ei_addr, &cq->head,
Ah yes, of course. I think we have a couple of other ->parent_obj
accesses that we should fix also.
>
> > + sizeof(cq->head));
> > + trace_pci_nvme_eventidx_cq(cq->cqid, cq->head);
>
> Surprisingly the trace event format was already present in Jinhao respin...
> https://lore.kernel.org/all/YqsIh+OUcWnHU3zp@apples/T/
>
> Could we move the event before the call?
>
Makes sense.
> > +}
> > +
> > static void nvme_update_cq_head(NvmeCQueue *cq)
> > {
> > pci_dma_read(&cq->ctrl->parent_obj, cq->db_addr, &cq->head,
> > @@ -1351,6 +1358,7 @@ static void nvme_post_cqes(void *opaque)
> > hwaddr addr;
> > if (n->dbbuf_enabled) {
> > + nvme_update_cq_eventidx(cq);
> > nvme_update_cq_head(cq);
> > }
>
signature.asc
Description: PGP signature