Re: [Qemu-devel] [PATCH v2 1/5] target-arm: Add the pmceid0 and pmceid1 registers

2016-02-16 Thread Peter Maydell 
On 10 February 2016 at 13:52, Aaron Lindsay  wrote:
> On Feb 09 15:11, Alistair Francis wrote:
>> On Tue, Feb 9, 2016 at 9:19 AM, Peter Maydell  
>> wrote:
>> > On 6 February 2016 at 00:55, Alistair Francis
>> >  wrote:
>> >> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
>> >> index 7ddbf3d..937f845 100644
>> >> --- a/target-arm/cpu.c
>> >> +++ b/target-arm/cpu.c
>> >> @@ -1156,6 +1156,8 @@ static void cortex_a15_initfn(Object *obj)
>> >>  cpu->id_pfr0 = 0x1131;
>> >>  cpu->id_pfr1 = 0x00011011;
>> >>  cpu->id_dfr0 = 0x02010555;
>> >> +cpu->pmceid0 = 0x0481; /* PMUv3 events 0x0, 0x8, and 0x11 */
>> >
>> > These are:
>> >  SW_INCR   # insn architecturally executed, cc pass, software increment
>> >  INST_RETIRED # insn architecturally executed
>> >  CPU_CYCLES # cycle
>> >
>> > However we don't actually implement any of these, so should
>> > we be advertising them?
>>
>> So this part I took directly from Chris's RFC. I'm happy to take it
>> out if you would like.
>
> I think removing the PMCEID0 change makes sense since these patches
> don't implement the advertised counters. We have other patches which do
> implement them, but they need some more work, so we can make this change
> if/when they're actually implemented.

I agree, so I propose to take Alistair's v3 series into target-arm.next
with the following change:

diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 1203783..e95b030 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -1156,7 +1156,7 @@ static void cortex_a15_initfn(Object *obj)
 cpu->id_pfr0 = 0x1131;
 cpu->id_pfr1 = 0x00011011;
 cpu->id_dfr0 = 0x02010555;
-cpu->pmceid0 = 0x0481; /* PMUv3 events 0x0, 0x8, and 0x11 */
+cpu->pmceid0 = 0x000;
 cpu->pmceid1 = 0x;
 cpu->id_afr0 = 0x;
 cpu->id_mmfr0 = 0x10201105;
diff --git a/target-arm/cpu64.c b/target-arm/cpu64.c
index fc336e1..fa5eda2 100644
--- a/target-arm/cpu64.c
+++ b/target-arm/cpu64.c
@@ -135,7 +135,7 @@ static void aarch64_a57_initfn(Object *obj)
 cpu->id_isar5 = 0x00011121;
 cpu->id_aa64pfr0 = 0x;
 cpu->id_aa64dfr0 = 0x10305106;
-cpu->pmceid0 = 0x0481; /* PMUv3 events 0x0, 0x8, and 0x11 */
+cpu->pmceid0 = 0x;
 cpu->pmceid1 = 0x;
 cpu->id_aa64isar0 = 0x00011120;
 cpu->id_aa64mmfr0 = 0x1124;

If anybody disagrees let me know; otherwise this will go into a pullreq
later this week.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 1/5] target-arm: Add the pmceid0 and pmceid1 registers

