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

[Dave Hansen]

On 9/12/22 16:41, Pawan Gupta wrote:
> On an Intel Atom N2600 (and presumable other Cedar Trail models)
> MSR_IA32_TSX_CTRL can be read, causing saved_msr.valid to be set for it
> by msr_build_context().

This changelog needs some help.  Shouldn't it be something like this?

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.

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.