[PATCH v2 5/5] PCI/powerpc/eeh: Add pcibios hooks for preparing to rescan

Sergey Miroshnichenko s.miroshnichenko at yadro.com
Tue Sep 11 02:00:52 AEST 2018


Hello Sam,

On 9/10/18 8:03 AM, Sam Bobroff wrote:
> Hi Sergey,
> 
> On Thu, Sep 06, 2018 at 02:57:52PM +0300, Sergey Miroshnichenko wrote:
>> Reading an empty slot returns all ones, which triggers a false
>> EEH error event on PowerNV.
>>
>> New callbacks pcibios_rescan_prepare/done are introduced to
>> pause/resume the EEH during rescan.
> 
> If I understand it correctly, this temporarily disables EEH for config space
> accesses on the whole PHB while the rescan runs. Is it possible that a
> real EEH event could be missed if it occurred during the rescan?
> 
> Even if it's not possible, I think it would be good to mention that in a
> comment.

Yes, missing a real EEH event is possible, unfortunately, and it is
indeed worth mentioning.

To reduce this probability the next patchset I'll post in a few days
among other things puts all the affected device drivers to pause during
rescan, mainly because of moving BARs and bridge windows, but it will
also help here a bit.

> 
>> Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko at yadro.com>
>> ---
>>  arch/powerpc/include/asm/eeh.h               |  2 ++
>>  arch/powerpc/kernel/eeh.c                    | 12 +++++++++++
>>  arch/powerpc/platforms/powernv/eeh-powernv.c | 22 ++++++++++++++++++++
>>  drivers/pci/probe.c                          | 14 +++++++++++++
>>  include/linux/pci.h                          |  2 ++
>>  5 files changed, 52 insertions(+)
>>
>> diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
>> index 219637ea69a1..926c3e31df99 100644
>> --- a/arch/powerpc/include/asm/eeh.h
>> +++ b/arch/powerpc/include/asm/eeh.h
>> @@ -219,6 +219,8 @@ struct eeh_ops {
>>  	int (*next_error)(struct eeh_pe **pe);
>>  	int (*restore_config)(struct pci_dn *pdn);
>>  	int (*notify_resume)(struct pci_dn *pdn);
>> +	int (*pause)(struct pci_bus *bus);
>> +	int (*resume)(struct pci_bus *bus);
> 
> I think these names are a bit too generic, what about naming them
> pause_bus()/resume_bus() or even prepare_rescan()/rescan_done()?
> 

Thanks! I will rename them to rescan_prepare/rescan_done to make friends
with reset_prepare/reset_done from struct pci_error_handlers.

>>  };
>>  
>>  extern int eeh_subsystem_flags;
>> diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
>> index 6ebba3e48b01..9fb5012f389d 100644
>> --- a/arch/powerpc/kernel/eeh.c
>> +++ b/arch/powerpc/kernel/eeh.c
>> @@ -1831,3 +1831,15 @@ static int __init eeh_init_proc(void)
>>  	return 0;
>>  }
>>  __initcall(eeh_init_proc);
>> +
>> +void pcibios_rescan_prepare(struct pci_bus *bus)
>> +{
>> +	if (eeh_ops && eeh_ops->pause)
>> +		eeh_ops->pause(bus);
>> +}
>> +
>> +void pcibios_rescan_done(struct pci_bus *bus)
>> +{
>> +	if (eeh_ops && eeh_ops->resume)
>> +		eeh_ops->resume(bus);
>> +}
>> diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
>> index 3c1beae29f2d..9724a58afcd2 100644
>> --- a/arch/powerpc/platforms/powernv/eeh-powernv.c
>> +++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
>> @@ -59,6 +59,26 @@ void pnv_pcibios_bus_add_device(struct pci_dev *pdev)
>>  	eeh_sysfs_add_device(pdev);
>>  }
>>  
>> +static int pnv_eeh_pause(struct pci_bus *bus)
>> +{
>> +	struct pci_controller *hose = pci_bus_to_host(bus);
>> +	struct pnv_phb *phb = hose->private_data;
>> +
>> +	phb->flags &= ~PNV_PHB_FLAG_EEH;
>> +	disable_irq(eeh_event_irq);
>> +	return 0;
>> +}
>> +
>> +static int pnv_eeh_resume(struct pci_bus *bus)
>> +{
>> +	struct pci_controller *hose = pci_bus_to_host(bus);
>> +	struct pnv_phb *phb = hose->private_data;
>> +
>> +	enable_irq(eeh_event_irq);
>> +	phb->flags |= PNV_PHB_FLAG_EEH;
>> +	return 0;
>> +}
>> +
>>  static int pnv_eeh_init(void)
>>  {
>>  	struct pci_controller *hose;
>> @@ -1710,6 +1730,8 @@ static struct eeh_ops pnv_eeh_ops = {
>>  	.write_config           = pnv_eeh_write_config,
>>  	.next_error		= pnv_eeh_next_error,
>>  	.restore_config		= pnv_eeh_restore_config,
>> +	.pause			= pnv_eeh_pause,
>> +	.resume			= pnv_eeh_resume,
>>  	.notify_resume		= NULL
>>  };
>>  
>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>> index ac876e32de4b..4a9045364809 100644
>> --- a/drivers/pci/probe.c
>> +++ b/drivers/pci/probe.c
>> @@ -2801,6 +2801,14 @@ void __weak pcibios_remove_bus(struct pci_bus *bus)
>>  {
>>  }
>>  
>> +void __weak pcibios_rescan_prepare(struct pci_bus *bus)
>> +{
>> +}
>> +
>> +void __weak pcibios_rescan_done(struct pci_bus *bus)
>> +{
>> +}
>> +
>>  struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
>>  		struct pci_ops *ops, void *sysdata, struct list_head *resources)
>>  {
>> @@ -3055,9 +3063,15 @@ unsigned int pci_rescan_bus_bridge_resize(struct pci_dev *bridge)
>>  unsigned int pci_rescan_bus(struct pci_bus *bus)
>>  {
>>  	unsigned int max;
>> +	struct pci_bus *root = bus;
>> +
>> +	while (!pci_is_root_bus(root))
>> +		root = root->parent;
>>  
>> +	pcibios_rescan_prepare(root);
>>  	max = pci_scan_child_bus(bus);
>>  	pci_assign_unassigned_bus_resources(bus);
>> +	pcibios_rescan_done(root);
>>  	pci_bus_add_devices(bus);
>>  
>>  	return max;
>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>> index 340029b2fb38..42930731c5a7 100644
>> --- a/include/linux/pci.h
>> +++ b/include/linux/pci.h
>> @@ -1929,6 +1929,8 @@ void pcibios_penalize_isa_irq(int irq, int active);
>>  int pcibios_alloc_irq(struct pci_dev *dev);
>>  void pcibios_free_irq(struct pci_dev *dev);
>>  resource_size_t pcibios_default_alignment(void);
>> +void pcibios_rescan_prepare(struct pci_bus *bus);
>> +void pcibios_rescan_done(struct pci_bus *bus);
>>  
>>  #ifdef CONFIG_HIBERNATE_CALLBACKS
>>  extern struct dev_pm_ops pcibios_pm_ops;
>> -- 
>> 2.17.1
>>

Best regards,
Serge

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <http://lists.ozlabs.org/pipermail/linuxppc-dev/attachments/20180910/18f13157/attachment.sig>


More information about the Linuxppc-dev mailing list