1. Patchew
  2. QEMU
  3. [PATCH v2 0/2] target/arm: Enable FEAT_Debugv8p8 for -cpu max
  4. View patch

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

Gustavo Romero posted 2 patches 1 year, 7 months ago
Maintainers: Peter Maydell <peter.maydell@linaro.org>
There is a newer version of this series
[PATCH v2 1/2] target/arm: Move initialization of debug ID registers
Posted by Gustavo Romero 1 year, 7 months ago 
Move the initialization of the debug ID registers to aa32_max_features,
which is used to set the 32-bit ID registers. This ensures that the
debug ID registers are consistently set for the max CPU 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 |  2 +-
 3 files changed, 30 insertions(+), 4 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..71e1bfcd4e 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;
 
-- 
2.34.1
Re: [PATCH v2 1/2] target/arm: Move initialization of debug ID registers
Posted by Peter Maydell 1 year, 7 months ago 
On Fri, 21 Jun 2024 at 15:39, Gustavo Romero <gustavo.romero@linaro.org> wrote:
>
> Move the initialization of the debug ID registers to aa32_max_features,
> which is used to set the 32-bit ID registers. This ensures that the
> debug ID registers are consistently set for the max CPU 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 |  2 +-
>  3 files changed, 30 insertions(+), 4 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);

Looks like we should have an initial "t = something" ("t = 0")
before this line? Otherwise we start the DBGDEVID value
with the value of DBGDIDR.

> +    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..71e1bfcd4e 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;

This indent fixup is unrelated so should really be its own patch.

thanks
-- PMM
Re: [PATCH v2 1/2] target/arm: Move initialization of debug ID registers
Posted by Gustavo Romero 1 year, 7 months ago 
Hi Peter!

On 6/24/24 10:26 AM, Peter Maydell wrote:
> On Fri, 21 Jun 2024 at 15:39, Gustavo Romero <gustavo.romero@linaro.org> wrote:
>>
>> Move the initialization of the debug ID registers to aa32_max_features,
>> which is used to set the 32-bit ID registers. This ensures that the
>> debug ID registers are consistently set for the max CPU 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 |  2 +-
>>   3 files changed, 30 insertions(+), 4 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);
> 
> Looks like we should have an initial "t = something" ("t = 0")
> before this line? Otherwise we start the DBGDEVID value
> with the value of DBGDIDR.

I'm setting all the 32 bits here so there is no bit left from previous t value.

But I agree it's better to be explicit here for readability. Done in v3.


>> +    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..71e1bfcd4e 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;
> 
> This indent fixup is unrelated so should really be its own patch.

Got it. Done in v3 so.


Cheers,
Gustavo

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.