[Skiboot] [RFC PATCH v5 13/16] skiboot/imc: Disable IMC node when UV enabled

[maddy]

On 3/12/20 6:49 AM, Alexey Kardashevskiy wrote:
>
> On 28/02/2020 07:40, Ryan Grimm wrote:
>> From: Madhavan Srinivasan <maddy at linux.vnet.ibm.com>
>>
>> Remove the IMC nodes when the ultravisor is enabled,
>> since both HOMER and IMC scoms are not accessable in
>> hypervisor state.
>
> They are available via UV_READ_SCOM.

SCOM yes, but the latency will be high and
also there are security concerns, hence to disable it.

>
>
>> Signed-off-by: Madhavan Srinivasan <maddy at linux.vnet.ibm.com>
>> ---
>>   hw/imc.c | 11 +++++++++++
>>   1 file changed, 11 insertions(+)
>>
>> diff --git a/hw/imc.c b/hw/imc.c
>> index 3a5382c0..576eac87 100644
>> --- a/hw/imc.c
>> +++ b/hw/imc.c
>> @@ -603,6 +603,17 @@ imc_mambo:
>>   	if (pause_microcode_at_boot())
>>   		goto err;
>>   
>> +	/*
>> +	 * If MSR(S) bit is set, disable IMC nodes.
>> +	 * IMC nodes need access to specific scom and HOMER region
>> +	 * which are not accessible from hypervisor.
>> +	 *
>> +	 * At this point uv_present cant be used since uv_init()
> s/cant/can't/
> s/uv_init/init_uv/
Will fix it.

>
>
>> +	 * is called much later. Hencing checking for the MSR bit here.
>> +	 */
>> +	if (is_msr_bit_set(MSR_S))
>> +		goto err;
>> +
> Do this at the very beginning of imc_init()? Or do not call it at all in
> main_cpu_entry() if (is_msr_bit_set(MSR_S))?

We do couple of call to this file before jumping to imc_init(). And
yes I agree, we could move the check to the beginning. Will fix it


Thanks for review
maddy

>
>
>>   	/*
>>   	 * If the dt_attach_root() fails, "imc-counters" node will not be
>>   	 * seen in the device-tree and hence OS should not make any
>>