On 07/10/2013 01:52 PM, Christian Sünkenberg wrote:
> Hello,
>
> On 05/01/2013 07:33 PM, H. Peter Anvin wrote:
>> On 05/01/2013 10:01 AM, Jonas Heinrich wrote:
>>> Hello, I tried the newest kernel, 3.9 today but the bug is still
>>> present. Applying the attached patch solves the bug for me.
>>>
>>> Best regards, Jonas Heinrich
>>
>> Okay... WTF is going on here? Does pmode_behavior just not get set up
>> correctly? Since it seems you can get it to wake up with your patch,
>> perhaps we can get read out the value of pmode_behavior and print it...
>
> indeed, arch/x86/kernel/acpi/sleep.c tries an rdmsr_safe(MSR_EFER, ...)
> and sets WAKEUP_BEHAVIOR_RESTORE_EFER bit on success, however,
> on 90 nm Pentium M (Family 6, Model 13), reading an invalid MSR
> is not guaranteed to trap, see Erratum X4 in "Intel® Pentium® M
> Processor on 90 nm Process with 2-MB L2 Cache and Intel® Processor A100
> and A110 on 90 nm process with 512-KB L2 Cache Specification Update".
> On Jonas' T43, which has an affected Pentium M without EFER,
> rdmsr_safe(MSR_EFER, ...) succeeds and WAKEUP_BEHAVIOR_RESTORE_EFER
> gets set, while on resume the corresponding wrmsr traps and thus resume
> fails.
>
> The pre-3.7 code snippet incidentally catched this by not restoring
> EFER when it would be restored to all 0s.
>
That does seem like a reasonable explanation.
Does this patch fix the problem? (Comment blatantly ripped off from
your email message.)
-hpa
diff --git a/arch/x86/kernel/acpi/sleep.c b/arch/x86/kernel/acpi/sleep.c
index b44577b..927c5ce 100644
--- a/arch/x86/kernel/acpi/sleep.c
+++ b/arch/x86/kernel/acpi/sleep.c
@@ -48,9 +48,20 @@ int acpi_suspend_lowlevel(void)
#ifndef CONFIG_64BIT
native_store_gdt((struct desc_ptr *)&header->pmode_gdt);
+ /*
+ * We have to check that we can write back the value, and not
+ * just read it. At least on 90 nm Pentium M (Family 6, Model
+ * 13), reading an invalid MSR is not guaranteed to trap, see
+ * Erratum X4 in "Intel Pentium M Processor on 90 nm Process
+ * with 2-MB L2 Cache and Intel® Processor A100 and A110 on 90
+ * nm process with 512-KB L2 Cache Specification Update".
+ */
if (!rdmsr_safe(MSR_EFER,
&header->pmode_efer_low,
- &header->pmode_efer_high))
+ &header->pmode_efer_high) &&
+ !wrmsr_safe(MSR_EFER,
+ header->pmode_efer_low,
+ header->pmode_efer_high))
header->pmode_behavior |= (1 << WAKEUP_BEHAVIOR_RESTORE_EFER);
#endif /* !CONFIG_64BIT */
