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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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.


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.

~Andrew

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