|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-changelog] [xen staging] x86/microcode: split out apply_microcode() from cpu_request_microcode()
commit 220f89505e77d716c412c8a7f8b20b3684b00160
Author: Chao Gao <chao.gao@xxxxxxxxx>
AuthorDate: Fri Sep 27 14:11:35 2019 +0200
Commit: Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Fri Sep 27 14:11:35 2019 +0200
x86/microcode: split out apply_microcode() from cpu_request_microcode()
During late microcode loading, apply_microcode() is invoked in
cpu_request_microcode(). To make late microcode update more reliable,
we want to put the apply_microcode() into stop_machine context. So
we split out it from cpu_request_microcode(). In general, for both
early loading on BSP and late loading, cpu_request_microcode() is
called first to get the matching microcode update contained by
the blob and then apply_microcode() is invoked explicitly on each
cpu in common code.
Given that all CPUs are supposed to have the same signature, parsing
microcode only needs to be done once. So cpu_request_microcode() is
also moved out of microcode_update_cpu().
In some cases (e.g. a broken bios), the system may have multiple
revisions of microcode update. So we would try to load a microcode
update as long as it covers current cpu. And if a cpu loads this patch
successfully, the patch would be stored into the patch cache.
Note that calling ->apply_microcode() itself doesn't require any
lock being held. But the parameter passed to it may be protected
by some locks. E.g. microcode_update_cpu() acquires microcode_mutex
to avoid microcode_cache being updated by others.
Signed-off-by: Chao Gao <chao.gao@xxxxxxxxx>
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
---
xen/arch/x86/microcode.c | 173 +++++++++++++++++++++++++++-------------
xen/arch/x86/microcode_amd.c | 35 ++++----
xen/arch/x86/microcode_intel.c | 61 +++++++-------
xen/include/asm-x86/microcode.h | 5 +-
4 files changed, 163 insertions(+), 111 deletions(-)
diff --git a/xen/arch/x86/microcode.c b/xen/arch/x86/microcode.c
index b44e4d7a66..3ea2a6e07d 100644
--- a/xen/arch/x86/microcode.c
+++ b/xen/arch/x86/microcode.c
@@ -189,12 +189,19 @@ static DEFINE_SPINLOCK(microcode_mutex);
DEFINE_PER_CPU(struct cpu_signature, cpu_sig);
-struct microcode_info {
- unsigned int cpu;
- uint32_t buffer_size;
- int error;
- char buffer[1];
-};
+/*
+ * Return a patch that covers current CPU. If there are multiple patches,
+ * return the one with the highest revision number. Return error If no
+ * patch is found and an error occurs during the parsing process. Otherwise
+ * return NULL.
+ */
+static struct microcode_patch *parse_blob(const char *buf, size_t len)
+{
+ if ( likely(!microcode_ops->collect_cpu_info(&this_cpu(cpu_sig))) )
+ return microcode_ops->cpu_request_microcode(buf, len);
+
+ return NULL;
+}
int microcode_resume_cpu(void)
{
@@ -220,15 +227,8 @@ void microcode_free_patch(struct microcode_patch
*microcode_patch)
xfree(microcode_patch);
}
-const struct microcode_patch *microcode_get_cache(void)
-{
- ASSERT(spin_is_locked(µcode_mutex));
-
- return microcode_cache;
-}
-
/* Return true if cache gets updated. Otherwise, return false */
-bool microcode_update_cache(struct microcode_patch *patch)
+static bool microcode_update_cache(struct microcode_patch *patch)
{
ASSERT(spin_is_locked(µcode_mutex));
@@ -249,49 +249,82 @@ bool microcode_update_cache(struct microcode_patch *patch)
return true;
}
-static int microcode_update_cpu(const void *buf, size_t size)
+/*
+ * Load a microcode update to current CPU.
+ *
+ * If no patch is provided, the cached patch will be loaded. Microcode update
+ * during APs bringup and CPU resuming falls into this case.
+ */
+static int microcode_update_cpu(const struct microcode_patch *patch)
{
- int err;
- unsigned int cpu = smp_processor_id();
- struct cpu_signature *sig = &per_cpu(cpu_sig, cpu);
+ int err = microcode_ops->collect_cpu_info(&this_cpu(cpu_sig));
- spin_lock(µcode_mutex);
+ if ( unlikely(err) )
+ return err;
- err = microcode_ops->collect_cpu_info(sig);
- if ( likely(!err) )
- err = microcode_ops->cpu_request_microcode(buf, size);
+ spin_lock(µcode_mutex);
+ if ( patch )
+ err = microcode_ops->apply_microcode(patch);
+ else if ( microcode_cache )
+ {
+ err = microcode_ops->apply_microcode(microcode_cache);
+ if ( err == -EIO )
+ {
+ microcode_free_patch(microcode_cache);
+ microcode_cache = NULL;
+ }
+ }
+ else
+ /* No patch to update */
+ err = -ENOENT;
spin_unlock(µcode_mutex);
return err;
}
-static long do_microcode_update(void *_info)
+static long do_microcode_update(void *patch)
{
- struct microcode_info *info = _info;
- int error;
-
- BUG_ON(info->cpu != smp_processor_id());
+ unsigned int cpu;
+ int ret = microcode_update_cpu(patch);
- error = microcode_update_cpu(info->buffer, info->buffer_size);
- if ( error )
- info->error = error;
+ /* Store the patch after a successful loading */
+ if ( !ret && patch )
+ {
+ spin_lock(µcode_mutex);
+ microcode_update_cache(patch);
+ spin_unlock(µcode_mutex);
+ patch = NULL;
+ }
if ( microcode_ops->end_update_percpu )
microcode_ops->end_update_percpu();
- info->cpu = cpumask_next(info->cpu, &cpu_online_map);
- if ( info->cpu < nr_cpu_ids )
- return continue_hypercall_on_cpu(info->cpu, do_microcode_update, info);
+ /*
+ * Each thread tries to load ucode. Only the first thread of a core
+ * would succeed while other threads would encounter -EINVAL which
+ * indicates current ucode revision is equal to or newer than the
+ * given patch. It is actually expected; so ignore this error.
+ */
+ if ( ret == -EINVAL )
+ ret = 0;
+
+ cpu = cpumask_next(smp_processor_id(), &cpu_online_map);
+ if ( cpu < nr_cpu_ids )
+ return continue_hypercall_on_cpu(cpu, do_microcode_update, patch) ?:
+ ret;
+
+ /* Free the patch if no CPU has loaded it successfully. */
+ if ( patch )
+ microcode_free_patch(patch);
- error = info->error;
- xfree(info);
- return error;
+ return ret;
}
int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void) buf, unsigned long len)
{
int ret;
- struct microcode_info *info;
+ void *buffer;
+ struct microcode_patch *patch;
if ( len != (uint32_t)len )
return -E2BIG;
@@ -299,32 +332,41 @@ int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void)
buf, unsigned long len)
if ( microcode_ops == NULL )
return -EINVAL;
- info = xmalloc_bytes(sizeof(*info) + len);
- if ( info == NULL )
+ buffer = xmalloc_bytes(len);
+ if ( !buffer )
return -ENOMEM;
- ret = copy_from_guest(info->buffer, buf, len);
- if ( ret != 0 )
+ ret = copy_from_guest(buffer, buf, len);
+ if ( ret )
+ {
+ xfree(buffer);
+ return -EFAULT;
+ }
+
+ patch = parse_blob(buffer, len);
+ xfree(buffer);
+ if ( IS_ERR(patch) )
{
- xfree(info);
+ ret = PTR_ERR(patch);
+ printk(XENLOG_WARNING "Parsing microcode blob error %d\n", ret);
return ret;
}
- info->buffer_size = len;
- info->error = 0;
- info->cpu = cpumask_first(&cpu_online_map);
+ if ( !patch )
+ return -ENOENT;
if ( microcode_ops->start_update )
{
ret = microcode_ops->start_update();
if ( ret != 0 )
{
- xfree(info);
+ microcode_free_patch(patch);
return ret;
}
}
- return continue_hypercall_on_cpu(info->cpu, do_microcode_update, info);
+ return continue_hypercall_on_cpu(cpumask_first(&cpu_online_map),
+ do_microcode_update, patch);
}
static int __init microcode_init(void)
@@ -371,23 +413,42 @@ int __init early_microcode_update_cpu(bool start_update)
microcode_ops->collect_cpu_info(&this_cpu(cpu_sig));
- if ( data )
+ if ( !data )
+ return -ENOMEM;
+
+ if ( start_update )
{
- if ( start_update && microcode_ops->start_update )
+ struct microcode_patch *patch;
+
+ patch = parse_blob(data, len);
+ if ( IS_ERR(patch) )
+ {
+ printk(XENLOG_WARNING "Parsing microcode blob error %ld\n",
+ PTR_ERR(patch));
+ return PTR_ERR(patch);
+ }
+
+ if ( !patch )
+ return -ENOENT;
+
+ spin_lock(µcode_mutex);
+ rc = microcode_update_cache(patch);
+ spin_unlock(µcode_mutex);
+ ASSERT(rc);
+
+ if ( microcode_ops->start_update )
rc = microcode_ops->start_update();
if ( rc )
return rc;
+ }
- rc = microcode_update_cpu(data, len);
+ rc = microcode_update_cpu(NULL);
- if ( microcode_ops->end_update_percpu )
- microcode_ops->end_update_percpu();
+ if ( microcode_ops->end_update_percpu )
+ microcode_ops->end_update_percpu();
- return rc;
- }
- else
- return -ENOMEM;
+ return rc;
}
int __init early_microcode_init(void)
diff --git a/xen/arch/x86/microcode_amd.c b/xen/arch/x86/microcode_amd.c
index 8e4cdabb3c..f6a5fcc793 100644
--- a/xen/arch/x86/microcode_amd.c
+++ b/xen/arch/x86/microcode_amd.c
@@ -455,9 +455,11 @@ static bool_t check_final_patch_levels(unsigned int cpu)
return 0;
}
-static int cpu_request_microcode(const void *buf, size_t bufsize)
+static struct microcode_patch *cpu_request_microcode(const void *buf,
+ size_t bufsize)
{
struct microcode_amd *mc_amd;
+ struct microcode_patch *patch = NULL;
size_t offset = 0;
int error = 0;
unsigned int current_cpu_id;
@@ -556,18 +558,16 @@ static int cpu_request_microcode(const void *buf, size_t
bufsize)
break;
}
- /* Update cache if this patch covers current CPU */
- if ( microcode_fits(new_patch->mc_amd) != MIS_UCODE )
- microcode_update_cache(new_patch);
- else
- microcode_free_patch(new_patch);
+ /*
+ * If the new patch covers current CPU, compare patches and store the
+ * one with higher revision.
+ */
+ if ( (microcode_fits(new_patch->mc_amd) != MIS_UCODE) &&
+ (!patch || (compare_patch(new_patch, patch) == NEW_UCODE)) )
+ SWAP(patch, new_patch);
- if ( match_cpu(microcode_get_cache()) )
- {
- error = apply_microcode(microcode_get_cache());
- if ( error )
- break;
- }
+ if ( new_patch )
+ microcode_free_patch(new_patch);
if ( offset >= bufsize )
break;
@@ -599,13 +599,10 @@ static int cpu_request_microcode(const void *buf, size_t
bufsize)
free_patch(mc_amd);
out:
- /*
- * In some cases we may return an error even if processor's microcode has
- * been updated. For example, the first patch in a container file is loaded
- * successfully but subsequent container file processing encounters a
- * failure.
- */
- return error;
+ if ( error && !patch )
+ patch = ERR_PTR(error);
+
+ return patch;
}
#ifdef CONFIG_HVM
diff --git a/xen/arch/x86/microcode_intel.c b/xen/arch/x86/microcode_intel.c
index 23197cacbe..a7125d8a19 100644
--- a/xen/arch/x86/microcode_intel.c
+++ b/xen/arch/x86/microcode_intel.c
@@ -285,14 +285,9 @@ static enum microcode_match_result compare_patch(
: OLD_UCODE;
}
-/*
- * return 0 - no update found
- * return 1 - found update
- * return < 0 - error
- */
-static int get_matching_microcode(const void *mc)
+static struct microcode_patch *alloc_microcode_patch(
+ const struct microcode_header_intel *mc_header)
{
- const struct microcode_header_intel *mc_header = mc;
unsigned long total_size = get_totalsize(mc_header);
void *new_mc = xmalloc_bytes(total_size);
struct microcode_patch *new_patch = xmalloc(struct microcode_patch);
@@ -301,25 +296,12 @@ static int get_matching_microcode(const void *mc)
{
xfree(new_patch);
xfree(new_mc);
- return -ENOMEM;
+ return ERR_PTR(-ENOMEM);
}
- memcpy(new_mc, mc, total_size);
+ memcpy(new_mc, mc_header, total_size);
new_patch->mc_intel = new_mc;
- /* Make sure that this patch covers current CPU */
- if ( microcode_update_match(mc) == MIS_UCODE )
- {
- microcode_free_patch(new_patch);
- return 0;
- }
-
- microcode_update_cache(new_patch);
-
- pr_debug("microcode: CPU%d found a matching microcode update with"
- " version %#x (current=%#x)\n",
- smp_processor_id(), mc_header->rev, this_cpu(cpu_sig).rev);
-
- return 1;
+ return new_patch;
}
static int apply_microcode(const struct microcode_patch *patch)
@@ -398,26 +380,39 @@ static long get_next_ucode_from_buffer(void **mc, const
u8 *buf,
return offset + total_size;
}
-static int cpu_request_microcode(const void *buf, size_t size)
+static struct microcode_patch *cpu_request_microcode(const void *buf,
+ size_t size)
{
long offset = 0;
int error = 0;
void *mc;
+ struct microcode_patch *patch = NULL;
while ( (offset = get_next_ucode_from_buffer(&mc, buf, size, offset)) > 0 )
{
+ struct microcode_patch *new_patch;
+
error = microcode_sanity_check(mc);
if ( error )
break;
- error = get_matching_microcode(mc);
- if ( error < 0 )
+
+ new_patch = alloc_microcode_patch(mc);
+ if ( IS_ERR(new_patch) )
+ {
+ error = PTR_ERR(new_patch);
break;
+ }
+
/*
- * It's possible the data file has multiple matching ucode,
- * lets keep searching till the latest version
+ * If the new patch covers current CPU, compare patches and store the
+ * one with higher revision.
*/
- if ( error == 1 )
- error = 0;
+ if ( (microcode_update_match(&new_patch->mc_intel->hdr) != MIS_UCODE)
&&
+ (!patch || (compare_patch(new_patch, patch) == NEW_UCODE)) )
+ SWAP(patch, new_patch);
+
+ if ( new_patch )
+ microcode_free_patch(new_patch);
xfree(mc);
}
@@ -426,10 +421,10 @@ static int cpu_request_microcode(const void *buf, size_t
size)
if ( offset < 0 )
error = offset;
- if ( !error && match_cpu(microcode_get_cache()) )
- error = apply_microcode(microcode_get_cache());
+ if ( error && !patch )
+ patch = ERR_PTR(error);
- return error;
+ return patch;
}
static const struct microcode_ops microcode_intel_ops = {
diff --git a/xen/include/asm-x86/microcode.h b/xen/include/asm-x86/microcode.h
index 02feb09e8a..7d5a1f8e8a 100644
--- a/xen/include/asm-x86/microcode.h
+++ b/xen/include/asm-x86/microcode.h
@@ -20,7 +20,8 @@ struct microcode_patch {
};
struct microcode_ops {
- int (*cpu_request_microcode)(const void *buf, size_t size);
+ struct microcode_patch *(*cpu_request_microcode)(const void *buf,
+ size_t size);
int (*collect_cpu_info)(struct cpu_signature *csig);
int (*apply_microcode)(const struct microcode_patch *patch);
int (*start_update)(void);
@@ -40,8 +41,6 @@ struct cpu_signature {
DECLARE_PER_CPU(struct cpu_signature, cpu_sig);
extern const struct microcode_ops *microcode_ops;
-const struct microcode_patch *microcode_get_cache(void);
-bool microcode_update_cache(struct microcode_patch *patch);
void microcode_free_patch(struct microcode_patch *patch);
#endif /* ASM_X86__MICROCODE_H */
--
generated by git-patchbot for /home/xen/git/xen.git#staging
_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/xen-changelog
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |