[Skiboot] [PATCH v2] core/init: Add ibm, processor-storage-keys property to /cpus DT node.

[Thiago Jung Bauermann]

Ram Pai <linuxram at us.ibm.com> writes:

> On Fri, Sep 08, 2017 at 07:31:02PM -0300, Thiago Jung Bauermann wrote:
>> LoPAPR says:
>> 
>>     “ibm,processor-storage-keys”
>> 
>>     property name indicating the number of virtual storage keys supported
>>     by the processor described by this node.
>> 
>>     prop-encoded-array: Consists of two cells encoded as with encode-int.
>>     The first cell represents the number of virtual storage keys supported
>>     for data accesses while the second cell represents the number of
>>     virtual storage keys supported for instruction accesses. The cell value
>>     of zero indicates that no storage keys are supported for the access
>>     type.
>> 
>> pHyp provides the property above but there's a bug in P8 firmware where the
>> second cell is zero even though POWER8 supports instruction access keys.
>> This bug will be fixed for P9.
>> 
>> While this is a PAPR property, it's useful to have it in powernv as well
>> so that Linux has a uniform way of checking for the feature regardless of
>> the platform it's running on.
>> 
>> On PAPR there is one property for each CPU node, but since it's highly
>> unlikely that different CPUs will support a different number of keys, we
>> put the property in the /cpus node instead.
>> 
>> Tested on QEMU POWER8 powernv model and Mambo P9.
>> 
>> Signed-off-by: Thiago Jung Bauermann <bauerman at linux.vnet.ibm.com>
>> ---
>>  core/init.c | 18 ++++++++++++++++++
>>  1 file changed, 18 insertions(+)
>> 
>> Changes in v2:
>> - Put property in /cpus instead of in each CPU node.
>> 
>> diff --git a/core/init.c b/core/init.c
>> index 8951e17b4c90..4a6f9155511d 100644
>> --- a/core/init.c
>> +++ b/core/init.c
>> @@ -563,6 +563,22 @@ void __noreturn load_and_boot_kernel(bool is_reboot)
>>  	start_kernel(kernel_entry, fdt, mem_top);
>>  }
>> 
>> +static void storage_keys_fixup(void)
>> +{
>> +	/* P7 doesn't support instruction access keys. */
>> +	const u32 insn_keys = (proc_gen == proc_gen_p7) ? 0 : 32;
> 		
> theortically we should be checking for earlier gens too.   right?
> It should be
> 	const u32 insn_keys = (proc_gen <= proc_gen_p7) ? 0 : 32;

Theoretically yes, but if you look at the definition of the proc_gen enum:

    /* Processor generation */
    enum proc_gen {
            proc_gen_unknown,
            proc_gen_p7,		/* P7 and P7+ */
            proc_gen_p8,
            proc_gen_p9,
    };
    extern enum proc_gen proc_gen;

In practice skiboot doesn't support anything older than a P7. So
proc_gen <= proc_gen_p7 really means proc_gen == proc_gen_unknown, and
storage_keys_fixup returns early in that case already.

-- 
Thiago Jung Bauermann
IBM Linux Technology Center