Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1420223 > unrolled thread

[PATCH v2 0/3] Reset Apple AirPort card

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 0/3] Reset Apple AirPort card Lukas Wunner <lukas@wunner.de> - 2016-06-12 12:40 +0200
    [PATCH v2 3/3] x86/quirks: Add early quirk to reset Apple AirPort  card Lukas Wunner <lukas@wunner.de> - 2016-06-12 12:40 +0200
      Re: [PATCH v2 3/3] x86/quirks: Add early quirk to reset Apple  AirPort card Ingo Molnar <mingo@kernel.org> - 2016-06-12 13:50 +0200
        Re: [PATCH v2 3/3] x86/quirks: Add early quirk to reset Apple  AirPort card Lukas Wunner <lukas@wunner.de> - 2016-06-12 14:40 +0200
          Re: [PATCH v2 3/3] x86/quirks: Add early quirk to reset Apple  AirPort card Matt Fleming <matt@codeblueprint.co.uk> - 2016-06-13 16:50 +0200
            Re: [PATCH v2 3/3] x86/quirks: Add early quirk to reset Apple  AirPort card Ingo Molnar <mingo@kernel.org> - 2016-06-14 09:40 +0200

#1420223 — [PATCH v2 0/3] Reset Apple AirPort card

Reset Apple AirPort card v2

Browsable on GitHub with green/red highlighting:
https://github.com/l1k/linux/commits/airport_reset_v2

Link to v1:
https://lkml.org/lkml/2016/5/28/163

Changes since v1:
  - Validate secondary bus number (Yinghai Lu)

  - Move amendment of nvidia_bugs quirk and scanning of secondary buses
    to separate patch to allow for easier reviewing, bisecting and
    reverting (Bjorn Helgaas)

  - Spell out reason for resetting the AirPort card in pr_info() message
    (Bjorn Helgaas)

  - Add dev_err() macro to emulate message format of PCI core
    (Bjorn Helgaas)

  - Remove line breaks in read/write_pci_config_16()
    (Ingo Molnar)

Lukas Wunner (3):
  x86/quirks: Apply nvidia_bugs quirk only on root bus
  x86/quirks: Reintroduce scanning of secondary buses
  x86/quirks: Add early quirk to reset Apple AirPort card

 arch/x86/kernel/early-quirks.c | 98 ++++++++++++++++++++++++++++++++++++------
 drivers/bcma/bcma_private.h    |  2 -
 include/linux/bcma/bcma.h      |  1 +
 3 files changed, 86 insertions(+), 15 deletions(-)

-- 
2.8.1

[toc] | [next] | [standalone]

#1420224 — [PATCH v2 3/3] x86/quirks: Add early quirk to reset Apple AirPort card

The EFI firmware on Macs contains a full-fledged network stack for
downloading OS X images from osrecovery.apple.com. Unfortunately
on Macs introduced 2011 and 2012, EFI brings up the Broadcom 4331
wireless card on every boot and leaves it enabled even after
ExitBootServices has been called. The card continues to assert its IRQ
line, causing spurious interrupts if the IRQ is shared. It also corrupts
memory by DMAing received packets, allowing for remote code execution
over the air. This only stops when a driver is loaded for the wireless
card, which may be never if the driver is not installed or blacklisted.

The issue seems to be constrained to the Broadcom 4331. Chris Milsted
has verified that the newer Broadcom 4360 built into the MacBookPro11,3
(2013/2014) does not exhibit this behaviour. The chances that Apple will
ever supply a firmware fix for the older machines appear to be zero.

