On Mon, Feb 12, 2024 at 11:37:12AM -0500, Steven Sistare wrote: > On 2/12/2024 10:56 AM, Michael S. Tsirkin wrote: > > 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. > > 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; right > - Steve > >> 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 > >>> > >
