[PATCH v1 0/5] Add sysfs interface to switchtec-dma

[PATCH v1 0/5] Add sysfs interface to switchtec-dma

[Logan Gunthorpe]

This patch set adds a handful of sysfs attributes to the switchtec-dma
driver.

The first two patches clean up and generalizes the technique that
ioat used to add sysfs attributes to a dma channel. The third and forth
patches add a couple different sets of sysfs attributes that are useful
in configuring and monitoring the hardware. The last patch just adds a
new device ID for the hardware.

This patch set is based on v7.2-rc2.

Logan Gunthorpe (5):
  dmaengine: add support for custom per-channel sysfs attributes
  dmaengine: ioatdma: use common channel sysfs attribute creation
  dmaengine: switchtec-dma: Add config sysfs attributes
  dmaengine: switchtec-dma: Add pmon sysfs attributes
  dmaengine: switchtec-dma: Add PCI1008 device ID

 drivers/dma/dmaengine.c     |  62 +++++++
 drivers/dma/dmaengine.h     |  11 ++
 drivers/dma/ioat/dma.h      |   2 -
 drivers/dma/ioat/init.c     |   4 +-
 drivers/dma/ioat/sysfs.c    |  88 +---------
 drivers/dma/switchtec_dma.c | 341 ++++++++++++++++++++++++++++++++++++
 include/linux/dmaengine.h   |   3 +
 7 files changed, 424 insertions(+), 87 deletions(-)


base-commit: 8cdeaa50eae8dad34885515f62559ee83e7e8dda
-- 
2.47.3

[PATCH v1 1/5] dmaengine: add support for custom per-channel sysfs attributes

[Logan Gunthorpe]

Add specific support for adding sysfs attributes to channels.

This will be used to replace similar functionality in ioat so it can
be used in other drivers.

Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
---
 drivers/dma/dmaengine.c   | 62 +++++++++++++++++++++++++++++++++++++++
 drivers/dma/dmaengine.h   | 11 +++++++
 include/linux/dmaengine.h |  3 ++
 3 files changed, 76 insertions(+)

diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index 9049171df857..7ea2a8709c07 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -125,6 +125,63 @@ static int dmaengine_summary_show(struct seq_file *s, void *data)
 }
 DEFINE_SHOW_ATTRIBUTE(dmaengine_summary);
 
+static ssize_t
+dma_chan_attr_show(struct kobject *kobj, struct attribute *attr, char *page)
+{
+	const struct dma_chan_sysfs_entry *entry;
+	struct dma_chan *dma_chan;
+
+	entry = container_of_const(attr, struct dma_chan_sysfs_entry, attr);
+	dma_chan = container_of(kobj, struct dma_chan, kobj);
+
+	if (!entry->show)
+		return -EIO;
+
+	return entry->show(dma_chan, page);
+}
+
+static ssize_t
+dma_chan_attr_store(struct kobject *kobj, struct attribute *attr,
+		    const char *page, size_t count)
+{
+	const struct dma_chan_sysfs_entry *entry;
+	struct dma_chan *dma_chan;
+
+	entry = container_of_const(attr, struct dma_chan_sysfs_entry, attr);
+	dma_chan = container_of(kobj, struct dma_chan, kobj);
+
+	if (!entry->store)
+		return -EIO;
+
+	return entry->store(dma_chan, page, count);
+}
+
+const struct sysfs_ops dma_chan_sysfs_ops = {
+	.show = dma_chan_attr_show,
+	.store = dma_chan_attr_store,
+};
+EXPORT_SYMBOL_GPL(dma_chan_sysfs_ops);
+
+void dma_chan_kobject_add(struct dma_device *dev, const struct kobj_type *type,
+			  const char *name)
+{
+	struct dma_chan *chan;
+	int err;
+
+	list_for_each_entry(chan, &dev->channels, device_node) {
+		chan->kobj_used = true;
+		err = kobject_init_and_add(&chan->kobj, type,
+					   &chan->dev->device.kobj, name);
+		if (err) {
+			dev_warn(dev->dev,
+				 "sysis init error(%d), continuinng...\n", err);
+			kobject_put(&chan->kobj);
+			chan->kobj_used = false;
+		}
+	}
+}
+EXPORT_SYMBOL_GPL(dma_chan_kobject_add);
+
 static void __init dmaengine_debugfs_init(void)
 {
 	rootdir = debugfs_create_dir("dmaengine", NULL);
@@ -1143,6 +1200,11 @@ static void __dma_async_device_channel_unregister(struct dma_device *device,
 	if (chan->local == NULL)
 		return;
 
+	if (chan->kobj_used) {
+		kobject_del(&chan->kobj);
+		kobject_put(&chan->kobj);
+	}
+
 	WARN_ONCE(!device->device_release && chan->client_count,
 		  "%s called while %d clients hold a reference\n",
 		  __func__, chan->client_count);
diff --git a/drivers/dma/dmaengine.h b/drivers/dma/dmaengine.h
index 53f16d3f0029..496cac056350 100644
--- a/drivers/dma/dmaengine.h
+++ b/drivers/dma/dmaengine.h
@@ -182,6 +182,17 @@ dmaengine_desc_callback_valid(struct dmaengine_desc_callback *cb)
 struct dma_chan *dma_get_slave_channel(struct dma_chan *chan);
 struct dma_chan *dma_get_any_slave_channel(struct dma_device *device);
 
+struct dma_chan_sysfs_entry {
+	struct attribute attr;
+	ssize_t (*show)(struct dma_chan *chan, char *page);
+	ssize_t (*store)(struct dma_chan *chan, const char *page, size_t count);
+};
+
+extern const struct sysfs_ops dma_chan_sysfs_ops;
+
+void dma_chan_kobject_add(struct dma_device *dev, const struct kobj_type *type,
+			  const char *name);
+
 #ifdef CONFIG_DEBUG_FS
 #include <linux/debugfs.h>
 
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index b3d251c9734e..852943ef46b8 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -358,6 +358,9 @@ struct dma_chan {
 	struct dma_router *router;
 	void *route_data;
 
+	bool kobj_used;
+	struct kobject kobj;
+
 	void *private;
 };
 
-- 
2.47.3

Re: [PATCH v1 1/5] dmaengine: add support for custom per-channel sysfs attributes

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Embedding a raw `kobject` into `struct dma_chan` without governing the channel's lifecycle creates an unavoidable Use-After-Free vulnerability due to a strict kobject lifecycle mismatch.
- [High] The `name` parameter is passed directly as the `fmt` string to `kobject_init_and_add`, leading to a format string vulnerability.
- [Medium] `dma_chan_kobject_add` calculates an invalid parent pointer if called before the channel is fully registered.
- [Low] Typographical errors in the kobject initialization failure warning message.
--

--- Patch [1]: [PATCH v1 1/5] dmaengine: add support for custom per-channel sysfs attributes ---
commit e1443713f7d441022da01ca370f8ab1129a41d63
Author: Logan Gunthorpe <logang@deltatee.com>

dmaengine: add support for custom per-channel sysfs attributes

This commit adds specific support for adding sysfs attributes to DMA channels.
It is intended to be used to replace similar functionality in ioat so it can
be utilized in other drivers.

> diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
> index 9049171df8578..7ea2a8709c078 100644
> --- a/drivers/dma/dmaengine.c
> +++ b/drivers/dma/dmaengine.c
[ ... ]
> @@ -125,6 +125,63 @@ static int dmaengine_summary_show(struct seq_file *s, void *data)
[ ... ]
> +void dma_chan_kobject_add(struct dma_device *dev, const struct kobj_type *type,
> +			  const char *name)
> +{
> +	struct dma_chan *chan;
> +	int err;
> +
> +	list_for_each_entry(chan, &dev->channels, device_node) {
> +		chan->kobj_used = true;
> +		err = kobject_init_and_add(&chan->kobj, type,
> +					   &chan->dev->device.kobj, name);

[Severity: High]
Could this introduce a format string vulnerability? kobject_init_and_add()
takes a format string as its fourth argument, and passing 'name' directly
means any format specifiers in the driver-provided name could cause
unpredictable behavior. Should this be passed safely as "%s", name?

[Severity: Medium]
Will this calculate an invalid parent pointer if a driver calls this function
before the channel is fully registered? 

Since chan->dev is dynamically allocated during dma_async_device_register(), 
wouldn't this perform pointer arithmetic on a NULL pointer if called during a 
driver's probe routine prior to registration?

> +		if (err) {
> +			dev_warn(dev->dev,
> +				 "sysis init error(%d), continuinng...\n", err);

[Severity: Low]
This isn't a bug, but there are typos in the warning message ("sysis" instead
of "sysfs" and "continuinng" instead of "continuing").

> +			kobject_put(&chan->kobj);
> +			chan->kobj_used = false;

[Severity: High]
Does this error handling path introduce a use-after-free? 

The kobject lifecycle API mandates that the release callback must free the 
structure containing the kobject. If the driver complies and frees 'chan' 
during kobject_put(), won't setting chan->kobj_used and the subsequent list 
iteration dereference freed memory?

> +		}
> +	}
> +}
> +EXPORT_SYMBOL_GPL(dma_chan_kobject_add);
[ ... ]
> @@ -1143,6 +1200,11 @@ static void __dma_async_device_channel_unregister(struct dma_device *device,
>  	if (chan->local == NULL)
>  		return;
>  
> +	if (chan->kobj_used) {
> +		kobject_del(&chan->kobj);
> +		kobject_put(&chan->kobj);

[Severity: High]
If a driver does not free 'chan' in the kobject release callback to avoid the
error path use-after-free in dma_chan_kobject_add(), does this create a 
different use-after-free here?

If userspace holds an open file descriptor to the sysfs attributes, the
kobject reference count remains above zero after kobject_put(). The driver
will proceed to free the dma_chan memory during unloading, leaving sysfs with
a reference to freed memory.

> +	}
> +
>  	WARN_ONCE(!device->device_release && chan->client_count,
[ ... ]
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index b3d251c9734e9..852943ef46b8e 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
[ ... ]
> @@ -358,6 +358,9 @@ struct dma_chan {
>  	struct dma_router *router;
>  	void *route_data;
>  
> +	bool kobj_used;
> +	struct kobject kobj;

[Severity: High]
Does embedding a raw kobject without governing the channel's lifecycle create
an unavoidable use-after-free vulnerability? 

This structural approach forces a mismatch between the kobject lifecycle and 
the dma_chan lifecycle, triggering one of the two use-after-free scenarios 
noted above.

> +
>  	void *private;
>  };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707162045.23910-1-logang@deltatee.com?part=1

[PATCH v1 2/5] dmaengine: ioatdma: use common channel sysfs attribute creation

[Logan Gunthorpe]

Instead of manually creating and adding sysfs attributes to each channel
use the common dma_chan_kobject_add() facility.

Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
---
 drivers/dma/ioat/dma.h   |  2 -
 drivers/dma/ioat/init.c  |  4 +-
 drivers/dma/ioat/sysfs.c | 88 +++-------------------------------------
 3 files changed, 7 insertions(+), 87 deletions(-)

diff --git a/drivers/dma/ioat/dma.h b/drivers/dma/ioat/dma.h
index e8a880f338c6..bea2a0101ede 100644
--- a/drivers/dma/ioat/dma.h
+++ b/drivers/dma/ioat/dma.h
@@ -393,8 +393,6 @@ void ioat_issue_pending(struct dma_chan *chan);
 /* IOAT Init functions */
 bool is_bwd_ioat(struct pci_dev *pdev);
 struct dca_provider *ioat_dca_init(struct pci_dev *pdev, void __iomem *iobase);
-void ioat_kobject_add(struct ioatdma_device *ioat_dma, const struct kobj_type *type);
-void ioat_kobject_del(struct ioatdma_device *ioat_dma);
 int ioat_dma_setup_interrupts(struct ioatdma_device *ioat_dma);
 void ioat_stop(struct ioatdma_chan *ioat_chan);
 #endif /* IOATDMA_H */
diff --git a/drivers/dma/ioat/init.c b/drivers/dma/ioat/init.c
index 737496391109..82f76fd0ccc1 100644
--- a/drivers/dma/ioat/init.c
+++ b/drivers/dma/ioat/init.c
@@ -541,8 +541,6 @@ static void ioat_dma_remove(struct ioatdma_device *ioat_dma)
 
 	ioat_disable_interrupts(ioat_dma);
 
-	ioat_kobject_del(ioat_dma);
-
 	dma_async_device_unregister(dma);
 }
 
@@ -1174,7 +1172,7 @@ static int ioat3_dma_probe(struct ioatdma_device *ioat_dma, int dca)
 	if (err)
 		goto err_disable_interrupts;
 
-	ioat_kobject_add(ioat_dma, &ioat_ktype);
+	dma_chan_kobject_add(&ioat_dma->dma_dev, &ioat_ktype, "quickdata");
 
 	if (dca)
 		ioat_dma->dca = ioat_dca_init(pdev, ioat_dma->reg_base);
diff --git a/drivers/dma/ioat/sysfs.c b/drivers/dma/ioat/sysfs.c
index e796ddb5383f..4dc61fdddb21 100644
--- a/drivers/dma/ioat/sysfs.c
+++ b/drivers/dma/ioat/sysfs.c
@@ -14,12 +14,6 @@
 
 #include "../dmaengine.h"
 
-struct ioat_sysfs_entry {
-	struct attribute attr;
-	ssize_t (*show)(struct dma_chan *, char *);
-	ssize_t (*store)(struct dma_chan *, const char *, size_t);
-};
-
 static ssize_t cap_show(struct dma_chan *c, char *page)
 {
 	struct dma_device *dma = c->device;
@@ -32,7 +26,7 @@ static ssize_t cap_show(struct dma_chan *c, char *page)
 		       dma_has_cap(DMA_INTERRUPT, dma->cap_mask) ? " intr" : "");
 
 }
-static const struct ioat_sysfs_entry ioat_cap_attr = __ATTR_RO(cap);
+static const struct dma_chan_sysfs_entry ioat_cap_attr = __ATTR_RO(cap);
 
 static ssize_t version_show(struct dma_chan *c, char *page)
 {
@@ -42,77 +36,7 @@ static ssize_t version_show(struct dma_chan *c, char *page)
 	return sprintf(page, "%d.%d\n",
 		       ioat_dma->version >> 4, ioat_dma->version & 0xf);
 }
-static const struct ioat_sysfs_entry ioat_version_attr = __ATTR_RO(version);
-
-static ssize_t
-ioat_attr_show(struct kobject *kobj, struct attribute *attr, char *page)
-{
-	const struct ioat_sysfs_entry *entry;
-	struct ioatdma_chan *ioat_chan;
-
-	entry = container_of_const(attr, struct ioat_sysfs_entry, attr);
-	ioat_chan = container_of(kobj, struct ioatdma_chan, kobj);
-
-	if (!entry->show)
-		return -EIO;
-	return entry->show(&ioat_chan->dma_chan, page);
-}
-
-static ssize_t
-ioat_attr_store(struct kobject *kobj, struct attribute *attr,
-const char *page, size_t count)
-{
-	const struct ioat_sysfs_entry *entry;
-	struct ioatdma_chan *ioat_chan;
-
-	entry = container_of_const(attr, struct ioat_sysfs_entry, attr);
-	ioat_chan = container_of(kobj, struct ioatdma_chan, kobj);
-
-	if (!entry->store)
-		return -EIO;
-	return entry->store(&ioat_chan->dma_chan, page, count);
-}
-
-static const struct sysfs_ops ioat_sysfs_ops = {
-	.show	= ioat_attr_show,
-	.store  = ioat_attr_store,
-};
-
-void ioat_kobject_add(struct ioatdma_device *ioat_dma, const struct kobj_type *type)
-{
-	struct dma_device *dma = &ioat_dma->dma_dev;
-	struct dma_chan *c;
-
-	list_for_each_entry(c, &dma->channels, device_node) {
-		struct ioatdma_chan *ioat_chan = to_ioat_chan(c);
-		struct kobject *parent = &c->dev->device.kobj;
-		int err;
-
-		err = kobject_init_and_add(&ioat_chan->kobj, type,
-					   parent, "quickdata");
-		if (err) {
-			dev_warn(to_dev(ioat_chan),
-				 "sysfs init error (%d), continuing...\n", err);
-			kobject_put(&ioat_chan->kobj);
-			set_bit(IOAT_KOBJ_INIT_FAIL, &ioat_chan->state);
-		}
-	}
-}
-
-void ioat_kobject_del(struct ioatdma_device *ioat_dma)
-{
-	struct dma_device *dma = &ioat_dma->dma_dev;
-	struct dma_chan *c;
-
-	list_for_each_entry(c, &dma->channels, device_node) {
-		struct ioatdma_chan *ioat_chan = to_ioat_chan(c);
-
-		if (!test_bit(IOAT_KOBJ_INIT_FAIL, &ioat_chan->state)) {
-			kobject_del(&ioat_chan->kobj);
-			kobject_put(&ioat_chan->kobj);
-		}
-	}
-}
+static const struct dma_chan_sysfs_entry ioat_version_attr = __ATTR_RO(version);
 
 static ssize_t ring_size_show(struct dma_chan *c, char *page)
 {
@@ -120,7 +44,7 @@ static ssize_t ring_size_show(struct dma_chan *c, char *page)
 
 	return sprintf(page, "%d\n", (1 << ioat_chan->alloc_order) & ~1);
 }
-static const struct ioat_sysfs_entry ring_size_attr = __ATTR_RO(ring_size);
+static const struct dma_chan_sysfs_entry ring_size_attr = __ATTR_RO(ring_size);
 
 static ssize_t ring_active_show(struct dma_chan *c, char *page)
 {
@@ -129,7 +53,7 @@ static ssize_t ring_active_show(struct dma_chan *c, char *page)
 	/* ...taken outside the lock, no need to be precise */
 	return sprintf(page, "%d\n", ioat_ring_active(ioat_chan));
 }
-static const struct ioat_sysfs_entry ring_active_attr = __ATTR_RO(ring_active);
+static const struct dma_chan_sysfs_entry ring_active_attr = __ATTR_RO(ring_active);
 
 static ssize_t intr_coalesce_show(struct dma_chan *c, char *page)
 {
@@ -154,7 +78,7 @@ size_t count)
 	return count;
 }
 
-static const struct ioat_sysfs_entry intr_coalesce_attr = __ATTR_RW(intr_coalesce);
+static const struct dma_chan_sysfs_entry intr_coalesce_attr = __ATTR_RW(intr_coalesce);
 
 static const struct attribute *const ioat_attrs[] = {
 	&ring_size_attr.attr,
@@ -167,6 +91,6 @@ static const struct attribute *const ioat_attrs[] = {
 ATTRIBUTE_GROUPS(ioat);
 
 const struct kobj_type ioat_ktype = {
-	.sysfs_ops = &ioat_sysfs_ops,
+	.sysfs_ops = &dma_chan_sysfs_ops,
 	.default_groups = ioat_groups,
 };
-- 
2.47.3

Re: [PATCH v1 2/5] dmaengine: ioatdma: use common channel sysfs attribute creation

[Dave Jiang]

On 7/7/26 9:20 AM, Logan Gunthorpe wrote:
> Instead of manually creating and adding sysfs attributes to each channel
> use the common dma_chan_kobject_add() facility.
> 
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>

Acked-by: Dave Jiang <dave.jiang@intel.com>

> ---
>  drivers/dma/ioat/dma.h   |  2 -
>  drivers/dma/ioat/init.c  |  4 +-
>  drivers/dma/ioat/sysfs.c | 88 +++-------------------------------------
>  3 files changed, 7 insertions(+), 87 deletions(-)
> 
> diff --git a/drivers/dma/ioat/dma.h b/drivers/dma/ioat/dma.h
> index e8a880f338c6..bea2a0101ede 100644
> --- a/drivers/dma/ioat/dma.h
> +++ b/drivers/dma/ioat/dma.h
> @@ -393,8 +393,6 @@ void ioat_issue_pending(struct dma_chan *chan);
>  /* IOAT Init functions */
>  bool is_bwd_ioat(struct pci_dev *pdev);
>  struct dca_provider *ioat_dca_init(struct pci_dev *pdev, void __iomem *iobase);
> -void ioat_kobject_add(struct ioatdma_device *ioat_dma, const struct kobj_type *type);
> -void ioat_kobject_del(struct ioatdma_device *ioat_dma);
>  int ioat_dma_setup_interrupts(struct ioatdma_device *ioat_dma);
>  void ioat_stop(struct ioatdma_chan *ioat_chan);
>  #endif /* IOATDMA_H */
> diff --git a/drivers/dma/ioat/init.c b/drivers/dma/ioat/init.c
> index 737496391109..82f76fd0ccc1 100644
> --- a/drivers/dma/ioat/init.c
> +++ b/drivers/dma/ioat/init.c
> @@ -541,8 +541,6 @@ static void ioat_dma_remove(struct ioatdma_device *ioat_dma)
>  
>  	ioat_disable_interrupts(ioat_dma);
>  
> -	ioat_kobject_del(ioat_dma);
> -
>  	dma_async_device_unregister(dma);
>  }
>  
> @@ -1174,7 +1172,7 @@ static int ioat3_dma_probe(struct ioatdma_device *ioat_dma, int dca)
>  	if (err)
>  		goto err_disable_interrupts;
>  
> -	ioat_kobject_add(ioat_dma, &ioat_ktype);
> +	dma_chan_kobject_add(&ioat_dma->dma_dev, &ioat_ktype, "quickdata");
>  
>  	if (dca)
>  		ioat_dma->dca = ioat_dca_init(pdev, ioat_dma->reg_base);
> diff --git a/drivers/dma/ioat/sysfs.c b/drivers/dma/ioat/sysfs.c
> index e796ddb5383f..4dc61fdddb21 100644
> --- a/drivers/dma/ioat/sysfs.c
> +++ b/drivers/dma/ioat/sysfs.c
> @@ -14,12 +14,6 @@
>  
>  #include "../dmaengine.h"
>  
> -struct ioat_sysfs_entry {
> -	struct attribute attr;
> -	ssize_t (*show)(struct dma_chan *, char *);
> -	ssize_t (*store)(struct dma_chan *, const char *, size_t);
> -};
> -
>  static ssize_t cap_show(struct dma_chan *c, char *page)
>  {
>  	struct dma_device *dma = c->device;
> @@ -32,7 +26,7 @@ static ssize_t cap_show(struct dma_chan *c, char *page)
>  		       dma_has_cap(DMA_INTERRUPT, dma->cap_mask) ? " intr" : "");
>  
>  }
> -static const struct ioat_sysfs_entry ioat_cap_attr = __ATTR_RO(cap);
> +static const struct dma_chan_sysfs_entry ioat_cap_attr = __ATTR_RO(cap);
>  
>  static ssize_t version_show(struct dma_chan *c, char *page)
>  {
> @@ -42,77 +36,7 @@ static ssize_t version_show(struct dma_chan *c, char *page)
>  	return sprintf(page, "%d.%d\n",
>  		       ioat_dma->version >> 4, ioat_dma->version & 0xf);
>  }
> -static const struct ioat_sysfs_entry ioat_version_attr = __ATTR_RO(version);
> -
> -static ssize_t
> -ioat_attr_show(struct kobject *kobj, struct attribute *attr, char *page)
> -{
> -	const struct ioat_sysfs_entry *entry;
> -	struct ioatdma_chan *ioat_chan;
> -
> -	entry = container_of_const(attr, struct ioat_sysfs_entry, attr);
> -	ioat_chan = container_of(kobj, struct ioatdma_chan, kobj);
> -
> -	if (!entry->show)
> -		return -EIO;
> -	return entry->show(&ioat_chan->dma_chan, page);
> -}
> -
> -static ssize_t
> -ioat_attr_store(struct kobject *kobj, struct attribute *attr,
> -const char *page, size_t count)
> -{
> -	const struct ioat_sysfs_entry *entry;
> -	struct ioatdma_chan *ioat_chan;
> -
> -	entry = container_of_const(attr, struct ioat_sysfs_entry, attr);
> -	ioat_chan = container_of(kobj, struct ioatdma_chan, kobj);
> -
> -	if (!entry->store)
> -		return -EIO;
> -	return entry->store(&ioat_chan->dma_chan, page, count);
> -}
> -
> -static const struct sysfs_ops ioat_sysfs_ops = {
> -	.show	= ioat_attr_show,
> -	.store  = ioat_attr_store,
> -};
> -
> -void ioat_kobject_add(struct ioatdma_device *ioat_dma, const struct kobj_type *type)
> -{
> -	struct dma_device *dma = &ioat_dma->dma_dev;
> -	struct dma_chan *c;
> -
> -	list_for_each_entry(c, &dma->channels, device_node) {
> -		struct ioatdma_chan *ioat_chan = to_ioat_chan(c);
> -		struct kobject *parent = &c->dev->device.kobj;
> -		int err;
> -
> -		err = kobject_init_and_add(&ioat_chan->kobj, type,
> -					   parent, "quickdata");
> -		if (err) {
> -			dev_warn(to_dev(ioat_chan),
> -				 "sysfs init error (%d), continuing...\n", err);
> -			kobject_put(&ioat_chan->kobj);
> -			set_bit(IOAT_KOBJ_INIT_FAIL, &ioat_chan->state);
> -		}
> -	}
> -}
> -
> -void ioat_kobject_del(struct ioatdma_device *ioat_dma)
> -{
> -	struct dma_device *dma = &ioat_dma->dma_dev;
> -	struct dma_chan *c;
> -
> -	list_for_each_entry(c, &dma->channels, device_node) {
> -		struct ioatdma_chan *ioat_chan = to_ioat_chan(c);
> -
> -		if (!test_bit(IOAT_KOBJ_INIT_FAIL, &ioat_chan->state)) {
> -			kobject_del(&ioat_chan->kobj);
> -			kobject_put(&ioat_chan->kobj);
> -		}
> -	}
> -}
> +static const struct dma_chan_sysfs_entry ioat_version_attr = __ATTR_RO(version);
>  
>  static ssize_t ring_size_show(struct dma_chan *c, char *page)
>  {
> @@ -120,7 +44,7 @@ static ssize_t ring_size_show(struct dma_chan *c, char *page)
>  
>  	return sprintf(page, "%d\n", (1 << ioat_chan->alloc_order) & ~1);
>  }
> -static const struct ioat_sysfs_entry ring_size_attr = __ATTR_RO(ring_size);
> +static const struct dma_chan_sysfs_entry ring_size_attr = __ATTR_RO(ring_size);
>  
>  static ssize_t ring_active_show(struct dma_chan *c, char *page)
>  {
> @@ -129,7 +53,7 @@ static ssize_t ring_active_show(struct dma_chan *c, char *page)
>  	/* ...taken outside the lock, no need to be precise */
>  	return sprintf(page, "%d\n", ioat_ring_active(ioat_chan));
>  }
> -static const struct ioat_sysfs_entry ring_active_attr = __ATTR_RO(ring_active);
> +static const struct dma_chan_sysfs_entry ring_active_attr = __ATTR_RO(ring_active);
>  
>  static ssize_t intr_coalesce_show(struct dma_chan *c, char *page)
>  {
> @@ -154,7 +78,7 @@ size_t count)
>  	return count;
>  }
>  
> -static const struct ioat_sysfs_entry intr_coalesce_attr = __ATTR_RW(intr_coalesce);
> +static const struct dma_chan_sysfs_entry intr_coalesce_attr = __ATTR_RW(intr_coalesce);
>  
>  static const struct attribute *const ioat_attrs[] = {
>  	&ring_size_attr.attr,
> @@ -167,6 +91,6 @@ static const struct attribute *const ioat_attrs[] = {
>  ATTRIBUTE_GROUPS(ioat);
>  
>  const struct kobj_type ioat_ktype = {
> -	.sysfs_ops = &ioat_sysfs_ops,
> +	.sysfs_ops = &dma_chan_sysfs_ops,
>  	.default_groups = ioat_groups,
>  };

Re: [PATCH v1 2/5] dmaengine: ioatdma: use common channel sysfs attribute creation

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

New issues:
- [Low] The patch leaves behind an unused `struct kobject kobj` and `IOAT_KOBJ_INIT_FAIL` state flag in `struct ioatdma_chan`.
- [Low] Unconditional reference to `dma_chan_sysfs_ops` and `dma_chan_kobject_add` causes build breakage if `CONFIG_DEBUG_FS` is disabled.

Pre-existing issues:
- [High] Missing `dma_async_device_unregister()` on error paths in `ioat3_dma_probe()` causes Use-After-Free and resource leaks.
- [High] Missing `release` callback in `kobj_type` violates kobject lifetime rules and causes Use-After-Free.
--

--- Patch [2]: [PATCH v1 2/5] dmaengine: ioatdma: use common channel sysfs attribute creation ---
commit 88321b4bdaabe072b5dfb4d9c84f04832a5bcc78
Author: Logan Gunthorpe <logang@deltatee.com>

dmaengine: ioatdma: use common channel sysfs attribute creation

This patch replaces the manual creation of sysfs attributes for each channel
by utilizing the common dma_chan_kobject_add() facility.

> --- a/drivers/dma/ioat/dma.h
> +++ b/drivers/dma/ioat/dma.h
[ ... ]
> @@ -393,8 +393,6 @@ void ioat_issue_pending(struct dma_chan *chan);
>  /* IOAT Init functions */
>  bool is_bwd_ioat(struct pci_dev *pdev);
>  struct dca_provider *ioat_dca_init(struct pci_dev *pdev, void __iomem *iobase);
> -void ioat_kobject_add(struct ioatdma_device *ioat_dma, const struct kobj_type *type);
> -void ioat_kobject_del(struct ioatdma_device *ioat_dma);

[Severity: Low]
Is it possible to remove the kobj and IOAT_KOBJ_INIT_FAIL fields from struct
ioatdma_chan as well? Since dma_chan_kobject_add() uses the kobject embedded
in struct dma_chan, these fields appear to be unused now.

> --- a/drivers/dma/ioat/init.c
> +++ b/drivers/dma/ioat/init.c
[ ... ]
> @@ -1174,7 +1172,7 @@ static int ioat3_dma_probe(struct ioatdma_device *ioat_dma, int dca)
>  	if (err)
>  		goto err_disable_interrupts;
>  
> -	ioat_kobject_add(ioat_dma, &ioat_ktype);
> +	dma_chan_kobject_add(&ioat_dma->dma_dev, &ioat_ktype, "quickdata");

[Severity: Low]
Will this cause a linker error if the kernel is compiled with CONFIG_DEBUG_FS
disabled? It looks like dma_chan_kobject_add() might depend on debugfs.

[Severity: High]
This is a pre-existing issue, but does this error path properly clean up
device registration?

If an error occurs during PCI capability configuration later in
ioat3_dma_probe() after dma_async_device_register() succeeds:

    err = pcie_capability_read_word(pdev, PCI_EXP_DEVCTL, &val16);
    if (err) {
        err = pcibios_err_to_errno(err);
        goto err_disable_interrupts;
    }

It jumps to err_disable_interrupts and returns an error without calling
dma_async_device_unregister(). Also, with this patch, do the newly added
kobjects need to be cleaned up on this path?

> --- a/drivers/dma/ioat/sysfs.c
> +++ b/drivers/dma/ioat/sysfs.c
[ ... ]
>  const struct kobj_type ioat_ktype = {
> -	.sysfs_ops = &ioat_sysfs_ops,
> +	.sysfs_ops = &dma_chan_sysfs_ops,

[Severity: Low]
Will referencing dma_chan_sysfs_ops here also cause a build failure when
CONFIG_DEBUG_FS is disabled?

[Severity: High]
This is a pre-existing issue, but does ioat_ktype need a .release callback?

Without it, if sysfs holds a reference to the kobject during driver unload,
could it access freed memory when the kobject core later tries to release it?

>  	.default_groups = ioat_groups,
>  };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707162045.23910-1-logang@deltatee.com?part=2

[PATCH v1 3/5] dmaengine: switchtec-dma: Add config sysfs attributes

[Logan Gunthorpe]

Add sysfs configuration options for switchtec-dma devices.

Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
---
 drivers/dma/switchtec_dma.c | 141 ++++++++++++++++++++++++++++++++++++
 1 file changed, 141 insertions(+)

diff --git a/drivers/dma/switchtec_dma.c b/drivers/dma/switchtec_dma.c
index 3ef928640615..4841134bd7b8 100644
--- a/drivers/dma/switchtec_dma.c
+++ b/drivers/dma/switchtec_dma.c
@@ -1027,6 +1027,145 @@ static int switchtec_dma_alloc_chan_resources(struct dma_chan *chan)
 	return SWITCHTEC_DMA_SQ_SIZE;
 }
 
+static __always_inline ssize_t perf_cfg_show(struct dma_chan *chan, char *page,
+					     unsigned int mask)
+{
+	struct switchtec_dma_chan *swdma_chan =
+		container_of(chan, struct switchtec_dma_chan, dma_chan);
+	struct chan_fw_regs __iomem *chan_fw = swdma_chan->mmio_chan_fw;
+	u32 perf_cfg;
+	int value;
+
+	rcu_read_lock();
+	if (!rcu_dereference(swdma_chan->swdma_dev->pdev)) {
+		rcu_read_unlock();
+		return -ENODEV;
+	}
+
+	perf_cfg = readl(&chan_fw->perf_cfg);
+	value = field_get(mask, perf_cfg);
+
+	rcu_read_unlock();
+	return sprintf(page, "0x%x\n", value);
+}
+
+static __always_inline ssize_t perf_cfg_store(struct dma_chan *chan,
+		const char *page, size_t count, unsigned int mask)
+{
+	struct switchtec_dma_chan *swdma_chan =
+		container_of(chan, struct switchtec_dma_chan, dma_chan);
+	struct chan_fw_regs __iomem *chan_fw = swdma_chan->mmio_chan_fw;
+	ssize_t ret = count;
+	u32 perf_cfg;
+	int value;
+
+	if (kstrtoint(page, 0, &value) < 0)
+		return -EINVAL;
+
+	if (value < 0 || value > field_max(mask))
+		return -EINVAL;
+
+	rcu_read_lock();
+	if (!rcu_dereference(swdma_chan->swdma_dev->pdev)) {
+		ret = -ENODEV;
+		goto err_unlock;
+	}
+
+	if (chan->client_count)
+		goto err_unlock;
+
+	perf_cfg = readl(&chan_fw->perf_cfg);
+	perf_cfg = (perf_cfg & ~mask) | field_prep(mask, value);
+	writel(perf_cfg, &chan_fw->perf_cfg);
+
+err_unlock:
+	rcu_read_unlock();
+	return ret;
+}
+
+static ssize_t burst_scale_show(struct dma_chan *chan, char *page)
+{
+	return perf_cfg_show(chan, page, PERF_BURST_SCALE_MASK);
+}
+
+static ssize_t burst_scale_store(struct dma_chan *chan, const char *page,
+				 size_t count)
+{
+	return perf_cfg_store(chan, page, count, PERF_BURST_SCALE_MASK);
+}
+static struct dma_chan_sysfs_entry burst_scale_attr = __ATTR_RW(burst_scale);
+
+static ssize_t mrrs_show(struct dma_chan *chan, char *page)
+{
+	return perf_cfg_show(chan, page, PERF_MRRS_MASK);
+}
+
+static ssize_t mrrs_store(struct dma_chan *chan, const char *page,
+				 size_t count)
+{
+	return perf_cfg_store(chan, page, count, PERF_MRRS_MASK);
+}
+static struct dma_chan_sysfs_entry mrrs_attr = __ATTR_RW(mrrs);
+
+static ssize_t interval_show(struct dma_chan *chan, char *page)
+{
+	return perf_cfg_show(chan, page, PERF_INTERVAL_MASK);
+}
+
+static ssize_t interval_store(struct dma_chan *chan, const char *page,
+				 size_t count)
+{
+	return perf_cfg_store(chan, page, count, PERF_INTERVAL_MASK);
+}
+static struct dma_chan_sysfs_entry interval_attr = __ATTR_RW(interval);
+
+static ssize_t burst_size_show(struct dma_chan *chan, char *page)
+{
+	return perf_cfg_show(chan, page, PERF_BURST_SIZE_MASK);
+}
+
+static ssize_t burst_size_store(struct dma_chan *chan, const char *page,
+				size_t count)
+{
+	return perf_cfg_store(chan, page, count, PERF_BURST_SIZE_MASK);
+}
+static struct dma_chan_sysfs_entry burst_size_attr = __ATTR_RW(burst_size);
+
+static ssize_t arb_weight_show(struct dma_chan *chan, char *page)
+{
+	return perf_cfg_show(chan, page, PERF_ARB_WEIGHT_MASK);
+}
+
+static ssize_t arb_weight_store(struct dma_chan *chan, const char *page,
+				 size_t count)
+{
+	return perf_cfg_store(chan, page, count, PERF_ARB_WEIGHT_MASK);
+}
+static struct dma_chan_sysfs_entry arb_weight_attr = __ATTR_RW(arb_weight);
+
+static struct attribute *switchtec_config_attrs[] = {
+	&burst_scale_attr.attr,
+	&mrrs_attr.attr,
+	&interval_attr.attr,
+	&burst_size_attr.attr,
+	&arb_weight_attr.attr,
+	NULL
+};
+
+static struct attribute_group switchtec_config_group = {
+	.attrs = switchtec_config_attrs,
+};
+
+static const struct attribute_group *switchtec_groups[] = {
+	&switchtec_config_group,
+	NULL,
+};
+
+static const struct kobj_type switchtec_ktype = {
+	.sysfs_ops = &dma_chan_sysfs_ops,
+	.default_groups = switchtec_groups,
+};
+
 static void switchtec_dma_free_chan_resources(struct dma_chan *chan)
 {
 	struct switchtec_dma_chan *swdma_chan =
@@ -1286,6 +1425,8 @@ static int switchtec_dma_create(struct pci_dev *pdev)
 		goto err_chans_release_exit;
 	}
 
+	dma_chan_kobject_add(dma, &switchtec_ktype, "switchtec");
+
 	pci_dbg(pdev, "Channel count: %d\n", chan_cnt);
 
 	list_for_each_entry(chan, &dma->channels, device_node)
-- 
2.47.3

Re: [PATCH v1 3/5] dmaengine: switchtec-dma: Add config sysfs attributes

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:

New issues:
- [Medium] Concurrent sysfs writes to different configuration attributes cause lost updates due to an unlocked read-modify-write sequence.
- [Medium] The configuration store function silently ignores writes when the channel is in use by returning success instead of an error code.
- [Medium] Lockless check of `chan->client_count` allows modifying channel configuration while the channel is concurrently allocated.

Pre-existing issues:
- [High] Memory leak of DMA channel structures on initialization failure.
- [High] switchtec_dma_alloc_chan_resources leaks DMA memory and descriptors on error paths.
--

--- Patch [3]: [PATCH v1 3/5] dmaengine: switchtec-dma: Add config sysfs attributes ---
commit 8760f8c3c1480cd451ac42e9181aa92c4b359981
Author: Logan Gunthorpe <logang@deltatee.com>

dmaengine: switchtec-dma: Add config sysfs attributes

Add sysfs configuration options for switchtec-dma devices.

> diff --git a/drivers/dma/switchtec_dma.c b/drivers/dma/switchtec_dma.c
> index 3ef9286406159..4841134bd7b83 100644
> --- a/drivers/dma/switchtec_dma.c
> +++ b/drivers/dma/switchtec_dma.c
> @@ -1027,6 +1027,145 @@ static int switchtec_dma_alloc_chan_resources(struct dma_chan *chan)
>  	return SWITCHTEC_DMA_SQ_SIZE;
>  }

[Severity: High]
This is a pre-existing issue, but I noticed that
switchtec_dma_alloc_chan_resources() allocates coherent DMA memory and
descriptor rings by calling switchtec_dma_alloc_desc(). If any subsequent
hardware initialization step fails, it returns directly with an error code:

	rc = switchtec_dma_alloc_desc(swdma_chan);
	if (rc)
		return rc;

	rc = enable_channel(swdma_chan);
	if (rc)
		return rc;

Since the dmaengine core handles allocation failures by dropping the module
reference without calling device_free_chan_resources(), does this permanently
leak the allocated descriptors and 32KB coherent DMA memory blocks?

> +static __always_inline ssize_t perf_cfg_store(struct dma_chan *chan,
> +		const char *page, size_t count, unsigned int mask)
> +{
> +	struct switchtec_dma_chan *swdma_chan =
> +		container_of(chan, struct switchtec_dma_chan, dma_chan);
> +	struct chan_fw_regs __iomem *chan_fw = swdma_chan->mmio_chan_fw;
> +	ssize_t ret = count;
> +	u32 perf_cfg;
> +	int value;
> +
> +	if (kstrtoint(page, 0, &value) < 0)
> +		return -EINVAL;
> +
> +	if (value < 0 || value > field_max(mask))
> +		return -EINVAL;
> +
> +	rcu_read_lock();
> +	if (!rcu_dereference(swdma_chan->swdma_dev->pdev)) {
> +		ret = -ENODEV;
> +		goto err_unlock;
> +	}
> +
> +	if (chan->client_count)
> +		goto err_unlock;

[Severity: Medium]
If the channel is in use (chan->client_count is non-zero), the code jumps to
err_unlock but returns ret (which is still initialized to count). Will this
falsely indicate to userspace that the write succeeded instead of returning an
error code like -EBUSY?

[Severity: Medium]
Also, this lockless check of chan->client_count appears to allow a TOCTOU
race. Could a concurrent thread allocate the channel via
dma_request_chan() immediately after this check passes, causing the sysfs
thread to modify the hardware configuration out from under an active DMA
client?

> +
> +	perf_cfg = readl(&chan_fw->perf_cfg);
> +	perf_cfg = (perf_cfg & ~mask) | field_prep(mask, value);
> +	writel(perf_cfg, &chan_fw->perf_cfg);

[Severity: Medium]
This read-modify-write sequence on the shared MMIO register
chan_fw->perf_cfg appears to be unprotected by hw_ctrl_lock or any other
lock. If there are concurrent sysfs writes to different attributes (like
burst_scale_store and mrrs_store), could they interleave and cause one of
the updates to be lost?

> +
> +err_unlock:
> +	rcu_read_unlock();
> +	return ret;
> +}

[ ... ]

> @@ -1286,6 +1425,8 @@ static int switchtec_dma_create(struct pci_dev *pdev)
>  		goto err_chans_release_exit;
>  	}
>  

[Severity: High]
This isn't a bug introduced by this patch, but if
dma_async_device_register() fails during probe (which jumps to
err_chans_release_exit just above here), does it leak the dynamically
allocated swdma_chan structures?

The error path calls switchtec_dma_chans_release(), which stops hardware
and frees IRQs, but never calls kfree() on the channels or the
swdma_dev->swdma_chans array itself.

> +	dma_chan_kobject_add(dma, &switchtec_ktype, "switchtec");
> +
>  	pci_dbg(pdev, "Channel count: %d\n", chan_cnt);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707162045.23910-1-logang@deltatee.com?part=3

[PATCH v1 4/5] dmaengine: switchtec-dma: Add pmon sysfs attributes

[Logan Gunthorpe]

Switchtec hardware exposes some performance monitor registers which
can be used to monitor various statistics of the hardware.

Expose these as a sysfs interface under the switchtec group in a
pmon sub-group.

Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
---
 drivers/dma/switchtec_dma.c | 199 ++++++++++++++++++++++++++++++++++++
 1 file changed, 199 insertions(+)

diff --git a/drivers/dma/switchtec_dma.c b/drivers/dma/switchtec_dma.c
index 4841134bd7b8..2ac43eb58995 100644
--- a/drivers/dma/switchtec_dma.c
+++ b/drivers/dma/switchtec_dma.c
@@ -12,6 +12,7 @@
 #include <linux/pci.h>
 #include <linux/delay.h>
 #include <linux/iopoll.h>
+#include <linux/io-64-nonatomic-lo-hi.h>
 
 #include "dmaengine.h"
 
@@ -91,6 +92,12 @@ struct chan_hw_regs {
 #define SE_BUF_LEN_MASK		GENMASK_U32(20, 12)
 #define SE_THRESH_MASK		GENMASK_U32(31, 23)
 
+#define SWITCHTEC_LAT_SE_FETCH   BIT(0)
+#define SWITCHTEC_LAT_VDM        BIT(1)
+#define SWITCHTEC_LAT_RD_IMM     BIT(2)
+#define SWITCHTEC_LAT_FW_NP      BIT(3)
+#define SWITCHTEC_LAT_SE_PROCESS BIT(4)
+
 #define SWITCHTEC_CHAN_ENABLE	BIT(1)
 
 struct chan_fw_regs {
@@ -1156,8 +1163,200 @@ static struct attribute_group switchtec_config_group = {
 	.attrs = switchtec_config_attrs,
 };
 
+#define pmon_show(chan, page, field, reader) ({				     \
+	struct switchtec_dma_chan *__swdma_chan =			     \
+		container_of((chan), struct switchtec_dma_chan, dma_chan);   \
+	struct chan_fw_regs __iomem *__chan_fw = __swdma_chan->mmio_chan_fw; \
+	u64 __pmon_val = 0;						     \
+	int __pmon_ret = 0;						     \
+									     \
+	rcu_read_lock();						     \
+	if (!rcu_dereference(__swdma_chan->swdma_dev->pdev)) {		     \
+		__pmon_ret = -ENODEV;					     \
+	} else {							     \
+		__pmon_val = reader(&__chan_fw->field);			     \
+		__pmon_ret = sysfs_emit((page), "0x%llx\n", __pmon_val);     \
+	}								     \
+	rcu_read_unlock();						     \
+	__pmon_ret;							     \
+})
+
+static ssize_t se_count_show(struct dma_chan *chan, char *page)
+{
+	return pmon_show(chan, page, perf_fetched_se_cnt_lo, lo_hi_readq);
+}
+static struct dma_chan_sysfs_entry se_count_attr = __ATTR_RO(se_count);
+
+static ssize_t byte_count_show(struct dma_chan *chan, char *page)
+{
+	return pmon_show(chan, page, perf_byte_cnt_lo, lo_hi_readq);
+}
+static struct dma_chan_sysfs_entry byte_count_attr = __ATTR_RO(byte_count);
+
+static ssize_t se_pending_show(struct dma_chan *chan, char *page)
+{
+	return pmon_show(chan, page, perf_se_pending, readw);
+}
+static struct dma_chan_sysfs_entry se_pending_attr = __ATTR_RO(se_pending);
+
+static ssize_t se_buf_empty_show(struct dma_chan *chan, char *page)
+{
+	return pmon_show(chan, page, perf_se_buf_empty, readw);
+}
+static struct dma_chan_sysfs_entry se_buf_empty_attr = __ATTR_RO(se_buf_empty);
+
+static ssize_t chan_idle_show(struct dma_chan *chan, char *page)
+{
+	return pmon_show(chan, page, perf_chan_idle, readl);
+}
+static struct dma_chan_sysfs_entry chan_idle_attr = __ATTR_RO(chan_idle);
+
+static ssize_t latency_max_show(struct dma_chan *chan, char *page)
+{
+	return pmon_show(chan, page, perf_lat_max, readl);
+}
+static struct dma_chan_sysfs_entry latency_max_attr = __ATTR_RO(latency_max);
+
+static ssize_t latency_min_show(struct dma_chan *chan, char *page)
+{
+	return pmon_show(chan, page, perf_lat_min, readl);
+}
+static struct dma_chan_sysfs_entry latency_min_attr = __ATTR_RO(latency_min);
+
+static ssize_t latency_last_show(struct dma_chan *chan, char *page)
+{
+	return pmon_show(chan, page, perf_lat_last, readl);
+}
+static struct dma_chan_sysfs_entry latency_last_attr = __ATTR_RO(latency_last);
+
+static ssize_t latency_selector_show(struct dma_chan *chan, char *page)
+{
+	struct switchtec_dma_chan *swdma_chan =
+		container_of(chan, struct switchtec_dma_chan, dma_chan);
+	struct chan_fw_regs __iomem *chan_fw = swdma_chan->mmio_chan_fw;
+	u32 lat = 0;
+
+	rcu_read_lock();
+	if (!rcu_dereference(swdma_chan->swdma_dev->pdev)) {
+		rcu_read_unlock();
+		return -ENODEV;
+	}
+
+	lat = readl(&chan_fw->perf_latency_selector);
+	rcu_read_unlock();
+
+	strcat(page, "To select a latency type, write the type number (1 ~ 5) to latency_selector\n\n");
+
+	strcat(page, "Latency Types:\n");
+	strcat(page, "(1) SE Fetch latency");
+	if (lat & SWITCHTEC_LAT_SE_FETCH)
+		strcat(page, " (*)\n");
+	else
+		strcat(page, "\n");
+
+	strcat(page, "(2) VDM latency");
+	if (lat & SWITCHTEC_LAT_VDM)
+		strcat(page, " (*)\n");
+	else
+		strcat(page, "\n");
+
+	strcat(page, "(3) Read Immediate latency");
+	if (lat & SWITCHTEC_LAT_RD_IMM)
+		strcat(page, " (*)\n");
+	else
+		strcat(page, "\n");
+
+	strcat(page, "(4) SE Processing latency");
+	if (lat & SWITCHTEC_LAT_SE_PROCESS)
+		strcat(page, " (*)\n");
+	else
+		strcat(page, "\n");
+
+	strcat(page, "(5) FW NP TLP latency");
+	if (lat & SWITCHTEC_LAT_FW_NP)
+		strcat(page, " (*)\n");
+	else
+		strcat(page, "\n");
+
+	strcat(page, "\n");
+
+	return strlen(page);
+}
+
+static ssize_t latency_selector_store(struct dma_chan *chan, const char *page,
+				      size_t count)
+{
+	struct switchtec_dma_chan *swdma_chan =
+		container_of(chan, struct switchtec_dma_chan, dma_chan);
+	struct chan_fw_regs __iomem *chan_fw = swdma_chan->mmio_chan_fw;
+	ssize_t ret = count;
+	int lat_type;
+
+	if (kstrtoint(page, 0, &lat_type) < 0)
+		return -EINVAL;
+
+	switch (lat_type) {
+	case 1:
+		lat_type = SWITCHTEC_LAT_SE_FETCH;
+		break;
+	case 2:
+		lat_type = SWITCHTEC_LAT_VDM;
+		break;
+	case 3:
+		lat_type = SWITCHTEC_LAT_RD_IMM;
+		break;
+	case 4:
+		lat_type = SWITCHTEC_LAT_SE_PROCESS;
+		break;
+	case 5:
+		lat_type = SWITCHTEC_LAT_FW_NP;
+		break;
+	default:
+		return -EINVAL;
+	};
+
+	rcu_read_lock();
+	if (!rcu_dereference(swdma_chan->swdma_dev->pdev)) {
+		ret = -ENODEV;
+		goto err_unlock;
+	}
+
+	if (chan->client_count) {
+		ret = -EBUSY;
+		goto err_unlock;
+	}
+
+	writel(lat_type, &chan_fw->perf_latency_selector);
+
+err_unlock:
+	rcu_read_unlock();
+
+	return ret;
+}
+static struct dma_chan_sysfs_entry
+latency_selector_attr = __ATTR_RW(latency_selector);
+
+static struct attribute *switchtec_pmon_attrs[] = {
+	&se_count_attr.attr,
+	&byte_count_attr.attr,
+	&se_pending_attr.attr,
+	&se_buf_empty_attr.attr,
+	&chan_idle_attr.attr,
+	&latency_max_attr.attr,
+	&latency_min_attr.attr,
+	&latency_last_attr.attr,
+	&latency_selector_attr.attr,
+	NULL,
+};
+
+static struct attribute_group switchtec_pmon_group = {
+	.name = "pmon",
+	.attrs = switchtec_pmon_attrs,
+};
+
 static const struct attribute_group *switchtec_groups[] = {
 	&switchtec_config_group,
+	&switchtec_pmon_group,
 	NULL,
 };
 
