Re: [PATCH v3 1/2] powerpc/perf: Use PVR rather than oprofile field to determine CPU version

From: kajoljain
Date: Thu Mar 04 2021 - 00:10:19 EST

Next message: kernel test robot: "kernel/sched/fair.c:8894:20: warning: stack frame size of 1072 bytes in function 'update_sd_lb_stats'"
Previous message: Paul E. McKenney: "Re: XDP socket rings, and LKMM litmus tests"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On 3/1/21 5:39 PM, Christophe Leroy wrote:
> From: Rashmica Gupta <rashmica.g@xxxxxxxxx>
>
> 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.
>
> Suggested-by: Michael Ellerman <mpe@xxxxxxxxxxxxxx>
> Signed-off-by: Rashmica Gupta <rashmica.g@xxxxxxxxx>
> Reviewed-by: Madhavan Srinivasan <maddy@xxxxxxxxxxxxxxxxxx>
> [chleroy: Added power10 and fixed checkpatch issues]
> Signed-off-by: Christophe Leroy <christophe.leroy@xxxxxxxxxx>

Patch looks good to me.

Reviewed-and-tested-By: Kajol Jain<kjain@xxxxxxxxxxxxx> [For 24x7 side changes]

Thanks,
Kajol Jain

> ---
> arch/powerpc/perf/e500-pmu.c | 9 +++++----
> arch/powerpc/perf/e6500-pmu.c | 5 +++--
> arch/powerpc/perf/hv-24x7.c | 6 +++---
> arch/powerpc/perf/mpc7450-pmu.c | 5 +++--
> arch/powerpc/perf/power10-pmu.c | 6 ++----
> 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 | 7 ++++---
> 12 files changed, 37 insertions(+), 33 deletions(-)
>
> diff --git a/arch/powerpc/perf/e500-pmu.c b/arch/powerpc/perf/e500-pmu.c
> index a59c33bed32a..e3e1a68eb1d5 100644
> --- a/arch/powerpc/perf/e500-pmu.c
> +++ b/arch/powerpc/perf/e500-pmu.c
> @@ -118,12 +118,13 @@ 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 44ad65da82ed..bd779a2338f8 100644
> --- a/arch/powerpc/perf/e6500-pmu.c
> +++ b/arch/powerpc/perf/e6500-pmu.c
> @@ -107,8 +107,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 e5eb33255066..f3f2472fa1c6 100644
> --- a/arch/powerpc/perf/hv-24x7.c
> +++ b/arch/powerpc/perf/hv-24x7.c
> @@ -1718,16 +1718,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 e39b15b79a83..552d51a925d3 100644
> --- a/arch/powerpc/perf/mpc7450-pmu.c
> +++ b/arch/powerpc/perf/mpc7450-pmu.c
> @@ -417,8 +417,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/power10-pmu.c b/arch/powerpc/perf/power10-pmu.c
> index a901c1348cad..d1395844a329 100644
> --- a/arch/powerpc/perf/power10-pmu.c
> +++ b/arch/powerpc/perf/power10-pmu.c
> @@ -566,12 +566,10 @@ int init_power10_pmu(void)
> unsigned int pvr;
> int rc;
>
> - /* Comes from cpu_specs[] */
> - if (!cur_cpu_spec->oprofile_cpu_type ||
> - strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power10"))
> + pvr = mfspr(SPRN_PVR);
> + if (PVR_VER(pvr) != PVR_POWER10)
> return -ENODEV;
>
> - pvr = mfspr(SPRN_PVR);
> /* Add the ppmu flag for power10 DD1 */
> if ((PVR_CFG(pvr) == 1))
> power10_pmu.flags |= PPMU_P10_DD1;
> diff --git a/arch/powerpc/perf/power5+-pmu.c b/arch/powerpc/perf/power5+-pmu.c
> index 18732267993a..a79eae40ef6d 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 = {
>
> int 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 cb611c1e7abe..35a9d7f3b4b9 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 = {
>
> int 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 69ef38216418..8aa220c712a7 100644
> --- a/arch/powerpc/perf/power6-pmu.c
> +++ b/arch/powerpc/perf/power6-pmu.c
> @@ -541,8 +541,9 @@ static struct power_pmu power6_pmu = {
>
> int 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 894c17f9a762..ca7373143b02 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 = {
>
> int 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 5282e8415ddf..5a396ba8bf58 100644
> --- a/arch/powerpc/perf/power8-pmu.c
> +++ b/arch/powerpc/perf/power8-pmu.c
> @@ -381,9 +381,10 @@ static struct power_pmu power8_pmu = {
> int 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 2a57e93a79dc..28ba1e98f93d 100644
> --- a/arch/powerpc/perf/power9-pmu.c
> +++ b/arch/powerpc/perf/power9-pmu.c
> @@ -444,9 +444,7 @@ int 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 1f8263785286..39a0a4d7841c 100644
> --- a/arch/powerpc/perf/ppc970-pmu.c
> +++ b/arch/powerpc/perf/ppc970-pmu.c
> @@ -491,9 +491,10 @@ static struct power_pmu ppc970_pmu = {
>
> int 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 message: kernel test robot: "kernel/sched/fair.c:8894:20: warning: stack frame size of 1072 bytes in function 'update_sd_lb_stats'"
Previous message: Paul E. McKenney: "Re: XDP socket rings, and LKMM litmus tests"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]