[PATCH v2 0/4] Fix tools/xen-mceinj to handle >=4GB guest memory

[PATCH v2 0/4] Fix tools/xen-mceinj to handle >=4GB guest memory

[Haozhong Zhang]

The existing xen-mceinj can not inject MCE through MSR_MCI_ADDR to a
domain w/ more than 4GB memory, e.g. if domain 0 has more than 4GB
memory, the execution of the command
        xen-mceinj -d 0 -t 0 -p 0x2721a900
will fail w/ a message "Failed to get pfn list ffffffff: Operation not
supported".

The cause is that the hypercall XEN_DOMCTL_getmemlist used by
xen-mceinj to translate the guest physical address (argument of '-p')
to the host machine address always fails if the domain has more than
4GB memory due to the mitigation of XSA-74.

This patchset fixes this problem by moving the translation from
xen-mceinj to the hypervisor, so that it is not necessary to use
XEN_DOMCTL_getmemlist.

The first two patches just fix serval code-style issues, while the
other two are the actual fix.

Changes from v1 to v2:
1. The correct trailing whitespaces are kept in patch 1 and 2.
2. Follow Xen code style in this version.
3. Fix several type and macro issues in patch 3.
4. Add a missing "put_gfn()" in patch 3.
5. Update the error messages in patch 3 to include more information.
6. Use parameterized domain id rather than a hardcoded 0 for several
   functions in xen-mceinj.c.
7. Update the commit message of patch 4 to explicitly state that the
   address translation is moved to the hypervisor.


Haozhong Zhang (4):
  x86/mce: Fix code style
  tools/xen-mceinj: Fix code style
  x86/mce: Translate passed-in GPA to host machine address
  tools/xen-mceinj: Pass in GPA when injecting through MSR_MCI_ADDR

 tools/tests/mce-test/tools/xen-mceinj.c | 192 ++++++++------------------------
 xen/arch/x86/cpu/mcheck/mce.c           |  66 ++++++++---
 xen/include/public/arch-x86/xen-mca.h   |  33 +++---
 3 files changed, 120 insertions(+), 171 deletions(-)

-- 
2.4.8

[PATCH v2 1/4] x86/mce: Fix code style

[Haozhong Zhang]

Remove trailing whitespaces and fix indentations in mce.c and xen_mca.h.

Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
 xen/arch/x86/cpu/mcheck/mce.c         | 10 +++++-----
 xen/include/public/arch-x86/xen-mca.h | 30 +++++++++++++++---------------
 2 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/xen/arch/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c
index 7c2cacc..561257d 100644
--- a/xen/arch/x86/cpu/mcheck/mce.c
+++ b/xen/arch/x86/cpu/mcheck/mce.c
@@ -105,7 +105,7 @@ void x86_mce_callback_register(x86_mce_callback_t cbfunc)
     mc_callback_bank_extended = cbfunc;
 }
 
-/* Machine check recoverable judgement callback handler 
+/* Machine check recoverable judgement callback handler
  * It is used to judge whether an UC error is recoverable by software
  */
 static mce_recoverable_t mc_recoverable_scan = NULL;
@@ -160,9 +160,9 @@ static void mcabank_clear(int banknum)
 }
 
 /* Judging whether to Clear Machine Check error bank callback handler
- * According to Intel latest MCA OS Recovery Writer's Guide, 
+ * According to Intel latest MCA OS Recovery Writer's Guide,
  * whether the error MCA bank needs to be cleared is decided by the mca_source
- * and MCi_status bit value. 
+ * and MCi_status bit value.
  */
 static mce_need_clearbank_t mc_need_clearbank_scan = NULL;
 
@@ -535,7 +535,7 @@ void mcheck_cmn_handler(const struct cpu_user_regs *regs)
         }
         atomic_set(&found_error, 0);
     }
-    mce_barrier_exit(&mce_trap_bar); 
+    mce_barrier_exit(&mce_trap_bar);
 
     /* Clear flags after above fatal check */
     mce_barrier_enter(&mce_trap_bar);
@@ -891,7 +891,7 @@ void x86_mcinfo_dump(struct mc_info *mi)
                "CPU%d: Machine Check Exception: %16"PRIx64"\n",
                mc_global->mc_coreid, mc_global->mc_gstatus);
     } else if (mc_global->mc_flags & MC_FLAG_CMCI) {
-        printk(XENLOG_WARNING "CMCI occurred on CPU %d.\n", 
+        printk(XENLOG_WARNING "CMCI occurred on CPU %d.\n",
                mc_global->mc_coreid);
     } else if (mc_global->mc_flags & MC_FLAG_POLLED) {
         printk(XENLOG_WARNING "POLLED occurred on CPU %d.\n",
diff --git a/xen/include/public/arch-x86/xen-mca.h b/xen/include/public/arch-x86/xen-mca.h
index 04382ed..ec1237a 100644
--- a/xen/include/public/arch-x86/xen-mca.h
+++ b/xen/include/public/arch-x86/xen-mca.h
@@ -1,11 +1,11 @@
 /******************************************************************************
  * arch-x86/mca.h
- * 
+ *
  * Contributed by Advanced Micro Devices, Inc.
  * Author: Christoph Egger <Christoph.Egger@amd.com>
  *
  * Guest OS machine check interface to x86 Xen.
- * 
+ *
  * 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
@@ -156,7 +156,7 @@ struct mcinfo_msr {
 };
 
 /* contains mc information from other
- * or additional mc MSRs */ 
+ * or additional mc MSRs */
 struct mcinfo_extended {
     struct mcinfo_common common;
 
@@ -193,10 +193,10 @@ struct mcinfo_extended {
 /* L3 cache disable Action */
 #define MC_ACTION_CACHE_SHRINK (0x1 << 2)
 
-/* Below interface used between XEN/DOM0 for passing XEN's recovery action 
- * information to DOM0. 
+/* Below interface used between XEN/DOM0 for passing XEN's recovery action
+ * information to DOM0.
  * usage Senario: After offlining broken page, XEN might pass its page offline
- * recovery action result to DOM0. DOM0 will save the information in 
+ * recovery action result to DOM0. DOM0 will save the information in
  * non-volatile memory for further proactive actions, such as offlining the
  * easy broken page earlier when doing next reboot.
 */
@@ -255,8 +255,8 @@ DEFINE_XEN_GUEST_HANDLE(mc_info_t);
 #define MC_CAPS_AMD_ECX	6	/* cpuid level 0x80000001 (%ecx) */
 
 struct mcinfo_logical_cpu {
-    uint32_t mc_cpunr;          
-    uint32_t mc_chipid; 
+    uint32_t mc_cpunr;
+    uint32_t mc_chipid;
     uint16_t mc_coreid;
     uint16_t mc_threadid;
     uint32_t mc_apicid;
@@ -281,7 +281,7 @@ typedef struct mcinfo_logical_cpu xen_mc_logical_cpu_t;
 DEFINE_XEN_GUEST_HANDLE(xen_mc_logical_cpu_t);
 
 
-/* 
+/*
  * OS's should use these instead of writing their own lookup function
  * each with its own bugs and drawbacks.
  * We use macros instead of static inline functions to allow guests
@@ -388,12 +388,12 @@ struct xen_mc_physcpuinfo {
 #define XEN_MC_msrinject    4
 #define MC_MSRINJ_MAXMSRS       8
 struct xen_mc_msrinject {
-       /* IN */
-	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 _pad0;
-	struct mcinfo_msr mcinj_msr[MC_MSRINJ_MAXMSRS];
+    /* IN */
+    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 _pad0;
+    struct mcinfo_msr mcinj_msr[MC_MSRINJ_MAXMSRS];
 };
 
 /* Flags for mcinj_flags above; bits 16-31 are reserved */
-- 
2.4.8

Re: [PATCH v2 1/4] x86/mce: Fix code style

[Egger, Christoph]

On 2015/09/16 7:35, Haozhong Zhang wrote:
> Remove trailing whitespaces and fix indentations in mce.c and xen_mca.h.
> 
> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>

Acked-by: Christoph Egger <chegger@amazon.de>

> ---
>  xen/arch/x86/cpu/mcheck/mce.c         | 10 +++++-----
>  xen/include/public/arch-x86/xen-mca.h | 30 +++++++++++++++---------------
>  2 files changed, 20 insertions(+), 20 deletions(-)
> 
> diff --git a/xen/arch/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c
> index 7c2cacc..561257d 100644
> --- a/xen/arch/x86/cpu/mcheck/mce.c
> +++ b/xen/arch/x86/cpu/mcheck/mce.c
> @@ -105,7 +105,7 @@ void x86_mce_callback_register(x86_mce_callback_t cbfunc)
>      mc_callback_bank_extended = cbfunc;
>  }
>  
> -/* Machine check recoverable judgement callback handler 
> +/* Machine check recoverable judgement callback handler
>   * It is used to judge whether an UC error is recoverable by software
>   */
>  static mce_recoverable_t mc_recoverable_scan = NULL;
> @@ -160,9 +160,9 @@ static void mcabank_clear(int banknum)
>  }
>  
>  /* Judging whether to Clear Machine Check error bank callback handler
> - * According to Intel latest MCA OS Recovery Writer's Guide, 
> + * According to Intel latest MCA OS Recovery Writer's Guide,
>   * whether the error MCA bank needs to be cleared is decided by the mca_source
> - * and MCi_status bit value. 
> + * and MCi_status bit value.
>   */
>  static mce_need_clearbank_t mc_need_clearbank_scan = NULL;
>  
> @@ -535,7 +535,7 @@ void mcheck_cmn_handler(const struct cpu_user_regs *regs)
>          }
>          atomic_set(&found_error, 0);
>      }
> -    mce_barrier_exit(&mce_trap_bar); 
> +    mce_barrier_exit(&mce_trap_bar);
>  
>      /* Clear flags after above fatal check */
>      mce_barrier_enter(&mce_trap_bar);
> @@ -891,7 +891,7 @@ void x86_mcinfo_dump(struct mc_info *mi)
>                 "CPU%d: Machine Check Exception: %16"PRIx64"\n",
>                 mc_global->mc_coreid, mc_global->mc_gstatus);
>      } else if (mc_global->mc_flags & MC_FLAG_CMCI) {
> -        printk(XENLOG_WARNING "CMCI occurred on CPU %d.\n", 
> +        printk(XENLOG_WARNING "CMCI occurred on CPU %d.\n",
>                 mc_global->mc_coreid);
>      } else if (mc_global->mc_flags & MC_FLAG_POLLED) {
>          printk(XENLOG_WARNING "POLLED occurred on CPU %d.\n",
> diff --git a/xen/include/public/arch-x86/xen-mca.h b/xen/include/public/arch-x86/xen-mca.h
> index 04382ed..ec1237a 100644
> --- a/xen/include/public/arch-x86/xen-mca.h
> +++ b/xen/include/public/arch-x86/xen-mca.h
> @@ -1,11 +1,11 @@
>  /******************************************************************************
>   * arch-x86/mca.h
> - * 
> + *
>   * Contributed by Advanced Micro Devices, Inc.
>   * Author: Christoph Egger <Christoph.Egger@amd.com>
>   *
>   * Guest OS machine check interface to x86 Xen.
> - * 
> + *
>   * 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
> @@ -156,7 +156,7 @@ struct mcinfo_msr {
>  };
>  
>  /* contains mc information from other
> - * or additional mc MSRs */ 
> + * or additional mc MSRs */
>  struct mcinfo_extended {
>      struct mcinfo_common common;
>  
> @@ -193,10 +193,10 @@ struct mcinfo_extended {
>  /* L3 cache disable Action */
>  #define MC_ACTION_CACHE_SHRINK (0x1 << 2)
>  
> -/* Below interface used between XEN/DOM0 for passing XEN's recovery action 
> - * information to DOM0. 
> +/* Below interface used between XEN/DOM0 for passing XEN's recovery action
> + * information to DOM0.
>   * usage Senario: After offlining broken page, XEN might pass its page offline
> - * recovery action result to DOM0. DOM0 will save the information in 
> + * recovery action result to DOM0. DOM0 will save the information in
>   * non-volatile memory for further proactive actions, such as offlining the
>   * easy broken page earlier when doing next reboot.
>  */
> @@ -255,8 +255,8 @@ DEFINE_XEN_GUEST_HANDLE(mc_info_t);
>  #define MC_CAPS_AMD_ECX	6	/* cpuid level 0x80000001 (%ecx) */
>  
>  struct mcinfo_logical_cpu {
> -    uint32_t mc_cpunr;          
> -    uint32_t mc_chipid; 
> +    uint32_t mc_cpunr;
> +    uint32_t mc_chipid;
>      uint16_t mc_coreid;
>      uint16_t mc_threadid;
>      uint32_t mc_apicid;
> @@ -281,7 +281,7 @@ typedef struct mcinfo_logical_cpu xen_mc_logical_cpu_t;
>  DEFINE_XEN_GUEST_HANDLE(xen_mc_logical_cpu_t);
>  
>  
> -/* 
> +/*
>   * OS's should use these instead of writing their own lookup function
>   * each with its own bugs and drawbacks.
>   * We use macros instead of static inline functions to allow guests
> @@ -388,12 +388,12 @@ struct xen_mc_physcpuinfo {
>  #define XEN_MC_msrinject    4
>  #define MC_MSRINJ_MAXMSRS       8
>  struct xen_mc_msrinject {
> -       /* IN */
> -	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 _pad0;
> -	struct mcinfo_msr mcinj_msr[MC_MSRINJ_MAXMSRS];
> +    /* IN */
> +    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 _pad0;
> +    struct mcinfo_msr mcinj_msr[MC_MSRINJ_MAXMSRS];
>  };
>  
>  /* Flags for mcinj_flags above; bits 16-31 are reserved */
> 

Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger
Ust-ID: DE289237879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B

[PATCH v2 2/4] tools/xen-mceinj: Fix code style

[Haozhong Zhang]

Remove trailing whitespaces in xen-mceinj.c.

Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
 tools/tests/mce-test/tools/xen-mceinj.c | 66 ++++++++++++++++-----------------
 1 file changed, 33 insertions(+), 33 deletions(-)

diff --git a/tools/tests/mce-test/tools/xen-mceinj.c b/tools/tests/mce-test/tools/xen-mceinj.c
index e2e49cb..71813c6 100644
--- a/tools/tests/mce-test/tools/xen-mceinj.c
+++ b/tools/tests/mce-test/tools/xen-mceinj.c
@@ -13,7 +13,7 @@
  *
  * You should have received a copy of the GNU General Public License along with
  * this program; If not, see <http://www.gnu.org/licenses/>.
- * 
+ *
  * Authors: Yunhong Jiang <yunhong.jiang@intel.com>
  *          Haicheng Li <haicheng.li@intel.com>
  *          Xudong Hao <xudong.hao@intel.com>
@@ -217,18 +217,18 @@ static uint64_t bank_addr(int bank, int type)
 
     switch ( type )
     {
-        case MCi_type_CTL:
-        case MCi_type_STATUS:
-        case MCi_type_ADDR:
-        case MCi_type_MISC:
-            addr = MSR_IA32_MC0_CTL + (bank * 4) + type;
-            break;
-        case MCi_type_CTL2:
-            addr = MSR_IA32_MC0_CTL2 + bank;
-            break;
-        default:
-            addr = INVALID_MSR;
-            break;
+    case MCi_type_CTL:
+    case MCi_type_STATUS:
+    case MCi_type_ADDR:
+    case MCi_type_MISC:
+        addr = MSR_IA32_MC0_CTL + (bank * 4) + type;
+        break;
+    case MCi_type_CTL2:
+        addr = MSR_IA32_MC0_CTL2 + bank;
+        break;
+    default:
+        addr = INVALID_MSR;
+        break;
     }
 
     return addr;
@@ -243,7 +243,7 @@ static int add_msr_intpose(xc_interface *xc_handle,
     uint32_t count;
 
     if ( (msr_inj.mcinj_count &&
-         (cpu_nr != msr_inj.mcinj_cpunr || flags != msr_inj.mcinj_flags)) ||
+          (cpu_nr != msr_inj.mcinj_cpunr || flags != msr_inj.mcinj_flags)) ||
          msr_inj.mcinj_count == MC_MSRINJ_MAXMSRS )
     {
         flush_msr_inj(xc_handle);
@@ -282,8 +282,8 @@ static int add_msr_bank_intpose(xc_interface *xc_handle,
 #define mfn_valid(_mfn) (_mfn != MCE_INVALID_MFN)
 #define mfn_to_pfn(_mfn) (live_m2p[(_mfn)])
 static uint64_t guest_mfn(xc_interface *xc_handle,
-                               uint32_t domain,
-                               uint64_t gpfn)
+                          uint32_t domain,
+                          uint64_t gpfn)
 {
     xen_pfn_t *live_m2p = NULL;
     int ret;
@@ -300,8 +300,8 @@ static uint64_t guest_mfn(xc_interface *xc_handle,
         return MCE_INVALID_MFN;
 
     /* Get max gpfn */
-    max_gpfn = do_memory_op(xc_handle, XENMEM_maximum_gpfn, &domain, 
-                               sizeof(domain)) + 1;
+    max_gpfn = do_memory_op(xc_handle, XENMEM_maximum_gpfn, &domain,
+                            sizeof(domain)) + 1;
     if ( max_gpfn <= 0 )
         err(xc_handle, "Failed to get max_gpfn 0x%lx", max_gpfn);
 
@@ -360,8 +360,8 @@ static uint64_t mca_gpfn_to_mfn(xc_interface *xc_handle,
     if ( domain == DOMID_XEN )
         return gfn;
 
-    max_gpfn = do_memory_op(xc_handle, XENMEM_maximum_gpfn, &domain, 
-                               sizeof(domain)) + 1;
+    max_gpfn = do_memory_op(xc_handle, XENMEM_maximum_gpfn, &domain,
+                            sizeof(domain)) + 1;
     if ( max_gpfn <= 0 )
         err(xc_handle, "Failed to get max_gpfn 0x%lx", max_gpfn);
     index = gfn % max_gpfn;
@@ -374,7 +374,7 @@ static int inject_mcg_status(xc_interface *xc_handle,
                              uint64_t val)
 {
     return add_msr_intpose(xc_handle, cpu_nr, MC_MSRINJ_F_INTERPOSE,
-                               MSR_IA32_MCG_STATUS, val);
+                           MSR_IA32_MCG_STATUS, val);
 }
 
 static int inject_mci_status(xc_interface *xc_handle,
@@ -383,25 +383,25 @@ static int inject_mci_status(xc_interface *xc_handle,
                              uint64_t val)
 {
     return add_msr_bank_intpose(xc_handle, cpu_nr, MC_MSRINJ_F_INTERPOSE,
-                                    MCi_type_STATUS, bank, val); 
+                                MCi_type_STATUS, bank, val);
 }
 
 static int inject_mci_misc(xc_interface *xc_handle,
-                             uint32_t cpu_nr,
-                             uint64_t bank,
-                             uint64_t val)
+                           uint32_t cpu_nr,
+                           uint64_t bank,
+                           uint64_t val)
 {
     return add_msr_bank_intpose(xc_handle, cpu_nr, MC_MSRINJ_F_INTERPOSE,
-                                    MCi_type_MISC, bank, val); 
+                                MCi_type_MISC, bank, val);
 }
 
 static int inject_mci_addr(xc_interface *xc_handle,
-                             uint32_t cpu_nr,
-                             uint64_t bank,
-                             uint64_t val)
+                           uint32_t cpu_nr,
+                           uint64_t bank,
+                           uint64_t val)
 {
     return add_msr_bank_intpose(xc_handle, cpu_nr, MC_MSRINJ_F_INTERPOSE,
-                                    MCi_type_ADDR, bank, val); 
+                                MCi_type_ADDR, bank, val);
 }
 
 static int inject(xc_interface *xc_handle, struct mce_info *mce,
@@ -553,7 +553,7 @@ int main(int argc, char *argv[])
             return 0;
         }
     }
-    
+
     if ( domid != DOMID_XEN ) {
         max_gpa = xs_get_dom_mem(domid);
         Lprintf("get domain %d max gpa is: 0x%lx", domid, max_gpa);
@@ -570,7 +570,7 @@ int main(int argc, char *argv[])
         haddr = (mfn << PAGE_SHIFT) | (gaddr & (PAGE_SIZE - 1));
         if ( domid == DOMID_XEN )
             Lprintf("Xen: mfn=0x%lx, haddr=0x%lx", mfn, haddr);
-        else 
+        else
             Lprintf("Dom%d: gaddr=0x%lx, gpfn=0x%lx, mfn=0x%lx, haddr=0x%lx",
                     domid, gaddr, gpfn, mfn, haddr);
         goto out;
@@ -583,7 +583,7 @@ int main(int argc, char *argv[])
 
     inject(xc_handle, &mce_table[type], cpu_nr, domid, gaddr);
 
-out:
+ out:
     xc_interface_close(xc_handle);
     return 0;
 }
-- 
2.4.8

Re: [PATCH v2 2/4] tools/xen-mceinj: Fix code style

[Wei Liu]

On Wed, Sep 16, 2015 at 01:35:13PM +0800, Haozhong Zhang wrote:
> Remove trailing whitespaces in xen-mceinj.c.
> 
> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

Haozhong, normally you should carry all the tags you received from the
last round unless there are substantive changes to your patch. If you're
unclear whether you should keep those tags, just ask for clarification.

Reviewers will check those tags to help determine whether they would
need to look at the patch.

Wei.

Re: [PATCH v2 2/4] tools/xen-mceinj: Fix code style

[Haozhong Zhang]

On Wed, Sep 16, 2015 at 08:45:58AM +0100, Wei Liu wrote:
> On Wed, Sep 16, 2015 at 01:35:13PM +0800, Haozhong Zhang wrote:
> > Remove trailing whitespaces in xen-mceinj.c.
> > 
> > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> 
> Acked-by: Wei Liu <wei.liu2@citrix.com>
> 
> Haozhong, normally you should carry all the tags you received from the
> last round unless there are substantive changes to your patch. If you're
> unclear whether you should keep those tags, just ask for clarification.
> 
> Reviewers will check those tags to help determine whether they would
> need to look at the patch.
> 
> Wei.

Got it. I'll resend this patchset w/ all tags soon. Thanks!

- Haozhong Zhang

Re: [PATCH v2 2/4] tools/xen-mceinj: Fix code style

[Wei Liu]

On Wed, Sep 16, 2015 at 03:52:39PM +0800, Haozhong Zhang wrote:
> On Wed, Sep 16, 2015 at 08:45:58AM +0100, Wei Liu wrote:
> > On Wed, Sep 16, 2015 at 01:35:13PM +0800, Haozhong Zhang wrote:
> > > Remove trailing whitespaces in xen-mceinj.c.
> > > 
> > > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> > 
> > Acked-by: Wei Liu <wei.liu2@citrix.com>
> > 
> > Haozhong, normally you should carry all the tags you received from the
> > last round unless there are substantive changes to your patch. If you're
> > unclear whether you should keep those tags, just ask for clarification.
> > 
> > Reviewers will check those tags to help determine whether they would
> > need to look at the patch.
> > 
> > Wei.
> 
> Got it. I'll resend this patchset w/ all tags soon. Thanks!
> 

There is no need to do that this time. :-)

It was just a reminder for you so that you know what to do next time.

Wei.

> - Haozhong Zhang

Re: [PATCH v2 2/4] tools/xen-mceinj: Fix code style

[Wei Liu]

On Wed, Sep 16, 2015 at 03:52:39PM +0800, Haozhong Zhang wrote:
> On Wed, Sep 16, 2015 at 08:45:58AM +0100, Wei Liu wrote:
> > On Wed, Sep 16, 2015 at 01:35:13PM +0800, Haozhong Zhang wrote:
> > > Remove trailing whitespaces in xen-mceinj.c.
> > > 
> > > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> > 
> > Acked-by: Wei Liu <wei.liu2@citrix.com>
> > 
> > Haozhong, normally you should carry all the tags you received from the
> > last round unless there are substantive changes to your patch. If you're
> > unclear whether you should keep those tags, just ask for clarification.
> > 
> > Reviewers will check those tags to help determine whether they would
> > need to look at the patch.
> > 
> > Wei.
> 
> Got it. I'll resend this patchset w/ all tags soon. Thanks!
> 

There is no need to do that this time. :-)

It was just a reminder for you so that you know what to do next time.

Wei.

> - Haozhong Zhang

Re: [PATCH v2 2/4] tools/xen-mceinj: Fix code style

[Haozhong Zhang]

On Wed, Sep 16, 2015 at 08:58:41AM +0100, Wei Liu wrote:
> On Wed, Sep 16, 2015 at 03:52:39PM +0800, Haozhong Zhang wrote:
> > On Wed, Sep 16, 2015 at 08:45:58AM +0100, Wei Liu wrote:
> > > On Wed, Sep 16, 2015 at 01:35:13PM +0800, Haozhong Zhang wrote:
> > > > Remove trailing whitespaces in xen-mceinj.c.
> > > > 
> > > > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> > > 
> > > Acked-by: Wei Liu <wei.liu2@citrix.com>
> > > 
> > > Haozhong, normally you should carry all the tags you received from the
> > > last round unless there are substantive changes to your patch. If you're
> > > unclear whether you should keep those tags, just ask for clarification.
> > > 
> > > Reviewers will check those tags to help determine whether they would
> > > need to look at the patch.
> > > 
> > > Wei.
> > 
> > Got it. I'll resend this patchset w/ all tags soon. Thanks!
> > 
> 
> There is no need to do that this time. :-)
> 
> It was just a reminder for you so that you know what to do next time.
> 
> Wei.
> 
> > - Haozhong Zhang

