Skip to contentSkip to conversation

x86: AMD microcode patch loading support v2

PATCH Applied Peter Oruba 7 participants 32 messages Jul 28, 2008 – Sep 20, 2008 x86

lore
Share & tools
lore
#0 lore raw

cover x86: AMD microcode patch loading support v2

AMD microcode patch loader v2, rebased to Linux master.

Thanks,
Peter Oruba

Quoted 1 line

From [email protected] Mon Jul 28 18:44:47 2008

Message-Id: <[email protected]>
User-Agent: quilt/0.46_cvs20080326-19.1
Date: Mon, 28 Jul 2008 18:44:12 +0200
From: Peter Oruba <[email protected]>
To: Ingo Molnar <[email protected]>,
Thomas Gleixner <[email protected]>,
Tigran Aivazian <[email protected]>
Cc: LKML <[email protected]>,
Peter Oruba <[email protected]>
Bcc: [email protected]
Subject: [patch 01/11] [PATCH 01/11] x86: Add entry to MAINTAINERS file
References: <[email protected]>
Content-Disposition: inline; filename=0001-x86-Add-entry-to-MAINTAINERS-file.patch

Add an entry to the MAINTAINERS file for AMD CPU microcode patch loading support.

---
 MAINTAINERS |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)
MAINTAINERS +5 -0
index 03c5d6c..28f867d 100644
@@ -384,6 +384,11 @@ M:	[email protected]
 L:	[email protected]
 S:	Supported
 
+AMD MICROCODE UPDATE SUPPORT
+P:      Peter Oruba
+M:      [email protected]
+S:      Supported
+
 AMS (Apple Motion Sensor) DRIVER
 P:	Stelian Pop
 M:	[email protected]
Signature · 1 line
1.5.4.5
#1 lore raw

PATCH 2/11 x86: Moved Intel microcode patch loader declarations to seperate header file.

Intel specific microcode declarations have been moved to a seperate header file. There are no code changes to the code itself and no side effects to other parts.

---
 arch/x86/kernel/microcode.c |    1 +
 include/asm-x86/microcode.h |   34 ++++++++++++++++++++++++++++++++++
 include/asm-x86/processor.h |   35 -----------------------------------
 3 files changed, 35 insertions(+), 35 deletions(-)
 create mode 100644 include/asm-x86/microcode.h
arch/x86/kernel/microcode.c +1 -0
index 6994c75..0d654bd 100644
@@ -93,6 +93,7 @@
 #include <asm/msr.h>
 #include <asm/uaccess.h>
 #include <asm/processor.h>
+#include <asm/microcode.h>
 
 MODULE_DESCRIPTION("Intel CPU (IA-32) Microcode Update Driver");
 MODULE_AUTHOR("Tigran Aivazian <[email protected]>");
include/asm-x86/microcode.h new file +34 -0
new file mode 100644
index 0000000..5a05568
@@ -0,0 +1,34 @@
+struct microcode_header {
+	unsigned int            hdrver;
+	unsigned int            rev;
+	unsigned int            date;
+	unsigned int            sig;
+	unsigned int            cksum;
+	unsigned int            ldrver;
+	unsigned int            pf;
+	unsigned int            datasize;
+	unsigned int            totalsize;
+	unsigned int            reserved[3];
+};
+
+struct microcode {
+	struct microcode_header hdr;
+	unsigned int            bits[0];
+};
+
+typedef struct microcode          microcode_t;
+typedef struct microcode_header   microcode_header_t;
+
+/* microcode format is extended from prescott processors */
+struct extended_signature {
+	unsigned int            sig;
+	unsigned int            pf;
+	unsigned int            cksum;
+};
+
+struct extended_sigtable {
+	unsigned int            count;
+	unsigned int            cksum;
+	unsigned int            reserved[3];
+	struct extended_signature sigs[0];
+};
include/asm-x86/processor.h +0 -35
index 5f58da4..58a76f6 100644
@@ -561,41 +561,6 @@ static inline void clear_in_cr4(unsigned long mask)
 	write_cr4(cr4);
 }
 
-struct microcode_header {
-	unsigned int		hdrver;
-	unsigned int		rev;
-	unsigned int		date;
-	unsigned int		sig;
-	unsigned int		cksum;
-	unsigned int		ldrver;
-	unsigned int		pf;
-	unsigned int		datasize;
-	unsigned int		totalsize;
-	unsigned int		reserved[3];
-};
-
-struct microcode {
-	struct microcode_header	hdr;
-	unsigned int		bits[0];
-};
-
-typedef struct microcode	microcode_t;
-typedef struct microcode_header	microcode_header_t;
-
-/* microcode format is extended from prescott processors */
-struct extended_signature {
-	unsigned int		sig;
-	unsigned int		pf;
-	unsigned int		cksum;
-};
-
-struct extended_sigtable {
-	unsigned int		count;
-	unsigned int		cksum;
-	unsigned int		reserved[3];
-	struct extended_signature sigs[0];
-};
-
 typedef struct {
 	unsigned long		seg;
 } mm_segment_t;
Signature · 1 line
1.5.4.5
#2 lore raw

PATCH 3/11 x86: Typedef removal.

In reply to Peter Oruba

Removed typedefs. No functional changes to the code.

---
 arch/x86/kernel/microcode.c |   24 ++++++++++++------------
 include/asm-x86/microcode.h |    3 ---
 2 files changed, 12 insertions(+), 15 deletions(-)
arch/x86/kernel/microcode.c +12 -12
index 0d654bd..74e6a77 100644
@@ -102,17 +102,17 @@ MODULE_LICENSE("GPL");
 #define MICROCODE_VERSION 	"1.14a"
 
 #define DEFAULT_UCODE_DATASIZE 	(2000) 	  /* 2000 bytes */
-#define MC_HEADER_SIZE		(sizeof (microcode_header_t))  	  /* 48 bytes */
+#define MC_HEADER_SIZE		(sizeof (struct microcode_header))  	  /* 48 bytes */
 #define DEFAULT_UCODE_TOTALSIZE (DEFAULT_UCODE_DATASIZE + MC_HEADER_SIZE) /* 2048 bytes */
 #define EXT_HEADER_SIZE		(sizeof (struct extended_sigtable)) /* 20 bytes */
 #define EXT_SIGNATURE_SIZE	(sizeof (struct extended_signature)) /* 12 bytes */
 #define DWSIZE			(sizeof (u32))
 #define get_totalsize(mc) \
-	(((microcode_t *)mc)->hdr.totalsize ? \
-	 ((microcode_t *)mc)->hdr.totalsize : DEFAULT_UCODE_TOTALSIZE)
+	(((struct microcode *)mc)->hdr.totalsize ? \
+	 ((struct microcode *)mc)->hdr.totalsize : DEFAULT_UCODE_TOTALSIZE)
 #define get_datasize(mc) \
-	(((microcode_t *)mc)->hdr.datasize ? \
-	 ((microcode_t *)mc)->hdr.datasize : DEFAULT_UCODE_DATASIZE)
+	(((struct microcode *)mc)->hdr.datasize ? \
+	 ((struct microcode *)mc)->hdr.datasize : DEFAULT_UCODE_DATASIZE)
 
 #define sigmatch(s1, s2, p1, p2) \
 	(((s1) == (s2)) && (((p1) & (p2)) || (((p1) == 0) && ((p2) == 0))))
