linux-sgx.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 1/3] x86/sgx: Report SGX memory in /sys/devices/system/node/node*/meminfo
@ 2021-09-10  0:17 Jarkko Sakkinen
  2021-09-10  0:17 ` [PATCH v4 2/3] x86/sgx: Report SGX memory in /proc/meminfo Jarkko Sakkinen
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Jarkko Sakkinen @ 2021-09-10  0:17 UTC (permalink / raw)
  To: Jarkko Sakkinen, Dave Hansen, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, x86, H. Peter Anvin, Greg Kroah-Hartman,
	Rafael J. Wysocki
  Cc: linux-sgx, linux-kernel

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 /sys/devices/system/node/node*/meminfo,
showing the total SGX memory in each NUMA node. The total memory for
each NUMA node is calculated by adding the sizes of contained EPC
sections together.

Introduce arch_node_read_meminfo(), which can optionally be rewritten by
the arch code, and rewrite it for x86 so it prints SGX_MemTotal.

Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
v4:
* A new patch.
 arch/x86/kernel/cpu/sgx/main.c | 14 ++++++++++++++
 arch/x86/kernel/cpu/sgx/sgx.h  |  6 ++++++
 drivers/base/node.c            | 10 +++++++++-
 3 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c
index 63d3de02bbcc..4c6da5f4a9d4 100644
--- a/arch/x86/kernel/cpu/sgx/main.c
+++ b/arch/x86/kernel/cpu/sgx/main.c
@@ -717,6 +717,7 @@ static bool __init sgx_page_cache_init(void)
 		}
 
 		sgx_epc_sections[i].node =  &sgx_numa_nodes[nid];
+		sgx_numa_nodes[nid].size += size;
 
 		sgx_nr_epc_sections++;
 	}
@@ -790,6 +791,19 @@ int sgx_set_attribute(unsigned long *allowed_attributes,
 }
 EXPORT_SYMBOL_GPL(sgx_set_attribute);
 
+ssize_t arch_node_read_meminfo(struct device *dev,
+			       struct device_attribute *attr,
+			       char *buf, int len)
+{
+	struct sgx_numa_node *node = &sgx_numa_nodes[dev->id];
+
+	len += sysfs_emit_at(buf, len,
+			     "Node %d SGX_MemTotal:   %8lu kB\n",
+			     dev->id, node->size);
+
+	return len;
+}
+
 static int __init sgx_init(void)
 {
 	int ret;
diff --git a/arch/x86/kernel/cpu/sgx/sgx.h b/arch/x86/kernel/cpu/sgx/sgx.h
index 4628acec0009..74713b98a859 100644
--- a/arch/x86/kernel/cpu/sgx/sgx.h
+++ b/arch/x86/kernel/cpu/sgx/sgx.h
@@ -39,6 +39,7 @@ struct sgx_epc_page {
  */
 struct sgx_numa_node {
 	struct list_head free_page_list;
+	unsigned long size;
 	spinlock_t lock;
 };
 
@@ -95,4 +96,9 @@ static inline int __init sgx_vepc_init(void)
 
 void sgx_update_lepubkeyhash(u64 *lepubkeyhash);
 
+extern ssize_t arch_node_read_meminfo(struct device *dev,
+				      struct device_attribute *attr,
+				      char *buf, int len);
+
+
 #endif /* _X86_SGX_H */
diff --git a/drivers/base/node.c b/drivers/base/node.c
index 4a4ae868ad9f..91eaa2e2ce33 100644
--- a/drivers/base/node.c
+++ b/drivers/base/node.c
@@ -361,6 +361,13 @@ static void node_init_caches(unsigned int nid) { }
 static void node_remove_caches(struct node *node) { }
 #endif
 
+ssize_t __weak arch_node_read_meminfo(struct device *dev,
+				      struct device_attribute *attr,
+				      char *buf, int len)
+{
+	return len;
+}
+
 #define K(x) ((x) << (PAGE_SHIFT - 10))
 static ssize_t node_read_meminfo(struct device *dev,
 			struct device_attribute *attr, char *buf)
@@ -473,7 +480,8 @@ static ssize_t node_read_meminfo(struct device *dev,
 #endif
 			    );
 	len += hugetlb_report_node_meminfo(buf, len, nid);
-	return len;
+
+	return arch_node_read_meminfo(dev, attr, buf, len);
 }
 
 #undef K
-- 
2.25.1


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

* [PATCH v4 2/3] x86/sgx: Report SGX memory in /proc/meminfo
  2021-09-10  0:17 [PATCH v4 1/3] x86/sgx: Report SGX memory in /sys/devices/system/node/node*/meminfo Jarkko Sakkinen
@ 2021-09-10  0:17 ` Jarkko Sakkinen
  2021-09-10  0:20   ` Jarkko Sakkinen
  2021-09-10  0:17 ` [PATCH v4 3/3] x86/sgx: Document SGX_MemTotal to Documentation/x86/meminfo.rst Jarkko Sakkinen
  2021-09-10  6:51 ` [PATCH v4 1/3] x86/sgx: Report SGX memory in /sys/devices/system/node/node*/meminfo Greg Kroah-Hartman
  2 siblings, 1 reply; 10+ messages in thread
From: Jarkko Sakkinen @ 2021-09-10  0:17 UTC (permalink / raw)
  To: Jarkko Sakkinen, Dave Hansen, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, x86, H. Peter Anvin, Andy Lutomirski,
	Peter Zijlstra
  Cc: linux-sgx, linux-kernel

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

Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
v4:
* Calculate sgx_mem_total by adding up the sizes of the NUMA node
  EPC's, only once all of them have been set up correctly.
* Move documentation to a separate patch, as it clearly is a
  separate issue.
v2:
* Move ifdef fix for sgx_set_attribute() to a separate patch.
---
 arch/x86/include/asm/sgx.h     |  2 ++
 arch/x86/kernel/cpu/sgx/main.c | 10 ++++++++--
 arch/x86/mm/pat/set_memory.c   |  5 +++++
 3 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/sgx.h b/arch/x86/include/asm/sgx.h
index 05f3e21f01a7..65124b05c145 100644
--- a/arch/x86/include/asm/sgx.h
+++ b/arch/x86/include/asm/sgx.h
@@ -365,6 +365,8 @@ struct sgx_sigstruct {
  * comment!
  */
 
+extern unsigned long sgx_mem_total;
+
 #ifdef CONFIG_X86_SGX_KVM
 int sgx_virt_ecreate(struct sgx_pageinfo *pageinfo, void __user *secs,
 		     int *trapnr);
diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c
index 4c6da5f4a9d4..d8f5769e4004 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. */
+/* Total size of the Enclave Page Cache (EPC) in the system. */
+unsigned long sgx_mem_total;
+
+/* The number of free EPC pages in all nodes. */
 static unsigned long sgx_nr_free_pages;
 
 /* Nodes with one or more EPC sections. */
@@ -727,6 +730,9 @@ static bool __init sgx_page_cache_init(void)
 		return false;
 	}
 
+	for (i = 0; i < num_possible_nodes(); i++)
+		sgx_mem_total += sgx_numa_nodes[nid].size;
+
 	return true;
 }
 
@@ -799,7 +805,7 @@ ssize_t arch_node_read_meminfo(struct device *dev,
 
 	len += sysfs_emit_at(buf, len,
 			     "Node %d SGX_MemTotal:   %8lu kB\n",
-			     dev->id, node->size);
+			     dev->id, node->size / 1024);
 
 	return len;
 }
diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
index ad8a5c586a35..b55ea89df801 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>
 
 #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_mem_total / 1024);
+#endif
 }
 #else
 static inline void split_page_count(int level) { }
-- 
2.25.1


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

* [PATCH v4 3/3] x86/sgx: Document SGX_MemTotal to Documentation/x86/meminfo.rst
  2021-09-10  0:17 [PATCH v4 1/3] x86/sgx: Report SGX memory in /sys/devices/system/node/node*/meminfo Jarkko Sakkinen
  2021-09-10  0:17 ` [PATCH v4 2/3] x86/sgx: Report SGX memory in /proc/meminfo Jarkko Sakkinen
@ 2021-09-10  0:17 ` Jarkko Sakkinen
  2021-09-10  6:51 ` [PATCH v4 1/3] x86/sgx: Report SGX memory in /sys/devices/system/node/node*/meminfo Greg Kroah-Hartman
  2 siblings, 0 replies; 10+ messages in thread
From: Jarkko Sakkinen @ 2021-09-10  0:17 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
	H. Peter Anvin, Jonathan Corbet, Jarkko Sakkinen, Dave Hansen
  Cc: Dave Hansen, linux-kernel, linux-doc, linux-sgx

Document SGX_MemTotal field of /proc/meminfo to
Documentation/x86/meminfo.rst. This is a new file for x86 specific
meminfo fields.

Suggested-by: Dave Hansen <dave.hansen@intel.com>
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
v4:
* A new patch.
 Documentation/x86/meminfo.rst | 11 +++++++++++
 1 file changed, 11 insertions(+)
 create mode 100644 Documentation/x86/meminfo.rst

diff --git a/Documentation/x86/meminfo.rst b/Documentation/x86/meminfo.rst
new file mode 100644
index 000000000000..999b676a04a8
--- /dev/null
+++ b/Documentation/x86/meminfo.rst
@@ -0,0 +1,11 @@
+This document describes x86 specific fields of /proc/meminfo.
+
+Supplemental fields for /proc/meminfo
+=====================================
+
+SGX_MemTotal
+                Total usable physical SGX memory, also known as Enclave
+                Page Cache (EPC), in kB's. This is the physical memory
+                used for enclave pages. It is below the amount set in
+                the BIOS: a portion of that memory is required for
+                the state of the enclave pages.
-- 
2.25.1


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

* Re: [PATCH v4 2/3] x86/sgx: Report SGX memory in /proc/meminfo
  2021-09-10  0:17 ` [PATCH v4 2/3] x86/sgx: Report SGX memory in /proc/meminfo Jarkko Sakkinen
@ 2021-09-10  0:20   ` Jarkko Sakkinen
  0 siblings, 0 replies; 10+ messages in thread
From: Jarkko Sakkinen @ 2021-09-10  0:20 UTC (permalink / raw)
  To: Dave Hansen, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
	H. Peter Anvin, Andy Lutomirski, Peter Zijlstra
  Cc: linux-sgx, linux-kernel

On Fri, 2021-09-10 at 03:17 +0300, Jarkko Sakkinen wrote:
> @@ -799,7 +805,7 @@ ssize_t arch_node_read_meminfo(struct device *dev,
>  
>  	len += sysfs_emit_at(buf, len,
>  			     "Node %d SGX_MemTotal:   %8lu kB\n",
> -			     dev->id, node->size);
> +			     dev->id, node->size / 1024);
>  
>  	return len;
>  }

This should have been in 1/3.

/Jarkko


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

* Re: [PATCH v4 1/3] x86/sgx: Report SGX memory in /sys/devices/system/node/node*/meminfo
  2021-09-10  0:17 [PATCH v4 1/3] x86/sgx: Report SGX memory in /sys/devices/system/node/node*/meminfo Jarkko Sakkinen
  2021-09-10  0:17 ` [PATCH v4 2/3] x86/sgx: Report SGX memory in /proc/meminfo Jarkko Sakkinen
  2021-09-10  0:17 ` [PATCH v4 3/3] x86/sgx: Document SGX_MemTotal to Documentation/x86/meminfo.rst Jarkko Sakkinen
@ 2021-09-10  6:51 ` Greg Kroah-Hartman
  2021-09-10 13:17   ` Jarkko Sakkinen
  2 siblings, 1 reply; 10+ messages in thread
From: Greg Kroah-Hartman @ 2021-09-10  6:51 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: Dave Hansen, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
	H. Peter Anvin, Rafael J. Wysocki, linux-sgx, linux-kernel

On Fri, Sep 10, 2021 at 03:17:24AM +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 /sys/devices/system/node/node*/meminfo,
> showing the total SGX memory in each NUMA node. The total memory for
> each NUMA node is calculated by adding the sizes of contained EPC
> sections together.
> 
> Introduce arch_node_read_meminfo(), which can optionally be rewritten by
> the arch code, and rewrite it for x86 so it prints SGX_MemTotal.
> 
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> ---
> v4:
> * A new patch.
>  arch/x86/kernel/cpu/sgx/main.c | 14 ++++++++++++++
>  arch/x86/kernel/cpu/sgx/sgx.h  |  6 ++++++
>  drivers/base/node.c            | 10 +++++++++-
>  3 files changed, 29 insertions(+), 1 deletion(-)

Where is the Documentation/ABI/ update for this new sysfs file?

