"Aneesh Kumar K.V" <[email protected]> writes:
> On 7/6/20 6:11 PM, Michael Ellerman wrote:
>> "Aneesh Kumar K.V" <[email protected]> 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
