[Skiboot] [RFC PATCH 4/4]ibm-fsp/firenze: Nest Intrumentation code

Madhavan Srinivasan maddy at linux.vnet.ibm.com
Thu Apr 23 12:59:47 AEST 2015



On Wednesday 22 April 2015 07:32 AM, Stewart Smith wrote:
> Madhavan Srinivasan <maddy at linux.vnet.ibm.com> writes:
>> +/*
>> + * OPAL call to start and stop PTS.
>> + */
> This comment is really not needed, it adds no extra information than the
> opal_call declaration below.
Ok.
>
>> +static int64_t opal_uncore_control(uint32_t value)
>
>
>> +{
>> +	struct proc_chip *chip;
>> +	int rc = IMA_PTS_ERROR;
>> +
>> +	chip = get_chip(pir_to_chip_id(this_cpu()->pir));
>> +
>> +	if (value)
>> +		rc = pore_slw_ima_scom(chip->id, IMA_PTS_START);
>> +	 else
>> +		rc = pore_slw_ima_scom(chip->id, IMA_PTS_STOP);
>> +
>> +	return rc;
>> +}
>> +opal_call (OPAL_UNCORE_CONTROL, opal_uncore_control, 1);
> You will need to add documentation to doc/opal-api/
>
> It may be better to define the options as:
> 1 = enable
> 0 = disable
> any other value = OPAL_PARAMETER returned.
>
> Why is chip being this_cpu better than passing in what chip we want to
> send it to? (Not saying it's better or worse, but why that API?)
If we need to pass it, then we need to figure that out in kernel
(may be using  topology_physical_package_id ), also currently
did not find any opal call sending chip id.

But this question scares me, is there a chance of kernel thread
which does opal call from a chip, can end up in a different
chip threads in opal side?

Thanks for review.
Maddy



More information about the Skiboot mailing list