[QUEUED v0 00/19] stm class/intel_th: Patches in my queue

[QUEUED v0 00/19] stm class/intel_th: Patches in my queue

[Alexander Shishkin]

Hi everybody,

These are the patches that I have so far that I haven't managed to
push upstream yet. If you have questions, concerns or objections,
please let me know.

These are mostly fixes; the first 9 patches I was trying to get into
4.4 during later -rcs, but couldn't, so they are still here.

Should I set up a git tree for this somewhere?

Alexander Shishkin (14):
  stm class: Fix locking in unbinding policy path
  stm class: Fix link list locking
  stm class: Prevent user-controllable allocations
  intel_th: pci: Add Apollo Lake SOC support
  intel_th: pci: Add Broxton SOC support
  intel_th: Depend on HAS_IOMEM
  intel_th: gth: Remove commented-out code
  intel_th: Update scratchpad bits according to enabled output activity
  stm class: Use driver's packet callback's return value
  intel_th: sth: Sanitize packet callback's return values
  stm class: Support devices with multiple instances
  stm class: dummy_stm: Create multiple devices
  intel_th: Set root device's drvdata early
  intel_th: Use real device index in the node names

Arnd Bergmann (1):
  stm class: Select CONFIG_SRCU

Chunyan Zhang (1):
  stm class: Fix an off-by-one in master array allocation

Geert Uytterhoeven (2):
  stm class: Hide STM-specific options if STM is disabled
  intel_th: INTEL_TH should depend on HAS_DMA

Lucas Tanure (1):
  stm class: Use a signed return type for stm_find_master_chan

 drivers/hwtracing/intel_th/Kconfig    |  1 +
 drivers/hwtracing/intel_th/core.c     | 30 ++++++++++++++--
 drivers/hwtracing/intel_th/gth.c      | 32 +++++++----------
 drivers/hwtracing/intel_th/gth.h      |  3 --
 drivers/hwtracing/intel_th/intel_th.h | 41 ++++++++++++++++++++++
 drivers/hwtracing/intel_th/pci.c      | 12 +++++--
 drivers/hwtracing/intel_th/sth.c      | 11 ++++--
 drivers/hwtracing/stm/Kconfig         |  5 +++
 drivers/hwtracing/stm/core.c          | 66 +++++++++++++++++++++++++----------
 drivers/hwtracing/stm/dummy_stm.c     | 55 ++++++++++++++++++++++++-----
 drivers/hwtracing/stm/policy.c        | 25 +++++++++----
 drivers/hwtracing/stm/stm.h           |  1 +
 include/linux/stm.h                   |  7 ++++
 13 files changed, 225 insertions(+), 64 deletions(-)

-- 
2.7.0

[QUEUED v0 01/19] stm class: Hide STM-specific options if STM is disabled

[Alexander Shishkin]

From: Geert Uytterhoeven <geert@linux-m68k.org>

If STM=n, it doesn't make sense to ask about STM_DUMMY and
STM_SOURCE_CONSOLE support, which are not even built when enabled
anyway. Hence hide these options if STM=n.

Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 drivers/hwtracing/stm/Kconfig | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/hwtracing/stm/Kconfig b/drivers/hwtracing/stm/Kconfig
index 83e9f591a5..4c13762f2b 100644
--- a/drivers/hwtracing/stm/Kconfig
+++ b/drivers/hwtracing/stm/Kconfig
@@ -8,6 +8,8 @@ config STM
 
 	  Say Y here to enable System Trace Module device support.
 
+if STM
+
 config STM_DUMMY
 	tristate "Dummy STM driver"
 	help
@@ -24,3 +26,5 @@ config STM_SOURCE_CONSOLE
 
 	  If you want to send kernel console messages over STM devices,
 	  say Y.
+
+endif
-- 
2.7.0

[QUEUED v0 02/19] intel_th: INTEL_TH should depend on HAS_DMA

[Alexander Shishkin]

From: Geert Uytterhoeven <geert@linux-m68k.org>

If NO_DMA=y:

    ERROR: "dma_free_coherent" [drivers/hwtracing/intel_th/intel_th_msu.ko] undefined!
    ERROR: "dma_alloc_coherent" [drivers/hwtracing/intel_th/intel_th_msu.ko] undefined!
    ERROR: "dma_supported" [drivers/hwtracing/intel_th/intel_th.ko] undefined!

Add a dependency on HAS_DMA to fix this.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 drivers/hwtracing/intel_th/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/hwtracing/intel_th/Kconfig b/drivers/hwtracing/intel_th/Kconfig
index b7a9073d96..90b0844093 100644
--- a/drivers/hwtracing/intel_th/Kconfig
+++ b/drivers/hwtracing/intel_th/Kconfig
@@ -1,5 +1,6 @@
 config INTEL_TH
 	tristate "Intel(R) Trace Hub controller"
+	depends on HAS_DMA
 	help
 	  Intel(R) Trace Hub (TH) is a set of hardware blocks (subdevices) that
 	  produce, switch and output trace data from multiple hardware and
-- 
2.7.0

[QUEUED v0 03/19] stm class: Select CONFIG_SRCU

[Alexander Shishkin]

From: Arnd Bergmann <arnd@arndb.de>

The newly added STM code uses SRCU, but does not ensure that
this code is part of the kernel:

drivers/built-in.o: In function `stm_source_link_show':
include/linux/srcu.h:221: undefined reference to `__srcu_read_lock'
include/linux/srcu.h:238: undefined reference to `__srcu_read_unlock'
drivers/built-in.o: In function `stm_source_link_drop':
include/linux/srcu.h:221: undefined reference to `__srcu_read_lock'
include/linux/srcu.h:238: undefined reference to `__srcu_read_unlock'

This adds a Kconfig 'select' statement like all the other SRCU using
drivers have.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: 7bd1d4093c2f ("stm class: Introduce an abstraction for System Trace Module devices")
Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 drivers/hwtracing/stm/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/hwtracing/stm/Kconfig b/drivers/hwtracing/stm/Kconfig
index 4c13762f2b..e0ac753955 100644
--- a/drivers/hwtracing/stm/Kconfig
+++ b/drivers/hwtracing/stm/Kconfig
@@ -1,6 +1,7 @@
 config STM
 	tristate "System Trace Module devices"
 	select CONFIGFS_FS
+	select SRCU
 	help
 	  A System Trace Module (STM) is a device exporting data in System
 	  Trace Protocol (STP) format as defined by MIPI STP standards.
-- 
2.7.0

[QUEUED v0 04/19] stm class: Fix locking in unbinding policy path

[Alexander Shishkin]

Right now, if stm device removal has to unbind from a policy (that is,
an stm device that has STP policy, gets removed), it will trigger a
nested lock on the stm device's policy mutex.

This patch fixes the problem by moving the locking from the policy
unbinding to policy removal (configfs path), where it's actually needed;
the other caller of the policy unbinding function already takes the
mutex around the call.

Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 drivers/hwtracing/stm/policy.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/hwtracing/stm/policy.c b/drivers/hwtracing/stm/policy.c
index 11ab6d01ad..94d3abfb73 100644
--- a/drivers/hwtracing/stm/policy.c
+++ b/drivers/hwtracing/stm/policy.c
@@ -272,13 +272,17 @@ void stp_policy_unbind(struct stp_policy *policy)
 {
 	struct stm_device *stm = policy->stm;
 
+	/*
+	 * stp_policy_release() will not call here if the policy is already
+	 * unbound; other users should not either, as no link exists between
+	 * this policy and anything else in that case
+	 */
 	if (WARN_ON_ONCE(!policy->stm))
 		return;
 
-	mutex_lock(&stm->policy_mutex);
-	stm->policy = NULL;
-	mutex_unlock(&stm->policy_mutex);
+	lockdep_assert_held(&stm->policy_mutex);
 
+	stm->policy = NULL;
 	policy->stm = NULL;
 
 	stm_put_device(stm);
@@ -287,8 +291,16 @@ void stp_policy_unbind(struct stp_policy *policy)
 static void stp_policy_release(struct config_item *item)
 {
 	struct stp_policy *policy = to_stp_policy(item);
+	struct stm_device *stm = policy->stm;
 
+	/* a policy *can* be unbound and still exist in configfs tree */
+	if (!stm)
+		return;
+
+	mutex_lock(&stm->policy_mutex);
 	stp_policy_unbind(policy);
+	mutex_unlock(&stm->policy_mutex);
+
 	kfree(policy);
 }
 
-- 
2.7.0

[QUEUED v0 05/19] stm class: Fix link list locking

[Alexander Shishkin]

Currently, the list of stm_sources linked to an stm device is protected by
a spinlock, which also means that sources' .unlink() method is called under
this spinlock. However, this method may (and does) sleep, which means
trouble.

This patch slightly reworks locking around stm::link_list so that bits that
might_sleep() are called with a mutex held instead. Modification of this
list requires both mutex and spinlock to be held, while looking at the list
can be done under either mutex or spinlock.

Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 drivers/hwtracing/stm/core.c | 38 +++++++++++++++++++++++++++++---------
 drivers/hwtracing/stm/stm.h  |  1 +
 2 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/drivers/hwtracing/stm/core.c b/drivers/hwtracing/stm/core.c
index b6445d9e54..ddcb606ace 100644
--- a/drivers/hwtracing/stm/core.c
+++ b/drivers/hwtracing/stm/core.c
@@ -641,6 +641,7 @@ int stm_register_device(struct device *parent, struct stm_data *stm_data,
 	if (err)
 		goto err_device;
 
+	mutex_init(&stm->link_mutex);
 	spin_lock_init(&stm->link_lock);
 	INIT_LIST_HEAD(&stm->link_list);
 
@@ -671,11 +672,11 @@ void stm_unregister_device(struct stm_data *stm_data)
 	struct stm_source_device *src, *iter;
 	int i;
 
-	spin_lock(&stm->link_lock);
+	mutex_lock(&stm->link_mutex);
 	list_for_each_entry_safe(src, iter, &stm->link_list, link_entry) {
 		__stm_source_link_drop(src, stm);
 	}
-	spin_unlock(&stm->link_lock);
+	mutex_unlock(&stm->link_mutex);
 
 	synchronize_srcu(&stm_source_srcu);
 
@@ -694,6 +695,17 @@ void stm_unregister_device(struct stm_data *stm_data)
 }
 EXPORT_SYMBOL_GPL(stm_unregister_device);
 
+/*
+ * stm::link_list access serialization uses a spinlock and a mutex; holding
+ * either of them guarantees that the list is stable; modification requires
+ * holding both of them.
+ *
+ * Lock ordering is as follows:
+ *   stm::link_mutex
+ *     stm::link_lock
+ *       src::link_lock
+ */
+
 /**
  * stm_source_link_add() - connect an stm_source device to an stm device
  * @src:	stm_source device
@@ -710,6 +722,7 @@ static int stm_source_link_add(struct stm_source_device *src,
 	char *id;
 	int err;
 
+	mutex_lock(&stm->link_mutex);
 	spin_lock(&stm->link_lock);
 	spin_lock(&src->link_lock);
 
@@ -719,6 +732,7 @@ static int stm_source_link_add(struct stm_source_device *src,
 
 	spin_unlock(&src->link_lock);
 	spin_unlock(&stm->link_lock);
+	mutex_unlock(&stm->link_mutex);
 
 	id = kstrdup(src->data->name, GFP_KERNEL);
 	if (id) {
@@ -756,6 +770,7 @@ fail_free_output:
 	stm_put_device(stm);
 
 fail_detach:
+	mutex_lock(&stm->link_mutex);
 	spin_lock(&stm->link_lock);
 	spin_lock(&src->link_lock);
 
@@ -764,6 +779,7 @@ fail_detach:
 
 	spin_unlock(&src->link_lock);
 	spin_unlock(&stm->link_lock);
+	mutex_unlock(&stm->link_mutex);
 
 	return err;
 }
@@ -776,13 +792,20 @@ fail_detach:
  * If @stm is @src::link, disconnect them from one another and put the
  * reference on the @stm device.
  *
- * Caller must hold stm::link_lock.
+ * Caller must hold stm::link_mutex.
  */
 static void __stm_source_link_drop(struct stm_source_device *src,
 				   struct stm_device *stm)
 {
 	struct stm_device *link;
 
+	lockdep_assert_held(&stm->link_mutex);
+
+	if (src->data->unlink)
+		src->data->unlink(src->data);
+
+	/* for stm::link_list modification, we hold both mutex and spinlock */
+	spin_lock(&stm->link_lock);
 	spin_lock(&src->link_lock);
 	link = srcu_dereference_check(src->link, &stm_source_srcu, 1);
 	if (WARN_ON_ONCE(link != stm)) {
@@ -791,13 +814,13 @@ static void __stm_source_link_drop(struct stm_source_device *src,
 	}
 
 	stm_output_free(link, &src->output);
-	/* caller must hold stm::link_lock */
 	list_del_init(&src->link_entry);
 	/* matches stm_find_device() from stm_source_link_store() */
 	stm_put_device(link);
 	rcu_assign_pointer(src->link, NULL);
 
 	spin_unlock(&src->link_lock);
+	spin_unlock(&stm->link_lock);
 }
 
 /**
@@ -819,12 +842,9 @@ static void stm_source_link_drop(struct stm_source_device *src)
 	stm = srcu_dereference(src->link, &stm_source_srcu);
 
 	if (stm) {
-		if (src->data->unlink)
-			src->data->unlink(src->data);
-
-		spin_lock(&stm->link_lock);
+		mutex_lock(&stm->link_mutex);
 		__stm_source_link_drop(src, stm);
-		spin_unlock(&stm->link_lock);
+		mutex_unlock(&stm->link_mutex);
 	}
 
 	srcu_read_unlock(&stm_source_srcu, idx);
diff --git a/drivers/hwtracing/stm/stm.h b/drivers/hwtracing/stm/stm.h
index 95ece0292c..97ee022414 100644
--- a/drivers/hwtracing/stm/stm.h
+++ b/drivers/hwtracing/stm/stm.h
@@ -45,6 +45,7 @@ struct stm_device {
 	int			major;
 	unsigned int		sw_nmasters;
 	struct stm_data		*data;
+	struct mutex		link_mutex;
 	spinlock_t		link_lock;
 	struct list_head	link_list;
 	/* master allocation */
-- 
2.7.0

[QUEUED v0 06/19] stm class: Fix an off-by-one in master array allocation

[Alexander Shishkin]

From: Chunyan Zhang <zhang.chunyan@linaro.org>

Since both sw_start and sw_end are master indices, the size of array
that holds them is sw_end - sw_start + 1, which the current code gets
wrong, allocating one item less than required.

This patch corrects the allocation size, avoiding potential slab
corruption.

Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org>
[alexander.shishkin@linux.intel.com: re-wrote the commit message]
Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 drivers/hwtracing/stm/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hwtracing/stm/core.c b/drivers/hwtracing/stm/core.c
index ddcb606ace..40a8b79ab7 100644
--- a/drivers/hwtracing/stm/core.c
+++ b/drivers/hwtracing/stm/core.c
@@ -618,7 +618,7 @@ int stm_register_device(struct device *parent, struct stm_data *stm_data,
 	if (!stm_data->packet || !stm_data->sw_nchannels)
 		return -EINVAL;
 
-	nmasters = stm_data->sw_end - stm_data->sw_start;
+	nmasters = stm_data->sw_end - stm_data->sw_start + 1;
 	stm = kzalloc(sizeof(*stm) + nmasters * sizeof(void *), GFP_KERNEL);
 	if (!stm)
 		return -ENOMEM;
-- 
2.7.0

[QUEUED v0 07/19] stm class: Prevent user-controllable allocations

[Alexander Shishkin]

Currently, the character device write method allocates a temporary buffer
for user's data, but the user's data size is not sanitized and can cause
arbitrarily large allocations via kzalloc() or an integer overflow that
will then result in overwriting kernel memory.

This patch trims the input buffer size to avoid these issues.

Reported-by: Sasha Levin <sasha.levin@oracle.com>
Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 drivers/hwtracing/stm/core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/hwtracing/stm/core.c b/drivers/hwtracing/stm/core.c
index 40a8b79ab7..aef8ddb244 100644
--- a/drivers/hwtracing/stm/core.c
+++ b/drivers/hwtracing/stm/core.c
@@ -406,6 +406,9 @@ static ssize_t stm_char_write(struct file *file, const char __user *buf,
 	char *kbuf;
 	int err;
 
+	if (count + 1 > PAGE_SIZE)
+		count = PAGE_SIZE - 1;
+
 	/*
 	 * if no m/c have been assigned to this writer up to this
 	 * point, use "default" policy entry
-- 
2.7.0

[QUEUED v0 08/19] intel_th: pci: Add Apollo Lake SOC support

[Alexander Shishkin]

This adds Intel(R) Trace Hub PCI ID for Apollo Lake SOC.

Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 drivers/hwtracing/intel_th/pci.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/hwtracing/intel_th/pci.c b/drivers/hwtracing/intel_th/pci.c
index 641e879360..b5760730e1 100644
--- a/drivers/hwtracing/intel_th/pci.c
+++ b/drivers/hwtracing/intel_th/pci.c
@@ -67,6 +67,11 @@ static const struct pci_device_id intel_th_pci_id_table[] = {
 		PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0xa126),
 		.driver_data = (kernel_ulong_t)0,
 	},
+	{
+		/* Apollo Lake */
+		PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x5a8e),
+		.driver_data = (kernel_ulong_t)0,
+	},
 	{ 0 },
 };
 
-- 
2.7.0

[QUEUED v0 09/19] intel_th: pci: Add Broxton SOC support

[Alexander Shishkin]

This adds Intel(R) Trace Hub PCI ID for Broxton SOC.

Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 drivers/hwtracing/intel_th/pci.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/hwtracing/intel_th/pci.c b/drivers/hwtracing/intel_th/pci.c
index b5760730e1..09017073d7 100644
--- a/drivers/hwtracing/intel_th/pci.c
+++ b/drivers/hwtracing/intel_th/pci.c
@@ -72,6 +72,11 @@ static const struct pci_device_id intel_th_pci_id_table[] = {
 		PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x5a8e),
 		.driver_data = (kernel_ulong_t)0,
 	},
+	{
+		/* Broxton */
+		PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x0a80),
+		.driver_data = (kernel_ulong_t)0,
+	},
 	{ 0 },
 };
 
-- 
2.7.0

[QUEUED v0 10/19] stm class: Use a signed return type for stm_find_master_chan

[Alexander Shishkin]

From: Lucas Tanure <tanure@linux.com>

The return type "unsigned int" was used by the stm_find_master_chan function
despite of the aspect that it will eventually return a negative error code.

Done with the help of Coccinelle.

Signed-off-by: Lucas Tanure <tanure@linux.com>
Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 drivers/hwtracing/stm/core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hwtracing/stm/core.c b/drivers/hwtracing/stm/core.c
index aef8ddb244..cdec240bd6 100644
--- a/drivers/hwtracing/stm/core.c
+++ b/drivers/hwtracing/stm/core.c
@@ -233,7 +233,7 @@ static int find_free_channels(unsigned long *bitmap, unsigned int start,
 	return -1;
 }
 
-static unsigned int
+static int
 stm_find_master_chan(struct stm_device *stm, unsigned int width,
 		     unsigned int *mstart, unsigned int mend,
 		     unsigned int *cstart, unsigned int cend)
@@ -293,7 +293,7 @@ static int stm_output_assign(struct stm_device *stm, unsigned int width,
 		goto unlock;
 
 	ret = stm_find_master_chan(stm, width, &midx, mend, &cidx, cend);
-	if (ret)
+	if (ret < 0)
 		goto unlock;
 
 	output->master = midx;
-- 
2.7.0

[QUEUED v0 11/19] intel_th: Depend on HAS_IOMEM

[Alexander Shishkin]

This driver requires io memory to operate, so don't even consider it
for NO_IOMEM architectures.

Reported-by: Richard Weinberger <richard@nod.at>
Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 drivers/hwtracing/intel_th/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hwtracing/intel_th/Kconfig b/drivers/hwtracing/intel_th/Kconfig
index 90b0844093..1b412f8a56 100644
--- a/drivers/hwtracing/intel_th/Kconfig
+++ b/drivers/hwtracing/intel_th/Kconfig
@@ -1,6 +1,6 @@
 config INTEL_TH
 	tristate "Intel(R) Trace Hub controller"
-	depends on HAS_DMA
+	depends on HAS_DMA && HAS_IOMEM
 	help
 	  Intel(R) Trace Hub (TH) is a set of hardware blocks (subdevices) that
 	  produce, switch and output trace data from multiple hardware and
-- 
2.7.0

[QUEUED v0 12/19] intel_th: gth: Remove commented-out code

[Alexander Shishkin]

There's a commented-out function in the GTH driver that's a leftover
from previous versions of the driver, where we tried to inherit the
pre-existing configuration, which didn't prove to be a sound idea.

This patch removes the function. No functional changes.

Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 drivers/hwtracing/intel_th/gth.c | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/drivers/hwtracing/intel_th/gth.c b/drivers/hwtracing/intel_th/gth.c
index 2dc5378ccd..e4c9811c1f 100644
--- a/drivers/hwtracing/intel_th/gth.c
+++ b/drivers/hwtracing/intel_th/gth.c
@@ -146,24 +146,6 @@ gth_master_set(struct gth_device *gth, unsigned int master, int port)
 	iowrite32(val, gth->base + reg);
 }
 
-/*static int gth_master_get(struct gth_device *gth, unsigned int master)
-{
-	unsigned int reg = REG_GTH_SWDEST0 + ((master >> 1) & ~3u);
-	unsigned int shift = (master & 0x7) * 4;
-	u32 val;
-
-	if (master >= 256) {
-		reg = REG_GTH_GSWTDEST;
-		shift = 0;
-	}
-
-	val = ioread32(gth->base + reg);
-	val &= (0xf << shift);
-	val >>= shift;
-
-	return val ? val & 0x7 : -1;
-	}*/
-
 static ssize_t master_attr_show(struct device *dev,
 				struct device_attribute *attr,
 				char *buf)
-- 
2.7.0

[QUEUED v0 13/19] intel_th: Update scratchpad bits according to enabled output activity

[Alexander Shishkin]

Intel TH implements a scratchpad register to indicate to the firmware
and external debuggers what trace configuration is enabled so that
everybody plays nicely together. The register is a bit field and the
bit assignment convention is described in the developer's manual.

This patch enables the driver to automatically set scratchpad register
bits according to the output configuration that's enabled.

Based on work by Yann Fouassier.

Signed-off-by: Yann Fouassier <yann.fouassier@intel.com>
Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 drivers/hwtracing/intel_th/core.c     |  5 +++++
 drivers/hwtracing/intel_th/gth.c      | 14 +++++++++++-
 drivers/hwtracing/intel_th/gth.h      |  3 ---
 drivers/hwtracing/intel_th/intel_th.h | 41 +++++++++++++++++++++++++++++++++++
 4 files changed, 59 insertions(+), 4 deletions(-)

diff --git a/drivers/hwtracing/intel_th/core.c b/drivers/hwtracing/intel_th/core.c
index 165d3001c3..b8b9895da5 100644
--- a/drivers/hwtracing/intel_th/core.c
+++ b/drivers/hwtracing/intel_th/core.c
@@ -319,6 +319,7 @@ static struct intel_th_subdevice {
 	unsigned		nres;
 	unsigned		type;
 	unsigned		otype;
+	unsigned		scrpd;
 	int			id;
 } intel_th_subdevices[TH_SUBDEVICE_MAX] = {
 	{
@@ -352,6 +353,7 @@ static struct intel_th_subdevice {
 		.id	= 0,
 		.type	= INTEL_TH_OUTPUT,
 		.otype	= GTH_MSU,
+		.scrpd	= SCRPD_MEM_IS_PRIM_DEST | SCRPD_MSC0_IS_ENABLED,
 	},
 	{
 		.nres	= 2,
@@ -371,6 +373,7 @@ static struct intel_th_subdevice {
 		.id	= 1,
 		.type	= INTEL_TH_OUTPUT,
 		.otype	= GTH_MSU,
+		.scrpd	= SCRPD_MEM_IS_PRIM_DEST | SCRPD_MSC1_IS_ENABLED,
 	},
 	{
 		.nres	= 2,
@@ -403,6 +406,7 @@ static struct intel_th_subdevice {
 		.name	= "pti",
 		.type	= INTEL_TH_OUTPUT,
 		.otype	= GTH_PTI,
+		.scrpd	= SCRPD_PTI_IS_PRIM_DEST,
 	},
 	{
 		.nres	= 1,
@@ -477,6 +481,7 @@ static int intel_th_populate(struct intel_th *th, struct resource *devres,
 			thdev->dev.devt = MKDEV(th->major, i);
 			thdev->output.type = subdev->otype;
 			thdev->output.port = -1;
+			thdev->output.scratchpad = subdev->scrpd;
 		}
 
 		err = device_add(&thdev->dev);
diff --git a/drivers/hwtracing/intel_th/gth.c b/drivers/hwtracing/intel_th/gth.c
index e4c9811c1f..9beea0b542 100644
--- a/drivers/hwtracing/intel_th/gth.c
+++ b/drivers/hwtracing/intel_th/gth.c
@@ -286,6 +286,10 @@ static int intel_th_gth_reset(struct gth_device *gth)
 	if (scratchpad & SCRPD_DEBUGGER_IN_USE)
 		return -EBUSY;
 
+	/* Always save/restore STH and TU registers in S0ix entry/exit */
+	scratchpad |= SCRPD_STH_IS_ENABLED | SCRPD_TRIGGER_IS_ENABLED;
+	iowrite32(scratchpad, gth->base + REG_GTH_SCRPD0);
+
 	/* output ports */
 	for (port = 0; port < 8; port++) {
 		if (gth_output_parm_get(gth, port, TH_OUTPUT_PARM(port)) ==
@@ -488,6 +492,10 @@ static void intel_th_gth_disable(struct intel_th_device *thdev,
 	if (!count)
 		dev_dbg(&thdev->dev, "timeout waiting for GTH[%d] PLE\n",
 			output->port);
+
+	reg = ioread32(gth->base + REG_GTH_SCRPD0);
+	reg &= ~output->scratchpad;
+	iowrite32(reg, gth->base + REG_GTH_SCRPD0);
 }
 
 /**
@@ -502,7 +510,7 @@ static void intel_th_gth_enable(struct intel_th_device *thdev,
 				struct intel_th_output *output)
 {
 	struct gth_device *gth = dev_get_drvdata(&thdev->dev);
-	u32 scr = 0xfc0000;
+	u32 scr = 0xfc0000, scrpd;
 	int master;
 
 	spin_lock(&gth->gth_lock);
@@ -517,6 +525,10 @@ static void intel_th_gth_enable(struct intel_th_device *thdev,
 	output->active = true;
 	spin_unlock(&gth->gth_lock);
 
+	scrpd = ioread32(gth->base + REG_GTH_SCRPD0);
+	scrpd |= output->scratchpad;
+	iowrite32(scrpd, gth->base + REG_GTH_SCRPD0);
+
 	iowrite32(scr, gth->base + REG_GTH_SCR);
 	iowrite32(0, gth->base + REG_GTH_SCR2);
 }
diff --git a/drivers/hwtracing/intel_th/gth.h b/drivers/hwtracing/intel_th/gth.h
index 3b714b7a61..56f0d26205 100644
--- a/drivers/hwtracing/intel_th/gth.h
+++ b/drivers/hwtracing/intel_th/gth.h
@@ -57,9 +57,6 @@ enum {
 	REG_GTH_SCRPD3		= 0xec, /* ScratchPad[3] */
 };
 
-/* Externall debugger is using Intel TH */
-#define SCRPD_DEBUGGER_IN_USE	BIT(24)
-
 /* waiting for Pipeline Empty bit(s) to assert for GTH */
 #define GTH_PLE_WAITLOOP_DEPTH	10000
 
diff --git a/drivers/hwtracing/intel_th/intel_th.h b/drivers/hwtracing/intel_th/intel_th.h
index 57fd72b20f..eedd09332d 100644
--- a/drivers/hwtracing/intel_th/intel_th.h
+++ b/drivers/hwtracing/intel_th/intel_th.h
@@ -30,6 +30,7 @@ enum {
  * struct intel_th_output - descriptor INTEL_TH_OUTPUT type devices
  * @port:	output port number, assigned by the switch
  * @type:	GTH_{MSU,CTP,PTI}
+ * @scratchpad:	scratchpad bits to flag when this output is enabled
  * @multiblock:	true for multiblock output configuration
  * @active:	true when this output is enabled
  *
@@ -41,6 +42,7 @@ enum {
 struct intel_th_output {
 	int		port;
 	unsigned int	type;
+	unsigned int	scratchpad;
 	bool		multiblock;
 	bool		active;
 };
@@ -241,4 +243,43 @@ enum {
 	GTH_PTI = 4,	/* MIPI-PTI */
 };
 
+/*
+ * Scratchpad bits: tell firmware and external debuggers
+ * what we are up to.
+ */
+enum {
+	/* Memory is the primary destination */
+	SCRPD_MEM_IS_PRIM_DEST		= BIT(0),
+	/* XHCI DbC is the primary destination */
+	SCRPD_DBC_IS_PRIM_DEST		= BIT(1),
+	/* PTI is the primary destination */
+	SCRPD_PTI_IS_PRIM_DEST		= BIT(2),
+	/* BSSB is the primary destination */
+	SCRPD_BSSB_IS_PRIM_DEST		= BIT(3),
+	/* PTI is the alternate destination */
+	SCRPD_PTI_IS_ALT_DEST		= BIT(4),
+	/* BSSB is the alternate destination */
+	SCRPD_BSSB_IS_ALT_DEST		= BIT(5),
+	/* DeepSx exit occurred */
+	SCRPD_DEEPSX_EXIT		= BIT(6),
+	/* S4 exit occurred */
+	SCRPD_S4_EXIT			= BIT(7),
+	/* S5 exit occurred */
+	SCRPD_S5_EXIT			= BIT(8),
+	/* MSU controller 0/1 is enabled */
+	SCRPD_MSC0_IS_ENABLED		= BIT(9),
+	SCRPD_MSC1_IS_ENABLED		= BIT(10),
+	/* Sx exit occurred */
+	SCRPD_SX_EXIT			= BIT(11),
+	/* Trigger Unit is enabled */
+	SCRPD_TRIGGER_IS_ENABLED	= BIT(12),
+	SCRPD_ODLA_IS_ENABLED		= BIT(13),
+	SCRPD_SOCHAP_IS_ENABLED		= BIT(14),
+	SCRPD_STH_IS_ENABLED		= BIT(15),
+	SCRPD_DCIH_IS_ENABLED		= BIT(16),
+	SCRPD_VER_IS_ENABLED		= BIT(17),
+	/* External debugger is using Intel TH */
+	SCRPD_DEBUGGER_IN_USE		= BIT(24),
+};
+
 #endif
-- 
2.7.0

[QUEUED v0 14/19] stm class: Use driver's packet callback's return value

[Alexander Shishkin]

STM drivers provide a callback to generate/send individual STP packets;
it also tells the stm core how many bytes of payload it has consumed.
However, we would also need to use the negative space of this return
value to communicate errors that occur during the packet generation,
in which case the stm core will have to take appropriate action.

For now, we need to account for the possibility that the stm driver may
not support certain combinations of packet type/flags, in which case
it is expected to signal an error.

Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 drivers/hwtracing/stm/core.c | 19 ++++++++++++-------
 include/linux/stm.h          |  7 +++++++
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/hwtracing/stm/core.c b/drivers/hwtracing/stm/core.c
index cdec240bd6..385db65994 100644
--- a/drivers/hwtracing/stm/core.c
+++ b/drivers/hwtracing/stm/core.c
@@ -380,8 +380,8 @@ static int stm_file_assign(struct stm_file *stmf, char *id, unsigned int width)
 	return ret;
 }
 
-static void stm_write(struct stm_data *data, unsigned int master,
-		      unsigned int channel, const char *buf, size_t count)
+static ssize_t stm_write(struct stm_data *data, unsigned int master,
+			  unsigned int channel, const char *buf, size_t count)
 {
 	unsigned int flags = STP_PACKET_TIMESTAMPED;
 	const unsigned char *p = buf, nil = 0;
@@ -393,9 +393,14 @@ static void stm_write(struct stm_data *data, unsigned int master,
 		sz = data->packet(data, master, channel, STP_PACKET_DATA, flags,
 				  sz, p);
 		flags = 0;
+
+		if (sz < 0)
+			break;
 	}
 
 	data->packet(data, master, channel, STP_PACKET_FLAG, 0, 0, &nil);
+
+	return pos;
 }
 
 static ssize_t stm_char_write(struct file *file, const char __user *buf,
@@ -433,8 +438,8 @@ static ssize_t stm_char_write(struct file *file, const char __user *buf,
 		return -EFAULT;
 	}
 
-	stm_write(stm->data, stmf->output.master, stmf->output.channel, kbuf,
-		  count);
+	count = stm_write(stm->data, stmf->output.master, stmf->output.channel,
+			  kbuf, count);
 
 	kfree(kbuf);
 
@@ -996,9 +1001,9 @@ int stm_source_write(struct stm_source_data *data, unsigned int chan,
 
 	stm = srcu_dereference(src->link, &stm_source_srcu);
 	if (stm)
-		stm_write(stm->data, src->output.master,
-			  src->output.channel + chan,
-			  buf, count);
+		count = stm_write(stm->data, src->output.master,
+				  src->output.channel + chan,
+				  buf, count);
 	else
 		count = -ENODEV;
 
diff --git a/include/linux/stm.h b/include/linux/stm.h
index 9d0083d364..ab8ceca4f5 100644
--- a/include/linux/stm.h
+++ b/include/linux/stm.h
@@ -67,6 +67,13 @@ struct stm_device;
  * description. That is, the lowest master that can be allocated to software
  * writers is @sw_start and data from this writer will appear is @sw_start
  * master in the STP stream.
+ *
+ * The @packet callback should adhere to the following rules:
+ *   1) it must return the number of bytes it consumed from the payload;
+ *   2) therefore, if it sent a packet that does not have payload (like FLAG),
+ *      it must return zero;
+ *   3) if it does not support the requested packet type/flag combination,
+ *      it must return -ENOTSUPP.
  */
 struct stm_data {
 	const char		*name;
-- 
2.7.0

[QUEUED v0 15/19] intel_th: sth: Sanitize packet callback's return values

[Alexander Shishkin]

According to the stm class interface, the packet callback should return
an error if it is asked to generate packets that it doesn't support.
When it succeeds, it should return number of bytes consumed from its
payload. Currently, for FLAG packet it mistakenly returns 1.

This patch addresses these issues.

Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 drivers/hwtracing/intel_th/sth.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/hwtracing/intel_th/sth.c b/drivers/hwtracing/intel_th/sth.c
index 56101c33e1..e1aee61dd7 100644
--- a/drivers/hwtracing/intel_th/sth.c
+++ b/drivers/hwtracing/intel_th/sth.c
@@ -94,10 +94,13 @@ static ssize_t sth_stm_packet(struct stm_data *stm_data, unsigned int master,
 	case STP_PACKET_TRIG:
 		if (flags & STP_PACKET_TIMESTAMPED)
 			reg += 4;
-		iowrite8(*payload, sth->base + reg);
+		writeb_relaxed(*payload, sth->base + reg);
 		break;
 
 	case STP_PACKET_MERR:
+		if (size > 4)
+			size = 4;
+
 		sth_iowrite(&out->MERR, payload, size);
 		break;
 
@@ -107,8 +110,8 @@ static ssize_t sth_stm_packet(struct stm_data *stm_data, unsigned int master,
 		else
 			outp = (u64 __iomem *)&out->FLAG;
 
-		size = 1;
-		sth_iowrite(outp, payload, size);
+		size = 0;
+		writeb_relaxed(0, outp);
 		break;
 
 	case STP_PACKET_USER:
@@ -129,6 +132,8 @@ static ssize_t sth_stm_packet(struct stm_data *stm_data, unsigned int master,
 
 		sth_iowrite(outp, payload, size);
 		break;
+	default:
+		return -ENOTSUPP;
 	}
 
 	return size;
-- 
2.7.0

[QUEUED v0 16/19] stm class: Support devices with multiple instances

[Alexander Shishkin]

By convention, the name of the stm policy directory in configfs consists of
the device name to which it applies and the actual policy name, separated
by a dot. Now, some devices already have dots in their names that separate
name of the actual device from its instance identifier. Such devices will
result in two (or more, who can tell) dots in the policy directory name.

Existing policy code, however, will treat the first dot as the one that
separates device name from policy name, therefore failing the above case.

This patch makes the last dot in the directory name be the separator, thus
prohibiting dots from being used in policy names.

Suggested-by: Chunyan Zhang <zhang.chunyan@linaro.org>
Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 drivers/hwtracing/stm/policy.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/hwtracing/stm/policy.c b/drivers/hwtracing/stm/policy.c
index 94d3abfb73..1db189657b 100644
--- a/drivers/hwtracing/stm/policy.c
+++ b/drivers/hwtracing/stm/policy.c
@@ -332,10 +332,11 @@ stp_policies_make(struct config_group *group, const char *name)
 
 	/*
 	 * node must look like <device_name>.<policy_name>, where
-	 * <device_name> is the name of an existing stm device and
-	 * <policy_name> is an arbitrary string
+	 * <device_name> is the name of an existing stm device; may
+	 *               contain dots;
+	 * <policy_name> is an arbitrary string; may not contain dots
 	 */
-	p = strchr(devname, '.');
+	p = strrchr(devname, '.');
 	if (!p) {
 		kfree(devname);
 		return ERR_PTR(-EINVAL);
-- 
2.7.0

[QUEUED v0 17/19] stm class: dummy_stm: Create multiple devices

[Alexander Shishkin]

STM framework should be able to handle multiple STM devices at a time,
each one with its own master allocation policy.

This patch changes dummy_stm driver to create multiple STM sinks to
help testing the framework.

Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 drivers/hwtracing/stm/dummy_stm.c | 55 ++++++++++++++++++++++++++++++++-------
 1 file changed, 46 insertions(+), 9 deletions(-)

diff --git a/drivers/hwtracing/stm/dummy_stm.c b/drivers/hwtracing/stm/dummy_stm.c
index 3709bef0b2..93de609688 100644
--- a/drivers/hwtracing/stm/dummy_stm.c
+++ b/drivers/hwtracing/stm/dummy_stm.c
@@ -40,22 +40,59 @@ dummy_stm_packet(struct stm_data *stm_data, unsigned int master,
 	return size;
 }
 
-static struct stm_data dummy_stm = {
-	.name		= "dummy_stm",
-	.sw_start	= 0x0000,
-	.sw_end		= 0xffff,
-	.sw_nchannels	= 0xffff,
-	.packet		= dummy_stm_packet,
-};
+#define DUMMY_STM_MAX 32
+
+static struct stm_data dummy_stm[DUMMY_STM_MAX];
+
+static int nr_dummies = 4;
+
+module_param(nr_dummies, int, 0600);
+
+static unsigned int dummy_stm_nr;
 
 static int dummy_stm_init(void)
 {
-	return stm_register_device(NULL, &dummy_stm, THIS_MODULE);
+	int i, ret = -ENOMEM, __nr_dummies = ACCESS_ONCE(nr_dummies);
+
+	if (__nr_dummies < 0 || __nr_dummies > DUMMY_STM_MAX)
+		return -EINVAL;
+
+	for (i = 0; i < __nr_dummies; i++) {
+		dummy_stm[i].name = kasprintf(GFP_KERNEL, "dummy_stm.%d", i);
+		if (!dummy_stm[i].name)
+			goto fail_unregister;
+
+		dummy_stm[i].sw_start		= 0x0000;
+		dummy_stm[i].sw_end		= 0xffff;
+		dummy_stm[i].sw_nchannels	= 0xffff;
+		dummy_stm[i].packet		= dummy_stm_packet;
+
+		ret = stm_register_device(NULL, &dummy_stm[i], THIS_MODULE);
+		if (ret)
+			goto fail_free;
+	}
+
+	dummy_stm_nr = __nr_dummies;
+
+	return 0;
+
+fail_unregister:
+	for (i--; i >= 0; i--) {
+		stm_unregister_device(&dummy_stm[i]);
+fail_free:
+		kfree(dummy_stm[i].name);
+	}
+
+	return ret;
+
 }
 
 static void dummy_stm_exit(void)
 {
-	stm_unregister_device(&dummy_stm);
+	int i;
+
+	for (i = 0; i < dummy_stm_nr; i++)
+		stm_unregister_device(&dummy_stm[i]);
 }
 
 module_init(dummy_stm_init);
-- 
2.7.0

[QUEUED v0 18/19] intel_th: Set root device's drvdata early

[Alexander Shishkin]

Already during the subdevice initialization time, devices will need
to reference Intel TH controller descriptor structure.

This patch moves setting the drvdata from the pci glue to intel_th
core, before subdevices are populated.

Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 drivers/hwtracing/intel_th/core.c | 2 ++
 drivers/hwtracing/intel_th/pci.c  | 2 --
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hwtracing/intel_th/core.c b/drivers/hwtracing/intel_th/core.c
index b8b9895da5..6df3cd9774 100644
--- a/drivers/hwtracing/intel_th/core.c
+++ b/drivers/hwtracing/intel_th/core.c
@@ -584,6 +584,8 @@ intel_th_alloc(struct device *dev, struct resource *devres,
 	}
 	th->dev = dev;
 
+	dev_set_drvdata(dev, th);
+
 	err = intel_th_populate(th, devres, ndevres, irq);
 	if (err)
 		goto err_chrdev;
diff --git a/drivers/hwtracing/intel_th/pci.c b/drivers/hwtracing/intel_th/pci.c
index 09017073d7..bca7a2ac00 100644
--- a/drivers/hwtracing/intel_th/pci.c
+++ b/drivers/hwtracing/intel_th/pci.c
@@ -46,8 +46,6 @@ static int intel_th_pci_probe(struct pci_dev *pdev,
 	if (IS_ERR(th))
 		return PTR_ERR(th);
 
-	pci_set_drvdata(pdev, th);
-
 	return 0;
 }
 
-- 
2.7.0

[QUEUED v0 19/19] intel_th: Use real device index in the node names

[Alexander Shishkin]

Most of the intel_th core supports multiple co-existing TH devices,
except for output device nodes, where intel_th device id is hardcoded
to be zero.

Fix this by fetching the actual intel_th device id from the parent
device's drvdata.

Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 drivers/hwtracing/intel_th/core.c | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/drivers/hwtracing/intel_th/core.c b/drivers/hwtracing/intel_th/core.c
index 6df3cd9774..4272f2ce5f 100644
--- a/drivers/hwtracing/intel_th/core.c
+++ b/drivers/hwtracing/intel_th/core.c
@@ -124,17 +124,34 @@ static struct device_type intel_th_source_device_type = {
 	.release	= intel_th_device_release,
 };
 
+static struct intel_th *to_intel_th(struct intel_th_device *thdev)
+{
+	/*
+	 * subdevice tree is flat: if this one is not a switch, its
+	 * parent must be
+	 */
+	if (thdev->type != INTEL_TH_SWITCH)
+		thdev = to_intel_th_hub(thdev);
+
+	if (WARN_ON_ONCE(!thdev || thdev->type != INTEL_TH_SWITCH))
+		return NULL;
+
+	return dev_get_drvdata(thdev->dev.parent);
+}
+
 static char *intel_th_output_devnode(struct device *dev, umode_t *mode,
 				     kuid_t *uid, kgid_t *gid)
 {
 	struct intel_th_device *thdev = to_intel_th_device(dev);
+	struct intel_th *th = to_intel_th(thdev);
 	char *node;
 
 	if (thdev->id >= 0)
-		node = kasprintf(GFP_KERNEL, "intel_th%d/%s%d", 0, thdev->name,
-				 thdev->id);
+		node = kasprintf(GFP_KERNEL, "intel_th%d/%s%d", th->id,
+				 thdev->name, thdev->id);
 	else
-		node = kasprintf(GFP_KERNEL, "intel_th%d/%s", 0, thdev->name);
+		node = kasprintf(GFP_KERNEL, "intel_th%d/%s", th->id,
+				 thdev->name);
 
 	return node;
 }
-- 
2.7.0