| Message ID | 20181113225734.8026-2-marc.zyngier@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | PCI: designware: Fixing MSI handling flow | expand |
On Tue, Nov 13, 2018 at 10:57:32PM +0000, Marc Zyngier wrote: > The dwc driver is showing an interesting level of brokeness, as it > insists on using the "enable" register to mask/unmask MSIs, meaning > that an MSIs being generated while the interrupt is in that "disabled" > state will simply be lost. > > Let's move to the MASK register, which offers the expected semantics. > > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> > --- > drivers/pci/controller/dwc/pcie-designware-host.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c > index 29a05759a294..c3aa8b5fb51d 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-host.c > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c > @@ -168,7 +168,7 @@ static void dw_pci_bottom_mask(struct irq_data *data) > bit = data->hwirq % MAX_MSI_IRQS_PER_CTRL; > > pp->irq_status[ctrl] &= ~(1 << bit); > - dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res, 4, > + dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_MASK + res, 4, > pp->irq_status[ctrl]); > } > > @@ -191,7 +191,7 @@ static void dw_pci_bottom_unmask(struct irq_data *data) > bit = data->hwirq % MAX_MSI_IRQS_PER_CTRL; > > pp->irq_status[ctrl] |= 1 << bit; > - dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res, 4, > + dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_MASK + res, 4, > pp->irq_status[ctrl]); > } > For this patch combined with the fix-up from: https://marc.info/?l=linux-pci&m=154218928401960&w=2 Tested-by: Niklas Cassel <niklas.cassel@linaro.org>
Hi, On 13/11/2018 22:57, Marc Zyngier wrote: > The dwc driver is showing an interesting level of brokeness, as it > insists on using the "enable" register to mask/unmask MSIs, meaning > that an MSIs being generated while the interrupt is in that "disabled" > state will simply be lost. > > Let's move to the MASK register, which offers the expected semantics. > > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> > --- > drivers/pci/controller/dwc/pcie-designware-host.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c > index 29a05759a294..c3aa8b5fb51d 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-host.c > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c > @@ -168,7 +168,7 @@ static void dw_pci_bottom_mask(struct irq_data *data) > bit = data->hwirq % MAX_MSI_IRQS_PER_CTRL; > > pp->irq_status[ctrl] &= ~(1 << bit); > - dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res, 4, > + dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_MASK + res, 4, > pp->irq_status[ctrl]); > } > > @@ -191,7 +191,7 @@ static void dw_pci_bottom_unmask(struct irq_data *data) > bit = data->hwirq % MAX_MSI_IRQS_PER_CTRL; > > pp->irq_status[ctrl] |= 1 << bit; > - dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res, 4, > + dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_MASK + res, 4, > pp->irq_status[ctrl]); > } > > I've tested this patch with the fix-up [1] some time ago, it worked fine under stress tests with a NVMe EP (MSI-X). [1] https://marc.info/?l=linux-pci&m=154218928401960&w=2 Tested-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c index 29a05759a294..c3aa8b5fb51d 100644 --- a/drivers/pci/controller/dwc/pcie-designware-host.c +++ b/drivers/pci/controller/dwc/pcie-designware-host.c @@ -168,7 +168,7 @@ static void dw_pci_bottom_mask(struct irq_data *data) bit = data->hwirq % MAX_MSI_IRQS_PER_CTRL; pp->irq_status[ctrl] &= ~(1 << bit); - dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res, 4, + dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_MASK + res, 4, pp->irq_status[ctrl]); } @@ -191,7 +191,7 @@ static void dw_pci_bottom_unmask(struct irq_data *data) bit = data->hwirq % MAX_MSI_IRQS_PER_CTRL; pp->irq_status[ctrl] |= 1 << bit; - dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res, 4, + dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_MASK + res, 4, pp->irq_status[ctrl]); }
The dwc driver is showing an interesting level of brokeness, as it insists on using the "enable" register to mask/unmask MSIs, meaning that an MSIs being generated while the interrupt is in that "disabled" state will simply be lost. Let's move to the MASK register, which offers the expected semantics. Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> --- drivers/pci/controller/dwc/pcie-designware-host.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)