All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] x86: Clean up remaining references to CONFIG_MICROCODE_AMD
@ 2023-08-25 14:12 Lukas Bulwahn
  2023-08-26 12:10 ` [tip: x86/microcode] x86/microcode: Remove " tip-bot2 for Lukas Bulwahn
  2023-11-12 15:03 ` [regression] microcode files missing in initramfs imgages from dracut (was Re: [PATCH] x86: Clean up remaining references to CONFIG_MICROCODE_AMD) Linux regression tracking (Thorsten Leemhuis)
  0 siblings, 2 replies; 22+ messages in thread
From: Lukas Bulwahn @ 2023-08-25 14:12 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H . Peter Anvin
  Cc: kernel-janitors, linux-kernel, Lukas Bulwahn

Commit e6bcfdd75d53 ("x86/microcode: Hide the config knob") removes config
MICROCODE_AMD, but left some references that have no effect on any kernel
build around.

Clean up those remaining config references. No functional change.

Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com>
---
 arch/x86/configs/i386_defconfig          | 1 -
 arch/x86/configs/x86_64_defconfig        | 1 -
 arch/x86/kernel/cpu/microcode/internal.h | 4 ++--
 3 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/x86/configs/i386_defconfig b/arch/x86/configs/i386_defconfig
index 75a343f10e58..1b411bbf3cb0 100644
--- a/arch/x86/configs/i386_defconfig
+++ b/arch/x86/configs/i386_defconfig
@@ -33,7 +33,6 @@ CONFIG_HYPERVISOR_GUEST=y
 CONFIG_PARAVIRT=y
 CONFIG_NR_CPUS=8
 CONFIG_X86_REROUTE_FOR_BROKEN_BOOT_IRQS=y
-CONFIG_MICROCODE_AMD=y
 CONFIG_X86_MSR=y
 CONFIG_X86_CPUID=y
 CONFIG_X86_CHECK_BIOS_CORRUPTION=y
diff --git a/arch/x86/configs/x86_64_defconfig b/arch/x86/configs/x86_64_defconfig
index 0902518e9b93..409e9182bd29 100644
--- a/arch/x86/configs/x86_64_defconfig
+++ b/arch/x86/configs/x86_64_defconfig
@@ -31,7 +31,6 @@ CONFIG_SMP=y
 CONFIG_HYPERVISOR_GUEST=y
 CONFIG_PARAVIRT=y
 CONFIG_X86_REROUTE_FOR_BROKEN_BOOT_IRQS=y
-CONFIG_MICROCODE_AMD=y
 CONFIG_X86_MSR=y
 CONFIG_X86_CPUID=y
 CONFIG_NUMA=y
diff --git a/arch/x86/kernel/cpu/microcode/internal.h b/arch/x86/kernel/cpu/microcode/internal.h
index 8ee9392e5c68..bf883aa71233 100644
--- a/arch/x86/kernel/cpu/microcode/internal.h
+++ b/arch/x86/kernel/cpu/microcode/internal.h
@@ -104,7 +104,7 @@ int save_microcode_in_initrd_amd(unsigned int family);
 void reload_ucode_amd(unsigned int cpu);
 struct microcode_ops *init_amd_microcode(void);
 void exit_amd_microcode(void);
-#else /* CONFIG_MICROCODE_AMD */
+#else /* CONFIG_CPU_SUP_AMD */
 static inline void load_ucode_amd_bsp(unsigned int family) { }
 static inline void load_ucode_amd_ap(unsigned int family) { }
 static inline void load_ucode_amd_early(unsigned int family) { }
@@ -112,7 +112,7 @@ static inline int save_microcode_in_initrd_amd(unsigned int family) { return -EI
 static inline void reload_ucode_amd(unsigned int cpu) { }
 static inline struct microcode_ops *init_amd_microcode(void) { return NULL; }
 static inline void exit_amd_microcode(void) { }
-#endif /* !CONFIG_MICROCODE_AMD */
+#endif /* !CONFIG_CPU_SUP_AMD */
 
 #ifdef CONFIG_CPU_SUP_INTEL
 void load_ucode_intel_bsp(void);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [tip: x86/microcode] x86/microcode: Remove remaining references to CONFIG_MICROCODE_AMD
  2023-08-25 14:12 [PATCH] x86: Clean up remaining references to CONFIG_MICROCODE_AMD Lukas Bulwahn
@ 2023-08-26 12:10 ` tip-bot2 for Lukas Bulwahn
  2023-11-12 15:03 ` [regression] microcode files missing in initramfs imgages from dracut (was Re: [PATCH] x86: Clean up remaining references to CONFIG_MICROCODE_AMD) Linux regression tracking (Thorsten Leemhuis)
  1 sibling, 0 replies; 22+ messages in thread
From: tip-bot2 for Lukas Bulwahn @ 2023-08-26 12:10 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Lukas Bulwahn, Ingo Molnar, Borislav Petkov (AMD), x86, linux-kernel

The following commit has been merged into the x86/microcode branch of tip:

Commit-ID:     4d2b748305e96fb76202a0d1072a285b1500bff3
Gitweb:        https://git.kernel.org/tip/4d2b748305e96fb76202a0d1072a285b1500bff3
Author:        Lukas Bulwahn <lukas.bulwahn@gmail.com>
AuthorDate:    Fri, 25 Aug 2023 16:12:26 +02:00
Committer:     Ingo Molnar <mingo@kernel.org>
CommitterDate: Sat, 26 Aug 2023 13:37:55 +02:00

x86/microcode: Remove remaining references to CONFIG_MICROCODE_AMD

Commit e6bcfdd75d53 ("x86/microcode: Hide the config knob") removed the
MICROCODE_AMD config, but left some references in defconfigs and comments,
that have no effect on any kernel build around.

Clean up those remaining config references. No functional change.

Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Borislav Petkov (AMD) <bp@alien8.de>
Link: https://lore.kernel.org/r/20230825141226.13566-1-lukas.bulwahn@gmail.com
---
 arch/x86/configs/i386_defconfig          | 1 -
 arch/x86/configs/x86_64_defconfig        | 1 -
 arch/x86/kernel/cpu/microcode/internal.h | 4 ++--
 3 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/x86/configs/i386_defconfig b/arch/x86/configs/i386_defconfig
index 3cf3491..c33250f 100644
--- a/arch/x86/configs/i386_defconfig
+++ b/arch/x86/configs/i386_defconfig
@@ -33,7 +33,6 @@ CONFIG_HYPERVISOR_GUEST=y
 CONFIG_PARAVIRT=y
 CONFIG_NR_CPUS=8
 CONFIG_X86_REROUTE_FOR_BROKEN_BOOT_IRQS=y
-CONFIG_MICROCODE_AMD=y
 CONFIG_X86_MSR=y
 CONFIG_X86_CPUID=y
 CONFIG_X86_CHECK_BIOS_CORRUPTION=y
diff --git a/arch/x86/configs/x86_64_defconfig b/arch/x86/configs/x86_64_defconfig
index 2775923..2aae0c0 100644
--- a/arch/x86/configs/x86_64_defconfig
+++ b/arch/x86/configs/x86_64_defconfig
@@ -31,7 +31,6 @@ CONFIG_SMP=y
 CONFIG_HYPERVISOR_GUEST=y
 CONFIG_PARAVIRT=y
 CONFIG_X86_REROUTE_FOR_BROKEN_BOOT_IRQS=y
-CONFIG_MICROCODE_AMD=y
 CONFIG_X86_MSR=y
 CONFIG_X86_CPUID=y
 CONFIG_NUMA=y
diff --git a/arch/x86/kernel/cpu/microcode/internal.h b/arch/x86/kernel/cpu/microcode/internal.h
index 8ee9392..bf883aa 100644
--- a/arch/x86/kernel/cpu/microcode/internal.h
+++ b/arch/x86/kernel/cpu/microcode/internal.h
@@ -104,7 +104,7 @@ int save_microcode_in_initrd_amd(unsigned int family);
 void reload_ucode_amd(unsigned int cpu);
 struct microcode_ops *init_amd_microcode(void);
 void exit_amd_microcode(void);
-#else /* CONFIG_MICROCODE_AMD */
+#else /* CONFIG_CPU_SUP_AMD */
 static inline void load_ucode_amd_bsp(unsigned int family) { }
 static inline void load_ucode_amd_ap(unsigned int family) { }
 static inline void load_ucode_amd_early(unsigned int family) { }
@@ -112,7 +112,7 @@ static inline int save_microcode_in_initrd_amd(unsigned int family) { return -EI
 static inline void reload_ucode_amd(unsigned int cpu) { }
 static inline struct microcode_ops *init_amd_microcode(void) { return NULL; }
 static inline void exit_amd_microcode(void) { }
-#endif /* !CONFIG_MICROCODE_AMD */
+#endif /* !CONFIG_CPU_SUP_AMD */
 
 #ifdef CONFIG_CPU_SUP_INTEL
 void load_ucode_intel_bsp(void);

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [regression] microcode files missing in initramfs imgages from dracut (was Re: [PATCH] x86: Clean up remaining references to CONFIG_MICROCODE_AMD)
  2023-08-25 14:12 [PATCH] x86: Clean up remaining references to CONFIG_MICROCODE_AMD Lukas Bulwahn
  2023-08-26 12:10 ` [tip: x86/microcode] x86/microcode: Remove " tip-bot2 for Lukas Bulwahn
@ 2023-11-12 15:03 ` Linux regression tracking (Thorsten Leemhuis)
  2023-11-12 18:10   ` Borislav Petkov
  2023-11-23  4:07   ` Linux regression tracking #update (Thorsten Leemhuis)
  1 sibling, 2 replies; 22+ messages in thread
From: Linux regression tracking (Thorsten Leemhuis) @ 2023-11-12 15:03 UTC (permalink / raw)
  To: lukas.bulwahn
  Cc: bp, dave.hansen, hpa, kernel-janitors, linux-kernel, mingo, tglx,
	x86, Linux kernel regressions list

Hi Lukas!

> Commit e6bcfdd75d53 ("x86/microcode: Hide the config knob") removes config
> MICROCODE_AMD, but left some references that have no effect on any kernel
> build around.
> 
> Clean up those remaining config references. No functional change.
> [...]

That patch became 4d2b748305e96f ("x86/microcode: Remove remaining
references to CONFIG_MICROCODE_AMD"). Not totally sure, but from briefly
looking into things it seems likely that it causes a regression with
dracut that was just reported here:

https://bugzilla.kernel.org/show_bug.cgi?id=218136

As I understand it older dracut releases due to that change do not
include microcode files in the generated initramfs images anymore.
That's because dracut until the recent commit
https://github.com/dracutdevs/dracut/commit/6c80408c8644a0add1907b0593eb83f90d6247b1
looked for CONFIG_MICROCODE_AMD and CONFIG_MICROCODE_INTEL in the config
file to decide what to include or not.

The reporter noticed, as some diag app suddenly reported the system as
vulnerable to "gather data sampling" vulnerability. See the ticket for
details.

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)

P.S.: Note, you have to use bugzilla to reach the reporter, as I
sadly[1] can not CCed them in mails like this. Furthermore:

[TLDR for the rest of this mail: I'm adding this report to the list of
tracked Linux kernel regressions; the text you find below is based on a
few templates paragraphs you might have encountered already in similar
form.]

BTW, let me use this mail to also add the report to the list of tracked
regressions to ensure it's doesn't fall through the cracks:

#regzbot introduced: 4d2b748305e96f
https://bugzilla.kernel.org/show_bug.cgi?id=218136
#regzbot title: x86: microcode files missing in initramfs imgages from
dracut
#regzbot ignore-activity

This isn't a regression? This issue or a fix for it are already
discussed somewhere else? It was fixed already? You want to clarify when
the regression started to happen? Or point out I got the title or
something else totally wrong? Then just reply and tell me -- ideally
while also telling regzbot about it, as explained by the page listed in
the footer of this mail.

Developers: When fixing the issue, remember to add 'Link:' tags pointing
to the report (e.g. the buzgzilla ticket and maybe this mail as well, if
this thread sees some discussion). See page linked in footer for details.

[1] because bugzilla.kernel.org tells users upon registration their
"email address will never be displayed to logged out users"

--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [regression] microcode files missing in initramfs imgages from dracut (was Re: [PATCH] x86: Clean up remaining references to CONFIG_MICROCODE_AMD)
  2023-11-12 15:03 ` [regression] microcode files missing in initramfs imgages from dracut (was Re: [PATCH] x86: Clean up remaining references to CONFIG_MICROCODE_AMD) Linux regression tracking (Thorsten Leemhuis)
@ 2023-11-12 18:10   ` Borislav Petkov
  2023-11-22  9:15     ` Linux regression tracking (Thorsten Leemhuis)
  2023-11-23  4:07   ` Linux regression tracking #update (Thorsten Leemhuis)
  1 sibling, 1 reply; 22+ messages in thread
From: Borislav Petkov @ 2023-11-12 18:10 UTC (permalink / raw)
  To: Linux regressions mailing list
  Cc: lukas.bulwahn, dave.hansen, hpa, kernel-janitors, linux-kernel,
	mingo, tglx, x86

On Sun, Nov 12, 2023 at 04:03:32PM +0100, Linux regression tracking (Thorsten Leemhuis) wrote:
> That's because dracut until the recent commit
> https://github.com/dracutdevs/dracut/commit/6c80408c8644a0add1907b0593eb83f90d6247b1
> looked for CONFIG_MICROCODE_AMD and CONFIG_MICROCODE_INTEL in the config
> file to decide what to include or not.

They've been told a bunch of times already that grepping .config for
specific symbols is not how one should check whether one should add
microcode blobs to the initrd or not because Kconfig symbols are not an
ABI.

And looking at that commit, now they're grepping for CONFIG_MICROCODE.
And that'll break again if one day we decide to make the microcode
loader built in unconditionally.

How to fix this reliably and properly?

Honestly, I don't have a good idea. If we do something like this:

grep microcode_init System.map

then that makes "microode_init" ABI and we won't be able to change it
eva. I'd need to do some digging here...

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [regression] microcode files missing in initramfs imgages from dracut (was Re: [PATCH] x86: Clean up remaining references to CONFIG_MICROCODE_AMD)
  2023-11-12 18:10   ` Borislav Petkov
@ 2023-11-22  9:15     ` Linux regression tracking (Thorsten Leemhuis)
  2023-11-22 11:58       ` Borislav Petkov
  2023-11-22 13:45       ` Lukas Bulwahn
  0 siblings, 2 replies; 22+ messages in thread
From: Linux regression tracking (Thorsten Leemhuis) @ 2023-11-22  9:15 UTC (permalink / raw)
  To: Borislav Petkov, Linux regressions mailing list
  Cc: lukas.bulwahn, dave.hansen, hpa, kernel-janitors, linux-kernel,
	mingo, tglx, x86

On 12.11.23 19:10, Borislav Petkov wrote:
> On Sun, Nov 12, 2023 at 04:03:32PM +0100, Linux regression tracking (Thorsten Leemhuis) wrote:
>> That's because dracut until the recent commit
>> https://github.com/dracutdevs/dracut/commit/6c80408c8644a0add1907b0593eb83f90d6247b1
>> looked for CONFIG_MICROCODE_AMD and CONFIG_MICROCODE_INTEL in the config
>> file to decide what to include or not.
> 
> They've been told a bunch of times already that grepping .config for
> specific symbols is not how one should check whether one should add
> microcode blobs to the initrd or not because Kconfig symbols are not an
> ABI.

Maybe, but you know how Linus sees things like this: what's considered
an ABI/API or not is nearly[1] irrelevant - if a change breaks something
that used to work then it needs to be fixed.

[1] unless you fiddle with things obviously internal; not sure if this
case would qualify for him, but somehow I doubt it -- but I might be
wrong there.

> And looking at that commit, now they're grepping for CONFIG_MICROCODE.
> And that'll break again if one day we decide to make the microcode
> loader built in unconditionally.
> 
> How to fix this reliably and properly?
> 
> Honestly, I don't have a good idea. If we do something like this:
> grep microcode_init System.map
> 
> then that makes "microode_init" ABI and we won't be able to change it
> eva. I'd need to do some digging here...

Any progress on this?

BTW: I see that this could help preventing problems like the current one
to happen in the far future. But how would that help the current
situation (e.g. users that have an old dracut and updated the kernel
without updating dracut)?

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.

#regzbot poke

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [regression] microcode files missing in initramfs imgages from dracut (was Re: [PATCH] x86: Clean up remaining references to CONFIG_MICROCODE_AMD)
  2023-11-22  9:15     ` Linux regression tracking (Thorsten Leemhuis)
@ 2023-11-22 11:58       ` Borislav Petkov
  2023-11-22 13:24         ` [PATCH] x86: Add a "x86" ELF note namespace Borislav Petkov
  2023-11-22 15:34         ` [regression] microcode files missing in initramfs imgages from dracut (was Re: [PATCH] x86: Clean up remaining references to CONFIG_MICROCODE_AMD) Linux regression tracking (Thorsten Leemhuis)
  2023-11-22 13:45       ` Lukas Bulwahn
  1 sibling, 2 replies; 22+ messages in thread
From: Borislav Petkov @ 2023-11-22 11:58 UTC (permalink / raw)
  To: Linux regressions mailing list
  Cc: lukas.bulwahn, dave.hansen, hpa, kernel-janitors, linux-kernel,
	mingo, tglx, x86

On Wed, Nov 22, 2023 at 10:15:42AM +0100, Linux regression tracking (Thorsten Leemhuis) wrote:
> [1] unless you fiddle with things obviously internal; not sure if this
> case would qualify for him, but somehow I doubt it -- but I might be
> wrong there.

Well, think about it - by that logic, if CONFIG_* items are an ABI, we
will never ever be able to change any of them. Now that would be awful.

> Any progress on this?

We're thinking...

We might need an official scheme of stating what any given kernel image
supports for use by external tools which need it.

> BTW: I see that this could help preventing problems like the current one
> to happen in the far future. But how would that help the current
> situation (e.g. users that have an old dracut and updated the kernel
> without updating dracut)?

Update dracut too?

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH] x86: Add a "x86" ELF note namespace
  2023-11-22 11:58       ` Borislav Petkov
@ 2023-11-22 13:24         ` Borislav Petkov
  2023-11-22 14:07           ` Borislav Petkov
  2023-11-22 15:34         ` [regression] microcode files missing in initramfs imgages from dracut (was Re: [PATCH] x86: Clean up remaining references to CONFIG_MICROCODE_AMD) Linux regression tracking (Thorsten Leemhuis)
  1 sibling, 1 reply; 22+ messages in thread
From: Borislav Petkov @ 2023-11-22 13:24 UTC (permalink / raw)
  To: Linux regressions mailing list
  Cc: lukas.bulwahn, dave.hansen, hpa, kernel-janitors, linux-kernel,
	mingo, tglx, x86, initramfs

On Wed, Nov 22, 2023 at 12:58:26PM +0100, Borislav Petkov wrote:
> On Wed, Nov 22, 2023 at 10:15:42AM +0100, Linux regression tracking (Thorsten Leemhuis) wrote:
> > [1] unless you fiddle with things obviously internal; not sure if this
> > case would qualify for him, but somehow I doubt it -- but I might be
> > wrong there.
> 
> Well, think about it - by that logic, if CONFIG_* items are an ABI, we
> will never ever be able to change any of them. Now that would be awful.
> 
> > Any progress on this?
> 
> We're thinking...

Turns out this is easier than I think and people have solved this
problem already - all I need to do is use it. Wonderful.

Lemme Cc initramfs@vger.kernel.org as an FYI and see whether dracut
folks would have any comments about this.

---
From: "Borislav Petkov (AMD)" <bp@alien8.de>
Date: Wed, 22 Nov 2023 13:59:40 +0100

Add a "x86" ELF note namespace to put ELF note structures with which to
communicate in a ABI-compliant manner what a kernel image supports.

Also, add the first note type of this - X86_ELFNOTE_MICROCODE - which
denotes that microcode support is built into the kernel image and thus
initrd-generating tools like dracut can parse the ELF .notes section for
this.

$ readelf -n vmlinux

Displaying notes found in: .notes
  Owner                Data size        Description
  ...
  x86                  0x00000004       Unknown note type: (0x00000000)
  description data: 01 00 00 00
  ^^^^^^^^^^^^^^

Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
---
 arch/x86/include/uapi/elfnote.h      | 18 ++++++++++++++++++
 arch/x86/kernel/cpu/microcode/core.c |  5 +++++
 2 files changed, 23 insertions(+)
 create mode 100644 arch/x86/include/uapi/elfnote.h

diff --git a/arch/x86/include/uapi/elfnote.h b/arch/x86/include/uapi/elfnote.h
new file mode 100644
index 000000000000..bef26c4944e8
--- /dev/null
+++ b/arch/x86/include/uapi/elfnote.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#ifndef _X86_UAPI_ELFNOTE_H_
+#define _X86_UAPI_ELFNOTE_H_
+
+/*
+ * "x86" namespaced ELF note structures to communicate features
+ * supported by the kernel binary to external utilities which need that
+ * info in order to do additional preparatory work based on the target
+ * kernel image.
+ */
+
+/*
+ * Used by the microcode loader to communicate support to external
+ * initrd generators like dracut.
+ */
+#define X86_ELFNOTE_MICROCODE	0
+
+#endif /* _X86_UAPI_ELFNOTE_H_ */
diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c
index 232026a239a6..f35444bafbbc 100644
--- a/arch/x86/kernel/cpu/microcode/core.c
+++ b/arch/x86/kernel/cpu/microcode/core.c
@@ -24,6 +24,7 @@
 #include <linux/capability.h>
 #include <linux/firmware.h>
 #include <linux/cpumask.h>
+#include <linux/elfnote.h>
 #include <linux/kernel.h>
 #include <linux/delay.h>
 #include <linux/mutex.h>
@@ -32,6 +33,8 @@
 #include <linux/fs.h>
 #include <linux/mm.h>
 
+#include <uapi/elfnote.h>
+
 #include <asm/apic.h>
 #include <asm/cpu_device_id.h>
 #include <asm/perf_event.h>
@@ -859,3 +862,5 @@ static int __init microcode_init(void)
 
 }
 late_initcall(microcode_init);
+
+ELFNOTE32("x86", X86_ELFNOTE_MICROCODE, CONFIG_MICROCODE);
-- 
2.42.0.rc0.25.ga82fb66fed25

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [regression] microcode files missing in initramfs imgages from dracut (was Re: [PATCH] x86: Clean up remaining references to CONFIG_MICROCODE_AMD)
  2023-11-22  9:15     ` Linux regression tracking (Thorsten Leemhuis)
  2023-11-22 11:58       ` Borislav Petkov
@ 2023-11-22 13:45       ` Lukas Bulwahn
  1 sibling, 0 replies; 22+ messages in thread
From: Lukas Bulwahn @ 2023-11-22 13:45 UTC (permalink / raw)
  To: Linux regressions mailing list
  Cc: Borislav Petkov, dave.hansen, hpa, kernel-janitors, linux-kernel,
	mingo, tglx, x86

On Wed, Nov 22, 2023 at 10:15 AM Linux regression tracking (Thorsten
Leemhuis) <regressions@leemhuis.info> wrote:
>
> On 12.11.23 19:10, Borislav Petkov wrote:
> > On Sun, Nov 12, 2023 at 04:03:32PM +0100, Linux regression tracking (Thorsten Leemhuis) wrote:
> >> That's because dracut until the recent commit
> >> https://github.com/dracutdevs/dracut/commit/6c80408c8644a0add1907b0593eb83f90d6247b1
> >> looked for CONFIG_MICROCODE_AMD and CONFIG_MICROCODE_INTEL in the config
> >> file to decide what to include or not.
> >
> > They've been told a bunch of times already that grepping .config for
> > specific symbols is not how one should check whether one should add
> > microcode blobs to the initrd or not because Kconfig symbols are not an
> > ABI.
>
> Maybe, but you know how Linus sees things like this: what's considered
> an ABI/API or not is nearly[1] irrelevant - if a change breaks something
> that used to work then it needs to be fixed.
>
> [1] unless you fiddle with things obviously internal; not sure if this
> case would qualify for him, but somehow I doubt it -- but I might be
> wrong there.
>

Thorsten, I think you are wrong here in this case. We are talking
about the kernel build configuration options and their names and these
are certainly not stable and never have been.

Some indication to show that the rate of change we are generally
talking about without anyone considering this stable ABI/API:

You can run "find . -name Kconfig* | xargs grep -h -E '^(menu)config '
| sort | uniq" on each kernel release. Then diff those lists of
configs with increasing kernel config versions.

If this would be stable, then only config options should appear and
little should disappear, but that is not the case. And if something
disappears, it should relate to a driver/feature that was removed, but
that is also not always the case.

Here are some quick numbers:

v6.0 to v6.1: 43 removals
v6.1 to v6.2: 40 removals
v6.2 to v6.3: 350 removals
v6.3 to v6.4: 86 removals
v6.4 to v6.5: 73 removals
v6.5 to v6.6: 61 removals

* Removals can also be potentially a renaming.

So, these config names are certainly not stable ABI/API. We can
investigate a bit deeper on which changes are due to driver removals,
which due to config removal but making a feature default and which are
simply a config renaming, but in the past, hardly any kernel developer
has considered this interface to be a special stable ABI/API.

Further, to my knowledge looking at kernel discussions and the
repository, there are currently no tools out there that would assist
in updating a kernel build configuration from one version to the next.

So, we are talking about roughly more than 50 removals to kernel
config options every release, and now there was this one special case
in one release, where a tool incorrectly relies on this one config
option to be stable. That is not a regression of a stable ABI/API,
that is a misuse of an internal non-stable ABI/API.


Lukas

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] x86: Add a "x86" ELF note namespace
  2023-11-22 13:24         ` [PATCH] x86: Add a "x86" ELF note namespace Borislav Petkov
@ 2023-11-22 14:07           ` Borislav Petkov
  0 siblings, 0 replies; 22+ messages in thread
From: Borislav Petkov @ 2023-11-22 14:07 UTC (permalink / raw)
  To: Linux regressions mailing list
  Cc: lukas.bulwahn, dave.hansen, hpa, kernel-janitors, linux-kernel,
	mingo, tglx, x86, initramfs

On Wed, Nov 22, 2023 at 02:24:43PM +0100, Borislav Petkov wrote:
> Displaying notes found in: .notes
>   Owner                Data size        Description
>   ...
>   x86                  0x00000004       Unknown note type: (0x00000000)
>   description data: 01 00 00 00
>   ^^^^^^^^^^^^^^

Note to self: since this is a u32, the next version should define that
only the 1st bit is valid and the rest are reserved. So that they can be
used as a bitfield in case something more needs to be communicated in
the future and we don't waste a whole u32 just for one bit of
information...

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [regression] microcode files missing in initramfs imgages from dracut (was Re: [PATCH] x86: Clean up remaining references to CONFIG_MICROCODE_AMD)
  2023-11-22 11:58       ` Borislav Petkov
  2023-11-22 13:24         ` [PATCH] x86: Add a "x86" ELF note namespace Borislav Petkov
@ 2023-11-22 15:34         ` Linux regression tracking (Thorsten Leemhuis)
  2023-11-22 15:57           ` Borislav Petkov
  1 sibling, 1 reply; 22+ messages in thread
From: Linux regression tracking (Thorsten Leemhuis) @ 2023-11-22 15:34 UTC (permalink / raw)
  To: Borislav Petkov, Linux regressions mailing list
  Cc: lukas.bulwahn, dave.hansen, hpa, kernel-janitors, linux-kernel,
	mingo, tglx, x86

Preface: considered CCing Linus here, as it's quite possible that I'm
wrong, as every situation is somewhat different. If anybody disagrees
with what I bring up below to hopefully clarify things thus please do me
a favor an CC Linus so he can clarify things.

Ohh, and sorry for being a PITA. I hate that, but when it comes to
regressions disagreements often happen, as all those discussions linked
at the end of https://docs.kernel.org/process/handling-regressions.html
illustrate.

On 22.11.23 12:58, Borislav Petkov wrote:
> On Wed, Nov 22, 2023 at 10:15:42AM +0100, Linux regression tracking (Thorsten Leemhuis) wrote:
>> [1] unless you fiddle with things obviously internal; not sure if this
>> case would qualify for him, but somehow I doubt it -- but I might be
>> wrong there.
> 
> Well, think about it - by that logic, if CONFIG_* items are an ABI, we
> will never ever be able to change any of them. [...]

Can't follow your logic (or the one from Lukas in the other reply), as
what's an ABI (or an API) is afaik not the important factor when it
comes to the "no regressions" rule: you can change things (including
ABIs or APIs) all you want, as long as nothing breaks. To quote Linus from
https://lore.kernel.org/all/CAHk-=wiVi7mSrsMP=fLXQrXK_UimybW=ziLOwSzFTtoXUacWVQ@mail.gmail.com/

```
The rules about regressions have never been about any kind of
documented behavior, or where the code lives.

The rules about regressions are always about "breaks user workflow".

The other side of the coin is that people who talk about "API
stability" are entirely wrong. API's don't matter either. You can make
any changes to an API you like - as long as nobody notices.

Again, the regression rule is not about documentation, not about
API's, and not about the phase of the moon.

It's entirely about "we caused problems for user space that used to work".
```

>> BTW: I see that this could help preventing problems like the current one
>> to happen in the far future. But how would that help the current
>> situation (e.g. users that have an old dracut and updated the kernel
>> without updating dracut)?
> Update dracut too?

To quote Linus again, this time from
https://lore.kernel.org/lkml/CA+55aFxW7NMAMvYhkvz1UPbUTUJewRt6Yb51QAx5RtrWOwjebg@mail.gmail.com/

```
People should basically always feel like they can update their kernel
and simply not have to worry about it.

I refuse to introduce "you can only update the kernel if you also
update that other program" kind of limitations. If the kernel used to
work for you, the rule is that it continues to work for you.

There have been exceptions, but they are few and far between,
[...]
But if something actually breaks, then the change must get fixed or
reverted. And it gets fixed in the *kernel*. Not by saying "well, fix
your user space then".
```

Are those quotes fitting to the situation at hand? Not totally sure.
Initramfs generators might be special and we have done exceptions for
them in the past if no other solution could be found to prevent a
regression[1]. We'd need Linus to clarify.

Ciao, Thorsten

[1] maybe it's a naive idea, but can't we just avoid the problem at hand
by adding CONFIG_MICROCODE_AMD and CONFIG_MICROCODE_INTEL back as a
hidden config stub and remove those in ~3 years? Yeah, ugly, but we have
done things way more ugly than that to prevent regressions.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [regression] microcode files missing in initramfs imgages from dracut (was Re: [PATCH] x86: Clean up remaining references to CONFIG_MICROCODE_AMD)
  2023-11-22 15:34         ` [regression] microcode files missing in initramfs imgages from dracut (was Re: [PATCH] x86: Clean up remaining references to CONFIG_MICROCODE_AMD) Linux regression tracking (Thorsten Leemhuis)
@ 2023-11-22 15:57           ` Borislav Petkov
  2023-11-22 20:35             ` Linus Torvalds
  0 siblings, 1 reply; 22+ messages in thread
From: Borislav Petkov @ 2023-11-22 15:57 UTC (permalink / raw)
  To: Linux regressions mailing list, Linus Torvalds
  Cc: lukas.bulwahn, dave.hansen, hpa, kernel-janitors, linux-kernel,
	mingo, tglx, x86

Sure,

lemme do that.

Hi Linus,

we have a disagreement on what is a userspace regression and what is
not.

The whole thread starts here:

https://lore.kernel.org/r/c67bd324-cec0-4fe4-b3b1-fc1d1e4f2967@leemhuis.info

and I'm leaving Thorsten's arguments fully quoted below for more
context.

Basically, dracut has been grepping the kernel's .config to figure out
whether to add microcode blobs to the intird or not.

Now, we changed a CONFIG and it broke. Again. It wasn't the first time.

It went and fixed it this way:

https://github.com/dracutdevs/dracut/commit/6c80408c8644a0add1907b0593eb83f90d6247b1

which will break next time we change stuff.

IMO, yes, we should not break userspace but dracut is special. And it
parses willy nilly kernel internals which are not ABI to begin with.

Looking at that dracut function check_kernel_config(), it does:

    # no kernel config file, so return true
    [[ $_config_file ]] || return 0

if it can't find a kernel .config at the two places it looks for which
is just silly: if it can't find a .config just return true and include
those microcode blobs. Might as well hide the config as a fix. :-)

What it should do, is parse the .notes section of vmlinux for which
I have a proper fix:

https://lore.kernel.org/r/20231122132419.GBZV4BA399sG2JRFAJ@fat_crate.local

So IMNSVHO, CONFIG symbols are not an ABI. 

If there's some other userspace tool which goes and greps the kernel
sources and looks for a particular function or symbol which is not even
exported, does that mean that we won't be able to change that function
name or symbol anymore just because some random tool touches it.

Yes, I know, we should not break userspace but there has to be some
sensible limit somewhere as to what constitutes a userspace breakage.

In the end of the day, that's your call.

If we consider this a userspace breakage, I would add back those
CONFIG_MICROCODE_INTEL and CONFIG_MICROCODE_AMD Kconfig symbols and
everytime I add a new CONFIG symbol, I should probably write a big fat
note above it that userspace should not rely on it existing forever...

Thx.

On Wed, Nov 22, 2023 at 04:34:03PM +0100, Linux regression tracking (Thorsten Leemhuis) wrote:
> Preface: considered CCing Linus here, as it's quite possible that I'm
> wrong, as every situation is somewhat different. If anybody disagrees
> with what I bring up below to hopefully clarify things thus please do me
> a favor an CC Linus so he can clarify things.
> 
> Ohh, and sorry for being a PITA. I hate that, but when it comes to
> regressions disagreements often happen, as all those discussions linked
> at the end of https://docs.kernel.org/process/handling-regressions.html
> illustrate.
> 
> On 22.11.23 12:58, Borislav Petkov wrote:
> > On Wed, Nov 22, 2023 at 10:15:42AM +0100, Linux regression tracking (Thorsten Leemhuis) wrote:
> >> [1] unless you fiddle with things obviously internal; not sure if this
> >> case would qualify for him, but somehow I doubt it -- but I might be
> >> wrong there.
> > 
> > Well, think about it - by that logic, if CONFIG_* items are an ABI, we
> > will never ever be able to change any of them. [...]
> 
> Can't follow your logic (or the one from Lukas in the other reply), as
> what's an ABI (or an API) is afaik not the important factor when it
> comes to the "no regressions" rule: you can change things (including
> ABIs or APIs) all you want, as long as nothing breaks. To quote Linus from
> https://lore.kernel.org/all/CAHk-=wiVi7mSrsMP=fLXQrXK_UimybW=ziLOwSzFTtoXUacWVQ@mail.gmail.com/
> 
> ```
> The rules about regressions have never been about any kind of
> documented behavior, or where the code lives.
> 
> The rules about regressions are always about "breaks user workflow".
> 
> The other side of the coin is that people who talk about "API
> stability" are entirely wrong. API's don't matter either. You can make
> any changes to an API you like - as long as nobody notices.
> 
> Again, the regression rule is not about documentation, not about
> API's, and not about the phase of the moon.
> 
> It's entirely about "we caused problems for user space that used to work".
> ```
> 
> >> BTW: I see that this could help preventing problems like the current one
> >> to happen in the far future. But how would that help the current
> >> situation (e.g. users that have an old dracut and updated the kernel
> >> without updating dracut)?
> > Update dracut too?
> 
> To quote Linus again, this time from
> https://lore.kernel.org/lkml/CA+55aFxW7NMAMvYhkvz1UPbUTUJewRt6Yb51QAx5RtrWOwjebg@mail.gmail.com/
> 
> ```
> People should basically always feel like they can update their kernel
> and simply not have to worry about it.
> 
> I refuse to introduce "you can only update the kernel if you also
> update that other program" kind of limitations. If the kernel used to
> work for you, the rule is that it continues to work for you.
> 
> There have been exceptions, but they are few and far between,
> [...]
> But if something actually breaks, then the change must get fixed or
> reverted. And it gets fixed in the *kernel*. Not by saying "well, fix
> your user space then".
> ```
> 
> Are those quotes fitting to the situation at hand? Not totally sure.
> Initramfs generators might be special and we have done exceptions for
> them in the past if no other solution could be found to prevent a
> regression[1]. We'd need Linus to clarify.
> 
> Ciao, Thorsten
> 
> [1] maybe it's a naive idea, but can't we just avoid the problem at hand
> by adding CONFIG_MICROCODE_AMD and CONFIG_MICROCODE_INTEL back as a
> hidden config stub and remove those in ~3 years? Yeah, ugly, but we have
> done things way more ugly than that to prevent regressions.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [regression] microcode files missing in initramfs imgages from dracut (was Re: [PATCH] x86: Clean up remaining references to CONFIG_MICROCODE_AMD)
  2023-11-22 15:57           ` Borislav Petkov
@ 2023-11-22 20:35             ` Linus Torvalds
  2023-11-22 20:51               ` Borislav Petkov
  0 siblings, 1 reply; 22+ messages in thread
From: Linus Torvalds @ 2023-11-22 20:35 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Linux regressions mailing list, lukas.bulwahn, dave.hansen, hpa,
	kernel-janitors, linux-kernel, mingo, tglx, x86

On Wed, 22 Nov 2023 at 07:58, Borislav Petkov <bp@alien8.de> wrote:
>
> IMO, yes, we should not break userspace but dracut is special. And it
> parses willy nilly kernel internals which are not ABI to begin with.

I don't think the "dracut is special" is the thing that matters.

The real issue is that hey, if dracut in its incompetence doesn't
include the microcode on the initrd, that doesn't really matter much.
It's fairly easily fixable, and at worst it will mean that we end up
having CPU mitigations that aren't optimal. Since most of those are BS
anyway, it really doesn't seem critical.

Sure, it's a "regression" in that you don't get the microcode update
included, but from a user perspective things should still continue to
work.

End result: this seems to be pretty solidly a distro issue.

IOW, the whole "users are the only thing that matters" pretty much
means that it's a non-issue. Things continued to work, to the point
that I'm actually surprised anybody even noticed.

That said, I don't think some ELF note is the fix either. I think we
might as well leave it at CONFIG_MICROCODE. Maybe add a note in the
kernel Kconfig that this thing matters for dracut.

Dracut also checks for CONFIG_ACPI_INITRD_TABLE_OVERRIDE. It's a
similar "normal users don't care".

              Linus

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [regression] microcode files missing in initramfs imgages from dracut (was Re: [PATCH] x86: Clean up remaining references to CONFIG_MICROCODE_AMD)
  2023-11-22 20:35             ` Linus Torvalds
@ 2023-11-22 20:51               ` Borislav Petkov
  2023-11-22 21:08                 ` Linus Torvalds
  0 siblings, 1 reply; 22+ messages in thread
From: Borislav Petkov @ 2023-11-22 20:51 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Linux regressions mailing list, lukas.bulwahn, dave.hansen, hpa,
	kernel-janitors, linux-kernel, mingo, tglx, x86

On Wed, Nov 22, 2023 at 12:35:54PM -0800, Linus Torvalds wrote:
> IOW, the whole "users are the only thing that matters" pretty much
> means that it's a non-issue. Things continued to work, to the point
> that I'm actually surprised anybody even noticed.

Right, the patch which did the changes is in 6.6:

e6bcfdd75d53 ("x86/microcode: Hide the config knob")

I'm still waiting for the other shoe to drop when 6.6 gets used more but
we'll see...

> That said, I don't think some ELF note is the fix either. I think we
> might as well leave it at CONFIG_MICROCODE. Maybe add a note in the
> kernel Kconfig that this thing matters for dracut.
> 
> Dracut also checks for CONFIG_ACPI_INITRD_TABLE_OVERRIDE. It's a
> similar "normal users don't care".

Ok.

My only worry here is that we're making a precedent and basically saying
that it is ok for tools to grep .config to figure out what is supported
by the kernel. And then other tools might follow.

I have no clue how many tools are actually interested in stuff enabled
in the kernel .config though. If only dracut then sure, don't care, but
what if it starts proliferating...

I'm just talking with my devil's advocate hat on.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [regression] microcode files missing in initramfs imgages from dracut (was Re: [PATCH] x86: Clean up remaining references to CONFIG_MICROCODE_AMD)
  2023-11-22 20:51               ` Borislav Petkov
@ 2023-11-22 21:08                 ` Linus Torvalds
  2023-11-22 21:35                   ` Borislav Petkov
  2023-11-23 11:20                   ` Borislav Petkov
  0 siblings, 2 replies; 22+ messages in thread
From: Linus Torvalds @ 2023-11-22 21:08 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Linux regressions mailing list, lukas.bulwahn, dave.hansen, hpa,
	kernel-janitors, linux-kernel, mingo, tglx, x86

On Wed, 22 Nov 2023 at 12:51, Borislav Petkov <bp@alien8.de> wrote:
>
> My only worry here is that we're making a precedent and basically saying
> that it is ok for tools to grep .config to figure out what is supported
> by the kernel. And then other tools might follow.

Yes, I agree that it's not optimal, but I would hate to have some odd
"let's add another ELF note" churn too, for (presumably) increasingly
obscure reasons.

It looks like dracut has been doing this forever, and in fact back in
2015 apparently had the exact same issue (that never made it to kernel
developers, or at least not to me), when the kernel
CONFIG_MICROCODE_xyz_EARLY config went away, and became just
CONFIG_MICROCODE_xyz.

The whole "check kernel config" in dracut seems to go back to 2014, so
it's been that way for almost a decade by now.

Honestly, I think the right approach may be to just remove the check
again from dracut entirely - the intent seems to be to make the initrd
smaller when people don't support microcode updates, but does that
ever actually *happen*?

There are dracut command lines, like "--early-microcode" and
"--no-early-microcode", so people who really want to save space could
just force it that way. Doing the CONFIG_xyz check seems broken.

But that's for the dracut people to worry about.

I guess we on the kernel side could help with "make install" etc, but
we've (intentionally) tried to insulate us from distros having
distro-specific installkernel scripts, so we don't really haev a good
way to pass information down to the installkernel side.

It *would* make sense if we just had some actual arguments we might
pass down. Right now we just do

        exec "${file}" "${KERNELRELEASE}" "${KBUILD_IMAGE}" System.map
"${INSTALL_PATH}"

so basically the only argument we pass down is that INSTALL_PATH
(which is just "/boot" by default).

            Linus

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [regression] microcode files missing in initramfs imgages from dracut (was Re: [PATCH] x86: Clean up remaining references to CONFIG_MICROCODE_AMD)
  2023-11-22 21:08                 ` Linus Torvalds
@ 2023-11-22 21:35                   ` Borislav Petkov
  2023-11-23 11:20                   ` Borislav Petkov
  1 sibling, 0 replies; 22+ messages in thread
From: Borislav Petkov @ 2023-11-22 21:35 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Linux regressions mailing list, lukas.bulwahn, dave.hansen, hpa,
	kernel-janitors, linux-kernel, mingo, tglx, x86, initramfs

Lemme add initramfs@vger.kernel.org to Cc again. I hope that's the
correct ML dracut folks use.

On Wed, Nov 22, 2023 at 01:08:41PM -0800, Linus Torvalds wrote:
> Yes, I agree that it's not optimal, but I would hate to have some odd
> "let's add another ELF note" churn too, for (presumably) increasingly
> obscure reasons.

Right, my angle with the ELF note is that it is at least something well
establshed and other things use it too (Xen, BUILD_SALT, other arches
too).

> It looks like dracut has been doing this forever, and in fact back in
> 2015 apparently had the exact same issue (that never made it to kernel
> developers, or at least not to me), when the kernel
> CONFIG_MICROCODE_xyz_EARLY config went away, and became just
> CONFIG_MICROCODE_xyz.

Yap, that was me. I merged the early loader because it didn't make any
sense to have a separate thing.

> The whole "check kernel config" in dracut seems to go back to 2014, so
> it's been that way for almost a decade by now.
> 
> Honestly, I think the right approach may be to just remove the check
> again from dracut entirely - the intent seems to be to make the initrd
> smaller when people don't support microcode updates, but does that
> ever actually *happen*?

That thought also crossed my mind. With the mitigations sh*te, you
basically must build in microcode. Lemme cook up a dracut patch for this
tomorrow and see what happens.

> There are dracut command lines, like "--early-microcode" and
> "--no-early-microcode", so people who really want to save space could
> just force it that way. Doing the CONFIG_xyz check seems broken.

Yap, exactly.

> I guess we on the kernel side could help with "make install" etc, but
> we've (intentionally) tried to insulate us from distros having
> distro-specific installkernel scripts, so we don't really haev a good
> way to pass information down to the installkernel side.
> 
> It *would* make sense if we just had some actual arguments we might
> pass down. Right now we just do
> 
>         exec "${file}" "${KERNELRELEASE}" "${KBUILD_IMAGE}" System.map
> "${INSTALL_PATH}"
> 
> so basically the only argument we pass down is that INSTALL_PATH
> (which is just "/boot" by default).

Right, and on debian they run initramfs-tools as part of
a post-installation step at the end of /sbin/installkernel which could
then pass in more configuration info.

Yap, that could be one way to do it. We could document it in
scripts/install.sh or somewhere more prominent so that tools can look it
up.

Yap, all better ideas than parsing .config.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [regression] microcode files missing in initramfs imgages from dracut (was Re: [PATCH] x86: Clean up remaining references to CONFIG_MICROCODE_AMD)
  2023-11-12 15:03 ` [regression] microcode files missing in initramfs imgages from dracut (was Re: [PATCH] x86: Clean up remaining references to CONFIG_MICROCODE_AMD) Linux regression tracking (Thorsten Leemhuis)
  2023-11-12 18:10   ` Borislav Petkov
@ 2023-11-23  4:07   ` Linux regression tracking #update (Thorsten Leemhuis)
  1 sibling, 0 replies; 22+ messages in thread
From: Linux regression tracking #update (Thorsten Leemhuis) @ 2023-11-23  4:07 UTC (permalink / raw)
  To: lukas.bulwahn
  Cc: bp, dave.hansen, hpa, kernel-janitors, linux-kernel, mingo, tglx,
	x86, Linux kernel regressions list

[TLDR: This mail in primarily relevant for Linux kernel regression
tracking. See link in footer if these mails annoy you.]

On 12.11.23 16:03, Linux regression tracking (Thorsten Leemhuis) wrote:
> 
>> Commit e6bcfdd75d53 ("x86/microcode: Hide the config knob") removes config
>> MICROCODE_AMD, but left some references that have no effect on any kernel
>> build around.
>>
>> Clean up those remaining config references. No functional change.
>> [...]
> 
> That patch became 4d2b748305e96f ("x86/microcode: Remove remaining
> references to CONFIG_MICROCODE_AMD"). Not totally sure, but from briefly
> looking into things it seems likely that it causes a regression with
> dracut that was just reported here:
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=218136

Linus doesn't consider this to be something that needs to be fixed (see
the "from a user perspective things should still continue to work."
later in this thread), so remove this from the tracking:

#regzbot resolve: Linus considers this nothing that needs fixing
#regzbot ignore-activity

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
That page also explains what to do if mails like this annoy you.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [regression] microcode files missing in initramfs imgages from dracut (was Re: [PATCH] x86: Clean up remaining references to CONFIG_MICROCODE_AMD)
  2023-11-22 21:08                 ` Linus Torvalds
  2023-11-22 21:35                   ` Borislav Petkov
@ 2023-11-23 11:20                   ` Borislav Petkov
  2023-11-24  8:49                     ` Antonio Feijoo
  1 sibling, 1 reply; 22+ messages in thread
From: Borislav Petkov @ 2023-11-23 11:20 UTC (permalink / raw)
  To: Linus Torvalds, Antonio Alvarez Feijoo
  Cc: Linux regressions mailing list, lukas.bulwahn, dave.hansen, hpa,
	kernel-janitors, linux-kernel, mingo, tglx, x86

Adding Antonio who did that last fix to dracut:

6c80408c8644 ("fix(dracut.sh): remove microcode check based on CONFIG_MICROCODE_[AMD|INTEL]")

On Wed, Nov 22, 2023 at 01:08:41PM -0800, Linus Torvalds wrote:
> There are dracut command lines, like "--early-microcode" and
> "--no-early-microcode", so people who really want to save space could
> just force it that way. Doing the CONFIG_xyz check seems broken.
> 
> But that's for the dracut people to worry about.

Yeah, I guess something like this below.

Antonio, how about something like the totally untested thing below?

dracut would simply always build in microcode - this is the majority of
the setups anyway - and people who want to save space, do:

--no-early-microcode

?

---
diff --git a/dracut.sh b/dracut.sh
index 3b292910f324..c0a88b083f8e 100755
--- a/dracut.sh
+++ b/dracut.sh
@@ -1561,20 +1561,16 @@ fi
 
 if [[ $early_microcode == yes ]]; then
     if [[ $hostonly ]]; then
-        if [[ $(get_cpu_vendor) == "AMD" || $(get_cpu_vendor) == "Intel" ]]; then
-            check_kernel_config CONFIG_MICROCODE || unset early_microcode
-        else
+        if [[ $(get_cpu_vendor) != "AMD" && $(get_cpu_vendor) != "Intel" ]]; then
             unset early_microcode
         fi
-    else
-        ! check_kernel_config CONFIG_MICROCODE \
-            && unset early_microcode
     fi
+
     # Do not complain on non-x86 architectures as it makes no sense
     case "${DRACUT_ARCH:-$(uname -m)}" in
         x86_64 | i?86)
             [[ $early_microcode != yes ]] \
-                && dwarn "Disabling early microcode, because kernel does not support it. CONFIG_MICROCODE!=y"
+                && dwarn "Disabling early microcode, unsupported configuration"
             ;;
         *) ;;
     esac

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [regression] microcode files missing in initramfs imgages from dracut (was Re: [PATCH] x86: Clean up remaining references to CONFIG_MICROCODE_AMD)
  2023-11-23 11:20                   ` Borislav Petkov
@ 2023-11-24  8:49                     ` Antonio Feijoo
  2023-11-24 12:15                       ` Borislav Petkov
  0 siblings, 1 reply; 22+ messages in thread
From: Antonio Feijoo @ 2023-11-24  8:49 UTC (permalink / raw)
  To: Borislav Petkov, Linus Torvalds
  Cc: Linux regressions mailing list, lukas.bulwahn, dave.hansen, hpa,
	kernel-janitors, linux-kernel, mingo, tglx, x86

As a side note, complaints about this issue reached the kernel because most
distros out there didn't do their homework, as this patch has been merged
upstream since 6.6-rc1 was released. Fortunately, this problem does not break
the system boot.

As Linus said, the `check_kernel_config` stuff was implemented in 2014 and this
is not the only kernel config option that it's being checked by dracut
(CONFIG_ACPI_TABLE_UPGRADE, CONFIG_ACPI_INITRD_TABLE_OVERRIDE, CONFIG_RD_ZSTD),
although I agree that it's fragile if something changes. But adding in CC the
initramfs list (like you did), would be enough to prepare a simple fix in time.

On 23/11/2023 12.20, Borislav Petkov wrote:
> Adding Antonio who did that last fix to dracut:
> 
> 6c80408c8644 ("fix(dracut.sh): remove microcode check based on CONFIG_MICROCODE_[AMD|INTEL]")
> 
> On Wed, Nov 22, 2023 at 01:08:41PM -0800, Linus Torvalds wrote:
>> There are dracut command lines, like "--early-microcode" and
>> "--no-early-microcode", so people who really want to save space could
>> just force it that way. Doing the CONFIG_xyz check seems broken.
>>
>> But that's for the dracut people to worry about.
> 
> Yeah, I guess something like this below.
> 
> Antonio, how about something like the totally untested thing below?
> 
> dracut would simply always build in microcode - this is the majority of
> the setups anyway - and people who want to save space, do:
> 
> --no-early-microcode
> 
> ?

The only problem I see in your patch is that we should also remove the
`--early-microcode` option, and dracut will fail if someone pass an option
available since 2013 (5f2c30d9bcd614d546d5c55c6897e33f88b9ab90) that would not
be recognized now (and by failing, I mean it will not build an initramfs if an
unrecognized option is passed).

Please, submit it to https://github.com/dracutdevs/dracut, so more people can
see it and discuss it. Thank you.

> ---
> diff --git a/dracut.sh b/dracut.sh
> index 3b292910f324..c0a88b083f8e 100755
> --- a/dracut.sh
> +++ b/dracut.sh
> @@ -1561,20 +1561,16 @@ fi
>  
>  if [[ $early_microcode == yes ]]; then
>      if [[ $hostonly ]]; then
> -        if [[ $(get_cpu_vendor) == "AMD" || $(get_cpu_vendor) == "Intel" ]]; then
> -            check_kernel_config CONFIG_MICROCODE || unset early_microcode
> -        else
> +        if [[ $(get_cpu_vendor) != "AMD" && $(get_cpu_vendor) != "Intel" ]]; then
>              unset early_microcode
>          fi
> -    else
> -        ! check_kernel_config CONFIG_MICROCODE \
> -            && unset early_microcode
>      fi
> +
>      # Do not complain on non-x86 architectures as it makes no sense
>      case "${DRACUT_ARCH:-$(uname -m)}" in
>          x86_64 | i?86)
>              [[ $early_microcode != yes ]] \
> -                && dwarn "Disabling early microcode, because kernel does not support it. CONFIG_MICROCODE!=y"
> +                && dwarn "Disabling early microcode, unsupported configuration"
>              ;;
>          *) ;;
>      esac
> 
> Thx.
> 

