[Skiboot] [PATCH] hw/imc: pause imc ucode at boot

Madhavan Srinivasan maddy at linux.vnet.ibm.com
Tue Oct 10 22:45:08 AEDT 2017



On Tuesday 10 October 2017 08:05 AM, Stewart Smith wrote:
> Madhavan Srinivasan<maddy at linux.vnet.ibm.com>  writes:
>> IMC nest counters has both in-band (ucode) and out of
>> band access to it. Since not all nest counter configurations
>> are supported by ucode, out of band tools are used to
>> characterize other configuration.
>>
>> So it is prefer to pause the nest microcode at boot to aid the
>> out of band tools. If the ucode not paused and OS does not
>> have IMC driver support, then out to band tools will race with
>> ucode and end up getting undesirable values. Patch to check and
>> pause the ucode at boot.
> Please also mention how this works with the OPAL API as the API is that
> OPAL_IMC_COUNTERS_INIT and START must be called for any counters that the OS
> wants to use.

Yes sure. Will do it.

> So, really, we were allowing buggy OSs to work as we were starting with
> some counters running.

Yeah, my bad. But OS will not have an impact ssince ucode
update the counter data to reserve memory (HOMER).

>> Signed-off-by: Madhavan Srinivasan<maddy at linux.vnet.ibm.com>
>> ---
>> Currently only Linux driver support In-Memory Collection counter
>> hardware, which do pause the nest imc counters during driver
>> initialisation. So this change should not have any impact.
>>
>>   hw/imc.c | 24 ++++++++++++++++++++++++
>>   1 file changed, 24 insertions(+)
>>
>> diff --git a/hw/imc.c b/hw/imc.c
>> index 10505f69d4bd..6f92c60d285d 100644
>> --- a/hw/imc.c
>> +++ b/hw/imc.c
>> @@ -128,6 +128,18 @@ static bool is_nest_mem_initialized(struct imc_chip_cb *ptr)
>>   	return true;
>>   }
>>
>> +static void pause_microcode_at_boot(void)
>> +{
>> +	struct proc_chip *chip;
>> +	struct imc_chip_cb *cb;
>> +
>> +	for_each_chip(chip) {
>> +		cb = (struct imc_chip_cb *)(chip->homer_base + P9_CB_STRUCT_OFFSET);
>> +		if (is_nest_mem_initialized(cb))
>> +			cb->imc_chip_command =  cpu_to_be64(NEST_IMC_DISABLE);
>> +	}
>> +}
> get_imc_cb() does all of this logic.

Yes thats true. Will rewrite it.

> The command here doesn't have the mambo quirk that
> opal_imc_counters_stop() has.

Thats yes. Incase of mambo, the function will be skip-ed at the caller.

Thanks for the review
Maddy
>> +
>>   /*
>>    * A Quad contains 4 cores in Power 9, and there are 4 addresses for
>>    * the Core Hardware Trace Macro (CHTM) attached to each core.
>> @@ -543,6 +555,18 @@ imc_mambo:
>>   		return;
>>
>>   	/*
>> +	 * IMC nest counters has both in-band (ucode access) and out of band
>> +	 * access to it. Since not all nest counter configurations are supported
>> +	 * by ucode, out of band tools are used to characterize other
>> +	 * configuration.
>> +	 *
>> +	 * If the ucode not paused and OS does not have IMC driver support,
>> +	 * then out to band tools will race with ucode and end up getting
>> +	 * undesirable values. Hence pause the ucode if it is already running.
>> +	 */
>> +	pause_microcode_at_boot();
>> +
>> +	/*
>>   	 * If the dt_attach_root() fails, "imc-counters" node will not be
>>   	 * seen in the device-tree and hence OS should not make any
>>   	 * OPAL_IMC_* calls.
>> -- 
>> 2.7.4
>>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ozlabs.org/pipermail/skiboot/attachments/20171010/c0477c06/attachment.html>


More information about the Skiboot mailing list