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

Re: [Xen-devel] [PATCH 03/15] xen: x86: add early stage SGX feature detection

On 7/20/2017 2:23 AM, Andrew Cooper wrote:
On 09/07/17 09:09, Kai Huang wrote:
This patch adds early stage SGX feature detection via SGX CPUID 0x12. Function
detect_sgx is added to detect SGX info on each CPU (called from vmx_cpu_up).
SDM says SGX info returned by CPUID is per-thread, and we cannot assume all
threads will return the same SGX info, so we have to detect SGX for each CPU.
For simplicity, currently SGX is only supported when all CPUs reports the same
SGX info.

SDM also says it's possible to have multiple EPC sections but this is only for
multiple-socket server, which we don't support now (there are other things
need to be done, ex, NUMA EPC, scheduling, etc, as well), so currently only
one EPC is supported.

Dedicated files sgx.c and sgx.h are added (under vmx directory as SGX is Intel
specific) for bulk of above SGX detection code detection code, and for further
SGX code as well.

Signed-off-by: Kai Huang <kai.huang@xxxxxxxxxxxxxxx>

I am not sure putting this under hvm/ is a sensible move.  Almost
everything in this patch is currently common, and I can forsee us
wanting to introduce PV support, so it would be good to introduce this
in a guest-neutral location to begin with.

  xen/arch/x86/hvm/vmx/Makefile     |   1 +
  xen/arch/x86/hvm/vmx/sgx.c        | 208 ++++++++++++++++++++++++++++++++++++++
  xen/arch/x86/hvm/vmx/vmcs.c       |   4 +
  xen/include/asm-x86/cpufeature.h  |   1 +
  xen/include/asm-x86/hvm/vmx/sgx.h |  45 +++++++++
  5 files changed, 259 insertions(+)
  create mode 100644 xen/arch/x86/hvm/vmx/sgx.c
  create mode 100644 xen/include/asm-x86/hvm/vmx/sgx.h

diff --git a/xen/arch/x86/hvm/vmx/Makefile b/xen/arch/x86/hvm/vmx/Makefile
index 04a29ce59d..f6bcf0d143 100644
--- a/xen/arch/x86/hvm/vmx/Makefile
+++ b/xen/arch/x86/hvm/vmx/Makefile
@@ -4,3 +4,4 @@ obj-y += realmode.o
  obj-y += vmcs.o
  obj-y += vmx.o
  obj-y += vvmx.o
+obj-y += sgx.o
diff --git a/xen/arch/x86/hvm/vmx/sgx.c b/xen/arch/x86/hvm/vmx/sgx.c
new file mode 100644
index 0000000000..6b41469371
--- /dev/null
+++ b/xen/arch/x86/hvm/vmx/sgx.c

This file looks like it should be arch/x86/sgx.c, given its current content.

@@ -0,0 +1,208 @@
+ * Intel Software Guard Extensions support

Please include a GPLv2 header.

+ *
+ * Author: Kai Huang <kai.huang@xxxxxxxxxxxxxxx>
+ */
+#include <asm/cpufeature.h>
+#include <asm/msr-index.h>
+#include <asm/msr.h>
+#include <asm/hvm/vmx/sgx.h>
+#include <asm/hvm/vmx/vmcs.h>
+static struct sgx_cpuinfo __read_mostly sgx_cpudata[NR_CPUS];
+static struct sgx_cpuinfo __read_mostly boot_sgx_cpudata;

I don't think any of this is necessary.  The description says that all
EPCs across the server will be reported in CPUID subleaves, and our
implementation gives up if the data are non-identical across CPUs.

Therefore, we only need to keep one copy of the data, and check check
APs against the master copy.

Right. boot_sgx_cpudata is what we need. Currently detect_sgx is called from vmx_cpu_up. How about changing to calling it from identify_cpu, and something like below ?

        if ( c == &boot_cpu_data )
        else {
                struct sgx_cpuinfo tmp;
                if ( memcmp(&boot_sgx_cpudata, &tmp, sizeof (tmp)) )
                        //disable SGX


Let me see about splitting up a few bits of the existing CPUID
infrastructure, so we can use the host cpuid policy more effectively for
Xen related things.


Xen-devel mailing list

Xen-devel mailing list



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