Re: [PATCH 3/6] powerpc/eeh: Do early EEH init only when required

[Sam Bobroff <sbobroff@linux.ibm.com>]

[-- Attachment #1: Type: text/plain, Size: 3515 bytes --]

On Mon, Feb 03, 2020 at 07:35:18PM +1100, Oliver O'Halloran wrote:
> The pci hotplug helper (pci_hp_add_devices()) calls
> eeh_add_device_tree_early() to scan the device-tree for new PCI devices and
> do the early EEH probe before the device is scanned. This early probe is a
> no-op in a lot of cases because:
> 
> a) The early init is only required to satisfy a PAPR requirement that EEH
>    be configured before we start doing config accesses. On PowerNV it is
>    a no-op.
> 
> b) It's a no-op for devices that have already had their eeh_dev
>    initialised.
> 
> There are four callers of pci_hp_add_devices():
> 
> 1. arch/powerpc/kernel/eeh_driver.c
> 	Here the hotplug helper is called when re-scanning pci_devs that
> 	were removed during an EEH recovery pass. The EEH stat for each
> 	removed device (the eeh_dev) is retained across a recovery pass
> 	so the early init is a no-op in this case.
> 
> 2. drivers/pci/hotplug/pnv_php.c
> 	This is also a no-op since the PowerNV hotplug driver is, suprisingly,
> 	PowerNV specific.
> 
> 3. drivers/pci/hotplug/rpaphp_core.c
> 4. drivers/pci/hotplug/rpaphp_pci.c
> 	In these two cases new devices have been hotplugged and FW has
> 	provided new DT nodes for each. These are the only two cases where
> 	the EEH we might have new PCI device nodes in the DT so these are
> 	the only two cases where the early EEH probe needs to be done.
> 
> We can move the calls to eeh_add_device_tree_early() to the locations where
> it's needed and remove it from the generic path. This is preparation for
> making the early EEH probe pseries specific.
> 
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>

Makes sense to me.
Reviewed-by: Sam Bobroff <sbobroff@linux.ibm.com>

> ---
>  arch/powerpc/kernel/pci-hotplug.c | 2 --
>  drivers/pci/hotplug/rpaphp_core.c | 2 ++
>  drivers/pci/hotplug/rpaphp_pci.c  | 4 +++-
>  3 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/pci-hotplug.c b/arch/powerpc/kernel/pci-hotplug.c
> index d6a67f8..bf83f76 100644
> --- a/arch/powerpc/kernel/pci-hotplug.c
> +++ b/arch/powerpc/kernel/pci-hotplug.c
> @@ -112,8 +112,6 @@ void pci_hp_add_devices(struct pci_bus *bus)
>  	struct pci_controller *phb;
>  	struct device_node *dn = pci_bus_to_OF_node(bus);
>  
> -	eeh_add_device_tree_early(PCI_DN(dn));
> -
>  	phb = pci_bus_to_host(bus);
>  
>  	mode = PCI_PROBE_NORMAL;
> diff --git a/drivers/pci/hotplug/rpaphp_core.c b/drivers/pci/hotplug/rpaphp_core.c
> index e408e40..9c1e43e 100644
> --- a/drivers/pci/hotplug/rpaphp_core.c
> +++ b/drivers/pci/hotplug/rpaphp_core.c
> @@ -494,6 +494,8 @@ static int enable_slot(struct hotplug_slot *hotplug_slot)
>  		return retval;
>  
>  	if (state == PRESENT) {
> +		eeh_add_device_tree_early(PCI_DN(slot->dn));
> +
>  		pci_lock_rescan_remove();
>  		pci_hp_add_devices(slot->bus);
>  		pci_unlock_rescan_remove();
> diff --git a/drivers/pci/hotplug/rpaphp_pci.c b/drivers/pci/hotplug/rpaphp_pci.c
> index beca61b..61ebbd8 100644
> --- a/drivers/pci/hotplug/rpaphp_pci.c
> +++ b/drivers/pci/hotplug/rpaphp_pci.c
> @@ -95,8 +95,10 @@ int rpaphp_enable_slot(struct slot *slot)
>  			return -EINVAL;
>  		}
>  
> -		if (list_empty(&bus->devices))
> +		if (list_empty(&bus->devices)) {
> +			eeh_add_device_tree_early(PCI_DN(slot->dn));
>  			pci_hp_add_devices(bus);
> +		}
>  
>  		if (!list_empty(&bus->devices)) {
>  			slot->state = CONFIGURED;
> -- 
> 2.9.5
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]