[PATCH v2 1/3] arm: handle demuxed ID registers

Cornelia Huck posted 3 patches 2 days, 16 hours ago
Maintainers: Peter Maydell <peter.maydell@linaro.org>, Paolo Bonzini <pbonzini@redhat.com>
[PATCH v2 1/3] arm: handle demuxed ID registers
Posted by Cornelia Huck 2 days, 16 hours ago
For some registers, we do not have a single ID register, but actually
an array of values (e.g. CCSIDR_EL1, where the actual value is
determined by whatever CSSELR_EL1 points to.) If we want to avoid
using a different way to handle registers like that for every
instance, we should provide some kind of infrastructure. Therefore,
add accessors {GET,SET}_IDREG_DEMUX that are similar to the accessors
we already use for regular ID registers.

Tested-by: Alireza Sanaee <alireza.sanaee@huawei.com>
Reviewed-by: Sebastian Ott <sebott@redhat.com>
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
 target/arm/cpu-sysregs.h | 13 +++++++++++++
 target/arm/cpu.h         | 20 ++++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/target/arm/cpu-sysregs.h b/target/arm/cpu-sysregs.h
index 7877a3b06a8e..911f54bc8a4f 100644
--- a/target/arm/cpu-sysregs.h
+++ b/target/arm/cpu-sysregs.h
@@ -35,6 +35,19 @@ typedef enum ARMSysRegs {
 
 #undef DEF
 
+/* ID registers that vary based upon another register */
+typedef enum ARMIDRegisterDemuxIdx {
+    NUM_ID_DEMUX_IDX,
+} ARMIDRegisterDemuxIdx;
+
+/*
+ * Number of register variants per demuxed register, trying to accommodate
+ * possible use cases.
+ * CCSIDR_EL1 currently needs 7*2, could be 7 more with FEAT_MTE2, in which
+ * case we would need to bump this number.
+ */
+#define ID_DEMUX_ARRAYLEN 16
+
 extern const uint32_t id_register_sysreg[NUM_ID_IDX];
 
 int get_sysreg_idx(ARMSysRegs sysreg);
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 21fee5e840b7..f9d51c0fc187 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -906,6 +906,25 @@ typedef struct {
         i_->idregs[REG ## _EL1_IDX];                                    \
     })
 
+#define SET_IDREG_DEMUX(ISAR, REG, INDEX, VALUE)                        \
+    ({                                                                  \
+        ARMISARegisters *i_ = (ISAR);                                   \
+        i_->idregs_demux[REG ## _EL1_DEMUX_IDX][INDEX] = VALUE;         \
+    })
+
+#define GET_IDREG_DEMUX(ISAR, REG, INDEX)                               \
+    ({                                                                  \
+        ARMISARegisters *i_ = (ISAR);                                   \
+        i_->idregs_demux[REG ## _EL1_DEMUX_IDX][INDEX];                 \
+    })
+
+#define COPY_IDREG_DEMUX(ISAR, REG, FROM_INDEX, TO_INDEX)               \
+    ({                                                                  \
+        ARMISARegisters *i_ = (ISAR);                                   \
+        i_->idregs_demux[REG ## _EL1_DEMUX_IDX][TO_INDEX] =             \
+            i_->idregs_demux[REG ## _EL1_DEMUX_IDX][FROM_INDEX];        \
+    })
+
 /**
  * ARMCPU:
  * @env: #CPUARMState
@@ -1084,6 +1103,7 @@ struct ArchCPU {
         uint32_t dbgdevid1;
         uint64_t reset_pmcr_el0;
         uint64_t idregs[NUM_ID_IDX];
+        uint64_t idregs_demux[NUM_ID_DEMUX_IDX][ID_DEMUX_ARRAYLEN];
     } isar;
     uint64_t midr;
     uint32_t revidr;
-- 
2.52.0
Re: [PATCH v2 1/3] arm: handle demuxed ID registers
Posted by Richard Henderson 2 days, 2 hours ago
On 2/4/26 23:32, Cornelia Huck wrote:
> For some registers, we do not have a single ID register, but actually
> an array of values (e.g. CCSIDR_EL1, where the actual value is
> determined by whatever CSSELR_EL1 points to.) If we want to avoid
> using a different way to handle registers like that for every
> instance, we should provide some kind of infrastructure. Therefore,
> add accessors {GET,SET}_IDREG_DEMUX that are similar to the accessors
> we already use for regular ID registers.
> 
> Tested-by: Alireza Sanaee <alireza.sanaee@huawei.com>
> Reviewed-by: Sebastian Ott <sebott@redhat.com>
> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> ---
>   target/arm/cpu-sysregs.h | 13 +++++++++++++
>   target/arm/cpu.h         | 20 ++++++++++++++++++++
>   2 files changed, 33 insertions(+)
> 
> diff --git a/target/arm/cpu-sysregs.h b/target/arm/cpu-sysregs.h
> index 7877a3b06a8e..911f54bc8a4f 100644
> --- a/target/arm/cpu-sysregs.h
> +++ b/target/arm/cpu-sysregs.h
> @@ -35,6 +35,19 @@ typedef enum ARMSysRegs {
>   
>   #undef DEF
>   
> +/* ID registers that vary based upon another register */
> +typedef enum ARMIDRegisterDemuxIdx {
> +    NUM_ID_DEMUX_IDX,
> +} ARMIDRegisterDemuxIdx;
> +
> +/*
> + * Number of register variants per demuxed register, trying to accommodate
> + * possible use cases.
> + * CCSIDR_EL1 currently needs 7*2, could be 7 more with FEAT_MTE2, in which
> + * case we would need to bump this number.
> + */
> +#define ID_DEMUX_ARRAYLEN 16
> +
>   extern const uint32_t id_register_sysreg[NUM_ID_IDX];
>   
>   int get_sysreg_idx(ARMSysRegs sysreg);
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 21fee5e840b7..f9d51c0fc187 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -906,6 +906,25 @@ typedef struct {
>           i_->idregs[REG ## _EL1_IDX];                                    \
>       })
>   
> +#define SET_IDREG_DEMUX(ISAR, REG, INDEX, VALUE)                        \
> +    ({                                                                  \
> +        ARMISARegisters *i_ = (ISAR);                                   \
> +        i_->idregs_demux[REG ## _EL1_DEMUX_IDX][INDEX] = VALUE;         \
> +    })
> +
> +#define GET_IDREG_DEMUX(ISAR, REG, INDEX)                               \
> +    ({                                                                  \
> +        ARMISARegisters *i_ = (ISAR);                                   \
> +        i_->idregs_demux[REG ## _EL1_DEMUX_IDX][INDEX];                 \
> +    })
> +
> +#define COPY_IDREG_DEMUX(ISAR, REG, FROM_INDEX, TO_INDEX)               \
> +    ({                                                                  \
> +        ARMISARegisters *i_ = (ISAR);                                   \
> +        i_->idregs_demux[REG ## _EL1_DEMUX_IDX][TO_INDEX] =             \
> +            i_->idregs_demux[REG ## _EL1_DEMUX_IDX][FROM_INDEX];        \
> +    })
> +
>   /**
>    * ARMCPU:
>    * @env: #CPUARMState
> @@ -1084,6 +1103,7 @@ struct ArchCPU {
>           uint32_t dbgdevid1;
>           uint64_t reset_pmcr_el0;
>           uint64_t idregs[NUM_ID_IDX];
> +        uint64_t idregs_demux[NUM_ID_DEMUX_IDX][ID_DEMUX_ARRAYLEN];

I'm not keen on this.  Surely we don't expect all multiplexed ID registers to have the 
same number of sub-registers?  A rectangular matrix isn't a great representation.  Also, 
you're wasting a hole in idregs[] with CSSIDR_EL1_IDX.

Why not just have the N sub-registers occupy N entries within the regular idregs[]? 
Something like

#define DEF_MUX(NAME, OP0, OP1, CRN, CRM, OP2, NUM) \
     NAME##_DEMUX_IDX,                               \
     NAME##_DEMUX_LAST = NAME##_DEMUX_IDX + NUM - 1,

#define SET_IDREG_DEMUX(ISAR, REG, INDEX, VALUE)                        \
     ({                                                                  \
         ARMISARegisters *i_ = (ISAR);                                   \
         i_->idregs_demux[REG ## _EL1_DEMUX_IDX + INDEX] = VALUE;        \
     })

etc.


r~