[Xen-devel] RE: [PATCH 4/6] MCE: Fix the vMCE address translation for HVM guest.

["Jiang, Yunhong" <yunhong.jiang@xxxxxxxxx>]

>-----Original Message-----
>From: Christoph Egger [mailto:Christoph.Egger@xxxxxxx]
>Sent: Friday, January 29, 2010 12:31 AM
>To: Jiang, Yunhong
>Cc: Keir Fraser; Frank.Vanderlinden@xxxxxxx; Jan Beulich;
>xen-devel@xxxxxxxxxxxxxxxxxxx
>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@xxxxxxx]
>> >Sent: Thursday, January 28, 2010 4:14 PM
>> >To: Jiang, Yunhong
>> >Cc: Keir Fraser; Frank.Vanderlinden@xxxxxxx; Jan Beulich;
>> >xen-devel@xxxxxxxxxxxxxxxxxxx
>> >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.

--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@xxxxxxxxx>
>> >>
>> >> 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@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel