| Message ID | 161286024513.458471.9155640961870508161.stgit@jupiter |
|---|---|
| State | Accepted |
| Headers | show |
| Series | phb4: Avoid MMIO load freeze escalation on every chip | expand |
On 2/9/21 2:14 PM, Mahesh Salgaonkar wrote: > The commit f397cc30bdf8 ("phb4: Only escalate freezes on MMIO load where > necessary") introduced a change to restrict escalation to the chips that > actually need it. However it missed one case which still causes the > escalation on every chip. This affects EEH recovery to cause full > PHB reset on some chips which is not necessary. This patch fixes that. > Also, add a check for p9 chip in phb4_escalation_required() function. > > Cc: skiboot-stable@lists.ozlabs.org > Signed-off-by: Mahesh Salgaonkar <mahesh@linux.ibm.com> > --- > hw/phb4.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/hw/phb4.c b/hw/phb4.c > index e7758d346b..edbcdb2179 100644 > --- a/hw/phb4.c > +++ b/hw/phb4.c > @@ -3590,6 +3590,10 @@ static bool phb4_escalation_required(void) > { > uint64_t pvr = mfspr(SPR_PVR); > > + /* Only on Power9 */ > + if (proc_gen != proc_gen_p9) > + return false; > + Do we really need this check? Below we have chip specific checks. -Vasant
On 2021-02-12 11:14:42 Fri, Vasant Hegde wrote: > On 2/9/21 2:14 PM, Mahesh Salgaonkar wrote: > > The commit f397cc30bdf8 ("phb4: Only escalate freezes on MMIO load where > > necessary") introduced a change to restrict escalation to the chips that > > actually need it. However it missed one case which still causes the > > escalation on every chip. This affects EEH recovery to cause full > > PHB reset on some chips which is not necessary. This patch fixes that. > > Also, add a check for p9 chip in phb4_escalation_required() function. > > > > Cc: skiboot-stable@lists.ozlabs.org > > Signed-off-by: Mahesh Salgaonkar <mahesh@linux.ibm.com> > > --- > > hw/phb4.c | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/hw/phb4.c b/hw/phb4.c > > index e7758d346b..edbcdb2179 100644 > > --- a/hw/phb4.c > > +++ b/hw/phb4.c > > @@ -3590,6 +3590,10 @@ static bool phb4_escalation_required(void) > > { > > uint64_t pvr = mfspr(SPR_PVR); > > > > + /* Only on Power9 */ > > + if (proc_gen != proc_gen_p9) > > + return false; > > + > > Do we really need this check? Below we have chip specific checks. Yes. The escalation workaround introduced was very specific to few chips in power9 generation. The below checks takes decision based on bit 50 in PVR register not based on proc generation. Adding this check makes sure that the escalation happens only for sepcific Power9 chips. Thanks, -Mahesh. > > -Vasant >
On 2/9/21 2:14 PM, Mahesh Salgaonkar wrote: > The commit f397cc30bdf8 ("phb4: Only escalate freezes on MMIO load where > necessary") introduced a change to restrict escalation to the chips that > actually need it. However it missed one case which still causes the > escalation on every chip. This affects EEH recovery to cause full > PHB reset on some chips which is not necessary. This patch fixes that. > Also, add a check for p9 chip in phb4_escalation_required() function. > Thanks! Merged to master as d51eb6f9. -Vasant
diff --git a/hw/phb4.c b/hw/phb4.c index e7758d346b..edbcdb2179 100644 --- a/hw/phb4.c +++ b/hw/phb4.c @@ -3590,6 +3590,10 @@ static bool phb4_escalation_required(void) { uint64_t pvr = mfspr(SPR_PVR); + /* Only on Power9 */ + if (proc_gen != proc_gen_p9) + return false; + /* * Escalation is required on the following chip versions: * - Cumulus DD1.0 @@ -3850,7 +3854,7 @@ static int64_t phb4_eeh_next_error(struct phb *phb, if (*first_frozen_pe != (uint64_t)(-1)) { pesta = phb4_get_pesta(p, *first_frozen_pe); - if (phb4_freeze_escalate(pesta)) { + if (phb4_escalation_required() && phb4_freeze_escalate(pesta)) { PHBINF(p, "Escalating freeze to fence. PESTA[%lli]=%016llx\n", *first_frozen_pe, pesta); p->err.err_class = PHB4_ERR_CLASS_FENCED;
The commit f397cc30bdf8 ("phb4: Only escalate freezes on MMIO load where necessary") introduced a change to restrict escalation to the chips that actually need it. However it missed one case which still causes the escalation on every chip. This affects EEH recovery to cause full PHB reset on some chips which is not necessary. This patch fixes that. Also, add a check for p9 chip in phb4_escalation_required() function. Cc: skiboot-stable@lists.ozlabs.org Signed-off-by: Mahesh Salgaonkar <mahesh@linux.ibm.com> --- hw/phb4.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)