While most features are now detected by probing the ID_* registers
kernels can (and do) use MIDR_EL1 for working out of they have to
apply errata. This can trip up warnings in the kernel as it tries to
work out if it should apply workarounds to features that don't
actually exist in the reported CPU type.

Avoid this problem by synthesising our own MIDR value.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

---
v2
  - don't leak QEMU version into ID reg
v3
  - move comment into one block
  - explicit setting of more fields
v4
  - minor reword of comment
---
 target/arm/cpu.h   |  6 ++++++
 target/arm/cpu64.c | 17 +++++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 94c990cddbd..67f2af0e169 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1611,6 +1611,12 @@ FIELD(V7M_FPCCR, ASPEN, 31, 1)
 /*
  * System register ID fields.
  */
+FIELD(MIDR_EL1, REVISION, 0, 4)
+FIELD(MIDR_EL1, PARTNUM, 4, 12)
+FIELD(MIDR_EL1, ARCHITECTURE, 16, 4)
+FIELD(MIDR_EL1, VARIANT, 20, 4)
+FIELD(MIDR_EL1, IMPLEMENTER, 24, 8)
+
 FIELD(ID_ISAR0, SWAP, 0, 4)
 FIELD(ID_ISAR0, BITCOUNT, 4, 4)
 FIELD(ID_ISAR0, BITFIELD, 8, 4)
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 1901997a064..511d31dce41 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -296,6 +296,23 @@ static void aarch64_max_initfn(Object *obj)
         uint32_t u;
         aarch64_a57_initfn(obj);
 
+        /*
+         * Reset MIDR so the guest doesn't mistake our 'max' CPU type for a real
+         * one and try to apply errata workarounds or use impdef features we
+         * don't provide.
+         * An IMPLEMENTER field of 0 means "reserved for software use";
+         * ARCHITECTURE must be 0xf indicating "v7 or later, check ID registers
+         * to see which features are present";
+         * the VARIANT, PARTNUM and REVISION fields are all implementation
+         * defined and we choose to define VARIANT and set the others to zero.
+         */
+        t = FIELD_DP64(0, MIDR_EL1, IMPLEMENTER, 0);
+        t = FIELD_DP64(t, MIDR_EL1, ARCHITECTURE, 0xf);
+        t = FIELD_DP64(t, MIDR_EL1, PARTNUM, 'Q');
+        t = FIELD_DP64(t, MIDR_EL1, VARIANT, 0);
+        t = FIELD_DP64(t, MIDR_EL1, REVISION, 0);
+        cpu->midr = t;
+
         t = cpu->isar.id_aa64isar0;
         t = FIELD_DP64(t, ID_AA64ISAR0, AES, 2); /* AES + PMULL */
         t = FIELD_DP64(t, ID_AA64ISAR0, SHA1, 1);
-- 
2.20.1

On Fri, 26 Jul 2019 at 11:30, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> While most features are now detected by probing the ID_* registers
> kernels can (and do) use MIDR_EL1 for working out of they have to
> apply errata. This can trip up warnings in the kernel as it tries to
> work out if it should apply workarounds to features that don't
> actually exist in the reported CPU type.
>
> Avoid this problem by synthesising our own MIDR value.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

> +        /*
> +         * Reset MIDR so the guest doesn't mistake our 'max' CPU type for a real
> +         * one and try to apply errata workarounds or use impdef features we
> +         * don't provide.
> +         * An IMPLEMENTER field of 0 means "reserved for software use";
> +         * ARCHITECTURE must be 0xf indicating "v7 or later, check ID registers
> +         * to see which features are present";
> +         * the VARIANT, PARTNUM and REVISION fields are all implementation
> +         * defined and we choose to define VARIANT and set the others to zero.
> +         */
> +        t = FIELD_DP64(0, MIDR_EL1, IMPLEMENTER, 0);
> +        t = FIELD_DP64(t, MIDR_EL1, ARCHITECTURE, 0xf);
> +        t = FIELD_DP64(t, MIDR_EL1, PARTNUM, 'Q');
> +        t = FIELD_DP64(t, MIDR_EL1, VARIANT, 0);
> +        t = FIELD_DP64(t, MIDR_EL1, REVISION, 0);

Comment still says we set VARIANT but code says we set PARTNUM...

thanks
-- PMM

On Fri, 26 Jul 2019 at 11:30, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Fri, 26 Jul 2019 at 11:30, Alex Bennée <alex.bennee@linaro.org> wrote:
> >
> > While most features are now detected by probing the ID_* registers
> > kernels can (and do) use MIDR_EL1 for working out of they have to
> > apply errata. This can trip up warnings in the kernel as it tries to
> > work out if it should apply workarounds to features that don't
> > actually exist in the reported CPU type.
> >
> > Avoid this problem by synthesising our own MIDR value.
> >
> > Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> > Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>
> > +        /*
> > +         * Reset MIDR so the guest doesn't mistake our 'max' CPU type for a real
> > +         * one and try to apply errata workarounds or use impdef features we
> > +         * don't provide.
> > +         * An IMPLEMENTER field of 0 means "reserved for software use";
> > +         * ARCHITECTURE must be 0xf indicating "v7 or later, check ID registers
> > +         * to see which features are present";
> > +         * the VARIANT, PARTNUM and REVISION fields are all implementation
> > +         * defined and we choose to define VARIANT and set the others to zero.
> > +         */
> > +        t = FIELD_DP64(0, MIDR_EL1, IMPLEMENTER, 0);
> > +        t = FIELD_DP64(t, MIDR_EL1, ARCHITECTURE, 0xf);
> > +        t = FIELD_DP64(t, MIDR_EL1, PARTNUM, 'Q');
> > +        t = FIELD_DP64(t, MIDR_EL1, VARIANT, 0);
> > +        t = FIELD_DP64(t, MIDR_EL1, REVISION, 0);
>
> Comment still says we set VARIANT but code says we set PARTNUM...

I guess we might also briefly mention why we set PARTNUM
("just in case guest code needs to distinguish this QEMU CPU from
other software implementations, though this shouldn't be needed",
or some such.)

thanks
-- PMM