Re: [PATCH v3 2/2] powerpc: Remove remaining parts of oprofile

Re: [PATCH v3 2/2] powerpc: Remove remaining parts of oprofile

[Viresh Kumar]

On 01-03-21, 12:09, Christophe Leroy wrote:
> Commit 9850b6c69356 ("arch: powerpc: Remove oprofile") removed
> oprofile.
> 
> Remove all remaining parts of it.
> 
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
>  arch/powerpc/include/asm/cputable.h       |  3 --
>  arch/powerpc/kernel/cputable.c            | 66 +----------------------
>  arch/powerpc/kernel/dt_cpu_ftrs.c         |  4 --
>  arch/powerpc/platforms/cell/spufs/spufs.h |  2 +-
>  4 files changed, 3 insertions(+), 72 deletions(-)

Great, I wasn't sure how the handle the cpu type stuff and so left it
for the right people to handle. :)

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

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

[kajoljain]

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);
>