On Thu, 2021-08-26 at 14:19 +1200, Kai Huang wrote:
> On Thu, 26 Aug 2021 02:52:33 +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@xxxxxxxxxx>
> >
> > ---
> > 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 996e56590a10..d8e526b5487b 100644
> > --- a/arch/x86/include/asm/sgx.h
> > +++ b/arch/x86/include/asm/sgx.h
> > @@ -367,6 +367,8 @@ struct sgx_sigstruct {
> >
> > #ifdef CONFIG_X86_SGX
> >
> > +extern unsigned long sgx_nr_all_pages;
> > +
> > int sgx_set_attribute(unsigned long *allowed_attributes,
> > unsigned int attribute_fd);
> >
> > 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(§ion->pages[i].list, &sgx_dirty_page_list);
> > }
> >
> > + sgx_nr_all_pages += nr_pages;
> > +
>
> EPC sections can be freed again in sgx_init() after they are successfully
> initialized, when any further initialization fails (i.e. when fails to create
> ksgxd, or fails to register /dev/sgx_provision). In which case, I think
> sgx_nr_all_pages should also be cleared. But current sgx_init() seems doesn't
> reset it. Do you need to fix that too?
sgx_nr_all_pages tells just the total pages in the system, i.e. it's a constant.
Maybe a rename to "sgx_nr_total_pages" would be a good idea? Would match with
the meminfo field better too.
>
> > 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>
>
> How about only include <asm/sgx.h> when CONFIG_X86_SGX is on, then you don't
> have to do #ifdef CONFIG_X86_SGX changes to sgx.h?
Why do it that way instead of doing it once in sgx.h for every site that wants
to include the file?
/Jarkko