Thanks! I'll keep this in mind.

- Haozhong Zhang

Re: [PATCH v2 2/4] tools/xen-mceinj: Fix code style

[Egger, Christoph]

On 2015/09/16 7:35, Haozhong Zhang wrote:
> Remove trailing whitespaces in xen-mceinj.c.
> 
> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>

Acked-by: Christoph Egger <chegger@amazon.de>

> ---
>  tools/tests/mce-test/tools/xen-mceinj.c | 66 ++++++++++++++++-----------------
>  1 file changed, 33 insertions(+), 33 deletions(-)
> 
> diff --git a/tools/tests/mce-test/tools/xen-mceinj.c b/tools/tests/mce-test/tools/xen-mceinj.c
> index e2e49cb..71813c6 100644
> --- a/tools/tests/mce-test/tools/xen-mceinj.c
> +++ b/tools/tests/mce-test/tools/xen-mceinj.c
> @@ -13,7 +13,7 @@
>   *
>   * You should have received a copy of the GNU General Public License along with
>   * this program; If not, see <http://www.gnu.org/licenses/>.
> - * 
> + *
>   * Authors: Yunhong Jiang <yunhong.jiang@intel.com>
>   *          Haicheng Li <haicheng.li@intel.com>
>   *          Xudong Hao <xudong.hao@intel.com>
> @@ -217,18 +217,18 @@ static uint64_t bank_addr(int bank, int type)
>  
>      switch ( type )
>      {
> -        case MCi_type_CTL:
> -        case MCi_type_STATUS:
> -        case MCi_type_ADDR:
> -        case MCi_type_MISC:
> -            addr = MSR_IA32_MC0_CTL + (bank * 4) + type;
> -            break;
> -        case MCi_type_CTL2:
> -            addr = MSR_IA32_MC0_CTL2 + bank;
> -            break;
> -        default:
> -            addr = INVALID_MSR;
> -            break;
> +    case MCi_type_CTL:
> +    case MCi_type_STATUS:
> +    case MCi_type_ADDR:
> +    case MCi_type_MISC:
> +        addr = MSR_IA32_MC0_CTL + (bank * 4) + type;
> +        break;
> +    case MCi_type_CTL2:
> +        addr = MSR_IA32_MC0_CTL2 + bank;
> +        break;
> +    default:
> +        addr = INVALID_MSR;
> +        break;
>      }
>  
>      return addr;
> @@ -243,7 +243,7 @@ static int add_msr_intpose(xc_interface *xc_handle,
>      uint32_t count;
>  
>      if ( (msr_inj.mcinj_count &&
> -         (cpu_nr != msr_inj.mcinj_cpunr || flags != msr_inj.mcinj_flags)) ||
> +          (cpu_nr != msr_inj.mcinj_cpunr || flags != msr_inj.mcinj_flags)) ||
>           msr_inj.mcinj_count == MC_MSRINJ_MAXMSRS )
>      {
>          flush_msr_inj(xc_handle);
> @@ -282,8 +282,8 @@ static int add_msr_bank_intpose(xc_interface *xc_handle,
>  #define mfn_valid(_mfn) (_mfn != MCE_INVALID_MFN)
>  #define mfn_to_pfn(_mfn) (live_m2p[(_mfn)])
>  static uint64_t guest_mfn(xc_interface *xc_handle,
> -                               uint32_t domain,
> -                               uint64_t gpfn)
> +                          uint32_t domain,
> +                          uint64_t gpfn)
>  {
>      xen_pfn_t *live_m2p = NULL;
>      int ret;
> @@ -300,8 +300,8 @@ static uint64_t guest_mfn(xc_interface *xc_handle,
>          return MCE_INVALID_MFN;
>  
>      /* Get max gpfn */
> -    max_gpfn = do_memory_op(xc_handle, XENMEM_maximum_gpfn, &domain, 
> -                               sizeof(domain)) + 1;
> +    max_gpfn = do_memory_op(xc_handle, XENMEM_maximum_gpfn, &domain,
> +                            sizeof(domain)) + 1;
>      if ( max_gpfn <= 0 )
>          err(xc_handle, "Failed to get max_gpfn 0x%lx", max_gpfn);
>  
> @@ -360,8 +360,8 @@ static uint64_t mca_gpfn_to_mfn(xc_interface *xc_handle,
>      if ( domain == DOMID_XEN )
>          return gfn;
>  
> -    max_gpfn = do_memory_op(xc_handle, XENMEM_maximum_gpfn, &domain, 
> -                               sizeof(domain)) + 1;
> +    max_gpfn = do_memory_op(xc_handle, XENMEM_maximum_gpfn, &domain,
> +                            sizeof(domain)) + 1;
>      if ( max_gpfn <= 0 )
>          err(xc_handle, "Failed to get max_gpfn 0x%lx", max_gpfn);
>      index = gfn % max_gpfn;
> @@ -374,7 +374,7 @@ static int inject_mcg_status(xc_interface *xc_handle,
>                               uint64_t val)
>  {
>      return add_msr_intpose(xc_handle, cpu_nr, MC_MSRINJ_F_INTERPOSE,
> -                               MSR_IA32_MCG_STATUS, val);
> +                           MSR_IA32_MCG_STATUS, val);
>  }
>  
>  static int inject_mci_status(xc_interface *xc_handle,
> @@ -383,25 +383,25 @@ static int inject_mci_status(xc_interface *xc_handle,
>                               uint64_t val)
>  {
>      return add_msr_bank_intpose(xc_handle, cpu_nr, MC_MSRINJ_F_INTERPOSE,
> -                                    MCi_type_STATUS, bank, val); 
> +                                MCi_type_STATUS, bank, val);
>  }
>  
>  static int inject_mci_misc(xc_interface *xc_handle,
> -                             uint32_t cpu_nr,
> -                             uint64_t bank,
> -                             uint64_t val)
> +                           uint32_t cpu_nr,
> +                           uint64_t bank,
> +                           uint64_t val)
>  {
>      return add_msr_bank_intpose(xc_handle, cpu_nr, MC_MSRINJ_F_INTERPOSE,
> -                                    MCi_type_MISC, bank, val); 
> +                                MCi_type_MISC, bank, val);
>  }
>  
>  static int inject_mci_addr(xc_interface *xc_handle,
> -                             uint32_t cpu_nr,
> -                             uint64_t bank,
> -                             uint64_t val)
> +                           uint32_t cpu_nr,
> +                           uint64_t bank,
> +                           uint64_t val)
>  {
>      return add_msr_bank_intpose(xc_handle, cpu_nr, MC_MSRINJ_F_INTERPOSE,
> -                                    MCi_type_ADDR, bank, val); 
> +                                MCi_type_ADDR, bank, val);
>  }
>  
>  static int inject(xc_interface *xc_handle, struct mce_info *mce,
> @@ -553,7 +553,7 @@ int main(int argc, char *argv[])
>              return 0;
>          }
>      }
> -    
> +
>      if ( domid != DOMID_XEN ) {
>          max_gpa = xs_get_dom_mem(domid);
>          Lprintf("get domain %d max gpa is: 0x%lx", domid, max_gpa);
> @@ -570,7 +570,7 @@ int main(int argc, char *argv[])
>          haddr = (mfn << PAGE_SHIFT) | (gaddr & (PAGE_SIZE - 1));
>          if ( domid == DOMID_XEN )
>              Lprintf("Xen: mfn=0x%lx, haddr=0x%lx", mfn, haddr);
> -        else 
> +        else
>              Lprintf("Dom%d: gaddr=0x%lx, gpfn=0x%lx, mfn=0x%lx, haddr=0x%lx",
>                      domid, gaddr, gpfn, mfn, haddr);
>          goto out;
> @@ -583,7 +583,7 @@ int main(int argc, char *argv[])
>  
>      inject(xc_handle, &mce_table[type], cpu_nr, domid, gaddr);
>  
> -out:
> + out:
>      xc_interface_close(xc_handle);
>      return 0;
>  }
> 

Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger
Ust-ID: DE289237879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B