> 
> diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c
> index 63d3de02bbcc..4c6da5f4a9d4 100644
> --- a/arch/x86/kernel/cpu/sgx/main.c
> +++ b/arch/x86/kernel/cpu/sgx/main.c
> @@ -717,6 +717,7 @@ static bool __init sgx_page_cache_init(void)
>  		}
>  
>  		sgx_epc_sections[i].node =  &sgx_numa_nodes[nid];
> +		sgx_numa_nodes[nid].size += size;
>  
>  		sgx_nr_epc_sections++;
>  	}
> @@ -790,6 +791,19 @@ int sgx_set_attribute(unsigned long *allowed_attributes,
>  }
>  EXPORT_SYMBOL_GPL(sgx_set_attribute);
>  
> +ssize_t arch_node_read_meminfo(struct device *dev,
> +			       struct device_attribute *attr,
> +			       char *buf, int len)
> +{
> +	struct sgx_numa_node *node = &sgx_numa_nodes[dev->id];
> +
> +	len += sysfs_emit_at(buf, len,
> +			     "Node %d SGX_MemTotal:   %8lu kB\n",
> +			     dev->id, node->size);

Wait, that is not how sysfs files work.  they are "one value per file"
Please do not have multiple values in a single sysfs file, that is not
acceptable at all.

thanks,

greg k-h

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

* Re: [PATCH v4 1/3] x86/sgx: Report SGX memory in /sys/devices/system/node/node*/meminfo
  2021-09-10  6:51 ` [PATCH v4 1/3] x86/sgx: Report SGX memory in /sys/devices/system/node/node*/meminfo Greg Kroah-Hartman
@ 2021-09-10 13:17   ` Jarkko Sakkinen
  2021-09-10 14:05     ` Greg Kroah-Hartman
  0 siblings, 1 reply; 10+ messages in thread
From: Jarkko Sakkinen @ 2021-09-10 13:17 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Dave Hansen, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
	H. Peter Anvin, Rafael J. Wysocki, linux-sgx, linux-kernel

On Fri, 2021-09-10 at 08:51 +0200, Greg Kroah-Hartman wrote:
> On Fri, Sep 10, 2021 at 03:17:24AM +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 /sys/devices/system/node/node*/meminfo,
> > showing the total SGX memory in each NUMA node. The total memory for
> > each NUMA node is calculated by adding the sizes of contained EPC
> > sections together.
> > 
> > Introduce arch_node_read_meminfo(), which can optionally be rewritten by
> > the arch code, and rewrite it for x86 so it prints SGX_MemTotal.
> > 
> > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> > ---
> > v4:
> > * A new patch.
> >  arch/x86/kernel/cpu/sgx/main.c | 14 ++++++++++++++
> >  arch/x86/kernel/cpu/sgx/sgx.h  |  6 ++++++
> >  drivers/base/node.c            | 10 +++++++++-
> >  3 files changed, 29 insertions(+), 1 deletion(-)
> 
> Where is the Documentation/ABI/ update for this new sysfs file?

It's has been existed for a long time, e.g.

 cat /sys/devices/system/node/node0/meminfo
Node 0 MemTotal:       32706792 kB
Node 0 MemFree:         5382988 kB
Node 0 MemUsed:        27323804 kB
Node 0 SwapCached:            8 kB
Node 0 Active:          3640612 kB
Node 0 Inactive:       21757684 kB
Node 0 Active(anon):    2833772 kB
Node 0 Inactive(anon):    14392 kB
Node 0 Active(file):     806840 kB
Node 0 Inactive(file): 21743292 kB
Node 0 Unevictable:       80640 kB
Node 0 Mlocked:           80640 kB
Node 0 Dirty:                36 kB
Node 0 Writeback:             0 kB
Node 0 FilePages:      22833124 kB
Node 0 Mapped:          1112832 kB
Node 0 AnonPages:       2645812 kB
Node 0 Shmem:            282984 kB
Node 0 KernelStack:       18544 kB
Node 0 PageTables:        46704 kB
Node 0 NFS_Unstable:          0 kB
Node 0 Bounce:                0 kB
Node 0 WritebackTmp:          0 kB
Node 0 KReclaimable:    1311812 kB
Node 0 Slab:            1542220 kB
Node 0 SReclaimable:    1311812 kB
Node 0 SUnreclaim:       230408 kB
Node 0 AnonHugePages:         0 kB
Node 0 ShmemHugePages:        0 kB
Node 0 ShmemPmdMapped:        0 kB
Node 0 FileHugePages:        0 kB
Node 0 FilePmdMapped:        0 kB
Node 0 HugePages_Total:     0
Node 0 HugePages_Free:      0
Node 0 HugePages_Surp:      0

This file is undocumented but the fields seem to reflect what is in
/proc/meminfo, so I added additional patch for documentation:

https://lore.kernel.org/linux-sgx/20210910001726.811497-3-jarkko@kernel.org/

I have no idea why things are how they are. I'm just merely trying to follow
the existing patterns. I'm also fully aware of the defacto sysfs pattern, i.e.
one value per file.

I figured, since the situation is how it is, that I end up doing this wrong
in a way or another, so this the anti-pattern I picked for my wrong doings
:-) I'm sorry about it.

> > diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c
> > index 63d3de02bbcc..4c6da5f4a9d4 100644
> > --- a/arch/x86/kernel/cpu/sgx/main.c
> > +++ b/arch/x86/kernel/cpu/sgx/main.c
> > @@ -717,6 +717,7 @@ static bool __init sgx_page_cache_init(void)
> >  		}
> >  
> >  		sgx_epc_sections[i].node =  &sgx_numa_nodes[nid];
> > +		sgx_numa_nodes[nid].size += size;
> >  
> >  		sgx_nr_epc_sections++;
> >  	}
> > @@ -790,6 +791,19 @@ int sgx_set_attribute(unsigned long *allowed_attributes,
> >  }
> >  EXPORT_SYMBOL_GPL(sgx_set_attribute);
> >  
> > +ssize_t arch_node_read_meminfo(struct device *dev,
> > +			       struct device_attribute *attr,
> > +			       char *buf, int len)
> > +{
> > +	struct sgx_numa_node *node = &sgx_numa_nodes[dev->id];
> > +
> > +	len += sysfs_emit_at(buf, len,
> > +			     "Node %d SGX_MemTotal:   %8lu kB\n",
> > +			     dev->id, node->size);
> 
> Wait, that is not how sysfs files work.  they are "one value per file"
> Please do not have multiple values in a single sysfs file, that is not
> acceptable at all.

Yeah, I'm wondering what would be the right corrective steps, given the
"established science".

> thanks,
> 
> greg k-h

/Jarkko


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

* Re: [PATCH v4 1/3] x86/sgx: Report SGX memory in /sys/devices/system/node/node*/meminfo
  2021-09-10 13:17   ` Jarkko Sakkinen
@ 2021-09-10 14:05     ` Greg Kroah-Hartman
  2021-09-10 15:02       ` Jarkko Sakkinen
  0 siblings, 1 reply; 10+ messages in thread
From: Greg Kroah-Hartman @ 2021-09-10 14:05 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: Dave Hansen, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
	H. Peter Anvin, Rafael J. Wysocki, linux-sgx, linux-kernel

On Fri, Sep 10, 2021 at 04:17:44PM +0300, Jarkko Sakkinen wrote:
> On Fri, 2021-09-10 at 08:51 +0200, Greg Kroah-Hartman wrote:
> > On Fri, Sep 10, 2021 at 03:17:24AM +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 /sys/devices/system/node/node*/meminfo,
> > > showing the total SGX memory in each NUMA node. The total memory for
> > > each NUMA node is calculated by adding the sizes of contained EPC
> > > sections together.
> > > 
> > > Introduce arch_node_read_meminfo(), which can optionally be rewritten by
> > > the arch code, and rewrite it for x86 so it prints SGX_MemTotal.
> > > 
> > > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> > > ---
> > > v4:
> > > * A new patch.
> > >  arch/x86/kernel/cpu/sgx/main.c | 14 ++++++++++++++
> > >  arch/x86/kernel/cpu/sgx/sgx.h  |  6 ++++++
> > >  drivers/base/node.c            | 10 +++++++++-
> > >  3 files changed, 29 insertions(+), 1 deletion(-)
> > 
> > Where is the Documentation/ABI/ update for this new sysfs file?
> 
> It's has been existed for a long time, e.g.
> 
>  cat /sys/devices/system/node/node0/meminfo
> Node 0 MemTotal:       32706792 kB
> Node 0 MemFree:         5382988 kB
> Node 0 MemUsed:        27323804 kB
> Node 0 SwapCached:            8 kB
> Node 0 Active:          3640612 kB
> Node 0 Inactive:       21757684 kB
> Node 0 Active(anon):    2833772 kB
> Node 0 Inactive(anon):    14392 kB
> Node 0 Active(file):     806840 kB
> Node 0 Inactive(file): 21743292 kB
> Node 0 Unevictable:       80640 kB
> Node 0 Mlocked:           80640 kB
> Node 0 Dirty:                36 kB
> Node 0 Writeback:             0 kB
> Node 0 FilePages:      22833124 kB
> Node 0 Mapped:          1112832 kB
> Node 0 AnonPages:       2645812 kB
> Node 0 Shmem:            282984 kB
> Node 0 KernelStack:       18544 kB
> Node 0 PageTables:        46704 kB
> Node 0 NFS_Unstable:          0 kB
> Node 0 Bounce:                0 kB
> Node 0 WritebackTmp:          0 kB
> Node 0 KReclaimable:    1311812 kB
> Node 0 Slab:            1542220 kB
> Node 0 SReclaimable:    1311812 kB
> Node 0 SUnreclaim:       230408 kB
> Node 0 AnonHugePages:         0 kB
> Node 0 ShmemHugePages:        0 kB
> Node 0 ShmemPmdMapped:        0 kB
> Node 0 FileHugePages:        0 kB
> Node 0 FilePmdMapped:        0 kB
> Node 0 HugePages_Total:     0
> Node 0 HugePages_Free:      0
> Node 0 HugePages_Surp:      0
> 
> This file is undocumented but the fields seem to reflect what is in
> /proc/meminfo, so I added additional patch for documentation:
> 
> https://lore.kernel.org/linux-sgx/20210910001726.811497-3-jarkko@kernel.org/
> 
> I have no idea why things are how they are. I'm just merely trying to follow
> the existing patterns. I'm also fully aware of the defacto sysfs pattern, i.e.
> one value per file.

Then please do not add anything else to this nightmare of a mess.

thanks,

greg k-h

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

* Re: [PATCH v4 1/3] x86/sgx: Report SGX memory in /sys/devices/system/node/node*/meminfo
  2021-09-10 14:05     ` Greg Kroah-Hartman
@ 2021-09-10 15:02       ` Jarkko Sakkinen
  2021-09-10 15:11         ` Greg Kroah-Hartman
  0 siblings, 1 reply; 10+ messages in thread
From: Jarkko Sakkinen @ 2021-09-10 15:02 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Dave Hansen, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
	H. Peter Anvin, Rafael J. Wysocki, linux-sgx, linux-kernel

On Fri, 2021-09-10 at 16:05 +0200, Greg Kroah-Hartman wrote:
> On Fri, Sep 10, 2021 at 04:17:44PM +0300, Jarkko Sakkinen wrote:
> > On Fri, 2021-09-10 at 08:51 +0200, Greg Kroah-Hartman wrote:
> > > On Fri, Sep 10, 2021 at 03:17:24AM +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 /sys/devices/system/node/node*/meminfo,
> > > > showing the total SGX memory in each NUMA node. The total memory for
> > > > each NUMA node is calculated by adding the sizes of contained EPC
> > > > sections together.
> > > > 
> > > > Introduce arch_node_read_meminfo(), which can optionally be rewritten by
> > > > the arch code, and rewrite it for x86 so it prints SGX_MemTotal.
> > > > 
> > > > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> > > > ---
> > > > v4:
> > > > * A new patch.
> > > >  arch/x86/kernel/cpu/sgx/main.c | 14 ++++++++++++++
> > > >  arch/x86/kernel/cpu/sgx/sgx.h  |  6 ++++++
> > > >  drivers/base/node.c            | 10 +++++++++-
> > > >  3 files changed, 29 insertions(+), 1 deletion(-)
> > > 
> > > Where is the Documentation/ABI/ update for this new sysfs file?
> > 
> > It's has been existed for a long time, e.g.
> > 
> >  cat /sys/devices/system/node/node0/meminfo
> > Node 0 MemTotal:       32706792 kB
> > Node 0 MemFree:         5382988 kB
> > Node 0 MemUsed:        27323804 kB
> > Node 0 SwapCached:            8 kB
> > Node 0 Active:          3640612 kB
> > Node 0 Inactive:       21757684 kB
> > Node 0 Active(anon):    2833772 kB
> > Node 0 Inactive(anon):    14392 kB
> > Node 0 Active(file):     806840 kB
> > Node 0 Inactive(file): 21743292 kB
> > Node 0 Unevictable:       80640 kB
> > Node 0 Mlocked:           80640 kB
> > Node 0 Dirty:                36 kB
> > Node 0 Writeback:             0 kB
> > Node 0 FilePages:      22833124 kB
> > Node 0 Mapped:          1112832 kB
> > Node 0 AnonPages:       2645812 kB
> > Node 0 Shmem:            282984 kB
> > Node 0 KernelStack:       18544 kB
> > Node 0 PageTables:        46704 kB
> > Node 0 NFS_Unstable:          0 kB
> > Node 0 Bounce:                0 kB
> > Node 0 WritebackTmp:          0 kB
> > Node 0 KReclaimable:    1311812 kB
> > Node 0 Slab:            1542220 kB
> > Node 0 SReclaimable:    1311812 kB
> > Node 0 SUnreclaim:       230408 kB
> > Node 0 AnonHugePages:         0 kB
> > Node 0 ShmemHugePages:        0 kB
> > Node 0 ShmemPmdMapped:        0 kB
> > Node 0 FileHugePages:        0 kB
> > Node 0 FilePmdMapped:        0 kB
> > Node 0 HugePages_Total:     0
> > Node 0 HugePages_Free:      0
> > Node 0 HugePages_Surp:      0

This was something also spinned my head a bit, i.e. why hugepages fields
are not aligned correctly.

> > 
> > This file is undocumented but the fields seem to reflect what is in
> > /proc/meminfo, so I added additional patch for documentation:
> > 
> > https://lore.kernel.org/linux-sgx/20210910001726.811497-3-jarkko@kernel.org/
> > 
> > I have no idea why things are how they are. I'm just merely trying to follow
> > the existing patterns. I'm also fully aware of the defacto sysfs pattern, i.e.
> > one value per file.
> 
> Then please do not add anything else to this nightmare of a mess.

There is already /sys/devices/system/node/node0/hugepages/.

It has alike data to the contents of meminfo:

/sys/devices/system/node/node0/hugepages/hugepages-2048kB:
surplus_hugepages
nr_hugepages
free_hugepages

/sys/devices/system/node/node0/hugepages/hugepages-1048576kB:
surplus_hugepages
nr_hugepages
free_hugepages

[I'm wondering tho, how the fields in meminfo are supposed to be interpreted,
 given that there are two types of huge pages, but let's just ignore that
 for the moment.]

Following this pattern, I'd perhaps go and create (and document):

/sys/devices/system/node/node0/sgx/memory_size

which would have the size in bytes.

> thanks,
> 
> greg k-h

/Jarkko


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

* Re: [PATCH v4 1/3] x86/sgx: Report SGX memory in /sys/devices/system/node/node*/meminfo
  2021-09-10 15:02       ` Jarkko Sakkinen
@ 2021-09-10 15:11         ` Greg Kroah-Hartman
  2021-09-12 19:37           ` Jarkko Sakkinen
  0 siblings, 1 reply; 10+ messages in thread
From: Greg Kroah-Hartman @ 2021-09-10 15:11 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: Dave Hansen, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
	H. Peter Anvin, Rafael J. Wysocki, linux-sgx, linux-kernel

On Fri, Sep 10, 2021 at 06:02:59PM +0300, Jarkko Sakkinen wrote:
> On Fri, 2021-09-10 at 16:05 +0200, Greg Kroah-Hartman wrote:
> > On Fri, Sep 10, 2021 at 04:17:44PM +0300, Jarkko Sakkinen wrote:
> > > On Fri, 2021-09-10 at 08:51 +0200, Greg Kroah-Hartman wrote:
> > > > On Fri, Sep 10, 2021 at 03:17:24AM +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 /sys/devices/system/node/node*/meminfo,
> > > > > showing the total SGX memory in each NUMA node. The total memory for
> > > > > each NUMA node is calculated by adding the sizes of contained EPC
> > > > > sections together.
> > > > > 
> > > > > Introduce arch_node_read_meminfo(), which can optionally be rewritten by
> > > > > the arch code, and rewrite it for x86 so it prints SGX_MemTotal.
> > > > > 
> > > > > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> > > > > ---
> > > > > v4:
> > > > > * A new patch.
> > > > >  arch/x86/kernel/cpu/sgx/main.c | 14 ++++++++++++++
> > > > >  arch/x86/kernel/cpu/sgx/sgx.h  |  6 ++++++
> > > > >  drivers/base/node.c            | 10 +++++++++-
> > > > >  3 files changed, 29 insertions(+), 1 deletion(-)
> > > > 
> > > > Where is the Documentation/ABI/ update for this new sysfs file?
> > > 
> > > It's has been existed for a long time, e.g.
> > > 
> > >  cat /sys/devices/system/node/node0/meminfo
> > > Node 0 MemTotal:       32706792 kB
> > > Node 0 MemFree:         5382988 kB
> > > Node 0 MemUsed:        27323804 kB
> > > Node 0 SwapCached:            8 kB
> > > Node 0 Active:          3640612 kB
> > > Node 0 Inactive:       21757684 kB
> > > Node 0 Active(anon):    2833772 kB
> > > Node 0 Inactive(anon):    14392 kB
> > > Node 0 Active(file):     806840 kB
> > > Node 0 Inactive(file): 21743292 kB
> > > Node 0 Unevictable:       80640 kB
> > > Node 0 Mlocked:           80640 kB
> > > Node 0 Dirty:                36 kB
> > > Node 0 Writeback:             0 kB
> > > Node 0 FilePages:      22833124 kB
> > > Node 0 Mapped:          1112832 kB
> > > Node 0 AnonPages:       2645812 kB
> > > Node 0 Shmem:            282984 kB
> > > Node 0 KernelStack:       18544 kB
> > > Node 0 PageTables:        46704 kB
> > > Node 0 NFS_Unstable:          0 kB
> > > Node 0 Bounce:                0 kB
> > > Node 0 WritebackTmp:          0 kB
> > > Node 0 KReclaimable:    1311812 kB
> > > Node 0 Slab:            1542220 kB
> > > Node 0 SReclaimable:    1311812 kB
> > > Node 0 SUnreclaim:       230408 kB
> > > Node 0 AnonHugePages:         0 kB
> > > Node 0 ShmemHugePages:        0 kB
> > > Node 0 ShmemPmdMapped:        0 kB
> > > Node 0 FileHugePages:        0 kB
> > > Node 0 FilePmdMapped:        0 kB
> > > Node 0 HugePages_Total:     0
> > > Node 0 HugePages_Free:      0
> > > Node 0 HugePages_Surp:      0
> 
> This was something also spinned my head a bit, i.e. why hugepages fields
> are not aligned correctly.
> 
> > > 
> > > This file is undocumented but the fields seem to reflect what is in
> > > /proc/meminfo, so I added additional patch for documentation:
> > > 
> > > https://lore.kernel.org/linux-sgx/20210910001726.811497-3-jarkko@kernel.org/
> > > 
> > > I have no idea why things are how they are. I'm just merely trying to follow
> > > the existing patterns. I'm also fully aware of the defacto sysfs pattern, i.e.
> > > one value per file.
> > 
> > Then please do not add anything else to this nightmare of a mess.
> 
> There is already /sys/devices/system/node/node0/hugepages/.
> 
> It has alike data to the contents of meminfo:
> 
> /sys/devices/system/node/node0/hugepages/hugepages-2048kB:
> surplus_hugepages
> nr_hugepages
> free_hugepages
> 
> /sys/devices/system/node/node0/hugepages/hugepages-1048576kB:
> surplus_hugepages
> nr_hugepages
> free_hugepages
> 
> [I'm wondering tho, how the fields in meminfo are supposed to be interpreted,
>  given that there are two types of huge pages, but let's just ignore that
>  for the moment.]
> 
> Following this pattern, I'd perhaps go and create (and document):
> 
> /sys/devices/system/node/node0/sgx/memory_size
> 
> which would have the size in bytes.

If it is one-value-per-file, that's fine with me.

thanks,

greg k-h

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

* Re: [PATCH v4 1/3] x86/sgx: Report SGX memory in /sys/devices/system/node/node*/meminfo
  2021-09-10 15:11         ` Greg Kroah-Hartman
@ 2021-09-12 19:37           ` Jarkko Sakkinen
  0 siblings, 0 replies; 10+ messages in thread
From: Jarkko Sakkinen @ 2021-09-12 19:37 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Dave Hansen, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
	H. Peter Anvin, Rafael J. Wysocki, linux-sgx, linux-kernel

On Fri, 2021-09-10 at 17:11 +0200, Greg Kroah-Hartman wrote:
> On Fri, Sep 10, 2021 at 06:02:59PM +0300, Jarkko Sakkinen wrote:
> > On Fri, 2021-09-10 at 16:05 +0200, Greg Kroah-Hartman wrote:
> > > On Fri, Sep 10, 2021 at 04:17:44PM +0300, Jarkko Sakkinen wrote:
> > > > On Fri, 2021-09-10 at 08:51 +0200, Greg Kroah-Hartman wrote:
> > > > > On Fri, Sep 10, 2021 at 03:17:24AM +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 /sys/devices/system/node/node*/meminfo,
> > > > > > showing the total SGX memory in each NUMA node. The total memory for
> > > > > > each NUMA node is calculated by adding the sizes of contained EPC
> > > > > > sections together.
> > > > > > 
> > > > > > Introduce arch_node_read_meminfo(), which can optionally be rewritten by
> > > > > > the arch code, and rewrite it for x86 so it prints SGX_MemTotal.
> > > > > > 
> > > > > > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> > > > > > ---
> > > > > > v4:
> > > > > > * A new patch.
> > > > > >  arch/x86/kernel/cpu/sgx/main.c | 14 ++++++++++++++
> > > > > >  arch/x86/kernel/cpu/sgx/sgx.h  |  6 ++++++
> > > > > >  drivers/base/node.c            | 10 +++++++++-
> > > > > >  3 files changed, 29 insertions(+), 1 deletion(-)
> > > > > 
> > > > > Where is the Documentation/ABI/ update for this new sysfs file?
> > > > 
> > > > It's has been existed for a long time, e.g.
> > > > 
> > > >  cat /sys/devices/system/node/node0/meminfo
> > > > Node 0 MemTotal:       32706792 kB
> > > > Node 0 MemFree:         5382988 kB
> > > > Node 0 MemUsed:        27323804 kB
> > > > Node 0 SwapCached:            8 kB
> > > > Node 0 Active:          3640612 kB
> > > > Node 0 Inactive:       21757684 kB
> > > > Node 0 Active(anon):    2833772 kB
> > > > Node 0 Inactive(anon):    14392 kB
> > > > Node 0 Active(file):     806840 kB
> > > > Node 0 Inactive(file): 21743292 kB
> > > > Node 0 Unevictable:       80640 kB
> > > > Node 0 Mlocked:           80640 kB
> > > > Node 0 Dirty:                36 kB
> > > > Node 0 Writeback:             0 kB
> > > > Node 0 FilePages:      22833124 kB
> > > > Node 0 Mapped:          1112832 kB
> > > > Node 0 AnonPages:       2645812 kB
> > > > Node 0 Shmem:            282984 kB
> > > > Node 0 KernelStack:       18544 kB
> > > > Node 0 PageTables:        46704 kB
> > > > Node 0 NFS_Unstable:          0 kB
> > > > Node 0 Bounce:                0 kB
> > > > Node 0 WritebackTmp:          0 kB
> > > > Node 0 KReclaimable:    1311812 kB
> > > > Node 0 Slab:            1542220 kB
> > > > Node 0 SReclaimable:    1311812 kB
> > > > Node 0 SUnreclaim:       230408 kB
> > > > Node 0 AnonHugePages:         0 kB
> > > > Node 0 ShmemHugePages:        0 kB
> > > > Node 0 ShmemPmdMapped:        0 kB
> > > > Node 0 FileHugePages:        0 kB
> > > > Node 0 FilePmdMapped:        0 kB
> > > > Node 0 HugePages_Total:     0
> > > > Node 0 HugePages_Free:      0
> > > > Node 0 HugePages_Surp:      0
> > 
> > This was something also spinned my head a bit, i.e. why hugepages fields
> > are not aligned correctly.
> > 
> > > > This file is undocumented but the fields seem to reflect what is in
> > > > /proc/meminfo, so I added additional patch for documentation:
> > > > 
> > > > https://lore.kernel.org/linux-sgx/20210910001726.811497-3-jarkko@kernel.org/
> > > > 
> > > > I have no idea why things are how they are. I'm just merely trying to follow
> > > > the existing patterns. I'm also fully aware of the defacto sysfs pattern, i.e.
> > > > one value per file.
> > > 
> > > Then please do not add anything else to this nightmare of a mess.
> > 
> > There is already /sys/devices/system/node/node0/hugepages/.
> > 
> > It has alike data to the contents of meminfo:
> > 
> > /sys/devices/system/node/node0/hugepages/hugepages-2048kB:
> > surplus_hugepages
> > nr_hugepages
> > free_hugepages
> > 
> > /sys/devices/system/node/node0/hugepages/hugepages-1048576kB:
> > surplus_hugepages
> > nr_hugepages
> > free_hugepages
> > 
> > [I'm wondering tho, how the fields in meminfo are supposed to be interpreted,
> >  given that there are two types of huge pages, but let's just ignore that
> >  for the moment.]
> > 
> > Following this pattern, I'd perhaps go and create (and document):
> > 
> > /sys/devices/system/node/node0/sgx/memory_size
> > 
> > which would have the size in bytes.
> 
> If it is one-value-per-file, that's fine with me.

And if it's represented like this to user space, I think that /proc/meminfo
change is unnecessary: it's less involving (e.g. in scripting) to parse these
files in a loop and sum then, than grep the appropriate line from /proc/meminfo.
Would no make much common sense to maintain it.

For me the only open here is: would it make more sense to add x86
subdirectory to each node? With this approach the path would be
something like:

* /sys/devices/system/node/node0/x86/sgx_memory_size

I guess huge pages is different in the sense that it's not just one arch thing,
whereas SGX is directly arch related, so I think that this would be a more
civilized way to deal with SGX. This way things won't bloat, if or when something
else ought to need a NUMA node specific attribute.

> thanks,
> 
> greg k-h

/Jarkko


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

end of thread, other threads:[~2021-09-12 19:37 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-10  0:17 [PATCH v4 1/3] x86/sgx: Report SGX memory in /sys/devices/system/node/node*/meminfo Jarkko Sakkinen
2021-09-10  0:17 ` [PATCH v4 2/3] x86/sgx: Report SGX memory in /proc/meminfo Jarkko Sakkinen
2021-09-10  0:20   ` Jarkko Sakkinen
2021-09-10  0:17 ` [PATCH v4 3/3] x86/sgx: Document SGX_MemTotal to Documentation/x86/meminfo.rst Jarkko Sakkinen
2021-09-10  6:51 ` [PATCH v4 1/3] x86/sgx: Report SGX memory in /sys/devices/system/node/node*/meminfo Greg Kroah-Hartman
2021-09-10 13:17   ` Jarkko Sakkinen
2021-09-10 14:05     ` Greg Kroah-Hartman
2021-09-10 15:02       ` Jarkko Sakkinen
2021-09-10 15:11         ` Greg Kroah-Hartman
2021-09-12 19:37           ` 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).