* [PATCH V2] error path improvement in dlpar add
@ 2026-02-04 22:38 wenxiong
2026-09-09 9:52 ` Nilay Shroff
0 siblings, 1 reply; 2+ messages in thread
From: wenxiong @ 2026-02-04 22:38 UTC (permalink / raw)
To: linuxppc-dev; +Cc: maddy, haren, tyreld, Wen Xiong
From: Wen Xiong <wenxiong@linux.ibm.com>
When doing a dlpar adding a device back, expect not loading device driver
if claiming pci resource failed.
Validating PHB DLPAR capability...yes.
[ 4928.798012] pci_bus 0187:c0: root bus resource [bus c0-ff]
[ 4928.798038] pci 0187:c0:00.0: No hypervisor support for SR-IOV on this device, IOV BARs disabled.
[ 4928.815867] pci 0187:c0:00.0: BAR 0 [mem 0xb0020000-0xb0027fff 64bit]: can't claim; no compatible bridge window
---> Expect to exit if claiming resources fails(BAR0)
[ 4928.815899] pci 0187:c0:00.0: ROM [mem 0xb0000000-0xb001ffff]: can't claim; no compatible bridge window
--->Expect to exit if claiming resources fails(ROM), don't load device
driver anymore, showed "dlpar add failed"
[ 4928.815913] pci 0187:c0:00.0: ROM [mem 0x40000000000-0x4000001ffff]: assigned
[ 4928.815923] pci 0187:c0:00.0: BAR 0 [mem 0x40800000000-0x40800007fff 64bit]: assigned
[ 4928.816220] pci 0187:c0:00.0: BAR 0: error updating (0x00000004 != 0xffffffff)
[ 4928.816364] pci 0187:c0:00.0: BAR 0: error updating (high 0x00060238 != 0xffffffff)
[ 4928.816839] pci 0187:c0:00.0: ibm,query-pe-dma-windows(53) b00000 8000000 20000187 returned -3, lb=0 ps=0 wn=0
[ 4928.816855] pci 0187:c0:00.0: spapr_tce_init_table_group: query_ddw failed
[ 4928.816871] pci 0187:c0:00.0: Adding to iommu group 1
[ 4928.816952] pci 0187:c0:00.0: of_irq_parse_pci: no interrupt-map found, INTx interrupts not available
[ 4928.818135] nvme 0187:c0:00.0: ibm,query-pe-dma-windows(53) b00000 8000000 20000187 returned -3, lb=0 ps=0 wn=0
[ 4928.818284] nvme nvme1: pci function 0187:c0:00.0
[ 4928.819321] rpaphp: Slot [U78DB.ND0.WZS08E2-P1-C7] registered
[ 4928.819338] rpadlpar_io: slot PHB 391 added
With adding error path checking, didn't load the device driver anymore
if claiming resources failed.
Validating PHB DLPAR capability...yes.
[ 260.414453] PCI host bridge /pci@800000020000187 ranges:
[ 260.414473] MEM 0x0000040000000000..0x0000040007ffffff -> 0x00000000b8000000
[ 260.414480] MEM 0x0000040800000000..0x0000040fffffffff -> 0x0006023800000000
[ 260.414915] PCI host bridge to bus 0187:c0
[ 260.414922] pci_bus 0187:c0: root bus resource [mem 0x40000000000-0x40007ffffff] (bus address [0xb8000000-0xbfffffff])
[ 260.414929] pci_bus 0187:c0: root bus resource [mem 0x40800000000-0x40fffffffff 64bit] (bus address [0x6023800000000-0x6023fffffffff])
[ 260.414936] pci_bus 0187:c0: root bus resource [bus c0-ff]
[ 260.414948] pci 0187:c0:00.0: No hypervisor support for SR-IOV on this device, IOV BARs disabled.
[ 260.423439] pci 0187:c0:00.0: BAR 0 [mem 0xb0020000-0xb0027fff 64bit]: can't claim; no compatible bridge window
[ 260.423463] after pci_cliam_Resource=-22 ----> pci_claim_resources fails.
[ 260.423471] rpadlpar_io: slot PHB 391 added failed rc=-5 -----> added failed.
Signed-off-by: Wen Xiong <wenxiong@linux.ibm.com>
Reviewed-by: Haren Myneni <haren@linux.ibm.com>
Reviewed-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
arch/powerpc/include/asm/pci.h | 4 ++--
arch/powerpc/kernel/pci-common.c | 23 +++++++++++++++++-----
arch/powerpc/kernel/pci-hotplug.c | 5 ++++-
arch/powerpc/platforms/pseries/pci_dlpar.c | 3 ++-
drivers/pci/hotplug/rpadlpar_core.c | 7 ++++++-
5 files changed, 32 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/include/asm/pci.h b/arch/powerpc/include/asm/pci.h
index 46a9c4491ed0..ebeb912a92bf 100644
--- a/arch/powerpc/include/asm/pci.h
+++ b/arch/powerpc/include/asm/pci.h
@@ -86,9 +86,9 @@ extern void pci_adjust_legacy_attr(struct pci_bus *bus,
enum pci_mmap_state mmap_type);
#define HAVE_PCI_LEGACY 1
-extern void pcibios_claim_one_bus(struct pci_bus *b);
+extern int pcibios_claim_one_bus(struct pci_bus *b);
-extern void pcibios_finish_adding_to_bus(struct pci_bus *bus);
+extern int pcibios_finish_adding_to_bus(struct pci_bus *bus);
extern void pcibios_resource_survey(void);
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index eac84d687b53..b16f43c7f02e 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1446,10 +1446,11 @@ void __init pcibios_resource_survey(void)
* rest of the code later, for now, keep it as-is as our main
* resource allocation function doesn't deal with sub-trees yet.
*/
-void pcibios_claim_one_bus(struct pci_bus *bus)
+int pcibios_claim_one_bus(struct pci_bus *bus)
{
struct pci_dev *dev;
struct pci_bus *child_bus;
+ int ret = 0;
list_for_each_entry(dev, &bus->devices, bus_list) {
struct resource *r;
@@ -1462,15 +1463,20 @@ void pcibios_claim_one_bus(struct pci_bus *bus)
pr_debug("PCI: Claiming %s: Resource %d: %pR\n",
pci_name(dev), i, r);
- if (pci_claim_resource(dev, i) == 0)
+ ret = pci_claim_resource(dev, i);
+ if (ret == 0)
continue;
+ else
+ return ret;
pci_claim_bridge_resource(dev, i);
}
}
list_for_each_entry(child_bus, &bus->children, node)
- pcibios_claim_one_bus(child_bus);
+ ret = pcibios_claim_one_bus(child_bus);
+
+ return ret;
}
EXPORT_SYMBOL_GPL(pcibios_claim_one_bus);
@@ -1481,14 +1487,19 @@ EXPORT_SYMBOL_GPL(pcibios_claim_one_bus);
* added to a bus, this include calling it for a PHB that is just
* being added
*/
-void pcibios_finish_adding_to_bus(struct pci_bus *bus)
+int pcibios_finish_adding_to_bus(struct pci_bus *bus)
{
+ int ret = 0;
+
pr_debug("PCI: Finishing adding to hotplug bus %04x:%02x\n",
pci_domain_nr(bus), bus->number);
/* Allocate bus and devices resources */
pcibios_allocate_bus_resources(bus);
- pcibios_claim_one_bus(bus);
+ ret = pcibios_claim_one_bus(bus);
+ if (ret)
+ return ret;
+
if (!pci_has_flag(PCI_PROBE_ONLY)) {
if (bus->self)
pci_assign_unassigned_bridge_resources(bus->self);
@@ -1498,6 +1509,8 @@ void pcibios_finish_adding_to_bus(struct pci_bus *bus)
/* Add new devices to global lists. Register in proc, sysfs. */
pci_bus_add_devices(bus);
+
+ return ret;
}
EXPORT_SYMBOL_GPL(pcibios_finish_adding_to_bus);
diff --git a/arch/powerpc/kernel/pci-hotplug.c b/arch/powerpc/kernel/pci-hotplug.c
index 6f444d0822d8..133ce03fdd82 100644
--- a/arch/powerpc/kernel/pci-hotplug.c
+++ b/arch/powerpc/kernel/pci-hotplug.c
@@ -140,6 +140,7 @@ void pci_hp_add_devices(struct pci_bus *bus)
struct pci_dev *dev;
struct pci_controller *phb;
struct device_node *dn = pci_bus_to_OF_node(bus);
+ int ret = 0;
if (!dn)
return;
@@ -176,6 +177,8 @@ void pci_hp_add_devices(struct pci_bus *bus)
for_each_pci_bridge(dev, bus)
max = pci_scan_bridge(bus, dev, max, 1);
}
- pcibios_finish_adding_to_bus(bus);
+ ret = pcibios_finish_adding_to_bus(bus);
+ if (ret)
+ pr_err("Unable to add hotplug pci device!\n");
}
EXPORT_SYMBOL_GPL(pci_hp_add_devices);
diff --git a/arch/powerpc/platforms/pseries/pci_dlpar.c b/arch/powerpc/platforms/pseries/pci_dlpar.c
index 8c77ec7980de..2b78faba40f6 100644
--- a/arch/powerpc/platforms/pseries/pci_dlpar.c
+++ b/arch/powerpc/platforms/pseries/pci_dlpar.c
@@ -58,7 +58,8 @@ struct pci_controller *init_phb_dynamic(struct device_node *dn)
pseries_eeh_init_edev_recursive(PCI_DN(dn));
pcibios_scan_phb(phb);
- pcibios_finish_adding_to_bus(phb->bus);
+ if (pcibios_finish_adding_to_bus(phb->bus))
+ return NULL;
return phb;
}
diff --git a/drivers/pci/hotplug/rpadlpar_core.c b/drivers/pci/hotplug/rpadlpar_core.c
index 980bb3afd092..4c507dd04fa1 100644
--- a/drivers/pci/hotplug/rpadlpar_core.c
+++ b/drivers/pci/hotplug/rpadlpar_core.c
@@ -240,8 +240,10 @@ static int dlpar_add_phb(char *drc_name, struct device_node *dn)
}
phb = init_phb_dynamic(dn);
- if (!phb)
+ if (!phb) {
+ pr_err("Unable to add hotplug slot %s\n", drc_name);
return -EIO;
+ }
if (rpaphp_add_slot(dn)) {
printk(KERN_ERR "%s: unable to add hotplug slot %s\n",
@@ -311,6 +313,9 @@ int dlpar_add_slot(char *drc_name)
}
of_node_put(dn);
+ if (rc)
+ goto exit;
+
printk(KERN_INFO "%s: slot %s added\n", DLPAR_MODULE_NAME, drc_name);
exit:
mutex_unlock(&rpadlpar_mutex);
--
2.47.3
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH V2] error path improvement in dlpar add
2026-02-04 22:38 [PATCH V2] error path improvement in dlpar add wenxiong
@ 2026-09-09 9:52 ` Nilay Shroff
0 siblings, 0 replies; 2+ messages in thread
From: Nilay Shroff @ 2026-09-09 9:52 UTC (permalink / raw)
To: wenxiong, linuxppc-dev; +Cc: maddy, haren, tyreld
On 2/5/26 4:08 AM, wenxiong@linux.ibm.com wrote:
> From: Wen Xiong <wenxiong@linux.ibm.com>
>
> When doing a dlpar adding a device back, expect not loading device driver
> if claiming pci resource failed.
>
> Validating PHB DLPAR capability...yes.
> [ 4928.798012] pci_bus 0187:c0: root bus resource [bus c0-ff]
> [ 4928.798038] pci 0187:c0:00.0: No hypervisor support for SR-IOV on this device, IOV BARs disabled.
> [ 4928.815867] pci 0187:c0:00.0: BAR 0 [mem 0xb0020000-0xb0027fff 64bit]: can't claim; no compatible bridge window
>
> ---> Expect to exit if claiming resources fails(BAR0)
>
> [ 4928.815899] pci 0187:c0:00.0: ROM [mem 0xb0000000-0xb001ffff]: can't claim; no compatible bridge window
>
> --->Expect to exit if claiming resources fails(ROM), don't load device
> driver anymore, showed "dlpar add failed"
>
> [ 4928.815913] pci 0187:c0:00.0: ROM [mem 0x40000000000-0x4000001ffff]: assigned
> [ 4928.815923] pci 0187:c0:00.0: BAR 0 [mem 0x40800000000-0x40800007fff 64bit]: assigned
> [ 4928.816220] pci 0187:c0:00.0: BAR 0: error updating (0x00000004 != 0xffffffff)
> [ 4928.816364] pci 0187:c0:00.0: BAR 0: error updating (high 0x00060238 != 0xffffffff)
> [ 4928.816839] pci 0187:c0:00.0: ibm,query-pe-dma-windows(53) b00000 8000000 20000187 returned -3, lb=0 ps=0 wn=0
> [ 4928.816855] pci 0187:c0:00.0: spapr_tce_init_table_group: query_ddw failed
> [ 4928.816871] pci 0187:c0:00.0: Adding to iommu group 1
> [ 4928.816952] pci 0187:c0:00.0: of_irq_parse_pci: no interrupt-map found, INTx interrupts not available
> [ 4928.818135] nvme 0187:c0:00.0: ibm,query-pe-dma-windows(53) b00000 8000000 20000187 returned -3, lb=0 ps=0 wn=0
> [ 4928.818284] nvme nvme1: pci function 0187:c0:00.0
> [ 4928.819321] rpaphp: Slot [U78DB.ND0.WZS08E2-P1-C7] registered
> [ 4928.819338] rpadlpar_io: slot PHB 391 added
>
> With adding error path checking, didn't load the device driver anymore
> if claiming resources failed.
>
> Validating PHB DLPAR capability...yes.
> [ 260.414453] PCI host bridge /pci@800000020000187 ranges:
> [ 260.414473] MEM 0x0000040000000000..0x0000040007ffffff -> 0x00000000b8000000
> [ 260.414480] MEM 0x0000040800000000..0x0000040fffffffff -> 0x0006023800000000
> [ 260.414915] PCI host bridge to bus 0187:c0
> [ 260.414922] pci_bus 0187:c0: root bus resource [mem 0x40000000000-0x40007ffffff] (bus address [0xb8000000-0xbfffffff])
> [ 260.414929] pci_bus 0187:c0: root bus resource [mem 0x40800000000-0x40fffffffff 64bit] (bus address [0x6023800000000-0x6023fffffffff])
> [ 260.414936] pci_bus 0187:c0: root bus resource [bus c0-ff]
> [ 260.414948] pci 0187:c0:00.0: No hypervisor support for SR-IOV on this device, IOV BARs disabled.
> [ 260.423439] pci 0187:c0:00.0: BAR 0 [mem 0xb0020000-0xb0027fff 64bit]: can't claim; no compatible bridge window
> [ 260.423463] after pci_cliam_Resource=-22 ----> pci_claim_resources fails.
>
> [ 260.423471] rpadlpar_io: slot PHB 391 added failed rc=-5 -----> added failed.
>
> Signed-off-by: Wen Xiong <wenxiong@linux.ibm.com>
> Reviewed-by: Haren Myneni <haren@linux.ibm.com>
> Reviewed-by: Tyrel Datwyler <tyreld@linux.ibm.com>
> ---
> arch/powerpc/include/asm/pci.h | 4 ++--
> arch/powerpc/kernel/pci-common.c | 23 +++++++++++++++++-----
> arch/powerpc/kernel/pci-hotplug.c | 5 ++++-
> arch/powerpc/platforms/pseries/pci_dlpar.c | 3 ++-
> drivers/pci/hotplug/rpadlpar_core.c | 7 ++++++-
> 5 files changed, 32 insertions(+), 10 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/pci.h b/arch/powerpc/include/asm/pci.h
> index 46a9c4491ed0..ebeb912a92bf 100644
> --- a/arch/powerpc/include/asm/pci.h
> +++ b/arch/powerpc/include/asm/pci.h
> @@ -86,9 +86,9 @@ extern void pci_adjust_legacy_attr(struct pci_bus *bus,
> enum pci_mmap_state mmap_type);
> #define HAVE_PCI_LEGACY 1
>
> -extern void pcibios_claim_one_bus(struct pci_bus *b);
> +extern int pcibios_claim_one_bus(struct pci_bus *b);
>
> -extern void pcibios_finish_adding_to_bus(struct pci_bus *bus);
> +extern int pcibios_finish_adding_to_bus(struct pci_bus *bus);
>
> extern void pcibios_resource_survey(void);
>
> diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
> index eac84d687b53..b16f43c7f02e 100644
> --- a/arch/powerpc/kernel/pci-common.c
> +++ b/arch/powerpc/kernel/pci-common.c
> @@ -1446,10 +1446,11 @@ void __init pcibios_resource_survey(void)
> * rest of the code later, for now, keep it as-is as our main
> * resource allocation function doesn't deal with sub-trees yet.
> */
> -void pcibios_claim_one_bus(struct pci_bus *bus)
> +int pcibios_claim_one_bus(struct pci_bus *bus)
> {
> struct pci_dev *dev;
> struct pci_bus *child_bus;
> + int ret = 0;
>
> list_for_each_entry(dev, &bus->devices, bus_list) {
> struct resource *r;
> @@ -1462,15 +1463,20 @@ void pcibios_claim_one_bus(struct pci_bus *bus)
> pr_debug("PCI: Claiming %s: Resource %d: %pR\n",
> pci_name(dev), i, r);
>
> - if (pci_claim_resource(dev, i) == 0)
> + ret = pci_claim_resource(dev, i);
> + if (ret == 0)
> continue;
> + else
> + return ret;
>
This looks incorrect. With this change we would always skip
pci_claim_bridge_resource(). In case pci_claim_resource() fails, the
existing fallback is to try pci_claim_bridge_resource(). So rather than
returning immediately after pci_claim_resource() fails, I think we
should preserve this fallback and propagate the failure only if the
bridge-resource claim also fails.
There is also another issue to handle: if the fallback
pci_claim_bridge_resource() fails, we may have already successfully
claimed resources earlier in this traversal. Since the caller now
treats this as an error and aborts adding the bus, we need to make sure
those resources are properly rolled back/cleaned up before returning
the failure.
> pci_claim_bridge_resource(dev, i);
> }
> }
>
> list_for_each_entry(child_bus, &bus->children, node)
> - pcibios_claim_one_bus(child_bus);
> + ret = pcibios_claim_one_bus(child_bus);
> +
> + return ret;
> }
> EXPORT_SYMBOL_GPL(pcibios_claim_one_bus);
>
> @@ -1481,14 +1487,19 @@ EXPORT_SYMBOL_GPL(pcibios_claim_one_bus);
> * added to a bus, this include calling it for a PHB that is just
> * being added
> */
> -void pcibios_finish_adding_to_bus(struct pci_bus *bus)
> +int pcibios_finish_adding_to_bus(struct pci_bus *bus)
> {
> + int ret = 0;
> +
> pr_debug("PCI: Finishing adding to hotplug bus %04x:%02x\n",
> pci_domain_nr(bus), bus->number);
>
> /* Allocate bus and devices resources */
> pcibios_allocate_bus_resources(bus);
> - pcibios_claim_one_bus(bus);
> + ret = pcibios_claim_one_bus(bus);
> + if (ret)
> + return ret;
> +
> if (!pci_has_flag(PCI_PROBE_ONLY)) {
> if (bus->self)
> pci_assign_unassigned_bridge_resources(bus->self);
> @@ -1498,6 +1509,8 @@ void pcibios_finish_adding_to_bus(struct pci_bus *bus)
>
> /* Add new devices to global lists. Register in proc, sysfs. */
> pci_bus_add_devices(bus);
> +
> + return ret;
> }
> EXPORT_SYMBOL_GPL(pcibios_finish_adding_to_bus);
>
> diff --git a/arch/powerpc/kernel/pci-hotplug.c b/arch/powerpc/kernel/pci-hotplug.c
> index 6f444d0822d8..133ce03fdd82 100644
> --- a/arch/powerpc/kernel/pci-hotplug.c
> +++ b/arch/powerpc/kernel/pci-hotplug.c
> @@ -140,6 +140,7 @@ void pci_hp_add_devices(struct pci_bus *bus)
> struct pci_dev *dev;
> struct pci_controller *phb;
> struct device_node *dn = pci_bus_to_OF_node(bus);
> + int ret = 0;
>
> if (!dn)
> return;
> @@ -176,6 +177,8 @@ void pci_hp_add_devices(struct pci_bus *bus)
> for_each_pci_bridge(dev, bus)
> max = pci_scan_bridge(bus, dev, max, 1);
> }
> - pcibios_finish_adding_to_bus(bus);
> + ret = pcibios_finish_adding_to_bus(bus);
> + if (ret)
> + pr_err("Unable to add hotplug pci device!\n");
I think, it would be useful to include the PCI bus/domain in this error message,
so that we can identify which hotplug bus failed.
> }
> EXPORT_SYMBOL_GPL(pci_hp_add_devices);
> diff --git a/arch/powerpc/platforms/pseries/pci_dlpar.c b/arch/powerpc/platforms/pseries/pci_dlpar.c
> index 8c77ec7980de..2b78faba40f6 100644
> --- a/arch/powerpc/platforms/pseries/pci_dlpar.c
> +++ b/arch/powerpc/platforms/pseries/pci_dlpar.c
> @@ -58,7 +58,8 @@ struct pci_controller *init_phb_dynamic(struct device_node *dn)
> pseries_eeh_init_edev_recursive(PCI_DN(dn));
>
> pcibios_scan_phb(phb);
> - pcibios_finish_adding_to_bus(phb->bus);
> + if (pcibios_finish_adding_to_bus(phb->bus))
> + return NULL;
>
Here as well, when pcibios_finish_adding_to_bus() fails, I think we
need to clean up the PHB before returning NULL. pcibios_scan_phb() has
already created/scanned the PCI bus hierarchy, so simply returning NULL
may leave the partially initialized PHB and scanned PCI devices behind.
Overall I think the intent is correct, however the commit message
should also be tightened up a bit to make the problem and the expected
behavior more descriptive.
Thanks,
--Nilay
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-09-09 9:52 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-04 22:38 [PATCH V2] error path improvement in dlpar add wenxiong
2026-09-09 9:52 ` Nilay Shroff
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox