Messages in this thread | | | Date | Wed, 16 Aug 2023 12:37:34 -0700 | Subject | Re: [PATCH v2] PCI: hv: Fix a crash in hv_pci_restore_msi_msg() during hibernation | From | Kuppuswamy Sathyanarayanan <> |
| |
On 8/16/2023 12:30 PM, Dexuan Cui wrote: >> From: Kuppuswamy Sathyanarayanan >> <sathyanarayanan.kuppuswamy@linux.intel.com> >> Sent: Wednesday, August 16, 2023 11:12 AM >> [...] >> On 8/16/2023 10:59 AM, Dexuan Cui wrote: >>> When a Linux VM with an assigned PCI device runs on Hyper-V, if the PCI >>> device driver is not loaded yet (i.e. MSI-X/MSI is not enabled on the >>> device yet), doing a VM hibernation triggers a panic in >>> hv_pci_restore_msi_msg() -> msi_lock_descs(&pdev->dev), because >>> pdev->dev.msi.data is still NULL. >>> >>> Avoid the panic by checking if MSI-X/MSI is enabled. >>> >>> Fixes: dc2b453290c4 ("PCI: hv: Rework MSI handling") >>> Signed-off-by: Dexuan Cui <decui@microsoft.com> >>> --- >>> >>> Changes in v2: >>> Replaced the test "if (!pdev->dev.msi.data)" with >>> "if (!pdev->msi_enabled && !pdev->msix_enabled)". >>> Thanks Michael! >>> Updated the changelog accordingly. >>> >>> drivers/pci/controller/pci-hyperv.c | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci- >> hyperv.c >>> index 2d93d0c4f10d..bed3cefdaf19 100644 >>> --- a/drivers/pci/controller/pci-hyperv.c >>> +++ b/drivers/pci/controller/pci-hyperv.c >>> @@ -3983,6 +3983,9 @@ static int hv_pci_restore_msi_msg(struct pci_dev >> *pdev, void *arg) >>> struct msi_desc *entry; >>> int ret = 0; >>> >>> + if (!pdev->msi_enabled && !pdev->msix_enabled) >>> + return 0; >> Isn't this is a error condition? Don't you want to return error here? > This is not an error. If a PCI device driver is not loaded or not installed, > MSI-X/MSI is not enabled on the device, so pdev->msi_enabled is 0 > and pdev->msix_enabled is 0. In this case, it's still legit for a user to request > the system (i.e. here it's a Linux VM running on Hyper-V) to hibernate -- in > this case, we should not try to save/restore the MSI/MSI-X state, and we > should not let the hibernation fail; here we should just ignore the device > by returning a success ("return 0;").
Got it. Looks good to me.
Reviewed-by: sathyanarayanan.kuppuswamy@linux.intel.com
> >>> + >>> msi_lock_descs(&pdev->dev); >>> msi_for_each_desc(entry, &pdev->dev, MSI_DESC_ASSOCIATED) { >>> irq_data = irq_get_irq_data(entry->irq);
| |