Jiang, Yunhong
2010-Jan-28 05:55 UTC
[Xen-devel] [PATCH 4/6] MCE: Fix the vMCE address translation for HVM guest.
Fix the vMCE address translation for HVM guest.
Fix address translation when we inject a virtual MCE to HVM guest.
Signed-off-by: Jiang, Yunhong <yunhong.jiang@intel.com>
diff -r ac2951705a86 xen/arch/x86/cpu/mcheck/mce_intel.c
--- a/xen/arch/x86/cpu/mcheck/mce_intel.c Tue Jan 26 20:04:51 2010 +0800
+++ b/xen/arch/x86/cpu/mcheck/mce_intel.c Tue Jan 26 22:49:11 2010 +0800
@@ -371,8 +371,8 @@ static void intel_UCR_handler(struct mci
gfn mfn_to_gmfn(d,
((bank->mc_addr) >> PAGE_SHIFT));
- bank->mc_addr - gfn
<< PAGE_SHIFT | (bank->mc_addr & PAGE_MASK);
+ bank->mc_addr = gfn << PAGE_SHIFT |
+ (bank->mc_addr & (PAGE_SIZE -1
));
if (fill_vmsr_data(bank, global->mc_gstatus) ==
-1)
{
mce_printk(MCE_QUIET, "Fill vMCE# data for
DOM%d "
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Christoph Egger
2010-Jan-28 08:14 UTC
[Xen-devel] Re: [PATCH 4/6] MCE: Fix the vMCE address translation for HVM guest.
On Thursday 28 January 2010 06:55:58 Jiang, Yunhong wrote:> Fix the vMCE address translation for HVM guest. > > Fix address translation when we inject a virtual MCE to HVM guest.IMO, the whole address translation should be x86 generic with hooks to handle AMD/Intel specific registers. Christoph> > Signed-off-by: Jiang, Yunhong <yunhong.jiang@intel.com> > > diff -r ac2951705a86 xen/arch/x86/cpu/mcheck/mce_intel.c > --- a/xen/arch/x86/cpu/mcheck/mce_intel.c Tue Jan 26 20:04:51 2010 +0800 > +++ b/xen/arch/x86/cpu/mcheck/mce_intel.c Tue Jan 26 22:49:11 2010 +0800 > @@ -371,8 +371,8 @@ static void intel_UCR_handler(struct mci > > gfn > mfn_to_gmfn(d, ((bank->mc_addr) >> > PAGE_SHIFT)); - bank->mc_addr > - gfn << PAGE_SHIFT | (bank->mc_addr & > PAGE_MASK); + bank->mc_addr = gfn << PAGE_SHIFT | > + (bank->mc_addr & (PAGE_SIZE -1 )); > if (fill_vmsr_data(bank, global->mc_gstatus) => -1) { > mce_printk(MCE_QUIET, "Fill vMCE# data for > DOM%d "-- ---to satisfy European Law for business letters: Advanced Micro Devices GmbH Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen Geschaeftsfuehrer: Andrew Bowd, Thomas M. McCoy, Giuliano Meroni Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jiang, Yunhong
2010-Jan-28 09:44 UTC
[Xen-devel] RE: [PATCH 4/6] MCE: Fix the vMCE address translation for HVM guest.
>-----Original Message----- >From: Christoph Egger [mailto:Christoph.Egger@amd.com] >Sent: Thursday, January 28, 2010 4:14 PM >To: Jiang, Yunhong >Cc: Keir Fraser; Frank.Vanderlinden@Sun.COM; Jan Beulich; >xen-devel@lists.xensource.com >Subject: Re: [PATCH 4/6] MCE: Fix the vMCE address translation for HVM guest. > >On Thursday 28 January 2010 06:55:58 Jiang, Yunhong wrote: >> Fix the vMCE address translation for HVM guest. >> >> Fix address translation when we inject a virtual MCE to HVM guest. > >IMO, the whole address translation should be x86 generic with hooks >to handle AMD/Intel specific registers.Agree. I think Frank/Jan has done a lot of work to merge the MCE staff. And still something left to merge the MCE handler (Frank triedto merge the handler before, I remember) . With the broadcast checking patch, it will be easier to merge. I have a patch on-hand that try to do some clean-up and prepare for merge. The main change is to change the mcheck_mca_logout, to pass-in a ops pointer and data pointer, as code below. --jyh + +struct mca_ops { + mctelem_class_t queue; + /* return 0 if no extended information created */ + int (*global_extended)(struct mcinfo_extended *gext, + struct mcinfo_global *mcg, void *data); + int (*parse_global)(struct mcinfo_global *mcg, + struct mcinfo_extended *gext, void *data); + int (*bank_extended)(struct mcinfo_extended *bext, struct mcinfo_back *mcb, + void *data); + /* Return 1 if owned error happens */ + int (*parse_bank)(struct mcinfo_bank *mcb, + struct mcinfo_global *mcg, + struct mcinfo_extended *gext, + struct mcinfo_extended *bext, + void *data); + int (*clear_bank)(struct mcinfo_bank *mcb, + struct mcinfo_global *mcg, + struct mcinfo_extended *gext, + struct mcinfo_extended *bext, + void *data); }; .... -mctelem_cookie_t mcheck_mca_logout(enum mca_source who, cpu_banks_t bankmask, - struct mca_summary *sp, cpu_banks_t* clear_bank) -{ - struct vcpu *v = current; - struct domain *d; - uint64_t gstatus, status, addr, misc; +mctelem_cookie_t mcheck_mca_logout(cpu_banks_t bankmask, struct mca_ops *handler, void *data) +{> >Christoph > >> >> Signed-off-by: Jiang, Yunhong <yunhong.jiang@intel.com> >> >> diff -r ac2951705a86 xen/arch/x86/cpu/mcheck/mce_intel.c >> --- a/xen/arch/x86/cpu/mcheck/mce_intel.c Tue Jan 26 20:04:51 2010 +0800 >> +++ b/xen/arch/x86/cpu/mcheck/mce_intel.c Tue Jan 26 22:49:11 2010 +0800 >> @@ -371,8 +371,8 @@ static void intel_UCR_handler(struct mci >> >> gfn >> mfn_to_gmfn(d, ((bank->mc_addr) >> >> PAGE_SHIFT)); - bank->mc_addr >> - gfn << PAGE_SHIFT | (bank->mc_addr & >> PAGE_MASK); + bank->mc_addr = gfn << >PAGE_SHIFT | >> + (bank->mc_addr & (PAGE_SIZE >-1 )); >> if (fill_vmsr_data(bank, global->mc_gstatus) =>> -1) { >> mce_printk(MCE_QUIET, "Fill vMCE# data >for >> DOM%d " > > > >-- >---to satisfy European Law for business letters: >Advanced Micro Devices GmbH >Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen >Geschaeftsfuehrer: Andrew Bowd, Thomas M. McCoy, Giuliano Meroni >Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen >Registergericht Muenchen, HRB Nr. 43632_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Christoph Egger
2010-Jan-28 16:31 UTC
[Xen-devel] Re: [PATCH 4/6] MCE: Fix the vMCE address translation for HVM guest.
On Thursday 28 January 2010 10:44:28 Jiang, Yunhong wrote:> >-----Original Message----- > >From: Christoph Egger [mailto:Christoph.Egger@amd.com] > >Sent: Thursday, January 28, 2010 4:14 PM > >To: Jiang, Yunhong > >Cc: Keir Fraser; Frank.Vanderlinden@Sun.COM; Jan Beulich; > >xen-devel@lists.xensource.com > >Subject: Re: [PATCH 4/6] MCE: Fix the vMCE address translation for HVM > > guest. > > > >On Thursday 28 January 2010 06:55:58 Jiang, Yunhong wrote: > >> Fix the vMCE address translation for HVM guest. > >> > >> Fix address translation when we inject a virtual MCE to HVM guest. > > > >IMO, the whole address translation should be x86 generic with hooks > >to handle AMD/Intel specific registers. > > Agree. > I think Frank/Jan has done a lot of work to merge the MCE staff. And still > something left to merge the MCE handler (Frank triedto merge the handler > before, I remember) . With the broadcast checking patch, it will be easier > to merge. > > I have a patch on-hand that try to do some clean-up and prepare for merge. > The main change is to change the mcheck_mca_logout, to pass-in a ops > pointer and data pointer, as code below. > > --jyh > > + > +struct mca_ops { > + mctelem_class_t queue; > + /* return 0 if no extended information created */ > + int (*global_extended)(struct mcinfo_extended *gext, > + struct mcinfo_global *mcg, void *data); > + int (*parse_global)(struct mcinfo_global *mcg, > + struct mcinfo_extended *gext, void *data); > + int (*bank_extended)(struct mcinfo_extended *bext, struct mcinfo_back > *mcb, + void *data); > + /* Return 1 if owned error happens */ > + int (*parse_bank)(struct mcinfo_bank *mcb, > + struct mcinfo_global *mcg, > + struct mcinfo_extended *gext, > + struct mcinfo_extended *bext, > + void *data); > + int (*clear_bank)(struct mcinfo_bank *mcb, > + struct mcinfo_global *mcg, > + struct mcinfo_extended *gext, > + struct mcinfo_extended *bext, > + void *data); > };IMO, a single struct mc_info argument for each hook does its job. This is more flexible and extensible. Christoph> .... > > -mctelem_cookie_t mcheck_mca_logout(enum mca_source who, cpu_banks_t > bankmask, - struct mca_summary *sp, cpu_banks_t* clear_bank) > -{ > - struct vcpu *v = current; > - struct domain *d; > - uint64_t gstatus, status, addr, misc; > +mctelem_cookie_t mcheck_mca_logout(cpu_banks_t bankmask, struct mca_ops > *handler, void *data) +{ > > >Christoph > > > >> Signed-off-by: Jiang, Yunhong <yunhong.jiang@intel.com> > >> > >> diff -r ac2951705a86 xen/arch/x86/cpu/mcheck/mce_intel.c > >> --- a/xen/arch/x86/cpu/mcheck/mce_intel.c Tue Jan 26 20:04:51 2010 +0800 > >> +++ b/xen/arch/x86/cpu/mcheck/mce_intel.c Tue Jan 26 22:49:11 2010 +0800 > >> @@ -371,8 +371,8 @@ static void intel_UCR_handler(struct mci > >> > >> gfn > >> mfn_to_gmfn(d, ((bank->mc_addr) >> > >> PAGE_SHIFT)); - bank->mc_addr > >> - gfn << PAGE_SHIFT | (bank->mc_addr & > >> PAGE_MASK); + bank->mc_addr = gfn << > > > >PAGE_SHIFT | > > > >> + (bank->mc_addr & (PAGE_SIZE > > > >-1 )); > > > >> if (fill_vmsr_data(bank, global->mc_gstatus) > >> == -1) { > >> mce_printk(MCE_QUIET, "Fill vMCE# data > > > >for > > > >> DOM%d " > > > >-- > >---to satisfy European Law for business letters: > >Advanced Micro Devices GmbH > >Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen > >Geschaeftsfuehrer: Andrew Bowd, Thomas M. McCoy, Giuliano Meroni > >Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen > >Registergericht Muenchen, HRB Nr. 43632-- ---to satisfy European Law for business letters: Advanced Micro Devices GmbH Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen Geschaeftsfuehrer: Andrew Bowd, Thomas M. McCoy, Giuliano Meroni Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jiang, Yunhong
2010-Jan-29 05:55 UTC
[Xen-devel] RE: [PATCH 4/6] MCE: Fix the vMCE address translation for HVM guest.
>-----Original Message----- >From: Christoph Egger [mailto:Christoph.Egger@amd.com] >Sent: Friday, January 29, 2010 12:31 AM >To: Jiang, Yunhong >Cc: Keir Fraser; Frank.Vanderlinden@Sun.COM; Jan Beulich; >xen-devel@lists.xensource.com >Subject: Re: [PATCH 4/6] MCE: Fix the vMCE address translation for HVM guest. > >On Thursday 28 January 2010 10:44:28 Jiang, Yunhong wrote: >> >-----Original Message----- >> >From: Christoph Egger [mailto:Christoph.Egger@amd.com] >> >Sent: Thursday, January 28, 2010 4:14 PM >> >To: Jiang, Yunhong >> >Cc: Keir Fraser; Frank.Vanderlinden@Sun.COM; Jan Beulich; >> >xen-devel@lists.xensource.com >> >Subject: Re: [PATCH 4/6] MCE: Fix the vMCE address translation for HVM >> > guest. >> > >> >On Thursday 28 January 2010 06:55:58 Jiang, Yunhong wrote: >> >> Fix the vMCE address translation for HVM guest. >> >> >> >> Fix address translation when we inject a virtual MCE to HVM guest. >> > >> >IMO, the whole address translation should be x86 generic with hooks >> >to handle AMD/Intel specific registers. >> >> Agree. >> I think Frank/Jan has done a lot of work to merge the MCE staff. And still >> something left to merge the MCE handler (Frank triedto merge the handler >> before, I remember) . With the broadcast checking patch, it will be easier >> to merge. >> >> I have a patch on-hand that try to do some clean-up and prepare for merge. >> The main change is to change the mcheck_mca_logout, to pass-in a ops >> pointer and data pointer, as code below. >> >> --jyh >> >> + >> +struct mca_ops { >> + mctelem_class_t queue; >> + /* return 0 if no extended information created */ >> + int (*global_extended)(struct mcinfo_extended *gext, >> + struct mcinfo_global *mcg, void *data); >> + int (*parse_global)(struct mcinfo_global *mcg, >> + struct mcinfo_extended *gext, void *data); >> + int (*bank_extended)(struct mcinfo_extended *bext, struct mcinfo_back >> *mcb, + void *data); >> + /* Return 1 if owned error happens */ >> + int (*parse_bank)(struct mcinfo_bank *mcb, >> + struct mcinfo_global *mcg, >> + struct mcinfo_extended *gext, >> + struct mcinfo_extended *bext, >> + void *data); >> + int (*clear_bank)(struct mcinfo_bank *mcb, >> + struct mcinfo_global *mcg, >> + struct mcinfo_extended *gext, >> + struct mcinfo_extended *bext, >> + void *data); >> }; > >IMO, a single struct mc_info argument for each hook does its job. >This is more flexible and extensible. > >ChristophFor global hook, it is ok to pass the mc_info, for the per_bank hook, we should pass the bank, to avoid search each time. I will send out the patch when it is ready, and we can have more discussion at that time. --jyh> > >> .... >> >> -mctelem_cookie_t mcheck_mca_logout(enum mca_source who, cpu_banks_t >> bankmask, - struct mca_summary *sp, cpu_banks_t* clear_bank) >> -{ >> - struct vcpu *v = current; >> - struct domain *d; >> - uint64_t gstatus, status, addr, misc; >> +mctelem_cookie_t mcheck_mca_logout(cpu_banks_t bankmask, struct mca_ops >> *handler, void *data) +{ >> >> >Christoph >> > >> >> Signed-off-by: Jiang, Yunhong <yunhong.jiang@intel.com> >> >> >> >> diff -r ac2951705a86 xen/arch/x86/cpu/mcheck/mce_intel.c >> >> --- a/xen/arch/x86/cpu/mcheck/mce_intel.c Tue Jan 26 20:04:51 2010 +0800 >> >> +++ b/xen/arch/x86/cpu/mcheck/mce_intel.c Tue Jan 26 22:49:11 2010 >+0800 >> >> @@ -371,8 +371,8 @@ static void intel_UCR_handler(struct mci >> >> >> >> gfn >> >> mfn_to_gmfn(d, ((bank->mc_addr) >> >> >> PAGE_SHIFT)); - bank->mc_addr >> >> - gfn << PAGE_SHIFT | (bank->mc_addr & >> >> PAGE_MASK); + bank->mc_addr = gfn << >> > >> >PAGE_SHIFT | >> > >> >> + (bank->mc_addr & >(PAGE_SIZE >> > >> >-1 )); >> > >> >> if (fill_vmsr_data(bank, global->mc_gstatus) >> >> == -1) { >> >> mce_printk(MCE_QUIET, "Fill vMCE# >data >> > >> >for >> > >> >> DOM%d " >> > >> >-- >> >---to satisfy European Law for business letters: >> >Advanced Micro Devices GmbH >> >Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen >> >Geschaeftsfuehrer: Andrew Bowd, Thomas M. McCoy, Giuliano Meroni >> >Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen >> >Registergericht Muenchen, HRB Nr. 43632 > > > >-- >---to satisfy European Law for business letters: >Advanced Micro Devices GmbH >Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen >Geschaeftsfuehrer: Andrew Bowd, Thomas M. McCoy, Giuliano Meroni >Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen >Registergericht Muenchen, HRB Nr. 43632_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Christoph Egger
2010-Jan-29 07:44 UTC
[Xen-devel] Re: [PATCH 4/6] MCE: Fix the vMCE address translation for HVM guest.
On Friday 29 January 2010 06:55:49 Jiang, Yunhong wrote:> >-----Original Message----- > >From: Christoph Egger [mailto:Christoph.Egger@amd.com] > >Sent: Friday, January 29, 2010 12:31 AM > >To: Jiang, Yunhong > >Cc: Keir Fraser; Frank.Vanderlinden@Sun.COM; Jan Beulich; > >xen-devel@lists.xensource.com > >Subject: Re: [PATCH 4/6] MCE: Fix the vMCE address translation for HVM > > guest. > > > >On Thursday 28 January 2010 10:44:28 Jiang, Yunhong wrote: > >> >-----Original Message----- > >> >From: Christoph Egger [mailto:Christoph.Egger@amd.com] > >> >Sent: Thursday, January 28, 2010 4:14 PM > >> >To: Jiang, Yunhong > >> >Cc: Keir Fraser; Frank.Vanderlinden@Sun.COM; Jan Beulich; > >> >xen-devel@lists.xensource.com > >> >Subject: Re: [PATCH 4/6] MCE: Fix the vMCE address translation for HVM > >> > guest. > >> > > >> >On Thursday 28 January 2010 06:55:58 Jiang, Yunhong wrote: > >> >> Fix the vMCE address translation for HVM guest. > >> >> > >> >> Fix address translation when we inject a virtual MCE to HVM guest. > >> > > >> >IMO, the whole address translation should be x86 generic with hooks > >> >to handle AMD/Intel specific registers. > >> > >> Agree. > >> I think Frank/Jan has done a lot of work to merge the MCE staff. And > >> still something left to merge the MCE handler (Frank triedto merge the > >> handler before, I remember) . With the broadcast checking patch, it will > >> be easier to merge. > >> > >> I have a patch on-hand that try to do some clean-up and prepare for > >> merge. The main change is to change the mcheck_mca_logout, to pass-in a > >> ops pointer and data pointer, as code below. > >> > >> --jyh > >> > >> + > >> +struct mca_ops { > >> + mctelem_class_t queue; > >> + /* return 0 if no extended information created */ > >> + int (*global_extended)(struct mcinfo_extended *gext, > >> + struct mcinfo_global *mcg, void *data); > >> + int (*parse_global)(struct mcinfo_global *mcg, > >> + struct mcinfo_extended *gext, void *data); > >> + int (*bank_extended)(struct mcinfo_extended *bext, struct > >> mcinfo_back *mcb, + void *data); > >> + /* Return 1 if owned error happens */ > >> + int (*parse_bank)(struct mcinfo_bank *mcb, > >> + struct mcinfo_global *mcg, > >> + struct mcinfo_extended *gext, > >> + struct mcinfo_extended *bext, > >> + void *data); > >> + int (*clear_bank)(struct mcinfo_bank *mcb, > >> + struct mcinfo_global *mcg, > >> + struct mcinfo_extended *gext, > >> + struct mcinfo_extended *bext, > >> + void *data); > >> }; > > > >IMO, a single struct mc_info argument for each hook does its job. > >This is more flexible and extensible. > > > >Christoph > > For global hook, it is ok to pass the mc_info, for the per_bank hook, we > should pass the bank, to avoid search each time. I will send out the patch > when it is ready, and we can have more discussion at that time. >This is reasonable. I agree on that. Christoph -- ---to satisfy European Law for business letters: Advanced Micro Devices GmbH Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen Geschaeftsfuehrer: Andrew Bowd, Thomas M. McCoy, Giuliano Meroni Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel