From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751855AbbKZPeP (ORCPT ); Thu, 26 Nov 2015 10:34:15 -0500 Received: from mga03.intel.com ([134.134.136.65]:57724 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751060AbbKZPeK convert rfc822-to-8bit (ORCPT ); Thu, 26 Nov 2015 10:34:10 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.20,347,1444719600"; d="scan'208";a="607577746" From: "Chen, Yu C" To: Ingo Molnar CC: "mingo@redhat.com" , "tglx@linutronix.de" , "hpa@zytor.com" , "rjw@rjwysocki.net" , "pavel@ucw.cz" , "Brown, Len" , "luto@kernel.org" , "bp@suse.de" , "linux@horizon.com" , "Kaszewski, Marcin" , "linux-pm@vger.kernel.org" , "x86@kernel.org" , "linux-kernel@vger.kernel.org" Subject: RE: [PATCH][v7] x86, suspend: Save/restore extra MSR registers for suspend Thread-Topic: [PATCH][v7] x86, suspend: Save/restore extra MSR registers for suspend Thread-Index: AQHRJtYUr+Pb8EIRVkOGRfigKPhFup6tgJQAgADxXSA= Date: Thu, 26 Nov 2015 15:34:04 +0000 Message-ID: <36DF59CE26D8EE47B0655C516E9CE64028677FF5@shsmsx102.ccr.corp.intel.com> References: <20151126090940.GA30403@gmail.com> In-Reply-To: <20151126090940.GA30403@gmail.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, > -----Original Message----- > From: Ingo Molnar [mailto:mingo.kernel.org@gmail.com] On Behalf Of Ingo > Molnar > Sent: Thursday, November 26, 2015 5:10 PM > To: Chen, Yu C > Cc: mingo@redhat.com; tglx@linutronix.de; hpa@zytor.com; > rjw@rjwysocki.net; pavel@ucw.cz; Brown, Len; luto@kernel.org; > bp@suse.de; linux@horizon.com; Kaszewski, Marcin; linux- > pm@vger.kernel.org; x86@kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH][v7] x86, suspend: Save/restore extra MSR registers for > suspend > > > * Chen Yu 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 > > Acked-by: Rafael J. Wysocki > > Acked-by: Pavel Machek > > Signed-off-by: Chen Yu > > --- > > 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