phb4: Add an option for disabling EEH MMIO in nvram

Submitted by Russell Currey on April 20, 2017, 12:55 a.m.

Details

Message ID 20170420005523.15000-1-ruscur@russell.cc
State New
Headers show

Commit Message

Russell Currey April 20, 2017, 12:55 a.m.
Having the option to disable EEH for MMIO without rebuilding skiboot
could be useful for testing, so check for disable-eeh-mmio=true in nvram.

Signed-off-by: Russell Currey <ruscur@russell.cc>
---
 hw/phb4.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

Comments

Gavin Shan April 20, 2017, 2:09 a.m.
On Thu, Apr 20, 2017 at 10:55:23AM +1000, Russell Currey wrote:
>Having the option to disable EEH for MMIO without rebuilding skiboot
>could be useful for testing, so check for disable-eeh-mmio=true in nvram.
>

Russell, I'm not sure if the parameter name has been finalized or not.
If not, it deserves a better name, for example "pci-eeh-disable-mmio",
use the prefix to identify the subsystem, for which the parameter is.

Other than that:

Reviewed-by: Gavin Shan <gwshan@linux.vnet.ibm.com>

>Signed-off-by: Russell Currey <ruscur@russell.cc>
>---
> hw/phb4.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
>diff --git a/hw/phb4.c b/hw/phb4.c
>index e9c59c52..59fd6407 100644
>--- a/hw/phb4.c
>+++ b/hw/phb4.c
>@@ -53,6 +53,7 @@
> #include <chip.h>
> #include <chiptod.h>
> #include <xive.h>
>+#include <nvram.h>
> 
> /* Enable this to disable error interrupts for debug purposes */
> #define DISABLE_ERR_INTS
>@@ -2841,9 +2842,13 @@ static void phb4_init_hw(struct phb4 *p, bool first_init)
> 	out_be64(p->regs + PHB_PCIE_CRESET,			   creset);
> 
> 	/* Init_16 - PHB Control */
>-	out_be64(p->regs + PHB_CTRLR,
>-		 PHB_CTRLR_IRQ_PGSZ_64K |
>-		 SETFIELD(PHB_CTRLR_TVT_ADDR_SEL, 0ull, TVT_2_PER_PE));
>+	val = PHB_CTRLR_IRQ_PGSZ_64K |
>+		SETFIELD(PHB_CTRLR_TVT_ADDR_SEL, 0ull, TVT_2_PER_PE);
>+
>+	if (nvram_query_eq("disable-eeh-mmio", "true"))
>+		val |= PHB_CTRLR_MMIO_EEH_DISABLE;
>+
>+	out_be64(p->regs + PHB_CTRLR, val);
> 
> 	/* Init_17..40 - Architected IODA3 inits */
> 	phb4_init_ioda3(p);
>-- 
>2.12.2
>
>_______________________________________________
>Skiboot mailing list
>Skiboot@lists.ozlabs.org
>https://lists.ozlabs.org/listinfo/skiboot
Russell Currey April 20, 2017, 3:19 a.m.
On Thu, 2017-04-20 at 12:09 +1000, Gavin Shan wrote:
> On Thu, Apr 20, 2017 at 10:55:23AM +1000, Russell Currey wrote:
> > Having the option to disable EEH for MMIO without rebuilding skiboot
> > could be useful for testing, so check for disable-eeh-mmio=true in nvram.
> > 
> 
> Russell, I'm not sure if the parameter name has been finalized or not.
> If not, it deserves a better name, for example "pci-eeh-disable-mmio",
> use the prefix to identify the subsystem, for which the parameter is.

Nothing finalised as yet, will change it next revision.  Cheers.

