Re: [PATCH v3 2/2] x86/pm: Add enumeration check before spec MSRs save/restore setup

[Pawan Gupta]

On Tue, Nov 15, 2022 at 08:23:35PM +0100, Rafael J. Wysocki wrote:
> On Tue, Nov 15, 2022 at 8:17 PM Pawan Gupta
> <pawan.kumar.gupta@xxxxxxxxxxxxxxx> wrote:
> > pm_save_spec_msr() keeps a list of all the MSRs which _might_ need to be
> > saved and restored at hibernate and resume.  However, it has zero
> > awareness of CPU support for these MSRs.  It mostly works by
> > unconditionally attempting to manipulate these MSRs and relying on
> > rdmsrl_safe() being able to handle a #GP on CPUs where the support is
> > unavailable.
> >
> > However, it's possible for reads (RDMSR) to be supported for a given MSR
> > while writes (WRMSR) are not.  In this case, msr_build_context() sees a
> > successful read (RDMSR) and marks the MSR as 'valid'.  Then, later, a
> > write (WRMSR) fails, producing a nasty (but harmless) error message.
> > This causes restore_processor_state() to try and restore it, but writing
> > this MSR is not allowed on the Intel Atom N2600 leading to:
> >
> >   unchecked MSR access error: WRMSR to 0x122 (tried to write 0x0000000000000002) \
> >      at rIP: 0xffffffff8b07a574 (native_write_msr+0x4/0x20)
> >   Call Trace:
> >    <TASK>
> >    restore_processor_state
> >    x86_acpi_suspend_lowlevel
> >    acpi_suspend_enter
> >    suspend_devices_and_enter
> >    pm_suspend.cold
> >    state_store
> >    kernfs_fop_write_iter
> >    vfs_write
> >    ksys_write
> >    do_syscall_64
> >    ? do_syscall_64
> >    ? up_read
> >    ? lock_is_held_type
> >    ? asm_exc_page_fault
> >    ? lockdep_hardirqs_on
> >    entry_SYSCALL_64_after_hwframe
> >
> > To fix this, add the corresponding X86_FEATURE bit for each MSR.  Avoid
> > trying to manipulate the MSR when the feature bit is clear. This
> > required adding a X86_FEATURE bit for MSRs that do not have one already,
> > but it's a small price to pay.
> >
> > Fixes: 73924ec4d560 ("x86/pm: Save the MSR validity status at context setup")
> > Reported-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> > Signed-off-by: Pawan Gupta <pawan.kumar.gupta@xxxxxxxxxxxxxxx>
> > Cc: stable@xxxxxxxxxx
>
> Fine with me:
>
> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>

Thanks.