[PATCH v2 1/2] x86/sgx: Add the missing ifdef for sgx_set_attribute()

[PATCH v2 1/2] x86/sgx: Add the missing ifdef for sgx_set_attribute()

[Jarkko Sakkinen]

Similarly as sgx_virt_*, decorate sgx_set_attribute() with ifdef, so that
calling it without appropraite config flags, will cause a compilation
error, and not a linking error.

Fixes: b3754e5d3da3 ("x86/sgx: Move provisioning device creation out of SGX driver")
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
 arch/x86/include/asm/sgx.h | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/sgx.h b/arch/x86/include/asm/sgx.h
index 05f3e21f01a7..38c397ef35a8 100644
--- a/arch/x86/include/asm/sgx.h
+++ b/arch/x86/include/asm/sgx.h
@@ -365,6 +365,11 @@ struct sgx_sigstruct {
  * comment!
  */
 
+#if defined(CONFIG_X86_SGX) || defined(CONFIG_X86_SGX_KVM)
+int sgx_set_attribute(unsigned long *allowed_attributes,
+		      unsigned int attribute_fd);
+#endif
+
 #ifdef CONFIG_X86_SGX_KVM
 int sgx_virt_ecreate(struct sgx_pageinfo *pageinfo, void __user *secs,
 		     int *trapnr);
@@ -372,7 +377,4 @@ int sgx_virt_einit(void __user *sigstruct, void __user *token,
 		   void __user *secs, u64 *lepubkeyhash, int *trapnr);
 #endif
 
-int sgx_set_attribute(unsigned long *allowed_attributes,
-		      unsigned int attribute_fd);
-
 #endif /* _ASM_X86_SGX_H */
-- 
2.25.1

[PATCH v2 2/2] x86/sgx: Add SGX_MemTotal to /proc/meminfo

[Jarkko Sakkinen]

The amount of SGX memory on the system is determined by the BIOS and it
varies wildly between systems.  It can be from dozens of MB's on desktops
or VM's, up to many GB's on servers.  Just like for regular memory, it is
sometimes useful to know the amount of usable SGX memory in the system.

Add SGX_MemTotal field to /proc/meminfo, which shows the total amount of
usable SGX memory in the system.  E.g. with 32 MB reserved for SGX from
BIOS, the printout would be:

SGX_MemTotal:      22528 kB

It is less than 32 MB because some of the space is reserved for Enclave
Page Cache Metadata (EPCM), which contains state variables for all the
pages in the Enclave Page Cache (EPC).  The latter contains the pages,
which applications can use to create enclaves.

Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---

v2:
* Move ifdef fix for sgx_set_attribute() to a separate patch.

---
 Documentation/x86/sgx.rst      | 6 ++++++
 arch/x86/include/asm/sgx.h     | 2 ++
 arch/x86/kernel/cpu/sgx/main.c | 7 ++++++-
 arch/x86/mm/pat/set_memory.c   | 5 +++++
 4 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/Documentation/x86/sgx.rst b/Documentation/x86/sgx.rst
index dd0ac96ff9ef..68ee171e1d8f 100644
--- a/Documentation/x86/sgx.rst
+++ b/Documentation/x86/sgx.rst
@@ -250,3 +250,9 @@ user wants to deploy SGX applications both on the host and in guests
 on the same machine, the user should reserve enough EPC (by taking out
 total virtual EPC size of all SGX VMs from the physical EPC size) for
 host SGX applications so they can run with acceptable performance.
+
+Supplemental fields for /proc/meminfo
+=====================================
+
+SGX_MemTotal
+	The total usable SGX protected memory in kilobytes.
diff --git a/arch/x86/include/asm/sgx.h b/arch/x86/include/asm/sgx.h
index 38c397ef35a8..2ae9dc8c9411 100644
--- a/arch/x86/include/asm/sgx.h
+++ b/arch/x86/include/asm/sgx.h
@@ -366,6 +366,8 @@ struct sgx_sigstruct {
  */
 
 #if defined(CONFIG_X86_SGX) || defined(CONFIG_X86_SGX_KVM)
+extern unsigned long sgx_nr_all_pages;
+
 int sgx_set_attribute(unsigned long *allowed_attributes,
 		      unsigned int attribute_fd);
 #endif
diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c
index 63d3de02bbcc..1fe26a8e80dc 100644
--- a/arch/x86/kernel/cpu/sgx/main.c
+++ b/arch/x86/kernel/cpu/sgx/main.c
@@ -28,7 +28,10 @@ static DECLARE_WAIT_QUEUE_HEAD(ksgxd_waitq);
 static LIST_HEAD(sgx_active_page_list);
 static DEFINE_SPINLOCK(sgx_reclaimer_lock);
 
-/* The free page list lock protected variables prepend the lock. */
+/* The number of usable EPC pages in the system. */
+unsigned long sgx_nr_all_pages;
+
+/* The number of free EPC pages in all nodes. */
 static unsigned long sgx_nr_free_pages;
 
 /* Nodes with one or more EPC sections. */
@@ -656,6 +659,8 @@ static bool __init sgx_setup_epc_section(u64 phys_addr, u64 size,
 		list_add_tail(&section->pages[i].list, &sgx_dirty_page_list);
 	}
 
+	sgx_nr_all_pages += nr_pages;
+
 	return true;
 }
 
diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
index ad8a5c586a35..82bb09c298de 100644
--- a/arch/x86/mm/pat/set_memory.c
+++ b/arch/x86/mm/pat/set_memory.c
@@ -29,6 +29,7 @@
 #include <asm/proto.h>
 #include <asm/memtype.h>
 #include <asm/set_memory.h>
+#include <asm/sgx.h>
 
 #include "../mm_internal.h"
 
@@ -116,6 +117,10 @@ void arch_report_meminfo(struct seq_file *m)
 	if (direct_gbpages)
 		seq_printf(m, "DirectMap1G:    %8lu kB\n",
 			direct_pages_count[PG_LEVEL_1G] << 20);
+
+#if defined(CONFIG_X86_SGX) || defined(CONFIG_X86_SGX_KVM)
+	seq_printf(m, "SGX_MemTotal:   %8lu kB\n", sgx_nr_all_pages << 2);
+#endif
 }
 #else
 static inline void split_page_count(int level) { }
-- 
2.25.1

Re: [PATCH v2 2/2] x86/sgx: Add SGX_MemTotal to /proc/meminfo

[Jarkko Sakkinen]

On Thu, 2021-08-19 at 15:48 +0300, Jarkko Sakkinen wrote:
> The amount of SGX memory on the system is determined by the BIOS and it
> varies wildly between systems.  It can be from dozens of MB's on desktops
> or VM's, up to many GB's on servers.  Just like for regular memory, it is
> sometimes useful to know the amount of usable SGX memory in the system.
> 
> Add SGX_MemTotal field to /proc/meminfo, which shows the total amount of
> usable SGX memory in the system.  E.g. with 32 MB reserved for SGX from
> BIOS, the printout would be:
> 
> SGX_MemTotal:      22528 kB
> 
> It is less than 32 MB because some of the space is reserved for Enclave
> Page Cache Metadata (EPCM), which contains state variables for all the
> pages in the Enclave Page Cache (EPC).  The latter contains the pages,
> which applications can use to create enclaves.
> 
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>

While working on this, I noticed that suddenly my i5-9660k desktop fully
supports SGX on Linux. I don't recall that it has worked in the patch.
Maybe this is because of some firmware/ucode update, do not really know,
but definitely not a bad thing.

Perhaps this casts through other 9th gen Core CPU's.

The motherboard I have in this machine is AORUS Elite z390.

/Jarkko

Re: [PATCH v2 2/2] x86/sgx: Add SGX_MemTotal to /proc/meminfo

[Kai Huang]

On Thu, 19 Aug 2021 15:52:48 +0300 Jarkko Sakkinen wrote:
> On Thu, 2021-08-19 at 15:48 +0300, Jarkko Sakkinen wrote:
> > The amount of SGX memory on the system is determined by the BIOS and it
> > varies wildly between systems.  It can be from dozens of MB's on desktops
> > or VM's, up to many GB's on servers.  Just like for regular memory, it is
> > sometimes useful to know the amount of usable SGX memory in the system.
> > 
> > Add SGX_MemTotal field to /proc/meminfo, which shows the total amount of
> > usable SGX memory in the system.  E.g. with 32 MB reserved for SGX from
> > BIOS, the printout would be:
> > 
> > SGX_MemTotal:      22528 kB
> > 
> > It is less than 32 MB because some of the space is reserved for Enclave
> > Page Cache Metadata (EPCM), which contains state variables for all the
> > pages in the Enclave Page Cache (EPC).  The latter contains the pages,
> > which applications can use to create enclaves.
> > 
> > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> While working on this, I noticed that suddenly my i5-9660k desktop fully
> supports SGX on Linux. I don't recall that it has worked in the patch.
> Maybe this is because of some firmware/ucode update, do not really know,
> but definitely not a bad thing.
> 
> Perhaps this casts through other 9th gen Core CPU's.
> 
> The motherboard I have in this machine is AORUS Elite z390.
> 
> /Jarkko
> 

Enabling SGX also requires BIOS support, so perhaps it's BIOS update that makes
SGX available.

Btw, with SGX KVM, EPC will be discovered regardless FLC is available
or not. You need to check whether /dev/sgx_enclave is present or not to truly
know whether SGX driver is usable.

Re: [PATCH v2 1/2] x86/sgx: Add the missing ifdef for sgx_set_attribute()

[Sean Christopherson]

On Thu, Aug 19, 2021, Jarkko Sakkinen wrote:
> Similarly as sgx_virt_*, decorate sgx_set_attribute() with ifdef, so that
> calling it without appropraite config flags, will cause a compilation
> error, and not a linking error.
> 
> Fixes: b3754e5d3da3 ("x86/sgx: Move provisioning device creation out of SGX driver")
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> ---
>  arch/x86/include/asm/sgx.h | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/include/asm/sgx.h b/arch/x86/include/asm/sgx.h
> index 05f3e21f01a7..38c397ef35a8 100644
> --- a/arch/x86/include/asm/sgx.h
> +++ b/arch/x86/include/asm/sgx.h
> @@ -365,6 +365,11 @@ struct sgx_sigstruct {
>   * comment!
>   */
>  
> +#if defined(CONFIG_X86_SGX) || defined(CONFIG_X86_SGX_KVM)

X86_SGX_KVM depends on X86_SGX

And rather than shuffle code, you can wrap all of the prototypes, which has the
added bonus of "documenting" the aformentioned dependency, e.g.

diff --git a/arch/x86/include/asm/sgx.h b/arch/x86/include/asm/sgx.h
index 05f3e21f01a7..d5f3e617cc38 100644
--- a/arch/x86/include/asm/sgx.h
+++ b/arch/x86/include/asm/sgx.h
@@ -365,6 +365,7 @@ struct sgx_sigstruct {
  * comment!
  */

+#ifdef CONFIG_X86_SGX
 #ifdef CONFIG_X86_SGX_KVM
 int sgx_virt_ecreate(struct sgx_pageinfo *pageinfo, void __user *secs,
                     int *trapnr);
@@ -374,5 +375,6 @@ int sgx_virt_einit(void __user *sigstruct, void __user *token,

 int sgx_set_attribute(unsigned long *allowed_attributes,
                      unsigned int attribute_fd);
+#endif /* CONFIG_X86_SGX */

 #endif /* _ASM_X86_SGX_H */

Re: [PATCH v2 1/2] x86/sgx: Add the missing ifdef for sgx_set_attribute()

[Jarkko Sakkinen]

On Thu, 2021-08-19 at 16:07 +0000, Sean Christopherson wrote:
> On Thu, Aug 19, 2021, Jarkko Sakkinen wrote:
> > Similarly as sgx_virt_*, decorate sgx_set_attribute() with ifdef, so that
> > calling it without appropraite config flags, will cause a compilation
> > error, and not a linking error.
> > 
> > Fixes: b3754e5d3da3 ("x86/sgx: Move provisioning device creation out of SGX driver")
> > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> > ---
> >  arch/x86/include/asm/sgx.h | 8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/x86/include/asm/sgx.h b/arch/x86/include/asm/sgx.h
> > index 05f3e21f01a7..38c397ef35a8 100644
> > --- a/arch/x86/include/asm/sgx.h
> > +++ b/arch/x86/include/asm/sgx.h
> > @@ -365,6 +365,11 @@ struct sgx_sigstruct {
> >   * comment!
> >   */
> >  
> > +#if defined(CONFIG_X86_SGX) || defined(CONFIG_X86_SGX_KVM)
> 
> X86_SGX_KVM depends on X86_SGX
> 
> And rather than shuffle code, you can wrap all of the prototypes, which has the
> added bonus of "documenting" the aformentioned dependency, e.g.
> 
> diff --git a/arch/x86/include/asm/sgx.h b/arch/x86/include/asm/sgx.h
> index 05f3e21f01a7..d5f3e617cc38 100644
> --- a/arch/x86/include/asm/sgx.h
> +++ b/arch/x86/include/asm/sgx.h
> @@ -365,6 +365,7 @@ struct sgx_sigstruct {
>   * comment!
>   */
> 
> +#ifdef CONFIG_X86_SGX
>  #ifdef CONFIG_X86_SGX_KVM
>  int sgx_virt_ecreate(struct sgx_pageinfo *pageinfo, void __user *secs,
>                      int *trapnr);
> @@ -374,5 +375,6 @@ int sgx_virt_einit(void __user *sigstruct, void __user *token,
> 
>  int sgx_set_attribute(unsigned long *allowed_attributes,
>                       unsigned int attribute_fd);
> +#endif /* CONFIG_X86_SGX */
> 
>  #endif /* _ASM_X86_SGX_H */

Right, makes sense.

A bit off from this patch set, but I'm wondering if there would be any benefit to
have a compile time flag for just the allocator part, since KVM depends only that.

I.e. you could compile a kernel with just KVM required bits for SGX.

/Jarkko