The solution is to reset the card on boot by writing to a reset bit in
its mmio space. This must be done as an early quirk and not as a plain
vanilla PCI quirk to successfully combat memory corruption by DMAed
packets: Matthew Garrett found out in 2012 that the packets are written
to EfiBootServicesData memory (http://mjg59.dreamwidth.org/11235.html).
This type of memory is made available to the page allocator by
efi_free_boot_services(). Plain vanilla PCI quirks run much later, in
subsys initcall level. In-between a time window would be open for memory
corruption. Random crashes occurring in this time window and attributed
to DMAed packets have indeed been observed in the wild by Chris
Bainbridge.

When Matthew Garrett analyzed the memory corruption issue in 2012, he
sought to fix it with a grub quirk which transitions the card to D3hot:
http://git.savannah.gnu.org/cgit/grub.git/commit/?id=9d34bb85da56

This approach does not help users with other bootloaders and while it
may prevent DMAed packets, it does not cure the spurious interrupts
emanating from the card. Unfortunately the card's mmio space is
inaccessible in D3hot, so to reset it, we have to undo the effect of
Matthew's grub patch and transition the card back to D0.

Note that the quirk takes a few shortcuts to reduce the amount of code:
The size of BAR 0 and the location of the PM capability is identical
on all affected machines and therefore hardcoded. Only the address of
BAR 0 differs between models. Also, it is assumed that the BCMA core
currently mapped is the 802.11 core. The EFI driver seems to always take
care of this.

Michael Büsch, Bjorn Helgaas and Matt Fleming contributed feedback
towards finding the best solution to this problem.

The following should be a comprehensive list of affected models:
    iMac13,1        2012  21.5"       [Root Port 00:1c.3 = 8086:1e16]
    iMac13,2        2012  27"         [Root Port 00:1c.3 = 8086:1e16]
    Macmini5,1      2011  i5 2.3 GHz  [Root Port 00:1c.1 = 8086:1c12]
    Macmini5,2      2011  i5 2.5 GHz  [Root Port 00:1c.1 = 8086:1c12]
    Macmini5,3      2011  i7 2.0 GHz  [Root Port 00:1c.1 = 8086:1c12]
    Macmini6,1      2012  i5 2.5 GHz  [Root Port 00:1c.1 = 8086:1e12]
    Macmini6,2      2012  i7 2.3 GHz  [Root Port 00:1c.1 = 8086:1e12]
    MacBookPro8,1   2011  13"         [Root Port 00:1c.1 = 8086:1c12]
    MacBookPro8,2   2011  15"         [Root Port 00:1c.1 = 8086:1c12]
    MacBookPro8,3   2011  17"         [Root Port 00:1c.1 = 8086:1c12]
    MacBookPro9,1   2012  15"         [Root Port 00:1c.1 = 8086:1e12]
    MacBookPro9,2   2012  13"         [Root Port 00:1c.1 = 8086:1e12]
    MacBookPro10,1  2012  15"         [Root Port 00:1c.1 = 8086:1e12]
    MacBookPro10,2  2012  13"         [Root Port 00:1c.1 = 8086:1e12]

For posterity, spurious interrupts caused by the Broadcom 4331 wireless
card resulted in splats like this (stacktrace omitted):
    irq 17: nobody cared (try booting with the "irqpoll" option)
    handlers:
    [<ffffffff81374370>] pcie_isr
    [<ffffffffc0704550>] sdhci_irq [sdhci] threaded [<ffffffffc07013c0>] sdhci_thread_irq [sdhci]
    [<ffffffffc0a0b960>] azx_interrupt [snd_hda_codec]
    Disabling IRQ #17

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=79301
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=111781
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=728916
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=895951#c16
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1009819
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1098621
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1149632#c5
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1279130
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1332732
Tested-by: Konstantin Simanov <k.simanov@stlk.ru>        # [MacBookPro8,1]
Tested-by: Lukas Wunner <lukas@wunner.de>                # [MacBookPro9,1]
Tested-by: Bryan Paradis <bryan.paradis@gmail.com>       # [MacBookPro9,2]
Tested-by: Andrew Worsley <amworsley@gmail.com>          # [MacBookPro10,1]
Tested-by: Chris Bainbridge <chris.bainbridge@gmail.com> # [MacBookPro10,2]
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Acked-by: Rafał Miłecki <zajec5@gmail.com>
Acked-by: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Chris Milsted <cmilsted@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Matthew Garrett <mjg59@srcf.ucam.org>
Cc: Michael Buesch <m@bues.ch>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: b43-dev@lists.infradead.org
Cc: linux-pci@vger.kernel.org
Cc: linux-wireless@vger.kernel.org
Cc: stable@vger.kernel.org # 123456789abc: x86/quirks: Apply nvidia_bugs quirk only on root bus
Cc: stable@vger.kernel.org # 123456789abc: x86/quirks: Reintroduce scanning of secondary buses
Cc: stable@vger.kernel.org
---
v2:
  - Move amendment of nvidia_bugs quirk and scanning of secondary buses
    to separate patch to allow for easier reviewing, bisecting and
    reverting (Bjorn Helgaas)

  - Spell out reason for resetting the AirPort card in pr_info() message
    (Bjorn Helgaas)

  - Add dev_err() macro to emulate message format of PCI core
    (Bjorn Helgaas)

  - Remove line breaks in read/write_pci_config_16()
    (Ingo Molnar)

 arch/x86/kernel/early-quirks.c | 57 ++++++++++++++++++++++++++++++++++++++++++
 drivers/bcma/bcma_private.h    |  2 --
 include/linux/bcma/bcma.h      |  1 +
 3 files changed, 58 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c
index ea60c05..c23fb76 100644
--- a/arch/x86/kernel/early-quirks.c
+++ b/arch/x86/kernel/early-quirks.c
@@ -11,7 +11,11 @@
 
 #include <linux/pci.h>
 #include <linux/acpi.h>
+#include <linux/delay.h>
+#include <linux/dmi.h>
 #include <linux/pci_ids.h>
+#include <linux/bcma/bcma.h>
+#include <linux/bcma/bcma_regs.h>
 #include <drm/i915_drm.h>
 #include <asm/pci-direct.h>
 #include <asm/dma.h>
@@ -21,6 +25,9 @@
 #include <asm/iommu.h>
 #include <asm/gart.h>
 #include <asm/irq_remapping.h>
+#include <asm/early_ioremap.h>
+
+#define dev_err(msg)  pr_err("pci 0000:%02x:%02x.%d: %s", bus, slot, func, msg)
 
 static void __init fix_hypertransport_config(int num, int slot, int func)
 {
@@ -597,6 +604,54 @@ static void __init force_disable_hpet(int num, int slot, int func)
 #endif
 }
 
+#define BCM4331_MMIO_SIZE	16384
+#define BCM4331_PM_CAP		0x40
+#define bcma_aread32(reg)	ioread32(mmio + 1 * BCMA_CORE_SIZE + reg)
+#define bcma_awrite32(reg, val)	iowrite32(val, mmio + 1 * BCMA_CORE_SIZE + reg)
+
+static void __init apple_airport_reset(int bus, int slot, int func)
+{
+	void __iomem *mmio;
+	u16 pmcsr;
+	u64 addr;
+	int i;
+
+	if (!dmi_match(DMI_SYS_VENDOR, "Apple Inc."))
+		return;
+
+	/* Card may have been put into PCI_D3hot by grub quirk */
+	pmcsr = read_pci_config_16(bus, slot, func, BCM4331_PM_CAP + PCI_PM_CTRL);
+	if ((pmcsr & PCI_PM_CTRL_STATE_MASK) != PCI_D0) {
+		pmcsr &= ~PCI_PM_CTRL_STATE_MASK;
+		write_pci_config_16(bus, slot, func, BCM4331_PM_CAP + PCI_PM_CTRL, pmcsr);
+		mdelay(10);
+		pmcsr = read_pci_config_16(bus, slot, func, BCM4331_PM_CAP + PCI_PM_CTRL);
+		if ((pmcsr & PCI_PM_CTRL_STATE_MASK) != PCI_D0) {
+			dev_err("Cannot power up Apple AirPort card\n");
+			return;
+		}
+	}
+
+	addr  =      read_pci_config(bus, slot, func, PCI_BASE_ADDRESS_0);
+	addr |= (u64)read_pci_config(bus, slot, func, PCI_BASE_ADDRESS_1) << 32;
+	addr &= PCI_BASE_ADDRESS_MEM_MASK;
+	mmio = early_ioremap(addr, BCM4331_MMIO_SIZE);
+	if (!mmio) {
+		dev_err("Cannot iomap Apple AirPort card\n");
+		return;
+	}
+
+	pr_info("Resetting Apple AirPort card (left enabled by EFI)\n");
+	for (i = 0; bcma_aread32(BCMA_RESET_ST) && i < 30; i++)
+		udelay(10);
+	bcma_awrite32(BCMA_RESET_CTL, BCMA_RESET_CTL_RESET);
+	bcma_aread32(BCMA_RESET_CTL);
+	udelay(1);
+	bcma_awrite32(BCMA_RESET_CTL, 0);
+	bcma_aread32(BCMA_RESET_CTL);
+	udelay(10);
+	early_iounmap(mmio, BCM4331_MMIO_SIZE);
+}
 
 #define QFLAG_APPLY_ONCE 	0x1
 #define QFLAG_APPLIED		0x2
@@ -639,6 +694,8 @@ static struct chipset early_qrk[] __initdata = {
 	 */
 	{ PCI_VENDOR_ID_INTEL, 0x0f00,
 		PCI_CLASS_BRIDGE_HOST, PCI_ANY_ID, 0, force_disable_hpet},
+	{ PCI_VENDOR_ID_BROADCOM, 0x4331,
+	  PCI_CLASS_NETWORK_OTHER, PCI_ANY_ID, 0, apple_airport_reset},
 	{}
 };
 
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/include/linux/bcma/bcma.h b/include/linux/bcma/bcma.h
index e6b41f4..3db25df 100644
--- a/include/linux/bcma/bcma.h
+++ b/include/linux/bcma/bcma.h
@@ -159,6 +159,7 @@ struct bcma_host_ops {
 #define BCMA_CORE_DEFAULT		0xFFF
 
 #define BCMA_MAX_NR_CORES		16
+#define BCMA_CORE_SIZE			0x1000
 
 /* Chip IDs of PCIe devices */
 #define BCMA_CHIP_ID_BCM4313	0x4313
-- 
2.8.1

[toc] | [prev] | [next] | [standalone]

#1420248 — Re: [PATCH v2 3/3] x86/quirks: Add early quirk to reset Apple AirPort card

* Lukas Wunner <lukas@wunner.de> wrote:

> The solution is to reset the card on boot by writing to a reset bit in
> its mmio space. This must be done as an early quirk and not as a plain
> vanilla PCI quirk to successfully combat memory corruption by DMAed
> packets: Matthew Garrett found out in 2012 that the packets are written
> to EfiBootServicesData memory (http://mjg59.dreamwidth.org/11235.html).
> This type of memory is made available to the page allocator by
> efi_free_boot_services(). [...]

Btw., would it also make sense to *not* free those pages by default, if firmware 
can occasionally corrupt them? What memory size are we talking about?

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1420264 — Re: [PATCH v2 3/3] x86/quirks: Add early quirk to reset Apple AirPort card

On Sun, Jun 12, 2016 at 01:49:02PM +0200, Ingo Molnar wrote:
> * Lukas Wunner <lukas@wunner.de> wrote:
> > The solution is to reset the card on boot by writing to a reset bit in
> > its mmio space. This must be done as an early quirk and not as a plain
> > vanilla PCI quirk to successfully combat memory corruption by DMAed
> > packets: Matthew Garrett found out in 2012 that the packets are written
> > to EfiBootServicesData memory (http://mjg59.dreamwidth.org/11235.html).
> > This type of memory is made available to the page allocator by
> > efi_free_boot_services(). [...]
> 
> Btw., would it also make sense to *not* free those pages by default,
> if firmware can occasionally corrupt them? What memory size are we
> talking about?

The memory area where packets are written is relatively small,
Matthew Garret writes: "I'd traced it down to a single 31MB region
of boot service data" (https://mjg59.dreamwidth.org/11235.html)

However memory corruption by DMAed packets is only one of two issues
here. The other is the interrupt storm unleashed by the wireless card.
The IRQ is frequently shared on these models. On my machine it's
shared with the Thunderbolt controller, SDXC controller and a sound
card. I can't even authenticate with cryptsetup on boot before getting
a spurious interrupt splat.

Resetting the card solves the problem at the root and fixes both,
the spurious interrupts and the memory corruption. 

Best regards,

Lukas

[toc] | [prev] | [next] | [standalone]

#1420955 — Re: [PATCH v2 3/3] x86/quirks: Add early quirk to reset Apple AirPort card

On Sun, 12 Jun, at 02:37:26PM, Lukas Wunner wrote:
> 
> Resetting the card solves the problem at the root and fixes both,
> the spurious interrupts and the memory corruption. 

It also avoids the need to figure out exactly which Boot Services
regions may have become corrupt.

This would be necessary since you can't keep all Boot Services regions
reserved because those regions can add up to be many gigabytes in
size, even on relatively low-end laptops that don't have huge amounts
of RAM to begin with.

Freeing Boot Services regions is pretty important.

[toc] | [prev] | [next] | [standalone]

#1421631 — Re: [PATCH v2 3/3] x86/quirks: Add early quirk to reset Apple AirPort card

* Matt Fleming <matt@codeblueprint.co.uk> wrote:

> On Sun, 12 Jun, at 02:37:26PM, Lukas Wunner wrote:
> > 
> > Resetting the card solves the problem at the root and fixes both,
> > the spurious interrupts and the memory corruption. 
> 
> It also avoids the need to figure out exactly which Boot Services
> regions may have become corrupt.
> 
> This would be necessary since you can't keep all Boot Services regions
> reserved because those regions can add up to be many gigabytes in
> size, even on relatively low-end laptops that don't have huge amounts
> of RAM to begin with.
> 
> Freeing Boot Services regions is pretty important.

Ok!

Thanks,

	Ingo

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web