Best regards,

-- 
Antonio Álvarez Feijoo
System Boot and Init
SUSE

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [regression] microcode files missing in initramfs imgages from dracut (was Re: [PATCH] x86: Clean up remaining references to CONFIG_MICROCODE_AMD)
  2023-11-24  8:49                     ` Antonio Feijoo
@ 2023-11-24 12:15                       ` Borislav Petkov
  2023-11-24 13:15                         ` Antonio Feijoo
  0 siblings, 1 reply; 22+ messages in thread
From: Borislav Petkov @ 2023-11-24 12:15 UTC (permalink / raw)
  To: Antonio Feijoo
  Cc: Linus Torvalds, Linux regressions mailing list, lukas.bulwahn,
	dave.hansen, hpa, kernel-janitors, linux-kernel, mingo, tglx,
	x86

On Fri, Nov 24, 2023 at 09:49:27AM +0100, Antonio Feijoo wrote:
> As Linus said, the `check_kernel_config` stuff was implemented in 2014 and this
> is not the only kernel config option that it's being checked by dracut
> (CONFIG_ACPI_TABLE_UPGRADE, CONFIG_ACPI_INITRD_TABLE_OVERRIDE, CONFIG_RD_ZSTD),
> although I agree that it's fragile if something changes. But adding in CC the
> initramfs list (like you did), would be enough to prepare a simple fix in time.

Right, how about we give you a more reliable way to check functionality
built into the kernel instead of grepping the .config file?

Like the ELF note thing, for example:

https://lore.kernel.org/r/20231122132419.GBZV4BA399sG2JRFAJ@fat_crate.local

The current thing is not ABI and will break everytime we change
something and even if you fix it on time, older dracuts will still be
broken.

> The only problem I see in your patch is that we should also remove the
> `--early-microcode` option, and dracut will fail if someone pass an option
> available since 2013 (5f2c30d9bcd614d546d5c55c6897e33f88b9ab90) that would not
> be recognized now (and by failing, I mean it will not build an initramfs if an
> unrecognized option is passed).

Ah ok, --early-microcode becomes a no-op with my change. Sure.

> Please, submit it to https://github.com/dracutdevs/dracut, so more people can
> see it and discuss it. Thank you.

I presume I should read this first:

https://github.com/dracutdevs/dracut/blob/master/docs/HACKING.md

and send a github pull request?

Anything else I need to pay attention to when sending dracut patches?

Or is there also an old school mailing list where I can send the patch
to?

:-)

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [regression] microcode files missing in initramfs imgages from dracut (was Re: [PATCH] x86: Clean up remaining references to CONFIG_MICROCODE_AMD)
  2023-11-24 12:15                       ` Borislav Petkov
@ 2023-11-24 13:15                         ` Antonio Feijoo
  2023-11-24 13:33                           ` Borislav Petkov
  0 siblings, 1 reply; 22+ messages in thread
From: Antonio Feijoo @ 2023-11-24 13:15 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Linus Torvalds, Linux regressions mailing list, lukas.bulwahn,
	dave.hansen, hpa, kernel-janitors, linux-kernel, mingo, tglx,
	x86



On 24/11/2023 13.15, Borislav Petkov wrote:
> On Fri, Nov 24, 2023 at 09:49:27AM +0100, Antonio Feijoo wrote:
>> As Linus said, the `check_kernel_config` stuff was implemented in 2014 and this
>> is not the only kernel config option that it's being checked by dracut
>> (CONFIG_ACPI_TABLE_UPGRADE, CONFIG_ACPI_INITRD_TABLE_OVERRIDE, CONFIG_RD_ZSTD),
>> although I agree that it's fragile if something changes. But adding in CC the
>> initramfs list (like you did), would be enough to prepare a simple fix in time.
> 
> Right, how about we give you a more reliable way to check functionality
> built into the kernel instead of grepping the .config file?
> 
> Like the ELF note thing, for example:
> 
> https://lore.kernel.org/r/20231122132419.GBZV4BA399sG2JRFAJ@fat_crate.local
> 
> The current thing is not ABI and will break everytime we change
> something and even if you fix it on time, older dracuts will still be
> broken.

The problem I see is having to add a new patch with a new note every time a
user space application requires new information to query. And also new dracuts
will be broken with older kernels that do not contain this info.
But (from a user space application point of view) if you (the kernel devs) are
ok with this approach, I don't see why we can at least get some info from there.

> 
>> The only problem I see in your patch is that we should also remove the
>> `--early-microcode` option, and dracut will fail if someone pass an option
>> available since 2013 (5f2c30d9bcd614d546d5c55c6897e33f88b9ab90) that would not
>> be recognized now (and by failing, I mean it will not build an initramfs if an
>> unrecognized option is passed).
> 
> Ah ok, --early-microcode becomes a no-op with my change. Sure.
> 
>> Please, submit it to https://github.com/dracutdevs/dracut, so more people can
>> see it and discuss it. Thank you.
> 
> I presume I should read this first:
> 
> https://github.com/dracutdevs/dracut/blob/master/docs/HACKING.md
> 
> and send a github pull request?

Yes, that would be enough.

> Anything else I need to pay attention to when sending dracut patches?

Just follow the Conventional Commit style for the commit messages, but that's
also specified in the HACKING.md doc.

> Or is there also an old school mailing list where I can send the patch
> to?
> 
> :-)

Unfortunately no. All the development process was moved to github.

> 
> Thx.
> 

