Re: [Xen-devel] [V5 PATCH 1/1] x86/xen: Set EFER.NX and EFER.SCE in PVH guests
On 11/09/14 00:36, Mukesh Rathor wrote:
> 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().
Applied to devel/for-linus-3.18
Thanks.
David
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Xen-devel] [V5 PATCH 1/1] x86/xen: Set EFER.NX and EFER.SCE in PVH guests
On 11/09/14 00:36, Mukesh Rathor wrote: 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(). Applied to devel/for-linus-3.18 Thanks. David -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Xen-devel] [V5 PATCH 1/1] x86/xen: Set EFER.NX and EFER.SCE in PVH guests
On Fri, 12 Sep 2014 16:42:58 -0400 Konrad Rzeszutek Wilk wrote: > On Wed, Sep 10, 2014 at 04:36:06PM -0700, Mukesh Rathor wrote: sorry, i didn't realize you had more comments... didn't scroll down :).. > > cpumask_var_t xen_cpu_initialized_map; > > > > @@ -99,10 +100,14 @@ 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. > > + */ > > Thank you for expanding on that (I totally forgot why we did that). sure. > > +* 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. > > And you ripped out the nice 'N.B' comment I added. Sad :-( > > */ > > + ctxt->user_regs.eip = (unsigned > > long)xen_pvh_early_cpu_init; ctxt->user_regs.rdi = cpu; > > + ctxt->user_regs.rsi = true; /* secondary cpu == > > true */ > > Oh, that is new. Ah yes we can use that [looking at Xen code]. > I wonder what other registers we can use to pass stuff around. All GPRs. I commented that we can do that in the Rogers PVH doc. Looks like David responded to other comments. Thanks, Mukesh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Xen-devel] [V5 PATCH 1/1] x86/xen: Set EFER.NX and EFER.SCE in PVH guests
On Mon, Sep 15, 2014 at 03:45:53PM +0100, David Vrabel wrote:
> On 12/09/14 21:42, Konrad Rzeszutek Wilk wrote:
> > On Wed, Sep 10, 2014 at 04:36:06PM -0700, Mukesh Rathor wrote:
> >>
> >> @@ -413,15 +417,18 @@ 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
> >> + 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.
> >
> > And you ripped out the nice 'N.B' comment I added. Sad :-(
>
> I think I removed that.
>
> I don't think passing parameters to a function is particularly subtle
> and this comment is largely superseded by the comment for
> xen_pvh_early_cpu_init() itself.
Good point.
>
> >> +#ifdef CONFIG_XEN_PVH
> >> +/*
> >> + * xen_pvh_early_cpu_init() - early PVH VCPU initialization
> >> + * @cpu: this cpu number (%rdi)
> >> + * @flag: boolean flag true to indicate this is a secondary vcpu coming up
> >> + *on this entry point or the primary cpu coming back online.
> >
> > Why do we do this? Why not just piggyback on the first parameter - the
> > 'cpu'?
> >
> > If it is zero it is boot CPU.
>
> "Changes in v5 (Mukesh):
> - Jan reminded us that vcpu 0 could go offline/online. So, add flag back"
Right, totally forgot about that.
With that I think I think you can tack on 'Reviewed-by: Konrad Rzeszutek
Wilk '.
Granted the PVH ABI doc needs to go in Xen base first as David requested.
Even a draft that explains some of this would be good.
CC-ing Roger - perhaps v1 of the draft could be posted and we can flesh it
out more?
>
> David
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Xen-devel] [V5 PATCH 1/1] x86/xen: Set EFER.NX and EFER.SCE in PVH guests
On 12/09/14 21:42, Konrad Rzeszutek Wilk wrote:
> On Wed, Sep 10, 2014 at 04:36:06PM -0700, Mukesh Rathor wrote:
>>
>> @@ -413,15 +417,18 @@ 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
>> +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.
>
> And you ripped out the nice 'N.B' comment I added. Sad :-(
I think I removed that.
I don't think passing parameters to a function is particularly subtle
and this comment is largely superseded by the comment for
xen_pvh_early_cpu_init() itself.
>> +#ifdef CONFIG_XEN_PVH
>> +/*
>> + * xen_pvh_early_cpu_init() - early PVH VCPU initialization
>> + * @cpu: this cpu number (%rdi)
>> + * @flag: boolean flag true to indicate this is a secondary vcpu coming up
>> + *on this entry point or the primary cpu coming back online.
>
> Why do we do this? Why not just piggyback on the first parameter - the 'cpu'?
>
> If it is zero it is boot CPU.
"Changes in v5 (Mukesh):
- Jan reminded us that vcpu 0 could go offline/online. So, add flag back"
David
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Xen-devel] [V5 PATCH 1/1] x86/xen: Set EFER.NX and EFER.SCE in PVH guests
On 12/09/14 21:42, Konrad Rzeszutek Wilk wrote:
On Wed, Sep 10, 2014 at 04:36:06PM -0700, Mukesh Rathor wrote:
@@ -413,15 +417,18 @@ 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
+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.
And you ripped out the nice 'N.B' comment I added. Sad :-(
I think I removed that.
I don't think passing parameters to a function is particularly subtle
and this comment is largely superseded by the comment for
xen_pvh_early_cpu_init() itself.
+#ifdef CONFIG_XEN_PVH
+/*
+ * xen_pvh_early_cpu_init() - early PVH VCPU initialization
+ * @cpu: this cpu number (%rdi)
+ * @flag: boolean flag true to indicate this is a secondary vcpu coming up
+ *on this entry point or the primary cpu coming back online.
Why do we do this? Why not just piggyback on the first parameter - the 'cpu'?
If it is zero it is boot CPU.
Changes in v5 (Mukesh):
- Jan reminded us that vcpu 0 could go offline/online. So, add flag back
David
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Xen-devel] [V5 PATCH 1/1] x86/xen: Set EFER.NX and EFER.SCE in PVH guests
On Mon, Sep 15, 2014 at 03:45:53PM +0100, David Vrabel wrote:
On 12/09/14 21:42, Konrad Rzeszutek Wilk wrote:
On Wed, Sep 10, 2014 at 04:36:06PM -0700, Mukesh Rathor wrote:
@@ -413,15 +417,18 @@ 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
+ 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.
And you ripped out the nice 'N.B' comment I added. Sad :-(
I think I removed that.
I don't think passing parameters to a function is particularly subtle
and this comment is largely superseded by the comment for
xen_pvh_early_cpu_init() itself.
Good point.
+#ifdef CONFIG_XEN_PVH
+/*
+ * xen_pvh_early_cpu_init() - early PVH VCPU initialization
+ * @cpu: this cpu number (%rdi)
+ * @flag: boolean flag true to indicate this is a secondary vcpu coming up
+ *on this entry point or the primary cpu coming back online.
Why do we do this? Why not just piggyback on the first parameter - the
'cpu'?
If it is zero it is boot CPU.
Changes in v5 (Mukesh):
- Jan reminded us that vcpu 0 could go offline/online. So, add flag back
Right, totally forgot about that.
With that I think I think you can tack on 'Reviewed-by: Konrad Rzeszutek
Wilk konrad.w...@oracle.com'.
Granted the PVH ABI doc needs to go in Xen base first as David requested.
Even a draft that explains some of this would be good.
CC-ing Roger - perhaps v1 of the draft could be posted and we can flesh it
out more?
David
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Xen-devel] [V5 PATCH 1/1] x86/xen: Set EFER.NX and EFER.SCE in PVH guests
On Fri, 12 Sep 2014 16:42:58 -0400 Konrad Rzeszutek Wilk konrad.w...@oracle.com wrote: On Wed, Sep 10, 2014 at 04:36:06PM -0700, Mukesh Rathor wrote: sorry, i didn't realize you had more comments... didn't scroll down :).. cpumask_var_t xen_cpu_initialized_map; @@ -99,10 +100,14 @@ 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. + */ Thank you for expanding on that (I totally forgot why we did that). sure. +* 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. And you ripped out the nice 'N.B' comment I added. Sad :-( */ + ctxt-user_regs.eip = (unsigned long)xen_pvh_early_cpu_init; ctxt-user_regs.rdi = cpu; + ctxt-user_regs.rsi = true; /* secondary cpu == true */ Oh, that is new. Ah yes we can use that [looking at Xen code]. I wonder what other registers we can use to pass stuff around. All GPRs. I commented that we can do that in the Rogers PVH doc. Looks like David responded to other comments. Thanks, Mukesh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Xen-devel] [V5 PATCH 1/1] x86/xen: Set EFER.NX and EFER.SCE in PVH guests
On Fri, 12 Sep 2014 16:42:58 -0400
Konrad Rzeszutek Wilk wrote:
> On Wed, Sep 10, 2014 at 04:36:06PM -0700, Mukesh Rathor wrote:
> > 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
> > Signed-off-by: David Vrabel
>
> Huh? So who wrote it? Or did you mean 'Reviewed-by'?
No, meant SOB. I wrote v1, v2, then David came up with V3 and v4,
then i took comments from v4 and came up with v5.
-Mukesh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Xen-devel] [V5 PATCH 1/1] x86/xen: Set EFER.NX and EFER.SCE in PVH guests
On Wed, Sep 10, 2014 at 04:36:06PM -0700, Mukesh Rathor wrote:
> 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
> Signed-off-by: David Vrabel
Huh? So who wrote it? Or did you mean 'Reviewed-by'?
> ---
> arch/x86/xen/enlighten.c | 6 ++
> arch/x86/xen/smp.c | 29 ++---
> arch/x86/xen/smp.h | 8
> arch/x86/xen/xen-head.S | 33 +
> 4 files changed, 65 insertions(+), 11 deletions(-)
>
> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> index c0cb11f..424d831 100644
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
> @@ -1463,6 +1463,7 @@ static void __ref xen_setup_gdt(int cpu)
> pv_cpu_ops.load_gdt = xen_load_gdt;
> }
>
> +#ifdef CONFIG_XEN_PVH
> /*
> * A PV guest starts with default flags that are not set for PVH, set them
> * here asap.
> @@ -1508,12 +1509,15 @@ static void __init xen_pvh_early_guest_init(void)
> return;
>
> xen_have_vector_callback = 1;
> +
> + xen_pvh_early_cpu_init(0, false);
> xen_pvh_set_cr_flags(0);
>
> #ifdef CONFIG_X86_32
> BUG(); /* PVH: Implement proper support. */
> #endif
> }
> +#endif/* CONFIG_XEN_PVH */
>
> /* First C function to be called on Xen boot */
> asmlinkage __visible void __init xen_start_kernel(void)
> @@ -1527,7 +1531,9 @@ asmlinkage __visible void __init xen_start_kernel(void)
> xen_domain_type = XEN_PV_DOMAIN;
>
> xen_setup_features();
> +#ifdef CONFIG_XEN_PVH
> xen_pvh_early_guest_init();
> +#endif
> xen_setup_machphys_mapping();
>
> /* Install Xen paravirt ops */
> diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
> index 7005974..b25f8942 100644
> --- a/arch/x86/xen/smp.c
> +++ b/arch/x86/xen/smp.c
> @@ -37,6 +37,7 @@
> #include
> #include "xen-ops.h"
> #include "mmu.h"
> +#include "smp.h"
>
> cpumask_var_t xen_cpu_initialized_map;
>
> @@ -99,10 +100,14 @@ 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.
> + */
Thank you for expanding on that (I totally forgot why we did that).
> +asmlinkage __visible void cpu_bringup_and_idle(int cpu)
> {
> -#ifdef CONFIG_X86_64
> +#ifdef CONFIG_XEN_PVH
> if (xen_feature(XENFEAT_auto_translated_physmap) &&
> xen_feature(XENFEAT_supervisor_mode_kernel))
> xen_pvh_secondary_vcpu_init(cpu);
> @@ -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(>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,18 @@ 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
> + 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.
And you ripped out the nice 'N.B' comment I added. Sad :-(
>*/
> + ctxt->user_regs.eip = (unsigned
Re: [Xen-devel] [V5 PATCH 1/1] x86/xen: Set EFER.NX and EFER.SCE in PVH guests
On Wed, Sep 10, 2014 at 04:36:06PM -0700, Mukesh Rathor wrote:
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.rat...@oracle.com
Signed-off-by: David Vrabel david.vra...@citrix.com
Huh? So who wrote it? Or did you mean 'Reviewed-by'?
---
arch/x86/xen/enlighten.c | 6 ++
arch/x86/xen/smp.c | 29 ++---
arch/x86/xen/smp.h | 8
arch/x86/xen/xen-head.S | 33 +
4 files changed, 65 insertions(+), 11 deletions(-)
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index c0cb11f..424d831 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1463,6 +1463,7 @@ static void __ref xen_setup_gdt(int cpu)
pv_cpu_ops.load_gdt = xen_load_gdt;
}
+#ifdef CONFIG_XEN_PVH
/*
* A PV guest starts with default flags that are not set for PVH, set them
* here asap.
@@ -1508,12 +1509,15 @@ static void __init xen_pvh_early_guest_init(void)
return;
xen_have_vector_callback = 1;
+
+ xen_pvh_early_cpu_init(0, false);
xen_pvh_set_cr_flags(0);
#ifdef CONFIG_X86_32
BUG(); /* PVH: Implement proper support. */
#endif
}
+#endif/* CONFIG_XEN_PVH */
/* First C function to be called on Xen boot */
asmlinkage __visible void __init xen_start_kernel(void)
@@ -1527,7 +1531,9 @@ asmlinkage __visible void __init xen_start_kernel(void)
xen_domain_type = XEN_PV_DOMAIN;
xen_setup_features();
+#ifdef CONFIG_XEN_PVH
xen_pvh_early_guest_init();
+#endif
xen_setup_machphys_mapping();
/* Install Xen paravirt ops */
diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
index 7005974..b25f8942 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,10 +100,14 @@ 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.
+ */
Thank you for expanding on that (I totally forgot why we did that).
+asmlinkage __visible void cpu_bringup_and_idle(int cpu)
{
-#ifdef CONFIG_X86_64
+#ifdef CONFIG_XEN_PVH
if (xen_feature(XENFEAT_auto_translated_physmap)
xen_feature(XENFEAT_supervisor_mode_kernel))
xen_pvh_secondary_vcpu_init(cpu);
@@ -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,18 @@ 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
+ 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.
And you ripped out the nice 'N.B' comment I added. Sad :-(
*/
+ ctxt-user_regs.eip = (unsigned long)xen_pvh_early_cpu_init;
ctxt-user_regs.rdi = cpu;
+
Re: [Xen-devel] [V5 PATCH 1/1] x86/xen: Set EFER.NX and EFER.SCE in PVH guests
On Fri, 12 Sep 2014 16:42:58 -0400 Konrad Rzeszutek Wilk konrad.w...@oracle.com wrote: On Wed, Sep 10, 2014 at 04:36:06PM -0700, Mukesh Rathor wrote: 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.rat...@oracle.com Signed-off-by: David Vrabel david.vra...@citrix.com Huh? So who wrote it? Or did you mean 'Reviewed-by'? No, meant SOB. I wrote v1, v2, then David came up with V3 and v4, then i took comments from v4 and came up with v5. -Mukesh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[V5 PATCH 1/1] x86/xen: Set EFER.NX and EFER.SCE in PVH guests
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
Signed-off-by: David Vrabel
---
arch/x86/xen/enlighten.c | 6 ++
arch/x86/xen/smp.c | 29 ++---
arch/x86/xen/smp.h | 8
arch/x86/xen/xen-head.S | 33 +
4 files changed, 65 insertions(+), 11 deletions(-)
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index c0cb11f..424d831 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1463,6 +1463,7 @@ static void __ref xen_setup_gdt(int cpu)
pv_cpu_ops.load_gdt = xen_load_gdt;
}
+#ifdef CONFIG_XEN_PVH
/*
* A PV guest starts with default flags that are not set for PVH, set them
* here asap.
@@ -1508,12 +1509,15 @@ static void __init xen_pvh_early_guest_init(void)
return;
xen_have_vector_callback = 1;
+
+ xen_pvh_early_cpu_init(0, false);
xen_pvh_set_cr_flags(0);
#ifdef CONFIG_X86_32
BUG(); /* PVH: Implement proper support. */
#endif
}
+#endif/* CONFIG_XEN_PVH */
/* First C function to be called on Xen boot */
asmlinkage __visible void __init xen_start_kernel(void)
@@ -1527,7 +1531,9 @@ asmlinkage __visible void __init xen_start_kernel(void)
xen_domain_type = XEN_PV_DOMAIN;
xen_setup_features();
+#ifdef CONFIG_XEN_PVH
xen_pvh_early_guest_init();
+#endif
xen_setup_machphys_mapping();
/* Install Xen paravirt ops */
diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
index 7005974..b25f8942 100644
--- a/arch/x86/xen/smp.c
+++ b/arch/x86/xen/smp.c
@@ -37,6 +37,7 @@
#include
#include "xen-ops.h"
#include "mmu.h"
+#include "smp.h"
cpumask_var_t xen_cpu_initialized_map;
@@ -99,10 +100,14 @@ 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
+#ifdef CONFIG_XEN_PVH
if (xen_feature(XENFEAT_auto_translated_physmap) &&
xen_feature(XENFEAT_supervisor_mode_kernel))
xen_pvh_secondary_vcpu_init(cpu);
@@ -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(>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,18 @@ 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
+ 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_early_cpu_init;
ctxt->user_regs.rdi = cpu;
+ ctxt->user_regs.rsi = true; /* 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..04a83a7 100644
--- a/arch/x86/xen/smp.h
+++
[V5 PATCH 1/1] x86/xen: Set EFER.NX and EFER.SCE in PVH guests
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.rat...@oracle.com
Signed-off-by: David Vrabel david.vra...@citrix.com
---
arch/x86/xen/enlighten.c | 6 ++
arch/x86/xen/smp.c | 29 ++---
arch/x86/xen/smp.h | 8
arch/x86/xen/xen-head.S | 33 +
4 files changed, 65 insertions(+), 11 deletions(-)
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index c0cb11f..424d831 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1463,6 +1463,7 @@ static void __ref xen_setup_gdt(int cpu)
pv_cpu_ops.load_gdt = xen_load_gdt;
}
+#ifdef CONFIG_XEN_PVH
/*
* A PV guest starts with default flags that are not set for PVH, set them
* here asap.
@@ -1508,12 +1509,15 @@ static void __init xen_pvh_early_guest_init(void)
return;
xen_have_vector_callback = 1;
+
+ xen_pvh_early_cpu_init(0, false);
xen_pvh_set_cr_flags(0);
#ifdef CONFIG_X86_32
BUG(); /* PVH: Implement proper support. */
#endif
}
+#endif/* CONFIG_XEN_PVH */
/* First C function to be called on Xen boot */
asmlinkage __visible void __init xen_start_kernel(void)
@@ -1527,7 +1531,9 @@ asmlinkage __visible void __init xen_start_kernel(void)
xen_domain_type = XEN_PV_DOMAIN;
xen_setup_features();
+#ifdef CONFIG_XEN_PVH
xen_pvh_early_guest_init();
+#endif
xen_setup_machphys_mapping();
/* Install Xen paravirt ops */
diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
index 7005974..b25f8942 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,10 +100,14 @@ 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
+#ifdef CONFIG_XEN_PVH
if (xen_feature(XENFEAT_auto_translated_physmap)
xen_feature(XENFEAT_supervisor_mode_kernel))
xen_pvh_secondary_vcpu_init(cpu);
@@ -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,18 @@ 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
+ 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_early_cpu_init;
ctxt-user_regs.rdi = cpu;
+ ctxt-user_regs.rsi = true; /* 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..04a83a7 100644
