diff mbox series

[1/2] target/arm: Move initialization of debug ID registers

Message ID 20240620181352.3590086-2-gustavo.romero@linaro.org
State Superseded
Headers show
Series target/arm: Enable FEAT_Debugv8p8 for -cpu max | expand

Commit Message

Gustavo Romero June 20, 2024, 6:13 p.m. UTC 
Move the initialization of the debug ID registers to aa32_max_features,
which is used to set the 32-bit ID registers for both 32-bit and 64-bit
max CPUs. This ensures that the debug ID registers are consistently set
for both max CPUs in a single place.

Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
---
 target/arm/cpu.h       |  2 ++
 target/arm/tcg/cpu32.c | 30 +++++++++++++++++++++++++++---
 target/arm/tcg/cpu64.c |  7 +++++--
 3 files changed, 34 insertions(+), 5 deletions(-)

Comments

Richard Henderson June 20, 2024, 6:21 p.m. UTC | #1 
On 6/20/24 11:13, Gustavo Romero wrote:
> @@ -1268,7 +1268,10 @@ void aarch64_max_tcg_initfn(Object *obj)
>       t = FIELD_DP64(t, ID_AA64SMFR0, FA64, 1);     /* FEAT_SME_FA64 */
>       cpu->isar.id_aa64smfr0 = t;
>   
> -    /* Replicate the same data to the 32-bit id registers.  */
> +    /*
> +     * Replicate the same values from the 32-bit max CPU to the 32-bit ID
> +     * registers.
> +     */
>       aa32_max_features(cpu);

I think the previous comment is more accurate.

There is no separate "32-bit max CPU". There is one "max CPU", which supports both 32-bit 
and 64-bit modes, and thus has both 32-bit and 64-bit ID registers.

The rest of the patch looks good.


r~
Gustavo Romero June 21, 2024, 2:41 p.m. UTC | #2 
Hi Richard,

On 6/20/24 3:21 PM, Richard Henderson wrote:
> On 6/20/24 11:13, Gustavo Romero wrote:
>> @@ -1268,7 +1268,10 @@ void aarch64_max_tcg_initfn(Object *obj)
>>       t = FIELD_DP64(t, ID_AA64SMFR0, FA64, 1);     /* FEAT_SME_FA64 */
>>       cpu->isar.id_aa64smfr0 = t;
>> -    /* Replicate the same data to the 32-bit id registers.  */
>> +    /*
>> +     * Replicate the same values from the 32-bit max CPU to the 32-bit ID
>> +     * registers.
>> +     */
>>       aa32_max_features(cpu);
> 
> I think the previous comment is more accurate.
> 
> There is no separate "32-bit max CPU". There is one "max CPU", which supports both 32-bit and 64-bit modes, and thus has both 32-bit and 64-bit ID registers.
I see. In v2 I reverted to the previous comment. Thanks a lot for the review.


Cheers,
Gustavo

> The rest of the patch looks good.
> 
> 
> r~
diff mbox series

Patch

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 3841359d0f..d8eb986a04 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -2299,6 +2299,8 @@  FIELD(DBGDEVID, DOUBLELOCK, 20, 4)
 FIELD(DBGDEVID, AUXREGS, 24, 4)
 FIELD(DBGDEVID, CIDMASK, 28, 4)
 
+FIELD(DBGDEVID1, PCSROFFSET, 0, 4)
+
 FIELD(MVFR0, SIMDREG, 0, 4)
 FIELD(MVFR0, FPSP, 4, 4)
 FIELD(MVFR0, FPDP, 8, 4)
diff --git a/target/arm/tcg/cpu32.c b/target/arm/tcg/cpu32.c
index bdd82d912a..b155a0136f 100644
--- a/target/arm/tcg/cpu32.c
+++ b/target/arm/tcg/cpu32.c
@@ -87,6 +87,33 @@  void aa32_max_features(ARMCPU *cpu)
     t = FIELD_DP32(t, ID_DFR0, PERFMON, 6);       /* FEAT_PMUv3p5 */
     cpu->isar.id_dfr0 = t;
 
