Re: [PATCH v2 06/13] powerpc: Support prefixed instructions in alignment handler
From: Christophe Leroy <hidden>
Date: 2020-02-11 06:16:26
Le 11/02/2020 à 06:33, Jordan Niethe a écrit :
quoted hunk ↗ jump to hunk
Alignment interrupts can be caused by prefixed instructions accessing memory. In the alignment handler the instruction that caused the exception is loaded and attempted emulate. If the instruction is a prefixed instruction load the prefix and suffix to emulate. After emulating increment the NIP by 8. Prefixed instructions are not permitted to cross 64-byte boundaries. If they do the alignment interrupt is invoked with SRR1 BOUNDARY bit set. If this occurs send a SIGBUS to the offending process if in user mode. If in kernel mode call bad_page_fault(). Signed-off-by: Jordan Niethe <redacted> --- v2: - Move __get_user_instr() and __get_user_instr_inatomic() to this commit (previously in "powerpc sstep: Prepare to support prefixed instructions"). - Rename sufx to suffix - Use a macro for calculating instruction length --- arch/powerpc/include/asm/uaccess.h | 30 ++++++++++++++++++++++++++++++ arch/powerpc/kernel/align.c | 8 +++++--- arch/powerpc/kernel/traps.c | 21 ++++++++++++++++++++- 3 files changed, 55 insertions(+), 4 deletions(-)diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h index 2f500debae21..30f63a81c8d8 100644 --- a/arch/powerpc/include/asm/uaccess.h +++ b/arch/powerpc/include/asm/uaccess.h@@ -474,4 +474,34 @@ static __must_check inline bool user_access_begin(const void __user *ptr, size_t #define unsafe_copy_to_user(d, s, l, e) \ unsafe_op_wrap(raw_copy_to_user_allowed(d, s, l), e)
Could it go close to other __get_user() and friends instead of being at the end of the file ?
+/*
+ * When reading an instruction iff it is a prefix, the suffix needs to be also
+ * loaded.
+ */
+#define __get_user_instr(x, y, ptr) \
+({ \
+ long __gui_ret = 0; \
+ y = 0; \
+ __gui_ret = __get_user(x, ptr); \
+ if (!__gui_ret) { \
+ if (IS_PREFIX(x)) \Does this apply to PPC32 ? If not, can we make sure IS_PREFIX is constant 0 on PPC32 so that the second read gets dropped at compile time ? Can we instead do : if (!__gui_ret && IS_PREFIX(x))
+ __gui_ret = __get_user(y, ptr + 1); \
+ } \
+ \
+ __gui_ret; \
+})
+
+#define __get_user_instr_inatomic(x, y, ptr) \
+({ \
+ long __gui_ret = 0; \
+ y = 0; \
+ __gui_ret = __get_user_inatomic(x, ptr); \
+ if (!__gui_ret) { \
+ if (IS_PREFIX(x)) \Same commments as above
quoted hunk ↗ jump to hunk
+ __gui_ret = __get_user_inatomic(y, ptr + 1); \ + } \ + \ + __gui_ret; \ +}) + #endif /* _ARCH_POWERPC_UACCESS_H */diff --git a/arch/powerpc/kernel/align.c b/arch/powerpc/kernel/align.c index ba3bf5c3ab62..e42cfaa616d3 100644 --- a/arch/powerpc/kernel/align.c +++ b/arch/powerpc/kernel/align.c@@ -293,7 +293,7 @@ static int emulate_spe(struct pt_regs *regs, unsigned int reg, int fix_alignment(struct pt_regs *regs) { - unsigned int instr; + unsigned int instr, suffix; struct instruction_op op; int r, type;@@ -303,13 +303,15 @@ int fix_alignment(struct pt_regs *regs) */ CHECK_FULL_REGS(regs); - if (unlikely(__get_user(instr, (unsigned int __user *)regs->nip))) + if (unlikely(__get_user_instr(instr, suffix, + (unsigned int __user *)regs->nip))) return -EFAULT; if ((regs->msr & MSR_LE) != (MSR_KERNEL & MSR_LE)) { /* We don't handle PPC little-endian any more... */ if (cpu_has_feature(CPU_FTR_PPC_LE)) return -EIO; instr = swab32(instr); + suffix = swab32(suffix); } #ifdef CONFIG_SPE@@ -334,7 +336,7 @@ int fix_alignment(struct pt_regs *regs) if ((instr & 0xfc0006fe) == (PPC_INST_COPY & 0xfc0006fe)) return -EIO; - r = analyse_instr(&op, regs, instr, PPC_NO_SUFFIX); + r = analyse_instr(&op, regs, instr, suffix); if (r < 0) return -EINVAL;diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c index 82a3438300fd..d80b82fc1ae3 100644 --- a/arch/powerpc/kernel/traps.c +++ b/arch/powerpc/kernel/traps.c@@ -583,6 +583,10 @@ static inline int check_io_access(struct pt_regs *regs) #define REASON_ILLEGAL (ESR_PIL | ESR_PUO) #define REASON_PRIVILEGED ESR_PPR #define REASON_TRAP ESR_PTR +#define REASON_PREFIXED 0 +#define REASON_BOUNDARY 0 + +#define inst_length(reason) 4 /* single-step stuff */ #define single_stepping(regs) (current->thread.debug.dbcr0 & DBCR0_IC)@@ -597,6 +601,10 @@ static inline int check_io_access(struct pt_regs *regs) #define REASON_ILLEGAL SRR1_PROGILL #define REASON_PRIVILEGED SRR1_PROGPRIV #define REASON_TRAP SRR1_PROGTRAP +#define REASON_PREFIXED SRR1_PREFIXED +#define REASON_BOUNDARY SRR1_BOUNDARY + +#define inst_length(reason) (((reason) & REASON_PREFIXED) ? 8 : 4) #define single_stepping(regs) ((regs)->msr & MSR_SE) #define clear_single_step(regs) ((regs)->msr &= ~MSR_SE)@@ -1593,11 +1601,20 @@ void alignment_exception(struct pt_regs *regs) { enum ctx_state prev_state = exception_enter(); int sig, code, fixed = 0; + unsigned long reason; /* We restore the interrupt state now */ if (!arch_irq_disabled_regs(regs)) local_irq_enable(); + reason = get_reason(regs); + + if (reason & REASON_BOUNDARY) { + sig = SIGBUS; + code = BUS_ADRALN; + goto bad; + } + if (tm_abort_check(regs, TM_CAUSE_ALIGNMENT | TM_CAUSE_PERSISTENT)) goto bail;@@ -1606,7 +1623,8 @@ void alignment_exception(struct pt_regs *regs) fixed = fix_alignment(regs); if (fixed == 1) { - regs->nip += 4; /* skip over emulated instruction */ + /* skip over emulated instruction */ + regs->nip += inst_length(reason); emulate_single_step(regs); goto bail; }@@ -1619,6 +1637,7 @@ void alignment_exception(struct pt_regs *regs) sig = SIGBUS; code = BUS_ADRALN; } +bad: if (user_mode(regs)) _exception(sig, regs, code, regs->dar); else
Christophe