[Skiboot] [PATCH v8 07/10] skiboot: Add opal call to enable/disable Nest IMC

[Madhavan Srinivasan]

On Tuesday 25 April 2017 06:14 PM, Michael Ellerman wrote:
> Madhavan Srinivasan <maddy at linux.vnet.ibm.com> writes:
>
>> diff --git a/hw/imc.c b/hw/imc.c
>> index 76b9659b752d..8436fa3933f6 100644
>> --- a/hw/imc.c
>> +++ b/hw/imc.c
>> @@ -255,3 +255,58 @@ err:
>>   	prerror("IMC Devices not added\n");
>>   	free(buf);
>>   }
>> +
>> +/*
>> + * opal_nest_imc_counters_control : This call controls the nest IMC microcode.
>> + *
>> + * mode      : For now, this call supports only OPAL_NEST_IMC_PRODUCTION_MODE.
>> + *             This mode can start/stop the Nest IMC Microcode for nest
>> + *             instrumentation from Host OS.
>> + * operation : Start(0x0) or Stop(0x1) the engine.
>> + *
>> + * This call can be extended to include more operations to use the multiple
>> + * debug modes provided by the nest IMC microcode and the parameters value_1
>> + * and value_2 for the same purpose.
>> + */
>> +static int64_t opal_nest_imc_counters_control(uint64_t mode,
>> +					      uint64_t operation,
>> +					      uint64_t value_1,
>> +					      uint64_t value_2)
>> +{
> Sorry but I don't like this API at all.
>
> Am I counting right that this gives us 73,786,976,294,838,206,464
> possible combinations of parameters? :)
>
> There's also no good way for Linux to know what values of mode,
> operation, value1 or value2 are supported by the currently running OPAL.
>
> I don't see why we wouldn't just have two OPAL calls, start and stop.

As suggested, on the other hand, we will have 3 opal calls, _INIT, _START
and _STOP with a parameter as IMC type (nest or core).

Thanks for review
Maddy


>
> If we want to add another mode in future, that can be a new OPAL call.
> Linux can detect a new OPAL call easily & cleanly with OPAL_CHECK_TOKEN.
>
> cheers
>