| Message ID | 51DDF4DF.4000902@zytor.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
Hello, On 07/11/2013 01:57 AM, H. Peter Anvin wrote: > 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.) Jonas tried your patch and it fixes suspend/resume on his T43, although IMHO the safest approach would be to just add an exception for Vendor==Intel && Family==6 && Model==13, or more generally Vendor==Intel && !supports_long_mode, as the same erratum also warns about wrmsr possibly not triggering a GP either. Anyways, at least on this specific MSR with the Pentium M Jonas tested, it behaved correctly on every try, so I'd say your patch does the trick, thank you very much! As a side note, I found a similar erratum #33 in "Pentium® Processor Specification Update" for Intel P54C (Family 5, Model 2), which would, supposed there are P54C systems with ACPI sleep/resume support, result in MSR 0 (P5_MC_ADDR) to be saved and restored instead of nonexistent EFER. Kind regards, Christian
On 07/12/2013 04:36 PM, Christian Sünkenberg wrote: > > Jonas tried your patch and it fixes suspend/resume on his T43, although > IMHO the safest approach would be to just add an exception for > Vendor==Intel && Family==6 && Model==13, or more generally Vendor==Intel > && !supports_long_mode, as the same erratum also warns about wrmsr > possibly not triggering a GP either. > Anyways, at least on this specific MSR with the Pentium M Jonas tested, > it behaved correctly on every try, so I'd say your patch does the trick, > thank you very much! > Using vendor matches is not really a great way to deal with things that can better be handled analytically. If WRMSR doesn't fault, it is not a problem... > As a side note, I found a similar erratum #33 in "Pentium® Processor > Specification Update" for Intel P54C (Family 5, Model 2), which would, > supposed there are P54C systems with ACPI sleep/resume support, result > in MSR 0 (P5_MC_ADDR) to be saved and restored instead of nonexistent EFER. Doesn't really matter, as we'd only read that one after an #MC. -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 */