@@ -130,7 +130,7 @@ static struct ucode_cpu_info {
 	unsigned int sig;
 	unsigned int pf;
 	unsigned int rev;
-	microcode_t *mc;
+	struct microcode *mc;
 } ucode_cpu_info[NR_CPUS];
 
 static void collect_cpu_info(int cpu_num)
@@ -171,7 +171,7 @@ static void collect_cpu_info(int cpu_num)
 }
 
 static inline int microcode_update_match(int cpu_num,
-	microcode_header_t *mc_header, int sig, int pf)
+	struct microcode_header *mc_header, int sig, int pf)
 {
 	struct ucode_cpu_info *uci = ucode_cpu_info + cpu_num;
 
@@ -183,7 +183,7 @@ static inline int microcode_update_match(int cpu_num,
 
 static int microcode_sanity_check(void *mc)
 {
-	microcode_header_t *mc_header = mc;
+	struct microcode_header *mc_header = mc;
 	struct extended_sigtable *ext_header = NULL;
 	struct extended_signature *ext_sig;
 	unsigned long total_size, data_size, ext_table_size;
@@ -268,7 +268,7 @@ static int microcode_sanity_check(void *mc)
 static int get_maching_microcode(void *mc, int cpu)
 {
 	struct ucode_cpu_info *uci = ucode_cpu_info + cpu;
-	microcode_header_t *mc_header = mc;
+	struct microcode_header *mc_header = mc;
 	struct extended_sigtable *ext_header;
 	unsigned long total_size = get_totalsize(mc_header);
 	int ext_sigcount, i;
@@ -355,7 +355,7 @@ static unsigned int user_buffer_size;	/* it's size */
 
 static long get_next_ucode(void **mc, long offset)
 {
-	microcode_header_t mc_header;
+	struct microcode_header mc_header;
 	unsigned long total_size;
 
 	/* No more data */
@@ -497,13 +497,13 @@ MODULE_ALIAS_MISCDEV(MICROCODE_MINOR);
 static long get_next_ucode_from_buffer(void **mc, const u8 *buf,
 	unsigned long size, long offset)
 {
-	microcode_header_t *mc_header;
+	struct microcode_header *mc_header;
 	unsigned long total_size;
 
 	/* No more data */
 	if (offset >= size)
 		return 0;
-	mc_header = (microcode_header_t *)(buf + offset);
+	mc_header = (struct microcode_header *)(buf + offset);
 	total_size = get_totalsize(mc_header);
 
 	if (offset + total_size > size) {
include/asm-x86/microcode.h +0 -3
index 5a05568..1519ef0 100644
@@ -16,9 +16,6 @@ struct microcode {
 	unsigned int            bits[0];
 };
 
-typedef struct microcode          microcode_t;
-typedef struct microcode_header   microcode_header_t;
-
 /* microcode format is extended from prescott processors */
 struct extended_signature {
 	unsigned int            sig;
Signature · 1 line
1.5.4.5
#3 lore raw

PATCH 4/11 x86: Moved per CPU microcode structure declaration to header file.

In reply to Peter Oruba

This structure will be later used by other modules as well and needs therfore to be moved out to a header file.

---
 arch/x86/kernel/microcode.c |    8 +-------
 include/asm-x86/microcode.h |    8 ++++++++
 2 files changed, 9 insertions(+), 7 deletions(-)
arch/x86/kernel/microcode.c +1 -7
index 74e6a77..4e7b2f6 100644
@@ -125,13 +125,7 @@ static DEFINE_SPINLOCK(microcode_update_lock);
 /* no concurrent ->write()s are allowed on /dev/cpu/microcode */
 static DEFINE_MUTEX(microcode_mutex);
 
-static struct ucode_cpu_info {
-	int valid;
-	unsigned int sig;
-	unsigned int pf;
-	unsigned int rev;
-	struct microcode *mc;
-} ucode_cpu_info[NR_CPUS];
+static struct ucode_cpu_info ucode_cpu_info[NR_CPUS];
 
 static void collect_cpu_info(int cpu_num)
 {
include/asm-x86/microcode.h +8 -0
index 1519ef0..d34a1fc 100644
@@ -29,3 +29,11 @@ struct extended_sigtable {
 	unsigned int            reserved[3];
 	struct extended_signature sigs[0];
 };
+
+struct ucode_cpu_info {
+	int valid;
+	unsigned int sig;
+	unsigned int pf;
+	unsigned int rev;
+	struct microcode *mc;
+};
Signature · 1 line
1.5.4.5
#4 lore raw

PATCH 5/11 x86: Moved microcode.c to microcode_intel.c.

In reply to Peter Oruba
---
 arch/x86/kernel/Makefile          |    2 +-
 arch/x86/kernel/microcode.c       |  855 -------------------------------------
 arch/x86/kernel/microcode_intel.c |  855 +++++++++++++++++++++++++++++++++++++
 3 files changed, 856 insertions(+), 856 deletions(-)
 delete mode 100644 arch/x86/kernel/microcode.c
 create mode 100644 arch/x86/kernel/microcode_intel.c
arch/x86/kernel/Makefile +1 -1
index 3db651f..a9be2a0 100644
@@ -51,7 +51,7 @@ obj-$(CONFIG_X86_BIOS_REBOOT)	+= reboot.o
 obj-$(CONFIG_MCA)		+= mca_32.o
 obj-$(CONFIG_X86_MSR)		+= msr.o
 obj-$(CONFIG_X86_CPUID)		+= cpuid.o
-obj-$(CONFIG_MICROCODE)		+= microcode.o
+obj-$(CONFIG_MICROCODE)		+= microcode_intel.o
 obj-$(CONFIG_PCI)		+= early-quirks.o
 apm-y				:= apm_32.o
 obj-$(CONFIG_APM)		+= apm.o
arch/x86/kernel/microcode.c deleted +0 -855

855 lines not on this page · raw message

arch/x86/kernel/microcode_intel.c new file +855 -0

855 lines not on this page · raw message

Signature · 1 line
1.5.4.5
#5 lore raw

Re: [patch 05/11] [PATCH 05/11] x86: Moved microcode.c to microcode_intel.c.

Quoted 4 lines · On Mon, 28 Jul 2008 18:44:16 +0200 Peter Oruba <[email protected]> wrote:
---
 arch/x86/kernel/Makefile          |    2 +-
 arch/x86/kernel/microcode.c       |  855

\

unfortunately this patch breaks userland for me; the Fedora initscripts suddenly fail (with a [FAILED] that will alarm users) because of this rename....
(well it also fails to update the microcode, and if you load the new module name by hand first it still fails in many ways ;-( )

That is a regression in -tip unfortunately....

#6 lore raw

Re: [patch 05/11] [PATCH 05/11] x86: Moved microcode.c to microcode_intel.c.

Arjan,

could you please provide some more information about that issue?

Thanks,
Peter

Quoted 20 lines · Arjan van de Ven
Quoted 4 lines · On Mon, 28 Jul 2008 18:44:16 +0200 Peter Oruba <[email protected]> wrote:
---
 arch/x86/kernel/Makefile          |    2 +-
 arch/x86/kernel/microcode.c       |  855

\

unfortunately this patch breaks userland for me; the Fedora initscripts suddenly fail (with a [FAILED] that will alarm users) because of this rename....
(well it also fails to update the microcode, and if you load the new module name by hand first it still fails in many ways ;-( )

That is a regression in -tip unfortunately....

#7 lore raw

Re: [patch 05/11] [PATCH 05/11] x86: Moved microcode.c to microcode_intel.c.

Quoted 3 lines · On Fri, 12 Sep 2008 13:54:20 +0200 Peter Oruba <[email protected]> wrote:

Arjan,

could you please provide some more information about that issue?

well.. the fedora boot script just plain fails.
I suspect it manually loads microcode.ko or something if CPUID says intel; and this now stops working...

#8 lore raw

Re: [patch 05/11] [PATCH 05/11] x86: Moved microcode.c to microcode_intel.c.

Quoted 10 lines · Arjan van de Ven
Quoted 3 lines · On Fri, 12 Sep 2008 13:54:20 +0200 Peter Oruba <[email protected]> wrote:

Arjan,

could you please provide some more information about that issue?

well.. the fedora boot script just plain fails.
I suspect it manually loads microcode.ko or something if CPUID says intel; and this now stops working...

Also Debian manually load the module, and it remove it after upgrading microcode (no need to have the driver loaded after boot), so changing the name of module is not so fortunate, also considering the long releases cycle of Debian.

I would like to have a single microcode module for the two drivers: generic kernel need both (thus reduce overhead); specialized kernel are configured only with the relevant driver, so no additional overhead.

ciao
cate

#9 lore raw

Re: [patch 05/11] [PATCH 05/11] x86: Moved microcode.c to microcode_intel.c.

In reply to Arjan van de Ven

Some additonal words regarding the current user space issues:

IMHO the most convenient way to update microcode is through the firmware loading interface instead of microcode_ctl. This reduces user-space responsibilities to loading the correct module at boot time and to place the microcode patch file at the right location via package installation. The problems mentioned in this thread would then probably disappear as well. What do you guys think?

Thanks,
Peter

Quoted 12 lines · Arjan van de Ven
Quoted 3 lines · On Fri, 12 Sep 2008 13:54:20 +0200 Peter Oruba <[email protected]> wrote:

Arjan,

could you please provide some more information about that issue?

well.. the fedora boot script just plain fails.
I suspect it manually loads microcode.ko or something if CPUID says intel; and this now stops working...

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 McCoy
#10 lore raw

Re: [patch 05/11] [PATCH 05/11] x86: Moved microcode.c to microcode_intel.c.

2008/9/19 Peter Oruba <[email protected]>:

Quoted 7 lines

Some additonal words regarding the current user space issues:

IMHO the most convenient way to update microcode is through the firmware loading interface instead of microcode_ctl. This reduces user-space responsibilities to loading the correct module at boot time and to place the microcode patch file at the right location via package installation. The problems mentioned in this thread would then probably disappear as well. What do you guys think?

It'd still require changes for all the setups that currently rely on the 'microcode_ctl' interface. Moreover, Arjan's setup failed not due to the 'microcode_ctl' per se but due to the altered kernel module name. After all, we can't break the established interface this way.

We can either reserve 'microcode' as a legacy name for intel cpus (== microcode_intel), or maybe we can use request_module() from microcode.ko to load a proper arch-specific module (I guess, it's not ok for !KMOD-enabled kernels).

Quoted 4 lines

Thanks,
Peter

Signature · 2 lines
Best regards,
Dmitry Adamushko
#11 lore raw

Re: [patch 05/11] [PATCH 05/11] x86: Moved microcode.c to microcode_intel.c.

Quoted 14 lines · Dmitry Adamushko

2008/9/19 Peter Oruba <[email protected]>:

Quoted 7 lines

Some additonal words regarding the current user space issues:

IMHO the most convenient way to update microcode is through the firmware loading interface instead of microcode_ctl. This reduces user-space responsibilities to loading the correct module at boot time and to place the microcode patch file at the right location via package installation. The problems mentioned in this thread would then probably disappear as well. What do you guys think?

It'd still require changes for all the setups that currently rely on the 'microcode_ctl' interface. Moreover, Arjan's setup failed not due to the 'microcode_ctl' per se but due to the altered kernel module name. After all, we can't break the established interface this way.

Quoted 4 lines

We can either reserve 'microcode' as a legacy name for intel cpus (== microcode_intel), or maybe we can use request_module() from microcode.ko to load a proper arch-specific module (I guess, it's not ok for !KMOD-enabled kernels).

I agree. A wrapper "microcode.ko" module would be nice, in order to allow independent kernel and user space upgrades.

The module name is important also on udev method: only a module load triggers the microcode request in udev, thus also the new method should have stable kernel module name.

ciao
cate

#12 lore raw

Re: [patch 05/11] [PATCH 05/11] x86: Moved microcode.c to microcode_intel.c.

Quoted 27 lines · Giacomo A. Catenazzi
Quoted 18 lines · Dmitry Adamushko

2008/9/19 Peter Oruba <[email protected]>:

Quoted 11 lines

Some additonal words regarding the current user space issues:

IMHO the most convenient way to update microcode is through the firmware loading
interface instead of microcode_ctl. This reduces user-space responsibilities to
loading the correct module at boot time and to place the microcode patch file at
the right location via package installation. The problems mentioned in this
thread would then probably disappear as well. What do you guys think?

It'd still require changes for all the setups that currently rely on the 'microcode_ctl' interface. Moreover, Arjan's setup failed not due to the 'microcode_ctl' per se but due to the altered kernel module name. After all, we can't break the established interface this way.

Quoted 4 lines

We can either reserve 'microcode' as a legacy name for intel cpus (== microcode_intel), or maybe we can use request_module() from microcode.ko to load a proper arch-specific module (I guess, it's not ok for !KMOD-enabled kernels).

I agree. A wrapper "microcode.ko" module would be nice, in order to allow independent kernel and user space upgrades.

Quoted 7 lines

The module name is important also on udev method: only a module load triggers the microcode request in udev, thus also the new method should have stable kernel module name.

ciao
cate

That sounds like a single-module solution would be the best way to go. All dependencies would then be handled inside the module.

-Peter

#13 lore raw

Re: [patch 05/11] [PATCH 05/11] x86: Moved microcode.c to microcode_intel.c.

Quoted 30 lines · Peter Oruba
Quoted 29 lines · Giacomo A. Catenazzi
Quoted 17 lines · Dmitry Adamushko

2008/9/19 Peter Oruba <[email protected]>:

Quoted 11 lines

Some additonal words regarding the current user space issues:

IMHO the most convenient way to update microcode is through the firmware loading
interface instead of microcode_ctl. This reduces user-space responsibilities to
loading the correct module at boot time and to place the microcode patch file at
the right location via package installation. The problems mentioned in this
thread would then probably disappear as well. What do you guys think?

It'd still require changes for all the setups that currently rely on the 'microcode_ctl' interface. Moreover, Arjan's setup failed not due to the 'microcode_ctl' per se but due to the altered kernel module name. After all, we can't break the established interface this way.

Quoted 4 lines

We can either reserve 'microcode' as a legacy name for intel cpus (== microcode_intel), or maybe we can use request_module() from microcode.ko to load a proper arch-specific module (I guess, it's not ok for !KMOD-enabled kernels).

I agree. A wrapper "microcode.ko" module would be nice, in order to allow independent kernel and user space upgrades.

The module name is important also on udev method: only a module load triggers the microcode request in udev, thus also the new method should have stable kernel module name.

Quoted 6 lines
Quoted 3 lines

ciao
cate

That sounds like a single-module solution would be the best way to go. All dependencies would then be handled inside the module.

Single module probably is more difficult to maintain.

I was thinking about a very simple additional module:
Check the cpu: load the relevant module, and wrapper the calls to the relevant module.

So every vendor could develop easier the own driver. Only the interface should be stable.

Eventually we could solve it in the distribution scripts (using module aliases), but it would be kernel version dependent, and it would works only on the new method (or a new microcode_ctl version, but people upgrade more often kernel than packages).

ciao
cate

#14 lore raw

Re: [patch 05/11] [PATCH 05/11] x86: Moved microcode.c to microcode_intel.c.

Quoted 4 lines · Giacomo A. Catenazzi
Quoted 2 lines

That sounds like a single-module solution would be the best way to go. All dependencies would then be handled inside the module.

Single module probably is more difficult to maintain.

why? In this case we have just two relevant CPU microcode drivers - and we dont expect their number to grow significantly. So it should be perfectly clean to link everything into a single module.

In fact as this example has shown it a single-module solution has numerous advantages, as it simplifies userspace interactions.

Taken to the extreme, a single-image (bzImage) kernel has many advantages over a many-modules kernel as well. So we only want to modularize when we absolutely have to, and this is not one of those cases.

Ingo

#15 lore raw

Re: [patch 05/11] [PATCH 05/11] x86: Moved microcode.c to microcode_intel.c.

In reply to Peter Oruba
Quoted 30 lines · Peter Oruba
Quoted 29 lines · Giacomo A. Catenazzi
Quoted 17 lines · Dmitry Adamushko

2008/9/19 Peter Oruba <[email protected]>:

Quoted 11 lines

Some additonal words regarding the current user space issues:

IMHO the most convenient way to update microcode is through the firmware loading
interface instead of microcode_ctl. This reduces user-space responsibilities to
loading the correct module at boot time and to place the microcode patch file at
the right location via package installation. The problems mentioned in this
thread would then probably disappear as well. What do you guys think?

It'd still require changes for all the setups that currently rely on the 'microcode_ctl' interface. Moreover, Arjan's setup failed not due to the 'microcode_ctl' per se but due to the altered kernel module name. After all, we can't break the established interface this way.

Quoted 4 lines

We can either reserve 'microcode' as a legacy name for intel cpus (== microcode_intel), or maybe we can use request_module() from microcode.ko to load a proper arch-specific module (I guess, it's not ok for !KMOD-enabled kernels).

I agree. A wrapper "microcode.ko" module would be nice, in order to allow independent kernel and user space upgrades.

The module name is important also on udev method: only a module load triggers the microcode request in udev, thus also the new method should have stable kernel module name.

Quoted 6 lines
Quoted 3 lines

ciao
cate

That sounds like a single-module solution would be the best way to go. All dependencies would then be handled inside the module.

Single module probably is more difficult to maintain.

I was thinking about a very simple additional module:
It checks the cpu: load the relevant module, and wrapper the calls to the relevant module.

So every vendor could develop easier the own driver. Only the interface should be stable.

Eventually we could solve it in the distribution scripts (using module aliases), but it would be kernel version dependent, and it would works only on the new method (or a new microcode_ctl version, but people upgrade more often kernel than packages).

ciao
cate

#16 lore raw

Re: [patch 05/11] [PATCH 05/11] x86: Moved microcode.c to microcode_intel.c.

Quoted 46 lines · On Fri, 19 Sep 2008 16:06:53 +0200 "Giacomo A. Catenazzi" <[email protected]>…
Quoted 32 lines · Peter Oruba
Quoted 31 lines · Giacomo A. Catenazzi
Quoted 19 lines · Dmitry Adamushko

2008/9/19 Peter Oruba <[email protected]>:

Quoted 12 lines

Some additonal words regarding the current user space issues:

IMHO the most convenient way to update microcode is through the firmware loading
interface instead of microcode_ctl. This reduces user-space responsibilities to
loading the correct module at boot time and to place the microcode patch file at
the right location via package installation. The problems mentioned in this
thread would then probably disappear as well. What do you guys think?

It'd still require changes for all the setups that currently rely on the 'microcode_ctl' interface. Moreover, Arjan's setup failed not due to the 'microcode_ctl' per se but due to the altered kernel module name. After all, we can't break the established interface this way.

Quoted 4 lines

We can either reserve 'microcode' as a legacy name for intel cpus (== microcode_intel), or maybe we can use request_module() from microcode.ko to load a proper arch-specific module (I guess, it's not ok for !KMOD-enabled kernels).

I agree. A wrapper "microcode.ko" module would be nice, in order to allow independent kernel and user space upgrades.

The module name is important also on udev method: only a module load triggers the microcode request in udev, thus also the new method should have stable kernel module name.

Quoted 6 lines
Quoted 3 lines

ciao
cate

That sounds like a single-module solution would be the best way to go. All dependencies would then be handled inside the module.

Single module probably is more difficult to maintain.

I was thinking about a very simple additional module:
It checks the cpu: load the relevant module, and wrapper the calls to the relevant module.

Quoted 7 lines

So every vendor could develop easier the own driver. Only the interface should be stable.

Eventually we could solve it in the distribution scripts (using module aliases), but it would be kernel version dependent, and it would works only on the new method (or a new microcode_ctl version, but people upgrade more often kernel than packages).

this would still fail the fedora microcode_ctl scripts; those also do an rmmod afterwards, which now will fail with our method

what a mess ;-(

Signature · 3 lines
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org
#17 lore raw

Re: [patch 05/11] [PATCH 05/11] x86: Moved microcode.c to microcode_intel.c.

In reply to Peter Oruba
Quoted 32 lines · Peter Oruba
Quoted 31 lines · Giacomo A. Catenazzi
Quoted 18 lines · Dmitry Adamushko

2008/9/19 Peter Oruba <[email protected]>:

Quoted 11 lines

Some additonal words regarding the current user space issues:

IMHO the most convenient way to update microcode is through the firmware loading
interface instead of microcode_ctl. This reduces user-space responsibilities to
loading the correct module at boot time and to place the microcode patch file at
the right location via package installation. The problems mentioned in this
thread would then probably disappear as well. What do you guys think?

It'd still require changes for all the setups that currently rely on the 'microcode_ctl' interface. Moreover, Arjan's setup failed not due to the 'microcode_ctl' per se but due to the altered kernel module name. After all, we can't break the established interface this way.

Quoted 4 lines

We can either reserve 'microcode' as a legacy name for intel cpus (== microcode_intel), or maybe we can use request_module() from microcode.ko to load a proper arch-specific module (I guess, it's not ok for !KMOD-enabled kernels).

I agree. A wrapper "microcode.ko" module would be nice, in order to allow independent kernel and user space upgrades.

The module name is important also on udev method: only a module load triggers the microcode request in udev, thus also the new method should have stable kernel module name.

Quoted 6 lines
Quoted 3 lines

ciao
cate

That sounds like a single-module solution would be the best way to go. All dependencies would then be handled inside the module.

yes - as long as the internal abstraction is clean (and it is rather clean with Dmitry's changes applied too), that should be fine and maintainable.

Ingo

#18 lore raw

Re: [patch 05/11] [PATCH 05/11] x86: Moved microcode.c to microcode_intel.c.

In reply to Peter Oruba
Quoted 8 lines · On Fri, 19 Sep 2008 13:59:39 +0200 Peter Oruba <[email protected]> wrote:

Some additonal words regarding the current user space issues:

IMHO the most convenient way to update microcode is through the firmware loading interface instead of microcode_ctl. This reduces user-space responsibilities to loading the correct module at boot time and to place the microcode patch file at the right location via package installation. The problems mentioned in this thread would then probably disappear as well. What do you guys think?

while I absolutely don't disagree with what you say, breaking existing userspace is not an option.
Yes I very much would like for microcode_ctl to die as well (in fact, that was one of the reasons we had the microcode code use request_firmware), but that doesn't mean we can just ditch compatibility ;(

#19 lore raw

PATCH 6/11 x86: Code split to two parts.

In reply to Peter Oruba

Split off existing code into two seperate files. One file holds general code, the other file vendor specific parts. No functional changes, only refactoring. Temporarily Introduced a new module name 'ucode' for result, due to already taken name 'microcode'.

---
 arch/x86/kernel/Makefile          |    3 +-
 arch/x86/kernel/microcode.c       |  463 +++++++++++++++++++++++++++++++++++++
 arch/x86/kernel/microcode_intel.c |  387 ++-----------------------------
 3 files changed, 488 insertions(+), 365 deletions(-)
 create mode 100644 arch/x86/kernel/microcode.c
arch/x86/kernel/Makefile +2 -1
index a9be2a0..abb32ae 100644
@@ -51,7 +51,8 @@ obj-$(CONFIG_X86_BIOS_REBOOT)	+= reboot.o
 obj-$(CONFIG_MCA)		+= mca_32.o
 obj-$(CONFIG_X86_MSR)		+= msr.o
 obj-$(CONFIG_X86_CPUID)		+= cpuid.o
-obj-$(CONFIG_MICROCODE)		+= microcode_intel.o
+obj-$(CONFIG_MICROCODE)		+= ucode.o
+ucode-objs                      := microcode.o microcode_intel.o
 obj-$(CONFIG_PCI)		+= early-quirks.o
 apm-y				:= apm_32.o
 obj-$(CONFIG_APM)		+= apm.o
arch/x86/kernel/microcode.c new file +463 -0

463 lines not on this page · raw message

arch/x86/kernel/microcode_intel.c +23 -364

485 lines not on this page · raw message

Signature · 1 line
1.5.4.5
#20 lore raw

PATCH 7/11 x86: Structure declaration renaming.

In reply to Peter Oruba

Renamed common structures to vendor specific naming scheme so other vendors will be able to use the same naming convention.

Conflicts:

---
 arch/x86/kernel/microcode_intel.c |   46 +++++++++++++++++++-----------------
 include/asm-x86/microcode.h       |   10 +++++---
 2 files changed, 30 insertions(+), 26 deletions(-)
arch/x86/kernel/microcode_intel.c +24 -22
index eded0a1..ca9861b 100644
@@ -100,17 +100,19 @@ MODULE_AUTHOR("Tigran Aivazian <[email protected]>");
 MODULE_LICENSE("GPL");
 
 #define DEFAULT_UCODE_DATASIZE 	(2000) 	  /* 2000 bytes */
-#define MC_HEADER_SIZE		(sizeof(struct microcode_header))  	  /* 48 bytes */
+#define MC_HEADER_SIZE		(sizeof(struct microcode_header_intel)) /* 48 bytes */
 #define DEFAULT_UCODE_TOTALSIZE (DEFAULT_UCODE_DATASIZE + MC_HEADER_SIZE) /* 2048 bytes */
 #define EXT_HEADER_SIZE		(sizeof(struct extended_sigtable)) /* 20 bytes */
 #define EXT_SIGNATURE_SIZE	(sizeof(struct extended_signature)) /* 12 bytes */
 #define DWSIZE			(sizeof(u32))
 #define get_totalsize(mc) \
-	(((struct microcode *)mc)->hdr.totalsize ? \
-	 ((struct microcode *)mc)->hdr.totalsize : DEFAULT_UCODE_TOTALSIZE)
+	(((struct microcode_intel *)mc)->hdr.totalsize ? \
+	 ((struct microcode_intel *)mc)->hdr.totalsize : \
+	 DEFAULT_UCODE_TOTALSIZE)
+
 #define get_datasize(mc) \
-	(((struct microcode *)mc)->hdr.datasize ? \
-	 ((struct microcode *)mc)->hdr.datasize : DEFAULT_UCODE_DATASIZE)
+	(((struct microcode_intel *)mc)->hdr.datasize ? \
+	 ((struct microcode_intel *)mc)->hdr.datasize : DEFAULT_UCODE_DATASIZE)
 
 #define sigmatch(s1, s2, p1, p2) \
 	(((s1) == (s2)) && (((p1) & (p2)) || (((p1) == 0) && ((p2) == 0))))
@@ -134,7 +136,7 @@ void collect_cpu_info(int cpu_num)
 	/* We should bind the task to the CPU */
 	BUG_ON(raw_smp_processor_id() != cpu_num);
 	uci->pf = uci->rev = 0;
-	uci->mc = NULL;
+	uci->mc.mc_intel = NULL;
 	uci->valid = 1;
 
 	if (c->x86_vendor != X86_VENDOR_INTEL || c->x86 < 6 ||
@@ -163,7 +165,7 @@ void collect_cpu_info(int cpu_num)
 }
 
 static inline int microcode_update_match(int cpu_num,
-	struct microcode_header *mc_header, int sig, int pf)
+	struct microcode_header_intel *mc_header, int sig, int pf)
 {
 	struct ucode_cpu_info *uci = ucode_cpu_info + cpu_num;
 
@@ -175,7 +177,7 @@ static inline int microcode_update_match(int cpu_num,
 
 int microcode_sanity_check(void *mc)
 {
-	struct microcode_header *mc_header = mc;
+	struct microcode_header_intel *mc_header = mc;
 	struct extended_sigtable *ext_header = NULL;
 	struct extended_signature *ext_sig;
 	unsigned long total_size, data_size, ext_table_size;
@@ -260,7 +262,7 @@ int microcode_sanity_check(void *mc)
 int get_matching_microcode(void *mc, int cpu)
 {
 	struct ucode_cpu_info *uci = ucode_cpu_info + cpu;
-	struct microcode_header *mc_header = mc;
+	struct microcode_header_intel *mc_header = mc;
 	struct extended_sigtable *ext_header;
 	unsigned long total_size = get_totalsize(mc_header);
 	int ext_sigcount, i;
@@ -294,10 +296,10 @@ find:
 	}
 
 	/* free previous update file */
-	vfree(uci->mc);
+	vfree(uci->mc.mc_intel);
 
 	memcpy(new_mc, mc, total_size);
-	uci->mc = new_mc;
+	uci->mc.mc_intel = new_mc;
 	return 1;
 }
 
@@ -311,7 +313,7 @@ void apply_microcode(int cpu)
 	/* We should bind the task to the CPU */
 	BUG_ON(cpu_num != cpu);
 
-	if (uci->mc == NULL)
+	if (uci->mc.mc_intel == NULL)
 		return;
 
 	/* serialize access to the physical write to MSR 0x79 */
@@ -319,8 +321,8 @@ void apply_microcode(int cpu)
 
 	/* write microcode via MSR 0x79 */
 	wrmsr(MSR_IA32_UCODE_WRITE,
-		(unsigned long) uci->mc->bits,
-		(unsigned long) uci->mc->bits >> 16 >> 16);
+	      (unsigned long) uci->mc.mc_intel->bits,
+	      (unsigned long) uci->mc.mc_intel->bits >> 16 >> 16);
 	wrmsr(MSR_IA32_UCODE_REV, 0, 0);
 
 	/* see notes above for revision 1.07.  Apparent chip bug */
@@ -330,14 +332,14 @@ void apply_microcode(int cpu)
 	rdmsr(MSR_IA32_UCODE_REV, val[0], val[1]);
 
 	spin_unlock_irqrestore(&microcode_update_lock, flags);
-	if (val[1] != uci->mc->hdr.rev) {
+	if (val[1] != uci->mc.mc_intel->hdr.rev) {
 		printk(KERN_ERR "microcode: CPU%d update from revision "
 			"0x%x to 0x%x failed\n", cpu_num, uci->rev, val[1]);
 		return;
 	}
 	printk(KERN_INFO "microcode: CPU%d updated from revision "
 	       "0x%x to 0x%x, date = %08x \n",
-	       cpu_num, uci->rev, val[1], uci->mc->hdr.date);
+	       cpu_num, uci->rev, val[1], uci->mc.mc_intel->hdr.date);
 	uci->rev = val[1];
 }
 
@@ -347,7 +349,7 @@ extern unsigned int user_buffer_size;   /* it's size */
 
 long get_next_ucode(void **mc, long offset)
 {
-	struct microcode_header mc_header;
+	struct microcode_header_intel mc_header;
 	unsigned long total_size;
 
 	/* No more data */
@@ -378,13 +380,13 @@ long get_next_ucode(void **mc, long offset)
 static long get_next_ucode_from_buffer(void **mc, const u8 *buf,
 	unsigned long size, long offset)
 {
-	struct microcode_header *mc_header;
+	struct microcode_header_intel *mc_header;
 	unsigned long total_size;
 
 	/* No more data */
 	if (offset >= size)
 		return 0;
-	mc_header = (struct microcode_header *)(buf + offset);
+	mc_header = (struct microcode_header_intel *)(buf + offset);
 	total_size = get_totalsize(mc_header);
 
 	if (offset + total_size > size) {
@@ -463,7 +465,7 @@ int apply_microcode_check_cpu(int cpu)
 	int err = 0;
 
 	/* Check if the microcode is available */
-	if (!uci->mc)
+	if (!uci->mc.mc_intel)
 		return 0;
 
 	old = current->cpus_allowed;
@@ -508,7 +510,7 @@ void microcode_fini_cpu(int cpu)
 
 	mutex_lock(&microcode_mutex);
 	uci->valid = 0;
-	kfree(uci->mc);
-	uci->mc = NULL;
+	kfree(uci->mc.mc_intel);
+	uci->mc.mc_intel = NULL;
 	mutex_unlock(&microcode_mutex);
 }
include/asm-x86/microcode.h +6 -4
index d34a1fc..ef77c6f 100644
@@ -1,4 +1,4 @@
-struct microcode_header {
+struct microcode_header_intel {
 	unsigned int            hdrver;
 	unsigned int            rev;
 	unsigned int            date;
@@ -11,8 +11,8 @@ struct microcode_header {
 	unsigned int            reserved[3];
 };
 
-struct microcode {
-	struct microcode_header hdr;
+struct microcode_intel {
+	struct microcode_header_intel hdr;
 	unsigned int            bits[0];
 };
 
@@ -35,5 +35,7 @@ struct ucode_cpu_info {
 	unsigned int sig;
 	unsigned int pf;
 	unsigned int rev;
-	struct microcode *mc;
+	union {
+		struct microcode_intel *mc_intel;
+	} mc;
 };
Signature · 1 line
1.5.4.5
#21 lore raw

PATCH 8/11 x86: Add AMD specific declarations.

In reply to Peter Oruba

Added AMD specific declarations to header file.

---
 include/asm-x86/microcode.h |   30 ++++++++++++++++++++++++++++++
 1 files changed, 30 insertions(+), 0 deletions(-)
include/asm-x86/microcode.h +30 -0
index ef77c6f..4e94172 100644
@@ -30,6 +30,35 @@ struct extended_sigtable {
 	struct extended_signature sigs[0];
 };
 
+struct equiv_cpu_entry {
+	unsigned int installed_cpu;
+	unsigned int fixed_errata_mask;
+	unsigned int fixed_errata_compare;
+	unsigned int equiv_cpu;
+};
+
+struct microcode_header_amd {
+	unsigned int  data_code;
+	unsigned int  patch_id;
+	unsigned char mc_patch_data_id[2];
+	unsigned char mc_patch_data_len;
+	unsigned char init_flag;
+	unsigned int  mc_patch_data_checksum;
+	unsigned int  nb_dev_id;
+	unsigned int  sb_dev_id;
+	unsigned char processor_rev_id[2];
+	unsigned char nb_rev_id;
+	unsigned char sb_rev_id;
+	unsigned char bios_api_rev;
+	unsigned char reserved1[3];
+	unsigned int  match_reg[8];
+};
+
+struct microcode_amd {
+	struct microcode_header_amd hdr;
+	unsigned int mpb[0];
+};
+
 struct ucode_cpu_info {
 	int valid;
 	unsigned int sig;
@@ -37,5 +66,6 @@ struct ucode_cpu_info {
 	unsigned int rev;
 	union {
 		struct microcode_intel *mc_intel;
+		struct microcode_amd *mc_amd;
 	} mc;
 };
Signature · 1 line
1.5.4.5
#22 lore raw

PATCH 9/11 x86: First step of refactoring, introducing microcode_ops.

In reply to Peter Oruba

Refactoring with the goal of having one general module and separate vendor specific modules that hook into the general one.

Microcode_ops is a function pointer structure in which vendor specific modules will enter all functions that differ between vendors and that need to be accessed from the general module.

---
 include/asm-x86/microcode.h |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)
include/asm-x86/microcode.h +13 -0
index 4e94172..9231c87 100644
@@ -1,3 +1,16 @@
+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;
Signature · 1 line
1.5.4.5
#23 lore raw

PATCH 10/11 x86: Major refactoring.

In reply to Peter Oruba

Refactored code by introducing a two-module solution. There is one general module in which vendor specific modules can hook into. However, that is exclusive, there is only one vendor specific module allowed at a time. A CPU vendor check makes sure only the corect module for the underlying system gets called. Functinally in terms of patch loading itself there are no changes. This refactoring provides a basis for future implementations of other vendors' patch loaders.

---
 arch/x86/Kconfig                  |   25 +++++++++--
 arch/x86/kernel/Makefile          |    4 +-
 arch/x86/kernel/microcode.c       |   80 +++++++++++++++++++++---------------
 arch/x86/kernel/microcode_intel.c |   50 ++++++++++++++++++-----
 include/asm-x86/microcode.h       |    3 +
 5 files changed, 112 insertions(+), 50 deletions(-)
arch/x86/Kconfig +20 -5
index b6fa287..6b0b885 100644
@@ -782,7 +782,7 @@ config X86_REBOOTFIXUPS
 	  Say N otherwise.
 
 config MICROCODE
-	tristate "/dev/cpu/microcode - Intel IA32 CPU microcode support"
+	tristate "/dev/cpu/microcode - microcode support"
 	select FW_LOADER
 	---help---
 	  If you say Y here, you will be able to update the microcode on
@@ -791,14 +791,29 @@ config MICROCODE
 	  actual microcode binary data itself which is not shipped with the
 	  Linux kernel.
 
-	  For latest news and information on obtaining all the required
-	  ingredients for this driver, check:
-	  <http://www.urbanmyth.org/microcode/>.
+	  This option selects the general module only, you need to select
+	  at least one vendor specific module as well.
 
 	  To compile this driver as a module, choose M here: the
 	  module will be called microcode.
 
-config MICROCODE_OLD_INTERFACE
+config MICROCODE_INTEL
+       tristate "Intel microcode patch loading support"
+       depends on MICROCODE
+       default MICROCODE
+       select FW_LOADER
+       --help---
+         This options enables microcode patch loading support for Intel
+         processors.
+
+         For latest news and information on obtaining all the required
+         Intel ingredients for this driver, check:
+         <http://www.urbanmyth.org/microcode/>.
+
+         This driver is only available as a module: the module
+         will be called microcode_intel.  
+
+   config MICROCODE_OLD_INTERFACE
 	def_bool y
 	depends on MICROCODE
 
arch/x86/kernel/Makefile +2 -2
index abb32ae..f2f9f6d 100644
@@ -51,8 +51,8 @@ obj-$(CONFIG_X86_BIOS_REBOOT)	+= reboot.o
 obj-$(CONFIG_MCA)		+= mca_32.o
 obj-$(CONFIG_X86_MSR)		+= msr.o
 obj-$(CONFIG_X86_CPUID)		+= cpuid.o
-obj-$(CONFIG_MICROCODE)		+= ucode.o
-ucode-objs                      := microcode.o microcode_intel.o
+obj-$(CONFIG_MICROCODE)		+= microcode.o
+obj-$(CONFIG_MICROCODE_INTEL)	+= microcode_intel.o
 obj-$(CONFIG_PCI)		+= early-quirks.o
 apm-y				:= apm_32.o
 obj-$(CONFIG_APM)		+= apm.o
arch/x86/kernel/microcode.c +47 -33

171 lines not on this page · raw message

arch/x86/kernel/microcode_intel.c +40 -10

109 lines not on this page · raw message

include/asm-x86/microcode.h +3 -0
index 9231c87..18b2aee 100644
@@ -1,3 +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);
Signature · 1 line
1.5.4.5
#24 lore raw

Re: [patch 10/11] [PATCH 10/11] x86: Major refactoring.

Quoted 10 lines · Peter Oruba

Refactored code by introducing a two-module solution. There is one general module in which vendor specific modules can hook into. However, that is exclusive, there is only one vendor specific module allowed at a time. A CPU vendor check makes sure only the corect module for the underlying system gets called. Functinally in terms of patch loading itself there are no changes. This refactoring provides a basis for future implementations of other vendors' patch loaders.

<snip>

Quoted 4 lines
arch/x86/kernel/microcode.c +0 -0
index c1047d7..1e42e79 100644

<snip>

Quoted 12 lines
arch/x86/kernel/microcode.c +2 -2
@@ -244,9 +243,9 @@ static void microcode_init_cpu(int cpu, int resume)
 
 	set_cpus_allowed_ptr(current, newmask);
 	mutex_lock(&microcode_mutex);
-	collect_cpu_info(cpu);
+	microcode_ops->collect_cpu_info(cpu);
 	if (uci->valid && system_state == SYSTEM_RUNNING && !resume)
-		cpu_request_microcode(cpu);
+		microcode_ops->cpu_request_microcode(cpu);
 	mutex_unlock(&microcode_mutex);
 	set_cpus_allowed_ptr(current, &old);

arch/x86/kernel/microcode.c static ssize_t reload_store(struct sys_device *dev, · line 279

Quoted 8 lines
arch/x86/kernel/microcode.c +1 -1
@@ -274,7 +273,7 @@ static ssize_t reload_store(struct sys_device *dev,
 
 		mutex_lock(&microcode_mutex);
 		if (uci->valid)
-			err = cpu_request_microcode(cpu);
+			err = microcode_ops->cpu_request_microcode(cpu);
 		mutex_unlock(&microcode_mutex);
 		put_online_cpus();
 		set_cpus_allowed_ptr(current, &old);

Peter, question while we're at it. This came up in another thread and I asked the same question to Tigran but he is either on vacation or not paying attention :).

Microcode cpu hotplug handler is messing with the cps_allowed flags of a random process and can race with sched_setaffinity() (pointed by Dmitry). It also makes some assumptions on the overall cpu hotplug sequence which is bad.

It's easy to fix but the question is - does the microcode update need to happen synchronously ? I'm thinking that it does not but I wanted to verify that. If it does not need to be synchronous then we can simply schedule a work queue and do the update there. If it does we could do collect_cpu_info() and load_microcode() in the IPIs.

Max

#25 lore raw

Re: [patch 10/11] [PATCH 10/11] x86: Major refactoring.

Hi Max,

Quoted 1 line · Max Krasnyansky

Peter, question while we're at it. This came up in another thread and I asked the same question to Tigran but he is either on vacation or not paying attention :).

I am neither on vacation nor not paying attention. I simply didn't know the answer to your question :)

Kind regards
Tigran

#26 lore raw

PATCH 11/11 x86: AMD microcode patch loading support.

In reply to Peter Oruba

This patch introduces microcode patch loading for AMD processors. It is based on previous corresponding work for Intel processors.

It hooks into the general patch loading module. Main difference is that a container file format is used to hold all patch data for multiple processors as well as an equivalent CPU table, which comes seperately, as opposed to Intel's microcode patching solution.

Kconfig and Makefile have been changed provice config and build option for new source file.

---
 arch/x86/Kconfig                |   21 ++-
 arch/x86/kernel/Makefile        |    1 +
 arch/x86/kernel/microcode_amd.c |  521 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 539 insertions(+), 4 deletions(-)
 create mode 100644 arch/x86/kernel/microcode_amd.c
arch/x86/Kconfig +17 -4
index 6b0b885..7bb4617 100644
@@ -786,10 +786,12 @@ config MICROCODE
 	select FW_LOADER
 	---help---
 	  If you say Y here, you will be able to update the microcode on
-	  Intel processors in the IA32 family, e.g. Pentium Pro, Pentium II,
-	  Pentium III, Pentium 4, Xeon etc.  You will obviously need the
-	  actual microcode binary data itself which is not shipped with the
-	  Linux kernel.
+	  certain Intel and AMD processors. The Intel support is for the
+	  IA32 family, e.g. Pentium Pro, Pentium II, Pentium III,
+	  Pentium 4, Xeon etc. The AMD support is for family 0x10 and
+	  0x11 processors, e.g. Opteron, Phenom and Turion 64 Ultra.
+	  You will obviously need the actual microcode binary data itself
+	  which is not shipped with the Linux kernel.
 
 	  This option selects the general module only, you need to select
 	  at least one vendor specific module as well.
@@ -813,6 +815,17 @@ config MICROCODE_INTEL
          This driver is only available as a module: the module
          will be called microcode_intel.  
 
+config MICROCODE_AMD
+       tristate "AMD microcode patch loading support"
+       depends on MICROCODE
+       select FW_LOADER
+       --help---
+         If you select this option, microcode patch loading support for AMD
+	 processors will be enabled.
+
+	 This driver is only available as a module: the module
+	 will be called microcode_amd.
+
    config MICROCODE_OLD_INTERFACE
 	def_bool y
 	depends on MICROCODE
arch/x86/kernel/Makefile +1 -0
index f2f9f6d..be454f3 100644
@@ -53,6 +53,7 @@ obj-$(CONFIG_X86_MSR)		+= msr.o
 obj-$(CONFIG_X86_CPUID)		+= cpuid.o
 obj-$(CONFIG_MICROCODE)		+= microcode.o
 obj-$(CONFIG_MICROCODE_INTEL)	+= microcode_intel.o
+obj-$(CONFIG_MICROCODE_AMD)	+= microcode_amd.o
 obj-$(CONFIG_PCI)		+= early-quirks.o
 apm-y				:= apm_32.o
 obj-$(CONFIG_APM)		+= apm.o
arch/x86/kernel/microcode_amd.c new file +521 -0

521 lines not on this page · raw message

Signature · 1 line
1.5.4.5
Applied
#27 lore raw
In reply to Peter Oruba
Quoted 1 line · Peter Oruba

AMD microcode patch loader v2, rebased to Linux master.

applied to tip/x86/microcode - thanks Peter.

One open issue i noticed is the checkpatch warnings:

 $ scripts/checkpatch.pl --file arch/x86/kernel/microcode_amd.c
 total: 0 errors, 10 warnings, 1 checks, 521 lines checked
 $ scripts/checkpatch.pl --file arch/x86/kernel/microcode.c
 total: 2 errors, 12 warnings, 477 lines checked
 $ scripts/checkpatch.pl --file arch/x86/kernel/microcode_intel.c
 total: 1 errors, 11 warnings, 546 lines checked

Could you send delta patch(es) to fix them up? You can pick up the committed patches via:

http://people.redhat.com/mingo/tip.git/README

and do something like:

git-checkout tip/master
git-merge tip/x86/microcode

to get the latest code plus your changes.

Thanks,

Ingo

#28 lore raw

PATCH 1/1 x86, microcode support: fix build error

Quoted 2 lines

From 45b1e23eca1c53fa79a611a2bc8c93697ede6c97 Mon Sep 17 00:00:00 2001

From: Ingo Molnar <[email protected]>
Date: Tue, 29 Jul 2008 09:42:17 +0200
Subject: [PATCH] x86, microcode support: fix build error
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit

fix:

arch/x86/kernel/microcode.c:412: error: static declaration of ‘microcode_init’ follows non-static declaration include/asm/microcode.h:1: error: previous declaration of ‘microcode_init’ was here arch/x86/kernel/microcode.c:454: error: static declaration of ‘microcode_exit’ follows non-static declaration include/asm/microcode.h:2: error: previous declaration of ‘microcode_exit’ was here

---
 arch/x86/kernel/microcode.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
arch/x86/kernel/microcode.c +2 -2
index 1e42e79..9a88184 100644
@@ -415,7 +415,7 @@ static struct notifier_block __refdata mc_cpu_notifier = {
 	.notifier_call = mc_cpu_callback,
 };
 
-static int microcode_init(void *opaque, struct module *module)
+int microcode_init(void *opaque, struct module *module)
 {
 	struct microcode_ops *ops = (struct microcode_ops *)opaque;
 	int error;
@@ -457,7 +457,7 @@ static int microcode_init(void *opaque, struct module *module)
 }
 EXPORT_SYMBOL_GPL(microcode_init);
 
-static void __exit microcode_exit (void)
+void __exit microcode_exit (void)
 {
 	microcode_dev_exit();
 
#29 lore raw
In reply to Ingo Molnar
Quoted 2 lines

From 224e946b81166d732f3e9b414cb69612072fb500 Mon Sep 17 00:00:00 2001

From: Ingo Molnar <[email protected]>
Date: Tue, 29 Jul 2008 09:52:55 +0200
Subject: [PATCH] x86, microcode: fix symbol exports

fix tons of build errors:

arch/x86/kernel/built-in.o: In function `microcode_fini_cpu':
microcode_intel.c:(.text+0x11598): undefined reference to microcode_mutex' microcode_intel.c:(.text+0x115a4): undefined reference to ucode_cpu_info' microcode_intel.c:(.text+0x115ae): undefined reference to ucode_cpu_info' microcode_intel.c:(.text+0x115bc): undefined reference to microcode_mutex' [...]

---
 arch/x86/kernel/microcode.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)
arch/x86/kernel/microcode.c +5 -5
index 9a88184..758b958 100644
@@ -104,16 +104,16 @@ MODULE_LICENSE("GPL");
 struct microcode_ops *microcode_ops;
 
 /* no concurrent ->write()s are allowed on /dev/cpu/microcode */
-static DEFINE_MUTEX(microcode_mutex);
+DEFINE_MUTEX(microcode_mutex);
 EXPORT_SYMBOL_GPL(microcode_mutex);
 
-static struct ucode_cpu_info ucode_cpu_info[NR_CPUS];
+struct ucode_cpu_info ucode_cpu_info[NR_CPUS];
 EXPORT_SYMBOL_GPL(ucode_cpu_info);
 
 #ifdef CONFIG_MICROCODE_OLD_INTERFACE
-static void __user *user_buffer;	/* user area microcode data buffer */
+void __user *user_buffer;		/* user area microcode data buffer */
 EXPORT_SYMBOL_GPL(user_buffer);
-static unsigned int user_buffer_size;	/* it's size */
+unsigned int user_buffer_size;		/* it's size */
 EXPORT_SYMBOL_GPL(user_buffer_size);
 
 static int do_microcode_update (void)
@@ -230,7 +230,7 @@ MODULE_ALIAS_MISCDEV(MICROCODE_MINOR);
 #endif
 
 /* fake device for request_firmware */
-static struct platform_device *microcode_pdev;
+struct platform_device *microcode_pdev;
 EXPORT_SYMBOL_GPL(microcode_pdev);
 
 static void microcode_init_cpu(int cpu, int resume)
#30 lore raw
In reply to Ingo Molnar
Quoted 2 lines

From 5d7b605245b1aa1a9cd6549b1f57d69273eb0c37 Mon Sep 17 00:00:00 2001

From: Ingo Molnar <[email protected]>
Date: Tue, 29 Jul 2008 10:07:36 +0200
Subject: [PATCH] x86, microcode: fix module license string

fix:

FATAL: modpost: GPL-incompatible module microcode_amd.ko uses GPL-only symbol 'set_cpus_allowed_ptr'

---
 arch/x86/kernel/microcode_amd.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
arch/x86/kernel/microcode_amd.c +1 -1
index db199e3..fd9e68e 100644
@@ -40,7 +40,7 @@
 
 MODULE_DESCRIPTION("AMD Microcode Update Driver");
 MODULE_AUTHOR("Peter Oruba <[email protected]>");
-MODULE_LICENSE("GPLv2");
+MODULE_LICENSE("GPL v2");
 
 #define UCODE_MAGIC                0x00414d44
 #define UCODE_EQUIV_CPU_TABLE_TYPE 0x00000000
#31 lore raw
In reply to Ingo Molnar
Quoted 6 lines · Ingo Molnar

and do something like:

git-checkout tip/master
git-merge tip/x86/microcode

to get the latest code plus your changes.

i've pushed out 3 build fixes into tip/x86/microcode and test-merged it into tip/master. I'll test it some more today before pushing out its integration.

Ingo