diff mbox series

[RFC,17/27] target/arm: Extract cpustate list manipulation to a file

Message ID 20230104215835.24692-18-farosas@suse.de (mailing list archive)
State New, archived
Headers show
Series target/arm: Allow CONFIG_TCG=n builds | expand

Commit Message

Fabiano Rosas Jan. 4, 2023, 9:58 p.m. UTC 
This code doesn't need to be buried in helper.c. Let's move it to its
own file to keep things cleaner.

Code moved verbatim.

Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
Extracted from:
https://lore.kernel.org/r/20210416162824.25131-15-cfontana@suse.de
[RFC v14 14/80] target/arm: split cpregs from tcg/helper.c
---
 target/arm/cpregs.h        |   5 ++
 target/arm/cpustate-list.c | 148 +++++++++++++++++++++++++++++++++++++
 target/arm/helper.c        | 136 ----------------------------------
 target/arm/meson.build     |   1 +
 4 files changed, 154 insertions(+), 136 deletions(-)
 create mode 100644 target/arm/cpustate-list.c

Comments

Richard Henderson Jan. 5, 2023, 4:55 a.m. UTC | #1 
On 1/4/23 13:58, Fabiano Rosas wrote:
> This code doesn't need to be buried in helper.c. Let's move it to its
> own file to keep things cleaner.
> 
> Code moved verbatim.
> 
> Signed-off-by: Fabiano Rosas<farosas@suse.de>
> ---
> Extracted from:
> https://lore.kernel.org/r/20210416162824.25131-15-cfontana@suse.de
> [RFC v14 14/80] target/arm: split cpregs from tcg/helper.c
> ---
>   target/arm/cpregs.h        |   5 ++
>   target/arm/cpustate-list.c | 148 +++++++++++++++++++++++++++++++++++++
>   target/arm/helper.c        | 136 ----------------------------------
>   target/arm/meson.build     |   1 +
>   4 files changed, 154 insertions(+), 136 deletions(-)
>   create mode 100644 target/arm/cpustate-list.c

I'd rather the new file be called cpregs.c, to match the header.
I've been thinking about moving all of that code out of helper.c...

Otherwise,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~
diff mbox series

Patch

diff --git a/target/arm/cpregs.h b/target/arm/cpregs.h
index 7e78c2c05c..1c35574102 100644
--- a/target/arm/cpregs.h
+++ b/target/arm/cpregs.h
@@ -442,6 +442,11 @@  void arm_cp_write_ignore(CPUARMState *env, const ARMCPRegInfo *ri,
 /* CPReadFn that can be used for read-as-zero behaviour */
 uint64_t arm_cp_read_zero(CPUARMState *env, const ARMCPRegInfo *ri);
 
+/*
+ * default raw read/write of coprocessor register field,
+ * behavior if no other function defined, and not const.
+ */
+uint64_t raw_read(CPUARMState *env, const ARMCPRegInfo *ri);
 /* CPWriteFn that just writes the value to ri->fieldoffset */
 void raw_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value);
 
