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

[Parav Pandit <parav@nvidia.com>]

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