[PATCH] x86: Add early quirk to reset Apple AirPort card

[PATCH] x86: Add early quirk to reset Apple AirPort card

[Lukas Wunner]

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.

Since commit 8659c406ade3 ("x86: only scan the root bus in early PCI
quirks"), early quirks can only be applied to devices on the root bus.
However the Broadcom 4331 card is located on a secondary bus behind a
PCIe root port. The present commit therefore reintroduces scanning of
secondary buses. The primary motivation of 8659c406ade3 was to prevent
application of the nvidia_bugs() quirk on secondary buses. Amend the
quirk to open code this requirement.

A secondary motivation was to speed up PCI scanning. The algorithm used
prior to 8659c406ade3 was particularly time consuming because it scanned
buses 0 to 31 brute force. The recursive algorithm used by the present
commit only scans buses that are actually reachable from the root bus
and should thus be a bit faster. If this algorithm is found to
significantly impact boot time, it would be possible to limit its
recursion depth: The Apple AirPort quirk applies at depth 1, all others
at depth 0, so the bus need not be scanned deeper than that for now. An
alternative approach would be to continue scanning only the root bus,
and apply the AirPort quirk to the root ports 8086:1c12, 8086:1e12 and
8086:1e16. Apple always positioned the Broadcom 4331 behind one of these
three ports (see model list below). The quirk would then check presence
of the Broadcom 4331 in slot 0 below the root port and do its deed.

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
Cc: Chris Milsted <cmilsted@redhat.com>
Cc: Matthew Garrett <mjg59@srcf.ucam.org>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Michael Buesch <m@bues.ch>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: x86@kernel.org
Cc: stable@vger.kernel.org
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>
---
 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(-)

diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c
index bca14c8..2d44f28 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,7 @@
 #include <asm/iommu.h>
 #include <asm/gart.h>
 #include <asm/irq_remapping.h>
+#include <asm/early_ioremap.h>
 
 static void __init fix_hypertransport_config(int num, int slot, int func)
 {
@@ -76,6 +81,13 @@ static void __init nvidia_bugs(int num, int slot, int func)
 #ifdef CONFIG_ACPI
 #ifdef CONFIG_X86_IO_APIC
 	/*
+	 * Only applies to Nvidia root ports (bus 0) and not to
+	 * Nvidia graphics cards with PCI ports on secondary buses.
+	 */
+	if (num)
+		return;
+
+	/*
 	 * All timer overrides on Nvidia are
 	 * wrong unless HPET is enabled.
 	 * Unfortunately that's not true on many Asus boards.
@@ -590,6 +602,57 @@ 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) {
+			pr_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) {
+		pr_err("Cannot iomap Apple AirPort card\n");
+		return;
+	}
+
+	pr_info("Resetting Apple AirPort card\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
@@ -603,12 +666,6 @@ struct chipset {
 	void (*f)(int num, int slot, int func);
 };
 
-/*
- * Only works for devices on the root bus. If you add any devices
- * not on bus 0 readd another loop level in early_quirks(). But
- * be careful because at least the Nvidia quirk here relies on
- * only matching on bus 0.
- */
 static struct chipset early_qrk[] __initdata = {
 	{ PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID,
 	  PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, QFLAG_APPLY_ONCE, nvidia_bugs },
@@ -638,9 +695,13 @@ 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},
 	{}
 };
 
+static void __init early_pci_scan_bus(int bus);
+
 /**
  * check_dev_quirk - apply early quirks to a given PCI device
  * @num: bus number
@@ -649,7 +710,7 @@ static struct chipset early_qrk[] __initdata = {
  *
  * Check the vendor & device ID against the early quirks table.
  *
- * If the device is single function, let early_quirks() know so we don't
+ * If the device is single function, let early_pci_scan_bus() know so we don't
  * poke at this device again.
  */
 static int __init check_dev_quirk(int num, int slot, int func)
@@ -658,6 +719,7 @@ static int __init check_dev_quirk(int num, int slot, int func)
 	u16 vendor;
 	u16 device;
 	u8 type;
+	u8 sec;
 	int i;
 
 	class = read_pci_config_16(num, slot, func, PCI_CLASS_DEVICE);
@@ -685,25 +747,35 @@ static int __init check_dev_quirk(int num, int slot, int func)
 
 	type = read_pci_config_byte(num, slot, func,
 				    PCI_HEADER_TYPE);
+
+	if ((type & 0x7f) == PCI_HEADER_TYPE_BRIDGE) {
+		sec = read_pci_config_byte(num, slot, func, PCI_SECONDARY_BUS);
+		early_pci_scan_bus(sec);
+	}
+
 	if (!(type & 0x80))
 		return -1;
 
 	return 0;
 }
 
-void __init early_quirks(void)
+static void __init early_pci_scan_bus(int bus)
 {
 	int slot, func;
 
-	if (!early_pci_allowed())
-		return;
-
 	/* Poor man's PCI discovery */
-	/* Only scan the root bus */
 	for (slot = 0; slot < 32; slot++)
 		for (func = 0; func < 8; func++) {
 			/* Only probe function 0 on single fn devices */
-			if (check_dev_quirk(0, slot, func))
+			if (check_dev_quirk(bus, slot, func))
 				break;
 		}
 }
+
+void __init early_quirks(void)
+{
+	if (!early_pci_allowed())
+		return;
+
+	early_pci_scan_bus(0);
+}
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 0367c63..5c37b58 100644
--- a/include/linux/bcma/bcma.h
+++ b/include/linux/bcma/bcma.h
@@ -158,6 +158,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

[PATCH] x86: Add early quirk to reset Apple AirPort card

[Lukas Wunner]

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.

Since commit 8659c406ade3 ("x86: only scan the root bus in early PCI
quirks"), early quirks can only be applied to devices on the root bus.
However the Broadcom 4331 card is located on a secondary bus behind a
PCIe root port. The present commit therefore reintroduces scanning of
secondary buses. The primary motivation of 8659c406ade3 was to prevent
application of the nvidia_bugs() quirk on secondary buses. Amend the
quirk to open code this requirement.

A secondary motivation was to speed up PCI scanning. The algorithm used
prior to 8659c406ade3 was particularly time consuming because it scanned
buses 0 to 31 brute force. The recursive algorithm used by the present
commit only scans buses that are actually reachable from the root bus
and should thus be a bit faster. If this algorithm is found to
significantly impact boot time, it would be possible to limit its
recursion depth: The Apple AirPort quirk applies at depth 1, all others
at depth 0, so the bus need not be scanned deeper than that for now. An
alternative approach would be to continue scanning only the root bus,
and apply the AirPort quirk to the root ports 8086:1c12, 8086:1e12 and
8086:1e16. Apple always positioned the Broadcom 4331 behind one of these
three ports (see model list below). The quirk would then check presence
of the Broadcom 4331 in slot 0 below the root port and do its deed.

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
Cc: Chris Milsted <cmilsted@redhat.com>
Cc: Matthew Garrett <mjg59@srcf.ucam.org>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Michael Buesch <m@bues.ch>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: x86 at kernel.org
Cc: stable at vger.kernel.org
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>
---
 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(-)

diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c
index bca14c8..2d44f28 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,7 @@
 #include <asm/iommu.h>
 #include <asm/gart.h>
 #include <asm/irq_remapping.h>
+#include <asm/early_ioremap.h>
 
 static void __init fix_hypertransport_config(int num, int slot, int func)
 {
@@ -76,6 +81,13 @@ static void __init nvidia_bugs(int num, int slot, int func)
 #ifdef CONFIG_ACPI
 #ifdef CONFIG_X86_IO_APIC
 	/*
+	 * Only applies to Nvidia root ports (bus 0) and not to
+	 * Nvidia graphics cards with PCI ports on secondary buses.
+	 */
+	if (num)
+		return;
+
+	/*
 	 * All timer overrides on Nvidia are
 	 * wrong unless HPET is enabled.
 	 * Unfortunately that's not true on many Asus boards.
@@ -590,6 +602,57 @@ 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) {
+			pr_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) {
+		pr_err("Cannot iomap Apple AirPort card\n");
+		return;
+	}
+
+	pr_info("Resetting Apple AirPort card\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
@@ -603,12 +666,6 @@ struct chipset {
 	void (*f)(int num, int slot, int func);
 };
 
-/*
- * Only works for devices on the root bus. If you add any devices
- * not on bus 0 readd another loop level in early_quirks(). But
- * be careful because at least the Nvidia quirk here relies on
- * only matching on bus 0.
- */
 static struct chipset early_qrk[] __initdata = {
 	{ PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID,
 	  PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, QFLAG_APPLY_ONCE, nvidia_bugs },
@@ -638,9 +695,13 @@ 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},
 	{}
 };
 
+static void __init early_pci_scan_bus(int bus);
+
 /**
  * check_dev_quirk - apply early quirks to a given PCI device
  * @num: bus number
@@ -649,7 +710,7 @@ static struct chipset early_qrk[] __initdata = {
  *
  * Check the vendor & device ID against the early quirks table.
  *
- * If the device is single function, let early_quirks() know so we don't
+ * If the device is single function, let early_pci_scan_bus() know so we don't
  * poke@this device again.
  */
 static int __init check_dev_quirk(int num, int slot, int func)
@@ -658,6 +719,7 @@ static int __init check_dev_quirk(int num, int slot, int func)
 	u16 vendor;
 	u16 device;
 	u8 type;
+	u8 sec;
 	int i;
 
 	class = read_pci_config_16(num, slot, func, PCI_CLASS_DEVICE);
@@ -685,25 +747,35 @@ static int __init check_dev_quirk(int num, int slot, int func)
 
 	type = read_pci_config_byte(num, slot, func,
 				    PCI_HEADER_TYPE);
+
+	if ((type & 0x7f) == PCI_HEADER_TYPE_BRIDGE) {
+		sec = read_pci_config_byte(num, slot, func, PCI_SECONDARY_BUS);
+		early_pci_scan_bus(sec);
+	}
+
 	if (!(type & 0x80))
 		return -1;
 
 	return 0;
 }
 
-void __init early_quirks(void)
+static void __init early_pci_scan_bus(int bus)
 {
 	int slot, func;
 
-	if (!early_pci_allowed())
-		return;
-
 	/* Poor man's PCI discovery */
-	/* Only scan the root bus */
 	for (slot = 0; slot < 32; slot++)
 		for (func = 0; func < 8; func++) {
 			/* Only probe function 0 on single fn devices */
-			if (check_dev_quirk(0, slot, func))
+			if (check_dev_quirk(bus, slot, func))
 				break;
 		}
 }
+
+void __init early_quirks(void)
+{
+	if (!early_pci_allowed())
+		return;
+
+	early_pci_scan_bus(0);
+}
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 0367c63..5c37b58 100644
--- a/include/linux/bcma/bcma.h
+++ b/include/linux/bcma/bcma.h
@@ -158,6 +158,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

Re: [PATCH] x86: Add early quirk to reset Apple AirPort card

[Matt Fleming]

On Sun, 29 May, at 01:35:28AM, Lukas Wunner wrote:
> 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.

[... Snip a very thorough changelog ...]

This patch looks fine to me from an EFI perspective.

Acked-by: Matt Fleming <matt@codeblueprint.co.uk>

[PATCH] x86: Add early quirk to reset Apple AirPort card

[Matt Fleming]

On Sun, 29 May, at 01:35:28AM, Lukas Wunner wrote:
> 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.

[... Snip a very thorough changelog ...]

This patch looks fine to me from an EFI perspective.

Acked-by: Matt Fleming <matt@codeblueprint.co.uk>

[tip:x86/urgent] x86/quirks: Add early quirk to reset Apple AirPort card

[tip-bot for Lukas Wunner]

Commit-ID:  625a99d9bfd038ea492f57308555bf4e607ce591
Gitweb:     http://git.kernel.org/tip/625a99d9bfd038ea492f57308555bf4e607ce591
Author:     Lukas Wunner <lukas@wunner.de>
AuthorDate: Sun, 29 May 2016 01:35:28 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 8 Jun 2016 15:06:06 +0200

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.

