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

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

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