* [V2 PATCH 0/1] PVH: set EFER bits @ 2014-09-03 1:19 Mukesh Rathor 2014-09-03 1:19 ` [V2 PATCH 1/1] PVH: set EFER.NX and EFER.SCE Mukesh Rathor 0 siblings, 1 reply; 6+ messages in thread From: Mukesh Rathor @ 2014-09-03 1:19 UTC (permalink / raw) To: boris.ostrovsky, david.vrabel; +Cc: xen-devel, linux-kernel Changes from V1: - Unify the patches into one - Unify the code to set the EFER bits. thanks, Mukesh ^ permalink raw reply [flat|nested] 6+ messages in thread
* [V2 PATCH 1/1] PVH: set EFER.NX and EFER.SCE 2014-09-03 1:19 [V2 PATCH 0/1] PVH: set EFER bits Mukesh Rathor @ 2014-09-03 1:19 ` Mukesh Rathor 2014-09-03 13:58 ` [Xen-devel] " David Vrabel 0 siblings, 1 reply; 6+ messages in thread From: Mukesh Rathor @ 2014-09-03 1:19 UTC (permalink / raw) To: boris.ostrovsky, david.vrabel; +Cc: xen-devel, linux-kernel This patch addresses two things for a pvh boot vcpu: - NX bug on intel: It was recenlty discovered that NX is not being honored in PVH on intel since EFER.NX is not being set. - PVH boot hang on newer xen: Following c/s on xen c/s 7645640: x86/PVH: don't set EFER_SCE for pvh guest removes setting of EFER.SCE for PVH guests. As such, existing intel pvh guest will no longer boot on xen after that c/s. Both above changes will be applicable to AMD also when xen support of AMD pvh is added. Also, we create a new glue assembly entry point for secondary vcpus because they come up on kernel page tables that have pte.NX bits set. As such, before anything is touched in DS/SS, EFER.NX must be set. Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com> --- arch/x86/xen/enlighten.c | 3 +++ arch/x86/xen/smp.c | 28 ++++++++++++++++++++-------- arch/x86/xen/smp.h | 1 + arch/x86/xen/xen-head.S | 29 +++++++++++++++++++++++++++++ 4 files changed, 53 insertions(+), 8 deletions(-) diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c index c0cb11f..e17fa2d 100644 --- a/arch/x86/xen/enlighten.c +++ b/arch/x86/xen/enlighten.c @@ -85,6 +85,8 @@ EXPORT_SYMBOL_GPL(hypercall_page); +extern void xen_pvh_configure_efer(void); + /* * Pointer to the xen_vcpu_info structure or * &HYPERVISOR_shared_info->vcpu_info[cpu]. See xen_hvm_init_shared_info @@ -1508,6 +1510,7 @@ static void __init xen_pvh_early_guest_init(void) return; xen_have_vector_callback = 1; + xen_pvh_configure_efer(); 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 7005974..073bbf4 100644 --- a/arch/x86/xen/smp.c +++ b/arch/x86/xen/smp.c @@ -37,6 +37,7 @@ #include <xen/hvc-console.h> #include "xen-ops.h" #include "mmu.h" +#include "smp.h" cpumask_var_t xen_cpu_initialized_map; @@ -99,8 +100,12 @@ static void cpu_bringup(void) wmb(); /* make sure everything is out */ } -/* Note: cpu parameter is only relevant for PVH */ -static void cpu_bringup_and_idle(int cpu) +/* + * Note: cpu parameter is only relevant for PVH. The reason for passing it + * is we can't do smp_processor_id until the percpu segments are loaded, for + * which we need the cpu number! So we pass it in rdi as first parameter. + */ +asmlinkage __visible void cpu_bringup_and_idle(int cpu) { #ifdef CONFIG_X86_64 if (xen_feature(XENFEAT_auto_translated_physmap) && @@ -374,11 +379,10 @@ cpu_initialize_context(unsigned int cpu, struct task_struct *idle) ctxt->user_regs.fs = __KERNEL_PERCPU; ctxt->user_regs.gs = __KERNEL_STACK_CANARY; #endif - ctxt->user_regs.eip = (unsigned long)cpu_bringup_and_idle; - memset(&ctxt->fpu_ctxt, 0, sizeof(ctxt->fpu_ctxt)); if (!xen_feature(XENFEAT_auto_translated_physmap)) { + ctxt->user_regs.eip = (unsigned long)cpu_bringup_and_idle; ctxt->flags = VGCF_IN_KERNEL; ctxt->user_regs.eflags = 0x1000; /* IOPL_RING1 */ ctxt->user_regs.ds = __USER_DS; @@ -416,12 +420,20 @@ cpu_initialize_context(unsigned int cpu, struct task_struct *idle) #ifdef CONFIG_X86_32 } #else - } else - /* N.B. The user_regs.eip (cpu_bringup_and_idle) is called with - * %rdi having the cpu number - which means are passing in - * as the first parameter the cpu. Subtle! + } else { + /* + * The vcpu comes on kernel page tables which have the NX pte + * 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)pvh_smp_cpu_bringup; + + /* N.B. The bringup function cpu_bringup_and_idle is called with + * %rdi having the cpu number - which means we are passing it in + * as the first parameter. Subtle! */ ctxt->user_regs.rdi = cpu; + } #endif ctxt->user_regs.esp = idle->thread.sp0 - sizeof(struct pt_regs); ctxt->ctrlreg[3] = xen_pfn_to_cr3(virt_to_mfn(swapper_pg_dir)); diff --git a/arch/x86/xen/smp.h b/arch/x86/xen/smp.h index c7c2d89..d6628cb 100644 --- a/arch/x86/xen/smp.h +++ b/arch/x86/xen/smp.h @@ -7,5 +7,6 @@ extern void xen_send_IPI_mask_allbutself(const struct cpumask *mask, extern void xen_send_IPI_allbutself(int vector); extern void xen_send_IPI_all(int vector); extern void xen_send_IPI_self(int vector); +extern void pvh_smp_cpu_bringup(int cpu); #endif diff --git a/arch/x86/xen/xen-head.S b/arch/x86/xen/xen-head.S index 485b695..97ee831 100644 --- a/arch/x86/xen/xen-head.S +++ b/arch/x86/xen/xen-head.S @@ -47,6 +47,35 @@ ENTRY(startup_xen) __FINIT +#ifdef CONFIG_XEN_PVH +#ifdef CONFIG_X86_64 +.macro PVH_EARLY_SET_EFER + /* Gather features to see if NX implemented. */ + movl $0x80000001, %eax + cpuid + movl %edx,%esi + + movl $MSR_EFER, %ecx + rdmsr + btsl $_EFER_SCE, %eax + + btl $20,%esi + jnc 1f /* No NX, skip setting it */ + btsl $_EFER_NX, %eax +1: wrmsr +.endm + +ENTRY(xen_pvh_configure_efer) + PVH_EARLY_SET_EFER + ret + +/* Note that rdi contains the cpu number and must be preserved */ +ENTRY(pvh_smp_cpu_bringup) + PVH_EARLY_SET_EFER + jmp cpu_bringup_and_idle +#endif /* CONFIG_X86_64 */ +#endif /* CONFIG_XEN_PVH */ + .pushsection .text .balign PAGE_SIZE ENTRY(hypercall_page) -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Xen-devel] [V2 PATCH 1/1] PVH: set EFER.NX and EFER.SCE 2014-09-03 1:19 ` [V2 PATCH 1/1] PVH: set EFER.NX and EFER.SCE Mukesh Rathor @ 2014-09-03 13:58 ` David Vrabel 2014-09-03 14:49 ` Boris Ostrovsky 2014-09-03 20:53 ` Mukesh Rathor 0 siblings, 2 replies; 6+ messages in thread From: David Vrabel @ 2014-09-03 13:58 UTC (permalink / raw) To: Mukesh Rathor, boris.ostrovsky, david.vrabel; +Cc: xen-devel, linux-kernel On 03/09/14 02:19, Mukesh Rathor wrote: > This patch addresses two things for a pvh boot vcpu: > > - NX bug on intel: It was recenlty discovered that NX is not being > honored in PVH on intel since EFER.NX is not being set. > > - PVH boot hang on newer xen: Following c/s on xen > > c/s 7645640: x86/PVH: don't set EFER_SCE for pvh guest > > removes setting of EFER.SCE for PVH guests. As such, existing intel > pvh guest will no longer boot on xen after that c/s. > > Both above changes will be applicable to AMD also when xen support > of AMD pvh is added. > > Also, we create a new glue assembly entry point for secondary vcpus > because they come up on kernel page tables that have pte.NX > bits set. As such, before anything is touched in DS/SS, EFER.NX > must be set. [...] > --- a/arch/x86/xen/xen-head.S > +++ b/arch/x86/xen/xen-head.S > @@ -47,6 +47,35 @@ ENTRY(startup_xen) > > __FINIT > > +#ifdef CONFIG_XEN_PVH > +#ifdef CONFIG_X86_64 > +.macro PVH_EARLY_SET_EFER I don't think a macro is the right way to do this. We can instead pass a parameter to say whether it is a boot or secondary CPU. Something like this (untested) patch? 8<---------------------------- x86/xen: Set EFER.NX and EFER.SCE in PVH guests This patch addresses two things for a pvh boot vcpu: - NX bug on intel: It was recenlty discovered that NX is not being honored in PVH on intel since EFER.NX is not being set. - PVH boot hang on newer xen: Following c/s on xen c/s 7645640: x86/PVH: don't set EFER_SCE for pvh guest removes setting of EFER.SCE for PVH guests. As such, existing intel pvh guest will no longer boot on xen after that c/s. Both above changes will be applicable to AMD also when xen support of AMD pvh is added. Also, we create a new glue assembly entry point for secondary vcpus because they come up on kernel page tables that have pte.NX bits set. As such, before anything is touched in DS/SS, EFER.NX must be set. Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com> Signed-off-by: David Vrabel <david.vrabel@citrix.com> --- arch/x86/xen/enlighten.c | 1 + arch/x86/xen/smp.c | 34 +++++++++++++++++++++++----------- arch/x86/xen/smp.h | 8 ++++++++ arch/x86/xen/xen-head.S | 34 ++++++++++++++++++++++++++++++++++ 4 files changed, 66 insertions(+), 11 deletions(-) diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c index c0cb11f..b111035 100644 --- a/arch/x86/xen/enlighten.c +++ b/arch/x86/xen/enlighten.c @@ -1508,6 +1508,7 @@ static void __init xen_pvh_early_guest_init(void) return; xen_have_vector_callback = 1; + xen_pvh_cpu_early_init(0, false); 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 7005974..dd4ce0d 100644 --- a/arch/x86/xen/smp.c +++ b/arch/x86/xen/smp.c @@ -37,6 +37,7 @@ #include <xen/hvc-console.h> #include "xen-ops.h" #include "mmu.h" +#include "smp.h" cpumask_var_t xen_cpu_initialized_map; @@ -99,8 +100,12 @@ static void cpu_bringup(void) wmb(); /* make sure everything is out */ } -/* Note: cpu parameter is only relevant for PVH */ -static void cpu_bringup_and_idle(int cpu) +/* + * Note: cpu parameter is only relevant for PVH. The reason for passing it + * is we can't do smp_processor_id until the percpu segments are loaded, for + * which we need the cpu number! So we pass it in rdi as first parameter. + */ +asmlinkage __visible void cpu_bringup_and_idle(int cpu) { #ifdef CONFIG_X86_64 if (xen_feature(XENFEAT_auto_translated_physmap) && @@ -374,11 +379,10 @@ cpu_initialize_context(unsigned int cpu, struct task_struct *idle) ctxt->user_regs.fs = __KERNEL_PERCPU; ctxt->user_regs.gs = __KERNEL_STACK_CANARY; #endif - ctxt->user_regs.eip = (unsigned long)cpu_bringup_and_idle; - memset(&ctxt->fpu_ctxt, 0, sizeof(ctxt->fpu_ctxt)); if (!xen_feature(XENFEAT_auto_translated_physmap)) { + ctxt->user_regs.eip = (unsigned long)cpu_bringup_and_idle; ctxt->flags = VGCF_IN_KERNEL; ctxt->user_regs.eflags = 0x1000; /* IOPL_RING1 */ ctxt->user_regs.ds = __USER_DS; @@ -413,15 +417,23 @@ cpu_initialize_context(unsigned int cpu, struct task_struct *idle) (unsigned long)xen_failsafe_callback; ctxt->user_regs.cs = __KERNEL_CS; per_cpu(xen_cr3, cpu) = __pa(swapper_pg_dir); -#ifdef CONFIG_X86_32 - } -#else - } else - /* N.B. The user_regs.eip (cpu_bringup_and_idle) is called with - * %rdi having the cpu number - which means are passing in - * as the first parameter the cpu. Subtle! + } +#ifdef CONFIG_X86_64 + else { + /* + * The vcpu comes on kernel page tables which have the NX pte + * 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_cpu_early_init; + + /* N.B. The bringup function cpu_bringup_and_idle is called with + * %rdi having the cpu number - which means we are passing it in + * as the first parameter. Subtle! */ ctxt->user_regs.rdi = cpu; + ctxt->user_regs.rsi = 1; /* secondary_cpu == true */ + } #endif ctxt->user_regs.esp = idle->thread.sp0 - sizeof(struct pt_regs); ctxt->ctrlreg[3] = xen_pfn_to_cr3(virt_to_mfn(swapper_pg_dir)); diff --git a/arch/x86/xen/smp.h b/arch/x86/xen/smp.h index c7c2d89..fc2575a 100644 --- a/arch/x86/xen/smp.h +++ b/arch/x86/xen/smp.h @@ -8,4 +8,12 @@ extern void xen_send_IPI_allbutself(int vector); extern void xen_send_IPI_all(int vector); extern void xen_send_IPI_self(int vector); +#if defined(CONFIG_XEN_PVH) && defined(CONFIG_X86_64) +extern void xen_pvh_cpu_early_init(int cpu, bool secondary_cpu); +#else +static inline void xen_pvh_cpu_early_init(int cpu, bool secondary_cpu) +{ +} +#endif + #endif diff --git a/arch/x86/xen/xen-head.S b/arch/x86/xen/xen-head.S index 485b695..a64b464 100644 --- a/arch/x86/xen/xen-head.S +++ b/arch/x86/xen/xen-head.S @@ -47,6 +47,40 @@ ENTRY(startup_xen) __FINIT +#ifdef CONFIG_XEN_PVH +#ifdef CONFIG_X86_64 + +/** + * xen_pvh_cpu_early_init() - early PVH VCPU initialization + * @cpu: this cpu number (%rdi) + * @secondary_cpu: true if this is a secondary (non-boot) CPU (%rsi) + * + * Note: This is called as a function on the boot CPU and as the secondary + * CPU entry point. + */ +ENTRY(xen_pvh_cpu_early_init) + mov %rsi, %r11 + /* Gather features to see if NX implemented. */ + mov $0x80000001, %eax + cpuid + mov %edx,%esi + + mov $MSR_EFER, %ecx + rdmsr + bts $_EFER_SCE, %eax + + bt $20,%esi + jnc 1f /* No NX, skip setting it */ + bts $_EFER_NX, %eax +1: wrmsr + + cmp $0,%r11b /* non-zero => secondary cpu */ + jne cpu_bringup_and_idle + ret + +#endif /* CONFIG_X86_64 */ +#endif /* CONFIG_XEN_PVH */ + .pushsection .text .balign PAGE_SIZE ENTRY(hypercall_page) -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Xen-devel] [V2 PATCH 1/1] PVH: set EFER.NX and EFER.SCE 2014-09-03 13:58 ` [Xen-devel] " David Vrabel @ 2014-09-03 14:49 ` Boris Ostrovsky 2014-09-03 14:53 ` Andrew Cooper 2014-09-03 20:53 ` Mukesh Rathor 1 sibling, 1 reply; 6+ messages in thread From: Boris Ostrovsky @ 2014-09-03 14:49 UTC (permalink / raw) To: David Vrabel, Mukesh Rathor; +Cc: xen-devel, linux-kernel On 09/03/2014 09:58 AM, David Vrabel wrote: > #endif > diff --git a/arch/x86/xen/xen-head.S b/arch/x86/xen/xen-head.S > index 485b695..a64b464 100644 > --- a/arch/x86/xen/xen-head.S > +++ b/arch/x86/xen/xen-head.S > @@ -47,6 +47,40 @@ ENTRY(startup_xen) > > __FINIT > > +#ifdef CONFIG_XEN_PVH > +#ifdef CONFIG_X86_64 CONFIG_XEN_PVH implies CONFIG_X86_64. -boris ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Xen-devel] [V2 PATCH 1/1] PVH: set EFER.NX and EFER.SCE 2014-09-03 14:49 ` Boris Ostrovsky @ 2014-09-03 14:53 ` Andrew Cooper 0 siblings, 0 replies; 6+ messages in thread From: Andrew Cooper @ 2014-09-03 14:53 UTC (permalink / raw) To: Boris Ostrovsky, David Vrabel, Mukesh Rathor; +Cc: xen-devel, linux-kernel On 03/09/14 15:49, Boris Ostrovsky wrote: > On 09/03/2014 09:58 AM, David Vrabel wrote: > >> #endif >> diff --git a/arch/x86/xen/xen-head.S b/arch/x86/xen/xen-head.S >> index 485b695..a64b464 100644 >> --- a/arch/x86/xen/xen-head.S >> +++ b/arch/x86/xen/xen-head.S >> @@ -47,6 +47,40 @@ ENTRY(startup_xen) >> __FINIT >> +#ifdef CONFIG_XEN_PVH >> +#ifdef CONFIG_X86_64 > > CONFIG_XEN_PVH implies CONFIG_X86_64. > > -boris Only because PVH is currently mal-specified and limited to 64bit. It will certainly not be true when PVH gains 32bit support. ~Andrew > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Xen-devel] [V2 PATCH 1/1] PVH: set EFER.NX and EFER.SCE 2014-09-03 13:58 ` [Xen-devel] " David Vrabel 2014-09-03 14:49 ` Boris Ostrovsky @ 2014-09-03 20:53 ` Mukesh Rathor 1 sibling, 0 replies; 6+ messages in thread From: Mukesh Rathor @ 2014-09-03 20:53 UTC (permalink / raw) To: David Vrabel; +Cc: boris.ostrovsky, xen-devel, linux-kernel On Wed, 3 Sep 2014 14:58:04 +0100 David Vrabel <david.vrabel@citrix.com> wrote: > On 03/09/14 02:19, Mukesh Rathor wrote: > > This patch addresses two things for a pvh boot vcpu: > > > > - NX bug on intel: It was recenlty discovered that NX is not being > > honored in PVH on intel since EFER.NX is not being set. > > > > - PVH boot hang on newer xen: Following c/s on xen > > > > c/s 7645640: x86/PVH: don't set EFER_SCE for pvh guest > > > > removes setting of EFER.SCE for PVH guests. As such, existing > > intel pvh guest will no longer boot on xen after that c/s. > > > > Both above changes will be applicable to AMD also when xen support > > of AMD pvh is added. > > > > Also, we create a new glue assembly entry point for secondary vcpus > > because they come up on kernel page tables that have pte.NX > > bits set. As such, before anything is touched in DS/SS, EFER.NX > > must be set. > [...] > > --- a/arch/x86/xen/xen-head.S > > +++ b/arch/x86/xen/xen-head.S > > @@ -47,6 +47,35 @@ ENTRY(startup_xen) > > > > __FINIT > > > > +#ifdef CONFIG_XEN_PVH > > +#ifdef CONFIG_X86_64 > > +.macro PVH_EARLY_SET_EFER > > I don't think a macro is the right way to do this. We can instead > pass a parameter to say whether it is a boot or secondary CPU. > > Something like this (untested) patch? That's fine too. But, since vcpu 0 is always primary vcpu, we can just use that and not worry about passing another parameter. -Mukesh ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-09-03 20:53 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-09-03 1:19 [V2 PATCH 0/1] PVH: set EFER bits Mukesh Rathor 2014-09-03 1:19 ` [V2 PATCH 1/1] PVH: set EFER.NX and EFER.SCE Mukesh Rathor 2014-09-03 13:58 ` [Xen-devel] " David Vrabel 2014-09-03 14:49 ` Boris Ostrovsky 2014-09-03 14:53 ` Andrew Cooper 2014-09-03 20:53 ` Mukesh Rathor
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox