On Thu, Dec 14, 2023 at 10:51:02AM -0600, Bjorn Helgaas wrote:
> From: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
>
> This reverts commit 40613da52b13fb21c5566f10b287e0ca8c12c4e9 and the
> subsequent fix to it:
>
> cc22522fd55e ("PCI: acpiphp: Use pci_assign_unassigned_bridge_resources() only for non-root bus")
>
> 40613da52b13 fixed a problem where hot-adding a device with large BARs
> failed if the bridge windows programmed by firmware were not large enough.
>
> cc22522fd55e ("PCI: acpiphp: Use pci_assign_unassigned_bridge_resources()
> only for non-root bus") fixed a problem with 40613da52b13: an ACPI hot-add
> of a device on a PCI root bus (common in the virt world) or firmware
> sending ACPI Bus Check to non-existent Root Ports (e.g., on Dell Inspiron
> 7352/0W6WV0) caused a NULL pointer dereference and suspend/resume hangs.
>
> Unfortunately the combination of 40613da52b13 and cc22522fd55e caused other
> problems:
>
> - Fiona reported that hot-add of SCSI disks in QEMU virtual machine fails
> sometimes.
>
> - Dongli reported a similar problem with hot-add of SCSI disks.
>
> - Jonathan reported a console freeze during boot on bare metal due to an
> error in radeon GPU initialization.
>
> Revert both patches to avoid adding these problems. This means we will
> again see the problems with hot-adding devices with large BARs and the NULL
> pointer dereferences and suspend/resume issues that 40613da52b13 and
> cc22522fd55e were intended to fix.
>
> Fixes: 40613da52b13 ("PCI: acpiphp: Reassign resources on bridge if necessary")
> Fixes: cc22522fd55e ("PCI: acpiphp: Use pci_assign_unassigned_bridge_resources() only for non-root bus")
> Reported-by: Fiona Ebner <f.ebner@xxxxxxxxxxx>
> Closes: https://lore.kernel.org/r/9eb669c0-d8f2-431d-a700-6da13053ae54@xxxxxxxxxxx
> Reported-by: Dongli Zhang <dongli.zhang@xxxxxxxxxx>
> Closes: https://lore.kernel.org/r/3c4a446a-b167-11b8-f36f-d3c1b49b42e9@xxxxxxxxxx
> Reported-by: Jonathan Woithe <jwoithe@xxxxxxxxxx>
> Closes: https://lore.kernel.org/r/ZXpaNCLiDM+Kv38H@xxxxxxxxxxxxxxxxxxx
> Signed-off-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
> Cc: <stable@xxxxxxxxxxxxxxx>
> Cc: Igor Mammedov <imammedo@xxxxxxxxxx>
It's up to you whether to apply the revert - hopefully a fix can be
developed soon. The revert itself looks like it's done correctly so from
that POV:
Acked-by: Michael S. Tsirkin <mst@xxxxxxxxxx>
> ---
> drivers/pci/hotplug/acpiphp_glue.c | 9 +++------
> 1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
> index 601129772b2d..5b1f271c6034 100644
> --- a/drivers/pci/hotplug/acpiphp_glue.c
> +++ b/drivers/pci/hotplug/acpiphp_glue.c
> @@ -512,15 +512,12 @@ 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 (pci_is_root_bus(bus))
> - __pci_bus_size_bridges(dev->subordinate, &add_list);
> + __pci_bus_size_bridges(dev->subordinate,
> + &add_list);
> }
> }
> }
> - if (pci_is_root_bus(bus))
> - __pci_bus_assign_resources(bus, &add_list, NULL);
> - else
> - pci_assign_unassigned_bridge_resources(bus->self);
> + __pci_bus_assign_resources(bus, &add_list, NULL);
> }
>
> acpiphp_sanitize_bus(bus);
> --
> 2.34.1
