x86/microcode update on systems without INITRD

x86/microcode update on systems without INITRD

[Markus Trippelsdorf]

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

Why?

I've used "echo -n 1 >| /sys/devices/system/cpu/microcode/reload" in my
init script for years without any issues.

-- 
Markus

Re: x86/microcode update on systems without INITRD

[Borislav Petkov]

On Thu, Nov 19, 2015 at 10:43:01PM +0100, Markus Trippelsdorf wrote:
> 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.

Thanks for letting me know.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

Re: x86/microcode update on systems without INITRD

[Borislav Petkov]

On Thu, Nov 19, 2015 at 10:55:43PM +0100, Borislav Petkov wrote:
> On Thu, Nov 19, 2015 at 10:43:01PM +0100, Markus Trippelsdorf wrote:
> > 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.

Btw, you should consider changing your setup to use the initrd for
loading microcode early because microcode should be loaded as early as
possible.

The ".../reload" method is aimed at updating microcode on long-running
boxes without rebooting them. And that means those updates need to be
tested first in a lab environment before running them on the production
systems because not every microcode patch can be applied late. Some
shouldn't be even tried.

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:

---
diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c
index 3aaffb601c91..faec7120c508 100644
--- a/arch/x86/kernel/cpu/microcode/core.c
+++ b/arch/x86/kernel/cpu/microcode/core.c
@@ -698,3 +698,4 @@ int __init microcode_init(void)
 	return error;
 
 }
+late_initcall(microcode_init);
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 29db25f9a745..d2bbe343fda7 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -1250,8 +1250,6 @@ void __init setup_arch(char **cmdline_p)
 	if (efi_enabled(EFI_BOOT))
 		efi_apply_memmap_quirks();
 #endif
-
-	microcode_init();
 }
 
 #ifdef CONFIG_X86_32
---

Thanks!

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

Re: x86/microcode update on systems without INITRD

[Markus Trippelsdorf]

On 2015.11.19 at 23:58 +0100, Borislav Petkov wrote:
> On Thu, Nov 19, 2015 at 10:55:43PM +0100, Borislav Petkov wrote:
> > On Thu, Nov 19, 2015 at 10:43:01PM +0100, Markus Trippelsdorf wrote:
> > > 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:

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

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index db3622f22b61..52c6964e24bd 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -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---
 

-- 
Markus

Re: x86/microcode update on systems without INITRD

[Borislav Petkov]

On Fri, Nov 20, 2015 at 08:19:20AM +0100, Markus Trippelsdorf wrote:
> On 2015.11.19 at 23:58 +0100, Borislav Petkov wrote:
> > On Thu, Nov 19, 2015 at 10:55:43PM +0100, Borislav Petkov wrote:
> > > On Thu, Nov 19, 2015 at 10:43:01PM +0100, Markus Trippelsdorf wrote:
> > > > 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:
> 
> Your patch works fine. Thanks.

Thanks for testing.

> But of course it needs this additional patch, otherwise the microcode
> loader wouldn't build at all:
> 
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index db3622f22b61..52c6964e24bd 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -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.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

Re: x86/microcode update on systems without INITRD

[Markus Trippelsdorf]

On 2015.11.20 at 09:27 +0100, Borislav Petkov wrote:
> On Fri, Nov 20, 2015 at 08:19:20AM +0100, Markus Trippelsdorf wrote:
> > On 2015.11.19 at 23:58 +0100, Borislav Petkov wrote:
> > > On Thu, Nov 19, 2015 at 10:55:43PM +0100, Borislav Petkov wrote:
> > > > On Thu, Nov 19, 2015 at 10:43:01PM +0100, Markus Trippelsdorf wrote:
> > > > > 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:
> > 
> > Your patch works fine. Thanks.
> 
> Thanks for testing.
> 
> > But of course it needs this additional patch, otherwise the microcode
> > loader wouldn't build at all:
> > 
> > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > index db3622f22b61..52c6964e24bd 100644
> > --- a/arch/x86/Kconfig
> > +++ b/arch/x86/Kconfig
> > @@ -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...

-- 
Markus

Re: x86/microcode update on systems without INITRD

[Borislav Petkov]

On Fri, Nov 20, 2015 at 09:33:48AM +0100, Markus Trippelsdorf wrote:
> Well, BLK_DEV_INITRD is not set in my config and it works just fine...

I'm assuming with "works just fine" you mean, you can load microcode.

Again, as I explained previously, you should consider moving to the
early method with an initrd. The late method is a fallback for certain
setups and the early method should be the preferred one.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

[PATCH] x86/microcode: Initialize the driver late when facilities are up

[Borislav Petkov]

From: Borislav Petkov <bp@suse.de>
Date: Fri, 20 Nov 2015 00:05:24 +0100
Subject: [PATCH] x86/microcode: Initialize the driver late when facilities are up

Running microcode_init() from setup_arch() is a bad idea because not
even kmalloc() is ready at that point and the loader does all kinds of
allocations and init/registration with various subsystems.

Make it a late initcall when required facilities are initialized so that
the microcode driver initialization can succeed too.

