Skip to contentSkip to conversation

arm64: Restrict undef hook for cpufeature registers

PATCH Applied Rob Herring 3 participants 5 messages May 17, 2021 – Jun 23, 2021 arm64

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 arm64: Restrict undef hook for cpufeature registers

From: Raphael Gault <[email protected]>

This commit modifies the mask of the mrs_hook declared in arch/arm64/kernel/cpufeatures.c which emulates only feature register access. This is necessary because this hook's mask was too large and thus masking any mrs instruction, even if not related to the emulated registers which made the pmu emulation inefficient.

---
I don't *think* I'm going to need this for the perf userspace counter access, but this patch stands on its own as the PMU registers are not emulated. So please apply it.

v2:
- Fix warning for set but unused sys_reg

---
 arch/arm64/kernel/cpufeature.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/arm64/kernel/cpufeature.c +2 -2
index efed2830d141..c773f7c3c007 100644
@@ -2905,8 +2905,8 @@ static int emulate_mrs(struct pt_regs *regs, u32 insn)
 }
 
 static struct undef_hook mrs_hook = {
-	.instr_mask = 0xfff00000,
-	.instr_val  = 0xd5300000,
+	.instr_mask = 0xffff0000,
+	.instr_val  = 0xd5380000,
 	.pstate_mask = PSR_AA32_MODE_MASK,
 	.pstate_val = PSR_MODE_EL0t,
 	.fn = emulate_mrs,
Signature · 7 lines
2.27.0


_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
maintainer Acked-by on v2 1/1
#1 lore raw
Quoted 26 lines · Rob Herring

From: Raphael Gault <[email protected]>

This commit modifies the mask of the mrs_hook declared in arch/arm64/kernel/cpufeatures.c which emulates only feature register access. This is necessary because this hook's mask was too large and thus masking any mrs instruction, even if not related to the emulated registers which made the pmu emulation inefficient.

---
I don't *think* I'm going to need this for the perf userspace counter access, but this patch stands on its own as the PMU registers are not emulated. So please apply it.

v2:
- Fix warning for set but unused sys_reg

---
 arch/arm64/kernel/cpufeature.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/arm64/kernel/cpufeature.c +0 -0
index efed2830d141..c773f7c3c007 100644

arch/arm64/kernel/cpufeature.c static int emulate_mrs(struct pt_regs *regs, u32 insn) · line 2909

Quoted 7 lines
arch/arm64/kernel/cpufeature.c +2 -2
@@ -2905,8 +2905,8 @@ static int emulate_mrs(struct pt_regs *regs, u32 insn)
 }
 
 static struct undef_hook mrs_hook = {
-	.instr_mask = 0xfff00000,
-	.instr_val  = 0xd5300000,
+	.instr_mask = 0xffff0000,
+	.instr_val  = 0xd5380000,

and changing Will's email address.

Signature · 6 lines
Catalin

_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
maintainer
#2 lore raw
Quoted 37 lines · Catalin Marinas
Quoted 26 lines · Rob Herring

From: Raphael Gault <[email protected]>

This commit modifies the mask of the mrs_hook declared in arch/arm64/kernel/cpufeatures.c which emulates only feature register access. This is necessary because this hook's mask was too large and thus masking any mrs instruction, even if not related to the emulated registers which made the pmu emulation inefficient.

---
I don't *think* I'm going to need this for the perf userspace counter access, but this patch stands on its own as the PMU registers are not emulated. So please apply it.

v2:
- Fix warning for set but unused sys_reg

---
 arch/arm64/kernel/cpufeature.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/arm64/kernel/cpufeature.c +0 -0
index efed2830d141..c773f7c3c007 100644

arch/arm64/kernel/cpufeature.c static int emulate_mrs(struct pt_regs *regs, u32 insn) · line 2909

Quoted 7 lines
arch/arm64/kernel/cpufeature.c +2 -2
@@ -2905,8 +2905,8 @@ static int emulate_mrs(struct pt_regs *regs, u32 insn)
 }
 
 static struct undef_hook mrs_hook = {
-	.instr_mask = 0xfff00000,
-	.instr_val  = 0xd5300000,
+	.instr_mask = 0xffff0000,
+	.instr_val  = 0xd5380000,
Quoted 1 line

and changing Will's email address.

Should we update is_emulated() at the same time, or at least try to generate the instr_val value here using the same constants?

Will

_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

#3 lore raw
Quoted 39 lines · Will Deacon
Quoted 37 lines · Catalin Marinas
Quoted 26 lines · Rob Herring

From: Raphael Gault <[email protected]>

This commit modifies the mask of the mrs_hook declared in arch/arm64/kernel/cpufeatures.c which emulates only feature register access. This is necessary because this hook's mask was too large and thus masking any mrs instruction, even if not related to the emulated registers which made the pmu emulation inefficient.

---
I don't *think* I'm going to need this for the perf userspace counter access, but this patch stands on its own as the PMU registers are not emulated. So please apply it.

v2:
- Fix warning for set but unused sys_reg

---
 arch/arm64/kernel/cpufeature.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/arm64/kernel/cpufeature.c +0 -0
index efed2830d141..c773f7c3c007 100644

arch/arm64/kernel/cpufeature.c static int emulate_mrs(struct pt_regs *regs, u32 insn) · line 2909

Quoted 7 lines
arch/arm64/kernel/cpufeature.c +2 -2
@@ -2905,8 +2905,8 @@ static int emulate_mrs(struct pt_regs *regs, u32 insn)
 }
 
 static struct undef_hook mrs_hook = {
-   .instr_mask = 0xfff00000,
-   .instr_val  = 0xd5300000,
+   .instr_mask = 0xffff0000,
+   .instr_val  = 0xd5380000,
Quoted 4 lines
Quoted 1 line

and changing Will's email address.

Should we update is_emulated() at the same time, or at least try to generate the instr_val value here using the same constants?

Something like the below patch? We can actually mask a bit more of the instruction (Crn and Crm:3) and then eliminate some of the is_emulated() checks. I'm not all that convinced it's an improvement in readability compared to a raw instr_val and mask nor having is_emulated depend on what was checked in the undef code.

arch/arm64/kernel/cpufeature.c +0 -0
index c773f7c3c007..6b3f50c11ab5 100644
@@ -2825,11 +2825,7 @@ static void __maybe_unused

cpu_enable_cnp(struct arm64_cpu_capabilities const *c
*/
static inline bool __attribute_const__ is_emulated(u32 id) {
- return (sys_reg_Op0(id) == 0x3 &&

-               sys_reg_CRn(id) == 0x0 &&
-               sys_reg_Op1(id) == 0x0 &&
-               (sys_reg_CRm(id) == 0 ||

- ((sys_reg_CRm(id) >= 4) && (sys_reg_CRm(id) <= 7))));
+ return ((sys_reg_CRm(id) == 0) || (sys_reg_CRm(id) >= 4));
}

/*
@@ -2905,8 +2901,8 @@ static int emulate_mrs(struct pt_regs *regs, u32 insn)
}

static struct undef_hook mrs_hook = {

-       .instr_mask = 0xffff0000,
-       .instr_val  = 0xd5380000,
+       .instr_mask = 0xffe00000 | sys_reg(Op0_mask, Op1_mask, CRn_mask, 8, 0),
+       .instr_val  = 0xd5200000 | sys_reg(3, 0, 0, 0, 0),

.pstate_mask = PSR_AA32_MODE_MASK,
.pstate_val = PSR_MODE_EL0t,
.fn = emulate_mrs,

_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

maintainer Applied
#4 lore raw
In reply to Rob Herring
Quoted 5 lines · Rob Herring

This commit modifies the mask of the mrs_hook declared in arch/arm64/kernel/cpufeatures.c which emulates only feature register access. This is necessary because this hook's mask was too large and thus masking any mrs instruction, even if not related to the emulated registers which made the pmu emulation inefficient.

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

[1/1] arm64: Restrict undef hook for cpufeature registers

      https://git.kernel.org/arm64/c/cf292e93f423

Cheers,

Signature · 10 lines
Will

https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev

_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel