Re: [PATCH v2 08/25] x86/sgx: Expose SGX architectural definitions to the kernel

From: Kai Huang <kai.huang@intel.com>
To: Sean Christopherson <seanjc@google.com>
Cc: kvm@vger.kernel.org, linux-sgx@vger.kernel.org, x86@kernel.org,
	linux-kernel@vger.kernel.org, jarkko@kernel.org, luto@kernel.org,
	dave.hansen@intel.com, rick.p.edgecombe@intel.com,
	haitao.huang@intel.com, pbonzini@redhat.com, bp@alien8.de,
	tglx@linutronix.de, mingo@redhat.com, hpa@zytor.com
Subject: Re: [PATCH v2 08/25] x86/sgx: Expose SGX architectural definitions to the kernel
Date: Mon, 15 Mar 2021 16:36:56 +1300	[thread overview]
Message-ID: <20210315163656.df89c4f52573724e492cc11d@intel.com> (raw)
In-Reply-To: <YEvkEJkM0D7oZWE3@google.com>

On Fri, 12 Mar 2021 13:58:40 -0800 Sean Christopherson wrote:
> On Tue, Mar 09, 2021, Kai Huang wrote:
> > From: Sean Christopherson <sean.j.christopherson@intel.com>
> > 
> > Expose SGX architectural structures, as KVM will use many of the
> > architectural constants and structs to virtualize SGX.
> > 
> > Name the new header file as asm/sgx.h, rather than asm/sgx_arch.h, to
> > have single header to provide SGX facilities to share with other kernel
> > componments.
> > 
> > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> > Co-developed-by: Kai Huang <kai.huang@intel.com>
> > Acked-by: Dave Hansen <dave.hansen@intel.com>
> 
> Same checkpatch warning.  Probably doesn't matter.

Will change order to make checkpatch happy for this whole series.

Thanks for pointing out.

> 
> > Signed-off-by: Kai Huang <kai.huang@intel.com>
> > ---
> >  .../cpu/sgx/arch.h => include/asm/sgx.h}      | 20 ++++++++++++++-----
> >  arch/x86/kernel/cpu/sgx/encl.c                |  2 +-
> >  arch/x86/kernel/cpu/sgx/sgx.h                 |  2 +-
> >  tools/testing/selftests/sgx/defines.h         |  2 +-
> >  4 files changed, 18 insertions(+), 8 deletions(-)
> >  rename arch/x86/{kernel/cpu/sgx/arch.h => include/asm/sgx.h} (95%)
> > 
> > diff --git a/arch/x86/kernel/cpu/sgx/arch.h b/arch/x86/include/asm/sgx.h
> > similarity index 95%
> > rename from arch/x86/kernel/cpu/sgx/arch.h
> > rename to arch/x86/include/asm/sgx.h
> > index abf99bb71fdc..d4ad35f6319a 100644
> > --- a/arch/x86/kernel/cpu/sgx/arch.h
> > +++ b/arch/x86/include/asm/sgx.h
> > @@ -2,15 +2,20 @@
> >  /**
> >   * Copyright(c) 2016-20 Intel Corporation.
> >   *
> > - * Contains data structures defined by the SGX architecture.  Data structures
> > - * defined by the Linux software stack should not be placed here.
> > + * Intel Software Guard Extensions (SGX) support.
> >   */
> > -#ifndef _ASM_X86_SGX_ARCH_H
> > -#define _ASM_X86_SGX_ARCH_H
> > +#ifndef _ASM_X86_SGX_H
> > +#define _ASM_X86_SGX_H
> >  
> >  #include <linux/bits.h>
> >  #include <linux/types.h>
> >  
> > +/*
> > + * This file contains both data structures defined by SGX architecture and Linux
> > + * defined software data structures and functions.  The two should not be mixed
> > + * together for better readibility.  The architectural definitions come first.
> > + */
> > +
> >  /* The SGX specific CPUID function. */
> >  #define SGX_CPUID		0x12
> >  /* EPC enumeration. */
> > @@ -337,4 +342,9 @@ struct sgx_sigstruct {
> >  
> >  #define SGX_LAUNCH_TOKEN_SIZE 304
> >  
> > -#endif /* _ASM_X86_SGX_ARCH_H */
> > +/*
> > + * Do not put any hardware-defined SGX structure representations below this
> > + * line!
> 
> Heh, which line?  Yep, it's Friday afternoon...

Hmm.. I will change to below this comment. :)