2021-07-05 14:38:22

by Jarkko Sakkinen

[permalink] [raw]
Subject: [PATCH 1/4] x86/sgx: Add sgx_nr_all_pages to the debugfs

Create /sys/kernel/debug/x86/sgx_nr_all_pages, which reports total
number of EPC pages available in the system.

Signed-off-by: Jarkko Sakkinen <[email protected]>
---
Documentation/x86/sgx.rst | 9 +++++++++
arch/x86/kernel/cpu/sgx/main.c | 10 +++++++++-
2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/Documentation/x86/sgx.rst b/Documentation/x86/sgx.rst
index dd0ac96ff9ef..3a8fefdebee0 100644
--- a/Documentation/x86/sgx.rst
+++ b/Documentation/x86/sgx.rst
@@ -250,3 +250,12 @@ 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
+=============
+
+The total number of available EPC pages can observed by:
+
+ # mount -t debugfs debugfs /sys/kernel/debug
+ # cat /sys/kernel/debug/x86/sgx_nr_all_pages
+ <the number of all EPC pages available in the system>
diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c
index 63d3de02bbcc..43e4b6215e62 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. */
+/* The number of EPC pages in total in all nodes. */
+static 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 +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_nr_all_pages += nr_pages;
+
return true;
}

@@ -823,6 +829,8 @@ static int __init sgx_init(void)
if (sgx_vepc_init() && ret)
goto err_provision;

+ debugfs_create_ulong("sgx_nr_all_pages", 0444, arch_debugfs_dir, &sgx_nr_all_pages);
+
return 0;

err_provision:
--
2.32.0


2021-07-06 14:58:22

by Dave Hansen

[permalink] [raw]
Subject: Re: [PATCH 1/4] x86/sgx: Add sgx_nr_all_pages to the debugfs

On 7/5/21 7:36 AM, Jarkko Sakkinen wrote:
> Create /sys/kernel/debug/x86/sgx_nr_all_pages, which reports total
> number of EPC pages available in the system.
Could we flesh this out a bit, please?

What's the value here when userspace could just re-enumerate the EPC
size from CPUID?

I'd really appreciate if we could draw parallels between these additions
to the "SGX VM" and their analogs in the "core VM". In this case, I
think the closest analog is probably "MemTotal" in /proc/meminfo.

Second, how is this going to be used?

Third, is this going to be the ABI forever?

2021-07-06 15:40:58

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH 1/4] x86/sgx: Add sgx_nr_all_pages to the debugfs

On Tue, Jul 06, 2021 at 07:56:40AM -0700, Dave Hansen wrote:
> On 7/5/21 7:36 AM, Jarkko Sakkinen wrote:
> > Create /sys/kernel/debug/x86/sgx_nr_all_pages, which reports total
> > number of EPC pages available in the system.
> Could we flesh this out a bit, please?
>
> What's the value here when userspace could just re-enumerate the EPC
> size from CPUID?
>
> I'd really appreciate if we could draw parallels between these additions
> to the "SGX VM" and their analogs in the "core VM". In this case, I
> think the closest analog is probably "MemTotal" in /proc/meminfo.
>
> Second, how is this going to be used?
>
> Third, is this going to be the ABI forever?

debugfs is never a stable abi. If it is being used as such, then the
kernel code is wrong. This better just be debugging stuff only.

thanks,

greg k-h

2021-07-06 22:10:33

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH 1/4] x86/sgx: Add sgx_nr_all_pages to the debugfs

On Tue, Jul 06, 2021 at 07:56:40AM -0700, Dave Hansen wrote:
> On 7/5/21 7:36 AM, Jarkko Sakkinen wrote:
> > Create /sys/kernel/debug/x86/sgx_nr_all_pages, which reports total
> > number of EPC pages available in the system.
> Could we flesh this out a bit, please?
>
> What's the value here when userspace could just re-enumerate the EPC
> size from CPUID?

My thinking is that it is better to use "kernel synthesized" value for the
EPC size, because kernel controls the EPC.

> I'd really appreciate if we could draw parallels between these additions
> to the "SGX VM" and their analogs in the "core VM". In this case, I
> think the closest analog is probably "MemTotal" in /proc/meminfo.

Would make sense.

> Second, how is this going to be used?

SGX kselftest creates a heap, of which size is the same as the total size
of the EPC reported by the kernel.

> Third, is this going to be the ABI forever?

AFAIK, debugfs is not part of the ABI.

/Jarkko