[PATCH] perf/x86: fix microcode revision check for SNB-PEBS

[PATCH] perf/x86: fix microcode revision check for SNB-PEBS

[Stephane Eranian]

The following patch, relative to 3.6.0-rc3, makes
the microcode update code path actually invoke the
perf_check_microcode() function and thus potentially
renabling SNB PEBS.

By default, CONFIG_MICROCODE_OLD_INTERFACE is
forced to Y in arch/x86/Kconfig. There is no
way to disable this. That means that the code
path used in arch/x86/kernel/microcode_core.c
did not include the call to perf_check_microcode().

Thus, even though the microcode was updated to a
version that fixes the SNB PEBS problem, perf_event
would still return EOPNOTSUPP when enabling precise
sampling.

This patch simply adds a call to perf_check_microcode()
in the call path used when OLD_INTERFACE=y.

Signed-off-by: Stephane Eranian <eranian@google.com>
---

--- a/arch/x86/kernel/microcode_core.c
+++ b/arch/x86/kernel/microcode_core.c
@@ -225,6 +225,9 @@ static ssize_t microcode_write(struct file *file, const char __user *buf,
 	if (do_microcode_update(buf, len) == 0)
 		ret = (ssize_t)len;
 
+	if (ret > 0)
+		perf_check_microcode();
+
 	mutex_unlock(&microcode_mutex);
 	put_online_cpus();
 

Re: [PATCH] perf/x86: fix microcode revision check for SNB-PEBS

[Borislav Petkov]

On Fri, Aug 24, 2012 at 03:34:34PM +0200, Stephane Eranian wrote:
> 
> The following patch, relative to 3.6.0-rc3, makes
> the microcode update code path actually invoke the
> perf_check_microcode() function and thus potentially
> renabling SNB PEBS.
> 
> By default, CONFIG_MICROCODE_OLD_INTERFACE is
> forced to Y in arch/x86/Kconfig. There is no
> way to disable this. That means that the code
> path used in arch/x86/kernel/microcode_core.c
> did not include the call to perf_check_microcode().
> 
> Thus, even though the microcode was updated to a
> version that fixes the SNB PEBS problem, perf_event
> would still return EOPNOTSUPP when enabling precise
> sampling.
> 
> This patch simply adds a call to perf_check_microcode()
> in the call path used when OLD_INTERFACE=y.

Ok, so c93dc84cbe324 added calls to perf_check_microcode but it looks
like you're updating the microcode from /dev/cpu/microcode, correct?

And if so, the old interface got missed.

Oh well, as long as we have to support it, we might as well add that
perf call there - it will go when the interface goes anyway so until
then:

Acked-by: Borislav Petkov <borislav.petkov@amd.com>

>
> Signed-off-by: Stephane Eranian <eranian@google.com>
> ---
> 
> --- a/arch/x86/kernel/microcode_core.c
> +++ b/arch/x86/kernel/microcode_core.c
> @@ -225,6 +225,9 @@ static ssize_t microcode_write(struct file *file, const char __user *buf,
>  	if (do_microcode_update(buf, len) == 0)
>  		ret = (ssize_t)len;
>  
> +	if (ret > 0)
> +		perf_check_microcode();
> +
>  	mutex_unlock(&microcode_mutex);
>  	put_online_cpus();
>  
> 

Thanks.

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

Re: [PATCH] perf/x86: fix microcode revision check for SNB-PEBS

[Stephane Eranian]

On Fri, Aug 24, 2012 at 6:08 PM, Borislav Petkov <bp@amd64.org> wrote:
> On Fri, Aug 24, 2012 at 03:34:34PM +0200, Stephane Eranian wrote:
>>
>> The following patch, relative to 3.6.0-rc3, makes
>> the microcode update code path actually invoke the
>> perf_check_microcode() function and thus potentially
>> renabling SNB PEBS.
>>
>> By default, CONFIG_MICROCODE_OLD_INTERFACE is
>> forced to Y in arch/x86/Kconfig. There is no
>> way to disable this. That means that the code
>> path used in arch/x86/kernel/microcode_core.c
>> did not include the call to perf_check_microcode().
>>
>> Thus, even though the microcode was updated to a
>> version that fixes the SNB PEBS problem, perf_event
>> would still return EOPNOTSUPP when enabling precise
>> sampling.
>>
>> This patch simply adds a call to perf_check_microcode()
>> in the call path used when OLD_INTERFACE=y.
>
> Ok, so c93dc84cbe324 added calls to perf_check_microcode but it looks
> like you're updating the microcode from /dev/cpu/microcode, correct?
>
> And if so, the old interface got missed.
>

So you're saying there is a cmdline tool that can use another interface
to pass the ucode to the kernel. I am using the microcode_ctl on Ubuntu
11.04. Apparently that one is still using the old interface. But I may
not be the
only one in this situation then, so I think it's worth fixing now.

What's the tool to update the ucode using the "new" interface then?

Thanks.

> Oh well, as long as we have to support it, we might as well add that
> perf call there - it will go when the interface goes anyway so until
> then:
>
> Acked-by: Borislav Petkov <borislav.petkov@amd.com>
>
>>
>> Signed-off-by: Stephane Eranian <eranian@google.com>
>> ---
>>
>> --- a/arch/x86/kernel/microcode_core.c
>> +++ b/arch/x86/kernel/microcode_core.c
>> @@ -225,6 +225,9 @@ static ssize_t microcode_write(struct file *file, const char __user *buf,
>>       if (do_microcode_update(buf, len) == 0)
>>               ret = (ssize_t)len;
>>
>> +     if (ret > 0)
>> +             perf_check_microcode();
>> +
>>       mutex_unlock(&microcode_mutex);
>>       put_online_cpus();
>>
>>
>
> Thanks.
>
> --
> Regards/Gruss,
> Boris.
>
> Advanced Micro Devices GmbH
> Einsteinring 24, 85609 Dornach
> GM: Alberto Bozzo
> Reg: Dornach, Landkreis Muenchen
> HRB Nr. 43632 WEEE Registernr: 129 19551

Re: [PATCH] perf/x86: fix microcode revision check for SNB-PEBS

[Borislav Petkov]

On Fri, Aug 24, 2012 at 06:14:51PM +0200, Stephane Eranian wrote:
> So you're saying there is a cmdline tool that can use another
> interface to pass the ucode to the kernel. I am using the
> microcode_ctl on Ubuntu 11.04. Apparently that one is still using the
> old interface. But I may not be the only one in this situation then,
> so I think it's worth fixing now.
>
> What's the tool to update the ucode using the "new" interface then?

Well,

last time we talked about it:
http://marc.info/?l=linux-kernel&m=134012203712243 I was advocating the
following way:

Put ucode image into /lib/firmware/amd-ucode or /lib/firmware/intel-ucode and then do

$ echo 1 > /sys/devices/system/cpu/microcode/reload

which I think is the easiest but hpa had some reservations about it and
wanted to keep the old method too.

But looking at the code, this *actually* should work on Intel now too.

So, I'd say you can use both on Intel, it depends on them whatever they
decide to do in the future.

On AMD we have only the "echo 1" thing.

Alternatively, "rmmod microcode; modprobe microcode" works too but some
people wouldn't want to reload modules in order to update ucode.

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

[tip:perf/urgent] perf/x86: Fix microcode revision check for SNB-PEBS

[tip-bot for Stephane Eranian]

Commit-ID:  e3e45c01ae690e65f2650e5288b9af802e95a136
Gitweb:     http://git.kernel.org/tip/e3e45c01ae690e65f2650e5288b9af802e95a136
Author:     Stephane Eranian <eranian@google.com>
AuthorDate: Fri, 24 Aug 2012 15:34:34 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 27 Aug 2012 08:48:19 +0200

perf/x86: Fix microcode revision check for SNB-PEBS

The following patch makes the microcode update code path
actually invoke the perf_check_microcode() function and
thus potentially renabling SNB PEBS.

By default, CONFIG_MICROCODE_OLD_INTERFACE is
forced to Y in arch/x86/Kconfig. There is no
way to disable this. That means that the code
path used in arch/x86/kernel/microcode_core.c
did not include the call to perf_check_microcode().

Thus, even though the microcode was updated to a
version that fixes the SNB PEBS problem, perf_event
would still return EOPNOTSUPP when enabling precise
sampling.

This patch simply adds a call to perf_check_microcode()
in the call path used when OLD_INTERFACE=y.

Signed-off-by: Stephane Eranian <eranian@google.com>
Acked-by: Borislav Petkov <borislav.petkov@amd.com>
Cc: peterz@infradead.org
Cc: andi@firstfloor.org
Link: http://lkml.kernel.org/r/20120824133434.GA8014@quad
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/microcode_core.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/microcode_core.c b/arch/x86/kernel/microcode_core.c
index 4873e62..9e5bcf1 100644
--- a/arch/x86/kernel/microcode_core.c
+++ b/arch/x86/kernel/microcode_core.c
@@ -225,6 +225,9 @@ static ssize_t microcode_write(struct file *file, const char __user *buf,
 	if (do_microcode_update(buf, len) == 0)
 		ret = (ssize_t)len;
 
+	if (ret > 0)
+		perf_check_microcode();
+
 	mutex_unlock(&microcode_mutex);
 	put_online_cpus();