Christophe Leroy <christophe.le...@csgroup.eu> writes:
> Le 25/11/2020 à 06:16, Aneesh Kumar K.V a écrit :
>> With hash translation use DSISR_KEYFAULT to identify a wrong access.
>> With Radix we look at the AMR value and type of fault.
>>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.ku...@linux.ibm.com>
>> ---
>> arch/powerpc/include/asm/book3s/32/kup.h | 4 +--
>> arch/powerpc/include/asm/book3s/64/kup.h | 27 ++++++++++++++++----
>> arch/powerpc/include/asm/kup.h | 4 +--
>> arch/powerpc/include/asm/nohash/32/kup-8xx.h | 4 +--
>> arch/powerpc/mm/fault.c | 2 +-
>> 5 files changed, 29 insertions(+), 12 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/book3s/32/kup.h
>> b/arch/powerpc/include/asm/book3s/32/kup.h
>> index 32fd4452e960..b18cd931e325 100644
>> --- a/arch/powerpc/include/asm/book3s/32/kup.h
>> +++ b/arch/powerpc/include/asm/book3s/32/kup.h
>> @@ -177,8 +177,8 @@ static inline void restore_user_access(unsigned long
>> flags)
>> allow_user_access(to, to, end - addr, KUAP_READ_WRITE);
>> }
>>
>> -static inline bool
>> -bad_kuap_fault(struct pt_regs *regs, unsigned long address, bool is_write)
>> +static inline bool bad_kuap_fault(struct pt_regs *regs, unsigned long
>> address,
>> + bool is_write, unsigned long error_code)
>> {
>> unsigned long begin = regs->kuap & 0xf0000000;
>> unsigned long end = regs->kuap << 28;
>> diff --git a/arch/powerpc/include/asm/book3s/64/kup.h
>> b/arch/powerpc/include/asm/book3s/64/kup.h
>> index 4a3d0d601745..2922c442a218 100644
>> --- a/arch/powerpc/include/asm/book3s/64/kup.h
>> +++ b/arch/powerpc/include/asm/book3s/64/kup.h
>> @@ -301,12 +301,29 @@ static inline void set_kuap(unsigned long value)
>> isync();
>> }
>>
>> -static inline bool
>> -bad_kuap_fault(struct pt_regs *regs, unsigned long address, bool is_write)
>> +#define RADIX_KUAP_BLOCK_READ UL(0x4000000000000000)
>> +#define RADIX_KUAP_BLOCK_WRITE UL(0x8000000000000000)
>> +
>> +static inline bool bad_kuap_fault(struct pt_regs *regs, unsigned long
>> address,
>> + bool is_write, unsigned long error_code)
>> {
>> - return WARN(mmu_has_feature(MMU_FTR_KUAP) &&
>> - (regs->kuap & (is_write ? AMR_KUAP_BLOCK_WRITE :
>> AMR_KUAP_BLOCK_READ)),
>> - "Bug: %s fault blocked by AMR!", is_write ? "Write" :
>> "Read");
>> + if (!mmu_has_feature(MMU_FTR_KUAP))
>> + return false;
>> +
>> + if (radix_enabled()) {
>> + /*
>> + * Will be a storage protection fault.
>> + * Only check the details of AMR[0]
>> + */
>> + return WARN((regs->kuap & (is_write ? RADIX_KUAP_BLOCK_WRITE :
>> RADIX_KUAP_BLOCK_READ)),
>> + "Bug: %s fault blocked by AMR!", is_write ? "Write"
>> : "Read");
>
> I think it is pointless to keep the WARN() here.
>
> I have a series aiming at removing them. See
> https://patchwork.ozlabs.org/project/linuxppc-dev/patch/cc9129bdda1dbc2f0a09cf45fece7d0b0e690784.1605541983.git.christophe.le...@csgroup.eu/
Can we do this as a spearate patch as you posted above? We can drop the
WARN in that while keeping the hash branch to look at DSISR value.
-aneesh
