thr3ads.net - Xen devel - [PATCH 1/3] X86: MPX support for HVM guest [Nov 2013]

If this information is useful, please help other people find it:
Share via:

Liu, Jinsong

2013-Nov-11 08:38 UTC

[PATCH 1/3] X86: MPX support for HVM guest

From be65afbcf37ca7349ab657f552859ec95a872268 Mon Sep 17 00:00:00 2001
From: Liu Jinsong <jinsong.liu@intel.com>
Date: Fri, 8 Nov 2013 00:49:41 +0800
Subject: [PATCH 1/3] X86: MPX support for HVM guest

Signed-off-by: Xudong Hao <xudong.hao@intel.com>
Acked-by: Liu Jinsong <jinsong.liu@intel.com>

---
 tools/libxc/xc_cpufeature.h      |    1 +
 tools/libxc/xc_cpuid_x86.c       |    4 +++-
 xen/arch/x86/xstate.c            |    6 ++++++
 xen/include/asm-x86/cpufeature.h |    2 ++
 xen/include/asm-x86/xstate.h     |    7 +++++--
 5 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/tools/libxc/xc_cpufeature.h b/tools/libxc/xc_cpufeature.h
index c464e3a..cd7befc 100644
--- a/tools/libxc/xc_cpufeature.h
+++ b/tools/libxc/xc_cpufeature.h
@@ -137,5 +137,6 @@
 #define X86_FEATURE_ERMS         9 /* Enhanced REP MOVSB/STOSB */
 #define X86_FEATURE_INVPCID     10 /* Invalidate Process Context ID */
 #define X86_FEATURE_RTM         11 /* Restricted Transactional Memory */
+#define X86_FEATURE_MPX         14 /* Memory Protection Extensions */
 
 #endif /* __LIBXC_CPUFEATURE_H */
diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c
index bbbf9b8..bcb7a59 100644
--- a/tools/libxc/xc_cpuid_x86.c
+++ b/tools/libxc/xc_cpuid_x86.c
@@ -371,7 +371,9 @@ static void xc_cpuid_hvm_policy(
                         bitmaskof(X86_FEATURE_ERMS) |
                         bitmaskof(X86_FEATURE_INVPCID) |
                         bitmaskof(X86_FEATURE_RTM)  |
-                        bitmaskof(X86_FEATURE_FSGSBASE));
+                        bitmaskof(X86_FEATURE_FSGSBASE) |
+                        bitmaskof(X86_FEATURE_MPX));
+
         } else
             regs[1] = 0;
         regs[0] = regs[2] = regs[3] = 0;
diff --git a/xen/arch/x86/xstate.c b/xen/arch/x86/xstate.c
index 9e74929..9f451b8 100644
--- a/xen/arch/x86/xstate.c
+++ b/xen/arch/x86/xstate.c
@@ -357,6 +357,9 @@ int handle_xsetbv(u32 index, u64 new_bv)
     if ( (new_bv & XSTATE_YMM) && !(new_bv & XSTATE_SSE) )
         return -EINVAL;
 
+    if ( (!!(XSTATE_BNDREGS & new_bv)) != (!!(XSTATE_BNDCSR & new_bv))
)
+        return -EINVAL;
+
     if ( !set_xcr0(new_bv) )
         return -EFAULT;
 
@@ -377,6 +380,9 @@ int handle_xsetbv(u32 index, u64 new_bv)
             write_cr0(cr0);
     }
 
+    if ( xsave_enabled(curr) && cpu_has_mpx )
+        curr->arch.nonlazy_xstate_used = !!(cpu_has_mpx);
+
     return 0;
 }
 
diff --git a/xen/include/asm-x86/cpufeature.h b/xen/include/asm-x86/cpufeature.h
index 1cfaf94..ced03b4 100644
--- a/xen/include/asm-x86/cpufeature.h
+++ b/xen/include/asm-x86/cpufeature.h
@@ -148,6 +148,7 @@
 #define X86_FEATURE_INVPCID	(7*32+10) /* Invalidate Process Context ID */
 #define X86_FEATURE_RTM 	(7*32+11) /* Restricted Transactional Memory */
 #define X86_FEATURE_NO_FPU_SEL 	(7*32+13) /* FPU CS/DS stored as zero */
+#define X86_FEATURE_MPX         (7*32+14) /* Memory Protection Extensions */
 #define X86_FEATURE_SMAP	(7*32+20) /* Supervisor Mode Access Prevention */
 
 #define cpu_has(c, bit)		test_bit(bit, (c)->x86_capability)
@@ -197,6 +198,7 @@
 #define cpu_has_xsave           boot_cpu_has(X86_FEATURE_XSAVE)
 #define cpu_has_avx             boot_cpu_has(X86_FEATURE_AVX)
 #define cpu_has_lwp             boot_cpu_has(X86_FEATURE_LWP)
+#define cpu_has_mpx             boot_cpu_has(X86_FEATURE_MPX)
 
 #define cpu_has_arch_perfmon    boot_cpu_has(X86_FEATURE_ARCH_PERFMON)
 
diff --git a/xen/include/asm-x86/xstate.h b/xen/include/asm-x86/xstate.h
index 5617963..81b3d29 100644
--- a/xen/include/asm-x86/xstate.h
+++ b/xen/include/asm-x86/xstate.h
@@ -26,12 +26,15 @@
 #define XSTATE_FP      (1ULL << 0)
 #define XSTATE_SSE     (1ULL << 1)
 #define XSTATE_YMM     (1ULL << 2)