Reported-and-tested-by: Markus Trippelsdorf <markus@trippelsdorf.de>
Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/kernel/cpu/microcode/core.c | 1 +
 arch/x86/kernel/setup.c              | 2 --
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c
index 7fc27f1cca58..b3e94ef461fd 100644
--- a/arch/x86/kernel/cpu/microcode/core.c
+++ b/arch/x86/kernel/cpu/microcode/core.c
@@ -698,3 +698,4 @@ int __init microcode_init(void)
 	return error;
 
 }
+late_initcall(microcode_init);
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 29db25f9a745..d2bbe343fda7 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -1250,8 +1250,6 @@ void __init setup_arch(char **cmdline_p)
 	if (efi_enabled(EFI_BOOT))
 		efi_apply_memmap_quirks();
 #endif
-
-	microcode_init();
 }
 
 #ifdef CONFIG_X86_32
-- 
2.3.5

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

[tip:x86/urgent] x86/microcode: Initialize the driver late when facilities are up

[tip-bot for Borislav Petkov]

Commit-ID:  2d5be37d686c4dae8e60d20283d6f44ac2c44f65
Gitweb:     http://git.kernel.org/tip/2d5be37d686c4dae8e60d20283d6f44ac2c44f65
Author:     Borislav Petkov <bp@suse.de>
AuthorDate: Fri, 20 Nov 2015 12:24:00 +0100
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 23 Nov 2015 10:39:49 +0100

x86/microcode: Initialize the driver late when facilities are up

Running microcode_init() from setup_arch() is a bad idea because
not even kmalloc() is ready at that point and the loader does
all kinds of allocations and init/registration with various
subsystems.

Make it a late initcall when required facilities are initialized
so that the microcode driver initialization can succeed too.

Reported-and-tested-by: Markus Trippelsdorf <markus@trippelsdorf.de>
Signed-off-by: Borislav Petkov <bp@suse.de>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/20151120112400.GC4028@pd.tnic
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/cpu/microcode/core.c | 1 +
 arch/x86/kernel/setup.c              | 2 --
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c
index 7fc27f1..b3e94ef 100644
--- a/arch/x86/kernel/cpu/microcode/core.c
+++ b/arch/x86/kernel/cpu/microcode/core.c
@@ -698,3 +698,4 @@ int __init microcode_init(void)
 	return error;
 
 }
+late_initcall(microcode_init);
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 29db25f..d2bbe34 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -1250,8 +1250,6 @@ void __init setup_arch(char **cmdline_p)
 	if (efi_enabled(EFI_BOOT))
 		efi_apply_memmap_quirks();
 #endif
-
-	microcode_init();
 }
 
 #ifdef CONFIG_X86_32

Re: x86/microcode update on systems without INITRD

[Thomas Voegtle]

On Fri, 20 Nov 2015, Borislav Petkov wrote:

> On Fri, Nov 20, 2015 at 08:19:20AM +0100, Markus Trippelsdorf wrote:
>> On 2015.11.19 at 23:58 +0100, Borislav Petkov wrote:
>>> On Thu, Nov 19, 2015 at 10:55:43PM +0100, Borislav Petkov wrote:
>>>> On Thu, Nov 19, 2015 at 10:43:01PM +0100, Markus Trippelsdorf wrote:
>>>>> 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:
>>
>> Your patch works fine. Thanks.
>
> Thanks for testing.
>
>> But of course it needs this additional patch, otherwise the microcode
>> loader wouldn't build at all:
>>
>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>> index db3622f22b61..52c6964e24bd 100644
>> --- a/arch/x86/Kconfig
>> +++ b/arch/x86/Kconfig
>> @@ -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".

Wouldn't be something like this much nicer to give a hint?

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index db3622f..75523a0 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1126,7 +1126,7 @@ config MICROCODE
         bool "CPU microcode loading support"
         default y
         depends on CPU_SUP_AMD || CPU_SUP_INTEL
-       depends on BLK_DEV_INITRD
+       select BLK_DEV_INITRD
         select FW_LOADER
         ---help---


At least I would have noticed it?


    Thomas

Re: x86/microcode update on systems without INITRD

[Borislav Petkov]

On Thu, Jan 07, 2016 at 01:12:16PM +0100, Thomas Voegtle wrote:
> 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?

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

Re: x86/microcode update on systems without INITRD

[Thomas Voegtle]

