All of lore.kernel.org
 help / color / mirror / Atom feed
From: Richard Henderson <richard.henderson@linaro.org>
To: qemu-devel@nongnu.org
Cc: qemu-arm@nongnu.org
Subject: [PATCH v3 34/60] target/arm: Cleanup add_cpreg_to_hashtable
Date: Sun, 17 Apr 2022 10:44:00 -0700	[thread overview]
Message-ID: <20220417174426.711829-35-richard.henderson@linaro.org> (raw)
In-Reply-To: <20220417174426.711829-1-richard.henderson@linaro.org>

Use a single memory allocation for name and reginfo.
Perform the override check early; use assert not printf+abort.
Use a switch statement to validate state.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/cpu.c    |  16 +----
 target/arm/helper.c | 154 +++++++++++++++++++++++---------------------
 2 files changed, 81 insertions(+), 89 deletions(-)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index af13b34697..3da8841eb2 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1061,27 +1061,13 @@ uint64_t arm_cpu_mp_affinity(int idx, uint8_t clustersz)
     return (Aff1 << ARM_AFF1_SHIFT) | Aff0;
 }
 
-static void cpreg_hashtable_data_destroy(gpointer data)
-{
-    /*
-     * Destroy function for cpu->cp_regs hashtable data entries.
-     * We must free the name string because it was g_strdup()ed in
-     * add_cpreg_to_hashtable(). It's OK to cast away the 'const'
-     * from r->name because we know we definitely allocated it.
-     */
-    ARMCPRegInfo *r = data;
-
-    g_free((void *)r->name);
-    g_free(r);
-}
-
 static void arm_cpu_initfn(Object *obj)
 {
     ARMCPU *cpu = ARM_CPU(obj);
 
     cpu_set_cpustate_pointers(cpu);
     cpu->cp_regs = g_hash_table_new_full(g_direct_hash, g_direct_equal,
-                                         NULL, cpreg_hashtable_data_destroy);
+                                         NULL, g_free);
 
     QLIST_INIT(&cpu->pre_el_change_hooks);
     QLIST_INIT(&cpu->el_change_hooks);
diff --git a/target/arm/helper.c b/target/arm/helper.c
index db9e75a42d..562ea5c418 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -8509,37 +8509,90 @@ CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
     return cpu_list;
 }
 
+/*
+ * Private utility function for define_one_arm_cp_reg_with_opaque():
+ * add a single reginfo struct to the hash table.
+ */
 static void add_cpreg_to_hashtable(ARMCPU *cpu, const ARMCPRegInfo *r,
                                    void *opaque, CPState state,
                                    CPSecureState secstate,
                                    int crm, int opc1, int opc2,
                                    const char *name)
 {
-    /* Private utility function for define_one_arm_cp_reg_with_opaque():
-     * add a single reginfo struct to the hash table.
-     */
     uint32_t key;
-    ARMCPRegInfo *r2 = g_memdup(r, sizeof(ARMCPRegInfo));
-    int is64 = (r->type & ARM_CP_64BIT) ? 1 : 0;
-    int ns = (secstate & ARM_CP_SECSTATE_NS) ? 1 : 0;
+    ARMCPRegInfo *r2;
+    bool is64 = r->type & ARM_CP_64BIT;
+    bool ns = secstate & ARM_CP_SECSTATE_NS;
+    int cp = r->cp;
+    bool isbanked;
+    size_t name_len;
 
-    r2->name = g_strdup(name);
-    /* Reset the secure state to the specific incoming state.  This is
-     * necessary as the register may have been defined with both states.
+    switch (state) {
+    case ARM_CP_STATE_AA32:
+        /* We assume it is a cp15 register if the .cp field is left unset. */
+        if (cp == 0 && r->state == ARM_CP_STATE_BOTH) {
+            cp = 15;
+        }
+        key = ENCODE_CP_REG(cp, is64, ns, r->crn, crm, opc1, opc2);
+        break;
+    case ARM_CP_STATE_AA64:
+        /*
+         * To allow abbreviation of ARMCPRegInfo definitions, we treat
+         * cp == 0 as equivalent to the value for "standard guest-visible
+         * sysreg".  STATE_BOTH definitions are also always "standard sysreg"
+         * in their AArch64 view (the .cp value may be non-zero for the
+         * benefit of the AArch32 view).
+         */
+        if (cp == 0 || r->state == ARM_CP_STATE_BOTH) {
+            cp = CP_REG_ARM64_SYSREG_CP;
+        }
+        key = ENCODE_AA64_CP_REG(cp, r->crn, crm, r->opc0, opc1, opc2);
+        break;
+    default:
+        g_assert_not_reached();
+    }
+
+    /* Overriding of an existing definition must be explicitly requested. */
+    if (!(r->type & ARM_CP_OVERRIDE)) {
+        const ARMCPRegInfo *oldreg = get_arm_cp_reginfo(cpu->cp_regs, key);
+        if (oldreg) {
+            assert(oldreg->type & ARM_CP_OVERRIDE);
+        }
+    }
+
+    /* Combine cpreg and name into one allocation. */
+    name_len = strlen(name) + 1;
+    r2 = g_malloc(sizeof(*r2) + name_len);
+    *r2 = *r;
+    r2->name = memcpy(r2 + 1, name, name_len);
+
+    /*
+     * Update fields to match the instantiation, overwiting wildcards
+     * such as CP_ANY, ARM_CP_STATE_BOTH, or ARM_CP_SECSTATE_BOTH.
      */
+    r2->cp = cp;
+    r2->crm = crm;
+    r2->opc1 = opc1;
+    r2->opc2 = opc2;
+    r2->state = state;
     r2->secure = secstate;
+    if (opaque) {
+        r2->opaque = opaque;
+    }
 
-    if (r->bank_fieldoffsets[0] && r->bank_fieldoffsets[1]) {
-        /* Register is banked (using both entries in array).
+    isbanked = r->bank_fieldoffsets[0] && r->bank_fieldoffsets[1];
+    if (isbanked) {
+        /*
+         * Register is banked (using both entries in array).
          * Overwriting fieldoffset as the array is only used to define
          * banked registers but later only fieldoffset is used.
          */
         r2->fieldoffset = r->bank_fieldoffsets[ns];
     }
