| Message ID | 1598874522-5186-1-git-send-email-sai.pavan.boddu@xilinx.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | target/arm: Configure number of pmu counters | expand |
On Mon, Aug 31, 2020 at 05:18:42PM +0530, Sai Pavan Boddu wrote: > Default the pmu counters to 4 and configure it a 6 for a53 cores. Your commit message looks like it needs a little rewording. I would probably split this into two patches, one that adds pmcrn and another that modifies the A53. > > Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com> > --- > target/arm/cpu.c | 3 +++ > target/arm/cpu.h | 3 +++ > target/arm/cpu64.c | 1 + > target/arm/helper.c | 2 +- > 4 files changed, 8 insertions(+), 1 deletion(-) > > diff --git a/target/arm/cpu.c b/target/arm/cpu.c > index 6b382fc..805a692 100644 > --- a/target/arm/cpu.c > +++ b/target/arm/cpu.c > @@ -1051,6 +1051,9 @@ static void arm_cpu_initfn(Object *obj) > cpu->psci_version = 1; /* By default assume PSCI v0.1 */ > cpu->kvm_target = QEMU_KVM_ARM_TARGET_NONE; > > + /* set number of pmu counters to 4 */ /* Set default number of PMU counters */ That way if the default changes you don't need to edit both code and comment. With those changes: Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> > + cpu->pmcrn = 4; > + > if (tcg_enabled()) { > cpu->psci_version = 2; /* TCG implements PSCI 0.2 */ > } > diff --git a/target/arm/cpu.h b/target/arm/cpu.h > index ac857bd..3b47ba8 100644 > --- a/target/arm/cpu.h > +++ b/target/arm/cpu.h > @@ -879,6 +879,9 @@ struct ARMCPU { > */ > int32_t core_count; > > + /* Number of pmu counters */ > + uint8_t pmcrn; > + > /* The instance init functions for implementation-specific subclasses > * set these fields to specify the implementation-dependent values of > * various constant registers and reset values of non-constant > diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c > index dd69618..76c879a 100644 > --- a/target/arm/cpu64.c > +++ b/target/arm/cpu64.c > @@ -190,6 +190,7 @@ static void aarch64_a53_initfn(Object *obj) > cpu->gic_vpribits = 5; > cpu->gic_vprebits = 5; > define_arm_cp_regs(cpu, cortex_a72_a57_a53_cp_reginfo); > + cpu->pmcrn = 6; > } > > static void aarch64_a72_initfn(Object *obj) > diff --git a/target/arm/helper.c b/target/arm/helper.c > index 44d6666..4afbefb 100644 > --- a/target/arm/helper.c > +++ b/target/arm/helper.c > @@ -6534,7 +6534,7 @@ static void define_pmu_regs(ARMCPU *cpu) > * field as main ID register, and we implement four counters in > * addition to the cycle count register. > */ > - unsigned int i, pmcrn = 4; > + unsigned int i, pmcrn = cpu->pmcrn; > ARMCPRegInfo pmcr = { > .name = "PMCR", .cp = 15, .crn = 9, .crm = 12, .opc1 = 0, .opc2 = 0, > .access = PL0_RW, > -- > 2.7.4 > >
On Mon, 31 Aug 2020 at 12:44, Sai Pavan Boddu <sai.pavan.boddu@xilinx.com> wrote: > > Default the pmu counters to 4 and configure it a 6 for a53 cores. > > Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com> > --- > target/arm/cpu.c | 3 +++ > target/arm/cpu.h | 3 +++ > target/arm/cpu64.c | 1 + > target/arm/helper.c | 2 +- > 4 files changed, 8 insertions(+), 1 deletion(-) > > diff --git a/target/arm/cpu.c b/target/arm/cpu.c > index 6b382fc..805a692 100644 > --- a/target/arm/cpu.c > +++ b/target/arm/cpu.c > @@ -1051,6 +1051,9 @@ static void arm_cpu_initfn(Object *obj) > cpu->psci_version = 1; /* By default assume PSCI v0.1 */ > cpu->kvm_target = QEMU_KVM_ARM_TARGET_NONE; > > + /* set number of pmu counters to 4 */ > + cpu->pmcrn = 4; > + > if (tcg_enabled()) { > cpu->psci_version = 2; /* TCG implements PSCI 0.2 */ > } > diff --git a/target/arm/cpu.h b/target/arm/cpu.h > index ac857bd..3b47ba8 100644 > --- a/target/arm/cpu.h > +++ b/target/arm/cpu.h > @@ -879,6 +879,9 @@ struct ARMCPU { > */ > int32_t core_count; > > + /* Number of pmu counters */ > + uint8_t pmcrn; > + > /* The instance init functions for implementation-specific subclasses > * set these fields to specify the implementation-dependent values of > * various constant registers and reset values of non-constant Rather than doing this, I think the better approach would be to switch to treating PMCR_EL0 as an ID register in the same way we do for other varies-per-CPU ID registers: * new field uint64_t pmcr_el0 in the ARMISARegisters sub-struct of ARMCPU * each CPU's initfn needs to set this (sorry, this is going to mean a fair amount of digging through TRMs to find the right values) * define_pmu_regs() just uses the isar.pmcr_el0 field as the resetvalue for PMCR_EL0 * define_pmu_regs() needs to use pmu_num_counters() to get the number of counters for its loops Side note: looking at the code I see we don't implement the effect that MDCR_EL2.HPMN has on this register... (and probably we don't in general get the EL2 handling of the PMU right in other places too). thanks -- PMM
Hi Peter, > -----Original Message----- > From: Peter Maydell <peter.maydell@linaro.org> > Sent: Tuesday, September 1, 2020 6:18 PM > To: Sai Pavan Boddu <saipava@xilinx.com> > Cc: Richard Henderson <richard.henderson@linaro.org>; Philippe Mathieu- > Daudé <philmd@redhat.com>; Andrew Jones <drjones@redhat.com>; Paolo > Bonzini <pbonzini@redhat.com>; Alex Bennée <alex.bennee@linaro.org>; > Eric Auger <eric.auger@redhat.com>; Edgar Iglesias <edgari@xilinx.com>; > qemu-arm <qemu-arm@nongnu.org>; QEMU Developers <qemu- > devel@nongnu.org> > Subject: Re: [PATCH] target/arm: Configure number of pmu counters > > On Mon, 31 Aug 2020 at 12:44, Sai Pavan Boddu > <sai.pavan.boddu@xilinx.com> wrote: > > > > Default the pmu counters to 4 and configure it a 6 for a53 cores. > > > > Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com> > > --- > > target/arm/cpu.c | 3 +++ > > target/arm/cpu.h | 3 +++ > > target/arm/cpu64.c | 1 + > > target/arm/helper.c | 2 +- > > 4 files changed, 8 insertions(+), 1 deletion(-) > > > > diff --git a/target/arm/cpu.c b/target/arm/cpu.c index > > 6b382fc..805a692 100644 > > --- a/target/arm/cpu.c > > +++ b/target/arm/cpu.c > > @@ -1051,6 +1051,9 @@ static void arm_cpu_initfn(Object *obj) > > cpu->psci_version = 1; /* By default assume PSCI v0.1 */ > > cpu->kvm_target = QEMU_KVM_ARM_TARGET_NONE; > > > > + /* set number of pmu counters to 4 */ > > + cpu->pmcrn = 4; > > + > > if (tcg_enabled()) { > > cpu->psci_version = 2; /* TCG implements PSCI 0.2 */ > > } > > diff --git a/target/arm/cpu.h b/target/arm/cpu.h index > > ac857bd..3b47ba8 100644 > > --- a/target/arm/cpu.h > > +++ b/target/arm/cpu.h > > @@ -879,6 +879,9 @@ struct ARMCPU { > > */ > > int32_t core_count; > > > > + /* Number of pmu counters */ > > + uint8_t pmcrn; > > + > > /* The instance init functions for implementation-specific subclasses > > * set these fields to specify the implementation-dependent values of > > * various constant registers and reset values of non-constant > > Rather than doing this, I think the better approach would be to switch to > treating PMCR_EL0 as an ID register in the same way we do for other varies- > per-CPU ID registers: > > * new field uint64_t pmcr_el0 in the ARMISARegisters sub-struct of ARMCPU > * each CPU's initfn needs to set this (sorry, this is going to mean > a fair amount of digging through TRMs to find the right values) > * define_pmu_regs() just uses the isar.pmcr_el0 field as the resetvalue > for PMCR_EL0 > * define_pmu_regs() needs to use pmu_num_counters() to get the number > of counters for its loops > > Side note: looking at the code I see we don't implement the effect that > MDCR_EL2.HPMN has on this register... (and probably we don't in general > get the EL2 handling of the PMU right in other places too). [Sai Pavan Boddu] Sure, I will get back on this. I need to understand more. Thanks, Sai Pavan > > thanks > -- PMM
diff --git a/target/arm/cpu.c b/target/arm/cpu.c index 6b382fc..805a692 100644 --- a/target/arm/cpu.c +++ b/target/arm/cpu.c @@ -1051,6 +1051,9 @@ static void arm_cpu_initfn(Object *obj) cpu->psci_version = 1; /* By default assume PSCI v0.1 */ cpu->kvm_target = QEMU_KVM_ARM_TARGET_NONE; + /* set number of pmu counters to 4 */ + cpu->pmcrn = 4; + if (tcg_enabled()) { cpu->psci_version = 2; /* TCG implements PSCI 0.2 */ } diff --git a/target/arm/cpu.h b/target/arm/cpu.h index ac857bd..3b47ba8 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -879,6 +879,9 @@ struct ARMCPU { */ int32_t core_count; + /* Number of pmu counters */ + uint8_t pmcrn; + /* The instance init functions for implementation-specific subclasses * set these fields to specify the implementation-dependent values of * various constant registers and reset values of non-constant diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c index dd69618..76c879a 100644 --- a/target/arm/cpu64.c +++ b/target/arm/cpu64.c @@ -190,6 +190,7 @@ static void aarch64_a53_initfn(Object *obj) cpu->gic_vpribits = 5; cpu->gic_vprebits = 5; define_arm_cp_regs(cpu, cortex_a72_a57_a53_cp_reginfo); + cpu->pmcrn = 6; } static void aarch64_a72_initfn(Object *obj) diff --git a/target/arm/helper.c b/target/arm/helper.c index 44d6666..4afbefb 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -6534,7 +6534,7 @@ static void define_pmu_regs(ARMCPU *cpu) * field as main ID register, and we implement four counters in * addition to the cycle count register. */ - unsigned int i, pmcrn = 4; + unsigned int i, pmcrn = cpu->pmcrn; ARMCPRegInfo pmcr = { .name = "PMCR", .cp = 15, .crn = 9, .crm = 12, .opc1 = 0, .opc2 = 0, .access = PL0_RW,
Default the pmu counters to 4 and configure it a 6 for a53 cores. Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com> --- target/arm/cpu.c | 3 +++ target/arm/cpu.h | 3 +++ target/arm/cpu64.c | 1 + target/arm/helper.c | 2 +- 4 files changed, 8 insertions(+), 1 deletion(-)