Thank you,

Best regards.

-- 
Antonio Álvarez Feijoo
System Boot and Init
SUSE

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [regression] microcode files missing in initramfs imgages from dracut (was Re: [PATCH] x86: Clean up remaining references to CONFIG_MICROCODE_AMD)
  2023-11-24 13:15                         ` Antonio Feijoo
@ 2023-11-24 13:33                           ` Borislav Petkov
  2023-11-28 13:17                             ` Borislav Petkov
  0 siblings, 1 reply; 22+ messages in thread
From: Borislav Petkov @ 2023-11-24 13:33 UTC (permalink / raw)
  To: Antonio Feijoo
  Cc: Linus Torvalds, Linux regressions mailing list, lukas.bulwahn,
	dave.hansen, hpa, kernel-janitors, linux-kernel, mingo, tglx,
	x86

On Fri, Nov 24, 2023 at 02:15:02PM +0100, Antonio Feijoo wrote:
> The problem I see is having to add a new patch with a new note every
> time a user space application requires new information to query.

Yeah, this is how it should work. The mere addition of the note would
document that userspace uses a certain aspect of the kernel machinery
and then we will know not to break it.

> And also new dracuts will be broken with older kernels that do not
> contain this info.

Yes, the meaning of the absence of the note would be: the kernel doesn't
provide that functionality. If you want it, upgrade your kernel.

> But (from a user space application point of view) if you (the kernel
> devs) are ok with this approach, I don't see why we can at least get
> some info from there.

Yes, my goal is to have some machinery which both sides agree upon and
have that proper contract in place vs you guys using things which are
internal and us breaking them unknowingly.

But Linus isn't that crazy about the note thing so I'd prefer if he
comments on what we should do instead.

> Just follow the Conventional Commit style for the commit messages, but
> that's also specified in the HACKING.md doc.

Thx, lemme get cracking on fancy github workflows. :-P

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [regression] microcode files missing in initramfs imgages from dracut (was Re: [PATCH] x86: Clean up remaining references to CONFIG_MICROCODE_AMD)
  2023-11-24 13:33                           ` Borislav Petkov
