virtio-comment

v2:

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:

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.

Attach the patches of Qemu and kernel:
Qemu
v1: 20230608025655.1674357-2-Jiqian.Chen@amd.com/">https://lore.kernel.org/qemu-devel/20230608025655.1674357-2-Jiqian.Chen@amd.com/
v2: 20230630070016.841459-1-Jiqian.Chen@amd.com/T/#t">https://lore.kernel.org/qemu-devel/20230630070016.841459-1-Jiqian.Chen@amd.com/T/#t
Kernel
v1: 20230608063857.1677973-1-Jiqian.Chen@amd.com/">https://lore.kernel.org/lkml/20230608063857.1677973-1-Jiqian.Chen@amd.com/
v2: 20230630073448.842767-1-Jiqian.Chen@amd.com/T/#t">https://lore.kernel.org/lkml/20230630073448.842767-1-Jiqian.Chen@amd.com/T/#t

Best regards,
Jiqian Chen.

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

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

--
2.34.1

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

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 freezing is supported or not.

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

diff --git a/device-types/gpu/description.tex b/device-types/gpu/description.tex
index 4435248..729dde5 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_FREEZING (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,

+ /* status */
+ VIRTIO_GPU_CMD_STATUS_FREEZING = 0x0400,
+
/* success responses */
VIRTIO_GPU_RESP_OK_NODATA = 0x1100,
VIRTIO_GPU_RESP_OK_DISPLAY_INFO,
@@ -838,6 +843,36 @@ \subsubsection{Device Operation: cursorq}\label{sec:Device Types / GPU Device /

\end{description}

+\subsubsection{Device Operation: status}\label{sec:Device Types / GPU Device / Device Operation / Device Operation: cursorq}
+
+\begin{lstlisting}
+struct virtio_gpu_status_freezing {
+ struct virtio_gpu_ctrl_hdr hdr;
+ __u32 freezing;
+};
+\end{lstlisting}
+
+\begin{description}
+
+\item[VIRTIO_GPU_CMD_STATUS_FREEZING]
+Notify freezing status through controlq.
+Request data is \field{struct virtio_gpu_status_freezing}.
+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 if virtio-gpu is in freezing status or not in
+\field{freezing}. Zero means it is not in freezing status, none-zero
+is the opposite. When virtio-gpu is in freezing status, 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.
+
+\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

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

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 freezing is supported or not.

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

diff --git a/device-types/gpu/description.tex b/device-types/gpu/description.tex
index 4435248..729dde5 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_FREEZING (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,

+ /* status */
+ VIRTIO_GPU_CMD_STATUS_FREEZING = 0x0400,
+
/* success responses */
VIRTIO_GPU_RESP_OK_NODATA = 0x1100,
VIRTIO_GPU_RESP_OK_DISPLAY_INFO,
@@ -838,6 +843,36 @@ \subsubsection{Device Operation: cursorq}\label{sec:Device Types / GPU Device /

\end{description}

+\subsubsection{Device Operation: status}\label{sec:Device Types / GPU Device / Device Operation / Device Operation: cursorq}
+
+\begin{lstlisting}
+struct virtio_gpu_status_freezing {
+ struct virtio_gpu_ctrl_hdr hdr;
+ __u32 freezing;
+};
+\end{lstlisting}
+
+\begin{description}
+
+\item[VIRTIO_GPU_CMD_STATUS_FREEZING]
+Notify freezing status through controlq.
+Request data is \field{struct virtio_gpu_status_freezing}.
+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 if virtio-gpu is in freezing status or not in
+\field{freezing}. Zero means it is not in freezing status, none-zero
+is the opposite. When virtio-gpu is in freezing status, 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.
+
+\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

> +\begin{lstlisting}
> +struct virtio_gpu_status_freezing {
> + struct virtio_gpu_ctrl_hdr hdr;
> + __u32 freezing;
> +};
> +\end{lstlisting}

> +This is added for S3 function in guest with virtio-gpu. When guest does
> +S3, let it notify QEMU if virtio-gpu is in freezing status or not in
> +\field{freezing}. Zero means it is not in freezing status, none-zero
> +is the opposite. When virtio-gpu is in freezing status, 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.

Good, thanks for the update.

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

So, eventually we will see changes to this in the future. I'd suggest
to use an enum for freezing, so this can be extended with more modes in
the future, i.e. something along the lines of:

enum virtio_gpu_freeze_mode {
VIRTIO_GPU_FREEZE_MODE_UNFREEZE = 0,
VIRTIO_GPU_FREEZE_MODE_FREEZE_S3 = 3,
};

Maybe also add a "_S3" postfix to the feature flag name. In case we add
S4 support at some point in the future we will need another feature flag
for that.

take care,
Gerd

> +\begin{lstlisting}
> +struct virtio_gpu_status_freezing {
> + struct virtio_gpu_ctrl_hdr hdr;
> + __u32 freezing;
> +};
> +\end{lstlisting}

> +This is added for S3 function in guest with virtio-gpu. When guest does
> +S3, let it notify QEMU if virtio-gpu is in freezing status or not in
> +\field{freezing}. Zero means it is not in freezing status, none-zero
> +is the opposite. When virtio-gpu is in freezing status, 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.

Good, thanks for the update.

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

So, eventually we will see changes to this in the future. I'd suggest
to use an enum for freezing, so this can be extended with more modes in
the future, i.e. something along the lines of:

enum virtio_gpu_freeze_mode {
VIRTIO_GPU_FREEZE_MODE_UNFREEZE = 0,
VIRTIO_GPU_FREEZE_MODE_FREEZE_S3 = 3,
};

Maybe also add a "_S3" postfix to the feature flag name. In case we add
S4 support at some point in the future we will need another feature flag
for that.

take care,
Gerd