Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1311553 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [RFC PATCH] x86/kconfig: Sanity-check config file during  oldconfig Thomas Voegtle <tv@lio96.de> - 2016-01-18 14:40 +0100
    Re: [RFC PATCH] x86/kconfig: Sanity-check config file during  oldconfig Borislav Petkov <bp@suse.de> - 2016-01-18 15:10 +0100
      Re: [RFC PATCH] x86/kconfig: Sanity-check config file during oldconfig Måns Rullgård <mans@mansr.com> - 2016-01-18 15:20 +0100
        Re: [RFC PATCH] x86/kconfig: Sanity-check config file during  oldconfig Borislav Petkov <bp@suse.de> - 2016-01-18 15:30 +0100
        Re: [RFC PATCH] x86/kconfig: Sanity-check config file during  oldconfig Borislav Petkov <bp@suse.de> - 2016-01-18 15:50 +0100
          Re: [RFC PATCH] x86/kconfig: Sanity-check config file during oldconfig Måns Rullgård <mans@mansr.com> - 2016-01-18 16:00 +0100
            Re: [RFC PATCH] x86/kconfig: Sanity-check config file during oldconfig Måns Rullgård <mans@mansr.com> - 2016-01-18 16:50 +0100
            Re: [RFC PATCH] x86/kconfig: Sanity-check config file during  oldconfig Borislav Petkov <bp@suse.de> - 2016-01-18 16:50 +0100

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

[Multipart message — attachments visible in raw view] — view raw

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

[toc] | [next] | [standalone]

#1311572

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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

#1311579

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

[toc] | [prev] | [next] | [standalone]

#1311592

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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

#1311626

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web