On 2021/02/03 12:08PM, Sandipan Das wrote:
> The Power ISA says that the fixed-point load and update
> instructions must neither use R0 for the base address (RA)
> nor have the destination (RT) and the base address (RA) as
> the same register. In these cases, the instruction is
> invalid. This applies to the following instructions.
> * Load Byte and Zero with Update (lbzu)
> * Load Byte and Zero with Update Indexed (lbzux)
> * Load Halfword and Zero with Update (lhzu)
> * Load Halfword and Zero with Update Indexed (lhzux)
> * Load Halfword Algebraic with Update (lhau)
> * Load Halfword Algebraic with Update Indexed (lhaux)
> * Load Word and Zero with Update (lwzu)
> * Load Word and Zero with Update Indexed (lwzux)
> * Load Word Algebraic with Update Indexed (lwaux)
> * Load Doubleword with Update (ldu)
> * Load Doubleword with Update Indexed (ldux)
>
> However, the following behaviour is observed using some
> invalid opcodes where RA = RT.
>
> An userspace program using an invalid instruction word like
> 0xe9ce0001, i.e. "ldu r14, 0(r14)", runs and exits without
> getting terminated abruptly. The instruction performs the
> load operation but does not write the effective address to
> the base address register.
While the processor (p8 in my test) doesn't seem to be throwing an
exception, I don't think it is necessarily loading the value. Qemu
throws an exception though. It's probably best to term the behavior as
being undefined.
> Attaching an uprobe at that
> instruction's address results in emulation which writes the
> effective address to the base register. Thus, the final value
> of the base address register is different.
>
> To remove any inconsistencies, this adds an additional check
> for the aforementioned instructions to make sure that they
> are treated as unknown by the emulation infrastructure when
> RA = 0 or RA = RT. The kernel will then fallback to executing
> the instruction on hardware.
>
> Fixes: 0016a4cf5582 ("powerpc: Emulate most Book I instructions in
> emulate_step()")
> Reviewed-by: Ravi Bangoria <ravi.bango...@linux.ibm.com>
> Signed-off-by: Sandipan Das <sandi...@linux.ibm.com>
> ---
> Previous versions can be found at:
> v1:
> https://lore.kernel.org/linuxppc-dev/20201119054139.244083-1-sandi...@linux.ibm.com/
>
> Changes in v2:
> - Jump to unknown_opcode instead of returning -1 for invalid
> instruction forms.
>
> ---
> arch/powerpc/lib/sstep.c | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
Wouldn't it be easier to just do the below at the end? Or, am I missing
something?
diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index ede093e9623472..a2d726d2a5e9d1 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c
@@ -2980,6 +2980,10 @@ int analyse_instr(struct instruction_op *op, const
struct pt_regs *regs,
}
#endif /* CONFIG_VSX */
+ if (GETTYPE(op->type) == LOAD && (op->type & UPDATE) &&
+ (ra == 0 || ra == rd))
+ goto unknown_opcode;
+
return 0;
logical_done:
- Naveen
