diff mbox series

arm64: cpufeature: remove duplicate ID_AA64ISAR2_EL1 entry

Message ID 20220511162030.1403386-1-kristina.martsenko@arm.com (mailing list archive)
State New, archived
Headers show
Series arm64: cpufeature: remove duplicate ID_AA64ISAR2_EL1 entry | expand

Commit Message

Kristina Martsenko May 11, 2022, 4:20 p.m. UTC
The ID register table should have one entry per ID register but
currently has two entries for ID_AA64ISAR2_EL1. Only one entry has an
override, and get_arm64_ftr_reg() can end up choosing the other, causing
the override to be ignored. Fix this by removing the duplicate entry.

While here, also make the check in sort_ftr_regs() more strict so that
duplicate entries can't be added in the future.

Fixes: def8c222f054 ("arm64: Add support of PAuth QARMA3 architected algorithm")
Signed-off-by: Kristina Martsenko <kristina.martsenko@arm.com>
---
 arch/arm64/kernel/cpufeature.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Suzuki K Poulose May 11, 2022, 5:49 p.m. UTC | #1
On 11/05/2022 17:20, Kristina Martsenko wrote:
> The ID register table should have one entry per ID register but
> currently has two entries for ID_AA64ISAR2_EL1. Only one entry has an
> override, and get_arm64_ftr_reg() can end up choosing the other, causing
> the override to be ignored. Fix this by removing the duplicate entry.
> 
> While here, also make the check in sort_ftr_regs() more strict so that
> duplicate entries can't be added in the future.
> 
> Fixes: def8c222f054 ("arm64: Add support of PAuth QARMA3 architected algorithm")
> Signed-off-by: Kristina Martsenko <kristina.martsenko@arm.com>

Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>

> ---
>   arch/arm64/kernel/cpufeature.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index d72c4b4d389c..2cb9cc9e0eff 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -654,7 +654,6 @@ static const struct __ftr_reg_entry {
>   	ARM64_FTR_REG(SYS_ID_AA64ISAR0_EL1, ftr_id_aa64isar0),
>   	ARM64_FTR_REG_OVERRIDE(SYS_ID_AA64ISAR1_EL1, ftr_id_aa64isar1,
>   			       &id_aa64isar1_override),
> -	ARM64_FTR_REG(SYS_ID_AA64ISAR2_EL1, ftr_id_aa64isar2),
>   	ARM64_FTR_REG_OVERRIDE(SYS_ID_AA64ISAR2_EL1, ftr_id_aa64isar2,
>   			       &id_aa64isar2_override),
>   
> @@ -810,7 +809,7 @@ static void __init sort_ftr_regs(void)
>   		 * to sys_id for subsequent binary search in get_arm64_ftr_reg()
>   		 * to work correctly.
>   		 */
> -		BUG_ON(arm64_ftr_regs[i].sys_id < arm64_ftr_regs[i - 1].sys_id);
> +		BUG_ON(arm64_ftr_regs[i].sys_id <= arm64_ftr_regs[i - 1].sys_id);
>   	}
>   }
>
Vladimir Murzin May 12, 2022, 8:15 a.m. UTC | #2
On 5/11/22 5:20 PM, Kristina Martsenko wrote:
> The ID register table should have one entry per ID register but
> currently has two entries for ID_AA64ISAR2_EL1. Only one entry has an
> override, and get_arm64_ftr_reg() can end up choosing the other, causing
> the override to be ignored. Fix this by removing the duplicate entry.
> 
> While here, also make the check in sort_ftr_regs() more strict so that
> duplicate entries can't be added in the future.
> 
> Fixes: def8c222f054 ("arm64: Add support of PAuth QARMA3 architected algorithm")
> Signed-off-by: Kristina Martsenko <kristina.martsenko@arm.com>
> ---
>   arch/arm64/kernel/cpufeature.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index d72c4b4d389c..2cb9cc9e0eff 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -654,7 +654,6 @@ static const struct __ftr_reg_entry {
>   	ARM64_FTR_REG(SYS_ID_AA64ISAR0_EL1, ftr_id_aa64isar0),
>   	ARM64_FTR_REG_OVERRIDE(SYS_ID_AA64ISAR1_EL1, ftr_id_aa64isar1,
>   			       &id_aa64isar1_override),
> -	ARM64_FTR_REG(SYS_ID_AA64ISAR2_EL1, ftr_id_aa64isar2),
>   	ARM64_FTR_REG_OVERRIDE(SYS_ID_AA64ISAR2_EL1, ftr_id_aa64isar2,
>   			       &id_aa64isar2_override),
>   
> @@ -810,7 +809,7 @@ static void __init sort_ftr_regs(void)
>   		 * to sys_id for subsequent binary search in get_arm64_ftr_reg()
>   		 * to work correctly.
>   		 */
> -		BUG_ON(arm64_ftr_regs[i].sys_id < arm64_ftr_regs[i - 1].sys_id);
> +		BUG_ON(arm64_ftr_regs[i].sys_id <= arm64_ftr_regs[i - 1].sys_id);
>   	}
>   }
>   

Nice catch! FWIW

Reviewed-by: Vladimir Murzin <vladimir.murzin@arm.com>


Cheers
Vladimir
Will Deacon May 12, 2022, 1:05 p.m. UTC | #3
On Wed, 11 May 2022 17:20:30 +0100, Kristina Martsenko wrote:
> The ID register table should have one entry per ID register but
> currently has two entries for ID_AA64ISAR2_EL1. Only one entry has an
> override, and get_arm64_ftr_reg() can end up choosing the other, causing
> the override to be ignored. Fix this by removing the duplicate entry.
> 
> While here, also make the check in sort_ftr_regs() more strict so that
> duplicate entries can't be added in the future.
> 
> [...]

Applied to arm64 (for-next/fixes), thanks!

[1/1] arm64: cpufeature: remove duplicate ID_AA64ISAR2_EL1 entry
      https://git.kernel.org/arm64/c/2de7689c7caa

Cheers,
diff mbox series

Patch

diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index d72c4b4d389c..2cb9cc9e0eff 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -654,7 +654,6 @@  static const struct __ftr_reg_entry {
 	ARM64_FTR_REG(SYS_ID_AA64ISAR0_EL1, ftr_id_aa64isar0),
 	ARM64_FTR_REG_OVERRIDE(SYS_ID_AA64ISAR1_EL1, ftr_id_aa64isar1,
 			       &id_aa64isar1_override),
-	ARM64_FTR_REG(SYS_ID_AA64ISAR2_EL1, ftr_id_aa64isar2),
 	ARM64_FTR_REG_OVERRIDE(SYS_ID_AA64ISAR2_EL1, ftr_id_aa64isar2,
 			       &id_aa64isar2_override),
 
@@ -810,7 +809,7 @@  static void __init sort_ftr_regs(void)
 		 * to sys_id for subsequent binary search in get_arm64_ftr_reg()
 		 * to work correctly.
 		 */
-		BUG_ON(arm64_ftr_regs[i].sys_id < arm64_ftr_regs[i - 1].sys_id);
+		BUG_ON(arm64_ftr_regs[i].sys_id <= arm64_ftr_regs[i - 1].sys_id);
 	}
 }