Re: [PATCH 7/8] powerpc/eeh: EEH for pSeries hot plug

[Alexey Kardashevskiy <aik@ozlabs.ru>]

On 20/03/2019 13:58, Sam Bobroff wrote:
> On PowerNV and pSeries, devices currently acquire EEH support from
> several different places: Boot-time devices from eeh_probe_devices()
> and eeh_addr_cache_build(), Virtual Function devices from the pcibios
> bus add device hooks and hot plugged devices from pci_hp_add_devices()
> (with other platforms using other methods as well).  Unfortunately,
> pSeries machines currently discover hot plugged devices using
> pci_rescan_bus(), not pci_hp_add_devices(), and so those devices do
> not receive EEH support.
> 
> Rather than adding another case for pci_rescan_bus(), this change
> widens the scope of the pcibios bus add device hooks so that they can
> handle all devices. As a side effect this also supports devices
> discovered after manually rescanning via /sys/bus/pci/rescan.
> 
> Note that on PowerNV, this change allows the EEH subsystem to become
> enabled after boot as long as it has not been forced off, which was
> not previously possible (it was already possible on pSeries).
> 
> Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
> ---
>  arch/powerpc/kernel/eeh.c                    |  2 +-
>  arch/powerpc/kernel/of_platform.c            |  3 +-
>  arch/powerpc/platforms/powernv/eeh-powernv.c |  8 ++-
>  arch/powerpc/platforms/pseries/eeh_pseries.c | 54 ++++++++++----------
>  4 files changed, 35 insertions(+), 32 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
> index 7a406d58d2c0..217e14bb1fb6 100644
> --- a/arch/powerpc/kernel/eeh.c
> +++ b/arch/powerpc/kernel/eeh.c
> @@ -1291,7 +1291,7 @@ void eeh_add_device_late(struct pci_dev *dev)
>  	struct pci_dn *pdn;
>  	struct eeh_dev *edev;
>  
> -	if (!dev || !eeh_enabled())
> +	if (!dev)
>  		return;
>  
>  	pr_debug("EEH: Adding device %s\n", pci_name(dev));
> diff --git a/arch/powerpc/kernel/of_platform.c b/arch/powerpc/kernel/of_platform.c
> index becaec990140..d5818e9c4069 100644
> --- a/arch/powerpc/kernel/of_platform.c
> +++ b/arch/powerpc/kernel/of_platform.c
> @@ -86,7 +86,8 @@ static int of_pci_phb_probe(struct platform_device *dev)
>  	pcibios_claim_one_bus(phb->bus);
>  
>  	/* Finish EEH setup */
> -	eeh_add_device_tree_late(phb->bus);
> +	if (!eeh_has_flag(EEH_FORCE_DISABLED))
> +		eeh_add_device_tree_late(phb->bus);
>  
>  	/* Add probed PCI devices to the device model */
>  	pci_bus_add_devices(phb->bus);
> diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
> index 51c5b6bb9b0e..81b0923cc55f 100644
> --- a/arch/powerpc/platforms/powernv/eeh-powernv.c
> +++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
> @@ -47,7 +47,7 @@ void pnv_pcibios_bus_add_device(struct pci_dev *pdev)
>  {
>  	struct pci_dn *pdn = pci_get_pdn(pdev);
>  
> -	if (!pdev->is_virtfn)
> +	if (eeh_has_flag(EEH_FORCE_DISABLED))
>  		return;
>  
>  	pr_debug("%s: EEH: Setting up device %s.\n", __func__, pci_name(pdev));
> @@ -479,7 +479,11 @@ static void *pnv_eeh_probe(struct pci_dn *pdn, void *data)
>  	 * Enable EEH explicitly so that we will do EEH check
>  	 * while accessing I/O stuff
>  	 */
> -	eeh_add_flag(EEH_ENABLED);
> +	if (!eeh_has_flag(EEH_ENABLED)) {
> +		enable_irq(eeh_event_irq);
> +		eeh_add_flag(EEH_PHB_ENABLED);


Except that I do not think we need EEH_PHB_ENABLED (commented elsewhere),

Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>




> +		eeh_add_flag(EEH_ENABLED);
> +	}
>  
>  	/* Save memory bars */
>  	eeh_save_bars(edev);
> diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c
> index ae06878fbdea..e68c79164974 100644
> --- a/arch/powerpc/platforms/pseries/eeh_pseries.c
> +++ b/arch/powerpc/platforms/pseries/eeh_pseries.c
> @@ -55,44 +55,44 @@ static int ibm_get_config_addr_info;
>  static int ibm_get_config_addr_info2;
>  static int ibm_configure_pe;
>  
> -#ifdef CONFIG_PCI_IOV
>  void pseries_pcibios_bus_add_device(struct pci_dev *pdev)
>  {
>  	struct pci_dn *pdn = pci_get_pdn(pdev);
> -	struct pci_dn *physfn_pdn;
> -	struct eeh_dev *edev;
>  
> -	if (!pdev->is_virtfn)
> +	if (eeh_has_flag(EEH_FORCE_DISABLED))
>  		return;
>  
>  	pr_debug("%s: EEH: Setting up device %s.\n", __func__, pci_name(pdev));
> +#ifdef CONFIG_PCI_IOV
> +	if (pdev->is_virtfn) {
> +		struct pci_dn *physfn_pdn;
>  
> -	pdn->device_id  =  pdev->device;
> -	pdn->vendor_id  =  pdev->vendor;
> -	pdn->class_code =  pdev->class;
> -	/*
> -	 * Last allow unfreeze return code used for retrieval
> -	 * by user space in eeh-sysfs to show the last command
> -	 * completion from platform.
> -	 */
> -	pdn->last_allow_rc =  0;
> -	physfn_pdn      =  pci_get_pdn(pdev->physfn);
> -	pdn->pe_number  =  physfn_pdn->pe_num_map[pdn->vf_index];
> -	edev = pdn_to_eeh_dev(pdn);
> -
> -	/*
> -	 * The following operations will fail if VF's sysfs files
> -	 * aren't created or its resources aren't finalized.
> -	 */
> +		pdn->device_id  =  pdev->device;
> +		pdn->vendor_id  =  pdev->vendor;
> +		pdn->class_code =  pdev->class;
> +		/*
> +		 * Last allow unfreeze return code used for retrieval
> +		 * by user space in eeh-sysfs to show the last command
> +		 * completion from platform.
> +		 */
> +		pdn->last_allow_rc =  0;
> +		physfn_pdn      =  pci_get_pdn(pdev->physfn);
> +		pdn->pe_number  =  physfn_pdn->pe_num_map[pdn->vf_index];
> +	}
> +#endif
>  	eeh_add_device_early(pdn);
>  	eeh_add_device_late(pdev);
> -	edev->pe_config_addr =  (pdn->busno << 16) | (pdn->devfn << 8);
> -	eeh_rmv_from_parent_pe(edev); /* Remove as it is adding to bus pe */
> -	eeh_add_to_parent_pe(edev);   /* Add as VF PE type */
> -	eeh_sysfs_add_device(pdev);
> +#ifdef CONFIG_PCI_IOV
> +	if (pdev->is_virtfn) {
> +		struct eeh_dev *edev = pdn_to_eeh_dev(pdn);
>  
> -}
> +		edev->pe_config_addr =  (pdn->busno << 16) | (pdn->devfn << 8);
> +		eeh_rmv_from_parent_pe(edev); /* Remove as it is adding to bus pe */
> +		eeh_add_to_parent_pe(edev);   /* Add as VF PE type */
> +	}
>  #endif
> +	eeh_sysfs_add_device(pdev);
> +}
>  
>  /*
>   * Buffer for reporting slot-error-detail rtas calls. Its here
> @@ -159,10 +159,8 @@ static int pseries_eeh_init(void)
>  	/* Set EEH probe mode */
>  	eeh_add_flag(EEH_PROBE_MODE_DEVTREE | EEH_ENABLE_IO_FOR_LOG);
>  
> -#ifdef CONFIG_PCI_IOV
>  	/* Set EEH machine dependent code */
>  	ppc_md.pcibios_bus_add_device = pseries_pcibios_bus_add_device;
> -#endif
>  
>  	return 0;
>  }
> 

-- 
Alexey