Re: [PATCH] PCI / PM: Restore the status of PCI devices across hibernation

[Chen Yu]

On Fri, Jun 02, 2017 at 01:22:28AM +0200, Rafael J. Wysocki wrote:
> On Thu, May 25, 2017 at 10:49 AM, Chen Yu <yu.c.chen@xxxxxxxxx> wrote:
> > Currently we saw a lot of "No irq handler" errors during hibernation,
> > which caused the system hang finally:
> >
> > [  710.141581] ata4.00: qc timeout (cmd 0xec)
> > [  710.147135] ata4.00: failed to IDENTIFY (I/O error, err_mask=0x4)
> > [  710.154593] ata4.00: revalidation failed (errno=-5)
> > [  710.468124] ata4: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
> > [  710.477746] do_IRQ: 31.151 No irq handler for vector
> >
> > According to above logs, there is an interrupt triggered and it is
> > dispatched to CPU31 with a vector number 151, but there is no handler
> > for it, thus this irq will not get acked and caused irq flood which kill
> > the system. To be more specific, the 31.151 is an interrupt from the ahci
> > host controller.
> >
> > After some investigation, the reason why this issue is triggered is
> > because the thaw_noirq() function does not restore the MSI/MSIX settings
> > across hibernation.
> >
> > The scenario is illustrated below:
> >
> > 1. Before the hibernation starts, the irq 34 is the handler for the ahci device,
> >    which is binded on cpu31.
> > 2. Hibernation starts, the ahci device is put into low power state.
> > 3. All the nonboot CPUs are put offline, so the irq 34 has to be migrated to
> >    the last alive one - CPU0.
> > 4. After the snapshot has been created, all the nonboot CPUs are brought up again,
> >    the CPU affinity for IRQ 34 remains to be 0.
> > 5. ahci device are put into D0.
> > 6. The snapshot is written to the disk.
> >
> > The issue is triggered in step 6, in theory the ahci interrupt should be
> > delivered to CPU0, however the actually result is that this interrupt is
> > delivered to the original CPU31 instead, which cause the "No irq handler" issue.
> >
> > Ying Huang has has provided a clue that, in step 3 it is possible that the writing
> > to the register might not take effect as the PCI devices have been put suspended.
> > Actually it is true:
> > In step 3, the irq 34 affinity is supposed to be modified from 31 to 0,
> > but actually it did not. In __pci_write_msi_msg(), if the device is already
> > in low power state, the low level msi message entry will not be updated
> > but cached. So in theory during the device restore process, the cached msi
> > modification information should be written back to the hardware, and this
> > is what pci_restore_msi_state() do during normal suspend-resume.
> > But this is not the case for hibernation, pci_restore_msi_state() is not
> > invoked currently, to be more specific, pci_restore_state() is not invoked
> > in pci_pm_thaw_noirq(), although pci_save_state() has saved the necessary
> > pci cached information in pci_pm_freeze_noirq().
> >
> > This patch tries to restore the pci status for the device during hibernation,
> > otherwise the status might be lost across hibernation(for example, the MSI/MSIX
> > message settings), which might cause problems during hibernation.
> >
> > Suggested-by: Ying Huang <ying.huang@xxxxxxxxx>
> > Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> > Cc: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> > Cc: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
> > Cc: Len Brown <len.brown@xxxxxxxxx>
> > Cc: Dan Williams <dan.j.williams@xxxxxxxxx>
> > Cc: Rui Zhang <rui.zhang@xxxxxxxxx>
> > Cc: Ying Huang <ying.huang@xxxxxxxxx>
> > Cc: linux-pci@xxxxxxxxxxxxxxx
> > Cc: linux-pm@xxxxxxxxxxxxxxx
> > Cc: linux-kernel@xxxxxxxxxxxxxxx
> > Signed-off-by: Chen Yu <yu.c.chen@xxxxxxxxx>
> 
> Thanks for the detailed description of what's going on!
> 
> The fix is correct IMO, so
> 
> Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
>
Thanks for the review!
Please also help add a tag for stable:
Cc: stable <stable@xxxxxxxxxxxxxxx>

thanks,
Yu