@ 2023-11-28 13:17                             ` Borislav Petkov
  0 siblings, 0 replies; 22+ messages in thread
From: Borislav Petkov @ 2023-11-28 13:17 UTC (permalink / raw)
  To: Antonio Feijoo
  Cc: Linus Torvalds, Linux regressions mailing list, lukas.bulwahn,
	dave.hansen, hpa, kernel-janitors, linux-kernel, mingo, tglx,
	x86

On Fri, Nov 24, 2023 at 02:33:03PM +0100, Borislav Petkov wrote:
> Thx, lemme get cracking on fancy github workflows. :-P

Uff:

https://github.com/dracutdevs/dracut/pull/2572

What a pain it is to do a pull request with github. :-\

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2023-11-28 13:17 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-25 14:12 [PATCH] x86: Clean up remaining references to CONFIG_MICROCODE_AMD Lukas Bulwahn
2023-08-26 12:10 ` [tip: x86/microcode] x86/microcode: Remove " tip-bot2 for Lukas Bulwahn
2023-11-12 15:03 ` [regression] microcode files missing in initramfs imgages from dracut (was Re: [PATCH] x86: Clean up remaining references to CONFIG_MICROCODE_AMD) Linux regression tracking (Thorsten Leemhuis)
2023-11-12 18:10   ` Borislav Petkov
2023-11-22  9:15     ` Linux regression tracking (Thorsten Leemhuis)
2023-11-22 11:58       ` Borislav Petkov
2023-11-22 13:24         ` [PATCH] x86: Add a "x86" ELF note namespace Borislav Petkov
2023-11-22 14:07           ` Borislav Petkov
2023-11-22 15:34         ` [regression] microcode files missing in initramfs imgages from dracut (was Re: [PATCH] x86: Clean up remaining references to CONFIG_MICROCODE_AMD) Linux regression tracking (Thorsten Leemhuis)
2023-11-22 15:57           ` Borislav Petkov
2023-11-22 20:35             ` Linus Torvalds
2023-11-22 20:51               ` Borislav Petkov
2023-11-22 21:08                 ` Linus Torvalds
2023-11-22 21:35                   ` Borislav Petkov
2023-11-23 11:20                   ` Borislav Petkov
2023-11-24  8:49                     ` Antonio Feijoo
2023-11-24 12:15                       ` Borislav Petkov
2023-11-24 13:15                         ` Antonio Feijoo
2023-11-24 13:33                           ` Borislav Petkov
2023-11-28 13:17                             ` Borislav Petkov
2023-11-22 13:45       ` Lukas Bulwahn
2023-11-23  4:07   ` Linux regression tracking #update (Thorsten Leemhuis)

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.