mbox series

[v7,0/4] arm64 SMMUv3 PMU driver with IORT support

Message ID 20190326151753.19384-1-shameerali.kolothum.thodi@huawei.com
Headers show
Series arm64 SMMUv3 PMU driver with IORT support | expand

Message

Shameerali Kolothum Thodi March 26, 2019, 3:17 p.m. UTC
This adds a driver for the SMMUv3 PMU into the perf framework.
It includes an IORT update to support PM Counter Groups.

This is based on the initial work done by Neil Leeder[1]

SMMUv3 PMCG devices are named as smmuv3_pmcg_<phys_addr_page>
where <phys_addr_page> is the physical page address of the SMMU PMCG.
For example, the PMCG at 0xff88840000 is named smmuv3_pmcg_ff88840

Usage example:
For common arch supported events:
perf stat -e smmuv3_pmcg_ff88840/transaction,filter_enable=1,
 filter_span=1,filter_stream_id=0x42/ -a netperf

For IMP DEF events:
perf stat -e smmuv3_pmcg_ff88840/event=id/ -a netperf

This is sanity tested on a HiSilicon platform that requires
a quirk to run  it properly. As per HiSilicon erratum  #162001800,
PMCG event counter registers (SMMU_PMCG_EVCNTRn) on HiSilicon Hip08
platforms are read only and this prevents the software from setting
the initial period on event start. Unfortunately we were a bit late
in the cycle to detect this issue and now require software workaround
for this. Patch #4 is added to this series to provide a workaround
for this issue.

Further testing on supported platforms are very much welcome.

v6 --> v7
-Addressed comments from Robin and Lorenzo.
-Added R-by from Robin/Hanjun and A-by from Lorenzo.

v5 ---> v6
-Addressed comments from Robin and Andrew.
-Changed the way global filter settings are applied as a probable
 fix to the v5 bug where in-use settings gets overwritten.
-Use of PMCG model number to identify the platform.
-Added R-by from Robin to patches #1 and #3.

v4 ---> v5
-IORT code is modified to pass the option/quirk flags to the driver
 through platform_data (patch #4), based on Robin's comments.
-Removed COMPILE_TEST (patch #2).

v3 --> v4

-Addressed comments from Jean and Robin.
-Merged dma config callbacks as per Lorenzo's comments(patch #1).
-Added handling of Global(Counter0) filter settings mode(patch #2).
-Added patch #4 to address HiSilicon erratum  #162001800
-
v2 --> v3

-Addressed comments from Robin.
-Removed iort helper function to retrieve the PMCG reference smmu.
-PMCG devices are now named using the base address

v1 --> v2

- Addressed comments from Robin.
- Added an helper to retrieve the associated smmu dev and named PMUs
  to make the association visible to user.
- Added MSI support  for overflow irq

[1]https://www.spinics.net/lists/arm-kernel/msg598591.html


Neil Leeder (2):
  ACPI/IORT: Add support for PMCG
  perf/smmuv3: Add arm64 smmuv3 pmu driver

Shameer Kolothum (2):
  perf/smmuv3: Add MSI irq support
  perf/smmuv3: Enable HiSilicon Erratum 162001800 quirk

 drivers/acpi/arm64/iort.c     | 131 +++++--
 drivers/perf/Kconfig          |   9 +
 drivers/perf/Makefile         |   1 +
 drivers/perf/arm_smmuv3_pmu.c | 868 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/acpi_iort.h     |   8 +
 5 files changed, 993 insertions(+), 24 deletions(-)
 create mode 100644 drivers/perf/arm_smmuv3_pmu.c

-- 
2.7.4

Comments

Lorenzo Pieralisi April 4, 2019, 2:49 p.m. UTC | #1
On Tue, Mar 26, 2019 at 03:17:53PM +0000, Shameer Kolothum wrote:
> HiSilicon erratum 162001800 describes the limitation of

> SMMUv3 PMCG implementation on HiSilicon Hip08 platforms.

> 

> On these platforms, the PMCG event counter registers

> (SMMU_PMCG_EVCNTRn) are read only and as a result it

> is not possible to set the initial counter period value

> on event monitor start.

> 

> To work around this, the current value of the counter

> is read and used for delta calculations. OEM information

> from ACPI header is used to identify the affected hardware

> platforms.

> 

> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>

> Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>

> Reviewed-by: Robin Murphy <robin.murphy@arm.com>

> ---

>  drivers/acpi/arm64/iort.c     | 16 ++++++++++++++-

>  drivers/perf/arm_smmuv3_pmu.c | 48 ++++++++++++++++++++++++++++++++++++-------

>  include/linux/acpi_iort.h     |  1 +

>  3 files changed, 57 insertions(+), 8 deletions(-)

> 

> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c

> index e2c9b26..4dc68de 100644

> --- a/drivers/acpi/arm64/iort.c

> +++ b/drivers/acpi/arm64/iort.c

> @@ -1366,9 +1366,23 @@ static void __init arm_smmu_v3_pmcg_init_resources(struct resource *res,

>  				       ACPI_EDGE_SENSITIVE, &res[2]);

>  }

>  

> +static struct acpi_platform_list pmcg_plat_info[] __initdata = {

> +	/* HiSilicon Hip08 Platform */

> +	{"HISI  ", "HIP08   ", 0, ACPI_SIG_IORT, greater_than_or_equal, 0,

> +	 IORT_SMMU_V3_PMCG_HISI_HIP08},

> +	{ }

> +};


Hopefully we won't have plaforms with *some* counters that eg are
read-only and others that are read-write, or any other quirks
combination that this hack can't solve, otherwise we are back to square
one, namely, to the specifications (IORT or PMCG, or both).

As it stands it is OK since we can revisit it later so:

Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>


>  static int __init arm_smmu_v3_pmcg_add_platdata(struct platform_device *pdev)

>  {

> -	u32 model = IORT_SMMU_V3_PMCG_GENERIC;

> +	u32 model;

> +	int idx;

> +

> +	idx = acpi_match_platform_list(pmcg_plat_info);

> +	if (idx >= 0)

> +		model = pmcg_plat_info[idx].data;

> +	else

> +		model = IORT_SMMU_V3_PMCG_GENERIC;

>  

>  	return platform_device_add_data(pdev, &model, sizeof(model));

>  }

> diff --git a/drivers/perf/arm_smmuv3_pmu.c b/drivers/perf/arm_smmuv3_pmu.c

> index 7803e9e..6b3c0ed 100644

> --- a/drivers/perf/arm_smmuv3_pmu.c

> +++ b/drivers/perf/arm_smmuv3_pmu.c

> @@ -35,6 +35,7 @@

>   */

>  

>  #include <linux/acpi.h>

> +#include <linux/acpi_iort.h>

>  #include <linux/bitfield.h>

>  #include <linux/bitops.h>

>  #include <linux/cpuhotplug.h>

> @@ -93,6 +94,8 @@

>  

>  #define SMMU_PMCG_PA_SHIFT              12

>  

> +#define SMMU_PMCG_EVCNTR_RDONLY         BIT(0)

> +

>  static int cpuhp_state_num;

>  

>  struct smmu_pmu {

> @@ -108,6 +111,7 @@ struct smmu_pmu {

>  	void __iomem *reg_base;

>  	void __iomem *reloc_base;

>  	u64 counter_mask;

> +	u32 options;

>  	bool global_filter;

>  	u32 global_filter_span;

>  	u32 global_filter_sid;

> @@ -222,15 +226,27 @@ static void smmu_pmu_set_period(struct smmu_pmu *smmu_pmu,

>  	u32 idx = hwc->idx;

>  	u64 new;

>  

> -	/*

> -	 * We limit the max period to half the max counter value of the counter

> -	 * size, so that even in the case of extreme interrupt latency the

> -	 * counter will (hopefully) not wrap past its initial value.

> -	 */

> -	new = smmu_pmu->counter_mask >> 1;

> +	if (smmu_pmu->options & SMMU_PMCG_EVCNTR_RDONLY) {

> +		/*

> +		 * On platforms that require this quirk, if the counter starts

> +		 * at < half_counter value and wraps, the current logic of

> +		 * handling the overflow may not work. It is expected that,

> +		 * those platforms will have full 64 counter bits implemented

> +		 * so that such a possibility is remote(eg: HiSilicon HIP08).

> +		 */

> +		new = smmu_pmu_counter_get_value(smmu_pmu, idx);

> +	} else {

> +		/*

> +		 * We limit the max period to half the max counter value

> +		 * of the counter size, so that even in the case of extreme

> +		 * interrupt latency the counter will (hopefully) not wrap

> +		 * past its initial value.

> +		 */

> +		new = smmu_pmu->counter_mask >> 1;

> +		smmu_pmu_counter_set_value(smmu_pmu, idx, new);

> +	}

>  

>  	local64_set(&hwc->prev_count, new);

> -	smmu_pmu_counter_set_value(smmu_pmu, idx, new);

>  }

>  

>  static void smmu_pmu_set_event_filter(struct perf_event *event,

> @@ -669,6 +685,22 @@ static void smmu_pmu_reset(struct smmu_pmu *smmu_pmu)

>  		       smmu_pmu->reloc_base + SMMU_PMCG_OVSCLR0);

>  }

>  

> +static void smmu_pmu_get_acpi_options(struct smmu_pmu *smmu_pmu)

> +{

> +	u32 model;

> +

> +	model = *(u32 *)dev_get_platdata(smmu_pmu->dev);

> +

> +	switch (model) {

> +	case IORT_SMMU_V3_PMCG_HISI_HIP08:

> +		/* HiSilicon Erratum 162001800 */

> +		smmu_pmu->options |= SMMU_PMCG_EVCNTR_RDONLY;

> +		break;

> +	}

> +

> +	dev_notice(smmu_pmu->dev, "option mask 0x%x\n", smmu_pmu->options);

> +}

> +

>  static int smmu_pmu_probe(struct platform_device *pdev)

>  {

>  	struct smmu_pmu *smmu_pmu;

> @@ -748,6 +780,8 @@ static int smmu_pmu_probe(struct platform_device *pdev)

>  		return -EINVAL;

>  	}

>  

> +	smmu_pmu_get_acpi_options(smmu_pmu);

> +

>  	/* Pick one CPU to be the preferred one to use */

>  	smmu_pmu->on_cpu = raw_smp_processor_id();

>  	WARN_ON(irq_set_affinity(smmu_pmu->irq, cpumask_of(smmu_pmu->on_cpu)));

> diff --git a/include/linux/acpi_iort.h b/include/linux/acpi_iort.h

> index 052ef7b..723e4df 100644

> --- a/include/linux/acpi_iort.h

> +++ b/include/linux/acpi_iort.h

> @@ -32,6 +32,7 @@

>   * do with hardware or with IORT specification.

>   */

>  #define IORT_SMMU_V3_PMCG_GENERIC        0x00000000 /* Generic SMMUv3 PMCG */

> +#define IORT_SMMU_V3_PMCG_HISI_HIP08     0x00000001 /* HiSilicon HIP08 PMCG */

>  

>  int iort_register_domain_token(int trans_id, phys_addr_t base,

>  			       struct fwnode_handle *fw_node);

> -- 

> 2.7.4

> 

>
Will Deacon April 4, 2019, 3:47 p.m. UTC | #2
On Tue, Mar 26, 2019 at 03:17:53PM +0000, Shameer Kolothum wrote:
> HiSilicon erratum 162001800 describes the limitation of

> SMMUv3 PMCG implementation on HiSilicon Hip08 platforms.

> 

> On these platforms, the PMCG event counter registers

> (SMMU_PMCG_EVCNTRn) are read only and as a result it

> is not possible to set the initial counter period value

> on event monitor start.

> 

> To work around this, the current value of the counter

> is read and used for delta calculations. OEM information

> from ACPI header is used to identify the affected hardware

> platforms.

> 

> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>

> Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>

> Reviewed-by: Robin Murphy <robin.murphy@arm.com>

> ---

>  drivers/acpi/arm64/iort.c     | 16 ++++++++++++++-

>  drivers/perf/arm_smmuv3_pmu.c | 48 ++++++++++++++++++++++++++++++++++++-------

>  include/linux/acpi_iort.h     |  1 +

>  3 files changed, 57 insertions(+), 8 deletions(-)

> 

> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c

> index e2c9b26..4dc68de 100644

> --- a/drivers/acpi/arm64/iort.c

> +++ b/drivers/acpi/arm64/iort.c

> @@ -1366,9 +1366,23 @@ static void __init arm_smmu_v3_pmcg_init_resources(struct resource *res,

>  				       ACPI_EDGE_SENSITIVE, &res[2]);

>  }

>  

> +static struct acpi_platform_list pmcg_plat_info[] __initdata = {

> +	/* HiSilicon Hip08 Platform */

> +	{"HISI  ", "HIP08   ", 0, ACPI_SIG_IORT, greater_than_or_equal, 0,


Passing integer constant 0 for the reason feels wrong to me. I'm going to
change it to "Erratum #162001800" and also add an entry to
silicon-errata.txt.

Please shout if that's not ok.

Will