From: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
To: linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH v2] powerpc/perf: Use PVR rather than oprofile field to determine CPU version
Date: Wed, 6 Feb 2019 17:41:16 +0530 [thread overview]
Message-ID: <3085b26c-d4af-1f41-c6b6-9d523290e843@linux.vnet.ibm.com> (raw)
In-Reply-To: <20190206063024.8489-1-rashmica.g@gmail.com>
On 06/02/19 12:00 PM, Rashmica Gupta wrote:
> Currently the perf CPU backend drivers detect what CPU they're on using
> cur_cpu_spec->oprofile_cpu_type.
>
> Although that works, it's a bit crufty to be using oprofile related fields,
> especially seeing as oprofile is more or less unused these days.
>
> It also means perf is reliant on the fragile logic in setup_cpu_spec()
> which detects when we're using a logical PVR and copies back the PMU
> related fields from the raw CPU entry. So lets check the PVR directly.
Nice work. I wanted to do this for some time now
but never got to it. Thanks for doing this.
Reviewed-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
> Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
> Signed-off-by: Rashmica Gupta <rashmica.g@gmail.com>
> ---
> v2: fixed misspelling of PVR_VER_E500V2
>
> arch/powerpc/perf/e500-pmu.c | 10 ++++++----
> arch/powerpc/perf/e6500-pmu.c | 5 +++--
> arch/powerpc/perf/hv-24x7.c | 6 +++---
> arch/powerpc/perf/mpc7450-pmu.c | 5 +++--
> arch/powerpc/perf/power5+-pmu.c | 6 +++---
> arch/powerpc/perf/power5-pmu.c | 5 +++--
> arch/powerpc/perf/power6-pmu.c | 5 +++--
> arch/powerpc/perf/power7-pmu.c | 7 ++++---
> arch/powerpc/perf/power8-pmu.c | 5 +++--
> arch/powerpc/perf/power9-pmu.c | 4 +---
> arch/powerpc/perf/ppc970-pmu.c | 8 +++++---
> 11 files changed, 37 insertions(+), 29 deletions(-)
>
> diff --git a/arch/powerpc/perf/e500-pmu.c b/arch/powerpc/perf/e500-pmu.c
> index fb664929f5da..e1a185a30928 100644
> --- a/arch/powerpc/perf/e500-pmu.c
> +++ b/arch/powerpc/perf/e500-pmu.c
> @@ -122,12 +122,14 @@ static struct fsl_emb_pmu e500_pmu = {
>
> static int init_e500_pmu(void)
> {
> - if (!cur_cpu_spec->oprofile_cpu_type)
> - return -ENODEV;
> + unsigned int pvr = mfspr(SPRN_PVR);
>
> - if (!strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc/e500mc"))
> + /* ec500mc */
> + if ((PVR_VER(pvr) == PVR_VER_E500MC) || (PVR_VER(pvr) == PVR_VER_E5500))
> num_events = 256;
> - else if (strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc/e500"))
> + /* e500 */
> + else if ((PVR_VER(pvr) != PVR_VER_E500V1) &&
> + (PVR_VER(pvr) != PVR_VER_E500V2))
> return -ENODEV;
>
> return register_fsl_emb_pmu(&e500_pmu);
> diff --git a/arch/powerpc/perf/e6500-pmu.c b/arch/powerpc/perf/e6500-pmu.c
> index 3d877aa777b5..47c93d13da1a 100644
> --- a/arch/powerpc/perf/e6500-pmu.c
> +++ b/arch/powerpc/perf/e6500-pmu.c
> @@ -111,8 +111,9 @@ static struct fsl_emb_pmu e6500_pmu = {
>
> static int init_e6500_pmu(void)
> {
> - if (!cur_cpu_spec->oprofile_cpu_type ||
> - strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc/e6500"))
> + unsigned int pvr = mfspr(SPRN_PVR);
> +
> + if (PVR_VER(pvr) != PVR_VER_E6500)
> return -ENODEV;
>
> return register_fsl_emb_pmu(&e6500_pmu);
> diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
> index 72238eedc360..30dd379ddcd3 100644
> --- a/arch/powerpc/perf/hv-24x7.c
> +++ b/arch/powerpc/perf/hv-24x7.c
> @@ -1583,16 +1583,16 @@ static int hv_24x7_init(void)
> {
> int r;
> unsigned long hret;
> + unsigned int pvr = mfspr(SPRN_PVR);
> struct hv_perf_caps caps;
>
> if (!firmware_has_feature(FW_FEATURE_LPAR)) {
> pr_debug("not a virtualized system, not enabling\n");
> return -ENODEV;
> - } else if (!cur_cpu_spec->oprofile_cpu_type)
> - return -ENODEV;
> + }
>
> /* POWER8 only supports v1, while POWER9 only supports v2. */
> - if (!strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power8"))
> + if (PVR_VER(pvr) == PVR_POWER8)
> interface_version = 1;
> else {
> interface_version = 2;
> diff --git a/arch/powerpc/perf/mpc7450-pmu.c b/arch/powerpc/perf/mpc7450-pmu.c
> index d115c5635bf3..17e69cabbcac 100644
> --- a/arch/powerpc/perf/mpc7450-pmu.c
> +++ b/arch/powerpc/perf/mpc7450-pmu.c
> @@ -413,8 +413,9 @@ struct power_pmu mpc7450_pmu = {
>
> static int __init init_mpc7450_pmu(void)
> {
> - if (!cur_cpu_spec->oprofile_cpu_type ||
> - strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc/7450"))
> + unsigned int pvr = mfspr(SPRN_PVR);
> +
> + if (PVR_VER(pvr) != PVR_7450)
> return -ENODEV;
>
> return register_power_pmu(&mpc7450_pmu);
> diff --git a/arch/powerpc/perf/power5+-pmu.c b/arch/powerpc/perf/power5+-pmu.c
> index 0526dac66007..17a32e7ef234 100644
> --- a/arch/powerpc/perf/power5+-pmu.c
> +++ b/arch/powerpc/perf/power5+-pmu.c
> @@ -679,9 +679,9 @@ static struct power_pmu power5p_pmu = {
>
> static int __init init_power5p_pmu(void)
> {
> - if (!cur_cpu_spec->oprofile_cpu_type ||
> - (strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power5+")
> - && strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power5++")))
> + unsigned int pvr = mfspr(SPRN_PVR);
> +
> + if (PVR_VER(pvr) != PVR_POWER5p)
> return -ENODEV;
>
> return register_power_pmu(&power5p_pmu);
> diff --git a/arch/powerpc/perf/power5-pmu.c b/arch/powerpc/perf/power5-pmu.c
> index 4dc99f9f7962..844782e6d367 100644
> --- a/arch/powerpc/perf/power5-pmu.c
> +++ b/arch/powerpc/perf/power5-pmu.c
> @@ -620,8 +620,9 @@ static struct power_pmu power5_pmu = {
>
> static int __init init_power5_pmu(void)
> {
> - if (!cur_cpu_spec->oprofile_cpu_type ||
> - strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power5"))
> + unsigned int pvr = mfspr(SPRN_PVR);
> +
> + if (PVR_VER(pvr) != PVR_POWER5)
> return -ENODEV;
>
> return register_power_pmu(&power5_pmu);
> diff --git a/arch/powerpc/perf/power6-pmu.c b/arch/powerpc/perf/power6-pmu.c
> index 9c9d646b68a1..9659b781f588 100644
> --- a/arch/powerpc/perf/power6-pmu.c
> +++ b/arch/powerpc/perf/power6-pmu.c
> @@ -542,8 +542,9 @@ static struct power_pmu power6_pmu = {
>
> static int __init init_power6_pmu(void)
> {
> - if (!cur_cpu_spec->oprofile_cpu_type ||
> - strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power6"))
> + unsigned int pvr = mfspr(SPRN_PVR);
> +
> + if (PVR_VER(pvr) != PVR_POWER6)
> return -ENODEV;
>
> return register_power_pmu(&power6_pmu);
> diff --git a/arch/powerpc/perf/power7-pmu.c b/arch/powerpc/perf/power7-pmu.c
> index 6dbae9884ec4..79f05a7f28c6 100644
> --- a/arch/powerpc/perf/power7-pmu.c
> +++ b/arch/powerpc/perf/power7-pmu.c
> @@ -447,11 +447,12 @@ static struct power_pmu power7_pmu = {
>
> static int __init init_power7_pmu(void)
> {
> - if (!cur_cpu_spec->oprofile_cpu_type ||
> - strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power7"))
> + unsigned int pvr = mfspr(SPRN_PVR);
> +
> + if ((PVR_VER(pvr) != PVR_POWER7) && (PVR_VER(pvr) != PVR_POWER7p))
> return -ENODEV;
>
> - if (pvr_version_is(PVR_POWER7p))
> + if (PVR_VER(pvr) == PVR_POWER7p)
> power7_pmu.flags |= PPMU_SIAR_VALID;
>
> return register_power_pmu(&power7_pmu);
> diff --git a/arch/powerpc/perf/power8-pmu.c b/arch/powerpc/perf/power8-pmu.c
> index d12a2db26353..81a5142efab0 100644
> --- a/arch/powerpc/perf/power8-pmu.c
> +++ b/arch/powerpc/perf/power8-pmu.c
> @@ -382,9 +382,10 @@ static struct power_pmu power8_pmu = {
> static int __init init_power8_pmu(void)
> {
> int rc;
> + unsigned int pvr = mfspr(SPRN_PVR);
>
> - if (!cur_cpu_spec->oprofile_cpu_type ||
> - strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power8"))
> + if ((PVR_VER(pvr) != PVR_POWER8E) && (PVR_VER(pvr) != PVR_POWER8NVL)
> + && PVR_VER(pvr) != PVR_POWER8)
> return -ENODEV;
>
> rc = register_power_pmu(&power8_pmu);
> diff --git a/arch/powerpc/perf/power9-pmu.c b/arch/powerpc/perf/power9-pmu.c
> index 0ff9c43733e9..6b414b8bedfd 100644
> --- a/arch/powerpc/perf/power9-pmu.c
> +++ b/arch/powerpc/perf/power9-pmu.c
> @@ -438,9 +438,7 @@ static int __init init_power9_pmu(void)
> int rc = 0;
> unsigned int pvr = mfspr(SPRN_PVR);
>
> - /* Comes from cpu_specs[] */
> - if (!cur_cpu_spec->oprofile_cpu_type ||
> - strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power9"))
> + if (PVR_VER(pvr) != PVR_POWER9)
> return -ENODEV;
>
> /* Blacklist events */
> diff --git a/arch/powerpc/perf/ppc970-pmu.c b/arch/powerpc/perf/ppc970-pmu.c
> index 8b6a8a36fa38..5832de10e073 100644
> --- a/arch/powerpc/perf/ppc970-pmu.c
> +++ b/arch/powerpc/perf/ppc970-pmu.c
> @@ -492,9 +492,11 @@ static struct power_pmu ppc970_pmu = {
>
> static int __init init_ppc970_pmu(void)
> {
> - if (!cur_cpu_spec->oprofile_cpu_type ||
> - (strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/970")
> - && strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/970MP")))
> + unsigned int pvr = mfspr(SPRN_PVR);
> +
> + if ((PVR_VER(pvr) != PVR_970) && (PVR_VER(pvr) != PVR_970MP)
> + && (PVR_VER(pvr) != PVR_970FX)
> + && (PVR_VER(pvr) != PVR_970GX))
> return -ENODEV;
>
> return register_power_pmu(&ppc970_pmu);
next prev parent reply other threads:[~2019-02-06 12:14 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-06 6:30 [PATCH v2] powerpc/perf: Use PVR rather than oprofile field to determine CPU version Rashmica Gupta
2019-02-06 12:11 ` Madhavan Srinivasan [this message]
2021-03-01 12:15 ` Christophe Leroy
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=3085b26c-d4af-1f41-c6b6-9d523290e843@linux.vnet.ibm.com \
--to=maddy@linux.vnet.ibm.com \
--cc=linuxppc-dev@lists.ozlabs.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).