Re: [PATCH] target/ppc: BHRB avoid using host pointer in translated code

[Richard Henderson]

On 2/15/24 07:15, Nicholas Piggin wrote:
> diff --git a/target/ppc/machine.c b/target/ppc/machine.c
> index 731dd8df35..3541cd83cd 100644
> --- a/target/ppc/machine.c
> +++ b/target/ppc/machine.c
> @@ -724,7 +724,7 @@ static const VMStateDescription vmstate_bhrb = {
>       .minimum_version_id = 1,
>       .needed = bhrb_needed,
>       .fields = (VMStateField[]) {
> -        VMSTATE_UINTTL(env.bhrb_offset, PowerPCCPU),
> +        VMSTATE_UINT32(env.bhrb_offset, PowerPCCPU),

This requires a version bump.

> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 81afc892de..05f0f1ac52 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -4167,21 +4167,24 @@ static void gen_rvwinkle(DisasContext *ctx)
>   #endif /* defined(CONFIG_USER_ONLY) */
>   }
>   
> -static inline TCGv gen_write_bhrb(TCGv base, TCGv offset, TCGv mask, TCGv value)
> +static TCGv_i32 gen_write_bhrb(TCGv_i32 offset, TCGv_i32 mask, TCGv_i64 value)
>   {
> -    TCGv tmp = tcg_temp_new();
> +    TCGv_ptr ptr = tcg_temp_new_ptr();
> +    TCGv_i32 tmp = tcg_temp_new_i32();
>   
> -    /* add base and offset to get address of bhrb entry */
> -    tcg_gen_add_tl(tmp, base, offset);
> +    /* add base and offset to tcg_env to get address of bhrb entry */
> +    tcg_gen_addi_i32(tmp, offset, offsetof(CPUPPCState, bhrb));
> +    tcg_gen_ext_i32_ptr(ptr, tmp);
> +    tcg_gen_add_ptr(ptr, ptr, tcg_env);
>   
>       /* store value into bhrb at bhrb_offset */
> -    tcg_gen_st_i64(value, (TCGv_ptr)tmp, 0);
> +    tcg_gen_st_i64(value, ptr, 0);

Better to add the constant with the store offset.

  tcg_gen_ext_i32_ptr(ptr, offset);
  tcg_gen_add_ptr(ptr, ptr, tcg_env);
  tcg_gen_st_i64(value, ptr, offsetof(bhrb));


r~