2016-02-10 Thread Aaron Lindsay 
On Feb 09 15:11, Alistair Francis wrote:
> On Tue, Feb 9, 2016 at 9:19 AM, Peter Maydell  
> wrote:
> > On 6 February 2016 at 00:55, Alistair Francis
> >  wrote:
> >> Signed-off-by: Aaron Lindsay 
> >> Signed-off-by: Alistair Francis 
> >> Tested-by: Nathan Rossi 
> >> ---
> >>
> >>  target-arm/cpu-qom.h | 2 ++
> >>  target-arm/cpu.c | 2 ++
> >>  target-arm/cpu64.c   | 2 ++
> >>  target-arm/helper.c  | 8 
> >>  4 files changed, 14 insertions(+)
> >>
> >> diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
> >> index 07c0a71..1cc4502 100644
> >> --- a/target-arm/cpu-qom.h
> >> +++ b/target-arm/cpu-qom.h
> >> @@ -148,6 +148,8 @@ typedef struct ARMCPU {
> >>  uint32_t id_pfr0;
> >>  uint32_t id_pfr1;
> >>  uint32_t id_dfr0;
> >> +uint32_t pmceid0;
> >> +uint32_t pmceid1;
> >>  uint32_t id_afr0;
> >>  uint32_t id_mmfr0;
> >>  uint32_t id_mmfr1;
> >> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> >> index 7ddbf3d..937f845 100644
> >> --- a/target-arm/cpu.c
> >> +++ b/target-arm/cpu.c
> >> @@ -1156,6 +1156,8 @@ static void cortex_a15_initfn(Object *obj)
> >>  cpu->id_pfr0 = 0x1131;
> >>  cpu->id_pfr1 = 0x00011011;
> >>  cpu->id_dfr0 = 0x02010555;
> >> +cpu->pmceid0 = 0x0481; /* PMUv3 events 0x0, 0x8, and 0x11 */
> >
> > These are:
> >  SW_INCR   # insn architecturally executed, cc pass, software increment
> >  INST_RETIRED # insn architecturally executed
> >  CPU_CYCLES # cycle
> >
> > However we don't actually implement any of these, so should
> > we be advertising them?
> 
> So this part I took directly from Chris's RFC. I'm happy to take it
> out if you would like.

I think removing the PMCEID0 change makes sense since these patches
don't implement the advertised counters. We have other patches which do
implement them, but they need some more work, so we can make this change
if/when they're actually implemented.

-Aaron

> 
> >
> >> +cpu->pmceid1 = 0x;
> >>  cpu->id_afr0 = 0x;
> >>  cpu->id_mmfr0 = 0x10201105;
> >>  cpu->id_mmfr1 = 0x2000;
> >> diff --git a/target-arm/cpu64.c b/target-arm/cpu64.c
> >> index c847513..8c4b6fd 100644
> >> --- a/target-arm/cpu64.c
> >> +++ b/target-arm/cpu64.c
> >> @@ -134,6 +134,8 @@ static void aarch64_a57_initfn(Object *obj)
> >>  cpu->id_isar5 = 0x00011121;
> >>  cpu->id_aa64pfr0 = 0x;
> >>  cpu->id_aa64dfr0 = 0x10305106;
> >> +cpu->pmceid0 = 0x0481; /* PMUv3 events 0x0, 0x8, and 0x11 */
> >> +cpu->pmceid1 = 0x;
> >>  cpu->id_aa64isar0 = 0x00011120;
> >>  cpu->id_aa64mmfr0 = 0x1124;
> >>  cpu->dbgdidr = 0x3516d000;
> >> diff --git a/target-arm/helper.c b/target-arm/helper.c
> >> index 5ea507f..66aa406 100644
> >> --- a/target-arm/helper.c
> >> +++ b/target-arm/helper.c
> >> @@ -4192,6 +4192,14 @@ void register_cp_regs_for_features(ARMCPU *cpu)
> >>.opc0 = 3, .opc1 = 0, .crn = 0, .crm = 5, .opc2 = 1,
> >>.access = PL1_R, .type = ARM_CP_CONST,
> >>.resetvalue = cpu->id_aa64dfr1 },
> >> +{ .name = "PMCEID0_EL0", .state = ARM_CP_STATE_AA64,
> >> +  .opc0 = 3, .opc1 = 3, .crn = 9, .crm = 12, .opc2 = 6,
> >> +  .access = PL1_R, .type = ARM_CP_CONST,
> >> +  .resetvalue = cpu->pmceid0},
> >
> > These have 32-bit versions from v8 and up (sadly not with the
> > right opc values to use STATE_BOTH, so second stanza needed).
> 
> Ok, I have added PMCEID0 and PMCEID1.
> 
> >
> > These are configurably RO from EL0, controlled by PMUSERENR_EL0.EN,
> > so you want
> >.access = PL0_R, .accessfn = pmreg_access
> >
> > Space before final "}", please.
> >
> > Can we move these down so they're not placed right in the
> > middle of the ID_AA64* registers ?
> 
> Fixed the rest.
> 
> Thanks,
> 
> Alistair
> 
> >
> >> +{ .name = "PMCEID1_EL0", .state = ARM_CP_STATE_AA64,
> >> +  .opc0 = 3, .opc1 = 3, .crn = 9, .crm = 12, .opc2 = 7,
> >> +  .access = PL1_R, .type = ARM_CP_CONST,
> >> +  .resetvalue = cpu->pmceid1},
> >
> > Ditto.
> >
> >>  { .name = "ID_AA64AFR0_EL1", .state = ARM_CP_STATE_AA64,
> >>.opc0 = 3, .opc1 = 0, .crn = 0, .crm = 5, .opc2 = 4,
> >>.access = PL1_R, .type = ARM_CP_CONST,
> >> --
> >> 2.5.0
> >
> > thanks
> > -- PMM
> >

Re: [Qemu-devel] [PATCH v2 1/5] target-arm: Add the pmceid0 and pmceid1 registers

2016-02-09 Thread Peter Maydell 
On 6 February 2016 at 00:55, Alistair Francis
 wrote:
> Signed-off-by: Aaron Lindsay 
> Signed-off-by: Alistair Francis 
> Tested-by: Nathan Rossi 
> ---
>
>  target-arm/cpu-qom.h | 2 ++
>  target-arm/cpu.c | 2 ++
>  target-arm/cpu64.c   | 2 ++
>  target-arm/helper.c  | 8 
>  4 files changed, 14 insertions(+)
>
> diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
> index 07c0a71..1cc4502 100644
> --- a/target-arm/cpu-qom.h
> +++ b/target-arm/cpu-qom.h
> @@ -148,6 +148,8 @@ typedef struct ARMCPU {
>  uint32_t id_pfr0;
>  uint32_t id_pfr1;
>  uint32_t id_dfr0;
> +uint32_t pmceid0;
> +uint32_t pmceid1;
>  uint32_t id_afr0;
>  uint32_t id_mmfr0;
>  uint32_t id_mmfr1;
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index 7ddbf3d..937f845 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -1156,6 +1156,8 @@ static void cortex_a15_initfn(Object *obj)
>  cpu->id_pfr0 = 0x1131;
>  cpu->id_pfr1 = 0x00011011;
>  cpu->id_dfr0 = 0x02010555;
> +cpu->pmceid0 = 0x0481; /* PMUv3 events 0x0, 0x8, and 0x11 */

These are:
 SW_INCR   # insn architecturally executed, cc pass, software increment
 INST_RETIRED # insn architecturally executed
 CPU_CYCLES # cycle

However we don't actually implement any of these, so should
we be advertising them?

> +cpu->pmceid1 = 0x;
>  cpu->id_afr0 = 0x;
>  cpu->id_mmfr0 = 0x10201105;
>  cpu->id_mmfr1 = 0x2000;
> diff --git a/target-arm/cpu64.c b/target-arm/cpu64.c
> index c847513..8c4b6fd 100644
> --- a/target-arm/cpu64.c
> +++ b/target-arm/cpu64.c
> @@ -134,6 +134,8 @@ static void aarch64_a57_initfn(Object *obj)
>  cpu->id_isar5 = 0x00011121;
>  cpu->id_aa64pfr0 = 0x;
>  cpu->id_aa64dfr0 = 0x10305106;
> +cpu->pmceid0 = 0x0481; /* PMUv3 events 0x0, 0x8, and 0x11 */
> +cpu->pmceid1 = 0x;
>  cpu->id_aa64isar0 = 0x00011120;
>  cpu->id_aa64mmfr0 = 0x1124;
>  cpu->dbgdidr = 0x3516d000;
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 5ea507f..66aa406 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -4192,6 +4192,14 @@ void register_cp_regs_for_features(ARMCPU *cpu)
>.opc0 = 3, .opc1 = 0, .crn = 0, .crm = 5, .opc2 = 1,
>.access = PL1_R, .type = ARM_CP_CONST,
>.resetvalue = cpu->id_aa64dfr1 },
> +{ .name = "PMCEID0_EL0", .state = ARM_CP_STATE_AA64,
> +  .opc0 = 3, .opc1 = 3, .crn = 9, .crm = 12, .opc2 = 6,
> +  .access = PL1_R, .type = ARM_CP_CONST,
> +  .resetvalue = cpu->pmceid0},

These have 32-bit versions from v8 and up (sadly not with the
right opc values to use STATE_BOTH, so second stanza needed).

These are configurably RO from EL0, controlled by PMUSERENR_EL0.EN,
so you want
   .access = PL0_R, .accessfn = pmreg_access

Space before final "}", please.

Can we move these down so they're not placed right in the
middle of the ID_AA64* registers ?

> +{ .name = "PMCEID1_EL0", .state = ARM_CP_STATE_AA64,
> +  .opc0 = 3, .opc1 = 3, .crn = 9, .crm = 12, .opc2 = 7,
> +  .access = PL1_R, .type = ARM_CP_CONST,
> +  .resetvalue = cpu->pmceid1},

Ditto.

>  { .name = "ID_AA64AFR0_EL1", .state = ARM_CP_STATE_AA64,
>.opc0 = 3, .opc1 = 0, .crn = 0, .crm = 5, .opc2 = 4,
>.access = PL1_R, .type = ARM_CP_CONST,
> --
> 2.5.0

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 1/5] target-arm: Add the pmceid0 and pmceid1 registers

2016-02-09 Thread Alistair Francis 
On Tue, Feb 9, 2016 at 9:19 AM, Peter Maydell  wrote:
> On 6 February 2016 at 00:55, Alistair Francis
>  wrote:
>> Signed-off-by: Aaron Lindsay 
>> Signed-off-by: Alistair Francis 
>> Tested-by: Nathan Rossi 
>> ---
>>
>>  target-arm/cpu-qom.h | 2 ++
>>  target-arm/cpu.c | 2 ++
>>  target-arm/cpu64.c   | 2 ++
>>  target-arm/helper.c  | 8 
>>  4 files changed, 14 insertions(+)
>>
>> diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
>> index 07c0a71..1cc4502 100644
>> --- a/target-arm/cpu-qom.h
>> +++ b/target-arm/cpu-qom.h
>> @@ -148,6 +148,8 @@ typedef struct ARMCPU {
>>  uint32_t id_pfr0;
>>  uint32_t id_pfr1;
>>  uint32_t id_dfr0;
>> +uint32_t pmceid0;
>> +uint32_t pmceid1;
>>  uint32_t id_afr0;
>>  uint32_t id_mmfr0;
>>  uint32_t id_mmfr1;
>> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
>> index 7ddbf3d..937f845 100644
>> --- a/target-arm/cpu.c
>> +++ b/target-arm/cpu.c
>> @@ -1156,6 +1156,8 @@ static void cortex_a15_initfn(Object *obj)
>>  cpu->id_pfr0 = 0x1131;
>>  cpu->id_pfr1 = 0x00011011;
>>  cpu->id_dfr0 = 0x02010555;
>> +cpu->pmceid0 = 0x0481; /* PMUv3 events 0x0, 0x8, and 0x11 */
>
> These are:
>  SW_INCR   # insn architecturally executed, cc pass, software increment
>  INST_RETIRED # insn architecturally executed
>  CPU_CYCLES # cycle
>
> However we don't actually implement any of these, so should
> we be advertising them?

So this part I took directly from Chris's RFC. I'm happy to take it
out if you would like.

>
>> +cpu->pmceid1 = 0x;
>>  cpu->id_afr0 = 0x;
>>  cpu->id_mmfr0 = 0x10201105;
>>  cpu->id_mmfr1 = 0x2000;
>> diff --git a/target-arm/cpu64.c b/target-arm/cpu64.c
>> index c847513..8c4b6fd 100644
>> --- a/target-arm/cpu64.c
>> +++ b/target-arm/cpu64.c
>> @@ -134,6 +134,8 @@ static void aarch64_a57_initfn(Object *obj)
>>  cpu->id_isar5 = 0x00011121;
>>  cpu->id_aa64pfr0 = 0x;
>>  cpu->id_aa64dfr0 = 0x10305106;
>> +cpu->pmceid0 = 0x0481; /* PMUv3 events 0x0, 0x8, and 0x11 */
>> +cpu->pmceid1 = 0x;
>>  cpu->id_aa64isar0 = 0x00011120;
>>  cpu->id_aa64mmfr0 = 0x1124;
>>  cpu->dbgdidr = 0x3516d000;
>> diff --git a/target-arm/helper.c b/target-arm/helper.c
>> index 5ea507f..66aa406 100644
>> --- a/target-arm/helper.c
>> +++ b/target-arm/helper.c
>> @@ -4192,6 +4192,14 @@ void register_cp_regs_for_features(ARMCPU *cpu)
>>.opc0 = 3, .opc1 = 0, .crn = 0, .crm = 5, .opc2 = 1,
>>.access = PL1_R, .type = ARM_CP_CONST,
>>.resetvalue = cpu->id_aa64dfr1 },
>> +{ .name = "PMCEID0_EL0", .state = ARM_CP_STATE_AA64,
>> +  .opc0 = 3, .opc1 = 3, .crn = 9, .crm = 12, .opc2 = 6,
>> +  .access = PL1_R, .type = ARM_CP_CONST,
>> +  .resetvalue = cpu->pmceid0},
>
> These have 32-bit versions from v8 and up (sadly not with the
> right opc values to use STATE_BOTH, so second stanza needed).

Ok, I have added PMCEID0 and PMCEID1.

>
> These are configurably RO from EL0, controlled by PMUSERENR_EL0.EN,
> so you want
>.access = PL0_R, .accessfn = pmreg_access
>
> Space before final "}", please.
>
> Can we move these down so they're not placed right in the
> middle of the ID_AA64* registers ?

Fixed the rest.

Thanks,

Alistair

>
>> +{ .name = "PMCEID1_EL0", .state = ARM_CP_STATE_AA64,
>> +  .opc0 = 3, .opc1 = 3, .crn = 9, .crm = 12, .opc2 = 7,
>> +  .access = PL1_R, .type = ARM_CP_CONST,
>> +  .resetvalue = cpu->pmceid1},
>
> Ditto.
>
>>  { .name = "ID_AA64AFR0_EL1", .state = ARM_CP_STATE_AA64,
>>.opc0 = 3, .opc1 = 0, .crn = 0, .crm = 5, .opc2 = 4,
>>.access = PL1_R, .type = ARM_CP_CONST,
>> --
>> 2.5.0
>
> thanks
> -- PMM
>

Re: [Qemu-devel] [PATCH v2 1/5] target-arm: Add the pmceid0 and pmceid1 registers

2016-02-09 Thread Peter Maydell 
On 9 February 2016 at 17:48, Christopher Covington  wrote:
> On 02/09/2016 12:19 PM, Peter Maydell wrote:
>> On 6 February 2016 at 00:55, Alistair Francis
>>  wrote:
>>> Signed-off-by: Aaron Lindsay 
>>> Signed-off-by: Alistair Francis 
>>> Tested-by: Nathan Rossi 
>>> ---
>>>
>>>  target-arm/cpu-qom.h | 2 ++
>>>  target-arm/cpu.c | 2 ++
>>>  target-arm/cpu64.c   | 2 ++
>>>  target-arm/helper.c  | 8 
>>>  4 files changed, 14 insertions(+)
>>>
>>> diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
>>> index 07c0a71..1cc4502 100644
>>> --- a/target-arm/cpu-qom.h
>>> +++ b/target-arm/cpu-qom.h
>>> @@ -148,6 +148,8 @@ typedef struct ARMCPU {
>>>  uint32_t id_pfr0;
>>>  uint32_t id_pfr1;
>>>  uint32_t id_dfr0;
>>> +uint32_t pmceid0;
>>> +uint32_t pmceid1;
>>>  uint32_t id_afr0;
>>>  uint32_t id_mmfr0;
>>>  uint32_t id_mmfr1;
>>> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
>>> index 7ddbf3d..937f845 100644
>>> --- a/target-arm/cpu.c
>>> +++ b/target-arm/cpu.c
>>> @@ -1156,6 +1156,8 @@ static void cortex_a15_initfn(Object *obj)
>>>  cpu->id_pfr0 = 0x1131;
>>>  cpu->id_pfr1 = 0x00011011;
>>>  cpu->id_dfr0 = 0x02010555;
>>> +cpu->pmceid0 = 0x0481; /* PMUv3 events 0x0, 0x8, and 0x11 */
>>
>> These are:
>>  SW_INCR   # insn architecturally executed, cc pass, software increment
>>  INST_RETIRED # insn architecturally executed
>>  CPU_CYCLES # cycle
>>
>> However we don't actually implement any of these, so should
>> we be advertising them?
>
> Perhaps I'm missing something, but I was under the impression that CPU
> cycle accounting was implemented as pmccntr_read/write in
> target-arm/helper.c.

Yes, but that's not the same as implementing it as an event
visible and controllable via the PM[X]EVCNTR and PM[X]EVTYPER
register interface. (The cycle count has its own set of
dedicated registers.)

> The instruction count event may need a wrapper around cpu_get_icount().
>
> SWINC is pretty trivial, but I don't think we actually use it, other
> than for some testing (but unfortunately not yet part of the
> kvm-unit-tests patchset).

Yep, so we could implement an event or two. The question is
(a) is it useful to do so? (quite possibly, though trying to get any
  kind of legitimate perf data out of a model is at best dicey)
(b) is it useful to advertise their support in this patchset but
  not actually implement the events? (much less clear)

If we want to add real events I think that should probably
be its own patchset, and for this patchset we should stick
to bringing the 64-bit PMU implementation into line with our
existing 32-bit implementation (which is a "we support no
event counters, only the separate cycle counter" implementation,
which I think is architecturally permitted: PMCR_EL0.N == 0)

Similarly in a later patch I don't think we should implement
the non-trivial PM[X]EVCNTR/PM[X]EVTYPER registers and
the selectors until we actually implement some real events
to count, ie we should postpone that to that future patchset,
not put it in this one.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 1/5] target-arm: Add the pmceid0 and pmceid1 registers

2016-02-09 Thread Christopher Covington 
On 02/09/2016 12:19 PM, Peter Maydell wrote:
> On 6 February 2016 at 00:55, Alistair Francis
>  wrote:
>> Signed-off-by: Aaron Lindsay 
>> Signed-off-by: Alistair Francis 
>> Tested-by: Nathan Rossi 
>> ---
>>
>>  target-arm/cpu-qom.h | 2 ++
>>  target-arm/cpu.c | 2 ++
>>  target-arm/cpu64.c   | 2 ++
>>  target-arm/helper.c  | 8 
>>  4 files changed, 14 insertions(+)
>>
>> diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
>> index 07c0a71..1cc4502 100644
>> --- a/target-arm/cpu-qom.h
>> +++ b/target-arm/cpu-qom.h
>> @@ -148,6 +148,8 @@ typedef struct ARMCPU {
>>  uint32_t id_pfr0;
>>  uint32_t id_pfr1;
>>  uint32_t id_dfr0;
>> +uint32_t pmceid0;
>> +uint32_t pmceid1;
>>  uint32_t id_afr0;
>>  uint32_t id_mmfr0;
>>  uint32_t id_mmfr1;
>> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
>> index 7ddbf3d..937f845 100644
>> --- a/target-arm/cpu.c
>> +++ b/target-arm/cpu.c
>> @@ -1156,6 +1156,8 @@ static void cortex_a15_initfn(Object *obj)
>>  cpu->id_pfr0 = 0x1131;
>>  cpu->id_pfr1 = 0x00011011;
>>  cpu->id_dfr0 = 0x02010555;
>> +cpu->pmceid0 = 0x0481; /* PMUv3 events 0x0, 0x8, and 0x11 */
> 
> These are:
>  SW_INCR   # insn architecturally executed, cc pass, software increment
>  INST_RETIRED # insn architecturally executed
>  CPU_CYCLES # cycle
> 
> However we don't actually implement any of these, so should
> we be advertising them?

Perhaps I'm missing something, but I was under the impression that CPU
cycle accounting was implemented as pmccntr_read/write in
target-arm/helper.c.

The instruction count event may need a wrapper around cpu_get_icount().

SWINC is pretty trivial, but I don't think we actually use it, other
than for some testing (but unfortunately not yet part of the
kvm-unit-tests patchset).

Thanks,
Cov

-- 
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[Qemu-devel] [PATCH v2 1/5] target-arm: Add the pmceid0 and pmceid1 registers

2016-02-05 Thread Alistair Francis 
Signed-off-by: Aaron Lindsay 
Signed-off-by: Alistair Francis 
Tested-by: Nathan Rossi 
---

 target-arm/cpu-qom.h | 2 ++
 target-arm/cpu.c | 2 ++
 target-arm/cpu64.c   | 2 ++
 target-arm/helper.c  | 8 
 4 files changed, 14 insertions(+)

diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
index 07c0a71..1cc4502 100644
--- a/target-arm/cpu-qom.h
+++ b/target-arm/cpu-qom.h
@@ -148,6 +148,8 @@ typedef struct ARMCPU {
 uint32_t id_pfr0;
 uint32_t id_pfr1;
 uint32_t id_dfr0;
+uint32_t pmceid0;
+uint32_t pmceid1;
 uint32_t id_afr0;
 uint32_t id_mmfr0;
 uint32_t id_mmfr1;
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 7ddbf3d..937f845 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -1156,6 +1156,8 @@ static void cortex_a15_initfn(Object *obj)
 cpu->id_pfr0 = 0x1131;
 cpu->id_pfr1 = 0x00011011;
 cpu->id_dfr0 = 0x02010555;
+cpu->pmceid0 = 0x0481; /* PMUv3 events 0x0, 0x8, and 0x11 */
+cpu->pmceid1 = 0x;
 cpu->id_afr0 = 0x;
 cpu->id_mmfr0 = 0x10201105;
 cpu->id_mmfr1 = 0x2000;
diff --git a/target-arm/cpu64.c b/target-arm/cpu64.c
index c847513..8c4b6fd 100644
--- a/target-arm/cpu64.c
+++ b/target-arm/cpu64.c
@@ -134,6 +134,8 @@ static void aarch64_a57_initfn(Object *obj)
 cpu->id_isar5 = 0x00011121;
 cpu->id_aa64pfr0 = 0x;
 cpu->id_aa64dfr0 = 0x10305106;
+cpu->pmceid0 = 0x0481; /* PMUv3 events 0x0, 0x8, and 0x11 */
+cpu->pmceid1 = 0x;
 cpu->id_aa64isar0 = 0x00011120;
 cpu->id_aa64mmfr0 = 0x1124;
 cpu->dbgdidr = 0x3516d000;
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 5ea507f..66aa406 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -4192,6 +4192,14 @@ void register_cp_regs_for_features(ARMCPU *cpu)
   .opc0 = 3, .opc1 = 0, .crn = 0, .crm = 5, .opc2 = 1,
   .access = PL1_R, .type = ARM_CP_CONST,
   .resetvalue = cpu->id_aa64dfr1 },
+{ .name = "PMCEID0_EL0", .state = ARM_CP_STATE_AA64,
+  .opc0 = 3, .opc1 = 3, .crn = 9, .crm = 12, .opc2 = 6,
+  .access = PL1_R, .type = ARM_CP_CONST,
+  .resetvalue = cpu->pmceid0},
+{ .name = "PMCEID1_EL0", .state = ARM_CP_STATE_AA64,
+  .opc0 = 3, .opc1 = 3, .crn = 9, .crm = 12, .opc2 = 7,
+  .access = PL1_R, .type = ARM_CP_CONST,
+  .resetvalue = cpu->pmceid1},
 { .name = "ID_AA64AFR0_EL1", .state = ARM_CP_STATE_AA64,
   .opc0 = 3, .opc1 = 0, .crn = 0, .crm = 5, .opc2 = 4,
   .access = PL1_R, .type = ARM_CP_CONST,
-- 
2.5.0