[v2,1/6] dma/ioat: fix device stop if no copies done

Message ID 20230116173738.562322-2-bruce.richardson@intel.com (mailing list archive)
State Superseded, archived
Delegated to: David Marchand
Headers
Series dma/ioat: fix issues with stopping and restarting device |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Bruce Richardson Jan. 16, 2023, 5:37 p.m. UTC
  The HW DMA devices supported by IOAT driver do not transition to
the "active" state until the first operation is started by the HW.
Therefore, if the user calls "rte_dma_stop()" on a device without
triggering any operations, the sequence of commands to be sent to
the HW is different, as is the final device state.

Update the IOAT driver "stop" function to take account of this
difference.

Fixes: 583f046dd404 ("dma/ioat: add start and stop")
Cc: conor.walsh@intel.com
Cc: stable@dpdk.org

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 drivers/dma/ioat/ioat_dmadev.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)
  

Comments

Conor Walsh Jan. 18, 2023, 10:47 a.m. UTC | #1
> The HW DMA devices supported by IOAT driver do not transition to
> the "active" state until the first operation is started by the HW.
> Therefore, if the user calls "rte_dma_stop()" on a device without
> triggering any operations, the sequence of commands to be sent to
> the HW is different, as is the final device state.
> 
> Update the IOAT driver "stop" function to take account of this
> difference.
> 
> Fixes: 583f046dd404 ("dma/ioat: add start and stop")
> Cc: conor.walsh@intel.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>

Reviewed-by: Conor Walsh <conor.walsh@intel.com>
  
Kevin Laatz Feb. 14, 2023, 4:04 p.m. UTC | #2
On 16/01/2023 17:37, Bruce Richardson wrote:
> The HW DMA devices supported by IOAT driver do not transition to
> the "active" state until the first operation is started by the HW.
> Therefore, if the user calls "rte_dma_stop()" on a device without
> triggering any operations, the sequence of commands to be sent to
> the HW is different, as is the final device state.
>
> Update the IOAT driver "stop" function to take account of this
> difference.
>
> Fixes: 583f046dd404 ("dma/ioat: add start and stop")
> Cc: conor.walsh@intel.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>   drivers/dma/ioat/ioat_dmadev.c | 19 +++++++++++++++----
>   1 file changed, 15 insertions(+), 4 deletions(-)
>
Acked-by: Kevin Laatz <kevin.laatz@intel.com>
  

Patch

diff --git a/drivers/dma/ioat/ioat_dmadev.c b/drivers/dma/ioat/ioat_dmadev.c
index 5906eb45aa..aff7bbbfde 100644
--- a/drivers/dma/ioat/ioat_dmadev.c
+++ b/drivers/dma/ioat/ioat_dmadev.c
@@ -166,17 +166,28 @@  static int
 ioat_dev_stop(struct rte_dma_dev *dev)
 {
 	struct ioat_dmadev *ioat = dev->fp_obj->dev_private;
+	unsigned int chansts;
 	uint32_t retry = 0;
 
-	ioat->regs->chancmd = IOAT_CHANCMD_SUSPEND;
+	chansts = (unsigned int)(ioat->regs->chansts & IOAT_CHANSTS_STATUS);
+	if (chansts == IOAT_CHANSTS_ACTIVE || chansts == IOAT_CHANSTS_IDLE)
+		ioat->regs->chancmd = IOAT_CHANCMD_SUSPEND;
+	else
+		ioat->regs->chancmd = IOAT_CHANCMD_RESET;
 
 	do {
 		rte_pause();
 		retry++;
-	} while ((ioat->regs->chansts & IOAT_CHANSTS_STATUS) != IOAT_CHANSTS_SUSPENDED
-			&& retry < 200);
+		chansts = (unsigned int)(ioat->regs->chansts & IOAT_CHANSTS_STATUS);
+	} while (chansts != IOAT_CHANSTS_SUSPENDED &&
+			chansts != IOAT_CHANSTS_HALTED && retry < 200);
+
+	if (chansts == IOAT_CHANSTS_SUSPENDED || chansts == IOAT_CHANSTS_HALTED)
+		return 0;
 
-	return ((ioat->regs->chansts & IOAT_CHANSTS_STATUS) == IOAT_CHANSTS_SUSPENDED) ? 0 : -1;
+	IOAT_PMD_WARN("Channel could not be suspended on stop. (chansts = %u [%s])",
+			chansts, chansts_readable[chansts]);
+	return -1;
 }
 
 /* Get device information of a device. */