On 2021/02/04 12:44PM, 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. Similarly, for fixed-point stores and
> floating-point loads and stores, the instruction is invalid
> when R0 is used as the base address (RA).
>
> This is applicable 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)
> * Load Floating Single with Update (lfsu)
> * Load Floating Single with Update Indexed (lfsux)
> * Load Floating Double with Update (lfdu)
> * Load Floating Double with Update Indexed (lfdux)
> * Store Byte with Update (stbu)
> * Store Byte with Update Indexed (stbux)
> * Store Halfword with Update (sthu)
> * Store Halfword with Update Indexed (sthux)
> * Store Word with Update (stwu)
> * Store Word with Update Indexed (stwux)
> * Store Doubleword with Update (stdu)
> * Store Doubleword with Update Indexed (stdux)
> * Store Floating Single with Update (stfsu)
> * Store Floating Single with Update Indexed (stfsux)
> * Store Floating Double with Update (stfdu)
> * Store Floating Double with Update Indexed (stfdux)
>
> E.g. the following behaviour is observed for an invalid
> load and update instruction having RA = RT.
>
> While an userspace program having an instruction word like
> 0xe9ce0001, i.e. ldu r14, 0(r14), runs without getting
> receiving a SIGILL on a Power system (observed on P8 and
> P9), the outcome of executing that instruction word varies
> and its behaviour can be considered to be undefined.
>
> Attaching an uprobe at that instruction's address results
> in emulation which currently performs the load as well as
> writes the effective address back to the base register.
> This might not match the outcome from hardware.
>
> To remove any inconsistencies, this adds additional checks
> for the aforementioned instructions to make sure that the
> emulation infrastructure treats them as unknown. The kernel
> can then fallback to executing such instructions on hardware.
>
> Fixes: 0016a4cf5582 ("powerpc: Emulate most Book I instructions in
> emulate_step()")
> Signed-off-by: Sandipan Das <sandi...@linux.ibm.com>
> ---
> Previous versions can be found at:
> v2:
> https://lore.kernel.org/linuxppc-dev/20210203063841.431063-1-sandi...@linux.ibm.com/
> v1:
> https://lore.kernel.org/linuxppc-dev/20201119054139.244083-1-sandi...@linux.ibm.com/
>
> Changes in v3:
> - Dropped CONFIG_PPC_FPU check as suggested by Michael.
> - Consolidated the checks as suggested by Naveen.
>
> Changes in v2:
> - Jump to unknown_opcode instead of returning -1 for invalid
> instruction forms.
>
> ---
> arch/powerpc/lib/sstep.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
> index e96cff845ef7..9138967eb82e 100644
> --- a/arch/powerpc/lib/sstep.c
> +++ b/arch/powerpc/lib/sstep.c
> @@ -3017,6 +3017,20 @@ int analyse_instr(struct instruction_op *op, const
> struct pt_regs *regs,
>
> }
>
> + if (OP_IS_LOAD_STORE(op->type) && (op->type & UPDATE)) {
> + switch (GETTYPE(op->type)) {
> + case LOAD:
> + if (ra == rd)
> + goto unknown_opcode;
> + fallthrough;
> + case STORE:
> + case LOAD_FP:
> + case STORE_FP:
> + if (ra == 0)
> + goto unknown_opcode;
> + }
> + }
> +
> #ifdef CONFIG_VSX
> if ((GETTYPE(op->type) == LOAD_VSX ||
> GETTYPE(op->type) == STORE_VSX) &&
I'm afraid there is one more thing. scripts/checkpatch.pl reports:
WARNING: 'an userspace' may be misspelled - perhaps 'a userspace'?
#52:
While an userspace program having an instruction word like
^^^^^^^^^^^^
ERROR: switch and case should be at the same indent
#96: FILE: arch/powerpc/lib/sstep.c:3021:
+ switch (GETTYPE(op->type)) {
+ case LOAD:
[...]
+ case STORE:
+ case LOAD_FP:
+ case STORE_FP:
- Naveen
