[virtio-dev] [VIRTIO GPU PATCH v3 0/1] Add new feature flag VIRTIO_GPU_F_FREEZE_S3

[virtio-dev] [VIRTIO GPU PATCH v3 0/1] Add new feature flag VIRTIO_GPU_F_FREEZE_S3

[Jiqian Chen]

v3:

Hi all,
Thanks for Gerd Hoffmann's advice. V3 makes below changes:
* Use enum for freeze mode, so this can be extended with more
  modes in the future.
* Rename functions and paratemers with "_S3" postfix.
* Explain in more detail

And latest version on QEMU and Linux kernel side:
	QEMU: https://lore.kernel.org/qemu-devel/20230720120816.8751-1-Jiqian.Chen@amd.com
	Kernel: https://lore.kernel.org/lkml/20230720115805.8206-1-Jiqian.Chen@amd.com/T/#t

Best regards,
Jiqian Chen.


v2:
link,
https://lists.oasis-open.org/archives/virtio-comment/202307/msg00160.html

Hi all,
Thanks to Gerd Hoffmann for his suggestions. V2 makes below changes:
* Elaborate on the types of resources.
* Add some descriptions for S3 and S4.


v1:
link,
https://lists.oasis-open.org/archives/virtio-comment/202306/msg00595.html

Hi all,
I am working to implement virtgpu S3 function on Xen.

Currently on Xen, if we start a guest through Qemu with enabling virtgpu,
and then suspend and s3resume guest. We can find that the guest kernel
comes back, but the display doesn't. It just shown a black screen.

That is because when guest was during suspending, it called into Qemu and
Qemu destroyed all resources and reset renderer. This made the display
gone after guest resumed.

So, I add a mechanism that when guest is suspending, it will notify Qemu,
and then Qemu will not destroy resources. That can help guest's display
come back.

As discussed and suggested by Robert Beckett and Gerd Hoffmann on v1
qemu's mailing list. Due to that mechanism needs cooperation between
guest and host. What's more, as virtio drivers by design paravirt
drivers, it is reasonable for guest to accept some cooperation with host
to manage suspend/resume. So I request to add a new feature flag, so that
guest and host can negotiate whenever freezing is supported or not.

Jiqian Chen (1):
  virtio-gpu: Add new feature flag VIRTIO_GPU_F_FREEZE_S3

 device-types/gpu/description.tex | 42 ++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

-- 
2.34.1


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] [VIRTIO GPU PATCH v3 1/1] virtio-gpu: Add new feature flag VIRTIO_GPU_F_FREEZE_S3

[Jiqian Chen]

When we suspend/resume guest on Xen, the display can't come back.
This is because when guest suspended, it called into Qemu. Then
Qemu destroyed all resources which is used for display. So that
guest's display can't come back to the time when it was suspended.

To solve above problem, I added a new mechanism that when guest is
suspending, it will notify Qemu, and then Qemu will not destroy
resourcesi which are created by using commands
VIRTIO_GPU_CMD_RESOURCE_CREATE_*.

Due to that mechanism needs cooperation between guest and host,
I need to add a new feature flag, so that guest and host can
negotiate whenever freeze_S3 is supported or not.

Signed-off-by: Jiqian Chen <Jiqian.Chen@amd.com>
---
 device-types/gpu/description.tex | 42 ++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/device-types/gpu/description.tex b/device-types/gpu/description.tex
index 4435248..1a137e7 100644
--- a/device-types/gpu/description.tex
+++ b/device-types/gpu/description.tex
@@ -37,6 +37,8 @@ \subsection{Feature bits}\label{sec:Device Types / GPU Device / Feature bits}
   resources is supported.
 \item[VIRTIO_GPU_F_CONTEXT_INIT (4)] multiple context types and
   synchronization timelines supported.  Requires VIRTIO_GPU_F_VIRGL.
+\item[VIRTIO_GPU_F_FREEZE_S3 (5)] freezing virtio-gpu and keeping resources
+  alive is supported.
 \end{description}
 
 \subsection{Device configuration layout}\label{sec:Device Types / GPU Device / Device configuration layout}
@@ -228,6 +230,9 @@ \subsubsection{Device Operation: Request header}\label{sec:Device Types / GPU De
         VIRTIO_GPU_CMD_UPDATE_CURSOR = 0x0300,
         VIRTIO_GPU_CMD_MOVE_CURSOR,
 
+        /* freeze mode */
+        VIRTIO_GPU_CMD_SET_FREEZE_MODE = 0x0400,
+
         /* success responses */
         VIRTIO_GPU_RESP_OK_NODATA = 0x1100,
         VIRTIO_GPU_RESP_OK_DISPLAY_INFO,
@@ -838,6 +843,43 @@ \subsubsection{Device Operation: cursorq}\label{sec:Device Types / GPU Device /
 
 \end{description}
 
+\subsubsection{Device Operation: freeze_mode}\label{sec:Device Types / GPU Device / Device Operation / Device Operation: freeze_mode}
+
+\begin{lstlisting}
+typedef enum {
+  VIRTIO_GPU_FREEZE_MODE_UNFREEZE = 0,
+  VIRTIO_GPU_FREEZE_MODE_FREEZE_S3 = 3,
+} virtio_gpu_freeze_mode_t;
+
+struct virtio_gpu_set_freeze_mode {
+  struct virtio_gpu_ctrl_hdr hdr;
+  virtio_gpu_freeze_mode_t freeze_mode;
+};
+\end{lstlisting}
+
+\begin{description}
+
+\item[VIRTIO_GPU_CMD_SET_FREEZE_MODE]
+Notify freeze mode through controlq.
+Request data is \field{struct virtio_gpu_set_freeze_mode}.
+Response type is VIRTIO_GPU_RESP_OK_NODATA.
+
+This is added for S3 function in guest with virtio-gpu. When guest does
+S3, let it notify QEMU that virtio-gpu is in what freeze mode in
+\field{freeze_mode}. VIRTIO_GPU_FREEZE_MODE_FREEZE_S3 means guest is
+doing S3 and virtio-gpu will be freezed, VIRTIO_GPU_FREEZE_MODE_UNFREEZE
+means virtio-gpu can be used as usual. When virtio-gpu is freezed, QEMU
+will not destroy resources which are created by using commands
+VIRTIO_GPU_CMD_RESOURCE_CREATE_*, so that guest can use those resources
+to resume display.
+
+Note: this change is not enough to solve the problems of S4 function.
+QEMU may lose resources after hibernation. It needs more research and
+development. If S4 is supported in the future, it may need another
+feature flag here.
+
+\end{description}
+
 \subsection{VGA Compatibility}\label{sec:Device Types / GPU Device / VGA Compatibility}
 
 Applies to Virtio Over PCI only.  The GPU device can come with and
-- 
2.34.1


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [VIRTIO GPU PATCH v3 0/1] Add new feature flag VIRTIO_GPU_F_FREEZE_S3

[Chen, Jiqian]

Hi all,

Do you have any other comments on the modification of virtio-gpu S3?
Looking forward to your reply and comments.

On 2023/7/20 20:18, Jiqian Chen wrote:
> v3:
> 
> Hi all,
> Thanks for Gerd Hoffmann's advice. V3 makes below changes:
> * Use enum for freeze mode, so this can be extended with more
>   modes in the future.
> * Rename functions and paratemers with "_S3" postfix.
> * Explain in more detail
> 
> And latest version on QEMU and Linux kernel side:
> 	QEMU: https://lore.kernel.org/qemu-devel/20230720120816.8751-1-Jiqian.Chen@amd.com
> 	Kernel: https://lore.kernel.org/lkml/20230720115805.8206-1-Jiqian.Chen@amd.com/T/#t
> 
> Best regards,
> Jiqian Chen.
> 
> 
> v2:
> link,
> https://lists.oasis-open.org/archives/virtio-comment/202307/msg00160.html
> 
> Hi all,
> Thanks to Gerd Hoffmann for his suggestions. V2 makes below changes:
> * Elaborate on the types of resources.
> * Add some descriptions for S3 and S4.
> 
> 
> v1:
> link,
> https://lists.oasis-open.org/archives/virtio-comment/202306/msg00595.html
> 
> Hi all,
> I am working to implement virtgpu S3 function on Xen.
> 
> Currently on Xen, if we start a guest through Qemu with enabling virtgpu,
> and then suspend and s3resume guest. We can find that the guest kernel
> comes back, but the display doesn't. It just shown a black screen.
> 
> That is because when guest was during suspending, it called into Qemu and
> Qemu destroyed all resources and reset renderer. This made the display
> gone after guest resumed.
> 
> So, I add a mechanism that when guest is suspending, it will notify Qemu,
> and then Qemu will not destroy resources. That can help guest's display
> come back.
> 
> As discussed and suggested by Robert Beckett and Gerd Hoffmann on v1
> qemu's mailing list. Due to that mechanism needs cooperation between
> guest and host. What's more, as virtio drivers by design paravirt
> drivers, it is reasonable for guest to accept some cooperation with host
> to manage suspend/resume. So I request to add a new feature flag, so that
> guest and host can negotiate whenever freezing is supported or not.
> 
> Jiqian Chen (1):
>   virtio-gpu: Add new feature flag VIRTIO_GPU_F_FREEZE_S3
> 
>  device-types/gpu/description.tex | 42 ++++++++++++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
> 

-- 
Best regards,
Jiqian Chen.

[virtio-dev] RE: [VIRTIO GPU PATCH v3 0/1] Add new feature flag VIRTIO_GPU_F_FREEZE_S3

[Parav Pandit]

> From: virtio-dev@lists.oasis-open.org <virtio-dev@lists.oasis-open.org> On
> Behalf Of Chen, Jiqian
> Sent: Wednesday, August 2, 2023 8:51 AM
> Hi all,
> 
> Do you have any other comments on the modification of virtio-gpu S3?
> Looking forward to your reply and comments.
> 
I am not familiar with the GPU, so a dumb question is, why the S3 state is gpu specific?
Can a transport specific suspend state be used and apply to all virtio devices?
And can you please add both the rationale to the commit message?

> On 2023/7/20 20:18, Jiqian Chen wrote:
> > v3:
> >
> > Hi all,
> > Thanks for Gerd Hoffmann's advice. V3 makes below changes:
> > * Use enum for freeze mode, so this can be extended with more
> >   modes in the future.
> > * Rename functions and paratemers with "_S3" postfix.
> > * Explain in more detail
> >
> > And latest version on QEMU and Linux kernel side:
> > 	QEMU: https://lore.kernel.org/qemu-devel/20230720120816.8751-1-
> Jiqian.Chen@amd.com
> > 	Kernel:
> > https://lore.kernel.org/lkml/20230720115805.8206-1-Jiqian.Chen@amd.com
> > /T/#t
> >
> > Best regards,
> > Jiqian Chen.
> >
> >
> > v2:
> > link,
> > https://lists.oasis-open.org/archives/virtio-comment/202307/msg00160.h
> > tml
> >
> > Hi all,
> > Thanks to Gerd Hoffmann for his suggestions. V2 makes below changes:
> > * Elaborate on the types of resources.
> > * Add some descriptions for S3 and S4.
> >
> >
> > v1:
> > link,
> > https://lists.oasis-open.org/archives/virtio-comment/202306/msg00595.h
> > tml
> >
> > Hi all,
> > I am working to implement virtgpu S3 function on Xen.
> >
> > Currently on Xen, if we start a guest through Qemu with enabling
> > virtgpu, and then suspend and s3resume guest. We can find that the
> > guest kernel comes back, but the display doesn't. It just shown a black screen.
> >
> > That is because when guest was during suspending, it called into Qemu
> > and Qemu destroyed all resources and reset renderer. This made the
> > display gone after guest resumed.
> >
> > So, I add a mechanism that when guest is suspending, it will notify
> > Qemu, and then Qemu will not destroy resources. That can help guest's
> > display come back.
> >
> > As discussed and suggested by Robert Beckett and Gerd Hoffmann on v1
> > qemu's mailing list. Due to that mechanism needs cooperation between
> > guest and host. What's more, as virtio drivers by design paravirt
> > drivers, it is reasonable for guest to accept some cooperation with
> > host to manage suspend/resume. So I request to add a new feature flag,
> > so that guest and host can negotiate whenever freezing is supported or not.
> >
> > Jiqian Chen (1):
> >   virtio-gpu: Add new feature flag VIRTIO_GPU_F_FREEZE_S3
> >
> >  device-types/gpu/description.tex | 42
> > ++++++++++++++++++++++++++++++++
> >  1 file changed, 42 insertions(+)
> >
> 
> --
> Best regards,
> Jiqian Chen.

[virtio-dev] Re: [VIRTIO GPU PATCH v3 0/1] Add new feature flag VIRTIO_GPU_F_FREEZE_S3

[Chen, Jiqian]

On 2023/8/2 12:49, Parav Pandit wrote:
> 
> 
>> From: virtio-dev@lists.oasis-open.org <virtio-dev@lists.oasis-open.org> On
>> Behalf Of Chen, Jiqian
>> Sent: Wednesday, August 2, 2023 8:51 AM
>> Hi all,
>>
>> Do you have any other comments on the modification of virtio-gpu S3?
>> Looking forward to your reply and comments.
>>

Hi Parav Pandit,
Thank you for your reply. Let me try to answer your question.

> I am not familiar with the GPU, so a dumb question is, why the S3 state is gpu specific?
S3 state is not gpu specific. I think different virtio devices may have different actions/problems when S3. 
When I do S3 on Xen, I found guest's display can't come back and the root cause is in virtio-gpu backend in QEMU.
So, to solve that problem, I change codes to let guest notify QEMU virtio-gpu's suspend state, and then QEMU will not destroy resources that used for display.
Please see attached kernel and QEMU patch links.
For above reason, Gerd suggest me to add a new feature flag specifically for virtio-gpu, so that guest and host can negotiate whenever to enable above mechanism.

> Can a transport specific suspend state be used and apply to all virtio devices?
Based on my limited knowledge. Different virtio devices have different virtio queues, my modifications let guest to notify QEMU by using virtio-gpu's control queue.
Other virtio devices can't get that notification unless you can traverse all virtio devices and notify one by one, or other global method?
But for now, this patch is to add a new feature flag just used for virtio-gpu.

> And can you please add both the rationale to the commit message?
Sure, I will expand the description of my commit message and add them.

> 
>> On 2023/7/20 20:18, Jiqian Chen wrote:
>>> v3:
>>>
>>> Hi all,
>>> Thanks for Gerd Hoffmann's advice. V3 makes below changes:
>>> * Use enum for freeze mode, so this can be extended with more
>>>   modes in the future.
>>> * Rename functions and paratemers with "_S3" postfix.
>>> * Explain in more detail
>>>
>>> And latest version on QEMU and Linux kernel side:
>>> 	QEMU: https://lore.kernel.org/qemu-devel/20230720120816.8751-1-
>> Jiqian.Chen@amd.com
>>> 	Kernel:
>>> https://lore.kernel.org/lkml/20230720115805.8206-1-Jiqian.Chen@amd.com
>>> /T/#t
>>>
>>> Best regards,
>>> Jiqian Chen.
>>>
>>>
>>> v2:
>>> link,
>>> https://lists.oasis-open.org/archives/virtio-comment/202307/msg00160.h
>>> tml
>>>
>>> Hi all,
>>> Thanks to Gerd Hoffmann for his suggestions. V2 makes below changes:
>>> * Elaborate on the types of resources.
>>> * Add some descriptions for S3 and S4.
>>>
>>>
>>> v1:
>>> link,
>>> https://lists.oasis-open.org/archives/virtio-comment/202306/msg00595.h
>>> tml
>>>
>>> Hi all,
>>> I am working to implement virtgpu S3 function on Xen.
>>>
>>> Currently on Xen, if we start a guest through Qemu with enabling
>>> virtgpu, and then suspend and s3resume guest. We can find that the
>>> guest kernel comes back, but the display doesn't. It just shown a black screen.
>>>
>>> That is because when guest was during suspending, it called into Qemu
>>> and Qemu destroyed all resources and reset renderer. This made the
>>> display gone after guest resumed.
>>>
>>> So, I add a mechanism that when guest is suspending, it will notify
>>> Qemu, and then Qemu will not destroy resources. That can help guest's
>>> display come back.
>>>
>>> As discussed and suggested by Robert Beckett and Gerd Hoffmann on v1
>>> qemu's mailing list. Due to that mechanism needs cooperation between
>>> guest and host. What's more, as virtio drivers by design paravirt
>>> drivers, it is reasonable for guest to accept some cooperation with
>>> host to manage suspend/resume. So I request to add a new feature flag,
>>> so that guest and host can negotiate whenever freezing is supported or not.
>>>
>>> Jiqian Chen (1):
>>>   virtio-gpu: Add new feature flag VIRTIO_GPU_F_FREEZE_S3
>>>
>>>  device-types/gpu/description.tex | 42
>>> ++++++++++++++++++++++++++++++++
>>>  1 file changed, 42 insertions(+)
>>>
>>
>> --
>> Best regards,
>> Jiqian Chen.

-- 
Best regards,
Jiqian Chen.

[virtio-dev] RE: [VIRTIO GPU PATCH v3 0/1] Add new feature flag VIRTIO_GPU_F_FREEZE_S3

[Parav Pandit]

> From: Chen, Jiqian <Jiqian.Chen@amd.com>
> Sent: Wednesday, August 2, 2023 11:28 AM
> 
> On 2023/8/2 12:49, Parav Pandit wrote:
> >
> >
> >> From: virtio-dev@lists.oasis-open.org
> >> <virtio-dev@lists.oasis-open.org> On Behalf Of Chen, Jiqian
> >> Sent: Wednesday, August 2, 2023 8:51 AM Hi all,
> >>
> >> Do you have any other comments on the modification of virtio-gpu S3?
> >> Looking forward to your reply and comments.
> >>
> 
> Hi Parav Pandit,
> Thank you for your reply. Let me try to answer your question.
> 
> > I am not familiar with the GPU, so a dumb question is, why the S3 state is gpu
> specific?
> S3 state is not gpu specific. I think different virtio devices may have different
> actions/problems when S3.
I am making assumption that the gpu device is pci. :)
If so can you please use the transport specific notification from gpu guest driver to notify to qemu?

> When I do S3 on Xen, I found guest's display can't come back and the root cause
> is in virtio-gpu backend in QEMU.
> So, to solve that problem, I change codes to let guest notify QEMU virtio-gpu's
> suspend state, and then QEMU will not destroy resources that used for display.
> Please see attached kernel and QEMU patch links.
> For above reason, Gerd suggest me to add a new feature flag specifically for
> virtio-gpu, so that guest and host can negotiate whenever to enable above
> mechanism.
> 
> > Can a transport specific suspend state be used and apply to all virtio devices?
> Based on my limited knowledge. Different virtio devices have different virtio
> queues, my modifications let guest to notify QEMU by using virtio-gpu's control
> queue.
> Other virtio devices can't get that notification unless you can traverse all virtio
> devices and notify one by one, or other global method?
> But for now, this patch is to add a new feature flag just used for virtio-gpu.
> 
If this is done at the pci transport level, all the virtio device benefit from it without inventing in each device type.

> > And can you please add both the rationale to the commit message?
> Sure, I will expand the description of my commit message and add them.
> 
> >
> >> On 2023/7/20 20:18, Jiqian Chen wrote:
> >>> v3:
> >>>
> >>> Hi all,
> >>> Thanks for Gerd Hoffmann's advice. V3 makes below changes:
> >>> * Use enum for freeze mode, so this can be extended with more
> >>>   modes in the future.
> >>> * Rename functions and paratemers with "_S3" postfix.
> >>> * Explain in more detail
> >>>
> >>> And latest version on QEMU and Linux kernel side:
> >>> 	QEMU: https://lore.kernel.org/qemu-devel/20230720120816.8751-1-
> >> Jiqian.Chen@amd.com
> >>> 	Kernel:
> >>> https://lore.kernel.org/lkml/20230720115805.8206-1-Jiqian.Chen@amd.c
> >>> om
> >>> /T/#t
> >>>
> >>> Best regards,
> >>> Jiqian Chen.
> >>>
> >>>
> >>> v2:
> >>> link,
> >>> https://lists.oasis-open.org/archives/virtio-comment/202307/msg00160
> >>> .h
> >>> tml
> >>>
> >>> Hi all,
> >>> Thanks to Gerd Hoffmann for his suggestions. V2 makes below changes:
> >>> * Elaborate on the types of resources.
> >>> * Add some descriptions for S3 and S4.
> >>>
> >>>
> >>> v1:
> >>> link,
> >>> https://lists.oasis-open.org/archives/virtio-comment/202306/msg00595
> >>> .h
> >>> tml
> >>>
> >>> Hi all,
> >>> I am working to implement virtgpu S3 function on Xen.
> >>>
> >>> Currently on Xen, if we start a guest through Qemu with enabling
> >>> virtgpu, and then suspend and s3resume guest. We can find that the
> >>> guest kernel comes back, but the display doesn't. It just shown a black
> screen.
> >>>
> >>> That is because when guest was during suspending, it called into
> >>> Qemu and Qemu destroyed all resources and reset renderer. This made
> >>> the display gone after guest resumed.
> >>>
> >>> So, I add a mechanism that when guest is suspending, it will notify
> >>> Qemu, and then Qemu will not destroy resources. That can help
> >>> guest's display come back.
> >>>
> >>> As discussed and suggested by Robert Beckett and Gerd Hoffmann on v1
> >>> qemu's mailing list. Due to that mechanism needs cooperation between
> >>> guest and host. What's more, as virtio drivers by design paravirt
> >>> drivers, it is reasonable for guest to accept some cooperation with
> >>> host to manage suspend/resume. So I request to add a new feature
> >>> flag, so that guest and host can negotiate whenever freezing is supported or
> not.
> >>>
> >>> Jiqian Chen (1):
> >>>   virtio-gpu: Add new feature flag VIRTIO_GPU_F_FREEZE_S3
> >>>
> >>>  device-types/gpu/description.tex | 42
> >>> ++++++++++++++++++++++++++++++++
> >>>  1 file changed, 42 insertions(+)
> >>>
> >>
> >> --
> >> Best regards,
> >> Jiqian Chen.
> 
> --
> Best regards,
> Jiqian Chen.

[virtio-dev] Re: [VIRTIO GPU PATCH v3 0/1] Add new feature flag VIRTIO_GPU_F_FREEZE_S3

[Chen, Jiqian]

Hi,

On 2023/8/2 15:13, Parav Pandit wrote:
> 
>> From: Chen, Jiqian <Jiqian.Chen@amd.com>
>> Sent: Wednesday, August 2, 2023 11:28 AM
>>
>> On 2023/8/2 12:49, Parav Pandit wrote:
>>>
>>>
>>>> From: virtio-dev@lists.oasis-open.org
>>>> <virtio-dev@lists.oasis-open.org> On Behalf Of Chen, Jiqian
>>>> Sent: Wednesday, August 2, 2023 8:51 AM Hi all,
>>>>
>>>> Do you have any other comments on the modification of virtio-gpu S3?
>>>> Looking forward to your reply and comments.
>>>>
>>
>> Hi Parav Pandit,
>> Thank you for your reply. Let me try to answer your question.
>>
>>> I am not familiar with the GPU, so a dumb question is, why the S3 state is gpu
>> specific?
>> S3 state is not gpu specific. I think different virtio devices may have different
>> actions/problems when S3.
> I am making assumption that the gpu device is pci. :)
> If so can you please use the transport specific notification from gpu guest driver to notify to qemu?
In my existing implementation, qemu is notified through it(gpu guest driver queue).

> 
>> When I do S3 on Xen, I found guest's display can't come back and the root cause
>> is in virtio-gpu backend in QEMU.
>> So, to solve that problem, I change codes to let guest notify QEMU virtio-gpu's
>> suspend state, and then QEMU will not destroy resources that used for display.
>> Please see attached kernel and QEMU patch links.
>> For above reason, Gerd suggest me to add a new feature flag specifically for
>> virtio-gpu, so that guest and host can negotiate whenever to enable above
>> mechanism.
>>
>>> Can a transport specific suspend state be used and apply to all virtio devices?
>> Based on my limited knowledge. Different virtio devices have different virtio
>> queues, my modifications let guest to notify QEMU by using virtio-gpu's control
>> queue.
>> Other virtio devices can't get that notification unless you can traverse all virtio
>> devices and notify one by one, or other global method?
>> But for now, this patch is to add a new feature flag just used for virtio-gpu.
>>
> If this is done at the pci transport level, all the virtio device benefit from it without inventing in each device type.
I think there is no need to improve to pci level. Because this feature is a compromise solution,
if resources are destroyed on qemu side, guest has not enough data to re-create them,
so we choose to keep them when suspension. That is a virtio-gpu specific scenario.

What's your opinion, Gerd Hoffmann, and Robert Beckett? Am I right?

> 
>>> And can you please add both the rationale to the commit message?
>> Sure, I will expand the description of my commit message and add them.
>>
>>>
>>>> On 2023/7/20 20:18, Jiqian Chen wrote:
>>>>> v3:
>>>>>
>>>>> Hi all,
>>>>> Thanks for Gerd Hoffmann's advice. V3 makes below changes:
>>>>> * Use enum for freeze mode, so this can be extended with more
>>>>>   modes in the future.
>>>>> * Rename functions and paratemers with "_S3" postfix.
>>>>> * Explain in more detail
>>>>>
>>>>> And latest version on QEMU and Linux kernel side:
>>>>> 	QEMU: https://lore.kernel.org/qemu-devel/20230720120816.8751-1-
>>>> Jiqian.Chen@amd.com
>>>>> 	Kernel:
>>>>> https://lore.kernel.org/lkml/20230720115805.8206-1-Jiqian.Chen@amd.c
>>>>> om
>>>>> /T/#t
>>>>>
>>>>> Best regards,
>>>>> Jiqian Chen.
>>>>>
>>>>>
>>>>> v2:
>>>>> link,
>>>>> https://lists.oasis-open.org/archives/virtio-comment/202307/msg00160
>>>>> .h
>>>>> tml
>>>>>
>>>>> Hi all,
>>>>> Thanks to Gerd Hoffmann for his suggestions. V2 makes below changes:
>>>>> * Elaborate on the types of resources.
>>>>> * Add some descriptions for S3 and S4.
>>>>>
>>>>>
>>>>> v1:
>>>>> link,
>>>>> https://lists.oasis-open.org/archives/virtio-comment/202306/msg00595
>>>>> .h
>>>>> tml
>>>>>
>>>>> Hi all,
>>>>> I am working to implement virtgpu S3 function on Xen.
>>>>>
>>>>> Currently on Xen, if we start a guest through Qemu with enabling
>>>>> virtgpu, and then suspend and s3resume guest. We can find that the
>>>>> guest kernel comes back, but the display doesn't. It just shown a black
>> screen.
>>>>>
>>>>> That is because when guest was during suspending, it called into
>>>>> Qemu and Qemu destroyed all resources and reset renderer. This made
>>>>> the display gone after guest resumed.
>>>>>
>>>>> So, I add a mechanism that when guest is suspending, it will notify
>>>>> Qemu, and then Qemu will not destroy resources. That can help
>>>>> guest's display come back.
>>>>>
>>>>> As discussed and suggested by Robert Beckett and Gerd Hoffmann on v1
>>>>> qemu's mailing list. Due to that mechanism needs cooperation between
>>>>> guest and host. What's more, as virtio drivers by design paravirt
>>>>> drivers, it is reasonable for guest to accept some cooperation with
>>>>> host to manage suspend/resume. So I request to add a new feature
>>>>> flag, so that guest and host can negotiate whenever freezing is supported or
>> not.
>>>>>
>>>>> Jiqian Chen (1):
>>>>>   virtio-gpu: Add new feature flag VIRTIO_GPU_F_FREEZE_S3
>>>>>
>>>>>  device-types/gpu/description.tex | 42
>>>>> ++++++++++++++++++++++++++++++++
>>>>>  1 file changed, 42 insertions(+)
>>>>>
>>>>
>>>> --
>>>> Best regards,
>>>> Jiqian Chen.
>>
>> --
>> Best regards,
>> Jiqian Chen.

-- 
Best regards,
Jiqian Chen.

[virtio-dev] RE: [VIRTIO GPU PATCH v3 0/1] Add new feature flag VIRTIO_GPU_F_FREEZE_S3

[Parav Pandit]

> From: Robert Beckett <bob.beckett@collabora.com>
> Sent: Thursday, August 3, 2023 9:59 PM

[..]

> > I think there is no need to improve to pci level. Because this feature
> > is a compromise solution, if resources are destroyed on qemu side,
> > guest has not enough data to re-create them, so we choose to keep them
> when suspension. That is a virtio-gpu specific scenario.
> >
> > What's your opinion, Gerd Hoffmann, and Robert Beckett? Am I right?
> 
> I think Parav asks a good question.
> 
> While the initial rationale for doing this work is virtio-gpu specific, the potential
> implementation of a solution need not remain gpu specific.
> If we want to genericise the solution, then then question becomes at what level
> should it be done?
> 
> Reading section 4.1.4.3 of the v1.2 spec, it looks like writing 0 to device_status
> as exposed via an MMIO BAR is the PCI transport reset method.
> We could add a new device_status value.
> device_status should be exposed in all transport methods, so we could update
> the spec section 2.1 to say
> 
> 
> DRIVER_SUSPEND (5)
> 
> Or something similar.
> 
> Alternatively a new driver_param register could be exposed that could indicate
> suspend (and any future other parameters to status changes).
> 
> 
> I guess it depends how significant a change we want to use to support
> this and how useful the are predicted to be in future.
> Are other people aware of any other desired parameterisation of
> device_status updates?
>
Some of the stateless devices may be just fine to destroy and recreate the resources in the PCI's driver's callback of suspend() and resume().

