Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1211124 > unrolled thread

[PATCH] [v2] x86, suspend: Save/restore extra MSR registers for suspend

Back to article view | Back to linux.kernel

Contents

  [PATCH] [v2] x86, suspend: Save/restore extra MSR registers for suspend Chen Yu <yu.c.chen@intel.com> - 2015-08-21 14:00 +0200
    Re: [PATCH] [v2] x86, suspend: Save/restore extra MSR registers for  suspend Nigel Cunningham <nigel@nigelcunningham.com.au> - 2015-08-21 14:40 +0200
      RE: [PATCH] [v2] x86, suspend: Save/restore extra MSR registers for  suspend "Chen, Yu C" <yu.c.chen@intel.com> - 2015-08-21 18:30 +0200
    Re: [PATCH] [v2] x86, suspend: Save/restore extra MSR registers for  suspend Pavel Machek <pavel@ucw.cz> - 2015-08-22 22:40 +0200
      Re: [PATCH] [v2] x86, suspend: Save/restore extra MSR registers for  suspend Ingo Molnar <mingo@kernel.org> - 2015-08-23 08:30 +0200
        Re: [PATCH] [v2] x86, suspend: Save/restore extra MSR registers for  suspend Pavel Machek <pavel@ucw.cz> - 2015-08-23 10:50 +0200
          RE: [PATCH] [v2] x86, suspend: Save/restore extra MSR registers for  suspend "Chen, Yu C" <yu.c.chen@intel.com> - 2015-08-24 05:30 +0200
        RE: [PATCH] [v2] x86, suspend: Save/restore extra MSR registers for  suspend "Chen, Yu C" <yu.c.chen@intel.com> - 2015-08-24 05:30 +0200
    Re: [PATCH] [v2] x86, suspend: Save/restore extra MSR registers for  suspend Borislav Petkov <bp@alien8.de> - 2015-08-24 10:50 +0200
      RE: [PATCH] [v2] x86, suspend: Save/restore extra MSR registers for  suspend "Chen, Yu C" <yu.c.chen@intel.com> - 2015-08-25 04:00 +0200

#1211124 — [PATCH] [v2] x86, suspend: Save/restore extra MSR registers for suspend

