Re: [PATCH V7 v7] virtio: introduce SUSPEND bit in device status

[Zhu Lingshan <lingshan.zhu@amd.com>]

On 8/15/2024 11:07 PM, Michael S. Tsirkin wrote:
> On Thu, Aug 15, 2024 at 10:59:45AM +0000, Parav Pandit wrote:
>>
>>> From: Michael S. Tsirkin <mst@redhat.com>
>>> Sent: Thursday, August 15, 2024 4:23 PM
>>>
>>> On Tue, Aug 13, 2024 at 06:55:04AM +0000, Parav Pandit wrote:
>>>> That means, PCI HW needs to return suspend=0, until the device is not
>>> suspended.
>>>> In this example, the device cannot build special circuitry to answer
>>> suspend=true within 50nsec, or in other words building special circuitry to
>>> return suspend=false is too complex for the slow operation.
>>>> If this understanding of burden is clear,
>>>>
>>>> The proposal is, can you please extend the interface such that,
>>>>
>>>> 1. driver writes suspend command.
>>>> 2. driver reads suspend_status, and receives not_completed=(false). This is
>>> the default value.
>>>> 3. When the device completes suspend, it changes the polarity of
>>> suspend_status=true.
>>>> This has two main benefits:
>>>> [A] This will enable software-based devices to write data to slow files and
>>> does not have to force VM_EXITs.
>>>> [B] It also enables hw based devices to not build special circuitry to answer
>>> within 50nsec, which can get very complicated for tens or hundreds of PCI
>>> PFs.
>>>
>>> I read this several times, and I don't understand what is proposed.
>>> A special register for suspend/resume? Is this the difference?
>>>
>> Yes, a command register for suspend/resume operation.
>> And device_status new bit that Lingshan defined returns the status of this operation.
>
>
> Ugh, it's all quite messy IMHO.
> We have 4 states:
> - operational (resumed)
> - suspend in progress
> - suspended
> - resume in progress
>
> What I'd do then is a two bit register.
> To suspend: 
> - write suspend in progress
> - re-read, waiting until suspended
> To resume
> - write resume in progress
> - re-read, waiting until operational (resumed)
>
> How does this sound?
This can work for sure. but is it a must?
I mean, the driver has its own knowledge of how it operate the device.
When device presents SUSPEND == 0, It know whether the device is in normal operational state or in the progress of SUSPENDING.

But if you think we should add a new register which applying for all device_status transitions, NOT only for SUSPEND. we can surely do that.

Thanks
>