| Message ID | 20210729073503.187-5-xieyongji@bytedance.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | Introduce VDUSE - vDPA Device in Userspace | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
在 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)
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
在 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 --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)
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(-)