On 12/5/18 9:32 AM, Aaron Lindsay wrote:
> On Dec 05 08:43, Aaron Lindsay wrote:
>> Signed-off-by: Aaron Lindsay <aa...@os.amperecomputing.com>
>> ---
>> target/arm/cpu.h | 4 ++--
>> target/arm/helper.c | 18 ++++++++++++++++--
>> 2 files changed, 18 insertions(+), 4 deletions(-)
>>
>> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
>> index 304e6e47b3..4216fe22db 100644
>> --- a/target/arm/cpu.h
>> +++ b/target/arm/cpu.h
>> @@ -837,8 +837,8 @@ struct ARMCPU {
>> uint32_t id_pfr0;
>> uint32_t id_pfr1;
>> uint32_t id_dfr0;
>> - uint32_t pmceid0;
>> - uint32_t pmceid1;
>> + uint64_t pmceid0;
>> + uint64_t pmceid1;
>> uint32_t id_afr0;
>> uint32_t id_mmfr0;
>> uint32_t id_mmfr1;
>> diff --git a/target/arm/helper.c b/target/arm/helper.c
>> index 71be6fb578..fb6939e99c 100644
>> --- a/target/arm/helper.c
>> +++ b/target/arm/helper.c
>> @@ -5256,6 +5256,20 @@ void register_cp_regs_for_features(ARMCPU *cpu)
>> } else {
>> define_arm_cp_regs(cpu, not_v7_cp_reginfo);
>> }
>> + if (FIELD_EX32(cpu->id_dfr0, ID_DFR0, PERFMON) >= 4) {
>
> After further discussion on my last version, this should be
>
> if (FIELD_EX32(cpu->id_dfr0, ID_DFR0, PERFMON) >= 4 &&
> FIELD_EX32(cpu->id_dfr0, ID_DFR0, PERFMON) != 0xf) {
>
> to guard against defining these registers for implementation-defined
> PMUs.
When id fields define values like 0b1111, that is a hint that the field should
be interpreted as signed, and you should still use a >= comparison. (See
D12.1.4, Principles of the ID scheme for fields in ID registers.)
A patch adding
#define FIELD_EX32S(storage, reg, field) \
sextract32((storage), R_ ## reg ## _ ## field ## _SHIFT, \
R_ ## reg ## _ ## field ## _LENGTH)
#define FIELD_EX64S(storage, reg, field) \
sextract64((storage), R_ ## reg ## _ ## field ## _SHIFT, \
R_ ## reg ## _ ## field ## _LENGTH)
to include/hw/registerfields.h would be welcome and appropriate, I think.
r~
