Re: [Xen-devel] [pvops-dom0] Adding MCA logging support in pv_ops

[Christoph Egger <Christoph.Egger@xxxxxxx>]

On Wednesday 05 August 2009 03:57:20 Ke, Liping wrote:
> Hi, Christoph
> Please see my below comments.
>
> And also, I found some interfaces are different in pv_ops kernel such as
> GUEST_HANDLE related. Seems we can't keep the same copy of common
> file between XEN and GUEST.

That's not a problem as long as the ABI doesn't change.

> We have to do slight changes to the XEN  file before copying it to guest
> kernel. 

Well, the comment updates I suppose.

> And also, for the header file, I modified a little according to Andi's
> feedback such as gigantic macros will be unacceptable according to kernel
> code conventions, etc. So I modify x86_mcinfo_lookup into inline function.

NetBSD also has some "local" guest header changes which aren't accepted
by Keir due to Xen conventions.
Keep in mind that you have to merge the headers whenever you sync up with
Xen.

> I will resend the new patch to all of you for further feedback. After the
> patch is accepted, I will sync the modified head file back to XEN for
> consistency.

Please practise friendly actions for non-Linux guests when changing
the headers. Changing the macros for only one guest isn't a friendly action
for all guests.

Please only sync back the comment updates.

If NetBSD, Solaris and Linux were trying to have all local changes in Xen 
headers, they would become a mess.

> Thanks a lot for your help!
> Criping
>
> Christoph Egger wrote:
> > Hi Ke Liping!
> >
> > The xen mca public header needs some comment updates in the xen tree
> > first before it populates to the guests:
>
> Yes and thanks!
>
> > In line 102, uncorrectable errors are not reported via nmi handlers.
> > Please update the comment to match the code.
>
> OK, I will modify this comment

Tnx.

> > In struct mcinfo_recovery, it is not obvious that the REC_ACTION_*
> > flags are intended to be used in action_flags. Please add a comment
> > to make this clear.
> >
> > In struct mcinfo_recovery, it is not obvious that the MC_ACTION_*
> > flags are intended to be used in action_types. Actually are these
> > flags?
>
> OK, I will add comments on this

Tnx.

> > These are defined as a bitfield, but the union can't store more than
> > one action. Please add a comment to clarify this.
>
> Currently, We only support one action per bank. Do you have requirement
> on multiple actions per bank?

Yes, because support for one action per bank is a limitation in the
(Intel specific) implementation but not in design.

> > For xen_mc_notifydomain, it is not clear if this has to be used
> > before or after acknowledging the fetched telemetry. Please update
> > the comment
> > to clarify this.
> >
> > In xen_mc_notifydomain, the comment this the flags field may contain
> > XEN_MC_TRAP which doesn't exist. Please update the comment.
>
> According to the latest do_mca hypercall implementation, seems
> XEN_MC_notifydomain is unsupported, so I never use this hypercall.
> Also, I did not see XEN_MC_CORRECTABLE definition in current code too.
> So I will remove both of them in comments here.
> When this hypercall is implemented, the author should update this comments
> here?

The notifydomain hypercall is currently broken due to the design and 
implementation changes between Xen 3.3 and Xen 3.4. It got broken
because noone adapted it to work with the new design.
It is a method for the dom0 to notify a domU.

Right, you don't use them in your Linux patch but that doesn't imply that 
others like Solaris don't use it.

Christoph

> > On Thursday 30 July 2009 07:15:05 Ke, Liping wrote:
> >> Hi, Jeremy and Keir
> >>
> >> This patch is backport from DOM0 cs902
> >>
> >> When an MCE/CMCI error happens (or by polling), the related error
> >> information will be sent to DOM0 by XEN. This patch will help to
> >> fetch the xen-logged information by hypercall and then convert
> >> XEN-format log into Linux format MCELOG. It makes using current
> >> available mcelog tools for native Linux possible.
> >>
> >> With this patch, after mce/cmci error log information is sent to
> >> DOM0, running mcelog tools in DOM0, you will get same detailed
> >> decoded mce information as in Native Linux.
> >>
> >> Signed-Off-By: Liping Ke <liping.ke@xxxxxxxxx>
> >> Signed-Off-By: Yunhong Jiang <yunhong.jiang@xxxxxxxxx>
> >>
> >>
> >> Thanks& Regards,
> >> Criping



-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: 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