diff --git a/target/arm/cpustate-list.c b/target/arm/cpustate-list.c
new file mode 100644
index 0000000000..9411b25b6f
--- /dev/null
+++ b/target/arm/cpustate-list.c
@@ -0,0 +1,148 @@ 
+/*
+ * ARM CPUState list read/write
+ *
+ * This code is licensed under the GNU GPL v2 or later.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "cpu.h"
+#include "cpregs.h"
+
+uint64_t raw_read(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+    assert(ri->fieldoffset);
+    if (cpreg_field_is_64bit(ri)) {
+        return CPREG_FIELD64(env, ri);
+    } else {
+        return CPREG_FIELD32(env, ri);
+    }
+}
+
+void raw_write(CPUARMState *env, const ARMCPRegInfo *ri,
+               uint64_t value)
+{
+    assert(ri->fieldoffset);
+    if (cpreg_field_is_64bit(ri)) {
+        CPREG_FIELD64(env, ri) = value;
+    } else {
+        CPREG_FIELD32(env, ri) = value;
+    }
+}
+
+const ARMCPRegInfo *get_arm_cp_reginfo(GHashTable *cpregs, uint32_t encoded_cp)
+{
+    return g_hash_table_lookup(cpregs, (gpointer)(uintptr_t)encoded_cp);
+}
+
+uint64_t read_raw_cp_reg(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+    /* Raw read of a coprocessor register (as needed for migration, etc). */
+    if (ri->type & ARM_CP_CONST) {
+        return ri->resetvalue;
+    } else if (ri->raw_readfn) {
+        return ri->raw_readfn(env, ri);
+    } else if (ri->readfn) {
+        return ri->readfn(env, ri);
+    } else {
+        return raw_read(env, ri);
+    }
+}
+
+static void write_raw_cp_reg(CPUARMState *env, const ARMCPRegInfo *ri,
+                             uint64_t v)
+{
+    /*
+     * Raw write of a coprocessor register (as needed for migration, etc).
+     * Note that constant registers are treated as write-ignored; the
+     * caller should check for success by whether a readback gives the
+     * value written.
+     */
+    if (ri->type & ARM_CP_CONST) {
+        return;
+    } else if (ri->raw_writefn) {
+        ri->raw_writefn(env, ri, v);
+    } else if (ri->writefn) {
+        ri->writefn(env, ri, v);
+    } else {
+        raw_write(env, ri, v);
+    }
+}
+
+bool write_cpustate_to_list(ARMCPU *cpu, bool kvm_sync)
+{
+    /* Write the coprocessor state from cpu->env to the (index,value) list. */
+    int i;
+    bool ok = true;
+
+    for (i = 0; i < cpu->cpreg_array_len; i++) {
+        uint32_t regidx = kvm_to_cpreg_id(cpu->cpreg_indexes[i]);
+        const ARMCPRegInfo *ri;
+        uint64_t newval;
+
+        ri = get_arm_cp_reginfo(cpu->cp_regs, regidx);
+        if (!ri) {
+            ok = false;
+            continue;
+        }
+        if (ri->type & ARM_CP_NO_RAW) {
+            continue;
+        }
+
+        newval = read_raw_cp_reg(&cpu->env, ri);
+        if (kvm_sync) {
+            /*
+             * Only sync if the previous list->cpustate sync succeeded.
+             * Rather than tracking the success/failure state for every
+             * item in the list, we just recheck "does the raw write we must
+             * have made in write_list_to_cpustate() read back OK" here.
+             */
+            uint64_t oldval = cpu->cpreg_values[i];
+
+            if (oldval == newval) {
+                continue;
+            }
+
+            write_raw_cp_reg(&cpu->env, ri, oldval);
+            if (read_raw_cp_reg(&cpu->env, ri) != oldval) {
+                continue;
+            }
+
+            write_raw_cp_reg(&cpu->env, ri, newval);
+        }
+        cpu->cpreg_values[i] = newval;
+    }
+    return ok;
+}
+
+bool write_list_to_cpustate(ARMCPU *cpu)
+{
+    int i;
+    bool ok = true;
+
+    for (i = 0; i < cpu->cpreg_array_len; i++) {
+        uint32_t regidx = kvm_to_cpreg_id(cpu->cpreg_indexes[i]);
+        uint64_t v = cpu->cpreg_values[i];
+        const ARMCPRegInfo *ri;
+
+        ri = get_arm_cp_reginfo(cpu->cp_regs, regidx);
+        if (!ri) {
+            ok = false;
+            continue;
+        }
+        if (ri->type & ARM_CP_NO_RAW) {
+            continue;
+        }
+        /*
+         * Write value and confirm it reads back as written
+         * (to catch read-only registers and partially read-only
+         * registers where the incoming migration value doesn't match)
+         */
+        write_raw_cp_reg(&cpu->env, ri, v);
+        if (read_raw_cp_reg(&cpu->env, ri) != v) {
+            ok = false;
+        }
+    }
+    return ok;
+}
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 506c057675..8361c57d4c 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -35,65 +35,11 @@ 
 
 static void switch_mode(CPUARMState *env, int mode);
 
-static uint64_t raw_read(CPUARMState *env, const ARMCPRegInfo *ri)
-{
-    assert(ri->fieldoffset);
-    if (cpreg_field_is_64bit(ri)) {
-        return CPREG_FIELD64(env, ri);
-    } else {
-        return CPREG_FIELD32(env, ri);
-    }
-}
-
-void raw_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
-{
-    assert(ri->fieldoffset);
-    if (cpreg_field_is_64bit(ri)) {
-        CPREG_FIELD64(env, ri) = value;
-    } else {
-        CPREG_FIELD32(env, ri) = value;
-    }
-}
-
 static void *raw_ptr(CPUARMState *env, const ARMCPRegInfo *ri)
 {
     return (char *)env + ri->fieldoffset;
 }
 
-uint64_t read_raw_cp_reg(CPUARMState *env, const ARMCPRegInfo *ri)
-{
-    /* Raw read of a coprocessor register (as needed for migration, etc). */
-    if (ri->type & ARM_CP_CONST) {
-        return ri->resetvalue;
-    } else if (ri->raw_readfn) {
-        return ri->raw_readfn(env, ri);
-    } else if (ri->readfn) {
-        return ri->readfn(env, ri);
-    } else {
-        return raw_read(env, ri);
-    }
-}
-
-static void write_raw_cp_reg(CPUARMState *env, const ARMCPRegInfo *ri,
-                             uint64_t v)
-{
-    /*
-     * Raw write of a coprocessor register (as needed for migration, etc).
-     * Note that constant registers are treated as write-ignored; the
-     * caller should check for success by whether a readback gives the
-     * value written.
-     */
-    if (ri->type & ARM_CP_CONST) {
-        return;
-    } else if (ri->raw_writefn) {
-        ri->raw_writefn(env, ri, v);
-    } else if (ri->writefn) {
-        ri->writefn(env, ri, v);
-    } else {
-        raw_write(env, ri, v);
-    }
-}
-
 static bool raw_accessors_invalid(const ARMCPRegInfo *ri)
 {
    /*
@@ -116,83 +62,6 @@  static bool raw_accessors_invalid(const ARMCPRegInfo *ri)
     return true;
 }
 
-bool write_cpustate_to_list(ARMCPU *cpu, bool kvm_sync)
-{
-    /* Write the coprocessor state from cpu->env to the (index,value) list. */
-    int i;
-    bool ok = true;
-
-    for (i = 0; i < cpu->cpreg_array_len; i++) {
-        uint32_t regidx = kvm_to_cpreg_id(cpu->cpreg_indexes[i]);
-        const ARMCPRegInfo *ri;
-        uint64_t newval;
-
-        ri = get_arm_cp_reginfo(cpu->cp_regs, regidx);
-        if (!ri) {
-            ok = false;
-            continue;
-        }
-        if (ri->type & ARM_CP_NO_RAW) {
-            continue;
-        }
-
-        newval = read_raw_cp_reg(&cpu->env, ri);
-        if (kvm_sync) {
-            /*
-             * Only sync if the previous list->cpustate sync succeeded.
-             * Rather than tracking the success/failure state for every
-             * item in the list, we just recheck "does the raw write we must
-             * have made in write_list_to_cpustate() read back OK" here.
-             */
-            uint64_t oldval = cpu->cpreg_values[i];
-
-            if (oldval == newval) {
-                continue;
-            }
-
-            write_raw_cp_reg(&cpu->env, ri, oldval);
-            if (read_raw_cp_reg(&cpu->env, ri) != oldval) {
-                continue;
-            }
-
-            write_raw_cp_reg(&cpu->env, ri, newval);
-        }
-        cpu->cpreg_values[i] = newval;
-    }
-    return ok;
-}
-
-bool write_list_to_cpustate(ARMCPU *cpu)
-{
-    int i;
-    bool ok = true;
-
-    for (i = 0; i < cpu->cpreg_array_len; i++) {
-        uint32_t regidx = kvm_to_cpreg_id(cpu->cpreg_indexes[i]);
-        uint64_t v = cpu->cpreg_values[i];
-        const ARMCPRegInfo *ri;
-
-        ri = get_arm_cp_reginfo(cpu->cp_regs, regidx);
-        if (!ri) {
-            ok = false;
-            continue;
-        }
-        if (ri->type & ARM_CP_NO_RAW) {
-            continue;
-        }
-        /*
-         * Write value and confirm it reads back as written
-         * (to catch read-only registers and partially read-only
-         * registers where the incoming migration value doesn't match)
-         */
-        write_raw_cp_reg(&cpu->env, ri, v);
-        if (read_raw_cp_reg(&cpu->env, ri) != v) {
-            ok = false;
-        }
-    }
-    return ok;
-}
-
 static void add_cpreg_to_list(gpointer key, gpointer opaque)
 {
     ARMCPU *cpu = opaque;
@@ -9048,11 +8917,6 @@  void modify_arm_cp_regs_with_len(ARMCPRegInfo *regs, size_t regs_len,
     }
 }
 
-const ARMCPRegInfo *get_arm_cp_reginfo(GHashTable *cpregs, uint32_t encoded_cp)
-{
-    return g_hash_table_lookup(cpregs, (gpointer)(uintptr_t)encoded_cp);
-}
-
 void arm_cp_write_ignore(CPUARMState *env, const ARMCPRegInfo *ri,
                          uint64_t value)
 {
diff --git a/target/arm/meson.build b/target/arm/meson.build
index 68a87dff0a..a42970fab8 100644
--- a/target/arm/meson.build
+++ b/target/arm/meson.build
@@ -1,6 +1,7 @@ 
 arm_ss = ss.source_set()
 arm_ss.add(files(
   'cpu.c',
+  'cpustate-list.c',
   'gdbstub.c',
   'helper.c',
   'vfp_helper.c',