Subject: Re: [PATCH v5 23/26] powerpc/book3s64/kuap: Move UAMOR setup to key init function

From: "Aneesh Kumar K.V" <[email-masked]>
Date: Tue, 07 Jul 2020 16:55:15 +0530

Michael Ellerman <[email-masked]> writes:

> "Aneesh Kumar K.V" <[email-masked]> writes:
>  
.....

>> @@ -232,8 +246,9 @@ void __init setup_kuap(bool disabled)
>>  		cur_cpu_spec->mmu_features |= MMU_FTR_KUAP;
>>  	}
>>  
>> -	/* Make sure userspace can't change the AMR */
>> -	mtspr(SPRN_UAMOR, 0);
>
> Why not just leave it there. It's extra insurance and it's good
> documentation.

We can't se the value to 0, because with hash kuap it is derived
from what other keys are used for. Are you suggesting to keep it as

if (radix_enabled())
	mtspr(SPRN_UAMOR, 0);

That would confuse w.r.t what happens with hash.

I can add a comment there explaining details? 

>
>> +	/*
>> +	 * Set the default kernel AMR values on all cpus.
>> +	 */
>>  	mtspr(SPRN_AMR, AMR_KUAP_BLOCKED);
>>  	isync();
>>  }
>> @@ -278,11 +293,6 @@ static inline u64 read_uamor(void)
>>  	return mfspr(SPRN_UAMOR);
>>  }
>>  
>> -static inline void write_uamor(u64 value)
>> -{
>> -	mtspr(SPRN_UAMOR, value);
>> -}
>> -
>>  static bool is_pkey_enabled(int pkey)
>>  {
>>  	u64 uamor = read_uamor();
>> @@ -353,7 +363,6 @@ void thread_pkey_regs_save(struct thread_struct *thread)
>>  	 */
>>  	thread->amr = read_amr();
>>  	thread->iamr = read_iamr();
>> -	thread->uamor = read_uamor();
>>  }
>>  
>>  void thread_pkey_regs_restore(struct thread_struct *new_thread,
>> @@ -366,8 +375,6 @@ void thread_pkey_regs_restore(struct thread_struct *new_thread,
>>  		write_amr(new_thread->amr);
>>  	if (old_thread->iamr != new_thread->iamr)
>>  		write_iamr(new_thread->iamr);
>> -	if (old_thread->uamor != new_thread->uamor)
>> -		write_uamor(new_thread->uamor);
>>  }
>>  
>>  void thread_pkey_regs_init(struct thread_struct *thread)
>> @@ -377,11 +384,9 @@ void thread_pkey_regs_init(struct thread_struct *thread)
>>  
>>  	thread->amr   = default_amr;
>>  	thread->iamr  = default_iamr;
>> -	thread->uamor = default_uamor;
>>  
>>  	write_amr(default_amr);
>>  	write_iamr(default_iamr);
>> -	write_uamor(default_uamor);
>>  }
>>  
>>  int execute_only_pkey(struct mm_struct *mm)
>
> cheers

-aneesh