All of lore.kernel.org
 help / color / mirror / Atom feed
From: kajoljain <kjain@linux.ibm.com>
To: Christophe Leroy <christophe.leroy@csgroup.eu>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Paul Mackerras <paulus@samba.org>,
	Michael Ellerman <mpe@ellerman.id.au>,
	Rashmica Gupta <rashmica.g@gmail.com>,
	Viresh Kumar <viresh.kumar@linaro.org>,
	Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
Cc: "Desnes A. Nunes do Rosario" <desnesn@linux.ibm.com>,
	linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 1/2] powerpc/perf: Use PVR rather than oprofile field to determine CPU version
Date: Thu, 4 Mar 2021 10:35:12 +0530	[thread overview]
Message-ID: <a8982899-8ee8-75e8-7921-5cb5a92fe210@linux.ibm.com> (raw)
In-Reply-To: <50ad16925a66ac53890286ceafbf84f6fc324baa.1614600516.git.christophe.leroy@csgroup.eu>



On 3/1/21 5:39 PM, Christophe Leroy wrote:
> From: Rashmica Gupta <rashmica.g@gmail.com>
> 
> 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@ellerman.id.au>
> Signed-off-by: Rashmica Gupta <rashmica.g@gmail.com>
> Reviewed-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
> [chleroy: Added power10 and fixed checkpatch issues]
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>

Patch looks good to me.

Reviewed-and-tested-By: Kajol Jain<kjain@linux.ibm.com> [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);
> 

      parent reply	other threads:[~2021-03-04  5:08 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-01 12:09 [PATCH v3 1/2] powerpc/perf: Use PVR rather than oprofile field to determine CPU version Christophe Leroy
2021-03-01 12:09 ` [PATCH v3 2/2] powerpc: Remove remaining parts of oprofile Christophe Leroy
2021-03-02  3:48   ` Viresh Kumar
2021-03-02  3:48     ` Viresh Kumar
2021-03-04  4:22 ` [PATCH v3 1/2] powerpc/perf: Use PVR rather than oprofile field to determine CPU version Athira Rajeev
2021-03-04  5:05 ` kajoljain [this message]

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=a8982899-8ee8-75e8-7921-5cb5a92fe210@linux.ibm.com \
    --to=kjain@linux.ibm.com \
    --cc=benh@kernel.crashing.org \
    --cc=christophe.leroy@csgroup.eu \
    --cc=desnesn@linux.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=maddy@linux.vnet.ibm.com \
    --cc=mpe@ellerman.id.au \
    --cc=paulus@samba.org \
    --cc=rashmica.g@gmail.com \
    --cc=viresh.kumar@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.