[PATCH] vdpa/ifc/base: wait for queue disable before saving q-state

[PATCH] vdpa/ifc/base: wait for queue disable before saving q-state

[Abhishek Maheshwari]

Some ifc hardware require synchronization between disabling a queue and
saving queue-state from LM registers. When queue is disabled from vDPA
driver, ifc device stops executing new virtio-cmds and then updates LM
registers with used/avail index. Before saving the queue-state, vDPA
driver should wait until the queue is disabled from backend.

Signed-off-by: Abhishek Maheshwari <abhishek.maheshwari@intel.com>
---
 drivers/vdpa/ifc/base/ifcvf.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/vdpa/ifc/base/ifcvf.c b/drivers/vdpa/ifc/base/ifcvf.c
index f1e1474447..76690c1f55 100644
--- a/drivers/vdpa/ifc/base/ifcvf.c
+++ b/drivers/vdpa/ifc/base/ifcvf.c
@@ -257,6 +257,7 @@ ifcvf_hw_disable(struct ifcvf_hw *hw)
 	u32 i;
 	struct ifcvf_pci_common_cfg *cfg;
 	u32 ring_state;
+	int q_disable_tries;
 
 	cfg = hw->common_cfg;
 	if (!cfg) {
@@ -275,6 +276,20 @@ ifcvf_hw_disable(struct ifcvf_hw *hw)
 			continue;
 		}
 
+		/* Some ifc hardware require synchronization between disabling a queue
+		 * and saving queue-state from LM registers. When queue is disabled from
+		 * vDPA driver, ifc device stops executing new virtio-cmds and then
+		 * updates LM registers with used/avail index. Before saving the
+		 * queue-state, vDPA driver waits until the queue is disabled from
+		 * backend.
+		 */
+		q_disable_tries = 10;
+		while (q_disable_tries-- && IFCVF_READ_REG16(&cfg->queue_enable))
+			msec_delay(10);
+
+		if (IFCVF_READ_REG16(&cfg->queue_enable))
+			WARNINGOUT("Failed Q disable: %d. saved state is invalid\n", i);
+
 		if (hw->device_type == IFCVF_BLK)
 			ring_state = *(u32 *)(hw->lm_cfg +
 					IFCVF_LM_RING_STATE_OFFSET +
-- 
2.31.1

Re: [PATCH] vdpa/ifc/base: wait for queue disable before saving q-state

[Maxime Coquelin]

Hi Abhishek,

On 10/25/22 09:51, Abhishek Maheshwari wrote:
> Some ifc hardware require synchronization between disabling a queue and
> saving queue-state from LM registers. When queue is disabled from vDPA
> driver, ifc device stops executing new virtio-cmds and then updates LM
> registers with used/avail index. Before saving the queue-state, vDPA
> driver should wait until the queue is disabled from backend.


I think it should be considered a fix, and so requires a Fixes tag and
stable@dpdk.org to be cc'ed.

> Signed-off-by: Abhishek Maheshwari <abhishek.maheshwari@intel.com>
> ---
>   drivers/vdpa/ifc/base/ifcvf.c | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/vdpa/ifc/base/ifcvf.c b/drivers/vdpa/ifc/base/ifcvf.c
> index f1e1474447..76690c1f55 100644
> --- a/drivers/vdpa/ifc/base/ifcvf.c
> +++ b/drivers/vdpa/ifc/base/ifcvf.c
> @@ -257,6 +257,7 @@ ifcvf_hw_disable(struct ifcvf_hw *hw)
>   	u32 i;
>   	struct ifcvf_pci_common_cfg *cfg;
>   	u32 ring_state;
> +	int q_disable_tries;
>   
>   	cfg = hw->common_cfg;
>   	if (!cfg) {
> @@ -275,6 +276,20 @@ ifcvf_hw_disable(struct ifcvf_hw *hw)
>   			continue;
>   		}
>   
> +		/* Some ifc hardware require synchronization between disabling a queue
> +		 * and saving queue-state from LM registers. When queue is disabled from
> +		 * vDPA driver, ifc device stops executing new virtio-cmds and then
> +		 * updates LM registers with used/avail index. Before saving the
> +		 * queue-state, vDPA driver waits until the queue is disabled from
> +		 * backend.
> +		 */
> +		q_disable_tries = 10;
> +		while (q_disable_tries-- && IFCVF_READ_REG16(&cfg->queue_enable))
> +			msec_delay(10);
> +
> +		if (IFCVF_READ_REG16(&cfg->queue_enable))
> +			WARNINGOUT("Failed Q disable: %d. saved state is invalid\n", i);

Maybe checking q_disable_tries > 0 is enough here.
Also, should we really continue the disablement if synchronization
failed?

> +
>   		if (hw->device_type == IFCVF_BLK)
>   			ring_state = *(u32 *)(hw->lm_cfg +
>   					IFCVF_LM_RING_STATE_OFFSET +

[PATCH] vdpa/ifc/base: wait for queue disable before saving q-state

[Abhishek Maheshwari]

Some ifc hardware require synchronization between disabling a queue and
saving queue-state from LM registers. When queue is disabled from vDPA
driver, ifc device stops executing new virtio-cmds and then updates LM
registers with used/avail index. Before saving the queue-state, vDPA
driver should wait until the queue is disabled from backend.

Signed-off-by: Abhishek Maheshwari <abhishek.maheshwari@intel.com>
---
 drivers/vdpa/ifc/base/ifcvf.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/vdpa/ifc/base/ifcvf.c b/drivers/vdpa/ifc/base/ifcvf.c
index f1e1474447..fb51c793a2 100644
--- a/drivers/vdpa/ifc/base/ifcvf.c
+++ b/drivers/vdpa/ifc/base/ifcvf.c
@@ -257,6 +257,7 @@ ifcvf_hw_disable(struct ifcvf_hw *hw)
 	u32 i;
 	struct ifcvf_pci_common_cfg *cfg;
 	u32 ring_state;
+	int q_disable_tries;
 
 	cfg = hw->common_cfg;
 	if (!cfg) {
@@ -275,6 +276,20 @@ ifcvf_hw_disable(struct ifcvf_hw *hw)
 			continue;
 		}
 
+        /* Some ifc hardware require synchronization between disabling a queue
+         * and saving queue-state from LM registers.
+         * When queue is disabled from vDPA driver, ifc device stops executing
+         * new virtio-cmds and then updates LM registers with used/avail index.
+         * Before saving the queue-state, vDPA driver waits until the queue is
+         * disabled from backend.
+		 */
+		q_disable_tries = 10;
+		while (q_disable_tries-- && IFCVF_READ_REG16(&cfg->queue_enable))
+			msec_delay(10);
+
+		if (IFCVF_READ_REG16(&cfg->queue_enable))
+			WARNINGOUT("Failed Q disable: %d. saved state is invalid\n", i);
+
 		if (hw->device_type == IFCVF_BLK)
 			ring_state = *(u32 *)(hw->lm_cfg +
 					IFCVF_LM_RING_STATE_OFFSET +
-- 
2.31.1