Re: [PATCH 1/4] linux-user/ppc: Fix XER access in save/restore_user_regs

[David Gibson]

On Thu, Oct 14, 2021 at 07:32:31PM -0300, matheus.ferst@eldorado.org.br wrote:
> From: Matheus Ferst <matheus.ferst@eldorado.org.br>
> 
> We should use cpu_read_xer/cpu_write_xer to save/restore the complete
> register since some of its bits are in other fields of CPUPPCState. A
> test is added to prevent future regressions.
> 
> Fixes: da91a00f191f ("target-ppc: Split out SO, OV, CA fields from XER")
> Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>

Applied to ppc-for-6.2, thanks.

> ---
>  linux-user/ppc/signal.c                     |  9 +++--
>  tests/tcg/ppc64/Makefile.target             |  2 +
>  tests/tcg/ppc64le/Makefile.target           |  2 +
>  tests/tcg/ppc64le/signal_save_restore_xer.c | 42 +++++++++++++++++++++
>  4 files changed, 52 insertions(+), 3 deletions(-)
>  create mode 100644 tests/tcg/ppc64le/signal_save_restore_xer.c
> 
> diff --git a/linux-user/ppc/signal.c b/linux-user/ppc/signal.c
> index edfad28a37..4413a4a28f 100644
> --- a/linux-user/ppc/signal.c
> +++ b/linux-user/ppc/signal.c
> @@ -244,7 +244,7 @@ static void save_user_regs(CPUPPCState *env, struct 
> target_mcontext *frame)
>      __put_user(env->nip, &frame->mc_gregs[TARGET_PT_NIP]);
>      __put_user(env->ctr, &frame->mc_gregs[TARGET_PT_CTR]);
>      __put_user(env->lr, &frame->mc_gregs[TARGET_PT_LNK]);
> -    __put_user(env->xer, &frame->mc_gregs[TARGET_PT_XER]);
> +    __put_user(cpu_read_xer(env), &frame->mc_gregs[TARGET_PT_XER]);
>  
>      for (i = 0; i < ARRAY_SIZE(env->crf); i++) {
>          ccr |= env->crf[i] << (32 - ((i + 1) * 4));
> @@ -319,6 +319,7 @@ static void restore_user_regs(CPUPPCState *env,
>  {
>      target_ulong save_r2 = 0;
>      target_ulong msr;
> +    target_ulong xer;
>      target_ulong ccr;
>  
>      int i;
> @@ -334,9 +335,11 @@ static void restore_user_regs(CPUPPCState *env,
>      __get_user(env->nip, &frame->mc_gregs[TARGET_PT_NIP]);
>      __get_user(env->ctr, &frame->mc_gregs[TARGET_PT_CTR]);
>      __get_user(env->lr, &frame->mc_gregs[TARGET_PT_LNK]);
> -    __get_user(env->xer, &frame->mc_gregs[TARGET_PT_XER]);
> +
> +    __get_user(xer, &frame->mc_gregs[TARGET_PT_XER]);
> +    cpu_write_xer(env, xer);
> +
>      __get_user(ccr, &frame->mc_gregs[TARGET_PT_CCR]);
> -
>      for (i = 0; i < ARRAY_SIZE(env->crf); i++) {
>          env->crf[i] = (ccr >> (32 - ((i + 1) * 4))) & 0xf;
>      }
> diff --git a/tests/tcg/ppc64/Makefile.target b/tests/tcg/ppc64/Makefile.target
> index a6a4ddaeca..6ab7934fdf 100644
> --- a/tests/tcg/ppc64/Makefile.target
> +++ b/tests/tcg/ppc64/Makefile.target
> @@ -23,4 +23,6 @@ run-plugin-byte_reverse-with-%:
>       $(call skip-test, "RUN of byte_reverse ($*)", "not built")
>  endif
>  
> +PPC64_TESTS += signal_save_restore_xer
> +
>  TESTS += $(PPC64_TESTS)
> diff --git a/tests/tcg/ppc64le/Makefile.target 
> b/tests/tcg/ppc64le/Makefile.target
> index c0c14ffbad..5e65b1590d 100644
> --- a/tests/tcg/ppc64le/Makefile.target
> +++ b/tests/tcg/ppc64le/Makefile.target
> @@ -22,4 +22,6 @@ run-plugin-byte_reverse-with-%:
>       $(call skip-test, "RUN of byte_reverse ($*)", "not built")
>  endif
>  
> +PPC64LE_TESTS += signal_save_restore_xer
> +
>  TESTS += $(PPC64LE_TESTS)
> diff --git a/tests/tcg/ppc64le/signal_save_restore_xer.c 
> b/tests/tcg/ppc64le/signal_save_restore_xer.c
> new file mode 100644
> index 0000000000..e4f8a07dd7
> --- /dev/null
> +++ b/tests/tcg/ppc64le/signal_save_restore_xer.c
> @@ -0,0 +1,42 @@
> +#include <assert.h>
> +#include <stdint.h>
> +#include <signal.h>
> +#include <sys/user.h>
> +
> +#define XER_SO   (1 << 31)
> +#define XER_OV   (1 << 30)
> +#define XER_CA   (1 << 29)
> +#define XER_OV32 (1 << 19)
> +#define XER_CA32 (1 << 18)
> +
> +uint64_t saved;
> +
> +void sigill_handler(int sig, siginfo_t *si, void *ucontext)
> +{
> +    ucontext_t *uc = ucontext;
> +    uc->uc_mcontext.regs->nip += 4;
> +    saved = uc->uc_mcontext.regs->xer;
> +    uc->uc_mcontext.regs->xer |= XER_OV | XER_OV32;
> +}
> +
> +int main(void)
> +{
> +    uint64_t initial = XER_CA | XER_CA32, restored;
> +    struct sigaction sa = {
> +        .sa_sigaction = sigill_handler,
> +        .sa_flags = SA_SIGINFO
> +    };
> +
> +    sigaction(SIGILL, &sa, NULL);
> +
> +    asm("mtspr 1, %1\n\t"
> +        ".long 0x0\n\t"
> +        "mfspr %0, 1\n\t"
> +        : "=r" (restored)
> +        : "r" (initial));
> +
> +    assert(saved == initial);
> +    assert(restored == (XER_OV | XER_OV32 | XER_CA | XER_CA32));
> +
> +    return 0;
> +}

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature