Re: [Xen-devel] [V0 PATCH 1/2] AMD-PVH: set EFER.NX and EFER.SCE for the boot vcpu
On Fri, 22 Aug 2014 12:09:27 -0700 Mukesh Rathor wrote: > On Fri, 22 Aug 2014 06:41:40 +0200 > Borislav Petkov wrote: > > > On Thu, Aug 21, 2014 at 07:46:56PM -0700, Mukesh Rathor wrote: > > > Intel doesn't have EFER.NX bit. > > > > Of course it does. > > > > Right, it does. Some code/comment is misleading... Anyways, reading > intel SDMs, if I understand the convoluted text correctly, EFER.NX is > not required to be set for l1.nx to be set, thus allowing for page > level protection. Where as on AMD, EFER.NX must be set for l1.nx to > be used. So, in the end, this patch would apply to both amd/intel > > I'll reword and submit. Err, try again, the section "4.1.1 Three Paging Modes" says: "Execute-disable access rights are applied only if IA32_EFER.NXE = 1" So, I guess NX is broken on Intel PVH because EFER.NX is currently not being set. While AMD will #GP if l1.NX is set and EFER.NX is not, I guess Intel just ignores the l1.XD if EFER.NX is not set. 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] [V0 PATCH 1/2] AMD-PVH: set EFER.NX and EFER.SCE for the boot vcpu
On Fri, 22 Aug 2014 06:41:40 +0200 Borislav Petkov wrote: > On Thu, Aug 21, 2014 at 07:46:56PM -0700, Mukesh Rathor wrote: > > Intel doesn't have EFER.NX bit. > > Of course it does. > Right, it does. Some code/comment is misleading... Anyways, reading intel SDMs, if I understand the convoluted text correctly, EFER.NX is not required to be set for l1.nx to be set, thus allowing for page level protection. Where as on AMD, EFER.NX must be set for l1.nx to be used. So, in the end, this patch would apply to both amd/intel I'll reword and submit. 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] [V0 PATCH 1/2] AMD-PVH: set EFER.NX and EFER.SCE for the boot vcpu
On Thu, Aug 21, 2014 at 07:46:56PM -0700, Mukesh Rathor wrote:
> On Thu, 21 Aug 2014 21:39:04 -0400
> Konrad Rzeszutek Wilk wrote:
>
> > On Wed, Aug 20, 2014 at 07:16:39PM -0700, Mukesh Rathor wrote:
> > > On AMD, NX feature must be enabled in the efer for NX to be honored
> > > in the pte entries, otherwise protection fault. We also set SC for
> > > system calls to be enabled.
> >
> > How come we don't need to do that for Intel (that is set the NX bit)?
> > Could you include the explanation here please?
>
> Intel doesn't have EFER.NX bit. The SC bit is being set in xen, but it
> doesn't need to be, and I'm going to submit a patch to undo it.
I understand that it does not have an EFER.NX bit. What I was trying
to figure out is _where_ do we set the NX bit for Intel CPUs? Is
it done in the hypervisor? In the Linux code?
Thank you.
>
> >
> > >
> > > Signed-off-by: Mukesh Rathor
> > > ---
> > > arch/x86/xen/enlighten.c | 12
> > > 1 file changed, 12 insertions(+)
> > >
> > > diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> > > index c0cb11f..4af512d 100644
> > > --- a/arch/x86/xen/enlighten.c
> > > +++ b/arch/x86/xen/enlighten.c
> > > @@ -1499,6 +1499,17 @@ void __ref xen_pvh_secondary_vcpu_init(int
> > > cpu) xen_pvh_set_cr_flags(cpu);
> > > }
> > >
> > > +/* This is done in secondary_startup_64 for hvm guests. */
> > > +static void __init xen_configure_efer(void)
> > > +{
> > > + u64 efer;
> > > +
> > > + rdmsrl(MSR_EFER, efer);
> > > + efer |= EFER_SCE;
> > > + efer |= (cpuid_edx(0x8001) & (1 << 20)) ? EFER_NX : 0;
> >
> > Ahem? #defines for these magic values please?
>
> Linux uses these directly all over the code as they are set in stone
> pretty much, and I didn't find any #defines. See cpu/common.c for one of
> the places. Also see secondary_startup_64, and others...
>
> > Or could you use 'boot_cpu_has'?
>
> Nop, it's not initialized at this point.
>
> 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] [V0 PATCH 1/2] AMD-PVH: set EFER.NX and EFER.SCE for the boot vcpu
On Fri, 22 Aug 2014 12:09:27 -0700 Mukesh Rathor mukesh.rat...@oracle.com wrote: On Fri, 22 Aug 2014 06:41:40 +0200 Borislav Petkov b...@alien8.de wrote: On Thu, Aug 21, 2014 at 07:46:56PM -0700, Mukesh Rathor wrote: Intel doesn't have EFER.NX bit. Of course it does. Right, it does. Some code/comment is misleading... Anyways, reading intel SDMs, if I understand the convoluted text correctly, EFER.NX is not required to be set for l1.nx to be set, thus allowing for page level protection. Where as on AMD, EFER.NX must be set for l1.nx to be used. So, in the end, this patch would apply to both amd/intel I'll reword and submit. Err, try again, the section 4.1.1 Three Paging Modes says: Execute-disable access rights are applied only if IA32_EFER.NXE = 1 So, I guess NX is broken on Intel PVH because EFER.NX is currently not being set. While AMD will #GP if l1.NX is set and EFER.NX is not, I guess Intel just ignores the l1.XD if EFER.NX is not set. 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] [V0 PATCH 1/2] AMD-PVH: set EFER.NX and EFER.SCE for the boot vcpu
On Thu, Aug 21, 2014 at 07:46:56PM -0700, Mukesh Rathor wrote:
On Thu, 21 Aug 2014 21:39:04 -0400
Konrad Rzeszutek Wilk konrad.w...@oracle.com wrote:
On Wed, Aug 20, 2014 at 07:16:39PM -0700, Mukesh Rathor wrote:
On AMD, NX feature must be enabled in the efer for NX to be honored
in the pte entries, otherwise protection fault. We also set SC for
system calls to be enabled.
How come we don't need to do that for Intel (that is set the NX bit)?
Could you include the explanation here please?
Intel doesn't have EFER.NX bit. The SC bit is being set in xen, but it
doesn't need to be, and I'm going to submit a patch to undo it.
I understand that it does not have an EFER.NX bit. What I was trying
to figure out is _where_ do we set the NX bit for Intel CPUs? Is
it done in the hypervisor? In the Linux code?
Thank you.
Signed-off-by: Mukesh Rathor mukesh.rat...@oracle.com
---
arch/x86/xen/enlighten.c | 12
1 file changed, 12 insertions(+)
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index c0cb11f..4af512d 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1499,6 +1499,17 @@ void __ref xen_pvh_secondary_vcpu_init(int
cpu) xen_pvh_set_cr_flags(cpu);
}
+/* This is done in secondary_startup_64 for hvm guests. */
+static void __init xen_configure_efer(void)
+{
+ u64 efer;
+
+ rdmsrl(MSR_EFER, efer);
+ efer |= EFER_SCE;
+ efer |= (cpuid_edx(0x8001) (1 20)) ? EFER_NX : 0;
Ahem? #defines for these magic values please?
Linux uses these directly all over the code as they are set in stone
pretty much, and I didn't find any #defines. See cpu/common.c for one of
the places. Also see secondary_startup_64, and others...
Or could you use 'boot_cpu_has'?
Nop, it's not initialized at this point.
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] [V0 PATCH 1/2] AMD-PVH: set EFER.NX and EFER.SCE for the boot vcpu
On Fri, 22 Aug 2014 06:41:40 +0200 Borislav Petkov b...@alien8.de wrote: On Thu, Aug 21, 2014 at 07:46:56PM -0700, Mukesh Rathor wrote: Intel doesn't have EFER.NX bit. Of course it does. Right, it does. Some code/comment is misleading... Anyways, reading intel SDMs, if I understand the convoluted text correctly, EFER.NX is not required to be set for l1.nx to be set, thus allowing for page level protection. Where as on AMD, EFER.NX must be set for l1.nx to be used. So, in the end, this patch would apply to both amd/intel I'll reword and submit. 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] [V0 PATCH 1/2] AMD-PVH: set EFER.NX and EFER.SCE for the boot vcpu
On Thu, Aug 21, 2014 at 07:46:56PM -0700, Mukesh Rathor wrote: > Intel doesn't have EFER.NX bit. Of course it does. -- Regards/Gruss, Boris. Sent from a fat crate under my desk. Formatting is fine. -- -- 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] [V0 PATCH 1/2] AMD-PVH: set EFER.NX and EFER.SCE for the boot vcpu
On Thu, 21 Aug 2014 21:39:04 -0400
Konrad Rzeszutek Wilk wrote:
> On Wed, Aug 20, 2014 at 07:16:39PM -0700, Mukesh Rathor wrote:
> > On AMD, NX feature must be enabled in the efer for NX to be honored
> > in the pte entries, otherwise protection fault. We also set SC for
> > system calls to be enabled.
>
> How come we don't need to do that for Intel (that is set the NX bit)?
> Could you include the explanation here please?
Intel doesn't have EFER.NX bit. The SC bit is being set in xen, but it
doesn't need to be, and I'm going to submit a patch to undo it.
>
> >
> > Signed-off-by: Mukesh Rathor
> > ---
> > arch/x86/xen/enlighten.c | 12
> > 1 file changed, 12 insertions(+)
> >
> > diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> > index c0cb11f..4af512d 100644
> > --- a/arch/x86/xen/enlighten.c
> > +++ b/arch/x86/xen/enlighten.c
> > @@ -1499,6 +1499,17 @@ void __ref xen_pvh_secondary_vcpu_init(int
> > cpu) xen_pvh_set_cr_flags(cpu);
> > }
> >
> > +/* This is done in secondary_startup_64 for hvm guests. */
> > +static void __init xen_configure_efer(void)
> > +{
> > + u64 efer;
> > +
> > + rdmsrl(MSR_EFER, efer);
> > + efer |= EFER_SCE;
> > + efer |= (cpuid_edx(0x8001) & (1 << 20)) ? EFER_NX : 0;
>
> Ahem? #defines for these magic values please?
Linux uses these directly all over the code as they are set in stone
pretty much, and I didn't find any #defines. See cpu/common.c for one of
the places. Also see secondary_startup_64, and others...
> Or could you use 'boot_cpu_has'?
Nop, it's not initialized at this point.
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] [V0 PATCH 1/2] AMD-PVH: set EFER.NX and EFER.SCE for the boot vcpu
On Wed, Aug 20, 2014 at 07:16:39PM -0700, Mukesh Rathor wrote:
> On AMD, NX feature must be enabled in the efer for NX to be honored in
> the pte entries, otherwise protection fault. We also set SC for
> system calls to be enabled.
How come we don't need to do that for Intel (that is set the NX bit)?
Could you include the explanation here please?
>
> Signed-off-by: Mukesh Rathor
> ---
> arch/x86/xen/enlighten.c | 12
> 1 file changed, 12 insertions(+)
>
> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> index c0cb11f..4af512d 100644
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
> @@ -1499,6 +1499,17 @@ void __ref xen_pvh_secondary_vcpu_init(int cpu)
> xen_pvh_set_cr_flags(cpu);
> }
>
> +/* This is done in secondary_startup_64 for hvm guests. */
> +static void __init xen_configure_efer(void)
> +{
> + u64 efer;
> +
> + rdmsrl(MSR_EFER, efer);
> + efer |= EFER_SCE;
> + efer |= (cpuid_edx(0x8001) & (1 << 20)) ? EFER_NX : 0;
Ahem? #defines for these magic values please?
Or could you use 'boot_cpu_has'?
> + wrmsrl(MSR_EFER, efer);
> +}
> +
> static void __init xen_pvh_early_guest_init(void)
> {
> if (!xen_feature(XENFEAT_auto_translated_physmap))
> @@ -1508,6 +1519,7 @@ static void __init xen_pvh_early_guest_init(void)
> return;
>
> xen_have_vector_callback = 1;
> + xen_configure_efer();
> xen_pvh_set_cr_flags(0);
>
> #ifdef CONFIG_X86_32
> --
> 1.8.3.1
>
>
> ___
> Xen-devel mailing list
> xen-de...@lists.xen.org
> http://lists.xen.org/xen-devel
--
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] [V0 PATCH 1/2] AMD-PVH: set EFER.NX and EFER.SCE for the boot vcpu
On Wed, Aug 20, 2014 at 07:16:39PM -0700, Mukesh Rathor wrote:
On AMD, NX feature must be enabled in the efer for NX to be honored in
the pte entries, otherwise protection fault. We also set SC for
system calls to be enabled.
How come we don't need to do that for Intel (that is set the NX bit)?
Could you include the explanation here please?
Signed-off-by: Mukesh Rathor mukesh.rat...@oracle.com
---
arch/x86/xen/enlighten.c | 12
1 file changed, 12 insertions(+)
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index c0cb11f..4af512d 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1499,6 +1499,17 @@ void __ref xen_pvh_secondary_vcpu_init(int cpu)
xen_pvh_set_cr_flags(cpu);
}
+/* This is done in secondary_startup_64 for hvm guests. */
+static void __init xen_configure_efer(void)
+{
+ u64 efer;
+
+ rdmsrl(MSR_EFER, efer);
+ efer |= EFER_SCE;
+ efer |= (cpuid_edx(0x8001) (1 20)) ? EFER_NX : 0;
Ahem? #defines for these magic values please?
Or could you use 'boot_cpu_has'?
+ wrmsrl(MSR_EFER, efer);
+}
+
static void __init xen_pvh_early_guest_init(void)
{
if (!xen_feature(XENFEAT_auto_translated_physmap))
@@ -1508,6 +1519,7 @@ static void __init xen_pvh_early_guest_init(void)
return;
xen_have_vector_callback = 1;
+ xen_configure_efer();
xen_pvh_set_cr_flags(0);
#ifdef CONFIG_X86_32
--
1.8.3.1
___
Xen-devel mailing list
xen-de...@lists.xen.org
http://lists.xen.org/xen-devel
--
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] [V0 PATCH 1/2] AMD-PVH: set EFER.NX and EFER.SCE for the boot vcpu
On Thu, 21 Aug 2014 21:39:04 -0400
Konrad Rzeszutek Wilk konrad.w...@oracle.com wrote:
On Wed, Aug 20, 2014 at 07:16:39PM -0700, Mukesh Rathor wrote:
On AMD, NX feature must be enabled in the efer for NX to be honored
in the pte entries, otherwise protection fault. We also set SC for
system calls to be enabled.
How come we don't need to do that for Intel (that is set the NX bit)?
Could you include the explanation here please?
Intel doesn't have EFER.NX bit. The SC bit is being set in xen, but it
doesn't need to be, and I'm going to submit a patch to undo it.
Signed-off-by: Mukesh Rathor mukesh.rat...@oracle.com
---
arch/x86/xen/enlighten.c | 12
1 file changed, 12 insertions(+)
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index c0cb11f..4af512d 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1499,6 +1499,17 @@ void __ref xen_pvh_secondary_vcpu_init(int
cpu) xen_pvh_set_cr_flags(cpu);
}
+/* This is done in secondary_startup_64 for hvm guests. */
+static void __init xen_configure_efer(void)
+{
+ u64 efer;
+
+ rdmsrl(MSR_EFER, efer);
+ efer |= EFER_SCE;
+ efer |= (cpuid_edx(0x8001) (1 20)) ? EFER_NX : 0;
Ahem? #defines for these magic values please?
Linux uses these directly all over the code as they are set in stone
pretty much, and I didn't find any #defines. See cpu/common.c for one of
the places. Also see secondary_startup_64, and others...
Or could you use 'boot_cpu_has'?
Nop, it's not initialized at this point.
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] [V0 PATCH 1/2] AMD-PVH: set EFER.NX and EFER.SCE for the boot vcpu
On Thu, Aug 21, 2014 at 07:46:56PM -0700, Mukesh Rathor wrote: Intel doesn't have EFER.NX bit. Of course it does. -- Regards/Gruss, Boris. Sent from a fat crate under my desk. Formatting is fine. -- -- 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/
[V0 PATCH 1/2] AMD-PVH: set EFER.NX and EFER.SCE for the boot vcpu
On AMD, NX feature must be enabled in the efer for NX to be honored in
the pte entries, otherwise protection fault. We also set SC for
system calls to be enabled.
Signed-off-by: Mukesh Rathor
---
arch/x86/xen/enlighten.c | 12
1 file changed, 12 insertions(+)
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index c0cb11f..4af512d 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1499,6 +1499,17 @@ void __ref xen_pvh_secondary_vcpu_init(int cpu)
xen_pvh_set_cr_flags(cpu);
}
+/* This is done in secondary_startup_64 for hvm guests. */
+static void __init xen_configure_efer(void)
+{
+ u64 efer;
+
+ rdmsrl(MSR_EFER, efer);
+ efer |= EFER_SCE;
+ efer |= (cpuid_edx(0x8001) & (1 << 20)) ? EFER_NX : 0;
+ wrmsrl(MSR_EFER, efer);
+}
+
static void __init xen_pvh_early_guest_init(void)
{
if (!xen_feature(XENFEAT_auto_translated_physmap))
@@ -1508,6 +1519,7 @@ static void __init xen_pvh_early_guest_init(void)
return;
xen_have_vector_callback = 1;
+ xen_configure_efer();
xen_pvh_set_cr_flags(0);
#ifdef CONFIG_X86_32
--
1.8.3.1
--
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/
[V0 PATCH 1/2] AMD-PVH: set EFER.NX and EFER.SCE for the boot vcpu
On AMD, NX feature must be enabled in the efer for NX to be honored in
the pte entries, otherwise protection fault. We also set SC for
system calls to be enabled.
Signed-off-by: Mukesh Rathor mukesh.rat...@oracle.com
---
arch/x86/xen/enlighten.c | 12
1 file changed, 12 insertions(+)
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index c0cb11f..4af512d 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1499,6 +1499,17 @@ void __ref xen_pvh_secondary_vcpu_init(int cpu)
xen_pvh_set_cr_flags(cpu);
}
+/* This is done in secondary_startup_64 for hvm guests. */
+static void __init xen_configure_efer(void)
+{
+ u64 efer;
+
+ rdmsrl(MSR_EFER, efer);
+ efer |= EFER_SCE;
+ efer |= (cpuid_edx(0x8001) (1 20)) ? EFER_NX : 0;
+ wrmsrl(MSR_EFER, efer);
+}
+
static void __init xen_pvh_early_guest_init(void)
{
if (!xen_feature(XENFEAT_auto_translated_physmap))
@@ -1508,6 +1519,7 @@ static void __init xen_pvh_early_guest_init(void)
return;
xen_have_vector_callback = 1;
+ xen_configure_efer();
xen_pvh_set_cr_flags(0);
#ifdef CONFIG_X86_32
--
1.8.3.1
--
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/
