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;
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
>>> 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;
Christoph Egger
2012-Jul-06 08:39 UTC
Re: [PATCH] Xen/MCE: stick all 1''s to MCi_CTL of vMCE
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
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;_______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel