| Message ID | 20230418085030.2154918-1-imammedo@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | pci: acpiphp: try to reassign resources on bridge if necessary | expand |
On Tue, Apr 18, 2023 at 10:50 AM Igor Mammedov <imammedo@redhat.com> wrote: > > When using ACPI PCI hotplug, hotplugging a device with > large BARs may fail if bridge windows programmed by > firmware are not large enough. > > Reproducer: > $ qemu-kvm -monitor stdio -M q35 -m 4G \ > -global ICH9-LPC.acpi-pci-hotplug-with-bridge-support=on \ > -device id=rp1,pcie-root-port,bus=pcie.0,chassis=4 \ > disk_image > > wait till linux guest boots, then hotplug device > (qemu) device_add qxl,bus=rp1 > > hotplug on guest side fails with: > pci 0000:01:00.0: [1b36:0100] type 00 class 0x038000 > pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x03ffffff] > pci 0000:01:00.0: reg 0x14: [mem 0x00000000-0x03ffffff] > pci 0000:01:00.0: reg 0x18: [mem 0x00000000-0x00001fff] > pci 0000:01:00.0: reg 0x1c: [io 0x0000-0x001f] > pci 0000:01:00.0: BAR 0: no space for [mem size 0x04000000] > pci 0000:01:00.0: BAR 0: failed to assign [mem size 0x04000000] > pci 0000:01:00.0: BAR 1: no space for [mem size 0x04000000] > pci 0000:01:00.0: BAR 1: failed to assign [mem size 0x04000000] > pci 0000:01:00.0: BAR 2: assigned [mem 0xfe800000-0xfe801fff] > pci 0000:01:00.0: BAR 3: assigned [io 0x1000-0x101f] > qxl 0000:01:00.0: enabling device (0000 -> 0003) > Unable to create vram_mapping > qxl: probe of 0000:01:00.0 failed with error -12 > > However when using native PCIe hotplug > '-global ICH9-LPC.acpi-pci-hotplug-with-bridge-support=off' > it works fine, since kernel attempts to reassign unused resources. > Use the same machinery as native PCIe hotplug to (re)assign resources. > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> or please let me know if you want me to pick this up. > --- > tested in QEMU with Q35 machine on PCIE root port and also > with nested conventional bridge attached to root port. > --- > drivers/pci/hotplug/acpiphp_glue.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c > index 5b1f271c6034..9aebde28a92f 100644 > --- a/drivers/pci/hotplug/acpiphp_glue.c > +++ b/drivers/pci/hotplug/acpiphp_glue.c > @@ -517,7 +517,7 @@ static void enable_slot(struct acpiphp_slot *slot, bool bridge) > } > } > } > - __pci_bus_assign_resources(bus, &add_list, NULL); > + pci_assign_unassigned_bridge_resources(bus->self); > } > > acpiphp_sanitize_bus(bus); > -- > 2.39.1 >
On Tue, 18 Apr 2023 14:55:29 +0200 "Rafael J. Wysocki" <rafael@kernel.org> wrote: > On Tue, Apr 18, 2023 at 10:50 AM Igor Mammedov <imammedo@redhat.com> wrote: > > > > When using ACPI PCI hotplug, hotplugging a device with > > large BARs may fail if bridge windows programmed by > > firmware are not large enough. > > > > Reproducer: > > $ qemu-kvm -monitor stdio -M q35 -m 4G \ > > -global ICH9-LPC.acpi-pci-hotplug-with-bridge-support=on \ > > -device id=rp1,pcie-root-port,bus=pcie.0,chassis=4 \ > > disk_image > > > > wait till linux guest boots, then hotplug device > > (qemu) device_add qxl,bus=rp1 > > > > hotplug on guest side fails with: > > pci 0000:01:00.0: [1b36:0100] type 00 class 0x038000 > > pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x03ffffff] > > pci 0000:01:00.0: reg 0x14: [mem 0x00000000-0x03ffffff] > > pci 0000:01:00.0: reg 0x18: [mem 0x00000000-0x00001fff] > > pci 0000:01:00.0: reg 0x1c: [io 0x0000-0x001f] > > pci 0000:01:00.0: BAR 0: no space for [mem size 0x04000000] > > pci 0000:01:00.0: BAR 0: failed to assign [mem size 0x04000000] > > pci 0000:01:00.0: BAR 1: no space for [mem size 0x04000000] > > pci 0000:01:00.0: BAR 1: failed to assign [mem size 0x04000000] > > pci 0000:01:00.0: BAR 2: assigned [mem 0xfe800000-0xfe801fff] > > pci 0000:01:00.0: BAR 3: assigned [io 0x1000-0x101f] > > qxl 0000:01:00.0: enabling device (0000 -> 0003) > > Unable to create vram_mapping > > qxl: probe of 0000:01:00.0 failed with error -12 > > > > However when using native PCIe hotplug > > '-global ICH9-LPC.acpi-pci-hotplug-with-bridge-support=off' > > it works fine, since kernel attempts to reassign unused resources. > > Use the same machinery as native PCIe hotplug to (re)assign resources. > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > or please let me know if you want me to pick this up. It would be nice if you could pick it up. Thanks! > > > --- > > tested in QEMU with Q35 machine on PCIE root port and also > > with nested conventional bridge attached to root port. > > --- > > drivers/pci/hotplug/acpiphp_glue.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c > > index 5b1f271c6034..9aebde28a92f 100644 > > --- a/drivers/pci/hotplug/acpiphp_glue.c > > +++ b/drivers/pci/hotplug/acpiphp_glue.c > > @@ -517,7 +517,7 @@ static void enable_slot(struct acpiphp_slot *slot, bool bridge) > > } > > } > > } > > - __pci_bus_assign_resources(bus, &add_list, NULL); > > + pci_assign_unassigned_bridge_resources(bus->self); > > } > > > > acpiphp_sanitize_bus(bus); > > -- > > 2.39.1 > > >
On Tue, Apr 18, 2023 at 4:17 PM Igor Mammedov <imammedo@redhat.com> wrote: > > On Tue, 18 Apr 2023 14:55:29 +0200 > "Rafael J. Wysocki" <rafael@kernel.org> wrote: > > > On Tue, Apr 18, 2023 at 10:50 AM Igor Mammedov <imammedo@redhat.com> wrote: > > > > > > When using ACPI PCI hotplug, hotplugging a device with > > > large BARs may fail if bridge windows programmed by > > > firmware are not large enough. > > > > > > Reproducer: > > > $ qemu-kvm -monitor stdio -M q35 -m 4G \ > > > -global ICH9-LPC.acpi-pci-hotplug-with-bridge-support=on \ > > > -device id=rp1,pcie-root-port,bus=pcie.0,chassis=4 \ > > > disk_image > > > > > > wait till linux guest boots, then hotplug device > > > (qemu) device_add qxl,bus=rp1 > > > > > > hotplug on guest side fails with: > > > pci 0000:01:00.0: [1b36:0100] type 00 class 0x038000 > > > pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x03ffffff] > > > pci 0000:01:00.0: reg 0x14: [mem 0x00000000-0x03ffffff] > > > pci 0000:01:00.0: reg 0x18: [mem 0x00000000-0x00001fff] > > > pci 0000:01:00.0: reg 0x1c: [io 0x0000-0x001f] > > > pci 0000:01:00.0: BAR 0: no space for [mem size 0x04000000] > > > pci 0000:01:00.0: BAR 0: failed to assign [mem size 0x04000000] > > > pci 0000:01:00.0: BAR 1: no space for [mem size 0x04000000] > > > pci 0000:01:00.0: BAR 1: failed to assign [mem size 0x04000000] > > > pci 0000:01:00.0: BAR 2: assigned [mem 0xfe800000-0xfe801fff] > > > pci 0000:01:00.0: BAR 3: assigned [io 0x1000-0x101f] > > > qxl 0000:01:00.0: enabling device (0000 -> 0003) > > > Unable to create vram_mapping > > > qxl: probe of 0000:01:00.0 failed with error -12 > > > > > > However when using native PCIe hotplug > > > '-global ICH9-LPC.acpi-pci-hotplug-with-bridge-support=off' > > > it works fine, since kernel attempts to reassign unused resources. > > > Use the same machinery as native PCIe hotplug to (re)assign resources. > > > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > > > Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > or please let me know if you want me to pick this up. > > It would be nice if you could pick it up. OK, I'll do that unless Bjorn tells me that he prefers to take it via the PCI tree. Thanks! > > > > > --- > > > tested in QEMU with Q35 machine on PCIE root port and also > > > with nested conventional bridge attached to root port. > > > --- > > > drivers/pci/hotplug/acpiphp_glue.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c > > > index 5b1f271c6034..9aebde28a92f 100644 > > > --- a/drivers/pci/hotplug/acpiphp_glue.c > > > +++ b/drivers/pci/hotplug/acpiphp_glue.c > > > @@ -517,7 +517,7 @@ static void enable_slot(struct acpiphp_slot *slot, bool bridge) > > > } > > > } > > > } > > > - __pci_bus_assign_resources(bus, &add_list, NULL); > > > + pci_assign_unassigned_bridge_resources(bus->self); > > > } > > > > > > acpiphp_sanitize_bus(bus); > > > -- > > > 2.39.1 > > > > > >
On Tue, 18 Apr 2023 11:31:14 -0500 Bjorn Helgaas <helgaas@kernel.org> wrote: > [+cc Mika, who made previous changes in this area] > > On Tue, Apr 18, 2023 at 05:38:15PM +0200, Rafael J. Wysocki wrote: > > On Tue, Apr 18, 2023 at 4:17 PM Igor Mammedov <imammedo@redhat.com> wrote: > > > On Tue, 18 Apr 2023 14:55:29 +0200 > > > "Rafael J. Wysocki" <rafael@kernel.org> wrote: > > > > On Tue, Apr 18, 2023 at 10:50 AM Igor Mammedov <imammedo@redhat.com> wrote: > > > > > > > > > > When using ACPI PCI hotplug, hotplugging a device with > > > > > large BARs may fail if bridge windows programmed by > > > > > firmware are not large enough. > > > > > > > > > > Reproducer: > > > > > $ qemu-kvm -monitor stdio -M q35 -m 4G \ > > > > > -global ICH9-LPC.acpi-pci-hotplug-with-bridge-support=on \ > > > > > -device id=rp1,pcie-root-port,bus=pcie.0,chassis=4 \ > > > > > disk_image > > > > > > > > > > wait till linux guest boots, then hotplug device > > > > > (qemu) device_add qxl,bus=rp1 > > > > > > > > > > hotplug on guest side fails with: > > > > > pci 0000:01:00.0: [1b36:0100] type 00 class 0x038000 > > > > > pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x03ffffff] > > > > > pci 0000:01:00.0: reg 0x14: [mem 0x00000000-0x03ffffff] > > > > > pci 0000:01:00.0: reg 0x18: [mem 0x00000000-0x00001fff] > > > > > pci 0000:01:00.0: reg 0x1c: [io 0x0000-0x001f] > > > > > pci 0000:01:00.0: BAR 0: no space for [mem size 0x04000000] > > > > > pci 0000:01:00.0: BAR 0: failed to assign [mem size 0x04000000] > > > > > pci 0000:01:00.0: BAR 1: no space for [mem size 0x04000000] > > > > > pci 0000:01:00.0: BAR 1: failed to assign [mem size 0x04000000] > > > > > pci 0000:01:00.0: BAR 2: assigned [mem 0xfe800000-0xfe801fff] > > > > > pci 0000:01:00.0: BAR 3: assigned [io 0x1000-0x101f] > > > > > qxl 0000:01:00.0: enabling device (0000 -> 0003) > > > > > Unable to create vram_mapping > > > > > qxl: probe of 0000:01:00.0 failed with error -12 > > > > > > > > > > However when using native PCIe hotplug > > > > > '-global ICH9-LPC.acpi-pci-hotplug-with-bridge-support=off' > > > > > it works fine, since kernel attempts to reassign unused resources. > > > > > Use the same machinery as native PCIe hotplug to (re)assign resources. > > Thanks for the nice reproducer and logs! > > > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > > > > > > > Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > > > > > or please let me know if you want me to pick this up. > > > > > > It would be nice if you could pick it up. > > > > OK, I'll do that unless Bjorn tells me that he prefers to take it via > > the PCI tree. > > It's OK with me if you pick this up, but please update the subject to > use the style of previous commits, e.g., > > PCI: acpiphp: Reassign resources on bridge if necessary > > Previous changes involving pci_assign_unassigned_bridge_resources() in > enable_slot() (these are from Mika, so I cc'd him in case he wants to > comment): > > 84c8b58ed3ad ("ACPI / hotplug / PCI: Don't scan bridges managed by native hotplug") > 77adf9355304 ("ACPI / hotplug / PCI: Allocate resources directly under the non-hotplug bridge") > > > > > > --- > > > > > tested in QEMU with Q35 machine on PCIE root port and also > > > > > with nested conventional bridge attached to root port. > > > > > --- > > > > > drivers/pci/hotplug/acpiphp_glue.c | 2 +- > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c > > > > > index 5b1f271c6034..9aebde28a92f 100644 > > > > > --- a/drivers/pci/hotplug/acpiphp_glue.c > > > > > +++ b/drivers/pci/hotplug/acpiphp_glue.c > > > > > @@ -517,7 +517,7 @@ static void enable_slot(struct acpiphp_slot *slot, bool bridge) > > Previous context: > > __pci_bus_size_bridges(dev->subordinate, > &add_list); > > > > > > } > > > > > } > > > > > } > > > > > - __pci_bus_assign_resources(bus, &add_list, NULL); > > > > > + pci_assign_unassigned_bridge_resources(bus->self); > > "add_list" is now used only for __pci_bus_size_bridges(), which > *looks* unnecessary unless there's some obscure side-effect of that > path when that parameter is non-NULL. > > If "add_list" is unnecessary, you would probably use > pci_bus_size_bridges() above instead of __pci_bus_size_bridges(). pci_assign_unassigned_bridge_resources() calls __pci_bus_size_bridges() so original one is not needed anymore, in addition it might leak entries added to add_list. I'll remove __pci_bus_size_bridges() and respin patch (incl. updated subject) > After this patch, we have: > > if (bridge && bus->self && hotplug_is_native(bus->self)) { > for_each_pci_bridge(dev, bus) > acpiphp_native_scan_bridge(dev); > } else { > ... > pci_assign_unassigned_bridge_resources(bus->self); > } > > We do not do pci_assign_unassigned_bridge_resources() in the "then" > part of the "if". Per the comment, that case may be used for adding > Thunderbolt controllers. Is there a reason we do not want > pci_assign_unassigned_bridge_resources() in that path, > or should it be > in both cases? acpiphp_native_scan_bridge() looks very similar to 'else' branch modulo skip native hp bridge condition. Otherwise both branches look similar, but I don't have means to test that usecase, so I'd avoid touching something nobody complains about. > > > > > > } > > > > > > > > > > acpiphp_sanitize_bus(bus); >
On Tue, 18 Apr 2023 07:08:09 -0400 "Michael S. Tsirkin" <mst@redhat.com> wrote: > On Tue, Apr 18, 2023 at 10:50:30AM +0200, Igor Mammedov wrote: > > When using ACPI PCI hotplug, hotplugging a device with > > large BARs may fail if bridge windows programmed by > > firmware are not large enough. > > > > Reproducer: > > $ qemu-kvm -monitor stdio -M q35 -m 4G \ > > -global ICH9-LPC.acpi-pci-hotplug-with-bridge-support=on \ > > -device id=rp1,pcie-root-port,bus=pcie.0,chassis=4 \ > > disk_image > > > > wait till linux guest boots, then hotplug device > > (qemu) device_add qxl,bus=rp1 > > > > hotplug on guest side fails with: > > pci 0000:01:00.0: [1b36:0100] type 00 class 0x038000 > > pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x03ffffff] > > pci 0000:01:00.0: reg 0x14: [mem 0x00000000-0x03ffffff] > > pci 0000:01:00.0: reg 0x18: [mem 0x00000000-0x00001fff] > > pci 0000:01:00.0: reg 0x1c: [io 0x0000-0x001f] > > pci 0000:01:00.0: BAR 0: no space for [mem size 0x04000000] > > pci 0000:01:00.0: BAR 0: failed to assign [mem size 0x04000000] > > pci 0000:01:00.0: BAR 1: no space for [mem size 0x04000000] > > pci 0000:01:00.0: BAR 1: failed to assign [mem size 0x04000000] > > pci 0000:01:00.0: BAR 2: assigned [mem 0xfe800000-0xfe801fff] > > pci 0000:01:00.0: BAR 3: assigned [io 0x1000-0x101f] > > qxl 0000:01:00.0: enabling device (0000 -> 0003) > > Unable to create vram_mapping > > qxl: probe of 0000:01:00.0 failed with error -12 > > > > However when using native PCIe hotplug > > '-global ICH9-LPC.acpi-pci-hotplug-with-bridge-support=off' > > it works fine, since kernel attempts to reassign unused resources. > > Use the same machinery as native PCIe hotplug to (re)assign resources. > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > Acked-by: Michael S. Tsirkin <mst@redhat.com> > > And I think: > > Fixes: d66ecb7220a7 ("PCI / ACPI: Use boot-time resource allocation rules during hotplug") Probably not, this commit basically added pcibios_resource_survey_bus() and nothing else important. Looking through history it was always broken this way. > > > --- > > tested in QEMU with Q35 machine on PCIE root port and also > > with nested conventional bridge attached to root port. > > --- > > drivers/pci/hotplug/acpiphp_glue.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c > > index 5b1f271c6034..9aebde28a92f 100644 > > --- a/drivers/pci/hotplug/acpiphp_glue.c > > +++ b/drivers/pci/hotplug/acpiphp_glue.c > > @@ -517,7 +517,7 @@ static void enable_slot(struct acpiphp_slot *slot, bool bridge) > > } > > } > > } > > - __pci_bus_assign_resources(bus, &add_list, NULL); > > + pci_assign_unassigned_bridge_resources(bus->self); > > } > > > > acpiphp_sanitize_bus(bus); > > -- > > 2.39.1 >
diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c index 5b1f271c6034..9aebde28a92f 100644 --- a/drivers/pci/hotplug/acpiphp_glue.c +++ b/drivers/pci/hotplug/acpiphp_glue.c @@ -517,7 +517,7 @@ static void enable_slot(struct acpiphp_slot *slot, bool bridge) } } } - __pci_bus_assign_resources(bus, &add_list, NULL); + pci_assign_unassigned_bridge_resources(bus->self); } acpiphp_sanitize_bus(bus);
When using ACPI PCI hotplug, hotplugging a device with large BARs may fail if bridge windows programmed by firmware are not large enough. Reproducer: $ qemu-kvm -monitor stdio -M q35 -m 4G \ -global ICH9-LPC.acpi-pci-hotplug-with-bridge-support=on \ -device id=rp1,pcie-root-port,bus=pcie.0,chassis=4 \ disk_image wait till linux guest boots, then hotplug device (qemu) device_add qxl,bus=rp1 hotplug on guest side fails with: pci 0000:01:00.0: [1b36:0100] type 00 class 0x038000 pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x03ffffff] pci 0000:01:00.0: reg 0x14: [mem 0x00000000-0x03ffffff] pci 0000:01:00.0: reg 0x18: [mem 0x00000000-0x00001fff] pci 0000:01:00.0: reg 0x1c: [io 0x0000-0x001f] pci 0000:01:00.0: BAR 0: no space for [mem size 0x04000000] pci 0000:01:00.0: BAR 0: failed to assign [mem size 0x04000000] pci 0000:01:00.0: BAR 1: no space for [mem size 0x04000000] pci 0000:01:00.0: BAR 1: failed to assign [mem size 0x04000000] pci 0000:01:00.0: BAR 2: assigned [mem 0xfe800000-0xfe801fff] pci 0000:01:00.0: BAR 3: assigned [io 0x1000-0x101f] qxl 0000:01:00.0: enabling device (0000 -> 0003) Unable to create vram_mapping qxl: probe of 0000:01:00.0 failed with error -12 However when using native PCIe hotplug '-global ICH9-LPC.acpi-pci-hotplug-with-bridge-support=off' it works fine, since kernel attempts to reassign unused resources. Use the same machinery as native PCIe hotplug to (re)assign resources. Signed-off-by: Igor Mammedov <imammedo@redhat.com> --- tested in QEMU with Q35 machine on PCIE root port and also with nested conventional bridge attached to root port. --- drivers/pci/hotplug/acpiphp_glue.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)