Since commit 8659c406ade3 ("x86: only scan the root bus in early PCI
quirks"), early quirks can only be applied to devices on the root bus.
However the Broadcom 4331 card is located on a secondary bus behind a
PCIe root port. The present commit therefore reintroduces scanning of
secondary buses. The primary motivation of 8659c406ade3 was to prevent
application of the nvidia_bugs() quirk on secondary buses. Amend the
quirk to open code this requirement.

A secondary motivation was to speed up PCI scanning. The algorithm used
prior to 8659c406ade3 was particularly time consuming because it scanned
buses 0 to 31 brute force. The recursive algorithm used by the present
commit only scans buses that are actually reachable from the root bus
and should thus be a bit faster. If this algorithm is found to
significantly impact boot time, it would be possible to limit its
recursion depth: The Apple AirPort quirk applies at depth 1, all others
at depth 0, so the bus need not be scanned deeper than that for now. An
alternative approach would be to continue scanning only the root bus,
and apply the AirPort quirk to the root ports 8086:1c12, 8086:1e12 and
8086:1e16. Apple always positioned the Broadcom 4331 behind one of these
three ports (see model list below). The quirk would then check presence
of the Broadcom 4331 in slot 0 below the root port and do its deed.

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>
[ Cleaned up the code a bit. ]
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: 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
Link: http://lkml.kernel.org/r/82c2548dffc6cfbc484b9111b1073f407c946061.1464477483.git.lukas@wunner.de
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/early-quirks.c | 95 ++++++++++++++++++++++++++++++++++++------
 drivers/bcma/bcma_private.h    |  2 -
 include/linux/bcma/bcma.h      |  1 +
 3 files changed, 83 insertions(+), 15 deletions(-)

diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c
index bca14c8..4e4e499 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,7 @@
 #include <asm/iommu.h>
 #include <asm/gart.h>
 #include <asm/irq_remapping.h>
+#include <asm/early_ioremap.h>
 
 static void __init fix_hypertransport_config(int num, int slot, int func)
 {
@@ -76,6 +81,13 @@ static void __init nvidia_bugs(int num, int slot, int func)
 #ifdef CONFIG_ACPI
 #ifdef CONFIG_X86_IO_APIC
 	/*
+	 * Only applies to Nvidia root ports (bus 0) and not to
+	 * Nvidia graphics cards with PCI ports on secondary buses.
+	 */
+	if (num)
+		return;
+
+	/*
 	 * All timer overrides on Nvidia are
 	 * wrong unless HPET is enabled.
 	 * Unfortunately that's not true on many Asus boards.
@@ -590,6 +602,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) {
+			pr_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) {
+		pr_err("Cannot iomap Apple AirPort card\n");
+		return;
+	}
+
+	pr_info("Resetting Apple AirPort card\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
@@ -603,12 +663,6 @@ struct chipset {
 	void (*f)(int num, int slot, int func);
 };
 
-/*
- * Only works for devices on the root bus. If you add any devices
- * not on bus 0 readd another loop level in early_quirks(). But
- * be careful because at least the Nvidia quirk here relies on
- * only matching on bus 0.
- */
 static struct chipset early_qrk[] __initdata = {
 	{ PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID,
 	  PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, QFLAG_APPLY_ONCE, nvidia_bugs },
@@ -638,9 +692,13 @@ 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},
 	{}
 };
 
+static void __init early_pci_scan_bus(int bus);
+
 /**
  * check_dev_quirk - apply early quirks to a given PCI device
  * @num: bus number
@@ -649,7 +707,7 @@ static struct chipset early_qrk[] __initdata = {
  *
  * Check the vendor & device ID against the early quirks table.
  *
- * If the device is single function, let early_quirks() know so we don't
+ * If the device is single function, let early_pci_scan_bus() know so we don't
  * poke at this device again.
  */
 static int __init check_dev_quirk(int num, int slot, int func)
@@ -658,6 +716,7 @@ static int __init check_dev_quirk(int num, int slot, int func)
 	u16 vendor;
 	u16 device;
 	u8 type;
+	u8 sec;
 	int i;
 
 	class = read_pci_config_16(num, slot, func, PCI_CLASS_DEVICE);
@@ -685,25 +744,35 @@ static int __init check_dev_quirk(int num, int slot, int func)
 
 	type = read_pci_config_byte(num, slot, func,
 				    PCI_HEADER_TYPE);
+
+	if ((type & 0x7f) == PCI_HEADER_TYPE_BRIDGE) {
+		sec = read_pci_config_byte(num, slot, func, PCI_SECONDARY_BUS);
+		early_pci_scan_bus(sec);
+	}
+
 	if (!(type & 0x80))
 		return -1;
 
 	return 0;
 }
 
-void __init early_quirks(void)
+static void __init early_pci_scan_bus(int bus)
 {
 	int slot, func;
 
-	if (!early_pci_allowed())
-		return;
-
 	/* Poor man's PCI discovery */
-	/* Only scan the root bus */
 	for (slot = 0; slot < 32; slot++)
 		for (func = 0; func < 8; func++) {
 			/* Only probe function 0 on single fn devices */
-			if (check_dev_quirk(0, slot, func))
+			if (check_dev_quirk(bus, slot, func))
 				break;
 		}
 }
+
+void __init early_quirks(void)
+{
+	if (!early_pci_allowed())
+		return;
+
+	early_pci_scan_bus(0);
+}
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

Re: [tip:x86/urgent] x86/quirks: Add early quirk to reset Apple AirPort card

[Yinghai Lu]

On Wed, Jun 8, 2016 at 7:23 AM, tip-bot for Lukas Wunner
<tipbot@zytor.com> wrote:
> Commit-ID:  625a99d9bfd038ea492f57308555bf4e607ce591
> Gitweb:     http://git.kernel.org/tip/625a99d9bfd038ea492f57308555bf4e607ce591
> Author:     Lukas Wunner <lukas@wunner.de>
> AuthorDate: Sun, 29 May 2016 01:35:28 +0200
...
> ---
>  arch/x86/kernel/early-quirks.c | 95 ++++++++++++++++++++++++++++++++++++------
>  drivers/bcma/bcma_private.h    |  2 -
>  include/linux/bcma/bcma.h      |  1 +
>  3 files changed, 83 insertions(+), 15 deletions(-)
>
> diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c
> index bca14c8..4e4e499 100644
> --- a/arch/x86/kernel/early-quirks.c
> +++ b/arch/x86/kernel/early-quirks.c
...

Extend bus scan range part should be in separated patch?

and put apple_airport_reset() related in second patch.

