* FAILED: patch "[PATCH] PCI: dwc: Move interrupt acking into the proper callback" failed to apply to 4.14-stable tree
@ 2019-01-15 8:24 gregkh
2019-01-16 23:43 ` Niklas Cassel
0 siblings, 1 reply; 6+ messages in thread
From: gregkh @ 2019-01-15 8:24 UTC (permalink / raw)
To: marc.zyngier, gustavo.pimentel, lorenzo.pieralisi, niklas.cassel,
svarbanov, tpiepho
Cc: stable
The patch below does not apply to the 4.14-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable@vger.kernel.org>.
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From 3f7bb2ec20ce07c02b2002349d256c91a463fcc5 Mon Sep 17 00:00:00 2001
From: Marc Zyngier <marc.zyngier@arm.com>
Date: Tue, 13 Nov 2018 22:57:34 +0000
Subject: [PATCH] PCI: dwc: Move interrupt acking into the proper callback
The write to the status register is really an ACK for the HW,
and should be treated as such by the driver. Let's move it to the
irq_ack() callback, which will prevent people from moving it around
in order to paper over other bugs.
Fixes: 8c934095fa2f ("PCI: dwc: Clear MSI interrupt status after it is handled,
not before")
Fixes: 7c5925afbc58 ("PCI: dwc: Move MSI IRQs allocation to IRQ domains
hierarchical API")
Link: https://lore.kernel.org/linux-pci/20181113225734.8026-1-marc.zyngier@arm.com/
Reported-by: Trent Piepho <tpiepho@impinj.com>
Tested-by: Niklas Cassel <niklas.cassel@linaro.org>
Tested-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
Tested-by: Stanimir Varbanov <svarbanov@mm-sol.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
[lorenzo.pieralisi@arm.com: updated commit log]
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: stable@vger.kernel.org
diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 610a5e018d27..0fa9e8fdce66 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -99,9 +99,6 @@ irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
(i * MAX_MSI_IRQS_PER_CTRL) +
pos);
generic_handle_irq(irq);
- dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_STATUS +
- (i * MSI_REG_CTRL_BLOCK_SIZE),
- 4, 1 << pos);
pos++;
}
}
@@ -200,14 +197,18 @@ static void dw_pci_bottom_unmask(struct irq_data *data)
static void dw_pci_bottom_ack(struct irq_data *d)
{
- struct msi_desc *msi = irq_data_get_msi_desc(d);
- struct pcie_port *pp;
+ struct pcie_port *pp = irq_data_get_irq_chip_data(d);
+ unsigned int res, bit, ctrl;
unsigned long flags;
- pp = msi_desc_to_pci_sysdata(msi);
+ ctrl = d->hwirq / MAX_MSI_IRQS_PER_CTRL;
+ res = ctrl * MSI_REG_CTRL_BLOCK_SIZE;
+ bit = d->hwirq % MAX_MSI_IRQS_PER_CTRL;
raw_spin_lock_irqsave(&pp->lock, flags);
+ dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_STATUS + res, 4, 1 << bit);
+
if (pp->ops->msi_irq_ack)
pp->ops->msi_irq_ack(d->hwirq, pp);
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: FAILED: patch "[PATCH] PCI: dwc: Move interrupt acking into the proper callback" failed to apply to 4.14-stable tree
2019-01-15 8:24 FAILED: patch "[PATCH] PCI: dwc: Move interrupt acking into the proper callback" failed to apply to 4.14-stable tree gregkh
@ 2019-01-16 23:43 ` Niklas Cassel
2019-01-17 9:58 ` Marc Zyngier
0 siblings, 1 reply; 6+ messages in thread
From: Niklas Cassel @ 2019-01-16 23:43 UTC (permalink / raw)
To: gregkh
Cc: marc.zyngier, gustavo.pimentel, lorenzo.pieralisi, svarbanov,
tpiepho, stable
On Tue, Jan 15, 2019 at 09:24:37AM +0100, gregkh@linuxfoundation.org wrote:
>
> The patch below does not apply to the 4.14-stable tree.
> If someone wants it applied there, or to any other stable or longterm
> tree, then please email the backport, including the original git commit
> id to <stable@vger.kernel.org>.
Hello Marc,
You said that you were going to backport this to 4.14, right?
IIRC, you said something about creating a simple dw_pci_bottom_ack()
(since 4.14 lacks a ack() function).
Kind regards,
Niklas
>
> thanks,
>
> greg k-h
>
> ------------------ original commit in Linus's tree ------------------
>
> From 3f7bb2ec20ce07c02b2002349d256c91a463fcc5 Mon Sep 17 00:00:00 2001
> From: Marc Zyngier <marc.zyngier@arm.com>
> Date: Tue, 13 Nov 2018 22:57:34 +0000
> Subject: [PATCH] PCI: dwc: Move interrupt acking into the proper callback
>
> The write to the status register is really an ACK for the HW,
> and should be treated as such by the driver. Let's move it to the
> irq_ack() callback, which will prevent people from moving it around
> in order to paper over other bugs.
>
> Fixes: 8c934095fa2f ("PCI: dwc: Clear MSI interrupt status after it is handled,
> not before")
> Fixes: 7c5925afbc58 ("PCI: dwc: Move MSI IRQs allocation to IRQ domains
> hierarchical API")
> Link: https://lore.kernel.org/linux-pci/20181113225734.8026-1-marc.zyngier@arm.com/
> Reported-by: Trent Piepho <tpiepho@impinj.com>
> Tested-by: Niklas Cassel <niklas.cassel@linaro.org>
> Tested-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
> Tested-by: Stanimir Varbanov <svarbanov@mm-sol.com>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> [lorenzo.pieralisi@arm.com: updated commit log]
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: stable@vger.kernel.org
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 610a5e018d27..0fa9e8fdce66 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -99,9 +99,6 @@ irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
> (i * MAX_MSI_IRQS_PER_CTRL) +
> pos);
> generic_handle_irq(irq);
> - dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_STATUS +
> - (i * MSI_REG_CTRL_BLOCK_SIZE),
> - 4, 1 << pos);
> pos++;
> }
> }
> @@ -200,14 +197,18 @@ static void dw_pci_bottom_unmask(struct irq_data *data)
>
> static void dw_pci_bottom_ack(struct irq_data *d)
> {
> - struct msi_desc *msi = irq_data_get_msi_desc(d);
> - struct pcie_port *pp;
> + struct pcie_port *pp = irq_data_get_irq_chip_data(d);
> + unsigned int res, bit, ctrl;
> unsigned long flags;
>
> - pp = msi_desc_to_pci_sysdata(msi);
> + ctrl = d->hwirq / MAX_MSI_IRQS_PER_CTRL;
> + res = ctrl * MSI_REG_CTRL_BLOCK_SIZE;
> + bit = d->hwirq % MAX_MSI_IRQS_PER_CTRL;
>
> raw_spin_lock_irqsave(&pp->lock, flags);
>
> + dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_STATUS + res, 4, 1 << bit);
> +
> if (pp->ops->msi_irq_ack)
> pp->ops->msi_irq_ack(d->hwirq, pp);
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: FAILED: patch "[PATCH] PCI: dwc: Move interrupt acking into the proper callback" failed to apply to 4.14-stable tree
2019-01-16 23:43 ` Niklas Cassel
@ 2019-01-17 9:58 ` Marc Zyngier
2019-01-18 16:00 ` Niklas Cassel
0 siblings, 1 reply; 6+ messages in thread
From: Marc Zyngier @ 2019-01-17 9:58 UTC (permalink / raw)
To: Niklas Cassel, gregkh
Cc: gustavo.pimentel, lorenzo.pieralisi, svarbanov, tpiepho, stable
On 16/01/2019 23:43, Niklas Cassel wrote:
> On Tue, Jan 15, 2019 at 09:24:37AM +0100, gregkh@linuxfoundation.org wrote:
>>
>> The patch below does not apply to the 4.14-stable tree.
>> If someone wants it applied there, or to any other stable or longterm
>> tree, then please email the backport, including the original git commit
>> id to <stable@vger.kernel.org>.
>
> Hello Marc,
>
> You said that you were going to backport this to 4.14, right?
Me, or anyone else. Preferably someone who, like you, has the HW at hand (I don't).
> IIRC, you said something about creating a simple dw_pci_bottom_ack()
> (since 4.14 lacks a ack() function).
Indeed. Something like this:
diff --git a/drivers/pci/dwc/pcie-designware-host.c b/drivers/pci/dwc/pcie-designware-host.c
index bc3e2d8d0cce..f4f3eeee10af 100644
--- a/drivers/pci/dwc/pcie-designware-host.c
+++ b/drivers/pci/dwc/pcie-designware-host.c
@@ -45,8 +45,19 @@ static int dw_pcie_wr_own_conf(struct pcie_port *pp, int where, int size,
return dw_pcie_write(pci->dbi_base + where, size, val);
}
+static void dwc_irq_ack(struct irq_data *d)
+{
+ struct msi_desc *msi = irq_data_get_msi_desc(d);
+ struct pcie_port *pp = msi_desc_to_pci_sysdata(msi);
+ int pos = d->hwirq % 32;
+ int i = d->hwirq / 32;
+
+ dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_STATUS + i * 12, 4, BIT(pos));
+}
+
static struct irq_chip dw_msi_irq_chip = {
.name = "PCI-MSI",
+ .irq_ack = dwc_irq_ack,
.irq_enable = pci_msi_unmask_irq,
.irq_disable = pci_msi_mask_irq,
.irq_mask = pci_msi_mask_irq,
@@ -72,8 +83,6 @@ irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
pos)) != 32) {
irq = irq_find_mapping(pp->irq_domain, i * 32 + pos);
generic_handle_irq(irq);
- dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_STATUS + i * 12,
- 4, 1 << pos);
pos++;
}
}
All I can say is that it compiles.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: FAILED: patch "[PATCH] PCI: dwc: Move interrupt acking into the proper callback" failed to apply to 4.14-stable tree
2019-01-17 9:58 ` Marc Zyngier
@ 2019-01-18 16:00 ` Niklas Cassel
2019-01-18 16:22 ` Marc Zyngier
0 siblings, 1 reply; 6+ messages in thread
From: Niklas Cassel @ 2019-01-18 16:00 UTC (permalink / raw)
To: Marc Zyngier
Cc: gregkh, gustavo.pimentel, lorenzo.pieralisi, svarbanov, tpiepho, stable
On Thu, Jan 17, 2019 at 09:58:43AM +0000, Marc Zyngier wrote:
> On 16/01/2019 23:43, Niklas Cassel wrote:
> > On Tue, Jan 15, 2019 at 09:24:37AM +0100, gregkh@linuxfoundation.org wrote:
> >>
> >> The patch below does not apply to the 4.14-stable tree.
> >> If someone wants it applied there, or to any other stable or longterm
> >> tree, then please email the backport, including the original git commit
> >> id to <stable@vger.kernel.org>.
> >
> > Hello Marc,
> >
> > You said that you were going to backport this to 4.14, right?
>
> Me, or anyone else. Preferably someone who, like you, has the HW at hand (I don't).
>
> > IIRC, you said something about creating a simple dw_pci_bottom_ack()
> > (since 4.14 lacks a ack() function).
>
> Indeed. Something like this:
>
> diff --git a/drivers/pci/dwc/pcie-designware-host.c b/drivers/pci/dwc/pcie-designware-host.c
> index bc3e2d8d0cce..f4f3eeee10af 100644
> --- a/drivers/pci/dwc/pcie-designware-host.c
> +++ b/drivers/pci/dwc/pcie-designware-host.c
> @@ -45,8 +45,19 @@ static int dw_pcie_wr_own_conf(struct pcie_port *pp, int where, int size,
> return dw_pcie_write(pci->dbi_base + where, size, val);
> }
>
> +static void dwc_irq_ack(struct irq_data *d)
> +{
> + struct msi_desc *msi = irq_data_get_msi_desc(d);
> + struct pcie_port *pp = msi_desc_to_pci_sysdata(msi);
> + int pos = d->hwirq % 32;
> + int i = d->hwirq / 32;
> +
> + dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_STATUS + i * 12, 4, BIT(pos));
> +}
> +
Thanks Marc.
This does not seem to work.
It appears that the ack function is never called.
I assume that this is because of:
irq_set_chip_and_handler(irq, &dw_msi_irq_chip, handle_simple_irq);
is it safe to simply change this to handle_edge_irq?
(Which seems to be the case after this after this driver's
MSI handling was heavily refactored in 4.17.)
I know that you were against reverting 8c934095fa2f ("PCI: dwc: Clear
MSI interrupt status after it is handled, not before") on mainline,
since that wouldn't hinder people from moving around stuff in the
future, but perhaps reverting this commit on 4.14 is safer than
changing the irq handler type?
Kind regards,
Niklas
> static struct irq_chip dw_msi_irq_chip = {
> .name = "PCI-MSI",
> + .irq_ack = dwc_irq_ack,
> .irq_enable = pci_msi_unmask_irq,
> .irq_disable = pci_msi_mask_irq,
> .irq_mask = pci_msi_mask_irq,
> @@ -72,8 +83,6 @@ irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
> pos)) != 32) {
> irq = irq_find_mapping(pp->irq_domain, i * 32 + pos);
> generic_handle_irq(irq);
> - dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_STATUS + i * 12,
> - 4, 1 << pos);
> pos++;
> }
> }
>
> All I can say is that it compiles.
>
> Thanks,
>
> M.
> --
> Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: FAILED: patch "[PATCH] PCI: dwc: Move interrupt acking into the proper callback" failed to apply to 4.14-stable tree
2019-01-18 16:00 ` Niklas Cassel
@ 2019-01-18 16:22 ` Marc Zyngier
2019-01-19 11:52 ` Niklas Cassel
0 siblings, 1 reply; 6+ messages in thread
From: Marc Zyngier @ 2019-01-18 16:22 UTC (permalink / raw)
To: Niklas Cassel
Cc: gregkh, gustavo.pimentel, lorenzo.pieralisi, svarbanov, tpiepho, stable
On 18/01/2019 16:00, Niklas Cassel wrote:
> On Thu, Jan 17, 2019 at 09:58:43AM +0000, Marc Zyngier wrote:
>> On 16/01/2019 23:43, Niklas Cassel wrote:
>>> On Tue, Jan 15, 2019 at 09:24:37AM +0100, gregkh@linuxfoundation.org wrote:
>>>>
>>>> The patch below does not apply to the 4.14-stable tree.
>>>> If someone wants it applied there, or to any other stable or longterm
>>>> tree, then please email the backport, including the original git commit
>>>> id to <stable@vger.kernel.org>.
>>>
>>> Hello Marc,
>>>
>>> You said that you were going to backport this to 4.14, right?
>>
>> Me, or anyone else. Preferably someone who, like you, has the HW at hand (I don't).
>>
>>> IIRC, you said something about creating a simple dw_pci_bottom_ack()
>>> (since 4.14 lacks a ack() function).
>>
>> Indeed. Something like this:
>>
>> diff --git a/drivers/pci/dwc/pcie-designware-host.c b/drivers/pci/dwc/pcie-designware-host.c
>> index bc3e2d8d0cce..f4f3eeee10af 100644
>> --- a/drivers/pci/dwc/pcie-designware-host.c
>> +++ b/drivers/pci/dwc/pcie-designware-host.c
>> @@ -45,8 +45,19 @@ static int dw_pcie_wr_own_conf(struct pcie_port *pp, int where, int size,
>> return dw_pcie_write(pci->dbi_base + where, size, val);
>> }
>>
>> +static void dwc_irq_ack(struct irq_data *d)
>> +{
>> + struct msi_desc *msi = irq_data_get_msi_desc(d);
>> + struct pcie_port *pp = msi_desc_to_pci_sysdata(msi);
>> + int pos = d->hwirq % 32;
>> + int i = d->hwirq / 32;
>> +
>> + dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_STATUS + i * 12, 4, BIT(pos));
>> +}
>> +
>
> Thanks Marc.
>
> This does not seem to work.
> It appears that the ack function is never called.
> I assume that this is because of:
>
> irq_set_chip_and_handler(irq, &dw_msi_irq_chip, handle_simple_irq);
Ah, indeed, I forgot about that particular nugget.
>
> is it safe to simply change this to handle_edge_irq?
> (Which seems to be the case after this after this driver's
> MSI handling was heavily refactored in 4.17.)
Yes, this should be just a matter of using the right interrupt flow.
Please let me know if that works for you.
>
> I know that you were against reverting 8c934095fa2f ("PCI: dwc: Clear
> MSI interrupt status after it is handled, not before") on mainline,
> since that wouldn't hinder people from moving around stuff in the
> future, but perhaps reverting this commit on 4.14 is safer than
> changing the irq handler type?
I still don't think that it is a good idea. Using the correct flow is
just as important in order to properly honors the interrupt masking
which the "simple" flow completely ignores.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: FAILED: patch "[PATCH] PCI: dwc: Move interrupt acking into the proper callback" failed to apply to 4.14-stable tree
2019-01-18 16:22 ` Marc Zyngier
@ 2019-01-19 11:52 ` Niklas Cassel
0 siblings, 0 replies; 6+ messages in thread
From: Niklas Cassel @ 2019-01-19 11:52 UTC (permalink / raw)
To: Marc Zyngier
Cc: gregkh, gustavo.pimentel, lorenzo.pieralisi, svarbanov, tpiepho, stable
On Fri, Jan 18, 2019 at 04:22:15PM +0000, Marc Zyngier wrote:
> On 18/01/2019 16:00, Niklas Cassel wrote:
> > On Thu, Jan 17, 2019 at 09:58:43AM +0000, Marc Zyngier wrote:
> >> On 16/01/2019 23:43, Niklas Cassel wrote:
> >>> On Tue, Jan 15, 2019 at 09:24:37AM +0100, gregkh@linuxfoundation.org wrote:
> >>>>
> >>>> The patch below does not apply to the 4.14-stable tree.
> >>>> If someone wants it applied there, or to any other stable or longterm
> >>>> tree, then please email the backport, including the original git commit
> >>>> id to <stable@vger.kernel.org>.
> >>>
> >>> Hello Marc,
> >>>
> >>> You said that you were going to backport this to 4.14, right?
> >>
> >> Me, or anyone else. Preferably someone who, like you, has the HW at hand (I don't).
> >>
> >>> IIRC, you said something about creating a simple dw_pci_bottom_ack()
> >>> (since 4.14 lacks a ack() function).
> >>
> >> Indeed. Something like this:
> >>
> >> diff --git a/drivers/pci/dwc/pcie-designware-host.c b/drivers/pci/dwc/pcie-designware-host.c
> >> index bc3e2d8d0cce..f4f3eeee10af 100644
> >> --- a/drivers/pci/dwc/pcie-designware-host.c
> >> +++ b/drivers/pci/dwc/pcie-designware-host.c
> >> @@ -45,8 +45,19 @@ static int dw_pcie_wr_own_conf(struct pcie_port *pp, int where, int size,
> >> return dw_pcie_write(pci->dbi_base + where, size, val);
> >> }
> >>
> >> +static void dwc_irq_ack(struct irq_data *d)
> >> +{
> >> + struct msi_desc *msi = irq_data_get_msi_desc(d);
> >> + struct pcie_port *pp = msi_desc_to_pci_sysdata(msi);
> >> + int pos = d->hwirq % 32;
> >> + int i = d->hwirq / 32;
> >> +
> >> + dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_STATUS + i * 12, 4, BIT(pos));
> >> +}
> >> +
> >
> > Thanks Marc.
> >
> > This does not seem to work.
> > It appears that the ack function is never called.
> > I assume that this is because of:
> >
> > irq_set_chip_and_handler(irq, &dw_msi_irq_chip, handle_simple_irq);
>
> Ah, indeed, I forgot about that particular nugget.
>
> >
> > is it safe to simply change this to handle_edge_irq?
> > (Which seems to be the case after this after this driver's
> > MSI handling was heavily refactored in 4.17.)
>
> Yes, this should be just a matter of using the right interrupt flow.
> Please let me know if that works for you.
Hello Marc,
I ran a simple stress test during the night with your patch plus:
- irq_set_chip_and_handler(irq, &dw_msi_irq_chip, handle_simple_irq);
+ irq_set_chip_and_handler(irq, &dw_msi_irq_chip, handle_edge_irq);
without any issues. (Feel free to add my Tested-by).
So this looks good to me :) Thanks!
Kind regards,
Niklas
>
> >
> > I know that you were against reverting 8c934095fa2f ("PCI: dwc: Clear
> > MSI interrupt status after it is handled, not before") on mainline,
> > since that wouldn't hinder people from moving around stuff in the
> > future, but perhaps reverting this commit on 4.14 is safer than
> > changing the irq handler type?
>
> I still don't think that it is a good idea. Using the correct flow is
> just as important in order to properly honors the interrupt masking
> which the "simple" flow completely ignores.
>
> Thanks,
>
> M.
> --
> Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-01-19 11:52 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-15 8:24 FAILED: patch "[PATCH] PCI: dwc: Move interrupt acking into the proper callback" failed to apply to 4.14-stable tree gregkh
2019-01-16 23:43 ` Niklas Cassel
2019-01-17 9:58 ` Marc Zyngier
2019-01-18 16:00 ` Niklas Cassel
2019-01-18 16:22 ` Marc Zyngier
2019-01-19 11:52 ` Niklas Cassel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).