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 <alind...@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.