Re: [Qemu-devel] [PATCH v4 1/8] target/arm: Move some system registers into a substructure

[Richard Henderson]

On 10/19/18 5:04 AM, Philippe Mathieu-Daudé wrote:
> Hi Richard,
> 
> On 17/10/2018 00:31, Richard Henderson wrote:
>> Create struct ARMISARegisters, to be accessed during translation.
>>
>> Signed-off-by: Richard Henderson <address@hidden>
>> ---
>>  target/arm/cpu.h      |  32 ++++----
>>  hw/intc/armv7m_nvic.c |  12 +--
>>  target/arm/cpu.c      | 178 +++++++++++++++++++++---------------------
>>  target/arm/cpu64.c    |  70 ++++++++---------
>>  target/arm/helper.c   |  28 +++----
>>  5 files changed, 162 insertions(+), 158 deletions(-)
>>
>> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
>> index f00c0444c4..cff739b74d 100644
>> --- a/target/arm/cpu.h
>> +++ b/target/arm/cpu.h
>> @@ -788,13 +788,28 @@ struct ARMCPU {
>>       * ARMv7AR ARM Architecture Reference Manual. A reset_ prefix
>>       * is used for reset values of non-constant registers; no reset_
>>       * prefix means a constant register.
>> +     * Some of these registers are split out into a substructure that
>> +     * is shared with the translators to control the ISA.
>>       */
>> +    struct ARMISARegisters {
>> +        uint32_t id_isar0;
>> +        uint32_t id_isar1;
>> +        uint32_t id_isar2;
>> +        uint32_t id_isar3;
>> +        uint32_t id_isar4;
>> +        uint32_t id_isar5;
>> +        uint32_t id_isar6;
>> +        uint32_t mvfr0;
>> +        uint32_t mvfr1;
>> +        uint32_t mvfr2;
>> +        uint64_t id_aa64isar0;
>> +        uint64_t id_aa64isar1;
>> +        uint64_t id_aa64pfr0;
>> +        uint64_t id_aa64pfr1;
>> +    } isar;
> 
> I understand and agree with the change, however I find the 'isar' name
> confusing. Sadly unnamed structure is not useful here.

I assume the naming of these registers has some history within ARM, but I find
the distribution of fields between "ISA Registers" and "Processor Feature
Registers" and "Media & VFP Feature Registers" confusing, since they all have
much the same function.

I struggled with the naming myself, but couldn't find anything better than "ISA
Registers" myself.  Which they all are, really, despite the other two names.


r~