[Skiboot] [PATCH v4 4/4] Advertise the self-save and self-restore attributes in the device tree

Pratik Sampat psampat at linux.ibm.com
Mon Feb 17 14:42:51 AEDT 2020


Thanks for the review Michael.

>>   libpore/p8_pore_table_gen_api.H               |   1 +
> Should this last file be in a different patch?
>
Sure, could be. However there's no real P8 related change, I just noticed the
absence of the supported SPR when I was advertising through the device-tree.
I don't see it fit in any earlier patch either, I could split it into a
different patch though as you suggest?

>> +
>> +	const uint64_t self_restore_regs[] = {
>> +		P8_SPR_HRMOR,
>> +		P8_SPR_HMEER,
>> +		P8_SPR_PMICR,
>> +		P8_SPR_PMCR,
>> +		P8_SPR_HID0,
>> +		P8_SPR_HID1,
>> +		P8_SPR_HID4,
>> +		P8_SPR_HID5,
>> +		P8_SPR_HSPRG0,
>> +		P8_SPR_LPCR,
>> +		P8_SPR_PSSCR,
>> +		P8_MSR_MSR
>> +	};
>> +
>> +	const uint64_t self_save_regs[] = {
>> +		P9_STOP_SPR_DAWR,
>> +		P9_STOP_SPR_HSPRG0,
>> +		P9_STOP_SPR_LDBAR,
>> +		P9_STOP_SPR_LPCR,
>> +		P9_STOP_SPR_PSSCR,
>> +		P9_STOP_SPR_MSR,
>> +		P9_STOP_SPR_HRMOR,
>> +		P9_STOP_SPR_HMEER,
>> +		P9_STOP_SPR_PMCR,
>> +		P9_STOP_SPR_PTCR
>> +	};
> Why does restore use P8 and save use P9?
>
>
Self-restore and self-save are two different APIs, where essentially both
perform the act of saving and restoring. The only difference is that in the
former, the SPR and the value of that SPR needs to be supplied and in the latter
only the SPR number is needed, although they aren't redundant APIs and have
their separate use-cases.

The Self-save API is only supported for machines p9 and above, Although in
terms of index, say a P8 LPCR is no different from its P9 counterpart. However,
there are SPRs that are only available in p8 like HID(1,4,5) and as a clean
abstraction already existed in the headers, I was ought to use it.

>>   	add_cpu_idle_state_properties();
>> +	if (has_deep_states)
>> +		add_cpu_self_save_properties();
> Why only when we have deep states?
>
The point of having a save-restore mechanism was when we woke up from a state
of loss, there are crucial SPRs that were needed to restore the state of the
machine.
If a deep state of loss didn't exist in the first place, then having support to
save-restore didn't make sense to me.

--
Pratik Sampat



More information about the Skiboot mailing list