linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/4] x86/sgx: Add /sys/kernel/debug/x86/sgx_total_mem
       [not found] <20210728225140.248408-1-jarkko@kernel.org>
@ 2021-07-28 22:51 ` Jarkko Sakkinen
  2021-07-29 16:27   ` Dave Hansen
  2021-07-28 22:51 ` [PATCH v2 2/4] selftests/sgx: Fix calculations for sub-maximum field sizes Jarkko Sakkinen
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 6+ messages in thread
From: Jarkko Sakkinen @ 2021-07-28 22:51 UTC (permalink / raw)
  To: Shuah Khan
  Cc: linux-kselftest, linux-sgx, Reinette Chatre, Borislav Petkov,
	Jarkko Sakkinen, Dave Hansen, Thomas Gleixner, Ingo Molnar, x86,
	H. Peter Anvin, Jonathan Corbet, linux-kernel, linux-doc

Create /sys/kernel/debug/x86/sgx_total_mem, after the driver is
initialized, describing the total amount of SGX reserved memory (also known
as Enclave Page Cache (EPC)) available in the system.

This can be used to implement stress that trigger the page reclaimer, and
it is probably useful information when admistering a system running
enclaves anyhow.

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

---

v2:
* sgx_nr_all_pages -> sgx_total_mem

 Documentation/x86/sgx.rst      |  6 ++++++
 arch/x86/kernel/cpu/sgx/main.c | 10 +++++++++-
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/Documentation/x86/sgx.rst b/Documentation/x86/sgx.rst
index dd0ac96ff9ef..15bb8f4e42af 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.
+
+SGX debugging
+=============
+
+*/sys/kernel/debug/x86/sgx_total_mem* describes the total available
+SGX mwemory available in the system.
diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c
index 63d3de02bbcc..b65da19a53ee 100644
--- a/arch/x86/kernel/cpu/sgx/main.c
+++ b/arch/x86/kernel/cpu/sgx/main.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 /*  Copyright(c) 2016-20 Intel Corporation. */
 
+#include <linux/debugfs.h>
 #include <linux/file.h>
 #include <linux/freezer.h>
 #include <linux/highmem.h>
@@ -28,7 +29,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. */
+/* Total EPC memory available in bytes. */
+static unsigned long sgx_total_mem;
+
+/* 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 +660,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_total_mem += nr_pages * PAGE_SIZE;
+
 	return true;
 }
 
@@ -823,6 +829,8 @@ static int __init sgx_init(void)
 	if (sgx_vepc_init() && ret)
 		goto err_provision;
 
+	debugfs_create_ulong("sgx_total_mem", 0444, arch_debugfs_dir, &sgx_total_mem);
+
 	return 0;
 
 err_provision:
-- 
2.32.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 2/4] selftests/sgx: Fix calculations for sub-maximum field sizes
       [not found] <20210728225140.248408-1-jarkko@kernel.org>
  2021-07-28 22:51 ` [PATCH v2 1/4] x86/sgx: Add /sys/kernel/debug/x86/sgx_total_mem Jarkko Sakkinen
@ 2021-07-28 22:51 ` Jarkko Sakkinen
  2021-07-28 22:51 ` [PATCH v2 3/4] selftests/sgx: Assign source for each segment Jarkko Sakkinen
  2021-07-28 22:51 ` [PATCH v2 4/4] selftests/sgx: Trigger the reclaimer and #PF handler Jarkko Sakkinen
  3 siblings, 0 replies; 6+ messages in thread
From: Jarkko Sakkinen @ 2021-07-28 22:51 UTC (permalink / raw)
  To: Shuah Khan
  Cc: linux-kselftest, linux-sgx, Reinette Chatre, Borislav Petkov,
	Tianjia Zhang, Dave Hansen, Jarkko Sakkinen, Borislav Petkov,
	Jethro Beekman, linux-kernel

From: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>

Q1 and Q2 are numbers with *maximum* length of 384 bytes. If the calculated
length of Q1 and Q2 is less than 384 bytes, things will go wrong.

E.g. if Q2 is 383 bytes, then

1. The bytes of q2 are copied to sigstruct->q2 in calc_q1q2().
2. The entire sigstruct->q2 is reversed, which results it being
   256 * Q2, given that the last byte of sigstruct->q2 is added
   to before the bytes given by calc_q1q2().

Either change in key or measurement can trigger the bug. E.g. an unmeasured
heap could cause a devastating change in Q1 or Q2.

Reverse exactly the bytes of Q1 and Q2 in calc_q1q2() before returning to
the caller.

The original patch did a bad job explaining the code change but it
turned out making sense. I wrote a new description.

Fixes: 2adcba79e69d ("selftests/x86: Add a selftest for SGX")
Link: https://lore.kernel.org/linux-sgx/20210301051836.30738-1-tianjia.zhang@linux.alibaba.com/
Reviewed-by: Dave Hansen <dave.hansen@linux.intel.com>
Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---

v4:
- Updated the short summary according to:
  https://lore.kernel.org/linux-sgx/4303b822-5861-ba2c-f620-0e752e499329@intel.com/

v3:
- Adjusted the fixes tag to refer to the correct commit

v2:
- Added a fixes tag.

 tools/testing/selftests/sgx/sigstruct.c | 41 +++++++++++++------------
 1 file changed, 21 insertions(+), 20 deletions(-)

diff --git a/tools/testing/selftests/sgx/sigstruct.c b/tools/testing/selftests/sgx/sigstruct.c
index dee7a3d6c5a5..92bbc5a15c39 100644
--- a/tools/testing/selftests/sgx/sigstruct.c
+++ b/tools/testing/selftests/sgx/sigstruct.c
@@ -55,10 +55,27 @@ static bool alloc_q1q2_ctx(const uint8_t *s, const uint8_t *m,
 	return true;
 }
 
+static void reverse_bytes(void *data, int length)
+{
+	int i = 0;
+	int j = length - 1;
+	uint8_t temp;
+	uint8_t *ptr = data;
+
+	while (i < j) {
+		temp = ptr[i];
+		ptr[i] = ptr[j];
+		ptr[j] = temp;
+		i++;
+		j--;
+	}
+}
+
 static bool calc_q1q2(const uint8_t *s, const uint8_t *m, uint8_t *q1,
 		      uint8_t *q2)
 {
 	struct q1q2_ctx ctx;
+	int len;
 
 	if (!alloc_q1q2_ctx(s, m, &ctx)) {
 		fprintf(stderr, "Not enough memory for Q1Q2 calculation\n");
@@ -89,8 +106,10 @@ static bool calc_q1q2(const uint8_t *s, const uint8_t *m, uint8_t *q1,
 		goto out;
 	}
 
-	BN_bn2bin(ctx.q1, q1);
-	BN_bn2bin(ctx.q2, q2);
+	len = BN_bn2bin(ctx.q1, q1);
+	reverse_bytes(q1, len);
+	len = BN_bn2bin(ctx.q2, q2);
+	reverse_bytes(q2, len);
 
 	free_q1q2_ctx(&ctx);
 	return true;
@@ -152,22 +171,6 @@ static RSA *gen_sign_key(void)
 	return key;
 }
 
-static void reverse_bytes(void *data, int length)
-{
-	int i = 0;
-	int j = length - 1;
-	uint8_t temp;
-	uint8_t *ptr = data;
-
-	while (i < j) {
-		temp = ptr[i];
-		ptr[i] = ptr[j];
-		ptr[j] = temp;
-		i++;
-		j--;
-	}
-}
-
 enum mrtags {
 	MRECREATE = 0x0045544145524345,
 	MREADD = 0x0000000044444145,
@@ -367,8 +370,6 @@ bool encl_measure(struct encl *encl)
 	/* BE -> LE */
 	reverse_bytes(sigstruct->signature, SGX_MODULUS_SIZE);
 	reverse_bytes(sigstruct->modulus, SGX_MODULUS_SIZE);
-	reverse_bytes(sigstruct->q1, SGX_MODULUS_SIZE);
-	reverse_bytes(sigstruct->q2, SGX_MODULUS_SIZE);
 
 	EVP_MD_CTX_destroy(ctx);
 	RSA_free(key);
-- 
2.32.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 3/4] selftests/sgx: Assign source for each segment
       [not found] <20210728225140.248408-1-jarkko@kernel.org>
  2021-07-28 22:51 ` [PATCH v2 1/4] x86/sgx: Add /sys/kernel/debug/x86/sgx_total_mem Jarkko Sakkinen
  2021-07-28 22:51 ` [PATCH v2 2/4] selftests/sgx: Fix calculations for sub-maximum field sizes Jarkko Sakkinen
@ 2021-07-28 22:51 ` Jarkko Sakkinen
  2021-07-28 22:51 ` [PATCH v2 4/4] selftests/sgx: Trigger the reclaimer and #PF handler Jarkko Sakkinen
  3 siblings, 0 replies; 6+ messages in thread
From: Jarkko Sakkinen @ 2021-07-28 22:51 UTC (permalink / raw)
  To: Shuah Khan
  Cc: linux-kselftest, linux-sgx, Reinette Chatre, Borislav Petkov,
	Jarkko Sakkinen, Dave Hansen, linux-kernel

Define source per segment so that enclave pages can be added from different
sources, e.g. anonymous VMA for zero pages. In other words, add 'src' field
to struct encl_segment, and assign it to 'encl->src' for pages inherited
from the enclave binary.

Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
 tools/testing/selftests/sgx/load.c      | 5 +++--
 tools/testing/selftests/sgx/main.h      | 1 +
 tools/testing/selftests/sgx/sigstruct.c | 8 ++++----
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/sgx/load.c b/tools/testing/selftests/sgx/load.c
index 00928be57fc4..9946fab2a3d6 100644
--- a/tools/testing/selftests/sgx/load.c
+++ b/tools/testing/selftests/sgx/load.c
@@ -107,7 +107,7 @@ static bool encl_ioc_add_pages(struct encl *encl, struct encl_segment *seg)
 	memset(&secinfo, 0, sizeof(secinfo));
 	secinfo.flags = seg->flags;
 
-	ioc.src = (uint64_t)encl->src + seg->offset;
+	ioc.src = (uint64_t)seg->src;
 	ioc.offset = seg->offset;
 	ioc.length = seg->size;
 	ioc.secinfo = (unsigned long)&secinfo;
@@ -226,6 +226,7 @@ bool encl_load(const char *path, struct encl *encl)
 
 		if (j == 0) {
 			src_offset = phdr->p_offset & PAGE_MASK;
+			encl->src = encl->bin + src_offset;
 
 			seg->prot = PROT_READ | PROT_WRITE;
 			seg->flags = SGX_PAGE_TYPE_TCS << 8;
@@ -238,13 +239,13 @@ bool encl_load(const char *path, struct encl *encl)
 
 		seg->offset = (phdr->p_offset & PAGE_MASK) - src_offset;
 		seg->size = (phdr->p_filesz + PAGE_SIZE - 1) & PAGE_MASK;
+		seg->src = encl->src + seg->offset;
 
 		j++;
 	}
 
 	assert(j == encl->nr_segments);
 
-	encl->src = encl->bin + src_offset;
 	encl->src_size = encl->segment_tbl[j - 1].offset +
 			 encl->segment_tbl[j - 1].size;
 
diff --git a/tools/testing/selftests/sgx/main.h b/tools/testing/selftests/sgx/main.h
index 68672fd86cf9..452d11dc4889 100644
--- a/tools/testing/selftests/sgx/main.h
+++ b/tools/testing/selftests/sgx/main.h
@@ -7,6 +7,7 @@
 #define MAIN_H
 
 struct encl_segment {
+	void *src;
 	off_t offset;
 	size_t size;
 	unsigned int prot;
diff --git a/tools/testing/selftests/sgx/sigstruct.c b/tools/testing/selftests/sgx/sigstruct.c
index 92bbc5a15c39..202a96fd81bf 100644
--- a/tools/testing/selftests/sgx/sigstruct.c
+++ b/tools/testing/selftests/sgx/sigstruct.c
@@ -289,14 +289,14 @@ static bool mrenclave_eextend(EVP_MD_CTX *ctx, uint64_t offset,
 static bool mrenclave_segment(EVP_MD_CTX *ctx, struct encl *encl,
 			      struct encl_segment *seg)
 {
-	uint64_t end = seg->offset + seg->size;
+	uint64_t end = seg->size;
 	uint64_t offset;
 
-	for (offset = seg->offset; offset < end; offset += PAGE_SIZE) {
-		if (!mrenclave_eadd(ctx, offset, seg->flags))
+	for (offset = 0; offset < end; offset += PAGE_SIZE) {
+		if (!mrenclave_eadd(ctx, seg->offset + offset, seg->flags))
 			return false;
 
-		if (!mrenclave_eextend(ctx, offset, encl->src + offset))
+		if (!mrenclave_eextend(ctx, seg->offset + offset, seg->src + offset))
 			return false;
 	}
 
-- 
2.32.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 4/4] selftests/sgx: Trigger the reclaimer and #PF handler
       [not found] <20210728225140.248408-1-jarkko@kernel.org>
                   ` (2 preceding siblings ...)
  2021-07-28 22:51 ` [PATCH v2 3/4] selftests/sgx: Assign source for each segment Jarkko Sakkinen
@ 2021-07-28 22:51 ` Jarkko Sakkinen
  3 siblings, 0 replies; 6+ messages in thread
From: Jarkko Sakkinen @ 2021-07-28 22:51 UTC (permalink / raw)
  To: Shuah Khan
  Cc: linux-kselftest, linux-sgx, Reinette Chatre, Borislav Petkov,
	Jarkko Sakkinen, Dave Hansen, linux-kernel

Create a heap for the test enclave, which has the same size as all
available Enclave Page Cache (EPC) pages in the system. This will guarantee
that all test_encl.elf pages *and* SGX Enclave Control Structure (SECS)
have been swapped out by the page reclaimer during the load time. Actually,
this adds a bit more stress than that since part of the EPC gets reserved
for the Version Array (VA) pages.

For each test, the page fault handler gets triggered in two occasions:

- When SGX_IOC_ENCLAVE_INIT is performed, SECS gets swapped in by the
  page fault handler.
- During the execution, each page that is referenced gets swapped in
  by the page fault handler.

In addition, add two new checks:

1. All of the EPC memory must be unused before the test execution
   starts.
2. Before SGX_IOC_ENCLAVE_INIT, the number of free EPC pages must
   be zero.

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

v2:
* sgx_nr_all_pages -> sgx_total_mem

 tools/testing/selftests/sgx/load.c      | 33 +++++++++++++++----
 tools/testing/selftests/sgx/main.c      | 42 ++++++++++++++++++++++++-
 tools/testing/selftests/sgx/main.h      |  3 +-
 tools/testing/selftests/sgx/sigstruct.c |  6 ++--
 4 files changed, 74 insertions(+), 10 deletions(-)

