| Message ID | 20240319062855.8025-6-philmd@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | target/microblaze: Sprint housekeeping | expand |
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>
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 >
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 --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 }
'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(-)