diff mbox series

[RFC,10/11] coresgith: etm-perf: Connect TRBE sink with ETE source

Message ID 1605012309-24812-11-git-send-email-anshuman.khandual@arm.com (mailing list archive)
State New, archived
Headers show
Series arm64: coresight: Enable ETE and TRBE | expand

Commit Message

Anshuman Khandual Nov. 10, 2020, 12:45 p.m. UTC
Unlike traditional sink devices, individual TRBE instances are not detected
via DT or ACPI nodes. Instead TRBE instances are detected during CPU online
process. Hence a path connecting ETE and TRBE on a given CPU would not have
been established until then. This adds two coresight helpers that will help
modify outward connections from a source device to establish and terminate
path to a given sink device. But this method might not be optimal and would
be reworked later.

Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 drivers/hwtracing/coresight/coresight-etm-perf.c | 30 ++++++++++++++++++++++++
 drivers/hwtracing/coresight/coresight-etm-perf.h |  4 ++++
 drivers/hwtracing/coresight/coresight-platform.c |  3 ++-
 drivers/hwtracing/coresight/coresight-trbe.c     |  2 ++
 include/linux/coresight.h                        |  2 ++
 5 files changed, 40 insertions(+), 1 deletion(-)

Comments

Suzuki K Poulose Nov. 12, 2020, 9:31 a.m. UTC | #1
Hi Anshuman,
On 11/10/20 12:45 PM, Anshuman Khandual wrote:
> Unlike traditional sink devices, individual TRBE instances are not detected
> via DT or ACPI nodes. Instead TRBE instances are detected during CPU online
> process. Hence a path connecting ETE and TRBE on a given CPU would not have
> been established until then. This adds two coresight helpers that will help
> modify outward connections from a source device to establish and terminate
> path to a given sink device. But this method might not be optimal and would
> be reworked later.
> 
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>

Instead of this, could we come up something like a percpu_sink concept ? That
way, the TRBE driver could register the percpu_sink for the corresponding CPU
and we don't have to worry about the order in which the ETE will be probed
on a hotplugged CPU. (i.e, if the TRBE is probed before the ETE, the following
approach would fail to register the sink).

And the default sink can be initialized when the ETE instance first starts
looking for it.

Suzuki
Anshuman Khandual Nov. 23, 2020, 5:37 a.m. UTC | #2
On 11/12/20 3:01 PM, Suzuki K Poulose wrote:
> Hi Anshuman,
> On 11/10/20 12:45 PM, Anshuman Khandual wrote:
>> Unlike traditional sink devices, individual TRBE instances are not detected
>> via DT or ACPI nodes. Instead TRBE instances are detected during CPU online
>> process. Hence a path connecting ETE and TRBE on a given CPU would not have
>> been established until then. This adds two coresight helpers that will help
>> modify outward connections from a source device to establish and terminate
>> path to a given sink device. But this method might not be optimal and would
>> be reworked later.
>>
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> 
> Instead of this, could we come up something like a percpu_sink concept ? That
> way, the TRBE driver could register the percpu_sink for the corresponding CPU
> and we don't have to worry about the order in which the ETE will be probed
> on a hotplugged CPU. (i.e, if the TRBE is probed before the ETE, the following
> approach would fail to register the sink).

Right, it wont work.

We already have a per cpu csdev sink. The current mechanism expects all ETEs
to have been established and the TRBEs just get plugged in during their init
while probing each individual cpus. During cpu hotplug in or out, a TRBE-ETE
link either gets created and destroyed. But it assumes that an ETE is always
present for TRBE to get plugged into or teared down from. csdev for TRBE sink
too gets released during cpu hot remove path.

Are you suggesting that there should be a percpu static csdev array defined
for potential all TRBEs so that the ETE-TRBE links be permanently established
given that the ETEs are permanent and never really go away with cpu hot remove
event (my assumption). TRBE csdevs should just get enabled or disabled without
really being destroyed during cpu hotplug, so that the corresponding TRBE-ETE
connection remains in place.

> 
> And the default sink can be initialized when the ETE instance first starts
> looking for it.

IIUC def_sink is the sink which will be selected by default for a source device
while creating a path, in case there is no clear preference from the user. ETE's
default sink should be fixed (TRBE) to be on the easy side and hence assigning
that during connection expansion procedure, does make sense. But then it can be
more complex where the 'default' sink for an ETE can be scenario specific and
may not be always be its TRBE.

The expanding connections fits into a scenario where the ETE is present with
all it's other traditional sinks and TRBE is the one which comes in or goes out
with the cpu.

If ETE also comes in and goes out with individual cpu hotplug which is preferred
ideally, we would need to also

1. Co-ordinate with TRBE bring up and connection creation to avoid race
2. Rediscover traditional sinks which were attached to the ETE before -
   go back, rescan the DT/ACPI entries for sinks with whom a path can
   be established etc.

Basically there are three choices we have here

1. ETE is permanent, TRBE and ETE-TRBE path gets created or destroyed with hotplug (current proposal)
2. ETE/TRBE/ETE-TRBE path are all permanent, ETE and TRBE get enabled or disabled with hotplug
3. ETE, TRBE and ETE-TRBE path, all get created, enabled and destroyed with hotplug in sync

- Anshuman
Mathieu Poirier Dec. 11, 2020, 9:31 p.m. UTC | #3
On Tue, Nov 10, 2020 at 06:15:08PM +0530, Anshuman Khandual wrote:
> Unlike traditional sink devices, individual TRBE instances are not detected
> via DT or ACPI nodes. Instead TRBE instances are detected during CPU online
> process. Hence a path connecting ETE and TRBE on a given CPU would not have
> been established until then. This adds two coresight helpers that will help
> modify outward connections from a source device to establish and terminate
> path to a given sink device. But this method might not be optimal and would
> be reworked later.
> 
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-etm-perf.c | 30 ++++++++++++++++++++++++
>  drivers/hwtracing/coresight/coresight-etm-perf.h |  4 ++++
>  drivers/hwtracing/coresight/coresight-platform.c |  3 ++-
>  drivers/hwtracing/coresight/coresight-trbe.c     |  2 ++
>  include/linux/coresight.h                        |  2 ++
>  5 files changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
> index 1a37991..b4ab1d4 100644
> --- a/drivers/hwtracing/coresight/coresight-etm-perf.c
> +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
> @@ -664,3 +664,33 @@ void __exit etm_perf_exit(void)
>  {
>  	perf_pmu_unregister(&etm_pmu);
>  }
> +
> +#ifdef CONFIG_CORESIGHT_TRBE
> +void coresight_trbe_connect_ete(struct coresight_device *csdev_trbe, int cpu)
> +{
> +	struct coresight_device *csdev_ete = per_cpu(csdev_src, cpu);

As Suzuki pointed out that won't work if the TRBE gets probed before the
ETMv4-ETE.  I also agree with Suzuki this situation should be better handled
with a per csdev_trbe that should be declared in the coresight-core.c file.
That way both sysfs and perf have access to it.  

> +
> +	if (!csdev_ete) {
> +		pr_err("Corresponding ETE device not present on cpu %d\n", cpu);
> +		return;
> +	}
> +	csdev_ete->def_sink = csdev_trbe;

That should be done in function coresight_find_default_sink().  If
per_cpu(csdev_trbe, cpu) exists then that's the what we pick.  If not then move
along with coresight_find_sink().


> +	csdev_ete->pdata->nr_outport++;
> +	if (!csdev_ete->pdata->conns)
> +		coresight_alloc_conns(&csdev_ete->dev, csdev_ete->pdata);
> +	csdev_ete->pdata->conns[csdev_ete->pdata->nr_outport - 1].child_dev = csdev_trbe;

I don't think we have to go through all that dance since the TRBE is directly
connected to the ETE.  With the above about coresight_find_default_sink() in
mind, all we need to do is fix coresight_build_path() to check if the sink
parameter is the same as csdev->def_sink.  If so then just add the sink to the
patch, no need to follow ports as we do for other classic components.

Thanks,
Mathieu

> +}
> +
> +void coresight_trbe_remove_ete(struct coresight_device *csdev_trbe, int cpu)
> +{
> +	struct coresight_device *csdev_ete = per_cpu(csdev_src, cpu);
> +
> +	if (!csdev_ete) {
> +		pr_err("Corresponding ETE device not present on cpu %d\n", cpu);
> +		return;
> +	}
> +	csdev_ete->pdata->conns[csdev_ete->pdata->nr_outport - 1].child_dev = NULL;
> +	csdev_ete->def_sink = NULL;
> +	csdev_ete->pdata->nr_outport--;
> +}
> +#endif
> diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.h b/drivers/hwtracing/coresight/coresight-etm-perf.h
> index 3e4f2ad..20386cf 100644
> --- a/drivers/hwtracing/coresight/coresight-etm-perf.h
> +++ b/drivers/hwtracing/coresight/coresight-etm-perf.h
> @@ -85,4 +85,8 @@ static inline void *etm_perf_sink_config(struct perf_output_handle *handle)
>  int __init etm_perf_init(void);
>  void __exit etm_perf_exit(void);
>  
> +#ifdef CONFIG_CORESIGHT_TRBE
> +void coresight_trbe_connect_ete(struct coresight_device *csdev, int cpu);
> +void coresight_trbe_remove_ete(struct coresight_device *csdev, int cpu);
> +#endif
>  #endif
> diff --git a/drivers/hwtracing/coresight/coresight-platform.c b/drivers/hwtracing/coresight/coresight-platform.c
> index c594f45..8fa7406 100644
> --- a/drivers/hwtracing/coresight/coresight-platform.c
> +++ b/drivers/hwtracing/coresight/coresight-platform.c
> @@ -23,7 +23,7 @@
>   * coresight_alloc_conns: Allocate connections record for each output
>   * port from the device.
>   */
> -static int coresight_alloc_conns(struct device *dev,
> +int coresight_alloc_conns(struct device *dev,
>  				 struct coresight_platform_data *pdata)
>  {
>  	if (pdata->nr_outport) {
> @@ -35,6 +35,7 @@ static int coresight_alloc_conns(struct device *dev,
>  
>  	return 0;
>  }
> +EXPORT_SYMBOL_GPL(coresight_alloc_conns);
>  
>  static struct device *
>  coresight_find_device_by_fwnode(struct fwnode_handle *fwnode)
> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
> index 48a8ec3..afd1a1c 100644
> --- a/drivers/hwtracing/coresight/coresight-trbe.c
> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
> @@ -507,6 +507,7 @@ static void arm_trbe_probe_coresight_cpu(void *info)
>  	if (IS_ERR(cpudata->csdev))
>  		goto cpu_clear;
>  
> +	coresight_trbe_connect_ete(cpudata->csdev, cpudata->cpu);
>  	dev_set_drvdata(&cpudata->csdev->dev, cpudata);
>  	cpudata->trbe_dbm = get_trbe_flag_update();
>  	cpudata->trbe_align = 1ULL << get_trbe_address_align();
> @@ -586,6 +587,7 @@ static int arm_trbe_cpu_teardown(unsigned int cpu, struct hlist_node *node)
>  
>  	if (cpumask_test_cpu(cpu, &drvdata->supported_cpus)) {
>  		cpudata = per_cpu_ptr(drvdata->cpudata, cpu);
> +		coresight_trbe_remove_ete(cpudata->csdev, cpu);
>  		if (cpudata->csdev) {
>  			coresight_unregister(cpudata->csdev);
>  			cpudata->drvdata = NULL;
> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
> index c2d0a2a..c657813 100644
> --- a/include/linux/coresight.h
> +++ b/include/linux/coresight.h
> @@ -496,6 +496,8 @@ void coresight_relaxed_write64(struct coresight_device *csdev,
>  			       u64 val, u32 offset);
>  void coresight_write64(struct coresight_device *csdev, u64 val, u32 offset);
>  
> +int coresight_alloc_conns(struct device *dev,
> +			  struct coresight_platform_data *pdata);
>  
>  #else
>  static inline struct coresight_device *
> -- 
> 2.7.4
>
diff mbox series

Patch

diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
index 1a37991..b4ab1d4 100644
--- a/drivers/hwtracing/coresight/coresight-etm-perf.c
+++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
@@ -664,3 +664,33 @@  void __exit etm_perf_exit(void)
 {
 	perf_pmu_unregister(&etm_pmu);
 }
+
+#ifdef CONFIG_CORESIGHT_TRBE
+void coresight_trbe_connect_ete(struct coresight_device *csdev_trbe, int cpu)
+{
+	struct coresight_device *csdev_ete = per_cpu(csdev_src, cpu);
+
+	if (!csdev_ete) {
+		pr_err("Corresponding ETE device not present on cpu %d\n", cpu);
+		return;
+	}
+	csdev_ete->def_sink = csdev_trbe;
+	csdev_ete->pdata->nr_outport++;
+	if (!csdev_ete->pdata->conns)
+		coresight_alloc_conns(&csdev_ete->dev, csdev_ete->pdata);
+	csdev_ete->pdata->conns[csdev_ete->pdata->nr_outport - 1].child_dev = csdev_trbe;
+}
+
+void coresight_trbe_remove_ete(struct coresight_device *csdev_trbe, int cpu)
+{
+	struct coresight_device *csdev_ete = per_cpu(csdev_src, cpu);
+
+	if (!csdev_ete) {
+		pr_err("Corresponding ETE device not present on cpu %d\n", cpu);
+		return;
+	}
+	csdev_ete->pdata->conns[csdev_ete->pdata->nr_outport - 1].child_dev = NULL;
+	csdev_ete->def_sink = NULL;
+	csdev_ete->pdata->nr_outport--;
+}
+#endif
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.h b/drivers/hwtracing/coresight/coresight-etm-perf.h
index 3e4f2ad..20386cf 100644
--- a/drivers/hwtracing/coresight/coresight-etm-perf.h
+++ b/drivers/hwtracing/coresight/coresight-etm-perf.h
@@ -85,4 +85,8 @@  static inline void *etm_perf_sink_config(struct perf_output_handle *handle)
 int __init etm_perf_init(void);
 void __exit etm_perf_exit(void);
 
+#ifdef CONFIG_CORESIGHT_TRBE
+void coresight_trbe_connect_ete(struct coresight_device *csdev, int cpu);
+void coresight_trbe_remove_ete(struct coresight_device *csdev, int cpu);
+#endif
 #endif
diff --git a/drivers/hwtracing/coresight/coresight-platform.c b/drivers/hwtracing/coresight/coresight-platform.c
index c594f45..8fa7406 100644
--- a/drivers/hwtracing/coresight/coresight-platform.c
+++ b/drivers/hwtracing/coresight/coresight-platform.c
@@ -23,7 +23,7 @@ 
  * coresight_alloc_conns: Allocate connections record for each output
  * port from the device.
  */
-static int coresight_alloc_conns(struct device *dev,
+int coresight_alloc_conns(struct device *dev,
 				 struct coresight_platform_data *pdata)
 {
 	if (pdata->nr_outport) {
@@ -35,6 +35,7 @@  static int coresight_alloc_conns(struct device *dev,
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(coresight_alloc_conns);
 
 static struct device *
 coresight_find_device_by_fwnode(struct fwnode_handle *fwnode)
diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
index 48a8ec3..afd1a1c 100644
--- a/drivers/hwtracing/coresight/coresight-trbe.c
+++ b/drivers/hwtracing/coresight/coresight-trbe.c
@@ -507,6 +507,7 @@  static void arm_trbe_probe_coresight_cpu(void *info)
 	if (IS_ERR(cpudata->csdev))
 		goto cpu_clear;
 
+	coresight_trbe_connect_ete(cpudata->csdev, cpudata->cpu);
 	dev_set_drvdata(&cpudata->csdev->dev, cpudata);
 	cpudata->trbe_dbm = get_trbe_flag_update();
 	cpudata->trbe_align = 1ULL << get_trbe_address_align();
@@ -586,6 +587,7 @@  static int arm_trbe_cpu_teardown(unsigned int cpu, struct hlist_node *node)
 
 	if (cpumask_test_cpu(cpu, &drvdata->supported_cpus)) {
 		cpudata = per_cpu_ptr(drvdata->cpudata, cpu);
+		coresight_trbe_remove_ete(cpudata->csdev, cpu);
 		if (cpudata->csdev) {
 			coresight_unregister(cpudata->csdev);
 			cpudata->drvdata = NULL;
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index c2d0a2a..c657813 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -496,6 +496,8 @@  void coresight_relaxed_write64(struct coresight_device *csdev,
 			       u64 val, u32 offset);
 void coresight_write64(struct coresight_device *csdev, u64 val, u32 offset);
 
+int coresight_alloc_conns(struct device *dev,
+			  struct coresight_platform_data *pdata);
 
 #else
 static inline struct coresight_device *