On 2/12/2024 10:56 AM, Michael S. Tsirkin wrote:

On Mon, Feb 12, 2024 at 09:56:31AM -0500, Steven Sistare wrote:

quoted

On 2/12/2024 3:19 AM, Michael S. Tsirkin wrote:

quoted

On Fri, Feb 09, 2024 at 02:29:59PM -0800, Steve Sistare wrote:

quoted

Calling suspend or resume requires VIRTIO_CONFIG_S_DRIVER_OK, for all
vdpa devices.

Suggested-by: Eugenio Perez Martin <eperezma@redhat.com>"
Signed-off-by: Steve Sistare <redacted>

I don't think failing suspend or resume makes sense though -
e.g. practically failing suspend will just prevent sleeping I think -
why should guest not having driver loaded prevent system suspend?

Got it, my fix is too heavy handed.

quoted

there's also state such as features set which does need to be
preserved.

I think the thing to do is to skip invoking suspend/resume callback

OK.

quoted

 and in
fact checking suspend/resume altogether.

Currently ops->suspend, vhost_vdpa_can_suspend(), and VHOST_BACKEND_F_SUSPEND
are equivalent.  Hence if !ops->suspend, then then the driver does not support
it, and indeed may break if suspend is used, so system suspend must be blocked,
AFAICT.  Yielding:

If DRIVER_OK is not set then there's nothing to be done for migration.
So callback not needed.

OK, I missed your point.  Next attempt:

   vhost_vdpa_suspend()
       if (!(ops->get_status(vdpa) & VIRTIO_CONFIG_S_DRIVER_OK))
           return 0;

       if (!ops->suspend)
           return -EOPNOTSUPP;

- Steve

quoted

    vhost_vdpa_suspend()
        if (!ops->suspend)
            return -EOPNOTSUPP;

        if (!(ops->get_status(vdpa) & VIRTIO_CONFIG_S_DRIVER_OK))
            return 0;

- Steve

quoted

quoted

---
 drivers/vhost/vdpa.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index bc4a51e4638b..ce1882acfc3b 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c

@@ -598,6 +598,9 @@ static long vhost_vdpa_suspend(struct vhost_vdpa *v)
 	if (!ops->suspend)
 		return -EOPNOTSUPP;
 
+	if (!(ops->get_status(vdpa) & VIRTIO_CONFIG_S_DRIVER_OK))
+		return -EINVAL;
+
 	ret = ops->suspend(vdpa);
 	if (!ret)
 		v->suspended = true;

@@ -618,6 +621,9 @@ static long vhost_vdpa_resume(struct vhost_vdpa *v)
 	if (!ops->resume)
 		return -EOPNOTSUPP;
 
+	if (!(ops->get_status(vdpa) & VIRTIO_CONFIG_S_DRIVER_OK))
+		return -EINVAL;
+
 	ret = ops->resume(vdpa);
 	if (!ret)
 		v->suspended = false;

-- 
2.39.3