-- 
2.47.3

Re: [PATCH v1 4/5] dmaengine: switchtec-dma: Add pmon sysfs attributes

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing `.release` callback in `switchtec_ktype` causes Use-After-Free of channel memory when sysfs files are held open during device removal.
- [Low] The `latency_selector_show` function violates sysfs rules by using `strcat()` to build a multi-line menu instead of using `sysfs_emit()`.
--

--- Patch [4]: [PATCH v1 4/5] dmaengine: switchtec-dma: Add pmon sysfs attributes ---
commit 9353b00f22d3df92fe9cfb71beccb84977c35e68
Author: Logan Gunthorpe <logang@deltatee.com>

dmaengine: switchtec-dma: Add pmon sysfs attributes

Switchtec hardware exposes some performance monitor registers which
can be used to monitor various statistics of the hardware. Expose
these as a sysfs interface under the switchtec group in a pmon
sub-group.

> diff --git a/drivers/dma/switchtec_dma.c b/drivers/dma/switchtec_dma.c
> index 4841134bd7b83..2ac43eb58995a 100644
> --- a/drivers/dma/switchtec_dma.c
> +++ b/drivers/dma/switchtec_dma.c

[ ... ]

> @@ -1156,8 +1163,200 @@ static struct attribute_group switchtec_config_group = {
>  	.attrs = switchtec_config_attrs,
>  };
>  
> +#define pmon_show(chan, page, field, reader) ({				     \
> +	struct switchtec_dma_chan *__swdma_chan =			     \
> +		container_of((chan), struct switchtec_dma_chan, dma_chan);   \

