[PATCH v8 28/30] powerpc: Support prefixed instructions in alignment handler

[Alistair Popple]

On Thursday, 14 May 2020 4:14:12 PM AEST Christophe Leroy wrote:
> Le 06/05/2020 à 05:40, Jordan Niethe a écrit :
> > If a prefixed instruction results in an alignment exception, the
> > SRR1_PREFIXED bit is set. The handler attempts to emulate the
> > responsible instruction and then increment the NIP past it. Use
> > SRR1_PREFIXED to determine by how much the NIP should be incremented.
> > 
> > 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().
> 
> Shouldn't this patch go before patch 23 ?

Is there a particular reason you think it needs to go before patch 23? Neither 
seem particularly related to one another and it seems logical to me at least 
that you would introduce prefix support to the datatype prior to using them.

- Alistair
 
> Christophe
> 
> > Reviewed-by: Alistair Popple <alistair at popple.id.au>
> > Signed-off-by: Jordan Niethe <jniethe5 at gmail.com>
> > ---
> > 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: Rolled into "Add prefixed instructions to instruction data type"
> > v5: Only one definition of inst_length()
> > ---
> > 
> >   arch/powerpc/kernel/traps.c | 19 ++++++++++++++++++-
> >   1 file changed, 18 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> > index 493a3fa0ac1a..105242cc2f28 100644
> > --- a/arch/powerpc/kernel/traps.c
> > +++ b/arch/powerpc/kernel/traps.c
> > @@ -583,6 +583,8 @@ 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
> > 
> >   /* single-step stuff */
> >   #define single_stepping(regs)	(current->thread.debug.dbcr0 & DBCR0_IC)
> > 
> > @@ -597,12 +599,16 @@ 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 single_stepping(regs)	((regs)->msr & MSR_SE)
> >   #define clear_single_step(regs)	((regs)->msr &= ~MSR_SE)
> >   #define clear_br_trace(regs)	((regs)->msr &= ~MSR_BE)
> >   #endif
> > 
> > +#define inst_length(reason)	(((reason) & REASON_PREFIXED) ? 8 : 4)
> > +
> > 
> >   #if defined(CONFIG_E500)
> >   int machine_check_e500mc(struct pt_regs *regs)
> >   {
> > 
> > @@ -1593,11 +1599,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 +1621,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 +1635,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