Re: [PATCH v1 4/7] powerpc/kernel Add EEH operations to notify resume

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 14/12/17 02:32, Bryant G. Ly wrote:
> When pseries SR-IOV is enabled and after a PF driver
> has resumed from EEH, platform has to be notified
> of the event so the child VFs can be allowed to
> resume their normal recovery path.
> 
> This patch makes the EEH operation allow unfreeze
> platform dependent code and adds the call to
> pseries EEH code.
> 
> Signed-off-by: Bryant G. Ly <bryantly@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Juan J. Alvarez <jjalvare@xxxxxxxxxxxxxxxxxx>
> ---
>  arch/powerpc/include/asm/eeh.h               |   1 +
>  arch/powerpc/kernel/eeh_driver.c             |   4 ++
>  arch/powerpc/platforms/powernv/eeh-powernv.c |   3 +-
>  arch/powerpc/platforms/pseries/eeh_pseries.c | 100 ++++++++++++++++++++++++++-
>  4 files changed, 106 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
> index 5161c37dd039..12d52a0cd447 100644
> --- a/arch/powerpc/include/asm/eeh.h
> +++ b/arch/powerpc/include/asm/eeh.h
> @@ -214,6 +214,7 @@ struct eeh_ops {
>  	int (*write_config)(struct pci_dn *pdn, int where, int size, u32 val);
>  	int (*next_error)(struct eeh_pe **pe);
>  	int (*restore_config)(struct pci_dn *pdn);
> +	int (*notify_resume)(struct pci_dn *pdn);
>  };
>  
>  extern int eeh_subsystem_flags;
> diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
> index c61bf770282b..dbda0cda559b 100644
> --- a/arch/powerpc/kernel/eeh_driver.c
> +++ b/arch/powerpc/kernel/eeh_driver.c
> @@ -361,6 +361,7 @@ static void *eeh_report_resume(void *data, void *userdata)
>  	bool was_in_error;
>  	struct pci_driver *driver;
>  	char *envp[] = {"EVENT=EEH_RESUME", "ONLINE=1", NULL};
> +	struct pci_dn *pdn = eeh_dev_to_pdn(edev);
>  
>  	if (!dev || eeh_dev_removed(edev) || eeh_pe_passed(edev->pe))
>  		return NULL;
> @@ -384,6 +385,9 @@ static void *eeh_report_resume(void *data, void *userdata)
>  	driver->err_handler->resume(dev);
>  	eeh_pcid_put(dev);
>  	kobject_uevent_env(&dev->dev.kobj, KOBJ_CHANGE, envp);
> +#ifdef CONFIG_PCI_IOV
> +	eeh_ops->notify_resume(pdn);

eeh_ops->notify_resume(eeh_dev_to_pdn(edev));

otherwise the compiler will complain at @pdn declaration if
!defined(CONFIG_PCI_IOV). Just try compiling without CONFIG_PCI_IOV.


> +#endif
>  	return NULL;
>  }
>  
> diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
> index 961e64115d92..8575b3a29e7c 100644
> --- a/arch/powerpc/platforms/powernv/eeh-powernv.c
> +++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
> @@ -1763,7 +1763,8 @@ static struct eeh_ops pnv_eeh_ops = {
>  	.read_config            = pnv_eeh_read_config,
>  	.write_config           = pnv_eeh_write_config,
>  	.next_error		= pnv_eeh_next_error,
> -	.restore_config		= pnv_eeh_restore_config
> +	.restore_config		= pnv_eeh_restore_config,
> +	.notify_resume		= NULL


.notify_resume is NULL already.


>  };
>  
>  #ifdef CONFIG_PCI_IOV
> diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c
> index 5bdd1678a9ff..2b36fbf4ce74 100644
> --- a/arch/powerpc/platforms/pseries/eeh_pseries.c
> +++ b/arch/powerpc/platforms/pseries/eeh_pseries.c
> @@ -798,6 +798,103 @@ static int pseries_eeh_restore_config(struct pci_dn *pdn)
>  	return 0;
>  }
>  
> +#ifdef CONFIG_PCI_IOV
> +int pseries_send_allow_unfreeze(struct eeh_pe *pe,
> +				u16 *vf_pe_array, int cur_vfs)
> +{
> +	int  rc, config_addr;
> +	int ibm_allow_unfreeze = rtas_token("ibm,open-sriov-allow-unfreeze");
> +
> +	config_addr = pe->config_addr;

@config_addr is pointless - it is used just once, even pr_warn few lines
below uses pe->config_addr.



> +	spin_lock(&rtas_data_buf_lock);
> +	memcpy(rtas_data_buf, vf_pe_array, RTAS_DATA_BUF_SIZE);
> +	rc = rtas_call(ibm_allow_unfreeze, 5, 1, NULL,
> +		       config_addr,
> +		       BUID_HI(pe->phb->buid),
> +		       BUID_LO(pe->phb->buid),
> +		       rtas_data_buf, cur_vfs * sizeof(u16));
> +	spin_unlock(&rtas_data_buf_lock);
> +	if (rc)
> +		pr_warn("%s: Failed to allow unfreeze for PHB#%x-PE#%x, rc=%x\n",
> +			__func__,
> +			pe->phb->global_number,
> +			pe->config_addr, rc);
> +	return rc;
> +}
> +
> +static int pseries_call_allow_unfreeze(struct eeh_dev *edev)
> +{
> +	struct eeh_pe *pe;
> +	struct pci_dn *pdn, *tmp, *parent, *physfn_pdn;
> +	int cur_vfs, rc, vf_index;
> +	u16 *vf_pe_array;
> +
> +	vf_pe_array = kzalloc(RTAS_DATA_BUF_SIZE, GFP_KERNEL);
> +	if (!vf_pe_array)
> +		return -ENOMEM;
> +
> +	memset(vf_pe_array, 0, RTAS_DATA_BUF_SIZE);


"z" from kzalloc says it reset the memory.


> +	cur_vfs = 0;
> +	rc = 0;
> +	if (edev->pdev->is_physfn) {
> +		pe = eeh_dev_to_pe(edev);
> +		cur_vfs = pci_num_vf(edev->pdev);
> +		pdn = eeh_dev_to_pdn(edev);
> +		parent  = pdn->parent;
> +		/* For each of its VF
> +		 * call allow unfreeze
> +		 */
> +		for (vf_index = 0; vf_index < cur_vfs; vf_index++)
> +			vf_pe_array[vf_index] =
> +				be16_to_cpu(pdn->pe_num_map[vf_index]);

It is kind of assumed that the number of VFs is always less than 2048 minus
rtas call parameters (RTAS_DATA_BUF_SIZE==4096 now)?

Is the upper limit of cur_vfs checked anywhere?

We can afford 4K on stack if we know for sure this is all we n


> +
> +		rc = pseries_send_allow_unfreeze(pe, vf_pe_array, cur_vfs);
> +		pdn->last_allow_rc = rc;


Is this PF or VF pdn (I assume PF)?


> +		for (vf_index = 0; vf_index < cur_vfs; vf_index++) {
> +			list_for_each_entry_safe(pdn, tmp, &parent->child_list,
> +						 list) {
> +				if (pdn->busno
> +				    != pci_iov_virtfn_bus(edev->pdev,
> +							  vf_index) ||
> +				    pdn->devfn
> +				    != pci_iov_virtfn_devfn(edev->pdev,
> +							    vf_index))
> +					continue;
> +				pdn->last_allow_rc = rc;


I am missing the point of copying last_allow_rc - cannot
eeh_notify_resume_show() just return it from the PF?

May be just add another flag to eeh_pe::state for last_allow_rc? How many
different @rc do we expect here?


> +			}
> +		}
> +	} else {
> +		pdn = pci_get_pdn(edev->pdev);
> +		vf_pe_array[0] = be16_to_cpu(pdn->pe_number);
> +		physfn_pdn      =  pci_get_pdn(edev->physfn);

Way too many spaces, why? :)


> +		edev = pdn_to_eeh_dev(physfn_pdn);
> +		pe = eeh_dev_to_pe(edev);
> +		rc = pseries_send_allow_unfreeze(pe, vf_pe_array, 1);
> +		pdn->last_allow_rc = rc;
> +	}
> +
> +	kfree(vf_pe_array);
> +	return rc;
> +}
> +
> +static int pseries_notify_resume(struct pci_dn *pdn)
> +{
> +	struct eeh_dev *edev = pdn_to_eeh_dev(pdn);
> +
> +	if (!edev)
> +		return -EEXIST;
> +
> +	if (rtas_token("ibm,open-sriov-allow-unfreeze")
> +	    == RTAS_UNKNOWN_SERVICE)
> +		return -EINVAL;

Is this the only possible error code?


> +
> +	if (edev->pdev->is_physfn  || edev->pdev->is_virtfn)
> +		return pseries_call_allow_unfreeze(edev);
> +
> +	return 0;
> +}
> +#endif
> +
>  static struct eeh_ops pseries_eeh_ops = {
>  	.name			= "pseries",
>  	.init			= pseries_eeh_init,
> @@ -813,7 +910,8 @@ static struct eeh_ops pseries_eeh_ops = {
>  	.read_config		= pseries_eeh_read_config,
>  	.write_config		= pseries_eeh_write_config,
>  	.next_error		= NULL,
> -	.restore_config		= pseries_eeh_restore_config
> +	.restore_config		= pseries_eeh_restore_config,
> +	.notify_resume		= pseries_notify_resume
>  };
>  
>  /**
> 


-- 
Alexey



[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux