RE: [PATCH v6 1/1] content: Add new feature VIRTIO_F_PRESERVE_RESOURCES

[Parav Pandit <parav@nvidia.com>]

> From: Chen, Jiqian <Jiqian.Chen@amd.com>
> Sent: Friday, January 12, 2024 2:55 PM
> 
> On 2024/1/12 16:47, Parav Pandit wrote:
> >
> >
> >> From: Chen, Jiqian <Jiqian.Chen@amd.com>
> >> Sent: Friday, January 12, 2024 1:55 PM
> >>
> >>
> >> On 2024/1/12 16:02, Parav Pandit wrote:
> >>> Hi Jiqian,
> >>>
> >>>> From: Chen, Jiqian <Jiqian.Chen@amd.com>
> >>>> Sent: Friday, January 12, 2024 1:11 PM
> >>>
> >>>>
> >>>> Hi all,
> >>>> Sorry to reply late.
> >>>> I don't know if you still remember this problem, let me briefly descript
> it.
> >>>> 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 resume guest. We can find that the
> >>>> guest kernel comes back, but the display doesn't. It just shown a
> >>>> black
> >> screen.
> >>>> That is because during suspending, guest called into qemu and qemu
> >>>> destroyed all display resources and reset renderer. This made the
> >>>> display gone after guest resumed.
> >>>> So, I add a new command for virtio-gpu that when guest is
> >>>> suspending, it will notify qemu and set
> >>>> parameter(preserver_resource) to 1 and then qemu will preserve
> >>>> resources, and when resuming, guest will notify qemu to set
> >>>> parameter to 0, and then qemu will keep the normal actions. That can
> help guest's display come back.
> >>>> When I upstream above implementation, Parav and MST suggest me to
> >> use
> >>>> the PM capability to fix this problem instead of adding a new
> >>>> command or state bit.
> >>>> Now, I have tried the PM capability of virtio-gpu, it can't be used
> >>>> to solve this problem.
> >>>> The reason is:
> >>>> during guest suspending, it will write D3 state through PM cap,
> >>>> then I can save the resources of virtio-gpu on Qemu side(set
> >>>> preserver_resource to 1), but during the process of resuming, the
> >>>> state of PM cap will be cleared by qemu
> >>>> resetting(qemu_system_wakeup-> qemu_devices_reset->
> >>>> virtio_vga_base_reset-> virtio_gpu_gl_reset), it causes that when
> >>>> guest reads state from PM cap, it will find the virtio-gpu has
> >>>> already been D0 state,
> >>> This behavior needs to be fixed. As the spec listed out, " This
> >>> 2-bit field is
> >> used both to determine the current power state of a Function"
> >> Do you mean it is wrong to reset PM cap when vritio_gpu reset in
> >> current qemu code? Why?
> > Because PM is implementing the support from D3->d0 transition and if it
> device in D3, the device needs to respond that it is in D3 to match the PCI
> spec.
> >
> >> Shouldn't all device states, including PM registers, be reset during
> >> the process of virtio-gpu reset?
> > If No_Soft_Reset== 0, no device context to be preserved.
> > If No_Soft_Reset == 1, device to preserve minimal registers and other
> things listed in pci spec.
> >
> >>
> >>>
> >>> So device needs to return D3 in the PowerState field.  This is must.
> >> But current behavior is a kind of soft reset, I
> >> think(!PCI_PM_CTRL_NO_SOFT_RESET). The pm cap is reset by qemu is
> >> reasonable.
> > What you described is, you need to do No_Soft_Reset=1, so please set the
> cap accordingly to achieve the restore.
> >
> > Also in your case the if the QEMU knows that it will have to resume the
> device soon.
> > Hence, it can well prepare the gpu context before resuming the vcpus.
> > In such case, the extra register wouldnât be necessary.
> > Having PM control register is anyway needed to drive the device properly.
> Even as you said, the reset behavior of PM in the current QEMU code is
> incorrect, and even if it is modified, it also cannot fix this problem with S3.
You meant D3?

> Because there is twice device reset during resuming.
> First is when we trigger a resume command to qemu, qemu will reset
> device(qemu_system_wakeup-> qemu_devices_reset->
> virtio_vga_base_reset-> virtio_gpu_gl_reset).
A device implementation that supports PM should not reset the device. Please fix it.

> After the first time, then second time happens in guest kernel,
> virtio_device_restore-> virtio_reset_device. But the PM state changes (D3 to
> D0) happens between those two, so the display resources will be still
> destroyed by the second time of device reset.
The driver that understands that device supports PM, will skip the reset steps.
Hence the 2nd reset will also not occur.
Therefore, device state will be restored.