Re: [Qemu-devel] [PATCH v4 04/29] target-ppc: Copy and split gen_spr_7xx() for 970

[Alexey Kardashevskiy]

On 06/04/2014 02:32 AM, Tom Musta wrote:
> On 6/3/2014 4:27 AM, Alexey Kardashevskiy wrote:
>> +static void gen_spr_book3s_pmu_user(CPUPPCState *env)
>> +{
>> +    spr_register(env, SPR_POWER_UMMCR0, "UMMCR0",
>> +                 &spr_read_ureg, SPR_NOACCESS,
>> +                 &spr_read_ureg, SPR_NOACCESS,
>> +                 0x00000000);
>> +    spr_register(env, SPR_POWER_UMMCR1, "UMMCR1",
>> +                 &spr_read_ureg, SPR_NOACCESS,
>> +                 &spr_read_ureg, SPR_NOACCESS,
>> +                 0x00000000);
>> +    spr_register(env, SPR_POWER_UPMC1, "UPMC1",
>> +                 &spr_read_ureg, SPR_NOACCESS,
>> +                 &spr_read_ureg, SPR_NOACCESS,
>> +                 0x00000000);
>> +    spr_register(env, SPR_POWER_UPMC2, "UPMC2",
>> +                 &spr_read_ureg, SPR_NOACCESS,
>> +                 &spr_read_ureg, SPR_NOACCESS,
>> +                 0x00000000);
>> +    spr_register(env, SPR_POWER_UPMC3, "UPMC3",
>> +                 &spr_read_ureg, SPR_NOACCESS,
>> +                 &spr_read_ureg, SPR_NOACCESS,
>> +                 0x00000000);
>> +    spr_register(env, SPR_POWER_UPMC4, "UPMC4",
>> +                 &spr_read_ureg, SPR_NOACCESS,
>> +                 &spr_read_ureg, SPR_NOACCESS,
>> +                 0x00000000);
>> +    spr_register(env, SPR_POWER_USIAR, "USIAR",
>> +                 &spr_read_ureg, SPR_NOACCESS,
>> +                 &spr_read_ureg, SPR_NOACCESS,
>> +                 0x00000000);
>> +}
> 
> The Uxxxx regs are writeable from supervisor state, aren't they?  (similar 
> comment as UCTRL).


Yes, they are. Will fix it.

However I did not understand the "similar comment as UCTRL" comment. UCTRL
is not writable at all.



> There is also this complicating factor in ISA 2.07 (P8) whereby the PMU Uxxxx 
> SPRs are
> readable/writeable based on the state of MMCR0[PMCC] (ick!).


I'll enable writes to Uxxxx for supermode in v5 of this patch.



> I think either of these can be handled in follow up patches.  I am also not 
> sure that I see a
> compelling reason to model the MMCR0[PMCC] accessibility unless we actually 
> start modeling the
> PMU (hard).
> 
> Reviewed-by: Tom Musta <address@hidden>
> 


-- 
Alexey