From mboxrd@z Thu Jan  1 00:00:00 1970
From: Andrew Cooper <andrew.cooper3@citrix.com>
Subject: Re: [PATCH v2 4/5] x86: Port the basic alternative
 mechanism from Linux to Xen
Date: Thu, 29 May 2014 15:59:16 +0100
Message-ID: <53874B44.4040703@citrix.com>
References: <1401341669-5237-1-git-send-email-feng.wu@intel.com>
	<1401341669-5237-5-git-send-email-feng.wu@intel.com>
	<5386F61E.7030408@citrix.com>
	<E959C4978C3B6342920538CF579893F001FD38C1@SHSMSX104.ccr.corp.intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <E959C4978C3B6342920538CF579893F001FD38C1@SHSMSX104.ccr.corp.intel.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: "Wu, Feng" <feng.wu@intel.com>, "xen-devel@lists.xen.org" <xen-devel@lists.xen.org>
Cc: "tim@xen.org" <tim@xen.org>, "keir.xen@gmail.com" <keir.xen@gmail.com>, "stefano.stabellini@citrix.com" <stefano.stabellini@citrix.com>, "ian.campbell@citrix.com" <ian.campbell@citrix.com>, "JBeulich@suse.com" <JBeulich@suse.com>
List-Id: xen-devel@lists.xenproject.org

On 29/05/2014 10:28, Wu, Feng wrote:
>
> 8 @@ void __init noreturn __start_xen(unsigned long
>> mbi_p)
>>>        if ( cpu_has_fsgsbase )
>>>            set_in_cr4(X86_CR4_FSGSBASE);
>>>
>>> +    alternative_instructions();
>>> +
>> Given this ordering, it might be cleaner to have an
>> ASSERT(!local_irq_enabled()) in the top of alternative_instructions(),
>> and forgo the local_irq_save/restore() in text_poke_early().
>>
>> If you can move this higher up before enabling MCEs in CR4, it might be
>> slightly more resilient.
>>
>> ~Andrew
> MCE bit in CR4 is set in identify_cpu() --> mcheck_init() --> set_in_cr4(X86_CR4_MCE), but
> apply_alternatives() needs boot_cpu_data.x86_capability being ready, since it calls boot_cpu_has().
> If we put alternative_instructions() before enabling MCEs in CR4, which place do you suggest? Thanks!
>
> Thanks,
> Feng

One option would be to temporarily disable it in cr4 at the same point 
that NMIs are nopped out, in the same way as temporarily disabling 
CR4.SMAP when building dom0.

At the end of the day, an MCE will certainly result in a crash, but at 
least it wouldn't be from a weird fault because some of the codepath in 
the MCE handler was midway through being patched.

~Andrew