On Mon, 31 Jan 2022 17:04:12 +0530 Abhishek Sahu <abhsahu@xxxxxxxxxx> wrote: > On 1/28/2022 5:35 AM, Alex Williamson wrote: > > External email: Use caution opening links or attachments > > > > > > On Mon, 24 Jan 2022 23:47:24 +0530 > > Abhishek Sahu <abhsahu@xxxxxxxxxx> wrote: > > > >> If needs_pm_restore is set (PCI device does not have support for no > >> soft reset), then the current PCI state will be saved during D0->D3hot > >> transition and same will be restored back during D3hot->D0 transition. > >> For saving the PCI state locally, pci_store_saved_state() is being > >> used and the pci_load_and_free_saved_state() will free the allocated > >> memory. > >> > >> But for reset related IOCTLs, vfio driver calls PCI reset related > >> API's which will internally change the PCI power state back to D0. So, > >> when the guest resumes, then it will get the current state as D0 and it > >> will skip the call to vfio_pci_set_power_state() for changing the > >> power state to D0 explicitly. In this case, the memory pointed by > >> pm_save will never be freed. > >> > >> Also, in malicious sequence, the state changing to D3hot followed by > >> VFIO_DEVICE_RESET/VFIO_DEVICE_PCI_HOT_RESET can be run in loop and > >> it can cause an OOM situation. This patch stores the power state locally > >> and uses the same for comparing the current power state. For the > >> places where D0 transition can happen, call vfio_pci_set_power_state() > >> to transition to D0 state. Since the vfio power state is still D3hot, > >> so this D0 transition will help in running the logic required > >> from D3hot->D0 transition. Also, to prevent any miss during > >> future development to detect this condition, this patch puts a > >> check and frees the memory after printing warning. > >> > >> This locally saved power state will help in subsequent patches > >> also. > > > > Ideally let's put fixes patches at the start of the series, or better > > yet send them separately, and don't include changes that only make > > sense in the context of a subsequent patch. > > > > Fixes: 51ef3a004b1e ("vfio/pci: Restore device state on PM transition") > > > > Thanks Alex for reviewing this patch. > I have added Fixes tag and sent this patch separately. > > Should I update this patch series or you are planning to review the > other patches first of this patch series first. Thanks for splitting this out. I'll keep the remainder of the series on the review queue, I expect I'll have some comments and it will be easy enough to imagine vfio_pci_core_device.power_state being declared in another patch if there's still a worthwhile use for it. Thanks, Alex