A bug is reported(https://bugzilla.redhat.com/show_bug.cgi?id=1227208)
that, after resuming from S3, CPU is working at a low speed.
After investigation, it is found that, BIOS has modified the value
of THERM_CONTROL register during S3, changes it from 0 to 0x10,
while the latter means CPU can only get 25% of the Duty Cycle,
and this caused the problem.

Simple scenario to reproduce:
1.Boot up system
2.Get MSR with address 0x19a, it should output 0
3.Put system into sleep, then wake up
4.Get MSR with address 0x19a, it should output 0(actual it outputs 0x10)

Although this is a BIOS issue, it would be more robust for linux to deal
with this situation. This patch fixes this issue by introducing a framework
for saving/restoring specify MSR registers(THERM_CONTROL in this case)
on suspend/resume.

When user finds a problematic platform that requires save/restore MSRs,
he can simply add quirk in msr_save_dmi_table, and customizes MSR
registers in quirk callback, for example:

unsigned int msr_id_need_to_save[] = {MSR_ID0, MSR_ID1, MSR_ID2...};

and system ensures that, once resumed from suspend, these MSR indicated
by IDs will be restored to their original values before suspend.

Since both 64/32-bit kernels are affected, this patch covers 64/32-bit
common code path. And because the MSR ids specified by user might not be
available or readable in any situation, we use rdmsrl_safe to safely
save these MSR registers.

Tested-by: Marcin Kaszewski <marcin.kaszewski@intel.com>
Signed-off-by: Chen Yu <yu.c.chen@intel.com>
---
v2:
 - Cover both 64/32-bit common code path.
   Use rdmsrl_safe to safely read MSR.
   Introduce a quirk framework for save/restore specified MSR on different
   platforms.
---
 arch/x86/include/asm/suspend_32.h | 12 +++++
 arch/x86/include/asm/suspend_64.h | 12 +++++
 arch/x86/power/cpu.c              | 93 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 117 insertions(+)

diff --git a/arch/x86/include/asm/suspend_32.h b/arch/x86/include/asm/suspend_32.h
index d1793f0..07b0443 100644
--- a/arch/x86/include/asm/suspend_32.h
+++ b/arch/x86/include/asm/suspend_32.h
@@ -9,12 +9,24 @@
 #include <asm/desc.h>
 #include <asm/fpu/api.h>
 
+struct msr_type {
+	unsigned int msr_id;
+	bool msr_saved;
+	u64 msr_value;
+};
+
+struct saved_msr {
+	unsigned short num;
+	struct msr_type *msr_array;
+} __attribute__((packed));
+
 /* image of the saved processor state */
 struct saved_context {
 	u16 es, fs, gs, ss;
 	unsigned long cr0, cr2, cr3, cr4;
 	u64 misc_enable;
 	bool misc_enable_saved;
+	struct saved_msr msr_for_save;
 	struct desc_ptr gdt_desc;
 	struct desc_ptr idt;
 	u16 ldt;
diff --git a/arch/x86/include/asm/suspend_64.h b/arch/x86/include/asm/suspend_64.h
index 7ebf0eb..321e288 100644
--- a/arch/x86/include/asm/suspend_64.h
+++ b/arch/x86/include/asm/suspend_64.h
@@ -9,6 +9,17 @@
 #include <asm/desc.h>
 #include <asm/fpu/api.h>
 
+struct msr_type {
+	unsigned int msr_id;
+	bool msr_saved;
+	u64 msr_value;
+};
+
+struct saved_msr {
+	unsigned short num;
+	struct msr_type *msr_array;
+} __attribute__((packed));
+
 /*
  * Image of the saved processor state, used by the low level ACPI suspend to
  * RAM code and by the low level hibernation code.
@@ -24,6 +35,7 @@ struct saved_context {
 	unsigned long cr0, cr2, cr3, cr4, cr8;
 	u64 misc_enable;
 	bool misc_enable_saved;
+	struct saved_msr msr_for_save;
 	unsigned long efer;
 	u16 gdt_pad; /* Unused */
 	struct desc_ptr gdt_desc;
diff --git a/arch/x86/power/cpu.c b/arch/x86/power/cpu.c
index 9ab5279..ed6c562 100644
--- a/arch/x86/power/cpu.c
+++ b/arch/x86/power/cpu.c
@@ -23,6 +23,7 @@
 #include <asm/debugreg.h>
 #include <asm/cpu.h>
 #include <asm/mmu_context.h>
+#include <linux/dmi.h>
 
 #ifdef CONFIG_X86_32
 __visible unsigned long saved_context_ebx;
@@ -32,6 +33,30 @@ __visible unsigned long saved_context_eflags;
 #endif
 struct saved_context saved_context;
 
+static void msr_save_context(struct saved_context *ctxt)
+{
+	int i = 0;
+
+	for (i = 0; i < ctxt->msr_for_save.num; i++) {
+		struct msr_type *msr =
+			&ctxt->msr_for_save.msr_array[i];
+		msr->msr_saved = !rdmsrl_safe(msr->msr_id,
+			&msr->msr_value);
+	}
+}
+
+static void msr_restore_context(struct saved_context *ctxt)
+{
+	int i = 0;
+
+	for (i = 0; i < ctxt->msr_for_save.num; i++) {
+		struct msr_type *msr =
+			&ctxt->msr_for_save.msr_array[i];
+		if (msr->msr_saved)
+			wrmsrl(msr->msr_id, msr->msr_value);
+	}
+}
+
 /**
  *	__save_processor_state - save CPU registers before creating a
  *		hibernation image and before restoring the memory state from it
@@ -111,6 +136,7 @@ static void __save_processor_state(struct saved_context *ctxt)
 #endif
 	ctxt->misc_enable_saved = !rdmsrl_safe(MSR_IA32_MISC_ENABLE,
 					       &ctxt->misc_enable);
+	msr_save_context(ctxt);
 }
 
 /* Needed by apm.c */
@@ -229,6 +255,7 @@ static void notrace __restore_processor_state(struct saved_context *ctxt)
 	x86_platform.restore_sched_clock_state();
 	mtrr_bp_restore();
 	perf_restore_debug_store();
+	msr_restore_context(ctxt);
 }
 
 /* Needed by apm.c */
@@ -320,3 +347,69 @@ static int __init bsp_pm_check_init(void)
 }
 
 core_initcall(bsp_pm_check_init);
+
+/* should be enough */
+#define MAX_MSR_SAVED	64
+static struct msr_type msr_context_array[MAX_MSR_SAVED];
+
+/*
+ * Following section is a quirk for some problematic BIOS:
+ * MSRs are modified by BIOS after suspending to ram, and
+ * causing unexpected bevavior after resume.
+ * Thus we save/restore these specific MSRs during suspend
+ * in order to workaround.
+ * A typical bug was reported at:
+ * https://bugzilla.redhat.com/show_bug.cgi?id=1227208
+ */
+static int msr_set_info(const unsigned int *msr_id, const int total_num)
+{
+	int i = 0;
+
+	if (total_num > MAX_MSR_SAVED) {
+		pr_err("PM: too many MSRs need to save\n");
+		return -EINVAL;
+	}
+	for (i = 0; i < total_num; i++) {
+		msr_context_array[i].msr_id = msr_id[i];
+		msr_context_array[i].msr_saved = false;
+		msr_context_array[i].msr_value = 0;
+	}
+	saved_context.msr_for_save.num = total_num;
+	saved_context.msr_for_save.msr_array = msr_context_array;
+	return 0;
+}
+/*
+ * For any further problematic BIOS/platforms,
+ * please add your own function similar to msr_initialize_bdw.
+ */
+static int msr_initialize_bdw(const struct dmi_system_id *d)
+{
+	/* Adding any extra MSR id into the array */
+	unsigned int bdw_msr_id[] = {MSR_IA32_THERM_CONTROL};
+
+	pr_info("PM: %s detected: MSR saving is needed during suspend\n",
+		d->ident);
+	return msr_set_info(bdw_msr_id, ARRAY_SIZE(bdw_msr_id));
+}
+
+static struct dmi_system_id msr_save_dmi_table[] = {
+	{
+	 .callback = msr_initialize_bdw,
+	 .ident = "BROADWELL BDX_EP",
+	 .matches = {
+		DMI_MATCH(DMI_SYS_VENDOR, "Intel Corporation"),
+		DMI_MATCH(DMI_PRODUCT_NAME, "GRANTLEY"),
+		DMI_MATCH(DMI_PRODUCT_VERSION, "E63448-400"),
+		},
+	},
+	{}
+};
+static int pm_check_save_msr(void)
+{
+	saved_context.msr_for_save.num = 0;
+	saved_context.msr_for_save.msr_array = NULL;
+	dmi_check_system(msr_save_dmi_table);
+	return 0;
+}
+
+late_initcall(pm_check_save_msr);
-- 
1.8.4.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1211137 — Re: [PATCH] [v2] x86, suspend: Save/restore extra MSR registers for suspend

Hi Chen.

Is there any issue with saving and restoring MSRs unconditionally? That would simplify the patch and make things 'just work'.

Regards,

Nigel
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1211241 — RE: [PATCH] [v2] x86, suspend: Save/restore extra MSR registers for suspend

Hi, Nigel

> -----Original Message-----
> From: Nigel Cunningham [mailto:nigel@nigelcunningham.com.au]
> Sent: Friday, August 21, 2015 8:35 PM
> To: Chen, Yu C; rjw@rjwysocki.net; pavel@ucw.cz; tglx@linutronix.de;
> mingo@redhat.com; hpa@zytor.com
> Cc: Zhang, Rui; lenb@kernel.org; x86@kernel.org; linux-pm@vger.kernel.org;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] [v2] x86, suspend: Save/restore extra MSR registers for
> suspend
> 
> Hi Chen.
> 
> Is there any issue with saving and restoring MSRs unconditionally? That
> would simplify the patch and make things 'just work'.
Saving/restoring unconditionally might take BIOS legal action into account,
for example, BIOS itself is willing to modify the MSR.  And Pavel suggests
using quirk to workaround in V1 patch:
https://patchwork.kernel.org/patch/7023891/

So I'm considering a common framework to save/restore these MSRs,
because user might need to protect more than one MSR during 
suspend, so..

I can modify this patch to a simpler version, for example,
Introducing two variables in struct saved_context , like
MSR_IA32_MISC_ENABLE does.

Thanks for your review

Best Regards,
Yu
> 
> Regards,
> 
> Nigel
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1211504 — Re: [PATCH] [v2] x86, suspend: Save/restore extra MSR registers for suspend

