[PATCH 0/2] target/m68k: add vmstate structure to migrate M68kCPU

[PATCH 0/2] target/m68k: add vmstate structure to migrate M68kCPU

[Laurent Vivier]

First patch is a cleanup patch.

The second patch defines the vmstate structure for M68kCPU.

I have tested the migration with my experimental machine virt-m68k.

I didn't check if q800 machine type has all the needed vmstates
for all the hardware devices it uses.

Thanks,
Laurent

Laurent Vivier (2):
  target/m68k: remove useless qregs array
  target/m68k: Add vmstate definition for M68kCPU

 target/m68k/cpu.h        |   5 +-
 target/m68k/cpu.c        | 193 ++++++++++++++++++++++++++++++++++++++-
 target/m68k/fpu_helper.c |  10 +-
 3 files changed, 198 insertions(+), 10 deletions(-)

-- 
2.26.2

[PATCH 1/2] target/m68k: remove useless qregs array

[Laurent Vivier]

They are unused since the target has been converted to TCG.

Fixes: e1f3808e03f7 ("Convert m68k target to TCG.")
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
 target/m68k/cpu.h | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/target/m68k/cpu.h b/target/m68k/cpu.h
index 521ac67cdd04..9a6f0400fcfe 100644
--- a/target/m68k/cpu.h
+++ b/target/m68k/cpu.h
@@ -33,8 +33,6 @@
 #define OS_PACKED   6
 #define OS_UNSIZED  7
 
-#define MAX_QREGS 32
-
 #define EXCP_ACCESS         2   /* Access (MMU) error.  */
 #define EXCP_ADDRESS        3   /* Address error.  */
 #define EXCP_ILLEGAL        4   /* Illegal instruction.  */
