* Re: [PATCH v2] PCI: acpiphp: Reassign resources on bridge if necessary
2023-04-24 19:15 [PATCH v2] PCI: acpiphp: Reassign resources on bridge if necessary Igor Mammedov
@ 2023-04-24 20:47 ` Michael S. Tsirkin
2023-05-09 12:44 ` Igor Mammedov
` (3 subsequent siblings)
4 siblings, 0 replies; 15+ messages in thread
From: Michael S. Tsirkin @ 2023-04-24 20:47 UTC (permalink / raw)
To: Igor Mammedov
Cc: linux-kernel, rafael, lenb, bhelgaas, linux-acpi, linux-pci,
helgaas, mika.westerberg
On Mon, Apr 24, 2023 at 09:15:57PM +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>
> ---
> tested in QEMU with Q35 machine on PCIE root port and also
> with nested conventional bridge attached to root port.
>
> v2:
> * fixup subject to match expected style
> * drop no longer needed __pci_bus_size_bridges() to avoid
> memory leak (Bjorn Helgaas <helgaas@kernel.org>)
> ---
> drivers/pci/hotplug/acpiphp_glue.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
> index 5b1f271c6034..328d1e416014 100644
> --- a/drivers/pci/hotplug/acpiphp_glue.c
> +++ b/drivers/pci/hotplug/acpiphp_glue.c
> @@ -498,7 +498,6 @@ 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);
> @@ -512,12 +511,10 @@ 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);
> - __pci_bus_size_bridges(dev->subordinate,
> - &add_list);
> }
> }
> }
> - __pci_bus_assign_resources(bus, &add_list, NULL);
> + pci_assign_unassigned_bridge_resources(bus->self);
> }
>
> acpiphp_sanitize_bus(bus);
> --
> 2.39.1
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v2] PCI: acpiphp: Reassign resources on bridge if necessary
2023-04-24 19:15 [PATCH v2] PCI: acpiphp: Reassign resources on bridge if necessary Igor Mammedov
2023-04-24 20:47 ` Michael S. Tsirkin
@ 2023-05-09 12:44 ` Igor Mammedov
2023-05-22 11:06 ` Igor Mammedov
` (2 subsequent siblings)
4 siblings, 0 replies; 15+ messages in thread
From: Igor Mammedov @ 2023-05-09 12:44 UTC (permalink / raw)
To: helgaas
Cc: linux-kernel, mst, rafael, lenb, bhelgaas, linux-acpi, linux-pci,
mika.westerberg
On Mon, 24 Apr 2023 21:15:57 +0200
Igor Mammedov <imammedo@redhat.com> wrote:
Bjorn,
I've fixed up issues you've mentioned on v1 review,
is it good enough to take it in now?
> 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.
>
> v2:
> * fixup subject to match expected style
> * drop no longer needed __pci_bus_size_bridges() to avoid
> memory leak (Bjorn Helgaas <helgaas@kernel.org>)
> ---
> drivers/pci/hotplug/acpiphp_glue.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
> index 5b1f271c6034..328d1e416014 100644
> --- a/drivers/pci/hotplug/acpiphp_glue.c
> +++ b/drivers/pci/hotplug/acpiphp_glue.c
> @@ -498,7 +498,6 @@ 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);
> @@ -512,12 +511,10 @@ 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);
> - __pci_bus_size_bridges(dev->subordinate,
> - &add_list);
> }
> }
> }
> - __pci_bus_assign_resources(bus, &add_list, NULL);
> + pci_assign_unassigned_bridge_resources(bus->self);
> }
>
> acpiphp_sanitize_bus(bus);
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v2] PCI: acpiphp: Reassign resources on bridge if necessary
2023-04-24 19:15 [PATCH v2] PCI: acpiphp: Reassign resources on bridge if necessary Igor Mammedov
2023-04-24 20:47 ` Michael S. Tsirkin
2023-05-09 12:44 ` Igor Mammedov
@ 2023-05-22 11:06 ` Igor Mammedov
2023-05-22 11:52 ` Rafael J. Wysocki
2023-05-22 21:51 ` Bjorn Helgaas
2023-05-30 17:12 ` Bjorn Helgaas
4 siblings, 1 reply; 15+ messages in thread
From: Igor Mammedov @ 2023-05-22 11:06 UTC (permalink / raw)
To: linux-kernel, rafael
Cc: mst, lenb, bhelgaas, linux-acpi, linux-pci, helgaas, mika.westerberg
On Mon, 24 Apr 2023 21:15:57 +0200
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.
Rafael,
Since there was no more comments for a while,
can you merge this patch through your tree?
> 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.
>
> v2:
> * fixup subject to match expected style
> * drop no longer needed __pci_bus_size_bridges() to avoid
> memory leak (Bjorn Helgaas <helgaas@kernel.org>)
> ---
> drivers/pci/hotplug/acpiphp_glue.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
> index 5b1f271c6034..328d1e416014 100644
> --- a/drivers/pci/hotplug/acpiphp_glue.c
> +++ b/drivers/pci/hotplug/acpiphp_glue.c
> @@ -498,7 +498,6 @@ 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);
> @@ -512,12 +511,10 @@ 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);
> - __pci_bus_size_bridges(dev->subordinate,
> - &add_list);
> }
> }
> }
> - __pci_bus_assign_resources(bus, &add_list, NULL);
> + pci_assign_unassigned_bridge_resources(bus->self);
> }
>
> acpiphp_sanitize_bus(bus);
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v2] PCI: acpiphp: Reassign resources on bridge if necessary
2023-05-22 11:06 ` Igor Mammedov
@ 2023-05-22 11:52 ` Rafael J. Wysocki
2023-05-22 19:21 ` Michael S. Tsirkin
0 siblings, 1 reply; 15+ messages in thread
From: Rafael J. Wysocki @ 2023-05-22 11:52 UTC (permalink / raw)
To: Igor Mammedov
Cc: linux-kernel, rafael, mst, lenb, bhelgaas, linux-acpi, linux-pci,
helgaas, mika.westerberg
On Mon, May 22, 2023 at 1:06 PM Igor Mammedov <imammedo@redhat.com> wrote:
>
> On Mon, 24 Apr 2023 21:15:57 +0200
> 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.
>
> Rafael,
>
> Since there was no more comments for a while,
> can you merge this patch through your tree?
I need an ACK from Bjorn on this, as the PCI maintainer, or it can go
in via the PCI tree as far as I'm concerned.
Thanks!
> > 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.
> >
> > v2:
> > * fixup subject to match expected style
> > * drop no longer needed __pci_bus_size_bridges() to avoid
> > memory leak (Bjorn Helgaas <helgaas@kernel.org>)
> > ---
> > drivers/pci/hotplug/acpiphp_glue.c | 5 +----
> > 1 file changed, 1 insertion(+), 4 deletions(-)
> >
> > diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
> > index 5b1f271c6034..328d1e416014 100644
> > --- a/drivers/pci/hotplug/acpiphp_glue.c
> > +++ b/drivers/pci/hotplug/acpiphp_glue.c
> > @@ -498,7 +498,6 @@ 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);
> > @@ -512,12 +511,10 @@ 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);
> > - __pci_bus_size_bridges(dev->subordinate,
> > - &add_list);
> > }
> > }
> > }
> > - __pci_bus_assign_resources(bus, &add_list, NULL);
> > + pci_assign_unassigned_bridge_resources(bus->self);
> > }
> >
> > acpiphp_sanitize_bus(bus);
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v2] PCI: acpiphp: Reassign resources on bridge if necessary
2023-05-22 11:52 ` Rafael J. Wysocki
@ 2023-05-22 19:21 ` Michael S. Tsirkin
0 siblings, 0 replies; 15+ messages in thread
From: Michael S. Tsirkin @ 2023-05-22 19:21 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Igor Mammedov, linux-kernel, lenb, bhelgaas, linux-acpi,
linux-pci, helgaas, mika.westerberg
On Mon, May 22, 2023 at 01:52:33PM +0200, Rafael J. Wysocki wrote:
> On Mon, May 22, 2023 at 1:06 PM Igor Mammedov <imammedo@redhat.com> wrote:
> >
> > On Mon, 24 Apr 2023 21:15:57 +0200
> > 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.
> >
> > Rafael,
> >
> > Since there was no more comments for a while,
> > can you merge this patch through your tree?
>
> I need an ACK from Bjorn on this, as the PCI maintainer, or it can go
> in via the PCI tree as far as I'm concerned.
>
> Thanks!
I'd also like an ACK for:
Cc: stable@vger.kernel.org
> > > 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.
> > >
> > > v2:
> > > * fixup subject to match expected style
> > > * drop no longer needed __pci_bus_size_bridges() to avoid
> > > memory leak (Bjorn Helgaas <helgaas@kernel.org>)
> > > ---
> > > drivers/pci/hotplug/acpiphp_glue.c | 5 +----
> > > 1 file changed, 1 insertion(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
> > > index 5b1f271c6034..328d1e416014 100644
> > > --- a/drivers/pci/hotplug/acpiphp_glue.c
> > > +++ b/drivers/pci/hotplug/acpiphp_glue.c
> > > @@ -498,7 +498,6 @@ 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);
> > > @@ -512,12 +511,10 @@ 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);
> > > - __pci_bus_size_bridges(dev->subordinate,
> > > - &add_list);
> > > }
> > > }
> > > }
> > > - __pci_bus_assign_resources(bus, &add_list, NULL);
> > > + pci_assign_unassigned_bridge_resources(bus->self);
> > > }
> > >
> > > acpiphp_sanitize_bus(bus);
> >
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2] PCI: acpiphp: Reassign resources on bridge if necessary
2023-04-24 19:15 [PATCH v2] PCI: acpiphp: Reassign resources on bridge if necessary Igor Mammedov
` (2 preceding siblings ...)
2023-05-22 11:06 ` Igor Mammedov
@ 2023-05-22 21:51 ` Bjorn Helgaas
2023-05-22 21:54 ` Michael S. Tsirkin
` (2 more replies)
2023-05-30 17:12 ` Bjorn Helgaas
4 siblings, 3 replies; 15+ messages in thread
From: Bjorn Helgaas @ 2023-05-22 21:51 UTC (permalink / raw)
To: Igor Mammedov
Cc: linux-kernel, mst, rafael, lenb, bhelgaas, linux-acpi, linux-pci,
mika.westerberg
On Mon, Apr 24, 2023 at 09:15:57PM +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>
Tentatively applied to pci/hotplug for v6.5.
Tentative only because I'm hoping for your ack or review, Rafael (I
see you acked v1, but this one is a little different).
And I think you'd like a stable tag, Michael? I'm happy to add one,
but wasn't sure if you wanted a separate ack for that.
> ---
> tested in QEMU with Q35 machine on PCIE root port and also
> with nested conventional bridge attached to root port.
>
> v2:
> * fixup subject to match expected style
> * drop no longer needed __pci_bus_size_bridges() to avoid
> memory leak (Bjorn Helgaas <helgaas@kernel.org>)
> ---
> drivers/pci/hotplug/acpiphp_glue.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
> index 5b1f271c6034..328d1e416014 100644
> --- a/drivers/pci/hotplug/acpiphp_glue.c
> +++ b/drivers/pci/hotplug/acpiphp_glue.c
> @@ -498,7 +498,6 @@ 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);
> @@ -512,12 +511,10 @@ 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);
> - __pci_bus_size_bridges(dev->subordinate,
> - &add_list);
> }
> }
> }
> - __pci_bus_assign_resources(bus, &add_list, NULL);
> + pci_assign_unassigned_bridge_resources(bus->self);
> }
>
> acpiphp_sanitize_bus(bus);
> --
> 2.39.1
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v2] PCI: acpiphp: Reassign resources on bridge if necessary
2023-05-22 21:51 ` Bjorn Helgaas
@ 2023-05-22 21:54 ` Michael S. Tsirkin
2023-05-22 21:55 ` Rafael J. Wysocki
2023-05-23 15:42 ` Bjorn Helgaas
2 siblings, 0 replies; 15+ messages in thread
From: Michael S. Tsirkin @ 2023-05-22 21:54 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Igor Mammedov, linux-kernel, rafael, lenb, bhelgaas, linux-acpi,
linux-pci, mika.westerberg
On Mon, May 22, 2023 at 04:51:41PM -0500, Bjorn Helgaas wrote:
> On Mon, Apr 24, 2023 at 09:15:57PM +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>
>
> Tentatively applied to pci/hotplug for v6.5.
>
> Tentative only because I'm hoping for your ack or review, Rafael (I
> see you acked v1, but this one is a little different).
>
> And I think you'd like a stable tag, Michael? I'm happy to add one,
> but wasn't sure if you wanted a separate ack for that.
I'd like a stable tag, yes. Just was not sure whom best to ask.
> > ---
> > tested in QEMU with Q35 machine on PCIE root port and also
> > with nested conventional bridge attached to root port.
> >
> > v2:
> > * fixup subject to match expected style
> > * drop no longer needed __pci_bus_size_bridges() to avoid
> > memory leak (Bjorn Helgaas <helgaas@kernel.org>)
> > ---
> > drivers/pci/hotplug/acpiphp_glue.c | 5 +----
> > 1 file changed, 1 insertion(+), 4 deletions(-)
> >
> > diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
> > index 5b1f271c6034..328d1e416014 100644
> > --- a/drivers/pci/hotplug/acpiphp_glue.c
> > +++ b/drivers/pci/hotplug/acpiphp_glue.c
> > @@ -498,7 +498,6 @@ 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);
> > @@ -512,12 +511,10 @@ 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);
> > - __pci_bus_size_bridges(dev->subordinate,
> > - &add_list);
> > }
> > }
> > }
> > - __pci_bus_assign_resources(bus, &add_list, NULL);
> > + pci_assign_unassigned_bridge_resources(bus->self);
> > }
> >
> > acpiphp_sanitize_bus(bus);
> > --
> > 2.39.1
> >
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v2] PCI: acpiphp: Reassign resources on bridge if necessary
2023-05-22 21:51 ` Bjorn Helgaas
2023-05-22 21:54 ` Michael S. Tsirkin
@ 2023-05-22 21:55 ` Rafael J. Wysocki
2023-05-23 15:42 ` Bjorn Helgaas
2 siblings, 0 replies; 15+ messages in thread
From: Rafael J. Wysocki @ 2023-05-22 21:55 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Igor Mammedov, linux-kernel, mst, rafael, lenb, bhelgaas,
linux-acpi, linux-pci, mika.westerberg
On Mon, May 22, 2023 at 11:51 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Mon, Apr 24, 2023 at 09:15:57PM +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>
>
> Tentatively applied to pci/hotplug for v6.5.
>
> Tentative only because I'm hoping for your ack or review, Rafael (I
> see you acked v1, but this one is a little different).
This one is fine with me too, so please feel free to add
Acked-by: Rafael J. Wysocki <rafael@kernel.org>
to it.
> And I think you'd like a stable tag, Michael? I'm happy to add one,
> but wasn't sure if you wanted a separate ack for that.
>
> > ---
> > tested in QEMU with Q35 machine on PCIE root port and also
> > with nested conventional bridge attached to root port.
> >
> > v2:
> > * fixup subject to match expected style
> > * drop no longer needed __pci_bus_size_bridges() to avoid
> > memory leak (Bjorn Helgaas <helgaas@kernel.org>)
> > ---
> > drivers/pci/hotplug/acpiphp_glue.c | 5 +----
> > 1 file changed, 1 insertion(+), 4 deletions(-)
> >
> > diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
> > index 5b1f271c6034..328d1e416014 100644
> > --- a/drivers/pci/hotplug/acpiphp_glue.c
> > +++ b/drivers/pci/hotplug/acpiphp_glue.c
> > @@ -498,7 +498,6 @@ 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);
> > @@ -512,12 +511,10 @@ 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);
> > - __pci_bus_size_bridges(dev->subordinate,
> > - &add_list);
> > }
> > }
> > }
> > - __pci_bus_assign_resources(bus, &add_list, NULL);
> > + pci_assign_unassigned_bridge_resources(bus->self);
> > }
> >
> > acpiphp_sanitize_bus(bus);
> > --
> > 2.39.1
> >
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v2] PCI: acpiphp: Reassign resources on bridge if necessary
2023-05-22 21:51 ` Bjorn Helgaas
2023-05-22 21:54 ` Michael S. Tsirkin
2023-05-22 21:55 ` Rafael J. Wysocki
@ 2023-05-23 15:42 ` Bjorn Helgaas
2 siblings, 0 replies; 15+ messages in thread
From: Bjorn Helgaas @ 2023-05-23 15:42 UTC (permalink / raw)
To: Igor Mammedov
Cc: linux-kernel, mst, rafael, lenb, bhelgaas, linux-acpi, linux-pci,
mika.westerberg
On Mon, May 22, 2023 at 04:51:41PM -0500, Bjorn Helgaas wrote:
> On Mon, Apr 24, 2023 at 09:15:57PM +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>
>
> Tentatively applied to pci/hotplug for v6.5.
>
> Tentative only because I'm hoping for your ack or review, Rafael (I
> see you acked v1, but this one is a little different).
>
> And I think you'd like a stable tag, Michael? I'm happy to add one,
> but wasn't sure if you wanted a separate ack for that.
Added stable tag and Rafael's ack. All set for v6.5, thanks!
> > ---
> > tested in QEMU with Q35 machine on PCIE root port and also
> > with nested conventional bridge attached to root port.
> >
> > v2:
> > * fixup subject to match expected style
> > * drop no longer needed __pci_bus_size_bridges() to avoid
> > memory leak (Bjorn Helgaas <helgaas@kernel.org>)
> > ---
> > drivers/pci/hotplug/acpiphp_glue.c | 5 +----
> > 1 file changed, 1 insertion(+), 4 deletions(-)
> >
> > diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
> > index 5b1f271c6034..328d1e416014 100644
> > --- a/drivers/pci/hotplug/acpiphp_glue.c
> > +++ b/drivers/pci/hotplug/acpiphp_glue.c
> > @@ -498,7 +498,6 @@ 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);
> > @@ -512,12 +511,10 @@ 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);
> > - __pci_bus_size_bridges(dev->subordinate,
> > - &add_list);
> > }
> > }
> > }
> > - __pci_bus_assign_resources(bus, &add_list, NULL);
> > + pci_assign_unassigned_bridge_resources(bus->self);
> > }
> >
> > acpiphp_sanitize_bus(bus);
> > --
> > 2.39.1
> >
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2] PCI: acpiphp: Reassign resources on bridge if necessary
2023-04-24 19:15 [PATCH v2] PCI: acpiphp: Reassign resources on bridge if necessary Igor Mammedov
` (3 preceding siblings ...)
2023-05-22 21:51 ` Bjorn Helgaas
@ 2023-05-30 17:12 ` Bjorn Helgaas
2023-05-30 18:16 ` Michael S. Tsirkin
4 siblings, 1 reply; 15+ messages in thread
From: Bjorn Helgaas @ 2023-05-30 17:12 UTC (permalink / raw)
To: Igor Mammedov
Cc: linux-kernel, mst, rafael, lenb, bhelgaas, linux-acpi, linux-pci,
mika.westerberg
On Mon, Apr 24, 2023 at 09:15:57PM +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)
Ugh, I just noticed that we turned on PCI_COMMAND_MEMORY even though
BARs 0 and 1 haven't been assigned. How did that happen? It looks
like pci_enable_resources() checks for that, but there must be a hole
somewhere.
> Unable to create vram_mapping
> qxl: probe of 0000:01:00.0 failed with error -12
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v2] PCI: acpiphp: Reassign resources on bridge if necessary
2023-05-30 17:12 ` Bjorn Helgaas
@ 2023-05-30 18:16 ` Michael S. Tsirkin
2023-05-30 18:55 ` Bjorn Helgaas
0 siblings, 1 reply; 15+ messages in thread
From: Michael S. Tsirkin @ 2023-05-30 18:16 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Igor Mammedov, linux-kernel, rafael, lenb, bhelgaas, linux-acpi,
linux-pci, mika.westerberg
On Tue, May 30, 2023 at 12:12:44PM -0500, Bjorn Helgaas wrote:
> On Mon, Apr 24, 2023 at 09:15:57PM +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)
>
> Ugh, I just noticed that we turned on PCI_COMMAND_MEMORY even though
> BARs 0 and 1 haven't been assigned. How did that happen? It looks
> like pci_enable_resources() checks for that, but there must be a hole
> somewhere.
Maybe because BAR2 was assigned? I think pci_enable_resources just
does
if (r->flags & IORESOURCE_MEM)
cmd |= PCI_COMMAND_MEMORY;
in a loop so if any memory BARs are assigned then PCI_COMMAND_MEMORY
is set.
> > Unable to create vram_mapping
> > qxl: probe of 0000:01:00.0 failed with error -12
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v2] PCI: acpiphp: Reassign resources on bridge if necessary
2023-05-30 18:16 ` Michael S. Tsirkin
@ 2023-05-30 18:55 ` Bjorn Helgaas
2023-05-30 19:06 ` Michael S. Tsirkin
2023-05-31 15:22 ` Igor Mammedov
0 siblings, 2 replies; 15+ messages in thread
From: Bjorn Helgaas @ 2023-05-30 18:55 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Igor Mammedov, linux-kernel, rafael, lenb, bhelgaas, linux-acpi,
linux-pci, mika.westerberg
On Tue, May 30, 2023 at 02:16:36PM -0400, Michael S. Tsirkin wrote:
> On Tue, May 30, 2023 at 12:12:44PM -0500, Bjorn Helgaas wrote:
> > On Mon, Apr 24, 2023 at 09:15:57PM +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)
> >
> > Ugh, I just noticed that we turned on PCI_COMMAND_MEMORY even though
> > BARs 0 and 1 haven't been assigned. How did that happen? It looks
> > like pci_enable_resources() checks for that, but there must be a hole
> > somewhere.
>
> Maybe because BAR2 was assigned? I think pci_enable_resources just
> does
> if (r->flags & IORESOURCE_MEM)
> cmd |= PCI_COMMAND_MEMORY;
> in a loop so if any memory BARs are assigned then PCI_COMMAND_MEMORY
> is set.
It does, but it also bails out if it finds IORESOURCE_UNSET:
pci_enable_resources()
{
...
pci_dev_for_each_resource(dev, r, i) {
...
if (r->flags & IORESOURCE_UNSET) {
pci_err(dev, "can't enable device: BAR %d %pR not assigned\n");
return -EINVAL;
}
...
if (r->flags & IORESOURCE_MEM)
cmd |= PCI_COMMAND_MEMORY;
}
...
}
I expected that IORESOURCE_UNSET would still be there from
pci_assign_resource(), since we saw the "failed to assign" messages,
but there must be more going on.
Bjorn
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v2] PCI: acpiphp: Reassign resources on bridge if necessary
2023-05-30 18:55 ` Bjorn Helgaas
@ 2023-05-30 19:06 ` Michael S. Tsirkin
2023-05-31 15:22 ` Igor Mammedov
1 sibling, 0 replies; 15+ messages in thread
From: Michael S. Tsirkin @ 2023-05-30 19:06 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Igor Mammedov, linux-kernel, rafael, lenb, bhelgaas, linux-acpi,
linux-pci, mika.westerberg
On Tue, May 30, 2023 at 01:55:12PM -0500, Bjorn Helgaas wrote:
> On Tue, May 30, 2023 at 02:16:36PM -0400, Michael S. Tsirkin wrote:
> > On Tue, May 30, 2023 at 12:12:44PM -0500, Bjorn Helgaas wrote:
> > > On Mon, Apr 24, 2023 at 09:15:57PM +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)
> > >
> > > Ugh, I just noticed that we turned on PCI_COMMAND_MEMORY even though
> > > BARs 0 and 1 haven't been assigned. How did that happen? It looks
> > > like pci_enable_resources() checks for that, but there must be a hole
> > > somewhere.
> >
> > Maybe because BAR2 was assigned? I think pci_enable_resources just
> > does
> > if (r->flags & IORESOURCE_MEM)
> > cmd |= PCI_COMMAND_MEMORY;
> > in a loop so if any memory BARs are assigned then PCI_COMMAND_MEMORY
> > is set.
>
> It does, but it also bails out if it finds IORESOURCE_UNSET:
>
> pci_enable_resources()
> {
> ...
> pci_dev_for_each_resource(dev, r, i) {
> ...
> if (r->flags & IORESOURCE_UNSET) {
> pci_err(dev, "can't enable device: BAR %d %pR not assigned\n");
> return -EINVAL;
> }
> ...
> if (r->flags & IORESOURCE_MEM)
> cmd |= PCI_COMMAND_MEMORY;
> }
> ...
> }
>
> I expected that IORESOURCE_UNSET would still be there from
> pci_assign_resource(), since we saw the "failed to assign" messages,
> but there must be more going on.
>
> Bjorn
Oh you are right. I donnu.
--
MST
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v2] PCI: acpiphp: Reassign resources on bridge if necessary
2023-05-30 18:55 ` Bjorn Helgaas
2023-05-30 19:06 ` Michael S. Tsirkin
@ 2023-05-31 15:22 ` Igor Mammedov
1 sibling, 0 replies; 15+ messages in thread
From: Igor Mammedov @ 2023-05-31 15:22 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Michael S. Tsirkin, linux-kernel, rafael, lenb, bhelgaas,
linux-acpi, linux-pci, mika.westerberg
On Tue, 30 May 2023 13:55:12 -0500
Bjorn Helgaas <helgaas@kernel.org> wrote:
> On Tue, May 30, 2023 at 02:16:36PM -0400, Michael S. Tsirkin wrote:
> > On Tue, May 30, 2023 at 12:12:44PM -0500, Bjorn Helgaas wrote:
> > > On Mon, Apr 24, 2023 at 09:15:57PM +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)
> > >
> > > Ugh, I just noticed that we turned on PCI_COMMAND_MEMORY even though
> > > BARs 0 and 1 haven't been assigned. How did that happen? It looks
> > > like pci_enable_resources() checks for that, but there must be a hole
> > > somewhere.
> >
> > Maybe because BAR2 was assigned? I think pci_enable_resources just
> > does
> > if (r->flags & IORESOURCE_MEM)
> > cmd |= PCI_COMMAND_MEMORY;
> > in a loop so if any memory BARs are assigned then PCI_COMMAND_MEMORY
> > is set.
>
> It does, but it also bails out if it finds IORESOURCE_UNSET:
>
> pci_enable_resources()
> {
> ...
> pci_dev_for_each_resource(dev, r, i) {
> ...
> if (r->flags & IORESOURCE_UNSET) {
> pci_err(dev, "can't enable device: BAR %d %pR not assigned\n");
> return -EINVAL;
> }
> ...
> if (r->flags & IORESOURCE_MEM)
> cmd |= PCI_COMMAND_MEMORY;
> }
> ...
> }
>
> I expected that IORESOURCE_UNSET would still be there from
> pci_assign_resource(), since we saw the "failed to assign" messages,
> but there must be more going on.
with current acpiphp code pci_assign_resource() isn't called,
instead it goes __pci_bus_assign_resources() route.
However I an reproduce similar issue with SHPC when using
hierarchy deeper than 1 bridge (for which relocation has never worked)
qemu-kvm -monitor stdio -M q35 -cpu host -enable-kvm -m 4G \
-global ICH9-LPC.acpi-pci-hotplug-with-bridge-support=off \
-device pcie-root-port,id=pr1,bus=pcie.0,chassis=4 \
-device pcie-pci-bridge,id=br1,bus=pr1
hotplug device
(qemu) device_add qxl,addr=1,bus=br1
shpchp 0000:01:00.0: Latch close on Slot(1)
shpchp 0000:01:00.0: Button pressed on Slot(1)
shpchp 0000:01:00.0: Card present on Slot(1)
shpchp 0000:01:00.0: PCI slot #1 - powering on due to button press
pci 0000:02:01.0: [1b36:0100] type 00 class 0x038000
pci 0000:02:01.0: reg 0x10: [mem 0x00000000-0x03ffffff]
pci 0000:02:01.0: reg 0x14: [mem 0x00000000-0x03ffffff]
pci 0000:02:01.0: reg 0x18: [mem 0x00000000-0x00001fff]
pci 0000:02:01.0: reg 0x1c: [io 0x0000-0x001f]
pci 0000:02:01.0: BAR 0: no space for [mem size 0x04000000]
pci 0000:02:01.0: BAR 0: failed to assign [mem size 0x04000000]
pci 0000:02:01.0: BAR 1: no space for [mem size 0x04000000]
pci 0000:02:01.0: BAR 1: failed to assign [mem size 0x04000000]
pci 0000:02:01.0: BAR 2: assigned [mem 0xfe600000-0xfe601fff]
pci 0000:02:01.0: BAR 3: assigned [io 0xc000-0xc01f]
^^^^^^^^^^^
shpchp 0000:01:00.0: PCI bridge to [bus 02]
shpchp 0000:01:00.0: bridge window [io 0xc000-0xcfff]
shpchp 0000:01:00.0: bridge window [mem 0xfe600000-0xfe7fffff]
shpchp 0000:01:00.0: bridge window [mem 0xfe000000-0xfe1fffff 64bit pref]
PCI: No. 2 try to assign unassigned res
release child resource [mem 0xfe600000-0xfe601fff]
shpchp 0000:01:00.0: resource 8 [mem 0xfe600000-0xfe7fffff] released
shpchp 0000:01:00.0: PCI bridge to [bus 02]
shpchp 0000:01:00.0: BAR 8: no space for [mem size 0x0a000000]
shpchp 0000:01:00.0: BAR 8: failed to assign [mem size 0x0a000000]
pci 0000:02:01.0: BAR 0: no space for [mem size 0x04000000]
pci 0000:02:01.0: BAR 0: failed to assign [mem size 0x04000000]
pci 0000:02:01.0: BAR 1: no space for [mem size 0x04000000]
pci 0000:02:01.0: BAR 1: failed to assign [mem size 0x04000000]
pci 0000:02:01.0: BAR 2: no space for [mem size 0x00002000]
pci 0000:02:01.0: BAR 2: failed to assign [mem size 0x00002000]
shpchp 0000:01:00.0: PCI bridge to [bus 02]
shpchp 0000:01:00.0: bridge window [io 0xc000-0xcfff]
shpchp 0000:01:00.0: bridge window [mem 0xfe000000-0xfe1fffff 64bit pref]
qxl 0000:02:01.0: enabling device (0000 -> 0001)
^^^ IO res only
where:
assign_requested_resources_sorted()
...
if (pci_assign_resource())
reset_resource(res);
reset wipes everything pci_assign_resource() has done for failing resources
leaving only assigned IO (on the 1st pass).
Then later pci_enable_device_flags() will build mask for available bars
for (i = 0; i <= PCI_ROM_RESOURCE; i++)
if (dev->resource[i].flags & flags)
bars |= (1 << i);
for (i = PCI_BRIDGE_RESOURCES; i < DEVICE_COUNT_RESOURCE; i++)
if (dev->resource[i].flags & flags)
bars |= (1 << i);
however since for failed MEM resources reset cleared flags along with everything else,
above snippet will ignore MEM bars, leaving only assigned IO resource.
Then
do_pci_enable_device() -> pcibios_enable_device() -> pci_enable_resources(,bars)
will happily succeed since mask tells it only to look into IO.
And well even if mask weren't excluding MEM ones, it won't help since
the resource was cleared out in assign_requested_resources_sorted().
Perhaps instead of playing with flags, we should somehow mark device
with unusable resources as disabled, and fail pci_enable_device() early.
(and also make sure its resources aren't accounted anymore on follow
up hotplug events to the bridge)
^ permalink raw reply [flat|nested] 15+ messages in thread