2009-11-10 11:06:06

by Andreas Herrmann

[permalink] [raw]
Subject: [PATCH 0/3] x86, ucode-amd: trim verbosity

Following patches make log messages of microcode_amd more terse.

Especially in conjunction with Dmitry's planned changes to summarize
patch level information this will help to trim messages to the
necessary minimum on systems with many CPUs.

Example (without patch 1 and 2) of a 48 CPU system:

microcode: CPU0: patch_level=0x10000b5
platform microcode: firmware: requesting amd-ucode/microcode_amd.bin
microcode: CPU0: cpu revision not listed in equivalent cpu table
microcode: CPU0: cpu revision not listed in equivalent cpu table
microcode: CPU1: patch_level=0x10000b5
platform microcode: firmware: requesting amd-ucode/microcode_amd.bin
microcode: CPU1: cpu revision not listed in equivalent cpu table
microcode: CPU1: cpu revision not listed in equivalent cpu table

...

microcode: CPU46: patch_level=0x10000b5
platform microcode: firmware: requesting amd-ucode/microcode_amd.bin
microcode: CPU46: cpu revision not listed in equivalent cpu table
microcode: CPU46: cpu revision not listed in equivalent cpu table
microcode: CPU47: patch_level=0x10000b5
platform microcode: firmware: requesting amd-ucode/microcode_amd.bin
microcode: CPU47: cpu revision not listed in equivalent cpu table
microcode: CPU47: cpu revision not listed in equivalent cpu table
Microcode Update Driver: v2.00 <[email protected]>, Peter Oruba

This is a lot of unnecessary noise. With patches 1 and 2 applied this
is shortened to:

platform microcode: firmware: requesting amd-ucode/microcode_amd.bin
microcode: CPU0: patch_level=0x10000b5
microcode: CPU1: patch_level=0x10000b5

...

microcode: CPU46: patch_level=0x10000b5
microcode: CPU47: patch_level=0x10000b5
Microcode Update Driver: v2.00 <[email protected]>, Peter Oruba

And with Dmitry's patches this will be trimmed down even further.
Note: Patch 3 is just a conversion from printk to pr_ macros.

Patches are against tip/master.
Please apply.


Thanks,

Andreas


2009-11-10 11:07:24

by Andreas Herrmann

[permalink] [raw]
Subject: [PATCH 1/3] x86, ucode-amd: Load ucode-patches once and not separately fo each CPU

This also implies that corresponding log messages, e.g.

platform microcode: firmware: requesting amd-ucode/microcode_amd.bin

show up only once on module load and not when ucode is updated for
each CPU.

Signed-off-by: Andreas Herrmann <[email protected]>
---
arch/x86/include/asm/microcode.h | 2 ++
arch/x86/kernel/microcode_amd.c | 24 +++++++++++++++++-------
arch/x86/kernel/microcode_core.c | 6 ++++++
3 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/arch/x86/include/asm/microcode.h b/arch/x86/include/asm/microcode.h
index ef51b50..c24ca9a 100644
--- a/arch/x86/include/asm/microcode.h
+++ b/arch/x86/include/asm/microcode.h
@@ -12,6 +12,8 @@ struct device;
enum ucode_state { UCODE_ERROR, UCODE_OK, UCODE_NFOUND };

struct microcode_ops {
+ void (*init)(struct device *device);
+ void (*fini)(void);
enum ucode_state (*request_microcode_user) (int cpu,
const void __user *buf, size_t size);

diff --git a/arch/x86/kernel/microcode_amd.c b/arch/x86/kernel/microcode_amd.c
index c043534..75538f6 100644
--- a/arch/x86/kernel/microcode_amd.c
+++ b/arch/x86/kernel/microcode_amd.c
@@ -33,6 +33,8 @@ MODULE_LICENSE("GPL v2");
#define UCODE_EQUIV_CPU_TABLE_TYPE 0x00000000
#define UCODE_UCODE_TYPE 0x00000001

+const struct firmware *firmware;
+
struct equiv_cpu_entry {
u32 installed_cpu;
u32 fixed_errata_mask;
@@ -301,14 +303,10 @@ generic_load_microcode(int cpu, const u8 *data, size_t size)

static enum ucode_state request_microcode_fw(int cpu, struct device *device)
{
- const char *fw_name = "amd-ucode/microcode_amd.bin";
- const struct firmware *firmware;
enum ucode_state ret;

- if (request_firmware(&firmware, fw_name, device)) {
- printk(KERN_ERR "microcode: failed to load file %s\n", fw_name);
+ if (firmware == NULL)
return UCODE_NFOUND;
- }

if (*(u32 *)firmware->data != UCODE_MAGIC) {
printk(KERN_ERR "microcode: invalid UCODE_MAGIC (0x%08x)\n",
@@ -318,8 +316,6 @@ static enum ucode_state request_microcode_fw(int cpu, struct device *device)

ret = generic_load_microcode(cpu, firmware->data, firmware->size);

- release_firmware(firmware);
-
return ret;
}

@@ -339,7 +335,21 @@ static void microcode_fini_cpu_amd(int cpu)
uci->mc = NULL;
}

+void init_microcode_amd(struct device *device)
+{
+ const char *fw_name = "amd-ucode/microcode_amd.bin";
+ if (request_firmware(&firmware, fw_name, device))
+ printk(KERN_ERR "microcode: failed to load file %s\n", fw_name);
+}
+
+void fini_microcode_amd(void)
+{
+ release_firmware(firmware);
+}
+
static struct microcode_ops microcode_amd_ops = {
+ .init = init_microcode_amd,
+ .fini = fini_microcode_amd,
.request_microcode_user = request_microcode_user,
.request_microcode_fw = request_microcode_fw,
.collect_cpu_info = collect_cpu_info_amd,
diff --git a/arch/x86/kernel/microcode_core.c b/arch/x86/kernel/microcode_core.c
index 2bcad39..230fb09 100644
--- a/arch/x86/kernel/microcode_core.c
+++ b/arch/x86/kernel/microcode_core.c
@@ -518,6 +518,9 @@ static int __init microcode_init(void)
return PTR_ERR(microcode_pdev);
}

+ if (microcode_ops->init)
+ microcode_ops->init(&microcode_pdev->dev);
+
get_online_cpus();
mutex_lock(&microcode_mutex);

@@ -561,6 +564,9 @@ static void __exit microcode_exit(void)

platform_device_unregister(microcode_pdev);

+ if (microcode_ops->fini)
+ microcode_ops->fini();
+
microcode_ops = NULL;

pr_info("Microcode Update Driver: v" MICROCODE_VERSION " removed.\n");
--
1.6.5.2

2009-11-10 11:08:24

by Andreas Herrmann

[permalink] [raw]
Subject: [PATCH 2/3] x86, ucode-amd: Don't warn when no ucode is available for a CPU revision

There is no point in warning when there is no ucode available for a
specific CPU revision. Currently the container-file, which provides the
AMD ucode patches for OS load, contains only a few ucode patches.

It's already clearly indicated by the printed patch_level whenever new
ucode was available and an update happened. So the warning message is
of no help but rather annoying on systems with many CPUs.

Signed-off-by: Andreas Herrmann <[email protected]>
---
arch/x86/kernel/microcode_amd.c | 5 +----
1 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/microcode_amd.c b/arch/x86/kernel/microcode_amd.c
index 75538f6..9f13324 100644
--- a/arch/x86/kernel/microcode_amd.c
+++ b/arch/x86/kernel/microcode_amd.c
@@ -105,11 +105,8 @@ static int get_matching_microcode(int cpu, void *mc, int rev)
i++;
}

- if (!equiv_cpu_id) {
- printk(KERN_WARNING "microcode: CPU%d: cpu revision "
- "not listed in equivalent cpu table\n", cpu);
+ if (!equiv_cpu_id)
return 0;
- }

if (mc_header->processor_rev_id != equiv_cpu_id)
return 0;
--
1.6.5.2

2009-11-10 11:09:20

by Andreas Herrmann

[permalink] [raw]
Subject: [PATCH 3/3] x86, ucode-amd: printk(KERN_* to pr_* conversion


Signed-off-by: Andreas Herrmann <[email protected]>
---
arch/x86/kernel/microcode_amd.c | 34 +++++++++++++++-------------------
1 files changed, 15 insertions(+), 19 deletions(-)

diff --git a/arch/x86/kernel/microcode_amd.c b/arch/x86/kernel/microcode_amd.c
index 9f13324..26e33bd 100644
--- a/arch/x86/kernel/microcode_amd.c
+++ b/arch/x86/kernel/microcode_amd.c
@@ -78,12 +78,12 @@ static int collect_cpu_info_amd(int cpu, struct cpu_signature *csig)

memset(csig, 0, sizeof(*csig));
if (c->x86_vendor != X86_VENDOR_AMD || c->x86 < 0x10) {
- printk(KERN_WARNING "microcode: CPU%d: AMD CPU family 0x%x not "
- "supported\n", cpu, c->x86);
+ pr_warning("microcode: CPU%d: AMD CPU family 0x%x not "
+ "supported\n", cpu, c->x86);
return -1;
}
rdmsr(MSR_AMD64_PATCH_LEVEL, csig->rev, dummy);
- printk(KERN_INFO "microcode: CPU%d: patch_level=0x%x\n", cpu, csig->rev);
+ pr_info("microcode: CPU%d: patch_level=0x%x\n", cpu, csig->rev);
return 0;
}

@@ -113,7 +113,7 @@ static int get_matching_microcode(int cpu, void *mc, int rev)

/* ucode might be chipset specific -- currently we don't support this */
if (mc_header->nb_dev_id || mc_header->sb_dev_id) {
- printk(KERN_ERR "microcode: CPU%d: loading of chipset "
+ pr_err(KERN_ERR "microcode: CPU%d: loading of chipset "
"specific code not yet supported\n", cpu);
return 0;
}
@@ -143,14 +143,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) {
- printk(KERN_ERR "microcode: CPU%d: update failed "
+ pr_err("microcode: CPU%d: update failed "
"(for patch_level=0x%x)\n", cpu, mc_amd->hdr.patch_id);
return -1;
}

- printk(KERN_INFO "microcode: CPU%d: updated (new patch_level=0x%x)\n",
- cpu, rev);
-
+ pr_info("microcode: CPU%d: updated (new patch_level=0x%x)\n", cpu, rev);
uci->cpu_sig.rev = rev;

return 0;
@@ -173,7 +171,7 @@ get_next_ucode(const u8 *buf, unsigned int size, unsigned int *mc_size)
return NULL;

if (section_hdr[0] != UCODE_UCODE_TYPE) {
- printk(KERN_ERR "microcode: error: invalid type field in "
+ pr_err("microcode: error: invalid type field in "
"container file section header\n");
return NULL;
}
@@ -181,7 +179,7 @@ get_next_ucode(const u8 *buf, unsigned int size, unsigned int *mc_size)
total_size = (unsigned long) (section_hdr[4] + (section_hdr[5] << 8));

if (total_size > size || total_size > UCODE_MAX_SIZE) {
- printk(KERN_ERR "microcode: error: size mismatch\n");
+ pr_err("microcode: error: size mismatch\n");
return NULL;
}

@@ -210,15 +208,14 @@ static int install_equiv_cpu_table(const u8 *buf)
size = buf_pos[2];

if (buf_pos[1] != UCODE_EQUIV_CPU_TABLE_TYPE || !size) {
- printk(KERN_ERR "microcode: error: invalid type field in "
+ pr_err("microcode: error: invalid type field in "
"container file section header\n");
return 0;
}

equiv_cpu_table = (struct equiv_cpu_entry *) vmalloc(size);
if (!equiv_cpu_table) {
- printk(KERN_ERR "microcode: failed to allocate "
- "equivalent CPU table\n");
+ pr_err("microcode: failed to allocate equivalent CPU table\n");
return 0;
}

@@ -251,8 +248,7 @@ generic_load_microcode(int cpu, const u8 *data, size_t size)

offset = install_equiv_cpu_table(ucode_ptr);
if (!offset) {
- printk(KERN_ERR "microcode: failed to create "
- "equivalent cpu table\n");
+ pr_err("microcode: failed to create equivalent cpu table\n");
return UCODE_ERROR;
}

@@ -306,7 +302,7 @@ static enum ucode_state request_microcode_fw(int cpu, struct device *device)
return UCODE_NFOUND;

if (*(u32 *)firmware->data != UCODE_MAGIC) {
- printk(KERN_ERR "microcode: invalid UCODE_MAGIC (0x%08x)\n",
+ pr_err("microcode: invalid UCODE_MAGIC (0x%08x)\n",
*(u32 *)firmware->data);
return UCODE_ERROR;
}
@@ -319,8 +315,8 @@ static enum ucode_state request_microcode_fw(int cpu, struct device *device)
static enum ucode_state
request_microcode_user(int cpu, const void __user *buf, size_t size)
{
- printk(KERN_INFO "microcode: AMD microcode update via "
- "/dev/cpu/microcode not supported\n");
+ pr_info("microcode: AMD microcode update via "
+ "/dev/cpu/microcode not supported\n");
return UCODE_ERROR;
}

@@ -336,7 +332,7 @@ void init_microcode_amd(struct device *device)
{
const char *fw_name = "amd-ucode/microcode_amd.bin";
if (request_firmware(&firmware, fw_name, device))
- printk(KERN_ERR "microcode: failed to load file %s\n", fw_name);
+ pr_err("microcode: failed to load file %s\n", fw_name);
}

void fini_microcode_amd(void)
--
1.6.5.2

2009-11-10 12:10:34

by Dmitry Adamushko

[permalink] [raw]
Subject: Re: [PATCH 1/3] x86, ucode-amd: Load ucode-patches once and not separately fo each CPU

2009/11/10 Andreas Herrmann <[email protected]>:
> This also implies that corresponding log messages, e.g.
>
> platform microcode: firmware: requesting amd-ucode/microcode_amd.bin
>
> show up only once on module load and not when ucode is updated for
> each CPU.

I like it.

One remark : should we perhaps provide a means of reloading the cached
firmware? Or is the standard procedure to reload microcode.ko in case
a new firmware file has been installed?

btw., if we could safely assume that all the cpus after the ucode
upgrade share the same version/patch-level of ucode, we would be able
to cache a single ucode instance once and use it for all. I don't
recall anyone clearly stating that such multi-cpu-type systems can't
really exist.

e.g. is it possible to have AMD systems with cpus which differ from
each other not only by their revisions (patch_level)?


-- Dmitry

2009-11-10 13:22:19

by Andreas Herrmann

[permalink] [raw]
Subject: [tip:x86/microcode] x86: ucode-amd: Load ucode-patches once and not separately of each CPU

Commit-ID: d1c84f79a6ba992dc01e312c44a21496303874d6
Gitweb: http://git.kernel.org/tip/d1c84f79a6ba992dc01e312c44a21496303874d6
Author: Andreas Herrmann <[email protected]>
AuthorDate: Tue, 10 Nov 2009 12:07:23 +0100
Committer: Ingo Molnar <[email protected]>
CommitDate: Tue, 10 Nov 2009 12:15:48 +0100

x86: ucode-amd: Load ucode-patches once and not separately of each CPU

This also implies that corresponding log messages, e.g.

platform microcode: firmware: requesting amd-ucode/microcode_amd.bin

show up only once on module load and not when ucode is updated
for each CPU.

Signed-off-by: Andreas Herrmann <[email protected]>
Cc: dimm <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/include/asm/microcode.h | 2 ++
arch/x86/kernel/microcode_amd.c | 24 +++++++++++++++++-------
arch/x86/kernel/microcode_core.c | 6 ++++++
3 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/arch/x86/include/asm/microcode.h b/arch/x86/include/asm/microcode.h
index ef51b50..c24ca9a 100644
--- a/arch/x86/include/asm/microcode.h
+++ b/arch/x86/include/asm/microcode.h
@@ -12,6 +12,8 @@ struct device;
enum ucode_state { UCODE_ERROR, UCODE_OK, UCODE_NFOUND };

struct microcode_ops {
+ void (*init)(struct device *device);
+ void (*fini)(void);
enum ucode_state (*request_microcode_user) (int cpu,
const void __user *buf, size_t size);

diff --git a/arch/x86/kernel/microcode_amd.c b/arch/x86/kernel/microcode_amd.c
index c043534..75538f6 100644
--- a/arch/x86/kernel/microcode_amd.c
+++ b/arch/x86/kernel/microcode_amd.c
@@ -33,6 +33,8 @@ MODULE_LICENSE("GPL v2");
#define UCODE_EQUIV_CPU_TABLE_TYPE 0x00000000
#define UCODE_UCODE_TYPE 0x00000001

+const struct firmware *firmware;
+
struct equiv_cpu_entry {
u32 installed_cpu;
u32 fixed_errata_mask;
@@ -301,14 +303,10 @@ generic_load_microcode(int cpu, const u8 *data, size_t size)

static enum ucode_state request_microcode_fw(int cpu, struct device *device)
{
- const char *fw_name = "amd-ucode/microcode_amd.bin";
- const struct firmware *firmware;
enum ucode_state ret;

- if (request_firmware(&firmware, fw_name, device)) {
- printk(KERN_ERR "microcode: failed to load file %s\n", fw_name);
+ if (firmware == NULL)
return UCODE_NFOUND;
- }

if (*(u32 *)firmware->data != UCODE_MAGIC) {
printk(KERN_ERR "microcode: invalid UCODE_MAGIC (0x%08x)\n",
@@ -318,8 +316,6 @@ static enum ucode_state request_microcode_fw(int cpu, struct device *device)

ret = generic_load_microcode(cpu, firmware->data, firmware->size);

- release_firmware(firmware);
-
return ret;
}

@@ -339,7 +335,21 @@ static void microcode_fini_cpu_amd(int cpu)
uci->mc = NULL;
}

+void init_microcode_amd(struct device *device)
+{
+ const char *fw_name = "amd-ucode/microcode_amd.bin";
+ if (request_firmware(&firmware, fw_name, device))
+ printk(KERN_ERR "microcode: failed to load file %s\n", fw_name);
+}
+
+void fini_microcode_amd(void)
+{
+ release_firmware(firmware);
+}
+
static struct microcode_ops microcode_amd_ops = {
+ .init = init_microcode_amd,
+ .fini = fini_microcode_amd,
.request_microcode_user = request_microcode_user,
.request_microcode_fw = request_microcode_fw,
.collect_cpu_info = collect_cpu_info_amd,
diff --git a/arch/x86/kernel/microcode_core.c b/arch/x86/kernel/microcode_core.c
index 378e9a8..d2a8160 100644
--- a/arch/x86/kernel/microcode_core.c
+++ b/arch/x86/kernel/microcode_core.c
@@ -520,6 +520,9 @@ static int __init microcode_init(void)
return PTR_ERR(microcode_pdev);
}

+ if (microcode_ops->init)
+ microcode_ops->init(&microcode_pdev->dev);
+
get_online_cpus();
mutex_lock(&microcode_mutex);

@@ -563,6 +566,9 @@ static void __exit microcode_exit(void)

platform_device_unregister(microcode_pdev);

+ if (microcode_ops->fini)
+ microcode_ops->fini();
+
microcode_ops = NULL;

pr_info("Microcode Update Driver: v" MICROCODE_VERSION " removed.\n");

2009-11-10 13:22:25

by Andreas Herrmann

[permalink] [raw]
Subject: [tip:x86/microcode] x86: ucode-amd: Don't warn when no ucode is available for a CPU revision

Commit-ID: 14c569425a0ae12cbeed72fdb8ebe78c48455dfd
Gitweb: http://git.kernel.org/tip/14c569425a0ae12cbeed72fdb8ebe78c48455dfd
Author: Andreas Herrmann <[email protected]>
AuthorDate: Tue, 10 Nov 2009 12:08:25 +0100
Committer: Ingo Molnar <[email protected]>
CommitDate: Tue, 10 Nov 2009 12:15:49 +0100

x86: ucode-amd: Don't warn when no ucode is available for a CPU revision

There is no point in warning when there is no ucode available
for a specific CPU revision. Currently the container-file, which
provides the AMD ucode patches for OS load, contains only a few
ucode patches.

It's already clearly indicated by the printed patch_level
whenever new ucode was available and an update happened. So the
warning message is of no help but rather annoying on systems
with many CPUs.

Signed-off-by: Andreas Herrmann <[email protected]>
Cc: dimm <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/kernel/microcode_amd.c | 5 +----
1 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/microcode_amd.c b/arch/x86/kernel/microcode_amd.c
index 75538f6..9f13324 100644
--- a/arch/x86/kernel/microcode_amd.c
+++ b/arch/x86/kernel/microcode_amd.c
@@ -105,11 +105,8 @@ static int get_matching_microcode(int cpu, void *mc, int rev)
i++;
}

- if (!equiv_cpu_id) {
- printk(KERN_WARNING "microcode: CPU%d: cpu revision "
- "not listed in equivalent cpu table\n", cpu);
+ if (!equiv_cpu_id)
return 0;
- }

if (mc_header->processor_rev_id != equiv_cpu_id)
return 0;

2009-11-10 13:22:55

by Andreas Herrmann

[permalink] [raw]
Subject: [tip:x86/microcode] x86: ucode-amd: Convert printk(KERN_*...) to pr_*(...)

Commit-ID: 1a74357066369be91e6f4f431621a00b052df964
Gitweb: http://git.kernel.org/tip/1a74357066369be91e6f4f431621a00b052df964
Author: Andreas Herrmann <[email protected]>
AuthorDate: Tue, 10 Nov 2009 12:09:20 +0100
Committer: Ingo Molnar <[email protected]>
CommitDate: Tue, 10 Nov 2009 12:15:50 +0100

x86: ucode-amd: Convert printk(KERN_*...) to pr_*(...)

Signed-off-by: Andreas Herrmann <[email protected]>
Cc: dimm <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/kernel/microcode_amd.c | 34 +++++++++++++++-------------------
1 files changed, 15 insertions(+), 19 deletions(-)

diff --git a/arch/x86/kernel/microcode_amd.c b/arch/x86/kernel/microcode_amd.c
index 9f13324..26e33bd 100644
--- a/arch/x86/kernel/microcode_amd.c
+++ b/arch/x86/kernel/microcode_amd.c
@@ -78,12 +78,12 @@ static int collect_cpu_info_amd(int cpu, struct cpu_signature *csig)

memset(csig, 0, sizeof(*csig));
if (c->x86_vendor != X86_VENDOR_AMD || c->x86 < 0x10) {
- printk(KERN_WARNING "microcode: CPU%d: AMD CPU family 0x%x not "
- "supported\n", cpu, c->x86);
+ pr_warning("microcode: CPU%d: AMD CPU family 0x%x not "
+ "supported\n", cpu, c->x86);
return -1;
}
rdmsr(MSR_AMD64_PATCH_LEVEL, csig->rev, dummy);
- printk(KERN_INFO "microcode: CPU%d: patch_level=0x%x\n", cpu, csig->rev);
+ pr_info("microcode: CPU%d: patch_level=0x%x\n", cpu, csig->rev);
return 0;
}

@@ -113,7 +113,7 @@ static int get_matching_microcode(int cpu, void *mc, int rev)

/* ucode might be chipset specific -- currently we don't support this */
if (mc_header->nb_dev_id || mc_header->sb_dev_id) {
- printk(KERN_ERR "microcode: CPU%d: loading of chipset "
+ pr_err(KERN_ERR "microcode: CPU%d: loading of chipset "
"specific code not yet supported\n", cpu);
return 0;
}
@@ -143,14 +143,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) {
- printk(KERN_ERR "microcode: CPU%d: update failed "
+ pr_err("microcode: CPU%d: update failed "
"(for patch_level=0x%x)\n", cpu, mc_amd->hdr.patch_id);
return -1;
}

- printk(KERN_INFO "microcode: CPU%d: updated (new patch_level=0x%x)\n",
- cpu, rev);
-
+ pr_info("microcode: CPU%d: updated (new patch_level=0x%x)\n", cpu, rev);
uci->cpu_sig.rev = rev;

return 0;
@@ -173,7 +171,7 @@ get_next_ucode(const u8 *buf, unsigned int size, unsigned int *mc_size)
return NULL;

if (section_hdr[0] != UCODE_UCODE_TYPE) {
- printk(KERN_ERR "microcode: error: invalid type field in "
+ pr_err("microcode: error: invalid type field in "
"container file section header\n");
return NULL;
}
@@ -181,7 +179,7 @@ get_next_ucode(const u8 *buf, unsigned int size, unsigned int *mc_size)
total_size = (unsigned long) (section_hdr[4] + (section_hdr[5] << 8));

if (total_size > size || total_size > UCODE_MAX_SIZE) {
- printk(KERN_ERR "microcode: error: size mismatch\n");
+ pr_err("microcode: error: size mismatch\n");
return NULL;
}

@@ -210,15 +208,14 @@ static int install_equiv_cpu_table(const u8 *buf)
size = buf_pos[2];

if (buf_pos[1] != UCODE_EQUIV_CPU_TABLE_TYPE || !size) {
- printk(KERN_ERR "microcode: error: invalid type field in "
+ pr_err("microcode: error: invalid type field in "
"container file section header\n");
return 0;
}

equiv_cpu_table = (struct equiv_cpu_entry *) vmalloc(size);
if (!equiv_cpu_table) {
- printk(KERN_ERR "microcode: failed to allocate "
- "equivalent CPU table\n");
+ pr_err("microcode: failed to allocate equivalent CPU table\n");
return 0;
}

@@ -251,8 +248,7 @@ generic_load_microcode(int cpu, const u8 *data, size_t size)

offset = install_equiv_cpu_table(ucode_ptr);
if (!offset) {
- printk(KERN_ERR "microcode: failed to create "
- "equivalent cpu table\n");
+ pr_err("microcode: failed to create equivalent cpu table\n");
return UCODE_ERROR;
}

@@ -306,7 +302,7 @@ static enum ucode_state request_microcode_fw(int cpu, struct device *device)
return UCODE_NFOUND;

if (*(u32 *)firmware->data != UCODE_MAGIC) {
- printk(KERN_ERR "microcode: invalid UCODE_MAGIC (0x%08x)\n",
+ pr_err("microcode: invalid UCODE_MAGIC (0x%08x)\n",
*(u32 *)firmware->data);
return UCODE_ERROR;
}
@@ -319,8 +315,8 @@ static enum ucode_state request_microcode_fw(int cpu, struct device *device)
static enum ucode_state
request_microcode_user(int cpu, const void __user *buf, size_t size)
{
- printk(KERN_INFO "microcode: AMD microcode update via "
- "/dev/cpu/microcode not supported\n");
+ pr_info("microcode: AMD microcode update via "
+ "/dev/cpu/microcode not supported\n");
return UCODE_ERROR;
}

@@ -336,7 +332,7 @@ void init_microcode_amd(struct device *device)
{
const char *fw_name = "amd-ucode/microcode_amd.bin";
if (request_firmware(&firmware, fw_name, device))
- printk(KERN_ERR "microcode: failed to load file %s\n", fw_name);
+ pr_err("microcode: failed to load file %s\n", fw_name);
}

void fini_microcode_amd(void)

2009-11-11 07:51:04

by Clemens Ladisch

[permalink] [raw]
Subject: Re: [PATCH 1/3] x86, ucode-amd: Load ucode-patches once and not separately fo each CPU

Dmitry Adamushko wrote:
> btw., if we could safely assume that all the cpus after the ucode
> upgrade share the same version/patch-level of ucode, we would be able
> to cache a single ucode instance once and use it for all. I don't
> recall anyone clearly stating that such multi-cpu-type systems can't
> really exist.
>
> e.g. is it possible to have AMD systems with cpus which differ from
> each other not only by their revisions (patch_level)?

The Revision Guide for AMD Family 10h Processors (#41322) says, in
section "Mixed Silicon Support", that the Opteron B steppings can be
used with each other (DR-BA, DR-B2, DR-B3; CPUID 00100F2xh). Are
different steppings considered equivalent?


Clemens

2009-11-11 11:27:33

by Andreas Herrmann

[permalink] [raw]
Subject: Re: [PATCH 1/3] x86, ucode-amd: Load ucode-patches once and not separately fo each CPU

On Wed, Nov 11, 2009 at 08:51:07AM +0100, Clemens Ladisch wrote:
> Dmitry Adamushko wrote:
> > btw., if we could safely assume that all the cpus after the ucode
> > upgrade share the same version/patch-level of ucode, we would be able
> > to cache a single ucode instance once and use it for all.

Hmm, I think all ucode-versions needed in a mixed silicon system need
to be cached. See below.

> > I don't recall anyone clearly stating that such multi-cpu-type
> > systems can't really exist.

> > e.g. is it possible to have AMD systems with cpus which differ from
> > each other not only by their revisions (patch_level)?

Mixed silicon is possible.

So at least in theory this could result in different patch_levels
because the required microcode is identified via processor revision
consisting of family/model/stepping.

> The Revision Guide for AMD Family 10h Processors (#41322) says, in
> section "Mixed Silicon Support", that the Opteron B steppings can be
> used with each other (DR-BA, DR-B2, DR-B3; CPUID 00100F2xh). Are
> different steppings considered equivalent?

The microcode to be loaded is identified by an Equivalent Processor
ID. The Equivalent Processor ID is determined with an equivalence
table which maps the processor revision IDs to Equivalent Processor
IDs.

Above revision B steppeings have different processor revision IDs:
(because the stepping differs)

00100F2Ah (DR-BA)
00100F22h (DR-B2)
00100F23h (DR-B3)

An example of an equivalence table is

0x00100F2A; 0x1020
0x00100F22; 0x1022
0x00100F23; 0x1022

This means when combining DR-BA and DR-B2 (or DR-B3) different ucode
gets installed for the two processors.


Regards,
Andreas

2009-11-12 15:05:43

by Andreas Herrmann

[permalink] [raw]
Subject: Re: [PATCH 1/3] x86, ucode-amd: Load ucode-patches once and not separately fo each CPU

On Tue, Nov 10, 2009 at 01:02:39PM +0100, Dmitry Adamushko wrote:
> 2009/11/10 Andreas Herrmann <[email protected]>:
> > This also implies that corresponding log messages, e.g.
> >
> > platform microcode: firmware: requesting amd-ucode/microcode_amd.bin
> >
> > show up only once on module load and not when ucode is updated for
> > each CPU.
>
> I like it.
>
> One remark : should we perhaps provide a means of reloading the cached
> firmware?

I'll think about this.

> Or is the standard procedure to reload microcode.ko in case
> a new firmware file has been installed?

module reload will do so far.

(And I should ensure that microcode_amd is only provided if ucode
loader is compiled as module.)


Regards,
Andreas