x86, microcode: Conversion from sysdev class caused regression
Hi,
commit 8a25a2fd126c621f44f3aeaef80d51f00fc11639 (cpu: convert 'cpu' and 'machinecheck' sysdev_class to a regular subsystem) causes a regression in the CPU microcode loader.
Previous behaviour was that module loading failed if no appropriate CPU was found. Now we get
[ 131.717212] WARNING: at fs/sysfs/group.c:138 mc_device_remove+0x5f/0x70 [microcode]()
[ 131.717218] Hardware name: 01972NG
[ 131.717223] sysfs group ffffffffa00013d0 not found for kobject 'cpu0'
[ 131.717228] Modules linked in: snd_hda_codec_hdmi snd_hda_codec_conexant snd_hda_intel
btusb snd_hda_codec bluetooth thinkpad_acpi rfkill microcode(-) [last unloaded: cfg80211]
[ 131.717254] Pid: 4560, comm: modprobe Not tainted 3.4.0-rc2-00002-g258f742 #5
[ 131.717260] Call Trace:
[ 131.717277] [<ffffffff8103113b>] ? warn_slowpath_common+0x7b/0xc0
[ 131.717287] [<ffffffff81031235>] ? warn_slowpath_fmt+0x45/0x50
[ 131.717300] [<ffffffff81120e74>] ? sysfs_remove_group+0x34/0x120
[ 131.717311] [<ffffffffa00000ef>] ? mc_device_remove+0x5f/0x70 [microcode]
[ 131.717325] [<ffffffff81331eb9>] ? subsys_interface_unregister+0x69/0xa0
[ 131.717336] [<ffffffff81563526>] ? mutex_lock+0x16/0x40
[ 131.717346] [<ffffffffa0000c3e>] ? microcode_exit+0x50/0x92 [microcode]
[ 131.717357] [<ffffffff8107051d>] ? sys_delete_module+0x16d/0x260
[ 131.717369] [<ffffffff810a0065>] ? wait_iff_congested+0x45/0x110
[ 131.717379] [<ffffffff815656af>] ? page_fault+0x1f/0x30
[ 131.717389] [<ffffffff81565ba2>] ? system_call_fastpath+0x16/0x1bafter loading the driver on an unsupported CPU and the subsequent attempt to unload the driver.
The reason for the error is that subsys_interface_register() doesn't handle the return value of sif->add_dev (and there's also no unwinding of the interface registration). Instead subsys_interface_register always returns 0. This is different to what we had for sysdev_driver_register(). The latter properly handled return codes of drv->add() functions.
See Boris' commit f4203e3032e5ae74c3e89df85a5a6d96022d0c49 (sysdev: Do not register with sysdev when erroring on add).
I don't know whether the microcode driver is the only driver that is affected.
What's your suggestion for a fix? Should subsys_interface_register() be fixed or should each affected driver be adapted?
Regards,
Andreas
Quoted 4 lines · Andreas Herrmann
The reason for the error is that
subsys_interface_register()doesn't handle the return value ofsif->add_dev(and there's also no unwinding of the interface registration). Insteadsubsys_interface_registeralways returns 0.
Which is the intended behaviour of 'subsystem interfaces' from the driver-core perspective. It should not matter if one of a bunch of devices do not 'like' this 'interface'. It is the same model as a 'driver', we do not cancel the link-in of a driver if one device does not like the driver.
Quoted 3 lines
This is different to what we had for
sysdev_driver_register(). The latter properly handled return codes ofdrv->add()functions.
A 'subsystem interface' is a global instance per subsystem, not something _all_ devices must agree on, to be registered. We can load almost all drivers in the kernel, without even having a device around; the 'interfaces' should not be that different in their model.
Quoted 2 lines
See Boris' commit
f4203e3032e5ae74c3e89df85a5a6d96022d0c49(sysdev: Do not register with sysdev when erroring on add).
It seems to paper over a bug of the microcode driver, we should not do that, or make such assumptions.
Quoted 2 lines
I don't know whether the microcode driver is the only driver that is affected.
All interfaces with the same assumption of the microcode driver might, but see below.
Quoted 2 lines
What's your suggestion for a fix? Should
subsys_interface_register()be fixed or should each affected driver be adapted?
All the drivers, if there are any more, who are as naive as the microcode driver. :)
"Subsystem interfaces' users must not assume that they can blindly unwind their own stuff, which they have never registered. mc_device_remove() just goes ahead without any check and calls sysfs_remove_group(), while it should check its own state of this specific CPU it acts on.
I think a quick return in the microcode driver, for a device which has no active interface state is the best solution here.
Thanks,
Kay
+ Dave.
Quoted 10 lines · Kay Sievers
Quoted 4 lines
The reason for the error is that
subsys_interface_register()doesn't handle the return value ofsif->add_dev(and there's also no unwinding of the interface registration). Insteadsubsys_interface_registeralways returns 0.Which is the intended behaviour of 'subsystem interfaces' from the driver-core perspective. It should not matter if one of a bunch of devices do not 'like' this 'interface'. It is the same model as a 'driver', we do not cancel the link-in of a driver if one device does not like the driver.
But you're not looking at the return value of sif->add_dev which looks strange to me. Let me put it this way: why do you have return values to ->add_dev's interface then if you're not going to look at them?
A warning that one of the dev_add's failed could probably make sense here...
[..]
Quoted 2 lines
I think a quick return in the microcode driver, for a device which has no active interface state is the best solution here.
Actually, it is even easier: the code clumsily does:
sysfs_create_groupmicrocode_init_cpu
if (err)
sysfs_remove_group
so we go and create sysfs group, THEN check whether this CPU is supported and if not, remove the group again which is a bunch of crap if you ask me. The right way to go should be:
microcode_init_cpu
if (err)
return;
sysfs_create_group
and then there's no need to do all that sysfs group dancing. Andreas, let me know if you wanna do it, or I should take care of it.
Thanks.
Signature · 8 lines
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
Quoted 29 lines · Andreas Herrmann
Hi,
commit
8a25a2fd126c621f44f3aeaef80d51f00fc11639(cpu: convert 'cpu' and 'machinecheck'sysdev_classto a regular subsystem) causes a regression in the CPU microcode loader.Previous behaviour was that module loading failed if no appropriate CPU was found. Now we get
[ 131.717212] WARNING: at fs/sysfs/group.c:138 mc_device_remove+0x5f/0x70 [microcode]() [ 131.717218] Hardware name: 01972NG [ 131.717223] sysfs group ffffffffa00013d0 not found for kobject 'cpu0' [ 131.717228] Modules linked in: snd_hda_codec_hdmi snd_hda_codec_conexant snd_hda_intel btusb snd_hda_codec bluetooth thinkpad_acpi rfkill microcode(-) [last unloaded: cfg80211] [ 131.717254] Pid: 4560, comm: modprobe Not tainted 3.4.0-rc2-00002-g258f742 #5 [ 131.717260] Call Trace: [ 131.717277] [<ffffffff8103113b>] ? warn_slowpath_common+0x7b/0xc0 [ 131.717287] [<ffffffff81031235>] ? warn_slowpath_fmt+0x45/0x50 [ 131.717300] [<ffffffff81120e74>] ? sysfs_remove_group+0x34/0x120 [ 131.717311] [<ffffffffa00000ef>] ? mc_device_remove+0x5f/0x70 [microcode] [ 131.717325] [<ffffffff81331eb9>] ? subsys_interface_unregister+0x69/0xa0 [ 131.717336] [<ffffffff81563526>] ? mutex_lock+0x16/0x40 [ 131.717346] [<ffffffffa0000c3e>] ? microcode_exit+0x50/0x92 [microcode] [ 131.717357] [<ffffffff8107051d>] ? sys_delete_module+0x16d/0x260 [ 131.717369] [<ffffffff810a0065>] ? wait_iff_congested+0x45/0x110 [ 131.717379] [<ffffffff815656af>] ? page_fault+0x1f/0x30 [ 131.717389] [<ffffffff81565ba2>] ? system_call_fastpath+0x16/0x1b
Quoted 2 lines
after loading the driver on an unsupported CPU and the subsequent attempt to unload the driver.
That's not nice, but why is the driver getting loaded for a CPU that it doesn't control? Startup scripts?
It's just a "warning", everything else still works properly, right?
Patches to fix this up are always gladly accepted :)
thanks,
greg k-h
Quoted 4 lines · Greg Kroah-Hartman
That's not nice, but why is the driver getting loaded for a CPU that it doesn't control? Startup scripts?
It's just a "warning", everything else still works properly, right?
Yes.
Quoted 1 line
Patches to fix this up are always gladly accepted :)
Yeah, we'll send you a fix probably tomorrow.
Signature · 8 lines
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
Quoted 1 line · Borislav Petkov
Yeah, we'll send you a fix probably tomorrow.
Ok, we ended up doing two patches, I'm sending them as a reply to this note. Will send them to Ingo if there are no complaints.
Thanks.
Signature · 8 lines
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
PATCH 1/2 x86, microcode: Fix sysfs warning during module unload
From: Andreas Herrmann <[email protected]>
Date: Thu, 12 Apr 2012 16:48:01 +0200
Subject: [PATCH 1/2] x86, microcode: Fix sysfs warning during module unload on unsupported CPUs
Loading microcode driver on an unsupported CPU and subsequently unloading the driver causes
WARNING: at fs/sysfs/group.c:138 mc_device_remove+0x5f/0x70 [microcode]() Hardware name: 01972NG sysfs group ffffffffa00013d0 not found for kobject 'cpu0' Modules linked in: snd_hda_codec_hdmi snd_hda_codec_conexant snd_hda_intel btusb snd_hda_codec bluetooth thinkpad_acpi rfkill microcode(-) [last unloaded: cfg80211] Pid: 4560, comm: modprobe Not tainted 3.4.0-rc2-00002-g258f742 #5 Call Trace: [<ffffffff8103113b>] ? warn_slowpath_common+0x7b/0xc0 [<ffffffff81031235>] ? warn_slowpath_fmt+0x45/0x50 [<ffffffff81120e74>] ? sysfs_remove_group+0x34/0x120 [<ffffffffa00000ef>] ? mc_device_remove+0x5f/0x70 [microcode] [<ffffffff81331eb9>] ? subsys_interface_unregister+0x69/0xa0 [<ffffffff81563526>] ? mutex_lock+0x16/0x40 [<ffffffffa0000c3e>] ? microcode_exit+0x50/0x92 [microcode] [<ffffffff8107051d>] ? sys_delete_module+0x16d/0x260 [<ffffffff810a0065>] ? wait_iff_congested+0x45/0x110 [<ffffffff815656af>] ? page_fault+0x1f/0x30 [<ffffffff81565ba2>] ? system_call_fastpath+0x16/0x1b
on recent kernels.
This is due to
commit 8a25a2fd126c621f44f3aeaef80d51f00fc11639
(cpu: convert 'cpu' and 'machinecheck' sysdev_class to a regular subsystem)
which renders
commit 6c53cbfced048c421e4f72cb2183465f68fbc5e7
(x86, microcode: Correct sysdev_add error path)
useless.
See http://marc.info/?l=linux-kernel&m=133416246406478
Avoid above warning by restoring the old driver behaviour before commit 6c53cbfced048c421e4f72cb2183465f68fbc5e7 (x86, microcode:
Correct sysdev_add error path)
--- arch/x86/kernel/microcode_core.c | 4 +--- 1 files changed, 1 insertions(+), 3 deletions(-)
arch/x86/kernel/microcode_core.c +1 -3
@@ -419,10 +419,8 @@ static int mc_device_add(struct device *dev, struct subsys_interface *sif)if (err)return err; -if (microcode_init_cpu (cpu) ==UCODE_ERROR ) { -sysfs_remove_group (&dev->kobj, &mc_attr_group); +if (microcode_init_cpu (cpu) ==UCODE_ERROR )return -EINVAL ; - }return err; }
Signature · 11 lines
1.7.8.4 -- 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 1/2] x86, microcode: Fix sysfs warning during module unload
Quoted 42 lines · Borislav Petkov
From: Andreas Herrmann <[email protected]>
Date: Thu, 12 Apr 2012 16:48:01 +0200
Subject: [PATCH 1/2] x86, microcode: Fix sysfs warning during module unload on unsupported CPUsLoading microcode driver on an unsupported CPU and subsequently unloading the driver causes
WARNING: at fs/sysfs/group.c:138 mc_device_remove+0x5f/0x70 [microcode]() Hardware name: 01972NG sysfs group ffffffffa00013d0 not found for kobject 'cpu0' Modules linked in: snd_hda_codec_hdmi snd_hda_codec_conexant snd_hda_intel btusb snd_hda_codec bluetooth thinkpad_acpi rfkill microcode(-) [last unloaded: cfg80211] Pid: 4560, comm: modprobe Not tainted 3.4.0-rc2-00002-g258f742 #5 Call Trace: [<ffffffff8103113b>] ? warn_slowpath_common+0x7b/0xc0 [<ffffffff81031235>] ? warn_slowpath_fmt+0x45/0x50 [<ffffffff81120e74>] ? sysfs_remove_group+0x34/0x120 [<ffffffffa00000ef>] ? mc_device_remove+0x5f/0x70 [microcode] [<ffffffff81331eb9>] ? subsys_interface_unregister+0x69/0xa0 [<ffffffff81563526>] ? mutex_lock+0x16/0x40 [<ffffffffa0000c3e>] ? microcode_exit+0x50/0x92 [microcode] [<ffffffff8107051d>] ? sys_delete_module+0x16d/0x260 [<ffffffff810a0065>] ? wait_iff_congested+0x45/0x110 [<ffffffff815656af>] ? page_fault+0x1f/0x30 [<ffffffff81565ba2>] ? system_call_fastpath+0x16/0x1bon recent kernels.
This is due to
commit
8a25a2fd126c621f44f3aeaef80d51f00fc11639
(cpu: convert 'cpu' and 'machinecheck'sysdev_classto a regular subsystem)which renders
commit
6c53cbfced048c421e4f72cb2183465f68fbc5e7
(x86, microcode: Correctsysdev_adderror path)useless.
Quoted 8 lines
Avoid above warning by restoring the old driver behaviour before commit
6c53cbfced048c421e4f72cb2183465f68fbc5e7(x86, microcode:
Correctsysdev_adderror path)Cc Tigran Aivazian <[email protected]>Cc Borislav Petkov <[email protected]>Signed-off-by Andreas Herrmann <[email protected]>
PATCH 2/2 x86, microcode: Ensure that module is only loaded for
In reply to Borislav PetkovFrom: Andreas Herrmann <[email protected]>
Date: Thu, 12 Apr 2012 16:51:57 +0200
Subject: [PATCH 2/2] x86, microcode: Ensure that module is only loaded for supported AMD CPUs
Exit early when there's no support for a particular CPU family. Also, fixup the "no support for this CPU vendor" to be issued only when the driver is attempted to be loaded on an unsupported vendor.
--- arch/x86/kernel/microcode_amd.c | 12 +++++++----- arch/x86/kernel/microcode_core.c | 6 +++--- 2 files changed, 10 insertions(+), 8 deletions(-)
arch/x86/kernel/microcode_amd.c +7 -5
@@ -82,11 +82,6 @@ static int collect_cpu_info_amd(int cpu, struct cpu_signature *csig) {struct cpuinfo_x86 *c = &cpu_data (cpu); -if (c->x86_vendor !=X86_VENDOR_AMD || c->x86 <0x10 ) { -pr_warning ("CPU%d: family %d not supported\n" , cpu, c->x86); -return -1 ; - } - csig->rev = c->microcode;pr_info ("CPU%d: patch_level=0x%08x\n" , cpu, csig->rev);
@@ -380,6 +375,13 @@ static struct microcode_ops microcode_amd_ops = {struct microcode_ops *__init init_amd_microcode (void ) { +struct cpuinfo_x86 *c = &cpu_data (0 ); + +if (c->x86_vendor !=X86_VENDOR_AMD || c->x86 <0x10 ) { +pr_warning ("AMD CPU family 0x%x not supported\n" , c->x86); +return NULL ; + } + patch = (void *)get_zeroed_page (GFP_KERNEL );if (!patch)return NULL ;
arch/x86/kernel/microcode_core.c +3 -3
@@ -526,11 +526,11 @@ static int __init microcode_init(void) microcode_ops =init_intel_microcode (); else if (c->x86_vendor == X86_VENDOR_AMD) microcode_ops =init_amd_microcode (); - -if (!microcode_ops) { + else pr_err("no support for this CPU vendor\n"); + +if (!microcode_ops)return -ENODEV ; - } microcode_pdev =platform_device_register_simple ("microcode" ,-1 ,NULL ,0 );
Signature · 11 lines
1.7.8.4 -- 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 2/2] x86, microcode: Ensure that module is only loaded for
Quoted 8 lines · Borislav Petkov
From: Andreas Herrmann <[email protected]>
Date: Thu, 12 Apr 2012 16:51:57 +0200
Subject: [PATCH 2/2] x86, microcode: Ensure that module is only loaded for supported AMD CPUsExit early when there's no support for a particular CPU family. Also, fixup the "no support for this CPU vendor" to be issued only when the driver is attempted to be loaded on an unsupported vendor.
Quoted 5 lines
Cc Tigran Aivazian <[email protected]>Cc Borislav Petkov <[email protected]>[Boris: added a commit msg because Andreas is lazy]Signed-off-by Andreas Herrmann <[email protected]>
Re: [PATCH 2/2] x86, microcode: Ensure that module is only loaded for
In reply to Borislav PetkovQuoted 8 lines · Borislav Petkov
From: Andreas Herrmann <[email protected]>
Date: Thu, 12 Apr 2012 16:51:57 +0200
Subject: [PATCH 2/2] x86, microcode: Ensure that module is only loaded for supported AMD CPUsExit early when there's no support for a particular CPU family. Also, fixup the "no support for this CPU vendor" to be issued only when the driver is attempted to be loaded on an unsupported vendor.
Quoted 6 lines
Cc Tigran Aivazian <[email protected]>Cc Borislav Petkov <[email protected]>[Boris: added a commit msg because Andreas is lazy]Signed-off-by Andreas Herrmann <[email protected]>---
I guess a similar change in intel microcode driver would also be worthwhile, right? If so, then the below patch might be useful:
----------------->
From: Srivatsa S. Bhat <[email protected]>
Subject: x86, microcode: Ensure that module is only loaded on supported Intel CPUs
Exit early when there's no support for a particular CPU family.
---
arch/x86/kernel/microcode_intel.c | 14 ++++++++------ 1 files changed, 8 insertions(+), 6 deletions(-)
arch/x86/kernel/microcode_intel.c +8 -6
@@ -147,12 +147,6 @@ static int collect_cpu_info(int cpu_num, struct cpu_signature *csig)memset (csig,0 ,sizeof (*csig)); -if (c->x86_vendor !=X86_VENDOR_INTEL || c->x86 <6 || -cpu_has (c,X86_FEATURE_IA64 )) { -pr_err ("CPU%d not a capable Intel processor\n" , cpu_num); -return -1 ; - } - csig->sig =cpuid_eax (0x00000001 );if ((c->x86_model >=5 ) || (c->x86 >6 )) {
@@ -463,6 +457,14 @@ static struct microcode_ops microcode_intel_ops = {struct microcode_ops *__init init_intel_microcode (void ) { +struct cpuinfo_x86 *c = &cpu_data (0 ); + +if (c->x86_vendor !=X86_VENDOR_INTEL || c->x86 <6 || +cpu_has (c,X86_FEATURE_IA64 )) { +pr_err ("Intel CPU family 0x%x not supported\n" , c->x86); +return NULL ; + } +return µcode_intel_ops; }
Re: [PATCH 2/2] x86, microcode: Ensure that module is only loaded for
Quoted 2 lines · Srivatsa S. Bhat
I guess a similar change in intel microcode driver would also be worthwhile, right? If so, then the below patch might be useful:
AFAICT, you'd need to test this on a box which is not supported (family check below). Or you can tweak the family check so that it doesn't apply and run it on a test box of yours to see whether the module still unloads properly in the error case (and doesn't remain loaded as in our case due to something in sysfs remaining registered although it shouldn't).
Thanks.
Signature · 8 lines
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 2/2] x86, microcode: Ensure that module is only loaded for
Quoted 11 lines · Borislav Petkov
Quoted 2 lines · Srivatsa S. Bhat
I guess a similar change in intel microcode driver would also be worthwhile, right? If so, then the below patch might be useful:
AFAICT, you'd need to test this on a box which is not supported (family check below). Or you can tweak the family check so that it doesn't apply and run it on a test box of yours to see whether the module still unloads properly in the error case (and doesn't remain loaded as in our case due to something in sysfs remaining registered although it shouldn't).
Judging by the nature of the code itself, its evident that this problem is not restricted to AMD alone. In any case, I went ahead and did the (tweaked) test that you suggested and here is the result:
1. dmesg logs :
[ 21.912451] microcode: CPU0 not a capable Intel processor [ 21.916028] microcode: CPU1 not a capable Intel processor [ 21.934624] microcode: CPU2 not a capable Intel processor [ 21.940276] microcode: CPU3 not a capable Intel processor [ 21.946023] microcode: CPU4 not a capable Intel processor [ 21.951678] microcode: CPU5 not a capable Intel processor [ 21.957326] microcode: CPU6 not a capable Intel processor [ 21.962935] microcode: CPU7 not a capable Intel processor [ 21.968611] microcode: CPU8 not a capable Intel processor [ 21.974212] microcode: CPU9 not a capable Intel processor [ 21.979842] microcode: CPU10 not a capable Intel processor [ 21.985577] microcode: CPU11 not a capable Intel processor [ 21.991323] microcode: CPU12 not a capable Intel processor [ 21.997055] microcode: CPU13 not a capable Intel processor [ 22.002774] microcode: CPU14 not a capable Intel processor [ 22.008460] microcode: CPU15 not a capable Intel processor [ 22.014600] microcode: Microcode Update Driver: v2.00 <[email protected]>, Peter Oruba
2. lsmod | grep microcode
microcode 117749 0
3. rmmod microcode
4. dmesg logs:
[ 64.297638] ------------[ cut here ]------------ [ 64.302332] WARNING: at fs/sysfs/group.c:142 sysfs_remove_group+0xd9/0xe0() [ 64.309446] Hardware name: IBM System x -[7870C4Q]- [ 64.309449] sysfs group ffffffffa03a5110 not found for kobject 'cpu0' [ 64.309451] Modules linked in: ipv6 cpufreq_conservative cpufreq_userspace cpufreq_powersave acpi_cpufreq mperf fuse loop dm_mod coretemp crc32c_intel shpchp microcode(-) cdc_ether usbnet ioatdma bnx2 mii serio_raw pcspkr pci_hotplug i2c_i801 i7core_edac i2c_core dca edac_core iTCO_wdt iTCO_vendor_support tpm_tis tpm sg tpm_bios rtc_cmos button uhci_hcd ehci_hcd usbcore usb_common sd_mod crc_t10dif edd ext3 mbcache jbd fan processor mptsas mptscsih mptbase scsi_transport_sas scsi_mod thermal thermal_sys hwmon [ 64.309499] Pid: 6529, comm: rmmod Not tainted 3.4.0-rc3-intelucodebroken-0.0.0.28.36b5ec9-default #2 [ 64.309501] Call Trace: [ 64.309506] [<ffffffff811fac69>] ? sysfs_remove_group+0xd9/0xe0 [ 64.309510] [<ffffffff811fac69>] ? sysfs_remove_group+0xd9/0xe0 [ 64.309516] [<ffffffff8103dd2a>] warn_slowpath_common+0x7a/0xb0 [ 64.309522] [<ffffffff8103de01>] warn_slowpath_fmt+0x41/0x50 [ 64.309526] [<ffffffff811f87b6>] ? sysfs_get_dirent+0x56/0x80 [ 64.309531] [<ffffffff811fac69>] sysfs_remove_group+0xd9/0xe0 [ 64.309538] [<ffffffffa03a2118>] mc_device_remove+0x78/0xa0 [microcode] [ 64.309545] [<ffffffff8134b219>] subsys_interface_unregister+0x89/0xd0 [ 64.309553] [<ffffffffa03a357a>] microcode_exit+0x5a/0xa4 [microcode] [ 64.309559] [<ffffffff810b0c8a>] sys_delete_module+0x16a/0x2b0 [ 64.309565] [<ffffffff810a408d>] ? trace_hardirqs_on_caller+0x12d/0x1b0 [ 64.309573] [<ffffffff814b36f9>] system_call_fastpath+0x16/0x1b [ 64.309576] ---[ end trace 748ddf8b4218f820 ]---
The above warning was repeated for each cpu, of course.
Regards,
Srivatsa S. Bhat
Re: [PATCH 2/2] x86, microcode: Ensure that module is only loaded for
Quoted 64 lines · Srivatsa S. Bhat
Quoted 11 lines · Borislav Petkov
Quoted 2 lines · Srivatsa S. Bhat
I guess a similar change in intel microcode driver would also be worthwhile, right? If so, then the below patch might be useful:
AFAICT, you'd need to test this on a box which is not supported (family check below). Or you can tweak the family check so that it doesn't apply and run it on a test box of yours to see whether the module still unloads properly in the error case (and doesn't remain loaded as in our case due to something in sysfs remaining registered although it shouldn't).
Judging by the nature of the code itself, its evident that this problem is not restricted to AMD alone. In any case, I went ahead and did the (tweaked) test that you suggested and here is the result:
1. dmesg logs :
[ 21.912451] microcode: CPU0 not a capable Intel processor [ 21.916028] microcode: CPU1 not a capable Intel processor [ 21.934624] microcode: CPU2 not a capable Intel processor [ 21.940276] microcode: CPU3 not a capable Intel processor [ 21.946023] microcode: CPU4 not a capable Intel processor [ 21.951678] microcode: CPU5 not a capable Intel processor [ 21.957326] microcode: CPU6 not a capable Intel processor [ 21.962935] microcode: CPU7 not a capable Intel processor [ 21.968611] microcode: CPU8 not a capable Intel processor [ 21.974212] microcode: CPU9 not a capable Intel processor [ 21.979842] microcode: CPU10 not a capable Intel processor [ 21.985577] microcode: CPU11 not a capable Intel processor [ 21.991323] microcode: CPU12 not a capable Intel processor [ 21.997055] microcode: CPU13 not a capable Intel processor [ 22.002774] microcode: CPU14 not a capable Intel processor [ 22.008460] microcode: CPU15 not a capable Intel processor [ 22.014600] microcode: Microcode Update Driver: v2.00 <[email protected]>, Peter Oruba2. lsmod | grep microcode
microcode 117749 03. rmmod microcode
4. dmesg logs:
[ 64.297638] ------------[ cut here ]------------ [ 64.302332] WARNING: at fs/sysfs/group.c:142 sysfs_remove_group+0xd9/0xe0() [ 64.309446] Hardware name: IBM System x -[7870C4Q]- [ 64.309449] sysfs group ffffffffa03a5110 not found for kobject 'cpu0' [ 64.309451] Modules linked in: ipv6 cpufreq_conservative cpufreq_userspace cpufreq_powersave acpi_cpufreq mperf fuse loop dm_mod coretemp crc32c_intel shpchp microcode(-) cdc_ether usbnet ioatdma bnx2 mii serio_raw pcspkr pci_hotplug i2c_i801 i7core_edac i2c_core dca edac_core iTCO_wdt iTCO_vendor_support tpm_tis tpm sg tpm_bios rtc_cmos button uhci_hcd ehci_hcd usbcore usb_common sd_mod crc_t10dif edd ext3 mbcache jbd fan processor mptsas mptscsih mptbase scsi_transport_sas scsi_mod thermal thermal_sys hwmon [ 64.309499] Pid: 6529, comm: rmmod Not tainted 3.4.0-rc3-intelucodebroken-0.0.0.28.36b5ec9-default #2 [ 64.309501] Call Trace: [ 64.309506] [<ffffffff811fac69>] ? sysfs_remove_group+0xd9/0xe0 [ 64.309510] [<ffffffff811fac69>] ? sysfs_remove_group+0xd9/0xe0 [ 64.309516] [<ffffffff8103dd2a>] warn_slowpath_common+0x7a/0xb0 [ 64.309522] [<ffffffff8103de01>] warn_slowpath_fmt+0x41/0x50 [ 64.309526] [<ffffffff811f87b6>] ? sysfs_get_dirent+0x56/0x80 [ 64.309531] [<ffffffff811fac69>] sysfs_remove_group+0xd9/0xe0 [ 64.309538] [<ffffffffa03a2118>] mc_device_remove+0x78/0xa0 [microcode] [ 64.309545] [<ffffffff8134b219>] subsys_interface_unregister+0x89/0xd0 [ 64.309553] [<ffffffffa03a357a>] microcode_exit+0x5a/0xa4 [microcode] [ 64.309559] [<ffffffff810b0c8a>] sys_delete_module+0x16a/0x2b0 [ 64.309565] [<ffffffff810a408d>] ? trace_hardirqs_on_caller+0x12d/0x1b0 [ 64.309573] [<ffffffff814b36f9>] system_call_fastpath+0x16/0x1b [ 64.309576] ---[ end trace 748ddf8b4218f820 ]---
Quoted 1 line
The above warning was repeated for each cpu, of course.
Right, this is the warning we're seeing too. Applying your patch fixes it, I s'pose? If so, maybe x86 people would pick it up.
Signature · 8 lines
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 2/2] x86, microcode: Ensure that module is only loaded for
Quoted 76 lines · Borislav Petkov
Quoted 75 lines · Srivatsa S. Bhat
Quoted 10 lines · Borislav Petkov
Quoted 3 lines · Srivatsa S. Bhat
I guess a similar change in intel microcode driver would also be
worthwhile, right? If so, then the below patch might be useful:
AFAICT, you'd need to test this on a box which is not supported (family check below). Or you can tweak the family check so that it doesn't apply and run it on a test box of yours to see whether the module still unloads properly in the error case (and doesn't remain loaded as in our case due to something in sysfs remaining registered although it shouldn't).
Judging by the nature of the code itself, its evident that this problem is not restricted to AMD alone. In any case, I went ahead and did the (tweaked) test that you suggested and here is the result:
1. dmesg logs :
[ 21.912451] microcode: CPU0 not a capable Intel processor [ 21.916028] microcode: CPU1 not a capable Intel processor [ 21.934624] microcode: CPU2 not a capable Intel processor [ 21.940276] microcode: CPU3 not a capable Intel processor [ 21.946023] microcode: CPU4 not a capable Intel processor [ 21.951678] microcode: CPU5 not a capable Intel processor [ 21.957326] microcode: CPU6 not a capable Intel processor [ 21.962935] microcode: CPU7 not a capable Intel processor [ 21.968611] microcode: CPU8 not a capable Intel processor [ 21.974212] microcode: CPU9 not a capable Intel processor [ 21.979842] microcode: CPU10 not a capable Intel processor [ 21.985577] microcode: CPU11 not a capable Intel processor [ 21.991323] microcode: CPU12 not a capable Intel processor [ 21.997055] microcode: CPU13 not a capable Intel processor [ 22.002774] microcode: CPU14 not a capable Intel processor [ 22.008460] microcode: CPU15 not a capable Intel processor [ 22.014600] microcode: Microcode Update Driver: v2.00<[email protected]>, Peter Oruba
2. lsmod | grep microcode
microcode 117749 03. rmmod microcode
4. dmesg logs:
[ 64.297638] ------------[ cut here ]------------ [ 64.302332] WARNING: at fs/sysfs/group.c:142 sysfs_remove_group+0xd9/0xe0() [ 64.309446] Hardware name: IBM System x -[7870C4Q]- [ 64.309449] sysfs group ffffffffa03a5110 not found for kobject'cpu0' [ 64.309451] Modules linked in: ipv6
cpufreq_conservativecpufreq_userspacecpufreq_powersaveacpi_cpufreqmperf fuse loopdm_modcoretempcrc32c_intelshpchp microcode(-)cdc_etherusbnet ioatdma bnx2 miiserio_rawpcspkrpci_hotplugi2c_i801i7core_edaci2c_coredcaedac_coreiTCO_wdtiTCO_vendor_supporttpm_tistpm sgtpm_biosrtc_cmosbuttonuhci_hcdehci_hcdusbcoreusb_commonsd_modcrc_t10difedd ext3 mbcache jbd fan processor mptsas mptscsih mptbasescsi_transport_sasscsi_modthermalthermal_syshwmon [ 64.309499]
Pid: 6529, comm: rmmod Not tainted
3.4.0-rc3-intelucodebroken-0.0.0.28.36b5ec9-default #2 [ 64.309501]Call Trace: [ 64.309506] [<ffffffff811fac69>] ? sysfs_remove_group+0xd9/0xe0 [ 64.309510] [<ffffffff811fac69>] ? sysfs_remove_group+0xd9/0xe0 [ 64.309516] [<ffffffff8103dd2a>] warn_slowpath_common+0x7a/0xb0 [ 64.309522] [<ffffffff8103de01>] warn_slowpath_fmt+0x41/0x50 [ 64.309526] [<ffffffff811f87b6>] ? sysfs_get_dirent+0x56/0x80 [ 64.309531] [<ffffffff811fac69>] sysfs_remove_group+0xd9/0xe0 [ 64.309538] [<ffffffffa03a2118>] mc_device_remove+0x78/0xa0 [microcode] [ 64.309545] [<ffffffff8134b219>] subsys_interface_unregister+0x89/0xd0 [ 64.309553] [<ffffffffa03a357a>] microcode_exit+0x5a/0xa4 [microcode] [ 64.309559] [<ffffffff810b0c8a>] sys_delete_module+0x16a/0x2b0 [ 64.309565] [<ffffffff810a408d>] ? trace_hardirqs_on_caller+0x12d/0x1b0 [ 64.309573] [<ffffffff814b36f9>] system_call_fastpath+0x16/0x1b [ 64.309576] ---[ end trace 748ddf8b4218f820 ]---
Quoted 4 lines
Quoted 1 line
The above warning was repeated for each cpu, of course.
Right, this is the warning we're seeing too. Applying your patch fixes it, I s'pose? If so, maybe x86 people would pick it up.
I do not think the above has anything to do with the current version shipping in some distro's, which is trying to find the intel code on kernels >2.6.39 when running on AMD stuff, so that script in /etc/init.d needs attention too. I have nuked about half of that script in my local copy so that it does install AMD code on AMD processors. The errors I was seeing were only the lack of its ability to find the Intel code and does not generate any output resembling the above.
That test in that script for a minimum minor kernel version to determine which microcode to apply seems totally bogus to me anyway, but I am not privy to enough cpu info to write the correct test.
Cheers, Gene
Signature · 8 lines
"There are four boxes to be used in defense of liberty: soap, ballot, jury, and ammo. Please use in that order." -Ed Howdershelt (Author) My web page: <http://coyoteden.dyndns-free.com:85/gene> "Most of us, when all is said and done, like what we like and make up reasons for it afterwards." -- Soren F. Petersen
Re: [PATCH 2/2] x86, microcode: Ensure that module is only loaded for
2012/4/17 Gene Heskett <[email protected]>:
Quoted 7 lines
I do not think the above has anything to do with the current version shipping in some distro's, which is trying to find the intel code on kernels >2.6.39 when running on AMD stuff, so that script in
/etc/init.dneeds attention too. I have nuked about half of that script in my local copy so that it does install AMD code on AMD processors. The errors I was seeing were only the lack of its ability to find the Intel code and does not generate any output resembling the above.
Just a note: the intel microcode is loaded by a kernel-initiated firmware request these days. There is no (broken) init script on recent systems, to (rather mindlessly) fiddle around in userspace with kernel internals.
Kay
Re: [PATCH 2/2] x86, microcode: Ensure that module is only loaded for
Quoted 10 lines · Kay Sievers
2012/4/17 Gene Heskett <[email protected]>:
Quoted 8 lines
I do not think the above has anything to do with the current version shipping in some distro's, which is trying to find the intel code on kernels >2.6.39 when running on AMD stuff, so that script in
/etc/init.dneeds attention too. آ I have nuked about half of that script in my local copy so that it does install AMD code on AMD processors. آ The errors I was seeing were only the lack of its ability to find the Intel code and does not generate any output resembling the above.
Quoted 6 lines
Just a note: the intel microcode is loaded by a kernel-initiated firmware request these days. There is no (broken) init script on recent systems, to (rather mindlessly) fiddle around in userspace with kernel internals.
Kay
That has to be a good deal in general, but it leaves us AMD users swinging in a strong breeze. That microcode loading was in the kernel until sometime in the 2.6.20ish time frame, I can recall scanning dmesg's where it was loaded by about line 75 in past history. Now its just an afterthought. :(
Thanks Kay.
Cheers, Gene
Signature · 7 lines
"There are four boxes to be used in defense of liberty: soap, ballot, jury, and ammo. Please use in that order." -Ed Howdershelt (Author) My web page: <http://coyoteden.dyndns-free.com:85/gene> The American nation in the sixth ward is a fine people; they love the eagle -- on the back of a dollar. -- Finlay Peter Dunne
Re: [PATCH 2/2] x86, microcode: Ensure that module is only loaded for
Quoted 16 lines · Gene Heskett
Quoted 15 lines · Kay Sievers
2012/4/17 Gene Heskett <[email protected]>:
Quoted 8 lines
I do not think the above has anything to do with the current version shipping in some distro's, which is trying to find the intel code on kernels >2.6.39 when running on AMD stuff, so that script in
/etc/init.dneeds attention too. آ I have nuked about half of that script in my local copy so that it does install AMD code on AMD processors. آ The errors I was seeing were only the lack of its ability to find the Intel code and does not generate any output resembling the above.Just a note: the intel microcode is loaded by a kernel-initiated firmware request these days. There is no (broken) init script on recent systems, to (rather mindlessly) fiddle around in userspace with kernel internals.
Quoted 6 lines
Quoted 1 line
Kay
That has to be a good deal in general, but it leaves us AMD users swinging in a strong breeze. That microcode loading was in the kernel until sometime in the 2.6.20ish time frame, I can recall scanning dmesg's where it was loaded by about line 75 in past history. Now its just an afterthought. :(
Well, I don't know what your script does but it should simply
$ modprobe microcode
and the module should figure it out itself which microcode to load, AFAICT.
Signature · 8 lines
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 2/2] x86, microcode: Ensure that module is only loaded for
In reply to Kay SieversQuoted 11 lines · Kay Sievers
2012/4/17 Gene Heskett <[email protected]>:
Quoted 7 lines
I do not think the above has anything to do with the current version shipping in some distro's, which is trying to find the intel code on kernels >2.6.39 when running on AMD stuff, so that script in
/etc/init.dneeds attention too. I have nuked about half of that script in my local copy so that it does install AMD code on AMD processors. The errors I was seeing were only the lack of its ability to find the Intel code and does not generate any output resembling the above.Just a note: the intel microcode is loaded by a kernel-initiated firmware request these days. There is no (broken) init script on
Is it? Which distros have deployed the required userspace and switched to Intel binary microcode already?
Signature · 4 lines
"One disk to rule them all, One disk to find them. One disk to bring them all and in the darkness grind them. In the Land of Redmond where the shadows lie." -- The Silicon Valley Tarot Henrique Holschuh
Re: [PATCH 2/2] x86, microcode: Ensure that module is only loaded for
Quoted 7 lines · Henrique de Moraes Holschuh
Quoted 2 lines · Kay Sievers
Just a note: the intel microcode is loaded by a kernel-initiated firmware request these days. There is no (broken) init script on
Is it? Which distros have deployed the required userspace and switched to Intel binary microcode already?
The tool is here:
http://pkgs.fedoraproject.org/repo/pkgs/microcode_ctl/intel-microcode2ucode.c/0efc5f6c74a4d7e61ca22683c93c98cf/intel-microcode2ucode.c
It's at least used here:
http://pkgs.fedoraproject.org/gitweb/?p=microcode_ctl.git;a=tree
https://build.opensuse.org/package/files?package=microcode_ctl&project=Base%3ASystem
http://sources.gentoo.org/cgi-bin/viewvc.cgi/gentoo-x86/sys-apps/microcode-data/files/
Kay
Re: [PATCH 2/2] x86, microcode: Ensure that module is only loaded for
In reply to Borislav PetkovQuoted 65 lines · Borislav Petkov
Quoted 64 lines · Srivatsa S. Bhat
Quoted 11 lines · Borislav Petkov
Quoted 2 lines · Srivatsa S. Bhat
I guess a similar change in intel microcode driver would also be worthwhile, right? If so, then the below patch might be useful:
AFAICT, you'd need to test this on a box which is not supported (family check below). Or you can tweak the family check so that it doesn't apply and run it on a test box of yours to see whether the module still unloads properly in the error case (and doesn't remain loaded as in our case due to something in sysfs remaining registered although it shouldn't).
Judging by the nature of the code itself, its evident that this problem is not restricted to AMD alone. In any case, I went ahead and did the (tweaked) test that you suggested and here is the result:
1. dmesg logs :
[ 21.912451] microcode: CPU0 not a capable Intel processor [ 21.916028] microcode: CPU1 not a capable Intel processor [ 21.934624] microcode: CPU2 not a capable Intel processor [ 21.940276] microcode: CPU3 not a capable Intel processor [ 21.946023] microcode: CPU4 not a capable Intel processor [ 21.951678] microcode: CPU5 not a capable Intel processor [ 21.957326] microcode: CPU6 not a capable Intel processor [ 21.962935] microcode: CPU7 not a capable Intel processor [ 21.968611] microcode: CPU8 not a capable Intel processor [ 21.974212] microcode: CPU9 not a capable Intel processor [ 21.979842] microcode: CPU10 not a capable Intel processor [ 21.985577] microcode: CPU11 not a capable Intel processor [ 21.991323] microcode: CPU12 not a capable Intel processor [ 21.997055] microcode: CPU13 not a capable Intel processor [ 22.002774] microcode: CPU14 not a capable Intel processor [ 22.008460] microcode: CPU15 not a capable Intel processor [ 22.014600] microcode: Microcode Update Driver: v2.00 <[email protected]>, Peter Oruba2. lsmod | grep microcode
microcode 117749 03. rmmod microcode
4. dmesg logs:
[ 64.297638] ------------[ cut here ]------------ [ 64.302332] WARNING: at fs/sysfs/group.c:142 sysfs_remove_group+0xd9/0xe0() [ 64.309446] Hardware name: IBM System x -[7870C4Q]- [ 64.309449] sysfs group ffffffffa03a5110 not found for kobject 'cpu0' [ 64.309451] Modules linked in: ipv6 cpufreq_conservative cpufreq_userspace cpufreq_powersave acpi_cpufreq mperf fuse loop dm_mod coretemp crc32c_intel shpchp microcode(-) cdc_ether usbnet ioatdma bnx2 mii serio_raw pcspkr pci_hotplug i2c_i801 i7core_edac i2c_core dca edac_core iTCO_wdt iTCO_vendor_support tpm_tis tpm sg tpm_bios rtc_cmos button uhci_hcd ehci_hcd usbcore usb_common sd_mod crc_t10dif edd ext3 mbcache jbd fan processor mptsas mptscsih mptbase scsi_transport_sas scsi_mod thermal thermal_sys hwmon [ 64.309499] Pid: 6529, comm: rmmod Not tainted 3.4.0-rc3-intelucodebroken-0.0.0.28.36b5ec9-default #2 [ 64.309501] Call Trace: [ 64.309506] [<ffffffff811fac69>] ? sysfs_remove_group+0xd9/0xe0 [ 64.309510] [<ffffffff811fac69>] ? sysfs_remove_group+0xd9/0xe0 [ 64.309516] [<ffffffff8103dd2a>] warn_slowpath_common+0x7a/0xb0 [ 64.309522] [<ffffffff8103de01>] warn_slowpath_fmt+0x41/0x50 [ 64.309526] [<ffffffff811f87b6>] ? sysfs_get_dirent+0x56/0x80 [ 64.309531] [<ffffffff811fac69>] sysfs_remove_group+0xd9/0xe0 [ 64.309538] [<ffffffffa03a2118>] mc_device_remove+0x78/0xa0 [microcode] [ 64.309545] [<ffffffff8134b219>] subsys_interface_unregister+0x89/0xd0 [ 64.309553] [<ffffffffa03a357a>] microcode_exit+0x5a/0xa4 [microcode] [ 64.309559] [<ffffffff810b0c8a>] sys_delete_module+0x16a/0x2b0 [ 64.309565] [<ffffffff810a408d>] ? trace_hardirqs_on_caller+0x12d/0x1b0 [ 64.309573] [<ffffffff814b36f9>] system_call_fastpath+0x16/0x1b [ 64.309576] ---[ end trace 748ddf8b4218f820 ]---
Quoted 4 lines
Quoted 1 line
The above warning was repeated for each cpu, of course.
Right, this is the warning we're seeing too. Applying your patch fixes it, I s'pose?
Of course.. Sorry I didn't mention it explicitly.
Strictly speaking, in the patch series by Andreas,
patch 1/2: gets rid of the warning,
patch 2/2: ensures that the microcode module doesn't remain loaded when unsupported cpus are detected - but it does it only for AMD cpus.
My patch is very much like 2/2 except that it is for the Intel cpus - if the underlying cpus are unsupported, then it ensures that the module loading fails. [The warning is already silenced by patch 1/2].
Quoted 2 lines
If so, maybe x86 people would pick it up.
Ok, thanks!
Regards,
Srivatsa S. Bhat
[tip:x86/urgent] x86/microcode: Ensure that module is only loaded on supported Intel CPUs
In reply to Srivatsa S. BhatCommit-ID: 7164b3f5e5461b58baa82e82544ca5af71af287b Gitweb: http://git.kernel.org/tip/7164b3f5e5461b58baa82e82544ca5af71af287b Author: Srivatsa S. Bhat <[email protected]> AuthorDate: Mon, 16 Apr 2012 14:12:00 +0530 Committer: Ingo Molnar <[email protected]> CommitDate: Mon, 7 May 2012 14:37:14 +0200
x86/microcode: Ensure that module is only loaded on supported Intel CPUs
Exit early when there's no support for a particular CPU family.
--- arch/x86/kernel/microcode_intel.c | 14 ++++++++------ 1 files changed, 8 insertions(+), 6 deletions(-)
arch/x86/kernel/microcode_intel.c +8 -6
@@ -147,12 +147,6 @@ static int collect_cpu_info(int cpu_num, struct cpu_signature *csig)memset (csig,0 ,sizeof (*csig)); -if (c->x86_vendor !=X86_VENDOR_INTEL || c->x86 <6 || -cpu_has (c,X86_FEATURE_IA64 )) { -pr_err ("CPU%d not a capable Intel processor\n" , cpu_num); -return -1 ; - } - csig->sig =cpuid_eax (0x00000001 );if ((c->x86_model >=5 ) || (c->x86 >6 )) {
@@ -463,6 +457,14 @@ static struct microcode_ops microcode_intel_ops = {struct microcode_ops *__init init_intel_microcode (void ) { +struct cpuinfo_x86 *c = &cpu_data (0 ); + +if (c->x86_vendor !=X86_VENDOR_INTEL || c->x86 <6 || +cpu_has (c,X86_FEATURE_IA64 )) { +pr_err ("Intel CPU family 0x%x not supported\n" , c->x86); +return NULL ; + } +return µcode_intel_ops; }
[tip:x86/urgent] x86, microcode: Fix sysfs warning during module unload on unsupported CPUs
In reply to Andreas HerrmannCommit-ID: a956bd6f8583326b18348ab1452b4686778f785d Gitweb: http://git.kernel.org/tip/a956bd6f8583326b18348ab1452b4686778f785d Author: Andreas Herrmann <[email protected]> AuthorDate: Thu, 12 Apr 2012 16:48:01 +0200 Committer: Borislav Petkov <[email protected]> CommitDate: Fri, 13 Apr 2012 11:49:06 +0200
x86, microcode: Fix sysfs warning during module unload on unsupported CPUs
Loading the microcode driver on an unsupported CPU and subsequently unloading the driver causes
WARNING: at fs/sysfs/group.c:138 mc_device_remove+0x5f/0x70 [microcode]() Hardware name: 01972NG sysfs group ffffffffa00013d0 not found for kobject 'cpu0' Modules linked in: snd_hda_codec_hdmi snd_hda_codec_conexant snd_hda_intel btusb snd_hda_codec bluetooth thinkpad_acpi rfkill microcode(-) [last unloaded: cfg80211] Pid: 4560, comm: modprobe Not tainted 3.4.0-rc2-00002-g258f742 #5 Call Trace: [<ffffffff8103113b>] ? warn_slowpath_common+0x7b/0xc0 [<ffffffff81031235>] ? warn_slowpath_fmt+0x45/0x50 [<ffffffff81120e74>] ? sysfs_remove_group+0x34/0x120 [<ffffffffa00000ef>] ? mc_device_remove+0x5f/0x70 [microcode] [<ffffffff81331eb9>] ? subsys_interface_unregister+0x69/0xa0 [<ffffffff81563526>] ? mutex_lock+0x16/0x40 [<ffffffffa0000c3e>] ? microcode_exit+0x50/0x92 [microcode] [<ffffffff8107051d>] ? sys_delete_module+0x16d/0x260 [<ffffffff810a0065>] ? wait_iff_congested+0x45/0x110 [<ffffffff815656af>] ? page_fault+0x1f/0x30 [<ffffffff81565ba2>] ? system_call_fastpath+0x16/0x1b
on recent kernels.
This is due to commit 8a25a2fd126c ("cpu: convert 'cpu' and 'machinecheck' sysdev_class to a regular subsystem") which renders commit 6c53cbfced04 ("x86, microcode: Correct sysdev_add error path") useless.
See http://marc.info/?l=linux-kernel&m=133416246406478
Avoid above warning by restoring the old driver behaviour before 6c53cbfced04 ("x86, microcode: Correct sysdev_add error path").
--- arch/x86/kernel/microcode_core.c | 4 +--- 1 files changed, 1 insertions(+), 3 deletions(-)
arch/x86/kernel/microcode_core.c +1 -3
@@ -419,10 +419,8 @@ static int mc_device_add(struct device *dev, struct subsys_interface *sif)if (err)return err; -if (microcode_init_cpu (cpu) ==UCODE_ERROR ) { -sysfs_remove_group (&dev->kobj, &mc_attr_group); +if (microcode_init_cpu (cpu) ==UCODE_ERROR )return -EINVAL ; - }return err; }
[tip:x86/urgent] x86, microcode: Ensure that module is only loaded on supported AMD CPUs
In reply to Andreas HerrmannCommit-ID: 283c1f2558ef4a4411fe908364b15b73b6ab44cf Gitweb: http://git.kernel.org/tip/283c1f2558ef4a4411fe908364b15b73b6ab44cf Author: Andreas Herrmann <[email protected]> AuthorDate: Thu, 12 Apr 2012 16:51:57 +0200 Committer: Borislav Petkov <[email protected]> CommitDate: Fri, 13 Apr 2012 11:51:05 +0200
x86, microcode: Ensure that module is only loaded on supported AMD CPUs
Exit early when there's no support for a particular CPU family. Also, fixup the "no support for this CPU vendor" to be issued only when the driver is attempted to be loaded on an unsupported vendor.
--- arch/x86/kernel/microcode_amd.c | 12 +++++++----- arch/x86/kernel/microcode_core.c | 6 +++--- 2 files changed, 10 insertions(+), 8 deletions(-)
arch/x86/kernel/microcode_amd.c +7 -5
@@ -82,11 +82,6 @@ static int collect_cpu_info_amd(int cpu, struct cpu_signature *csig) {struct cpuinfo_x86 *c = &cpu_data (cpu); -if (c->x86_vendor !=X86_VENDOR_AMD || c->x86 <0x10 ) { -pr_warning ("CPU%d: family %d not supported\n" , cpu, c->x86); -return -1 ; - } - csig->rev = c->microcode;pr_info ("CPU%d: patch_level=0x%08x\n" , cpu, csig->rev);
@@ -380,6 +375,13 @@ static struct microcode_ops microcode_amd_ops = {struct microcode_ops *__init init_amd_microcode (void ) { +struct cpuinfo_x86 *c = &cpu_data (0 ); + +if (c->x86_vendor !=X86_VENDOR_AMD || c->x86 <0x10 ) { +pr_warning ("AMD CPU family 0x%x not supported\n" , c->x86); +return NULL ; + } + patch = (void *)get_zeroed_page (GFP_KERNEL );if (!patch)return NULL ;
arch/x86/kernel/microcode_core.c +3 -3
@@ -526,11 +526,11 @@ static int __init microcode_init(void) microcode_ops =init_intel_microcode (); else if (c->x86_vendor == X86_VENDOR_AMD) microcode_ops =init_amd_microcode (); - -if (!microcode_ops) { + else pr_err("no support for this CPU vendor\n"); + +if (!microcode_ops)return -ENODEV ; - } microcode_pdev =platform_device_register_simple ("microcode" ,-1 ,NULL ,0 );