Re: [dpdk-dev] [PATCH] net/virtio: wait device ready in device reset

[Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>]

On 8/23/21 4:54 PM, Xueming(Steven) Li wrote:
> 
> 
>> -----Original Message-----
>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>> Sent: Monday, August 23, 2021 5:57 PM
>> To: Xueming(Steven) Li <xuemingl@nvidia.com>
>> Cc: dev@dpdk.org; Maxime Coquelin <maxime.coquelin@redhat.com>; Chenbo Xia <chenbo.xia@intel.com>
>> Subject: Re: [dpdk-dev] [PATCH] net/virtio: wait device ready in device reset
>>
>> On 8/23/21 9:39 AM, Xueming Li wrote:
>>> According to virtio spec, the device MUST reset when 0 is written to
>>> device_status, and present a 0 in device_status once that is done.
>>>
>>> This patch adds the missing part of waiting status 0 in reset function.
>>>
>>> Signed-off-by: Xueming Li <xuemingl@nvidia.com>
>>> ---
>>>  drivers/net/virtio/virtio.c | 7 +++++--
>>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/virtio/virtio.c b/drivers/net/virtio/virtio.c
>>> index 7e1e77797f..f003f612d6 100644
>>> --- a/drivers/net/virtio/virtio.c
>>> +++ b/drivers/net/virtio/virtio.c
>>> @@ -3,6 +3,8 @@
>>>   * Copyright(c) 2020 Red Hat, Inc.
>>>   */
>>>
>>> +#include <unistd.h>
>>> +
>>>  #include "virtio.h"
>>>
>>>  uint64_t
>>> @@ -39,8 +41,9 @@ void
>>>  virtio_reset(struct virtio_hw *hw)
>>>  {
>>>  	VIRTIO_OPS(hw)->set_status(hw, VIRTIO_CONFIG_STATUS_RESET);
>>> -	/* flush status write */
>>> -	VIRTIO_OPS(hw)->get_status(hw);
>>> +	/* Flush status write and wait device ready. */
>>> +	while (VIRTIO_OPS(hw)->get_status(hw) != VIRTIO_CONFIG_STATUS_RESET)
>>> +		usleep(1000L);
>>
>> Don't we need a protection against forever loop here?
> 
> Good question, ideally we need, kernel driver function vp_reset() seems to have same issue.

Yes, I've seen it.

> How about leaving an error message before return? 

@Maxime, @Chenbo, what do you think?