| Message ID | 20221212113215.33135-5-its@irrelevant.dk |
|---|---|
| State | New |
| Headers | show |
| Series | hw/nvme: fix broken shadow doorbells on some platforms | expand |
On 12/12/22 12:32, 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") > Cc: qemu-stable@nongnu.org > Cc: qemu-riscv@nongnu.org > Reported-by: Guenter Roeck <linux@roeck-us.net> > Signed-off-by: Klaus Jensen <k.jensen@samsung.com> > --- > hw/nvme/ctrl.c | 17 +++++++++++++++-- > 1 file changed, 15 insertions(+), 2 deletions(-) > > diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c > index 8af70f0216f0..3df29ea68b2f 100644 > --- a/hw/nvme/ctrl.c > +++ b/hw/nvme/ctrl.c > @@ -1334,10 +1334,22 @@ static inline void nvme_blk_write(BlockBackend *blk, int64_t offset, > } > } > static void nvme_update_cq_head(NvmeCQueue *cq) > { > - pci_dma_read(PCI_DEVICE(cq->ctrl), cq->db_addr, &cq->head, > - sizeof(cq->head)); > + uint32_t v; > + > + pci_dma_read(PCI_DEVICE(cq->ctrl), cq->db_addr, &v, sizeof(v)); > + > + cq->head = le32_to_cpu(v); > Isn't this be part of the previous patch? > trace_pci_nvme_update_cq_head(cq->cqid, cq->head); > }
On Dec 12 12:37, Philippe Mathieu-Daudé wrote: > On 12/12/22 12:32, 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") > > Cc: qemu-stable@nongnu.org > > Cc: qemu-riscv@nongnu.org > > Reported-by: Guenter Roeck <linux@roeck-us.net> > > Signed-off-by: Klaus Jensen <k.jensen@samsung.com> > > --- > > hw/nvme/ctrl.c | 17 +++++++++++++++-- > > 1 file changed, 15 insertions(+), 2 deletions(-) > > > > diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c > > index 8af70f0216f0..3df29ea68b2f 100644 > > --- a/hw/nvme/ctrl.c > > +++ b/hw/nvme/ctrl.c > > @@ -1334,10 +1334,22 @@ static inline void nvme_blk_write(BlockBackend *blk, int64_t offset, > > } > > } > > > > static void nvme_update_cq_head(NvmeCQueue *cq) > > { > > - pci_dma_read(PCI_DEVICE(cq->ctrl), cq->db_addr, &cq->head, > > - sizeof(cq->head)); > > + uint32_t v; > > + > > + pci_dma_read(PCI_DEVICE(cq->ctrl), cq->db_addr, &v, sizeof(v)); > > + > > + cq->head = le32_to_cpu(v); > > Isn't this be part of the previous patch? > Argh, I screwed it up. I'll fix it.
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c index 8af70f0216f0..3df29ea68b2f 100644 --- a/hw/nvme/ctrl.c +++ b/hw/nvme/ctrl.c @@ -1334,10 +1334,22 @@ static inline void nvme_blk_write(BlockBackend *blk, int64_t offset, } } +static void nvme_update_cq_eventidx(const NvmeCQueue *cq) +{ + uint32_t v = cpu_to_le32(cq->head); + + trace_pci_nvme_update_cq_eventidx(cq->cqid, cq->head); + + pci_dma_write(PCI_DEVICE(cq->ctrl), cq->ei_addr, &v, sizeof(v)); +} + static void nvme_update_cq_head(NvmeCQueue *cq) { - pci_dma_read(PCI_DEVICE(cq->ctrl), cq->db_addr, &cq->head, - sizeof(cq->head)); + uint32_t v; + + pci_dma_read(PCI_DEVICE(cq->ctrl), cq->db_addr, &v, sizeof(v)); + + cq->head = le32_to_cpu(v); trace_pci_nvme_update_cq_head(cq->cqid, cq->head); } @@ -1355,6 +1367,7 @@ static void nvme_post_cqes(void *opaque) hwaddr addr; if (n->dbbuf_enabled) { + nvme_update_cq_eventidx(cq); nvme_update_cq_head(cq); }