[PATCH v2 3/4] x86/mce: Translate passed-in GPA to host machine address

[Haozhong Zhang]

This patch adds a new flag MC_MSRINJ_F_GPADDR to
xen_mc_msrinject.mcinj_flags, and makes do_mca() to translate the
guest physical address passed-in through
xen_mc_msrinject.mcinj_msr[i].value to the host machine address if
this flag is present.

Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
 xen/arch/x86/cpu/mcheck/mce.c         | 56 ++++++++++++++++++++++++++++++-----
 xen/include/public/arch-x86/xen-mca.h |  5 +++-
 2 files changed, 52 insertions(+), 9 deletions(-)

diff --git a/xen/arch/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c
index 561257d..08cd3f2 100644
--- a/xen/arch/x86/cpu/mcheck/mce.c
+++ b/xen/arch/x86/cpu/mcheck/mce.c
@@ -21,6 +21,7 @@
 #include <asm/processor.h>
 #include <asm/system.h>
 #include <asm/msr.h>
+#include <asm/p2m.h>
 
 #include "mce.h"
 #include "barrier.h"
@@ -48,14 +49,15 @@ struct mca_banks *mca_allbanks;
 #define _MC_MSRINJ_F_REQ_HWCR_WREN (1 << 16)
 
 #if 0
-static int x86_mcerr(const char *msg, int err)
-{
-    gdprintk(XENLOG_WARNING, "x86_mcerr: %s, returning %d\n",
-             msg != NULL ? msg : "", err);
-    return err;
-}
+#define x86_mcerr(fmt, err, args...)                                    \
+    ({                                                                  \
+        int _err = (err);                                               \
+        gdprintk(XENLOG_WARNING, "x86_mcerr: " fmt ", returning %d\n",  \
+                 ## args, _err);                                        \
+        _err;                                                           \
+    })
 #else
-#define x86_mcerr(msg, err) (err)
+#define x86_mcerr(fmt, err, args...) (err)
 #endif
 
 int mce_verbosity;
@@ -1307,7 +1309,7 @@ long do_mca(XEN_GUEST_HANDLE_PARAM(xen_mc_t) u_xen_mc)
 
     ret = xsm_do_mca(XSM_PRIV);
     if ( ret )
-        return x86_mcerr(NULL, ret);
+        return x86_mcerr("", ret);
 
     if ( copy_from_guest(op, u_xen_mc, 1) )
         return x86_mcerr("do_mca: failed copyin of xen_mc_t", -EFAULT);
@@ -1422,6 +1424,44 @@ long do_mca(XEN_GUEST_HANDLE_PARAM(xen_mc_t) u_xen_mc)
         if (mc_msrinject->mcinj_count == 0)
             return 0;
 
+        if ( mc_msrinject->mcinj_flags & MC_MSRINJ_F_GPADDR )
+        {
+            struct domain *d;
+            struct mcinfo_msr *msr;
+            unsigned int i;
+            paddr_t gaddr;
+            unsigned long gfn, mfn;
+            p2m_type_t t;
+
+            d = get_domain_by_id(mc_msrinject->mcinj_domid);
+            if ( d == NULL )
+                return x86_mcerr("do_mca inject: bad domain id %d",
+                                 -EINVAL, mc_msrinject->mcinj_domid);
+
+            for ( i = 0, msr = &mc_msrinject->mcinj_msr[0];
+                  i < mc_msrinject->mcinj_count;
+                  i++, msr++ )
+            {
+                gaddr = msr->value;
+                gfn = PFN_DOWN(gaddr);
+                mfn = mfn_x(get_gfn(d, gfn, &t));
+
+                if ( mfn == INVALID_MFN )
+                {
+                    put_gfn(d, gfn);
+                    put_domain(d);
+                    return x86_mcerr("do_mca inject: bad gfn %#lx of domain %d",
+                                     -EINVAL, gfn, mc_msrinject->mcinj_domid);
+                }
+
+                msr->value = pfn_to_paddr(mfn) | (gaddr & (PAGE_SIZE - 1));
+
+                put_gfn(d, gfn);
+            }
+
+            put_domain(d);
+        }
+
         if (!x86_mc_msrinject_verify(mc_msrinject))
             return x86_mcerr("do_mca inject: illegal MSR", -EINVAL);
 
diff --git a/xen/include/public/arch-x86/xen-mca.h b/xen/include/public/arch-x86/xen-mca.h
index ec1237a..a97e821 100644
--- a/xen/include/public/arch-x86/xen-mca.h
+++ b/xen/include/public/arch-x86/xen-mca.h
@@ -392,12 +392,15 @@ struct xen_mc_msrinject {
     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 _pad0;
+    domid_t  mcinj_domid;           /* valid only if MC_MSRINJ_F_GPADDR is
+                                       present in mcinj_flags */
+    uint16_t _pad0;
     struct mcinfo_msr mcinj_msr[MC_MSRINJ_MAXMSRS];
 };
 
 /* Flags for mcinj_flags above; bits 16-31 are reserved */
 #define MC_MSRINJ_F_INTERPOSE   0x1
+#define MC_MSRINJ_F_GPADDR      0x2
 
 #define XEN_MC_mceinject    5
 struct xen_mc_mceinject {
-- 
2.4.8

Re: [PATCH v2 3/4] x86/mce: Translate passed-in GPA to host machine address

[Egger, Christoph]

On 2015/09/16 7:35, Haozhong Zhang wrote:
> This patch adds a new flag MC_MSRINJ_F_GPADDR to
> xen_mc_msrinject.mcinj_flags, and makes do_mca() to translate the
> guest physical address passed-in through
> xen_mc_msrinject.mcinj_msr[i].value to the host machine address if
> this flag is present.
> 
> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>

Acked-by: Christoph Egger <chegger@amazon.de>

> ---
>  xen/arch/x86/cpu/mcheck/mce.c         | 56 ++++++++++++++++++++++++++++++-----
>  xen/include/public/arch-x86/xen-mca.h |  5 +++-
>  2 files changed, 52 insertions(+), 9 deletions(-)
> 
> diff --git a/xen/arch/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c
> index 561257d..08cd3f2 100644
> --- a/xen/arch/x86/cpu/mcheck/mce.c
> +++ b/xen/arch/x86/cpu/mcheck/mce.c
> @@ -21,6 +21,7 @@
>  #include <asm/processor.h>
>  #include <asm/system.h>
>  #include <asm/msr.h>
> +#include <asm/p2m.h>
>  
>  #include "mce.h"
>  #include "barrier.h"
> @@ -48,14 +49,15 @@ struct mca_banks *mca_allbanks;
>  #define _MC_MSRINJ_F_REQ_HWCR_WREN (1 << 16)
>  
>  #if 0
> -static int x86_mcerr(const char *msg, int err)
> -{
> -    gdprintk(XENLOG_WARNING, "x86_mcerr: %s, returning %d\n",
> -             msg != NULL ? msg : "", err);
> -    return err;
> -}
> +#define x86_mcerr(fmt, err, args...)                                    \
> +    ({                                                                  \
> +        int _err = (err);                                               \
> +        gdprintk(XENLOG_WARNING, "x86_mcerr: " fmt ", returning %d\n",  \
> +                 ## args, _err);                                        \
> +        _err;                                                           \
> +    })
>  #else
> -#define x86_mcerr(msg, err) (err)
> +#define x86_mcerr(fmt, err, args...) (err)
>  #endif
>  
>  int mce_verbosity;
> @@ -1307,7 +1309,7 @@ long do_mca(XEN_GUEST_HANDLE_PARAM(xen_mc_t) u_xen_mc)
>  
>      ret = xsm_do_mca(XSM_PRIV);
>      if ( ret )
> -        return x86_mcerr(NULL, ret);
> +        return x86_mcerr("", ret);
>  
>      if ( copy_from_guest(op, u_xen_mc, 1) )
>          return x86_mcerr("do_mca: failed copyin of xen_mc_t", -EFAULT);
> @@ -1422,6 +1424,44 @@ long do_mca(XEN_GUEST_HANDLE_PARAM(xen_mc_t) u_xen_mc)
>          if (mc_msrinject->mcinj_count == 0)
>              return 0;
>  
> +        if ( mc_msrinject->mcinj_flags & MC_MSRINJ_F_GPADDR )
> +        {
> +            struct domain *d;
> +            struct mcinfo_msr *msr;
> +            unsigned int i;
> +            paddr_t gaddr;
> +            unsigned long gfn, mfn;
> +            p2m_type_t t;
> +
> +            d = get_domain_by_id(mc_msrinject->mcinj_domid);
> +            if ( d == NULL )
> +                return x86_mcerr("do_mca inject: bad domain id %d",
> +                                 -EINVAL, mc_msrinject->mcinj_domid);
> +
> +            for ( i = 0, msr = &mc_msrinject->mcinj_msr[0];
> +                  i < mc_msrinject->mcinj_count;
> +                  i++, msr++ )
> +            {
> +                gaddr = msr->value;
> +                gfn = PFN_DOWN(gaddr);
> +                mfn = mfn_x(get_gfn(d, gfn, &t));
> +
> +                if ( mfn == INVALID_MFN )
> +                {
> +                    put_gfn(d, gfn);
> +                    put_domain(d);
> +                    return x86_mcerr("do_mca inject: bad gfn %#lx of domain %d",
> +                                     -EINVAL, gfn, mc_msrinject->mcinj_domid);
> +                }
> +
> +                msr->value = pfn_to_paddr(mfn) | (gaddr & (PAGE_SIZE - 1));
> +
> +                put_gfn(d, gfn);
> +            }
> +
> +            put_domain(d);
> +        }
> +
>          if (!x86_mc_msrinject_verify(mc_msrinject))
>              return x86_mcerr("do_mca inject: illegal MSR", -EINVAL);
>  
> diff --git a/xen/include/public/arch-x86/xen-mca.h b/xen/include/public/arch-x86/xen-mca.h
> index ec1237a..a97e821 100644
> --- a/xen/include/public/arch-x86/xen-mca.h
> +++ b/xen/include/public/arch-x86/xen-mca.h
> @@ -392,12 +392,15 @@ struct xen_mc_msrinject {
>      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 _pad0;
> +    domid_t  mcinj_domid;           /* valid only if MC_MSRINJ_F_GPADDR is
> +                                       present in mcinj_flags */
> +    uint16_t _pad0;
>      struct mcinfo_msr mcinj_msr[MC_MSRINJ_MAXMSRS];
>  };
>  
>  /* Flags for mcinj_flags above; bits 16-31 are reserved */
>  #define MC_MSRINJ_F_INTERPOSE   0x1
> +#define MC_MSRINJ_F_GPADDR      0x2
>  
>  #define XEN_MC_mceinject    5
>  struct xen_mc_mceinject {
> 

Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger
Ust-ID: DE289237879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B

[PATCH v2 4/4] tools/xen-mceinj: Pass in GPA when injecting through MSR_MCI_ADDR

[Haozhong Zhang]

This patch removes the address translation in xen-mceinj which
translates the guest physical address passed-in through the argument of
'-p' to the host machine address. Instead, xen-mceinj now passes a flag
MC_MSRINJ_F_GPADDR to ask do_mca() in the hypervisor to do this
translation.

Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
 tools/tests/mce-test/tools/xen-mceinj.c | 152 ++++++--------------------------
 1 file changed, 29 insertions(+), 123 deletions(-)

diff --git a/tools/tests/mce-test/tools/xen-mceinj.c b/tools/tests/mce-test/tools/xen-mceinj.c
index 71813c6..31cdb04 100644
--- a/tools/tests/mce-test/tools/xen-mceinj.c
+++ b/tools/tests/mce-test/tools/xen-mceinj.c
@@ -238,12 +238,14 @@ static int add_msr_intpose(xc_interface *xc_handle,
                            uint32_t cpu_nr,
                            uint32_t flags,
                            uint64_t msr,
-                           uint64_t val)
+                           uint64_t val,
+                           domid_t domid)
 {
     uint32_t count;
 
     if ( (msr_inj.mcinj_count &&
-          (cpu_nr != msr_inj.mcinj_cpunr || flags != msr_inj.mcinj_flags)) ||
+          (cpu_nr != msr_inj.mcinj_cpunr || flags != msr_inj.mcinj_flags ||
+           domid != msr_inj.mcinj_domid)) ||
          msr_inj.mcinj_count == MC_MSRINJ_MAXMSRS )
     {
         flush_msr_inj(xc_handle);
@@ -255,6 +257,7 @@ static int add_msr_intpose(xc_interface *xc_handle,
     {
         msr_inj.mcinj_cpunr = cpu_nr;
         msr_inj.mcinj_flags = flags;
+        msr_inj.mcinj_domid = domid;
     }
     msr_inj.mcinj_msr[count].reg = msr;
     msr_inj.mcinj_msr[count].value = val;
@@ -268,168 +271,77 @@ static int add_msr_bank_intpose(xc_interface *xc_handle,
                                 uint32_t flags,
                                 uint32_t type,
                                 uint32_t bank,
-                                uint64_t val)
+                                uint64_t val,
+                                domid_t domid)
 {
     uint64_t msr;
 
     msr = bank_addr(bank, type);
     if ( msr == INVALID_MSR )
         return -1;
-    return add_msr_intpose(xc_handle, cpu_nr, flags, msr, val);
-}
-
-#define MCE_INVALID_MFN ~0UL
-#define mfn_valid(_mfn) (_mfn != MCE_INVALID_MFN)
-#define mfn_to_pfn(_mfn) (live_m2p[(_mfn)])
-static uint64_t guest_mfn(xc_interface *xc_handle,
-                          uint32_t domain,
-                          uint64_t gpfn)
-{
-    xen_pfn_t *live_m2p = NULL;
-    int ret;
-    unsigned long hvirt_start;
-    unsigned int pt_levels;
-    uint64_t * pfn_buf = NULL;
-    unsigned long max_mfn = 0; /* max mfn of the whole machine */
-    unsigned long m2p_mfn0;
-    unsigned int guest_width;
-    long max_gpfn,i;
-    uint64_t mfn = MCE_INVALID_MFN;
-
-    if ( domain > DOMID_FIRST_RESERVED )
-        return MCE_INVALID_MFN;
-
-    /* Get max gpfn */
-    max_gpfn = do_memory_op(xc_handle, XENMEM_maximum_gpfn, &domain,
-                            sizeof(domain)) + 1;
-    if ( max_gpfn <= 0 )
-        err(xc_handle, "Failed to get max_gpfn 0x%lx", max_gpfn);
-
-    Lprintf("Maxium gpfn for dom %d is 0x%lx", domain, max_gpfn);
-
-    /* Get max mfn */
-    if ( !get_platform_info(xc_handle, domain,
-                            &max_mfn, &hvirt_start,
-                            &pt_levels, &guest_width) )
-        err(xc_handle, "Failed to get platform information");
-
-    /* Get guest's pfn list */
-    pfn_buf = calloc(max_gpfn, sizeof(uint64_t));
-    if ( !pfn_buf )
-        err(xc_handle, "Failed to alloc pfn buf");
-
-    ret = xc_get_pfn_list(xc_handle, domain, pfn_buf, max_gpfn);
-    if ( ret < 0 ) {
-        free(pfn_buf);
-        err(xc_handle, "Failed to get pfn list %x", ret);
-    }
-
-    /* Now get the m2p table */
-    live_m2p = xc_map_m2p(xc_handle, max_mfn, PROT_READ, &m2p_mfn0);
-    if ( !live_m2p )
-        err(xc_handle, "Failed to map live M2P table");
-
-    /* match the mapping */
-    for ( i = 0; i < max_gpfn; i++ )
-    {
-        uint64_t tmp;
-        tmp = pfn_buf[i];
-
-        if (mfn_valid(tmp) &&  (mfn_to_pfn(tmp) == gpfn))
-        {
-            mfn = tmp;
-            Lprintf("We get the mfn 0x%lx for this injection", mfn);
-            break;
-        }
-    }
-
-    munmap(live_m2p, M2P_SIZE(max_mfn));
-
-    free(pfn_buf);
-    return mfn;
-}
-
-static uint64_t mca_gpfn_to_mfn(xc_interface *xc_handle,
-                                uint32_t domain,
-                                uint64_t gfn)
-{
-    uint64_t index;
-    long max_gpfn;
-
-    /* If domain is xen, means we want pass index directly */
-    if ( domain == DOMID_XEN )
-        return gfn;
-
-    max_gpfn = do_memory_op(xc_handle, XENMEM_maximum_gpfn, &domain,
-                            sizeof(domain)) + 1;
-    if ( max_gpfn <= 0 )
-        err(xc_handle, "Failed to get max_gpfn 0x%lx", max_gpfn);
-    index = gfn % max_gpfn;
-
-    return guest_mfn(xc_handle, domain, index);
+    return add_msr_intpose(xc_handle, cpu_nr, flags, msr, val, domid);
 }
 
 static int inject_mcg_status(xc_interface *xc_handle,
                              uint32_t cpu_nr,
-                             uint64_t val)
+                             uint64_t val,
+                             domid_t domid)
 {
     return add_msr_intpose(xc_handle, cpu_nr, MC_MSRINJ_F_INTERPOSE,
-                           MSR_IA32_MCG_STATUS, val);
+                           MSR_IA32_MCG_STATUS, val, domid);
 }
 
 static int inject_mci_status(xc_interface *xc_handle,
                              uint32_t cpu_nr,
                              uint64_t bank,
-                             uint64_t val)
+                             uint64_t val,
+                             domid_t domid)
 {
     return add_msr_bank_intpose(xc_handle, cpu_nr, MC_MSRINJ_F_INTERPOSE,
-                                MCi_type_STATUS, bank, val);
+                                MCi_type_STATUS, bank, val, domid);
 }
 
 static int inject_mci_misc(xc_interface *xc_handle,
                            uint32_t cpu_nr,
                            uint64_t bank,
-                           uint64_t val)
+                           uint64_t val,
+                           domid_t domid)
 {
     return add_msr_bank_intpose(xc_handle, cpu_nr, MC_MSRINJ_F_INTERPOSE,
-                                MCi_type_MISC, bank, val);
+                                MCi_type_MISC, bank, val, domid);
 }
 
 static int inject_mci_addr(xc_interface *xc_handle,
                            uint32_t cpu_nr,
                            uint64_t bank,
-                           uint64_t val)
+                           uint64_t val,
+                           domid_t domid)
 {
-    return add_msr_bank_intpose(xc_handle, cpu_nr, MC_MSRINJ_F_INTERPOSE,
-                                MCi_type_ADDR, bank, val);
+    return add_msr_bank_intpose(xc_handle, cpu_nr,
+                                MC_MSRINJ_F_INTERPOSE | MC_MSRINJ_F_GPADDR,
+                                MCi_type_ADDR, bank, val, domid);
 }
 
 static int inject(xc_interface *xc_handle, struct mce_info *mce,
                   uint32_t cpu_nr, uint32_t domain, uint64_t gaddr)
 {
-    uint64_t gpfn, mfn, haddr;
     int ret = 0;
 
-    ret = inject_mcg_status(xc_handle, cpu_nr, mce->mcg_stat);
+    ret = inject_mcg_status(xc_handle, cpu_nr, mce->mcg_stat, domain);
     if ( ret )
         err(xc_handle, "Failed to inject MCG_STATUS MSR");
 
     ret = inject_mci_status(xc_handle, cpu_nr,
-                            mce->bank, mce->mci_stat);
+                            mce->bank, mce->mci_stat, domain);
     if ( ret )
         err(xc_handle, "Failed to inject MCi_STATUS MSR");
 
     ret = inject_mci_misc(xc_handle, cpu_nr,
-                          mce->bank, mce->mci_misc);
+                          mce->bank, mce->mci_misc, domain);
     if ( ret )
         err(xc_handle, "Failed to inject MCi_MISC MSR");
 
-    gpfn = gaddr >> PAGE_SHIFT;
-    mfn = mca_gpfn_to_mfn(xc_handle, domain, gpfn);
-    if (!mfn_valid(mfn))
-        err(xc_handle, "The MFN is not valid");
-    haddr = (mfn << PAGE_SHIFT) | (gaddr & (PAGE_SIZE - 1));
-    ret = inject_mci_addr(xc_handle, cpu_nr, mce->bank, haddr);
+    ret = inject_mci_addr(xc_handle, cpu_nr, mce->bank, gaddr, domain);
     if ( ret )
         err(xc_handle, "Failed to inject MCi_ADDR MSR");
 
@@ -507,7 +419,7 @@ int main(int argc, char *argv[])
     uint32_t domid;
     xc_interface *xc_handle;
     int cpu_nr;
-    int64_t gaddr, gpfn, mfn, haddr, max_gpa;
+    uint64_t gaddr, max_gpa;
 
     /* Default Value */
     domid = DOMID_XEN;
@@ -563,16 +475,10 @@ int main(int argc, char *argv[])
     Lprintf("get gaddr of error inject is: 0x%lx", gaddr);
 
     if ( dump ) {
-        gpfn = gaddr >> PAGE_SHIFT;
-        mfn = mca_gpfn_to_mfn(xc_handle, domid, gpfn);
-        if (!mfn_valid(mfn))
-            err(xc_handle, "The MFN is not valid");
-        haddr = (mfn << PAGE_SHIFT) | (gaddr & (PAGE_SIZE - 1));
         if ( domid == DOMID_XEN )
-            Lprintf("Xen: mfn=0x%lx, haddr=0x%lx", mfn, haddr);
+            Lprintf("Xen: gaddr=0x%lx", gaddr);
         else
-            Lprintf("Dom%d: gaddr=0x%lx, gpfn=0x%lx, mfn=0x%lx, haddr=0x%lx",
-                    domid, gaddr, gpfn, mfn, haddr);
+            Lprintf("Dom%d: gaddr=0x%lx", domid, gaddr);
         goto out;
     }
 
