Re: [virtio-comment] RE: [PATCH v6 1/1] content: Add new feature VIRTIO_F_PRESERVE_RESOURCES

["Chen, Jiqian" <Jiqian.Chen@amd.com>]

On 2024/1/15 18:52, Parav Pandit wrote:
> 
>> From: Chen, Jiqian <Jiqian.Chen@amd.com>
>> Sent: Monday, January 15, 2024 4:17 PM
>>
>> On 2024/1/15 17:46, Parav Pandit wrote:
>>>
>>>> From: Chen, Jiqian <Jiqian.Chen@amd.com>
>>>> Sent: Monday, January 15, 2024 3:10 PM
>>>
>>>>> Display resources should be only restored when following conditions
>>>>> are
>>>> met.
>>>>> 1. PCI PM cap is reported.
>>>>> 2. PCI PM cap has non_soft_reset=1
>>>>> 3. virtio guest driver do not perform reset when transport offers a
>>>>> restore
>>>> capability using #1 and #2.
>>>>>
>>>>> Do you agree? Yes?
>>>> Yes, I think this problem (display resources are destroyed during S3)
>>>> can be sorted to two situations:
>>>> First is what you said above, in this situation, the devices_reset of
>>>> qemu is unreasonable, if a device has PM cap and non_soft_reset=1,
>>>> qemu should not do resetting.
>>>
>>>> Second is without #1 or #2, the reset behavior is fine. My patch is
>>>> to fix this situation, that sending a new virtio-gpu command to
>>>> notify qemu to prevent destroying display resources during S3.
>>>
>>> I still have hard time following "My patch is to fix this situation...".
>>>
>>> When #1 and #2 is not done, there is nothing to restore.
>>> Driver should not send some new virtio specific command when #1 and #2 is
>> not there.
>>> Instead, if the device wants to restore the context, all #1, #2 and #3 should
>> be done to implement the restore functionality.
>> When #1 and #2 is done. The "device reset behavior" should not happen. And
>> then the display resources are not destroyed.
>> I didnât say send command to restore context.
> 
>> I mean when #1 and #2 is not done. Driver and qemu both reset devices when
>> resuming, at that time, 
> Above behavior is as per the spec guidelines. Hence it is fine.
> 
>> we need a method to prevent the display resources from destroying.
> I disagree to above as it is not as per the spec guidelines.
> Just follow #1, #2 and #3 to not destroy the display resource destroying.
I agree that follow #1, #2 and #3 will not destroy display resource.
So the question goes back: If there is no #2 and you say that the reset behavior complies with the spec guidelines, how can I make the virtio gpu work properly with display resources destroyed?
What my patch do is not to prevent resetting, is to prevent destroying resources during resetting. Gerd Hoffmann has agreed to this point in my qemu patch reviewing.
Even if I use PM cap, I still need to prevent destroying resources during resetting.

> No need for any check command etc.
> 
>> I don't mean anything else, I just feel like you may not have understood the
>> issues I encountered during the S3 process and the implementation logic of
>> my patch from the beginning.
>> In current kernel and qemu codes, when we do S3(suspend and resume) for
>> guest, during the resuming, qemu and guest driver will reset the virtio-
>> gpu(because #2 is not done), but once the resetting happens, the display
>> resources will be destroyed and we can't restore, so we must add a method to
>> prevent destroying resources at that situation. What my patch do is to add a
>> new virtio-gpu command to notify qemu to check if the display resources
>> should be destroyed during resetting.
>>
>>>
>>> In other words, if one wants to use device context restore on PM state
>> transition it should do #1, #2 and #3.
>>> (and avoid inventing new infrastructure because PCI PM has necessary
>> things).
>>
>>
>> --
>> Best regards,
>> Jiqian Chen.

-- 
Best regards,
Jiqian Chen.