> -/*
> - * Only works for devices on the root bus. If you add any devices
> - * not on bus 0 readd another loop level in early_quirks(). But
> - * be careful because at least the Nvidia quirk here relies on
> - * only matching on bus 0.
> - */
>  static struct chipset early_qrk[] __initdata = {
>         { PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID,
>           PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, QFLAG_APPLY_ONCE, nvidia_bugs },
> @@ -638,9 +692,13 @@ 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},
>         {}
>  };
>
> +static void __init early_pci_scan_bus(int bus);
> +
>  /**
>   * check_dev_quirk - apply early quirks to a given PCI device
>   * @num: bus number
> @@ -649,7 +707,7 @@ static struct chipset early_qrk[] __initdata = {
>   *
>   * Check the vendor & device ID against the early quirks table.
>   *
> - * If the device is single function, let early_quirks() know so we don't
> + * If the device is single function, let early_pci_scan_bus() know so we don't
>   * poke at this device again.
>   */
>  static int __init check_dev_quirk(int num, int slot, int func)
> @@ -658,6 +716,7 @@ static int __init check_dev_quirk(int num, int slot, int func)
>         u16 vendor;
>         u16 device;
>         u8 type;
> +       u8 sec;
>         int i;
>
>         class = read_pci_config_16(num, slot, func, PCI_CLASS_DEVICE);
> @@ -685,25 +744,35 @@ static int __init check_dev_quirk(int num, int slot, int func)
>
>         type = read_pci_config_byte(num, slot, func,
>                                     PCI_HEADER_TYPE);
> +
> +       if ((type & 0x7f) == PCI_HEADER_TYPE_BRIDGE) {
> +               sec = read_pci_config_byte(num, slot, func, PCI_SECONDARY_BUS);
> +               early_pci_scan_bus(sec);

How do you know that sec is valid ?

How about on the system that have one bridge that still have sec num register 0?

it will be get into dead loop.

> +       }
> +
>         if (!(type & 0x80))
>                 return -1;
>
>         return 0;
>  }
>
> -void __init early_quirks(void)
> +static void __init early_pci_scan_bus(int bus)
>  {
>         int slot, func;
>
> -       if (!early_pci_allowed())
> -               return;
> -
>         /* Poor man's PCI discovery */
> -       /* Only scan the root bus */
>         for (slot = 0; slot < 32; slot++)
>                 for (func = 0; func < 8; func++) {
>                         /* Only probe function 0 on single fn devices */
> -                       if (check_dev_quirk(0, slot, func))
> +                       if (check_dev_quirk(bus, slot, func))
>                                 break;
>                 }
>  }
> +
> +void __init early_quirks(void)
> +{
> +       if (!early_pci_allowed())
> +               return;
> +
> +       early_pci_scan_bus(0);
> +}

Would just revert to scan domain 0, bus 0 to 255.

Thanks

Yinghai

Re: [tip:x86/urgent] x86/quirks: Add early quirk to reset Apple AirPort card

[Lukas Wunner]

On Wed, Jun 08, 2016 at 11:56:14AM -0700, Yinghai Lu wrote:
> On Wed, Jun 8, 2016 at 7:23 AM, tip-bot for Lukas Wunner <tipbot@zytor.com> wrote:
> > --- a/arch/x86/kernel/early-quirks.c
> > +++ b/arch/x86/kernel/early-quirks.c
> ...
> 
> Extend bus scan range part should be in separated patch?
> 
> and put apple_airport_reset() related in second patch.

I had considered that but decided against it as it would make things
a bit more complicated for stable maintainers.

Nevertheless I'll be happy to split in two if this is the consensus?
I'm not sure though if x86/urgent is a rebasing branch?


