Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933199AbbGGUui (ORCPT ); Tue, 7 Jul 2015 16:50:38 -0400 Received: from aserp1040.oracle.com ([141.146.126.69]:31814 "EHLO aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933073AbbGGUub (ORCPT ); Tue, 7 Jul 2015 16:50:31 -0400 Message-ID: <559C3B6E.6030304@oracle.com> Date: Tue, 07 Jul 2015 16:49:50 -0400 From: Boris Ostrovsky User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Konrad Rzeszutek Wilk CC: david.vrabel@citrix.com, roger.pau@citrix.com, elena.ufimtseva@oracle.com, stefano.stabellini@eu.citrix.com, tim@xen.org, jbeulich@suse.com, andrew.cooper3@citrix.com, ian.campbell@citrix.com, wei.liu2@citrix.com, ian.jackson@eu.citrix.com, xen-devel@lists.xenproject.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/6] xen/x86/pvh: Save %rbx in xen_pvh_early_cpu_init() References: <1436240065-10813-1-git-send-email-boris.ostrovsky@oracle.com> <1436240065-10813-2-git-send-email-boris.ostrovsky@oracle.com> <20150707201626.GG6372@l.oracle.com> In-Reply-To: <20150707201626.GG6372@l.oracle.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-Source-IP: aserv0021.oracle.com [141.146.126.233] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4565 Lines: 126 On 07/07/2015 04:16 PM, Konrad Rzeszutek Wilk wrote: > On Mon, Jul 06, 2015 at 11:34:20PM -0400, Boris Ostrovsky wrote: >> x86-64 ABI requires that functions preserve %rbx. When >> xen_pvh_early_cpu_init() is executed on boot cpu it is invoked as a >> function and 'cpuid' instruction will clobber %rbx. (This is not a >> concern on secondary processors since there xen_pvh_early_cpu_init() is >> the entry point and thus does not need to preserve registers.) >> >> Since we cannot access stack on secondary processors (PTE's NX bit will >> cause #GP on AMD --- see commit a2ef5dc2c7cb ("x86/xen: Set EFER.NX and >> EFER.SCE in PVH guests")) we can't always save %rbx on the stack. We >> work around this by creating a new entry point for those processors --- >> xen_pvh_early_cpu_init_secondary(). Note that on secondary CPUs we will >> load %rbx with garbage, which is OK. > .. as we overwrite it with %edx. I meant here that we do 'mov -8(%rsp), %rbx' before returning and on secondary CPUs we will grab garbage from the stack. But it's OK because we are in the entry point and cpu_bringup_and_idle (which is where we will be jumping to) doesn't care what's in %rbx. Overwriting with %edx happens earlier. -boris > >> As a side effect of these changes we don't need to save %rsi anymore, >> since we can use %rbx instead of %rsi as a temporary register. >> >> (We could store %rbx into another scratch register such as %r8 but since >> we will need new entry point for 32-bit PVH anyway we go with this >> approach for symmetry) >> >> Signed-off-by: Boris Ostrovsky > Reviewed-by: Konrad Rzeszutek Wilk > > >> --- >> arch/x86/xen/smp.c | 3 ++- >> arch/x86/xen/smp.h | 4 ++++ >> arch/x86/xen/xen-head.S | 14 +++++++------- >> 3 files changed, 13 insertions(+), 8 deletions(-) >> >> diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c >> index 8648438..ca7ee1f 100644 >> --- a/arch/x86/xen/smp.c >> +++ b/arch/x86/xen/smp.c >> @@ -423,7 +423,8 @@ cpu_initialize_context(unsigned int cpu, struct task_struct *idle) >> * bit set. This means before DS/SS is touched, NX in >> * EFER must be set. Hence the following assembly glue code. >> */ >> - ctxt->user_regs.eip = (unsigned long)xen_pvh_early_cpu_init; >> + ctxt->user_regs.eip = >> + (unsigned long)xen_pvh_early_cpu_init_secondary; >> ctxt->user_regs.rdi = cpu; >> ctxt->user_regs.rsi = true; /* entry == true */ >> } >> diff --git a/arch/x86/xen/smp.h b/arch/x86/xen/smp.h >> index 963d62a..bf2b43c 100644 >> --- a/arch/x86/xen/smp.h >> +++ b/arch/x86/xen/smp.h >> @@ -10,10 +10,14 @@ extern void xen_send_IPI_self(int vector); >> >> #ifdef CONFIG_XEN_PVH >> extern void xen_pvh_early_cpu_init(int cpu, bool entry); >> +extern void xen_pvh_early_cpu_init_secondary(int cpu, bool entry); >> #else >> static inline void xen_pvh_early_cpu_init(int cpu, bool entry) >> { >> } >> +static inline void xen_pvh_early_cpu_init_secondary(int cpu, bool entry) >> +{ >> +} >> #endif >> >> #endif >> diff --git a/arch/x86/xen/xen-head.S b/arch/x86/xen/xen-head.S >> index 8afdfcc..b1508a8 100644 >> --- a/arch/x86/xen/xen-head.S >> +++ b/arch/x86/xen/xen-head.S >> @@ -56,28 +56,28 @@ ENTRY(startup_xen) >> * @entry: true if this is a secondary vcpu coming up on this entry >> * point, false if this is the boot CPU being initialized for >> * the first time (%rsi) >> - * >> - * Note: This is called as a function on the boot CPU, and is the entry point >> - * on the secondary CPU. >> */ >> ENTRY(xen_pvh_early_cpu_init) >> - mov %rsi, %r11 >> + mov %rbx, -8(%rsp) >> >> +/* Entry point for secondary CPUs */ >> +ENTRY(xen_pvh_early_cpu_init_secondary) >> /* Gather features to see if NX implemented. */ >> mov $0x80000001, %eax >> cpuid >> - mov %edx, %esi >> + mov %edx, %ebx >> >> mov $MSR_EFER, %ecx >> rdmsr >> bts $_EFER_SCE, %eax >> >> - bt $20, %esi >> + bt $20, %ebx >> jnc 1f /* No NX, skip setting it */ >> bts $_EFER_NX, %eax >> 1: wrmsr >> + mov -8(%rsp), %rbx >> #ifdef CONFIG_SMP >> - cmp $0, %r11b >> + cmp $0, %esi >> jne cpu_bringup_and_idle >> #endif >> ret >> -- >> 1.8.1.4 >> -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/