| Message ID | 20250224171444.440135-25-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | target/m68k: fpu improvements | expand |
Hi Richard, On 24/2/25 18:14, Richard Henderson wrote: > So far, this is only read-as-written. > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2497 > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > target/m68k/cpu.h | 1 + > target/m68k/cpu.c | 23 ++++++++++++++++++++++- > target/m68k/helper.c | 14 ++++++++------ > target/m68k/translate.c | 3 ++- > 4 files changed, 33 insertions(+), 8 deletions(-) > diff --git a/target/m68k/helper.c b/target/m68k/helper.c > index 6e3bb96762..bc787cbf05 100644 > --- a/target/m68k/helper.c > +++ b/target/m68k/helper.c > @@ -45,8 +45,8 @@ static int cf_fpu_gdb_get_reg(CPUState *cs, GByteArray *mem_buf, int n) > return gdb_get_reg32(mem_buf, env->fpcr); > case 9: /* fpstatus */ > return gdb_get_reg32(mem_buf, env->fpsr); > - case 10: /* fpiar, not implemented */ > - return gdb_get_reg32(mem_buf, 0); > + case 10: /* fpiar */ > + return gdb_get_reg32(mem_buf, env->fpiar); > } > return 0; > } > @@ -69,7 +69,8 @@ static int cf_fpu_gdb_set_reg(CPUState *cs, uint8_t *mem_buf, int n) > case 9: /* fpstatus */ > env->fpsr = ldl_be_p(mem_buf); > return 4; > - case 10: /* fpiar, not implemented */ > + case 10: /* fpiar */ > + env->fpiar = ldl_p(mem_buf); Should we consider target endianness? > return 4; > } > return 0; > @@ -91,8 +92,8 @@ static int m68k_fpu_gdb_get_reg(CPUState *cs, GByteArray *mem_buf, int n) > return gdb_get_reg32(mem_buf, env->fpcr); > case 9: /* fpstatus */ > return gdb_get_reg32(mem_buf, env->fpsr); > - case 10: /* fpiar, not implemented */ > - return gdb_get_reg32(mem_buf, 0); > + case 10: /* fpiar */ > + return gdb_get_reg32(mem_buf, env->fpiar); > } > return 0; > } > @@ -114,7 +115,8 @@ static int m68k_fpu_gdb_set_reg(CPUState *cs, uint8_t *mem_buf, int n) > case 9: /* fpstatus */ > env->fpsr = ldl_be_p(mem_buf); > return 4; > - case 10: /* fpiar, not implemented */ > + case 10: /* fpiar */ > + env->fpiar = ldl_p(mem_buf); (ditto) > return 4; > } > return 0;
On 3/3/25 10:39, Philippe Mathieu-Daudé wrote: > Hi Richard, > > On 24/2/25 18:14, Richard Henderson wrote: >> So far, this is only read-as-written. >> >> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2497 >> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> >> --- >> target/m68k/cpu.h | 1 + >> target/m68k/cpu.c | 23 ++++++++++++++++++++++- >> target/m68k/helper.c | 14 ++++++++------ >> target/m68k/translate.c | 3 ++- >> 4 files changed, 33 insertions(+), 8 deletions(-) > > >> diff --git a/target/m68k/helper.c b/target/m68k/helper.c >> index 6e3bb96762..bc787cbf05 100644 >> --- a/target/m68k/helper.c >> +++ b/target/m68k/helper.c >> @@ -45,8 +45,8 @@ static int cf_fpu_gdb_get_reg(CPUState *cs, GByteArray *mem_buf, int n) >> return gdb_get_reg32(mem_buf, env->fpcr); >> case 9: /* fpstatus */ >> return gdb_get_reg32(mem_buf, env->fpsr); >> - case 10: /* fpiar, not implemented */ >> - return gdb_get_reg32(mem_buf, 0); >> + case 10: /* fpiar */ >> + return gdb_get_reg32(mem_buf, env->fpiar); >> } >> return 0; >> } >> @@ -69,7 +69,8 @@ static int cf_fpu_gdb_set_reg(CPUState *cs, uint8_t *mem_buf, int n) >> case 9: /* fpstatus */ >> env->fpsr = ldl_be_p(mem_buf); >> return 4; >> - case 10: /* fpiar, not implemented */ >> + case 10: /* fpiar */ >> + env->fpiar = ldl_p(mem_buf); > > Should we consider target endianness? I am. Are you suggesting that the TARGET_BIG_ENDIAN shorthand be eliminated entirely, even from target-specific code? r~
On 3/3/25 21:40, Richard Henderson wrote: > On 3/3/25 10:39, Philippe Mathieu-Daudé wrote: >> Hi Richard, >> >> On 24/2/25 18:14, Richard Henderson wrote: >>> So far, this is only read-as-written. >>> >>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2497 >>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> >>> --- >>> target/m68k/cpu.h | 1 + >>> target/m68k/cpu.c | 23 ++++++++++++++++++++++- >>> target/m68k/helper.c | 14 ++++++++------ >>> target/m68k/translate.c | 3 ++- >>> 4 files changed, 33 insertions(+), 8 deletions(-) >> >> >>> diff --git a/target/m68k/helper.c b/target/m68k/helper.c >>> index 6e3bb96762..bc787cbf05 100644 >>> --- a/target/m68k/helper.c >>> +++ b/target/m68k/helper.c >>> @@ -45,8 +45,8 @@ static int cf_fpu_gdb_get_reg(CPUState *cs, >>> GByteArray *mem_buf, int n) >>> return gdb_get_reg32(mem_buf, env->fpcr); >>> case 9: /* fpstatus */ >>> return gdb_get_reg32(mem_buf, env->fpsr); >>> - case 10: /* fpiar, not implemented */ >>> - return gdb_get_reg32(mem_buf, 0); >>> + case 10: /* fpiar */ >>> + return gdb_get_reg32(mem_buf, env->fpiar); >>> } >>> return 0; >>> } >>> @@ -69,7 +69,8 @@ static int cf_fpu_gdb_set_reg(CPUState *cs, uint8_t >>> *mem_buf, int n) >>> case 9: /* fpstatus */ >>> env->fpsr = ldl_be_p(mem_buf); >>> return 4; >>> - case 10: /* fpiar, not implemented */ >>> + case 10: /* fpiar */ >>> + env->fpiar = ldl_p(mem_buf); >> >> Should we consider target endianness? > > I am. Are you suggesting that the TARGET_BIG_ENDIAN shorthand be > eliminated entirely, even from target-specific code? As we figured earlier, while ldl_p() is expanded as ldl_be_p() for m68k, it is easier to review using the expanded form, as done previously in this file in commit 3a76d302047: commit 3a76d302047ce4518cedef7a9feca1238a00f97b Author: Philippe Mathieu-Daudé <philmd@linaro.org> Date: Fri Oct 4 13:30:34 2024 -0300 target/m68k: Use explicit big-endian LD/ST API The M68K architecture uses big endianness. Directly use the big-endian LD/ST API. So using ldl_be_p(): Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Regards, Phil.
diff --git a/target/m68k/cpu.h b/target/m68k/cpu.h index c22d5223f0..0bb26720cf 100644 --- a/target/m68k/cpu.h +++ b/target/m68k/cpu.h @@ -110,6 +110,7 @@ typedef struct CPUArchState { uint32_t fpsr; bool fpsr_inex1; /* live only with an in-flight decimal operand */ float_status fp_status; + uint32_t fpiar; uint64_t mactmp; /* diff --git a/target/m68k/cpu.c b/target/m68k/cpu.c index 21ebc198cd..18d5e6a98c 100644 --- a/target/m68k/cpu.c +++ b/target/m68k/cpu.c @@ -412,6 +412,23 @@ static const VMStateDescription vmstate_freg = { } }; +static bool fpu_fpiar_needed(void *opaque) +{ + M68kCPU *s = opaque; + return s->env.fpiar != 0; +} + +static const VMStateDescription vmstate_fpiar = { + .name = "cpu/fpu/fpiar", + .version_id = 1, + .minimum_version_id = 1, + .needed = fpu_fpiar_needed, + .fields = (const VMStateField[]) { + VMSTATE_UINT32(env.fpiar, M68kCPU), + VMSTATE_END_OF_LIST() + } +}; + static int fpu_post_load(void *opaque, int version) { M68kCPU *s = opaque; @@ -432,7 +449,11 @@ static const VMStateDescription vmstate_fpu = { VMSTATE_STRUCT_ARRAY(env.fregs, M68kCPU, 8, 0, vmstate_freg, FPReg), VMSTATE_STRUCT(env.fp_result, M68kCPU, 0, vmstate_freg, FPReg), VMSTATE_END_OF_LIST() - } + }, + .subsections = (const VMStateDescription * const []) { + &vmstate_fpiar, + NULL + }, }; static bool cf_spregs_needed(void *opaque) diff --git a/target/m68k/helper.c b/target/m68k/helper.c index 6e3bb96762..bc787cbf05 100644 --- a/target/m68k/helper.c +++ b/target/m68k/helper.c @@ -45,8 +45,8 @@ static int cf_fpu_gdb_get_reg(CPUState *cs, GByteArray *mem_buf, int n) return gdb_get_reg32(mem_buf, env->fpcr); case 9: /* fpstatus */ return gdb_get_reg32(mem_buf, env->fpsr); - case 10: /* fpiar, not implemented */ - return gdb_get_reg32(mem_buf, 0); + case 10: /* fpiar */ + return gdb_get_reg32(mem_buf, env->fpiar); } return 0; } @@ -69,7 +69,8 @@ static int cf_fpu_gdb_set_reg(CPUState *cs, uint8_t *mem_buf, int n) case 9: /* fpstatus */ env->fpsr = ldl_be_p(mem_buf); return 4; - case 10: /* fpiar, not implemented */ + case 10: /* fpiar */ + env->fpiar = ldl_p(mem_buf); return 4; } return 0; @@ -91,8 +92,8 @@ static int m68k_fpu_gdb_get_reg(CPUState *cs, GByteArray *mem_buf, int n) return gdb_get_reg32(mem_buf, env->fpcr); case 9: /* fpstatus */ return gdb_get_reg32(mem_buf, env->fpsr); - case 10: /* fpiar, not implemented */ - return gdb_get_reg32(mem_buf, 0); + case 10: /* fpiar */ + return gdb_get_reg32(mem_buf, env->fpiar); } return 0; } @@ -114,7 +115,8 @@ static int m68k_fpu_gdb_set_reg(CPUState *cs, uint8_t *mem_buf, int n) case 9: /* fpstatus */ env->fpsr = ldl_be_p(mem_buf); return 4; - case 10: /* fpiar, not implemented */ + case 10: /* fpiar */ + env->fpiar = ldl_p(mem_buf); return 4; } return 0; diff --git a/target/m68k/translate.c b/target/m68k/translate.c index 29e64d3908..fdda7aeb99 100644 --- a/target/m68k/translate.c +++ b/target/m68k/translate.c @@ -4674,7 +4674,7 @@ static void gen_load_fcr(DisasContext *s, TCGv res, int reg) { switch (reg) { case M68K_FPIAR: - tcg_gen_movi_i32(res, 0); + tcg_gen_ld_i32(res, tcg_env, offsetof(CPUM68KState, fpiar)); break; case M68K_FPSR: tcg_gen_ld_i32(res, tcg_env, offsetof(CPUM68KState, fpsr)); @@ -4689,6 +4689,7 @@ static void gen_store_fcr(DisasContext *s, TCGv val, int reg) { switch (reg) { case M68K_FPIAR: + tcg_gen_st_i32(val, tcg_env, offsetof(CPUM68KState, fpiar)); break; case M68K_FPSR: tcg_gen_st_i32(val, tcg_env, offsetof(CPUM68KState, fpsr));
So far, this is only read-as-written. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2497 Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- target/m68k/cpu.h | 1 + target/m68k/cpu.c | 23 ++++++++++++++++++++++- target/m68k/helper.c | 14 ++++++++------ target/m68k/translate.c | 3 ++- 4 files changed, 33 insertions(+), 8 deletions(-)