* [PATCH RFC v1] vdpa/ifcvf: Time out and report device reset failure
@ 2026-09-30 15:59 Yuho Choi
2026-10-01 0:01 ` Michael S. Tsirkin
0 siblings, 1 reply; 3+ messages in thread
From: Yuho Choi @ 2026-09-30 15:59 UTC (permalink / raw)
To: Zhu Lingshan, Michael S. Tsirkin, Jason Wang
Cc: Eugenio Pérez, Xuan Zhuo, virtualization, linux-kernel, Yuho Choi
ifcvf_reset() writes 0 to device_status and polls it without a bound,
and ifcvf_vdpa_reset() always returns 0. A device that does not finish
the reset, or one that was surprise-removed and reads back 0xff, keeps
the caller in the reset path forever.
Bound the poll with readx_poll_timeout() and return -ETIMEDOUT from
.reset, so that callers such as VHOST_VDPA_SET_STATUS(0) see the failure
instead of hanging. Stop right away with -ENODEV once the device reads
back all ones and is no longer present, as there is nothing left to
wait for.
Fixes: 386a26208524 ("vDPA/ifcvf: synchronize irqs in the reset routine")
Signed-off-by: Yuho Choi <oss.patchbox@gmail.com>
---
RFC for two reasons:
- The spec gives no bound for a reset; 10 seconds follows octeon_ep's
OCTEP_HW_TIMEOUT.
- vhost_vdpa_release() ignores a reset failure and still tears the
mappings down, so on close a timeout turns a hang into teardown while
the device may still be active. That needs a separate fix in
vhost-vdpa.
Compile-tested only (x86_64 defconfig + IFCVF, W=1).
drivers/vdpa/ifcvf/ifcvf_base.c | 24 +++++++++++++++++++++---
drivers/vdpa/ifcvf/ifcvf_base.h | 3 ++-
drivers/vdpa/ifcvf/ifcvf_main.c | 4 +---
3 files changed, 24 insertions(+), 7 deletions(-)
diff --git a/drivers/vdpa/ifcvf/ifcvf_base.c b/drivers/vdpa/ifcvf/ifcvf_base.c
index d5507b63b6cdb..8a6c8f8314d53 100644
--- a/drivers/vdpa/ifcvf/ifcvf_base.c
+++ b/drivers/vdpa/ifcvf/ifcvf_base.c
@@ -8,6 +8,8 @@
*
*/
+#include <linux/iopoll.h>
+
#include "ifcvf_base.h"
u16 ifcvf_set_vq_vector(struct ifcvf_hw *hw, u16 qid, int vector)
@@ -201,11 +203,27 @@ void ifcvf_set_status(struct ifcvf_hw *hw, u8 status)
vp_iowrite8(status, &hw->common_cfg->device_status);
}
-void ifcvf_reset(struct ifcvf_hw *hw)
+static bool ifcvf_reset_done(struct ifcvf_hw *hw, u8 status)
{
+ /* A surprise-removed device reads back all ones and never resets */
+ return !status || (status == 0xff && !pci_device_is_present(hw->pdev));
+}
+
+int ifcvf_reset(struct ifcvf_hw *hw)
+{
+ u8 status;
+ int ret;
+
ifcvf_set_status(hw, 0);
- while (ifcvf_get_status(hw))
- msleep(1);
+ ret = readx_poll_timeout(ifcvf_get_status, hw, status,
+ ifcvf_reset_done(hw, status),
+ 1000, IFCVF_RESET_TIMEOUT_US);
+ if (ret) {
+ IFCVF_ERR(hw->pdev, "timed out waiting for device reset\n");
+ return ret;
+ }
+
+ return status ? -ENODEV : 0;
}
u64 ifcvf_get_hw_features(struct ifcvf_hw *hw)
diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h
index aa36de361c10e..57d00b881582b 100644
--- a/drivers/vdpa/ifcvf/ifcvf_base.h
+++ b/drivers/vdpa/ifcvf/ifcvf_base.h
@@ -29,6 +29,7 @@
#define IFCVF_LM_BAR 4
#define IFCVF_MIN_VQ_SIZE 64
+#define IFCVF_RESET_TIMEOUT_US (10 * USEC_PER_SEC)
#define IFCVF_ERR(pdev, fmt, ...) dev_err(&pdev->dev, fmt, ##__VA_ARGS__)
#define IFCVF_DBG(pdev, fmt, ...) dev_dbg(&pdev->dev, fmt, ##__VA_ARGS__)
@@ -112,7 +113,7 @@ void ifcvf_write_dev_config(struct ifcvf_hw *hw, u64 offset,
const void *src, int length);
u8 ifcvf_get_status(struct ifcvf_hw *hw);
void ifcvf_set_status(struct ifcvf_hw *hw, u8 status);
-void ifcvf_reset(struct ifcvf_hw *hw);
+int ifcvf_reset(struct ifcvf_hw *hw);
u64 ifcvf_get_dev_features(struct ifcvf_hw *hw);
u64 ifcvf_get_hw_features(struct ifcvf_hw *hw);
int ifcvf_verify_min_features(struct ifcvf_hw *hw, u64 features);
diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c
index 2af1cec958848..6c569f01bec91 100644
--- a/drivers/vdpa/ifcvf/ifcvf_main.c
+++ b/drivers/vdpa/ifcvf/ifcvf_main.c
@@ -444,9 +444,7 @@ static int ifcvf_vdpa_reset(struct vdpa_device *vdpa_dev)
if (status & VIRTIO_CONFIG_S_DRIVER_OK)
ifcvf_free_irq(vf);
- ifcvf_reset(vf);
-
- return 0;
+ return ifcvf_reset(vf);
}
static u16 ifcvf_vdpa_get_vq_num_max(struct vdpa_device *vdpa_dev)
base-commit: 551c722f40809618230001baccf219193e22fc5a
--
2.43.0
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH RFC v1] vdpa/ifcvf: Time out and report device reset failure
2026-09-30 15:59 [PATCH RFC v1] vdpa/ifcvf: Time out and report device reset failure Yuho Choi
@ 2026-10-01 0:01 ` Michael S. Tsirkin
2026-10-01 4:48 ` Yuho Choi
0 siblings, 1 reply; 3+ messages in thread
From: Michael S. Tsirkin @ 2026-10-01 0:01 UTC (permalink / raw)
To: Yuho Choi
Cc: Zhu Lingshan, Jason Wang, Eugenio Pérez, Xuan Zhuo,
virtualization, linux-kernel
On Wed, Sep 30, 2026 at 11:59:01AM -0400, Yuho Choi wrote:
> ifcvf_reset() writes 0 to device_status and polls it without a bound,
> and ifcvf_vdpa_reset() always returns 0. A device that does not finish
> the reset, or one that was surprise-removed and reads back 0xff, keeps
> the caller in the reset path forever.
>
> Bound the poll with readx_poll_timeout() and return -ETIMEDOUT from
> .reset, so that callers such as VHOST_VDPA_SET_STATUS(0) see the failure
> instead of hanging.
No one handles reset errors. You can not timeout this.
> Stop right away with -ENODEV once the device reads
> back all ones and is no longer present, as there is nothing left to
> wait for.
>
> Fixes: 386a26208524 ("vDPA/ifcvf: synchronize irqs in the reset routine")
> Signed-off-by: Yuho Choi <oss.patchbox@gmail.com>
Use standard is present APIs not "all ones" hacks.
> ---
> RFC for two reasons:
> - The spec gives no bound for a reset; 10 seconds follows octeon_ep's
> OCTEP_HW_TIMEOUT.
> - vhost_vdpa_release() ignores a reset failure and still tears the
> mappings down, so on close a timeout turns a hang into teardown while
> the device may still be active. That needs a separate fix in
> vhost-vdpa.
>
> Compile-tested only (x86_64 defconfig + IFCVF, W=1).
>
> drivers/vdpa/ifcvf/ifcvf_base.c | 24 +++++++++++++++++++++---
> drivers/vdpa/ifcvf/ifcvf_base.h | 3 ++-
> drivers/vdpa/ifcvf/ifcvf_main.c | 4 +---
> 3 files changed, 24 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/vdpa/ifcvf/ifcvf_base.c b/drivers/vdpa/ifcvf/ifcvf_base.c
> index d5507b63b6cdb..8a6c8f8314d53 100644
> --- a/drivers/vdpa/ifcvf/ifcvf_base.c
> +++ b/drivers/vdpa/ifcvf/ifcvf_base.c
> @@ -8,6 +8,8 @@
> *
> */
>
> +#include <linux/iopoll.h>
> +
> #include "ifcvf_base.h"
>
> u16 ifcvf_set_vq_vector(struct ifcvf_hw *hw, u16 qid, int vector)
> @@ -201,11 +203,27 @@ void ifcvf_set_status(struct ifcvf_hw *hw, u8 status)
> vp_iowrite8(status, &hw->common_cfg->device_status);
> }
>
> -void ifcvf_reset(struct ifcvf_hw *hw)
> +static bool ifcvf_reset_done(struct ifcvf_hw *hw, u8 status)
> {
> + /* A surprise-removed device reads back all ones and never resets */
> + return !status || (status == 0xff && !pci_device_is_present(hw->pdev));
> +}
> +
> +int ifcvf_reset(struct ifcvf_hw *hw)
> +{
> + u8 status;
> + int ret;
> +
> ifcvf_set_status(hw, 0);
> - while (ifcvf_get_status(hw))
> - msleep(1);
> + ret = readx_poll_timeout(ifcvf_get_status, hw, status,
> + ifcvf_reset_done(hw, status),
> + 1000, IFCVF_RESET_TIMEOUT_US);
> + if (ret) {
> + IFCVF_ERR(hw->pdev, "timed out waiting for device reset\n");
> + return ret;
> + }
> +
> + return status ? -ENODEV : 0;
> }
>
> u64 ifcvf_get_hw_features(struct ifcvf_hw *hw)
> diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h
> index aa36de361c10e..57d00b881582b 100644
> --- a/drivers/vdpa/ifcvf/ifcvf_base.h
> +++ b/drivers/vdpa/ifcvf/ifcvf_base.h
> @@ -29,6 +29,7 @@
>
> #define IFCVF_LM_BAR 4
> #define IFCVF_MIN_VQ_SIZE 64
> +#define IFCVF_RESET_TIMEOUT_US (10 * USEC_PER_SEC)
>
> #define IFCVF_ERR(pdev, fmt, ...) dev_err(&pdev->dev, fmt, ##__VA_ARGS__)
> #define IFCVF_DBG(pdev, fmt, ...) dev_dbg(&pdev->dev, fmt, ##__VA_ARGS__)
> @@ -112,7 +113,7 @@ void ifcvf_write_dev_config(struct ifcvf_hw *hw, u64 offset,
> const void *src, int length);
> u8 ifcvf_get_status(struct ifcvf_hw *hw);
> void ifcvf_set_status(struct ifcvf_hw *hw, u8 status);
> -void ifcvf_reset(struct ifcvf_hw *hw);
> +int ifcvf_reset(struct ifcvf_hw *hw);
> u64 ifcvf_get_dev_features(struct ifcvf_hw *hw);
> u64 ifcvf_get_hw_features(struct ifcvf_hw *hw);
> int ifcvf_verify_min_features(struct ifcvf_hw *hw, u64 features);
> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c
> index 2af1cec958848..6c569f01bec91 100644
> --- a/drivers/vdpa/ifcvf/ifcvf_main.c
> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c
> @@ -444,9 +444,7 @@ static int ifcvf_vdpa_reset(struct vdpa_device *vdpa_dev)
> if (status & VIRTIO_CONFIG_S_DRIVER_OK)
> ifcvf_free_irq(vf);
>
> - ifcvf_reset(vf);
> -
> - return 0;
> + return ifcvf_reset(vf);
> }
>
> static u16 ifcvf_vdpa_get_vq_num_max(struct vdpa_device *vdpa_dev)
>
> base-commit: 551c722f40809618230001baccf219193e22fc5a
> --
> 2.43.0
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH RFC v1] vdpa/ifcvf: Time out and report device reset failure
2026-10-01 0:01 ` Michael S. Tsirkin
@ 2026-10-01 4:48 ` Yuho Choi
0 siblings, 0 replies; 3+ messages in thread
From: Yuho Choi @ 2026-10-01 4:48 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Zhu Lingshan, Jason Wang, Eugenio Pérez, Xuan Zhuo,
virtualization, linux-kernel
Thanks for the review.
On Wed, 30 Sept 2026 at 20:01, Michael S. Tsirkin <mst@redhat.com> wrote:
>
> No one handles reset errors. You can not timeout this.
Agreed, returning early lets callers free memory the device may still
DMA into.
For a device that is present but never completes the reset, would it
be acceptable to fence it instead: pci_clear_master() after a bounded wait,
and pci_set_master() only once a later reset completes? .reset would
then always return with the device quiet, so caller can keep ignoring
the result.
> Use standard is present APIs not "all ones" hacks.
Will do, v2 will use the present APIs.
Thanks,
Yuho
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-10-01 4:48 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-30 15:59 [PATCH RFC v1] vdpa/ifcvf: Time out and report device reset failure Yuho Choi
2026-10-01 0:01 ` Michael S. Tsirkin
2026-10-01 4:48 ` Yuho Choi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®