diff mbox series

[v10,04/17] vdpa: Fail the vdpa_reset() if fail to set device status to zero

Message ID 20210729073503.187-5-xieyongji@bytedance.com (mailing list archive)
State New, archived
Headers show
Series Introduce VDUSE - vDPA Device in Userspace | expand

Commit Message

Yongji Xie July 29, 2021, 7:34 a.m. UTC
Re-read the device status to ensure it's set to zero during
resetting. Otherwise, fail the vdpa_reset() after timeout.

Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
---
 include/linux/vdpa.h | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

Comments

Jason Wang Aug. 3, 2021, 7:58 a.m. UTC | #1
在 2021/7/29 下午3:34, Xie Yongji 写道:
> Re-read the device status to ensure it's set to zero during
> resetting. Otherwise, fail the vdpa_reset() after timeout.
>
> Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
> ---
>   include/linux/vdpa.h | 15 ++++++++++++++-
>   1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
> index 406d53a606ac..d1a80ef05089 100644
> --- a/include/linux/vdpa.h
> +++ b/include/linux/vdpa.h
> @@ -6,6 +6,7 @@
>   #include <linux/device.h>
>   #include <linux/interrupt.h>
>   #include <linux/vhost_iotlb.h>
> +#include <linux/delay.h>
>   
>   /**
>    * struct vdpa_calllback - vDPA callback definition.
> @@ -340,12 +341,24 @@ static inline struct device *vdpa_get_dma_dev(struct vdpa_device *vdev)
>   	return vdev->dma_dev;
>   }
>   
> -static inline void vdpa_reset(struct vdpa_device *vdev)
> +#define VDPA_RESET_TIMEOUT_MS 1000
> +
> +static inline int vdpa_reset(struct vdpa_device *vdev)
>   {
>   	const struct vdpa_config_ops *ops = vdev->config;
> +	int timeout = 0;
>   
>   	vdev->features_valid = false;
>   	ops->set_status(vdev, 0);
> +	while (ops->get_status(vdev)) {
> +		timeout += 20;
> +		if (timeout > VDPA_RESET_TIMEOUT_MS)
> +			return -EIO;
> +
> +		msleep(20);
> +	}


I wonder if it's better to do this in the vDPA parent?

Thanks


> +
> +	return 0;
>   }
>   
>   static inline int vdpa_set_features(struct vdpa_device *vdev, u64 features)
Yongji Xie Aug. 3, 2021, 9:31 a.m. UTC | #2
On Tue, Aug 3, 2021 at 3:58 PM Jason Wang <jasowang@redhat.com> wrote:
>
>
> 在 2021/7/29 下午3:34, Xie Yongji 写道:
> > Re-read the device status to ensure it's set to zero during
> > resetting. Otherwise, fail the vdpa_reset() after timeout.
> >
> > Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
> > ---
> >   include/linux/vdpa.h | 15 ++++++++++++++-
> >   1 file changed, 14 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
> > index 406d53a606ac..d1a80ef05089 100644
> > --- a/include/linux/vdpa.h
> > +++ b/include/linux/vdpa.h
> > @@ -6,6 +6,7 @@
> >   #include <linux/device.h>
> >   #include <linux/interrupt.h>
> >   #include <linux/vhost_iotlb.h>
> > +#include <linux/delay.h>
> >
> >   /**
> >    * struct vdpa_calllback - vDPA callback definition.
> > @@ -340,12 +341,24 @@ static inline struct device *vdpa_get_dma_dev(struct vdpa_device *vdev)
> >       return vdev->dma_dev;
> >   }
> >
> > -static inline void vdpa_reset(struct vdpa_device *vdev)
> > +#define VDPA_RESET_TIMEOUT_MS 1000
> > +
> > +static inline int vdpa_reset(struct vdpa_device *vdev)
> >   {
> >       const struct vdpa_config_ops *ops = vdev->config;
> > +     int timeout = 0;
> >
> >       vdev->features_valid = false;
> >       ops->set_status(vdev, 0);
> > +     while (ops->get_status(vdev)) {
> > +             timeout += 20;
> > +             if (timeout > VDPA_RESET_TIMEOUT_MS)
> > +                     return -EIO;
> > +
> > +             msleep(20);
> > +     }
>
>
> I wonder if it's better to do this in the vDPA parent?
>
> Thanks
>

Sorry, I didn't get you here. Do you mean vDPA parent driver (e.g.
VDUSE)? Actually I didn't find any other place where I can do
set_status() and get_status().

Thanks,
Yongji
Jason Wang Aug. 4, 2021, 8:30 a.m. UTC | #3
在 2021/8/3 下午5:31, Yongji Xie 写道:
> On Tue, Aug 3, 2021 at 3:58 PM Jason Wang <jasowang@redhat.com> wrote:
>>
>> 在 2021/7/29 下午3:34, Xie Yongji 写道:
>>> Re-read the device status to ensure it's set to zero during
>>> resetting. Otherwise, fail the vdpa_reset() after timeout.
>>>
>>> Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
>>> ---
>>>    include/linux/vdpa.h | 15 ++++++++++++++-
>>>    1 file changed, 14 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
>>> index 406d53a606ac..d1a80ef05089 100644
>>> --- a/include/linux/vdpa.h
>>> +++ b/include/linux/vdpa.h
>>> @@ -6,6 +6,7 @@
>>>    #include <linux/device.h>
>>>    #include <linux/interrupt.h>
>>>    #include <linux/vhost_iotlb.h>
>>> +#include <linux/delay.h>
>>>
>>>    /**
>>>     * struct vdpa_calllback - vDPA callback definition.
>>> @@ -340,12 +341,24 @@ static inline struct device *vdpa_get_dma_dev(struct vdpa_device *vdev)
>>>        return vdev->dma_dev;
>>>    }
>>>
>>> -static inline void vdpa_reset(struct vdpa_device *vdev)
>>> +#define VDPA_RESET_TIMEOUT_MS 1000
>>> +
>>> +static inline int vdpa_reset(struct vdpa_device *vdev)
>>>    {
>>>        const struct vdpa_config_ops *ops = vdev->config;
>>> +     int timeout = 0;
>>>
>>>        vdev->features_valid = false;
>>>        ops->set_status(vdev, 0);
>>> +     while (ops->get_status(vdev)) {
>>> +             timeout += 20;
>>> +             if (timeout > VDPA_RESET_TIMEOUT_MS)
>>> +                     return -EIO;
>>> +
>>> +             msleep(20);
>>> +     }
>>
>> I wonder if it's better to do this in the vDPA parent?
>>
>> Thanks
>>
> Sorry, I didn't get you here. Do you mean vDPA parent driver (e.g.
> VDUSE)?


Yes, since the how it's expected to behave depends on the specific hardware.

Even for the spec, the behavior is transport specific:

PCI: requires reread until 0
MMIO: doesn't require but it might not work for the hardware so we 
decide to change
CCW: the succeed of the ccw command means the success of the reset

Thanks


> Actually I didn't find any other place where I can do
> set_status() and get_status().
>
> Thanks,
> Yongji
>
diff mbox series

Patch

diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
index 406d53a606ac..d1a80ef05089 100644
--- a/include/linux/vdpa.h
+++ b/include/linux/vdpa.h
@@ -6,6 +6,7 @@ 
 #include <linux/device.h>
 #include <linux/interrupt.h>
 #include <linux/vhost_iotlb.h>
+#include <linux/delay.h>
 
 /**
  * struct vdpa_calllback - vDPA callback definition.
@@ -340,12 +341,24 @@  static inline struct device *vdpa_get_dma_dev(struct vdpa_device *vdev)
 	return vdev->dma_dev;
 }
 
-static inline void vdpa_reset(struct vdpa_device *vdev)
+#define VDPA_RESET_TIMEOUT_MS 1000
+
+static inline int vdpa_reset(struct vdpa_device *vdev)
 {
 	const struct vdpa_config_ops *ops = vdev->config;
+	int timeout = 0;
 
 	vdev->features_valid = false;
 	ops->set_status(vdev, 0);
+	while (ops->get_status(vdev)) {
+		timeout += 20;
+		if (timeout > VDPA_RESET_TIMEOUT_MS)
+			return -EIO;
+
+		msleep(20);
+	}
+
+	return 0;
 }
 
 static inline int vdpa_set_features(struct vdpa_device *vdev, u64 features)