[PATCH v5 19/26] powerpc/book3s64/kuap: Move KUAP related function outside radix

[Michael Ellerman]

"Aneesh Kumar K.V" <aneesh.kumar at linux.ibm.com> writes:
> On 7/6/20 6:11 PM, Michael Ellerman wrote:
>> "Aneesh Kumar K.V" <aneesh.kumar at linux.ibm.com> writes:
>>> The next set of patches adds support for kuap with hash translation.
>> 
>> That's no longer true of this series.
>> 
>>> diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
>>> index 0d72c0246052..e93b65a0e6e7 100644
>>> --- a/arch/powerpc/mm/book3s64/pkeys.c
>>> +++ b/arch/powerpc/mm/book3s64/pkeys.c
>>> @@ -199,6 +200,24 @@ void __init pkey_early_init_devtree(void)
>>>   	return;
>>>   }
>>>   
>>> +#ifdef CONFIG_PPC_KUAP
>>> +void __init setup_kuap(bool disabled)
>>> +{
>>> +	if (disabled || !early_radix_enabled())
>>> +		return;
>>> +
>>> +	if (smp_processor_id() == boot_cpuid) {
>>> +		pr_info("Activating Kernel Userspace Access Prevention\n");
>>> +		cur_cpu_spec->mmu_features |= MMU_FTR_RADIX_KUAP;
>>> +	}
>>> +
>>> +	/* Make sure userspace can't change the AMR */
>>> +	mtspr(SPRN_UAMOR, 0);
>>> +	mtspr(SPRN_AMR, AMR_KUAP_BLOCKED);
>>> +	isync();
>>> +}
>>> +#endif
>> 
>> This makes this code depend on CONFIG_PPC_MEM_KEYS=y, which it didn't
>> used to.
>> 
>> That risks breaking people's existing .configs, if they have
>> PPC_MEM_KEYS=n they will now lose KUAP.
>> 
>> And I'm not convinced the two features should be tied together, at least
>> at the user-visible Kconfig level.
>
> That simplifies the addition of hash kuap a lot. Especially in the 
> exception entry and return paths.  I did try to consider them as 
> independent options. But then the feature fixup in asm code gets 
> unnecessarily complicated. Also the UAMOR handling also get complicated.

Yep. I'm OK if most of the code is enabled for either/both options, but
I think the user-visible options should not depend on each other.

So something like:

config PPC_PKEY
	def_bool y
        depends on PPC_MEM_KEYS || PPC_KUAP

And then the low-level code is guarded by PPC_PKEY.

Or we just say that making MEM_KEYS configurable is not worth the
added complexity and turn it on always.

cheers