On Fri, Sep 22, 2023 at 4:07 PM Chen, Jiqian <jiqian.c...@amd.com> wrote:
>
> 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?
Yes.
> 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?
Because the function you describe is to preserve the render resources
which have nothing specific to PCI. It's a core virtio function.
I'd suggest fixing it at the virtio level instead of PCI level.
> Or improve it to a higher level, like virtio-bus? I am looking forward to
> getting your suggestions, thanks.
As pointed out in other thread, reusing the existing status state
machine is a perfect way for doing this.
We can tweak the state machine to be able to do both migration and S3.
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.
---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
