From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758372AbbGQPKG (ORCPT ); Fri, 17 Jul 2015 11:10:06 -0400 Received: from aserp1040.oracle.com ([141.146.126.69]:17767 "EHLO aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758169AbbGQPKD (ORCPT ); Fri, 17 Jul 2015 11:10:03 -0400 Date: Fri, 17 Jul 2015 11:09:50 -0400 From: Konrad Rzeszutek Wilk To: Boris Ostrovsky 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, ian.jackson@eu.citrix.com, xen-devel@lists.xenproject.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 1/6] xen/x86/pvh: Save %rbx in xen_pvh_early_cpu_init() Message-ID: <20150717150950.GA18085@l.oracle.com> References: <1437083021-24488-1-git-send-email-boris.ostrovsky@oracle.com> <1437083021-24488-2-git-send-email-boris.ostrovsky@oracle.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1437083021-24488-2-git-send-email-boris.ostrovsky@oracle.com> User-Agent: Mutt/1.5.23 (2014-03-12) 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 On Thu, Jul 16, 2015 at 05:43:36PM -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 from the stack, which is OK. > > 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). > > Konrad also sugested that with separate entry point for secondary > processors we don't need the second argument ('bool entry') to > xen_pvh_early_cpu_init[_secondary](). So it is dropped. > > Signed-off-by: Boris Ostrovsky Reviewed-by: Konrad Rzeszutek Wilk > --- > Changes in v2: > * Dropped second argument to xen_pvh_early_cpu_init() > > arch/x86/xen/enlighten.c | 2 +- > arch/x86/xen/smp.c | 4 ++-- > arch/x86/xen/smp.h | 8 ++++++-- > arch/x86/xen/xen-head.S | 27 ++++++++++++++------------- > 4 files changed, 23 insertions(+), 18 deletions(-) > > diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c > index 0b95c9b..f8dc398 100644 > --- a/arch/x86/xen/enlighten.c > +++ b/arch/x86/xen/enlighten.c > @@ -1454,7 +1454,7 @@ static void __init xen_pvh_early_guest_init(void) > > xen_have_vector_callback = 1; > > - xen_pvh_early_cpu_init(0, false); > + xen_pvh_early_cpu_init(0); > xen_pvh_set_cr_flags(0); > > #ifdef CONFIG_X86_32 > diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c > index 8648438..e53be3b 100644 > --- a/arch/x86/xen/smp.c > +++ b/arch/x86/xen/smp.c > @@ -423,9 +423,9 @@ 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 */ > } > #endif > ctxt->user_regs.esp = idle->thread.sp0 - sizeof(struct pt_regs); > diff --git a/arch/x86/xen/smp.h b/arch/x86/xen/smp.h > index 963d62a..538c00d 100644 > --- a/arch/x86/xen/smp.h > +++ b/arch/x86/xen/smp.h > @@ -9,9 +9,13 @@ extern void xen_send_IPI_all(int vector); > 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(int cpu); > +extern void xen_pvh_early_cpu_init_secondary(int cpu); > #else > -static inline void xen_pvh_early_cpu_init(int cpu, bool entry) > +static inline void xen_pvh_early_cpu_init(int cpu) > +{ > +} > +static inline void xen_pvh_early_cpu_init_secondary(int cpu) > { > } > #endif > diff --git a/arch/x86/xen/xen-head.S b/arch/x86/xen/xen-head.S > index 8afdfcc..944b08b 100644 > --- a/arch/x86/xen/xen-head.S > +++ b/arch/x86/xen/xen-head.S > @@ -53,31 +53,32 @@ ENTRY(startup_xen) > /* > * xen_pvh_early_cpu_init() - early PVH VCPU initialization > * @cpu: this cpu number (%rdi) > - * @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) > + xor %esi, %esi > + jmp 1f > + > +/* Entry point for secondary CPUs. Can't touch stack until NX is dealt with. */ > +ENTRY(xen_pvh_early_cpu_init_secondary) > + mov $1, %esi > +1: > /* 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 > - jnc 1f /* No NX, skip setting it */ > + bt $20, %ebx > + jnc 2f /* No NX, skip setting it */ > bts $_EFER_NX, %eax > -1: wrmsr > +2: 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 >