Re: [Qemu-devel] [PATCH] ARM64: support access to more performance registers in AA64 mode

[Chengyu Song]

Hi Christopher,

> On Dec 3, 2014, at 3:25 PM, Christopher Covington <address@hidden> wrote:
> On 12/03/2014 02:12 AM, Chengyu Song wrote:
>> In AA64 mode, certain system registers are access through MSR/MRS 
>> instructions instead of MCR/MRC. This patch added more such registers:
> 
> If you don't mind sharing, I'm curious what motivated this patch. I have a
> particular interest in having `perf stat -e instructions:u echo` inside
> qemu-system-aarch64 function correctly, and this looks like a good step in
> that direction.

I'm playing with the Nexus 9 kernel and it died with a undef instruction 
exception caused by those MSR/MRS instructions.

>> /* ARMv8 manual, D8.4.10 */
>> PMINTENCLR_EL1
> 
> It'd probably good to mention the version of the document.

The version I use is A.a Non-Confidential Beta, release on 04 September 2013. I 
don't have any later version.

>> +      .opc0 = 3, .crn = 9, .crm = 12, .opc1 = 3, .opc2 = 3,
> 
> It might be helpful to order the fields opc0, opc1, crn, crm, opc2 to match
> the documentation and some (most?) of the other A64 QEMU code.

Good point. Let me try to resubmit the patch.

> 
>> +      .access = PL0_RW, .fieldoffset = offsetof(CPUARMState, 
>> cp15.c9_pmovsr),
>> +      .accessfn = pmreg_access,
>> +      .writefn = pmovsr_write,
>> +      .raw_writefn = raw_write },
> 
> Should this contain .type = ARM_CP_NO_MIGRATE, if PMOVSCLR_EL0, PMOVSSET_EL0,
> and PMOVSR all refer to the same underlying variable?

I don't know. I'm not an expert here, so I simply copied from the AA32 version. 
It would be nicer if you can create a correct patch :)

> Should PMOVSSET_EL0 also be added?

PMOVSSET is not available in QEMU, so I'm not sure how it should be added

> Again, should .type = ARM_CP_NO_MIGRATE be used as this shares a variable with
> PMUSERENR?

Same as above.

> Again, should .type = ARM_CP_NO_MIGRATE be used as this shares a variable with
> PMINTSET?

Same as above.

BTW, I think the current mask (0x7E000000) in the pmccfiltr_write function is 
not right.

Thanks,
Chengyu