And DRIVER_SUSPEND may not be even needed.

Some stateful device may need to resume from the same place where it left off.
I do not know about gpu enough..

My question is: in the virtio gpu driver, can you use PCI's suspend() and resume() callbacks to destroy and recreate the needed resources?
This way, guest driver handles what it needs to, flow is also symmetric.
From QEMU point of view, its driver reinitializing, so QEMU should be just fine. (not just destroy things)
Why wouldn’t it work? Will it be slow or?
Have you explored this route?

For example, radeon300 gpu seems to reset the device during resuming and seems to be fine without persisting.

[virtio-dev] Re: [VIRTIO GPU PATCH v3 0/1] Add new feature flag VIRTIO_GPU_F_FREEZE_S3

[Chen, Jiqian]

Hi,

On 2023/8/4 14:56, Parav Pandit wrote:
> 
> 
>> From: Robert Beckett <bob.beckett@collabora.com>
>> Sent: Thursday, August 3, 2023 9:59 PM
> 
> [..]
> 
>>> I think there is no need to improve to pci level. Because this feature
>>> is a compromise solution, if resources are destroyed on qemu side,
>>> guest has not enough data to re-create them, so we choose to keep them
>> when suspension. That is a virtio-gpu specific scenario.
>>>
>>> What's your opinion, Gerd Hoffmann, and Robert Beckett? Am I right?
>>
>> I think Parav asks a good question.
>>
>> While the initial rationale for doing this work is virtio-gpu specific, the potential
>> implementation of a solution need not remain gpu specific.
>> If we want to genericise the solution, then then question becomes at what level
>> should it be done?
>>
>> Reading section 4.1.4.3 of the v1.2 spec, it looks like writing 0 to device_status
>> as exposed via an MMIO BAR is the PCI transport reset method.
>> We could add a new device_status value.
>> device_status should be exposed in all transport methods, so we could update
>> the spec section 2.1 to say
>>
>>
>> DRIVER_SUSPEND (5)
>>
>> Or something similar.
>>
>> Alternatively a new driver_param register could be exposed that could indicate
>> suspend (and any future other parameters to status changes).
>>
>>
>> I guess it depends how significant a change we want to use to support
>> this and how useful the are predicted to be in future.
>> Are other people aware of any other desired parameterisation of
>> device_status updates?
>>
> Some of the stateless devices may be just fine to destroy and recreate the resources in the PCI's driver's callback of suspend() and resume().
> 
> And DRIVER_SUSPEND may not be even needed.
> 
> Some stateful device may need to resume from the same place where it left off.
> I do not know about gpu enough..
Yes, we want after resuming, the display and applications of guest will restore the state when guest are suspended.

> 
> My question is: in the virtio gpu driver, can you use PCI's suspend() and resume() callbacks to destroy and recreate the needed resources?
> This way, guest driver handles what it needs to, flow is also symmetric.
> From QEMU point of view, its driver reinitializing, so QEMU should be just fine. (not just destroy things)
> Why wouldn’t it work? Will it be slow or?
> Have you explored this route?
Understand. Your questions have been discussed in the Qemu mailing thread. 
The point is not where to destroy the resources, it is how to re-create resources.
But in current codes, the guest virtio-gpu driver wouldn't have any of the pixel data to transfer to host after resume. And Mesa often will decide that it won't need to access a resource again after initial upload.
So, for guest driver, it is not able to reproduce the resources including pixel data or gl rendering.
Based on the above reasons, Gerd and Bob agree to retain resources and suggest adding a new feature flag.

> 
> For example, radeon300 gpu seems to reset the device during resuming and seems to be fine without persisting.

-- 
Best regards,
Jiqian Chen.