[Skiboot] [PATCH v12 09/10] skiboot: Add core IMC related counter configuration

Michael Neuling mikey at neuling.org
Thu Jun 15 13:51:27 AEST 2017


> > > +		core_id = phys_core_id % 4;
> > 
> > core_id or thread_id?  why %4?
> 
> These are more of hw procedure in setting up the core imc logic.
> We use the core id to select the pdbar scom port to program the
> address to.
> 
> +unsigned int pdbar_scom_index[] = {
> 
> > +	0x1001220B,
> > +	0x1001230B,
> > +	0x1001260B,
> > +	0x1001270B
> 
> And we need to setup all. I guess i shoud fix the "core_id" to
> pdbar_scom_idx to avoid any confusion.

Ok, thanks.  Needed some documentation.

>>>  	OPAL_IMC_COUNTERS_NEST = 1,
> > > 
> > > +	OPAL_IMC_COUNTERS_CORE = 2,
> > 
> > How do we advertise to linux that it can do CORE vs NEST calls?
> > 
> > Say we need to remove one, how does linux know which one it can call without
> > trying and getting OPAL_PARAMETER?
> > 
> > If it's based on the device tree, then the patch series the wrong way around
> > since the earlier patches start adding the device tree before the calls are
> > added.  So if you apply just patches 1-7, Linux is going to blow up with a
> > bunch
> > of OPAL_PARAMETER since it'll start seeing the device tree before the OPAL
> > calls
> > have been added.
> > 
> > Not a big issues for this series, but it does make me wonder.  If someone
> > updates the device tree in the PNOR to start adding a new section type for
> > IMC,
> > how do we tell linux that even though the device tree supports it, OPAL
> > itself
> > doesn't?
> > 
> > Say we have PHB counters in the future (just made up).  We add them to the
> > IMA
> > catalog and linux, but not skiboot.  Linux is going to OPAL_PARAMETER and
> > blow
> > up.  Linux needs to check if the device tree and skiboot have the
> > capability.
> > Not just the device tree.
> 
> Currently we do have a disable_unavailable_units() function which
> will remove the unsupported units by the microcode from the device tree.
> So even if we have add a new nest unit in the imc catalog dts file, only if
> the nest microcode supports it, it will be enabled.
> 
> But that said, disable_unavailable_units() is only for the nest units.
> If we have  new type (or class) of imc device, then OPAL will not
> catch that and we could end up in the mess.
> 
> As ben suggested, I will add another device tree loop for the incoming
> device tree to remove any unsupported or unknown type of IMC device
> in the OPAL.

Yep, perfect.  That locks down all the firmware to a base level of support so we
don't advertise something that someone doesn't actually support.

Mikey


More information about the Skiboot mailing list