On Fri, Oct 15, 2021 at 03:19:40PM -0300, matheus.fe...@eldorado.org.br wrote: > From: Matheus Ferst <matheus.fe...@eldorado.org.br> > > PowerISA says that mtmsr[d] "does not alter MSR[HV], MSR[S], MSR[ME], or > MSR[LE]", but the current code only filters the GPR-provided value if > L=1. This behavior caused some problems in FreeBSD, and a build option > was added to work around the issue [1], but it seems that the bug was > not reported in launchpad/gitlab. This patch address the issue in qemu, > so the option on FreeBSD should no longer be required. > > [1] > https://cgit.freebsd.org/src/commit/?id=4efb1ca7d2a44cfb33d7f9e18bd92f8d68dcfee0 > > Signed-off-by: Matheus Ferst <matheus.fe...@eldorado.org.br>
Applied to ppc-for-6.2, thanks.
> ---
> target/ppc/cpu.h | 1 +
> target/ppc/translate.c | 73 +++++++++++++++++++++++-------------------
> 2 files changed, 41 insertions(+), 33 deletions(-)
>
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index baa4e7c34d..e94e82297b 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -314,6 +314,7 @@ typedef struct ppc_v3_pate_t {
> #define MSR_AP 23 /* Access privilege state on 602 hflags
> */
> #define MSR_VSX 23 /* Vector Scalar Extension (ISA 2.06 and later) x hflags
> */
> #define MSR_SA 22 /* Supervisor access mode on 602 hflags
> */
> +#define MSR_S 22 /* Secure state
> */
> #define MSR_KEY 19 /* key bit on 603e
> */
> #define MSR_POW 18 /* Power management
> */
> #define MSR_TGPR 17 /* TGPR usage on 602/603 x
> */
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index b985e9e55b..a5ebe03e2a 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -4940,32 +4940,40 @@ static void gen_mtmsrd(DisasContext *ctx)
> CHK_SV;
>
> #if !defined(CONFIG_USER_ONLY)
> + TCGv t0, t1;
> + target_ulong mask;
> +
> + t0 = tcg_temp_new();
> + t1 = tcg_temp_new();
> +
> gen_icount_io_start(ctx);
> +
> if (ctx->opcode & 0x00010000) {
> /* L=1 form only updates EE and RI */
> - TCGv t0 = tcg_temp_new();
> - TCGv t1 = tcg_temp_new();
> - tcg_gen_andi_tl(t0, cpu_gpr[rS(ctx->opcode)],
> - (1 << MSR_RI) | (1 << MSR_EE));
> - tcg_gen_andi_tl(t1, cpu_msr,
> - ~(target_ulong)((1 << MSR_RI) | (1 << MSR_EE)));
> - tcg_gen_or_tl(t1, t1, t0);
> -
> - gen_helper_store_msr(cpu_env, t1);
> - tcg_temp_free(t0);
> - tcg_temp_free(t1);
> -
> + mask = (1ULL << MSR_RI) | (1ULL << MSR_EE);
> } else {
> + /* mtmsrd does not alter HV, S, ME, or LE */
> + mask = ~((1ULL << MSR_LE) | (1ULL << MSR_ME) | (1ULL << MSR_S) |
> + (1ULL << MSR_HV));
> /*
> * XXX: we need to update nip before the store if we enter
> * power saving mode, we will exit the loop directly from
> * ppc_store_msr
> */
> gen_update_nip(ctx, ctx->base.pc_next);
> - gen_helper_store_msr(cpu_env, cpu_gpr[rS(ctx->opcode)]);
> }
> +
> + tcg_gen_andi_tl(t0, cpu_gpr[rS(ctx->opcode)], mask);
> + tcg_gen_andi_tl(t1, cpu_msr, ~mask);
> + tcg_gen_or_tl(t0, t0, t1);
> +
> + gen_helper_store_msr(cpu_env, t0);
> +
> /* Must stop the translation as machine state (may have) changed */
> ctx->base.is_jmp = DISAS_EXIT_UPDATE;
> +
> + tcg_temp_free(t0);
> + tcg_temp_free(t1);
> #endif /* !defined(CONFIG_USER_ONLY) */
> }
> #endif /* defined(TARGET_PPC64) */
> @@ -4975,23 +4983,19 @@ static void gen_mtmsr(DisasContext *ctx)
> CHK_SV;
>
> #if !defined(CONFIG_USER_ONLY)
> + TCGv t0, t1;
> + target_ulong mask = 0xFFFFFFFF;
> +
> + t0 = tcg_temp_new();
> + t1 = tcg_temp_new();
> +
> gen_icount_io_start(ctx);
> if (ctx->opcode & 0x00010000) {
> /* L=1 form only updates EE and RI */
> - TCGv t0 = tcg_temp_new();
> - TCGv t1 = tcg_temp_new();
> - tcg_gen_andi_tl(t0, cpu_gpr[rS(ctx->opcode)],
> - (1 << MSR_RI) | (1 << MSR_EE));
> - tcg_gen_andi_tl(t1, cpu_msr,
> - ~(target_ulong)((1 << MSR_RI) | (1 << MSR_EE)));
> - tcg_gen_or_tl(t1, t1, t0);
> -
> - gen_helper_store_msr(cpu_env, t1);
> - tcg_temp_free(t0);
> - tcg_temp_free(t1);
> -
> + mask &= (1ULL << MSR_RI) | (1ULL << MSR_EE);
> } else {
> - TCGv msr = tcg_temp_new();
> + /* mtmsr does not alter S, ME, or LE */
> + mask &= ~((1ULL << MSR_LE) | (1ULL << MSR_ME) | (1ULL << MSR_S));
>
> /*
> * XXX: we need to update nip before the store if we enter
> @@ -4999,16 +5003,19 @@ static void gen_mtmsr(DisasContext *ctx)
> * ppc_store_msr
> */
> gen_update_nip(ctx, ctx->base.pc_next);
> -#if defined(TARGET_PPC64)
> - tcg_gen_deposit_tl(msr, cpu_msr, cpu_gpr[rS(ctx->opcode)], 0, 32);
> -#else
> - tcg_gen_mov_tl(msr, cpu_gpr[rS(ctx->opcode)]);
> -#endif
> - gen_helper_store_msr(cpu_env, msr);
> - tcg_temp_free(msr);
> }
> +
> + tcg_gen_andi_tl(t0, cpu_gpr[rS(ctx->opcode)], mask);
> + tcg_gen_andi_tl(t1, cpu_msr, ~mask);
> + tcg_gen_or_tl(t0, t0, t1);
> +
> + gen_helper_store_msr(cpu_env, t0);
> +
> /* Must stop the translation as machine state (may have) changed */
> ctx->base.is_jmp = DISAS_EXIT_UPDATE;
> +
> + tcg_temp_free(t0);
> + tcg_temp_free(t1);
> #endif
> }
>
--
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