[ ... ]

> +static ssize_t latency_selector_show(struct dma_chan *chan, char *page)
> +{
> +	struct switchtec_dma_chan *swdma_chan =
> +		container_of(chan, struct switchtec_dma_chan, dma_chan);
> +	struct chan_fw_regs __iomem *chan_fw = swdma_chan->mmio_chan_fw;
> +	u32 lat = 0;
> +
> +	rcu_read_lock();
> +	if (!rcu_dereference(swdma_chan->swdma_dev->pdev)) {
> +		rcu_read_unlock();
> +		return -ENODEV;
> +	}
> +
> +	lat = readl(&chan_fw->perf_latency_selector);
> +	rcu_read_unlock();
> +
> +	strcat(page, "To select a latency type, write the type number (1 ~ 5) to latency_selector\n\n");
> +
> +	strcat(page, "Latency Types:\n");
> +	strcat(page, "(1) SE Fetch latency");

[Severity: Low]
Should this code use sysfs_emit() instead of strcat() to construct the menu on
the raw page buffer? Sysfs documentation requires using sysfs_emit() and
generally expects a single value per file rather than a multi-line string.

> +	if (lat & SWITCHTEC_LAT_SE_FETCH)
> +		strcat(page, " (*)\n");
> +	else
> +		strcat(page, "\n");

[ ... ]

>  static const struct attribute_group *switchtec_groups[] = {
>  	&switchtec_config_group,
> +	&switchtec_pmon_group,
>  	NULL,
>  };

[Severity: High]
Does the driver's kobj_type (switchtec_ktype) lack a .release callback?

If a userspace process holds a channel's sysfs file open, keeping the
embedded kobject refcount elevated, and the device is unregistered,
switchtec_dma_release() unconditionally frees the channel memory.

When the file is closed later and the refcount drops to zero, the kernel
would access the already-freed kobject to invoke the missing release
function, leading to a Use-After-Free.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707162045.23910-1-logang@deltatee.com?part=4

[PATCH v1 5/5] dmaengine: switchtec-dma: Add PCI1008 device ID

[Logan Gunthorpe]

Add the missing PC1008 device ID for switchtec-dma.

Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
---
 drivers/dma/switchtec_dma.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/dma/switchtec_dma.c b/drivers/dma/switchtec_dma.c
index 2ac43eb58995..0a268b6a3ccc 100644
--- a/drivers/dma/switchtec_dma.c
+++ b/drivers/dma/switchtec_dma.c
@@ -1764,6 +1764,7 @@ static const struct pci_device_id switchtec_dma_pci_tbl[] = {
 	SW_ID(PCI_VENDOR_ID_EFAR,      0x1004), /* PCI1004 16XG4 */
 	SW_ID(PCI_VENDOR_ID_EFAR,      0x1005), /* PCI1005 16XG4 */
 	SW_ID(PCI_VENDOR_ID_EFAR,      0x1006), /* PCI1006 16XG4 */
+	SW_ID(PCI_VENDOR_ID_EFAR,      0x1008), /* PCI1008 16XG4 */
 	{0}
 };
 MODULE_DEVICE_TABLE(pci, switchtec_dma_pci_tbl);
-- 
2.47.3

Re: [PATCH v1 5/5] dmaengine: switchtec-dma: Add PCI1008 device ID

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The new PCI1008 device ID is added to the switchtec-dma driver but is missing from the main switchtec management driver.
--

--- Patch [5]: [PATCH v1 5/5] dmaengine: switchtec-dma: Add PCI1008 device ID ---
commit 2cb21123b320b7cdd1842ad29a28e1934555c74a
Author: Logan Gunthorpe <logang@deltatee.com>

dmaengine: switchtec-dma: Add PCI1008 device ID

Add the missing PC1008 device ID for switchtec-dma.

> diff --git a/drivers/dma/switchtec_dma.c b/drivers/dma/switchtec_dma.c
> index 2ac43eb58995a..0a268b6a3ccc0 100644
> --- a/drivers/dma/switchtec_dma.c
> +++ b/drivers/dma/switchtec_dma.c
> @@ -1764,6 +1764,7 @@ static const struct pci_device_id switchtec_dma_pci_tbl[] = {
>  	SW_ID(PCI_VENDOR_ID_EFAR,      0x1004), /* PCI1004 16XG4 */
>  	SW_ID(PCI_VENDOR_ID_EFAR,      0x1005), /* PCI1005 16XG4 */
>  	SW_ID(PCI_VENDOR_ID_EFAR,      0x1006), /* PCI1006 16XG4 */
> +	SW_ID(PCI_VENDOR_ID_EFAR,      0x1008), /* PCI1008 16XG4 */

[Severity: Medium]
Should the PCI1008 device ID also be added to the core switchtec management
driver's PCI ID table?

Without adding it there, it appears the core management endpoint won't be
bound by the switchtec driver, potentially preventing userspace tools from
configuring or monitoring the switch and leaving NTB functionality
unavailable.

For example, other PCI100X variants are included in switchtec_pci_tbl:

drivers/pci/switch/switchtec.c:switchtec_pci_tbl[] = {
	...
	SWITCHTEC_PCI100X_DEVICE(0x1005, SWITCHTEC_GEN4),  /* PCI1005 16XG4 */
	SWITCHTEC_PCI100X_DEVICE(0x1006, SWITCHTEC_GEN4),  /* PCI1006 16XG4 */
	...
};

>  	{0}
>  };
>  MODULE_DEVICE_TABLE(pci, switchtec_dma_pci_tbl);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707162045.23910-1-logang@deltatee.com?part=5