Hi Jason,
On 2023/9/22 11:17, Jason Wang wrote:
> On Thu, Sep 21, 2023 at 2:28 PM Chen, Jiqian <jiqian.c...@amd.com> wrote:
>>
>> Hi Jason,
>>
>> On 2023/9/21 12:22, Jason Wang wrote:
>>> On Tue, Sep 19, 2023 at 7:43 PM Jiqian Chen <jiqian.c...@amd.com> 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.
>>>
>>> A simple question, why is this issue specific to pci?
>> I thought you possibly missed the previous version patches. At the
>> beginning, I just wanted to add a new feature flag VIRTIO_GPU_F_FREEZE_S3
>> for virtio-gpu since I encountered virtio-gpu issue during guest S3, so that
>> the guest and qemu can negotiate and change the reset behavior during S3.
>> But Parav and Mikhail hoped me can improve the feature to a pci level, then
>> other virtio devices can also benefit from it. Although I am not sure if
>> expanding its influence is appropriate, I have not received any feedback
>> from others, so I change it to the pci level and made this version.
>> If you are interested, please see the previous version:
>> https://lists.oasis-open.org/archives/virtio-comment/202307/msg00209.html,
>> thank you.
>
> This is not a good answer. Let me ask you differently, why don't you
> see it in other forms of transport like virtio-gpu-mmio?
Sorry for not understanding your question before. Did you mean why did I add
freeze_mode for virtio-pci type devices instead of virtio-mmio type devices?
If that's the case, it isn't well thought out by me, because there is only
virtio-gpu with pci support in my environment, so I, as a matter of course,
improve it from virtio-gpu-pci to a virtio-pci level instead of virtio-mmio.
Do you think I need to add freeze_mode for virtio-mmio too? Or improve it to a
higher level, like virtio-bus? I am looking forward to getting your
suggestions, thanks.
>
> Thanks
>
>>
>>>
>>> Thanks
>>>
>>>
>>>>
>>>> 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 */
>>>>
>>>> @@ -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.
>>>> +
>>>> \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
>>>>
>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
>>> For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
>>>
>>
>> --
>> Best regards,
>> Jiqian Chen.
>
--
Best regards,
Jiqian Chen.
