From: Stephen Warren <swarren@wwwdotorg.org>
To: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
Cc: Jingoo Han <jingoohan1@gmail.com>,
Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
Bjorn Helgaas <bhelgaas@google.com>,
"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
Stephen Warren <swarren@nvidia.com>,
Arnd Bergmann <arnd@arndb.de>, Faiz Abbas <faiz_abbas@ti.com>,
Harro Haan <hrhaan@gmail.com>, Jingoo Han <jg1.han@samsung.com>,
Joao Pinto <joao.pinto@synopsys.com>,
Juergen Beisert <jbe@pengutronix.de>, Marek Vasut <marex@denx.de>,
Matthias Mann <m.mann@arkona-technologies.de>,
Mohit Kumar <mohit.kumar@st.com>,
Pratyush Anand <pratyush.anand@st.com>,
Richard Zhu <hong-xing.zhu@freescale.com>,
Sean Cross <xobs@kosagi.com>, Shawn Guo <shawn.guo@linaro.org>,
Siva Reddy Kallam <siva.kallam@samsung.com>,
Srikanth T Shivanand <ts.srikanth@samsung.com>,
Tim Harvey <tharvey@gateworks.com>
Subject: Re: [PATCH] PCI: dwc: fix MSI IRQ handler ordering
Date: Mon, 7 Jan 2019 10:16:29 -0700 [thread overview]
Message-ID: <567e726d-e3c5-c48d-ec99-af1c1576180f@wwwdotorg.org> (raw)
In-Reply-To: <520c9d1b-8b1f-46ac-687a-fdb703328567@synopsys.com>
On 1/7/19 2:36 AM, Gustavo Pimentel wrote:
> Hi Stephen,
>
> On 04/01/2019 21:45, Stephen Warren wrote:
>> From: Stephen Warren <swarren@nvidia.com>
>>
>> The current code does this when handling MSI IRQs:
>>
>> a) Process the irq.
>> b) Clear the latched IRQ status.
>>
>> If a new IRQ occurs any time after (a) has read the IRQ status for the
>> last time and before (b), it will be lost. For example, this occurs in
>> practice when using a Marvell 9171 AHCI controller with NCQ enabled;
>> many command timeouts occur with certain disk access patterns.
>>
>> Fix the code to do the following instead, so that if any new IRQs are
>> raised during the processing of the IRQ, the IRQ status is not cleared,
>> so that the IRQ is not lost.
>>
>> a) Clear the latched IRQ status.
>> b) Process the IRQ.
>>
>> This change reverts commit 8c934095fa2f ("PCI: dwc: Clear MSI interrupt
>> status after it is handled, not before")
>>
>> This change re-applies commit ca1658921b63 ("PCI: designware: Fix
>> missing MSI IRQs")
>>
>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
>> Cc: Arnd Bergmann <arnd@arndb.de>
>> Cc: Bjorn Helgaas <bhelgaas@google.com>
>> Cc: Faiz Abbas <faiz_abbas@ti.com>
>> Cc: Harro Haan <hrhaan@gmail.com>
>> Cc: Jingoo Han <jg1.han@samsung.com>
>> Cc: Joao Pinto <jpinto@synopsys.com>
>> Cc: Juergen Beisert <jbe@pengutronix.de>
>> Cc: Marek Vasut <marex@denx.de>
>> Cc: Matthias Mann <m.mann@arkona-technologies.de>
>> Cc: Mohit Kumar <mohit.kumar@st.com>
>> Cc: Pratyush Anand <pratyush.anand@st.com>
>> Cc: Richard Zhu <hong-xing.zhu@freescale.com>
>> Cc: Sean Cross <xobs@kosagi.com>
>> Cc: Shawn Guo <shawn.guo@linaro.org>
>> Cc: Siva Reddy Kallam <siva.kallam@samsung.com>
>> Cc: Srikanth T Shivanand <ts.srikanth@samsung.com>
>> Cc: Tim Harvey <tharvey@gateworks.com>
>> ---
>> Note: This issue was found in downstream NVIDIA 4.9 and 4.14 kernels.
>> However, the exact same code structure is present in mainline and I have
>> no reason to believe the problem would not reproduce there. I have
>> compile tested but not runtime tested it in mainline, since my board is
>> not yet supported in mainline.
>> ---
>> drivers/pci/controller/dwc/pcie-designware-host.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
>> index 692dd1b264fb..7fd6c56a6f35 100644
>> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
>> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
>> @@ -98,10 +98,10 @@ irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
>> irq = irq_find_mapping(pp->irq_domain,
>> (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);
>> + generic_handle_irq(irq);
>> pos++;
>> }
>> }
>
> This fix was already suggested by Trent Piepho, however, after review by Marc
> Zyngier and some information obtained from Synopsys IP development team, the
> following set of patches was generated to correct the problem.
>
> 830920e065e9 ("PCI: dwc: Use interrupt masking instead of disabling")
> fce5423e4f43 ("PCI: dwc: Take lock when ACKing an interrupt")
> 3f7bb2ec20ce ("PCI: dwc: Move interrupt acking into the proper callback")
OK. I assume that dw_pci_bottom_ack() gets called before
dw_handle_msi_irq() for each IRQ? In which case, the last of those 3
changes is essentially identical to the patch I proposed, it's just that
the acking is handled in a separate function rather than earlier in the
same function.
next prev parent reply other threads:[~2019-01-07 17:16 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-01-04 21:45 [PATCH] PCI: dwc: fix MSI IRQ handler ordering Stephen Warren
2019-01-04 22:45 ` Trent Piepho
2019-01-04 23:00 ` Stephen Warren
2019-01-04 23:23 ` Stephen Warren
2019-01-05 0:15 ` Trent Piepho
2019-01-07 9:36 ` Gustavo Pimentel
2019-01-07 17:16 ` Stephen Warren [this message]
2019-01-07 18:09 ` Gustavo Pimentel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=567e726d-e3c5-c48d-ec99-af1c1576180f@wwwdotorg.org \
--to=swarren@wwwdotorg.org \
--cc=arnd@arndb.de \
--cc=bhelgaas@google.com \
--cc=faiz_abbas@ti.com \
--cc=gustavo.pimentel@synopsys.com \
--cc=hong-xing.zhu@freescale.com \
--cc=hrhaan@gmail.com \
--cc=jbe@pengutronix.de \
--cc=jg1.han@samsung.com \
--cc=jingoohan1@gmail.com \
--cc=joao.pinto@synopsys.com \
--cc=linux-pci@vger.kernel.org \
--cc=lorenzo.pieralisi@arm.com \
--cc=m.mann@arkona-technologies.de \
--cc=marex@denx.de \
--cc=mohit.kumar@st.com \
--cc=pratyush.anand@st.com \
--cc=shawn.guo@linaro.org \
--cc=siva.kallam@samsung.com \
--cc=swarren@nvidia.com \
--cc=tharvey@gateworks.com \
--cc=ts.srikanth@samsung.com \
--cc=xobs@kosagi.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).