+#define XSTATE_BNDREGS (1ULL << 3)
+#define XSTATE_BNDCSR  (1ULL << 4)
 #define XSTATE_LWP     (1ULL << 62) /* AMD lightweight profiling */
 #define XSTATE_FP_SSE  (XSTATE_FP | XSTATE_SSE)
-#define XCNTXT_MASK    (XSTATE_FP | XSTATE_SSE | XSTATE_YMM | XSTATE_LWP)
+#define XCNTXT_MASK    (XSTATE_FP | XSTATE_SSE | XSTATE_YMM | \
+                        XSTATE_BNDREGS | XSTATE_BNDCSR | XSTATE_LWP)
 
 #define XSTATE_ALL     (~0)
-#define XSTATE_NONLAZY (XSTATE_LWP)
+#define XSTATE_NONLAZY (XSTATE_LWP | XSTATE_BNDREGS | XSTATE_BNDCSR)
 #define XSTATE_LAZY    (XSTATE_ALL & ~XSTATE_NONLAZY)
 
 extern u64 xfeature_mask;
-- 
1.7.1


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

Jan Beulich

2013-Nov-11 10:22 UTC

head link

Re: [PATCH 1/3] X86: MPX support for HVM guest

>>> On 11.11.13 at 09:38, "Liu, Jinsong"
<jinsong.liu@intel.com> wrote:
> From be65afbcf37ca7349ab657f552859ec95a872268 Mon Sep 17 00:00:00 2001
> From: Liu Jinsong <jinsong.liu@intel.com>
> Date: Fri, 8 Nov 2013 00:49:41 +0800
> Subject: [PATCH 1/3] X86: MPX support for HVM guest
> 
> Signed-off-by: Xudong Hao <xudong.hao@intel.com>
> Acked-by: Liu Jinsong <jinsong.liu@intel.com>
First of all - please consider using the (to be slightly extended)
patch attached instead of the hypervisor parts of this one. (You
likely also recall that it''s generally easier for the maintainers if
you submit hypervisor and tools changes in separate patches.)
> --- a/xen/arch/x86/xstate.c
> +++ b/xen/arch/x86/xstate.c
> @@ -357,6 +357,9 @@ int handle_xsetbv(u32 index, u64 new_bv)
>      if ( (new_bv & XSTATE_YMM) && !(new_bv & XSTATE_SSE) )
>          return -EINVAL;
>  
> +    if ( (!!(XSTATE_BNDREGS & new_bv)) != (!!(XSTATE_BNDCSR &
new_bv)) )
> +        return -EINVAL;
A single ! each would fully suffice. And a similar check - see
attached patch - is needed in validate_xstate() too.
> @@ -377,6 +380,9 @@ int handle_xsetbv(u32 index, u64 new_bv)
>              write_cr0(cr0);
>      }
>  
> +    if ( xsave_enabled(curr) && cpu_has_mpx )
> +        curr->arch.nonlazy_xstate_used = !!(cpu_has_mpx);
So you set the flag no matter whether the guest enables the
feature? That''s surely too eager. (And using !!cpu_has_mpx in
the body of a conditional having checked that cpu_has_mpx is
non-zero is dubious too.)

But yes, something along those lines is what my patch is
currently lacking (apart from the MSR related aspects mentioned
at its top). The main question is whether it is really necessary for
the flag to be sticky.

Jan



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

Liu, Jinsong

2013-Nov-15 16:27 UTC

head link

Re: [PATCH 1/3] X86: MPX support for HVM guest

Jan Beulich wrote:>>>> On 11.11.13 at 09:38, "Liu, Jinsong"
<jinsong.liu@intel.com> wrote:
>> From be65afbcf37ca7349ab657f552859ec95a872268 Mon Sep 17 00:00:00
>> 2001 From: Liu Jinsong <jinsong.liu@intel.com>
>> Date: Fri, 8 Nov 2013 00:49:41 +0800
>> Subject: [PATCH 1/3] X86: MPX support for HVM guest
>> 
>> Signed-off-by: Xudong Hao <xudong.hao@intel.com>
>> Acked-by: Liu Jinsong <jinsong.liu@intel.com>
> 
> First of all - please consider using the (to be slightly extended)
> patch attached instead of the hypervisor parts of this one. (You
> likely also recall that it''s generally easier for the maintainers
if
> you submit hypervisor and tools changes in separate patches.)
> 
>> --- a/xen/arch/x86/xstate.c
>> +++ b/xen/arch/x86/xstate.c
>> @@ -357,6 +357,9 @@ int handle_xsetbv(u32 index, u64 new_bv)
>>      if ( (new_bv & XSTATE_YMM) && !(new_bv &
XSTATE_SSE) )
>> return -EINVAL; 
>> 
>> +    if ( (!!(XSTATE_BNDREGS & new_bv)) != (!!(XSTATE_BNDCSR &
>> new_bv)) ) +        return -EINVAL;
> 
> A single ! each would fully suffice. And a similar check - see
> attached patch - is needed in validate_xstate() too.
OK, I will rebase patches based on your attached patch.

Thanks,
Jinsong

Xen devel - Nov 2013 - [PATCH 1/3] X86: MPX support for HVM guest

[PATCH 1/3] X86: MPX support for HVM guest

Re: [PATCH 1/3] X86: MPX support for HVM guest

Re: [PATCH 1/3] X86: MPX support for HVM guest