[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: RE: [PATCH v6 1/1] content: Add new feature VIRTIO_F_PRESERVE_RESOURCES
> From: Chen, Jiqian <Jiqian.Chen@amd.com> > Sent: Monday, January 15, 2024 1:03 PM > > On 2024/1/12 17:44, Parav Pandit wrote: > > > > > >> 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. > Do you mean the devices_reset behavior of qemu shouldn't happen when a > device has PM cap? Right. Device reset should not happen on PM capable device that offers no_soft_reset==1. > Or just the state of PM cap shouldn't be reset? > > > > >> 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. > But it happens for now. This should be enhanced in the guest driver. > In current qemu codes, the bit PCI_PM_CTRL_NO_SOFT_RESET of virtio-pci > devices isn't set, is it a bug? It means the no_soft_reset==0. > It should be set when/if the device can store and restore the device context on d3->d0 transition. > > Hence the 2nd reset will also not occur. > > Therefore, device state will be restored. > > -- > Best regards, > Jiqian Chen.
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]