> > +
> > +       if ((type & 0x7f) == PCI_HEADER_TYPE_BRIDGE) {
> > +               sec = read_pci_config_byte(num, slot, func, PCI_SECONDARY_BUS);
> > +               early_pci_scan_bus(sec);
> 
> How do you know that sec is valid ?
> 
> How about on the system that have one bridge that still have sec num register 0?
> 
> it will be get into dead loop.

Good point. I've just checked pci_scan_bridge() and it does verify that
and avoids recursing to a child bus if it's number is zero. It also
ensures that sec > num before recursing.

I can provide a follow-up patch to fix that, will wait a bit though to
see if there are further comments.


> > -void __init early_quirks(void)
> > +static void __init early_pci_scan_bus(int bus)
> >  {
> >         int slot, func;
> >
> > -       if (!early_pci_allowed())
> > -               return;
> > -
> >         /* Poor man's PCI discovery */
> > -       /* Only scan the root bus */
> >         for (slot = 0; slot < 32; slot++)
> >                 for (func = 0; func < 8; func++) {
> >                         /* Only probe function 0 on single fn devices */
> > -                       if (check_dev_quirk(0, slot, func))
> > +                       if (check_dev_quirk(bus, slot, func))
> >                                 break;
> >                 }
> >  }
> > +
> > +void __init early_quirks(void)
> > +{
> > +       if (!early_pci_allowed())
> > +               return;
> > +
> > +       early_pci_scan_bus(0);
> > +}
> 
> Would just revert to scan domain 0, bus 0 to 255.

The commit message of 8659c406ade3 sounds like this would lengthen
boot time noticeably, so I expected significant backlash had I just
gone back to this old algorithm.

Thanks,

Lukas

Re: [tip:x86/urgent] x86/quirks: Add early quirk to reset Apple AirPort card

[Ingo Molnar]

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

> > How do you know that sec is valid ?
> > 
> > How about on the system that have one bridge that still have sec num register 0?
> > 
> > it will be get into dead loop.
> 
> Good point. I've just checked pci_scan_bridge() and it does verify that and 
> avoids recursing to a child bus if it's number is zero. It also ensures that sec
> > num before recursing.
> 
> I can provide a follow-up patch to fix that, will wait a bit though to see if 
> there are further comments.

Please send a delta fix on top of x86/urgent. (I'll probably squash the two fixes 
together.)

Thanks,

	Ingo

Re: [tip:x86/urgent] x86/quirks: Add early quirk to reset Apple AirPort card

[Lukas Wunner]

On Thu, Jun 09, 2016 at 08:48:03AM +0200, Ingo Molnar wrote:
> * Lukas Wunner <lukas@wunner.de> wrote:
> > On Wed, Jun 08, 2016 at 11:56:14AM -0700, Yinghai Lu wrote:
> > > How do you know that sec is valid ?
> > > How about on the system that have one bridge that still have sec num
> > > register 0?
> > > it will be get into dead loop.
> > 
> > Good point. I've just checked pci_scan_bridge() and it does verify that
> > and avoids recursing to a child bus if it's number is zero. It also
> > ensures that sec > num before recursing.
> 
> Please send a delta fix on top of x86/urgent. (I'll probably squash the
> two fixes together.)

Please find my suggestion for a delta fix included below. Can be squashed
or applied on top as you see fit. I've also tested this on my machine and
verified that the AirPort card is still properly reset. If there is
anything else I can do please let me know.

Thanks,

Lukas

-- >8 --
Subject: [PATCH] x86/quirks: Validate secondary bus number

We used to brute-force scan buses 0 to 31 until commit 8659c406ade3
("x86: only scan the root bus in early PCI quirks") constrained the scan
to the root bus, in part to shorten boot time.

Commit 625a99d9bfd0 ("x86/quirks: Add early quirk to reset Apple AirPort
card") reintroduced scanning of secondary buses, but used a recursive
strategy to scan only buses that are actually present.

However the secondary bus number read from a bridge's config space may
be invalid, in particular a value of 0 causes an infinite loop. The PCI
core goes beyond that and recurses to a child bus only if its bus number
is greater than the parent bus number (see pci_scan_bridge()). Since the
root bus is numbered 0, this implies that secondary buses may not be 0.
Do the same on early scanning.

Suggested-by: Yinghai Lu <yinghai@kernel.org>
Fixes: 625a99d9bfd0 ("x86/quirks: Add early quirk to reset Apple AirPort card")
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 arch/x86/kernel/early-quirks.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c
index 4e4e499..c24070e 100644
--- a/arch/x86/kernel/early-quirks.c
+++ b/arch/x86/kernel/early-quirks.c
@@ -747,7 +747,8 @@ static int __init check_dev_quirk(int num, int slot, int func)
 
 	if ((type & 0x7f) == PCI_HEADER_TYPE_BRIDGE) {
 		sec = read_pci_config_byte(num, slot, func, PCI_SECONDARY_BUS);
-		early_pci_scan_bus(sec);
+		if (sec > num)
+			early_pci_scan_bus(sec);
 	}
 
 	if (!(type & 0x80))
-- 
2.8.1

Re: [tip:x86/urgent] x86/quirks: Add early quirk to reset Apple AirPort card

[Yinghai Lu]

On Thu, Jun 9, 2016 at 4:04 AM, Lukas Wunner <lukas@wunner.de> wrote:
> Subject: [PATCH] x86/quirks: Validate secondary bus number
>
> We used to brute-force scan buses 0 to 31 until commit 8659c406ade3
> ("x86: only scan the root bus in early PCI quirks") constrained the scan
> to the root bus, in part to shorten boot time.
>
> Commit 625a99d9bfd0 ("x86/quirks: Add early quirk to reset Apple AirPort
> card") reintroduced scanning of secondary buses, but used a recursive
> strategy to scan only buses that are actually present.
>
> However the secondary bus number read from a bridge's config space may
> be invalid, in particular a value of 0 causes an infinite loop. The PCI
> core goes beyond that and recurses to a child bus only if its bus number
> is greater than the parent bus number (see pci_scan_bridge()). Since the
> root bus is numbered 0, this implies that secondary buses may not be 0.
> Do the same on early scanning.
>
> Suggested-by: Yinghai Lu <yinghai@kernel.org>
> Fixes: 625a99d9bfd0 ("x86/quirks: Add early quirk to reset Apple AirPort card")
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> ---
>  arch/x86/kernel/early-quirks.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c
> index 4e4e499..c24070e 100644
> --- a/arch/x86/kernel/early-quirks.c
> +++ b/arch/x86/kernel/early-quirks.c
> @@ -747,7 +747,8 @@ static int __init check_dev_quirk(int num, int slot, int func)
>
>         if ((type & 0x7f) == PCI_HEADER_TYPE_BRIDGE) {
>                 sec = read_pci_config_byte(num, slot, func, PCI_SECONDARY_BUS);
> -               early_pci_scan_bus(sec);
> +               if (sec > num)
> +                       early_pci_scan_bus(sec);
>         }
>
>         if (!(type & 0x80))

If two bridges have sec, then early_pci_scan_bus still could be called
two times.

Maybe we can add one static array to recorded scanned bus.

static unsigned char scanned_bus[256];

and in early_pci_scan_bus check and set that before scanning.

Thanks

Yinghai

Re: [tip:x86/urgent] x86/quirks: Add early quirk to reset Apple AirPort card

[Lukas Wunner]

On Thu, Jun 09, 2016 at 09:37:53AM -0700, Yinghai Lu wrote:
> On Thu, Jun 9, 2016 at 4:04 AM, Lukas Wunner <lukas@wunner.de> wrote:
> > --- a/arch/x86/kernel/early-quirks.c
> > +++ b/arch/x86/kernel/early-quirks.c
> > @@ -747,7 +747,8 @@ static int __init check_dev_quirk(int num, int slot, int func)
> >
> >         if ((type & 0x7f) == PCI_HEADER_TYPE_BRIDGE) {
> >                 sec = read_pci_config_byte(num, slot, func, PCI_SECONDARY_BUS);
> > -               early_pci_scan_bus(sec);
> > +               if (sec > num)
> > +                       early_pci_scan_bus(sec);
> >         }
> >
> >         if (!(type & 0x80))
> 
> If two bridges have sec, then early_pci_scan_bus still could be called
> two times.
> 
> Maybe we can add one static array to recorded scanned bus.
> static unsigned char scanned_bus[256];
> and in early_pci_scan_bus check and set that before scanning.

Well, the PCI core would also scan such a bus twice AFAICS.
And the performance penalty of scanning it twice seems negligible.
Early quirks can prevent double execution by setting QFLAG_APPLY_ONCE.
(Three quirks have set that flag already.)

So I think this shouldn't be a concern.

Thanks,

Lukas

Re: [tip:x86/urgent] x86/quirks: Add early quirk to reset Apple AirPort card

[Yinghai Lu]

On 6/9/16, Lukas Wunner <lukas@wunner.de> wrote:
>
> Well, the PCI core would also scan such a bus twice AFAICS.
> And the performance penalty of scanning it twice seems negligible.
> Early quirks can prevent double execution by setting QFLAG_APPLY_ONCE.
> (Three quirks have set that flag already.)
>
> So I think this shouldn't be a concern.

I don't know. I would like see sth like following, and that is simple enough.

Index: linux-2.6/arch/x86/kernel/early-quirks.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/early-quirks.c
+++ linux-2.6/arch/x86/kernel/early-quirks.c
@@ -755,10 +755,16 @@ static int __init check_dev_quirk(int nu
        return 0;
 }

+static unsigned char __initdata scanned[256];
 static void __init early_pci_scan_bus(int bus)
 {
        int slot, func;

+       if (scanned[bus])
+               return;
+
+       scanned[bus] = 1;
+
        /* Poor man's PCI discovery */
        for (slot = 0; slot < 32; slot++)
                for (func = 0; func < 8; func++) {

Re: [tip:x86/urgent] x86/quirks: Add early quirk to reset Apple AirPort card

[Ingo Molnar]

* Yinghai Lu <yinghai@kernel.org> wrote:

> On 6/9/16, Lukas Wunner <lukas@wunner.de> wrote:
> >
> > Well, the PCI core would also scan such a bus twice AFAICS.
> > And the performance penalty of scanning it twice seems negligible.
> > Early quirks can prevent double execution by setting QFLAG_APPLY_ONCE.
> > (Three quirks have set that flag already.)
> >
> > So I think this shouldn't be a concern.
> 
> I don't know. I would like see sth like following, and that is simple enough.
> 
> Index: linux-2.6/arch/x86/kernel/early-quirks.c
> ===================================================================
> --- linux-2.6.orig/arch/x86/kernel/early-quirks.c
> +++ linux-2.6/arch/x86/kernel/early-quirks.c
> @@ -755,10 +755,16 @@ static int __init check_dev_quirk(int nu
>         return 0;
>  }
> 
> +static unsigned char __initdata scanned[256];
>  static void __init early_pci_scan_bus(int bus)
>  {
>         int slot, func;
> 
> +       if (scanned[bus])
> +               return;
> +
> +       scanned[bus] = 1;
> +
>         /* Poor man's PCI discovery */
>         for (slot = 0; slot < 32; slot++)
>                 for (func = 0; func < 8; func++) {

Ok, I removed the fix from tip:x86/urgent from the time being - could you guys 
please send a full version once a final approach is agreed upon?

Thanks,

	Ingo

Re: [tip:x86/urgent] x86/quirks: Add early quirk to reset Apple AirPort card

[Lukas Wunner]

On Fri, Jun 10, 2016 at 01:58:45PM +0200, Ingo Molnar wrote:
> * Yinghai Lu <yinghai@kernel.org> wrote:
> > On 6/9/16, Lukas Wunner <lukas@wunner.de> wrote:
> > > Well, the PCI core would also scan such a bus twice AFAICS.
> > > And the performance penalty of scanning it twice seems negligible.
> > > Early quirks can prevent double execution by setting QFLAG_APPLY_ONCE.
> > > (Three quirks have set that flag already.)
> > >
> > > So I think this shouldn't be a concern.
> > 
> > I don't know. I would like see sth like following, and that is simple
> > enough.
> > 
> > --- linux-2.6.orig/arch/x86/kernel/early-quirks.c
> > +++ linux-2.6/arch/x86/kernel/early-quirks.c
> > @@ -755,10 +755,16 @@ static int __init check_dev_quirk(int nu
> >         return 0;
> >  }
> > 
> > +static unsigned char __initdata scanned[256];
> >  static void __init early_pci_scan_bus(int bus)
> >  {
> >         int slot, func;
> > 
> > +       if (scanned[bus])
> > +               return;
> > +
> > +       scanned[bus] = 1;
> > +
> >         /* Poor man's PCI discovery */
> >         for (slot = 0; slot < 32; slot++)
> >                 for (func = 0; func < 8; func++) {
> 
> Ok, I removed the fix from tip:x86/urgent from the time being - could you
> guys  please send a full version once a final approach is agreed upon?

IMHO the above patch to prevent double scanning isn't needed
and less code is usually better. So my suggestion would be the
patch as originally sent plus the delta fix I sent yesterday,
either squashed or applied separately.

Since Yinghai Lu seems to disagree I guess you as the maintainer
will have to make a decision. :-)

Thanks,

Lukas

Re: [tip:x86/urgent] x86/quirks: Add early quirk to reset Apple AirPort card

[Ingo Molnar]

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

> On Fri, Jun 10, 2016 at 01:58:45PM +0200, Ingo Molnar wrote:
> > * Yinghai Lu <yinghai@kernel.org> wrote:
> > > On 6/9/16, Lukas Wunner <lukas@wunner.de> wrote:
> > > > Well, the PCI core would also scan such a bus twice AFAICS.
> > > > And the performance penalty of scanning it twice seems negligible.
> > > > Early quirks can prevent double execution by setting QFLAG_APPLY_ONCE.
> > > > (Three quirks have set that flag already.)
> > > >
> > > > So I think this shouldn't be a concern.
> > > 
> > > I don't know. I would like see sth like following, and that is simple
> > > enough.
> > > 
> > > --- linux-2.6.orig/arch/x86/kernel/early-quirks.c
> > > +++ linux-2.6/arch/x86/kernel/early-quirks.c
> > > @@ -755,10 +755,16 @@ static int __init check_dev_quirk(int nu
> > >         return 0;
> > >  }
> > > 
> > > +static unsigned char __initdata scanned[256];
> > >  static void __init early_pci_scan_bus(int bus)
> > >  {
> > >         int slot, func;
> > > 
> > > +       if (scanned[bus])
> > > +               return;
> > > +
> > > +       scanned[bus] = 1;
> > > +
> > >         /* Poor man's PCI discovery */
> > >         for (slot = 0; slot < 32; slot++)
> > >                 for (func = 0; func < 8; func++) {
> > 
> > Ok, I removed the fix from tip:x86/urgent from the time being - could you
> > guys  please send a full version once a final approach is agreed upon?
> 
> IMHO the above patch to prevent double scanning isn't needed
> and less code is usually better. So my suggestion would be the
> patch as originally sent plus the delta fix I sent yesterday,
> either squashed or applied separately.
> 
> Since Yinghai Lu seems to disagree I guess you as the maintainer
> will have to make a decision. :-)

So I'd lean towards lower complexity, but since this is essentially PCI code I'd 
like to defer to Bjorn on that detail.

Thanks,

	Ingo

Re: [tip:x86/urgent] x86/quirks: Add early quirk to reset Apple AirPort card

[Lukas Wunner]

On Fri, Jun 10, 2016 at 02:59:57PM +0200, Ingo Molnar wrote:
> * Lukas Wunner <lukas@wunner.de> wrote:
> > On Fri, Jun 10, 2016 at 01:58:45PM +0200, Ingo Molnar wrote:
> > > * Yinghai Lu <yinghai@kernel.org> wrote:
> > > > On 6/9/16, Lukas Wunner <lukas@wunner.de> wrote:
> > > > > Well, the PCI core would also scan such a bus twice AFAICS.
> > > > > And the performance penalty of scanning it twice seems negligible.
> > > > > Early quirks can prevent double execution by setting QFLAG_APPLY_ONCE.
> > > > > (Three quirks have set that flag already.)
> > > > >
> > > > > So I think this shouldn't be a concern.
> > > > 
> > > > I don't know. I would like see sth like following, and that is simple
> > > > enough.
> > > > 
> > > > --- linux-2.6.orig/arch/x86/kernel/early-quirks.c
> > > > +++ linux-2.6/arch/x86/kernel/early-quirks.c
> > > > @@ -755,10 +755,16 @@ static int __init check_dev_quirk(int nu
> > > >         return 0;
> > > >  }
> > > > 
> > > > +static unsigned char __initdata scanned[256];
> > > >  static void __init early_pci_scan_bus(int bus)
> > > >  {
> > > >         int slot, func;
> > > > 
> > > > +       if (scanned[bus])
> > > > +               return;
> > > > +
> > > > +       scanned[bus] = 1;
> > > > +
> > > >         /* Poor man's PCI discovery */
> > > >         for (slot = 0; slot < 32; slot++)
> > > >                 for (func = 0; func < 8; func++) {
> > > 
> > > Ok, I removed the fix from tip:x86/urgent from the time being - could you
> > > guys  please send a full version once a final approach is agreed upon?
> > 
> > IMHO the above patch to prevent double scanning isn't needed
> > and less code is usually better. So my suggestion would be the
> > patch as originally sent plus the delta fix I sent yesterday,
> > either squashed or applied separately.
> > 
> > Since Yinghai Lu seems to disagree I guess you as the maintainer
> > will have to make a decision. :-)
> 
> So I'd lean towards lower complexity, but since this is essentially
> PCI code I'd like to defer to Bjorn on that detail.

If I may add some additional information:

drivers/pci/probe.c contains the following comment:

	/*
	 * The bus might already exist for two reasons: Either we are
	 * rescanning the bus or the bus is reachable through more than
	 * one bridge. The second case can happen with the i450NX
	 * chipset.
	 */

So buses reachable through more than one bridge do exist, albeit they're
assumedly rare. The question is, what are the consequences?

(1) A minimal performance hit from scanning the bus multiple times.
(2) Quirks for devices on that bus are executed multiple times.
    As said quirks can set QFLAG_APPLY_ONCE to prevent that. If it turns
    out this patch regresses because quirks are executed multiple times
    but shouldn't, that's a bug in those quirks and they need to be
    amended to set QFLAG_APPLY_ONCE. If we avoid scanning the bus
    multiple times, we'd essentially be papering over those bugs.

Best regards,

Lukas

Re: [tip:x86/urgent] x86/quirks: Add early quirk to reset Apple AirPort card

[Bjorn Helgaas]

[+cc Matthew, Doug]

On Fri, Jun 10, 2016 at 03:59:04PM +0200, Lukas Wunner wrote:
> On Fri, Jun 10, 2016 at 02:59:57PM +0200, Ingo Molnar wrote:
> > * Lukas Wunner <lukas@wunner.de> wrote:
> > > On Fri, Jun 10, 2016 at 01:58:45PM +0200, Ingo Molnar wrote:
> > > > * Yinghai Lu <yinghai@kernel.org> wrote:
> > > > > On 6/9/16, Lukas Wunner <lukas@wunner.de> wrote:
> > > > > > Well, the PCI core would also scan such a bus twice AFAICS.
> > > > > > And the performance penalty of scanning it twice seems negligible.
> > > > > > Early quirks can prevent double execution by setting QFLAG_APPLY_ONCE.
> > > > > > (Three quirks have set that flag already.)
> > > > > >
> > > > > > So I think this shouldn't be a concern.
> > > > > 
> > > > > I don't know. I would like see sth like following, and that is simple
> > > > > enough.
> > > > > 
> > > > > --- linux-2.6.orig/arch/x86/kernel/early-quirks.c
> > > > > +++ linux-2.6/arch/x86/kernel/early-quirks.c
> > > > > @@ -755,10 +755,16 @@ static int __init check_dev_quirk(int nu
> > > > >         return 0;
> > > > >  }
> > > > > 
> > > > > +static unsigned char __initdata scanned[256];
> > > > >  static void __init early_pci_scan_bus(int bus)
> > > > >  {
> > > > >         int slot, func;
> > > > > 
> > > > > +       if (scanned[bus])
> > > > > +               return;
> > > > > +
> > > > > +       scanned[bus] = 1;
> > > > > +
> > > > >         /* Poor man's PCI discovery */
> > > > >         for (slot = 0; slot < 32; slot++)
> > > > >                 for (func = 0; func < 8; func++) {
> > > > 
> > > > Ok, I removed the fix from tip:x86/urgent from the time being - could you
> > > > guys  please send a full version once a final approach is agreed upon?
> > > 
> > > IMHO the above patch to prevent double scanning isn't needed
> > > and less code is usually better. So my suggestion would be the
> > > patch as originally sent plus the delta fix I sent yesterday,
> > > either squashed or applied separately.
> > > 
> > > Since Yinghai Lu seems to disagree I guess you as the maintainer
> > > will have to make a decision. :-)
> > 
> > So I'd lean towards lower complexity, but since this is essentially
> > PCI code I'd like to defer to Bjorn on that detail.

Since you asked my opinion, here it is, but it's probably more than
you wanted :)

This is a serious problem; thank you, Lukas, for chasing this all
down and for the detailed changelog.

- I would split the early_pci_scan_bus() and Nvidia changes to a
  separate patch.  It's a little bit more to track, but will make it
  easier to review, bisect, and revert.

- The delta ("Validate secondary bus number") patch looks good and
  should be folded into the early_pci_scan_bus() patch.

- I know you can't use dev_printk, but you could fake it by hand,
  e.g., pr_err("pci 0000:%02x:%02x.%d: Cannot power up ...\n");

- The "Resetting" message could mention the reason, e.g., "to
  workaround platform firmware issue".

> If I may add some additional information:
> 
> drivers/pci/probe.c contains the following comment:
> 
> 	/*
> 	 * The bus might already exist for two reasons: Either we are
> 	 * rescanning the bus or the bus is reachable through more than
> 	 * one bridge. The second case can happen with the i450NX
> 	 * chipset.

- In your case, I do not think it is necessary to keep track of which
  buses have been scanned.

  The i450NX comment about reaching a bus via multiple bridges was
  added by Matthew Wilcox in
  http://git.kernel.org/cgit/linux/kernel/git/tglx/history.git/commit/?id=82987569b869 

  I also found this enlightening explanation from Doug Ledford at
  https://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.0/2.6.0-mm1/broken-out/i450nx-scanning-fix.patch:

    On Sun, Nov 09, 2003 at 11:51:36AM -0500, Doug Ledford wrote:
    > I can tell you what's going on here.  This is a 450NX based
    > motherboard.  The 450NX chipset from Intel was the first chipset
    > to have peer PCI busses.  For backwards compatibility, some
    > machine makers hacked their PCI BIOS to have a fake bridge
    > device on PCI bus 0 that points to the same bus number as the
    > peer bus.  This way if the OS didn't know about the peer bus
    > registers it would still find the devices by scanning behind the
    > bridge.

  So if we booted one of those i450NX machines today, we would see
  something like this:

    ACPI: PCI Root Bridge [PCI0] (domain 0000 [bus 00-7f])
    pci 0000:00:1c.0: PCI bridge to [bus 80-ff]
    ACPI: PCI Root Bridge [PCI1] (domain 0000 [bus 80-ff])

  x86 asks the PCI core to scan the PCI0 hierarchy starting at bus 00
  (including any subtree on bus 80), then to scan the PCI1 hierarchy
  starting at bus 80, so it has to keep track of what it has seen.

  In your case, you're scanning only the hierarchy starting at bus 00.
  You could see a subtree at bus 80, but since you never initiate a
  scan from bus 80, you should never see duplicates.

Bjorn

Re: [tip:x86/urgent] x86/quirks: Add early quirk to reset Apple AirPort card

[Lukas Wunner]

On Fri, Jun 10, 2016 at 03:08:02PM -0500, Bjorn Helgaas wrote:
> On Fri, Jun 10, 2016 at 03:59:04PM +0200, Lukas Wunner wrote:
> > On Fri, Jun 10, 2016 at 02:59:57PM +0200, Ingo Molnar wrote:
> > > So I'd lean towards lower complexity, but since this is essentially
> > > PCI code I'd like to defer to Bjorn on that detail.
> 
> Since you asked my opinion, here it is, but it's probably more than
> you wanted :)

That is one sublime review. Thanks a lot.

I've just posted v2, taking into account all of Bjorn's suggestions:
https://lkml.org/lkml/2016/6/12/65

@Ingo Molnar: I noticed that you've removed the line breaks in
read/write_pci_config_16(). I've carried that change over to v2
even though it exceeds 80 chars/line and thus doesn't seem to
comply with Documentation/CodingStyle, chapter 2.

Please note that the stable designation in patch [3/3] needs to be
amended with the sha1 of the two preceding patches after they have
been merged.

Thanks again & best regards,

Lukas

Re: [PATCH] x86: Add early quirk to reset Apple AirPort card

[Bjorn Helgaas]

On Sun, May 29, 2016 at 01:35:28AM +0200, Lukas Wunner wrote:
> 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.
> ...

> 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

I think I saw mail about this being applied via the x86 tree.  Let me
know if I need to do anything more here.

[PATCH] x86: Add early quirk to reset Apple AirPort card

[Bjorn Helgaas]

On Sun, May 29, 2016 at 01:35:28AM +0200, Lukas Wunner wrote:
> 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.
> ...

> 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

I think I saw mail about this being applied via the x86 tree.  Let me
know if I need to do anything more here.

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

[Lukas Wunner]

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

[tip:x86/urgent] x86/quirks: Add early quirk to reset Apple AirPort card

[tip-bot for Lukas Wunner]

Commit-ID:  abb2bafd295fe962bbadc329dbfb2146457283ac
Gitweb:     http://git.kernel.org/tip/abb2bafd295fe962bbadc329dbfb2146457283ac
Author:     Lukas Wunner <lukas@wunner.de>
AuthorDate: Sun, 12 Jun 2016 12:31:53 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Sun, 10 Jul 2016 20:13:53 +0200

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: Andy Lutomirski <luto@kernel.org>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Chris Milsted <cmilsted@redhat.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Matthew Garrett <mjg59@srcf.ucam.org>
Cc: Michael Buesch <m@bues.ch>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Yinghai Lu <yinghai@kernel.org>
Cc: b43-dev@lists.infradead.org
Cc: linux-pci@vger.kernel.org
Cc: linux-wireless@vger.kernel.org
Cc: stable@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
Link: http://lkml.kernel.org/r/48d0972ac82a53d460e5fce77a07b2560db95203.1465690253.git.lukas@wunner.de
[ Did minor readability edits. ]
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/early-quirks.c | 64 ++++++++++++++++++++++++++++++++++++++++++
 drivers/bcma/bcma_private.h    |  2 --
 include/linux/bcma/bcma.h      |  1 +
 3 files changed, 65 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c
index ea60c05..57b7137 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,61 @@ 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 +701,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