Re: [PATCH 1/2] dmaengine: idxd: Expose DSA3.0 capabilities through sysfs

[Fenghua Yu <fenghuay@nvidia.com>]

Hi, Yi,

On 6/13/25 09:18, Yi Sun wrote:
> Introduce sysfs interfaces for 3 new Data Streaming Accelerator (DSA)
> capability registers (dsacap0-2) to enable userspace awareness of hardware
> features in DSA version 3 and later devices.
>
> Userspace components (e.g. configure libraries, workload Apps) require this
> information to:
> 1. Select optimal data transfer strategies based on SGL capabilities
> 2. Enable hardware-specific optimizations for floating-point operations
> 3. Configure memory operations with proper numerical handling
> 4. Verify compute operation compatibility before submitting jobs
>
> The output consists of values from the three dsacap registers, concatenated
> in order and separated by commas.
>
> Example:
> cat /sys/bus/dsa/devices/dsa0/dsacap
>   0014000e000007aa,00fa01ff01ff03ff,000000000000f18d
>
> Signed-off-by: Yi Sun <yi.sun@intel.com>
> Co-developed-by: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
> Signed-off-by: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
>
> diff --git a/Documentation/ABI/stable/sysfs-driver-dma-idxd b/Documentation/ABI/stable/sysfs-driver-dma-idxd
> index 4a355e6747ae..f9568ea52b2f 100644
> --- a/Documentation/ABI/stable/sysfs-driver-dma-idxd
> +++ b/Documentation/ABI/stable/sysfs-driver-dma-idxd
> @@ -136,6 +136,21 @@ Description:	The last executed device administrative command's status/error.
>   		Also last configuration error overloaded.
>   		Writing to it will clear the status.
>   
> +What:		/sys/bus/dsa/devices/dsa<m>/dsacap

Since 3 dsa caps are shown together, it's better to change this ABI name 
to "dsacaps"?

> +Date:		June 1, 2025
> +KernelVersion:	6.17.0
> +Contact:	dmaengine@vger.kernel.org
> +Description:	The DSA3 specification introduces three new capability
> +		registers: dsacap[0-2]. User components (e.g., configuration
> +		libraries and workload applications) require this information
> +		to properly utilize the DSA3 features.
> +		This includes SGL capability support, Enabling hardware-specific
> +		optimizations, Configuring memory, etc.
> +		The output consists of values from the three dsacap registers,
> +		concatenated in order and separated by commas.
> +		This attribute should only be visible on DSA devices of version
> +		3 or later.
> +

It's better to document the "order" of the output of the caps. So apps 
can parse the caps. Something like:

"The output format is <dsacap2>,<dsacap1>,<dsacap0> where each DSA cap 
value is a 64 bit hex value."

>   What:		/sys/bus/dsa/devices/dsa<m>/iaa_cap
>   Date:		Sept 14, 2022
>   KernelVersion: 6.0.0
> diff --git a/drivers/dma/idxd/idxd.h b/drivers/dma/idxd/idxd.h
> index 74e6695881e6..cc0a3fe1c957 100644
> --- a/drivers/dma/idxd/idxd.h
> +++ b/drivers/dma/idxd/idxd.h
> @@ -252,6 +252,9 @@ struct idxd_hw {
>   	struct opcap opcap;
>   	u32 cmd_cap;
>   	union iaa_cap_reg iaa_cap;
> +	union dsacap0_reg dsacap0;
> +	union dsacap1_reg dsacap1;
> +	union dsacap2_reg dsacap2;
>   };
>   
>   enum idxd_device_state {
> diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
> index 80355d03004d..cc8203320d40 100644
> --- a/drivers/dma/idxd/init.c
> +++ b/drivers/dma/idxd/init.c
> @@ -582,6 +582,10 @@ static void idxd_read_caps(struct idxd_device *idxd)
>   	}
>   	multi_u64_to_bmap(idxd->opcap_bmap, &idxd->hw.opcap.bits[0], 4);
>   
> +	idxd->hw.dsacap0.bits = ioread64(idxd->reg_base + IDXD_DSACAP0_OFFSET);
> +	idxd->hw.dsacap1.bits = ioread64(idxd->reg_base + IDXD_DSACAP1_OFFSET);
> +	idxd->hw.dsacap2.bits = ioread64(idxd->reg_base + IDXD_DSACAP2_OFFSET);
> +
>   	/* read iaa cap */
>   	if (idxd->data->type == IDXD_TYPE_IAX && idxd->hw.version >= DEVICE_VERSION_2)
>   		idxd->hw.iaa_cap.bits = ioread64(idxd->reg_base + IDXD_IAACAP_OFFSET);
> diff --git a/drivers/dma/idxd/registers.h b/drivers/dma/idxd/registers.h
> index 006ba206ab1b..45485ecd7bb6 100644
> --- a/drivers/dma/idxd/registers.h
> +++ b/drivers/dma/idxd/registers.h
> @@ -13,6 +13,7 @@
>   
>   #define DEVICE_VERSION_1		0x100
>   #define DEVICE_VERSION_2		0x200
> +#define DEVICE_VERSION_3		0x300
>   
>   #define IDXD_MMIO_BAR		0
>   #define IDXD_WQ_BAR		2
> @@ -582,6 +583,21 @@ union evl_status_reg {
>   	u64 bits;
>   } __packed;
>   
> +#define IDXD_DSACAP0_OFFSET		0x180
> +union dsacap0_reg {
> +	u64 bits;
> +};
I forgot the format of dsacap. Is there any field in each dsa cap 
register? If yes, better to add a structure inside to describe the fields.
> +
> +#define IDXD_DSACAP1_OFFSET		0x188
> +union dsacap1_reg {
> +	u64 bits;
> +};
> +
> +#define IDXD_DSACAP2_OFFSET		0x190
> +union dsacap2_reg {
> +	u64 bits;
> +};
> +
>   #define IDXD_MAX_BATCH_IDENT	256
>   
>   struct __evl_entry {
> diff --git a/drivers/dma/idxd/sysfs.c b/drivers/dma/idxd/sysfs.c
> index 9f0701021af0..624b7d1b193f 100644
> --- a/drivers/dma/idxd/sysfs.c
> +++ b/drivers/dma/idxd/sysfs.c
> @@ -1713,6 +1713,21 @@ static ssize_t event_log_size_store(struct device *dev,
>   }
>   static DEVICE_ATTR_RW(event_log_size);
>   
> +static ssize_t dsacap_show(struct device *dev,
> +			   struct device_attribute *attr, char *buf)
> +{
> +	struct idxd_device *idxd = confdev_to_idxd(dev);
> +
> +	return sysfs_emit(buf, "%08x,%08x,%08x,%08x,%08x,%08x\n",
> +			  upper_32_bits(idxd->hw.dsacap0.bits),
> +			  lower_32_bits(idxd->hw.dsacap0.bits),
> +			  upper_32_bits(idxd->hw.dsacap1.bits),
> +			  lower_32_bits(idxd->hw.dsacap1.bits),
> +			  upper_32_bits(idxd->hw.dsacap2.bits),
> +			  lower_32_bits(idxd->hw.dsacap2.bits));

The output format of this sysfs_emit() doesn't match the format in your 
earlier example:

cat /sys/bus/dsa/devices/dsa0/dsacap
  0014000e000007aa,00fa01ff01ff03ff,000000000000f18d


And this sysfs_emit() is too complex and can be simplified as well.

So it might be changed to this?

+	return sysfs_emit(buf, "%016llx,%016llx,%016llx\n",
+			  (u64)idxd->hw.dsacap0.bits,
+			  (u64)idxd->hw.dsacap1.bits,
+			  (u64)idxd->hw.dsacap2.bits);

> +}
> +static DEVICE_ATTR_RO(dsacap);

Since 3 dsa caps are shown together, do you need to change the ABI name 
to "dsacaps" instead of "dsacap"?


> +
>   static bool idxd_device_attr_max_batch_size_invisible(struct attribute *attr,
>   						      struct idxd_device *idxd)
>   {
> @@ -1750,6 +1765,14 @@ static bool idxd_device_attr_event_log_size_invisible(struct attribute *attr,
>   		!idxd->hw.gen_cap.evl_support);
>   }
>   
> +static bool idxd_device_attr_dsacap_invisible(struct attribute *attr,
> +					      struct idxd_device *idxd)
> +{
> +	return attr == &dev_attr_dsacap.attr &&
> +		(idxd->data->type != IDXD_TYPE_DSA ||
> +		idxd->hw.version < DEVICE_VERSION_3);
> +}
> +
>   static umode_t idxd_device_attr_visible(struct kobject *kobj,
>   					struct attribute *attr, int n)
>   {
> @@ -1768,6 +1791,9 @@ static umode_t idxd_device_attr_visible(struct kobject *kobj,
>   	if (idxd_device_attr_event_log_size_invisible(attr, idxd))
>   		return 0;
>   
> +	if (idxd_device_attr_dsacap_invisible(attr, idxd))
> +		return 0;
> +
>   	return attr->mode;
>   }
>   
> @@ -1795,6 +1821,7 @@ static struct attribute *idxd_device_attributes[] = {
>   	&dev_attr_cmd_status.attr,
>   	&dev_attr_iaa_cap.attr,
>   	&dev_attr_event_log_size.attr,
> +	&dev_attr_dsacap.attr,
>   	NULL,
>   };
>   

Thanks.

-Fenghua