> 
> Other than that:
> 
> Reviewed-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
> 
> > Signed-off-by: Russell Currey <ruscur@russell.cc>
> > ---
> > hw/phb4.c | 11 ++++++++---
> > 1 file changed, 8 insertions(+), 3 deletions(-)
> > 
> > diff --git a/hw/phb4.c b/hw/phb4.c
> > index e9c59c52..59fd6407 100644
> > --- a/hw/phb4.c
> > +++ b/hw/phb4.c
> > @@ -53,6 +53,7 @@
> > #include <chip.h>
> > #include <chiptod.h>
> > #include <xive.h>
> > +#include <nvram.h>
> > 
> > /* Enable this to disable error interrupts for debug purposes */
> > #define DISABLE_ERR_INTS
> > @@ -2841,9 +2842,13 @@ static void phb4_init_hw(struct phb4 *p, bool
> > first_init)
> > 	out_be64(p->regs + PHB_PCIE_CRESET,			   creset);
> > 
> > 	/* Init_16 - PHB Control */
> > -	out_be64(p->regs + PHB_CTRLR,
> > -		 PHB_CTRLR_IRQ_PGSZ_64K |
> > -		 SETFIELD(PHB_CTRLR_TVT_ADDR_SEL, 0ull, TVT_2_PER_PE));
> > +	val = PHB_CTRLR_IRQ_PGSZ_64K |
> > +		SETFIELD(PHB_CTRLR_TVT_ADDR_SEL, 0ull, TVT_2_PER_PE);
> > +
> > +	if (nvram_query_eq("disable-eeh-mmio", "true"))
> > +		val |= PHB_CTRLR_MMIO_EEH_DISABLE;
> > +
> > +	out_be64(p->regs + PHB_CTRLR, val);
> > 
> > 	/* Init_17..40 - Architected IODA3 inits */
> > 	phb4_init_ioda3(p);
> > -- 
> > 2.12.2
> > 
> > _______________________________________________
> > Skiboot mailing list
> > Skiboot@lists.ozlabs.org
> > https://lists.ozlabs.org/listinfo/skiboot
> 
>
Michael Neuling April 20, 2017, 4:12 a.m.
On Thu, 2017-04-20 at 10:55 +1000, Russell Currey wrote:
> Having the option to disable EEH for MMIO without rebuilding skiboot
> could be useful for testing, so check for disable-eeh-mmio=true in nvram.

Can we make this option 'eeh-mmio=disabled'  ?

#bikeshedding!

Mikey
Russell Currey April 20, 2017, 4:17 a.m.
On Thu, 2017-04-20 at 14:12 +1000, Michael Neuling wrote:
> On Thu, 2017-04-20 at 10:55 +1000, Russell Currey wrote:
> > Having the option to disable EEH for MMIO without rebuilding skiboot
> > could be useful for testing, so check for disable-eeh-mmio=true in nvram.
> 
> Can we make this option 'eeh-mmio=disabled'  ?

I don't care at all.  Gavin thinks it should at least mention PCI.  Ultimately
it's not something the vast majority of people will care about, so it's not all
that important.

This isn't the slightest bit urgent so maybe we should wait and give Stewart
final bikeshedding rights...

> 
> #bikeshedding!
> 
> Mikey
Michael Neuling April 20, 2017, 5:42 a.m.
On Thu, 2017-04-20 at 14:17 +1000, Russell Currey wrote:
> On Thu, 2017-04-20 at 14:12 +1000, Michael Neuling wrote:
> > On Thu, 2017-04-20 at 10:55 +1000, Russell Currey wrote:
> > > Having the option to disable EEH for MMIO without rebuilding skiboot
> > > could be useful for testing, so check for disable-eeh-mmio=true in nvram.
> > 
> > Can we make this option 'eeh-mmio=disabled'  ?
> 
> I don't care at all.  Gavin thinks it should at least mention PCI.  Ultimately
> it's not something the vast majority of people will care about, so it's not
> all
> that important.

yeah agreed, it should have pci in it.

> This isn't the slightest bit urgent so maybe we should wait and give Stewart
> final bikeshedding rights...

OK.

Mikey

Patch hide | download patch | download mbox

diff --git a/hw/phb4.c b/hw/phb4.c
index e9c59c52..59fd6407 100644
--- a/hw/phb4.c
+++ b/hw/phb4.c
@@ -53,6 +53,7 @@ 
 #include <chip.h>
 #include <chiptod.h>
 #include <xive.h>
+#include <nvram.h>
 
 /* Enable this to disable error interrupts for debug purposes */
 #define DISABLE_ERR_INTS
@@ -2841,9 +2842,13 @@  static void phb4_init_hw(struct phb4 *p, bool first_init)
 	out_be64(p->regs + PHB_PCIE_CRESET,			   creset);
 
 	/* Init_16 - PHB Control */
-	out_be64(p->regs + PHB_CTRLR,
-		 PHB_CTRLR_IRQ_PGSZ_64K |
-		 SETFIELD(PHB_CTRLR_TVT_ADDR_SEL, 0ull, TVT_2_PER_PE));
+	val = PHB_CTRLR_IRQ_PGSZ_64K |
+		SETFIELD(PHB_CTRLR_TVT_ADDR_SEL, 0ull, TVT_2_PER_PE);
+
+	if (nvram_query_eq("disable-eeh-mmio", "true"))
+		val |= PHB_CTRLR_MMIO_EEH_DISABLE;
+
+	out_be64(p->regs + PHB_CTRLR, val);
 
 	/* Init_17..40 - Architected IODA3 inits */
 	phb4_init_ioda3(p);