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;
- 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
>>>
>
