x86 mce: fix c/s 17968 for 32-on-64

32-on-64 aspects were not properly considered. Add respective
checking, and adjust structure layouts for the cases where the
checking pointed out issues.

Also,
- fix a potential memory corruption issue (do_mca() could write beyond
  log_cpus' end if the guest specified less than the number of online
  CPUs
- there is no reason to make the (not even properly prefixed)
  definitions in xen/public/arch-x86/xen-mca.h globally visible by
  including the file from xen/public/arch-x86/xen.h.

Signed-off-by: Jan Beulich <jbeulich@novell.com>

6 files changed, 135 insertions, 61 deletions

diff --git a/xen/arch/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c
index 3685056cba..bc18e3048b 100644
--- a/xen/arch/x86/cpu/mcheck/mce.c
+++ b/xen/arch/x86/cpu/mcheck/mce.c
@@ -984,14 +984,76 @@ static void x86_mc_mceinject(void *data)
 #error BITS_PER_LONG definition absent
 #endif
 
+#ifdef CONFIG_COMPAT
+# include <compat/arch-x86/xen-mca.h>
+
+# define xen_mcinfo_msr              mcinfo_msr
+CHECK_mcinfo_msr;
+# undef xen_mcinfo_msr
+# undef CHECK_mcinfo_msr
+# define CHECK_mcinfo_msr            struct mcinfo_msr
+
+# define xen_mcinfo_common           mcinfo_common
+CHECK_mcinfo_common;
+# undef xen_mcinfo_common
+# undef CHECK_mcinfo_common
+# define CHECK_mcinfo_common         struct mcinfo_common
+
+CHECK_FIELD_(struct, mc_fetch, flags);
+CHECK_FIELD_(struct, mc_fetch, fetch_id);
+# define CHECK_compat_mc_fetch       struct mc_fetch
+
+CHECK_FIELD_(struct, mc_physcpuinfo, ncpus);
+# define CHECK_compat_mc_physcpuinfo struct mc_physcpuinfo
+
+CHECK_mc;
+# undef CHECK_compat_mc_fetch
+# undef CHECK_compat_mc_physcpuinfo
+
+# define xen_mc_info                 mc_info
+CHECK_mc_info;
+# undef xen_mc_info
+
+# define xen_mcinfo_global           mcinfo_global
+CHECK_mcinfo_global;
+# undef xen_mcinfo_global
+
+# define xen_mcinfo_bank             mcinfo_bank
+CHECK_mcinfo_bank;
+# undef xen_mcinfo_bank
+
+# define xen_mcinfo_extended         mcinfo_extended
+CHECK_mcinfo_extended;
+# undef xen_mcinfo_extended
+
+# define xen_mcinfo_recovery         mcinfo_recovery
+# define xen_cpu_offline_action      cpu_offline_action
+# define xen_page_offline_action     page_offline_action
+CHECK_mcinfo_recovery;
+# undef xen_cpu_offline_action
+# undef xen_page_offline_action
+# undef xen_mcinfo_recovery
+#else
+# define compat_mc_fetch xen_mc_fetch
+# define compat_mc_physcpuinfo xen_mc_physcpuinfo
+# define compat_handle_is_null guest_handle_is_null
+# define copy_to_compat copy_to_guest
+#endif
+
 /* Machine Check Architecture Hypercall */
 long do_mca(XEN_GUEST_HANDLE(xen_mc_t) u_xen_mc)
 {
 	long ret = 0;
 	struct xen_mc curop, *op = &curop;
 	struct vcpu *v = current;
-	struct xen_mc_fetch *mc_fetch;
-	struct xen_mc_physcpuinfo *mc_physcpuinfo;
+	union {
+		struct xen_mc_fetch *nat;
+		struct compat_mc_fetch *cmp;
+	} mc_fetch;
+	union {
+		struct xen_mc_physcpuinfo *nat;
+		struct compat_mc_physcpuinfo *cmp;
+	} mc_physcpuinfo;
 	uint32_t flags, cmdflags;
 	int nlcpu;
 	xen_mc_logical_cpu_t *log_cpus = NULL;
@@ -1009,8 +1071,8 @@ long do_mca(XEN_GUEST_HANDLE(xen_mc_t) u_xen_mc)
 
 	switch (op->cmd) {
 	case XEN_MC_fetch:
-		mc_fetch = &op->u.mc_fetch;
-		cmdflags = mc_fetch->flags;
+		mc_fetch.nat = &op->u.mc_fetch;
+		cmdflags = mc_fetch.nat->flags;
 
 		/* This hypercall is for Dom0 only */
 		if (!IS_PRIV(v->domain) )
@@ -1032,30 +1094,35 @@ long do_mca(XEN_GUEST_HANDLE(xen_mc_t) u_xen_mc)
 		flags = XEN_MC_OK;
 
 		if (cmdflags & XEN_MC_ACK) {
-			mctelem_cookie_t cookie = ID2COOKIE(mc_fetch->fetch_id);
+			mctelem_cookie_t cookie = ID2COOKIE(mc_fetch.nat->fetch_id);
 			mctelem_ack(which, cookie);
 		} else {
-			if (guest_handle_is_null(mc_fetch->data))
+			if (!is_pv_32on64_vcpu(v)
+			    ? guest_handle_is_null(mc_fetch.nat->data)
+			    : compat_handle_is_null(mc_fetch.cmp->data))
 				return x86_mcerr("do_mca fetch: guest buffer "
 				    "invalid", -EINVAL);
 
 			if ((mctc = mctelem_consume_oldest_begin(which))) {
 				struct mc_info *mcip = mctelem_dataptr(mctc);
-				if (copy_to_guest(mc_fetch->data, mcip, 1)) {
+				if (!is_pv_32on64_vcpu(v)
+				    ? copy_to_guest(mc_fetch.nat->data, mcip, 1)
+				    : copy_to_compat(mc_fetch.cmp->data,
+						     mcip, 1)) {
 					ret = -EFAULT;
 					flags |= XEN_MC_FETCHFAILED;
-					mc_fetch->fetch_id = 0;
+					mc_fetch.nat->fetch_id = 0;
 				} else {
-					mc_fetch->fetch_id = COOKIE2ID(mctc);
+					mc_fetch.nat->fetch_id = COOKIE2ID(mctc);
 				}
 				mctelem_consume_oldest_end(mctc);
 			} else {
 				/* There is no data */
 				flags |= XEN_MC_NODATA;
-				mc_fetch->fetch_id = 0;
+				mc_fetch.nat->fetch_id = 0;
 			}
 
-			mc_fetch->flags = flags;
+			mc_fetch.nat->flags = flags;
 			if (copy_to_guest(u_xen_mc, op, 1) != 0)
 				ret = -EFAULT;
 		}
@@ -1069,14 +1136,16 @@ long do_mca(XEN_GUEST_HANDLE(xen_mc_t) u_xen_mc)
 		if ( !IS_PRIV(v->domain) )
 			return x86_mcerr("do_mca cpuinfo", -EPERM);
 
-		mc_physcpuinfo = &op->u.mc_physcpuinfo;
+		mc_physcpuinfo.nat = &op->u.mc_physcpuinfo;
 		nlcpu = num_online_cpus();
 
-		if (!guest_handle_is_null(mc_physcpuinfo->info)) {
-			if (mc_physcpuinfo->ncpus <= 0)
+		if (!is_pv_32on64_vcpu(v)
+		    ? !guest_handle_is_null(mc_physcpuinfo.nat->info)
+		    : !compat_handle_is_null(mc_physcpuinfo.cmp->info)) {
+			if (mc_physcpuinfo.nat->ncpus <= 0)
 				return x86_mcerr("do_mca cpuinfo: ncpus <= 0",
 				    -EINVAL);
-			nlcpu = min(nlcpu, (int)mc_physcpuinfo->ncpus);
+			nlcpu = min(nlcpu, (int)mc_physcpuinfo.nat->ncpus);
 			log_cpus = xmalloc_array(xen_mc_logical_cpu_t, nlcpu);
 			if (log_cpus == NULL)
 				return x86_mcerr("do_mca cpuinfo", -ENOMEM);
@@ -1086,22 +1155,20 @@ long do_mca(XEN_GUEST_HANDLE(xen_mc_t) u_xen_mc)
 				xfree(log_cpus);
 				return x86_mcerr("do_mca cpuinfo", -EIO);
 			}
+			if (!is_pv_32on64_vcpu(v)
+			    ? copy_to_guest(mc_physcpuinfo.nat->info,
+					    log_cpus, nlcpu)
+			    : copy_to_compat(mc_physcpuinfo.cmp->info,
+					    log_cpus, nlcpu))
+				ret = -EFAULT;
+			xfree(log_cpus);
 		}
 
-		mc_physcpuinfo->ncpus = nlcpu;
+		mc_physcpuinfo.nat->ncpus = nlcpu;
 
-		if (copy_to_guest(u_xen_mc, op, 1)) {
-			if (log_cpus != NULL)
-				xfree(log_cpus);
+		if (copy_to_guest(u_xen_mc, op, 1))
 			return x86_mcerr("do_mca cpuinfo", -EFAULT);
-		}
 
-		if (!guest_handle_is_null(mc_physcpuinfo->info)) {
-			if (copy_to_guest(mc_physcpuinfo->info,
-			    log_cpus, nlcpu))
-				ret = -EFAULT;
-			xfree(log_cpus);
-		}
 		break;
 
 	case XEN_MC_msrinject:
diff --git a/xen/arch/x86/cpu/mcheck/x86_mca.h b/xen/arch/x86/cpu/mcheck/x86_mca.h
index ac98744932..6333ccd3d0 100644
--- a/xen/arch/x86/cpu/mcheck/x86_mca.h
+++ b/xen/arch/x86/cpu/mcheck/x86_mca.h
@@ -18,9 +18,9 @@
  */
 
 #ifndef X86_MCA_H
-
 #define X86_MCA_H
 
+#include <public/arch-x86/xen-mca.h>
 
 /* The MCA/MCE MSRs should not be used anywhere else.
  * They are cpu family/model specific and are only for use
diff --git a/xen/include/public/arch-x86/xen-mca.h b/xen/include/public/arch-x86/xen-mca.h
index b02ebf0e07..ee92d48ef3 100644
--- a/xen/include/public/arch-x86/xen-mca.h
+++ b/xen/include/public/arch-x86/xen-mca.h
@@ -62,7 +62,7 @@
  * choose a different version number range that is numerically less
  * than that used in xen-unstable.
  */
-#define XEN_MCA_INTERFACE_VERSION 0x01ecc002
+#define XEN_MCA_INTERFACE_VERSION 0x01ecc003
 
 /* IN: Dom0 calls hypercall to retrieve nonurgent telemetry */
 #define XEN_MC_NONURGENT  0x0001
@@ -125,13 +125,13 @@ struct mcinfo_global {
 
     /* running domain at the time in error (most likely the impacted one) */
     uint16_t mc_domid;
+    uint16_t mc_vcpuid; /* virtual cpu scheduled for mc_domid */
     uint32_t mc_socketid; /* physical socket of the physical core */
     uint16_t mc_coreid; /* physical impacted core */
-    uint32_t mc_apicid;
     uint16_t mc_core_threadid; /* core thread of physical core */
-    uint16_t mc_vcpuid; /* virtual cpu scheduled for mc_domid */
-    uint64_t mc_gstatus; /* global status */
+    uint32_t mc_apicid;
     uint32_t mc_flags;
+    uint64_t mc_gstatus; /* global status */
 };
 
 /* contains bank local x86 mc information */
@@ -216,8 +216,9 @@ struct cpu_offline_action
 };
 
 #define MAX_UNION_SIZE 16
-struct mc_recovery
+struct mcinfo_recovery
 {
+    struct mcinfo_common common;
     uint16_t mc_bank; /* bank nr */
     uint8_t action_flags;
     uint8_t action_types;
@@ -228,12 +229,6 @@ struct mc_recovery
     } action_info;
 };
 
-struct mcinfo_recovery
-{
-    struct mcinfo_common common;
-    struct mc_recovery mc_action;
-};
-
 
 #define MCINFO_HYPERCALLSIZE	1024
 #define MCINFO_MAXSIZE		768
@@ -241,8 +236,8 @@ struct mcinfo_recovery
 struct mc_info {
     /* Number of mcinfo_* entries in mi_data */
     uint32_t mi_nentries;
-
-    uint8_t mi_data[MCINFO_MAXSIZE - sizeof(uint32_t)];
+    uint32_t _pad0;
+    uint64_t mi_data[(MCINFO_MAXSIZE - 1) / 8];
 };
 typedef struct mc_info mc_info_t;
 DEFINE_XEN_GUEST_HANDLE(mc_info_t);
@@ -258,7 +253,7 @@ DEFINE_XEN_GUEST_HANDLE(mc_info_t);
 #define MC_CAPS_VIA	5	/* cpuid level 0xc0000001 */
 #define MC_CAPS_AMD_ECX	6	/* cpuid level 0x80000001 (%ecx) */
 
-typedef struct mcinfo_logical_cpu {
+struct mcinfo_logical_cpu {
     uint32_t mc_cpunr;          
     uint32_t mc_chipid; 
     uint16_t mc_coreid;
@@ -280,7 +275,8 @@ typedef struct mcinfo_logical_cpu {
     uint32_t mc_cache_alignment;
     int32_t mc_nmsrvals;
     struct mcinfo_msr mc_msrvalues[__MC_MSR_ARRAYSIZE];
-} xen_mc_logical_cpu_t;
+};
+typedef struct mcinfo_logical_cpu xen_mc_logical_cpu_t;
 DEFINE_XEN_GUEST_HANDLE(xen_mc_logical_cpu_t);
 
 
@@ -299,12 +295,12 @@ DEFINE_XEN_GUEST_HANDLE(xen_mc_logical_cpu_t);
  *    struct mcinfo_common *x86_mcinfo_first(struct mc_info *mi);
  */
 #define x86_mcinfo_first(_mi)       \
-    (struct mcinfo_common *)((_mi)->mi_data)
+    ((struct mcinfo_common *)(_mi)->mi_data)
 /* Prototype:
  *    struct mcinfo_common *x86_mcinfo_next(struct mcinfo_common *mic);
  */
 #define x86_mcinfo_next(_mic)       \
-    (struct mcinfo_common *)((uint8_t *)(_mic) + (_mic)->size)
+    ((struct mcinfo_common *)((uint8_t *)(_mic) + (_mic)->size))
 
 /* Prototype:
  *    void x86_mcinfo_lookup(void *ret, struct mc_info *mi, uint16_t type);
@@ -350,6 +346,7 @@ struct xen_mc_fetch {
                            XEN_MC_ACK if ack'ing an earlier fetch */
 			/* OUT: XEN_MC_OK, XEN_MC_FETCHFAILED,
 			   XEN_MC_NODATA, XEN_MC_NOMATCH */
+    uint32_t _pad0;
     uint64_t fetch_id;	/* OUT: id for ack, IN: id we are ack'ing */
 
     /* OUT variables. */
@@ -382,7 +379,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_mc_notifydomain_t);
 struct xen_mc_physcpuinfo {
 	/* IN/OUT */
 	uint32_t ncpus;
-	uint32_t pad0;
+	uint32_t _pad0;
 	/* OUT */
 	XEN_GUEST_HANDLE(xen_mc_logical_cpu_t) info;
 };
@@ -391,10 +388,10 @@ struct xen_mc_physcpuinfo {
 #define MC_MSRINJ_MAXMSRS       8
 struct xen_mc_msrinject {
        /* IN */
-	unsigned int mcinj_cpunr;       /* target processor id */
+	uint32_t mcinj_cpunr;           /* target processor id */
 	uint32_t mcinj_flags;           /* see MC_MSRINJ_F_* below */
 	uint32_t mcinj_count;           /* 0 .. count-1 in array are valid */
-	uint32_t mcinj_pad0;
+	uint32_t _pad0;
 	struct mcinfo_msr mcinj_msr[MC_MSRINJ_MAXMSRS];
 };
 
@@ -406,18 +403,16 @@ struct xen_mc_mceinject {
 	unsigned int mceinj_cpunr;      /* target processor id */
 };
 
-typedef union {
-    struct xen_mc_fetch        mc_fetch;
-    struct xen_mc_notifydomain mc_notifydomain;
-    struct xen_mc_physcpuinfo  mc_physcpuinfo;
-    struct xen_mc_msrinject    mc_msrinject;
-    struct xen_mc_mceinject    mc_mceinject;
-} xen_mc_arg_t;
-
 struct xen_mc {
     uint32_t cmd;
     uint32_t interface_version; /* XEN_MCA_INTERFACE_VERSION */
-    xen_mc_arg_t u;
+    union {
+        struct xen_mc_fetch        mc_fetch;
+        struct xen_mc_notifydomain mc_notifydomain;
+        struct xen_mc_physcpuinfo  mc_physcpuinfo;
+        struct xen_mc_msrinject    mc_msrinject;
+        struct xen_mc_mceinject    mc_mceinject;
+    } u;
 };
 typedef struct xen_mc xen_mc_t;
 DEFINE_XEN_GUEST_HANDLE(xen_mc_t);
diff --git a/xen/include/public/arch-x86/xen.h b/xen/include/public/arch-x86/xen.h
index 084348fbc2..5f7579aab1 100644
--- a/xen/include/public/arch-x86/xen.h
+++ b/xen/include/public/arch-x86/xen.h
@@ -76,10 +76,6 @@ typedef unsigned long xen_pfn_t;
 /* Maximum number of virtual CPUs in multi-processor guests. */
 #define MAX_VIRT_CPUS 32
 
-
-/* Machine check support */
-#include "xen-mca.h"
-
 #ifndef __ASSEMBLY__
 
 typedef unsigned long xen_ulong_t;
diff --git a/xen/include/xlat.lst b/xen/include/xlat.lst
index 17f47e91ed..f2e4597648 100644
--- a/xen/include/xlat.lst
+++ b/xen/include/xlat.lst
@@ -10,6 +10,22 @@
 !	cpu_user_regs			arch-x86/xen-@arch@.h
 !	trap_info			arch-x86/xen.h
 !	vcpu_guest_context		arch-x86/xen.h
+?	cpu_offline_action		arch-x86/xen-mca.h
+?	mc				arch-x86/xen-mca.h
+?	mcinfo_bank			arch-x86/xen-mca.h
+?	mcinfo_common			arch-x86/xen-mca.h
+?	mcinfo_extended			arch-x86/xen-mca.h
+?	mcinfo_global			arch-x86/xen-mca.h
+?	mcinfo_logical_cpu		arch-x86/xen-mca.h
+?	mcinfo_msr			arch-x86/xen-mca.h
+?	mcinfo_recovery			arch-x86/xen-mca.h
+!	mc_fetch			arch-x86/xen-mca.h
+?	mc_info				arch-x86/xen-mca.h
+?	mc_mceinject			arch-x86/xen-mca.h
+?	mc_msrinject			arch-x86/xen-mca.h
+?	mc_notifydomain			arch-x86/xen-mca.h
+!	mc_physcpuinfo			arch-x86/xen-mca.h
+?	page_offline_action		arch-x86/xen-mca.h
 ?	evtchn_alloc_unbound		event_channel.h
 ?	evtchn_bind_interdomain		event_channel.h
 ?	evtchn_bind_ipi			event_channel.h
diff --git a/xen/tools/get-fields.sh b/xen/tools/get-fields.sh
index 49019a43be..2537fc49a1 100644
--- a/xen/tools/get-fields.sh
+++ b/xen/tools/get-fields.sh
@@ -328,7 +328,7 @@ check_field ()
 				struct|union)
 					;;
 				[a-zA-Z_]*)
-					echo -n "    CHECK_$n"
+					echo -n "    CHECK_${n#xen_}"
 					break
 					;;
 				*)