-
     if (state == ARM_CP_STATE_AA32) {
-        if (r->bank_fieldoffsets[0] && r->bank_fieldoffsets[1]) {
-            /* If the register is banked then we don't need to migrate or
+        if (isbanked) {
+            /*
+             * If the register is banked then we don't need to migrate or
              * reset the 32-bit instance in certain cases:
              *
              * 1) If the register has both 32-bit and 64-bit instances then we
@@ -8554,56 +8607,22 @@ static void add_cpreg_to_hashtable(ARMCPU *cpu, const ARMCPRegInfo *r,
                 r2->type |= ARM_CP_ALIAS;
             }
         } else if ((secstate != r->secure) && !ns) {
-            /* The register is not banked so we only want to allow migration of
-             * the non-secure instance.
+            /*
+             * The register is not banked so we only want to allow migration
+             * of the non-secure instance.
              */
             r2->type |= ARM_CP_ALIAS;
         }
 
-        if (r->state == ARM_CP_STATE_BOTH) {
-            /* We assume it is a cp15 register if the .cp field is left unset.
-             */
-            if (r2->cp == 0) {
-                r2->cp = 15;
-            }
-
+        if (r->state == ARM_CP_STATE_BOTH && r->fieldoffset) {
 #ifdef HOST_WORDS_BIGENDIAN
-            if (r2->fieldoffset) {
-                r2->fieldoffset += sizeof(uint32_t);
-            }
+            r2->fieldoffset += sizeof(uint32_t);
 #endif
         }
     }
-    if (state == ARM_CP_STATE_AA64) {
-        /* To allow abbreviation of ARMCPRegInfo
-         * definitions, we treat cp == 0 as equivalent to
-         * the value for "standard guest-visible sysreg".
-         * STATE_BOTH definitions are also always "standard
-         * sysreg" in their AArch64 view (the .cp value may
-         * be non-zero for the benefit of the AArch32 view).
-         */
-        if (r->cp == 0 || r->state == ARM_CP_STATE_BOTH) {
-            r2->cp = CP_REG_ARM64_SYSREG_CP;
-        }
-        key = ENCODE_AA64_CP_REG(r2->cp, r2->crn, crm,
-                                 r2->opc0, opc1, opc2);
-    } else {
-        key = ENCODE_CP_REG(r2->cp, is64, ns, r2->crn, crm, opc1, opc2);
-    }
-    if (opaque) {
-        r2->opaque = opaque;
-    }
-    /* reginfo passed to helpers is correct for the actual access,
-     * and is never ARM_CP_STATE_BOTH:
-     */
-    r2->state = state;
-    /* Make sure reginfo passed to helpers for wildcarded regs
-     * has the correct crm/opc1/opc2 for this reg, not CP_ANY:
-     */
-    r2->crm = crm;
-    r2->opc1 = opc1;
-    r2->opc2 = opc2;
-    /* By convention, for wildcarded registers only the first
+
+    /*
+     * By convention, for wildcarded registers only the first
      * entry is used for migration; the others are marked as
      * ALIAS so we don't try to transfer the register
      * multiple times. Special registers (ie NOP/WFI) are
@@ -8612,13 +8631,14 @@ static void add_cpreg_to_hashtable(ARMCPU *cpu, const ARMCPRegInfo *r,
     if (r->type & ARM_CP_SPECIAL_MASK) {
         r2->type |= ARM_CP_NO_RAW;
     }
-    if (((r->crm == CP_ANY) && crm != 0) ||
-        ((r->opc1 == CP_ANY) && opc1 != 0) ||
-        ((r->opc2 == CP_ANY) && opc2 != 0)) {
+    if ((r->crm == CP_ANY && crm != 0) ||
+        (r->opc1 == CP_ANY && opc1 != 0) ||
+        (r->opc2 == CP_ANY && opc2 != 0)) {
         r2->type |= ARM_CP_ALIAS | ARM_CP_NO_GDB;
     }
 
-    /* Check that raw accesses are either forbidden or handled. Note that
+    /*
+     * Check that raw accesses are either forbidden or handled. Note that
      * we can't assert this earlier because the setup of fieldoffset for
      * banked registers has to be done first.
      */
@@ -8626,20 +8646,6 @@ static void add_cpreg_to_hashtable(ARMCPU *cpu, const ARMCPRegInfo *r,
         assert(!raw_accessors_invalid(r2));
     }
 
-    /* Overriding of an existing definition must be explicitly
-     * requested.
-     */
-    if (!(r->type & ARM_CP_OVERRIDE)) {
-        const ARMCPRegInfo *oldreg = get_arm_cp_reginfo(cpu->cp_regs, key);
-        if (oldreg && !(oldreg->type & ARM_CP_OVERRIDE)) {
-            fprintf(stderr, "Register redefined: cp=%d %d bit "
-                    "crn=%d crm=%d opc1=%d opc2=%d, "
-                    "was %s, now %s\n", r2->cp, 32 + 32 * is64,
-                    r2->crn, r2->crm, r2->opc1, r2->opc2,
-                    oldreg->name, r2->name);
-            g_assert_not_reached();
-        }
-    }
     g_hash_table_insert(cpu->cp_regs, (gpointer)(uintptr_t)key, r2);
 }
 
-- 
2.25.1



  parent reply	other threads:[~2022-04-17 18:28 UTC|newest]

Thread overview: 121+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-17 17:43 [PATCH v3 00/60] target/arm: Cleanups, new features, new cpus Richard Henderson
2022-04-17 17:43 ` [PATCH v3 01/60] tcg: Add tcg_constant_ptr Richard Henderson
2022-04-19 10:41   ` Alex Bennée
2022-04-17 17:43 ` [PATCH v3 02/60] target/arm: Update ISAR fields for ARMv8.8 Richard Henderson
2022-04-19 11:10   ` Alex Bennée
2022-04-17 17:43 ` [PATCH v3 03/60] target/arm: Update SCR_EL3 bits to ARMv8.8 Richard Henderson
2022-04-19 11:13   ` Alex Bennée
2022-04-19 11:14   ` Alex Bennée
2022-04-17 17:43 ` [PATCH v3 04/60] target/arm: Update SCTLR bits to ARMv9.2 Richard Henderson
2022-04-19 11:16   ` Alex Bennée
2022-04-17 17:43 ` [PATCH v3 05/60] target/arm: Change DisasContext.aarch64 to bool Richard Henderson
2022-04-19 11:16   ` Alex Bennée
2022-04-17 17:43 ` [PATCH v3 06/60] target/arm: Change CPUArchState.aarch64 " Richard Henderson
2022-04-19 11:17   ` Alex Bennée
2022-04-17 17:43 ` [PATCH v3 07/60] target/arm: Extend store_cpu_offset to take field size Richard Henderson
2022-04-21 16:15   ` Peter Maydell
2022-04-22 13:58   ` Alex Bennée
2022-04-17 17:43 ` [PATCH v3 08/60] target/arm: Change DisasContext.thumb to bool Richard Henderson
2022-04-21 16:15   ` Peter Maydell
2022-04-22 13:59   ` Alex Bennée
2022-04-22 14:04     ` Peter Maydell
2022-04-22 15:24       ` Richard Henderson
2022-04-17 17:43 ` [PATCH v3 09/60] target/arm: Change CPUArchState.thumb " Richard Henderson
2022-04-21 16:18   ` Peter Maydell
2022-04-17 17:43 ` [PATCH v3 10/60] target/arm: Remove fpexc32_access Richard Henderson
2022-04-21 16:25   ` Peter Maydell
2022-04-17 17:43 ` [PATCH v3 11/60] target/arm: Split out set_btype_raw Richard Henderson
2022-04-21 16:27   ` Peter Maydell
2022-04-17 17:43 ` [PATCH v3 12/60] target/arm: Split out gen_rebuild_hflags Richard Henderson
2022-04-21 18:47   ` Peter Maydell
2022-04-17 17:43 ` [PATCH v3 13/60] target/arm: Use tcg_constant in translate-a64.c Richard Henderson
2022-04-21 18:49   ` Peter Maydell
2022-04-17 17:43 ` [PATCH v3 14/60] target/arm: Simplify GEN_SHIFT in translate.c Richard Henderson
2022-04-21 18:56   ` Peter Maydell
2022-04-17 17:43 ` [PATCH v3 15/60] target/arm: Simplify gen_sar Richard Henderson
2022-04-21 18:57   ` Peter Maydell
2022-04-17 17:43 ` [PATCH v3 16/60] target/arm: Simplify aa32 DISAS_WFI Richard Henderson
2022-04-21 19:00   ` Peter Maydell
2022-04-17 17:43 ` [PATCH v3 17/60] target/arm: Use tcg_constant in translate.c Richard Henderson
2022-04-21 19:00   ` Peter Maydell
2022-04-17 17:43 ` [PATCH v3 18/60] target/arm: Use tcg_constant in translate-m-nocp.c Richard Henderson
2022-04-21 19:03   ` Peter Maydell
2022-04-21 21:37     ` Richard Henderson
2022-04-17 17:43 ` [PATCH v3 19/60] target/arm: Use tcg_constant in translate-neon.c Richard Henderson
2022-04-21 19:06   ` Peter Maydell
2022-04-17 17:43 ` [PATCH v3 20/60] target/arm: Use smin/smax for do_sat_addsub_32 Richard Henderson
2022-04-21 19:07   ` Peter Maydell
2022-04-17 17:43 ` [PATCH v3 21/60] target/arm: Use tcg_constant in translate-sve.c Richard Henderson
2022-04-21 19:08   ` Peter Maydell
2022-04-17 17:43 ` [PATCH v3 22/60] target/arm: Use tcg_constant in translate-vfp.c Richard Henderson
2022-04-21 19:10   ` Peter Maydell
2022-04-17 17:43 ` [PATCH v3 23/60] target/arm: Use tcg_constant_i32 in translate.h Richard Henderson
2022-04-21 19:11   ` Peter Maydell
2022-04-17 17:43 ` [PATCH v3 24/60] target/arm: Split out cpregs.h Richard Henderson
2022-04-21 19:14   ` Peter Maydell
2022-04-22 15:21   ` Alex Bennée
2022-04-17 17:43 ` [PATCH v3 25/60] target/arm: Reorg CPAccessResult and access_check_cp_reg Richard Henderson
2022-04-22  9:32   ` Peter Maydell
2022-04-22 15:31   ` Alex Bennée
2022-04-17 17:43 ` [PATCH v3 26/60] target/arm: Replace sentinels with ARRAY_SIZE in cpregs.h Richard Henderson
2022-04-22  9:37   ` Peter Maydell
2022-04-22 10:39     ` Richard Henderson
2022-04-22 15:36       ` Alex Bennée
2022-05-01  0:10         ` Richard Henderson
2022-04-17 17:43 ` [PATCH v3 27/60] target/arm: Make some more cpreg data static const Richard Henderson
2022-04-22  9:38   ` Peter Maydell
2022-04-22 15:38   ` Alex Bennée
2022-04-17 17:43 ` [PATCH v3 28/60] target/arm: Reorg ARMCPRegInfo type field bits Richard Henderson
2022-04-22  9:49   ` Peter Maydell
2022-04-17 17:43 ` [PATCH v3 29/60] target/arm: Change cpreg access permissions to enum Richard Henderson
2022-04-22  9:52   ` Peter Maydell
2022-04-17 17:43 ` [PATCH v3 30/60] target/arm: Name CPState type Richard Henderson
2022-04-22  9:53   ` Peter Maydell
2022-04-22 15:51   ` Alex Bennée
2022-04-17 17:43 ` [PATCH v3 31/60] target/arm: Name CPSecureState type Richard Henderson
2022-04-22  9:57   ` Peter Maydell
2022-04-17 17:43 ` [PATCH v3 32/60] target/arm: Update sysreg fields when redirecting for E2H Richard Henderson
2022-04-22 10:39   ` Peter Maydell
2022-05-01  1:03     ` Richard Henderson
2022-04-17 17:43 ` [PATCH v3 33/60] target/arm: Store cpregs key in the hash table directly Richard Henderson
2022-04-22 10:46   ` Peter Maydell
2022-04-17 17:44 ` Richard Henderson [this message]
2022-04-22 10:48   ` [PATCH v3 34/60] target/arm: Cleanup add_cpreg_to_hashtable Peter Maydell
2022-04-17 17:44 ` [PATCH v3 35/60] target/arm: Handle cpreg registration for missing EL Richard Henderson
2022-04-22 10:57   ` Peter Maydell
2022-04-26  9:40     ` Peter Maydell
2022-04-26 15:31     ` Peter Maydell
2022-04-17 17:44 ` [PATCH v3 36/60] target/arm: Drop EL3 no EL2 fallbacks Richard Henderson
2022-04-17 17:44 ` [PATCH v3 37/60] target/arm: Merge zcr reginfo Richard Henderson
2022-04-17 17:44 ` [PATCH v3 38/60] target/arm: Add isar predicates for FEAT_Debugv8p2 Richard Henderson
2022-04-17 17:44 ` [PATCH v3 39/60] target/arm: Adjust definition of CONTEXTIDR_EL2 Richard Henderson
2022-04-17 17:44 ` [PATCH v3 40/60] target/arm: Move cortex impdef sysregs to cpu_tcg.c Richard Henderson
2022-04-22 11:01   ` Peter Maydell
2022-04-17 17:44 ` [PATCH v3 41/60] target/arm: Update qemu-system-arm -cpu max to cortex-a57 Richard Henderson
2022-04-22 11:02   ` Peter Maydell
2022-04-17 17:44 ` [PATCH v3 42/60] target/arm: Set ID_DFR0.PerfMon for qemu-system-arm -cpu max Richard Henderson
2022-04-17 17:44 ` [PATCH v3 43/60] target/arm: Split out aa32_max_features Richard Henderson
2022-04-22 11:03   ` Peter Maydell
2022-04-17 17:44 ` [PATCH v3 44/60] target/arm: Annotate arm_max_initfn with FEAT identifiers Richard Henderson
2022-04-17 17:44 ` [PATCH v3 45/60] target/arm: Use field names for manipulating EL2 and EL3 modes Richard Henderson
2022-04-17 17:44 ` [PATCH v3 46/60] target/arm: Enable FEAT_Debugv8p2 for -cpu max Richard Henderson
2022-04-17 17:44 ` [PATCH v3 47/60] target/arm: Enable FEAT_Debugv8p4 " Richard Henderson
2022-04-17 17:44 ` [PATCH v3 48/60] target/arm: Add isar_feature_{aa64,any}_ras Richard Henderson
2022-04-17 17:44 ` [PATCH v3 49/60] target/arm: Add minimal RAS registers Richard Henderson
2022-04-17 17:44 ` [PATCH v3 50/60] target/arm: Enable SCR and HCR bits for RAS Richard Henderson
2022-04-17 17:44 ` [PATCH v3 51/60] target/arm: Implement virtual SError exceptions Richard Henderson
2022-04-22 11:06   ` Peter Maydell
2022-04-17 17:44 ` [PATCH v3 52/60] target/arm: Implement ESB instruction Richard Henderson
2022-04-17 17:44 ` [PATCH v3 53/60] target/arm: Enable FEAT_RAS for -cpu max Richard Henderson
2022-04-17 17:44 ` [PATCH v3 54/60] target/arm: Enable FEAT_IESB " Richard Henderson
2022-04-17 17:44 ` [PATCH v3 55/60] target/arm: Enable FEAT_CSV2 " Richard Henderson
2022-04-17 17:44 ` [PATCH v3 56/60] target/arm: Enable FEAT_CSV2_2 " Richard Henderson
2022-04-29  9:52   ` Damien Hedde
2022-04-29 18:06     ` Richard Henderson
2022-04-17 17:44 ` [PATCH v3 57/60] target/arm: Enable FEAT_CSV3 " Richard Henderson
2022-04-17 17:44 ` [PATCH v3 58/60] target/arm: Enable FEAT_DGH " Richard Henderson
2022-04-17 17:44 ` [PATCH v3 59/60] target/arm: Define cortex-a76 Richard Henderson
2022-04-22 11:08   ` Peter Maydell
2022-04-17 17:44 ` [PATCH v3 60/60] target/arm: Define neoverse-n1 Richard Henderson
2022-04-22 11:08   ` Peter Maydell
2022-04-22  9:01 ` [PATCH v3 00/60] target/arm: Cleanups, new features, new cpus Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220417174426.711829-35-richard.henderson@linaro.org \
    --to=richard.henderson@linaro.org \
    --cc=qemu-arm@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.