x86/microcode update on systems without INITRD

It looks like the ability to update x86/microcode without using an initrd was removed this merge window.

Quoted 2 lines · Markus Trippelsdorf

It looks like the ability to update x86/microcode without using an initrd was removed this merge window.

Whoops, that shouldnt've happened. Will debug it tomorrow and provide a fix.

Quoted 6 lines · Borislav Petkov

Quoted 2 lines · Markus Trippelsdorf

It looks like the ability to update x86/microcode without using an initrd was removed this merge window.

Whoops, that shouldnt've happened. Will debug it tomorrow and provide a fix.

Anyway, the hunk below seems to work in my guest here, I'll run it on the rest of the boxes tomorrow. In case you want to give it a try before:

Quoted 5 lines · Borislav Petkov

Quoted 4 lines · Borislav Petkov

Quoted 2 lines · Markus Trippelsdorf

It looks like the ability to update x86/microcode without using an initrd was removed this merge window.

Quoted 6 lines

Quoted 2 lines

Whoops, that shouldnt've happened. Will debug it tomorrow and provide a fix.

Anyway, the hunk below seems to work in my guest here, I'll run it on the rest of the boxes tomorrow. In case you want to give it a try before:

Your patch works fine. Thanks.

But of course it needs this additional patch, otherwise the microcode loader wouldn't build at all:

arch/x86/Kconfig +0 -0

index db3622f22b61..52c6964e24bd 100644

Quoted 6 lines · Markus Trippelsdorf

Quoted 5 lines · Borislav Petkov

Quoted 4 lines · Borislav Petkov

Quoted 2 lines · Markus Trippelsdorf

It looks like the ability to update x86/microcode without using an initrd was removed this merge window.

Quoted 8 lines

Quoted 6 lines

Quoted 2 lines

Whoops, that shouldnt've happened. Will debug it tomorrow and provide a fix.

Anyway, the hunk below seems to work in my guest here, I'll run it on the rest of the boxes tomorrow. In case you want to give it a try before:

Your patch works fine. Thanks.

Thanks for testing.

Quoted 3 lines

But of course it needs this additional patch, otherwise the microcode loader wouldn't build at all:

arch/x86/Kconfig config MICROCODE · line 1130

Quoted 11 lines

arch/x86/Kconfig +0 -1

index db3622f22b61..52c6964e24bd 100644
@@ -1126,7 +1126,6 @@ config MICROCODE
 	bool "CPU microcode loading support"
 	default y
 	depends on CPU_SUP_AMD || CPU_SUP_INTEL
-	depends on BLK_DEV_INITRD
 	select FW_LOADER
 	---help---

That dependency is a must as the early loader needs the initrd support.

Well, BLK_DEV_INITRD is not set in my config and it works just fine...

Quoted 6 lines · Markus Trippelsdorf

Quoted 5 lines · Borislav Petkov

Quoted 4 lines · Borislav Petkov

Quoted 2 lines · Markus Trippelsdorf

It looks like the ability to update x86/microcode without using an initrd was removed this merge window.

Quoted 8 lines

Quoted 6 lines

Quoted 2 lines

Whoops, that shouldnt've happened. Will debug it tomorrow and provide a fix.

Anyway, the hunk below seems to work in my guest here, I'll run it on the rest of the boxes tomorrow. In case you want to give it a try before:

Your patch works fine. Thanks.

Thanks for testing.

Quoted 3 lines

But of course it needs this additional patch, otherwise the microcode loader wouldn't build at all:

arch/x86/Kconfig config MICROCODE · line 1130

Quoted 11 lines

arch/x86/Kconfig +0 -1

index db3622f22b61..52c6964e24bd 100644
@@ -1126,7 +1126,6 @@ config MICROCODE
 	bool "CPU microcode loading support"
 	default y
 	depends on CPU_SUP_AMD || CPU_SUP_INTEL
-	depends on BLK_DEV_INITRD
 	select FW_LOADER
 	---help---

That dependency is a must as the early loader needs the initrd support.

I just diffed my 4.3 config with the 4.4 config and saw that the whole Microcode stuff was silently dropped by a normal "make oldconfig".

Quoted 2 lines · Thomas Voegtle

I just diffed my 4.3 config with the 4.4 config and saw that the whole Microcode stuff was silently dropped by a normal "make oldconfig".

Can you send me that 4.3 config please?

Quoted 4 lines · Borislav Petkov

Quoted 2 lines · Thomas Voegtle

I just diffed my 4.3 config with the 4.4 config and saw that the whole Microcode stuff was silently dropped by a normal "make oldconfig".

Quoted 1 line

Can you send me that 4.3 config please?

Attached. It is a little bit unusual config without modules etc.

But doesn't dropping Microcde stuff in the config happen to anyone who hasn't INITRD stuff switched on?

Quoted 8 lines · Thomas Voegtle

Quoted 5 lines · Borislav Petkov

Quoted 2 lines · Thomas Voegtle

I just diffed my 4.3 config with the 4.4 config and saw that the whole Microcode stuff was silently dropped by a normal "make oldconfig".

Can you send me that 4.3 config please?

Quoted 4 lines

Attached. It is a little bit unusual config without modules etc.

But doesn't dropping Microcde stuff in the config happen to anyone who hasn't INITRD stuff switched on?

Yes. But, as I wrote above, if you simply drop the BLK_DEV_INITRD dependency, it will work just fine.

Quoted 7 lines · Thomas Voegtle

Quoted 5 lines · Borislav Petkov

Quoted 3 lines · Thomas Voegtle

I just diffed my 4.3 config with the 4.4 config and saw that the whole Microcode stuff was silently dropped by a normal "make oldconfig".

Quoted 6 lines

Quoted 1 line

Can you send me that 4.3 config please?

Attached. It is a little bit unusual config without modules etc.

But doesn't dropping Microcde stuff in the config happen to anyone who hasn't INITRD stuff switched on?

Yes. But, as I wrote above, if you simply drop the BLK_DEV_INITRD dependency, it will work just fine.
I also don't use modules and the firmware gets applied at boot time.

...

[    2.573261] EDAC amd64: MCT channel count: 2
[    2.573401] EDAC MC0: Giving out device to module amd64_edac

controller F10h: DEV 0000:00:18.2 (INTERRUPT) [ 2.573414] EDAC
PCI0: Giving out device to module amd64_edac controller EDAC PCI
controller: DEV 0000:00:18.2 (POLLED) [ 2.573424] hidraw: raw HID
events driver (C) Jiri Kosina [ 2.573449] usbcore: registered new
interface driver usbhid [ 2.573449] usbhid: USB HID core driver

[    2.573770] usbcore: registered new interface driver snd-usb-audio
[    2.573786] Netfilter messages via NETLINK v0.30. [    2.573797]

nf_conntrack version 0.5.0 (65536 buckets, 262144 max) [ 2.573966]
ctnetlink v0.93: registering with nfnetlink. [ 2.574427]
ip_tables: (C) 2000-2006 Netfilter Core Team [ 2.574477] NET:
Registered protocol family 17 [ 2.574487] 9pnet: Installing 9P2000
support [ 2.574717] microcode: CPU0: patch_level=0x010000db

[    2.574724] microcode: CPU1: patch_level=0x010000db
[    2.574731] microcode: CPU2: patch_level=0x010000db
[    2.574736] microcode: CPU3: patch_level=0x010000db
[    2.574761] microcode: Microcode Update Driver: v2.01

<[email protected]>, Peter Oruba [ 2.574876] registered
taskstats version 1 [ 2.575132] Btrfs loaded
...

Quoted 34 lines · On Thu, 7 Jan 2016 13:41:11 +0100 Markus Trippelsdorf <[email protected]>…

Quoted 7 lines · Thomas Voegtle

Quoted 5 lines · Borislav Petkov

Quoted 3 lines · Thomas Voegtle

I just diffed my 4.3 config with the 4.4 config and saw that the whole Microcode stuff was silently dropped by a normal "make oldconfig".

Quoted 6 lines

Quoted 1 line

Can you send me that 4.3 config please?

Attached. It is a little bit unusual config without modules etc.

But doesn't dropping Microcde stuff in the config happen to anyone who hasn't INITRD stuff switched on?

Yes. But, as I wrote above, if you simply drop the BLK_DEV_INITRD dependency, it will work just fine.
I also don't use modules and the firmware gets applied at boot time.

...

[    2.573261] EDAC amd64: MCT channel count: 2
[    2.573401] EDAC MC0: Giving out device to module amd64_edac

controller F10h: DEV 0000:00:18.2 (INTERRUPT) [ 2.573414] EDAC
PCI0: Giving out device to module amd64_edac controller EDAC PCI
controller: DEV 0000:00:18.2 (POLLED) [ 2.573424] hidraw: raw HID
events driver (C) Jiri Kosina [ 2.573449] usbcore: registered new
interface driver usbhid [ 2.573449] usbhid: USB HID core driver

[    2.573770] usbcore: registered new interface driver snd-usb-audio
[    2.573786] Netfilter messages via NETLINK v0.30. [    2.573797]

nf_conntrack version 0.5.0 (65536 buckets, 262144 max) [ 2.573966]
ctnetlink v0.93: registering with nfnetlink. [ 2.574427]
ip_tables: (C) 2000-2006 Netfilter Core Team [ 2.574477] NET:
Registered protocol family 17 [ 2.574487] 9pnet: Installing 9P2000
support [ 2.574717] microcode: CPU0: patch_level=0x010000db

Quoted 8 lines

[    2.574724] microcode: CPU1: patch_level=0x010000db
[    2.574731] microcode: CPU2: patch_level=0x010000db
[    2.574736] microcode: CPU3: patch_level=0x010000db
[    2.574761] microcode: Microcode Update Driver: v2.01

<[email protected]>, Peter Oruba [ 2.574876] registered
taskstats version 1 [ 2.575132] Btrfs loaded
...

I'm not sure what you mean by "drop the BLK_DEV_INITRD dependency", but my microcode does not get loaded unless I have BLK_DEV_INITRD=Y in my .config.

I don't use initrd, but I do use modules.

Attached. It is a little bit unusual config without modules etc.

Quoted 1 line · Thomas Voegtle

Attached. It is a little bit unusual config without modules etc.

Ok, I see it.

Please do a proper patch explaining why we're changing "depends on" to "select" and we can try it, see who complains then and why.

Neither "depends on" nor "select" makes sense to me here. The driver apparently works without it,

and simply having BLK_DEV_INITRD enabled doesn't prevent improper (according to some people) use of the driver. If updating microcode is inherently unsafe when a real disk is mounted, the driver ought to detect this and refuse the operation (possibly with an override option).