[-- Attachment #1: Type: text/plain, Size: 483 bytes --]

On Thu, 7 Jan 2016, Borislav Petkov wrote:

> On Thu, Jan 07, 2016 at 01:12:16PM +0100, Thomas Voegtle wrote:
>> 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?

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?


   Thomas


[-- Attachment #2: Type: application/x-config, Size: 76669 bytes --]

Re: x86/microcode update on systems without INITRD

[Markus Trippelsdorf]

On 2016.01.07 at 13:36 +0100, Thomas Voegtle wrote:
> On Thu, 7 Jan 2016, Borislav Petkov wrote:
> 
> >On Thu, Jan 07, 2016 at 01:12:16PM +0100, Thomas Voegtle wrote:
> >>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?
> 
> 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 <tigran@aivazian.fsnet.co.uk>, Peter Oruba
[    2.574876] registered taskstats version 1
[    2.575132] Btrfs loaded
...

-- 
Markus

Re: x86/microcode update on systems without INITRD

[Thomas Voegtle]

On Thu, 7 Jan 2016, Markus Trippelsdorf wrote:

> On 2016.01.07 at 13:36 +0100, Thomas Voegtle wrote:
>> On Thu, 7 Jan 2016, Borislav Petkov wrote:
>>
>>> On Thu, Jan 07, 2016 at 01:12:16PM +0100, Thomas Voegtle wrote:
>>>> 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?
>>
>> 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.


That's not an option for me.

For me this is a serious regression, when a normal make oldconfig silently 
drops a feature which I had switched on before.



   Thomas

Re: x86/microcode update on systems without INITRD

[Mike Keehan]

On Thu, 7 Jan 2016 13:41:11 +0100
Markus Trippelsdorf <markus@trippelsdorf.de> wrote:

> On 2016.01.07 at 13:36 +0100, Thomas Voegtle wrote:
> > On Thu, 7 Jan 2016, Borislav Petkov wrote:
> > 
> > >On Thu, Jan 07, 2016 at 01:12:16PM +0100, Thomas Voegtle wrote:
> > >>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?
> > 
> > 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
> <tigran@aivazian.fsnet.co.uk>, 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.

Mike.

Re: x86/microcode update on systems without INITRD

[Markus Trippelsdorf]

On 2016.01.08 at 12:18 +0000, Mike Keehan wrote:
> On Thu, 7 Jan 2016 13:41:11 +0100
> Markus Trippelsdorf <markus@trippelsdorf.de> wrote:
> 
> > On 2016.01.07 at 13:36 +0100, Thomas Voegtle wrote:
> > > On Thu, 7 Jan 2016, Borislav Petkov wrote:
> > > 
> > > >On Thu, Jan 07, 2016 at 01:12:16PM +0100, Thomas Voegtle wrote:
> > > >>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?
> > > 
> > > 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
> > <tigran@aivazian.fsnet.co.uk>, 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.

I mean:

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index db3622f22b61..52c6964e24bd 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -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---

-- 
Markus

Re: x86/microcode update on systems without INITRD

[Borislav Petkov]

On Thu, Jan 07, 2016 at 01:36:00PM +0100, Thomas Voegtle wrote:
> 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.

Thanks.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

Re: x86/microcode update on systems without INITRD

[Måns Rullgård]

Borislav Petkov <bp@suse.de> writes:

> On Thu, Jan 07, 2016 at 01:36:00PM +0100, Thomas Voegtle wrote:
>> 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).

-- 
Måns Rullgård

Re: x86/microcode update on systems without INITRD

[Borislav Petkov]

On Fri, Jan 08, 2016 at 11:18:51AM +0000, Måns Rullgård wrote:
> 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.

> 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.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

Re: x86/microcode update on systems without INITRD

[Måns Rullgård]

Borislav Petkov <bp@suse.de> writes:

> On Fri, Jan 08, 2016 at 11:18:51AM +0000, Måns Rullgård wrote:
>> 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.

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

>> 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.

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.

-- 
Måns Rullgård

Re: x86/microcode update on systems without INITRD

[Borislav Petkov]

On Fri, Jan 08, 2016 at 11:46:28AM +0000, Måns Rullgård wrote:
> 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().

> 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 hope that makes it more clear.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

Re: x86/microcode update on systems without INITRD

[Markus Trippelsdorf]

On 2016.01.08 at 13:08 +0100, Borislav Petkov wrote:
> On Fri, Jan 08, 2016 at 11:46:28AM +0000, Måns Rullgård wrote:
> > 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().
> 
> > 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.

-- 
Markus

Re: x86/microcode update on systems without INITRD

[Borislav Petkov]

On Fri, Jan 08, 2016 at 01:16:01PM +0100, Markus Trippelsdorf wrote:
> 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. :-)

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

Re: x86/microcode update on systems without INITRD

[Måns Rullgård]

Borislav Petkov <bp@suse.de> writes:

> On Fri, Jan 08, 2016 at 01:16:01PM +0100, Markus Trippelsdorf wrote:
>> 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.

That seems perfectly reasonable to me.

-- 
Måns Rullgård

Re: x86/microcode update on systems without INITRD

[Michal Marek]

On 2016-01-08 13:27, Borislav Petkov wrote:
> On Fri, Jan 08, 2016 at 01:16:01PM +0100, Markus Trippelsdorf wrote:
>> 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.

Michal

Re: x86/microcode update on systems without INITRD

[Borislav Petkov]

On Fri, Jan 08, 2016 at 01:48:40PM +0100, Michal Marek wrote:
> 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...

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

Re: x86/microcode update on systems without INITRD

[Michal Marek]

