2009-10-23 23:38:35

by Mike Travis

[permalink] [raw]
Subject: [PATCH 6/8] SGI x86_64 UV: Limit the number of microcode messages

Limit number of microcode messages of the form:

[ 50.887135] microcode: CPU0 sig=0x206e5, pf=0x4, revision=0xffff001

Cc: Tigran Aivazian <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: [email protected]
Cc: Dmitry Adamushko <[email protected]>
Cc: Andreas Mohr <[email protected]>
Cc: Hugh Dickins <[email protected]>
Cc: Hannes Eder <[email protected]>
Cc: [email protected]
Signed-off-by: Mike Travis <[email protected]>
---
arch/x86/kernel/microcode_intel.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

--- linux.orig/arch/x86/kernel/microcode_intel.c
+++ linux/arch/x86/kernel/microcode_intel.c
@@ -165,7 +165,9 @@
/* get the current revision from MSR 0x8B */
rdmsr(MSR_IA32_UCODE_REV, val[0], csig->rev);

- printk(KERN_INFO "microcode: CPU%d sig=0x%x, pf=0x%x, revision=0x%x\n",
+ if (cpu_num < 4 || !limit_console_output(false))
+ printk(KERN_INFO
+ "microcode: CPU%d sig=0x%x, pf=0x%x, revision=0x%x\n",
cpu_num, csig->sig, csig->pf, csig->rev);

return 0;

--


2009-10-24 20:09:40

by Dmitry Adamushko

[permalink] [raw]
Subject: Re: [PATCH 6/8] SGI x86_64 UV: Limit the number of microcode messages

2009/10/24 Mike Travis <[email protected]>:
> Limit number of microcode messages of the form:
>
> [ ? 50.887135] microcode: CPU0 sig=0x206e5, pf=0x4, revision=0xffff001
>
> [ ... ]
>
> --- linux.orig/arch/x86/kernel/microcode_intel.c
> +++ linux/arch/x86/kernel/microcode_intel.c
> @@ -165,7 +165,9 @@
> ? ? ? ?/* get the current revision from MSR 0x8B */
> ? ? ? ?rdmsr(MSR_IA32_UCODE_REV, val[0], csig->rev);
>
> - ? ? ? printk(KERN_INFO "microcode: CPU%d sig=0x%x, pf=0x%x, revision=0x%x\n",
> + ? ? ? if (cpu_num < 4 || !limit_console_output(false))
> + ? ? ? ? ? ? ? printk(KERN_INFO
> + ? ? ? ? ? ? ? ? ? ? ? "microcode: CPU%d sig=0x%x, pf=0x%x, revision=0x%x\n",
> ? ? ? ? ? ? ? ? ? ? ? ?cpu_num, csig->sig, csig->pf, csig->rev);
>

Hmm, I guess we wouldn't lose a lot by simply removing those messages
completely. Per-cpu pf/revision is available via /sys anyway.

Alternatively, we might move the output into
microcode_core.c::collect_cpu_info() (or even microcode_init_cpu()) so
that the same logic is also applied for amd and do something as
following:

don't print if a cpu info is equal to the info of CPU#0. I guess, any
non-0 cpu would be even better as the microcode for cpu#0 can be
loaded by BIOS, if I'm not mistaken. But then we can only be sure
about the presence of cpu#0.

Anyway, it's not worthy of any additional complexity so I'd say let's
just remove the output :-)


-- Dmitry

2009-10-24 22:24:15

by Tigran Aivazian

[permalink] [raw]
Subject: Re: [PATCH 6/8] SGI x86_64 UV: Limit the number of microcode messages

On Sat, 24 Oct 2009, Dmitry Adamushko wrote:
>> - ? ? ? printk(KERN_INFO "microcode: CPU%d sig=0x%x, pf=0x%x, revision=0x%x\n",
>> + ? ? ? if (cpu_num < 4 || !limit_console_output(false))
>> + ? ? ? ? ? ? ? printk(KERN_INFO
>> + ? ? ? ? ? ? ? ? ? ? ? "microcode: CPU%d sig=0x%x, pf=0x%x, revision=0x%x\n",
>> ? ? ? ? ? ? ? ? ? ? ? ?cpu_num, csig->sig, csig->pf, csig->rev);
>>
>
> Hmm, I guess we wouldn't lose a lot by simply removing those messages
> completely. Per-cpu pf/revision is available via /sys anyway.

The reason for printing them is that the pf (possibly others?) can change
by the update and so the log has this info handy.

Kind regards
Tigran

2009-10-24 22:45:26

by Dmitry Adamushko

[permalink] [raw]
Subject: Re: [PATCH 6/8] SGI x86_64 UV: Limit the number of microcode messages

2009/10/24 Tigran Aivazian <[email protected]>:
> On Sat, 24 Oct 2009, Dmitry Adamushko wrote:
>>>
>>> - ? ? ? printk(KERN_INFO "microcode: CPU%d sig=0x%x, pf=0x%x,
>>> revision=0x%x\n",
>>> + ? ? ? if (cpu_num < 4 || !limit_console_output(false))
>>> + ? ? ? ? ? ? ? printk(KERN_INFO
>>> + ? ? ? ? ? ? ? ? ? ? ? "microcode: CPU%d sig=0x%x, pf=0x%x,
>>> revision=0x%x\n",
>>> ? ? ? ? ? ? ? ? ? ? ? ?cpu_num, csig->sig, csig->pf, csig->rev);
>>>
>>
>> Hmm, I guess we wouldn't lose a lot by simply removing those messages
>> completely. Per-cpu pf/revision is available via /sys anyway.
>
> The reason for printing them is that the pf (possibly others?) can change by the update and so the log has this info handy.

We might store the old sig/pf/revision set as well, export them via
/sys or/and print them at update-to-new-microcode time.

If it's really so useful to have this info in the log and, at the same
time, to avoid the flood of messages (which, I guess for the majority
of systems, are the same) at startup time, we might delay the printout
until the end of microcode_init(). Then do something like this:

microcode cpu0: up to date version sig, pf, rev // let's say,
it was updated by BIOS
microcode cpus [1 ... 16] : update from sig, pf, rev to sig, pf2, rev2.

Anyway, my humble opinion, is that (at the very least) the current
patch should be accompanied by a similar version for amd.


>
> Kind regards
> Tigran


-- Dmitry

2009-10-25 16:37:22

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 6/8] SGI x86_64 UV: Limit the number of microcode messages


* Dmitry Adamushko <[email protected]> wrote:

> 2009/10/24 Tigran Aivazian <[email protected]>:
> > On Sat, 24 Oct 2009, Dmitry Adamushko wrote:
> >>>
> >>> - ? ? ? printk(KERN_INFO "microcode: CPU%d sig=0x%x, pf=0x%x,
> >>> revision=0x%x\n",
> >>> + ? ? ? if (cpu_num < 4 || !limit_console_output(false))
> >>> + ? ? ? ? ? ? ? printk(KERN_INFO
> >>> + ? ? ? ? ? ? ? ? ? ? ? "microcode: CPU%d sig=0x%x, pf=0x%x,
> >>> revision=0x%x\n",
> >>> ? ? ? ? ? ? ? ? ? ? ? ?cpu_num, csig->sig, csig->pf, csig->rev);
> >>>
> >>
> >> Hmm, I guess we wouldn't lose a lot by simply removing those messages
> >> completely. Per-cpu pf/revision is available via /sys anyway.
> >
> > The reason for printing them is that the pf (possibly others?) can change by the update and so the log has this info handy.
>
> We might store the old sig/pf/revision set as well, export them via
> /sys or/and print them at update-to-new-microcode time.
>
> If it's really so useful to have this info in the log and, at the same
> time, to avoid the flood of messages (which, I guess for the majority
> of systems, are the same) at startup time, we might delay the printout
> until the end of microcode_init(). Then do something like this:
>
> microcode cpu0: up to date version sig, pf, rev // let's say,
> it was updated by BIOS
> microcode cpus [1 ... 16] : update from sig, pf, rev to sig, pf2, rev2.
>
> Anyway, my humble opinion, is that (at the very least) the current
> patch should be accompanied by a similar version for amd.

yeah. Since we load new microcode on all cpus it's enough to print it
for the boot CPU or so.

Having the precise microcode version printed (or exposed somewhere in
/sys) is useful - sometimes when there's a weird crash in some prototype
CPU one of the first questions from hw vendors is 'which precise
microcode version was that?'.

Ingo

2009-10-25 17:12:20

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [PATCH 6/8] SGI x86_64 UV: Limit the number of microcode messages

On Sun, 25 Oct 2009 17:37:04 +0100
Ingo Molnar <[email protected]> wrote:

>
> Having the precise microcode version printed (or exposed somewhere in
> /sys) is useful - sometimes when there's a weird crash in some
> prototype CPU one of the first questions from hw vendors is 'which
> precise microcode version was that?'.

something like
/sys/devices/system/cpu/cpu0/microcode/version ?

(yes that is there today ;-)


--
Arjan van de Ven Intel Open Source Technology Centre
For development, discussion and tips for power savings,
visit http://www.lesswatts.org

2009-10-25 17:28:13

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 6/8] SGI x86_64 UV: Limit the number of microcode messages


* Arjan van de Ven <[email protected]> wrote:

> On Sun, 25 Oct 2009 17:37:04 +0100
> Ingo Molnar <[email protected]> wrote:
>
> >
> > Having the precise microcode version printed (or exposed somewhere in
> > /sys) is useful - sometimes when there's a weird crash in some
> > prototype CPU one of the first questions from hw vendors is 'which
> > precise microcode version was that?'.
>
> something like /sys/devices/system/cpu/cpu0/microcode/version ?
>
> (yes that is there today ;-)

yeah, i used that for a bug recently.

Nevertheless it makes sense to print the boot CPU message too - for bugs
that crash before we can read out
/sys/devices/system/cpu/cpu0/microcode/version.

Ingo

2009-10-26 07:05:17

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH 6/8] SGI x86_64 UV: Limit the number of microcode messages

Mike Travis <[email protected]> writes:

> Limit number of microcode messages of the form:
>
> [ 50.887135] microcode: CPU0 sig=0x206e5, pf=0x4, revision=0xffff001

Having a summary message that tells how many CPUs got updated at the
end would seem like the right approach here.

-Andi

--
[email protected] -- Speaking for myself only.

2009-10-26 18:18:39

by Mike Travis

[permalink] [raw]
Subject: Re: [PATCH 6/8] SGI x86_64 UV: Limit the number of microcode messages



Dmitry Adamushko wrote:
> 2009/10/24 Mike Travis <[email protected]>:
>> Limit number of microcode messages of the form:
>>
>> [ 50.887135] microcode: CPU0 sig=0x206e5, pf=0x4, revision=0xffff001
>>
>> [ ... ]
>>
>> --- linux.orig/arch/x86/kernel/microcode_intel.c
>> +++ linux/arch/x86/kernel/microcode_intel.c
>> @@ -165,7 +165,9 @@
>> /* get the current revision from MSR 0x8B */
>> rdmsr(MSR_IA32_UCODE_REV, val[0], csig->rev);
>>
>> - printk(KERN_INFO "microcode: CPU%d sig=0x%x, pf=0x%x, revision=0x%x\n",
>> + if (cpu_num < 4 || !limit_console_output(false))
>> + printk(KERN_INFO
>> + "microcode: CPU%d sig=0x%x, pf=0x%x, revision=0x%x\n",
>> cpu_num, csig->sig, csig->pf, csig->rev);
>>
>
> Hmm, I guess we wouldn't lose a lot by simply removing those messages
> completely. Per-cpu pf/revision is available via /sys anyway.
>
> Alternatively, we might move the output into
> microcode_core.c::collect_cpu_info() (or even microcode_init_cpu()) so
> that the same logic is also applied for amd and do something as
> following:
>
> don't print if a cpu info is equal to the info of CPU#0. I guess, any
> non-0 cpu would be even better as the microcode for cpu#0 can be
> loaded by BIOS, if I'm not mistaken. But then we can only be sure
> about the presence of cpu#0.
>
> Anyway, it's not worthy of any additional complexity so I'd say let's
> just remove the output :-)
>
>
> -- Dmitry

I would be more than happy to remove messages but I didn't want to override
the original author's intent on why they choose to add these messages in the
first place.

Plus if you have a 64 or 128 cpu system, it might give you pleasure in seeing
all those cpu messages. ;-) Just when it hits around 256 and up that it
really starts getting annoying.

Thanks,
Mike

2009-10-26 18:24:16

by Mike Travis

[permalink] [raw]
Subject: Re: [PATCH 6/8] SGI x86_64 UV: Limit the number of microcode messages



Tigran Aivazian wrote:
> On Sat, 24 Oct 2009, Dmitry Adamushko wrote:
>>> - printk(KERN_INFO "microcode: CPU%d sig=0x%x, pf=0x%x,
>>> revision=0x%x\n",
>>> + if (cpu_num < 4 || !limit_console_output(false))
>>> + printk(KERN_INFO
>>> + "microcode: CPU%d sig=0x%x, pf=0x%x,
>>> revision=0x%x\n",
>>> cpu_num, csig->sig, csig->pf, csig->rev);
>>>
>>
>> Hmm, I guess we wouldn't lose a lot by simply removing those messages
>> completely. Per-cpu pf/revision is available via /sys anyway.
>
> The reason for printing them is that the pf (possibly others?) can
> change by the update and so the log has this info handy.
>
> Kind regards
> Tigran

Is there any reason to need this on the console before being able to
look at them with dmesg? (Or use some filter program to hunt through
the system log?)

And if all the cpus are the same, would the printing of each one give
you any more information? I could add something that attempts to
print the new line if it's different than the previous, but this would
add complexity, maybe unnecessarily? And I was going for an approach
that optimizes to zero code when not enabled.

Thanks,
Mike

2009-10-26 18:25:29

by Mike Travis

[permalink] [raw]
Subject: Re: [PATCH 6/8] SGI x86_64 UV: Limit the number of microcode messages



Dmitry Adamushko wrote:
> 2009/10/24 Tigran Aivazian <[email protected]>:
>> On Sat, 24 Oct 2009, Dmitry Adamushko wrote:
>>>> - printk(KERN_INFO "microcode: CPU%d sig=0x%x, pf=0x%x,
>>>> revision=0x%x\n",
>>>> + if (cpu_num < 4 || !limit_console_output(false))
>>>> + printk(KERN_INFO
>>>> + "microcode: CPU%d sig=0x%x, pf=0x%x,
>>>> revision=0x%x\n",
>>>> cpu_num, csig->sig, csig->pf, csig->rev);
>>>>
>>> Hmm, I guess we wouldn't lose a lot by simply removing those messages
>>> completely. Per-cpu pf/revision is available via /sys anyway.
>> The reason for printing them is that the pf (possibly others?) can change by the update and so the log has this info handy.
>
> We might store the old sig/pf/revision set as well, export them via
> /sys or/and print them at update-to-new-microcode time.
>
> If it's really so useful to have this info in the log and, at the same
> time, to avoid the flood of messages (which, I guess for the majority
> of systems, are the same) at startup time, we might delay the printout
> until the end of microcode_init(). Then do something like this:
>
> microcode cpu0: up to date version sig, pf, rev // let's say,
> it was updated by BIOS
> microcode cpus [1 ... 16] : update from sig, pf, rev to sig, pf2, rev2.
>
> Anyway, my humble opinion, is that (at the very least) the current
> patch should be accompanied by a similar version for amd.

I could add it for AMD but I can't test it, and I'm always reluctant to
change things I can't verify.

Thanks,
Mike

>
>
>> Kind regards
>> Tigran
>
>
> -- Dmitry

2009-10-26 18:29:01

by Mike Travis

[permalink] [raw]
Subject: Re: [PATCH 6/8] SGI x86_64 UV: Limit the number of microcode messages



Ingo Molnar wrote:
> * Dmitry Adamushko <[email protected]> wrote:
>
>> 2009/10/24 Tigran Aivazian <[email protected]>:
>>> On Sat, 24 Oct 2009, Dmitry Adamushko wrote:
>>>>> - printk(KERN_INFO "microcode: CPU%d sig=0x%x, pf=0x%x,
>>>>> revision=0x%x\n",
>>>>> + if (cpu_num < 4 || !limit_console_output(false))
>>>>> + printk(KERN_INFO
>>>>> + "microcode: CPU%d sig=0x%x, pf=0x%x,
>>>>> revision=0x%x\n",
>>>>> cpu_num, csig->sig, csig->pf, csig->rev);
>>>>>
>>>> Hmm, I guess we wouldn't lose a lot by simply removing those messages
>>>> completely. Per-cpu pf/revision is available via /sys anyway.
>>> The reason for printing them is that the pf (possibly others?) can change by the update and so the log has this info handy.
>> We might store the old sig/pf/revision set as well, export them via
>> /sys or/and print them at update-to-new-microcode time.
>>
>> If it's really so useful to have this info in the log and, at the same
>> time, to avoid the flood of messages (which, I guess for the majority
>> of systems, are the same) at startup time, we might delay the printout
>> until the end of microcode_init(). Then do something like this:
>>
>> microcode cpu0: up to date version sig, pf, rev // let's say,
>> it was updated by BIOS
>> microcode cpus [1 ... 16] : update from sig, pf, rev to sig, pf2, rev2.
>>
>> Anyway, my humble opinion, is that (at the very least) the current
>> patch should be accompanied by a similar version for amd.
>
> yeah. Since we load new microcode on all cpus it's enough to print it
> for the boot CPU or so.
>
> Having the precise microcode version printed (or exposed somewhere in
> /sys) is useful - sometimes when there's a weird crash in some prototype
> CPU one of the first questions from hw vendors is 'which precise
> microcode version was that?'.
>
> Ingo

I would agree especially in the case where not all the cpus are exactly
the same. But so far, I've only seen variations of the speed of the cpus
not it's generic type, in an SSI. So the version of the microcode was
identical in all cases.

Thanks,
Mike

2009-10-26 18:29:39

by Mike Travis

[permalink] [raw]
Subject: Re: [PATCH 6/8] SGI x86_64 UV: Limit the number of microcode messages



Arjan van de Ven wrote:
> On Sun, 25 Oct 2009 17:37:04 +0100
> Ingo Molnar <[email protected]> wrote:
>
>> Having the precise microcode version printed (or exposed somewhere in
>> /sys) is useful - sometimes when there's a weird crash in some
>> prototype CPU one of the first questions from hw vendors is 'which
>> precise microcode version was that?'.
>
> something like
> /sys/devices/system/cpu/cpu0/microcode/version ?
>
> (yes that is there today ;-)
>
>

Thanks. That's good information.

Mike

2009-10-26 18:33:32

by Mike Travis

[permalink] [raw]
Subject: Re: [PATCH 6/8] SGI x86_64 UV: Limit the number of microcode messages



Ingo Molnar wrote:
> * Arjan van de Ven <[email protected]> wrote:
>
>> On Sun, 25 Oct 2009 17:37:04 +0100
>> Ingo Molnar <[email protected]> wrote:
>>
>>> Having the precise microcode version printed (or exposed somewhere in
>>> /sys) is useful - sometimes when there's a weird crash in some
>>> prototype CPU one of the first questions from hw vendors is 'which
>>> precise microcode version was that?'.
>> something like /sys/devices/system/cpu/cpu0/microcode/version ?
>>
>> (yes that is there today ;-)
>
> yeah, i used that for a bug recently.
>
> Nevertheless it makes sense to print the boot CPU message too - for bugs
> that crash before we can read out
> /sys/devices/system/cpu/cpu0/microcode/version.
>
> Ingo

I added the printout of the first few cpus. I had thought that maybe
printing the cpu info for each new socket discovered, perhaps reducing
that to each new blade or chassis as the number grew, but again it quickly
became more complex that I thought was necessary...(?)

If you get the first cpu started ok, then using a kernel debugger like
KDB, you can usually get the remaining information.

Thanks,
Mike

2009-10-26 18:34:48

by Mike Travis

[permalink] [raw]
Subject: Re: [PATCH 6/8] SGI x86_64 UV: Limit the number of microcode messages



Andi Kleen wrote:
> Mike Travis <[email protected]> writes:
>
>> Limit number of microcode messages of the form:
>>
>> [ 50.887135] microcode: CPU0 sig=0x206e5, pf=0x4, revision=0xffff001
>
> Having a summary message that tells how many CPUs got updated at the
> end would seem like the right approach here.
>
> -Andi
>

I can do that if you think it's necessary. I believe it still does
print the error information if one or more fails to update.

Thanks,
Mike

Subject: Re: [PATCH 6/8] SGI x86_64 UV: Limit the number of microcode messages

On Mon, Oct 26, 2009 at 11:25:36AM -0700, Mike Travis wrote:
> I could add it for AMD but I can't test it, and I'm always reluctant to
> change things I can't verify.

Just send it to me, I'll test it for ya :)

--
Regards/Gruss,
Boris.

Operating | Advanced Micro Devices GmbH
System | Karl-Hammerschmidt-Str. 34, 85609 Dornach b. M?nchen, Germany
Research | Gesch?ftsf?hrer: Andrew Bowd, Thomas M. McCoy, Giuliano Meroni
Center | Sitz: Dornach, Gemeinde Aschheim, Landkreis M?nchen
(OSRC) | Registergericht M?nchen, HRB Nr. 43632

2009-10-26 20:11:52

by Dmitry Adamushko

[permalink] [raw]
Subject: Re: [PATCH 6/8] SGI x86_64 UV: Limit the number of microcode messages

2009/10/26 Mike Travis <[email protected]>:
>
>
> Ingo Molnar wrote:
>>
>> * Dmitry Adamushko <[email protected]> wrote:
>>
>>> 2009/10/24 Tigran Aivazian <[email protected]>:
>>>>
>>>> On Sat, 24 Oct 2009, Dmitry Adamushko wrote:
>>>>>>
>>>>>> - ? ? ? printk(KERN_INFO "microcode: CPU%d sig=0x%x, pf=0x%x,
>>>>>> revision=0x%x\n",
>>>>>> + ? ? ? if (cpu_num < 4 || !limit_console_output(false))
>>>>>> + ? ? ? ? ? ? ? printk(KERN_INFO
>>>>>> + ? ? ? ? ? ? ? ? ? ? ? "microcode: CPU%d sig=0x%x, pf=0x%x,
>>>>>> revision=0x%x\n",
>>>>>> ? ? ? ? ? ? ? ? ? ? ? cpu_num, csig->sig, csig->pf, csig->rev);
>>>>>>
>>>>> Hmm, I guess we wouldn't lose a lot by simply removing those messages
>>>>> completely. Per-cpu pf/revision is available via /sys anyway.
>>>>
>>>> The reason for printing them is that the pf (possibly others?) can
>>>> change by the update and so the log has this info handy.
>>>
>>> We might store the old sig/pf/revision set as well, export them via
>>> /sys or/and print them at update-to-new-microcode time.
>>>
>>> If it's really so useful to have this info in the log and, at the same
>>> time, to avoid the flood of messages (which, I guess for the majority
>>> of systems, are the same) at startup time, we might delay the printout
>>> until the end of microcode_init(). Then do something like this:
>>>
>>> microcode cpu0: up to date version sig, pf, rev ? ? ? ? ?// let's say,
>>> it was updated by BIOS
>>> microcode cpus [1 ... 16] : update from sig, pf, rev to sig, pf2, rev2.
>>>
>>> Anyway, my humble opinion, is that (at the very least) the current
>>> patch should be accompanied by a similar version for amd.
>>
>> yeah. Since we load new microcode on all cpus it's enough to print it for
>> the boot CPU or so.
>>
>> Having the precise microcode version printed (or exposed somewhere in
>> /sys) is useful - sometimes when there's a weird crash in some prototype CPU
>> one of the first questions from hw vendors is 'which precise microcode
>> version was that?'.
>>
>> ? ? ? ?Ingo
>
> I would agree especially in the case where not all the cpus are exactly
> the same. ?But so far, I've only seen variations of the speed of the cpus
> not it's generic type, in an SSI. ?So the version of the microcode was
> identical in all cases.

I guess that (at least) a bootup cpu can be updated by BIOS so that it
may appear to be different.
Perhaps, cases where some 'broken' cpus have been replaced for others
with a different "revision" (but still compatible otherwise) might be
rare but possible (say, big machines with hot-pluggable cpus) ?

btw., I was thinking of having something like this:

microcode: cpus [K...L] platform-specific-format (e.g. for Intel :
sig, pf, rev)
microcode: updating...
microcode: cpus [K...L] platform-specific-format (e.g. for Intel :
sig, pf, rev)

or even just,

microcode: cpus [ K...L] updated from platform-specific-format-1 to
platform-specific-format-2


>
> Thanks,
> Mike
>


-- Dmitry

2009-10-27 15:21:01

by Mike Travis

[permalink] [raw]
Subject: Re: [PATCH 6/8] SGI x86_64 UV: Limit the number of microcode messages

[Another approach is shown below.]

Dmitry Adamushko wrote:
> 2009/10/26 Mike Travis <[email protected]>:
>>
>> Ingo Molnar wrote:
>>> * Dmitry Adamushko <[email protected]> wrote:
>>>
>>>> 2009/10/24 Tigran Aivazian <[email protected]>:
>>>>> On Sat, 24 Oct 2009, Dmitry Adamushko wrote:
>>>>>>> - printk(KERN_INFO "microcode: CPU%d sig=0x%x, pf=0x%x,
>>>>>>> revision=0x%x\n",
>>>>>>> + if (cpu_num < 4 || !limit_console_output(false))
>>>>>>> + printk(KERN_INFO
>>>>>>> + "microcode: CPU%d sig=0x%x, pf=0x%x,
>>>>>>> revision=0x%x\n",
>>>>>>> cpu_num, csig->sig, csig->pf, csig->rev);
>>>>>>>
>>>>>> Hmm, I guess we wouldn't lose a lot by simply removing those messages
>>>>>> completely. Per-cpu pf/revision is available via /sys anyway.
>>>>> The reason for printing them is that the pf (possibly others?) can
>>>>> change by the update and so the log has this info handy.
>>>> We might store the old sig/pf/revision set as well, export them via
>>>> /sys or/and print them at update-to-new-microcode time.
>>>>
>>>> If it's really so useful to have this info in the log and, at the same
>>>> time, to avoid the flood of messages (which, I guess for the majority
>>>> of systems, are the same) at startup time, we might delay the printout
>>>> until the end of microcode_init(). Then do something like this:
>>>>
>>>> microcode cpu0: up to date version sig, pf, rev // let's say,
>>>> it was updated by BIOS
>>>> microcode cpus [1 ... 16] : update from sig, pf, rev to sig, pf2, rev2.
>>>>
>>>> Anyway, my humble opinion, is that (at the very least) the current
>>>> patch should be accompanied by a similar version for amd.
>>> yeah. Since we load new microcode on all cpus it's enough to print it for
>>> the boot CPU or so.
>>>
>>> Having the precise microcode version printed (or exposed somewhere in
>>> /sys) is useful - sometimes when there's a weird crash in some prototype CPU
>>> one of the first questions from hw vendors is 'which precise microcode
>>> version was that?'.
>>>
>>> Ingo
>> I would agree especially in the case where not all the cpus are exactly
>> the same. But so far, I've only seen variations of the speed of the cpus
>> not it's generic type, in an SSI. So the version of the microcode was
>> identical in all cases.
>
> I guess that (at least) a bootup cpu can be updated by BIOS so that it
> may appear to be different.
> Perhaps, cases where some 'broken' cpus have been replaced for others
> with a different "revision" (but still compatible otherwise) might be
> rare but possible (say, big machines with hot-pluggable cpus) ?
>
> btw., I was thinking of having something like this:
>
> microcode: cpus [K...L] platform-specific-format (e.g. for Intel :
> sig, pf, rev)
> microcode: updating...
> microcode: cpus [K...L] platform-specific-format (e.g. for Intel :
> sig, pf, rev)
>
> or even just,
>
> microcode: cpus [ K...L] updated from platform-specific-format-1 to
> platform-specific-format-2
>
>
>> Thanks,
>> Mike
>>
>
>
> -- Dmitry

Here's another approach...

I wasn't sure how to trigger the printing of the summary, so I winged it.
Looking closer it would appear that perhaps adding a new "summarize"
function to the microcode_ops struct could trigger it? Note this version
builds but I haven't yet tested it on a live system.

Thanks,
Mike

SGI x86_64 UV: Limit the number of microcode messages

Limit number of microcode messages of the form:

[ 50.887135] microcode: CPU0 sig=0x206e5, pf=0x4, revision=0xffff001

Cc: Tigran Aivazian <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: [email protected]
Cc: Dmitry Adamushko <[email protected]>
Cc: Andreas Mohr <[email protected]>
Cc: Hannes Eder <[email protected]>
Cc: [email protected]
Signed-off-by: Mike Travis <[email protected]>
---
arch/x86/kernel/microcode_intel.c | 73 +++++++++++++++++++++++++++++++++++++-
1 file changed, 72 insertions(+), 1 deletion(-)

--- linux.orig/arch/x86/kernel/microcode_intel.c
+++ linux/arch/x86/kernel/microcode_intel.c
@@ -137,6 +137,52 @@

#define exttable_size(et) ((et)->count * EXT_SIGNATURE_SIZE + EXT_HEADER_SIZE)

+static struct cpu_signature *cpusigs;
+static cpumask_var_t cpusigslist;
+static int cpusigs_error;
+
+static void summarize_cpu_info(void)
+{
+ char buf[128];
+ int cpu;
+ cpumask_var_t cpulist;
+
+ if (cpusigs_error || !alloc_cpumask_var(&cpulist, GFP_KERNEL)) {
+ printk(KERN_INFO "Can't print microcode summary\n");
+ return;
+ }
+
+ while ((cpu = cpumask_first(cpusigslist)) < nr_cpu_ids) {
+ struct cpu_signature *csig = &cpusigs[cpu];
+ int ncpu = cpu;
+
+ cpumask_clear(cpulist);
+ cpumask_set_cpu(cpu, cpulist);
+
+ /* gather all cpu info with same data */
+ while ((ncpu = cpumask_next(ncpu, cpusigslist)) < nr_cpu_ids)
+ if (csig->sig == cpusigs[ncpu].sig &&
+ csig->pf == cpusigs[ncpu].pf &&
+ csig->rev == cpusigs[ncpu].rev)
+ cpumask_set_cpu(ncpu, cpulist);
+
+ cpulist_scnprintf(buf, sizeof(buf), cpulist);
+
+ printk(KERN_INFO
+ "microcode: CPU%s: sig=0x%x, pf=0x%x, revision=0x%x\n",
+ buf, csig->sig, csig->pf, csig->rev);
+
+ /* clear bits we just processed */
+ cpumask_xor(cpusigslist, cpusigslist, cpulist);
+ }
+
+ /* cleanup */
+ free_cpumask_var(cpulist);
+ free_cpumask_var(cpusigslist);
+ vfree(cpusigs);
+ cpusigs_error = 0;
+}
+
static int collect_cpu_info(int cpu_num, struct cpu_signature *csig)
{
struct cpuinfo_x86 *c = &cpu_data(cpu_num);
@@ -165,9 +211,34 @@
/* get the current revision from MSR 0x8B */
rdmsr(MSR_IA32_UCODE_REV, val[0], csig->rev);

- printk(KERN_INFO "microcode: CPU%d sig=0x%x, pf=0x%x, revision=0x%x\n",
+ if (!cpusigs && !cpusigs_error) {
+ if (!alloc_cpumask_var(&cpusigslist, GFP_KERNEL))
+ cpusigs_error = 1;
+ else {
+ cpusigs = vmalloc(sizeof(*cpusigs) * nr_cpu_ids);
+ if (!cpusigs) {
+ free_cpumask_var(cpusigslist);
+ cpusigs_error = 1;
+ }
+ }
+ }
+
+ if (cpusigs_error || cpu_num == 0)
+ printk(KERN_INFO
+ "microcode: CPU%d sig=0x%x, pf=0x%x, revision=0x%x\n",
cpu_num, csig->sig, csig->pf, csig->rev);

+ else if (!cpusigs_error) {
+ cpusigs[cpu_num].sig = csig->sig;
+ cpusigs[cpu_num].pf = csig->pf;
+ cpusigs[cpu_num].rev = csig->rev;
+ cpumask_set_cpu(cpu_num, cpusigslist);
+
+ /* (XXX Need better method for when to print summary) */
+ if (cpu_num == (num_present_cpus() - 1))
+ summarize_cpu_info();
+ }
+
return 0;
}

2009-10-30 19:40:21

by Mike Travis

[permalink] [raw]
Subject: [PATCH] x86_64: Limit the number of microcode messages

x86_64: Limit the number of microcode messages

Presented as an example of summarizing startup information,
but as others have pointed out this particular message can be
removed completely from the console log.

Summarize microcode messages to the form:

[ 8.961953] microcode: CPU0-23: sig=0x206e5, pf=0x4, revision=0xffff0016
[ 8.969494] Microcode Update Driver: v2.00 <[email protected]>, Peter Oruba

(Note: I need a better method for triggering the summary message.)

Cc: Tigran Aivazian <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: [email protected]
Cc: Dmitry Adamushko <[email protected]>
Cc: Andreas Mohr <[email protected]>
Cc: Hannes Eder <[email protected]>
Cc: [email protected]
Signed-off-by: Mike Travis <[email protected]>
---
arch/x86/kernel/microcode_intel.c | 75 +++++++++++++++++++++++++++++++++++++-
1 file changed, 74 insertions(+), 1 deletion(-)

--- linux.orig/arch/x86/kernel/microcode_intel.c
+++ linux/arch/x86/kernel/microcode_intel.c
@@ -137,6 +137,52 @@

#define exttable_size(et) ((et)->count * EXT_SIGNATURE_SIZE + EXT_HEADER_SIZE)

+static struct cpu_signature *cpusigs;
+static cpumask_var_t cpusigslist;
+static int cpusigs_error;
+
+static void summarize_cpu_info(void)
+{
+ char buf[128];
+ int cpu;
+ cpumask_var_t cpulist;
+
+ if (cpusigs_error || !alloc_cpumask_var(&cpulist, GFP_KERNEL)) {
+ printk(KERN_INFO "Can't print microcode summary\n");
+ return;
+ }
+
+ while ((cpu = cpumask_first(cpusigslist)) < nr_cpu_ids) {
+ struct cpu_signature *csig = &cpusigs[cpu];
+ int ncpu = cpu;
+
+ cpumask_clear(cpulist);
+ cpumask_set_cpu(cpu, cpulist);
+
+ /* gather all cpu info with same data */
+ while ((ncpu = cpumask_next(ncpu, cpusigslist)) < nr_cpu_ids)
+ if (csig->sig == cpusigs[ncpu].sig &&
+ csig->pf == cpusigs[ncpu].pf &&
+ csig->rev == cpusigs[ncpu].rev)
+ cpumask_set_cpu(ncpu, cpulist);
+
+ cpulist_scnprintf(buf, sizeof(buf), cpulist);
+
+ printk(KERN_INFO
+ "microcode: CPU%s: sig=0x%x, pf=0x%x, revision=0x%x\n",
+ buf, csig->sig, csig->pf, csig->rev);
+
+ /* clear bits we just processed */
+ cpumask_xor(cpusigslist, cpusigslist, cpulist);
+ }
+
+ /* cleanup */
+ free_cpumask_var(cpulist);
+ free_cpumask_var(cpusigslist);
+ kfree(cpusigs);
+ cpusigs_error = 0;
+}
+
static int collect_cpu_info(int cpu_num, struct cpu_signature *csig)
{
struct cpuinfo_x86 *c = &cpu_data(cpu_num);
@@ -165,9 +211,36 @@
/* get the current revision from MSR 0x8B */
rdmsr(MSR_IA32_UCODE_REV, val[0], csig->rev);

- printk(KERN_INFO "microcode: CPU%d sig=0x%x, pf=0x%x, revision=0x%x\n",
+ if (!cpusigs && !cpusigs_error) {
+ if (!alloc_cpumask_var(&cpusigslist, GFP_KERNEL))
+ cpusigs_error = 1;
+ else {
+ int size = sizeof(*cpusigs) * nr_cpu_ids;
+ cpusigs = kmalloc(size, GFP_KERNEL);
+ if (!cpusigs) {
+ free_cpumask_var(cpusigslist);
+ cpusigs_error = 1;
+ }
+ }
+ }
+
+ /* will only print microcode revision of first cpu if cannot do all */
+ if (cpusigs_error && cpu_num == 0)
+ printk(KERN_INFO
+ "microcode: CPU%d sig=0x%x, pf=0x%x, revision=0x%x\n",
cpu_num, csig->sig, csig->pf, csig->rev);

+ else if (!cpusigs_error) {
+ cpusigs[cpu_num].sig = csig->sig;
+ cpusigs[cpu_num].pf = csig->pf;
+ cpusigs[cpu_num].rev = csig->rev;
+ cpumask_set_cpu(cpu_num, cpusigslist);
+
+ /* (XXX Need better method for when to print summary) */
+ if (cpu_num == (num_present_cpus() - 1))
+ summarize_cpu_info();
+ }
+
return 0;
}