[intel-sgx-kernel-dev] intel_sgx: depend on CPU_SUP_INTEL instead of X86
diff mbox

Message ID 1500580423-12801-1-git-send-email-sean.j.christopherson@intel.com
State New
Headers show

Commit Message

Sean Christopherson July 20, 2017, 7:53 p.m. UTC
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
---
 drivers/platform/x86/intel_sgx/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jarkko Sakkinen Aug. 1, 2017, 1:21 p.m. UTC | #1
On Thu, Jul 20, 2017 at 12:53:43PM -0700, Sean Christopherson wrote:
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> ---
>  drivers/platform/x86/intel_sgx/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/intel_sgx/Kconfig b/drivers/platform/x86/intel_sgx/Kconfig
> index 9641728..442a8ef 100644
> --- a/drivers/platform/x86/intel_sgx/Kconfig
> +++ b/drivers/platform/x86/intel_sgx/Kconfig
> @@ -5,7 +5,7 @@
>  config INTEL_SGX
>  	tristate "Intel(R) SGX Driver"
>  	default n
> -	depends on X86
> +	depends on CPU_SUP_INTEL
>  	select MMU_NOTIFIER
>  	---help---
>  	Intel(R) SGX is a set of CPU instructions that can be used by
> -- 
> 2.7.4
> 
> _______________________________________________
> intel-sgx-kernel-dev mailing list
> intel-sgx-kernel-dev@lists.01.org
> https://lists.01.org/mailman/listinfo/intel-sgx-kernel-dev


We need to depend on X86_64 for first upstream version because

1. My in-kernel LE supports only 64-bit *initially* for *simplicity*. It
   can be extended later on.
2. There is no a clean way to setup PAT when SGX driver is running in
   32-bit mode because symbols like reserve_memtype() are not exported
   for kernel modules [1]. The best way to support 32-bit  would be to
   use [2] but it needs to be extended to support WB. Right now it is
   only used by DRM and therefore has only WC-support. For simplicity,
   it is better to clean the hacks for 32-bit environment and move
   later on to [2].

Because of these reasons I think the line should be

depends on CPU_SUP_INTEL && X86_64

[1] arch/x86/mm/pat.c
[2] include/linux/io-mapping.h

/Jarkko
Sean Christopherson Aug. 4, 2017, 8:44 p.m. UTC | #2
Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> wrote:
> On Thu, Jul 20, 2017 at 12:53:43PM -0700, Sean Christopherson wrote:
> > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> > ---
> >  drivers/platform/x86/intel_sgx/Kconfig | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/platform/x86/intel_sgx/Kconfig b/drivers/platform/x86/intel_sgx/Kconfig
> > index 9641728..442a8ef 100644
> > --- a/drivers/platform/x86/intel_sgx/Kconfig
> > +++ b/drivers/platform/x86/intel_sgx/Kconfig
> > @@ -5,7 +5,7 @@
> >  config INTEL_SGX
> >  	tristate "Intel(R) SGX Driver"
> >  	default n
> > -	depends on X86
> > +	depends on CPU_SUP_INTEL
> >  	select MMU_NOTIFIER
> >  	---help---
> >  	Intel(R) SGX is a set of CPU instructions that can be used by
> > -- 
> > 2.7.4
> > 
> > _______________________________________________
> > intel-sgx-kernel-dev mailing list
> > intel-sgx-kernel-dev@lists.01.org
> > https://lists.01.org/mailman/listinfo/intel-sgx-kernel-dev
> 
> 
> We need to depend on X86_64 for first upstream version because
> 
> 1. My in-kernel LE supports only 64-bit *initially* for *simplicity*. It
>    can be extended later on.
> 2. There is no a clean way to setup PAT when SGX driver is running in
>    32-bit mode because symbols like reserve_memtype() are not exported
>    for kernel modules [1]. The best way to support 32-bit  would be to
>    use [2] but it needs to be extended to support WB. Right now it is
>    only used by DRM and therefore has only WC-support. For simplicity,
>    it is better to clean the hacks for 32-bit environment and move
>    later on to [2].
> 
> Because of these reasons I think the line should be
> 
> depends on CPU_SUP_INTEL && X86_64
> 
> [1] arch/x86/mm/pat.c
> [2] include/linux/io-mapping.h
> 
> /Jarkko

> On Thu, Jul 20, 2017 at 12:53:43PM -0700, Sean Christopherson wrote:
> > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> > ---
> >  drivers/platform/x86/intel_sgx/Kconfig | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/platform/x86/intel_sgx/Kconfig b/drivers/platform/x86/intel_sgx/Kconfig
> > index 9641728..442a8ef 100644
> > --- a/drivers/platform/x86/intel_sgx/Kconfig
> > +++ b/drivers/platform/x86/intel_sgx/Kconfig
> > @@ -5,7 +5,7 @@
> >  config INTEL_SGX
> >  	tristate "Intel(R) SGX Driver"
> >  	default n
> > -	depends on X86
> > +	depends on CPU_SUP_INTEL
> >  	select MMU_NOTIFIER
> >  	---help---
> >  	Intel(R) SGX is a set of CPU instructions that can be used by
> > -- 
> > 2.7.4
> > 
> > _______________________________________________
> > intel-sgx-kernel-dev mailing list
> > intel-sgx-kernel-dev@lists.01.org
> > https://lists.01.org/mailman/listinfo/intel-sgx-kernel-dev
> 
> 
> We need to depend on X86_64 for first upstream version because
> 
> 1. My in-kernel LE supports only 64-bit *initially* for *simplicity*. It
>    can be extended later on.
> 2. There is no a clean way to setup PAT when SGX driver is running in
>    32-bit mode because symbols like reserve_memtype() are not exported
>    for kernel modules [1]. The best way to support 32-bit  would be to
>    use [2] but it needs to be extended to support WB. Right now it is
>    only used by DRM and therefore has only WC-support. For simplicity,
>    it is better to clean the hacks for 32-bit environment and move
>    later on to [2].
> 
> Because of these reasons I think the line should be
> 
> depends on CPU_SUP_INTEL && X86_64
> 
> [1] arch/x86/mm/pat.c
> [2] include/linux/io-mapping.h
> 
> /Jarkko

I don't think we have to worry about the PAT, my understanding is that
the memtype from the PRMRR overrides everything, e.g. PAT, EPT, MTRR...
Jarkko Sakkinen Aug. 6, 2017, 12:46 p.m. UTC | #3
On Fri, Aug 04, 2017 at 08:44:31PM +0000, Christopherson, Sean J wrote:
> Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> wrote:
> > On Thu, Jul 20, 2017 at 12:53:43PM -0700, Sean Christopherson wrote:
> > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> > > ---
> > >  drivers/platform/x86/intel_sgx/Kconfig | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/platform/x86/intel_sgx/Kconfig b/drivers/platform/x86/intel_sgx/Kconfig
> > > index 9641728..442a8ef 100644
> > > --- a/drivers/platform/x86/intel_sgx/Kconfig
> > > +++ b/drivers/platform/x86/intel_sgx/Kconfig
> > > @@ -5,7 +5,7 @@
> > >  config INTEL_SGX
> > >  	tristate "Intel(R) SGX Driver"
> > >  	default n
> > > -	depends on X86
> > > +	depends on CPU_SUP_INTEL
> > >  	select MMU_NOTIFIER
> > >  	---help---
> > >  	Intel(R) SGX is a set of CPU instructions that can be used by
> > > -- 
> > > 2.7.4
> > > 
> > > _______________________________________________
> > > intel-sgx-kernel-dev mailing list
> > > intel-sgx-kernel-dev@lists.01.org
> > > https://lists.01.org/mailman/listinfo/intel-sgx-kernel-dev
> > 
> > 
> > We need to depend on X86_64 for first upstream version because
> > 
> > 1. My in-kernel LE supports only 64-bit *initially* for *simplicity*. It
> >    can be extended later on.
> > 2. There is no a clean way to setup PAT when SGX driver is running in
> >    32-bit mode because symbols like reserve_memtype() are not exported
> >    for kernel modules [1]. The best way to support 32-bit  would be to
> >    use [2] but it needs to be extended to support WB. Right now it is
> >    only used by DRM and therefore has only WC-support. For simplicity,
> >    it is better to clean the hacks for 32-bit environment and move
> >    later on to [2].
> > 
> > Because of these reasons I think the line should be
> > 
> > depends on CPU_SUP_INTEL && X86_64
> > 
> > [1] arch/x86/mm/pat.c
> > [2] include/linux/io-mapping.h
> > 
> > /Jarkko
> 
> > On Thu, Jul 20, 2017 at 12:53:43PM -0700, Sean Christopherson wrote:
> > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> > > ---
> > >  drivers/platform/x86/intel_sgx/Kconfig | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/platform/x86/intel_sgx/Kconfig b/drivers/platform/x86/intel_sgx/Kconfig
> > > index 9641728..442a8ef 100644
> > > --- a/drivers/platform/x86/intel_sgx/Kconfig
> > > +++ b/drivers/platform/x86/intel_sgx/Kconfig
> > > @@ -5,7 +5,7 @@
> > >  config INTEL_SGX
> > >  	tristate "Intel(R) SGX Driver"
> > >  	default n
> > > -	depends on X86
> > > +	depends on CPU_SUP_INTEL
> > >  	select MMU_NOTIFIER
> > >  	---help---
> > >  	Intel(R) SGX is a set of CPU instructions that can be used by
> > > -- 
> > > 2.7.4
> > > 
> > > _______________________________________________
> > > intel-sgx-kernel-dev mailing list
> > > intel-sgx-kernel-dev@lists.01.org
> > > https://lists.01.org/mailman/listinfo/intel-sgx-kernel-dev
> > 
> > 
> > We need to depend on X86_64 for first upstream version because
> > 
> > 1. My in-kernel LE supports only 64-bit *initially* for *simplicity*. It
> >    can be extended later on.
> > 2. There is no a clean way to setup PAT when SGX driver is running in
> >    32-bit mode because symbols like reserve_memtype() are not exported
> >    for kernel modules [1]. The best way to support 32-bit  would be to
> >    use [2] but it needs to be extended to support WB. Right now it is
> >    only used by DRM and therefore has only WC-support. For simplicity,
> >    it is better to clean the hacks for 32-bit environment and move
> >    later on to [2].
> > 
> > Because of these reasons I think the line should be
> > 
> > depends on CPU_SUP_INTEL && X86_64
> > 
> > [1] arch/x86/mm/pat.c
> > [2] include/linux/io-mapping.h
> > 
> > /Jarkko
> 
> I don't think we have to worry about the PAT, my understanding is that
> the memtype from the PRMRR overrides everything, e.g. PAT, EPT, MTRR...

Yes, we do. For iomem we can only set PAT attributes correctly and at
least before EPC has been by default uncached. For 64-bit we get it
cached by using ioremap_cache(). PRMRR defines the range but with PAT
we set the caching.

/Jarkko

Patch
diff mbox

diff --git a/drivers/platform/x86/intel_sgx/Kconfig b/drivers/platform/x86/intel_sgx/Kconfig
index 9641728..442a8ef 100644
--- a/drivers/platform/x86/intel_sgx/Kconfig
+++ b/drivers/platform/x86/intel_sgx/Kconfig
@@ -5,7 +5,7 @@ 
 config INTEL_SGX
 	tristate "Intel(R) SGX Driver"
 	default n
-	depends on X86
+	depends on CPU_SUP_INTEL
 	select MMU_NOTIFIER
 	---help---
 	Intel(R) SGX is a set of CPU instructions that can be used by