| Message ID | 1521232280-13089-2-git-send-email-alindsay@codeaurora.org |
|---|---|
| State | New |
| Headers | show |
| Series | More fully implement ARM PMUv3 | expand |
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.
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. >
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.
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.
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
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;
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(+)