x86-microcode: run ucode update via workqueue
cover x86-microcode: run ucode update via workqueue
Referring to: http://lkml.org/lkml/2008/8/3/114
Dmitry, thanks for your work.
Rebased Dmitry's patch set to x86-microcode and incorporated changes into AMD microcode patch loader. No issues with cpu-hotplug anymore, tested on Intel as well as AMD.
--Peter
PATCH 1/5 x86: microcode generic updates
--- arch/x86/kernel/microcode.c | 83 ++++++++++++++----------------- arch/x86/kernel/microcode_intel.c | 97 +++++++++++++++++++------------------ include/asm-x86/microcode.h | 27 +++++----- 3 files changed, 101 insertions(+), 106 deletions(-)
arch/x86/kernel/microcode.c +37 -46
@@ -234,22 +234,6 @@ MODULE_ALIAS_MISCDEV(MICROCODE_MINOR);struct platform_device *microcode_pdev;EXPORT_SYMBOL_GPL (microcode_pdev); -static void microcode_init_cpu (int cpu,int resume) -{ -cpumask_t old; -struct ucode_cpu_info *uci = ucode_cpu_info + cpu; - - old = current->cpus_allowed; - -set_cpus_allowed_ptr (current, &cpumask_of_cpu (cpu)); -mutex_lock (µcode_mutex); - microcode_ops->collect_cpu_info (cpu); -if (uci->valid && system_state ==SYSTEM_RUNNING && !resume) - microcode_ops->cpu_request_microcode (cpu); -mutex_unlock (µcode_mutex); -set_cpus_allowed_ptr (current, &old); -} -static ssize_t reload_store (struct sys_device *dev,struct sysdev_attribute *attr,const char *buf,size_t sz)
@@ -266,14 +250,15 @@ static ssize_t reload_store(struct sys_device *dev,cpumask_t old = current->cpus_allowed;get_online_cpus (); -set_cpus_allowed_ptr (current, &cpumask_of_cpu (cpu)); - -mutex_lock (µcode_mutex); -if (uci->valid) - err = microcode_ops->cpu_request_microcode (cpu); -mutex_unlock (µcode_mutex); +if (cpu_online (cpu)) { +set_cpus_allowed_ptr (current, &cpumask_of_cpu (cpu)); +mutex_lock (µcode_mutex); +if (uci->valid) + err = microcode_ops->cpu_request_microcode (cpu); +mutex_unlock (µcode_mutex); +set_cpus_allowed_ptr (current, &old); + }put_online_cpus (); -set_cpus_allowed_ptr (current, &old); }if (err)return err;
@@ -312,7 +297,16 @@ static struct attribute_group mc_attr_group = { .name = "microcode", }; -static int __mc_sysdev_add (struct sys_device *sys_dev,int resume) +static void microcode_init_cpu (int cpu) +{ +cpumask_t old = current->cpus_allowed; + +set_cpus_allowed_ptr (current, &cpumask_of_cpu (cpu)); + microcode_ops->microcode_update_cpu (cpu); +set_cpus_allowed_ptr (current, &old); +} + +static int mc_sysdev_add (struct sys_device *sys_dev) {int err, cpu = sys_dev->id;struct ucode_cpu_info *uci = ucode_cpu_info + cpu;
@@ -327,16 +321,11 @@ static int __mc_sysdev_add(struct sys_device *sys_dev, int resume)if (err)return err; -microcode_init_cpu (cpu, resume); +microcode_init_cpu (cpu);return 0 ; } -static int mc_sysdev_add (struct sys_device *sys_dev) -{ -return __mc_sysdev_add (sys_dev,0 ); -} -static int mc_sysdev_remove (struct sys_device *sys_dev) {int cpu = sys_dev->id;
@@ -362,6 +351,16 @@ static int mc_sysdev_resume(struct sys_device *dev)return 0 ; } +static void microcode_set_cpu_frozen (int cpu) +{ +struct ucode_cpu_info *uci = ucode_cpu_info + cpu; + +mutex_lock (µcode_mutex); +if (uci->valid) + uci->resume =1 ; +mutex_unlock (µcode_mutex); +} +static struct sysdev_driver mc_sysdev_driver = { .add = mc_sysdev_add, .remove = mc_sysdev_remove,
@@ -376,34 +375,26 @@ mc_cpu_callback(struct notifier_block *nb, unsigned long action, void *hcpu) sys_dev =get_cpu_sysdev (cpu);switch (action) { -case CPU_UP_CANCELED_FROZEN : -/* The CPU refused to come up during a system resume */ - microcode_ops->microcode_fini_cpu (cpu); -break ;case CPU_ONLINE : -case CPU_DOWN_FAILED : -mc_sysdev_add (sys_dev); -break ;case CPU_ONLINE_FROZEN : -/* System-wide resume is in progress, try to apply microcode */ -if (microcode_ops->apply_microcode_check_cpu (cpu)) { -/* The application of microcode failed */ - microcode_ops->microcode_fini_cpu (cpu); -__mc_sysdev_add (sys_dev,1 ); -break ; - } +case CPU_DOWN_FAILED :case CPU_DOWN_FAILED_FROZEN :if (sysfs_create_group (&sys_dev->kobj, &mc_attr_group))printk (KERN_ERR "microcode: Failed to create the sysfs " "group for CPU%d\n" , cpu);break ;case CPU_DOWN_PREPARE : -mc_sysdev_remove (sys_dev); -break ;case CPU_DOWN_PREPARE_FROZEN :/* Suspend is in progress, only remove the interface */ sysfs_remove_group (&sys_dev->kobj, &mc_attr_group);break ; +case CPU_DEAD : +case CPU_UP_CANCELED_FROZEN : +/* The CPU refused to come up during a system resume */ + microcode_ops->microcode_fini_cpu (cpu); +break ; +case CPU_DEAD_FROZEN : +microcode_set_cpu_frozen (cpu); }return NOTIFY_OK ; }
arch/x86/kernel/microcode_intel.c +50 -47
130 lines not on this page · raw message
include/asm-x86/microcode.h +14 -13
@@ -1,19 +1,6 @@extern int microcode_init (void *opaque,struct module *module);extern void microcode_exit (void ); -struct microcode_ops { -long (*get_next_ucode)(void **mc,long offset); -long (*microcode_get_next_ucode)(void **mc,long offset); -int (*get_matching_microcode)(void *mc,int cpu); -int (*apply_microcode_check_cpu)(int cpu); -int (*microcode_sanity_check)(void *mc); -int (*cpu_request_microcode)(int cpu); -void (*collect_cpu_info)(int cpu_num); -void (*apply_microcode)(int cpu); -void (*microcode_fini_cpu)(int cpu); -void (*clear_patch)(void *data); -}; -struct microcode_header_intel {unsigned int hdrver;unsigned int rev;
@@ -77,6 +64,7 @@ struct microcode_amd {struct ucode_cpu_info {int valid; +int resume;unsigned int sig;unsigned int pf;unsigned int rev;
@@ -85,3 +73,16 @@ struct ucode_cpu_info {struct microcode_amd *mc_amd; } mc; }; + +struct microcode_ops { +long (*get_next_ucode)(void **mc,long offset); +void (*microcode_update_cpu)(int cpu); +long (*microcode_get_next_ucode)(void **mc,long offset); +int (*get_matching_microcode)(void *mc,int cpu); +int (*microcode_sanity_check)(void *mc); +int (*cpu_request_microcode)(int cpu); +void (*collect_cpu_info)(int cpu_num,struct ucode_cpu_info *uci); +void (*apply_microcode)(int cpu); +void (*microcode_fini_cpu)(int cpu); +void (*clear_patch)(void *data); +};
Signature · 1 line
1.5.4.5
Re: [patch 1/5] [PATCH 1/5] x86: microcode generic updates
Quoted 2 lines · Peter Oruba
Signed-off-by Dmitry Adamushko <[email protected]>Signed-off-by Peter Oruba <[email protected]>
hm, missing changelog entry.
Ingo
PATCH 2/5 x86: Minor correction to header file
In reply to Peter Oruba--- include/asm-x86/microcode.h | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
include/asm-x86/microcode.h +2 -2
@@ -1,5 +1,5 @@ -extern int microcode_init (void *opaque,struct module *module); -extern void microcode_exit (void ); +int microcode_init (void *opaque,struct module *module); +void microcode_exit (void );struct microcode_header_intel {unsigned int hdrver;
Signature · 1 line
1.5.4.5
Re: [patch 2/5] [PATCH 2/5] x86: Minor correction to header file
Quoted 10 lines · Peter Oruba
Signed-off-by Peter Oruba <[email protected]>--- include/asm-x86/microcode.h | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)include/asm-x86/microcode.h +0
-0
include/asm-x86/microcode.h line 2
Quoted 4 lines
include/asm-x86/microcode.h +2
-2@@ -1,5 +1,5 @@ -extern int microcode_init (void *opaque,struct module *module); -extern void microcode_exit (void ); +int microcode_init (void *opaque,struct module *module); +void microcode_exit (void );
why? extern is the typical way to specify interfaces.
Ingo
PATCH 3/5 x86: Run Intel ucode-updates via workqueue.
In reply to Peter Oruba--- arch/x86/kernel/microcode.c | 49 ++++++++++++++---------- arch/x86/kernel/microcode_intel.c | 74 ++++++++++++++++++++++-------------- include/asm-x86/microcode.h | 9 +++- 3 files changed, 80 insertions(+), 52 deletions(-)
arch/x86/kernel/microcode.c +28 -21
@@ -110,6 +110,14 @@ EXPORT_SYMBOL_GPL(microcode_mutex);struct ucode_cpu_info ucode_cpu_info[NR_CPUS ];EXPORT_SYMBOL_GPL (ucode_cpu_info); +struct ucode_work_info ucode_work_info[NR_CPUS ]; +EXPORT_SYMBOL_GPL (ucode_work_info); + +static inline struct ucode_cpu_info *get_ucode_cpu_info (int cpu) +{ +return &ucode_work_info[cpu].uci; +} +#ifdef CONFIG_MICROCODE_OLD_INTERFACE void __user *user_buffer;/* user area microcode data buffer */ EXPORT_SYMBOL_GPL (user_buffer);
@@ -238,7 +246,7 @@ static ssize_t reload_store(struct sys_device *dev,struct sysdev_attribute *attr, const char *buf, size_t sz) { - struct ucode_cpu_info *uci = ucode_cpu_info + dev->id; + struct ucode_cpu_info *uci = get_ucode_cpu_info(dev->id);char *end;unsigned long val =simple_strtoul (buf, &end,0 );int err =0 ;
@@ -268,7 +276,7 @@ static ssize_t reload_store(struct sys_device *dev,static ssize_t version_show (struct sys_device *dev,struct sysdev_attribute *attr,char *buf) { -struct ucode_cpu_info *uci = ucode_cpu_info + dev->id; +struct ucode_cpu_info *uci =get_ucode_cpu_info (dev->id);return sprintf (buf,"0x%x\n" , uci->rev); }
@@ -276,7 +284,7 @@ static ssize_t version_show(struct sys_device *dev,static ssize_t pf_show (struct sys_device *dev,struct sysdev_attribute *attr,char *buf) { -struct ucode_cpu_info *uci = ucode_cpu_info + dev->id; +struct ucode_cpu_info *uci =get_ucode_cpu_info (dev->id);return sprintf (buf,"0x%x\n" , uci->pf); }
@@ -297,31 +305,22 @@ static struct attribute_group mc_attr_group = { .name = "microcode", }; -static void microcode_init_cpu (int cpu) -{ -cpumask_t old = current->cpus_allowed; - -set_cpus_allowed_ptr (current, &cpumask_of_cpu (cpu)); - microcode_ops->microcode_update_cpu (cpu); -set_cpus_allowed_ptr (current, &old); -} -static int mc_sysdev_add (struct sys_device *sys_dev) {int err, cpu = sys_dev->id; -struct ucode_cpu_info *uci = ucode_cpu_info + cpu; +struct ucode_work_info *wi = ucode_work_info + cpu;if (!cpu_online (cpu))return 0 ;pr_debug ("microcode: CPU%d added\n" , cpu); -memset (uci,0 ,sizeof (*uci)); +memset (&wi->uci,0 ,sizeof (wi->uci)); err =sysfs_create_group (&sys_dev->kobj, &mc_attr_group);if (err)return err; -microcode_init_cpu (cpu); +schedule_work_on (cpu, &wi->work);return 0 ; }
@@ -353,12 +352,14 @@ static int mc_sysdev_resume(struct sys_device *dev)static void microcode_set_cpu_frozen (int cpu) { -struct ucode_cpu_info *uci = ucode_cpu_info + cpu; +struct ucode_work_info *wi = ucode_work_info + cpu; + +pr_debug ("microcode: CPU%d frozen\n" , cpu); -mutex_lock (µcode_mutex); -if (uci->valid) - uci->resume =1 ; -mutex_unlock (µcode_mutex); +mutex_lock (µcode_mutex); +if (wi->uci.valid) + wi->resume =1 ; +mutex_unlock (µcode_mutex); }static struct sysdev_driver mc_sysdev_driver = {
@@ -377,6 +378,7 @@ mc_cpu_callback(struct notifier_block *nb, unsigned long action, void *hcpu)switch (action) {case CPU_ONLINE :case CPU_ONLINE_FROZEN : +schedule_work_on (cpu, &ucode_work_info[cpu].work);case CPU_DOWN_FAILED :case CPU_DOWN_FAILED_FROZEN :if (sysfs_create_group (&sys_dev->kobj, &mc_attr_group))
@@ -387,6 +389,7 @@ mc_cpu_callback(struct notifier_block *nb, unsigned long action, void *hcpu)case CPU_DOWN_PREPARE_FROZEN :/* Suspend is in progress, only remove the interface */ sysfs_remove_group (&sys_dev->kobj, &mc_attr_group); +cancel_work_sync (&ucode_work_info[cpu].work);break ;case CPU_DEAD :case CPU_UP_CANCELED_FROZEN :
@@ -406,7 +409,7 @@ static struct notifier_block __refdata mc_cpu_notifier = {int microcode_init (void *opaque,struct module *module) {struct microcode_ops *ops = (struct microcode_ops *)opaque; -int error; +int error, i;if (microcode_ops) {printk (KERN_ERR "microcode: already loaded the other module\n" );
@@ -425,6 +428,10 @@ int microcode_init(void *opaque, struct module *module)return PTR_ERR (microcode_pdev); } +for (i =0 ; i <NR_CPUS ; i++) +INIT_WORK (&ucode_work_info[i].work, + microcode_ops->microcode_update_cpu); +get_online_cpus (); error =sysdev_driver_register (&cpu_sysdev_class, &mc_sysdev_driver);put_online_cpus ();
arch/x86/kernel/microcode_intel.c +45 -29
132 lines not on this page · raw message
include/asm-x86/microcode.h +7 -2
@@ -64,7 +64,6 @@ struct microcode_amd {struct ucode_cpu_info {int valid; -int resume;unsigned int sig;unsigned int pf;unsigned int rev;
@@ -74,9 +73,15 @@ struct ucode_cpu_info { } mc; }; +struct ucode_work_info { +struct ucode_cpu_info uci; +int resume; +struct work_struct work; +}; +struct microcode_ops {long (*get_next_ucode)(void **mc,long offset); -void (*microcode_update_cpu)(int cpu); +void (*microcode_update_cpu)(struct work_struct *w);long (*microcode_get_next_ucode)(void **mc,long offset);int (*get_matching_microcode)(void *mc,int cpu);int (*microcode_sanity_check)(void *mc);
Signature · 1 line
1.5.4.5
Re: [patch 3/5] [PATCH 3/5] x86: Run Intel ucode-updates via workqueue.
On Wed, 6 Aug 2008 17:21:20 +0200
Peter Oruba <[email protected]> wrote:
[ no description or reason ]
Why is this?
I'm not very happy about this.. it means practically that this stuff *has* to run late. Probably later than we want to. (Like.. we may want to redo the microcode during resume.. which is not a schedulable context)
Signature · 3 lines
If you want to reach me at my work email, use [email protected] For development, discussion and tips for power savings, visit http://www.lesswatts.org
Re: [patch 3/5] [PATCH 3/5] x86: Run Intel ucode-updates via workqueue.
2008/8/6 Arjan van de Ven <[email protected]>:
Quoted 6 lines
On Wed, 6 Aug 2008 17:21:20 +0200
Peter Oruba <[email protected]> wrote:[ no description or reason ]
Why is this?
More details are available here.
I've also suggested to do ucode-updates as early as possible from start_secondary() (or whatever low-level code).
The second patch (do via workqueue) was just a proof-of-concept (to show that it does fix a crash).
Quoted 3 lines
I'm not very happy about this.. it means practically that this stuff *has* to run late. Probably later than we want to.
Currently, it runs from cpu-hotplug-notifier(CPU_ONLINE, ...) [ and crashes with .26+ due to a reason you may find via the aforementioned link ] - by this moment, at least kernel threads may running on this cpu -- so it's not that early.
Quoted 2 lines
(Like.. we may want to redo the microcode during resume.. which is not a schedulable context)
I'm not sure what's "schedulable context" here terms but the existing code makes use of set_cpus_allowed_ptr() which means a caller expects to be able to be migrated onto a target cpu and do ucode-update while running on it.
Moreover, the way set_cpus_allowed_ptr() is used there seems to be race wrt. sched_setaffinity().
Signature · 2 lines
Best regards, Dmitry Adamushko
Re: [patch 3/5] [PATCH 3/5] x86: Run Intel ucode-updates via workqueue.
2008/8/6 Dmitry Adamushko <[email protected]>:
Quoted 9 lines
2008/8/6 Arjan van de Ven <[email protected]>:
Quoted 6 lines
On Wed, 6 Aug 2008 17:21:20 +0200
Peter Oruba <[email protected]> wrote:[ no description or reason ]
Why is this?
More details are available here.
forgot a link:
http://lkml.org/lkml/2008/8/3/114
Signature · 2 lines
Best regards, Dmitry Adamushko
Re: [patch 3/5] [PATCH 3/5] x86: Run Intel ucode-updates via workqueue.
In reply to Arjan van de VenQuoted 11 lines · Arjan van de Ven
On Wed, 6 Aug 2008 17:21:20 +0200
Peter Oruba <[email protected]> wrote:[ no description or reason ]
Why is this?
I'm not very happy about this.. it means practically that this stuff *has* to run late. Probably later than we want to. (Like.. we may want to redo the microcode during resume.. which is not a schedulable context)
Dmitry and I tried to figure out how soon does it need to run.
Nobody had a strong argument why it must run synchronously in the hotplug path. Sure we want it as soon as possible and I'd say workqueue is soon enough. Existing hotplug path does not guaranty any ordering and original microcode interface was driven from user-space. So clearly it was not considered very critical.
Max
Re: [patch 3/5] [PATCH 3/5] x86: Run Intel ucode-updates via workqueue.
Quoted 6 lines · On Wed, 06 Aug 2008 13:31:18 -0700 Max Krasnyansky <[email protected]> wrote:
Quoted 5 lines · Arjan van de Ven
On Wed, 6 Aug 2008 17:21:20 +0200
Peter Oruba <[email protected]> wrote:[ no description or reason ]
Quoted 8 lines
Quoted 6 lines
Why is this?
I'm not very happy about this.. it means practically that this stuff *has* to run late. Probably later than we want to. (Like.. we may want to redo the microcode during resume.. which is not a schedulable context)
Dmitry and I tried to figure out how soon does it need to run.
you're not going to like the answer, but it's "as soon as possible". Unlike normal boot, hotplug is a case where the bios hasn't been able to put a normal microcode in.
Quoted 2 lines
Nobody had a strong argument why it must run synchronously in the hotplug path.
Ok we as Intel really want it as early as possible.
Quoted 5 lines
Sure we want it as soon as possible and I'd say
workqueue is soon enough.
Existing hotplug path does not guaranty any ordering and original microcode interface was driven from user-space. So clearly it was not considered very critical.
well that's why it changed to no longer use the userspace driven thing..
Quoted 2 lines
Max
Signature · 3 lines
If you want to reach me at my work email, use [email protected] For development, discussion and tips for power savings, visit http://www.lesswatts.org
Re: [patch 3/5] [PATCH 3/5] x86: Run Intel ucode-updates via workqueue.
In reply to Arjan van de VenQuoted 11 lines · Arjan van de Ven
On Wed, 6 Aug 2008 17:21:20 +0200
Peter Oruba <[email protected]> wrote:[ no description or reason ]
Why is this?
I'm not very happy about this.. it means practically that this stuff *has* to run late. Probably later than we want to. (Like.. we may want to redo the microcode during resume.. which is not a schedulable context)
also, with the hotplug notifier call sequence fixed/changed now, there's no pressing reason to do this change.
Ingo
PATCH 4/5 x86: Minor correction to Intel microcode patch loader.
In reply to Peter Oruba--- arch/x86/kernel/microcode_intel.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
arch/x86/kernel/microcode_intel.c +1 -1
@@ -315,7 +315,7 @@ static void apply_microcode(int cpu)unsigned long flags;unsigned int val[2 ];int cpu_num =raw_smp_processor_id (); -struct ucode_cpu_info *uci =get_ucode_cpu_info (cpu); +struct ucode_cpu_info *uci =get_ucode_cpu_info (cpu_num);/* We should bind the task to the CPU */ BUG_ON (cpu_num != cpu);
Signature · 1 line
1.5.4.5
Re: [patch 4/5] [PATCH 4/5] x86: Minor correction to Intel microcode patch loader.
Quoted 10 lines · Peter Oruba
Signed-off-by Peter Oruba <[email protected]>--- arch/x86/kernel/microcode_intel.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)arch/x86/kernel/microcode_intel.c +0
-0
arch/x86/kernel/microcode_intel.c static void apply_microcode(int cpu) · line 318
Quoted 5 lines
arch/x86/kernel/microcode_intel.c +1
-1@@ -315,7 +315,7 @@ static void apply_microcode(int cpu)unsigned long flags;unsigned int val[2 ];int cpu_num =raw_smp_processor_id (); -struct ucode_cpu_info *uci =get_ucode_cpu_info (cpu); +struct ucode_cpu_info *uci =get_ucode_cpu_info (cpu_num);
hm, this does not apply to tip/x86/microcode nor tip/master nor linus/master. Does it fix a bug in one of the earlier patches in this 5/5 queue?
Ingo
Re: [patch 4/5] [PATCH 4/5] x86: Minor correction to Intel microcode patch loader.
Quoted 20 lines · Ingo Molnar
Quoted 10 lines · Peter Oruba
Signed-off-by Peter Oruba <[email protected]>--- arch/x86/kernel/microcode_intel.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)arch/x86/kernel/microcode_intel.c +0
-0arch/x86/kernel/microcode_intel.c static void apply_microcode(int cpu) · line 318
Quoted 5 lines
arch/x86/kernel/microcode_intel.c +1
-1@@ -315,7 +315,7 @@ static void apply_microcode(int cpu)unsigned long flags;unsigned int val[2 ];int cpu_num =raw_smp_processor_id (); -struct ucode_cpu_info *uci =get_ucode_cpu_info (cpu); +struct ucode_cpu_info *uci =get_ucode_cpu_info (cpu_num);hm, this does not apply to tip/x86/microcode nor tip/master nor linus/master. Does it fix a bug in one of the earlier patches in this 5/5 queue?
Quoted 2 lines
Ingo
Yes, it's a fix for PATCH 3/5
-Peter
Signature · 6 lines
| AMD Saxony Limited Liability Company & Co. KG
Operating | Wilschdorfer Landstr. 101, 01109 Dresden, Germany
System | Register Court Dresden: HRA 4896
Research | General Partner authorized to represent:
Center | AMD Saxony LLC (Wilmington, Delaware, US)
| General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoyPATCH 5/5 x86: Run AMD microcode updates via workqueue.
In reply to Peter Oruba--- arch/x86/kernel/microcode_amd.c | 106 ++++++++++++++++++++++++-------------- 1 files changed, 67 insertions(+), 39 deletions(-)
arch/x86/kernel/microcode_amd.c +67 -39
@@ -14,6 +14,7 @@* License version 2. See file COPYING for details. */ +/* #define DEBUG */ /* pr_debug */ #include <linux/capability.h> #include <linux/kernel.h> #include <linux/init.h>
@@ -40,7 +41,7 @@MODULE_DESCRIPTION ("AMD Microcode Update Driver" );MODULE_AUTHOR ("Peter Oruba <[email protected]>" ); -MODULE_LICENSE ("GPL v2" ); +MODULE_LICENSE ("GPL" );#define UCODE_MAGIC 0x00414d44#define UCODE_EQUIV_CPU_TABLE_TYPE 0x00000000
@@ -65,11 +66,16 @@ extern struct mutex (microcode_mutex);struct equiv_cpu_entry *equiv_cpu_table;extern struct ucode_cpu_info ucode_cpu_info[NR_CPUS ]; +extern struct ucode_work_info ucode_work_info[NR_CPUS ]; -static void collect_cpu_info_amd (int cpu) +static inline struct ucode_cpu_info *get_ucode_cpu_info (int cpu) +{ +return &ucode_work_info[cpu].uci; +} + +static void collect_cpu_info_amd (int cpu,struct ucode_cpu_info *uci) {struct cpuinfo_x86 *c = &cpu_data (cpu); -struct ucode_cpu_info *uci = ucode_cpu_info + cpu;/* We should bind the task to the CPU */ BUG_ON (raw_smp_processor_id () != cpu);
@@ -95,7 +101,7 @@ static void collect_cpu_info_amd(int cpu)static int get_matching_microcode_amd (void *mc,int cpu) { -struct ucode_cpu_info *uci = ucode_cpu_info + cpu; +struct ucode_cpu_info *uci =get_ucode_cpu_info (cpu);struct microcode_header_amd *mc_header = mc;unsigned long total_size =get_totalsize (mc_header);void *new_mc;
@@ -215,7 +221,7 @@ static void apply_microcode_amd(int cpu)unsigned int eax, edx;unsigned int rev;int cpu_num =raw_smp_processor_id (); -struct ucode_cpu_info *uci = ucode_cpu_info + cpu_num; +struct ucode_cpu_info *uci =get_ucode_cpu_info (cpu_num);/* We should bind the task to the CPU */ BUG_ON (cpu_num != cpu);
@@ -437,61 +443,83 @@ static int cpu_request_microcode_amd(int cpu)return error; } -static int apply_microcode_check_cpu_amd (int cpu) +static void microcode_fini_cpu_amd (int cpu) { -struct cpuinfo_x86 *c = &cpu_data (cpu); -struct ucode_cpu_info *uci = ucode_cpu_info + cpu; -unsigned int rev; -cpumask_t old; -int err =0 ; +struct ucode_work_info *wi = ucode_work_info + cpu; -/* Check if the microcode is available */ -if (!uci->mc.mc_amd) -return 0 ; +mutex_lock (µcode_mutex); + wi->uci.valid =0 ; +vfree (wi->uci.mc.mc_amd); + wi->uci.mc.mc_amd =NULL ; + wi->resume =0 ; +mutex_unlock (µcode_mutex); +} + +static int microcode_resume_cpu (int cpu,struct ucode_cpu_info *new_uci) +{ +struct ucode_work_info *wi = ucode_work_info + cpu; +struct ucode_cpu_info *uci = &wi->uci; - old = current->cpus_allowed; -set_cpus_allowed_ptr (current, &cpumask_of_cpu (cpu)); +if (!wi->resume) +return 0 ; -/* Check if the microcode we have in memory matches the CPU */ -if (c->x86_vendor !=X86_VENDOR_AMD || c->x86 <16 ) - err = -EINVAL ; +pr_debug ("microcode: CPU%d resumed (ucode: %p)\n" , + cpu, uci->mc.mc_amd); -if (!err) { - asmvolatile ("movl %1, %%ecx; rdmsr" - :"=a" (rev) - :"i" (0x0000008B ) :"ecx" ); + wi->resume =0 ; -if (uci->rev != rev) - err = -EINVAL ; - } +/* check if the 'cached' microcode is ok: */ +if (!uci->mc.mc_amd) +return 1 ; -if (!err) +if (new_uci->sig == uci->sig && + new_uci->pf == uci->pf && + new_uci->rev == uci->rev) {apply_microcode_amd (cpu); -else -printk (KERN_ERR "microcode: Could not apply microcode to CPU%d:" -" rev=0x%x\n" , - cpu, uci->rev); + }else { +microcode_fini_cpu_amd (cpu); + *uci = *new_uci; + } -set_cpus_allowed (current, old); -return err; +return 1 ; } -static void microcode_fini_cpu_amd (int cpu) +static void microcode_update_cpu_amd (struct work_struct *w) { -struct ucode_cpu_info *uci = ucode_cpu_info + cpu; + struct ucode_work_info *wi = container_of(w, struct ucode_work_info, + work); +int cpu =raw_smp_processor_id ();/* keventd is per-cpu */ +struct ucode_cpu_info new_uci; + +pr_debug ("microcode: CPU%d workqueue\n" , cpu); + +memset (&new_uci,0 ,sizeof (new_uci));mutex_lock (µcode_mutex); - uci->valid =0 ; -vfree (uci->mc.mc_amd); - uci->mc.mc_amd =NULL ; + +collect_cpu_info_amd (cpu, &new_uci); + +if (new_uci.valid) { +/* +* Check if the system resume is in progress, +* otherwise just request a firmware: +*/ +if (!microcode_resume_cpu (cpu, &new_uci)) { + wi->uci = new_uci; + +if (system_state ==SYSTEM_RUNNING ) +cpu_request_microcode_amd (cpu); + } + } +mutex_unlock (µcode_mutex); }static struct microcode_ops microcode_amd_ops = { .get_next_ucode = get_next_ucode_amd, + .microcode_update_cpu = microcode_update_cpu_amd, .get_matching_microcode = get_matching_microcode_amd, .microcode_sanity_check =NULL , - .apply_microcode_check_cpu = apply_microcode_check_cpu_amd, .cpu_request_microcode = cpu_request_microcode_amd, .collect_cpu_info = collect_cpu_info_amd, .apply_microcode = apply_microcode_amd,
Signature · 1 line
1.5.4.5
Hi Peter,
Quoted 3 lines
Rebased Dmitry's patch set to x86-microcode and incorporated changes into AMD microcode patch loader. No issues with cpu-hotplug anymore, tested on Intel as well as AMD.
btw., I still think we should consider running ucode-updates from the low-level arch code. But as it's perhaps a material for .28, there should be plenty of time. I also have in mind a few possible generalizations/improvemets.
I'll base them on top of your recent patches.
Quoted 3 lines
--Peter
Signature · 2 lines
Best regards, Dmitry Adamushko
Quoted 8 lines · Dmitry Adamushko
Hi Peter,
Quoted 2 lines
Rebased Dmitry's patch set to x86-microcode and incorporated changes into AMD microcode patch loader. No issues with cpu-hotplug anymore, tested on Intel as well as AMD.
btw., I still think we should consider running ucode-updates from the low-level arch code. But as it's perhaps a material for .28, there should be plenty of time.
That'd be ok. Somehow I think it's not worth the effort though. If the machine was able to boot to the point that it can run user-space (request_firmware) with the old microcode it can probably sustain itself a bit longer while waiting for the workqueue thread to be scheduled. You probably have more exciting things to do ;-)
The latest patches look ok btw.
Max