-- 
2.4.8

Re: [PATCH v2 4/4] tools/xen-mceinj: Pass in GPA when injecting through MSR_MCI_ADDR

[Egger, Christoph]

On 2015/09/16 7:35, Haozhong Zhang wrote:
> This patch removes the address translation in xen-mceinj which
> translates the guest physical address passed-in through the argument of
> '-p' to the host machine address. Instead, xen-mceinj now passes a flag
> MC_MSRINJ_F_GPADDR to ask do_mca() in the hypervisor to do this
> translation.
> 
> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>

Acked-by: Christoph Egger <chegger@amazon.de>

> ---
>  tools/tests/mce-test/tools/xen-mceinj.c | 152 ++++++--------------------------
>  1 file changed, 29 insertions(+), 123 deletions(-)
> 
> diff --git a/tools/tests/mce-test/tools/xen-mceinj.c b/tools/tests/mce-test/tools/xen-mceinj.c
> index 71813c6..31cdb04 100644
> --- a/tools/tests/mce-test/tools/xen-mceinj.c
> +++ b/tools/tests/mce-test/tools/xen-mceinj.c
> @@ -238,12 +238,14 @@ static int add_msr_intpose(xc_interface *xc_handle,
>                             uint32_t cpu_nr,
>                             uint32_t flags,
>                             uint64_t msr,
> -                           uint64_t val)
> +                           uint64_t val,
> +                           domid_t domid)
>  {
>      uint32_t count;
>  
>      if ( (msr_inj.mcinj_count &&
> -          (cpu_nr != msr_inj.mcinj_cpunr || flags != msr_inj.mcinj_flags)) ||
> +          (cpu_nr != msr_inj.mcinj_cpunr || flags != msr_inj.mcinj_flags ||
> +           domid != msr_inj.mcinj_domid)) ||
>           msr_inj.mcinj_count == MC_MSRINJ_MAXMSRS )
>      {
>          flush_msr_inj(xc_handle);
> @@ -255,6 +257,7 @@ static int add_msr_intpose(xc_interface *xc_handle,
>      {
>          msr_inj.mcinj_cpunr = cpu_nr;
>          msr_inj.mcinj_flags = flags;
> +        msr_inj.mcinj_domid = domid;
>      }
>      msr_inj.mcinj_msr[count].reg = msr;
>      msr_inj.mcinj_msr[count].value = val;
> @@ -268,168 +271,77 @@ static int add_msr_bank_intpose(xc_interface *xc_handle,
>                                  uint32_t flags,
>                                  uint32_t type,
>                                  uint32_t bank,
> -                                uint64_t val)
> +                                uint64_t val,
> +                                domid_t domid)
>  {
>      uint64_t msr;
>  
>      msr = bank_addr(bank, type);
>      if ( msr == INVALID_MSR )
>          return -1;
> -    return add_msr_intpose(xc_handle, cpu_nr, flags, msr, val);
> -}
> -
> -#define MCE_INVALID_MFN ~0UL
> -#define mfn_valid(_mfn) (_mfn != MCE_INVALID_MFN)
> -#define mfn_to_pfn(_mfn) (live_m2p[(_mfn)])
> -static uint64_t guest_mfn(xc_interface *xc_handle,
> -                          uint32_t domain,
> -                          uint64_t gpfn)
> -{
> -    xen_pfn_t *live_m2p = NULL;
> -    int ret;
> -    unsigned long hvirt_start;
> -    unsigned int pt_levels;
> -    uint64_t * pfn_buf = NULL;
> -    unsigned long max_mfn = 0; /* max mfn of the whole machine */
> -    unsigned long m2p_mfn0;
> -    unsigned int guest_width;
> -    long max_gpfn,i;
> -    uint64_t mfn = MCE_INVALID_MFN;
> -
> -    if ( domain > DOMID_FIRST_RESERVED )
> -        return MCE_INVALID_MFN;
> -
> -    /* Get max gpfn */
> -    max_gpfn = do_memory_op(xc_handle, XENMEM_maximum_gpfn, &domain,
> -                            sizeof(domain)) + 1;
> -    if ( max_gpfn <= 0 )
> -        err(xc_handle, "Failed to get max_gpfn 0x%lx", max_gpfn);
> -
> -    Lprintf("Maxium gpfn for dom %d is 0x%lx", domain, max_gpfn);
> -
> -    /* Get max mfn */
> -    if ( !get_platform_info(xc_handle, domain,
> -                            &max_mfn, &hvirt_start,
> -                            &pt_levels, &guest_width) )
> -        err(xc_handle, "Failed to get platform information");
> -
> -    /* Get guest's pfn list */
> -    pfn_buf = calloc(max_gpfn, sizeof(uint64_t));
> -    if ( !pfn_buf )
> -        err(xc_handle, "Failed to alloc pfn buf");
> -
> -    ret = xc_get_pfn_list(xc_handle, domain, pfn_buf, max_gpfn);
> -    if ( ret < 0 ) {
> -        free(pfn_buf);
> -        err(xc_handle, "Failed to get pfn list %x", ret);
> -    }
> -
> -    /* Now get the m2p table */
> -    live_m2p = xc_map_m2p(xc_handle, max_mfn, PROT_READ, &m2p_mfn0);
> -    if ( !live_m2p )
> -        err(xc_handle, "Failed to map live M2P table");
> -
> -    /* match the mapping */
> -    for ( i = 0; i < max_gpfn; i++ )
> -    {
> -        uint64_t tmp;
> -        tmp = pfn_buf[i];
> -
> -        if (mfn_valid(tmp) &&  (mfn_to_pfn(tmp) == gpfn))
> -        {
> -            mfn = tmp;
> -            Lprintf("We get the mfn 0x%lx for this injection", mfn);
> -            break;
> -        }
> -    }
> -
> -    munmap(live_m2p, M2P_SIZE(max_mfn));
> -
> -    free(pfn_buf);
> -    return mfn;
> -}
> -
> -static uint64_t mca_gpfn_to_mfn(xc_interface *xc_handle,
> -                                uint32_t domain,
> -                                uint64_t gfn)
> -{
> -    uint64_t index;
> -    long max_gpfn;
> -
> -    /* If domain is xen, means we want pass index directly */
> -    if ( domain == DOMID_XEN )
> -        return gfn;
> -
> -    max_gpfn = do_memory_op(xc_handle, XENMEM_maximum_gpfn, &domain,
> -                            sizeof(domain)) + 1;
> -    if ( max_gpfn <= 0 )
> -        err(xc_handle, "Failed to get max_gpfn 0x%lx", max_gpfn);
> -    index = gfn % max_gpfn;
> -
> -    return guest_mfn(xc_handle, domain, index);
> +    return add_msr_intpose(xc_handle, cpu_nr, flags, msr, val, domid);
>  }
>  
>  static int inject_mcg_status(xc_interface *xc_handle,
>                               uint32_t cpu_nr,
> -                             uint64_t val)
> +                             uint64_t val,
> +                             domid_t domid)
>  {
>      return add_msr_intpose(xc_handle, cpu_nr, MC_MSRINJ_F_INTERPOSE,
> -                           MSR_IA32_MCG_STATUS, val);
> +                           MSR_IA32_MCG_STATUS, val, domid);
>  }
>  
>  static int inject_mci_status(xc_interface *xc_handle,
>                               uint32_t cpu_nr,
>                               uint64_t bank,
> -                             uint64_t val)
> +                             uint64_t val,
> +                             domid_t domid)
>  {
>      return add_msr_bank_intpose(xc_handle, cpu_nr, MC_MSRINJ_F_INTERPOSE,
> -                                MCi_type_STATUS, bank, val);
> +                                MCi_type_STATUS, bank, val, domid);
>  }
>  
>  static int inject_mci_misc(xc_interface *xc_handle,
>                             uint32_t cpu_nr,
>                             uint64_t bank,
> -                           uint64_t val)
> +                           uint64_t val,
> +                           domid_t domid)
>  {
>      return add_msr_bank_intpose(xc_handle, cpu_nr, MC_MSRINJ_F_INTERPOSE,
> -                                MCi_type_MISC, bank, val);
> +                                MCi_type_MISC, bank, val, domid);
>  }
>  
>  static int inject_mci_addr(xc_interface *xc_handle,
>                             uint32_t cpu_nr,
>                             uint64_t bank,
> -                           uint64_t val)
> +                           uint64_t val,
> +                           domid_t domid)
>  {
> -    return add_msr_bank_intpose(xc_handle, cpu_nr, MC_MSRINJ_F_INTERPOSE,
> -                                MCi_type_ADDR, bank, val);
> +    return add_msr_bank_intpose(xc_handle, cpu_nr,
> +                                MC_MSRINJ_F_INTERPOSE | MC_MSRINJ_F_GPADDR,
> +                                MCi_type_ADDR, bank, val, domid);
>  }
>  
>  static int inject(xc_interface *xc_handle, struct mce_info *mce,
>                    uint32_t cpu_nr, uint32_t domain, uint64_t gaddr)
>  {
> -    uint64_t gpfn, mfn, haddr;
>      int ret = 0;
>  
> -    ret = inject_mcg_status(xc_handle, cpu_nr, mce->mcg_stat);
> +    ret = inject_mcg_status(xc_handle, cpu_nr, mce->mcg_stat, domain);
>      if ( ret )
>          err(xc_handle, "Failed to inject MCG_STATUS MSR");
>  
>      ret = inject_mci_status(xc_handle, cpu_nr,
> -                            mce->bank, mce->mci_stat);
> +                            mce->bank, mce->mci_stat, domain);
>      if ( ret )
>          err(xc_handle, "Failed to inject MCi_STATUS MSR");
>  
>      ret = inject_mci_misc(xc_handle, cpu_nr,
> -                          mce->bank, mce->mci_misc);
> +                          mce->bank, mce->mci_misc, domain);
>      if ( ret )
>          err(xc_handle, "Failed to inject MCi_MISC MSR");
>  
> -    gpfn = gaddr >> PAGE_SHIFT;
> -    mfn = mca_gpfn_to_mfn(xc_handle, domain, gpfn);
> -    if (!mfn_valid(mfn))
> -        err(xc_handle, "The MFN is not valid");
> -    haddr = (mfn << PAGE_SHIFT) | (gaddr & (PAGE_SIZE - 1));
> -    ret = inject_mci_addr(xc_handle, cpu_nr, mce->bank, haddr);
> +    ret = inject_mci_addr(xc_handle, cpu_nr, mce->bank, gaddr, domain);
>      if ( ret )
>          err(xc_handle, "Failed to inject MCi_ADDR MSR");
>  
> @@ -507,7 +419,7 @@ int main(int argc, char *argv[])
>      uint32_t domid;
>      xc_interface *xc_handle;
>      int cpu_nr;
> -    int64_t gaddr, gpfn, mfn, haddr, max_gpa;
> +    uint64_t gaddr, max_gpa;
>  
>      /* Default Value */
>      domid = DOMID_XEN;
> @@ -563,16 +475,10 @@ int main(int argc, char *argv[])
>      Lprintf("get gaddr of error inject is: 0x%lx", gaddr);
>  
>      if ( dump ) {
> -        gpfn = gaddr >> PAGE_SHIFT;
> -        mfn = mca_gpfn_to_mfn(xc_handle, domid, gpfn);
> -        if (!mfn_valid(mfn))
> -            err(xc_handle, "The MFN is not valid");
> -        haddr = (mfn << PAGE_SHIFT) | (gaddr & (PAGE_SIZE - 1));
>          if ( domid == DOMID_XEN )
> -            Lprintf("Xen: mfn=0x%lx, haddr=0x%lx", mfn, haddr);
> +            Lprintf("Xen: gaddr=0x%lx", gaddr);
>          else
> -            Lprintf("Dom%d: gaddr=0x%lx, gpfn=0x%lx, mfn=0x%lx, haddr=0x%lx",
> -                    domid, gaddr, gpfn, mfn, haddr);
> +            Lprintf("Dom%d: gaddr=0x%lx", domid, gaddr);
>          goto out;
>      }
>  
> 

Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger
Ust-ID: DE289237879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B