diff --git a/tools/testing/selftests/sgx/load.c b/tools/testing/selftests/sgx/load.c
index 9946fab2a3d6..a312a132ac33 100644
--- a/tools/testing/selftests/sgx/load.c
+++ b/tools/testing/selftests/sgx/load.c
@@ -21,6 +21,8 @@
 
 void encl_delete(struct encl *encl)
 {
+	struct encl_segment *heap_seg = &encl->segment_tbl[encl->nr_segments - 1];
+
 	if (encl->encl_base)
 		munmap((void *)encl->encl_base, encl->encl_size);
 
@@ -30,6 +32,8 @@ void encl_delete(struct encl *encl)
 	if (encl->fd)
 		close(encl->fd);
 
+	munmap(heap_seg->src, heap_seg->size);
+
 	if (encl->segment_tbl)
 		free(encl->segment_tbl);
 
@@ -111,7 +115,10 @@ static bool encl_ioc_add_pages(struct encl *encl, struct encl_segment *seg)
 	ioc.offset = seg->offset;
 	ioc.length = seg->size;
 	ioc.secinfo = (unsigned long)&secinfo;
-	ioc.flags = SGX_PAGE_MEASURE;
+	if (seg->measure)
+		ioc.flags = SGX_PAGE_MEASURE;
+	else
+		ioc.flags = 0;
 
 	rc = ioctl(encl->fd, SGX_IOC_ENCLAVE_ADD_PAGES, &ioc);
 	if (rc < 0) {
@@ -124,9 +131,10 @@ static bool encl_ioc_add_pages(struct encl *encl, struct encl_segment *seg)
 
 
 
-bool encl_load(const char *path, struct encl *encl)
+bool encl_load(const char *path, struct encl *encl, unsigned long heap_size)
 {
 	const char device_path[] = "/dev/sgx_enclave";
+	struct encl_segment *seg;
 	Elf64_Phdr *phdr_tbl;
 	off_t src_offset;
 	Elf64_Ehdr *ehdr;
@@ -188,6 +196,8 @@ bool encl_load(const char *path, struct encl *encl)
 	ehdr = encl->bin;
 	phdr_tbl = encl->bin + ehdr->e_phoff;
 
+	encl->nr_segments = 1; /* one for the heap */
+
 	for (i = 0; i < ehdr->e_phnum; i++) {
 		Elf64_Phdr *phdr = &phdr_tbl[i];
 
@@ -203,7 +213,6 @@ bool encl_load(const char *path, struct encl *encl)
 	for (i = 0, j = 0; i < ehdr->e_phnum; i++) {
 		Elf64_Phdr *phdr = &phdr_tbl[i];
 		unsigned int flags = phdr->p_flags;
-		struct encl_segment *seg;
 
 		if (phdr->p_type != PT_LOAD)
 			continue;
@@ -240,14 +249,26 @@ bool encl_load(const char *path, struct encl *encl)
 		seg->offset = (phdr->p_offset & PAGE_MASK) - src_offset;
 		seg->size = (phdr->p_filesz + PAGE_SIZE - 1) & PAGE_MASK;
 		seg->src = encl->src + seg->offset;
+		seg->measure = true;
 
 		j++;
 	}
 
-	assert(j == encl->nr_segments);
+	assert(j == encl->nr_segments - 1);
+
+	seg = &encl->segment_tbl[j];
+	seg->offset =  encl->segment_tbl[j - 1].offset + encl->segment_tbl[j - 1].size;
+	seg->size = heap_size;
+	seg->src = mmap(NULL, heap_size, PROT_READ | PROT_WRITE,
+			MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
+	seg->prot = PROT_READ | PROT_WRITE;
+	seg->flags = (SGX_PAGE_TYPE_REG << 8) | seg->prot;
+	seg->measure = false;
+
+	if (seg->src == MAP_FAILED)
+		goto err;
 
-	encl->src_size = encl->segment_tbl[j - 1].offset +
-			 encl->segment_tbl[j - 1].size;
+	encl->src_size = encl->segment_tbl[j].offset + encl->segment_tbl[j].size;
 
 	for (encl->encl_size = 4096; encl->encl_size < encl->src_size; )
 		encl->encl_size <<= 1;
diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c
index e252015e0c15..fec85e1afd60 100644
--- a/tools/testing/selftests/sgx/main.c
+++ b/tools/testing/selftests/sgx/main.c
@@ -112,17 +112,57 @@ FIXTURE(enclave) {
 	struct sgx_enclave_run run;
 };
 
+#define SGX_TOTAL_MEM_PATH "/sys/kernel/debug/x86/sgx_total_mem"
+
+static int sysfs_get_ulong(const char *path, unsigned long *value)
+{
+	struct stat sbuf;
+	ssize_t ret = 0;
+	char buf[128];
+	int fd;
+
+	ret = stat(path, &sbuf);
+	if (ret)
+		return ret;
+
+	fd = open(path, O_RDONLY);
+	if (fd < 0)
+		return fd;
+
+	ret = read(fd, buf, sizeof(buf));
+	if (ret < 0)
+		goto out;
+
+	/* Clear the read bytes count. */
+	ret = 0;
+
+	errno = 0;
+	*value = strtoul(buf, NULL, 0);
+	if (errno)
+		ret = -1;
+
+out:
+	close(fd);
+	return ret;
+}
+
 FIXTURE_SETUP(enclave)
 {
 	Elf64_Sym *sgx_enter_enclave_sym = NULL;
 	struct vdso_symtab symtab;
 	struct encl_segment *seg;
+	unsigned long total_mem;
 	char maps_line[256];
 	FILE *maps_file;
 	unsigned int i;
 	void *addr;
+	int ret;
+
+	ret = sysfs_get_ulong(SGX_TOTAL_MEM_PATH, &total_mem);
+	if (ret)
+		ksft_exit_skip("Failed to read " SGX_TOTAL_MEM_PATH "\n");
 
-	if (!encl_load("test_encl.elf", &self->encl)) {
+	if (!encl_load("test_encl.elf", &self->encl, total_mem)) {
 		encl_delete(&self->encl);
 		ksft_exit_skip("cannot load enclaves\n");
 	}
diff --git a/tools/testing/selftests/sgx/main.h b/tools/testing/selftests/sgx/main.h
index 452d11dc4889..a286861dc289 100644
--- a/tools/testing/selftests/sgx/main.h
+++ b/tools/testing/selftests/sgx/main.h
@@ -12,6 +12,7 @@ struct encl_segment {
 	size_t size;
 	unsigned int prot;
 	unsigned int flags;
+	bool measure;
 };
 
 struct encl {
@@ -32,7 +33,7 @@ extern unsigned char sign_key[];
 extern unsigned char sign_key_end[];
 
 void encl_delete(struct encl *ctx);
-bool encl_load(const char *path, struct encl *encl);
+bool encl_load(const char *path, struct encl *encl, unsigned long heap_size);
 bool encl_measure(struct encl *encl);
 bool encl_build(struct encl *encl);
 
diff --git a/tools/testing/selftests/sgx/sigstruct.c b/tools/testing/selftests/sgx/sigstruct.c
index 202a96fd81bf..50c5ab1aa6fa 100644
--- a/tools/testing/selftests/sgx/sigstruct.c
+++ b/tools/testing/selftests/sgx/sigstruct.c
@@ -296,8 +296,10 @@ static bool mrenclave_segment(EVP_MD_CTX *ctx, struct encl *encl,
 		if (!mrenclave_eadd(ctx, seg->offset + offset, seg->flags))
 			return false;
 
-		if (!mrenclave_eextend(ctx, seg->offset + offset, seg->src + offset))
-			return false;
+		if (seg->measure) {
+			if (!mrenclave_eextend(ctx, seg->offset + offset, seg->src + offset))
+				return false;
+		}
 	}
 
 	return true;
-- 
2.32.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/4] x86/sgx: Add /sys/kernel/debug/x86/sgx_total_mem
  2021-07-28 22:51 ` [PATCH v2 1/4] x86/sgx: Add /sys/kernel/debug/x86/sgx_total_mem Jarkko Sakkinen
@ 2021-07-29 16:27   ` Dave Hansen
  2021-07-30  0:59     ` Jarkko Sakkinen
  0 siblings, 1 reply; 6+ messages in thread
From: Dave Hansen @ 2021-07-29 16:27 UTC (permalink / raw)
  To: Jarkko Sakkinen, Shuah Khan
  Cc: linux-kselftest, linux-sgx, Reinette Chatre, Borislav Petkov,
	Dave Hansen, Thomas Gleixner, Ingo Molnar, x86, H. Peter Anvin,
	Jonathan Corbet, linux-kernel, linux-doc

On 7/28/21 3:51 PM, Jarkko Sakkinen wrote:
> Create /sys/kernel/debug/x86/sgx_total_mem, after the driver is
> initialized, describing the total amount of SGX reserved memory (also known
> as Enclave Page Cache (EPC)) available in the system.
> 
> This can be used to implement stress that trigger the page reclaimer, and
> it is probably useful information when admistering a system running
> enclaves anyhow.

How about this as a rewrite?

Just like normal memory, SGX memory can be overcommitted.  SGX has its
own reclaim mechanism which kicks in when physical SGX memory (Enclave
Page Cache / EPC) is exhausted.  That reclaim mechanism is relatively
rarely exercised and needs selftests to poke at it.

The amount of EPC on the system is determined by the BIOS and it varies
wildly between systems.  It can be dozens of MB on desktops, or many GB
on servers.

To run in a reasonable amount of time, the selftest needs to know how
much EPC there is in the system.

Introduce a new debugfs file to export that information.

> diff --git a/Documentation/x86/sgx.rst b/Documentation/x86/sgx.rst
> index dd0ac96ff9ef..15bb8f4e42af 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.
> +
> +SGX debugging
> +=============
> +
> +*/sys/kernel/debug/x86/sgx_total_mem* describes the total available
> +SGX mwemory available in the system.

	^ memory

It would also be nice to say what the units are here, even if they are
bytes.

> diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c
> index 63d3de02bbcc..b65da19a53ee 100644
> --- a/arch/x86/kernel/cpu/sgx/main.c
> +++ b/arch/x86/kernel/cpu/sgx/main.c
> @@ -1,6 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0
>  /*  Copyright(c) 2016-20 Intel Corporation. */
>  
> +#include <linux/debugfs.h>
>  #include <linux/file.h>
>  #include <linux/freezer.h>
>  #include <linux/highmem.h>
> @@ -28,7 +29,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. */
> +/* Total EPC memory available in bytes. */
> +static unsigned long sgx_total_mem;
> +
> +/* 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 +660,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_total_mem += nr_pages * PAGE_SIZE;
> +
>  	return true;
>  }
>  
> @@ -823,6 +829,8 @@ static int __init sgx_init(void)
>  	if (sgx_vepc_init() && ret)
>  		goto err_provision;
>  
> +	debugfs_create_ulong("sgx_total_mem", 0444, arch_debugfs_dir, &sgx_total_mem);
> +
>  	return 0;
>  
>  err_provision:
> 

Assuming the new changelog is adopted and typos fixed:

Acked-by: Dave Hansen <dave.hansen@linux.intel.com>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/4] x86/sgx: Add /sys/kernel/debug/x86/sgx_total_mem
  2021-07-29 16:27   ` Dave Hansen
@ 2021-07-30  0:59     ` Jarkko Sakkinen
  0 siblings, 0 replies; 6+ messages in thread
From: Jarkko Sakkinen @ 2021-07-30  0:59 UTC (permalink / raw)
  To: Dave Hansen
  Cc: Shuah Khan, linux-kselftest, linux-sgx, Reinette Chatre,
	Borislav Petkov, Dave Hansen, Thomas Gleixner, Ingo Molnar, x86,
	H. Peter Anvin, Jonathan Corbet, linux-kernel, linux-doc

On Thu, Jul 29, 2021 at 09:27:41AM -0700, Dave Hansen wrote:
> On 7/28/21 3:51 PM, Jarkko Sakkinen wrote:
> > Create /sys/kernel/debug/x86/sgx_total_mem, after the driver is
> > initialized, describing the total amount of SGX reserved memory (also known
> > as Enclave Page Cache (EPC)) available in the system.
> > 
> > This can be used to implement stress that trigger the page reclaimer, and
> > it is probably useful information when admistering a system running
> > enclaves anyhow.
> 
> How about this as a rewrite?
> 
> Just like normal memory, SGX memory can be overcommitted.  SGX has its
> own reclaim mechanism which kicks in when physical SGX memory (Enclave
> Page Cache / EPC) is exhausted.  That reclaim mechanism is relatively
> rarely exercised and needs selftests to poke at it.
> 
> The amount of EPC on the system is determined by the BIOS and it varies
> wildly between systems.  It can be dozens of MB on desktops, or many GB
> on servers.
> 
> To run in a reasonable amount of time, the selftest needs to know how
> much EPC there is in the system.
> 
> Introduce a new debugfs file to export that information.
> 
> > diff --git a/Documentation/x86/sgx.rst b/Documentation/x86/sgx.rst
> > index dd0ac96ff9ef..15bb8f4e42af 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.
> > +
> > +SGX debugging
> > +=============
> > +
> > +*/sys/kernel/debug/x86/sgx_total_mem* describes the total available
> > +SGX mwemory available in the system.
> 
> 	^ memory
> 
> It would also be nice to say what the units are here, even if they are
> bytes.
> 
> > diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c
> > index 63d3de02bbcc..b65da19a53ee 100644
> > --- a/arch/x86/kernel/cpu/sgx/main.c
> > +++ b/arch/x86/kernel/cpu/sgx/main.c
> > @@ -1,6 +1,7 @@
> >  // SPDX-License-Identifier: GPL-2.0
> >  /*  Copyright(c) 2016-20 Intel Corporation. */
> >  
> > +#include <linux/debugfs.h>
> >  #include <linux/file.h>
> >  #include <linux/freezer.h>
> >  #include <linux/highmem.h>
> > @@ -28,7 +29,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. */
> > +/* Total EPC memory available in bytes. */
> > +static unsigned long sgx_total_mem;
> > +
> > +/* 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 +660,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_total_mem += nr_pages * PAGE_SIZE;
> > +
> >  	return true;
> >  }
> >  
> > @@ -823,6 +829,8 @@ static int __init sgx_init(void)
> >  	if (sgx_vepc_init() && ret)
> >  		goto err_provision;
> >  
> > +	debugfs_create_ulong("sgx_total_mem", 0444, arch_debugfs_dir, &sgx_total_mem);
> > +
> >  	return 0;
> >  
> >  err_provision:
> > 
> 
> Assuming the new changelog is adopted and typos fixed:
> 
> Acked-by: Dave Hansen <dave.hansen@linux.intel.com>

Thanks, I'll commit these changes and include to the next version.

/Jarkko

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-07-30  0:59 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20210728225140.248408-1-jarkko@kernel.org>
2021-07-28 22:51 ` [PATCH v2 1/4] x86/sgx: Add /sys/kernel/debug/x86/sgx_total_mem Jarkko Sakkinen
2021-07-29 16:27   ` Dave Hansen
2021-07-30  0:59     ` Jarkko Sakkinen
2021-07-28 22:51 ` [PATCH v2 2/4] selftests/sgx: Fix calculations for sub-maximum field sizes Jarkko Sakkinen
2021-07-28 22:51 ` [PATCH v2 3/4] selftests/sgx: Assign source for each segment Jarkko Sakkinen
2021-07-28 22:51 ` [PATCH v2 4/4] selftests/sgx: Trigger the reclaimer and #PF handler Jarkko Sakkinen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).