[PATCH 0/3] Xen Microcode update driver for 2.6.38

[PATCH 0/3] Xen Microcode update driver for 2.6.38

[Jeremy Fitzhardinge]

From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>

Hi all,

This series adds a new microcode driver for Xen.  The Xen hypervisor
can deal with all the low-level details of doing a microcode update
(Intel vs AMD, doing all the physical CPUs present on the system,
current and future, etc), so all the driver has to do is make a
hypercall to upload the microcode into Xen.

This only works on a privileged domain, of course.  But the Xen driver
also detects any time we're running paravirtualized under Xen to
prevent any attempts at a microcode update from a non-privileged
domain as well.

Thanks,
	J

Jeremy Fitzhardinge (2):
  xen: add CPU microcode update driver
  xen/microcode: partially enable even for non-privileged kernels

Stephen Tweedie (1):
  xen dom0: Add support for the platform_ops hypercall

 arch/x86/include/asm/microcode.h     |    9 ++
 arch/x86/include/asm/xen/hypercall.h |    8 ++
 arch/x86/kernel/Makefile             |    1 +
 arch/x86/kernel/microcode_core.c     |    5 +-
 arch/x86/kernel/microcode_xen.c      |  201 ++++++++++++++++++++++++++++++
 arch/x86/xen/Kconfig                 |    8 ++
 include/xen/interface/platform.h     |  222 ++++++++++++++++++++++++++++++++++
 include/xen/interface/xen.h          |    2 +
 8 files changed, 455 insertions(+), 1 deletions(-)
 create mode 100644 arch/x86/kernel/microcode_xen.c
 create mode 100644 include/xen/interface/platform.h

-- 
1.7.2.3

[PATCH 1/3] xen dom0: Add support for the platform_ops hypercall

[Jeremy Fitzhardinge]

From: Stephen Tweedie <sct@redhat.com>

Minimal changes to get platform ops (renamed dom0_ops on pv_ops) working
on pv_ops builds.  Pulls in upstream linux-2.6.18-xen.hg's platform.h

[ Impact: add Xen hypercall definitions ]

Signed-off-by: Stephen Tweedie <sct@redhat.com>
Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
 arch/x86/include/asm/xen/hypercall.h |    8 ++
 include/xen/interface/platform.h     |  222 ++++++++++++++++++++++++++++++++++
 include/xen/interface/xen.h          |    2 +
 3 files changed, 232 insertions(+), 0 deletions(-)
 create mode 100644 include/xen/interface/platform.h

diff --git a/arch/x86/include/asm/xen/hypercall.h b/arch/x86/include/asm/xen/hypercall.h
index a3c28ae..3d10d04 100644
--- a/arch/x86/include/asm/xen/hypercall.h
+++ b/arch/x86/include/asm/xen/hypercall.h
@@ -45,6 +45,7 @@
 #include <xen/interface/xen.h>
 #include <xen/interface/sched.h>
 #include <xen/interface/physdev.h>
+#include <xen/interface/platform.h>
 
 /*
  * The hypercall asms have to meet several constraints:
@@ -299,6 +300,13 @@ HYPERVISOR_set_timer_op(u64 timeout)
 }
 
 static inline int
+HYPERVISOR_dom0_op(struct xen_platform_op *platform_op)
+{
+	platform_op->interface_version = XENPF_INTERFACE_VERSION;
+	return _hypercall1(int, dom0_op, platform_op);
+}
+
+static inline int
 HYPERVISOR_set_debugreg(int reg, unsigned long value)
 {
 	return _hypercall2(int, set_debugreg, reg, value);
diff --git a/include/xen/interface/platform.h b/include/xen/interface/platform.h
new file mode 100644
index 0000000..83e4714
--- /dev/null
+++ b/include/xen/interface/platform.h
@@ -0,0 +1,222 @@
+/******************************************************************************
+ * platform.h
+ *
+ * Hardware platform operations. Intended for use by domain-0 kernel.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
+ * deal in the Software without restriction, including without limitation the
+ * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ * Copyright (c) 2002-2006, K Fraser
+ */
+
+#ifndef __XEN_PUBLIC_PLATFORM_H__
+#define __XEN_PUBLIC_PLATFORM_H__
+
+#include "xen.h"
+
+#define XENPF_INTERFACE_VERSION 0x03000001
+
+/*
+ * Set clock such that it would read <secs,nsecs> after 00:00:00 UTC,
+ * 1 January, 1970 if the current system time was <system_time>.
+ */
+#define XENPF_settime             17
+struct xenpf_settime {
+    /* IN variables. */
+    uint32_t secs;
+    uint32_t nsecs;
+    uint64_t system_time;
+};
+typedef struct xenpf_settime xenpf_settime_t;
+DEFINE_GUEST_HANDLE_STRUCT(xenpf_settime_t);
+
+/*
+ * Request memory range (@mfn, @mfn+@nr_mfns-1) to have type @type.
+ * On x86, @type is an architecture-defined MTRR memory type.
+ * On success, returns the MTRR that was used (@reg) and a handle that can
+ * be passed to XENPF_DEL_MEMTYPE to accurately tear down the new setting.
+ * (x86-specific).
+ */
+#define XENPF_add_memtype         31
+struct xenpf_add_memtype {
+    /* IN variables. */
+    unsigned long mfn;
+    uint64_t nr_mfns;
+    uint32_t type;
+    /* OUT variables. */
+    uint32_t handle;
+    uint32_t reg;
+};
+typedef struct xenpf_add_memtype xenpf_add_memtype_t;
+DEFINE_GUEST_HANDLE_STRUCT(xenpf_add_memtype_t);
+
+/*
+ * Tear down an existing memory-range type. If @handle is remembered then it
+ * should be passed in to accurately tear down the correct setting (in case
+ * of overlapping memory regions with differing types). If it is not known
+ * then @handle should be set to zero. In all cases @reg must be set.
+ * (x86-specific).
+ */
+#define XENPF_del_memtype         32
+struct xenpf_del_memtype {
+    /* IN variables. */
+    uint32_t handle;
+    uint32_t reg;
+};
+typedef struct xenpf_del_memtype xenpf_del_memtype_t;
+DEFINE_GUEST_HANDLE_STRUCT(xenpf_del_memtype_t);
+
+/* Read current type of an MTRR (x86-specific). */
+#define XENPF_read_memtype        33
+struct xenpf_read_memtype {
+    /* IN variables. */
+    uint32_t reg;
+    /* OUT variables. */
+    unsigned long mfn;
+    uint64_t nr_mfns;
+    uint32_t type;
+};
+typedef struct xenpf_read_memtype xenpf_read_memtype_t;
+DEFINE_GUEST_HANDLE_STRUCT(xenpf_read_memtype_t);
+
+#define XENPF_microcode_update    35
+struct xenpf_microcode_update {
+    /* IN variables. */
+    GUEST_HANDLE(void) data;          /* Pointer to microcode data */
+    uint32_t length;                  /* Length of microcode data. */
+};
+typedef struct xenpf_microcode_update xenpf_microcode_update_t;
+DEFINE_GUEST_HANDLE_STRUCT(xenpf_microcode_update_t);
+
+#define XENPF_platform_quirk      39
+#define QUIRK_NOIRQBALANCING      1 /* Do not restrict IO-APIC RTE targets */
+#define QUIRK_IOAPIC_BAD_REGSEL   2 /* IO-APIC REGSEL forgets its value    */
+#define QUIRK_IOAPIC_GOOD_REGSEL  3 /* IO-APIC REGSEL behaves properly     */
+struct xenpf_platform_quirk {
+    /* IN variables. */
+    uint32_t quirk_id;
+};
+typedef struct xenpf_platform_quirk xenpf_platform_quirk_t;
+DEFINE_GUEST_HANDLE_STRUCT(xenpf_platform_quirk_t);
+
+#define XENPF_firmware_info       50
+#define XEN_FW_DISK_INFO          1 /* from int 13 AH=08/41/48 */
+#define XEN_FW_DISK_MBR_SIGNATURE 2 /* from MBR offset 0x1b8 */
+#define XEN_FW_VBEDDC_INFO        3 /* from int 10 AX=4f15 */
+struct xenpf_firmware_info {
+	/* IN variables. */
+	uint32_t type;
+	uint32_t index;
+	/* OUT variables. */
+	union {
+		struct {
+			/* Int13, Fn48: Check Extensions Present. */
+			uint8_t device;                   /* %dl: bios device number */
+			uint8_t version;                  /* %ah: major version      */
+			uint16_t interface_support;       /* %cx: support bitmap     */
+			/* Int13, Fn08: Legacy Get Device Parameters. */
+			uint16_t legacy_max_cylinder;     /* %cl[7:6]:%ch: max cyl # */
+			uint8_t legacy_max_head;          /* %dh: max head #         */
+			uint8_t legacy_sectors_per_track; /* %cl[5:0]: max sector #  */
+			/* Int13, Fn41: Get Device Parameters (as filled into %ds:%esi). */
+			/* NB. First uint16_t of buffer must be set to buffer size.      */
+			GUEST_HANDLE(void) edd_params;
+		} disk_info; /* XEN_FW_DISK_INFO */
+		struct {
+			uint8_t device;                   /* bios device number  */
+			uint32_t mbr_signature;           /* offset 0x1b8 in mbr */
+		} disk_mbr_signature; /* XEN_FW_DISK_MBR_SIGNATURE */
+		struct {
+			/* Int10, AX=4F15: Get EDID info. */
+			uint8_t capabilities;
+			uint8_t edid_transfer_time;
+			/* must refer to 128-byte buffer */
+			GUEST_HANDLE(uchar) edid;
+		} vbeddc_info; /* XEN_FW_VBEDDC_INFO */
+	} u;
+};
+typedef struct xenpf_firmware_info xenpf_firmware_info_t;
+DEFINE_GUEST_HANDLE_STRUCT(xenpf_firmware_info_t);
+
+#define XENPF_enter_acpi_sleep    51
+struct xenpf_enter_acpi_sleep {
+	/* IN variables */
+	uint16_t pm1a_cnt_val;      /* PM1a control value. */
+	uint16_t pm1b_cnt_val;      /* PM1b control value. */
+	uint32_t sleep_state;       /* Which state to enter (Sn). */
+	uint32_t flags;             /* Must be zero. */
+};
+typedef struct xenpf_enter_acpi_sleep xenpf_enter_acpi_sleep_t;
+DEFINE_GUEST_HANDLE_STRUCT(xenpf_enter_acpi_sleep_t);
+
+#define XENPF_change_freq         52
+struct xenpf_change_freq {
+	/* IN variables */
+	uint32_t flags; /* Must be zero. */
+	uint32_t cpu;   /* Physical cpu. */
+	uint64_t freq;  /* New frequency (Hz). */
+};
+typedef struct xenpf_change_freq xenpf_change_freq_t;
+DEFINE_GUEST_HANDLE_STRUCT(xenpf_change_freq_t);
+
+/*
+ * Get idle times (nanoseconds since boot) for physical CPUs specified in the
+ * @cpumap_bitmap with range [0..@cpumap_nr_cpus-1]. The @idletime array is
+ * indexed by CPU number; only entries with the corresponding @cpumap_bitmap
+ * bit set are written to. On return, @cpumap_bitmap is modified so that any
+ * non-existent CPUs are cleared. Such CPUs have their @idletime array entry
+ * cleared.
+ */
+#define XENPF_getidletime         53
+struct xenpf_getidletime {
+	/* IN/OUT variables */
+	/* IN: CPUs to interrogate; OUT: subset of IN which are present */
+	GUEST_HANDLE(uchar) cpumap_bitmap;
+	/* IN variables */
+	/* Size of cpumap bitmap. */
+	uint32_t cpumap_nr_cpus;
+	/* Must be indexable for every cpu in cpumap_bitmap. */
+	GUEST_HANDLE(uint64_t) idletime;
+	/* OUT variables */
+	/* System time when the idletime snapshots were taken. */
+	uint64_t now;
+};
+typedef struct xenpf_getidletime xenpf_getidletime_t;
+DEFINE_GUEST_HANDLE_STRUCT(xenpf_getidletime_t);
+
+struct xen_platform_op {
+	uint32_t cmd;
+	uint32_t interface_version; /* XENPF_INTERFACE_VERSION */
+	union {
+		struct xenpf_settime           settime;
+		struct xenpf_add_memtype       add_memtype;
+		struct xenpf_del_memtype       del_memtype;
+		struct xenpf_read_memtype      read_memtype;
+		struct xenpf_microcode_update  microcode;
+		struct xenpf_platform_quirk    platform_quirk;
+		struct xenpf_firmware_info     firmware_info;
+		struct xenpf_enter_acpi_sleep  enter_acpi_sleep;
+		struct xenpf_change_freq       change_freq;
+		struct xenpf_getidletime       getidletime;
+		uint8_t                        pad[128];
+	} u;
+};
+typedef struct xen_platform_op xen_platform_op_t;
+DEFINE_GUEST_HANDLE_STRUCT(xen_platform_op_t);
+
+#endif /* __XEN_PUBLIC_PLATFORM_H__ */
diff --git a/include/xen/interface/xen.h b/include/xen/interface/xen.h
index 2befa3e..18b5599 100644
--- a/include/xen/interface/xen.h
+++ b/include/xen/interface/xen.h
@@ -461,6 +461,8 @@ typedef uint8_t xen_domain_handle_t[16];
 #define __mk_unsigned_long(x) x ## UL
 #define mk_unsigned_long(x) __mk_unsigned_long(x)
 
+DEFINE_GUEST_HANDLE(uint64_t);
+
 #else /* __ASSEMBLY__ */
 
 /* In assembly code we cannot use C numeric constant suffixes. */
-- 
1.7.2.3

[PATCH 2/3] xen: add CPU microcode update driver

[Jeremy Fitzhardinge]

From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>

Xen does all the hard work for us, including choosing the right update
method for this cpu type and actually doing it for all cpus.  We just
need to supply it with the firmware blob.

Because Xen updates all CPUs (and the kernel's virtual cpu numbers have
no fixed relationship with the underlying physical cpus), we only bother
doing anything for cpu "0".

[ Impact: allow CPU microcode update in Xen dom0 ]
Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
 arch/x86/include/asm/microcode.h |    9 ++
 arch/x86/kernel/Makefile         |    1 +
 arch/x86/kernel/microcode_core.c |    5 +-
 arch/x86/kernel/microcode_xen.c  |  201 ++++++++++++++++++++++++++++++++++++++
 arch/x86/xen/Kconfig             |    4 +
 5 files changed, 219 insertions(+), 1 deletions(-)
 create mode 100644 arch/x86/kernel/microcode_xen.c

diff --git a/arch/x86/include/asm/microcode.h b/arch/x86/include/asm/microcode.h
index ef51b50..e15fca1 100644
--- a/arch/x86/include/asm/microcode.h
+++ b/arch/x86/include/asm/microcode.h
@@ -55,4 +55,13 @@ static inline struct microcode_ops * __init init_amd_microcode(void)
 }
 #endif
 
+#ifdef CONFIG_MICROCODE_XEN
+extern struct microcode_ops * __init init_xen_microcode(void);
+#else
+static inline struct microcode_ops * __init init_xen_microcode(void)
+{
+	return NULL;
+}
+#endif
+
 #endif /* _ASM_X86_MICROCODE_H */
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 9e13763..0036150 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -104,6 +104,7 @@ obj-$(CONFIG_PCSPKR_PLATFORM)	+= pcspeaker.o
 microcode-y				:= microcode_core.o
 microcode-$(CONFIG_MICROCODE_INTEL)	+= microcode_intel.o
 microcode-$(CONFIG_MICROCODE_AMD)	+= microcode_amd.o
+microcode-$(CONFIG_MICROCODE_XEN)	+= microcode_xen.o
 obj-$(CONFIG_MICROCODE)			+= microcode.o
 
 obj-$(CONFIG_X86_CHECK_BIOS_CORRUPTION) += check.o
diff --git a/arch/x86/kernel/microcode_core.c b/arch/x86/kernel/microcode_core.c
index 1cca374..6550539 100644
--- a/arch/x86/kernel/microcode_core.c
+++ b/arch/x86/kernel/microcode_core.c
@@ -83,6 +83,7 @@
 #include <linux/fs.h>
 #include <linux/mm.h>
 
+#include <xen/xen.h>
 #include <asm/microcode.h>
 #include <asm/processor.h>
 
@@ -506,7 +507,9 @@ static int __init microcode_init(void)
 	struct cpuinfo_x86 *c = &cpu_data(0);
 	int error;
 
-	if (c->x86_vendor == X86_VENDOR_INTEL)
+	if (xen_pv_domain())
+		microcode_ops = init_xen_microcode();
+	else if (c->x86_vendor == X86_VENDOR_INTEL)
 		microcode_ops = init_intel_microcode();
 	else if (c->x86_vendor == X86_VENDOR_AMD)
 		microcode_ops = init_amd_microcode();
diff --git a/arch/x86/kernel/microcode_xen.c b/arch/x86/kernel/microcode_xen.c
new file mode 100644
index 0000000..16c742e
--- /dev/null
+++ b/arch/x86/kernel/microcode_xen.c
@@ -0,0 +1,201 @@
+/*
+ * Xen microcode update driver
+ *
+ * Xen does most of the work here.  We just pass the whole blob into
+ * Xen, and it will apply it to all CPUs as appropriate.  Xen will
+ * worry about how different CPU models are actually updated.
+ */
+#include <linux/sched.h>
+#include <linux/module.h>
+#include <linux/firmware.h>
+#include <linux/vmalloc.h>
+#include <linux/uaccess.h>
+
+#include <asm/microcode.h>
+
+#include <xen/xen.h>
+#include <xen/interface/platform.h>
+#include <xen/interface/xen.h>
+
+#include <asm/xen/hypercall.h>
+#include <asm/xen/hypervisor.h>
+
+MODULE_DESCRIPTION("Xen microcode update driver");
+MODULE_LICENSE("GPL");
+
+struct xen_microcode {
+	size_t len;
+	char data[0];
+};
+
+static int xen_microcode_update(int cpu)
+{
+	int err;
+	struct xen_platform_op op;
+	struct ucode_cpu_info *uci = ucode_cpu_info + cpu;
+	struct xen_microcode *uc = uci->mc;
+
+	if (uc == NULL || uc->len == 0) {
+		/*
+		 * We do all cpus at once, so we don't need to do
+		 * other cpus explicitly (besides, these vcpu numbers
+		 * have no relationship to underlying physical cpus).
+		 */
+		return 0;
+	}
+
+	op.cmd = XENPF_microcode_update;
+	set_xen_guest_handle(op.u.microcode.data, uc->data);
+	op.u.microcode.length = uc->len;
+
+	err = HYPERVISOR_dom0_op(&op);
+
+	if (err != 0)
+		printk(KERN_WARNING "microcode_xen: microcode update failed: %d\n", err);
+
+	return err;
+}
+
+static enum ucode_state xen_request_microcode_fw(int cpu, struct device *device)
+{
+	char name[30];
+	struct cpuinfo_x86 *c = &cpu_data(cpu);
+	const struct firmware *firmware;
+	struct ucode_cpu_info *uci = ucode_cpu_info + cpu;
+	enum ucode_state ret;
+	struct xen_microcode *uc;
+	size_t size;
+	int err;
+
+	switch (c->x86_vendor) {
+	case X86_VENDOR_INTEL:
+		snprintf(name, sizeof(name), "intel-ucode/%02x-%02x-%02x",
+			 c->x86, c->x86_model, c->x86_mask);
+		break;
+
+	case X86_VENDOR_AMD:
+		snprintf(name, sizeof(name), "amd-ucode/microcode_amd.bin");
+		break;
+
+	default:
+		return UCODE_NFOUND;
+	}
+
+	err = request_firmware(&firmware, name, device);
+	if (err) {
+		pr_debug("microcode: data file %s load failed\n", name);
+		return UCODE_NFOUND;
+	}
+
+	/*
+	 * Only bother getting real firmware for cpu 0; the others get
+	 * dummy placeholders.
+	 */
+	if (cpu == 0)
+		size = firmware->size;
+	else
+		size = 0;
+
+	if (uci->mc != NULL) {
+		vfree(uci->mc);
+		uci->mc = NULL;
+	}
+
+	ret = UCODE_ERROR;
+	uc = vmalloc(sizeof(*uc) + size);
+	if (uc == NULL)
+		goto out;
+
+	ret = UCODE_OK;
+	uc->len = size;
+	memcpy(uc->data, firmware->data, uc->len);
+
+	uci->mc = uc;
+
+out:
+	release_firmware(firmware);
+
+	return ret;
+}
+
+static enum ucode_state xen_request_microcode_user(int cpu,
+						   const void __user *buf, size_t size)
+{
+	struct ucode_cpu_info *uci = ucode_cpu_info + cpu;
+	struct xen_microcode *uc;
+	enum ucode_state ret;
+	size_t unread;
+
+	if (cpu != 0) {
+		/* No real firmware for non-zero cpus; just store a
+		   placeholder */
+		size = 0;
+	}
+
+	if (uci->mc != NULL) {
+		vfree(uci->mc);
+		uci->mc = NULL;
+	}
+
+	ret = UCODE_ERROR;
+	uc = vmalloc(sizeof(*uc) + size);
+	if (uc == NULL)
+		goto out;
+
+	uc->len = size;
+
+	ret = UCODE_NFOUND;
+
+	/* XXX This sporadically returns uncopied bytes, so we return
+	   EFAULT.  As far as I can see, the usermode code
+	   (microcode_ctl) isn't doing anything wrong... */
+	unread = copy_from_user(uc->data, buf, size);
+
+	if (unread != 0) {
+		printk(KERN_WARNING "failed to read %zd of %zd bytes at %p -> %p\n",
+		       unread, size, buf, uc->data);
+		goto out;
+	}
+
+	ret = UCODE_OK;
+
+out:
+	if (ret == 0)
+		uci->mc = uc;
+	else
+		vfree(uc);
+
+	return ret;
+}
+
+static void xen_microcode_fini_cpu(int cpu)
+{
+	struct ucode_cpu_info *uci = ucode_cpu_info + cpu;
+
+	vfree(uci->mc);
+	uci->mc = NULL;
+}
+
+static int xen_collect_cpu_info(int cpu, struct cpu_signature *sig)
+{
+	sig->sig = 0;
+	sig->pf = 0;
+	sig->rev = 0;
+
+	return 0;
+}
+
+static struct microcode_ops microcode_xen_ops = {
+	.request_microcode_user		  = xen_request_microcode_user,
+	.request_microcode_fw             = xen_request_microcode_fw,
+	.collect_cpu_info                 = xen_collect_cpu_info,
+	.apply_microcode                  = xen_microcode_update,
+	.microcode_fini_cpu               = xen_microcode_fini_cpu,
+};
+
+struct microcode_ops * __init init_xen_microcode(void)
+{
+	if (!xen_initial_domain())
+		return NULL;
+	return &microcode_xen_ops;
+}
diff --git a/arch/x86/xen/Kconfig b/arch/x86/xen/Kconfig
index 5b54892..384e0a5 100644
--- a/arch/x86/xen/Kconfig
+++ b/arch/x86/xen/Kconfig
@@ -48,3 +48,7 @@ config XEN_DEBUG_FS
 	help
 	  Enable statistics output and various tuning options in debugfs.
 	  Enabling this option may incur a significant performance overhead.
+
+config MICROCODE_XEN
+       def_bool y
+       depends on XEN_DOM0 && MICROCODE
\ No newline at end of file
-- 
1.7.2.3

[PATCH 3/3] xen/microcode: partially enable even for non-privileged kernels

[Jeremy Fitzhardinge]

From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>

Don't attempt to load microcode on non-privileged kernels.  Kernels
compiled without privileged support just get vestigial Xen detection
to skip loading altogether; kernels with privileged support will
load microcode if running privileged.  In either case, the normal
Intel/AMD microcode loader is skipped under Xen.

Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
 arch/x86/include/asm/microcode.h |    2 +-
 arch/x86/kernel/Makefile         |    2 +-
 arch/x86/xen/Kconfig             |    6 +++++-
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/microcode.h b/arch/x86/include/asm/microcode.h
index e15fca1..3e61afe 100644
--- a/arch/x86/include/asm/microcode.h
+++ b/arch/x86/include/asm/microcode.h
@@ -55,7 +55,7 @@ static inline struct microcode_ops * __init init_amd_microcode(void)
 }
 #endif
 
-#ifdef CONFIG_MICROCODE_XEN
+#ifdef CONFIG_MICROCODE_XEN_LOADER
 extern struct microcode_ops * __init init_xen_microcode(void);
 #else
 static inline struct microcode_ops * __init init_xen_microcode(void)
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 0036150..768c5dc 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -104,7 +104,7 @@ obj-$(CONFIG_PCSPKR_PLATFORM)	+= pcspeaker.o
 microcode-y				:= microcode_core.o
 microcode-$(CONFIG_MICROCODE_INTEL)	+= microcode_intel.o
 microcode-$(CONFIG_MICROCODE_AMD)	+= microcode_amd.o
-microcode-$(CONFIG_MICROCODE_XEN)	+= microcode_xen.o
+microcode-$(CONFIG_MICROCODE_XEN_LOADER)	+= microcode_xen.o
 obj-$(CONFIG_MICROCODE)			+= microcode.o
 
 obj-$(CONFIG_X86_CHECK_BIOS_CORRUPTION) += check.o
diff --git a/arch/x86/xen/Kconfig b/arch/x86/xen/Kconfig
index 384e0a5..9a0d2fc 100644
--- a/arch/x86/xen/Kconfig
+++ b/arch/x86/xen/Kconfig
@@ -51,4 +51,8 @@ config XEN_DEBUG_FS
 
 config MICROCODE_XEN
        def_bool y
-       depends on XEN_DOM0 && MICROCODE
\ No newline at end of file
+       depends on XEN && MICROCODE
+
+config MICROCODE_XEN_LOADER
+	def_bool y
+	depends on MICROCODE_XEN && DOM0
-- 
1.7.2.3

Re: [PATCH 1/3] xen dom0: Add support for the platform_ops hypercall

[Konrad Rzeszutek Wilk]

> +typedef struct xenpf_settime xenpf_settime_t;
> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_settime_t);

We aren't using this..
> +typedef struct xenpf_add_memtype xenpf_add_memtype_t;
> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_add_memtype_t);

.. nor this..
> +typedef struct xenpf_del_memtype xenpf_del_memtype_t;
> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_del_memtype_t);

.. neither this one.
> +typedef struct xenpf_read_memtype xenpf_read_memtype_t;
> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_read_memtype_t);

..whoa, didn't know we had that many.
> +
> +#define XENPF_microcode_update    35
> +struct xenpf_microcode_update {
> +    /* IN variables. */
> +    GUEST_HANDLE(void) data;          /* Pointer to microcode data */
> +    uint32_t length;                  /* Length of microcode data. */
> +};
> +typedef struct xenpf_microcode_update xenpf_microcode_update_t;
> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_microcode_update_t);

OK, we are using this one.

.. snip .. 

Why not just introduce the one we are using (just one right now)
and on subsequent patches that enable the functionality add it in this file?

Re: [PATCH 2/3] xen: add CPU microcode update driver

[Konrad Rzeszutek Wilk]

> +static enum ucode_state xen_request_microcode_user(int cpu,
> +						   const void __user *buf, size_t size)
> +{
> +	struct ucode_cpu_info *uci = ucode_cpu_info + cpu;
> +	struct xen_microcode *uc;
> +	enum ucode_state ret;
> +	size_t unread;
> +
> +	if (cpu != 0) {
> +		/* No real firmware for non-zero cpus; just store a
> +		   placeholder */
> +		size = 0;
> +	}
> +
> +	if (uci->mc != NULL) {
> +		vfree(uci->mc);
> +		uci->mc = NULL;
> +	}
> +
> +	ret = UCODE_ERROR;
> +	uc = vmalloc(sizeof(*uc) + size);
> +	if (uc == NULL)
> +		goto out;
> +
> +	uc->len = size;
> +
> +	ret = UCODE_NFOUND;
> +
> +	/* XXX This sporadically returns uncopied bytes, so we return
> +	   EFAULT.  As far as I can see, the usermode code
           ^^^^^ UCODE_NFOUND.
> +	   (microcode_ctl) isn't doing anything wrong... */

Is this still valid? Looking at AMD it checks for a magic key and the Intel
just copies without checks.

Re: [PATCH 3/3] xen/microcode: partially enable even for non-privileged kernels

[Konrad Rzeszutek Wilk]

On Thu, Nov 11, 2010 at 03:58:06PM -0800, Jeremy Fitzhardinge wrote:
> From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
> 
> Don't attempt to load microcode on non-privileged kernels.  Kernels

The previous patch had this:

 if (!xen_initial_domain())
+               return NULL;

which pretty much made non-priviliged kernels not do any micro code loading.

It sounds like this patch is just to not compile the Xen microcode code
if CONFIG_DOM0 is not enabled?

> compiled without privileged support just get vestigial Xen detection
> to skip loading altogether; kernels with privileged support will
> load microcode if running privileged.  In either case, the normal
> Intel/AMD microcode loader is skipped under Xen.

Re: [Xen-devel] Re: [PATCH 1/3] xen dom0: Add support for the platform_ops hypercall

[Jeremy Fitzhardinge]

On 11/15/2010 07:48 AM, Konrad Rzeszutek Wilk wrote:
>> +typedef struct xenpf_settime xenpf_settime_t;
>> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_settime_t);
> We aren't using this..
>> +typedef struct xenpf_add_memtype xenpf_add_memtype_t;
>> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_add_memtype_t);
> .. nor this..
>> +typedef struct xenpf_del_memtype xenpf_del_memtype_t;
>> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_del_memtype_t);
> .. neither this one.
>> +typedef struct xenpf_read_memtype xenpf_read_memtype_t;
>> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_read_memtype_t);
> ..whoa, didn't know we had that many.
>> +
>> +#define XENPF_microcode_update    35
>> +struct xenpf_microcode_update {
>> +    /* IN variables. */
>> +    GUEST_HANDLE(void) data;          /* Pointer to microcode data */
>> +    uint32_t length;                  /* Length of microcode data. */
>> +};
>> +typedef struct xenpf_microcode_update xenpf_microcode_update_t;
>> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_microcode_update_t);
> OK, we are using this one.
>
> .. snip .. 
>
> Why not just introduce the one we are using (just one right now)
> and on subsequent patches that enable the functionality add it in this file?

