Messages in this thread Patch in this message | | | Date | Tue, 31 Jan 2017 08:43:55 +0100 | From | Ingo Molnar <> | Subject | Re: [PATCH] x86/microcode: Do not access the initrd after it has been freed |
| |
(Cc:-ed Mike as this could explain his early boot crash/hang? Mike: please try -tip f18a8a0143b1 that I just pushed out. )
* Borislav Petkov <bp@alien8.de> wrote:
> On Mon, Jan 30, 2017 at 09:46:32AM +0100, Ingo Molnar wrote: > > Ok, I have applied this to tip:x86/urgent. > > > > Note that there are new conflicts with your pending work in tip:x86/microcode, and > > I fixed them up in: > > > > 7c5b4112040e Merge branch 'x86/urgent' into x86/microcode, to resolve conflicts > > > > Could you please double-check my conflict resolution? > > Almost, this part is wrong: > > --------------------- arch/x86/kernel/cpu/microcode/amd.c --------------------- > index 7889ae492af0,079e81733a58..73082365ed1c > @@@ -268,20 -316,43 +268,20 @@@ void __load_ucode_amd(unsigned int cpui > use_pa = false; > } > > - if (!get_builtin_microcode(&cp, x86_family(cpuid_1_eax))) > - if (!get_builtin_microcode(&cp, family)) > ++ if (!get_builtin_microcode(&cp, x86_family(cpuid_1_eax)) && !initrd_gone) > cp = find_microcode_in_initrd(path, use_pa); > > -- > > Btw, I did experiment with the merging because I knew it'll cause > trouble due to the urgent fix and here's what I did: > > You're merging tip/x86/urgent into tip/x86/microcode so I checked out > the microcode branch and did: > > $ git checkout -b tip-microcode tip/x86/microcode > $ git merge -s recursive -X ours tip/x86/urgent > > This way I'm favouring our changes in the conflicting files. It merges > cleanly and the resulting diff is below.
Nice - I've updated the branch with your resolution. Could you please double-check the double checked resolution?
> The logic behind it is is that tip/x86/microcode does away with a bunch > of code and the urgent change touches some of that code but that's only > for 4.10. > > It goes away in 4.11 and that's why we should prefer "ours" as the merge > option. > > [ Btw, I'll send a patch for 4.11 later to make initrd_gone static as > it is going to be used only in microcode/core.c after the cleanup. ] > > However, I still haven't figured out how to say "prefer ours but only > for specific files or subtree" because the diff has that hunk in > arch/x86/kernel/fpu/core.c too which should definitely not be "ours" as > it is a fix and there the urgent version should be the one going in. > > Hmmm.
So the diff between your resolution and mine is attached below - now fpu/core.c changes, so I'm not sure why fpu/core.c is in your diff?
Thanks,
Ingo
diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c index 73082365ed1c..7889ae492af0 100644 --- a/arch/x86/kernel/cpu/microcode/amd.c +++ b/arch/x86/kernel/cpu/microcode/amd.c @@ -268,7 +268,7 @@ void __load_ucode_amd(unsigned int cpuid_1_eax, struct cpio_data *ret) use_pa = false; } - if (!get_builtin_microcode(&cp, x86_family(cpuid_1_eax)) && !initrd_gone) + if (!get_builtin_microcode(&cp, x86_family(cpuid_1_eax))) cp = find_microcode_in_initrd(path, use_pa); /* Needed in load_microcode_amd() */ diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c index e51eeaed8016..b4a4cd39b358 100644 --- a/arch/x86/kernel/cpu/microcode/core.c +++ b/arch/x86/kernel/cpu/microcode/core.c @@ -230,7 +230,7 @@ static int __init save_microcode_in_initrd(void) break; case X86_VENDOR_AMD: if (c->x86 >= 0x10) - ret = save_microcode_in_initrd_amd(cpuid_eax(1)); + return save_microcode_in_initrd_amd(cpuid_eax(1)); break; default: break;
| |