Re: [Qemu-devel] [PATCH RFC v4 08/20] target-arm: Store cp15 c0_c1 and c0_c2 in ARMCPUClass

[Peter Maydell]

On 15 March 2012 19:29, Alexey Starikovskiy <address@hidden> wrote:
> On Thu, Mar 15, 2012 at 11:20 PM, Peter Maydell
> <address@hidden> wrote:
>> On 15 March 2012 19:08, Paul Brook <address@hidden> wrote:
>>>> For now set them in the reset function.
>>>
>>>> +    /* TODO Move these into arm_cpu_initfn() once no longer zeroed 
>>>> above.*/
>>>> +    memcpy(env->cp15.c0_c1, klass->cp15.c0_c1, 8 * sizeof(uint32_t));
>>>> +    memcpy(env->cp15.c0_c2, klass->cp15.c0_c2, 8 * sizeof(uint32_t)); +
>>>
>>> Why bother copying them into the CPU state?  These are readonly, so anything
>>> that needs them should be able to use the value straight from the class
>>> definitions.
>>
>> In my (hugely delayed) cp15 rework attempt these probably go away anyway
>> in favour of having each CPU register a pile of registers along the lines of
>>
>>    { .name = "ID_PFR0", .cp = 15, .crn = 0, .crm = 1, .opc1 = 0, .opc2 = 0,
>>      .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = 0x00001231 },
>>    { .name = "ID_PFR1", .cp = 15, .crn = 0, .crm = 1, .opc1 = 0, .opc2 = 1,
>>      .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = 0x00000011 },
>> etc.
>>
>> I'm not sure there's any need to retain the CPUState (or equivalent) fields
>> for them at that point.
>>
>> -- PMM
>>
> Peter, with the reserved bits in most registers it might make sense to
> have resetvalue in two
> masks -- ones and zeros, so that you can prevent changing of those bits.

Well, in this case it's totally read only. Mostly QEMU at the moment doesn't
enforce selectively read-only bits (we probably should!) so I haven't
put in read-only-masks in my conversion so far, but it's an obvious
possibility.

(Current status:
http://git.linaro.org/gitweb?p=people/pmaydell/qemu-arm.git;a=shortlog;h=refs/heads/cp15-rework
still TODO:
 cp15 crn=0 conversion ;
 drop special case of mcrr in favour of explicitly implementing
 block cache ops registers ;
 reset handling of SCTLR still relies on cpu_reset_model_id ;
 arrangement of define_arm_cp_regs() calls could probably be improved.
I'm also toying with the idea of just having each CPU we support go
through and register its own cp15 registers in a big long list (ie
no attempt at factoring out "these are registers all v7 cores have",
"these are registers for feature X", etc). That sounds kind of weird
but it might help in separating out changes to migration state for
CPU A from those for CPU B.)

-- PMM