Re: [PATCH v13 2/2] x86/sgx: Add an attribute for the amount of SGX memory in a NUMA node

From: Jarkko Sakkinen <jarkko@kernel.org>
To: Dave Hansen <dave.hansen@intel.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	x86@kernel.org, "H. Peter Anvin" <hpa@zytor.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>
Cc: reinette.chatre@intel.com, linux-kernel@vger.kernel.org,
	linux-sgx@vger.kernel.org
Subject: Re: [PATCH v13 2/2] x86/sgx: Add an attribute for the amount of SGX memory in a NUMA node
Date: Sat, 11 Dec 2021 17:36:31 +0200	[thread overview]
Message-ID: <d6ec8429763db7ddba42d9ff658a929b274f2711.camel@kernel.org> (raw)
In-Reply-To: <f25d95e6-e129-597b-5d93-d7264feae8b8@intel.com>

On Tue, 2021-12-07 at 11:36 -0800, Dave Hansen wrote:
> On 11/16/21 8:21 AM, 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.
> > 
> > Introduce CONFIG_HAVE_ARCH_NODE_DEV_GROUP opt-in flag to expose an arch
> > specific attribute group, and add an attribute for the amount of SGX
> > memory in bytes to each NUMA node:
> > 
> > /sys/devices/system/node/nodeX/x86/sgx_total_bytes
> 
> There's some context missing here:
> 
> This serves the same function for SGX memory as /proc/meminfo or
> /sys/devices/system/node/nodeX/meminfo does for normal RAM.  It
> enumerates how much physical SGX memory is present so that you can size
> enclaves on different systems.
> 
> This specific file (sgx_total_bytes) is needed today to help drive the
> SGX selftests.  The SGX selftests need to create overcommitted enclaves
> which are larger than the physical SGX memory on the system.  They
> currently use a CPUID-based approach which can diverge from the actual
> amount of SGX memory available.  This file ensures that the selftests
> can work efficiently and do not attempt stupid things like creating a
> 100,000 MB enclave on a system with 128 MB  of SGX memory.
> 
> The nodeX/x86 directory is used because SGX is highly x86-specific.
> It's very unlikely that any other architecture (or even non-Intel x86
> vendor) will ever implement SGX.  It needs its own directory (as opposed
> to being in the nodeX/ "root") because this is expected to be the first
> of a few different things that need to get exported.  This avoids
> cluttering the root with several "sgx_*" files.
> 
> How many of these files will there be?  Just scanning /proc/meminfo,
> these are the no-brainers that we have for RAM, but we need for SGX:
> 
> MemTotal:       xxxx kB // sgx_total_bytes (this patch)
> MemFree:        yyyy kB // sgx_free_bytes
> SwapTotal:      zzzz kB // sgx_swapped_bytes
> 
> So, at *least* three.  I think we will eventually end up needing
> something more along the lines of a dozen.

These three I've had in mind for the moment. The latter two will be
trivial to add now that we have pattern how to add the sysfs attribute
in the correct way.

/Jarkko