On 2016-01-08 14:37, Borislav Petkov wrote:
> On Fri, Jan 08, 2016 at 01:48:40PM +0100, Michal Marek wrote:
>> 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.

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.


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

Oh, get well soon.

Michal

Re: x86/microcode update on systems without INITRD

[Borislav Petkov]

On Fri, Jan 08, 2016 at 03:48:37PM +0100, Michal Marek wrote:
> 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


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...

Thoughts?

---
 arch/x86/scripts/check-configs.sh | 41 +++++++++++++++++++++++++++++++++++++++
 scripts/kconfig/Makefile          |  3 +++
 2 files changed, 44 insertions(+)
 create mode 100644 arch/x86/scripts/check-configs.sh

diff --git a/arch/x86/scripts/check-configs.sh b/arch/x86/scripts/check-configs.sh
new file mode 100644
index 000000000000..51ae9bde6965
--- /dev/null
+++ b/arch/x86/scripts/check-configs.sh
@@ -0,0 +1,41 @@
+#!/bin/bash
+
+if [ "$1" != "oldconfig" ]; then
+	exit 0
+fi
+
+srctree=$2
+ARCH="$3"
+UNAME_RELEASE=$(uname -r)
+
+KCONFIGS=".config /lib/modules/$UNAME_RELEASE/.config /etc/kernel-config /boot/config-$UNAME_RELEASE"
+
+if [ "$ARCH" = "X86_32" ]; then
+	KCONFIGS="$KCONFIGS $srctree/arch/x86/configs/i386_defconfig"
+else
+	KCONFIGS="$KCONFIGS $srctree/arch/x86/configs/x86_64_defconfig"
+fi
+
+for k in $KCONFIGS;
+do
+	if [ -e $k ]; then
+		OLD_CONFIG=$k
+		break
+	fi
+done
+
+if [ -z "$OLD_CONFIG" ]; then exit 0; fi
+
+# Check optimal microcode loader .config settings
+if ! grep -q MICROCODE $OLD_CONFIG; then
+	exit 0
+fi
+
+MSG="You have CONFIG_MICROCODE enabled without BLK_DEV_INITRD. Enable\n\
+it and make sure microcode is added to your initrd as explained in\n\
+Documentation/x86/early-microcode.txt\n"
+
+if ! grep -v "^#" $OLD_CONFIG | grep -q BLK_DEV_INITRD; then
+	echo -e $MSG
+	exit 1
+fi
diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
index d79cba4ce3eb..136ae9744efc 100644
--- a/scripts/kconfig/Makefile
+++ b/scripts/kconfig/Makefile
@@ -81,6 +81,9 @@ simple-targets := oldconfig allnoconfig allyesconfig allmodconfig \
 PHONY += $(simple-targets)
 
 $(simple-targets): $(obj)/conf
+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
-- 
2.3.5

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

Re: x86/microcode update on systems without INITRD

[Måns Rullgård]

Borislav Petkov <bp@suse.de> writes:

> On Fri, Jan 08, 2016 at 03:48:37PM +0100, Michal Marek wrote:
>> 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.

-- 
Måns Rullgård

Re: x86/microcode update on systems without INITRD

[Borislav Petkov]

On Mon, Jan 11, 2016 at 08:29:01PM +0000, Måns Rullgård wrote:
> But this is wrong.  Microcode update doesn't need initrd.

You need to read my mail to the end:

> 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...

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

Re: x86/microcode update on systems without INITRD

[Måns Rullgård]

Borislav Petkov <bp@suse.de> writes:

> On Mon, Jan 11, 2016 at 08:29:01PM +0000, Måns Rullgård wrote:
>> But this is wrong.  Microcode update doesn't need initrd.
>
> You need to read my mail to the end:
>
>> 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?

-- 
Måns Rullgård

Re: x86/microcode update on systems without INITRD

[Borislav Petkov]

On Mon, Jan 11, 2016 at 09:04:45PM +0000, Måns Rullgård wrote:
> What do modules have to do with anything?

I'm trying to address Markus' case where he spoke of having modules off
and building everything into the kernel. But CONFIG_MODULES is wrong
here, I'm not going to grep for it. As I said, I'll have to think about
it more.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

[RFC PATCH] x86/kconfig: Sanity-check config file during oldconfig

[Borislav Petkov]

From: Borislav Petkov <bp@suse.de>

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 <tv@lio96.de>
Cc: Markus Trippelsdorf <markus@trippelsdorf.de>
Cc: Måns Rullgård <mans@mansr.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
---
 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

diff --git a/arch/x86/scripts/check-configs.sh b/arch/x86/scripts/check-configs.sh
new file mode 100644
index 000000000000..775d07e37df5
--- /dev/null
+++ b/arch/x86/scripts/check-configs.sh
@@ -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
diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
index d79cba4ce3eb..136ae9744efc 100644
--- a/scripts/kconfig/Makefile
+++ b/scripts/kconfig/Makefile
@@ -81,6 +81,9 @@ simple-targets := oldconfig allnoconfig allyesconfig allmodconfig \
 PHONY += $(simple-targets)
 
 $(simple-targets): $(obj)/conf
+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
-- 
2.3.5

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

Re: [RFC PATCH] x86/kconfig: Sanity-check config file during oldconfig

[Thomas Voegtle]

[-- Attachment #1: Type: text/plain, Size: 3426 bytes --]

On Thu, 14 Jan 2016, Borislav Petkov wrote:

> From: Borislav Petkov <bp@suse.de>
>
> 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 <tv@lio96.de>
> Cc: Markus Trippelsdorf <markus@trippelsdorf.de>
> Cc: Måns Rullgård <mans@mansr.com>
> Signed-off-by: Borislav Petkov <bp@suse.de>
> ---
> 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
>
> diff --git a/arch/x86/scripts/check-configs.sh b/arch/x86/scripts/check-configs.sh
> new file mode 100644
> index 000000000000..775d07e37df5
> --- /dev/null
> +++ b/arch/x86/scripts/check-configs.sh
> @@ -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
> diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
> index d79cba4ce3eb..136ae9744efc 100644
> --- a/scripts/kconfig/Makefile
> +++ b/scripts/kconfig/Makefile
> @@ -81,6 +81,9 @@ simple-targets := oldconfig allnoconfig allyesconfig allmodconfig \
> PHONY += $(simple-targets)
>
> $(simple-targets): $(obj)/conf
> +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


   Thomas

Re: [RFC PATCH] x86/kconfig: Sanity-check config file during oldconfig

[Borislav Petkov]

On Mon, Jan 18, 2016 at 02:36:09PM +0100, Thomas Voegtle wrote:
> 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/

> What's with olddefconfig

Yes, I'll update the patch.

> 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.

Regarding this issue, yes, I'll add its name to the script too.

> btw that patch has to go to stable 4.4, too

Sure, once we agree on the approach.

Thanks.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

Re: [RFC PATCH] x86/kconfig: Sanity-check config file during oldconfig

[Måns Rullgård]

Borislav Petkov <bp@suse.de> writes:

> On Mon, Jan 18, 2016 at 02:36:09PM +0100, Thomas Voegtle wrote:
>> 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. 

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

> 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/
>
>> What's with olddefconfig
>
> Yes, I'll update the patch.
>
>> 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.  olddefconfig asks no questions and sets new
options to their defaults.

-- 
Måns Rullgård

Re: [RFC PATCH] x86/kconfig: Sanity-check config file during oldconfig

[Borislav Petkov]

On Mon, Jan 18, 2016 at 02:11:49PM +0000, Måns Rullgård wrote:
> It asks about new options but doesn't print (as many) unchanged ones.
> That makes it more silent.

So it should be called

make moresilentoldconfigbutnotcompletelyconfig

or so.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

Re: [RFC PATCH] x86/kconfig: Sanity-check config file during oldconfig

[Borislav Petkov]

On Mon, Jan 18, 2016 at 02:11:49PM +0000, Måns Rullgård wrote:
> Wasn't the idea *not* to disable CONFIG_MICROCODE?

Is the error message not understandable?

+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"

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

Re: [RFC PATCH] x86/kconfig: Sanity-check config file during oldconfig

[Måns Rullgård]

Borislav Petkov <bp@suse.de> writes:

> On Mon, Jan 18, 2016 at 02:11:49PM +0000, Måns Rullgård wrote:
>> Wasn't the idea *not* to disable CONFIG_MICROCODE?
>
> Is the error message not understandable?
>
> +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"

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?

-- 
Måns Rullgård

Re: [RFC PATCH] x86/kconfig: Sanity-check config file during oldconfig

[Borislav Petkov]

On Mon, Jan 18, 2016 at 02:51:00PM +0000, Måns Rullgård wrote:
> 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.

But I guess you're reading it the way you wanna read it so I'm going to
leave you thinking whatever you want to think.

> 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.

So I'm going to stop wasting time with you.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

Re: [RFC PATCH] x86/kconfig: Sanity-check config file during oldconfig

[Måns Rullgård]

Borislav Petkov <bp@suse.de> writes:

> On Mon, Jan 18, 2016 at 02:51:00PM +0000, Måns Rullgård wrote:
>> 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.

Forcing users to press a damn key on the keyboard after your pointless
warning message sure feels like bludgeoning to me.

>> 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.

Clearly you didn't explain it very well.

-- 
Måns Rullgård

[RFC] CONFIG_FORCE_MINIMALLY_SANE_CONFIG=y (was: Re: [RFC PATCH] x86/kconfig: Sanity-check config file during oldconfig)

[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. )

* Borislav Petkov <bp@suse.de> wrote:

> From: Borislav Petkov <bp@suse.de>
> 
> 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.

>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:

+config FORCE_MINIMALLY_SANE_CONFIG
+	bool
+	default y
+
+	# so that capset() works (sudo, etc.):
+	select SECURITY
+	select SECURITY_CAPABILITIES
+	select BINFMT_ELF
+
+	select SYSFS
+	select SYSFS_DEPRECATED
+	select PROC_FS
+	select FUTEX
+
+	# 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
+
+	# won't boot otherwise:
+	select RD_GZIP
+	select BLK_DEV_INITRD
+
+	# old F6 userspace needs vsyscalls:
+	select X86_VSYSCALL_EMULATION if X86_64
+	select IA32_EMULATION if X86_64

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!

Note that some of these have sub-dependencies (and super-dependencies) so the list 
isn't complete from a Kconfig language POV - but it lists most of the 'must have' 
leaf features and would form a good starting point.

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).

