On Mon, Jan 16, 2023 at 10:14:07PM +0100, Klaus Jensen wrote: [ ... ] > > I noticed that the Linux driver does not use the INTMS/INTMC registers > to mask interrupts on the controller while processing CQEs. While not > required by the spec, it is *recommended* in setups not using MSI-X to > reduce the risk of spurious and/or missed interrupts. > > With the patch below, running 100 boot iterations, no timeouts were > observed on QEMU emulated riscv64 or mips64. > > No changes are required in the QEMU hw/nvme interrupt logic. >
Yes, but isn't that technically similar to dropping the interrupt request and raising it again, or in other words pulsing the interrupt ? I still don't understand why the (level triggered) interrupt pin would require pulsing in the first place. Thanks, Guenter > > diff --git i/drivers/nvme/host/pci.c w/drivers/nvme/host/pci.c > index b13baccedb4a..75f6b87c4c3f 100644 > --- i/drivers/nvme/host/pci.c > +++ w/drivers/nvme/host/pci.c > @@ -1128,6 +1128,27 @@ static inline int nvme_poll_cq(struct nvme_queue > *nvmeq, > } > > static irqreturn_t nvme_irq(int irq, void *data) > +{ > + struct nvme_queue *nvmeq = data; > + struct nvme_dev *dev = nvmeq->dev; > + u32 mask = 1 << nvmeq->cq_vector; > + irqreturn_t ret = IRQ_NONE; > + DEFINE_IO_COMP_BATCH(iob); > + > + writel(mask, dev->bar + NVME_REG_INTMS); > + > + if (nvme_poll_cq(nvmeq, &iob)) { > + if (!rq_list_empty(iob.req_list)) > + nvme_pci_complete_batch(&iob); > + ret = IRQ_HANDLED; > + } > + > + writel(mask, dev->bar + NVME_REG_INTMC); > + > + return ret; > +} > + > +static irqreturn_t nvme_irq_msix(int irq, void *data) > { > struct nvme_queue *nvmeq = data; > DEFINE_IO_COMP_BATCH(iob); > @@ -1602,12 +1623,13 @@ static int queue_request_irq(struct nvme_queue *nvmeq) > { > struct pci_dev *pdev = to_pci_dev(nvmeq->dev->dev); > int nr = nvmeq->dev->ctrl.instance; > + irq_handler_t handler = pdev->msix_enabled ? nvme_irq_msix : nvme_irq; > > if (use_threaded_interrupts) { > return pci_request_irq(pdev, nvmeq->cq_vector, nvme_irq_check, > - nvme_irq, nvmeq, "nvme%dq%d", nr, nvmeq->qid); > + handler, nvmeq, "nvme%dq%d", nr, nvmeq->qid); > } else { > - return pci_request_irq(pdev, nvmeq->cq_vector, nvme_irq, > + return pci_request_irq(pdev, nvmeq->cq_vector, handler, > NULL, nvmeq, "nvme%dq%d", nr, nvmeq->qid); > } > } > >