From: "Philippe Mathieu-Daudé" <philmd@redhat.com>
To: Peter Maydell <peter.maydell@linaro.org>,
qemu-arm@nongnu.org, qemu-devel@nongnu.org
Cc: Eric Auger <eric.auger@redhat.com>,
Aaron Lindsay <aaron@os.amperecomputing.com>,
Richard Henderson <richard.henderson@linaro.org>
Subject: Re: [PATCH 04/13] target/arm: Factor out PMU register definitions
Date: Wed, 12 Feb 2020 07:40:55 +0100 [thread overview]
Message-ID: <f9e4aa0e-4368-611a-c31b-2ddc6b2a89b2@redhat.com> (raw)
In-Reply-To: <20200211173726.22541-5-peter.maydell@linaro.org>
On 2/11/20 6:37 PM, Peter Maydell wrote:
> Pull the code that defines the various PMU registers out
> into its own function, matching the pattern we have
> already for the debug registers.
>
> Apart from one style fix to a multi-line comment, this
> is purely movement of code with no changes to it.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> target/arm/helper.c | 158 +++++++++++++++++++++++---------------------
> 1 file changed, 82 insertions(+), 76 deletions(-)
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index bf083c369fc..0011a22f42d 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -5822,6 +5822,87 @@ static void define_debug_regs(ARMCPU *cpu)
> }
> }
>
> +static void define_pmu_regs(ARMCPU *cpu)
> +{
> + /*
> + * v7 performance monitor control register: same implementor
> + * field as main ID register, and we implement four counters in
> + * addition to the cycle count register.
> + */
> + unsigned int i, pmcrn = 4;
> + ARMCPRegInfo pmcr = {
> + .name = "PMCR", .cp = 15, .crn = 9, .crm = 12, .opc1 = 0, .opc2 = 0,
> + .access = PL0_RW,
> + .type = ARM_CP_IO | ARM_CP_ALIAS,
> + .fieldoffset = offsetoflow32(CPUARMState, cp15.c9_pmcr),
> + .accessfn = pmreg_access, .writefn = pmcr_write,
> + .raw_writefn = raw_write,
> + };
> + ARMCPRegInfo pmcr64 = {
> + .name = "PMCR_EL0", .state = ARM_CP_STATE_AA64,
> + .opc0 = 3, .opc1 = 3, .crn = 9, .crm = 12, .opc2 = 0,
> + .access = PL0_RW, .accessfn = pmreg_access,
> + .type = ARM_CP_IO,
> + .fieldoffset = offsetof(CPUARMState, cp15.c9_pmcr),
> + .resetvalue = (cpu->midr & 0xff000000) | (pmcrn << PMCRN_SHIFT),
> + .writefn = pmcr_write, .raw_writefn = raw_write,
> + };
> + define_one_arm_cp_reg(cpu, &pmcr);
> + define_one_arm_cp_reg(cpu, &pmcr64);
> + for (i = 0; i < pmcrn; i++) {
> + char *pmevcntr_name = g_strdup_printf("PMEVCNTR%d", i);
> + char *pmevcntr_el0_name = g_strdup_printf("PMEVCNTR%d_EL0", i);
> + char *pmevtyper_name = g_strdup_printf("PMEVTYPER%d", i);
> + char *pmevtyper_el0_name = g_strdup_printf("PMEVTYPER%d_EL0", i);
> + ARMCPRegInfo pmev_regs[] = {
> + { .name = pmevcntr_name, .cp = 15, .crn = 14,
> + .crm = 8 | (3 & (i >> 3)), .opc1 = 0, .opc2 = i & 7,
> + .access = PL0_RW, .type = ARM_CP_IO | ARM_CP_ALIAS,
> + .readfn = pmevcntr_readfn, .writefn = pmevcntr_writefn,
> + .accessfn = pmreg_access },
> + { .name = pmevcntr_el0_name, .state = ARM_CP_STATE_AA64,
> + .opc0 = 3, .opc1 = 3, .crn = 14, .crm = 8 | (3 & (i >> 3)),
> + .opc2 = i & 7, .access = PL0_RW, .accessfn = pmreg_access,
> + .type = ARM_CP_IO,
> + .readfn = pmevcntr_readfn, .writefn = pmevcntr_writefn,
> + .raw_readfn = pmevcntr_rawread,
> + .raw_writefn = pmevcntr_rawwrite },
> + { .name = pmevtyper_name, .cp = 15, .crn = 14,
> + .crm = 12 | (3 & (i >> 3)), .opc1 = 0, .opc2 = i & 7,
> + .access = PL0_RW, .type = ARM_CP_IO | ARM_CP_ALIAS,
> + .readfn = pmevtyper_readfn, .writefn = pmevtyper_writefn,
> + .accessfn = pmreg_access },
> + { .name = pmevtyper_el0_name, .state = ARM_CP_STATE_AA64,
> + .opc0 = 3, .opc1 = 3, .crn = 14, .crm = 12 | (3 & (i >> 3)),
> + .opc2 = i & 7, .access = PL0_RW, .accessfn = pmreg_access,
> + .type = ARM_CP_IO,
> + .readfn = pmevtyper_readfn, .writefn = pmevtyper_writefn,
> + .raw_writefn = pmevtyper_rawwrite },
> + REGINFO_SENTINEL
> + };
> + define_arm_cp_regs(cpu, pmev_regs);
> + g_free(pmevcntr_name);
> + g_free(pmevcntr_el0_name);
> + g_free(pmevtyper_name);
> + g_free(pmevtyper_el0_name);
> + }
> + if (FIELD_EX32(cpu->id_dfr0, ID_DFR0, PERFMON) >= 4 &&
> + FIELD_EX32(cpu->id_dfr0, ID_DFR0, PERFMON) != 0xf) {
> + ARMCPRegInfo v81_pmu_regs[] = {
> + { .name = "PMCEID2", .state = ARM_CP_STATE_AA32,
> + .cp = 15, .opc1 = 0, .crn = 9, .crm = 14, .opc2 = 4,
> + .access = PL0_R, .accessfn = pmreg_access, .type = ARM_CP_CONST,
> + .resetvalue = extract64(cpu->pmceid0, 32, 32) },
> + { .name = "PMCEID3", .state = ARM_CP_STATE_AA32,
> + .cp = 15, .opc1 = 0, .crn = 9, .crm = 14, .opc2 = 5,
> + .access = PL0_R, .accessfn = pmreg_access, .type = ARM_CP_CONST,
> + .resetvalue = extract64(cpu->pmceid1, 32, 32) },
> + REGINFO_SENTINEL
> + };
> + define_arm_cp_regs(cpu, v81_pmu_regs);
> + }
> +}
> +
> /* We don't know until after realize whether there's a GICv3
> * attached, and that is what registers the gicv3 sysregs.
> * So we have to fill in the GIC fields in ID_PFR/ID_PFR1_EL1/ID_AA64PFR0_EL1
> @@ -6244,67 +6325,6 @@ void register_cp_regs_for_features(ARMCPU *cpu)
> define_arm_cp_regs(cpu, pmovsset_cp_reginfo);
> }
> if (arm_feature(env, ARM_FEATURE_V7)) {
> - /* v7 performance monitor control register: same implementor
> - * field as main ID register, and we implement four counters in
> - * addition to the cycle count register.
> - */
> - unsigned int i, pmcrn = 4;
> - ARMCPRegInfo pmcr = {
> - .name = "PMCR", .cp = 15, .crn = 9, .crm = 12, .opc1 = 0, .opc2 = 0,
> - .access = PL0_RW,
> - .type = ARM_CP_IO | ARM_CP_ALIAS,
> - .fieldoffset = offsetoflow32(CPUARMState, cp15.c9_pmcr),
> - .accessfn = pmreg_access, .writefn = pmcr_write,
> - .raw_writefn = raw_write,
> - };
> - ARMCPRegInfo pmcr64 = {
> - .name = "PMCR_EL0", .state = ARM_CP_STATE_AA64,
> - .opc0 = 3, .opc1 = 3, .crn = 9, .crm = 12, .opc2 = 0,
> - .access = PL0_RW, .accessfn = pmreg_access,
> - .type = ARM_CP_IO,
> - .fieldoffset = offsetof(CPUARMState, cp15.c9_pmcr),
> - .resetvalue = (cpu->midr & 0xff000000) | (pmcrn << PMCRN_SHIFT),
> - .writefn = pmcr_write, .raw_writefn = raw_write,
> - };
> - define_one_arm_cp_reg(cpu, &pmcr);
> - define_one_arm_cp_reg(cpu, &pmcr64);
> - for (i = 0; i < pmcrn; i++) {
> - char *pmevcntr_name = g_strdup_printf("PMEVCNTR%d", i);
> - char *pmevcntr_el0_name = g_strdup_printf("PMEVCNTR%d_EL0", i);
> - char *pmevtyper_name = g_strdup_printf("PMEVTYPER%d", i);
> - char *pmevtyper_el0_name = g_strdup_printf("PMEVTYPER%d_EL0", i);
> - ARMCPRegInfo pmev_regs[] = {
> - { .name = pmevcntr_name, .cp = 15, .crn = 14,
> - .crm = 8 | (3 & (i >> 3)), .opc1 = 0, .opc2 = i & 7,
> - .access = PL0_RW, .type = ARM_CP_IO | ARM_CP_ALIAS,
> - .readfn = pmevcntr_readfn, .writefn = pmevcntr_writefn,
> - .accessfn = pmreg_access },
> - { .name = pmevcntr_el0_name, .state = ARM_CP_STATE_AA64,
> - .opc0 = 3, .opc1 = 3, .crn = 14, .crm = 8 | (3 & (i >> 3)),
> - .opc2 = i & 7, .access = PL0_RW, .accessfn = pmreg_access,
> - .type = ARM_CP_IO,
> - .readfn = pmevcntr_readfn, .writefn = pmevcntr_writefn,
> - .raw_readfn = pmevcntr_rawread,
> - .raw_writefn = pmevcntr_rawwrite },
> - { .name = pmevtyper_name, .cp = 15, .crn = 14,
> - .crm = 12 | (3 & (i >> 3)), .opc1 = 0, .opc2 = i & 7,
> - .access = PL0_RW, .type = ARM_CP_IO | ARM_CP_ALIAS,
> - .readfn = pmevtyper_readfn, .writefn = pmevtyper_writefn,
> - .accessfn = pmreg_access },
> - { .name = pmevtyper_el0_name, .state = ARM_CP_STATE_AA64,
> - .opc0 = 3, .opc1 = 3, .crn = 14, .crm = 12 | (3 & (i >> 3)),
> - .opc2 = i & 7, .access = PL0_RW, .accessfn = pmreg_access,
> - .type = ARM_CP_IO,
> - .readfn = pmevtyper_readfn, .writefn = pmevtyper_writefn,
> - .raw_writefn = pmevtyper_rawwrite },
> - REGINFO_SENTINEL
> - };
> - define_arm_cp_regs(cpu, pmev_regs);
> - g_free(pmevcntr_name);
> - g_free(pmevcntr_el0_name);
> - g_free(pmevtyper_name);
> - g_free(pmevtyper_el0_name);
TIL git-diff --color-moved
Maybe move PERFMON block first, then extract define_pmu_regs()?
In any case,
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> - }
> ARMCPRegInfo clidr = {
> .name = "CLIDR", .state = ARM_CP_STATE_BOTH,
> .opc0 = 3, .crn = 0, .crm = 0, .opc1 = 1, .opc2 = 1,
> @@ -6315,24 +6335,10 @@ void register_cp_regs_for_features(ARMCPU *cpu)
> define_one_arm_cp_reg(cpu, &clidr);
> define_arm_cp_regs(cpu, v7_cp_reginfo);
> define_debug_regs(cpu);
> + define_pmu_regs(cpu);
> } else {
> define_arm_cp_regs(cpu, not_v7_cp_reginfo);
> }
> - if (FIELD_EX32(cpu->id_dfr0, ID_DFR0, PERFMON) >= 4 &&
> - FIELD_EX32(cpu->id_dfr0, ID_DFR0, PERFMON) != 0xf) {
> - ARMCPRegInfo v81_pmu_regs[] = {
> - { .name = "PMCEID2", .state = ARM_CP_STATE_AA32,
> - .cp = 15, .opc1 = 0, .crn = 9, .crm = 14, .opc2 = 4,
> - .access = PL0_R, .accessfn = pmreg_access, .type = ARM_CP_CONST,
> - .resetvalue = extract64(cpu->pmceid0, 32, 32) },
> - { .name = "PMCEID3", .state = ARM_CP_STATE_AA32,
> - .cp = 15, .opc1 = 0, .crn = 9, .crm = 14, .opc2 = 5,
> - .access = PL0_R, .accessfn = pmreg_access, .type = ARM_CP_CONST,
> - .resetvalue = extract64(cpu->pmceid1, 32, 32) },
> - REGINFO_SENTINEL
> - };
> - define_arm_cp_regs(cpu, v81_pmu_regs);
> - }
> if (arm_feature(env, ARM_FEATURE_V8)) {
> /* AArch64 ID registers, which all have impdef reset values.
> * Note that within the ID register ranges the unused slots
>
next prev parent reply other threads:[~2020-02-12 6:42 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-11 17:37 [PATCH 00/13] arm: Implement ARMv8.1-PMU and ARMv8.4-PMU Peter Maydell
2020-02-11 17:37 ` [PATCH 01/13] target/arm: Add _aa32_ to isar_feature functions testing 32-bit ID registers Peter Maydell
2020-02-11 18:25 ` Richard Henderson
2020-02-12 6:23 ` Philippe Mathieu-Daudé
2020-02-11 17:37 ` [PATCH 02/13] target/arm: Add isar_feature_any_fp16 and document naming/usage conventions Peter Maydell
2020-02-11 18:28 ` Richard Henderson
2020-02-12 6:24 ` Philippe Mathieu-Daudé
2020-02-12 11:32 ` Peter Maydell
2020-02-11 17:37 ` [PATCH 03/13] target/arm: Define and use any_predinv isar_feature test Peter Maydell
2020-02-11 18:29 ` Richard Henderson
2020-02-12 6:24 ` Philippe Mathieu-Daudé
2020-02-11 17:37 ` [PATCH 04/13] target/arm: Factor out PMU register definitions Peter Maydell
2020-02-11 18:30 ` Richard Henderson
2020-02-12 6:40 ` Philippe Mathieu-Daudé [this message]
2020-02-11 17:37 ` [PATCH 05/13] target/arm: Add and use FIELD definitions for ID_AA64DFR0_EL1 Peter Maydell
2020-02-11 18:34 ` Richard Henderson
2020-02-12 6:44 ` Philippe Mathieu-Daudé
2020-02-11 17:37 ` [PATCH 06/13] target/arm: Use FIELD macros for clearing ID_DFR0 PERFMON field Peter Maydell
2020-02-11 18:34 ` Richard Henderson
2020-02-12 6:48 ` Philippe Mathieu-Daudé
2020-02-11 17:37 ` [PATCH 07/13] target/arm: Define an aa32_pmu_8_1 isar feature test function Peter Maydell
2020-02-11 18:38 ` Richard Henderson
2020-02-11 17:37 ` [PATCH 08/13] target/arm: Add _aa64_ and _any_ versions of pmu_8_1 isar checks Peter Maydell
2020-02-11 18:40 ` Richard Henderson
2020-02-12 6:56 ` Philippe Mathieu-Daudé
2020-02-11 17:37 ` [PATCH 09/13] target/arm: Implement ARMv8.1-PMU extension Peter Maydell
2020-02-11 18:45 ` Richard Henderson
2020-02-11 17:37 ` [PATCH 10/13] target/arm: Implement ARMv8.4-PMU extension Peter Maydell
2020-02-11 18:49 ` Richard Henderson
2020-02-11 17:37 ` [PATCH 11/13] target/arm: Provide ARMv8.4-PMU in '-cpu max' Peter Maydell
2020-02-11 18:50 ` Richard Henderson
2020-02-11 17:37 ` [PATCH 12/13] target/arm: Correct definition of PMCRDP Peter Maydell
2020-02-11 18:52 ` Richard Henderson
2020-02-12 7:00 ` Philippe Mathieu-Daudé
2020-02-11 17:37 ` [PATCH 13/13] target/arm: Correct handling of PMCR_EL0.LC bit Peter Maydell
2020-02-11 18:55 ` Richard Henderson
2020-02-12 7:14 ` Philippe Mathieu-Daudé
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=f9e4aa0e-4368-611a-c31b-2ddc6b2a89b2@redhat.com \
--to=philmd@redhat.com \
--cc=aaron@os.amperecomputing.com \
--cc=eric.auger@redhat.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-arm@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=richard.henderson@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).