Re: [PATCH] x86/microcode: Allow early loading without initrd

[Borislav Petkov <bp@alien8.de>]

On Sun, Apr 26, 2015 at 07:27:56PM +0200, Alexander Hirsch wrote:
> I wasn't to happy about the nested ifdefs either, but given my
> inexperience in the kernel code I didn't want to push things around
> too much.

Right, so this is the current situation: The intel early loader is being
aggressively cleaned up currently.

Then, the second early loading method which gets the builtin microcode,
i.e., not initrd, is not even upstream yet. I did test it a bit and it
worked fine but it needs more testing before it goes into 4.1.

Now, I would like to design the two methods cleanly by having Kconfig
suboptions which select BLK_DEV_INITRD or FW_LOADER and both options
have a good Kconfig help text explaining how one does either methods.

And, most importantly, untangle those two methods so that we can support
either and do that cleanly.

> I trimmed my kernel config down to what I need, think I need or don't
> trust myself to know if I can disable it. There is nothing actually
> hindering me from using an initrd, so this patch, at least for me
> personally, does not have a high priority to become mainlined. It
> still would be nice of course.

Right, so please use the current state with enabling BLK_DEV_INITRD
until this is done right. I have this on my radar and am working on it
so I'll get it done sooner or later.

> I had some segfaults due to the broken lock elision features of my CPU
> and found out that built-in microcode isn't considered (in the stable
> kernel). When I saw your patch for loading built-in microcode I simply
> thought that this would allow me to have early microcode patching
> without the need of an initrd.
>
> Boot-time might be improved by this and of course a few bytes are
> saved, but all in all probably not noteworthy. I did it, because (I
> thought) I can.

No, that's fine - you actually made me realize that we probably should
separate the early loading methods so that was a good thing. Thanks!

> It seems logical that the code was written with initrd in mind for
> early boot.

Oh sure. Perhaps the only advantage of the initrd method I can think of
is that if you want to upgrade microcode, you need to regenerate only
the initrd, not the whole kernel but you would have to reboot anyway to
load it.

For later loading without reboot we have a different method where we
don't need initrd or whatever.

So yeah, if we're going to support more than one early loading method,
I'd like to have those nice and clean and separated from one-another.

> What would be the downside of a microcode cache?

Oh nothing - it just needs to be written and tested :-)

> I didn't follow the whole flow of how the CPUs get to their
> microcode patches, but I thought the mc_saved* stuff did caching
> of the microcode, since scan_microcode, the only user of
> load_builtin_intel_microcode, is only called for the BSP and the other
> cores thus seem to get it from somewhere else.

Yeah, it is a bunch jumping through hoops which needs to be simplified
first :-)

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--