[Qemu-devel] [PATCH v3 01/22] target/arm: A53: Initialize PMCEID[01]

Aaron Lindsay posted 22 patches 7 years, 7 months ago
There is a newer version of this series
[Qemu-devel] [PATCH v3 01/22] target/arm: A53: Initialize PMCEID[01]
Posted by Aaron Lindsay 7 years, 7 months ago
A53 advertises ARM_FEATURE_PMU, but wasn't initializing pmceid[01].
pmceid[01] are already being initialized to zero for both A15 and A57.

Signed-off-by: Aaron Lindsay <alindsay@codeaurora.org>
---
 target/arm/cpu64.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 991d764..8c4db31 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -201,6 +201,8 @@ static void aarch64_a53_initfn(Object *obj)
     cpu->id_isar5 = 0x00011121;
     cpu->id_aa64pfr0 = 0x00002222;
     cpu->id_aa64dfr0 = 0x10305106;
+    cpu->pmceid0 = 0x00000000;
+    cpu->pmceid1 = 0x00000000;
     cpu->id_aa64isar0 = 0x00011120;
     cpu->id_aa64mmfr0 = 0x00001122; /* 40 bit physical addr */
     cpu->dbgdidr = 0x3516d000;
-- 
Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.


Re: [Qemu-devel] [Qemu-arm] [PATCH v3 01/22] target/arm: A53: Initialize PMCEID[01]
Posted by Philippe Mathieu-Daudé 7 years, 7 months ago
Hi Aaron,

On 03/16/2018 09:30 PM, Aaron Lindsay wrote:
> A53 advertises ARM_FEATURE_PMU, but wasn't initializing pmceid[01].
> pmceid[01] are already being initialized to zero for both A15 and A57.
> 
> Signed-off-by: Aaron Lindsay <alindsay@codeaurora.org>
> ---
>  target/arm/cpu64.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> index 991d764..8c4db31 100644
> --- a/target/arm/cpu64.c
> +++ b/target/arm/cpu64.c
> @@ -201,6 +201,8 @@ static void aarch64_a53_initfn(Object *obj)
>      cpu->id_isar5 = 0x00011121;
>      cpu->id_aa64pfr0 = 0x00002222;
>      cpu->id_aa64dfr0 = 0x10305106;
> +    cpu->pmceid0 = 0x00000000;
> +    cpu->pmceid1 = 0x00000000;
>      cpu->id_aa64isar0 = 0x00011120;
>      cpu->id_aa64mmfr0 = 0x00001122; /* 40 bit physical addr */
>      cpu->dbgdidr = 0x3516d000;
> 

Maybe we can move this at a single place in arm_cpu_post_init():

    if (arm_feature(&cpu->env, ARM_FEATURE_PMU)) {
        cpu->pmceid0 = 0x00000000;
        cpu->pmceid1 = 0x00000000;
    }

Regards,

Phil.

Re: [Qemu-devel] [Qemu-arm] [PATCH v3 01/22] target/arm: A53: Initialize PMCEID[01]
Posted by Philippe Mathieu-Daudé 7 years, 7 months ago
On 03/18/2018 11:35 PM, Philippe Mathieu-Daudé wrote:
> Hi Aaron,
> 
> On 03/16/2018 09:30 PM, Aaron Lindsay wrote:
>> A53 advertises ARM_FEATURE_PMU, but wasn't initializing pmceid[01].
>> pmceid[01] are already being initialized to zero for both A15 and A57.
>>
>> Signed-off-by: Aaron Lindsay <alindsay@codeaurora.org>
>> ---
>>  target/arm/cpu64.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
>> index 991d764..8c4db31 100644
>> --- a/target/arm/cpu64.c
>> +++ b/target/arm/cpu64.c
>> @@ -201,6 +201,8 @@ static void aarch64_a53_initfn(Object *obj)
>>      cpu->id_isar5 = 0x00011121;
>>      cpu->id_aa64pfr0 = 0x00002222;
>>      cpu->id_aa64dfr0 = 0x10305106;
>> +    cpu->pmceid0 = 0x00000000;
>> +    cpu->pmceid1 = 0x00000000;
>>      cpu->id_aa64isar0 = 0x00011120;
>>      cpu->id_aa64mmfr0 = 0x00001122; /* 40 bit physical addr */
>>      cpu->dbgdidr = 0x3516d000;
>>
> 
> Maybe we can move this at a single place in arm_cpu_post_init():

Err, arm_cpu_reset() :)

> 
>     if (arm_feature(&cpu->env, ARM_FEATURE_PMU)) {
>         cpu->pmceid0 = 0x00000000;
>         cpu->pmceid1 = 0x00000000;
>     }
> 
> Regards,
> 
> Phil.
> 

Re: [Qemu-devel] [Qemu-arm] [PATCH v3 01/22] target/arm: A53: Initialize PMCEID[01]
Posted by Aaron Lindsay 7 years, 7 months ago
On Mar 18 23:35, Philippe Mathieu-Daudé wrote:
> Hi Aaron,
> 
> On 03/16/2018 09:30 PM, Aaron Lindsay wrote:
> > A53 advertises ARM_FEATURE_PMU, but wasn't initializing pmceid[01].
> > pmceid[01] are already being initialized to zero for both A15 and A57.
> > 
> > Signed-off-by: Aaron Lindsay <alindsay@codeaurora.org>
> > ---
> >  target/arm/cpu64.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> > index 991d764..8c4db31 100644
> > --- a/target/arm/cpu64.c
> > +++ b/target/arm/cpu64.c
> > @@ -201,6 +201,8 @@ static void aarch64_a53_initfn(Object *obj)
> >      cpu->id_isar5 = 0x00011121;
> >      cpu->id_aa64pfr0 = 0x00002222;
> >      cpu->id_aa64dfr0 = 0x10305106;
> > +    cpu->pmceid0 = 0x00000000;
> > +    cpu->pmceid1 = 0x00000000;
> >      cpu->id_aa64isar0 = 0x00011120;
> >      cpu->id_aa64mmfr0 = 0x00001122; /* 40 bit physical addr */
> >      cpu->dbgdidr = 0x3516d000;
> > 
> 
> Maybe we can move this at a single place in arm_cpu_post_init():
> 
>     if (arm_feature(&cpu->env, ARM_FEATURE_PMU)) {
>         cpu->pmceid0 = 0x00000000;
>         cpu->pmceid1 = 0x00000000;
>     }

I like consolidating the initialization - though I think it can go in
arm_cpu_realizefn() with the preexisting PMU-related id_aa64dfr0
initialization since it is constant once you've chosen a type of
processor. One of the other patches in this set actually already adds
some PMCEID initialization there based on PMCR.N.

-Aaron

> 
> Regards,
> 
> Phil.