On Fri 2015-08-21 19:53:34, Chen Yu wrote:
> A bug is reported(https://bugzilla.redhat.com/show_bug.cgi?id=1227208)
> that, after resuming from S3, CPU is working at a low speed.
> After investigation, it is found that, BIOS has modified the value
> of THERM_CONTROL register during S3, changes it from 0 to 0x10,
> while the latter means CPU can only get 25% of the Duty Cycle,
> and this caused the problem.
> 
> Simple scenario to reproduce:
> 1.Boot up system
> 2.Get MSR with address 0x19a, it should output 0
> 3.Put system into sleep, then wake up
> 4.Get MSR with address 0x19a, it should output 0(actual it outputs 0x10)
> 
> Although this is a BIOS issue, it would be more robust for linux to deal
> with this situation. This patch fixes this issue by introducing a framework
> for saving/restoring specify MSR registers(THERM_CONTROL in this case)
> on suspend/resume.
> 
> When user finds a problematic platform that requires save/restore MSRs,
> he can simply add quirk in msr_save_dmi_table, and customizes MSR
> registers in quirk callback, for example:
> 
> unsigned int msr_id_need_to_save[] = {MSR_ID0, MSR_ID1, MSR_ID2...};
> 
> and system ensures that, once resumed from suspend, these MSR indicated
> by IDs will be restored to their original values before suspend.
> 
> Since both 64/32-bit kernels are affected, this patch covers 64/32-bit
> common code path. And because the MSR ids specified by user might not be
> available or readable in any situation, we use rdmsrl_safe to safely
> save these MSR registers.
> 
> Tested-by: Marcin Kaszewski <marcin.kaszewski@intel.com>
> Signed-off-by: Chen Yu <yu.c.chen@intel.com>

Acked-by: Pavel Machek <pavel@ucw.cz>

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1211561 — Re: [PATCH] [v2] x86, suspend: Save/restore extra MSR registers for suspend

* Pavel Machek <pavel@ucw.cz> wrote:

> On Fri 2015-08-21 19:53:34, Chen Yu wrote:
> > A bug is reported(https://bugzilla.redhat.com/show_bug.cgi?id=1227208)
> > that, after resuming from S3, CPU is working at a low speed.
> > After investigation, it is found that, BIOS has modified the value
> > of THERM_CONTROL register during S3, changes it from 0 to 0x10,
> > while the latter means CPU can only get 25% of the Duty Cycle,
> > and this caused the problem.
> > 
> > Simple scenario to reproduce:
> > 1.Boot up system
> > 2.Get MSR with address 0x19a, it should output 0
> > 3.Put system into sleep, then wake up
> > 4.Get MSR with address 0x19a, it should output 0(actual it outputs 0x10)
> > 
> > Although this is a BIOS issue, it would be more robust for linux to deal
> > with this situation. This patch fixes this issue by introducing a framework
> > for saving/restoring specify MSR registers(THERM_CONTROL in this case)
> > on suspend/resume.
> > 
> > When user finds a problematic platform that requires save/restore MSRs,
> > he can simply add quirk in msr_save_dmi_table, and customizes MSR
> > registers in quirk callback, for example:
> > 
> > unsigned int msr_id_need_to_save[] = {MSR_ID0, MSR_ID1, MSR_ID2...};
> > 
> > and system ensures that, once resumed from suspend, these MSR indicated
> > by IDs will be restored to their original values before suspend.
> > 
> > Since both 64/32-bit kernels are affected, this patch covers 64/32-bit
> > common code path. And because the MSR ids specified by user might not be
> > available or readable in any situation, we use rdmsrl_safe to safely
> > save these MSR registers.
> > 
> > Tested-by: Marcin Kaszewski <marcin.kaszewski@intel.com>
> > Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> 
> Acked-by: Pavel Machek <pavel@ucw.cz>

So I like the general structure of the patch and I like the MSR saving mechanism 
it introduces, but it is full of typos and small stylistic uncleanlinesses which 
need to be fixed before this patch can be applied. Please don't ack incomplete 
patches.

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1211567 — Re: [PATCH] [v2] x86, suspend: Save/restore extra MSR registers for suspend

On Sun 2015-08-23 08:20:33, Ingo Molnar wrote:
> 
> * Pavel Machek <pavel@ucw.cz> wrote:
> 
> > On Fri 2015-08-21 19:53:34, Chen Yu wrote:
> > > A bug is reported(https://bugzilla.redhat.com/show_bug.cgi?id=1227208)
> > > that, after resuming from S3, CPU is working at a low speed.
> > > After investigation, it is found that, BIOS has modified the value
> > > of THERM_CONTROL register during S3, changes it from 0 to 0x10,
> > > while the latter means CPU can only get 25% of the Duty Cycle,
> > > and this caused the problem.
> > > 
> > > Simple scenario to reproduce:
> > > 1.Boot up system
> > > 2.Get MSR with address 0x19a, it should output 0
> > > 3.Put system into sleep, then wake up
> > > 4.Get MSR with address 0x19a, it should output 0(actual it outputs 0x10)
> > > 
> > > Although this is a BIOS issue, it would be more robust for linux to deal
> > > with this situation. This patch fixes this issue by introducing a framework
> > > for saving/restoring specify MSR registers(THERM_CONTROL in this case)
> > > on suspend/resume.
> > > 
> > > When user finds a problematic platform that requires save/restore MSRs,
> > > he can simply add quirk in msr_save_dmi_table, and customizes MSR
> > > registers in quirk callback, for example:
> > > 
> > > unsigned int msr_id_need_to_save[] = {MSR_ID0, MSR_ID1, MSR_ID2...};
> > > 
> > > and system ensures that, once resumed from suspend, these MSR indicated
> > > by IDs will be restored to their original values before suspend.
> > > 
> > > Since both 64/32-bit kernels are affected, this patch covers 64/32-bit
> > > common code path. And because the MSR ids specified by user might not be
> > > available or readable in any situation, we use rdmsrl_safe to safely
> > > save these MSR registers.
> > > 
> > > Tested-by: Marcin Kaszewski <marcin.kaszewski@intel.com>
> > > Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> > 
> > Acked-by: Pavel Machek <pavel@ucw.cz>
> 
> So I like the general structure of the patch and I like the MSR saving mechanism 
> it introduces, but it is full of typos and small stylistic uncleanlinesses which 
> need to be fixed before this patch can be applied. Please don't ack incomplete 
> patches.

Plus, I acked V2 when V3 was available.

I was under impression it was a regression, so "slightly urgent", but
it looks that was wrong impression, too.

Sorry,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1211747 — RE: [PATCH] [v2] x86, suspend: Save/restore extra MSR registers for suspend

Hi, Pavel,
> Plus, I acked V2 when V3 was available.
> 
V3 is to only save/restore THERM_CONTRO msr,  
so I think I'll do some check based on V2, It is more extensible.
Thanks,
Best Regards,
Yu
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1211746 — RE: [PATCH] [v2] x86, suspend: Save/restore extra MSR registers for suspend

Hi Ingo, thanks for your reply,
 
> So I like the general structure of the patch and I like the MSR saving
> mechanism it introduces, but it is full of typos and small stylistic
> uncleanlinesses which need to be fixed before this patch can be applied.
> Please don't ack incomplete patches.
> 
Sorry for my poor English, I'll check these typos.

Best Regards,
Yu

> Thanks,
> 
> 	Ingo

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1211924 — Re: [PATCH] [v2] x86, suspend: Save/restore extra MSR registers for suspend

On Fri, Aug 21, 2015 at 07:53:34PM +0800, Chen Yu wrote:
> A bug is reported(https://bugzilla.redhat.com/show_bug.cgi?id=1227208)
> that, after resuming from S3, CPU is working at a low speed.
> After investigation, it is found that, BIOS has modified the value
> of THERM_CONTROL register during S3, changes it from 0 to 0x10,
> while the latter means CPU can only get 25% of the Duty Cycle,
> and this caused the problem.
> 
> Simple scenario to reproduce:
> 1.Boot up system
> 2.Get MSR with address 0x19a, it should output 0
> 3.Put system into sleep, then wake up
> 4.Get MSR with address 0x19a, it should output 0(actual it outputs 0x10)
> 
> Although this is a BIOS issue, it would be more robust for linux to deal
> with this situation. This patch fixes this issue by introducing a framework
> for saving/restoring specify MSR registers(THERM_CONTROL in this case)
> on suspend/resume.
> 
> When user finds a problematic platform that requires save/restore MSRs,
> he can simply add quirk in msr_save_dmi_table, and customizes MSR
> registers in quirk callback, for example:
> 
> unsigned int msr_id_need_to_save[] = {MSR_ID0, MSR_ID1, MSR_ID2...};
> 
> and system ensures that, once resumed from suspend, these MSR indicated
> by IDs will be restored to their original values before suspend.
> 
> Since both 64/32-bit kernels are affected, this patch covers 64/32-bit
> common code path. And because the MSR ids specified by user might not be
> available or readable in any situation, we use rdmsrl_safe to safely
> save these MSR registers.
> 
> Tested-by: Marcin Kaszewski <marcin.kaszewski@intel.com>
> Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> ---
> v2:
>  - Cover both 64/32-bit common code path.
>    Use rdmsrl_safe to safely read MSR.
>    Introduce a quirk framework for save/restore specified MSR on different
>    platforms.
> ---
>  arch/x86/include/asm/suspend_32.h | 12 +++++
>  arch/x86/include/asm/suspend_64.h | 12 +++++
>  arch/x86/power/cpu.c              | 93 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 117 insertions(+)
> 
> diff --git a/arch/x86/include/asm/suspend_32.h b/arch/x86/include/asm/suspend_32.h
> index d1793f0..07b0443 100644
> --- a/arch/x86/include/asm/suspend_32.h
> +++ b/arch/x86/include/asm/suspend_32.h
> @@ -9,12 +9,24 @@
>  #include <asm/desc.h>
>  #include <asm/fpu/api.h>
>  
> +struct msr_type {
> +	unsigned int msr_id;
> +	bool msr_saved;
> +	u64 msr_value;
> +};

These definitions look awfully close to struct msr_info in
include/asm/msr.h

Maybe reuse them instead of growing yet another type...

> +
> +struct saved_msr {
> +	unsigned short num;
> +	struct msr_type *msr_array;
> +} __attribute__((packed));



> +
>  /* image of the saved processor state */
>  struct saved_context {
>  	u16 es, fs, gs, ss;
>  	unsigned long cr0, cr2, cr3, cr4;
>  	u64 misc_enable;
>  	bool misc_enable_saved;
> +	struct saved_msr msr_for_save;
>  	struct desc_ptr gdt_desc;
>  	struct desc_ptr idt;
>  	u16 ldt;
> diff --git a/arch/x86/include/asm/suspend_64.h b/arch/x86/include/asm/suspend_64.h
> index 7ebf0eb..321e288 100644
> --- a/arch/x86/include/asm/suspend_64.h
> +++ b/arch/x86/include/asm/suspend_64.h
> @@ -9,6 +9,17 @@
>  #include <asm/desc.h>
>  #include <asm/fpu/api.h>
>  
> +struct msr_type {
> +	unsigned int msr_id;
> +	bool msr_saved;
> +	u64 msr_value;
> +};
> +
> +struct saved_msr {
> +	unsigned short num;
> +	struct msr_type *msr_array;
> +} __attribute__((packed));
> +
>  /*
>   * Image of the saved processor state, used by the low level ACPI suspend to
>   * RAM code and by the low level hibernation code.
> @@ -24,6 +35,7 @@ struct saved_context {
>  	unsigned long cr0, cr2, cr3, cr4, cr8;
>  	u64 misc_enable;
>  	bool misc_enable_saved;
> +	struct saved_msr msr_for_save;
>  	unsigned long efer;
>  	u16 gdt_pad; /* Unused */
>  	struct desc_ptr gdt_desc;
> diff --git a/arch/x86/power/cpu.c b/arch/x86/power/cpu.c
> index 9ab5279..ed6c562 100644
> --- a/arch/x86/power/cpu.c
> +++ b/arch/x86/power/cpu.c
> @@ -23,6 +23,7 @@
>  #include <asm/debugreg.h>
>  #include <asm/cpu.h>
>  #include <asm/mmu_context.h>
> +#include <linux/dmi.h>
>  
>  #ifdef CONFIG_X86_32
>  __visible unsigned long saved_context_ebx;
> @@ -32,6 +33,30 @@ __visible unsigned long saved_context_eflags;
>  #endif
>  struct saved_context saved_context;
>  
> +static void msr_save_context(struct saved_context *ctxt)
> +{
> +	int i = 0;
> +
> +	for (i = 0; i < ctxt->msr_for_save.num; i++) {
> +		struct msr_type *msr =
> +			&ctxt->msr_for_save.msr_array[i];

No need for the line breaks here, let them stick out for better readability.

> +		msr->msr_saved = !rdmsrl_safe(msr->msr_id,
> +			&msr->msr_value);
> +	}
> +}
> +
> +static void msr_restore_context(struct saved_context *ctxt)
> +{
> +	int i = 0;
> +
> +	for (i = 0; i < ctxt->msr_for_save.num; i++) {
> +		struct msr_type *msr =
> +			&ctxt->msr_for_save.msr_array[i];
> +		if (msr->msr_saved)
> +			wrmsrl(msr->msr_id, msr->msr_value);

Ditto.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1212635 — RE: [PATCH] [v2] x86, suspend: Save/restore extra MSR registers for suspend

SGksIEJvcmlzLHRoYW5rcyBmb3IgeW91ciByZXZpZXcNCg0KPiAtLS0tLU9yaWdpbmFsIE1lc3Nh
Z2UtLS0tLQ0KPiBGcm9tOiBCb3Jpc2xhdiBQZXRrb3YgW21haWx0bzpicEBhbGllbjguZGVdDQo+
IFNlbnQ6IE1vbmRheSwgQXVndXN0IDI0LCAyMDE1IDQ6NTAgUE0NCj4gVG86IENoZW4sIFl1IEMN
Cj4gQ2M6IHJqd0Byand5c29ja2kubmV0OyBwYXZlbEB1Y3cuY3o7IHRnbHhAbGludXRyb25peC5k
ZTsNCj4gbWluZ29AcmVkaGF0LmNvbTsgaHBhQHp5dG9yLmNvbTsgWmhhbmcsIFJ1aTsgbGVuYkBr
ZXJuZWwub3JnOw0KPiB4ODZAa2VybmVsLm9yZzsgbGludXgtcG1Admdlci5rZXJuZWwub3JnOyBs
aW51eC1rZXJuZWxAdmdlci5rZXJuZWwub3JnDQo+IFN1YmplY3Q6IFJlOiBbUEFUQ0hdIFt2Ml0g
eDg2LCBzdXNwZW5kOiBTYXZlL3Jlc3RvcmUgZXh0cmEgTVNSIHJlZ2lzdGVycyBmb3INCj4gc3Vz
cGVuZA0KPiANCj4gT24gRnJpLCBBdWcgMjEsIDIwMTUgYXQgMDc6NTM6MzRQTSArMDgwMCwgQ2hl
biBZdSB3cm90ZToNCj4gPiArc3RydWN0IG1zcl90eXBlIHsNCj4gPiArCXVuc2lnbmVkIGludCBt
c3JfaWQ7DQo+ID4gKwlib29sIG1zcl9zYXZlZDsNCj4gPiArCXU2NCBtc3JfdmFsdWU7DQo+ID4g
K307DQo+IA0KPiBUaGVzZSBkZWZpbml0aW9ucyBsb29rIGF3ZnVsbHkgY2xvc2UgdG8gc3RydWN0
IG1zcl9pbmZvIGluIGluY2x1ZGUvYXNtL21zci5oDQo+IA0KPiBNYXliZSByZXVzZSB0aGVtIGlu
c3RlYWQgb2YgZ3Jvd2luZyB5ZXQgYW5vdGhlciB0eXBlLi4uDQo+IA0KT0ssICBJJ2xsIHVzZSBt
c3JfaW5mbyBpbnN0ZWFkIG9mICBtc3JfaWQgYW5kIG1zcl92YWx1ZToNCnN0cnVjdCBtc3JfdHlw
ZSB7DQoJYm9vbCBtc3Jfc2F2ZWQ7DQoJc3RydWN0IG1zcl9pbmZvIHJ2Ow0KfTsNCg0KPiA+ICtz
dGF0aWMgdm9pZCBtc3Jfc2F2ZV9jb250ZXh0KHN0cnVjdCBzYXZlZF9jb250ZXh0ICpjdHh0KSB7
DQo+ID4gKwlpbnQgaSA9IDA7DQo+ID4gKw0KPiA+ICsJZm9yIChpID0gMDsgaSA8IGN0eHQtPm1z
cl9mb3Jfc2F2ZS5udW07IGkrKykgew0KPiA+ICsJCXN0cnVjdCBtc3JfdHlwZSAqbXNyID0NCj4g
PiArCQkJJmN0eHQtPm1zcl9mb3Jfc2F2ZS5tc3JfYXJyYXlbaV07DQo+IA0KPiBObyBuZWVkIGZv
ciB0aGUgbGluZSBicmVha3MgaGVyZSwgbGV0IHRoZW0gc3RpY2sgb3V0IGZvciBiZXR0ZXIgcmVh
ZGFiaWxpdHkuDQo+IA0KT0ssIHdpbGwgZG8uDQo+ID4gKwkJbXNyLT5tc3Jfc2F2ZWQgPSAhcmRt
c3JsX3NhZmUobXNyLT5tc3JfaWQsDQo+ID4gKwkJCSZtc3ItPm1zcl92YWx1ZSk7DQo+ID4gKwl9
DQo+ID4gKwkJc3RydWN0IG1zcl90eXBlICptc3IgPQ0KPiA+ICsJCQkmY3R4dC0+bXNyX2Zvcl9z
YXZlLm1zcl9hcnJheVtpXTsNCj4gPiArCQlpZiAobXNyLT5tc3Jfc2F2ZWQpDQo+ID4gKwkJCXdy
bXNybChtc3ItPm1zcl9pZCwgbXNyLT5tc3JfdmFsdWUpOw0KPiANCj4gRGl0dG8uDQo+IA0KT0ss
IHdpbGwgZG8uDQoNCg0KQmVzdCBSZWdhcmRzLA0KWXUNCg==
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web