[PATCH] powerpc/rtas_pci: No hotplug on permanently removed device on pSeries

[PATCH] powerpc/rtas_pci: No hotplug on permanently removed device on pSeries

[Shivaprasad G Bhat]

The eeh_driver disables and offlines the PE permanently when it
exceeds the freeze count beyond eeh_max_freeze within the last hour.
The PE is only offline, so the device tree entries, eeh device
references are all intact till the real unplug of the device from
the guest/host takes place.

On pSeries, with a new hotplug of any PCI device, the drmgr initiates
a system-wide PCI rescan, which finds devices offlined by the eeh_driver
and there will be attempts to bring them online. This leads to
recurring EEHs either at the config read time itself or a bit
later depending on the type of the problem.

For PowerNV, the commit d2b0f6f77ee5 ("powerpc/eeh: No hotplug on
permanently removed dev") introduced the EEH_DEV_REMOVED flag to
prevent such inadvertent rescans on hierarchical toplogies relavent in
Baremetal setups. For pSeries, such topologies don't really make sense
as the devices are either part of the same PE OR exposed as independent
devices on multiple virtual PHBs. However, the inadvertent rescans are
still a possibility with either hotplug of a new device or otherwise
with manual system-wide pci bus rescan attempts.

So the patch checks for EEH_DEV_REMOVED before allowing config space
access just like PowerNV, making the PCI core omit the PE, and thus
preventing subsequent EEH recurances. The patch is tested on PowerVM
and KVM machines with single and multi-function devices, and on the
devices behind a switch. The unplug of the affected devices post EEH
removal is also working fine as expected.

Signed-off-by: Shivaprasad G Bhat <sbhat@linux.ibm.com>
References: d2b0f6f77ee5 ("powerpc/eeh: No hotplug on permanently removed dev")
---
 arch/powerpc/kernel/rtas_pci.c |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/powerpc/kernel/rtas_pci.c b/arch/powerpc/kernel/rtas_pci.c
index fccf96e897f6..ce24b18712ca 100644
--- a/arch/powerpc/kernel/rtas_pci.c
+++ b/arch/powerpc/kernel/rtas_pci.c
@@ -57,6 +57,9 @@ int rtas_pci_dn_read_config(struct pci_dn *pdn, int where, int size, u32 *val)
 	if (pdn->edev && pdn->edev->pe &&
 	    (pdn->edev->pe->state & EEH_PE_CFG_BLOCKED))
 		return PCIBIOS_SET_FAILED;
+
+	if (pdn->edev && pdn->edev->mode & EEH_DEV_REMOVED)
+		return PCIBIOS_SET_FAILED;
 #endif
 
 	addr = rtas_config_addr(pdn->busno, pdn->devfn, where);
@@ -108,6 +111,9 @@ int rtas_pci_dn_write_config(struct pci_dn *pdn, int where, int size, u32 val)
 	if (pdn->edev && pdn->edev->pe &&
 	    (pdn->edev->pe->state & EEH_PE_CFG_BLOCKED))
 		return PCIBIOS_SET_FAILED;
+
+	if (pdn->edev && pdn->edev->mode & EEH_DEV_REMOVED)
+		return PCIBIOS_SET_FAILED;
 #endif
 
 	addr = rtas_config_addr(pdn->busno, pdn->devfn, where);

Re: [PATCH] powerpc/rtas_pci: No hotplug on permanently removed device on pSeries

[Harsh Prateek Bora]

On 27/04/26 8:25 am, Shivaprasad G Bhat wrote:
> The eeh_driver disables and offlines the PE permanently when it
> exceeds the freeze count beyond eeh_max_freeze within the last hour.
> The PE is only offline, so the device tree entries, eeh device
> references are all intact till the real unplug of the device from
> the guest/host takes place.
> 
> On pSeries, with a new hotplug of any PCI device, the drmgr initiates
> a system-wide PCI rescan, which finds devices offlined by the eeh_driver
> and there will be attempts to bring them online. This leads to
> recurring EEHs either at the config read time itself or a bit
> later depending on the type of the problem.
> 
> For PowerNV, the commit d2b0f6f77ee5 ("powerpc/eeh: No hotplug on
> permanently removed dev") introduced the EEH_DEV_REMOVED flag to
> prevent such inadvertent rescans on hierarchical toplogies relavent in
> Baremetal setups. For pSeries, such topologies don't really make sense
> as the devices are either part of the same PE OR exposed as independent
> devices on multiple virtual PHBs. However, the inadvertent rescans are
> still a possibility with either hotplug of a new device or otherwise
> with manual system-wide pci bus rescan attempts.
> 
> So the patch checks for EEH_DEV_REMOVED before allowing config space
> access just like PowerNV, making the PCI core omit the PE, and thus
> preventing subsequent EEH recurances. The patch is tested on PowerVM
> and KVM machines with single and multi-function devices, and on the
> devices behind a switch. The unplug of the affected devices post EEH
> removal is also working fine as expected.
> 
> Signed-off-by: Shivaprasad G Bhat <sbhat@linux.ibm.com>
> References: d2b0f6f77ee5 ("powerpc/eeh: No hotplug on permanently removed dev")
> ---
>   arch/powerpc/kernel/rtas_pci.c |    6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/arch/powerpc/kernel/rtas_pci.c b/arch/powerpc/kernel/rtas_pci.c
> index fccf96e897f6..ce24b18712ca 100644
> --- a/arch/powerpc/kernel/rtas_pci.c
> +++ b/arch/powerpc/kernel/rtas_pci.c
> @@ -57,6 +57,9 @@ int rtas_pci_dn_read_config(struct pci_dn *pdn, int where, int size, u32 *val)
>   	if (pdn->edev && pdn->edev->pe &&
>   	    (pdn->edev->pe->state & EEH_PE_CFG_BLOCKED))
>   		return PCIBIOS_SET_FAILED;
> +
> +	if (pdn->edev && pdn->edev->mode & EEH_DEV_REMOVED)

Consider using paranthesis for (pdn->edev->mode & EEH_DEV_REMOVED) and 
moving to next line for readability (similar to prev one).

> +		return PCIBIOS_SET_FAILED;

Why not return PCIBIOS_DEVICE_NOT_FOUND ? Returning SET_FAILED for a 
removed device could be misleading.

Above comments applies to below changes also.

>   #endif
>   
>   	addr = rtas_config_addr(pdn->busno, pdn->devfn, where);
> @@ -108,6 +111,9 @@ int rtas_pci_dn_write_config(struct pci_dn *pdn, int where, int size, u32 val)
>   	if (pdn->edev && pdn->edev->pe &&
>   	    (pdn->edev->pe->state & EEH_PE_CFG_BLOCKED))
>   		return PCIBIOS_SET_FAILED;
> +
> +	if (pdn->edev && pdn->edev->mode & EEH_DEV_REMOVED)
> +		return PCIBIOS_SET_FAILED;
>   #endif
>   
>   	addr = rtas_config_addr(pdn->busno, pdn->devfn, where);
> 
> 
> 

Re: [PATCH] powerpc/rtas_pci: No hotplug on permanently removed device on pSeries

[Harsh Prateek Bora]

On 12/05/26 10:47 pm, Harsh Prateek Bora wrote:
> 
> 
> On 27/04/26 8:25 am, Shivaprasad G Bhat wrote:
>> The eeh_driver disables and offlines the PE permanently when it
>> exceeds the freeze count beyond eeh_max_freeze within the last hour.
>> The PE is only offline, so the device tree entries, eeh device
>> references are all intact till the real unplug of the device from
>> the guest/host takes place.
>>
>> On pSeries, with a new hotplug of any PCI device, the drmgr initiates
>> a system-wide PCI rescan, which finds devices offlined by the eeh_driver
>> and there will be attempts to bring them online. This leads to
>> recurring EEHs either at the config read time itself or a bit
>> later depending on the type of the problem.
>>
>> For PowerNV, the commit d2b0f6f77ee5 ("powerpc/eeh: No hotplug on
>> permanently removed dev") introduced the EEH_DEV_REMOVED flag to
>> prevent such inadvertent rescans on hierarchical toplogies relavent in
>> Baremetal setups. For pSeries, such topologies don't really make sense
>> as the devices are either part of the same PE OR exposed as independent
>> devices on multiple virtual PHBs. However, the inadvertent rescans are
>> still a possibility with either hotplug of a new device or otherwise
>> with manual system-wide pci bus rescan attempts.
>>
>> So the patch checks for EEH_DEV_REMOVED before allowing config space
>> access just like PowerNV, making the PCI core omit the PE, and thus

Also, not sure if this commit description is correct as the patch only
changes behaviour of RTAS config-space accesses and not making changes
for omission semantics.
The existing code in arch/powerpc/kernel/pci_of_scan.c already
suppresses discovery of removed device:

#ifdef CONFIG_EEH
         if (edev && (edev->mode & EEH_DEV_REMOVED))
                 return NULL;
#endif

>> preventing subsequent EEH recurances. The patch is tested on PowerVM
>> and KVM machines with single and multi-function devices, and on the
>> devices behind a switch. The unplug of the affected devices post EEH
>> removal is also working fine as expected.
>>
>> Signed-off-by: Shivaprasad G Bhat <sbhat@linux.ibm.com>
>> References: d2b0f6f77ee5 ("powerpc/eeh: No hotplug on permanently 
>> removed dev")
>> ---
>>   arch/powerpc/kernel/rtas_pci.c |    6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/arch/powerpc/kernel/rtas_pci.c b/arch/powerpc/kernel/ 
>> rtas_pci.c
>> index fccf96e897f6..ce24b18712ca 100644
>> --- a/arch/powerpc/kernel/rtas_pci.c
>> +++ b/arch/powerpc/kernel/rtas_pci.c
>> @@ -57,6 +57,9 @@ int rtas_pci_dn_read_config(struct pci_dn *pdn, int 
>> where, int size, u32 *val)
>>       if (pdn->edev && pdn->edev->pe &&
>>           (pdn->edev->pe->state & EEH_PE_CFG_BLOCKED))
>>           return PCIBIOS_SET_FAILED;
>> +
>> +    if (pdn->edev && pdn->edev->mode & EEH_DEV_REMOVED)
> 
> Consider using paranthesis for (pdn->edev->mode & EEH_DEV_REMOVED) and 
> moving to next line for readability (similar to prev one).
> 
>> +        return PCIBIOS_SET_FAILED;
> 
> Why not return PCIBIOS_DEVICE_NOT_FOUND ? Returning SET_FAILED for a 
> removed device could be misleading.
> 

Also we should check for EEH_DEV_REMOVED before checking for
EEH_PE_CFG_BLOCKED to avoid returning early when the latter is still set
for a removed device.

> Above comments applies to below changes also.
> 
>>   #endif
>>       addr = rtas_config_addr(pdn->busno, pdn->devfn, where);
>> @@ -108,6 +111,9 @@ int rtas_pci_dn_write_config(struct pci_dn *pdn, 
>> int where, int size, u32 val)
>>       if (pdn->edev && pdn->edev->pe &&
>>           (pdn->edev->pe->state & EEH_PE_CFG_BLOCKED))
>>           return PCIBIOS_SET_FAILED;
>> +
>> +    if (pdn->edev && pdn->edev->mode & EEH_DEV_REMOVED)
>> +        return PCIBIOS_SET_FAILED;
>>   #endif
>>       addr = rtas_config_addr(pdn->busno, pdn->devfn, where);
>>
>>
>>
> 
> 

Re: [PATCH] powerpc/rtas_pci: No hotplug on permanently removed device on pSeries

[Shivaprasad G Bhat]

Hi Harsh,

On 5/12/26 11:14 PM, Harsh Prateek Bora wrote:
>
>
> On 12/05/26 10:47 pm, Harsh Prateek Bora wrote:
>>
>>
>> On 27/04/26 8:25 am, Shivaprasad G Bhat wrote:
>>> The eeh_driver disables and offlines the PE permanently when it
>>> exceeds the freeze count beyond eeh_max_freeze within the last hour.
>>> The PE is only offline, so the device tree entries, eeh device
>>> references are all intact till the real unplug of the device from
>>> the guest/host takes place.
>>>
>>> On pSeries, with a new hotplug of any PCI device, the drmgr initiates
>>> a system-wide PCI rescan, which finds devices offlined by the 
>>> eeh_driver
>>> and there will be attempts to bring them online. This leads to
>>> recurring EEHs either at the config read time itself or a bit
>>> later depending on the type of the problem.
>>>
>>> For PowerNV, the commit d2b0f6f77ee5 ("powerpc/eeh: No hotplug on
>>> permanently removed dev") introduced the EEH_DEV_REMOVED flag to
>>> prevent such inadvertent rescans on hierarchical toplogies relavent in
>>> Baremetal setups. For pSeries, such topologies don't really make sense
>>> as the devices are either part of the same PE OR exposed as independent
>>> devices on multiple virtual PHBs. However, the inadvertent rescans are
>>> still a possibility with either hotplug of a new device or otherwise
>>> with manual system-wide pci bus rescan attempts.
>>>
>>> So the patch checks for EEH_DEV_REMOVED before allowing config space
>>> access just like PowerNV, making the PCI core omit the PE, and thus
>
> Also, not sure if this commit description is correct as the patch only
> changes behaviour of RTAS config-space accesses and not making changes
> for omission semantics.

This is for the pci core rescan attempts to be skipped, when rescan is 
attempted

with write to rescan sysfs file.

  pci_bus_generic_read_dev_vendor_id+0x48/0x220
  pci_scan_single_device+0xa8/0x130
  pci_scan_slot+0x9c/0x2e0
  pci_scan_child_bus_extend+0x70/0x410
  pci_rescan_bus+0x2c/0x70
  rescan_store+0xa4/0xe0
  bus_attr_store+0x3c/0x60
  sysfs_kf_write+0xb0/0xe0

> The existing code in arch/powerpc/kernel/pci_of_scan.c already
> suppresses discovery of removed device:
>
> #ifdef CONFIG_EEH
>         if (edev && (edev->mode & EEH_DEV_REMOVED))
>                 return NULL;
> #endif
>
This is boot time only scan from the pcibios_init() path.


>>> preventing subsequent EEH recurances. The patch is tested on PowerVM
>>> and KVM machines with single and multi-function devices, and on the
>>> devices behind a switch. The unplug of the affected devices post EEH
>>> removal is also working fine as expected.
>>>
>>> Signed-off-by: Shivaprasad G Bhat <sbhat@linux.ibm.com>
>>> References: d2b0f6f77ee5 ("powerpc/eeh: No hotplug on permanently 
>>> removed dev")
>>> ---
>>>   arch/powerpc/kernel/rtas_pci.c |    6 ++++++
>>>   1 file changed, 6 insertions(+)
>>>
>>> diff --git a/arch/powerpc/kernel/rtas_pci.c b/arch/powerpc/kernel/ 
>>> rtas_pci.c
>>> index fccf96e897f6..ce24b18712ca 100644
>>> --- a/arch/powerpc/kernel/rtas_pci.c
>>> +++ b/arch/powerpc/kernel/rtas_pci.c
>>> @@ -57,6 +57,9 @@ int rtas_pci_dn_read_config(struct pci_dn *pdn, 
>>> int where, int size, u32 *val)
>>>       if (pdn->edev && pdn->edev->pe &&
>>>           (pdn->edev->pe->state & EEH_PE_CFG_BLOCKED))
>>>           return PCIBIOS_SET_FAILED;
>>> +
>>> +    if (pdn->edev && pdn->edev->mode & EEH_DEV_REMOVED)
>>
>> Consider using paranthesis for (pdn->edev->mode & EEH_DEV_REMOVED) 
>> and moving to next line for readability (similar to prev one).
Sure.
>>
>>> +        return PCIBIOS_SET_FAILED;
>>
>> Why not return PCIBIOS_DEVICE_NOT_FOUND ? Returning SET_FAILED for a 
>> removed device could be misleading.
>>
>

Sure.


> Also we should check for EEH_DEV_REMOVED before checking for
> EEH_PE_CFG_BLOCKED to avoid returning early when the latter is still set
> for a removed device.
>
Sure.


v2 posted here - 
https://lore.kernel.org/all/178246517230.1267.12206176311111155505.stgit@linux.ibm.com/


Thanks,

Shivaprasad

<snip/>