-- 
Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [Qemu-devel] [Qemu-arm] [PATCH v3 01/22] target/arm: A53: Initialize PMCEID[01]
Posted by Philippe Mathieu-Daudé 7 years, 7 months ago
On 03/19/2018 09:35 PM, Aaron Lindsay wrote:
> On Mar 18 23:35, Philippe Mathieu-Daudé wrote:
>> Hi Aaron,
>>
>> On 03/16/2018 09:30 PM, Aaron Lindsay wrote:
>>> A53 advertises ARM_FEATURE_PMU, but wasn't initializing pmceid[01].
>>> pmceid[01] are already being initialized to zero for both A15 and A57.
>>>
>>> Signed-off-by: Aaron Lindsay <alindsay@codeaurora.org>
>>> ---
>>>  target/arm/cpu64.c | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
>>> index 991d764..8c4db31 100644
>>> --- a/target/arm/cpu64.c
>>> +++ b/target/arm/cpu64.c
>>> @@ -201,6 +201,8 @@ static void aarch64_a53_initfn(Object *obj)
>>>      cpu->id_isar5 = 0x00011121;
>>>      cpu->id_aa64pfr0 = 0x00002222;
>>>      cpu->id_aa64dfr0 = 0x10305106;
>>> +    cpu->pmceid0 = 0x00000000;
>>> +    cpu->pmceid1 = 0x00000000;
>>>      cpu->id_aa64isar0 = 0x00011120;
>>>      cpu->id_aa64mmfr0 = 0x00001122; /* 40 bit physical addr */
>>>      cpu->dbgdidr = 0x3516d000;
>>>
>>
>> Maybe we can move this at a single place in arm_cpu_post_init():
>>
>>     if (arm_feature(&cpu->env, ARM_FEATURE_PMU)) {
>>         cpu->pmceid0 = 0x00000000;
>>         cpu->pmceid1 = 0x00000000;
>>     }
> 
> I like consolidating the initialization - though I think it can go in
> arm_cpu_realizefn() with the preexisting PMU-related id_aa64dfr0
> initialization since it is constant once you've chosen a type of
> processor. One of the other patches in this set actually already adds
> some PMCEID initialization there based on PMCR.N.

Indeed, arm_cpu_realizefn() is a good place.

Re: [Qemu-devel] [Qemu-arm] [PATCH v3 01/22] target/arm: A53: Initialize PMCEID[01]
Posted by Aaron Lindsay 7 years, 7 months ago
On Mar 20 02:03, Philippe Mathieu-Daudé wrote:
> On 03/19/2018 09:35 PM, Aaron Lindsay wrote:
> > On Mar 18 23:35, Philippe Mathieu-Daudé wrote:
> >> Hi Aaron,
> >>
> >> On 03/16/2018 09:30 PM, Aaron Lindsay wrote:
> >>> A53 advertises ARM_FEATURE_PMU, but wasn't initializing pmceid[01].
> >>> pmceid[01] are already being initialized to zero for both A15 and A57.
> >>>
> >>> Signed-off-by: Aaron Lindsay <alindsay@codeaurora.org>
> >>> ---
> >>>  target/arm/cpu64.c | 2 ++
> >>>  1 file changed, 2 insertions(+)
> >>>
> >>> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> >>> index 991d764..8c4db31 100644
> >>> --- a/target/arm/cpu64.c
> >>> +++ b/target/arm/cpu64.c
> >>> @@ -201,6 +201,8 @@ static void aarch64_a53_initfn(Object *obj)
> >>>      cpu->id_isar5 = 0x00011121;
> >>>      cpu->id_aa64pfr0 = 0x00002222;
> >>>      cpu->id_aa64dfr0 = 0x10305106;
> >>> +    cpu->pmceid0 = 0x00000000;
> >>> +    cpu->pmceid1 = 0x00000000;
> >>>      cpu->id_aa64isar0 = 0x00011120;
> >>>      cpu->id_aa64mmfr0 = 0x00001122; /* 40 bit physical addr */
> >>>      cpu->dbgdidr = 0x3516d000;
> >>>
> >>
> >> Maybe we can move this at a single place in arm_cpu_post_init():
> >>
> >>     if (arm_feature(&cpu->env, ARM_FEATURE_PMU)) {
> >>         cpu->pmceid0 = 0x00000000;
> >>         cpu->pmceid1 = 0x00000000;
> >>     }
> > 
> > I like consolidating the initialization - though I think it can go in
> > arm_cpu_realizefn() with the preexisting PMU-related id_aa64dfr0
> > initialization since it is constant once you've chosen a type of
> > processor. One of the other patches in this set actually already adds
> > some PMCEID initialization there based on PMCR.N.
> 
> Indeed, arm_cpu_realizefn() is a good place.

I've consolidated the pmceid[01] initialization into arm_cpu_realizefn()
for v4.

-Aaron

-- 
Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.