From: "Michael Büsch" <m@bues.ch>
To: Bjorn Helgaas <helgaas@kernel.org>
Cc: Lukas Wunner <lukas@wunner.de>,
linux-pci@vger.kernel.org, linux-wireless@vger.kernel.org,
b43-dev@lists.infradead.org,
Matthew Garrett <mjg59@srcf.ucam.org>
Subject: [PATCH] PCI: Add Broadcom 4331 reset quirk to prevent IRQ storm
Date: Tue, 5 Apr 2016 21:49:45 +0200 [thread overview]
Message-ID: <20160405214945.1214e13b@wiggum> (raw)
In-Reply-To: <20160405194015.GC15353@localhost>
On Tue, 5 Apr 2016 14:40:15 -0500
Bjorn Helgaas <helgaas@kernel.org> wrote:
> [+cc Matthew]
>
> Hi Lukas,
>
> On Tue, Mar 29, 2016 at 08:20:30PM +0200, Lukas Wunner wrote:
> > Broadcom 4331 wireless cards built into Apple Macs unleash an IRQ storm
> > on boot until they are reset, causing spurious interrupts if the IRQ is
> > shared. Apparently the EFI bootloader enables the device and does not
> > disable it before passing control to the OS. The bootloader contains a
> > driver for the wireless card which allows it to phone home to Cupertino.
> > This is used for Internet Recovery (download and install OS X images)
> > and probably also for Back to My Mac (remote access, RFC 6281) and to
> > discover stolen hardware.
> >
> > The issue is most pronounced on 2011 and 2012 MacBook Pros where the IRQ
> > is shared with 3 other devices (Light Ridge Thunderbolt controller, SDXC
> > reader, HDA card on discrete GPU). As soon as an interrupt handler is
> > installed for one of these devices, the ensuing storm of spurious IRQs
> > causes the kernel to disable the IRQ and switch to polling. This lasts
> > until the b43 driver loads and resets the device.
> >
> > Loading the b43 driver first is not always an option, in particular with
> > the Light Ridge Thunderbolt controller: The PCI hotplug IRQ handler gets
> > installed early on because it is built in, unlike b43 which is usually
> > a module.
> >
> > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=79301
> > Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=895951
> > Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1009819
> > Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1149632
> > Tested-by: Lukas Wunner <lukas@wunner.de> [MacBookPro9,1]
> > Signed-off-by: Lukas Wunner <lukas@wunner.de>
> > ---
> > drivers/bcma/bcma_private.h | 2 --
> > drivers/pci/quirks.c | 27 +++++++++++++++++++++++++++
> > include/linux/bcma/bcma.h | 1 +
> > 3 files changed, 28 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/bcma/bcma_private.h b/drivers/bcma/bcma_private.h
> > index eda0909..f642c42 100644
> > --- a/drivers/bcma/bcma_private.h
> > +++ b/drivers/bcma/bcma_private.h
> > @@ -8,8 +8,6 @@
> > #include <linux/bcma/bcma.h>
> > #include <linux/delay.h>
> >
> > -#define BCMA_CORE_SIZE 0x1000
> > -
> > #define bcma_err(bus, fmt, ...) \
> > pr_err("bus%d: " fmt, (bus)->num, ##__VA_ARGS__)
> > #define bcma_warn(bus, fmt, ...) \
> > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> > index 8e67802..d4fb5ee 100644
> > --- a/drivers/pci/quirks.c
> > +++ b/drivers/pci/quirks.c
> > @@ -25,6 +25,8 @@
> > #include <linux/sched.h>
> > #include <linux/ktime.h>
> > #include <linux/mm.h>
> > +#include <linux/bcma/bcma.h>
> > +#include <linux/bcma/bcma_regs.h>
> > #include <asm/dma.h> /* isa_dma_bridge_buggy */
> > #include "pci.h"
> >
> > @@ -3282,6 +3284,31 @@ DECLARE_PCI_FIXUP_RESUME_EARLY(PCI_VENDOR_ID_INTEL, 0x156d,
> > quirk_apple_wait_for_thunderbolt);
> > #endif
> >
> > +/*
> > + * Broadcom 4331 wireless cards built into Apple Macs unleash an IRQ storm
> > + * on boot until they are reset, causing spurious interrupts if the IRQ is
> > + * shared. Apparently the EFI bootloader enables the device to phone home
> > + * to Cupertino and does not disable it before passing control to the OS.
> > + */
> > +static void quirk_apple_b43_reset(struct pci_dev *dev)
> > +{
> > + void __iomem *mmio;
> > +
> > + if (!dmi_match(DMI_BOARD_VENDOR, "Apple Inc.") || !dev->bus->self ||
> > + pci_pcie_type(dev->bus->self) != PCI_EXP_TYPE_ROOT_PORT)
> > + return;
> > +
> > + mmio = pci_iomap(dev, 0, 0);
> > + if (!mmio) {
> > + pr_err("b43 quirk: Cannot iomap device, IRQ storm ahead\n");
> > + return;
> > + }
> > + iowrite32(BCMA_RESET_CTL_RESET,
> > + mmio + (1 * BCMA_CORE_SIZE) + BCMA_RESET_CTL);
> > + pci_iounmap(dev, mmio);
>
> https://bugzilla.kernel.org/show_bug.cgi?id=111781 and
> https://mjg59.dreamwidth.org/11235.html describe a sort of similar
> issue, but with DMA. An interrupt from the device is probably to
> signal a DMA completion, but these problem reports only mention the
> "IRQ nobody cared" issue; I don't see anything about memory
> corruption.
>
> If this resets the device, I guess that should prevent future DMA as
> well as future interrupts. Would pci_reset_function() do the same
> thing in a more generic way?
>
> I'm a little bit hesitant to put a quirk like this in Linux because
> it's only a 90% solution. If the only problem is the interrupts, it's
> probably OK since a few extra interrupts doesn't hurt anything. But
> if there is also DMA that might corrupt something, a 90% solution just
> makes it harder to debug for the remaining 10%.
This was already discussed in this thread.
It is not a 90% solution. It fixes the IRQ issue. It is not supposed to
fix the DMA issue. And it can't. It does mitigate it a tiny bit though.
--
Michael
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/b43-dev/attachments/20160405/209a7273/attachment.sig>
WARNING: multiple messages have this Message-ID (diff)
From: "Michael Büsch" <m@bues.ch>
To: Bjorn Helgaas <helgaas@kernel.org>
Cc: Lukas Wunner <lukas@wunner.de>,
linux-pci@vger.kernel.org, linux-wireless@vger.kernel.org,
b43-dev@lists.infradead.org,
Matthew Garrett <mjg59@srcf.ucam.org>
Subject: Re: [PATCH] PCI: Add Broadcom 4331 reset quirk to prevent IRQ storm
Date: Tue, 5 Apr 2016 21:49:45 +0200 [thread overview]
Message-ID: <20160405214945.1214e13b@wiggum> (raw)
In-Reply-To: <20160405194015.GC15353@localhost>
[-- Attachment #1: Type: text/plain, Size: 5080 bytes --]
On Tue, 5 Apr 2016 14:40:15 -0500
Bjorn Helgaas <helgaas@kernel.org> wrote:
> [+cc Matthew]
>
> Hi Lukas,
>
> On Tue, Mar 29, 2016 at 08:20:30PM +0200, Lukas Wunner wrote:
> > Broadcom 4331 wireless cards built into Apple Macs unleash an IRQ storm
> > on boot until they are reset, causing spurious interrupts if the IRQ is
> > shared. Apparently the EFI bootloader enables the device and does not
> > disable it before passing control to the OS. The bootloader contains a
> > driver for the wireless card which allows it to phone home to Cupertino.
> > This is used for Internet Recovery (download and install OS X images)
> > and probably also for Back to My Mac (remote access, RFC 6281) and to
> > discover stolen hardware.
> >
> > The issue is most pronounced on 2011 and 2012 MacBook Pros where the IRQ
> > is shared with 3 other devices (Light Ridge Thunderbolt controller, SDXC
> > reader, HDA card on discrete GPU). As soon as an interrupt handler is
> > installed for one of these devices, the ensuing storm of spurious IRQs
> > causes the kernel to disable the IRQ and switch to polling. This lasts
> > until the b43 driver loads and resets the device.
> >
> > Loading the b43 driver first is not always an option, in particular with
> > the Light Ridge Thunderbolt controller: The PCI hotplug IRQ handler gets
> > installed early on because it is built in, unlike b43 which is usually
> > a module.
> >
> > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=79301
> > Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=895951
> > Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1009819
> > Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1149632
> > Tested-by: Lukas Wunner <lukas@wunner.de> [MacBookPro9,1]
> > Signed-off-by: Lukas Wunner <lukas@wunner.de>
> > ---
> > drivers/bcma/bcma_private.h | 2 --
> > drivers/pci/quirks.c | 27 +++++++++++++++++++++++++++
> > include/linux/bcma/bcma.h | 1 +
> > 3 files changed, 28 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/bcma/bcma_private.h b/drivers/bcma/bcma_private.h
> > index eda0909..f642c42 100644
> > --- a/drivers/bcma/bcma_private.h
> > +++ b/drivers/bcma/bcma_private.h
> > @@ -8,8 +8,6 @@
> > #include <linux/bcma/bcma.h>
> > #include <linux/delay.h>
> >
> > -#define BCMA_CORE_SIZE 0x1000
> > -
> > #define bcma_err(bus, fmt, ...) \
> > pr_err("bus%d: " fmt, (bus)->num, ##__VA_ARGS__)
> > #define bcma_warn(bus, fmt, ...) \
> > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> > index 8e67802..d4fb5ee 100644
> > --- a/drivers/pci/quirks.c
> > +++ b/drivers/pci/quirks.c
> > @@ -25,6 +25,8 @@
> > #include <linux/sched.h>
> > #include <linux/ktime.h>
> > #include <linux/mm.h>
> > +#include <linux/bcma/bcma.h>
> > +#include <linux/bcma/bcma_regs.h>
> > #include <asm/dma.h> /* isa_dma_bridge_buggy */
> > #include "pci.h"
> >
> > @@ -3282,6 +3284,31 @@ DECLARE_PCI_FIXUP_RESUME_EARLY(PCI_VENDOR_ID_INTEL, 0x156d,
> > quirk_apple_wait_for_thunderbolt);
> > #endif
> >
> > +/*
> > + * Broadcom 4331 wireless cards built into Apple Macs unleash an IRQ storm
> > + * on boot until they are reset, causing spurious interrupts if the IRQ is
> > + * shared. Apparently the EFI bootloader enables the device to phone home
> > + * to Cupertino and does not disable it before passing control to the OS.
> > + */
> > +static void quirk_apple_b43_reset(struct pci_dev *dev)
> > +{
> > + void __iomem *mmio;
> > +
> > + if (!dmi_match(DMI_BOARD_VENDOR, "Apple Inc.") || !dev->bus->self ||
> > + pci_pcie_type(dev->bus->self) != PCI_EXP_TYPE_ROOT_PORT)
> > + return;
> > +
> > + mmio = pci_iomap(dev, 0, 0);
> > + if (!mmio) {
> > + pr_err("b43 quirk: Cannot iomap device, IRQ storm ahead\n");
> > + return;
> > + }
> > + iowrite32(BCMA_RESET_CTL_RESET,
> > + mmio + (1 * BCMA_CORE_SIZE) + BCMA_RESET_CTL);
> > + pci_iounmap(dev, mmio);
>
> https://bugzilla.kernel.org/show_bug.cgi?id=111781 and
> https://mjg59.dreamwidth.org/11235.html describe a sort of similar
> issue, but with DMA. An interrupt from the device is probably to
> signal a DMA completion, but these problem reports only mention the
> "IRQ nobody cared" issue; I don't see anything about memory
> corruption.
>
> If this resets the device, I guess that should prevent future DMA as
> well as future interrupts. Would pci_reset_function() do the same
> thing in a more generic way?
>
> I'm a little bit hesitant to put a quirk like this in Linux because
> it's only a 90% solution. If the only problem is the interrupts, it's
> probably OK since a few extra interrupts doesn't hurt anything. But
> if there is also DMA that might corrupt something, a 90% solution just
> makes it harder to debug for the remaining 10%.
This was already discussed in this thread.
It is not a 90% solution. It fixes the IRQ issue. It is not supposed to
fix the DMA issue. And it can't. It does mitigate it a tiny bit though.
--
Michael
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
next prev parent reply other threads:[~2016-04-05 19:49 UTC|newest]
Thread overview: 57+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-03-29 18:20 [PATCH] PCI: Add Broadcom 4331 reset quirk to prevent IRQ storm Lukas Wunner
2016-03-31 18:51 ` Rafał Miłecki
2016-03-31 18:51 ` Rafał Miłecki
2016-04-05 19:40 ` Bjorn Helgaas
2016-04-05 19:40 ` Bjorn Helgaas
2016-04-05 19:49 ` Michael Büsch [this message]
2016-04-05 19:49 ` Michael Büsch
2016-04-06 13:31 ` Bjorn Helgaas
2016-04-06 13:31 ` Bjorn Helgaas
2016-04-06 15:17 ` Michael Büsch
2016-04-06 15:17 ` Michael Büsch
2016-04-06 21:36 ` Lukas Wunner
2016-04-06 21:36 ` Lukas Wunner
2016-04-05 19:59 ` Matthew Garrett
2016-04-05 19:59 ` Matthew Garrett
2016-04-06 11:28 ` Andrew Worsley
2016-04-06 11:28 ` Andrew Worsley
2016-04-06 21:30 ` Lukas Wunner
2016-04-06 21:30 ` Lukas Wunner
2016-04-06 21:30 ` Lukas Wunner
2016-04-06 22:19 ` Matthew Garrett
2016-04-06 22:19 ` Matthew Garrett
2016-04-06 22:19 ` Matthew Garrett
2016-04-09 12:00 ` Matt Fleming
2016-04-09 12:00 ` Matt Fleming
2016-04-24 16:58 ` Lukas Wunner
2016-04-24 16:58 ` Lukas Wunner
[not found] <E1akxli-00030z-BC@bombadil.infradead.org>
2016-03-31 23:13 ` Chris Bainbridge
2016-03-31 23:13 ` Chris Bainbridge
2016-04-01 4:59 ` Michael Büsch
2016-04-01 4:59 ` Michael Büsch
2016-04-01 22:46 ` Lukas Wunner
2016-04-01 22:46 ` Lukas Wunner
2016-04-02 7:30 ` Michael Büsch
2016-04-02 7:30 ` Michael Büsch
2016-04-02 11:40 ` Andrew Worsley
2016-04-03 11:49 ` Lukas Wunner
2016-04-03 11:49 ` Lukas Wunner
2016-04-07 12:04 ` Andrew Worsley
2016-04-07 12:04 ` Andrew Worsley
2016-04-10 10:09 ` Andrew Worsley
2016-04-10 10:09 ` Andrew Worsley
2016-04-12 18:32 ` Lukas Wunner
2016-04-12 18:32 ` Lukas Wunner
2016-04-13 20:42 ` Andrew Worsley
2016-04-13 20:42 ` Andrew Worsley
2016-04-24 17:04 ` Lukas Wunner
2016-04-24 17:04 ` Lukas Wunner
2016-05-23 14:42 ` Lukas Wunner
2016-05-23 14:42 ` Lukas Wunner
2016-05-24 23:38 ` Chris Bainbridge
2016-05-24 23:38 ` Chris Bainbridge
[not found] <E1akxli-00030z-Jz@bombadil.infradead.org>
2016-03-29 17:46 ` Lukas Wunner
2016-03-29 17:46 ` Lukas Wunner
2016-03-31 19:09 ` Michael Büsch
2016-03-31 19:09 ` Michael Büsch
-- strict thread matches above, loose matches on Subject: below --
2016-03-29 17:41 Lukas Wunner
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=20160405214945.1214e13b@wiggum \
--to=m@bues.ch \
--cc=b43-dev@lists.infradead.org \
--cc=helgaas@kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=linux-wireless@vger.kernel.org \
--cc=lukas@wunner.de \
--cc=mjg59@srcf.ucam.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.