Re: [PATCH 4/6] MCE: Fix the vMCE address translation for HVM guest.

[Christoph Egger <Christoph.Egger@amd.com>]

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