Re: [Qemu-devel] [PATCH target-arm v1 3/9] target-arm/helper.c: define MPUIR register

[Peter Maydell]

On 2 June 2015 at 10:29, Peter Crosthwaite <address@hidden> wrote:
> On Mon, Jun 1, 2015 at 11:50 AM, Peter Maydell <address@hidden> wrote:
>> On 1 June 2015 at 19:04, Peter Crosthwaite <address@hidden> wrote:
>>> Just hardcoded to 16way unified MPU.
>>>
>>> Signed-off-by: Peter Crosthwaite <address@hidden>
>>> ---
>>>  target-arm/cpu.h    | 2 ++
>>>  target-arm/helper.c | 4 ++++
>>>  2 files changed, 6 insertions(+)
>>>
>>> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
>>> index 21b5b8e..09cc16d 100644
>>> --- a/target-arm/cpu.h
>>> +++ b/target-arm/cpu.h
>>> @@ -115,6 +115,8 @@ typedef struct ARMGenericTimer {
>>>  #define GTIMER_VIRT 1
>>>  #define NUM_GTIMERS 2
>>>
>>> +#define PMSAV7_MPU_NUM_REGIONS 16
>>> +
>>>  typedef struct {
>>>      uint64_t raw_tcr;
>>>      uint32_t mask;
>>> diff --git a/target-arm/helper.c b/target-arm/helper.c
>>> index 78b6406..cb21bbf 100644
>>> --- a/target-arm/helper.c
>>> +++ b/target-arm/helper.c
>>> @@ -3387,6 +3387,10 @@ void register_cp_regs_for_features(ARMCPU *cpu)
>>>              { .name = "TLBTR",
>>>                .cp = 15, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 3,
>>>                .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = 0 },
>>> +            { .name = "MPUIR",
>>> +              .cp = 15, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 4,
>>> +              .access = PL1_R, .type = ARM_CP_CONST,
>>> +              .resetvalue = PMSAV7_MPU_NUM_REGIONS << 8 },
>>>              REGINFO_SENTINEL
>>>          };
>>>          ARMCPRegInfo crn0_wi_reginfo = {
>>
>> Isn't this going to define the register for VMSA as well?
>>
>
> Yes. So I was going for symmetry with TLBTR which is VMSA only but
> defined for PMSA. Should we put MPUIR in the PMSA register defs?

If we have VMSA-only registers which get defined for PMSA this
is basically just a bug resulting from the fact that thus far
nobody's cared very much about PMSA cores.

In fact on the only core we have with MPU and cp15 (the 946)
the TLBTR (0, c0, c0, 3) should be an alias of the main ID
register, so having it be the TLBTR on that core is definitely
wrong. Ideally you should fish that out of the common definitions
so we only define it on VMSA cores.

-- PMM