Re: [PATCH RFC v1] vdpa/ifcvf: Time out and report device reset failure

[Michael S. Tsirkin]

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@xxxxxxxxx>

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