Fix boot hang in 3.11-rc1/2 because of bug in AMD errata check
PATCH 1/1 Fix boot hang in 3.11-rc1/2 because of bug in AMD errata check
cpu_has_amd_erratum() is buggy, because it uses the per-cpu cpu_info before it is filled by smp_store_boot_cpu_info() / smp_store_cpu_info().
If early microcode loading is enabled its collect_cpu_info_amd_early() will fill ->x86 and so the fallback to boot_cpu_data is not used.
But ->x86_vendor was not filled and is still 0 == X86_VENDOR_INTEL resulting in no errata fixes getting applied and my system hangs on boot.
Using cpu_info in cpu_has_amd_erratum() is wrong anyway: Its only caller init_amd() will have a struct cpuinfo_x86 as parameter and the set_cpu_bug() that is controlled by cpu_has_amd_erratum() also only uses that struct.
So pass the struct cpuinfo_x86 from init_amd() to cpu_has_amd_erratum() and the broken fallback can be dropped.
I also turned the vendor check into an BUG_ON() because init_amd() can only be used by AMD CPUs and if the current failure hadn't been silent this bug would have been much more obvious.
arch/x86/kernel/cpu/amd.c +7 -14
@@ -512,7 +512,7 @@ static void early_init_amd(struct cpuinfstatic const int amd_erratum_383[];static const int amd_erratum_400[]; -static bool cpu_has_amd_erratum (const int *erratum); +static bool cpu_has_amd_erratum (struct cpuinfo_x86 *cpu,const int *erratum);static void init_amd (struct cpuinfo_x86 *c) {
@@ -729,11 +729,11 @@ static void init_amd(struct cpuinfo_x86 value &= ~(1ULL <<24 );wrmsrl_safe (MSR_AMD64_BU_CFG2 , value); -if (cpu_has_amd_erratum (amd_erratum_383)) +if (cpu_has_amd_erratum (c, amd_erratum_383))set_cpu_bug (c,X86_BUG_AMD_TLB_MMATCH ); } -if (cpu_has_amd_erratum (amd_erratum_400)) +if (cpu_has_amd_erratum (c, amd_erratum_400))set_cpu_bug (c,X86_BUG_AMD_APIC_C1E );rdmsr_safe (MSR_AMD64_PATCH_LEVEL , &c->microcode, &dummy);
@@ -878,22 +878,15 @@ static const int amd_erratum_400[] =static const int amd_erratum_383[] =AMD_OSVW_ERRATUM (3 ,AMD_MODEL_RANGE (0x10 ,0 ,0 ,0xff ,0xf )); -static bool cpu_has_amd_erratum (const int *erratum) + +static bool cpu_has_amd_erratum (struct cpuinfo_x86 *cpu,const int *erratum) { -struct cpuinfo_x86 *cpu =__this_cpu_ptr (&cpu_info);int osvw_id = *erratum++;u32 range;u32 ms; -/* -* If called early enough that current_cpu_data hasn't been initialized -* yet, fall back to boot_cpu_data. -*/ -if (cpu->x86 ==0 ) - cpu = &boot_cpu_data; - -if (cpu->x86_vendor !=X86_VENDOR_AMD ) -return false; +/* Should never be called on non-AMD-CPUs */ +BUG_ON (cpu->x86_vendor !=X86_VENDOR_AMD );if (osvw_id >=0 && osvw_id <65536 &&cpu_has (cpu,X86_FEATURE_OSVW )) {
Borislav Petkov
maintainerBasically this patch looks ok, just a couple of nitpicks/comments.
Please change the subject to something more meaningful like
"x86, AMD: Make cpu_has_amd_erratum use currect cpuinfo descriptor" or so.
Quoted 12 lines · Torsten Kaiser
cpu_has_amd_erratum()is buggy, because it uses the per-cpucpu_infobefore it is filled bysmp_store_boot_cpu_info()/smp_store_cpu_info().If early microcode loading is enabled its
collect_cpu_info_amd_early()will fill ->x86 and so the fallback toboot_cpu_datais not used.
But ->x86_vendor was not filled and is still 0 ==X86_VENDOR_INTELresulting in no errata fixes getting applied and my system hangs on boot.Using
cpu_infoincpu_has_amd_erratum()is wrong anyway: Its only callerinit_amd()will have a structcpuinfo_x86as parameter and theset_cpu_bug()that is controlled bycpu_has_amd_erratum()also only uses that struct.
Quoted 6 lines
So pass the struct
cpuinfo_x86frominit_amd()tocpu_has_amd_erratum()and the broken fallback can be dropped.I also turned the vendor check into an
BUG_ON()becauseinit_amd()can only be used by AMD CPUs and if the current failure hadn't been silent this bug would have been much more obvious.
BUG_ON is kinda heavy-handed here, a WARN_ON should be fine.
Other than that, I like the commit message: it explains the whole deal in a very detailed manner and I'd wish more commit messages on lkml would be like that. Good job!
Quoted 55 lines
Signed-off-by Torsten Kaiser <[email protected]>arch/x86/kernel/cpu/amd.c +7
-14@@ -512,7 +512,7 @@ static void early_init_amd(struct cpuinfstatic const int amd_erratum_383[];static const int amd_erratum_400[]; -static bool cpu_has_amd_erratum (const int *erratum); +static bool cpu_has_amd_erratum (struct cpuinfo_x86 *cpu,const int *erratum);static void init_amd (struct cpuinfo_x86 *c) {@@ -729,11 +729,11 @@ static void init_amd(struct cpuinfo_x86 value &= ~(1ULL <<24 );wrmsrl_safe (MSR_AMD64_BU_CFG2 , value); -if (cpu_has_amd_erratum (amd_erratum_383)) +if (cpu_has_amd_erratum (c, amd_erratum_383))set_cpu_bug (c,X86_BUG_AMD_TLB_MMATCH ); } -if (cpu_has_amd_erratum (amd_erratum_400)) +if (cpu_has_amd_erratum (c, amd_erratum_400))set_cpu_bug (c,X86_BUG_AMD_APIC_C1E );rdmsr_safe (MSR_AMD64_PATCH_LEVEL , &c->microcode, &dummy);@@ -878,22 +878,15 @@ static const int amd_erratum_400[] =static const int amd_erratum_383[] =AMD_OSVW_ERRATUM (3 ,AMD_MODEL_RANGE (0x10 ,0 ,0 ,0xff ,0xf )); -static bool cpu_has_amd_erratum (const int *erratum) + +static bool cpu_has_amd_erratum (struct cpuinfo_x86 *cpu,const int *erratum) { -struct cpuinfo_x86 *cpu =__this_cpu_ptr (&cpu_info);int osvw_id = *erratum++;u32 range;u32 ms; -/* -* If called early enough that current_cpu_data hasn't been initialized -* yet, fall back to boot_cpu_data. -*/ -if (cpu->x86 ==0 ) - cpu = &boot_cpu_data; - -if (cpu->x86_vendor !=X86_VENDOR_AMD ) -return false; +/* Should never be called on non-AMD-CPUs */ +BUG_ON (cpu->x86_vendor !=X86_VENDOR_AMD );if (osvw_id >=0 && osvw_id <65536 &&cpu_has (cpu,X86_FEATURE_OSVW )) {
Signature · 5 lines
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--