diff mbox series

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

Message ID 20240319062855.8025-6-philmd@linaro.org
State New
Headers show
Series target/microblaze: Sprint housekeeping | expand

Commit Message

Philippe Mathieu-Daudé March 19, 2024, 6:28 a.m. UTC
'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.

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(-)

Comments

Anton Johansson March 19, 2024, 4:01 p.m. UTC | #1
On 19/03/24, 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.
> 
> 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
> 
Reviewed-by: Anton Johansson <anjo@rev.ng>
Edgar E. Iglesias March 19, 2024, 4:27 p.m. UTC | #2
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
>
Philippe Mathieu-Daudé March 19, 2024, 6:17 p.m. UTC | #3
On 19/3/24 17:27, Edgar E. Iglesias wrote:
> 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.

Now I see I missed cpu_res_addr. Thanks for the quick review!

> 
> 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 mbox series

Patch

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
 }