Received-SPF: pass (google.com: domain of linux-kernel+bounces-61850-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Message-ID: <a4d2626d-7d74-4243-93a9-01d7adc8cda4@oracle.com>
Date: Mon, 12 Feb 2024 09:56:31 -0500
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH V1] vdpa: suspend and resume require DRIVER_OK
Content-Language: en-US
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: virtualization@lists.linux-foundation.org, linux-kernel@vger.kernel.org,
        Jason Wang <jasowang@redhat.com>, Si-Wei Liu <si-wei.liu@oracle.com>,
        Eugenio Perez Martin <eperezma@redhat.com>,
        Stefano Garzarella <sgarzare@redhat.com>
References: <1707517799-137286-1-git-send-email-steven.sistare@oracle.com>
 <20240212031722-mutt-send-email-mst@kernel.org>
From: Steven Sistare <steven.sistare@oracle.com>
Organization: Oracle Corporation
In-Reply-To: <20240212031722-mutt-send-email-mst@kernel.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: 
	=?utf-8?B?b1FnWEhJdmVGNlh6MElCUVo3VUdPY2FGcXVNOFo1bFNTRmZ6VlBqUEIvOHBs?=
 =?utf-8?B?TS9TUDRmcmdkalZXZ3RSVHU4aVl1YmQreldpTkVLOEVqM2d2ZDY1Z21Ib2s0?=
 =?utf-8?B?K29wNVpNd3lFdlU5N1o2VXZwV2VTRUdSVjRkamJpSVNJZlpYVXc2OXVLWnU3?=
 =?utf-8?B?bml0dmZISC9QRS81NmRWcFlPM2JyTFpqak5sejhXWHNNYXVLZ2hlTElCU1ND?=
 =?utf-8?B?MWhiSmJ5RVNBbTNCTWhDOE1jKzM5eU5vK3R2Wnh6RWxwOXpnc2p1M0R2Ym9H?=
 =?utf-8?B?ODErZlhSR3VLSnp5ZDJOaUY3ZFhaWWViTkxrZUJIcnJ5ZTRleW9JZVIyWUdM?=
 =?utf-8?B?TzVUMHJ1SkQrUlhuc1gyUndIcVRnR3FxcUhXTnkyenBrVVpLcGlLWXFjY3ZO?=
 =?utf-8?B?VHdRVlBSSm9wSWxjcnFTQVNuN0VwTFd2NnRHQy8yb3ZMdElTOFRJRUN1em1W?=
 =?utf-8?B?TVVJdVVlZzExQ3YzU1M2b3dhNWpPSGtIbzE1djZZY3dYWnRWTUtGbUREL1hK?=
 =?utf-8?B?L2tMZ1FJVk5QUTR6VGlOWERXV2xsQ0lMQUZ1cTBVOUdOdmpqdlJHb1pmYm92?=
 =?utf-8?B?SHYrbHQycEhJVDN5VlV3WmhkMWpNaTFPSzdXdk5NamlMR0pqc2JDSitUT2N1?=
 =?utf-8?B?VzZHakhvTkRvbmU2UUZET0piUTArRWpmeEZ0VWZrMjBPdFR2N2ZRcEhycUJi?=
 =?utf-8?B?ZGhMdlRKOHY4bUgycW1tZHZHRkVrK1NKYTFTUi95eGhwNmN0V3B1dTkwZVpV?=
 =?utf-8?B?a3hiYnVqQ2dUN2M1Qk9YaGlQblNrK0trcWpDZFY3UmhHVktRK0xnMzNZUkNn?=
 =?utf-8?B?dnd1NVg4d1Z1VzhHcWg4OFloUWhVWFl0bGg5dG1oRGdiWUY2dXh6R0dWWEJB?=
 =?utf-8?B?bE9BWWxYL1h6WEpzeVAra200U0ZSL3h1Q0tQdzJsNzBBeDlMcExGR2RqOVZW?=
 =?utf-8?B?ZVhyLzJXdHZrRjBBRTdtQlVsLzllQ29FbGMzdDNWZEhaTmFUaFc1SW1ZYytw?=
 =?utf-8?B?dEI3ZzNLdUFxbHNCNW9xR0NlcHpnWmRUTW5lLzZXdUpRc1BONWVYS2ZGY2lo?=
 =?utf-8?B?Y3hzeG4vUDJTbmgwMUhUcXV1UG51U1VJWDlXbmRGazVlcGlOQ3BpME12bXNs?=
 =?utf-8?B?MW1pRkl2M0ZlNVJodXRla0c4dnpZTGlaUWFFUnZvalFoQUhxbnN4dkhHajht?=
 =?utf-8?B?dHQ0S1E5R0tQRkVCcGhEbjRDRDNJU04ycTFqTEJVdTFya3NpWEM2dnp4b2E2?=
 =?utf-8?B?anBRSmRwM2F0TnF4Smp3UUJQdFp0REZmL3ZDZzc2bHg0WWNPRXp6eTJtM29h?=
 =?utf-8?B?S2lZdytqUlZ5WEo2OTdmMGVDeHZjVTRGd1NPUFFGMFpkNXlEU1VoMXpZejU0?=
 =?utf-8?B?WWgzRUlZVGhxOCtUSnFwT05DcW8vS1hPMHRlNHVCQldlcWVVemNYc0VGTXh0?=
 =?utf-8?B?bUtHcFpFVlZ0WG01U1F1cHBJUjBsUGdvSElTUHNQOG40aG94OVVsblZ0aVpW?=
 =?utf-8?B?d1lKZWsxWXpKTG9acXFnS3RSaGR4SmhpMlpvLzZ6dTVWemYyZWNZWGFwWklz?=
 =?utf-8?B?TjdZSWc5VnNDSTBJUkp4aVFTYXlaZFA1OGtWMGI2R09CeVlyWndQMjZFUTlh?=
 =?utf-8?B?N1NicjZ6azRsV1NkNitNU0VHdVBBdTZodFFkdWV0aUlRNEdwamxySWV6blFO?=
 =?utf-8?B?Yjh6NUdNb0xveVhRUmM5SmQ3dWMySlZHQTA5akhWU1ZOYWd5OUdzZU9rdWto?=
 =?utf-8?B?a0Y2WkJYV0xuWWRTa2Q1R1hIdXJDdVZVbk92eVMrVWN1bWVpOUlRTExkZ2pi?=
 =?utf-8?B?bWhadC9WOUZ2V3VIZ0hicUVuVlcyZExOR2Z0WUx5eC84Z0dVTkdyNmhoWnRN?=
 =?utf-8?B?VVJhbzU4aUhBK1BkYzBDTXd5a1ltVEgvQU9STnNvbFJWZG55SjZxdzZWam1z?=
 =?utf-8?B?cHp3aUdxQjBaaTdzWGFITU1NQkxVamlJY1ZvQWNObWh6UGZFbEJsVm40N0Q0?=
 =?utf-8?B?d2VFeHQ1NGJva3l0OThHSGQwMEg2MHN4STV5NUhZQ0cxRU1qRzMrbHp5ZmZ2?=
 =?utf-8?B?WmJ3amJ2V3pMTThHT0dkYm42dnhhQ2x2NkJXTWlZNWIxbngwazN1OTBOdWpS?=
 =?utf-8?B?QWE5bm45YUdrZUtqSEwyWHRRVjk3L2J3RVpOdHJvN1k0ZzhJdXFWMjZxenI0?=
 =?utf-8?B?UFE9PQ==?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	1/D68Vg6EAFaj8XIjOPboAOjZlnYs1wsDLO6W4LhoByqO/l3WpC2jg1OnWahESvFWYmSva9oZakhocbxt9qzCRfz3OZ8Q1O+xxXx6TcMIwCHrmTE0RBAaseMH2IodVwCSnpzQzCLlsIPE1cTRvMFh75xRjr4/FLoSy2EBso6i+wcnYZla8ukyMmXFoOAD9ZL/HDmOWooPLrI4mZZi+4UYI0f3kUx6oNLmCXfV1t3re15vcSVQwcludZV4SMT+FPiMFtQOHPXynyveXGgcKnTRc0Wv4Wo0fsWXst5jqfRIPtfFEaa+UMWkrzayTWLbgIL0NHB82VjWZmIGU9xOY3Q2fqQbrA7p515jjkGENnXYBejx07NoOQ0Zl2jllvT9J47Vv9yoDSj9XJNszY+GoM96dCdpFngYf2TywnlHG18feQyd8oG7QfJ8XLg6oKj5+jVwL4XVW2QD6p9vdYGozcgyMUqON9A5MX1cbyT6dSI2C+nhtLR3l/IGEbFvi51WBhT5le2vKsea6mPs9erX3f/FefV61nrMj+Hp+1QQKtbgVy91f+v3eQrUa98/PrnPghM/V8N10sTyV/aij9zhA6ZR78Jvyjn0G0d3Tg+YQY4ij4=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 6614a214-2194-4139-f42d-08dc2bdacf31
X-MS-Exchange-CrossTenant-AuthSource: SA2PR10MB4684.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 Feb 2024 14:56:36.0249
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: GkYcka4AKHr3gZZQUZtzqbII0u6nZa8/ZqdmSeczwCqcfuG4Woc1CmvisoiYFcabBSZ/MLD+xPw70SgmP76yjj0YqA6Lrx6V4jfeevutpX0=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR10MB6352
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.1011,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2024-02-12_12,2024-02-12_03,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxscore=0 bulkscore=0 suspectscore=0
 phishscore=0 adultscore=0 spamscore=0 mlxlogscore=999 malwarescore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2311290000
 definitions=main-2402120113
X-Proofpoint-GUID: 3K56VeA2O8Wh2SvJ4fvy7a7BkZw8qdA5
X-Proofpoint-ORIG-GUID: 3K56VeA2O8Wh2SvJ4fvy7a7BkZw8qdA5

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@redhat.com>"
>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> 
> 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:

    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
>