Re: [PATCH v2 4/4] tools/xen-mceinj: Pass in GPA when injecting through MSR_MCI_ADDR

[Jan Beulich]

>>> On 16.09.15 at 07:35, <haozhong.zhang@intel.com> wrote:
> This patch removes the address translation in xen-mceinj which
> translates the guest physical address passed-in through the argument of
> '-p' to the host machine address. Instead, xen-mceinj now passes a flag
> MC_MSRINJ_F_GPADDR to ask do_mca() in the hypervisor to do this
> translation.
> 
> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>

I applied patches 1..3; this one is still pending a tool stack maintainer's
ack.

Jan

Re: [PATCH v2 4/4] tools/xen-mceinj: Pass in GPA when injecting through MSR_MCI_ADDR

[Wei Liu]

On Wed, Sep 16, 2015 at 01:35:15PM +0800, Haozhong Zhang wrote:
> This patch removes the address translation in xen-mceinj which
> translates the guest physical address passed-in through the argument of
> '-p' to the host machine address. Instead, xen-mceinj now passes a flag
> MC_MSRINJ_F_GPADDR to ask do_mca() in the hypervisor to do this
> translation.
> 
> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

Re: [PATCH v2 0/4] Fix tools/xen-mceinj to handle >=4GB guest memory

[Ian Jackson]

Haozhong Zhang writes ("[PATCH v2 0/4] Fix tools/xen-mceinj to handle >=4GB guest memory"):
> The existing xen-mceinj can not inject MCE through MSR_MCI_ADDR to a
> domain w/ more than 4GB memory, e.g. if domain 0 has more than 4GB
> memory, the execution of the command
>         xen-mceinj -d 0 -t 0 -p 0x2721a900
> will fail w/ a message "Failed to get pfn list ffffffff: Operation not
> supported".

xen-mceinj is a test utility, not a piece of code run in projection.
Accordingly, I don't think it needs a very thorough review.

I have had a quick look at the parts of this in tools/ and saw nothing
untoward.  I am not qualified to review the design but I hope
hypervisor and x86 maintaintainers will do that.

So as far as tools patches go,

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

Hypervisor and x86 maintainers: feel free to commit the tools parts of
this along with the hypervisor changes, subject to your own design and
code review and acks.

Thanks,
Ian.