linux-kernel - [PATCH -v3] PCI, ACPI, hotplug: Fix BUS

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date:	Thu, 25 Apr 2013 18:46:38 -0700
From:	Yinghai Lu <yinghai@...nel.org>
To:	Bjorn Helgaas <bhelgaas@...gle.com>,
	"Rafael J. Wysocki" <rjw@...k.pl>, Jiang Liu <jiang.liu@...wei.com>
Cc:	Gavin Guo <tuffkidtt@...il.com>, linux-pci@...r.kernel.org,
	linux-acpi@...r.kernel.org, linux-kernel@...r.kernel.org,
	Yinghai Lu <yinghai@...nel.org>
Subject: [PATCH -v3] PCI, ACPI, hotplug: Fix BUS_CHECK event handle on root bridge

Gavin found that acpiphp does not handle hotplug anymore even after
now we have acpiphp built-in preparing for v3.10.

Bjorn analyzed bootlog, he found that 
    acpiphp: ACPI Hot Plug PCI Controller Driver version: 0.5
    ACPI: PCI Root Bridge [PCI0] (domain 0000 [bus 00-3e])
    \_SB_.PCI0:_OSC invalid UUID
    acpiphp: Slot [1] registered
    acpiphp: Slot [1-1] registered
   acpi root: \_SB_.PCI0 notify handler is installed
    _handle_hotplug_event_root: Bus check notify on \_SB_.PCI0
    _handle_hotplug_event_root: Bus check notify on \_SB_.PCI0
And that means:
   So we should be using acpiphp, which you do have built in statically,
   and it found a couple slots.  And we did get two bus check notifies on
   \_SB_.PCI0, so we *should* be re-enumerating PCI bus 0000:00.  But it
   looks like we're handling this as a host bridge hotplug event instead
   of a PCI device hotplug.  My guess is that
   handle_root_bridge_insertion() does nothing because the PCI0 ACPI
   device already exists, though I would expect to see the "acpi device
   exists..." in your dmesg log if this were the case.

Also according to Rafael and Bjorn, it is perfect fine that we
should enumerate bus by sending event to root bridge after hotadd
device to slots under that root bridge or children bridges.

It turns out that it is regression caused by
| commit 668192b678201d2fff27c6cc76bb003c1ec4a52a
| Author: Yinghai Lu <yinghai@...nel.org>
| Date:   Mon Jan 21 13:20:48 2013 -0800
|
|    PCI: acpiphp: Move host bridge hotplug to pci_root.c

We should check slots when BUS_CHECK is sent to root bridge acpi handle.

Restore the old behavoir by calling acpi_check_bridge and check_sub_bridge
in acpiphp.

Jiang Liu acctually have simimar patch but it forgets calling
acpi_check_bridge() for system that have slots on root bus directly.
That is still valid, as in QEMU we still have that slots on bus 0 at
least. But my first version patch wrongly check if root bridge exists
before check_sub_bridge for children bridges. 

-v2: Don't check bridge for acpi_walk_namespace with check_sub_bridges.
     also put back bridge reference.
-v3: More changelog and etc.

Reported-by: Gavin Guo <tuffkidtt@...il.com>
Tested-by: Gavin Guo <tuffkidtt@...il.com>
Signed-off-by: Yinghai Lu <yinghai@...nel.org>

---
 drivers/acpi/pci_root.c            |    2 ++
 drivers/pci/hotplug/acpiphp_glue.c |   14 ++++++++++++++
 include/linux/pci-acpi.h           |    2 ++
 3 files changed, 18 insertions(+)

Index: linux-2.6/drivers/acpi/pci_root.c
===================================================================
--- linux-2.6.orig/drivers/acpi/pci_root.c
+++ linux-2.6/drivers/acpi/pci_root.c
@@ -643,6 +643,8 @@ static void _handle_hotplug_event_root(s
 				 (char *)buffer.pointer);
 		if (!root)
 			handle_root_bridge_insertion(handle);
+		else
+			acpiphp_check_host_bridge(handle);
 
 		break;
 
Index: linux-2.6/drivers/pci/hotplug/acpiphp_glue.c
===================================================================
--- linux-2.6.orig/drivers/pci/hotplug/acpiphp_glue.c
+++ linux-2.6/drivers/pci/hotplug/acpiphp_glue.c
@@ -950,6 +950,20 @@ check_sub_bridges(acpi_handle handle, u3
 	return AE_OK ;
 }
 
+void acpiphp_check_host_bridge(acpi_handle handle)
+{
+	struct acpiphp_bridge *bridge;
+
+	bridge = acpiphp_handle_to_bridge(handle);
+	if (bridge) {
+		acpiphp_check_bridge(bridge);
+		put_bridge(bridge);
+	}
+
+	acpi_walk_namespace(ACPI_TYPE_DEVICE, handle,
+		ACPI_UINT32_MAX, check_sub_bridges, NULL, NULL, NULL);
+}
+
 static void _handle_hotplug_event_bridge(struct work_struct *work)
 {
 	struct acpiphp_bridge *bridge;
Index: linux-2.6/include/linux/pci-acpi.h
===================================================================
--- linux-2.6.orig/include/linux/pci-acpi.h
+++ linux-2.6/include/linux/pci-acpi.h
@@ -60,11 +60,13 @@ static inline void acpi_pci_slot_remove(
 void acpiphp_init(void);
 void acpiphp_enumerate_slots(struct pci_bus *bus, acpi_handle handle);
 void acpiphp_remove_slots(struct pci_bus *bus);
+void acpiphp_check_host_bridge(acpi_handle handle);
 #else
 static inline void acpiphp_init(void) { }
 static inline void acpiphp_enumerate_slots(struct pci_bus *bus,
 					   acpi_handle handle) { }
 static inline void acpiphp_remove_slots(struct pci_bus *bus) { }
+static inline void acpiphp_check_host_bridge(acpi_handle handle) { }
 #endif
 
 #else	/* CONFIG_ACPI */
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/