@@ -139,8 +137,6 @@ typedef struct CPUM68KState {
     int pending_vector;
     int pending_level;
 
-    uint32_t qregs[MAX_QREGS];
-
     /* Fields up to this point are cleared by a CPU reset */
     struct {} end_reset_fields;
 
-- 
2.26.2

Re: [PATCH 1/2] target/m68k: remove useless qregs array

[Philippe Mathieu-Daudé]

On 10/22/20 10:29 PM, Laurent Vivier wrote:
> They are unused since the target has been converted to TCG.
> 
> Fixes: e1f3808e03f7 ("Convert m68k target to TCG.")
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>   target/m68k/cpu.h | 4 ----
>   1 file changed, 4 deletions(-)
> 
> diff --git a/target/m68k/cpu.h b/target/m68k/cpu.h
> index 521ac67cdd04..9a6f0400fcfe 100644
> --- a/target/m68k/cpu.h
> +++ b/target/m68k/cpu.h
> @@ -33,8 +33,6 @@
>   #define OS_PACKED   6
>   #define OS_UNSIZED  7
>   
> -#define MAX_QREGS 32
> -
>   #define EXCP_ACCESS         2   /* Access (MMU) error.  */
>   #define EXCP_ADDRESS        3   /* Address error.  */
>   #define EXCP_ILLEGAL        4   /* Illegal instruction.  */
> @@ -139,8 +137,6 @@ typedef struct CPUM68KState {
>       int pending_vector;
>       int pending_level;
>   
> -    uint32_t qregs[MAX_QREGS];
> -
>       /* Fields up to this point are cleared by a CPU reset */
>       struct {} end_reset_fields;
>   
> 

Re: [PATCH 1/2] target/m68k: remove useless qregs array

[Thomas Huth]

On 22/10/2020 22.29, Laurent Vivier wrote:
> They are unused since the target has been converted to TCG.
> 
> Fixes: e1f3808e03f7 ("Convert m68k target to TCG.")
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> ---
>  target/m68k/cpu.h | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/target/m68k/cpu.h b/target/m68k/cpu.h
> index 521ac67cdd04..9a6f0400fcfe 100644
> --- a/target/m68k/cpu.h
> +++ b/target/m68k/cpu.h
> @@ -33,8 +33,6 @@
>  #define OS_PACKED   6
>  #define OS_UNSIZED  7
>  
> -#define MAX_QREGS 32
> -
>  #define EXCP_ACCESS         2   /* Access (MMU) error.  */
>  #define EXCP_ADDRESS        3   /* Address error.  */
>  #define EXCP_ILLEGAL        4   /* Illegal instruction.  */
> @@ -139,8 +137,6 @@ typedef struct CPUM68KState {
>      int pending_vector;
>      int pending_level;
>  
> -    uint32_t qregs[MAX_QREGS];
> -
>      /* Fields up to this point are cleared by a CPU reset */
>      struct {} end_reset_fields;

Reviewed-by: Thomas Huth <thuth@redhat.com>

There are still some references to the term qreg in translate.c:

target/m68k/translate.c:#include "qregs.def"
target/m68k/translate.c:#include "qregs.def"
target/m68k/translate.c:/* Generate a jump to the address in qreg DEST.  */

Should these get renamed?

 Thomas

Re: [PATCH 1/2] target/m68k: remove useless qregs array

[Laurent Vivier]

Le 23/10/2020 à 09:27, Thomas Huth a écrit :
> On 22/10/2020 22.29, Laurent Vivier wrote:
>> They are unused since the target has been converted to TCG.
>>
>> Fixes: e1f3808e03f7 ("Convert m68k target to TCG.")
>> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
>> ---
>>  target/m68k/cpu.h | 4 ----
>>  1 file changed, 4 deletions(-)
>>
>> diff --git a/target/m68k/cpu.h b/target/m68k/cpu.h
>> index 521ac67cdd04..9a6f0400fcfe 100644
>> --- a/target/m68k/cpu.h
>> +++ b/target/m68k/cpu.h
>> @@ -33,8 +33,6 @@
>>  #define OS_PACKED   6
>>  #define OS_UNSIZED  7
>>  
>> -#define MAX_QREGS 32
>> -
>>  #define EXCP_ACCESS         2   /* Access (MMU) error.  */
>>  #define EXCP_ADDRESS        3   /* Address error.  */
>>  #define EXCP_ILLEGAL        4   /* Illegal instruction.  */
>> @@ -139,8 +137,6 @@ typedef struct CPUM68KState {
>>      int pending_vector;
>>      int pending_level;
>>  
>> -    uint32_t qregs[MAX_QREGS];
>> -
>>      /* Fields up to this point are cleared by a CPU reset */
>>      struct {} end_reset_fields;
> 
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> 
> There are still some references to the term qreg in translate.c:
> 
> target/m68k/translate.c:#include "qregs.def"
> target/m68k/translate.c:#include "qregs.def"
> target/m68k/translate.c:/* Generate a jump to the address in qreg DEST.  */
> 
> Should these get renamed?
We always have something like qregs but they are implemented differently.
I don't know if it is worth renaming.

Thanks,
Laurent

[PATCH 2/2] target/m68k: Add vmstate definition for M68kCPU

[Laurent Vivier]

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
 target/m68k/cpu.h        |   1 +
 target/m68k/cpu.c        | 193 ++++++++++++++++++++++++++++++++++++++-
 target/m68k/fpu_helper.c |  10 +-
 3 files changed, 198 insertions(+), 6 deletions(-)

diff --git a/target/m68k/cpu.h b/target/m68k/cpu.h
index 9a6f0400fcfe..de5b9875fea3 100644
--- a/target/m68k/cpu.h
+++ b/target/m68k/cpu.h
@@ -179,6 +179,7 @@ int cpu_m68k_signal_handler(int host_signum, void *pinfo,
 uint32_t cpu_m68k_get_ccr(CPUM68KState *env);
 void cpu_m68k_set_ccr(CPUM68KState *env, uint32_t);
 void cpu_m68k_set_sr(CPUM68KState *env, uint32_t);
+void cpu_m68k_restore_fp_status(CPUM68KState *env);
 void cpu_m68k_set_fpcr(CPUM68KState *env, uint32_t val);
 
 
diff --git a/target/m68k/cpu.c b/target/m68k/cpu.c
index 72c545149e9b..b811a0bdde2d 100644
--- a/target/m68k/cpu.c
+++ b/target/m68k/cpu.c
@@ -260,10 +260,198 @@ static void m68k_cpu_initfn(Object *obj)
     cpu_set_cpustate_pointers(cpu);
 }
 
+#if defined(CONFIG_SOFTMMU)
+static bool fpu_needed(void *opaque)
+{
+    M68kCPU *s = opaque;
+
+    return m68k_feature(&s->env, M68K_FEATURE_CF_FPU) ||
+           m68k_feature(&s->env, M68K_FEATURE_FPU);
+}
+
+typedef struct m68k_FPReg_tmp {
+    FPReg *parent;
+    uint64_t tmp_mant;
+    uint16_t tmp_exp;
+} m68k_FPReg_tmp;
+
+static void cpu_get_fp80(uint64_t *pmant, uint16_t *pexp, floatx80 f)
+{
+    CPU_LDoubleU temp;
+
+    temp.d = f;
+    *pmant = temp.l.lower;
+    *pexp = temp.l.upper;
+}
+
+static floatx80 cpu_set_fp80(uint64_t mant, uint16_t upper)
+{
+    CPU_LDoubleU temp;
+
+    temp.l.upper = upper;
+    temp.l.lower = mant;
+    return temp.d;
+}
+
+static int freg_pre_save(void *opaque)
+{
+    m68k_FPReg_tmp *tmp = opaque;
+
+    cpu_get_fp80(&tmp->tmp_mant, &tmp->tmp_exp, tmp->parent->d);
+
+    return 0;
+}
+
+static int freg_post_load(void *opaque, int version)
+{
+    m68k_FPReg_tmp *tmp = opaque;
+
+    tmp->parent->d = cpu_set_fp80(tmp->tmp_mant, tmp->tmp_exp);
+
+    return 0;
+}
+
+static const VMStateDescription vmstate_freg_tmp = {
+    .name = "freg_tmp",
+    .post_load = freg_post_load,
+    .pre_save  = freg_pre_save,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT64(tmp_mant, m68k_FPReg_tmp),
+        VMSTATE_UINT16(tmp_exp, m68k_FPReg_tmp),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static const VMStateDescription vmstate_freg = {
+    .name = "freg",
+    .fields = (VMStateField[]) {
+        VMSTATE_WITH_TMP(FPReg, m68k_FPReg_tmp, vmstate_freg_tmp),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static int fpu_post_load(void *opaque, int version)
+{
+    M68kCPU *s = opaque;
+
+    cpu_m68k_restore_fp_status(&s->env);
+
+    return 0;
+}
+
+const VMStateDescription vmmstate_fpu = {
+    .name = "cpu/fpu",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = fpu_needed,
+    .post_load = fpu_post_load,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32(env.fpcr, M68kCPU),
+        VMSTATE_UINT32(env.fpsr, M68kCPU),
+        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()
+    }
+};
+
+static bool cf_spregs_needed(void *opaque)
+{
+    M68kCPU *s = opaque;
+
+    return m68k_feature(&s->env, M68K_FEATURE_CF_ISA_A);
+}
+
+const VMStateDescription vmstate_cf_spregs = {
+    .name = "cpu/cf_spregs",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = cf_spregs_needed,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT64_ARRAY(env.macc, M68kCPU, 4),
+        VMSTATE_UINT32(env.macsr, M68kCPU),
+        VMSTATE_UINT32(env.mac_mask, M68kCPU),
+        VMSTATE_UINT32(env.rambar0, M68kCPU),
+        VMSTATE_UINT32(env.mbar, M68kCPU),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static bool cpu_68040_mmu_needed(void *opaque)
+{
+    M68kCPU *s = opaque;
+
+    return m68k_feature(&s->env, M68K_FEATURE_M68040);
+}
+
+const VMStateDescription vmstate_68040_mmu = {
+    .name = "cpu/68040_mmu",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = cpu_68040_mmu_needed,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32(env.mmu.ar, M68kCPU),
+        VMSTATE_UINT32(env.mmu.ssw, M68kCPU),
+        VMSTATE_UINT16(env.mmu.tcr, M68kCPU),
+        VMSTATE_UINT32(env.mmu.urp, M68kCPU),
+        VMSTATE_UINT32(env.mmu.srp, M68kCPU),
+        VMSTATE_BOOL(env.mmu.fault, M68kCPU),
+        VMSTATE_UINT32_ARRAY(env.mmu.ttr, M68kCPU, 4),
+        VMSTATE_UINT32(env.mmu.mmusr, M68kCPU),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static bool cpu_68040_spregs_needed(void *opaque)
+{
+    M68kCPU *s = opaque;
+
+    return m68k_feature(&s->env, M68K_FEATURE_M68040);
+}
+
+const VMStateDescription vmstate_68040_spregs = {
+    .name = "cpu/68040_spregs",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = cpu_68040_spregs_needed,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32(env.vbr, M68kCPU),
+        VMSTATE_UINT32(env.cacr, M68kCPU),
+        VMSTATE_UINT32(env.sfc, M68kCPU),
+        VMSTATE_UINT32(env.dfc, M68kCPU),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static const VMStateDescription vmstate_m68k_cpu = {
     .name = "cpu",
-    .unmigratable = 1,
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields      = (VMStateField[]) {
+        VMSTATE_UINT32_ARRAY(env.dregs, M68kCPU, 8),
+        VMSTATE_UINT32_ARRAY(env.aregs, M68kCPU, 8),
+        VMSTATE_UINT32(env.pc, M68kCPU),
+        VMSTATE_UINT32(env.sr, M68kCPU),
+        VMSTATE_INT32(env.current_sp, M68kCPU),
+        VMSTATE_UINT32_ARRAY(env.sp, M68kCPU, 3),
+        VMSTATE_UINT32(env.cc_op, M68kCPU),
+        VMSTATE_UINT32(env.cc_x, M68kCPU),
+        VMSTATE_UINT32(env.cc_n, M68kCPU),
+        VMSTATE_UINT32(env.cc_v, M68kCPU),
+        VMSTATE_UINT32(env.cc_c, M68kCPU),
+        VMSTATE_UINT32(env.cc_z, M68kCPU),
+        VMSTATE_INT32(env.pending_vector, M68kCPU),
+        VMSTATE_INT32(env.pending_level, M68kCPU),
+        VMSTATE_END_OF_LIST()
+    },
+    .subsections = (const VMStateDescription * []) {
+        &vmmstate_fpu,
+        &vmstate_cf_spregs,
+        &vmstate_68040_mmu,
+        &vmstate_68040_spregs,
+        NULL
+    },
 };
+#endif
 
 static void m68k_cpu_class_init(ObjectClass *c, void *data)
 {
@@ -287,13 +475,12 @@ static void m68k_cpu_class_init(ObjectClass *c, void *data)
 #if defined(CONFIG_SOFTMMU)
     cc->do_transaction_failed = m68k_cpu_transaction_failed;
     cc->get_phys_page_debug = m68k_cpu_get_phys_page_debug;
+    dc->vmsd = &vmstate_m68k_cpu;
 #endif
     cc->disas_set_info = m68k_cpu_disas_set_info;
     cc->tcg_initialize = m68k_tcg_init;
 
     cc->gdb_num_core_regs = 18;
-
-    dc->vmsd = &vmstate_m68k_cpu;
 }
 
 static void m68k_cpu_class_init_cf_core(ObjectClass *c, void *data)
diff --git a/target/m68k/fpu_helper.c b/target/m68k/fpu_helper.c
index 9acf60dfd443..797000e7482c 100644
--- a/target/m68k/fpu_helper.c
+++ b/target/m68k/fpu_helper.c
@@ -135,10 +135,8 @@ static void restore_rounding_mode(CPUM68KState *env)
     }
 }
 
-void cpu_m68k_set_fpcr(CPUM68KState *env, uint32_t val)
+void cpu_m68k_restore_fp_status(CPUM68KState *env)
 {
-    env->fpcr = val & 0xffff;
-
     if (m68k_feature(env, M68K_FEATURE_CF_FPU)) {
         cf_restore_precision_mode(env);
     } else {
@@ -147,6 +145,12 @@ void cpu_m68k_set_fpcr(CPUM68KState *env, uint32_t val)
     restore_rounding_mode(env);
 }
 
+void cpu_m68k_set_fpcr(CPUM68KState *env, uint32_t val)
+{
+    env->fpcr = val & 0xffff;
+    cpu_m68k_restore_fp_status(env);
+}
+
 void HELPER(fitrunc)(CPUM68KState *env, FPReg *res, FPReg *val)
 {
     FloatRoundMode rounding_mode = get_float_rounding_mode(&env->fp_status);
-- 
2.26.2

Re: [PATCH 0/2] target/m68k: add vmstate structure to migrate M68kCPU

[Laurent Vivier]

Le 22/10/2020 à 22:29, Laurent Vivier a écrit :
> First patch is a cleanup patch.
> 
> The second patch defines the vmstate structure for M68kCPU.
> 
> I have tested the migration with my experimental machine virt-m68k.
> 
> I didn't check if q800 machine type has all the needed vmstates
> for all the hardware devices it uses.
> 
> Thanks,
> Laurent
> 
> Laurent Vivier (2):
>   target/m68k: remove useless qregs array
>   target/m68k: Add vmstate definition for M68kCPU
> 
>  target/m68k/cpu.h        |   5 +-
>  target/m68k/cpu.c        | 193 ++++++++++++++++++++++++++++++++++++++-
>  target/m68k/fpu_helper.c |  10 +-
>  3 files changed, 198 insertions(+), 10 deletions(-)
> 

Applied to my m68k-for-6.0 branch

Thanks,
Laurent