Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1443626 > unrolled thread

[PATCH v9 0/4] perf: Add APM X-Gene SoC Performance Monitoring Unit driver

Back to article view | Back to linux.kernel

Contents

  [PATCH v9 0/4] perf: Add APM X-Gene SoC Performance Monitoring Unit driver  Tai Nguyen <ttnguyen@apm.com> - 2016-07-14 19:40 +0200
    [PATCH v9 2/4] Documentation: Add documentation for APM X-Gene SoC PMU DTS binding Tai Nguyen <ttnguyen@apm.com> - 2016-07-14 19:40 +0200
    [PATCH v9 1/4] MAINTAINERS: Add entry for APM X-Gene SoC PMU driver Tai Nguyen <ttnguyen@apm.com> - 2016-07-14 19:40 +0200
    [PATCH v9 4/4] arm64: dts: apm: Add APM X-Gene SoC PMU DTS entries Tai Nguyen <ttnguyen@apm.com> - 2016-07-14 19:40 +0200
    Re: [PATCH v9 3/4] perf: xgene: Add APM X-Gene SoC Performance  Monitoring Unit driver Joe Perches <joe@perches.com> - 2016-07-14 19:50 +0200
      Re: [PATCH v9 3/4] perf: xgene: Add APM X-Gene SoC Performance  Monitoring Unit driver Tai Tri Nguyen <ttnguyen@apm.com> - 2016-07-14 20:00 +0200
        Re: [PATCH v9 3/4] perf: xgene: Add APM X-Gene SoC Performance  Monitoring Unit driver Tai Tri Nguyen <ttnguyen@apm.com> - 2016-07-14 20:10 +0200
          Re: [PATCH v9 3/4] perf: xgene: Add APM X-Gene SoC Performance  Monitoring Unit driver Mark Rutland <mark.rutland@arm.com> - 2016-07-15 11:50 +0200
            Re: [PATCH v9 3/4] perf: xgene: Add APM X-Gene SoC Performance  Monitoring Unit driver Tai Tri Nguyen <ttnguyen@apm.com> - 2016-07-15 19:40 +0200

#1443626 — [PATCH v9 0/4] perf: Add APM X-Gene SoC Performance Monitoring Unit driver

In addition to the X-Gene ARM CPU performance monitoring unit (PMU), there
are PMU for the SoC system devices such as L3 cache(s), I/O bridge(s),
memory controller bridges and memory. These PMU devices are loosely
architected to follow the same model as the PMU for ARM cores.

Signed-off-by: Tai Nguyen <ttnguyen@apm.com>
---

v9:
 * Add commmit messages to the patches.

v8:
 * MAINTAINERS: Fix section header in one line
 * Change module_platform_driver to builtin_platform_driver
   Get rid of the use of module.h and its no-ops macros

v7:
 * Remove const from the definition of xgene_pmu_cpumask_attrs
 * Validate the event group as a whole, disallow creating groups containing
   mixed PMUs
 * Implement pmu::pmu_enable() and pmu::pmu_disable() to let the perf core
   starts and stops the counters properly
 * Using list_for_each_entry() instead of list_for_each_entry_safe() to iterate
   over the list of pmu sub-devices
 * Fix resource leak issue in case of registering perf devices fails
 * Pass on returned error if acpi_walk_namespace() fails
 * Remove unused xgene_pmu_data::data
 * Move enable interrupt after probing pmu sub-devices

v6:
 * Add IRQF_NOBALANCING and IRQF_NO_THREAD flags to the PMU overflow interrupt
   Exclude the interrupt from irq balancing and prevent the context from being
   threaded

v5:
 * Remove hw_perf_event::extra_reg field use
   Change GET_CNTR to use hw_perf_event::idx
   Change GET_AGENTID/GET_AGEN1ID to use hw_perf_event::config_base
 * Use compound literal structure defines for format and event attribute groups
   to statically define them at compile time
 * Bitwise invert the meaning of agent mask in config1 field.
 * Fix update pmu_counter_event pointer before starting event
 * Add reset of pmu_dev->pmu_counter_event to NULL in xgene_perf_del
 * Use exactly half of max period to fix the overflow counter issue and account
   for the possiblity of extreme interrupt latency
 * Use spin lock instead of interrupt masking in overflow interrupt handler
 * Remove unnecessary update of hw_perf_event::period_left

