[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-arm] [PATCH for 4.2] target/arm: generate a custom MIDR for -c
From: |
Alex Bennée |
Subject: |
Re: [Qemu-arm] [PATCH for 4.2] target/arm: generate a custom MIDR for -cpu max |
Date: |
Fri, 26 Jul 2019 08:24:04 +0100 |
User-agent: |
mu4e 1.3.3; emacs 27.0.50 |
Peter Maydell <address@hidden> writes:
> On Tue, 23 Jul 2019 at 12:33, Alex Bennée <address@hidden> 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 <address@hidden>
>>
>> ---
>> 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