This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
Home Products Support Community News


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

To: "Ke, Liping" <liping.ke@xxxxxxxxx>
Subject: Re: [Xen-devel] [pvops-dom0] Adding MCA logging support in pv_ops
From: Jeremy Fitzhardinge <jeremy@xxxxxxxx>
Date: Fri, 07 Aug 2009 11:07:35 -0700
Cc: "Kleen, Andi" <andi.kleen@xxxxxxxxx>, Christoph Egger <Christoph.Egger@xxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>, Keir Fraser <keir.fraser@xxxxxxxxxxxxx>, Jan Beulich <JBeulich@xxxxxxxxxx>
Delivery-date: Fri, 07 Aug 2009 11:08:05 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <E2263E4A5B2284449EEBD0AAB751098402CE0B6567@xxxxxxxxxxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <E2263E4A5B2284449EEBD0AAB751098402CDF6599F@xxxxxxxxxxxxxxxxxxxxxxxxxxxx> <4A7174D3020000780000D350@xxxxxxxxxxxxxxxxxx> <E2263E4A5B2284449EEBD0AAB751098402CDF65BEB@xxxxxxxxxxxxxxxxxxxxxxxxxxxx> <E2263E4A5B2284449EEBD0AAB751098402CDF65C35@xxxxxxxxxxxxxxxxxxxxxxxxxxxx> <4A71D4F8.70804@xxxxxxxx> <E2263E4A5B2284449EEBD0AAB751098402CE0B5780@xxxxxxxxxxxxxxxxxxxxxxxxxxxx> <4A7A0DF7.5040706@xxxxxxxx> <E2263E4A5B2284449EEBD0AAB751098402CE0B6567@xxxxxxxxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1b3pre) Gecko/20090513 Fedora/3.0-2.3.beta2.fc11 Lightning/1.0pre Thunderbird/3.0b2
On 08/06/09 19:32, Ke, Liping wrote:
> Hi, Jeremy and all
> This is the new formated patch and I have tried linux patch format check 
> tools on it.

Thanks.  I have a couple of little comments:

    * when referring to Xen hg changesets, use the hex changeset ID
      rather than the number, since the number only makes sense locally
      (ie, your change 902 may not match anyone else's)
    * don't put "dom0" in the driver filename; drivers/xen/mce.c is
      enough.  Same with the various symbols and messages mentioning dom0.
    * don't use "#if define" if #ifdef would do; but try to avoid #ifdef
      anyway.  In particular, this *definitely* shouldn't be duplicating
      code between the #if and #else blocks.  At the very least, you
      should just need one #ifdef to re-add the MCE/MCA flags back into
      the mask.

      +#if !defined(CONFIG_XEN_MCE)
        cpuid_leaf1_edx_mask =
                ~((1 << X86_FEATURE_MCE)  |  /* disable MCE */
                  (1 << X86_FEATURE_MCA)  |  /* disable MCA */
                  (1 << X86_FEATURE_PAT)  |  /* disable PAT */
                  (1 << X86_FEATURE_ACC));   /* thermal monitoring */
      + cpuid_leaf1_edx_mask =
      +         ~((1 << X86_FEATURE_PAT)  |  /* disable PAT */
      +           (1 << X86_FEATURE_ACC));   /* thermal monitoring */

Also, if you include the patch inline it makes it easier to review and
comment on.


Xen-devel mailing list