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

2021-08-04 Thread Jason Wang 


在 2021/8/3 下午5:31, Yongji Xie 写道:

On Tue, Aug 3, 2021 at 3:58 PM Jason Wang  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 
---
   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 
   #include 
   #include 
+#include 

   /**
* 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



___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

2021-08-03 Thread Yongji Xie 
On Tue, Aug 3, 2021 at 3:58 PM Jason Wang  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 
> > ---
> >   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 
> >   #include 
> >   #include 
> > +#include 
> >
> >   /**
> >* 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
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

2021-08-03 Thread Jason Wang 


在 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 
---
  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 
  #include 
  #include 
+#include 
  
  /**

   * 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)


___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

2021-07-29 Thread 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 
---
 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 
 #include 
 #include 
+#include 
 
 /**
  * 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)
-- 
2.11.0

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu