dmaengine: idxd: Narrow the restriction on BATCH to ver. 1 only

[PATCH v1] dmaengine: idxd: Narrow the restriction on BATCH to ver. 1 only

Posted by Vinicius Costa Gomes 9 months, 1 week ago

Allow BATCH operations to be submitted and the capability to be
exposed for DSA version 2 (or later) devices.

DSA version 2 devices allow safe submission of BATCH operations.

Signed-off-by: Anil Keshavamurthy <anil.s.keshavamurthy@intel.com>
Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
---
 drivers/dma/idxd/cdev.c  | 6 ++++--
 drivers/dma/idxd/sysfs.c | 6 ++++--
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/idxd/cdev.c b/drivers/dma/idxd/cdev.c
index ff94ee892339..6a1dc15ee485 100644
--- a/drivers/dma/idxd/cdev.c
+++ b/drivers/dma/idxd/cdev.c
@@ -439,10 +439,12 @@ static int idxd_submit_user_descriptor(struct idxd_user_context *ctx,
 	 * DSA devices are capable of indirect ("batch") command submission.
 	 * On devices where direct user submissions are not safe, we cannot
 	 * allow this since there is no good way for us to verify these
-	 * indirect commands.
+	 * indirect commands. Narrow the restriction of operations with the
+	 * BATCH opcode to only DSA version 1 devices.
 	 */
 	if (is_dsa_dev(idxd_dev) && descriptor.opcode == DSA_OPCODE_BATCH &&
-		!wq->idxd->user_submission_safe)
+	    wq->idxd->hw.version == DEVICE_VERSION_1 &&
+	    !wq->idxd->user_submission_safe)
 		return -EINVAL;
 	/*
 	 * As per the programming specification, the completion address must be
diff --git a/drivers/dma/idxd/sysfs.c b/drivers/dma/idxd/sysfs.c
index 6af493f6ba77..9f0701021af0 100644
--- a/drivers/dma/idxd/sysfs.c
+++ b/drivers/dma/idxd/sysfs.c
@@ -1208,9 +1208,11 @@ static ssize_t op_cap_show_common(struct device *dev, char *buf, unsigned long *
 
 		/* On systems where direct user submissions are not safe, we need to clear out
 		 * the BATCH capability from the capability mask in sysfs since we cannot support
-		 * that command on such systems.
+		 * that command on such systems. Narrow the restriction of operations with the
+		 * BATCH opcode to only DSA version 1 devices.
 		 */
-		if (i == DSA_OPCODE_BATCH/64 && !confdev_to_idxd(dev)->user_submission_safe)
+		if (i == DSA_OPCODE_BATCH/64 && !confdev_to_idxd(dev)->user_submission_safe &&
+		    confdev_to_idxd(dev)->hw.version == DEVICE_VERSION_1)
 			clear_bit(DSA_OPCODE_BATCH % 64, &val);
 
 		pos += sysfs_emit_at(buf, pos, "%*pb", 64, &val);
-- 
2.48.1

Re: [PATCH v1] dmaengine: idxd: Narrow the restriction on BATCH to ver. 1 only

Posted by Vinod Koul 7 months, 1 week ago

On Wed, 12 Mar 2025 15:15:10 -0700, Vinicius Costa Gomes wrote:
> Allow BATCH operations to be submitted and the capability to be
> exposed for DSA version 2 (or later) devices.
> 
> DSA version 2 devices allow safe submission of BATCH operations.
> 
> 

Applied, thanks!

[1/1] dmaengine: idxd: Narrow the restriction on BATCH to ver. 1 only
      commit: 31f04b537152675adca2f97582660e3f178dcfff

Best regards,
-- 
~Vinod

Re: [PATCH v1] dmaengine: idxd: Narrow the restriction on BATCH to ver. 1 only

Posted by Dave Jiang 7 months, 3 weeks ago


On 3/12/25 3:15 PM, Vinicius Costa Gomes wrote:
> Allow BATCH operations to be submitted and the capability to be
> exposed for DSA version 2 (or later) devices.
> 
> DSA version 2 devices allow safe submission of BATCH operations.
> 
> Signed-off-by: Anil Keshavamurthy <anil.s.keshavamurthy@intel.com>
> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>

Hi Vinod, is this patch ok to merge? Thanks!