The main reason is that if some other branch also brings in platform.h
then its easier to merge two copies of the same file rather than two
subsets.

    J

Re: [Xen-devel] Re: [PATCH 2/3] xen: add CPU microcode update driver

[Jeremy Fitzhardinge]

On 11/15/2010 07:59 AM, Konrad Rzeszutek Wilk wrote:
>> +static enum ucode_state xen_request_microcode_user(int cpu,
>> +						   const void __user *buf, size_t size)
>> +{
>> +	struct ucode_cpu_info *uci = ucode_cpu_info + cpu;
>> +	struct xen_microcode *uc;
>> +	enum ucode_state ret;
>> +	size_t unread;
>> +
>> +	if (cpu != 0) {
>> +		/* No real firmware for non-zero cpus; just store a
>> +		   placeholder */
>> +		size = 0;
>> +	}
>> +
>> +	if (uci->mc != NULL) {
>> +		vfree(uci->mc);
>> +		uci->mc = NULL;
>> +	}
>> +
>> +	ret = UCODE_ERROR;
>> +	uc = vmalloc(sizeof(*uc) + size);
>> +	if (uc == NULL)
>> +		goto out;
>> +
>> +	uc->len = size;
>> +
>> +	ret = UCODE_NFOUND;
>> +
>> +	/* XXX This sporadically returns uncopied bytes, so we return
>> +	   EFAULT.  As far as I can see, the usermode code
>            ^^^^^ UCODE_NFOUND.
>> +	   (microcode_ctl) isn't doing anything wrong... */
> Is this still valid? Looking at AMD it checks for a magic key and the Intel
> just copies without checks.

Probably not.

    J

Re: [Xen-devel] Re: [PATCH 3/3] xen/microcode: partially enable even for non-privileged kernels

[Jeremy Fitzhardinge]

On 11/15/2010 08:02 AM, Konrad Rzeszutek Wilk wrote:
> On Thu, Nov 11, 2010 at 03:58:06PM -0800, Jeremy Fitzhardinge wrote:
>> From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
>>
>> Don't attempt to load microcode on non-privileged kernels.  Kernels
> The previous patch had this:
>
>  if (!xen_initial_domain())
> +               return NULL;
>
> which pretty much made non-priviliged kernels not do any micro code loading.
>
> It sounds like this patch is just to not compile the Xen microcode code
> if CONFIG_DOM0 is not enabled?

Yeah, I think the patch is a brainfart, and the existing code already
did the right thing.

    J

Re: [Xen-devel] Re: [PATCH 1/3] xen dom0: Add support for the platform_ops hypercall

[Konrad Rzeszutek Wilk]

> > Why not just introduce the one we are using (just one right now)
> > and on subsequent patches that enable the functionality add it in this file?
> 
> The main reason is that if some other branch also brings in platform.h
> then its easier to merge two copies of the same file rather than two
> subsets.

Sure, but in linux-next that is just one touch patch and Stephen said
he is Ok with those.

I like doing it (just stick in those that we use), b/c that way you have
an excellent view of when this feature went in the code and the relevant
git commits to blame.