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

["Sun, Yi" <yi.sun@intel.com>]

On 16.06.2025 11:08, Fenghua Yu wrote:
>Hi, Philip,
>
>On 6/13/25 15:26, Lantz, Philip wrote:
>>
>>Fenghua wrote:
>>
>>>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
>>>>+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.
>>>>+
>>>>   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);
>>>>+
>>>The dsacaps are invalid for DSA 1 and 2. Not safe to read and assign the
>>>bits on DSA 1 and 2.
>>>
>>>Better to assign the dsacap bits only when idxd.hw.version >= DSA_VERSION_3.
>>The registers are architecturally guaranteed to return 0 on prior versions, so it is
>>safe to read them on DSA 1 and 2 and there is no need for an additional check.
>
>Although it's safe to read them here on DSA 1 and 2, reading a 
>reserved value generally is not a good code practice in the kernel. I 
>would still suggest to avoid to read the reserved values on DSA 1 and 
>2.

My previous understanding was that ioread64() would ensure safe behavior
on DSA1.0 and DSA2.0. However, I'm fine with Fenghua's suggestion adding
the condition version >= DSA_VERSION_3. It can provide future-proofing
in case the behavior of ioread64() changes.

Thanks
    --Sun, Yi