Re: [PATCH-for-9.1 5/8] target/microblaze: Restrict 64-bit 'res_addr' to system emulation

[Edgar E. Iglesias]

On Tue, Mar 19, 2024 at 07:28:52AM +0100, Philippe Mathieu-Daudé wrote:
> 'res_addr' is only used in system emulation, where we have
> TARGET_LONG_BITS = 64, so we can directly use the native
> uint64_t type instead of target_ulong.


Hi Philippe,

This breaks linux-user, lwx and swx are valid user-mode insns.

Best regards,
Edgar


> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  target/microblaze/cpu.h       | 10 +++++-----
>  target/microblaze/cpu.c       |  2 ++
>  target/microblaze/machine.c   |  2 +-
>  target/microblaze/translate.c |  9 +++++++--
>  4 files changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/target/microblaze/cpu.h b/target/microblaze/cpu.h
> index c0c7574dbd..c3e2aba0ec 100644
> --- a/target/microblaze/cpu.h
> +++ b/target/microblaze/cpu.h
> @@ -260,11 +260,6 @@ struct CPUArchState {
>      /* Stack protectors. Yes, it's a hw feature.  */
>      uint32_t slr, shr;
>  
> -    /* lwx/swx reserved address */
> -#define RES_ADDR_NONE 0xffffffff /* Use 0xffffffff to indicate no 
> reservation */
> -    target_ulong res_addr;
> -    uint32_t res_val;
> -
>      /* Internal flags.  */
>  #define IMM_FLAG        (1 << 0)
>  #define BIMM_FLAG       (1 << 1)
> @@ -286,6 +281,11 @@ struct CPUArchState {
>      uint32_t iflags;
>  
>  #if !defined(CONFIG_USER_ONLY)
> +    /* lwx/swx reserved address */
> +#define RES_ADDR_NONE 0xffffffff /* Use 0xffffffff to indicate no 
> reservation */
> +    uint64_t res_addr;
> +    uint32_t res_val;
> +
>      /* Unified MMU.  */
>      MicroBlazeMMU mmu;
>  #endif
> diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
> index 96c2b71f7f..9e393cf217 100644
> --- a/target/microblaze/cpu.c
> +++ b/target/microblaze/cpu.c
> @@ -193,7 +193,9 @@ static void mb_cpu_reset_hold(Object *obj)
>      }
>  
>      memset(env, 0, offsetof(CPUMBState, end_reset_fields));
> +#ifndef CONFIG_USER_ONLY
>      env->res_addr = RES_ADDR_NONE;
> +#endif
>  
>      /* Disable stack protector.  */
>      env->shr = ~0;
> diff --git a/target/microblaze/machine.c b/target/microblaze/machine.c
> index 51705e4f5c..4daf8a2471 100644
> --- a/target/microblaze/machine.c
> +++ b/target/microblaze/machine.c
> @@ -78,7 +78,7 @@ static const VMStateField vmstate_env_fields[] = {
>      VMSTATE_UINT32(iflags, CPUMBState),
>  
>      VMSTATE_UINT32(res_val, CPUMBState),
> -    VMSTATE_UINTTL(res_addr, CPUMBState),
> +    VMSTATE_UINT64(res_addr, CPUMBState),
>  
>      VMSTATE_STRUCT(mmu, CPUMBState, 0, vmstate_mmu, MicroBlazeMMU),
>  
> diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
> index d6a42381bb..493850c544 100644
> --- a/target/microblaze/translate.c
> +++ b/target/microblaze/translate.c
> @@ -1872,7 +1872,9 @@ void mb_tcg_init(void)
>          SP(iflags),
>          SP(bvalue),
>          SP(btarget),
> +#if !defined(CONFIG_USER_ONLY)
>          SP(res_val),
> +#endif
>      };
>  
>  #undef R
> @@ -1883,6 +1885,9 @@ void mb_tcg_init(void)
>            tcg_global_mem_new_i32(tcg_env, i32s[i].ofs, i32s[i].name);
>      }
>  
> -    cpu_res_addr =
> -        tcg_global_mem_new(tcg_env, offsetof(CPUMBState, res_addr), 
> "res_addr");
> +#if !defined(CONFIG_USER_ONLY)
> +    cpu_res_addr = tcg_global_mem_new_i64(tcg_env,
> +                                          offsetof(CPUMBState, res_addr),
> +                                          "res_addr");
> +#endif
>  }
> -- 
> 2.41.0
>