I'd keep this option x86-only at least initially, because that's still the space 
where most of our newbie testers come from, and because I'd like to see how this 
evolves before trying to generalize it to 44 architectures...

Also, I'd not try to be per distro, I'd use a single superset of such config 
options: 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 ...

Thoughs?

Thanks,

	Ingo

Re: [RFC] CONFIG_FORCE_MINIMALLY_SANE_CONFIG=y (was: Re: [RFC PATCH] x86/kconfig: Sanity-check config file during oldconfig)

[Markus Trippelsdorf]

On 2016.01.19 at 09:20 +0100, Ingo Molnar wrote:
> 
> ( 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. )
> 
> * Borislav Petkov <bp@suse.de> wrote:
> 
> > From: Borislav Petkov <bp@suse.de>
> > 
> > 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.

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.

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. 

-- 
Markus

Re: [RFC] CONFIG_FORCE_MINIMALLY_SANE_CONFIG=y (was: Re: [RFC PATCH] x86/kconfig: Sanity-check config file during oldconfig)

[Ingo Molnar]

* Markus Trippelsdorf <markus@trippelsdorf.de> wrote:

> On 2016.01.19 at 09:20 +0100, Ingo Molnar wrote:
> > 
> > ( 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. )
> > 
> > * Borislav Petkov <bp@suse.de> wrote:
> > 
> > > From: Borislav Petkov <bp@suse.de>
> > > 
> > > 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.
> 
> 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.

Well, I think the argument I raised later on is important:

> >  [...] 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 ...

I.e. the costs of quirks are _massively_ assymetric: having an extra system call 
or compat option quirk enabled is essentially unmeasurable for those who don't 
technically need them, while it can be a big and hard to debug show-stopper for 
others.

'default y' was supposed to cover such cases, but arguably it's too opaque, I 
think we need a separate, more obvious layer - such as the 
CONFIG_FORCE_MINIMALLY_SANE_CONFIG=y option I suggested.

> 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.

It sure can be easily disabled, that's a given.

The key point is that I'd like "naively configured" kernels to work on just about 
any Linux distro that allow kernel testing - so the superset of all quirks should 
be included - as long as enabling a quirk does not break things (and none of the 
ones I listed do as far as I've tested).

Thanks,

	Ingo

Re: [RFC] CONFIG_FORCE_MINIMALLY_SANE_CONFIG=y (was: Re: [RFC PATCH] x86/kconfig: Sanity-check config file during oldconfig)

[Borislav Petkov]

On Tue, Jan 19, 2016 at 09:54:12AM +0100, Ingo Molnar wrote:
> The key point is that I'd like "naively configured" kernels to work on
> just about any Linux distro that allow kernel testing

Vehemently yes!

This is magnitudes more important than having a couple of passive
kilobytes dangling in the kernel image.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

Re: [RFC] CONFIG_FORCE_MINIMALLY_SANE_CONFIG=y (was: Re: [RFC PATCH] x86/kconfig: Sanity-check config file during oldconfig)

[Peter Zijlstra]

On Tue, Jan 19, 2016 at 09:20:22AM +0100, Ingo Molnar wrote:
> +	# 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!

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

Re: [RFC] CONFIG_FORCE_MINIMALLY_SANE_CONFIG=y (was: Re: [RFC PATCH] x86/kconfig: Sanity-check config file during oldconfig)

[Ingo Molnar]

* Peter Zijlstra <peterz@infradead.org> wrote:

> On Tue, Jan 19, 2016 at 09:20:22AM +0100, Ingo Molnar wrote:
> > +	# 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!
> 
> So why not group those under CONFIG_SYSTEMD_BLOWS? I still dont have a
> machine with that turd on.

I'd definitely (try to) list the reasons for each quirk in the Kconfig lines (as I 
did above), but I'd still keep a single generic option not tied to systemd in 
particular, for the following reasons:

 - I am using many systemd systems, so the quirks are naturally mostly systemd
   related. There might be more non-systemd quirks that I never triggered
   personally. They can be added once people trigger them.

 - Also, even that considered, not all of the options I listed are systemd quirks,
   as I still have a single (albeit simple) non-systemd test machine.

 - I'd like to have a single superset option that principally makes the kernel
   'just work' for newbie testers - without them having to be even aware of
   whether their distro version uses systemd or something else.

Thanks,

	Ingo

Re: [RFC] CONFIG_FORCE_MINIMALLY_SANE_CONFIG=y (was: Re: [RFC PATCH] x86/kconfig: Sanity-check config file during oldconfig)

[Borislav Petkov]

On Tue, Jan 19, 2016 at 09:20:22AM +0100, Ingo Molnar wrote:
> 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...

Tell me about it. SCSI SAS recent breakage case-in-point...

> 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.

Yeah, so I have a different idea how to fix it. I'm going to drop both

	depends on BLK_DEV_INITRD
        select FW_LOADER

and make it build with or without them enabled so that people are free
to do whatever they want and not get the feeling that I'm forcing shit
down their throats.

HOWEVER(!), this, IMHO, won't help with the normal users because then
they'd have to read Kconfig:

+         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.

+         Alternatively, you can build-in the microcode into the kernel. For
+         that you need the functionality behind CONFIG_FW_LOADER.

and figure out what to do exactly to have microcode applied.

And this is crap, IMO.

It should JustWork.

I dunno, maybe I should do a separate config option which let people
choose between FW_LOADER and BLK_DEV_INITRD if CONFIG_MICROCODE is
enabled. I need to hack it in and see what it becomes.

Anyway, I'm just giving my example here as a POV for the discussion.

> 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'm working on untangling CONFIG_MICROCODE from BLK_DEV_INITRD so you
won't need to touch the Kconfig. See above.

> 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.

Yeah, people complained that they don't want to run with initrds.

> 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.

CONFIG_EXPERT_MORE

?

> 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:
> 
> +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!

I know *exactly* what you're talking about!

Yeah, so having an option select *sane* settings but leaving the
possibility to change that for expert users makes sense.

...

> 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).

Yap.

...

> Thoughs?

Sounds like a good idea to me.

Thanks.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

Re: [RFC] CONFIG_FORCE_MINIMALLY_SANE_CONFIG=y

[Michal Marek]

On 2016-01-19 09:20, Ingo Molnar wrote:
> 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.


> 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:
> 
> +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.

Michal

Re: [RFC] CONFIG_GENERIC_BOOTABLE_CONFIG=y

[Ingo Molnar]

* Michal Marek <mmarek@suse.cz> wrote:

> On 2016-01-19 09:20, Ingo Molnar wrote:
> > 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.
> 
> 
> > 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:
> > 
> > +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.

Yeah, it sure should be interactive. This was pasted from my automated testing 
that isn't interested in unbootable kernels.

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'.)

Thanks,

	Ingo

Re: [RFC] CONFIG_GENERIC_BOOTABLE_CONFIG=y

[Linus Torvalds]

On Tue, Jan 19, 2016 at 2:30 AM, Ingo Molnar <mingo@kernel.org> wrote:
>
>
> 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).

Done right, you should be able to

 (a) select your CPU (and things like "do you want virtualization etc")
 (b) select your distro
 (c) select your drivers

and pretty much be done with it.

            Linus

Re: [RFC] CONFIG_GENERIC_BOOTABLE_CONFIG=y

[Austin S. Hemmelgarn]

On 2016-01-19 12:22, Linus Torvalds wrote:
> On Tue, Jan 19, 2016 at 2:30 AM, Ingo Molnar <mingo@kernel.org> wrote:
>>
>>
>> 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).
It might be worth looking at Gentoo's kernel patches, they've had one 
for a while now (I think roughly a year and a half) that does pretty 
much this, you get options for each of their officially supported init 
systems (OpenRC, and SystemD, although the systemd option is not 
anywhere near as complete as what was proposed here), one for portage 
(which needs some specific namespace stuff for sandboxing builds), and I 
think one for udev.  Theirs however uses depends instead of selects, but 
I think that fits a bit better (If I tell the config to make sure I can 
boot using a particular init system, I expect it to not let me change 
things that would prevent me from booting using that init system).

Re: [RFC] CONFIG_FORCE_MINIMALLY_SANE_CONFIG=y

[Måns Rullgård]

Ingo Molnar <mingo@kernel.org> writes:

> 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?

The problem with *any* approach involving Kconfig is that it still
leaves users free to load microcode whenever they want, even if they're
not supposed to.  Doing so apparently works correctly (almost) every
time (how does it fail if it does?), and no warnings are issued to
suggest it might be a bad idea.  Force-enabling BLK_DEV_INITRD isn't
going to make anyone change their boot scripts.  Many might not even
notice that it got silently enabled. So what purpose does it serve to
play games with the config when it has no actual impact on the usage of
the driver?

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.

-- 
Måns Rullgård

Re: [RFC] CONFIG_FORCE_MINIMALLY_SANE_CONFIG=y

[Michal Marek]

Dne 19.1.2016 v 13:29 Måns Rullgård napsal(a):
> 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.


> 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.

Michal

Re: [RFC] CONFIG_FORCE_MINIMALLY_SANE_CONFIG=y

[Måns Rullgård]

Michal Marek <mmarek@suse.cz> writes:

> Dne 19.1.2016 v 13:29 Måns Rullgård napsal(a):
>> 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.

