xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Xen/MCE: stick all 1's to MCi_CTL of vMCE
@ 2012-07-05 18:27 Liu, Jinsong
  2012-07-06  7:50 ` Jan Beulich
  2012-07-06  8:39 ` Christoph Egger
  0 siblings, 2 replies; 4+ messages in thread
From: Liu, Jinsong @ 2012-07-05 18:27 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Christoph Egger, Luck, Tony, Keir Fraser, Ian Campbell,
	Jiang, Yunhong, Auld, Will, xen-devel@lists.xensource.com

[-- Attachment #1: Type: text/plain, Size: 3878 bytes --]

Jan,

This patch just used to stick all 1's to MCi_CTL, it should not involve much argue, so I sent it separately.

Thanks,
Jinsong

====================
Xen/MCE: stick all 1's to MCi_CTL of vMCE

This patch is a middle-work patch, prepare for future new vMCE model.
It remove mci_ctl array, and keep MCi_CTL all 1's.

Signed-off-by: Liu, Jinsong <jinsong.liu@intel.com>

diff -r 4f92bdf3370c xen/arch/x86/cpu/mcheck/vmce.c
--- a/xen/arch/x86/cpu/mcheck/vmce.c	Wed Jun 27 09:36:43 2012 +0200
+++ b/xen/arch/x86/cpu/mcheck/vmce.c	Fri Jul 06 10:05:46 2012 +0800
@@ -25,7 +25,6 @@
 
 /* Real value in physical CTL MSR */
 static uint64_t __read_mostly h_mcg_ctl;
-static uint64_t *__read_mostly h_mci_ctrl;
 
 int vmce_init_msr(struct domain *d)
 {
@@ -33,15 +32,6 @@
     if ( !dom_vmce(d) )
         return -ENOMEM;
 
-    dom_vmce(d)->mci_ctl = xmalloc_array(uint64_t, nr_mce_banks);
-    if ( !dom_vmce(d)->mci_ctl )
-    {
-        xfree(dom_vmce(d));
-        return -ENOMEM;
-    }
-    memset(dom_vmce(d)->mci_ctl, ~0,
-           nr_mce_banks * sizeof(*dom_vmce(d)->mci_ctl));
-
     dom_vmce(d)->mcg_status = 0x0;
     dom_vmce(d)->mcg_ctl = ~(uint64_t)0x0;
     dom_vmce(d)->nr_injection = 0;
@@ -56,7 +46,6 @@
 {
     if ( !dom_vmce(d) )
         return;
-    xfree(dom_vmce(d)->mci_ctl);
     xfree(dom_vmce(d));
     dom_vmce(d) = NULL;
 }
@@ -93,9 +82,8 @@
     switch ( msr & (MSR_IA32_MC0_CTL | 3) )
     {
     case MSR_IA32_MC0_CTL:
-        if ( bank < nr_mce_banks )
-            *val = vmce->mci_ctl[bank] &
-                   (h_mci_ctrl ? h_mci_ctrl[bank] : ~0UL);
+        /* stick all 1's to MCi_CTL */
+        *val = ~0UL;
         mce_printk(MCE_VERBOSE, "MCE: rdmsr MC%u_CTL 0x%"PRIx64"\n",
                    bank, *val);
         break;
@@ -220,8 +208,10 @@
     switch ( msr & (MSR_IA32_MC0_CTL | 3) )
     {
     case MSR_IA32_MC0_CTL:
-        if ( bank < nr_mce_banks )
-            vmce->mci_ctl[bank] = val;
+        /*
+         * if guest crazy clear any bit of MCi_CTL,
+         * treat it as not implement and ignore write change it.
+         */
         break;
     case MSR_IA32_MC0_STATUS:
         if ( entry && (entry->bank == bank) )
@@ -523,22 +513,6 @@
 int vmce_init(struct cpuinfo_x86 *c)
 {
     u64 value;
-    unsigned int i;
-
-    if ( !h_mci_ctrl )
-    {
-        h_mci_ctrl = xmalloc_array(uint64_t, nr_mce_banks);
-        if (!h_mci_ctrl)
-        {
-            dprintk(XENLOG_INFO, "Failed to alloc h_mci_ctrl\n");
-            return -ENOMEM;
-        }
-        /* Don't care banks before firstbank */
-        memset(h_mci_ctrl, ~0,
-               min(firstbank, nr_mce_banks) * sizeof(*h_mci_ctrl));
-        for (i = firstbank; i < nr_mce_banks; i++)
-            rdmsrl(MSR_IA32_MCx_CTL(i), h_mci_ctrl[i]);
-    }
 
     rdmsrl(MSR_IA32_MCG_CAP, value);
     /* For Guest vMCE usage */
@@ -551,18 +525,13 @@
 
 static int mca_ctl_conflict(struct mcinfo_bank *bank, struct domain *d)
 {
-    int bank_nr;
-
-    if ( !bank || !d || !h_mci_ctrl )
+    if ( !bank || !d )
         return 1;
 
     /* Will MCE happen in host if If host mcg_ctl is 0? */
     if ( ~d->arch.vmca_msrs->mcg_ctl & h_mcg_ctl )
         return 1;
 
-    bank_nr = bank->mc_bank;
-    if (~d->arch.vmca_msrs->mci_ctl[bank_nr] & h_mci_ctrl[bank_nr] )
-        return 1;
     return 0;
 }
 
diff -r 4f92bdf3370c xen/include/asm-x86/mce.h
--- a/xen/include/asm-x86/mce.h	Wed Jun 27 09:36:43 2012 +0200
+++ b/xen/include/asm-x86/mce.h	Fri Jul 06 10:05:46 2012 +0800
@@ -18,7 +18,6 @@
     /* Guest should not change below values after DOM boot up */
     uint64_t mcg_ctl;
     uint64_t mcg_status;
-    uint64_t *mci_ctl;
     uint16_t nr_injection;
     struct list_head impact_header;
     spinlock_t lock;

[-- Attachment #2: remove_mci_ctl.patch --]
[-- Type: application/octet-stream, Size: 3597 bytes --]

Xen/MCE: stick all 1's to MCi_CTL of vMCE

This patch is a middle-work patch, prepare for future new vMCE model.
It remove mci_ctl array, and keep MCi_CTL all 1's.

Signed-off-by: Liu, Jinsong <jinsong.liu@intel.com>

diff -r 4f92bdf3370c xen/arch/x86/cpu/mcheck/vmce.c
--- a/xen/arch/x86/cpu/mcheck/vmce.c	Wed Jun 27 09:36:43 2012 +0200
+++ b/xen/arch/x86/cpu/mcheck/vmce.c	Fri Jul 06 10:05:46 2012 +0800
@@ -25,7 +25,6 @@
 
 /* Real value in physical CTL MSR */
 static uint64_t __read_mostly h_mcg_ctl;
-static uint64_t *__read_mostly h_mci_ctrl;
 
 int vmce_init_msr(struct domain *d)
 {
@@ -33,15 +32,6 @@
     if ( !dom_vmce(d) )
         return -ENOMEM;
 
-    dom_vmce(d)->mci_ctl = xmalloc_array(uint64_t, nr_mce_banks);
-    if ( !dom_vmce(d)->mci_ctl )
-    {
-        xfree(dom_vmce(d));
-        return -ENOMEM;
-    }
-    memset(dom_vmce(d)->mci_ctl, ~0,
-           nr_mce_banks * sizeof(*dom_vmce(d)->mci_ctl));
-
     dom_vmce(d)->mcg_status = 0x0;
     dom_vmce(d)->mcg_ctl = ~(uint64_t)0x0;
     dom_vmce(d)->nr_injection = 0;
@@ -56,7 +46,6 @@
 {
     if ( !dom_vmce(d) )
         return;
-    xfree(dom_vmce(d)->mci_ctl);
     xfree(dom_vmce(d));
     dom_vmce(d) = NULL;
 }
@@ -93,9 +82,8 @@
     switch ( msr & (MSR_IA32_MC0_CTL | 3) )
     {
     case MSR_IA32_MC0_CTL:
-        if ( bank < nr_mce_banks )
-            *val = vmce->mci_ctl[bank] &
-                   (h_mci_ctrl ? h_mci_ctrl[bank] : ~0UL);
+        /* stick all 1's to MCi_CTL */
+        *val = ~0UL;
         mce_printk(MCE_VERBOSE, "MCE: rdmsr MC%u_CTL 0x%"PRIx64"\n",
                    bank, *val);
         break;
@@ -220,8 +208,10 @@
     switch ( msr & (MSR_IA32_MC0_CTL | 3) )
     {
     case MSR_IA32_MC0_CTL:
-        if ( bank < nr_mce_banks )
-            vmce->mci_ctl[bank] = val;
+        /*
+         * if guest crazy clear any bit of MCi_CTL,
+         * treat it as not implement and ignore write change it.
+         */
         break;
     case MSR_IA32_MC0_STATUS:
         if ( entry && (entry->bank == bank) )
@@ -523,22 +513,6 @@
 int vmce_init(struct cpuinfo_x86 *c)
 {
     u64 value;
-    unsigned int i;
-
-    if ( !h_mci_ctrl )
-    {
-        h_mci_ctrl = xmalloc_array(uint64_t, nr_mce_banks);
-        if (!h_mci_ctrl)
-        {
-            dprintk(XENLOG_INFO, "Failed to alloc h_mci_ctrl\n");
-            return -ENOMEM;
-        }
-        /* Don't care banks before firstbank */
-        memset(h_mci_ctrl, ~0,
-               min(firstbank, nr_mce_banks) * sizeof(*h_mci_ctrl));
-        for (i = firstbank; i < nr_mce_banks; i++)
-            rdmsrl(MSR_IA32_MCx_CTL(i), h_mci_ctrl[i]);
-    }
 
     rdmsrl(MSR_IA32_MCG_CAP, value);
     /* For Guest vMCE usage */
@@ -551,18 +525,13 @@
 
 static int mca_ctl_conflict(struct mcinfo_bank *bank, struct domain *d)
 {
-    int bank_nr;
-
-    if ( !bank || !d || !h_mci_ctrl )
+    if ( !bank || !d )
         return 1;
 
     /* Will MCE happen in host if If host mcg_ctl is 0? */
     if ( ~d->arch.vmca_msrs->mcg_ctl & h_mcg_ctl )
         return 1;
 
-    bank_nr = bank->mc_bank;
-    if (~d->arch.vmca_msrs->mci_ctl[bank_nr] & h_mci_ctrl[bank_nr] )
-        return 1;
     return 0;
 }
 
diff -r 4f92bdf3370c xen/include/asm-x86/mce.h
--- a/xen/include/asm-x86/mce.h	Wed Jun 27 09:36:43 2012 +0200
+++ b/xen/include/asm-x86/mce.h	Fri Jul 06 10:05:46 2012 +0800
@@ -18,7 +18,6 @@
     /* Guest should not change below values after DOM boot up */
     uint64_t mcg_ctl;
     uint64_t mcg_status;
-    uint64_t *mci_ctl;
     uint16_t nr_injection;
     struct list_head impact_header;
     spinlock_t lock;

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Xen/MCE: stick all 1's to MCi_CTL of vMCE
  2012-07-05 18:27 [PATCH] Xen/MCE: stick all 1's to MCi_CTL of vMCE Liu, Jinsong
@ 2012-07-06  7:50 ` Jan Beulich
  2012-07-10 14:59   ` Liu, Jinsong
  2012-07-06  8:39 ` Christoph Egger
  1 sibling, 1 reply; 4+ messages in thread
From: Jan Beulich @ 2012-07-06  7:50 UTC (permalink / raw)
  To: Jinsong Liu
  Cc: Christoph Egger, xen-devel@lists.xensource.com, Keir Fraser,
	Ian Campbell, Yunhong Jiang, Will Auld, Tony Luck

>>> On 05.07.12 at 20:27, "Liu, Jinsong" <jinsong.liu@intel.com> wrote:
> This patch just used to stick all 1's to MCi_CTL, it should not involve much 
> argue, so I sent it separately.

Yes, this one is definitely fine.

Jan

> ====================
> Xen/MCE: stick all 1's to MCi_CTL of vMCE
> 
> This patch is a middle-work patch, prepare for future new vMCE model.
> It remove mci_ctl array, and keep MCi_CTL all 1's.
> 
> Signed-off-by: Liu, Jinsong <jinsong.liu@intel.com>
> 
> diff -r 4f92bdf3370c xen/arch/x86/cpu/mcheck/vmce.c
> --- a/xen/arch/x86/cpu/mcheck/vmce.c	Wed Jun 27 09:36:43 2012 +0200
> +++ b/xen/arch/x86/cpu/mcheck/vmce.c	Fri Jul 06 10:05:46 2012 +0800
> @@ -25,7 +25,6 @@
>  
>  /* Real value in physical CTL MSR */
>  static uint64_t __read_mostly h_mcg_ctl;
> -static uint64_t *__read_mostly h_mci_ctrl;
>  
>  int vmce_init_msr(struct domain *d)
>  {
> @@ -33,15 +32,6 @@
>      if ( !dom_vmce(d) )
>          return -ENOMEM;
>  
> -    dom_vmce(d)->mci_ctl = xmalloc_array(uint64_t, nr_mce_banks);
> -    if ( !dom_vmce(d)->mci_ctl )
> -    {
> -        xfree(dom_vmce(d));
> -        return -ENOMEM;
> -    }
> -    memset(dom_vmce(d)->mci_ctl, ~0,
> -           nr_mce_banks * sizeof(*dom_vmce(d)->mci_ctl));
> -
>      dom_vmce(d)->mcg_status = 0x0;
>      dom_vmce(d)->mcg_ctl = ~(uint64_t)0x0;
>      dom_vmce(d)->nr_injection = 0;
> @@ -56,7 +46,6 @@
>  {
>      if ( !dom_vmce(d) )
>          return;
> -    xfree(dom_vmce(d)->mci_ctl);
>      xfree(dom_vmce(d));
>      dom_vmce(d) = NULL;
>  }
> @@ -93,9 +82,8 @@
>      switch ( msr & (MSR_IA32_MC0_CTL | 3) )
>      {
>      case MSR_IA32_MC0_CTL:
> -        if ( bank < nr_mce_banks )
> -            *val = vmce->mci_ctl[bank] &
> -                   (h_mci_ctrl ? h_mci_ctrl[bank] : ~0UL);
> +        /* stick all 1's to MCi_CTL */
> +        *val = ~0UL;
>          mce_printk(MCE_VERBOSE, "MCE: rdmsr MC%u_CTL 0x%"PRIx64"\n",
>                     bank, *val);
>          break;
> @@ -220,8 +208,10 @@
>      switch ( msr & (MSR_IA32_MC0_CTL | 3) )
>      {
>      case MSR_IA32_MC0_CTL:
> -        if ( bank < nr_mce_banks )
> -            vmce->mci_ctl[bank] = val;
> +        /*
> +         * if guest crazy clear any bit of MCi_CTL,
> +         * treat it as not implement and ignore write change it.
> +         */
>          break;
>      case MSR_IA32_MC0_STATUS:
>          if ( entry && (entry->bank == bank) )
> @@ -523,22 +513,6 @@
>  int vmce_init(struct cpuinfo_x86 *c)
>  {
>      u64 value;
> -    unsigned int i;
> -
> -    if ( !h_mci_ctrl )
> -    {
> -        h_mci_ctrl = xmalloc_array(uint64_t, nr_mce_banks);
> -        if (!h_mci_ctrl)
> -        {
> -            dprintk(XENLOG_INFO, "Failed to alloc h_mci_ctrl\n");
> -            return -ENOMEM;
> -        }
> -        /* Don't care banks before firstbank */
> -        memset(h_mci_ctrl, ~0,
> -               min(firstbank, nr_mce_banks) * sizeof(*h_mci_ctrl));
> -        for (i = firstbank; i < nr_mce_banks; i++)
> -            rdmsrl(MSR_IA32_MCx_CTL(i), h_mci_ctrl[i]);
> -    }
>  
>      rdmsrl(MSR_IA32_MCG_CAP, value);
>      /* For Guest vMCE usage */
> @@ -551,18 +525,13 @@
>  
>  static int mca_ctl_conflict(struct mcinfo_bank *bank, struct domain *d)
>  {
> -    int bank_nr;
> -
> -    if ( !bank || !d || !h_mci_ctrl )
> +    if ( !bank || !d )
>          return 1;
>  
>      /* Will MCE happen in host if If host mcg_ctl is 0? */
>      if ( ~d->arch.vmca_msrs->mcg_ctl & h_mcg_ctl )
>          return 1;
>  
> -    bank_nr = bank->mc_bank;
> -    if (~d->arch.vmca_msrs->mci_ctl[bank_nr] & h_mci_ctrl[bank_nr] )
> -        return 1;
>      return 0;
>  }
>  
> diff -r 4f92bdf3370c xen/include/asm-x86/mce.h
> --- a/xen/include/asm-x86/mce.h	Wed Jun 27 09:36:43 2012 +0200
> +++ b/xen/include/asm-x86/mce.h	Fri Jul 06 10:05:46 2012 +0800
> @@ -18,7 +18,6 @@
>      /* Guest should not change below values after DOM boot up */
>      uint64_t mcg_ctl;
>      uint64_t mcg_status;
> -    uint64_t *mci_ctl;
>      uint16_t nr_injection;
>      struct list_head impact_header;
>      spinlock_t lock;

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Xen/MCE: stick all 1's to MCi_CTL of vMCE
  2012-07-05 18:27 [PATCH] Xen/MCE: stick all 1's to MCi_CTL of vMCE Liu, Jinsong
  2012-07-06  7:50 ` Jan Beulich
@ 2012-07-06  8:39 ` Christoph Egger
  1 sibling, 0 replies; 4+ messages in thread
From: Christoph Egger @ 2012-07-06  8:39 UTC (permalink / raw)
  To: Liu, Jinsong
  Cc: Luck, Tony, Keir Fraser, Ian Campbell, Jiang, Yunhong, Auld, Will,
	xen-devel@lists.xensource.com, Jan Beulich

On 07/05/12 20:27, Liu, Jinsong wrote:

> Jan,
> 
> This patch just used to stick all 1's to MCi_CTL,

> it should not involve much argue, so I sent it separately.

Ok from my side.

Christoph


> Thanks,
> Jinsong
> 
> ====================
> Xen/MCE: stick all 1's to MCi_CTL of vMCE
> 
> This patch is a middle-work patch, prepare for future new vMCE model.
> It remove mci_ctl array, and keep MCi_CTL all 1's.
> 
> Signed-off-by: Liu, Jinsong <jinsong.liu@intel.com>
> 
> diff -r 4f92bdf3370c xen/arch/x86/cpu/mcheck/vmce.c
> --- a/xen/arch/x86/cpu/mcheck/vmce.c	Wed Jun 27 09:36:43 2012 +0200
> +++ b/xen/arch/x86/cpu/mcheck/vmce.c	Fri Jul 06 10:05:46 2012 +0800
> @@ -25,7 +25,6 @@
>  
>  /* Real value in physical CTL MSR */
>  static uint64_t __read_mostly h_mcg_ctl;
> -static uint64_t *__read_mostly h_mci_ctrl;
>  
>  int vmce_init_msr(struct domain *d)
>  {
> @@ -33,15 +32,6 @@
>      if ( !dom_vmce(d) )
>          return -ENOMEM;
>  
> -    dom_vmce(d)->mci_ctl = xmalloc_array(uint64_t, nr_mce_banks);
> -    if ( !dom_vmce(d)->mci_ctl )
> -    {
> -        xfree(dom_vmce(d));
> -        return -ENOMEM;
> -    }
> -    memset(dom_vmce(d)->mci_ctl, ~0,
> -           nr_mce_banks * sizeof(*dom_vmce(d)->mci_ctl));
> -
>      dom_vmce(d)->mcg_status = 0x0;
>      dom_vmce(d)->mcg_ctl = ~(uint64_t)0x0;
>      dom_vmce(d)->nr_injection = 0;
> @@ -56,7 +46,6 @@
>  {
>      if ( !dom_vmce(d) )
>          return;
> -    xfree(dom_vmce(d)->mci_ctl);
>      xfree(dom_vmce(d));
>      dom_vmce(d) = NULL;
>  }
> @@ -93,9 +82,8 @@
>      switch ( msr & (MSR_IA32_MC0_CTL | 3) )
>      {
>      case MSR_IA32_MC0_CTL:
> -        if ( bank < nr_mce_banks )
> -            *val = vmce->mci_ctl[bank] &
> -                   (h_mci_ctrl ? h_mci_ctrl[bank] : ~0UL);
> +        /* stick all 1's to MCi_CTL */
> +        *val = ~0UL;
>          mce_printk(MCE_VERBOSE, "MCE: rdmsr MC%u_CTL 0x%"PRIx64"\n",
>                     bank, *val);
>          break;
> @@ -220,8 +208,10 @@
>      switch ( msr & (MSR_IA32_MC0_CTL | 3) )
>      {
>      case MSR_IA32_MC0_CTL:
> -        if ( bank < nr_mce_banks )
> -            vmce->mci_ctl[bank] = val;
> +        /*
> +         * if guest crazy clear any bit of MCi_CTL,
> +         * treat it as not implement and ignore write change it.
> +         */
>          break;
>      case MSR_IA32_MC0_STATUS:
>          if ( entry && (entry->bank == bank) )
> @@ -523,22 +513,6 @@
>  int vmce_init(struct cpuinfo_x86 *c)
>  {
>      u64 value;
> -    unsigned int i;
> -
> -    if ( !h_mci_ctrl )
> -    {
> -        h_mci_ctrl = xmalloc_array(uint64_t, nr_mce_banks);
> -        if (!h_mci_ctrl)
> -        {
> -            dprintk(XENLOG_INFO, "Failed to alloc h_mci_ctrl\n");
> -            return -ENOMEM;
> -        }
> -        /* Don't care banks before firstbank */
> -        memset(h_mci_ctrl, ~0,
> -               min(firstbank, nr_mce_banks) * sizeof(*h_mci_ctrl));
> -        for (i = firstbank; i < nr_mce_banks; i++)
> -            rdmsrl(MSR_IA32_MCx_CTL(i), h_mci_ctrl[i]);
> -    }
>  
>      rdmsrl(MSR_IA32_MCG_CAP, value);
>      /* For Guest vMCE usage */
> @@ -551,18 +525,13 @@
>  
>  static int mca_ctl_conflict(struct mcinfo_bank *bank, struct domain *d)
>  {
> -    int bank_nr;
> -
> -    if ( !bank || !d || !h_mci_ctrl )
> +    if ( !bank || !d )
>          return 1;
>  
>      /* Will MCE happen in host if If host mcg_ctl is 0? */
>      if ( ~d->arch.vmca_msrs->mcg_ctl & h_mcg_ctl )
>          return 1;
>  
> -    bank_nr = bank->mc_bank;
> -    if (~d->arch.vmca_msrs->mci_ctl[bank_nr] & h_mci_ctrl[bank_nr] )
> -        return 1;
>      return 0;
>  }
>  
> diff -r 4f92bdf3370c xen/include/asm-x86/mce.h
> --- a/xen/include/asm-x86/mce.h	Wed Jun 27 09:36:43 2012 +0200
> +++ b/xen/include/asm-x86/mce.h	Fri Jul 06 10:05:46 2012 +0800
> @@ -18,7 +18,6 @@
>      /* Guest should not change below values after DOM boot up */
>      uint64_t mcg_ctl;
>      uint64_t mcg_status;
> -    uint64_t *mci_ctl;
>      uint16_t nr_injection;
>      struct list_head impact_header;
>      spinlock_t lock;



-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Xen/MCE: stick all 1's to MCi_CTL of vMCE
  2012-07-06  7:50 ` Jan Beulich
@ 2012-07-10 14:59   ` Liu, Jinsong
  0 siblings, 0 replies; 4+ messages in thread
From: Liu, Jinsong @ 2012-07-10 14:59 UTC (permalink / raw)
  To: Keir (Xen.org), Ian Campbell; +Cc: xen-devel@lists.xensource.com, Jan Beulich

[-- Attachment #1: Type: text/plain, Size: 4749 bytes --]

Keir, Ian

As Jan take leave recently, and Xen 4.2 would release, so would you
please add this patch to Xen upstream and merged it into Xen4.2?

This patch just change a little to current vMCE (remove mci_ctl, and
stick all 1's to MCi_CTL). With it, Xen 4.2 would have no semantics
difference with future new vMCE. This patch is an uncontroversial
patch agreed by Jan.

Thanks,
Jinsong


Jan Beulich wrote:
>>>> On 05.07.12 at 20:27, "Liu, Jinsong" <jinsong.liu@intel.com> wrote:
>> This patch just used to stick all 1's to MCi_CTL, it should not
>> involve much argue, so I sent it separately.
> 
> Yes, this one is definitely fine.
> 
> Jan
> 
>> ====================
>> Xen/MCE: stick all 1's to MCi_CTL of vMCE
>> 
>> This patch is a middle-work patch, prepare for future new vMCE model.
>> It remove mci_ctl array, and keep MCi_CTL all 1's.
>> 
>> Signed-off-by: Liu, Jinsong <jinsong.liu@intel.com>
>> 
>> diff -r 4f92bdf3370c xen/arch/x86/cpu/mcheck/vmce.c
>> --- a/xen/arch/x86/cpu/mcheck/vmce.c	Wed Jun 27 09:36:43 2012 +0200
>> +++ b/xen/arch/x86/cpu/mcheck/vmce.c	Fri Jul 06 10:05:46 2012 +0800
>> @@ -25,7 +25,6 @@ 
>> 
>>  /* Real value in physical CTL MSR */
>>  static uint64_t __read_mostly h_mcg_ctl;
>> -static uint64_t *__read_mostly h_mci_ctrl;
>> 
>>  int vmce_init_msr(struct domain *d)
>>  {
>> @@ -33,15 +32,6 @@
>>      if ( !dom_vmce(d) )
>>          return -ENOMEM;
>> 
>> -    dom_vmce(d)->mci_ctl = xmalloc_array(uint64_t, nr_mce_banks);
>> -    if ( !dom_vmce(d)->mci_ctl )
>> -    {
>> -        xfree(dom_vmce(d));
>> -        return -ENOMEM;
>> -    }
>> -    memset(dom_vmce(d)->mci_ctl, ~0,
>> -           nr_mce_banks * sizeof(*dom_vmce(d)->mci_ctl)); -
>>      dom_vmce(d)->mcg_status = 0x0;
>>      dom_vmce(d)->mcg_ctl = ~(uint64_t)0x0;
>>      dom_vmce(d)->nr_injection = 0;
>> @@ -56,7 +46,6 @@
>>  {
>>      if ( !dom_vmce(d) )
>>          return;
>> -    xfree(dom_vmce(d)->mci_ctl);
>>      xfree(dom_vmce(d));
>>      dom_vmce(d) = NULL;
>>  }
>> @@ -93,9 +82,8 @@
>>      switch ( msr & (MSR_IA32_MC0_CTL | 3) )
>>      {
>>      case MSR_IA32_MC0_CTL:
>> -        if ( bank < nr_mce_banks )
>> -            *val = vmce->mci_ctl[bank] &
>> -                   (h_mci_ctrl ? h_mci_ctrl[bank] : ~0UL);
>> +        /* stick all 1's to MCi_CTL */
>> +        *val = ~0UL;
>>          mce_printk(MCE_VERBOSE, "MCE: rdmsr MC%u_CTL 0x%"PRIx64"\n",
>>                     bank, *val);
>>          break;
>> @@ -220,8 +208,10 @@
>>      switch ( msr & (MSR_IA32_MC0_CTL | 3) )
>>      {
>>      case MSR_IA32_MC0_CTL:
>> -        if ( bank < nr_mce_banks )
>> -            vmce->mci_ctl[bank] = val;
>> +        /*
>> +         * if guest crazy clear any bit of MCi_CTL,
>> +         * treat it as not implement and ignore write change it. + 
>>          */ break;
>>      case MSR_IA32_MC0_STATUS:
>>          if ( entry && (entry->bank == bank) )
>> @@ -523,22 +513,6 @@
>>  int vmce_init(struct cpuinfo_x86 *c)
>>  {
>>      u64 value;
>> -    unsigned int i;
>> -
>> -    if ( !h_mci_ctrl )
>> -    {
>> -        h_mci_ctrl = xmalloc_array(uint64_t, nr_mce_banks);
>> -        if (!h_mci_ctrl)
>> -        {
>> -            dprintk(XENLOG_INFO, "Failed to alloc h_mci_ctrl\n");
>> -            return -ENOMEM;
>> -        }
>> -        /* Don't care banks before firstbank */
>> -        memset(h_mci_ctrl, ~0,
>> -               min(firstbank, nr_mce_banks) * sizeof(*h_mci_ctrl));
>> -        for (i = firstbank; i < nr_mce_banks; i++)
>> -            rdmsrl(MSR_IA32_MCx_CTL(i), h_mci_ctrl[i]);
>> -    }
>> 
>>      rdmsrl(MSR_IA32_MCG_CAP, value);
>>      /* For Guest vMCE usage */
>> @@ -551,18 +525,13 @@
>> 
>>  static int mca_ctl_conflict(struct mcinfo_bank *bank, struct domain
>> *d)  { 
>> -    int bank_nr;
>> -
>> -    if ( !bank || !d || !h_mci_ctrl )
>> +    if ( !bank || !d )
>>          return 1;
>> 
>>      /* Will MCE happen in host if If host mcg_ctl is 0? */
>>      if ( ~d->arch.vmca_msrs->mcg_ctl & h_mcg_ctl )          return
>> 1; 
>> 
>> -    bank_nr = bank->mc_bank;
>> -    if (~d->arch.vmca_msrs->mci_ctl[bank_nr] & h_mci_ctrl[bank_nr] )
>> -        return 1;
>>      return 0;
>>  }
>> 
>> diff -r 4f92bdf3370c xen/include/asm-x86/mce.h
>> --- a/xen/include/asm-x86/mce.h	Wed Jun 27 09:36:43 2012 +0200
>> +++ b/xen/include/asm-x86/mce.h	Fri Jul 06 10:05:46 2012 +0800 @@
>>      -18,7 +18,6 @@ /* Guest should not change below values after
>>      DOM boot up */      uint64_t mcg_ctl; uint64_t mcg_status;
>> -    uint64_t *mci_ctl;
>>      uint16_t nr_injection;
>>      struct list_head impact_header;
>>      spinlock_t lock;


[-- Attachment #2: remove_mci_ctl_1.patch --]
[-- Type: application/octet-stream, Size: 3597 bytes --]

Xen/MCE: stick all 1's to MCi_CTL of vMCE

This patch is a middle-work patch, prepare for future new vMCE model.
It remove mci_ctl array, and keep MCi_CTL all 1's.

Signed-off-by: Liu, Jinsong <jinsong.liu@intel.com>

diff -r 4f92bdf3370c xen/arch/x86/cpu/mcheck/vmce.c
--- a/xen/arch/x86/cpu/mcheck/vmce.c	Wed Jun 27 09:36:43 2012 +0200
+++ b/xen/arch/x86/cpu/mcheck/vmce.c	Fri Jul 06 10:05:46 2012 +0800
@@ -25,7 +25,6 @@
 
 /* Real value in physical CTL MSR */
 static uint64_t __read_mostly h_mcg_ctl;
-static uint64_t *__read_mostly h_mci_ctrl;
 
 int vmce_init_msr(struct domain *d)
 {
@@ -33,15 +32,6 @@
     if ( !dom_vmce(d) )
         return -ENOMEM;
 
-    dom_vmce(d)->mci_ctl = xmalloc_array(uint64_t, nr_mce_banks);
-    if ( !dom_vmce(d)->mci_ctl )
-    {
-        xfree(dom_vmce(d));
-        return -ENOMEM;
-    }
-    memset(dom_vmce(d)->mci_ctl, ~0,
-           nr_mce_banks * sizeof(*dom_vmce(d)->mci_ctl));
-
     dom_vmce(d)->mcg_status = 0x0;
     dom_vmce(d)->mcg_ctl = ~(uint64_t)0x0;
     dom_vmce(d)->nr_injection = 0;
@@ -56,7 +46,6 @@
 {
     if ( !dom_vmce(d) )
         return;
-    xfree(dom_vmce(d)->mci_ctl);
     xfree(dom_vmce(d));
     dom_vmce(d) = NULL;
 }
@@ -93,9 +82,8 @@
     switch ( msr & (MSR_IA32_MC0_CTL | 3) )
     {
     case MSR_IA32_MC0_CTL:
-        if ( bank < nr_mce_banks )
-            *val = vmce->mci_ctl[bank] &
-                   (h_mci_ctrl ? h_mci_ctrl[bank] : ~0UL);
+        /* stick all 1's to MCi_CTL */
+        *val = ~0UL;
         mce_printk(MCE_VERBOSE, "MCE: rdmsr MC%u_CTL 0x%"PRIx64"\n",
                    bank, *val);
         break;
@@ -220,8 +208,10 @@
     switch ( msr & (MSR_IA32_MC0_CTL | 3) )
     {
     case MSR_IA32_MC0_CTL:
-        if ( bank < nr_mce_banks )
-            vmce->mci_ctl[bank] = val;
+        /*
+         * if guest crazy clear any bit of MCi_CTL,
+         * treat it as not implement and ignore write change it.
+         */
         break;
     case MSR_IA32_MC0_STATUS:
         if ( entry && (entry->bank == bank) )
@@ -523,22 +513,6 @@
 int vmce_init(struct cpuinfo_x86 *c)
 {
     u64 value;
-    unsigned int i;
-
-    if ( !h_mci_ctrl )
-    {
-        h_mci_ctrl = xmalloc_array(uint64_t, nr_mce_banks);
-        if (!h_mci_ctrl)
-        {
-            dprintk(XENLOG_INFO, "Failed to alloc h_mci_ctrl\n");
-            return -ENOMEM;
-        }
-        /* Don't care banks before firstbank */
-        memset(h_mci_ctrl, ~0,
-               min(firstbank, nr_mce_banks) * sizeof(*h_mci_ctrl));
-        for (i = firstbank; i < nr_mce_banks; i++)
-            rdmsrl(MSR_IA32_MCx_CTL(i), h_mci_ctrl[i]);
-    }
 
     rdmsrl(MSR_IA32_MCG_CAP, value);
     /* For Guest vMCE usage */
@@ -551,18 +525,13 @@
 
 static int mca_ctl_conflict(struct mcinfo_bank *bank, struct domain *d)
 {
-    int bank_nr;
-
-    if ( !bank || !d || !h_mci_ctrl )
+    if ( !bank || !d )
         return 1;
 
     /* Will MCE happen in host if If host mcg_ctl is 0? */
     if ( ~d->arch.vmca_msrs->mcg_ctl & h_mcg_ctl )
         return 1;
 
-    bank_nr = bank->mc_bank;
-    if (~d->arch.vmca_msrs->mci_ctl[bank_nr] & h_mci_ctrl[bank_nr] )
-        return 1;
     return 0;
 }
 
diff -r 4f92bdf3370c xen/include/asm-x86/mce.h
--- a/xen/include/asm-x86/mce.h	Wed Jun 27 09:36:43 2012 +0200
+++ b/xen/include/asm-x86/mce.h	Fri Jul 06 10:05:46 2012 +0800
@@ -18,7 +18,6 @@
     /* Guest should not change below values after DOM boot up */
     uint64_t mcg_ctl;
     uint64_t mcg_status;
-    uint64_t *mci_ctl;
     uint16_t nr_injection;
     struct list_head impact_header;
     spinlock_t lock;

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2012-07-10 14:59 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-07-05 18:27 [PATCH] Xen/MCE: stick all 1's to MCi_CTL of vMCE Liu, Jinsong
2012-07-06  7:50 ` Jan Beulich
2012-07-10 14:59   ` Liu, Jinsong
2012-07-06  8:39 ` Christoph Egger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).