* [PATCH] PCI: Avoid broken MSI on SB600 USB devices
@ 2022-03-21 18:34 Bjorn Helgaas
2022-03-23 2:26 ` Bjorn Helgaas
0 siblings, 1 reply; 8+ messages in thread
From: Bjorn Helgaas @ 2022-03-21 18:34 UTC (permalink / raw)
To: linux-pci
Cc: Andy Shevchenko, linux-kernel, Mick Lorain, Alex Williamson,
Thomas Gleixner, Bjorn Helgaas
From: Bjorn Helgaas <bhelgaas@google.com>
Some ATI SB600 USB adapters advertise MSI, but if INTx is disabled by
setting PCI_COMMAND_INTX_DISABLE, MSI doesn't work either. The PCI/PCIe
specs do not require software to set PCI_COMMAND_INTX_DISABLE when enabling
MSI, but Linux has done that for many years.
Mick reported that 306c54d0edb6 ("usb: hcd: Try MSI interrupts on PCI
devices") broke these devices. Prior to 306c54d0edb6, they used INTx.
Starting with 306c54d0edb6, they use MSI, and and the fact that Linux sets
PCI_COMMAND_INTX_DISABLE means both INTx and MSI are disabled on these
devices.
Avoid this SB600 defect by disabling MSI so we use INTx as before.
Fixes: 306c54d0edb6 ("usb: hcd: Try MSI interrupts on PCI devices")
Link: https://bugzilla.kernel.org/show_bug.cgi?id=215690
Link: https://lore.kernel.org/all/PxIByDyBRcsbpcmVhGSNDFAoUcMmb78ctXCkw6fbpx25TGlCHvA6SJjjFkNr1FfQZMntYPTNyvEnblxzAZ8a6jP9ddLpKeCN6Chi_2FuexU=@protonmail.com/
BugLink: https://lore.kernel.org/all/20200702143045.23429-1-andriy.shevchenko@linux.intel.com/
Link: https://lore.kernel.org/r/20220314101448.90074-1-andriy.shevchenko@linux.intel.com
Reported-by: Mick Lorain <micklorain@protonmail.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/pci/quirks.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index d2dd6a6cda60..5f46fed01e6c 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -1811,6 +1811,18 @@ static void quirk_alder_ioapic(struct pci_dev *pdev)
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_EESSC, quirk_alder_ioapic);
#endif
+static void quirk_no_msi(struct pci_dev *dev)
+{
+ pci_info(dev, "avoiding MSI to work around a hardware defect\n");
+ dev->no_msi = 1;
+}
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x4386, quirk_no_msi);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x4387, quirk_no_msi);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x4388, quirk_no_msi);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x4389, quirk_no_msi);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x438a, quirk_no_msi);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x438b, quirk_no_msi);
+
static void quirk_pcie_mch(struct pci_dev *pdev)
{
pdev->no_msi = 1;
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] PCI: Avoid broken MSI on SB600 USB devices
2022-03-21 18:34 [PATCH] PCI: Avoid broken MSI on SB600 USB devices Bjorn Helgaas
@ 2022-03-23 2:26 ` Bjorn Helgaas
2022-03-23 8:03 ` Andy Shevchenko
0 siblings, 1 reply; 8+ messages in thread
From: Bjorn Helgaas @ 2022-03-23 2:26 UTC (permalink / raw)
To: linux-pci
Cc: Andy Shevchenko, linux-kernel, Mick Lorain, Alex Williamson,
Thomas Gleixner, Bjorn Helgaas
On Mon, Mar 21, 2022 at 01:34:46PM -0500, Bjorn Helgaas wrote:
> From: Bjorn Helgaas <bhelgaas@google.com>
>
> Some ATI SB600 USB adapters advertise MSI, but if INTx is disabled by
> setting PCI_COMMAND_INTX_DISABLE, MSI doesn't work either. The PCI/PCIe
> specs do not require software to set PCI_COMMAND_INTX_DISABLE when enabling
> MSI, but Linux has done that for many years.
>
> Mick reported that 306c54d0edb6 ("usb: hcd: Try MSI interrupts on PCI
> devices") broke these devices. Prior to 306c54d0edb6, they used INTx.
> Starting with 306c54d0edb6, they use MSI, and and the fact that Linux sets
> PCI_COMMAND_INTX_DISABLE means both INTx and MSI are disabled on these
> devices.
>
> Avoid this SB600 defect by disabling MSI so we use INTx as before.
>
> Fixes: 306c54d0edb6 ("usb: hcd: Try MSI interrupts on PCI devices")
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=215690
> Link: https://lore.kernel.org/all/PxIByDyBRcsbpcmVhGSNDFAoUcMmb78ctXCkw6fbpx25TGlCHvA6SJjjFkNr1FfQZMntYPTNyvEnblxzAZ8a6jP9ddLpKeCN6Chi_2FuexU=@protonmail.com/
> BugLink: https://lore.kernel.org/all/20200702143045.23429-1-andriy.shevchenko@linux.intel.com/
> Link: https://lore.kernel.org/r/20220314101448.90074-1-andriy.shevchenko@linux.intel.com
> Reported-by: Mick Lorain <micklorain@protonmail.com>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Unless there's objection, I plan to include this in the v5.18 pull
request in the next few days.
It was in the 20220322 linux-next tree:
https://lore.kernel.org/linux-next/20220322203829.2bb0166c@canb.auug.org.au/
> ---
> drivers/pci/quirks.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index d2dd6a6cda60..5f46fed01e6c 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -1811,6 +1811,18 @@ static void quirk_alder_ioapic(struct pci_dev *pdev)
> DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_EESSC, quirk_alder_ioapic);
> #endif
>
> +static void quirk_no_msi(struct pci_dev *dev)
> +{
> + pci_info(dev, "avoiding MSI to work around a hardware defect\n");
> + dev->no_msi = 1;
> +}
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x4386, quirk_no_msi);
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x4387, quirk_no_msi);
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x4388, quirk_no_msi);
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x4389, quirk_no_msi);
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x438a, quirk_no_msi);
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x438b, quirk_no_msi);
> +
> static void quirk_pcie_mch(struct pci_dev *pdev)
> {
> pdev->no_msi = 1;
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] PCI: Avoid broken MSI on SB600 USB devices
2022-03-23 2:26 ` Bjorn Helgaas
@ 2022-03-23 8:03 ` Andy Shevchenko
2022-03-23 11:11 ` Bjorn Helgaas
2022-03-23 12:43 ` David Woodhouse
0 siblings, 2 replies; 8+ messages in thread
From: Andy Shevchenko @ 2022-03-23 8:03 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-pci, Linux Kernel Mailing List, Mick Lorain,
Alex Williamson, Thomas Gleixner, Bjorn Helgaas
On Wed, Mar 23, 2022 at 4:26 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
> On Mon, Mar 21, 2022 at 01:34:46PM -0500, Bjorn Helgaas wrote:
> > From: Bjorn Helgaas <bhelgaas@google.com>
> >
> > Some ATI SB600 USB adapters advertise MSI, but if INTx is disabled by
> > setting PCI_COMMAND_INTX_DISABLE,
> > MSI doesn't work either.
I think this is not correct.
> > The PCI/PCIe
> > specs do not require software to set PCI_COMMAND_INTX_DISABLE when enabling
> > MSI, but Linux has done that for many years.
> >
> > Mick reported that 306c54d0edb6 ("usb: hcd: Try MSI interrupts on PCI
> > devices") broke these devices. Prior to 306c54d0edb6, they used INTx.
> > Starting with 306c54d0edb6, they use MSI, and and the fact that Linux sets
> > PCI_COMMAND_INTX_DISABLE means both INTx and MSI are disabled on these
> > devices.
> > Avoid this SB600 defect by disabling MSI so we use INTx as before.
And this is kinda too conservative approach.
> > Fixes: 306c54d0edb6 ("usb: hcd: Try MSI interrupts on PCI devices")
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=215690
> > Link: https://lore.kernel.org/all/PxIByDyBRcsbpcmVhGSNDFAoUcMmb78ctXCkw6fbpx25TGlCHvA6SJjjFkNr1FfQZMntYPTNyvEnblxzAZ8a6jP9ddLpKeCN6Chi_2FuexU=@protonmail.com/
> > BugLink: https://lore.kernel.org/all/20200702143045.23429-1-andriy.shevchenko@linux.intel.com/
> > Link: https://lore.kernel.org/r/20220314101448.90074-1-andriy.shevchenko@linux.intel.com
> > Reported-by: Mick Lorain <micklorain@protonmail.com>
> > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
And it hasn't been tested by the reporter.
> Unless there's objection, I plan to include this in the v5.18 pull
> request in the next few days.
You are the maintainer here and it's your choice. I'm not going to stop you.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] PCI: Avoid broken MSI on SB600 USB devices
2022-03-23 8:03 ` Andy Shevchenko
@ 2022-03-23 11:11 ` Bjorn Helgaas
2022-03-23 12:23 ` Andy Shevchenko
2022-03-23 12:43 ` David Woodhouse
1 sibling, 1 reply; 8+ messages in thread
From: Bjorn Helgaas @ 2022-03-23 11:11 UTC (permalink / raw)
To: Andy Shevchenko
Cc: linux-pci, Linux Kernel Mailing List, Mick Lorain,
Alex Williamson, Thomas Gleixner, Bjorn Helgaas
On Wed, Mar 23, 2022 at 10:03:38AM +0200, Andy Shevchenko wrote:
> On Wed, Mar 23, 2022 at 4:26 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > On Mon, Mar 21, 2022 at 01:34:46PM -0500, Bjorn Helgaas wrote:
> > > From: Bjorn Helgaas <bhelgaas@google.com>
> > >
> > > Some ATI SB600 USB adapters advertise MSI, but if INTx is disabled by
> > > setting PCI_COMMAND_INTX_DISABLE,
>
> > > MSI doesn't work either.
>
> I think this is not correct.
I'd like to make it correct. What would make this better? I was
trying to say the same as your original commit log:
ATI PCIe-USB adapter advertises MSI, but it doesn't work if INTx is
disabled.
Bjorn
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] PCI: Avoid broken MSI on SB600 USB devices
2022-03-23 11:11 ` Bjorn Helgaas
@ 2022-03-23 12:23 ` Andy Shevchenko
0 siblings, 0 replies; 8+ messages in thread
From: Andy Shevchenko @ 2022-03-23 12:23 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-pci, Linux Kernel Mailing List, Mick Lorain,
Alex Williamson, Thomas Gleixner, Bjorn Helgaas
On Wed, Mar 23, 2022 at 1:11 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> On Wed, Mar 23, 2022 at 10:03:38AM +0200, Andy Shevchenko wrote:
> > On Wed, Mar 23, 2022 at 4:26 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > > On Mon, Mar 21, 2022 at 01:34:46PM -0500, Bjorn Helgaas wrote:
> > > > From: Bjorn Helgaas <bhelgaas@google.com>
> > > >
> > > > Some ATI SB600 USB adapters advertise MSI, but if INTx is disabled by
> > > > setting PCI_COMMAND_INTX_DISABLE,
> >
> > > > MSI doesn't work either.
> >
> > I think this is not correct.
>
> I'd like to make it correct. What would make this better?
MSI with the quirk (not in this patch) is working and has been tested.
That said, the part that I commented on is confusing and states the
opposite. I would change the patch if you ask me how to improve it,
> I was
> trying to say the same as your original commit log:
>
> ATI PCIe-USB adapter advertises MSI, but it doesn't work if INTx is
> disabled.
I'm not a native speaker, maybe I was wrong in formulating that MSI
enabling needs a quirk.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] PCI: Avoid broken MSI on SB600 USB devices
2022-03-23 8:03 ` Andy Shevchenko
2022-03-23 11:11 ` Bjorn Helgaas
@ 2022-03-23 12:43 ` David Woodhouse
2022-03-23 13:23 ` Bjorn Helgaas
2022-03-23 13:43 ` Andy Shevchenko
1 sibling, 2 replies; 8+ messages in thread
From: David Woodhouse @ 2022-03-23 12:43 UTC (permalink / raw)
To: Andy Shevchenko, Bjorn Helgaas
Cc: linux-pci, Linux Kernel Mailing List, Mick Lorain,
Alex Williamson, Thomas Gleixner, Bjorn Helgaas
[-- Attachment #1: Type: text/plain, Size: 753 bytes --]
On Wed, 2022-03-23 at 10:03 +0200, Andy Shevchenko wrote:
> On Wed, Mar 23, 2022 at 4:26 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> > On Mon, Mar 21, 2022 at 01:34:46PM -0500, Bjorn Helgaas wrote:
> > > From: Bjorn Helgaas <bhelgaas@google.com>
> > > Some ATI SB600 USB adapters advertise MSI, but if INTx is disabled by
> > > setting PCI_COMMAND_INTX_DISABLE,
>
> > > MSI doesn't work either.
>
> I think this is not correct.
I think it was perfectly correct until you added a couple of newlines
in the middle of the sentence, then took it out of context. :)
"If INTX is disabled, MSI doesn't work either".
But really, in that case surely the solution is *not* to disable INTX
for this device. Then MSI will work, right?
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] PCI: Avoid broken MSI on SB600 USB devices
2022-03-23 12:43 ` David Woodhouse
@ 2022-03-23 13:23 ` Bjorn Helgaas
2022-03-23 13:43 ` Andy Shevchenko
1 sibling, 0 replies; 8+ messages in thread
From: Bjorn Helgaas @ 2022-03-23 13:23 UTC (permalink / raw)
To: David Woodhouse
Cc: Andy Shevchenko, linux-pci, Linux Kernel Mailing List,
Mick Lorain, Alex Williamson, Thomas Gleixner, Bjorn Helgaas
On Wed, Mar 23, 2022 at 12:43:48PM +0000, David Woodhouse wrote:
> On Wed, 2022-03-23 at 10:03 +0200, Andy Shevchenko wrote:
> > On Wed, Mar 23, 2022 at 4:26 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
> >
> > > On Mon, Mar 21, 2022 at 01:34:46PM -0500, Bjorn Helgaas wrote:
> > > > From: Bjorn Helgaas <bhelgaas@google.com>
> > > > Some ATI SB600 USB adapters advertise MSI, but if INTx is disabled by
> > > > setting PCI_COMMAND_INTX_DISABLE,
> >
> > > > MSI doesn't work either.
> >
> > I think this is not correct.
>
> I think it was perfectly correct until you added a couple of newlines
> in the middle of the sentence, then took it out of context. :)
>
> "If INTX is disabled, MSI doesn't work either".
>
> But really, in that case surely the solution is *not* to disable INTX
> for this device. Then MSI will work, right?
That's what Andy's original patch [1] does, and MSI *does* work if we
skip disabling INTx.
I'm hesitant [2] about that approach because it creates two classes of
devices using MSI (most have INTx disabled but a few do not), which
makes it harder to reason about them. For example, there are non-MSI
paths that read or set the "disable INTx" bit, so we have to consider:
- will readers be surprised if a device using MSI has INTx enabled?
- will writers care disabling INTx disables *all* interrupts, not
just INTx?
So skipping the INTx disable certainly works most of the time and
*likely* works all the time, but there are cases that could be
problems and we don't have a compelling reason to use MSI on these
devices.
[1] https://lore.kernel.org/r/20220314101448.90074-1-andriy.shevchenko@linux.intel.com
[2] https://lore.kernel.org/r/20220318210947.GA845994@bhelgaas
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] PCI: Avoid broken MSI on SB600 USB devices
2022-03-23 12:43 ` David Woodhouse
2022-03-23 13:23 ` Bjorn Helgaas
@ 2022-03-23 13:43 ` Andy Shevchenko
1 sibling, 0 replies; 8+ messages in thread
From: Andy Shevchenko @ 2022-03-23 13:43 UTC (permalink / raw)
To: David Woodhouse
Cc: Bjorn Helgaas, linux-pci, Linux Kernel Mailing List, Mick Lorain,
Alex Williamson, Thomas Gleixner, Bjorn Helgaas
On Wed, Mar 23, 2022 at 2:43 PM David Woodhouse <dwmw2@infradead.org> wrote:
> On Wed, 2022-03-23 at 10:03 +0200, Andy Shevchenko wrote:
> > On Wed, Mar 23, 2022 at 4:26 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > > On Mon, Mar 21, 2022 at 01:34:46PM -0500, Bjorn Helgaas wrote:
> > > > From: Bjorn Helgaas <bhelgaas@google.com>
> > > > Some ATI SB600 USB adapters advertise MSI, but if INTx is disabled by
> > > > setting PCI_COMMAND_INTX_DISABLE,
> >
> > > > MSI doesn't work either.
> >
> > I think this is not correct.
>
> I think it was perfectly correct until you added a couple of newlines
> in the middle of the sentence, then took it out of context. :)
>
> "If INTX is disabled, MSI doesn't work either".
Ah, I stand corrected. Thanks for the English lesson!
> But really, in that case surely the solution is *not* to disable INTX
> for this device. Then MSI will work, right?
That was my intention, but Bjorn has concerns.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-03-23 13:44 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-21 18:34 [PATCH] PCI: Avoid broken MSI on SB600 USB devices Bjorn Helgaas
2022-03-23 2:26 ` Bjorn Helgaas
2022-03-23 8:03 ` Andy Shevchenko
2022-03-23 11:11 ` Bjorn Helgaas
2022-03-23 12:23 ` Andy Shevchenko
2022-03-23 12:43 ` David Woodhouse
2022-03-23 13:23 ` Bjorn Helgaas
2022-03-23 13:43 ` Andy Shevchenko
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).