From 6c53cbfced048c421e4f72cb2183465f68fbc5e7 Mon Sep 17 00:00:00 2001 From: Borislav Petkov Date: Thu, 6 Jan 2011 16:56:51 +0100 Subject: x86, microcode: Correct sysdev_add error path When we encounter an error while initting the microcode driver on a CPU, we must undo the previously added sysfs group. Cc: Tigran Aivazian Signed-off-by: Borislav Petkov Acked-by: Andreas Herrmann diff --git a/arch/x86/kernel/microcode_core.c b/arch/x86/kernel/microcode_core.c index 1cca374..87af68e 100644 --- a/arch/x86/kernel/microcode_core.c +++ b/arch/x86/kernel/microcode_core.c @@ -417,8 +417,10 @@ static int mc_sysdev_add(struct sys_device *sys_dev) if (err) return err; - if (microcode_init_cpu(cpu) == UCODE_ERROR) - err = -EINVAL; + if (microcode_init_cpu(cpu) == UCODE_ERROR) { + sysfs_remove_group(&sys_dev->kobj, &mc_attr_group); + return -EINVAL; + } return err; } -- cgit v0.10.2 From ffc7e8ac820bf9dd6106b01d3e64fecb5177cf43 Mon Sep 17 00:00:00 2001 From: Borislav Petkov Date: Thu, 30 Dec 2010 21:06:01 +0100 Subject: x86, microcode, AMD: Release firmware on error When the ucode magic is wrong, for whatever reason, we don't release the loaded firmware binary and its related resources. Make sure we do. Also, fix function naming to fit this driver's convention and shorten variable names. Signed-off-by: Borislav Petkov Acked-by: Andreas Herrmann diff --git a/arch/x86/kernel/microcode_amd.c b/arch/x86/kernel/microcode_amd.c index 0fe6d1a..ef91df0 100644 --- a/arch/x86/kernel/microcode_amd.c +++ b/arch/x86/kernel/microcode_amd.c @@ -278,27 +278,29 @@ generic_load_microcode(int cpu, const u8 *data, size_t size) return state; } -static enum ucode_state request_microcode_fw(int cpu, struct device *device) +static enum ucode_state request_microcode_amd(int cpu, struct device *device) { const char *fw_name = "amd-ucode/microcode_amd.bin"; - const struct firmware *firmware; - enum ucode_state ret; + const struct firmware *fw; + enum ucode_state ret = UCODE_NFOUND; - if (request_firmware(&firmware, fw_name, device)) { + if (request_firmware(&fw, fw_name, device)) { printk(KERN_ERR "microcode: failed to load file %s\n", fw_name); - return UCODE_NFOUND; + goto out; } - if (*(u32 *)firmware->data != UCODE_MAGIC) { - pr_err("invalid UCODE_MAGIC (0x%08x)\n", - *(u32 *)firmware->data); - return UCODE_ERROR; + ret = UCODE_ERROR; + if (*(u32 *)fw->data != UCODE_MAGIC) { + pr_err("Invalid UCODE_MAGIC (0x%08x)\n", *(u32 *)fw->data); + goto fw_release; } - ret = generic_load_microcode(cpu, firmware->data, firmware->size); + ret = generic_load_microcode(cpu, fw->data, fw->size); - release_firmware(firmware); +fw_release: + release_firmware(fw); +out: return ret; } @@ -319,7 +321,7 @@ static void microcode_fini_cpu_amd(int cpu) static struct microcode_ops microcode_amd_ops = { .request_microcode_user = request_microcode_user, - .request_microcode_fw = request_microcode_fw, + .request_microcode_fw = request_microcode_amd, .collect_cpu_info = collect_cpu_info_amd, .apply_microcode = apply_microcode_amd, .microcode_fini_cpu = microcode_fini_cpu_amd, -- cgit v0.10.2 From 10de52d6655ef0d4a1b8d2804db30208c26601ed Mon Sep 17 00:00:00 2001 From: Borislav Petkov Date: Thu, 30 Dec 2010 22:10:12 +0100 Subject: x86, microcode, AMD: Simplify install_equiv_cpu_table There's no need to memcpy the ucode header in order to look at it only in this function - use the original buffer instead. Also, fix return type semantics by returning a negative value on error and a positive otherwise. Signed-off-by: Borislav Petkov Acked-by: Andreas Herrmann diff --git a/arch/x86/kernel/microcode_amd.c b/arch/x86/kernel/microcode_amd.c index ef91df0..9a451d7 100644 --- a/arch/x86/kernel/microcode_amd.c +++ b/arch/x86/kernel/microcode_amd.c @@ -188,27 +188,22 @@ get_next_ucode(const u8 *buf, unsigned int size, unsigned int *mc_size) static int install_equiv_cpu_table(const u8 *buf) { - u8 *container_hdr[UCODE_CONTAINER_HEADER_SIZE]; - unsigned int *buf_pos = (unsigned int *)container_hdr; - unsigned long size; + unsigned int *ibuf = (unsigned int *)buf; + unsigned int type = ibuf[1]; + unsigned int size = ibuf[2]; - get_ucode_data(&container_hdr, buf, UCODE_CONTAINER_HEADER_SIZE); - - size = buf_pos[2]; - - if (buf_pos[1] != UCODE_EQUIV_CPU_TABLE_TYPE || !size) { + if (type != UCODE_EQUIV_CPU_TABLE_TYPE || !size) { pr_err("error: invalid type field in container file section header\n"); - return 0; + return -EINVAL; } equiv_cpu_table = vmalloc(size); if (!equiv_cpu_table) { pr_err("failed to allocate equivalent CPU table\n"); - return 0; + return -ENOMEM; } - buf += UCODE_CONTAINER_HEADER_SIZE; - get_ucode_data(equiv_cpu_table, buf, size); + get_ucode_data(equiv_cpu_table, buf + UCODE_CONTAINER_HEADER_SIZE, size); return size + UCODE_CONTAINER_HEADER_SIZE; /* add header length */ } @@ -232,7 +227,7 @@ generic_load_microcode(int cpu, const u8 *data, size_t size) enum ucode_state state = UCODE_OK; offset = install_equiv_cpu_table(ucode_ptr); - if (!offset) { + if (offset < 0) { pr_err("failed to create equivalent cpu table\n"); return UCODE_ERROR; } -- cgit v0.10.2 From 7cc27349cbfec271eecec9488b4bf3f3fadb2ce4 Mon Sep 17 00:00:00 2001 From: Borislav Petkov Date: Fri, 31 Dec 2010 16:57:48 +0100 Subject: x86, microcode, AMD: Simplify get_next_ucode Do not copy the section header but look at it directly through the pointer. Also, make it return a ptr to a ucode header directly thus dropping a bunch of unneeded casts. Finally, simplify generic_load_microcode(), while at it. Signed-off-by: Borislav Petkov Acked-by: Andreas Herrmann diff --git a/arch/x86/kernel/microcode_amd.c b/arch/x86/kernel/microcode_amd.c index 9a451d7..8b58fc1 100644 --- a/arch/x86/kernel/microcode_amd.c +++ b/arch/x86/kernel/microcode_amd.c @@ -88,9 +88,9 @@ static int collect_cpu_info_amd(int cpu, struct cpu_signature *csig) return 0; } -static int get_matching_microcode(int cpu, void *mc, int rev) +static int get_matching_microcode(int cpu, struct microcode_header_amd *mc_hdr, + int rev) { - struct microcode_header_amd *mc_header = mc; unsigned int current_cpu_id; u16 equiv_cpu_id = 0; unsigned int i = 0; @@ -109,17 +109,17 @@ static int get_matching_microcode(int cpu, void *mc, int rev) if (!equiv_cpu_id) return 0; - if (mc_header->processor_rev_id != equiv_cpu_id) + if (mc_hdr->processor_rev_id != equiv_cpu_id) return 0; /* ucode might be chipset specific -- currently we don't support this */ - if (mc_header->nb_dev_id || mc_header->sb_dev_id) { + if (mc_hdr->nb_dev_id || mc_hdr->sb_dev_id) { pr_err("CPU%d: loading of chipset specific code not yet supported\n", cpu); return 0; } - if (mc_header->patch_id <= rev) + if (mc_hdr->patch_id <= rev) return 0; return 1; @@ -155,21 +155,18 @@ static int apply_microcode_amd(int cpu) return 0; } -static void * +static struct microcode_header_amd * get_next_ucode(const u8 *buf, unsigned int size, unsigned int *mc_size) { + struct microcode_header_amd *mc; unsigned int total_size; - u8 section_hdr[UCODE_CONTAINER_SECTION_HDR]; - void *mc; - get_ucode_data(section_hdr, buf, UCODE_CONTAINER_SECTION_HDR); - - if (section_hdr[0] != UCODE_UCODE_TYPE) { + if (buf[0] != UCODE_UCODE_TYPE) { pr_err("error: invalid type field in container file section header\n"); return NULL; } - total_size = (unsigned long) (section_hdr[4] + (section_hdr[5] << 8)); + total_size = buf[4] + (buf[5] << 8); if (total_size > size || total_size > UCODE_MAX_SIZE) { pr_err("error: size mismatch\n"); @@ -218,12 +215,12 @@ static enum ucode_state generic_load_microcode(int cpu, const u8 *data, size_t size) { struct ucode_cpu_info *uci = ucode_cpu_info + cpu; + struct microcode_header_amd *mc_hdr = NULL; + unsigned int mc_size, leftover; + unsigned long offset; const u8 *ucode_ptr = data; void *new_mc = NULL; - void *mc; int new_rev = uci->cpu_sig.rev; - unsigned int leftover; - unsigned long offset; enum ucode_state state = UCODE_OK; offset = install_equiv_cpu_table(ucode_ptr); @@ -236,38 +233,37 @@ generic_load_microcode(int cpu, const u8 *data, size_t size) leftover = size - offset; while (leftover) { - unsigned int uninitialized_var(mc_size); - struct microcode_header_amd *mc_header; - - mc = get_next_ucode(ucode_ptr, leftover, &mc_size); - if (!mc) + mc_hdr = get_next_ucode(ucode_ptr, leftover, &mc_size); + if (!mc_hdr) break; - mc_header = (struct microcode_header_amd *)mc; - if (get_matching_microcode(cpu, mc, new_rev)) { + if (get_matching_microcode(cpu, mc_hdr, new_rev)) { vfree(new_mc); - new_rev = mc_header->patch_id; - new_mc = mc; + new_rev = mc_hdr->patch_id; + new_mc = mc_hdr; } else - vfree(mc); + vfree(mc_hdr); ucode_ptr += mc_size; leftover -= mc_size; } - if (new_mc) { - if (!leftover) { - vfree(uci->mc); - uci->mc = new_mc; - pr_debug("CPU%d found a matching microcode update with version 0x%x (current=0x%x)\n", - cpu, new_rev, uci->cpu_sig.rev); - } else { - vfree(new_mc); - state = UCODE_ERROR; - } - } else + if (!new_mc) { state = UCODE_NFOUND; + goto free_table; + } + + if (!leftover) { + vfree(uci->mc); + uci->mc = new_mc; + pr_debug("CPU%d update ucode to version 0x%x (from 0x%x)\n", + cpu, new_rev, uci->cpu_sig.rev); + } else { + vfree(new_mc); + state = UCODE_ERROR; + } +free_table: free_equiv_cpu_table(); return state; -- cgit v0.10.2 From 05ff02e4c0686051fcb074aec92df03f2c184fd1 Mon Sep 17 00:00:00 2001 From: Borislav Petkov Date: Wed, 5 Jan 2011 18:04:11 +0100 Subject: x86, microcode, AMD: Remove unneeded memset call collect_cpu_info_amd() clears its csig arg but this is done in the microcode_core's collect_cpu_info() by clearing the embedding struct ucode_cpu_info. Drop it. Signed-off-by: Borislav Petkov Acked-by: Andreas Herrmann diff --git a/arch/x86/kernel/microcode_amd.c b/arch/x86/kernel/microcode_amd.c index 8b58fc1..274fa40 100644 --- a/arch/x86/kernel/microcode_amd.c +++ b/arch/x86/kernel/microcode_amd.c @@ -77,7 +77,6 @@ static int collect_cpu_info_amd(int cpu, struct cpu_signature *csig) struct cpuinfo_x86 *c = &cpu_data(cpu); u32 dummy; - memset(csig, 0, sizeof(*csig)); if (c->x86_vendor != X86_VENDOR_AMD || c->x86 < 0x10) { pr_warning("microcode: CPU%d: AMD CPU family 0x%x not " "supported\n", cpu, c->x86); -- cgit v0.10.2 From 258721ef34fce97a7a6ca9cebebb303827645868 Mon Sep 17 00:00:00 2001 From: Borislav Petkov Date: Wed, 5 Jan 2011 18:13:19 +0100 Subject: x86, microcode, AMD: Cleanup dmesg output Unify pr_* to use pr_fmt, shorten messages, correct type formatting. Signed-off-by: Borislav Petkov Acked-by: Andreas Herrmann diff --git a/arch/x86/kernel/microcode_amd.c b/arch/x86/kernel/microcode_amd.c index 274fa40..adbcef4 100644 --- a/arch/x86/kernel/microcode_amd.c +++ b/arch/x86/kernel/microcode_amd.c @@ -78,12 +78,13 @@ static int collect_cpu_info_amd(int cpu, struct cpu_signature *csig) u32 dummy; if (c->x86_vendor != X86_VENDOR_AMD || c->x86 < 0x10) { - pr_warning("microcode: CPU%d: AMD CPU family 0x%x not " - "supported\n", cpu, c->x86); + pr_warning("CPU%d: family %d not supported\n", cpu, c->x86); return -1; } + rdmsr(MSR_AMD64_PATCH_LEVEL, csig->rev, dummy); - pr_info("CPU%d: patch_level=0x%x\n", cpu, csig->rev); + pr_info("CPU%d: patch_level=0x%08x\n", cpu, csig->rev); + return 0; } @@ -113,7 +114,7 @@ static int get_matching_microcode(int cpu, struct microcode_header_amd *mc_hdr, /* ucode might be chipset specific -- currently we don't support this */ if (mc_hdr->nb_dev_id || mc_hdr->sb_dev_id) { - pr_err("CPU%d: loading of chipset specific code not yet supported\n", + pr_err("CPU%d: chipset specific code not yet supported\n", cpu); return 0; } @@ -143,12 +144,12 @@ static int apply_microcode_amd(int cpu) /* check current patch id and patch's id for match */ if (rev != mc_amd->hdr.patch_id) { - pr_err("CPU%d: update failed (for patch_level=0x%x)\n", + pr_err("CPU%d: update failed for patch_level=0x%08x\n", cpu, mc_amd->hdr.patch_id); return -1; } - pr_info("CPU%d: updated (new patch_level=0x%x)\n", cpu, rev); + pr_info("CPU%d: new patch_level=0x%08x\n", cpu, rev); uci->cpu_sig.rev = rev; return 0; @@ -161,14 +162,14 @@ get_next_ucode(const u8 *buf, unsigned int size, unsigned int *mc_size) unsigned int total_size; if (buf[0] != UCODE_UCODE_TYPE) { - pr_err("error: invalid type field in container file section header\n"); + pr_err("invalid type field in container file section header\n"); return NULL; } total_size = buf[4] + (buf[5] << 8); if (total_size > size || total_size > UCODE_MAX_SIZE) { - pr_err("error: size mismatch\n"); + pr_err("section size mismatch\n"); return NULL; } @@ -189,7 +190,8 @@ static int install_equiv_cpu_table(const u8 *buf) unsigned int size = ibuf[2]; if (type != UCODE_EQUIV_CPU_TABLE_TYPE || !size) { - pr_err("error: invalid type field in container file section header\n"); + pr_err("empty section/" + "invalid type field in container file section header\n"); return -EINVAL; } @@ -219,7 +221,7 @@ generic_load_microcode(int cpu, const u8 *data, size_t size) unsigned long offset; const u8 *ucode_ptr = data; void *new_mc = NULL; - int new_rev = uci->cpu_sig.rev; + unsigned int new_rev = uci->cpu_sig.rev; enum ucode_state state = UCODE_OK; offset = install_equiv_cpu_table(ucode_ptr); @@ -255,8 +257,8 @@ generic_load_microcode(int cpu, const u8 *data, size_t size) if (!leftover) { vfree(uci->mc); uci->mc = new_mc; - pr_debug("CPU%d update ucode to version 0x%x (from 0x%x)\n", - cpu, new_rev, uci->cpu_sig.rev); + pr_debug("CPU%d update ucode (0x%08x -> 0x%08x)\n", + cpu, uci->cpu_sig.rev, new_rev); } else { vfree(new_mc); state = UCODE_ERROR; @@ -275,13 +277,13 @@ static enum ucode_state request_microcode_amd(int cpu, struct device *device) enum ucode_state ret = UCODE_NFOUND; if (request_firmware(&fw, fw_name, device)) { - printk(KERN_ERR "microcode: failed to load file %s\n", fw_name); + pr_err("failed to load file %s\n", fw_name); goto out; } ret = UCODE_ERROR; if (*(u32 *)fw->data != UCODE_MAGIC) { - pr_err("Invalid UCODE_MAGIC (0x%08x)\n", *(u32 *)fw->data); + pr_err("invalid magic value (0x%08x)\n", *(u32 *)fw->data); goto fw_release; } -- cgit v0.10.2 From 44d60c0f5c58c2168f31df9a481761451840eb54 Mon Sep 17 00:00:00 2001 From: Borislav Petkov Date: Thu, 10 Feb 2011 12:19:47 +0100 Subject: x86, microcode, AMD: Extend ucode size verification The different families have a different max size for the ucode patch, adjust size checking to the family we're running on. Also, do not vzalloc the max size of the ucode but only the actual size that is passed on from the firmware loader. Signed-off-by: Borislav Petkov diff --git a/arch/x86/kernel/microcode_amd.c b/arch/x86/kernel/microcode_amd.c index adbcef4..9fb8405 100644 --- a/arch/x86/kernel/microcode_amd.c +++ b/arch/x86/kernel/microcode_amd.c @@ -66,7 +66,6 @@ struct microcode_amd { unsigned int mpb[0]; }; -#define UCODE_MAX_SIZE 2048 #define UCODE_CONTAINER_SECTION_HDR 8 #define UCODE_CONTAINER_HEADER_SIZE 12 @@ -155,31 +154,60 @@ static int apply_microcode_amd(int cpu) return 0; } +static unsigned int verify_ucode_size(int cpu, const u8 *buf, unsigned int size) +{ + struct cpuinfo_x86 *c = &cpu_data(cpu); + unsigned int max_size, actual_size; + +#define F1XH_MPB_MAX_SIZE 2048 +#define F14H_MPB_MAX_SIZE 1824 +#define F15H_MPB_MAX_SIZE 4096 + + switch (c->x86) { + case 0x14: + max_size = F14H_MPB_MAX_SIZE; + break; + case 0x15: + max_size = F15H_MPB_MAX_SIZE; + break; + default: + max_size = F1XH_MPB_MAX_SIZE; + break; + } + + actual_size = buf[4] + (buf[5] << 8); + + if (actual_size > size || actual_size > max_size) { + pr_err("section size mismatch\n"); + return 0; + } + + return actual_size; +} + static struct microcode_header_amd * -get_next_ucode(const u8 *buf, unsigned int size, unsigned int *mc_size) +get_next_ucode(int cpu, const u8 *buf, unsigned int size, unsigned int *mc_size) { - struct microcode_header_amd *mc; - unsigned int total_size; + struct microcode_header_amd *mc = NULL; + unsigned int actual_size = 0; if (buf[0] != UCODE_UCODE_TYPE) { pr_err("invalid type field in container file section header\n"); - return NULL; + goto out; } - total_size = buf[4] + (buf[5] << 8); - - if (total_size > size || total_size > UCODE_MAX_SIZE) { - pr_err("section size mismatch\n"); - return NULL; - } + actual_size = verify_ucode_size(cpu, buf, size); + if (!actual_size) + goto out; - mc = vzalloc(UCODE_MAX_SIZE); + mc = vzalloc(actual_size); if (!mc) - return NULL; + goto out; - get_ucode_data(mc, buf + UCODE_CONTAINER_SECTION_HDR, total_size); - *mc_size = total_size + UCODE_CONTAINER_SECTION_HDR; + get_ucode_data(mc, buf + UCODE_CONTAINER_SECTION_HDR, actual_size); + *mc_size = actual_size + UCODE_CONTAINER_SECTION_HDR; +out: return mc; } @@ -234,7 +262,7 @@ generic_load_microcode(int cpu, const u8 *data, size_t size) leftover = size - offset; while (leftover) { - mc_hdr = get_next_ucode(ucode_ptr, leftover, &mc_size); + mc_hdr = get_next_ucode(cpu, ucode_ptr, leftover, &mc_size); if (!mc_hdr) break; -- cgit v0.10.2 From 1396fa9cd2e34669253b7ca8c75f12103481f71c Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Fri, 18 Feb 2011 12:17:16 +0300 Subject: x86, microcode, AMD: Fix signedness bug in generic_load_microcode() install_equiv_cpu_table() returns type int. It uses negative error codes so using an unsigned type breaks the error handling. Signed-off-by: Dan Carpenter Acked-by: Borislav Petkov Cc: open list:AMD MICROCODE UPD... Cc: Andreas Herrmann LKML-Reference: <20110218091716.GA4384@bicker> Signed-off-by: Ingo Molnar diff --git a/arch/x86/kernel/microcode_amd.c b/arch/x86/kernel/microcode_amd.c index 9fb8405..c561038 100644 --- a/arch/x86/kernel/microcode_amd.c +++ b/arch/x86/kernel/microcode_amd.c @@ -246,7 +246,7 @@ generic_load_microcode(int cpu, const u8 *data, size_t size) struct ucode_cpu_info *uci = ucode_cpu_info + cpu; struct microcode_header_amd *mc_hdr = NULL; unsigned int mc_size, leftover; - unsigned long offset; + int offset; const u8 *ucode_ptr = data; void *new_mc = NULL; unsigned int new_rev = uci->cpu_sig.rev; -- cgit v0.10.2