[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 1/2] x86/xen: Set EFER.NX and EFER.SCE in PVH guests



On 09/05/2014 10:11 AM, David Vrabel wrote:
From: Mukesh Rathor <mukesh.rathor@xxxxxxxxxx>

This fixes two bugs in PVH guests:

   - Not setting EFER.NX means the NX bit in page table entries is
     ignored on Intel processors and causes reserved bit page faults on
     AMD processors.

   - After the Xen commit 7645640d6ff1 (x86/PVH: don't set EFER_SCE for
     pvh guest) PVH guests are required to set EFER.SCE to enable the
     SYSCALL instruction.

Secondary VCPUs are started with pagetables with the NX bit set so
EFER.NX must be set before using any stack or data segment.
xen_pvh_cpu_early_init() is the new secondary VCPU entry point that
sets EFER before jumping to cpu_bringup_and_idle().

Signed-off-by: Mukesh Rathor <mukesh.rathor@xxxxxxxxxx>
Signed-off-by: David Vrabel <david.vrabel@xxxxxxxxxx>
---
  arch/x86/xen/enlighten.c |    2 ++
  arch/x86/xen/smp.c       |   28 +++++++++++++++++-----------
  arch/x86/xen/smp.h       |    8 ++++++++
  arch/x86/xen/xen-head.S  |   30 ++++++++++++++++++++++++++++++
  4 files changed, 57 insertions(+), 11 deletions(-)

diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index c0cb11f..134d6a0 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1508,6 +1508,8 @@ static void __init xen_pvh_early_guest_init(void)
                return;
xen_have_vector_callback = 1;
+
+       xen_pvh_cpu_early_init(0);

Nit: for consistency may be better to call this xen_pvh_early_cpu_init().


        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..1b668dc 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,17 @@ 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_XEN_PVH

Since you are wrapping this in ifdef here we should probably also do the same for xen_pvh_early_guest_init() xen_cpu_secondary_cpu_init().

-boris

+       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;
                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..d5dbc65 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);
+#ifdef CONFIG_XEN_PVH
+extern void xen_pvh_cpu_early_init(int cpu);
+#else
+static inline void xen_pvh_cpu_early_init(int cpu)
+{
+}
+#endif
+
  #endif
diff --git a/arch/x86/xen/xen-head.S b/arch/x86/xen/xen-head.S
index 485b695..62b3f10 100644
--- a/arch/x86/xen/xen-head.S
+++ b/arch/x86/xen/xen-head.S
@@ -47,6 +47,36 @@ ENTRY(startup_xen)
__FINIT +#ifdef CONFIG_XEN_PVH
+
+/**
+ * xen_pvh_cpu_early_init() - early PVH VCPU initialization
+ * @cpu: this cpu number (%rdi)
+ *
+ * Note: This is called as a function on the boot CPU and as the secondary
+ * CPU entry point.
+ */
+ENTRY(xen_pvh_cpu_early_init)
+       /* 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,%rdi /* non-zero => secondary cpu */
+       jne     cpu_bringup_and_idle
+       ret
+
+#endif /* CONFIG_XEN_PVH */
+
  .pushsection .text
        .balign PAGE_SIZE
  ENTRY(hypercall_page)


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.