Skip to contentSkip to conversation

x86/sgx: Add the missing ifdef for sgx_set_attribute()

PATCH Superseded Jarkko Sakkinen 3 participants 6 messages Aug 19, 2021 – Aug 25, 2021

Subsystems (4)

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Jarkko Sakkinen

author maintainer
#0 lore raw

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

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")
---
 arch/x86/include/asm/sgx.h | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)
arch/x86/include/asm/sgx.h +5 -3
index 05f3e21f01a7..38c397ef35a8 100644
@@ -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 */
Signature · 1 line
2.25.1
author maintainer
#1 lore raw

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

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.

---

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(-)
Documentation/x86/sgx.rst +6 -0
index dd0ac96ff9ef..68ee171e1d8f 100644
@@ -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.
arch/x86/include/asm/sgx.h +2 -0
index 38c397ef35a8..2ae9dc8c9411 100644
@@ -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
arch/x86/kernel/cpu/sgx/main.c +6 -1
index 63d3de02bbcc..1fe26a8e80dc 100644
@@ -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;
 }
 
arch/x86/mm/pat/set_memory.c +5 -0
index ad8a5c586a35..82bb09c298de 100644
@@ -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) { }
Signature · 1 line
2.25.1
author maintainer
#2 lore raw

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

Quoted 9 lines · 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:

Quoted 8 lines

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.

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

#3 lore raw

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

Quoted 24 lines · On Thu, 19 Aug 2021 15:52:48 +0300 Jarkko Sakkinen wrote:
Quoted 9 lines · 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:

Quoted 8 lines

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.

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.

Quoted 6 lines

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.

#4 lore raw
In reply to Jarkko Sakkinen
Quoted 15 lines · 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")
---
 arch/x86/include/asm/sgx.h | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)
arch/x86/include/asm/sgx.h +0 -0
index 05f3e21f01a7..38c397ef35a8 100644

arch/x86/include/asm/sgx.h struct sgx_sigstruct { · line 368

Quoted 4 lines
arch/x86/include/asm/sgx.h +1 -0
@@ -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.

arch/x86/include/asm/sgx.h +2 -0
index 05f3e21f01a7..d5f3e617cc38 100644
@@ -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 */
author maintainer
#5 lore raw
Quoted 39 lines · Sean Christopherson
Quoted 15 lines · 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")
---
 arch/x86/include/asm/sgx.h | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)
arch/x86/include/asm/sgx.h +0 -0
index 05f3e21f01a7..38c397ef35a8 100644

arch/x86/include/asm/sgx.h struct sgx_sigstruct { · line 368

Quoted 4 lines
arch/x86/include/asm/sgx.h +1 -0
@@ -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.

arch/x86/include/asm/sgx.h +1 -0
index 05f3e21f01a7..d5f3e617cc38 100644
@@ -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);

arch/x86/include/asm/sgx.h int sgx_virt_einit(void __user *sigstruct, void __user *token, · line 380

Quoted 6 lines
arch/x86/include/asm/sgx.h +1 -0
@@ -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