Peter Maydell <peter.mayd...@linaro.org> writes:
> On Tue, 23 Jul 2019 at 12:33, Alex Bennée <alex.ben...@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 using the >> reserved value of 0 for the implementer and telling kernels the ID >> registers should tell them everything they need to know. >> >> Signed-off-by: Alex Bennée <alex.ben...@linaro.org> >> >> --- >> v2 >> - don't leak QEMU version into ID reg >> --- >> target/arm/cpu.h | 6 ++++++ >> target/arm/cpu64.c | 6 ++++++ >> 2 files changed, 12 insertions(+) >> >> diff --git a/target/arm/cpu.h b/target/arm/cpu.h >> index 7efbb488d9d..61eaef924e4 100644 >> --- a/target/arm/cpu.h >> +++ b/target/arm/cpu.h >> @@ -1605,6 +1605,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, VARIENT, 20, 4) > > "VARIANT". > >> +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 b1bb394c6dd..e88aadfd2fd 100644 >> --- a/target/arm/cpu64.c >> +++ b/target/arm/cpu64.c >> @@ -296,6 +296,12 @@ static void aarch64_max_initfn(Object *obj) >> uint32_t u; >> aarch64_a57_initfn(obj); >> >> + /* reset MIDR so our franken-max-cpu type isn't mistaken for a real >> one */ >> + t = FIELD_DP64(0, MIDR_EL1, IMPLEMENTER, 0); /* Reserved for SW */ >> + t = FIELD_DP64(t, MIDR_EL1, ARCHITECTURE, 0xf); /* See ID_* for >> details */ >> + /* the rest is enigmatically empty lest kernels assume it means >> something */ >> + cpu->midr = t; > > I think this would be easier to read if you used one big block > comment rather than being extremely terse so as to fit the > comments on the end of the lines: > > /* > * 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 leave them all at zero. > */ > > It's also a bit inconsistent to do an explicit deposit of 0 > for the IMPLEMENTER field but not for the VARIANT/PARTNUM/REVISION. > > I wonder if we should put 0x51 (ascii 'Q') in the PARTNUM field; > then if somebody really needs to distinguish QEMU from random > other software-models they have a way to do it. Q is reserved for Qualcomm - It would be nice if ARM could assign QEMU a code but I suspect that's not part of the business model. > > thanks > -- PMM -- Alex Bennée
