Re: [PATCH v4 13/16] powerpc: Support prefixed instructions in alignment handler

On Mon, Mar 23, 2020 at 6:09 PM Nicholas Piggin [off-list ref] wrote:

Jordan Niethe's on March 20, 2020 3:18 pm:

quoted

Alignment interrupts can be caused by prefixed instructions accessing
memory. 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
v3: Move __get_user_{instr(), instr_inatomic()} up with the other
get_user definitions and remove nested if.
v4: Just do the things for alignment_exception(). Other changes handled
elsewhere.
---
 arch/powerpc/kernel/traps.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index a4764b039749..cd8b3043c268 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)

Looks good. If you define REASON_BOUNDARY 0, then this will constant
fold away so no need to define it twice.

Good point.

Thanks,
Nick