On Thu, Apr 18, 2019 at 07:51:40PM +1000, Oliver O'Halloran wrote: > On Wed, 2019-03-20 at 13:58 +1100, Sam Bobroff wrote: > > The PHB flag, PNV_PHB_FLAG_EEH, is set (on PowerNV) individually on > > each PHB once the EEH subsystem is ready. It is the only use of the > > flags member of the phb struct. > > > > However there is no need to store this separately on each PHB, so > > convert it to a global flag. For symmetry, the flag is now also set > > for pSeries; although it is currently unused it may be useful in the > > future. > > I'd drop this patch and keep it as a seperate flag. Making this a > global flag doesn't really buy us anything either. It's also worth > remembering that we do have virtual PHBs, like the NVLink ones, that > don't have real EEH support and we should be doing something more > intelligent about that. > > If you are going to remove the PNV_PHB flag then i would look at moving > that flag into the generic pci_controller structure that we use for > tracking PHBs since that would give us a way to toggle EEH support on a > per PHB basis on pseries and powernv.
OK. I want to keep these changes as small as possible, so I'll try a
simpler approach and use the exsiting flag. (I only touched this area
because it's now necessary to set the EEH_ENABLED flag both at boot time
and after it.)
> > Signed-off-by: Sam Bobroff <sbobr...@linux.ibm.com>
> > ---
> > arch/powerpc/include/asm/eeh.h | 11 +++++++++++
> > arch/powerpc/platforms/powernv/eeh-powernv.c | 14 +++-----------
> > arch/powerpc/platforms/powernv/pci.c | 7 +++----
> > arch/powerpc/platforms/powernv/pci.h | 2 --
> > arch/powerpc/platforms/pseries/pci.c | 4 ++++
> > 5 files changed, 21 insertions(+), 17 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
> > index 3613a56281f2..fe4cf7208890 100644
> > --- a/arch/powerpc/include/asm/eeh.h
> > +++ b/arch/powerpc/include/asm/eeh.h
> > @@ -43,6 +43,7 @@ struct pci_dn;
> > #define EEH_VALID_PE_ZERO 0x10 /* PE#0 is valid */
> > #define EEH_ENABLE_IO_FOR_LOG 0x20 /* Enable IO for log
> > */
> > #define EEH_EARLY_DUMP_LOG 0x40 /* Dump log immediately */
> > +#define EEH_PHB_ENABLED 0x80 /* PHB recovery uses EEH
> > */
> >
> > /*
> > * Delay for PE reset, all in ms
> > @@ -245,6 +246,11 @@ static inline bool eeh_enabled(void)
> > return eeh_has_flag(EEH_ENABLED) && !eeh_has_flag(EEH_FORCE_DISABLED);
> > }
> >
> > +static inline bool eeh_phb_enabled(void)
> > +{
> > + return eeh_has_flag(EEH_PHB_ENABLED);
> > +}
> > +
> > static inline void eeh_serialize_lock(unsigned long *flags)
> > {
> > raw_spin_lock_irqsave(&confirm_error_lock, *flags);
> > @@ -332,6 +338,11 @@ static inline bool eeh_enabled(void)
> > return false;
> > }
> >
> > +static inline bool eeh_phb_enabled(void)
> > +{
> > + return false;
> > +}
> > +
> > static inline void eeh_probe_devices(void) { }
> >
> > static inline void *eeh_dev_init(struct pci_dn *pdn, void *data)
> > diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c
> > b/arch/powerpc/platforms/powernv/eeh-powernv.c
> > index 6fc1a463b796..f0a95f663810 100644
> > --- a/arch/powerpc/platforms/powernv/eeh-powernv.c
> > +++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
> > @@ -264,22 +264,14 @@ int pnv_eeh_post_init(void)
> > return ret;
> > }
> >
> > - if (!eeh_enabled())
> > + if (eeh_enabled())
> > + eeh_add_flag(EEH_PHB_ENABLED);
> > + else
> > disable_irq(eeh_event_irq);
> >
> > list_for_each_entry(hose, &hose_list, list_node) {
> > phb = hose->private_data;
> >
> > - /*
> > - * If EEH is enabled, we're going to rely on that.
> > - * Otherwise, we restore to conventional mechanism
> > - * to clear frozen PE during PCI config access.
> > - */
> > - if (eeh_enabled())
> > - phb->flags |= PNV_PHB_FLAG_EEH;
> > - else
> > - phb->flags &= ~PNV_PHB_FLAG_EEH;
> > -
> > /* Create debugfs entries */
> > #ifdef CONFIG_DEBUG_FS
> > if (phb->has_dbgfs || !phb->dbgfs)
> > diff --git a/arch/powerpc/platforms/powernv/pci.c
> > b/arch/powerpc/platforms/powernv/pci.c
> > index 307181fd8a17..d2b50f3bf6b1 100644
> > --- a/arch/powerpc/platforms/powernv/pci.c
> > +++ b/arch/powerpc/platforms/powernv/pci.c
> > @@ -717,10 +717,9 @@ int pnv_pci_cfg_write(struct pci_dn *pdn,
> > static bool pnv_pci_cfg_check(struct pci_dn *pdn)
> > {
> > struct eeh_dev *edev = NULL;
> > - struct pnv_phb *phb = pdn->phb->private_data;
> >
> > /* EEH not enabled ? */
> > - if (!(phb->flags & PNV_PHB_FLAG_EEH))
> > + if (!eeh_phb_enabled())
> > return true;
> >
> > /* PE reset or device removed ? */
> > @@ -761,7 +760,7 @@ static int pnv_pci_read_config(struct pci_bus *bus,
> >
> > ret = pnv_pci_cfg_read(pdn, where, size, val);
> > phb = pdn->phb->private_data;
> > - if (phb->flags & PNV_PHB_FLAG_EEH && pdn->edev) {
> > + if (eeh_phb_enabled() && pdn->edev) {
> > if (*val == EEH_IO_ERROR_VALUE(size) &&
> > eeh_dev_check_failure(pdn->edev))
> > return PCIBIOS_DEVICE_NOT_FOUND;
> > @@ -789,7 +788,7 @@ static int pnv_pci_write_config(struct pci_bus *bus,
> >
> > ret = pnv_pci_cfg_write(pdn, where, size, val);
> > phb = pdn->phb->private_data;
> > - if (!(phb->flags & PNV_PHB_FLAG_EEH))
> > + if (!eeh_phb_enabled())
> > pnv_pci_config_check_eeh(pdn);
> >
> > return ret;
> > diff --git a/arch/powerpc/platforms/powernv/pci.h
> > b/arch/powerpc/platforms/powernv/pci.h
> > index 8e36da379252..eb0add61397b 100644
> > --- a/arch/powerpc/platforms/powernv/pci.h
> > +++ b/arch/powerpc/platforms/powernv/pci.h
> > @@ -85,8 +85,6 @@ struct pnv_ioda_pe {
> > struct list_head list;
> > };
> >
> > -#define PNV_PHB_FLAG_EEH (1 << 0)
> > -
> > struct pnv_phb {
> > struct pci_controller *hose;
> > enum pnv_phb_type type;
> > diff --git a/arch/powerpc/platforms/pseries/pci.c
> > b/arch/powerpc/platforms/pseries/pci.c
> > index 37a77e57893e..7be80882c08d 100644
> > --- a/arch/powerpc/platforms/pseries/pci.c
> > +++ b/arch/powerpc/platforms/pseries/pci.c
> > @@ -244,6 +244,10 @@ void __init pSeries_final_fixup(void)
> >
> > eeh_probe_devices();
> > eeh_addr_cache_build();
> > +#ifdef CONFIG_EEH
> > + if (eeh_enabled())
> > + eeh_add_flag(EEH_PHB_ENABLED);
> > +#endif
> >
> > #ifdef CONFIG_PCI_IOV
> > ppc_md.pcibios_sriov_enable = pseries_pcibios_sriov_enable;
>
signature.asc
Description: PGP signature
