From: Igor Mammedov <imammedo@redhat.com>
To: linux-kernel@vger.kernel.org
Cc: terraluna977@gmail.com, bhelgaas@google.com,
linux-pci@vger.kernel.org, imammedo@redhat.com, mst@redhat.com,
rafael@kernel.org, linux-acpi@vger.kernel.org
Subject: [PATCH 1/1] PCI: acpiphp:: use pci_assign_unassigned_bridge_resources() only if bus->self not NULL
Date: Wed, 26 Jul 2023 14:35:18 +0200 [thread overview]
Message-ID: <20230726123518.2361181-2-imammedo@redhat.com> (raw)
In-Reply-To: <20230726123518.2361181-1-imammedo@redhat.com>
Commit [1] switched acpiphp hotplug to use
pci_assign_unassigned_bridge_resources()
which depends on bridge being available, however in some cases
when acpiphp is in use, enable_slot() can get a slot without
bridge associated.
1. legitimate case of hotplug on root bus
(likely not exiting on real hw, but widely used in virt world)
2. broken firmware, that sends 'Bus check' events to non
existing root ports (Dell Inspiron 7352/0W6WV0), which somehow
endup at acpiphp:enable_slot(..., bridge = 0) and with bus
without bridge assigned to it.
Issue is easy to reproduce with QEMU's 'pc' machine provides
PCI hotplug on hostbridge slots. to reproduce boot kernel at
commit [1] in VM started with followin CLI and hotplug a device:
once guest OS is fully booted at qemu prompt:
(qemu) device_add e1000
it will cause NULL pointer dereference at
void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge)
{
struct pci_bus *parent = bridge->subordinate;
[ 612.277651] BUG: kernel NULL pointer dereference, address: 0000000000000018
[...]
[ 612.277798] ? pci_assign_unassigned_bridge_resources+0x1f/0x260
[ 612.277804] ? pcibios_allocate_dev_resources+0x3c/0x2a0
[ 612.277809] enable_slot+0x21f/0x3e0
[ 612.277816] acpiphp_hotplug_notify+0x13d/0x260
[ 612.277822] ? __pfx_acpiphp_hotplug_notify+0x10/0x10
[ 612.277827] acpi_device_hotplug+0xbc/0x540
[ 612.277834] acpi_hotplug_work_fn+0x15/0x20
[ 612.277839] process_one_work+0x1f7/0x370
[ 612.277845] worker_thread+0x45/0x3b0
[ 612.277850] ? __pfx_worker_thread+0x10/0x10
[ 612.277854] kthread+0xdc/0x110
[ 612.277860] ? __pfx_kthread+0x10/0x10
[ 612.277866] ret_from_fork+0x28/0x40
[ 612.277871] ? __pfx_kthread+0x10/0x10
[ 612.277876] ret_from_fork_asm+0x1b/0x30
The issue was discovered on Dell Inspiron 7352/0W6WV0 laptop with
following sequence:
1. suspend to RAM
2. wake up with the same backtrace being observed:
3. 2nd suspend to RAM attempt makes laptop freeze
Fix it by using __pci_bus_assign_resources() instead of
pci_assign_unassigned_bridge_resources()as we used to do
but only in case when bus doesn't have a bridge associated
with it.
That let us keep hotplug on root bus working like it used to be
but at the same time keeps resource reassignment usable on
root ports (and other 1st level bridges) that was fixed by [1].
1)
Fixes: 40613da52b13 ("PCI: acpiphp: Reassign resources on bridge if necessary")
Link: https://lore.kernel.org/r/11fc981c-af49-ce64-6b43-3e282728bd1a@gmail.com
Reported-by: Woody Suwalski <terraluna977@gmail.com>
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
drivers/pci/hotplug/acpiphp_glue.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
index 328d1e416014..3bc4e1f3efee 100644
--- a/drivers/pci/hotplug/acpiphp_glue.c
+++ b/drivers/pci/hotplug/acpiphp_glue.c
@@ -498,6 +498,7 @@ static void enable_slot(struct acpiphp_slot *slot, bool bridge)
acpiphp_native_scan_bridge(dev);
}
} else {
+ LIST_HEAD(add_list);
int max, pass;
acpiphp_rescan_slot(slot);
@@ -511,10 +512,15 @@ static void enable_slot(struct acpiphp_slot *slot, bool bridge)
if (pass && dev->subordinate) {
check_hotplug_bridge(slot, dev);
pcibios_resource_survey_bus(dev->subordinate);
+ if (!bus->self)
+ __pci_bus_size_bridges(dev->subordinate, &add_list);
}
}
}
- pci_assign_unassigned_bridge_resources(bus->self);
+ if (bus->self)
+ pci_assign_unassigned_bridge_resources(bus->self);
+ else
+ __pci_bus_assign_resources(bus, &add_list, NULL);
}
acpiphp_sanitize_bus(bus);
--
2.39.3
next prev parent reply other threads:[~2023-07-26 12:36 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-07-26 12:35 [PATCH 0/1] PCI: acpiphp: fix regression introduced by 'Reassign resources on bridge if necessary' Igor Mammedov
2023-07-26 12:35 ` Igor Mammedov [this message]
2023-07-26 14:24 ` [PATCH 1/1] PCI: acpiphp:: use pci_assign_unassigned_bridge_resources() only if bus->self not NULL Rafael J. Wysocki
2023-07-27 6:04 ` Michael S. Tsirkin
2023-07-27 17:41 ` Bjorn Helgaas
2023-07-28 9:32 ` Igor Mammedov
2023-07-29 21:50 ` Bjorn Helgaas
2023-07-31 12:44 ` Igor Mammedov
2023-07-31 21:42 ` Bjorn Helgaas
2023-07-31 21:54 ` Michael S. Tsirkin
2023-08-01 9:57 ` Igor Mammedov
2023-08-01 12:57 ` Michael S. Tsirkin
2023-08-04 14:11 ` Igor Mammedov
2023-07-31 12:46 ` [PATCH QEMU] acpiphp: hack to send BusCheck to missing device on root bus Igor Mammedov
2023-08-04 23:27 ` [PATCH 1/1] PCI: acpiphp:: use pci_assign_unassigned_bridge_resources() only if bus->self not NULL Bjorn Helgaas
2023-08-07 13:07 ` Igor Mammedov
2023-08-07 17:28 ` Bjorn Helgaas
2023-08-08 11:47 ` Igor Mammedov
2023-08-08 9:25 ` Michal Koutný
2023-08-08 19:35 ` Bjorn Helgaas
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230726123518.2361181-2-imammedo@redhat.com \
--to=imammedo@redhat.com \
--cc=bhelgaas@google.com \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=mst@redhat.com \
--cc=rafael@kernel.org \
--cc=terraluna977@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).