On Mon, Feb 12, 2024 at 09:56:31AM -0500, Steven Sistare wrote: > On 2/12/2024 3:19 AM, Michael S. Tsirkin wrote: > > On Fri, Feb 09, 2024 at 02:29:59PM -0800, Steve Sistare wrote: > >> Calling suspend or resume requires VIRTIO_CONFIG_S_DRIVER_OK, for all > >> vdpa devices. > >> > >> Suggested-by: Eugenio Perez Martin <eperezma@xxxxxxxxxx>" > >> Signed-off-by: Steve Sistare <steven.sistare@xxxxxxxxxx> > > > > 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. > > > 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. > > > 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. > vhost_vdpa_suspend() > if (!ops->suspend) > return -EOPNOTSUPP; > > if (!(ops->get_status(vdpa) & VIRTIO_CONFIG_S_DRIVER_OK)) > return 0; > > - Steve > > >> --- > >> 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 > >