v4:
 * Alphabetically sorting header files
 * Remove dynamic allocation for PMU format and event attribute groups
   Create shared constant attribute groups per each class
 * Remove perf_sample_data as this perf driver doesn't support sampling
 * Consistently use the PCP_PMU_V{1,2} defines
 * Set affinity to make sure the overflow interrupt is handled by the
   same assigned CPU

v3:
 * Remove index property use in PMU device sub nodes

v2:
 * Use bitmask for event asignned counter mask pmu_dev->cntr_assign_mask
 * Remove unnecessary spinlocks in perf add/del operations
 * Remove unnecessary condition checks
 * Enforce CPU assignment to one CPU for perf operarations
 * Set the task_ctx_nr to perf_invalid_context for perf driver
 * Remove irrelevant pt_rregs
 * Change perf sysfs attributes to be fixed instead of dynamic
 * Fix checking for an ACPI companion device instead of EFI enable
 * Add documentation for config/config1 fields format and perf tool example

---

Tai Nguyen (4):
  MAINTAINERS: Add entry for APM X-Gene SoC PMU driver
  Documentation: Add documentation for APM X-Gene SoC PMU DTS binding
  perf: xgene: Add APM X-Gene SoC Performance Monitoring Unit driver
  arm64: dts: apm: Add APM X-Gene SoC PMU DTS entries

 .../devicetree/bindings/perf/apm-xgene-pmu.txt     |  112 ++
 Documentation/perf/xgene-pmu.txt                   |   48 +
 MAINTAINERS                                        |    7 +
 arch/arm64/boot/dts/apm/apm-storm.dtsi             |   58 +
 drivers/perf/Kconfig                               |    7 +
 drivers/perf/Makefile                              |    1 +
 drivers/perf/xgene_pmu.c                           | 1392 ++++++++++++++++++++
 7 files changed, 1625 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/perf/apm-xgene-pmu.txt
 create mode 100644 Documentation/perf/xgene-pmu.txt
 create mode 100644 drivers/perf/xgene_pmu.c

-- 
1.9.1

[toc] | [next] | [standalone]

#1443628 — [PATCH v9 2/4] Documentation: Add documentation for APM X-Gene SoC PMU DTS binding

Driver providing perf backend for the SoC-wide PMU hardware found
in APM X-Gene SoCs.

Signed-off-by: Tai Nguyen <ttnguyen@apm.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 .../devicetree/bindings/perf/apm-xgene-pmu.txt     | 112 +++++++++++++++++++++
 1 file changed, 112 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/perf/apm-xgene-pmu.txt

diff --git a/Documentation/devicetree/bindings/perf/apm-xgene-pmu.txt b/Documentation/devicetree/bindings/perf/apm-xgene-pmu.txt
new file mode 100644
index 0000000..afb11cf
--- /dev/null
+++ b/Documentation/devicetree/bindings/perf/apm-xgene-pmu.txt
@@ -0,0 +1,112 @@
+* APM X-Gene SoC PMU bindings
+
+This is APM X-Gene SoC PMU (Performance Monitoring Unit) module.
+The following PMU devices are supported:
+
+  L3C			- L3 cache controller
+  IOB			- IO bridge
+  MCB			- Memory controller bridge
+  MC			- Memory controller
+
+The following section describes the SoC PMU DT node binding.
+
+Required properties:
+- compatible		: Shall be "apm,xgene-pmu" for revision 1 or
+                          "apm,xgene-pmu-v2" for revision 2.
+- regmap-csw		: Regmap of the CPU switch fabric (CSW) resource.
+- regmap-mcba		: Regmap of the MCB-A (memory bridge) resource.
+- regmap-mcbb		: Regmap of the MCB-B (memory bridge) resource.
+- reg			: First resource shall be the CPU bus PMU resource.
+- interrupts            : Interrupt-specifier for PMU IRQ.
+
+Required properties for L3C subnode:
+- compatible		: Shall be "apm,xgene-pmu-l3c".
+- reg			: First resource shall be the L3C PMU resource.
+
+Required properties for IOB subnode:
+- compatible		: Shall be "apm,xgene-pmu-iob".
+- reg			: First resource shall be the IOB PMU resource.
+
+Required properties for MCB subnode:
+- compatible		: Shall be "apm,xgene-pmu-mcb".
+- reg			: First resource shall be the MCB PMU resource.
+- enable-bit-index	: The bit indicates if the according MCB is enabled.
+
+Required properties for MC subnode:
+- compatible		: Shall be "apm,xgene-pmu-mc".
+- reg			: First resource shall be the MC PMU resource.
+- enable-bit-index	: The bit indicates if the according MC is enabled.
+
+Example:
+	csw: csw@7e200000 {
+		compatible = "apm,xgene-csw", "syscon";
+		reg = <0x0 0x7e200000 0x0 0x1000>;
+	};
+
+	mcba: mcba@7e700000 {
+		compatible = "apm,xgene-mcb", "syscon";
+		reg = <0x0 0x7e700000 0x0 0x1000>;
+	};
+
+	mcbb: mcbb@7e720000 {
+		compatible = "apm,xgene-mcb", "syscon";
+		reg = <0x0 0x7e720000 0x0 0x1000>;
+	};
+
+	pmu: pmu@78810000 {
+		compatible = "apm,xgene-pmu-v2";
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+		regmap-csw = <&csw>;
+		regmap-mcba = <&mcba>;
+		regmap-mcbb = <&mcbb>;
+		reg = <0x0 0x78810000 0x0 0x1000>;
+		interrupts = <0x0 0x22 0x4>;
+
+		pmul3c@7e610000 {
+			compatible = "apm,xgene-pmu-l3c";
+			reg = <0x0 0x7e610000 0x0 0x1000>;
+		};
+
+		pmuiob@7e940000 {
+			compatible = "apm,xgene-pmu-iob";
+			reg = <0x0 0x7e940000 0x0 0x1000>;
+		};
+
+		pmucmcb@7e710000 {
+			compatible = "apm,xgene-pmu-mcb";
+			reg = <0x0 0x7e710000 0x0 0x1000>;
+			enable-bit-index = <0>;
+		};
+
+		pmucmcb@7e730000 {
+			compatible = "apm,xgene-pmu-mcb";
+			reg = <0x0 0x7e730000 0x0 0x1000>;
+			enable-bit-index = <1>;
+		};
+
+		pmucmc@7e810000 {
+			compatible = "apm,xgene-pmu-mc";
+			reg = <0x0 0x7e810000 0x0 0x1000>;
+			enable-bit-index = <0>;
+		};
+
+		pmucmc@7e850000 {
+			compatible = "apm,xgene-pmu-mc";
+			reg = <0x0 0x7e850000 0x0 0x1000>;
+			enable-bit-index = <1>;
+		};
+
+		pmucmc@7e890000 {
+			compatible = "apm,xgene-pmu-mc";
+			reg = <0x0 0x7e890000 0x0 0x1000>;
+			enable-bit-index = <2>;
+		};
+
+		pmucmc@7e8d0000 {
+			compatible = "apm,xgene-pmu-mc";
+			reg = <0x0 0x7e8d0000 0x0 0x1000>;
+			enable-bit-index = <3>;
+		};
+	};
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1443633 — [PATCH v9 1/4] MAINTAINERS: Add entry for APM X-Gene SoC PMU driver

This patch adds the MAINTAINERS entry for APM X-Gene SoC PMU driver.

Signed-off-by: Tai Nguyen <ttnguyen@apm.com>
---
 MAINTAINERS | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 1209323..41938e7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -841,6 +841,13 @@ S:	Supported
 F:	drivers/net/ethernet/apm/xgene/
 F:	Documentation/devicetree/bindings/net/apm-xgene-enet.txt
 
+APPLIED MICRO (APM) X-GENE SOC PMU
+M:	Tai Nguyen <ttnguyen@apm.com>
+S:	Supported
+F:	drivers/perf/xgene_pmu.c
+F:	Documentation/perf/xgene-pmu.txt
+F:	Documentation/devicetree/bindings/perf/apm-xgene-pmu.txt
+
 APTINA CAMERA SENSOR PLL
 M:	Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
 L:	linux-media@vger.kernel.org
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1443635 — [PATCH v9 4/4] arm64: dts: apm: Add APM X-Gene SoC PMU DTS entries

This patch adds APM X-Gene SoC PMU DTS entries.

Signed-off-by: Tai Nguyen <ttnguyen@apm.com>
---
 arch/arm64/boot/dts/apm/apm-storm.dtsi | 58 ++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/arch/arm64/boot/dts/apm/apm-storm.dtsi b/arch/arm64/boot/dts/apm/apm-storm.dtsi
index 5147d76..1d57820 100644
--- a/arch/arm64/boot/dts/apm/apm-storm.dtsi
+++ b/arch/arm64/boot/dts/apm/apm-storm.dtsi
@@ -572,6 +572,64 @@
 			};
 		};
 
+		pmu: pmu@78810000 {
+			compatible = "apm,xgene-pmu-v2";
+			#address-cells = <2>;
+			#size-cells = <2>;
+			ranges;
+			regmap-csw = <&csw>;
+			regmap-mcba = <&mcba>;
+			regmap-mcbb = <&mcbb>;
+			reg = <0x0 0x78810000 0x0 0x1000>;
+			interrupts = <0x0 0x22 0x4>;
+
+			pmul3c@7e610000 {
+				compatible = "apm,xgene-pmu-l3c";
+				reg = <0x0 0x7e610000 0x0 0x1000>;
+			};
+
+			pmuiob@7e940000 {
+				compatible = "apm,xgene-pmu-iob";
+				reg = <0x0 0x7e940000 0x0 0x1000>;
+			};
+
+			pmucmcb@7e710000 {
+				compatible = "apm,xgene-pmu-mcb";
+				reg = <0x0 0x7e710000 0x0 0x1000>;
+				enable-bit-index = <0>;
+			};
+
+			pmucmcb@7e730000 {
+				compatible = "apm,xgene-pmu-mcb";
+				reg = <0x0 0x7e730000 0x0 0x1000>;
+				enable-bit-index = <1>;
+			};
+
+			pmucmc@7e810000 {
+				compatible = "apm,xgene-pmu-mc";
+				reg = <0x0 0x7e810000 0x0 0x1000>;
+				enable-bit-index = <0>;
+			};
+
+			pmucmc@7e850000 {
+				compatible = "apm,xgene-pmu-mc";
+				reg = <0x0 0x7e850000 0x0 0x1000>;
+				enable-bit-index = <1>;
+			};
+
+			pmucmc@7e890000 {
+				compatible = "apm,xgene-pmu-mc";
+				reg = <0x0 0x7e890000 0x0 0x1000>;
+				enable-bit-index = <2>;
+			};
+
+			pmucmc@7e8d0000 {
+				compatible = "apm,xgene-pmu-mc";
+				reg = <0x0 0x7e8d0000 0x0 0x1000>;
+				enable-bit-index = <3>;
+			};
+		};
+
 		pcie0: pcie@1f2b0000 {
 			status = "disabled";
 			device_type = "pci";
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1443636 — Re: [PATCH v9 3/4] perf: xgene: Add APM X-Gene SoC Performance Monitoring Unit driver

On Thu, 2016-07-14 at 10:27 -0700, Tai Nguyen wrote:
> This patch adds a driver for the SoC-wide (AKA uncore) PMU hardware
> found in APM X-Gene SoCs.

trivia:

> diff --git a/drivers/perf/xgene_pmu.c b/drivers/perf/xgene_pmu.c
[]
> +struct xgene_pmu_dev_ctx {
> +	char *name;
> +	struct list_head next;
> +	struct xgene_pmu_dev *pmu_dev;
> +	struct hw_pmu_info inf;
> +};

Probably better to use something like
	char	name[20];
as the kasprintf can fail and this doesn't
seem to be freed anywhere.
> +static char *xgene_pmu_dev_name(u32 type, int id)
> +{
> +	switch (type) {
> +	case PMU_TYPE_L3C:
> +		return kasprintf(GFP_KERNEL, "l3c%d", id);
> +	case PMU_TYPE_IOB:
> +		return kasprintf(GFP_KERNEL, "iob%d", id);
> +	case PMU_TYPE_MCB:
> +		return kasprintf(GFP_KERNEL, "mcb%d", id);
> +	case PMU_TYPE_MC:
> +		return kasprintf(GFP_KERNEL, "mc%d", id);
> +	default:
> +		return kasprintf(GFP_KERNEL, "unknown");
> +	}
> +}

[]
	
> +static struct
> +xgene_pmu_dev_ctx *acpi_get_pmu_hw_inf(struct xgene_pmu *xgene_pmu,
> +				       struct acpi_device *adev, u32 type)
> +{
[]
> +	ctx->name = xgene_pmu_dev_name(type, enable_bit);
> 

[toc] | [prev] | [next] | [standalone]

#1443653 — Re: [PATCH v9 3/4] perf: xgene: Add APM X-Gene SoC Performance Monitoring Unit driver

Hi Joe,

On Thu, Jul 14, 2016 at 10:47 AM, Joe Perches <joe@perches.com> wrote:
> On Thu, 2016-07-14 at 10:27 -0700, Tai Nguyen wrote:
>> This patch adds a driver for the SoC-wide (AKA uncore) PMU hardware
>> found in APM X-Gene SoCs.
>
> trivia:
>
>> diff --git a/drivers/perf/xgene_pmu.c b/drivers/perf/xgene_pmu.c
> []
>> +struct xgene_pmu_dev_ctx {
>> +     char *name;
>> +     struct list_head next;
>> +     struct xgene_pmu_dev *pmu_dev;
>> +     struct hw_pmu_info inf;
>> +};
>
> Probably better to use something like
>         char    name[20];
> as the kasprintf can fail and this doesn't
> seem to be freed anywhere.

Okay. I'll fix it shortly.

[...]

Thanks,
-- 
Tai

[toc] | [prev] | [next] | [standalone]

#1443656 — Re: [PATCH v9 3/4] perf: xgene: Add APM X-Gene SoC Performance Monitoring Unit driver

Hi Joe,

On Thu, Jul 14, 2016 at 10:54 AM, Tai Tri Nguyen <ttnguyen@apm.com> wrote:
> Hi Joe,
>
> On Thu, Jul 14, 2016 at 10:47 AM, Joe Perches <joe@perches.com> wrote:
>> On Thu, 2016-07-14 at 10:27 -0700, Tai Nguyen wrote:
>>> This patch adds a driver for the SoC-wide (AKA uncore) PMU hardware
>>> found in APM X-Gene SoCs.
>>
>> trivia:
>>
>>> diff --git a/drivers/perf/xgene_pmu.c b/drivers/perf/xgene_pmu.c
>> []
>>> +struct xgene_pmu_dev_ctx {
>>> +     char *name;
>>> +     struct list_head next;
>>> +     struct xgene_pmu_dev *pmu_dev;
>>> +     struct hw_pmu_info inf;
>>> +};
>>
>> Probably better to use something like
>>         char    name[20];
>> as the kasprintf can fail and this doesn't
>> seem to be freed anywhere.
>
> Okay. I'll fix it shortly.
>

I take it back.
I refer many other drivers using kasprintf and they do the same way I do.
Can you please check it again?

Thanks,
-- 
Tai

[toc] | [prev] | [next] | [standalone]

#1444123 — Re: [PATCH v9 3/4] perf: xgene: Add APM X-Gene SoC Performance Monitoring Unit driver

On Thu, Jul 14, 2016 at 11:05:48AM -0700, Tai Tri Nguyen wrote:
> Hi Joe,
> 
> On Thu, Jul 14, 2016 at 10:54 AM, Tai Tri Nguyen <ttnguyen@apm.com> wrote:
> > Hi Joe,
> >
> > On Thu, Jul 14, 2016 at 10:47 AM, Joe Perches <joe@perches.com> wrote:
> >> On Thu, 2016-07-14 at 10:27 -0700, Tai Nguyen wrote:
> >>> This patch adds a driver for the SoC-wide (AKA uncore) PMU hardware
> >>> found in APM X-Gene SoCs.
> >>
> >> trivia:
> >>
> >>> diff --git a/drivers/perf/xgene_pmu.c b/drivers/perf/xgene_pmu.c
> >> []
> >>> +struct xgene_pmu_dev_ctx {
> >>> +     char *name;
> >>> +     struct list_head next;
> >>> +     struct xgene_pmu_dev *pmu_dev;
> >>> +     struct hw_pmu_info inf;
> >>> +};
> >>
> >> Probably better to use something like
> >>         char    name[20];
> >> as the kasprintf can fail and this doesn't
> >> seem to be freed anywhere.
> >
> > Okay. I'll fix it shortly.
> >
> 
> I take it back.
> I refer many other drivers using kasprintf and they do the same way I do.
> Can you please check it again?

Joe is correct that you allocate a string with kasprintf, and this never
gets freed, even if the driver is removed. Thus, memory may be leaked.

If other drivers do the same, they are similarly wrong.

Even if this is a rare case, it's not good practice to leave allocations
unbalanced. So please fix this.

If you don't want to change the struct, another option is to use
devm_kasprintf. However, I suspect with all the accounting data
structures that will take up more space.

Thanks,
Mark.

[toc] | [prev] | [next] | [standalone]

#1444473 — Re: [PATCH v9 3/4] perf: xgene: Add APM X-Gene SoC Performance Monitoring Unit driver

Hi Mark and Joe,

On Fri, Jul 15, 2016 at 2:45 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> On Thu, Jul 14, 2016 at 11:05:48AM -0700, Tai Tri Nguyen wrote:
>> Hi Joe,
>>
>> On Thu, Jul 14, 2016 at 10:54 AM, Tai Tri Nguyen <ttnguyen@apm.com> wrote:
>> > Hi Joe,
>> >
>> > On Thu, Jul 14, 2016 at 10:47 AM, Joe Perches <joe@perches.com> wrote:
>> >> On Thu, 2016-07-14 at 10:27 -0700, Tai Nguyen wrote:
>> >>> This patch adds a driver for the SoC-wide (AKA uncore) PMU hardware
>> >>> found in APM X-Gene SoCs.
>> >>
>> >> trivia:
>> >>
>> >>> diff --git a/drivers/perf/xgene_pmu.c b/drivers/perf/xgene_pmu.c
>> >> []
>> >>> +struct xgene_pmu_dev_ctx {
>> >>> +     char *name;
>> >>> +     struct list_head next;
>> >>> +     struct xgene_pmu_dev *pmu_dev;
>> >>> +     struct hw_pmu_info inf;
>> >>> +};
>> >>
>> >> Probably better to use something like
>> >>         char    name[20];
>> >> as the kasprintf can fail and this doesn't
>> >> seem to be freed anywhere.
>> >
>> > Okay. I'll fix it shortly.
>> >
>>
>> I take it back.
>> I refer many other drivers using kasprintf and they do the same way I do.
>> Can you please check it again?
>
> Joe is correct that you allocate a string with kasprintf, and this never
> gets freed, even if the driver is removed. Thus, memory may be leaked.
>
> If other drivers do the same, they are similarly wrong.
>
> Even if this is a rare case, it's not good practice to leave allocations
> unbalanced. So please fix this.
>
> If you don't want to change the struct, another option is to use
> devm_kasprintf. However, I suspect with all the accounting data
> structures that will take up more space.
>

Thanks, I will change to use devm_kasprintf because I don't want to
change the struct.

Regards,
-- 
Tai

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web