diff mbox series

phb4: Avoid MMIO load freeze escalation on every chip

Message ID 161286024513.458471.9155640961870508161.stgit@jupiter
State Under Review
Headers show
Series phb4: Avoid MMIO load freeze escalation on every chip | expand

Commit Message

Mahesh J Salgaonkar Feb. 9, 2021, 8:44 a.m. UTC
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(-)

Comments

Vasant Hegde Feb. 12, 2021, 5:44 a.m. UTC | #1
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
Mahesh J Salgaonkar Feb. 15, 2021, 5:30 a.m. UTC | #2
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
>
diff mbox series

Patch

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;