linux-kernel - Re: [PATCH] arm64/mm: Drop ESR_ELx_FSC

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <cea3ced2-b8b3-426b-8c40-a47881a5bff6@arm.com>
Date: Mon, 17 Jun 2024 14:09:27 +0530
From: Anshuman Khandual <anshuman.khandual@....com>
To: Marc Zyngier <maz@...nel.org>
Cc: linux-arm-kernel@...ts.infradead.org, mark.rutland@....com,
 ryan.roberts@....com, Catalin Marinas <catalin.marinas@....com>,
 Will Deacon <will@...nel.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] arm64/mm: Drop ESR_ELx_FSC_TYPE



On 6/17/24 13:13, Marc Zyngier wrote:
> On Mon, 17 Jun 2024 04:15:40 +0100,
> Anshuman Khandual <anshuman.khandual@....com> wrote:
>>
>> Does the following re-worked patch looks okay ? Earlier set_thread_esr() changes
>> can be dropped from arch/arm64/mm/fault.c and also the original commit message
>> still makes sense.
>>
>> diff --git a/arch/arm64/include/asm/esr.h b/arch/arm64/include/asm/esr.h
>> index 7abf09df7033..6cd13ac61005 100644
>> --- a/arch/arm64/include/asm/esr.h
>> +++ b/arch/arm64/include/asm/esr.h
>> @@ -121,6 +121,13 @@
>>  #define ESR_ELx_FSC_SECC       (0x18)
>>  #define ESR_ELx_FSC_SECC_TTW(n)        (0x1c + (n))
>>  
>> +/* Status codes for individual page table levels */
>> +#define ESR_ELx_FSC_ACCESS_L(n)        (ESR_ELx_FSC_ACCESS + n)
>> +#define ESR_ELx_FSC_FAULT_nL   (0x2C)
>> +#define ESR_ELx_FSC_FAULT_L(n) (((n) < 0 ? ESR_ELx_FSC_FAULT_nL : \
>> +                                           ESR_ELx_FSC_FAULT) + (n))
>> +#define ESR_ELx_FSC_PERM_L(n)  (ESR_ELx_FSC_PERM + n)
>> +
>>  /* ISS field definitions for Data Aborts */
>>  #define ESR_ELx_ISV_SHIFT      (24)
>>  #define ESR_ELx_ISV            (UL(1) << ESR_ELx_ISV_SHIFT)
>> @@ -388,20 +395,33 @@ static inline bool esr_is_data_abort(unsigned long esr)
>>  
>>  static inline bool esr_fsc_is_translation_fault(unsigned long esr)
>>  {
>> -       /* Translation fault, level -1 */
>> -       if ((esr & ESR_ELx_FSC) == 0b101011)
>> -               return true;
>> -       return (esr & ESR_ELx_FSC_TYPE) == ESR_ELx_FSC_FAULT;
>> +       esr = esr & ESR_ELx_FSC;
>> +
>> +       return (esr == ESR_ELx_FSC_FAULT_L(3)) ||
>> +              (esr == ESR_ELx_FSC_FAULT_L(2)) ||
>> +              (esr == ESR_ELx_FSC_FAULT_L(1)) ||
>> +              (esr == ESR_ELx_FSC_FAULT_L(0)) ||
>> +              (esr == ESR_ELx_FSC_FAULT_L(-1));
>>  }
>>  
>>  static inline bool esr_fsc_is_permission_fault(unsigned long esr)
>>  {
>> -       return (esr & ESR_ELx_FSC_TYPE) == ESR_ELx_FSC_PERM;
>> +       esr = esr & ESR_ELx_FSC;
>> +
>> +       return (esr == ESR_ELx_FSC_PERM_L(3)) ||
>> +              (esr == ESR_ELx_FSC_PERM_L(2)) ||
>> +              (esr == ESR_ELx_FSC_PERM_L(1)) ||
>> +              (esr == ESR_ELx_FSC_PERM_L(0));
>>  }
>>  
>>  static inline bool esr_fsc_is_access_flag_fault(unsigned long esr)
>>  {
>> -       return (esr & ESR_ELx_FSC_TYPE) == ESR_ELx_FSC_ACCESS;
>> +       esr = esr & ESR_ELx_FSC;
>> +
>> +       return (esr == ESR_ELx_FSC_ACCESS_L(3)) ||
>> +              (esr == ESR_ELx_FSC_ACCESS_L(2)) ||
>> +              (esr == ESR_ELx_FSC_ACCESS_L(1)) ||
>> +              (esr == ESR_ELx_FSC_ACCESS_L(0));
>>  }
>>  
>>  /* Indicate whether ESR.EC==0x1A is for an ERETAx instruction */
> 
> This looks better indeed.

Thanks Marc.

Hello Mark/Ryan,

Could I still keep your tags for the patch, or it's better to just
drop them as there are some new changes being folded in. Please do
advise. Thank you.

- Anshuman