Hi, > -----Original Message----- > From: Ingo Molnar [mailto:mingo.kernel....@gmail.com] On Behalf Of Ingo > Molnar > Sent: Thursday, November 26, 2015 5:10 PM > To: Chen, Yu C > Cc: mi...@redhat.com; t...@linutronix.de; h...@zytor.com; > r...@rjwysocki.net; pa...@ucw.cz; Brown, Len; l...@kernel.org; > b...@suse.de; li...@horizon.com; Kaszewski, Marcin; linux- > p...@vger.kernel.org; x...@kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH][v7] x86, suspend: Save/restore extra MSR registers for > suspend > > > * Chen Yu <yu.c.c...@intel.com> wrote: > > > A bug was reported that on certain Broadwell platforms, after resuming > > from S3, the CPU is running at an anomalously low speed. > > > > It turns out that the BIOS has modified the value of the THERM_CONTROL > > register during S3, and changed it from 0 to 0x10, thus enabled clock > > modulation(bit4), but with undefined CPU Duty Cycle(bit1:3) - which causes > the problem. > > > > Here is a simple scenario to reproduce the issue: > > > > 1. Boot up the system > > 2. Get MSR 0x19a, it should be 0 > > 3. Put the system into sleep, then wake it up 4. Get MSR 0x19a, it > > shows 0x10, while it should be 0 > > > > Although some BIOSen want to change the CPU Duty Cycle during S3, in > > our case we don't want the BIOS to do any modification. > > > > Fix this issue by introducing a more generic x86 framework to > > save/restore specified MSR registers(THERM_CONTROL in this case) for > > suspend/resume. This allows us to fix similar bugs in a much simpler way in > the future. > > > > When the kernel wants to protect certain MSRs during suspending, we > > simply add a quirk entry in msr_save_dmi_table, and customize the MSR > > registers inside the quirk callback, for example: > > > > u32 msr_id_need_to_save[] = {MSR_ID0, MSR_ID1, MSR_ID2...}; > > > > and the quirk mechanism ensures that, once resumed from suspend, the > > MSRs indicated by these IDs will be restored to their original, pre-suspend > values. > > > > Since both 64-bit and 32-bit kernels are affected, this patch covers > > the common 64/32-bit suspend/resume code path. And because the MSRs > > specified by the user might not be available or readable in any > > situation, we use rdmsrl_safe() to safely save these MSRs. > > > > Reported-and-tested-by: Marcin Kaszewski <marcin.kaszew...@intel.com> > > Acked-by: Rafael J. Wysocki <rafael.j.wyso...@intel.com> > > Acked-by: Pavel Machek <pa...@ucw.cz> > > Signed-off-by: Chen Yu <yu.c.c...@intel.com> > > --- > > v7: > > - Use the improved version of changelog, and > > modify the patch according to: > > https://patchwork.kernel.org/patch/7637861/ > > --- > > arch/x86/include/asm/msr.h | 10 +++++ > > arch/x86/include/asm/suspend_32.h | 1 + > > arch/x86/include/asm/suspend_64.h | 1 + > > arch/x86/power/cpu.c | 94 > +++++++++++++++++++++++++++++++++++++++ > > 4 files changed, 106 insertions(+) > > Ok, this version looks mostly good - I've applied it with some other minor > edits to field and variable naming. Please double check the end result that > you'll see in the tip-bot notification email once I've pushed it out after > some > testing. > OK, thanks!
Yu -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
