Re: [PATCH 18/61] target/arm: Remove cp argument to ENCODE_AA64_CP_REG

[Manos Pitsidianakis <manos.pitsidianakis@linaro.org>]

On Wed, 27 Aug 2025 04:04, Richard Henderson <richard.henderson@linaro.org> wrote:
>All invocations were required to pass the same value,
>CP_REG_ARM64_SYSREG_CP.  Bake that in to the result directly.
>Remove CP_REG_ARM64_SYSREG_CP as unused.
>
>Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>---

Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>

> target/arm/cpregs.h            |  5 ++---
> target/arm/kvm-consts.h        |  3 ---
> target/arm/helper.c            | 11 +++++------
> target/arm/hvf/hvf.c           |  3 +--
> target/arm/tcg/translate-a64.c |  6 ++----
> 5 files changed, 10 insertions(+), 18 deletions(-)
>
>diff --git a/target/arm/cpregs.h b/target/arm/cpregs.h
>index 7ebe404163..95b0b9c78e 100644
>--- a/target/arm/cpregs.h
>+++ b/target/arm/cpregs.h
>@@ -187,9 +187,8 @@ enum {
>      ((is64) << CP_REG_AA32_64BIT_SHIFT) |                  \
>      ((cp) << 16) | ((crn) << 11) | ((crm) << 7) | ((opc1) << 3) | (opc2))
> 
>-#define ENCODE_AA64_CP_REG(cp, crn, crm, op0, op1, op2) \
>-    (CP_REG_AA64_MASK |                                 \
>-     ((cp) << CP_REG_ARM_COPROC_SHIFT) |                \
>+#define ENCODE_AA64_CP_REG(crn, crm, op0, op1, op2) \
>+    (CP_REG_AA64_MASK | CP_REG_ARM64_SYSREG |           \
>      ((op0) << CP_REG_ARM64_SYSREG_OP0_SHIFT) |         \
>      ((op1) << CP_REG_ARM64_SYSREG_OP1_SHIFT) |         \
>      ((crn) << CP_REG_ARM64_SYSREG_CRN_SHIFT) |         \
>diff --git a/target/arm/kvm-consts.h b/target/arm/kvm-consts.h
>index fdb305eea1..54ae5da7ce 100644
>--- a/target/arm/kvm-consts.h
>+++ b/target/arm/kvm-consts.h
>@@ -160,9 +160,6 @@ MISMATCH_CHECK(QEMU_KVM_ARM_TARGET_CORTEX_A53, KVM_ARM_TARGET_CORTEX_A53);
> #define CP_REG_ARM64_SYSREG_OP2_MASK   0x0000000000000007
> #define CP_REG_ARM64_SYSREG_OP2_SHIFT  0
> 
>-/* No kernel define but it's useful to QEMU */
>-#define CP_REG_ARM64_SYSREG_CP (CP_REG_ARM64_SYSREG >> CP_REG_ARM_COPROC_SHIFT)
>-
> MISMATCH_CHECK(CP_REG_ARM64, KVM_REG_ARM64);
> MISMATCH_CHECK(CP_REG_ARM_COPROC_MASK, KVM_REG_ARM_COPROC_MASK);
> MISMATCH_CHECK(CP_REG_ARM_COPROC_SHIFT, KVM_REG_ARM_COPROC_SHIFT);
>diff --git a/target/arm/helper.c b/target/arm/helper.c
>index 268cad905f..93cae888e2 100644
>--- a/target/arm/helper.c
>+++ b/target/arm/helper.c
>@@ -4527,7 +4527,7 @@ static void define_arm_vh_e2h_redirects_aliases(ARMCPU *cpu)
>     };
> 
> #define K(op0, op1, crn, crm, op2) \
>-    ENCODE_AA64_CP_REG(CP_REG_ARM64_SYSREG_CP, crn, crm, op0, op1, op2)
>+    ENCODE_AA64_CP_REG(crn, crm, op0, op1, op2)
> 
>     static const struct E2HAlias aliases[] = {
>         { K(3, 0,  1, 0, 0), K(3, 4,  1, 0, 0), K(3, 5, 1, 0, 0),
>@@ -7297,10 +7297,9 @@ static void add_cpreg_to_hashtable(ARMCPU *cpu, const ARMCPRegInfo *r,
>          * 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);
>+        assert(cp == 0 || r->state == ARM_CP_STATE_BOTH);
>+        cp = 0;
>+        key = ENCODE_AA64_CP_REG(r->crn, crm, r->opc0, opc1, opc2);
>         break;
>     default:
>         g_assert_not_reached();
>@@ -7525,7 +7524,7 @@ void define_one_arm_cp_reg(ARMCPU *cpu, const ARMCPRegInfo *r)
>         }
>         break;
>     case ARM_CP_STATE_AA64:
>-        assert(r->cp == 0 || r->cp == CP_REG_ARM64_SYSREG_CP);
>+        assert(r->cp == 0);
>         break;
>     default:
>         g_assert_not_reached();
>diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
>index 9dffa99ed1..af03fc7fc1 100644
>--- a/target/arm/hvf/hvf.c
>+++ b/target/arm/hvf/hvf.c
>@@ -1124,8 +1124,7 @@ static bool is_id_sysreg(uint32_t reg)
> 
> static uint32_t hvf_reg2cp_reg(uint32_t reg)
> {
>-    return ENCODE_AA64_CP_REG(CP_REG_ARM64_SYSREG_CP,
>-                              (reg >> SYSREG_CRN_SHIFT) & SYSREG_CRN_MASK,
>+    return ENCODE_AA64_CP_REG((reg >> SYSREG_CRN_SHIFT) & SYSREG_CRN_MASK,
>                               (reg >> SYSREG_CRM_SHIFT) & SYSREG_CRM_MASK,
>                               (reg >> SYSREG_OP0_SHIFT) & SYSREG_OP0_MASK,
>                               (reg >> SYSREG_OP1_SHIFT) & SYSREG_OP1_MASK,
>diff --git a/target/arm/tcg/translate-a64.c b/target/arm/tcg/translate-a64.c
>index dbf47595db..743c5a10e1 100644
>--- a/target/arm/tcg/translate-a64.c
>+++ b/target/arm/tcg/translate-a64.c
>@@ -2466,8 +2466,7 @@ static void handle_sys(DisasContext *s, bool isread,
>                        unsigned int op0, unsigned int op1, unsigned int op2,
>                        unsigned int crn, unsigned int crm, unsigned int rt)
> {
>-    uint32_t key = ENCODE_AA64_CP_REG(CP_REG_ARM64_SYSREG_CP,
>-                                      crn, crm, op0, op1, op2);
>+    uint32_t key = ENCODE_AA64_CP_REG(crn, crm, op0, op1, op2);
>     const ARMCPRegInfo *ri = get_arm_cp_reginfo(s->cp_regs, key);
>     bool need_exit_tb = false;
>     bool nv_trap_to_el2 = false;
>@@ -2603,8 +2602,7 @@ static void handle_sys(DisasContext *s, bool isread,
>          * We don't use the EL1 register's access function, and
>          * fine-grained-traps on EL1 also do not apply here.
>          */
>-        key = ENCODE_AA64_CP_REG(CP_REG_ARM64_SYSREG_CP,
>-                                 crn, crm, op0, 0, op2);
>+        key = ENCODE_AA64_CP_REG(crn, crm, op0, 0, op2);
>         ri = get_arm_cp_reginfo(s->cp_regs, key);
>         assert(ri);
>         assert(cp_access_ok(s->current_el, ri, isread));
>-- 
>2.43.0
>
>