| Message ID | 1515645816-14063-1-git-send-email-nicoleotsuka@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 11/01/18 04:43, Nicolin Chen wrote: > CONFIG_COMPAT allows ARM64 machine to run 32-bit instructions. > Since the ARCH_TIMER_USR_VCT_ACCESS_EN might be disabled if a > timer workaround is detected, accessing cntvct via mrrc will > also trigger a trap. > > So this patch adds support to handle this situation. > > Tested with a user program generated by 32-bit compiler: > int main() > { > unsigned long long cval; > asm volatile("mrrc p15, 1, %Q0, %R0, c14" : "=r" (cval)); > return 0; > } > > Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com> > --- > > [ I also added cntfrq here for safety as theoretically it could > trigger the trap as well. However, my another test case (with > mrc insturction) doesn't seem to trigger a trap. So I would > drop it in the next version if someone can confirm it's not > required. Thanks -- Nicolin ] See my previous series on this very subject[1] as well as Will's reply. > > arch/arm64/include/asm/esr.h | 25 +++++++++++++++++++ > arch/arm64/include/asm/ptrace.h | 15 ++++++++++++ > arch/arm64/kernel/entry.S | 4 ++-- > arch/arm64/kernel/traps.c | 53 +++++++++++++++++++++++++++++++++++++++-- > 4 files changed, 93 insertions(+), 4 deletions(-) > > diff --git a/arch/arm64/include/asm/esr.h b/arch/arm64/include/asm/esr.h > index 014d7d8..55dea62 100644 > --- a/arch/arm64/include/asm/esr.h > +++ b/arch/arm64/include/asm/esr.h > @@ -220,6 +220,31 @@ > (((e) & ESR_ELx_SYS64_ISS_OP2_MASK) >> \ > ESR_ELx_SYS64_ISS_OP2_SHIFT)) > > +/* ISS fields for MRC and MRS are very similar, so reuse SYS64 macros */ > +#define ESR_ELx_CP15_32_ISS_MRC_MASK ESR_ELx_SYS64_ISS_SYS_MASK > +#define ESR_ELx_CP15_32_ISS_MRC_CNTFRQ (ESR_ELx_SYS64_ISS_SYS_VAL(0, 0, 0, 14, 0) | \ > + ESR_ELx_SYS64_ISS_DIR_READ) > + > +/* ISS field definitions for CP15 MRRC/MCRR instructions (same for CP14) */ > +#define ESR_ELx_CP15_64_ISS_OPC1_SHIFT 16 > +#define ESR_ELx_CP15_64_ISS_OPC1_MASK (UL(0xf) << ESR_ELx_CP15_64_ISS_OPC1_SHIFT) > +#define ESR_ELx_CP15_64_ISS_RT2_SHIFT 10 > +#define ESR_ELx_CP15_64_ISS_RT2_MASK (UL(0x1f) << ESR_ELx_CP15_64_ISS_RT2_SHIFT) > +#define ESR_ELx_CP15_64_ISS_RT_SHIFT 5 > +#define ESR_ELx_CP15_64_ISS_RT_MASK (UL(0x1f) << ESR_ELx_CP15_64_ISS_RT_SHIFT) > +#define ESR_ELx_CP15_64_ISS_CRM_SHIFT 1 > +#define ESR_ELx_CP15_64_ISS_CRM_MASK (UL(0xf) << ESR_ELx_CP15_64_ISS_CRM_SHIFT) > +#define ESR_ELx_CP15_64_ISS_MRRC_MASK (ESR_ELx_CP15_64_ISS_OPC1_MASK | \ > + ESR_ELx_CP15_64_ISS_CRM_MASK| \ > + ESR_ELx_SYS64_ISS_DIR_MASK) > + > +#define ESR_ELx_CP15_64_ISS_MRRC_VAL(opc1, crm) \ > + (((opc1) << ESR_ELx_CP15_64_ISS_OPC1_SHIFT) | \ > + ((crm) << ESR_ELx_CP15_64_ISS_CRM_SHIFT)) > + > +#define ESR_ELx_CP15_64_ISS_MRRC_CNTVCT (ESR_ELx_CP15_64_ISS_MRRC_VAL(1, 14) | \ > + ESR_ELx_SYS64_ISS_DIR_READ) > + > #ifndef __ASSEMBLY__ > #include <asm/types.h> > > diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h > index 6069d66..50caf11 100644 > --- a/arch/arm64/include/asm/ptrace.h > +++ b/arch/arm64/include/asm/ptrace.h > @@ -243,6 +243,21 @@ static inline void pt_regs_write_reg(struct pt_regs *regs, int r, > regs->regs[r] = val; > } > > +/* > + * Write two registers given architectural register index r and r2. > + * Used by 32-bit MRRC and MCRR instrutions for 64-bit results > + */ > +static inline void pt_regs_write_regs(struct pt_regs *regs, int r, int r2, > + unsigned long val) > +{ > + if (r != 31 && r2 != 31) { > + /* Save lower 32 bits to register r */ > + regs->regs[r] = val & 0xffffffff; > + /* Save higher 32 bits to register r2 */ > + regs->regs[r2] = val >> 32; > + } > +} > + > /* Valid only for Kernel mode traps. */ > static inline unsigned long kernel_stack_pointer(struct pt_regs *regs) > { > diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S > index 6d14b8f..9d6cd95 100644 > --- a/arch/arm64/kernel/entry.S > +++ b/arch/arm64/kernel/entry.S > @@ -636,9 +636,9 @@ el0_sync_compat: > cmp x24, #ESR_ELx_EC_UNKNOWN // unknown exception in EL0 > b.eq el0_undef > cmp x24, #ESR_ELx_EC_CP15_32 // CP15 MRC/MCR trap > - b.eq el0_undef > + b.eq el0_sys > cmp x24, #ESR_ELx_EC_CP15_64 // CP15 MRRC/MCRR trap > - b.eq el0_undef > + b.eq el0_sys > cmp x24, #ESR_ELx_EC_CP14_MR // CP14 MRC/MCR trap > b.eq el0_undef > cmp x24, #ESR_ELx_EC_CP14_LS // CP14 LDC/STC trap > diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c > index 3d3588f..211cce7 100644 > --- a/arch/arm64/kernel/traps.c > +++ b/arch/arm64/kernel/traps.c > @@ -454,6 +454,17 @@ static void cntvct_read_handler(unsigned int esr, struct pt_regs *regs) > arm64_skip_faulting_instruction(regs, AARCH64_INSN_SIZE); > } > > +#ifdef CONFIG_COMPAT > +static void cntvct_read_32_handler(unsigned int esr, struct pt_regs *regs) > +{ > + int rt2 = (esr & ESR_ELx_CP15_64_ISS_RT2_MASK) >> ESR_ELx_CP15_64_ISS_RT2_SHIFT; > + int rt = (esr & ESR_ELx_CP15_64_ISS_RT_MASK) >> ESR_ELx_CP15_64_ISS_RT_SHIFT; > + > + pt_regs_write_regs(regs, rt, rt2, arch_counter_get_cntvct()); > + arm64_skip_faulting_instruction(regs, AARCH64_INSN_SIZE); > +} > +#endif > + > static void cntfrq_read_handler(unsigned int esr, struct pt_regs *regs) > { > int rt = (esr & ESR_ELx_SYS64_ISS_RT_MASK) >> ESR_ELx_SYS64_ISS_RT_SHIFT; > @@ -495,11 +506,49 @@ static struct sys64_hook sys64_hooks[] = { > {}, > }; > > +#ifdef CONFIG_COMPAT > +static struct sys64_hook cp15_32_hooks[] = { > + { > + /* Trap read access to CNTFRQ via 32-bit insturction MRC */ > + .esr_mask = ESR_ELx_CP15_32_ISS_MRC_MASK, > + .esr_val = ESR_ELx_CP15_32_ISS_MRC_CNTFRQ, > + .handler = cntfrq_read_handler, > + }, > + {}, > +}; > + > +static struct sys64_hook cp15_64_hooks[] = { > + { > + /* Trap read access to CNTVCT via 32-bit insturction MRRC */ > + .esr_mask = ESR_ELx_CP15_64_ISS_MRRC_MASK, > + .esr_val = ESR_ELx_CP15_64_ISS_MRRC_CNTVCT, > + .handler = cntvct_read_32_handler, > + }, > + {}, > +}; > +#endif > + > asmlinkage void __exception do_sysinstr(unsigned int esr, struct pt_regs *regs) > { > - struct sys64_hook *hook; > + struct sys64_hook *hook = NULL; > + > + switch (ESR_ELx_EC(esr)) { > +#ifdef CONFIG_COMPAT > + case ESR_ELx_EC_CP15_32: > + hook = cp15_32_hooks; > + break; > + case ESR_ELx_EC_CP15_64: > + hook = cp15_64_hooks; > + break; > +#endif > + case ESR_ELx_EC_SYS64: > + hook = sys64_hooks; > + break; > + default: > + break; > + } > > - for (hook = sys64_hooks; hook->handler; hook++) > + for (; hook && hook->handler; hook++) > if ((hook->esr_mask & esr) == hook->esr_val) { > hook->handler(esr, regs); > return; > Also, this code is fairly broken in its handling of conditional instructions. Thanks, M. [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2017-July/520426.html
Hello Marc, On Thu, Jan 11, 2018 at 08:51:37AM +0000, Marc Zyngier wrote: > > [ I also added cntfrq here for safety as theoretically it could > > trigger the trap as well. However, my another test case (with > > mrc insturction) doesn't seem to trigger a trap. So I would > > drop it in the next version if someone can confirm it's not > > required. Thanks -- Nicolin ] > > See my previous series on this very subject[1] as well as Will's reply. Thanks for the background. > > - for (hook = sys64_hooks; hook->handler; hook++) > > + for (; hook && hook->handler; hook++) > > if ((hook->esr_mask & esr) == hook->esr_val) { > > hook->handler(esr, regs); > > return; > > > > Also, this code is fairly broken in its handling of conditional > instructions. I understand that it should take care of the condition field as a general instruction handler. Just for curiosity: If we confine the topic to read access of CNTVCT/CNTFRQ, what'd be the penalty by ignoring the condition field and executing it anyway? Thank you Nicolin
On Tue, 16 Jan 2018 20:32:19 +0000, Nicolin Chen wrote: > > Hello Marc, > > On Thu, Jan 11, 2018 at 08:51:37AM +0000, Marc Zyngier wrote: > > > [ I also added cntfrq here for safety as theoretically it could > > > trigger the trap as well. However, my another test case (with > > > mrc insturction) doesn't seem to trigger a trap. So I would > > > drop it in the next version if someone can confirm it's not > > > required. Thanks -- Nicolin ] > > > > See my previous series on this very subject[1] as well as Will's reply. > > Thanks for the background. > > > > - for (hook = sys64_hooks; hook->handler; hook++) > > > + for (; hook && hook->handler; hook++) > > > if ((hook->esr_mask & esr) == hook->esr_val) { > > > hook->handler(esr, regs); > > > return; > > > > > > > Also, this code is fairly broken in its handling of conditional > > instructions. > > I understand that it should take care of the condition field as > a general instruction handler. Just for curiosity: If we confine > the topic to read access of CNTVCT/CNTFRQ, what'd be the penalty > by ignoring the condition field and executing it anyway? Do you mean, apart from severely corrupting userspace execution? That's a rhetorical question, right? M.
On Tue, Jan 16, 2018 at 09:19:13PM +0000, Marc Zyngier wrote: > > I understand that it should take care of the condition field as > > a general instruction handler. Just for curiosity: If we confine > > the topic to read access of CNTVCT/CNTFRQ, what'd be the penalty > > by ignoring the condition field and executing it anyway? > > Do you mean, apart from severely corrupting userspace execution? > That's a rhetorical question, right? I don't quite understand the corrupting userspace execution part. What I see for a conditional CNTVCT read is more likely: if (condition) { // in this case, if (true) r1 = lower32(cntvct); r2 = higher32(cntvct); } Could you please elaborate a bit? Thank you.
On Tue, Jan 16, 2018 at 01:37:46PM -0800, Nicolin Chen wrote: > On Tue, Jan 16, 2018 at 09:19:13PM +0000, Marc Zyngier wrote: > > > > I understand that it should take care of the condition field as > > > a general instruction handler. Just for curiosity: If we confine > > > the topic to read access of CNTVCT/CNTFRQ, what'd be the penalty > > > by ignoring the condition field and executing it anyway? > > > > Do you mean, apart from severely corrupting userspace execution? > > That's a rhetorical question, right? > > I don't quite understand the corrupting userspace execution part. > What I see for a conditional CNTVCT read is more likely: > if (condition) { // in this case, if (true) > r1 = lower32(cntvct); > r2 = higher32(cntvct); > } > > Could you please elaborate a bit? Thank you. I guess I got it now. The concern seems to be Thumb instructions. So ignoring a condition for a Thumb instruction may cause its IT scope shifting. For ARM mode, the only penalty could be two Rts getting written -- which shouldn't corrupt userspace execution. Please correct me if I am wrong or not thorough. Thanks Nicolin
On 17/01/18 02:13, Nicolin Chen wrote: > On Tue, Jan 16, 2018 at 01:37:46PM -0800, Nicolin Chen wrote: >> On Tue, Jan 16, 2018 at 09:19:13PM +0000, Marc Zyngier wrote: >> >>>> I understand that it should take care of the condition field as >>>> a general instruction handler. Just for curiosity: If we confine >>>> the topic to read access of CNTVCT/CNTFRQ, what'd be the penalty >>>> by ignoring the condition field and executing it anyway? >>> >>> Do you mean, apart from severely corrupting userspace execution? >>> That's a rhetorical question, right? >> >> I don't quite understand the corrupting userspace execution part. >> What I see for a conditional CNTVCT read is more likely: >> if (condition) { // in this case, if (true) >> r1 = lower32(cntvct); >> r2 = higher32(cntvct); >> } >> >> Could you please elaborate a bit? Thank you. > > I guess I got it now. The concern seems to be Thumb instructions. Not only. > So ignoring a condition for a Thumb instruction may cause its IT > scope shifting. For ARM mode, the only penalty could be two Rts > getting written -- which shouldn't corrupt userspace execution. > > Please correct me if I am wrong or not thorough. Consider the following: mov r0, #0 mov r1, #0 cmp r1, #3 mrrceq r0, r1, cntvct // simplified version Oh look, you've corrupted r0 and r1, which should never have be changed. Whatever uses the content r0 and r1 after the mrrc will misbehave. How is that an acceptable behaviour? How do you expect userspace to cope with such a brain damage? If you intend to emulate the CPU, you must emulate it fully, to the letter of the architecture. No ifs, no buts. M.
On Wed, Jan 17, 2018 at 09:03:48AM +0000, Marc Zyngier wrote: > > So ignoring a condition for a Thumb instruction may cause its IT > > scope shifting. For ARM mode, the only penalty could be two Rts > > getting written -- which shouldn't corrupt userspace execution. > > > > Please correct me if I am wrong or not thorough. > > Consider the following: > > mov r0, #0 > mov r1, #0 > cmp r1, #3 > mrrceq r0, r1, cntvct // simplified version > > Oh look, you've corrupted r0 and r1, which should never have be changed. > Whatever uses the content r0 and r1 after the mrrc will misbehave. How > is that an acceptable behaviour? How do you expect userspace to cope > with such a brain damage? > > If you intend to emulate the CPU, you must emulate it fully, to the > letter of the architecture. No ifs, no buts. Thanks for the explain. I see the point here. I saw your version for arm64 compat doesn't check if (rt != 31) as MRS handler does. Is there any reason for that? Thank you Nicolin
On Wed, 17 Jan 2018 12:41:56 -0800 Nicolin Chen <nicoleotsuka@gmail.com> wrote: > On Wed, Jan 17, 2018 at 09:03:48AM +0000, Marc Zyngier wrote: > > > > So ignoring a condition for a Thumb instruction may cause its IT > > > scope shifting. For ARM mode, the only penalty could be two Rts > > > getting written -- which shouldn't corrupt userspace execution. > > > > > > Please correct me if I am wrong or not thorough. > > > > Consider the following: > > > > mov r0, #0 > > mov r1, #0 > > cmp r1, #3 > > mrrceq r0, r1, cntvct // simplified version > > > > Oh look, you've corrupted r0 and r1, which should never have be > > changed. Whatever uses the content r0 and r1 after the mrrc will > > misbehave. How is that an acceptable behaviour? How do you expect > > userspace to cope with such a brain damage? > > > > If you intend to emulate the CPU, you must emulate it fully, to the > > letter of the architecture. No ifs, no buts. > > Thanks for the explain. I see the point here. > > I saw your version for arm64 compat doesn't check if (rt != 31) > as MRS handler does. Is there any reason for that? Um, perhaps because it's *compat*? How do you envision an AArch32 MR{R}C instruction targeting r31, exactly? ;) Robin.
On Wed, Jan 17, 2018 at 11:35:08PM +0000, Robin Murphy wrote: > On Wed, 17 Jan 2018 12:41:56 -0800 > Nicolin Chen <nicoleotsuka@gmail.com> wrote: > > > On Wed, Jan 17, 2018 at 09:03:48AM +0000, Marc Zyngier wrote: > > > > > > So ignoring a condition for a Thumb instruction may cause its IT > > > > scope shifting. For ARM mode, the only penalty could be two Rts > > > > getting written -- which shouldn't corrupt userspace execution. > > > > > > > > Please correct me if I am wrong or not thorough. > > > > > > Consider the following: > > > > > > mov r0, #0 > > > mov r1, #0 > > > cmp r1, #3 > > > mrrceq r0, r1, cntvct // simplified version > > > > > > Oh look, you've corrupted r0 and r1, which should never have be > > > changed. Whatever uses the content r0 and r1 after the mrrc will > > > misbehave. How is that an acceptable behaviour? How do you expect > > > userspace to cope with such a brain damage? > > > > > > If you intend to emulate the CPU, you must emulate it fully, to the > > > letter of the architecture. No ifs, no buts. > > > > Thanks for the explain. I see the point here. > > > > I saw your version for arm64 compat doesn't check if (rt != 31) > > as MRS handler does. Is there any reason for that? > > Um, perhaps because it's *compat*? How do you envision an AArch32 > MR{R}C instruction targeting r31, exactly? ;) I see. Thank you, Robin.
diff --git a/arch/arm64/include/asm/esr.h b/arch/arm64/include/asm/esr.h index 014d7d8..55dea62 100644 --- a/arch/arm64/include/asm/esr.h +++ b/arch/arm64/include/asm/esr.h @@ -220,6 +220,31 @@ (((e) & ESR_ELx_SYS64_ISS_OP2_MASK) >> \ ESR_ELx_SYS64_ISS_OP2_SHIFT)) +/* ISS fields for MRC and MRS are very similar, so reuse SYS64 macros */ +#define ESR_ELx_CP15_32_ISS_MRC_MASK ESR_ELx_SYS64_ISS_SYS_MASK +#define ESR_ELx_CP15_32_ISS_MRC_CNTFRQ (ESR_ELx_SYS64_ISS_SYS_VAL(0, 0, 0, 14, 0) | \ + ESR_ELx_SYS64_ISS_DIR_READ) + +/* ISS field definitions for CP15 MRRC/MCRR instructions (same for CP14) */ +#define ESR_ELx_CP15_64_ISS_OPC1_SHIFT 16 +#define ESR_ELx_CP15_64_ISS_OPC1_MASK (UL(0xf) << ESR_ELx_CP15_64_ISS_OPC1_SHIFT) +#define ESR_ELx_CP15_64_ISS_RT2_SHIFT 10 +#define ESR_ELx_CP15_64_ISS_RT2_MASK (UL(0x1f) << ESR_ELx_CP15_64_ISS_RT2_SHIFT) +#define ESR_ELx_CP15_64_ISS_RT_SHIFT 5 +#define ESR_ELx_CP15_64_ISS_RT_MASK (UL(0x1f) << ESR_ELx_CP15_64_ISS_RT_SHIFT) +#define ESR_ELx_CP15_64_ISS_CRM_SHIFT 1 +#define ESR_ELx_CP15_64_ISS_CRM_MASK (UL(0xf) << ESR_ELx_CP15_64_ISS_CRM_SHIFT) +#define ESR_ELx_CP15_64_ISS_MRRC_MASK (ESR_ELx_CP15_64_ISS_OPC1_MASK | \ + ESR_ELx_CP15_64_ISS_CRM_MASK| \ + ESR_ELx_SYS64_ISS_DIR_MASK) + +#define ESR_ELx_CP15_64_ISS_MRRC_VAL(opc1, crm) \ + (((opc1) << ESR_ELx_CP15_64_ISS_OPC1_SHIFT) | \ + ((crm) << ESR_ELx_CP15_64_ISS_CRM_SHIFT)) + +#define ESR_ELx_CP15_64_ISS_MRRC_CNTVCT (ESR_ELx_CP15_64_ISS_MRRC_VAL(1, 14) | \ + ESR_ELx_SYS64_ISS_DIR_READ) + #ifndef __ASSEMBLY__ #include <asm/types.h> diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h index 6069d66..50caf11 100644 --- a/arch/arm64/include/asm/ptrace.h +++ b/arch/arm64/include/asm/ptrace.h @@ -243,6 +243,21 @@ static inline void pt_regs_write_reg(struct pt_regs *regs, int r, regs->regs[r] = val; } +/* + * Write two registers given architectural register index r and r2. + * Used by 32-bit MRRC and MCRR instrutions for 64-bit results + */ +static inline void pt_regs_write_regs(struct pt_regs *regs, int r, int r2, + unsigned long val) +{ + if (r != 31 && r2 != 31) { + /* Save lower 32 bits to register r */ + regs->regs[r] = val & 0xffffffff; + /* Save higher 32 bits to register r2 */ + regs->regs[r2] = val >> 32; + } +} + /* Valid only for Kernel mode traps. */ static inline unsigned long kernel_stack_pointer(struct pt_regs *regs) { diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S index 6d14b8f..9d6cd95 100644 --- a/arch/arm64/kernel/entry.S +++ b/arch/arm64/kernel/entry.S @@ -636,9 +636,9 @@ el0_sync_compat: cmp x24, #ESR_ELx_EC_UNKNOWN // unknown exception in EL0 b.eq el0_undef cmp x24, #ESR_ELx_EC_CP15_32 // CP15 MRC/MCR trap - b.eq el0_undef + b.eq el0_sys cmp x24, #ESR_ELx_EC_CP15_64 // CP15 MRRC/MCRR trap - b.eq el0_undef + b.eq el0_sys cmp x24, #ESR_ELx_EC_CP14_MR // CP14 MRC/MCR trap b.eq el0_undef cmp x24, #ESR_ELx_EC_CP14_LS // CP14 LDC/STC trap diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c index 3d3588f..211cce7 100644 --- a/arch/arm64/kernel/traps.c +++ b/arch/arm64/kernel/traps.c @@ -454,6 +454,17 @@ static void cntvct_read_handler(unsigned int esr, struct pt_regs *regs) arm64_skip_faulting_instruction(regs, AARCH64_INSN_SIZE); } +#ifdef CONFIG_COMPAT +static void cntvct_read_32_handler(unsigned int esr, struct pt_regs *regs) +{ + int rt2 = (esr & ESR_ELx_CP15_64_ISS_RT2_MASK) >> ESR_ELx_CP15_64_ISS_RT2_SHIFT; + int rt = (esr & ESR_ELx_CP15_64_ISS_RT_MASK) >> ESR_ELx_CP15_64_ISS_RT_SHIFT; + + pt_regs_write_regs(regs, rt, rt2, arch_counter_get_cntvct()); + arm64_skip_faulting_instruction(regs, AARCH64_INSN_SIZE); +} +#endif + static void cntfrq_read_handler(unsigned int esr, struct pt_regs *regs) { int rt = (esr & ESR_ELx_SYS64_ISS_RT_MASK) >> ESR_ELx_SYS64_ISS_RT_SHIFT; @@ -495,11 +506,49 @@ static struct sys64_hook sys64_hooks[] = { {}, }; +#ifdef CONFIG_COMPAT +static struct sys64_hook cp15_32_hooks[] = { + { + /* Trap read access to CNTFRQ via 32-bit insturction MRC */ + .esr_mask = ESR_ELx_CP15_32_ISS_MRC_MASK, + .esr_val = ESR_ELx_CP15_32_ISS_MRC_CNTFRQ, + .handler = cntfrq_read_handler, + }, + {}, +}; + +static struct sys64_hook cp15_64_hooks[] = { + { + /* Trap read access to CNTVCT via 32-bit insturction MRRC */ + .esr_mask = ESR_ELx_CP15_64_ISS_MRRC_MASK, + .esr_val = ESR_ELx_CP15_64_ISS_MRRC_CNTVCT, + .handler = cntvct_read_32_handler, + }, + {}, +}; +#endif + asmlinkage void __exception do_sysinstr(unsigned int esr, struct pt_regs *regs) { - struct sys64_hook *hook; + struct sys64_hook *hook = NULL; + + switch (ESR_ELx_EC(esr)) { +#ifdef CONFIG_COMPAT + case ESR_ELx_EC_CP15_32: + hook = cp15_32_hooks; + break; + case ESR_ELx_EC_CP15_64: + hook = cp15_64_hooks; + break; +#endif + case ESR_ELx_EC_SYS64: + hook = sys64_hooks; + break; + default: + break; + } - for (hook = sys64_hooks; hook->handler; hook++) + for (; hook && hook->handler; hook++) if ((hook->esr_mask & esr) == hook->esr_val) { hook->handler(esr, regs); return;
CONFIG_COMPAT allows ARM64 machine to run 32-bit instructions. Since the ARCH_TIMER_USR_VCT_ACCESS_EN might be disabled if a timer workaround is detected, accessing cntvct via mrrc will also trigger a trap. So this patch adds support to handle this situation. Tested with a user program generated by 32-bit compiler: int main() { unsigned long long cval; asm volatile("mrrc p15, 1, %Q0, %R0, c14" : "=r" (cval)); return 0; } Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com> --- [ I also added cntfrq here for safety as theoretically it could trigger the trap as well. However, my another test case (with mrc insturction) doesn't seem to trigger a trap. So I would drop it in the next version if someone can confirm it's not required. Thanks -- Nicolin ] arch/arm64/include/asm/esr.h | 25 +++++++++++++++++++ arch/arm64/include/asm/ptrace.h | 15 ++++++++++++ arch/arm64/kernel/entry.S | 4 ++-- arch/arm64/kernel/traps.c | 53 +++++++++++++++++++++++++++++++++++++++-- 4 files changed, 93 insertions(+), 4 deletions(-)