[PATCH 0/2] target/arm: Enable FEAT_Debugv8p8 for -cpu max

[PATCH 0/2] target/arm: Enable FEAT_Debugv8p8 for -cpu max

[Gustavo Romero]

Enable FEAT_Debugv8p8 on Arm 32 and 64-bit max CPUs.

Gustavo Romero (2):
  target/arm: Move initialization of debug ID registers
  target/arm: Enable FEAT_Debugv8p8 for -cpu max

 target/arm/cpu.h       |  2 ++
 target/arm/tcg/cpu32.c | 34 +++++++++++++++++++++++++++++-----
 target/arm/tcg/cpu64.c |  9 ++++++---
 3 files changed, 37 insertions(+), 8 deletions(-)

-- 
2.34.1

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

[Gustavo Romero]

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(-)

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
-- 
2.34.1

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

[Richard Henderson]

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~

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

[Gustavo Romero]

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~

[PATCH 2/2] target/arm: Enable FEAT_Debugv8p8 for -cpu max

[Gustavo Romero]

Enable FEAT_Debugv8p8 for 32-bit and 64-bit max CPUs. This feature is
out of scope for QEMU since it concerns the external debug interface for
JTAG, but is mandatory in Armv8.8 implementations, hence it is reported
as supported in the ID registers.

Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
---
 target/arm/tcg/cpu32.c | 6 +++---
 target/arm/tcg/cpu64.c | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/target/arm/tcg/cpu32.c b/target/arm/tcg/cpu32.c
index b155a0136f..a1273a73a3 100644
--- a/target/arm/tcg/cpu32.c
+++ b/target/arm/tcg/cpu32.c
@@ -82,8 +82,8 @@ void aa32_max_features(ARMCPU *cpu)
     cpu->isar.id_pfr2 = t;
 
     t = cpu->isar.id_dfr0;
-    t = FIELD_DP32(t, ID_DFR0, COPDBG, 9);        /* FEAT_Debugv8p4 */
-    t = FIELD_DP32(t, ID_DFR0, COPSDBG, 9);       /* FEAT_Debugv8p4 */
+    t = FIELD_DP32(t, ID_DFR0, COPDBG, 10);       /* FEAT_Debugv8p8 */
+    t = FIELD_DP32(t, ID_DFR0, COPSDBG, 10);      /* FEAT_Debugv8p8 */
     t = FIELD_DP32(t, ID_DFR0, PERFMON, 6);       /* FEAT_PMUv3p5 */
     cpu->isar.id_dfr0 = t;
 
@@ -93,7 +93,7 @@ void aa32_max_features(ARMCPU *cpu)
     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, VERSION, 10);      /* FEAT_Debugv8p8 */
     t = FIELD_DP32(t, DBGDIDR, CTX_CMPS, 1);
     t = FIELD_DP32(t, DBGDIDR, BRPS, 5);
     t = FIELD_DP32(t, DBGDIDR, WRPS, 3);
diff --git a/target/arm/tcg/cpu64.c b/target/arm/tcg/cpu64.c
index 7d4b88d787..d011755753 100644
--- a/target/arm/tcg/cpu64.c
+++ b/target/arm/tcg/cpu64.c
@@ -1253,7 +1253,7 @@ void aarch64_max_tcg_initfn(Object *obj)
     cpu->isar.id_aa64zfr0 = t;
 
     t = cpu->isar.id_aa64dfr0;
-    t = FIELD_DP64(t, ID_AA64DFR0, DEBUGVER, 9);  /* FEAT_Debugv8p4 */
+    t = FIELD_DP64(t, ID_AA64DFR0, DEBUGVER, 10); /* FEAT_Debugv8p8 */
     t = FIELD_DP64(t, ID_AA64DFR0, PMUVER, 6);    /* FEAT_PMUv3p5 */
     t = FIELD_DP64(t, ID_AA64DFR0, HPMN0, 1);     /* FEAT_HPMN0 */
     cpu->isar.id_aa64dfr0 = t;
-- 
2.34.1