Re: [RFC] [PATCH 0/2] Some clean-up to MCA handling

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

On Monday 19 April 2010 17:32:17 Jiang, Yunhong wrote:
> >-----Original Message-----
> >From: Christoph Egger [mailto:Christoph.Egger@amd.com]
> >Sent: Monday, April 19, 2010 6:07 PM
> >To: Jiang, Yunhong
> >Cc: Frank.Vanderlinden@Sun.COM; Keir Fraser; xen-devel@lists.xensource.com
> >Subject: Re: [RFC] [PATCH 0/2] Some clean-up to MCA handling
> >
> >On Monday 19 April 2010 10:58:44 Jiang, Yunhong wrote:
> >> These two patches includes some clean-up/changes to MCA handling.
> >>
> >> The mce_extended.patch change the method to get the extended MCA
> >> information. This change is somwhat straightforward and is easy to
> >> understand. But please notice it changes some interface in
> >> include/public/arch-x86/xen-mca.h.
> >
> >This breaks backward-compatibility. You can't change the API/ABI just for
> > the sake of "easier" internal handling.
> >
> >Please explain:
> >- what exactly is wrong with the interface as it is in upstream ?
> >- what *requries* an API/ABI change ?
>
> It is not "easier" internalhandling. In fact, it makes no difference to
> internal handling at all. The reason is: 1) In amd_f10.c, it will only
> occupy 4 mc_msr,

well, 4 in the generic handler plus 3 MSRs via mcinfo_extended which have
been introduced in family10h.

> while in Intel platform, it may occupy 32 mc_msr, that is 
> sure to cost extra space. The mc_info buffer is quite limited and can't be
> expanded in run time, so reduce the size is quite important.
> 2) sizeof(void *) is different in 64 hypervisor and 32 bit dom0. I'm not
> sure if it is tested in compatibility mode, which might be confused.
>
> In fact, since we have mc_msrs included in mcinfo_extended already, the
> caller can get the size of the buffer quite easy.
>
> Of course, if you *really* don't care the waste of size in AMD platform,
> it's ok for me. After all, in intel platform, either there is no extended
> information, or it will occupy all of them, so it really does not matter to
> me. But the (void*) issue should be resolved, I suspect.

Is it possible to change the internal infrastructure to deal with multiple
mc_info's ? The user (Dom0) will keep to see just one because the switch
happens underneath.

What does not fit into one mc_info will be put into the next.

In xen you will need some operations:
lowlevel: allocate, free, switch, read, write
highlevel: get and put

The mce code itself should just use the highlevel operations and also just
see one mc_info. The highlevel operations see as many mc_info as needed and
use the lowlevel operations which work on mc_info directly.

Does that make sense to you ?

> How about your option to the other patch?

Still need to have a look at it.

> Thanks
> --jyh
>
> >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



-- 
---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