Quoted 2 lines · Måns Rullgård

Neither "depends on" nor "select" makes sense to me here. The driver apparently works without it,

The driver works without it if you build your microcode into the kernel.

There are use cases where building microcode into the kernel is *not* a viable option so we have to support both builtin microcode and microcode from the initrd.

Quoted 5 lines

and simply having BLK_DEV_INITRD enabled doesn't prevent improper (according to some people) use of the driver. If updating microcode is inherently unsafe when a real disk is mounted, the driver ought to detect this and refuse the operation (possibly with an override option).

Huh, what?

-ENOPARSE.

How is an initrd different from a real filesystem as seen by the microcode update driver?

The objection against removing the dependency was that updating microcode "late" isn't safe. I don't see how turning on BLK_DEV_INITRD stops anyone doing those allegedly unsafe updates anyway.

Quoted 2 lines · Måns Rullgård

How is an initrd different from a real filesystem as seen by the microcode update driver?

For starters, initrd is available much earlier, even before paging is enabled on 32-bit, for example. See find_cpio_data().

Quoted 3 lines

The objection against removing the dependency was that updating microcode "late" isn't safe. I don't see how turning on BLK_DEV_INITRD stops anyone doing those allegedly unsafe updates anyway.

No one is stopping anyone from doing late updates. It is a valid use case, and we have to support it. And late updates are not necessarily unsafe, per se.

Lemme put it this way: it is a lot less unproblematic to do early updates. Mind you, there's no 100% guarantee that early updates would always work either. It all depends on what the microcode patch does. But they do work 99,9999999...% of the time. :)

IOW, I haven't heard of an early update breaking the machine. But it is possible.

So the *general* flow should be that people enable BLK_DEV_INITRD, put the microcode in there and it gets updated as early as possible. This is what the distros do and it is the most tested path. The other possibilities are there too, but only for cases where initrd is out of the question.

But you take the choice away from people like me, who don't need initrd at all. BLK_DEV_INITRD is a superfluous dependency in this case, because microcode update works perfectly well without it.

Quoted 3 lines · Markus Trippelsdorf

But you take the choice away from people like me, who don't need initrd at all. BLK_DEV_INITRD is a superfluous dependency in this case, because microcode update works perfectly well without it.

Damn, you have a valid point too. I need to think about it...

... well, the only thing I can think of right now is to remove the dependency on BLK_DEV_INITRD completely - I believe this is what you proposed initially Markus - and add a BIG FAT SUGGESTION to Kconfig saying that people should strive for enabling early microcode loading if possible.

Quoted 3 lines · Markus Trippelsdorf

But you take the choice away from people like me, who don't need initrd at all. BLK_DEV_INITRD is a superfluous dependency in this case, because microcode update works perfectly well without it.

Damn, you have a valid point too. I need to think about it...

... well, the only thing I can think of right now is to remove the dependency on BLK_DEV_INITRD completely - I believe this is what you proposed initially Markus - and add a BIG FAT SUGGESTION to Kconfig saying that people should strive for enabling early microcode loading if possible.

This is the only way I see we can handle "make oldconfig" without BLK_DEV_INITRD and microcode built-in into the kernel cases relatively fair.

Hmmm.

Or I could hack "oldconfig" to issue that warning... Lemme think about it and see how much Michal would scream at me for it. :-)

You can add a conditional comment like this

comment "WARNING: Early microcode loader requires initramfs support"

	depends on MICROCODE && !BLK_DEV_INITRD

and hope that somebody reads it.

Quoted 6 lines · Michal Marek

You can add a conditional comment like this

comment "WARNING: Early microcode loader requires initramfs support"

	depends on MICROCODE && !BLK_DEV_INITRD

and hope that somebody reads it.

Actually, I was thinking about something which gets printed when doing "make oldconfig". In addition to the Kconfig text.

I'll take a look once
the stupid cold goes away...

The comments are printed during make oldconfig if a symbol in the current menu needs updating. Which is probably not the case here, both MICROCODE and BLK_DEV_INITRD are existing symbols.

Quoted 3 lines · Michal Marek

The comments are printed during make oldconfig if a symbol in the current menu needs updating. Which is probably not the case here, both MICROCODE and BLK_DEV_INITRD are existing symbols.

Right, and the problem is that "make oldconfig" sees that BLK_DEV_INITRD is not enabled in that case and disables CONFIG_MICROCODE too. So by the time the comment gets evaluated, CONFIG_MICROCODE is off so no workie.

I tried a different thing, see below. It is a bit lengthly but it does what it should and we can always extend it for other stuff later as it might turn useful, according to my suspicion :-)

With Thomas' config it says:

$ make oldconfig
You have CONFIG_MICROCODE enabled without BLK_DEV_INITRD. Enable
it and make sure microcode is added to your initrd as explained in
Documentation/x86/early-microcode.txt

scripts/kconfig/Makefile:85: recipe for target 'oldconfig' failed make[1]: *** [oldconfig] Error 1
Makefile:531: recipe for target 'oldconfig' failed
make: *** [oldconfig] Error 2

But this is wrong. Microcode update doesn't need initrd.

It needs to be made to look at CONFIG_MODULES and make that warning just a hint. I'll have to think about it more...

Quoted 1 line · Måns Rullgård

But this is wrong. Microcode update doesn't need initrd.

You need to read my mail to the end:

Quoted 2 lines

It needs to be made to look at CONFIG_MODULES and make that warning just a hint. I'll have to think about it more...

What do modules have to do with anything?

From: Borislav Petkov <[email protected]>

Thomas Voegtle reported that doing oldconfig with a .config which has CONFIG_MICROCODE enabled but BLK_DEV_INITRD disabled prevents the microcode loading mechanism from being built.

Add a short script which hooks into the "make oldconfig" handling and sanity-checks the config file for that discrepancy. It issues a message which should hopefully sensitize the user to that issue and point her into the right direction.

The other useful thing with this solution is that it can be extended to other config file sanity-checking, should the need arise.

Reported-by Thomas Voegtle <[email protected]>

Cc Markus Trippelsdorf <[email protected]>

Cc Måns Rullgård <[email protected]>

Signed-off-by Borislav Petkov <[email protected]>

---
arch/x86/scripts/check-configs.sh | 44 +++++++++++++++++++++++++++++++++++++++
scripts/kconfig/Makefile | 3 +++

2 files changed, 47 insertions(+)

create mode 100644 arch/x86/scripts/check-configs.sh

arch/x86/scripts/check-configs.sh new file +44 -0

new file mode 100644
index 000000000000..775d07e37df5
@@ -0,0 +1,44 @@
+#!/bin/bash
+
+if [ "$1" != "oldconfig" ]; then
+	exit 0
+fi
+
+srctree=$2
+ARCH="$3"
+UNAME_RELEASE=$(uname -r)
+
+CONFIGS=".config /lib/modules/$UNAME_RELEASE/.config /etc/kernel-config /boot/config-$UNAME_RELEASE"
+
+if [ "$ARCH" = "X86_32" ]; then
+	CONFIGS="$CONFIGS $srctree/arch/x86/configs/i386_defconfig"
+else
+	CONFIGS="$CONFIGS $srctree/arch/x86/configs/x86_64_defconfig"
+fi
+
+for c in $CONFIGS;
+do
+	if [ -e $c ]; then
+		OLD_CONFIG=$c
+		break
+	fi
+done
+
+if [ -z "$OLD_CONFIG" ]; then exit 0; fi
+
+# Check optimal microcode loader .config settings
+if ! grep -v "^#" $OLD_CONFIG | grep -q MICROCODE; then
+	exit 0
+fi
+
+MSG="\nYou have CONFIG_MICROCODE enabled without BLK_DEV_INITRD. The preferred\n\
+way is to enable it and make sure microcode is added to your initrd as\n\
+explained in Documentation/x86/early-microcode.txt. This is also the\n\
+most tested method as the majority of distros do it. Alternatively, and\n\
+if you don't want to enable modules, you should make sure the microcode\n\
+is built into the kernel.\n"
+
+if ! grep -v "^#" $OLD_CONFIG | grep -q BLK_DEV_INITRD; then
+	echo -e $MSG
+	read -p "Press any key... "
+fi

scripts/kconfig/Makefile +0 -0

index d79cba4ce3eb..136ae9744efc 100644
@@ -81,6 +81,9 @@ simple-targets := oldconfig allnoconfig allyesconfig allmodconfig \
 PHONY += $(simple-targets)
 

$(simple-targets): $(obj)/conf

scripts/kconfig/Makefile +3 -0

+ifneq ($(wildcard $(srctree)/arch/$(SRCARCH)/scripts/check-configs.sh),)
+	$(Q)$(CONFIG_SHELL) $(srctree)/arch/$(SRCARCH)/scripts/check-configs.sh $@ $(srctree) $(ARCH)
+endif

	$< $(silent) --$@ $(Kconfig)

PHONY += oldnoconfig savedefconfig defconfig

My problem was, CONFIG_MICROCODE got dropped silently, and yes that is fixed for me with this patch. But I think this is a little bit odd way to fix it, but I don't have a better idea.

What's with olddefconfig

and silentoldconfig ?

btw that patch has to go to stable 4.4, too

Quoted 3 lines · Thomas Voegtle

My problem was, CONFIG_MICROCODE got dropped silently, and yes that is fixed for me with this patch. But I think this is a little bit odd way to fix it, but I don't have a better idea.

Me neither. The problem is, I need to grep the config that goes into the "oldconfig" *before* scripts/kconfig/conf gets a hold of it and satisfies deps and thus turns off CONFIG_MICROCODE in the process.

This is a simpler solution short of hacking scripts/kconfig/conf and it can be used for other stuff we might need in arch/x86/

Quoted 1 line

What's with olddefconfig

Yes, I'll update the patch.

Quoted 1 line

and silentoldconfig ?

This one is funny. Makefile's help says:

silentoldconfig - Same as oldconfig, but quietly, additionally update deps

and yet doing

$ make silentoldconfig

is not very silent and goes through all new prompts asking me about them.

It asks about new options but doesn't print (as many) unchanged ones. That makes it more silent.

Wasn't the idea *not* to disable CONFIG_MICROCODE?

From the point of view of the actual update mechanism, what difference

I understand and disagree. I think you're being overzealous in trying to bludgeon people into doing things the way you think they should be done.

From the point of view of the actual update mechanism, what difference does it make where the microcode data was retrieved from? If you want to warn about what you consider "unsafe" updates, do that when the update happens instead. With this patch, simply enabling BLK_DEV_INITRD will shut up the warning even if an initrd is never actually used. Also, what do modules have to do with anything?

Quoted 3 lines · Måns Rullgård

I understand and disagree. I think you're being overzealous in trying to bludgeon people into doing things the way you think they should be done.

So I did explain why it is better to do microcode updates from the initrd. And nowhere in that explanation I am "bludgeoning" people into doing things the way I want. Which is silly, I'd never even *think* of wanting to do that - I have enough other shit to deal with.

Quoted 6 lines

From the point of view of the actual update mechanism, what difference does it make where the microcode data was retrieved from? If you want to warn about what you consider "unsafe" updates, do that when the update happens instead. With this patch, simply enabling BLK_DEV_INITRD will shut up the warning even if an initrd is never actually used. Also, what do modules have to do with anything?

This reads like your mail from a couple of days ago. Which leads me to think that you haven't understood at all what I've been writing this whole time.

From: Borislav Petkov <[email protected]>

Thomas Voegtle reported that doing oldconfig with a .config which has CONFIG_MICROCODE enabled but BLK_DEV_INITRD disabled prevents the microcode loading mechanism from being built.

Add a short script which hooks into the "make oldconfig" handling and sanity-checks the config file for that discrepancy. It issues a message which should hopefully sensitize the user to that issue and point her into the right direction.

From my own randconfig testing I can give a core list of must-have kernel options,

( I've Cc:-ed Linus, Greg and Andrew, to see whether doing something like what I
suggest below in the x86 architecture would be acceptable. )

Quoted 10 lines · Borislav Petkov

From: Borislav Petkov <[email protected]>

Thomas Voegtle reported that doing oldconfig with a .config which has CONFIG_MICROCODE enabled but BLK_DEV_INITRD disabled prevents the microcode loading mechanism from being built.

Add a short script which hooks into the "make oldconfig" handling and sanity-checks the config file for that discrepancy. It issues a message which should hopefully sensitize the user to that issue and point her into the right direction.

So it would be much better to just do such things automatically, and only allow 'safe' combination of options - without the user having to do anything.

The guiding principle is: kernel configuration is (still...) our worst barrier of entry for new users/developers, and kernel configuration still sucks very much from a UI point of view.

In fact our kernel configuration UI and workflow is still so bad that it's an effort to stay current even with a standalone and working .config, even for experienced kernel developers...

Adding a (somewhat hacky) post processing script and forcing users to read something 99% of them does not have a clue about is a step in the wrong direction, IMHO.

So can we do something more intelligent instead, such as modifying the Kconfigs in a way that it's not possible to have CONFIG_MICROCODE enabled while BLK_DEV_INITRD is disabled?

I'd be fine with a 'select BLK_DEV_INITRD' for example. If people doing super specialized setups disagree because they really need that nonsensical combination of config options, they can complain and provide a better solution.

In fact on x86 I'd suggest we go farther than that and add a core set of selects that can be disabled only through a sufficiently scary "I really know I'm doing something utmost weird" (and default disabled) config option.

Quoted 40 lines · Ingo Molnar

( I've Cc:-ed Linus, Greg and Andrew, to see whether doing something like what I
suggest below in the x86 architecture would be acceptable. )

Quoted 10 lines · Borislav Petkov

From: Borislav Petkov <[email protected]>

Thomas Voegtle reported that doing oldconfig with a .config which has CONFIG_MICROCODE enabled but BLK_DEV_INITRD disabled prevents the microcode loading mechanism from being built.

Add a short script which hooks into the "make oldconfig" handling and sanity-checks the config file for that discrepancy. It issues a message which should hopefully sensitize the user to that issue and point her into the right direction.

So it would be much better to just do such things automatically, and only allow 'safe' combination of options - without the user having to do anything.

The guiding principle is: kernel configuration is (still...) our worst barrier of entry for new users/developers, and kernel configuration still sucks very much from a UI point of view.

In fact our kernel configuration UI and workflow is still so bad that it's an effort to stay current even with a standalone and working .config, even for experienced kernel developers...

Adding a (somewhat hacky) post processing script and forcing users to read something 99% of them does not have a clue about is a step in the wrong direction, IMHO.

So can we do something more intelligent instead, such as modifying the Kconfigs in a way that it's not possible to have CONFIG_MICROCODE enabled while BLK_DEV_INITRD is disabled?

I'd be fine with a 'select BLK_DEV_INITRD' for example. If people doing super specialized setups disagree because they really need that nonsensical combination of config options, they can complain and provide a better solution.

Quoted 3 lines

In fact on x86 I'd suggest we go farther than that and add a core set of selects that can be disabled only through a sufficiently scary "I really know I'm doing something utmost weird" (and default disabled) config option.

This is essential. Because, believe it or not, there are still users out there that don't use systemd. And to force enable totally superfluous config options for them would be bad.

Quoted 3 lines

[...] from a usability POV it's _much_ better to have a few more options
enabled in a .config of thousands of entries, than to accidentally have the one option not enabled that your user-space somehow critically depends on ...

So, as long as this "systemd config" could be easily disabled, your approach looks fine and would definitely be helpful to many mainstream distro users.

The key point is that I'd like "naively configured" kernels to work on just about any Linux distro that allow kernel testing

(quoted diff) +35 -0

+	# newer systemd silently relies on the presence of the epoll system call:
+	select EPOLL
+	select ANON_INODES
+
+	# newer systemd silently hangs durig early init without these:
+	select PROC_SYSCTL
+	select SYSCTL
+	select POSIX_MQUEUE
+	select POSIX_MQUEUE_SYSCTL
+
+	# systemd needs this syscall:
+	select FHANDLE
+
+	# systemd needs devtmpfs: "systemd[1]: Failed to mount devtmpfs at /dev: No such device"
+	select DEVTMPFS
+
+	# systemd needs tmpfs: "systemd[1]: Failed to mount tmpfs at /sys/fs/cgroup: No such file or directory"
+	select SHMEM
+	select TMPFS
+
+	# systemd needs timerfd syscalls: "[    8.198625] systemd[1]: Failed to create timerfd: Function not implemented^"
+	select TIMERFD
+
+	# systemd needs signalfd support: "[   45.536725] systemd[1]: Failed to allocate manager object: Function not implemented"
+	select SIGNALFD
+
+	# systemd hangs during bootup without cgroup support:
+	select CGROUPS
+
+	# systemd fails during bootup without this option, with a nonsensical message: "[DEPEND] Dependency failed for File System Check on /dev/sda1."
+	select FILE_LOCKING
+
+	# systemd fails during bootup without this option:
+	select FSNOTIFY
+	select INOTIFY_USER

And yes, many of these options are members of the 'SystemD debuggability Hall Of Shame'... It cost me many, many days of painful config-bisection to figure the often obscure dependencies out, so we might as well upstream this information.

Many braincells died to bring us this information!

Quoted 35 lines · Ingo Molnar

(quoted diff) +35 -0

+	# newer systemd silently relies on the presence of the epoll system call:
+	select EPOLL
+	select ANON_INODES
+
+	# newer systemd silently hangs durig early init without these:
+	select PROC_SYSCTL
+	select SYSCTL
+	select POSIX_MQUEUE
+	select POSIX_MQUEUE_SYSCTL
+
+	# systemd needs this syscall:
+	select FHANDLE
+
+	# systemd needs devtmpfs: "systemd[1]: Failed to mount devtmpfs at /dev: No such device"
+	select DEVTMPFS
+
+	# systemd needs tmpfs: "systemd[1]: Failed to mount tmpfs at /sys/fs/cgroup: No such file or directory"
+	select SHMEM
+	select TMPFS
+
+	# systemd needs timerfd syscalls: "[    8.198625] systemd[1]: Failed to create timerfd: Function not implemented^"
+	select TIMERFD
+
+	# systemd needs signalfd support: "[   45.536725] systemd[1]: Failed to allocate manager object: Function not implemented"
+	select SIGNALFD
+
+	# systemd hangs during bootup without cgroup support:
+	select CGROUPS
+
+	# systemd fails during bootup without this option, with a nonsensical message: "[DEPEND] Dependency failed for File System Check on /dev/sda1."
+	select FILE_LOCKING
+
+	# systemd fails during bootup without this option:
+	select FSNOTIFY
+	select INOTIFY_USER

Quoted 5 lines

And yes, many of these options are members of the 'SystemD debuggability Hall Of Shame'... It cost me many, many days of painful config-bisection to figure the often obscure dependencies out, so we might as well upstream this information.

Many braincells died to bring us this information!

So why not group those under CONFIG_SYSTEMD_BLOWS? I still dont have a machine with that turd on.

In fact our kernel configuration UI and workflow is still so bad that it's an effort to stay current even with a standalone and working .config, even for experienced kernel developers...

Adding a (somewhat hacky) post processing script and forcing users to read something 99% of them does not have a clue about is a step in the wrong direction, IMHO.

So can we do something more intelligent instead, such as modifying the Kconfigs in a way that it's not possible to have CONFIG_MICROCODE enabled while BLK_DEV_INITRD is disabled?

I'd be fine with a 'select BLK_DEV_INITRD' for example. If people doing super specialized setups disagree because they really need that nonsensical combination of config options, they can complain and provide a better solution.

In fact on x86 I'd suggest we go farther than that and add a core set of selects that can be disabled only through a sufficiently scary "I really know I'm doing something utmost weird" (and default disabled) config option.

From my own randconfig testing I can give a core list of must-have kernel options, without which most distros (Fedora, RHEL, Ubuntu, SuSE) won't boot properly:

(quoted diff) +3 -0

+config FORCE_MINIMALLY_SANE_CONFIG
+	bool
+	default y

And yes, many of these options are members of the 'SystemD debuggability Hall Of Shame'... It cost me many, many days of painful config-bisection to figure the often obscure dependencies out, so we might as well upstream this information.

Many braincells died to bring us this information!

The idea is that if you have this option enabled, the rest of kernel config should be 'fool proof' - or at least failures should be a lot more obvious (such as a missing hardware driver or a missing filesystem driver).

Thoughs?

In fact on x86 I'd suggest we go farther than that and add a core set of selects that can be disabled only through a sufficiently scary "I really know I'm doing something utmost weird" (and default disabled) config option.

Quoted 3 lines · Ingo Molnar

In fact on x86 I'd suggest we go farther than that and add a core set of selects that can be disabled only through a sufficiently scary "I really know I'm doing something utmost weird" (and default disabled) config option.

Agreed.

Quoted 3 lines

From my own randconfig testing I can give a core list of must-have kernel options, without which most distros (Fedora, RHEL, Ubuntu, SuSE) won't boot properly:

Review

Quoted 3 lines

(quoted diff) +3 -0

+config FORCE_MINIMALLY_SANE_CONFIG
+	bool
+	default y

You should add a prompt so that the option can be disabled. Or make it default !EXPERT, to have a single "I know what I'm doing"-type of option.

So it should be something like:

config GENERIC_BOOTABLE_CONFIG
bool "Enable kernel options that are needed to boot typical Linux distributions" default y
...

(I removed the 'SANE' naming as disabling this option is obviously not 'insane'.)

Quoted 10 lines · Ingo Molnar

So it should be something like:

config GENERIC_BOOTABLE_CONFIG
bool "Enable kernel options that are needed to boot typical Linux distributions" default y
...

(I removed the 'SANE' naming as disabling this option is obviously not 'insane'.)

I think we should just make it distro-specific rather than claiming it is generic (and inevitably failing).

So we could have a config option for SYSTEMD, which selects stuff systemd wants, and then distros that use systemd can select that etc.

It shouldn't be about just bootability either. Some of the networking options end up being security-critical (ie your firewall might not work if you don't have the right options enabled, leaving you wide open after you boot).

So can we do something more intelligent instead, such as modifying the Kconfigs in a way that it's not possible to have CONFIG_MICROCODE enabled while BLK_DEV_INITRD is disabled?

Force-enabling BLK_DEV_INITRD isn't going to make anyone change their boot scripts.

I'd also like to get a coherent answer to why microcode update is preferably done from an initrd as opposed to shortly after mounting a regular disk. My systems seem perfectly happy doing the latter.

Dne 19.1.2016 v 13:29 Måns Rullgård napsal(a):

Quoted 2 lines

Force-enabling BLK_DEV_INITRD isn't going to make anyone change their boot scripts.

If you are on a regular distro, /sbin/installkernel should do the right
thing: Run mkinitrd / dracut and if the tools are recent enough and there is a microcode update for your CPU, a cpio with the microcode blob will be prepended to the initrd. So this is more or less covered.

Quoted 3 lines

I'd also like to get a coherent answer to why microcode update is preferably done from an initrd as opposed to shortly after mounting a regular disk. My systems seem perfectly happy doing the latter.

It's not even done *from* the initrd but way earlier. We learned the hard way when Intel released a microcode update for Haswell which disabled TSX: Userspace did not expect the feature flags to change and previously valid instructions to start trapping. This can in principle happen again and with any vendor.

(quoted diff) +1 -0

+	select SYSFS_DEPRECATED

And yes, many of these options are members of the 'SystemD debuggability Hall Of Shame'... It cost me many, many days of painful config-bisection to figure the often obscure dependencies out, so we might as well upstream this information.

Quoted 1 line · Ingo Molnar

(quoted diff) +1 -0

+	select SYSFS_DEPRECATED

...

Quoted 3 lines

And yes, many of these options are members of the 'SystemD debuggability Hall Of Shame'... It cost me many, many days of painful config-bisection to figure the often obscure dependencies out, so we might as well upstream this information.

And you've got it wrong ;)

http://cgit.freedesktop.org/systemd/systemd/tree/README#n54

udev will fail to work with the legacy sysfs layout:
CONFIG_SYSFS_DEPRECATED=n

Quoted 2 lines · Måns Rullgård

How is an initrd different from a real filesystem as seen by the microcode update driver?

For starters, initrd is available much earlier, even before paging is enabled on 32-bit, for example. See find_cpio_data().

Quoted 3 lines

The objection against removing the dependency was that updating microcode "late" isn't safe. I don't see how turning on BLK_DEV_INITRD stops anyone doing those allegedly unsafe updates anyway.

No one is stopping anyone from doing late updates. It is a valid use case, and we have to support it. And late updates are not necessarily unsafe, per se.

Lemme put it this way: it is a lot less unproblematic to do early updates. Mind you, there's no 100% guarantee that early updates would always work either. It all depends on what the microcode patch does. But they do work 99,9999999...% of the time. :)

IOW, I haven't heard of an early update breaking the machine. But it is possible.

So the *general* flow should be that people enable BLK_DEV_INITRD, put the microcode in there and it gets updated as early as possible. This is what the distros do and it is the most tested path. The other possibilities are there too, but only for cases where initrd is out of the question.

I just diffed my 4.3 config with the 4.4 config and saw that the whole Microcode stuff was silently dropped by a normal "make oldconfig".

Quoted 2 lines · Thomas Voegtle

I just diffed my 4.3 config with the 4.4 config and saw that the whole Microcode stuff was silently dropped by a normal "make oldconfig".

Ok, here's a different version.

It takes care of the oldconfig aspect, you'd still need to either prepare an initrd with the microcode or build it into the kernel. Ask if something's not clear.

It works fine in qemu+kvm but I'll run it on my boxes before I queue it for stable.

Thanks.

---
From: Borislav Petkov <[email protected]>
Date: Mon, 18 Jan 2016 19:07:08 +0100
Subject: [PATCH] x86/microcode: Untangle from BLK_DEV_INITRD

Thomas Voegtle reported that doing oldconfig with a .config which has CONFIG_MICROCODE enabled but BLK_DEV_INITRD disabled prevents the microcode loading mechanism from being built.

So untangle it from the BLK_DEV_INITRD dependency so that oldconfig doesn't turn it off and add an explanatory text to its Kconfig help what the supported methods for supplying microcode are.

Follow-on work will do the same with CONFIG_FW_LOADER. This patch is kept minimal for stable backporting.

Reported-by Thomas Voegtle <[email protected]>

Signed-off-by Borislav Petkov <[email protected]>

Cc <[email protected]> # 4.4

---

arch/x86/Kconfig                      | 12 ++++++------
arch/x86/include/asm/microcode.h      | 26 ++++++++++++++++++++++++++
arch/x86/kernel/cpu/microcode/intel.c | 14 ++++----------
3 files changed, 36 insertions(+), 16 deletions(-)

arch/x86/Kconfig +6 -6

index db3622f22b61..9743b8bb500e 100644
@@ -1126,10 +1126,8 @@ config MICROCODE
 	bool "CPU microcode loading support"
 	default y
 	depends on CPU_SUP_AMD || CPU_SUP_INTEL
-	depends on BLK_DEV_INITRD
 	select FW_LOADER
 	---help---
-
 	  If you say Y here, you will be able to update the microcode on
 	  certain Intel and AMD processors. The Intel support is for the
 	  IA32 family, e.g. Pentium Pro, Pentium II, Pentium III, Pentium 4,

@@ -1137,11 +1135,13 @@ config MICROCODE
 	  obviously need the actual microcode binary data itself which is not
 	  shipped with the Linux kernel.
 
-	  This option selects the general module only, you need to select
-	  at least one vendor specific module as well.
+	  The preferred method to load microcode is described in
+	  Documentation/x86/early-microcode.txt. For that you need to enable
+	  CONFIG_BLK_DEV_INITRD in order for the loader to be able to scan the
+	  initrd for microcode blobs.
 
-	  To compile this driver as a module, choose M here: the module
-	  will be called microcode.
+	  Alternatively, you can build-in the microcode into the kernel. For
+	  that you need the functionality behind CONFIG_FW_LOADER.
 

config MICROCODE_INTEL
bool "Intel microcode loading support"

arch/x86/include/asm/microcode.h +1 -0

index 34e62b1dcfce..712b24ed3a64 100644
@@ -2,6 +2,7 @@
 #define _ASM_X86_MICROCODE_H
 
 #include <linux/earlycpio.h>
+#include <linux/initrd.h>
 
 #define native_rdmsr(msr, val1, val2)			\
 do {							\

@@ -168,4 +169,29 @@ static inline void reload_early_microcode(void)			{ }

static inline bool
get_builtin_firmware(struct cpio_data *cd, const char *name) { return false; }
#endif

arch/x86/include/asm/microcode.h +25 -0

+
+static inline unsigned long get_initrd_start(void)
+{
+#ifdef CONFIG_BLK_DEV_INITRD
+	return initrd_start;
+#else
+	return 0;
+#endif
+}
+
+static inline unsigned long get_initrd_start_addr(void)
+{
+#ifdef CONFIG_BLK_DEV_INITRD
+#ifdef CONFIG_X86_32
+	unsigned long *initrd_start_p = (unsigned long *)__pa_nodebug(&initrd_start);
+
+	return (unsigned long)__pa_nodebug(*initrd_start_p);
+#else
+	return get_initrd_start();
+#endif
+#else /* CONFIG_BLK_DEV_INITRD */
+	return 0;
+#endif
+}
+

#endif /* _ASM_X86_MICROCODE_H */

arch/x86/kernel/cpu/microcode/intel.c +3 -9

index ce47402eb2f9..eea5978a0e33 100644
@@ -694,7 +694,7 @@ int __init save_microcode_in_initrd_intel(void)
 	if (count == 0)
 		return ret;
 
-	copy_initrd_ptrs(mc_saved, mc_saved_in_initrd, initrd_start, count);
+	copy_initrd_ptrs(mc_saved, mc_saved_in_initrd, get_initrd_start(), count);
 	ret = save_microcode(&mc_saved_data, mc_saved, count);
 	if (ret)
 		pr_err("Cannot save microcode patches from initrd.\n");

@@ -752,20 +752,14 @@ void load_ucode_intel_ap(void)
 	struct mc_saved_data *mc_saved_data_p;
 	struct ucode_cpu_info uci;
 	unsigned long *mc_saved_in_initrd_p;
-	unsigned long initrd_start_addr;
 	enum ucode_state ret;
 #ifdef CONFIG_X86_32
-	unsigned long *initrd_start_p;
 
-	mc_saved_in_initrd_p =
-		(unsigned long *)__pa_nodebug(mc_saved_in_initrd);
+	mc_saved_in_initrd_p = (unsigned long *)__pa_nodebug(mc_saved_in_initrd);
 	mc_saved_data_p = (struct mc_saved_data *)__pa_nodebug(&mc_saved_data);
-	initrd_start_p = (unsigned long *)__pa_nodebug(&initrd_start);
-	initrd_start_addr = (unsigned long)__pa_nodebug(*initrd_start_p);
 #else
-	mc_saved_data_p = &mc_saved_data;
 	mc_saved_in_initrd_p = mc_saved_in_initrd;
-	initrd_start_addr = initrd_start;
+	mc_saved_data_p = &mc_saved_data;
 #endif
 
 	/*

I checked that in several ways, took my old 4.3-config, ran it through "make oldconfig" and the others, etc.

Works like a charm.