+    /* Debug ID registers. */
+
+    /* Bit[15] is RES1, Bit[13] and Bits[11:0] are RES0. */
+    t = 0x00008000;
+    t = FIELD_DP32(t, DBGDIDR, SE_IMP, 1);
+    t = FIELD_DP32(t, DBGDIDR, NSUHD_IMP, 1);
+    t = FIELD_DP32(t, DBGDIDR, VERSION, 6);       /* Armv8 debug */
+    t = FIELD_DP32(t, DBGDIDR, CTX_CMPS, 1);
+    t = FIELD_DP32(t, DBGDIDR, BRPS, 5);
+    t = FIELD_DP32(t, DBGDIDR, WRPS, 3);
+    cpu->isar.dbgdidr = t;
+
+    t = FIELD_DP32(t, DBGDEVID, PCSAMPLE, 3);
+    t = FIELD_DP32(t, DBGDEVID, WPADDRMASK, 1);
+    t = FIELD_DP32(t, DBGDEVID, BPADDRMASK, 15);
+    t = FIELD_DP32(t, DBGDEVID, VECTORCATCH, 0);
+    t = FIELD_DP32(t, DBGDEVID, VIRTEXTNS, 1);
+    t = FIELD_DP32(t, DBGDEVID, DOUBLELOCK, 1);
+    t = FIELD_DP32(t, DBGDEVID, AUXREGS, 0);
+    t = FIELD_DP32(t, DBGDEVID, CIDMASK, 0);
+    cpu->isar.dbgdevid = t;
+
+    /* Bits[31:4] are RES0. */
+    t = 0;
+    t = FIELD_DP32(t, DBGDEVID1, PCSROFFSET, 2);
+    cpu->isar.dbgdevid1 = t;
+
     t = cpu->isar.id_dfr1;
     t = FIELD_DP32(t, ID_DFR1, HPMN0, 1);         /* FEAT_HPMN0 */
     cpu->isar.id_dfr1 = t;
@@ -955,9 +982,6 @@  static void arm_max_initfn(Object *obj)
     cpu->isar.id_isar4 = 0x00011142;
     cpu->isar.id_isar5 = 0x00011121;
     cpu->isar.id_isar6 = 0;
-    cpu->isar.dbgdidr = 0x3516d000;
-    cpu->isar.dbgdevid = 0x00110f13;
-    cpu->isar.dbgdevid1 = 0x2;
     cpu->isar.reset_pmcr_el0 = 0x41013000;
     cpu->clidr = 0x0a200023;
     cpu->ccsidr[0] = 0x701fe00a; /* 32KB L1 dcache */
diff --git a/target/arm/tcg/cpu64.c b/target/arm/tcg/cpu64.c
index 0899251eef..7d4b88d787 100644
--- a/target/arm/tcg/cpu64.c
+++ b/target/arm/tcg/cpu64.c
@@ -1167,7 +1167,7 @@  void aarch64_max_tcg_initfn(Object *obj)
 
     t = cpu->isar.id_aa64isar2;
     t = FIELD_DP64(t, ID_AA64ISAR2, MOPS, 1);     /* FEAT_MOPS */
-    t = FIELD_DP64(t, ID_AA64ISAR2, BC, 1);      /* FEAT_HBC */
+    t = FIELD_DP64(t, ID_AA64ISAR2, BC, 1);       /* FEAT_HBC */
     t = FIELD_DP64(t, ID_AA64ISAR2, WFXT, 2);     /* FEAT_WFxT */
     cpu->isar.id_aa64isar2 = t;
 
@@ -1268,7 +1268,10 @@  void aarch64_max_tcg_initfn(Object *obj)
     t = FIELD_DP64(t, ID_AA64SMFR0, FA64, 1);     /* FEAT_SME_FA64 */
     cpu->isar.id_aa64smfr0 = t;
 
-    /* Replicate the same data to the 32-bit id registers.  */
+    /*
+     * Replicate the same values from the 32-bit max CPU to the 32-bit ID
+     * registers.
+     */
     aa32_max_features(cpu);
 
 #ifdef CONFIG_USER_ONLY