I'd be rather cross if something suddenly started building initrds on my
systems.  To me they're just a useless level of complexity to maintain.
(I'm not denying they can be useful to others.)

>> 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.

OK, I can see how that might be a problem.

-- 
Måns Rullgård

Re: [RFC] CONFIG_FORCE_MINIMALLY_SANE_CONFIG=y (was: Re: [RFC PATCH] x86/kconfig: Sanity-check config file during oldconfig)

[Kirill A. Shutemov]

On Tue, Jan 19, 2016 at 09:20:22AM +0100, Ingo Molnar wrote:
> +	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.

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
-- 
 Kirill A. Shutemov

Re: [RFC] CONFIG_FORCE_MINIMALLY_SANE_CONFIG=y (was: Re: [RFC PATCH] x86/kconfig: Sanity-check config file during oldconfig)

[Ingo Molnar]

* Kirill A. Shutemov <kirill@shutemov.name> wrote:

> On Tue, Jan 19, 2016 at 09:20:22AM +0100, Ingo Molnar wrote:
> > +	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.
> 
> 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

That's with new systemd. The Fedora 19 system I tested it on certainly failed in 
the way I documented it.

So we now have a kernel option that breaks older SystemD if unset, and breaks 
newer SystemD if set.

Bravo!

Thanks,

	Ingo

Re: x86/microcode update on systems without INITRD

[Måns Rullgård]

Borislav Petkov <bp@suse.de> writes:

> On Fri, Jan 08, 2016 at 11:46:28AM +0000, Måns Rullgård wrote:
>> 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().

Yes, but the microcode driver doesn't care about this AFAICT.

>> 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.

So it's meant to be supported, good.

> 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.

Yes, that's the common case, and those users will have BLK_DEV_INITRD
enabled anyway.  Now why should someone who, for whatever reasons, is
doing microcode updates late be forced to enable BLK_DEV_INITRD even
though he doesn't use it?

-- 
Måns Rullgård

Re: x86/microcode update on systems without INITRD

[Borislav Petkov]

On Thu, Jan 07, 2016 at 01:12:16PM +0100, Thomas Voegtle wrote:
> 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 <bp@suse.de>
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 <tv@lio96.de>
Signed-off-by: Borislav Petkov <bp@suse.de>
Cc: <stable@vger.kernel.org> # 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(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index db3622f22b61..9743b8bb500e 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -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"
diff --git a/arch/x86/include/asm/microcode.h b/arch/x86/include/asm/microcode.h
index 34e62b1dcfce..712b24ed3a64 100644
--- a/arch/x86/include/asm/microcode.h
+++ b/arch/x86/include/asm/microcode.h
@@ -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
+
+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 */
diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
index ce47402eb2f9..eea5978a0e33 100644
--- a/arch/x86/kernel/cpu/microcode/intel.c
+++ b/arch/x86/kernel/cpu/microcode/intel.c
@@ -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
 
 	/*
@@ -777,7 +771,7 @@ void load_ucode_intel_ap(void)
 
 	collect_cpu_info_early(&uci);
 	ret = load_microcode(mc_saved_data_p, mc_saved_in_initrd_p,
-			     initrd_start_addr, &uci);
+			     get_initrd_start_addr(), &uci);
 
 	if (ret != UCODE_OK)
 		return;
-- 
2.3.5

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

Re: x86/microcode update on systems without INITRD

[Thomas Voegtle]

On Tue, 19 Jan 2016, Borislav Petkov wrote:

> On Thu, Jan 07, 2016 at 01:12:16PM +0100, Thomas Voegtle wrote:
>> 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.


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.

Thank you,

    Thomas



>
> 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 <bp@suse.de>
> 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 <tv@lio96.de>
> Signed-off-by: Borislav Petkov <bp@suse.de>
> Cc: <stable@vger.kernel.org> # 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(-)
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index db3622f22b61..9743b8bb500e 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -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"
> diff --git a/arch/x86/include/asm/microcode.h b/arch/x86/include/asm/microcode.h
> index 34e62b1dcfce..712b24ed3a64 100644
> --- a/arch/x86/include/asm/microcode.h
> +++ b/arch/x86/include/asm/microcode.h
> @@ -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
> +
> +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 */
> diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
> index ce47402eb2f9..eea5978a0e33 100644
> --- a/arch/x86/kernel/cpu/microcode/intel.c
> +++ b/arch/x86/kernel/cpu/microcode/intel.c
> @@ -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
>
> 	/*
> @@ -777,7 +771,7 @@ void load_ucode_intel_ap(void)
>
> 	collect_cpu_info_early(&uci);
> 	ret = load_microcode(mc_saved_data_p, mc_saved_in_initrd_p,
> -			     initrd_start_addr, &uci);
> +			     get_initrd_start_addr(), &uci);
>
> 	if (ret != UCODE_OK)
> 		return;
>

       Thomas

-- 
  Thomas V

Re: x86/microcode update on systems without INITRD

[Borislav Petkov]

On Wed, Jan 20, 2016 at 10:14:49AM +0100, Thomas Voegtle wrote:
> 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.

Cool, thanks for testing.

I'll have a full patchset ready later, if you wanna test it for real, I
can let you know.

Thanks!

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--