> ---
>  drivers/dma/idxd/cdev.c  | 6 ++++--
>  drivers/dma/idxd/sysfs.c | 6 ++++--
>  2 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/dma/idxd/cdev.c b/drivers/dma/idxd/cdev.c
> index ff94ee892339..6a1dc15ee485 100644
> --- a/drivers/dma/idxd/cdev.c
> +++ b/drivers/dma/idxd/cdev.c
> @@ -439,10 +439,12 @@ static int idxd_submit_user_descriptor(struct idxd_user_context *ctx,
>  	 * DSA devices are capable of indirect ("batch") command submission.
>  	 * On devices where direct user submissions are not safe, we cannot
>  	 * allow this since there is no good way for us to verify these
> -	 * indirect commands.
> +	 * indirect commands. Narrow the restriction of operations with the
> +	 * BATCH opcode to only DSA version 1 devices.
>  	 */
>  	if (is_dsa_dev(idxd_dev) && descriptor.opcode == DSA_OPCODE_BATCH &&
> -		!wq->idxd->user_submission_safe)
> +	    wq->idxd->hw.version == DEVICE_VERSION_1 &&
> +	    !wq->idxd->user_submission_safe)
>  		return -EINVAL;
>  	/*
>  	 * As per the programming specification, the completion address must be
> diff --git a/drivers/dma/idxd/sysfs.c b/drivers/dma/idxd/sysfs.c
> index 6af493f6ba77..9f0701021af0 100644
> --- a/drivers/dma/idxd/sysfs.c
> +++ b/drivers/dma/idxd/sysfs.c
> @@ -1208,9 +1208,11 @@ static ssize_t op_cap_show_common(struct device *dev, char *buf, unsigned long *
>  
>  		/* On systems where direct user submissions are not safe, we need to clear out
>  		 * the BATCH capability from the capability mask in sysfs since we cannot support
> -		 * that command on such systems.
> +		 * that command on such systems. Narrow the restriction of operations with the
> +		 * BATCH opcode to only DSA version 1 devices.
>  		 */
> -		if (i == DSA_OPCODE_BATCH/64 && !confdev_to_idxd(dev)->user_submission_safe)
> +		if (i == DSA_OPCODE_BATCH/64 && !confdev_to_idxd(dev)->user_submission_safe &&
> +		    confdev_to_idxd(dev)->hw.version == DEVICE_VERSION_1)
>  			clear_bit(DSA_OPCODE_BATCH % 64, &val);
>  
>  		pos += sysfs_emit_at(buf, pos, "%*pb", 64, &val);

Re: [PATCH v1] dmaengine: idxd: Narrow the restriction on BATCH to ver. 1 only

Posted by Fenghua Yu 8 months, 2 weeks ago

Hi, Vinicius,

On 3/12/25 15:15, Vinicius Costa Gomes wrote:
> Allow BATCH operations to be submitted and the capability to be
> exposed for DSA version 2 (or later) devices.
>
> DSA version 2 devices allow safe submission of BATCH operations.
>
> Signed-off-by: Anil Keshavamurthy <anil.s.keshavamurthy@intel.com>
> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
> ---
>   drivers/dma/idxd/cdev.c  | 6 ++++--
>   drivers/dma/idxd/sysfs.c | 6 ++++--
>   2 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/dma/idxd/cdev.c b/drivers/dma/idxd/cdev.c
> index ff94ee892339..6a1dc15ee485 100644
> --- a/drivers/dma/idxd/cdev.c
> +++ b/drivers/dma/idxd/cdev.c
> @@ -439,10 +439,12 @@ static int idxd_submit_user_descriptor(struct idxd_user_context *ctx,
>   	 * DSA devices are capable of indirect ("batch") command submission.
>   	 * On devices where direct user submissions are not safe, we cannot
>   	 * allow this since there is no good way for us to verify these
> -	 * indirect commands.
> +	 * indirect commands. Narrow the restriction of operations with the
> +	 * BATCH opcode to only DSA version 1 devices.
>   	 */
>   	if (is_dsa_dev(idxd_dev) && descriptor.opcode == DSA_OPCODE_BATCH &&
> -		!wq->idxd->user_submission_safe)
> +	    wq->idxd->hw.version == DEVICE_VERSION_1 &&
> +	    !wq->idxd->user_submission_safe)
>   		return -EINVAL;
>   	/*
>   	 * As per the programming specification, the completion address must be
> diff --git a/drivers/dma/idxd/sysfs.c b/drivers/dma/idxd/sysfs.c
> index 6af493f6ba77..9f0701021af0 100644
> --- a/drivers/dma/idxd/sysfs.c
> +++ b/drivers/dma/idxd/sysfs.c
> @@ -1208,9 +1208,11 @@ static ssize_t op_cap_show_common(struct device *dev, char *buf, unsigned long *
>   
>   		/* On systems where direct user submissions are not safe, we need to clear out
>   		 * the BATCH capability from the capability mask in sysfs since we cannot support
> -		 * that command on such systems.
> +		 * that command on such systems. Narrow the restriction of operations with the
> +		 * BATCH opcode to only DSA version 1 devices.
>   		 */
> -		if (i == DSA_OPCODE_BATCH/64 && !confdev_to_idxd(dev)->user_submission_safe)
> +		if (i == DSA_OPCODE_BATCH/64 && !confdev_to_idxd(dev)->user_submission_safe &&
> +		    confdev_to_idxd(dev)->hw.version == DEVICE_VERSION_1)
>   			clear_bit(DSA_OPCODE_BATCH % 64, &val);
>   
>   		pos += sysfs_emit_at(buf, pos, "%*pb", 64, &val)

Maybe folder the DEVICE_VERSION_1 check into user_submission_safe variable?

This way patch is a bit smaller, a bit faster in run-time,  and easier 
to be extend in case there are other restriction changes in the future?

diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
index 86075cdc4420..80f95cb815c8 100644
--- a/drivers/dma/idxd/init.c
+++ b/drivers/dma/idxd/init.c
@@ -1258,7 +1258,8 @@ int idxd_pci_probe_alloc(struct idxd_device *idxd, 
struct pci_dev *pdev,
                  idxd->hw.version);

         if (data)
-               idxd->user_submission_safe = data->user_submission_safe;
+               idxd->user_submission_safe = data->user_submission_safe |
+                                            (idxd->hw.version != 
DEVICE_VERSION_1);

Thanks.


-Fenghua

Re: [PATCH v1] dmaengine: idxd: Narrow the restriction on BATCH to ver. 1 only

Posted by Vinicius Costa Gomes 8 months, 2 weeks ago

Hi Fenghua,

Fenghua Yu <fenghuay@nvidia.com> writes:

> Hi, Vinicius,
>
> On 3/12/25 15:15, Vinicius Costa Gomes wrote:
>> Allow BATCH operations to be submitted and the capability to be
>> exposed for DSA version 2 (or later) devices.
>>
>> DSA version 2 devices allow safe submission of BATCH operations.
>>
>> Signed-off-by: Anil Keshavamurthy <anil.s.keshavamurthy@intel.com>
>> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
>> ---
>>   drivers/dma/idxd/cdev.c  | 6 ++++--
>>   drivers/dma/idxd/sysfs.c | 6 ++++--
>>   2 files changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/dma/idxd/cdev.c b/drivers/dma/idxd/cdev.c
>> index ff94ee892339..6a1dc15ee485 100644
>> --- a/drivers/dma/idxd/cdev.c
>> +++ b/drivers/dma/idxd/cdev.c
>> @@ -439,10 +439,12 @@ static int idxd_submit_user_descriptor(struct idxd_user_context *ctx,
>>   	 * DSA devices are capable of indirect ("batch") command submission.
>>   	 * On devices where direct user submissions are not safe, we cannot
>>   	 * allow this since there is no good way for us to verify these
>> -	 * indirect commands.
>> +	 * indirect commands. Narrow the restriction of operations with the
>> +	 * BATCH opcode to only DSA version 1 devices.
>>   	 */
>>   	if (is_dsa_dev(idxd_dev) && descriptor.opcode == DSA_OPCODE_BATCH &&
>> -		!wq->idxd->user_submission_safe)
>> +	    wq->idxd->hw.version == DEVICE_VERSION_1 &&
>> +	    !wq->idxd->user_submission_safe)
>>   		return -EINVAL;
>>   	/*
>>   	 * As per the programming specification, the completion address must be
>> diff --git a/drivers/dma/idxd/sysfs.c b/drivers/dma/idxd/sysfs.c
>> index 6af493f6ba77..9f0701021af0 100644
>> --- a/drivers/dma/idxd/sysfs.c
>> +++ b/drivers/dma/idxd/sysfs.c
>> @@ -1208,9 +1208,11 @@ static ssize_t op_cap_show_common(struct device *dev, char *buf, unsigned long *
>>   
>>   		/* On systems where direct user submissions are not safe, we need to clear out
>>   		 * the BATCH capability from the capability mask in sysfs since we cannot support
>> -		 * that command on such systems.
>> +		 * that command on such systems. Narrow the restriction of operations with the
>> +		 * BATCH opcode to only DSA version 1 devices.
>>   		 */
>> -		if (i == DSA_OPCODE_BATCH/64 && !confdev_to_idxd(dev)->user_submission_safe)
>> +		if (i == DSA_OPCODE_BATCH/64 && !confdev_to_idxd(dev)->user_submission_safe &&
>> +		    confdev_to_idxd(dev)->hw.version == DEVICE_VERSION_1)
>>   			clear_bit(DSA_OPCODE_BATCH % 64, &val);
>>   
>>   		pos += sysfs_emit_at(buf, pos, "%*pb", 64, &val)
>
> Maybe folder the DEVICE_VERSION_1 check into user_submission_safe variable?
>
> This way patch is a bit smaller, a bit faster in run-time,  and easier 
> to be extend in case there are other restriction changes in the future?
>
> diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
> index 86075cdc4420..80f95cb815c8 100644
> --- a/drivers/dma/idxd/init.c
> +++ b/drivers/dma/idxd/init.c
> @@ -1258,7 +1258,8 @@ int idxd_pci_probe_alloc(struct idxd_device *idxd, 
> struct pci_dev *pdev,
>                   idxd->hw.version);
>
>          if (data)
> -               idxd->user_submission_safe = data->user_submission_safe;
> +               idxd->user_submission_safe = data->user_submission_safe |
> +                                            (idxd->hw.version != 
> DEVICE_VERSION_1);
>

I don't think so, this would lift the restriction that we have on mmap()
for regular userspace applications.

The reality is that the "only" change is that the submission of
operations with the BATCH opcode is now allowed for regular applications
(via write()) with V2 devices, mmap() should still be restricted.


Cheers,
-- 
Vinicius

Re: [PATCH v1] dmaengine: idxd: Narrow the restriction on BATCH to ver. 1 only

Posted by Dave Jiang 9 months, 1 week ago


On 3/12/25 3:15 PM, Vinicius Costa Gomes wrote:
> Allow BATCH operations to be submitted and the capability to be
> exposed for DSA version 2 (or later) devices.
> 
> DSA version 2 devices allow safe submission of BATCH operations.
> 
> Signed-off-by: Anil Keshavamurthy <anil.s.keshavamurthy@intel.com>
> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  drivers/dma/idxd/cdev.c  | 6 ++++--
>  drivers/dma/idxd/sysfs.c | 6 ++++--
>  2 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/dma/idxd/cdev.c b/drivers/dma/idxd/cdev.c
> index ff94ee892339..6a1dc15ee485 100644
> --- a/drivers/dma/idxd/cdev.c
> +++ b/drivers/dma/idxd/cdev.c
> @@ -439,10 +439,12 @@ static int idxd_submit_user_descriptor(struct idxd_user_context *ctx,
>  	 * DSA devices are capable of indirect ("batch") command submission.
>  	 * On devices where direct user submissions are not safe, we cannot
>  	 * allow this since there is no good way for us to verify these
> -	 * indirect commands.
> +	 * indirect commands. Narrow the restriction of operations with the
> +	 * BATCH opcode to only DSA version 1 devices.
>  	 */
>  	if (is_dsa_dev(idxd_dev) && descriptor.opcode == DSA_OPCODE_BATCH &&
> -		!wq->idxd->user_submission_safe)
> +	    wq->idxd->hw.version == DEVICE_VERSION_1 &&
> +	    !wq->idxd->user_submission_safe)
>  		return -EINVAL;
>  	/*
>  	 * As per the programming specification, the completion address must be
> diff --git a/drivers/dma/idxd/sysfs.c b/drivers/dma/idxd/sysfs.c
> index 6af493f6ba77..9f0701021af0 100644
> --- a/drivers/dma/idxd/sysfs.c
> +++ b/drivers/dma/idxd/sysfs.c
> @@ -1208,9 +1208,11 @@ static ssize_t op_cap_show_common(struct device *dev, char *buf, unsigned long *
>  
>  		/* On systems where direct user submissions are not safe, we need to clear out
>  		 * the BATCH capability from the capability mask in sysfs since we cannot support
> -		 * that command on such systems.
> +		 * that command on such systems. Narrow the restriction of operations with the
> +		 * BATCH opcode to only DSA version 1 devices.
>  		 */
> -		if (i == DSA_OPCODE_BATCH/64 && !confdev_to_idxd(dev)->user_submission_safe)
> +		if (i == DSA_OPCODE_BATCH/64 && !confdev_to_idxd(dev)->user_submission_safe &&
> +		    confdev_to_idxd(dev)->hw.version == DEVICE_VERSION_1)
>  			clear_bit(DSA_OPCODE_BATCH % 64, &val);
>  
>  		pos += sysfs_emit_at(buf, pos, "%*pb", 64, &val);

Re: [PATCH v1] dmaengine: idxd: Narrow the restriction on BATCH to ver. 1 only

Posted by Yi Sun 9 months, 1 week ago

On 12.03.2025 15:18, Dave Jiang wrote:
>
>
>On 3/12/25 3:15 PM, Vinicius Costa Gomes wrote:
>> Allow BATCH operations to be submitted and the capability to be
>> exposed for DSA version 2 (or later) devices.
>>
>> DSA version 2 devices allow safe submission of BATCH operations.
>>
>> Signed-off-by: Anil Keshavamurthy <anil.s.keshavamurthy@intel.com>
>> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
Reported-by: Yi Sun <yi.sun@intel.com>
Tested-by: Yi Sun <yi.sun@intel.com>
>
>Reviewed-by: Dave Jiang <dave.jiang@intel.com>
>> ---
>>  drivers/dma/idxd/cdev.c  | 6 ++++--
>>  drivers/dma/idxd/sysfs.c | 6 ++++--
>>  2 files changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/dma/idxd/cdev.c b/drivers/dma/idxd/cdev.c
>> index ff94ee892339..6a1dc15ee485 100644
>> --- a/drivers/dma/idxd/cdev.c
>> +++ b/drivers/dma/idxd/cdev.c
>> @@ -439,10 +439,12 @@ static int idxd_submit_user_descriptor(struct idxd_user_context *ctx,
>>  	 * DSA devices are capable of indirect ("batch") command submission.
>>  	 * On devices where direct user submissions are not safe, we cannot
>>  	 * allow this since there is no good way for us to verify these
>> -	 * indirect commands.
>> +	 * indirect commands. Narrow the restriction of operations with the
>> +	 * BATCH opcode to only DSA version 1 devices.
>>  	 */
>>  	if (is_dsa_dev(idxd_dev) && descriptor.opcode == DSA_OPCODE_BATCH &&
>> -		!wq->idxd->user_submission_safe)
>> +	    wq->idxd->hw.version == DEVICE_VERSION_1 &&
>> +	    !wq->idxd->user_submission_safe)
>>  		return -EINVAL;
>>  	/*
>>  	 * As per the programming specification, the completion address must be
>> diff --git a/drivers/dma/idxd/sysfs.c b/drivers/dma/idxd/sysfs.c
>> index 6af493f6ba77..9f0701021af0 100644
>> --- a/drivers/dma/idxd/sysfs.c
>> +++ b/drivers/dma/idxd/sysfs.c
>> @@ -1208,9 +1208,11 @@ static ssize_t op_cap_show_common(struct device *dev, char *buf, unsigned long *
>>
>>  		/* On systems where direct user submissions are not safe, we need to clear out
>>  		 * the BATCH capability from the capability mask in sysfs since we cannot support
>> -		 * that command on such systems.
>> +		 * that command on such systems. Narrow the restriction of operations with the
>> +		 * BATCH opcode to only DSA version 1 devices.
>>  		 */
>> -		if (i == DSA_OPCODE_BATCH/64 && !confdev_to_idxd(dev)->user_submission_safe)
>> +		if (i == DSA_OPCODE_BATCH/64 && !confdev_to_idxd(dev)->user_submission_safe &&
>> +		    confdev_to_idxd(dev)->hw.version == DEVICE_VERSION_1)
>>  			clear_bit(DSA_OPCODE_BATCH % 64, &val);
>>
>>  		pos += sysfs_emit_at(buf, pos, "%*pb", 64, &val);
>
>