Hi Michael S. Tsirkin,
On 2023/9/19 20:31, Michael S. Tsirkin wrote:
> On Tue, Sep 19, 2023 at 07:42:42PM +0800, Jiqian Chen wrote:
>> When guest vm does S3, Qemu will reset and clear some things of virtio
>> devices, but guest can't aware that, so that may cause some problems.
>> For excample, Qemu calls virtio_reset->virtio_gpu_gl_reset when guest
>> resume, that function will destroy render resources of virtio-gpu. As
>> a result, after guest resume, the display can't come back and we only
>> saw a black screen. Due to guest can't re-create all the resources, so
>> we need to let Qemu not to destroy them when S3.
>>
>> For above purpose, we need a mechanism that allows guests and QEMU to
>> negotiate their reset behavior. So this patch add a new parameter
>> named freeze_mode to struct virtio_pci_common_cfg. And when guest
>> suspends, it can write freeze_mode to be FREEZE_S3, and then virtio
>> devices can change their reset behavior on Qemu side according to
>> freeze_mode. What's more, freeze_mode can be used for all virtio
>> devices to affect the behavior of Qemu, not just virtio gpu device.
>>
>> Signed-off-by: Jiqian Chen <jiqian.c...@amd.com>
>> ---
>> transport-pci.tex | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/transport-pci.tex b/transport-pci.tex
>> index a5c6719..2543536 100644
>> --- a/transport-pci.tex
>> +++ b/transport-pci.tex
>> @@ -319,6 +319,7 @@ \subsubsection{Common configuration structure
>> layout}\label{sec:Virtio Transport
>> le64 queue_desc; /* read-write */
>> le64 queue_driver; /* read-write */
>> le64 queue_device; /* read-write */
>> + le16 freeze_mode; /* read-write */
>> le16 queue_notif_config_data; /* read-only for driver */
>> le16 queue_reset; /* read-write */
>>
>
> we can't add fields in the middle of the structure like this -
> offset of queue_notif_config_data and queue_reset changes.
I have confused about this. I found in latest kernel code(master branch):
struct virtio_pci_common_cfg {
/* About the whole device. */
__le32 device_feature_select; /* read-write */
__le32 device_feature; /* read-only */
__le32 guest_feature_select; /* read-write */
__le32 guest_feature; /* read-write */
__le16 msix_config; /* read-write */
__le16 num_queues; /* read-only */
__u8 device_status; /* read-write */
__u8 config_generation; /* read-only */
/* About a specific virtqueue. */
__le16 queue_select; /* read-write */
__le16 queue_size; /* read-write, power of 2. */
__le16 queue_msix_vector; /* read-write */
__le16 queue_enable; /* read-write */
__le16 queue_notify_off; /* read-only */
__le32 queue_desc_lo; /* read-write */
__le32 queue_desc_hi; /* read-write */
__le32 queue_avail_lo; /* read-write */
__le32 queue_avail_hi; /* read-write */
__le32 queue_used_lo; /* read-write */
__le32 queue_used_hi; /* read-write */
__le16 freeze_mode; /* read-write */
};
There is no queue_notif_config_data or queue_reset, and freeze_mode I added is
at the end. Why is it different from virtio-spec?
I add the offset for freeze_mode(VIRTIO_PCI_COMMON_F_MODE), and change the
offset of Q_NDATA and Q_RESET
-#define VIRTIO_PCI_COMMON_Q_NDATA 56
-#define VIRTIO_PCI_COMMON_Q_RESET 58
+#define VIRTIO_PCI_COMMON_F_MODE 56
+#define VIRTIO_PCI_COMMON_Q_NDATA 58
+#define VIRTIO_PCI_COMMON_Q_RESET 60
>
>
>> @@ -393,6 +394,12 @@ \subsubsection{Common configuration structure
>> layout}\label{sec:Virtio Transport
>> \item[\field{queue_device}]
>> The driver writes the physical address of Device Area here. See
>> section \ref{sec:Basic Facilities of a Virtio Device / Virtqueues}.
>>
>> +\item[\field{freeze_mode}]
>> + The driver writes this to set the freeze mode of virtio pci.
>> + VIRTIO_PCI_FREEZE_MODE_UNFREEZE - virtio-pci is running;
>> + VIRTIO_PCI_FREEZE_MODE_FREEZE_S3 - guest vm is doing S3, and
>> virtio-pci enters S3 suspension;
>> + Other values are reserved for future use, like S4, etc.
>> +
>
> we need to specify these values then.
Thanks, I will add the values.
>
> we also need
> - feature bit to detect support for S3
Do I need to add feature bit to DEFINE_VIRTIO_COMMON_FEATURES? And each time
when I write freeze_mode filed on kernel driver side, I need to check this bit?
> - conformance statements documenting behavious under S3
Sorry, I am not very sure. Do you mean when freeze_mode is set FREEZE_S3, what
operations should driver and device to do? Can you elaborate on it, or give an
example?
>
>
>> \item[\field{queue_notif_config_data}]
>> This field exists only if VIRTIO_F_NOTIF_CONFIG_DATA has been
>> negotiated.
>> The driver will use this value when driver sends available buffer
>> --
>> 2.34.1
>
--
Best regards,
Jiqian Chen.
