On Dec 8 11:14, Philippe Mathieu-Daudé wrote: > On 8/12/22 09:29, Klaus Jensen wrote: > > From: Klaus Jensen <[email protected]> > > > > 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 <[email protected]> > > Signed-off-by: Klaus Jensen <[email protected]> > > --- > > 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
