Re: [PATCH v5 1/5] powerpc/sstep: Emulate prefixed instructions only when CPU_FTR_ARCH_31 is set
From: Daniel Axtens <hidden>
Date: 2020-10-12 13:58:49
Hi, To review this, I looked through the supported instructions to see if there were any that I thought might have been missed. I didn't find any other v3.1 ones, although I don't have a v3.1 ISA to hand so I was basically looking for instructions I didn't recognise. I did, however, find a number of instructions that are new in ISA 3.0 that aren't guarded: - addpcis - lxvl/stxvl - lxvll/stxvll - lxvwsx - stxvx - lxsipzx - lxvh8x - lxsihzx - lxvb16x/stxvb16x - stxsibx - stxsihx - lxvb16x - lxsd/stxsd - lxssp/stxssp - lxv/stxv Also, I don't know how bothered we are about P7, but if I'm reading the ISA correctly, lqarx/stqcx. are not supported before ISA 2.07. Likewise a number of the vector instructions like lxsiwzx and lxsiwax (and the companion stores). I realise it's not really the point of this particular patch, so I don't think this should block acceptance. What I would like to know - and maybe this is something where we need mpe to weigh in - is whether we need consistent guards for 2.07 and 3.0. We have some 3.0 guards already but clearly not everywhere. With all that said - the patch does what it says it does, and looks good to me: Reviewed-by: Daniel Axtens <redacted> Kind regards, Daniel
quoted hunk ↗ jump to hunk
From: Balamuruhan S <redacted> Unconditional emulation of prefixed instructions will allow emulation of them on Power10 predecessors which might cause issues. Restrict that. Fixes: 3920742b92f5 ("powerpc sstep: Add support for prefixed fixed-point arithmetic") Fixes: 50b80a12e4cc ("powerpc sstep: Add support for prefixed load/stores") Signed-off-by: Balamuruhan S <redacted> Signed-off-by: Ravi Bangoria <redacted> --- arch/powerpc/lib/sstep.c | 6 ++++++ 1 file changed, 6 insertions(+)diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c index e9dcaba9a4f8..e6242744c71b 100644 --- a/arch/powerpc/lib/sstep.c +++ b/arch/powerpc/lib/sstep.c@@ -1346,6 +1346,9 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs, switch (opcode) { #ifdef __powerpc64__ case 1: + if (!cpu_has_feature(CPU_FTR_ARCH_31)) + return -1; + prefix_r = GET_PREFIX_R(word); ra = GET_PREFIX_RA(suffix); rd = (suffix >> 21) & 0x1f;@@ -2733,6 +2736,9 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs, } break; case 1: /* Prefixed instructions */ + if (!cpu_has_feature(CPU_FTR_ARCH_31)) + return -1; + prefix_r = GET_PREFIX_R(word); ra = GET_PREFIX_RA(suffix); op->update_reg = ra;-- 2.26.2