[PATCH v4 0/4] Add interfaces for ACPI MRRM table

[PATCH v4 0/4] Add interfaces for ACPI MRRM table

[Tony Luck]

Memory used to be homogeneous. Then NUMA came along. Later different
types of memory (persistent memory, on-package high bandwidth memory,
CXL attached memory).

Each type of memory has its own performance characteristics, and users
will need to monitor and control access by type.

The MRRM solution is to tag physical address ranges with "region IDs"
so that platform firmware[1] can indicate the type of memory for each
range (with separate tags available for local vs. remote access to
each range). Note that these ranges can include addresses reserved
for future hotplugged memory.

The region IDs will be used to provide separate event counts for each
region for "perf" and for the "resctrl" file system to monitor and
control memory bandwidth in each region.

Users will need to know the address range(s) that are part of each
region. This patch series adds
	/sys/firmware/acpi/memory_ranges/rangeX
directories to provide user space accessible enumeration.

-Tony

[1] MRRM definition allow for future expansion for the OS to assign
these region IDs.

Changes since version 3 here:
https://lore.kernel.org/all/20250410223207.257722-1-tony.luck@intel.com/

1) Rebase to v6.15-rc4
2) Removed ugly #ifdef in acpi_mrrm.c with better fix for CONFIG_NUMA=n
3) Moved stub for acpi_mrrm_max_mem_region() into #else !ACPI section
   of <linux/acpi.h>

Tony Luck (4):
  ACPICA: Define MRRM ACPI table
  ACPI/MRRM: Minimal parse of ACPI MRRM table
  ACPI/MRRM: Add /sys files to describe memory ranges
  ACPI: Add documentation for exposing MRRM data

 include/linux/acpi.h                          |   9 +
 include/acpi/actbl1.h                         |   7 +
 include/acpi/actbl2.h                         |  42 ++++
 drivers/acpi/acpi_mrrm.c                      | 183 ++++++++++++++++++
 Documentation/ABI/testing/sysfs-firmware-acpi |  21 ++
 arch/x86/Kconfig                              |   1 +
 drivers/acpi/Kconfig                          |   3 +
 drivers/acpi/Makefile                         |   1 +
 8 files changed, 267 insertions(+)
 create mode 100644 drivers/acpi/acpi_mrrm.c


base-commit: b4432656b36e5cc1d50a1f2dc15357543add530e
-- 
2.48.1

[PATCH v4 1/4] ACPICA: Define MRRM ACPI table

[Tony Luck]

Patch for reference, this has already been applied to
https://github.com/acpica/acpica and will in due course make its way
into Linux when the next ACPICA release is ported over.

Signed-off-by: Tony Luck <tony.luck@intel.com>
---
 include/acpi/actbl1.h |  7 +++++++
 include/acpi/actbl2.h | 42 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+)

diff --git a/include/acpi/actbl1.h b/include/acpi/actbl1.h
index 387fc821703a..4cb36392e9e9 100644
--- a/include/acpi/actbl1.h
+++ b/include/acpi/actbl1.h
@@ -110,6 +110,13 @@ struct acpi_whea_header {
 	u64 mask;		/* Bitmask required for this register instruction */
 };
 
+/* Larger subtable header (when Length can exceed 255) */
+
+struct acpi_subtbl_hdr_16 {
+	u16 type;
+	u16 length;
+};
+
 /* https://docs.microsoft.com/en-us/windows-hardware/drivers/ddi/acpitabl/ns-acpitabl-aspt_table */
 #define ASPT_REVISION_ID 0x01
 struct acpi_table_aspt {
diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
index 2e917a8f8bca..e7423db6e24b 100644
--- a/include/acpi/actbl2.h
+++ b/include/acpi/actbl2.h
@@ -37,6 +37,7 @@
 #define ACPI_SIG_MCHI           "MCHI"	/* Management Controller Host Interface table */
 #define ACPI_SIG_MPAM           "MPAM"	/* Memory System Resource Partitioning and Monitoring Table */
 #define ACPI_SIG_MPST           "MPST"	/* Memory Power State Table */
+#define ACPI_SIG_MRRM           "MRRM"      /* Memory Range and Region Mapping table */
 #define ACPI_SIG_MSDM           "MSDM"	/* Microsoft Data Management Table */
 #define ACPI_SIG_NFIT           "NFIT"	/* NVDIMM Firmware Interface Table */
 #define ACPI_SIG_NHLT           "NHLT"	/* Non HD Audio Link Table */
@@ -1736,6 +1737,47 @@ struct acpi_msct_proximity {
 	u64 memory_capacity;	/* In bytes */
 };
 
+/*******************************************************************************
+ *
+ * MRRM - Memory Range and Region Mapping (MRRM) table
+ * Conforms to "Intel Resource Director Technology Architecture Specification"
+ * Version 1.1, January 2025
+ *
+ ******************************************************************************/
+
+struct acpi_table_mrrm {
+	struct acpi_table_header header;	/* Common ACPI table header */
+	u8 max_mem_region;			/* Max Memory Regions supported */
+	u8 flags;				/* Region assignment type */
+	u8 reserved[26];
+	u8 memory_range_entry[];
+};
+
+/* Flags */
+#define ACPI_MRRM_FLAGS_REGION_ASSIGNMENT_OS (1<<0)
+
+/*******************************************************************************
+	*
+	* Memory Range entry - Memory Range entry in MRRM table
+	*
+	******************************************************************************/
+
+struct acpi_mrrm_mem_range_entry {
+	struct acpi_subtbl_hdr_16 header;
+	u32 reserved0;		/* Reserved */
+	u64 addr_base;		/* Base addr of the mem range */
+	u64 addr_len;		/* Length of the mem range */
+	u16 region_id_flags;	/* Valid local or remote Region-ID */
+	u8 local_region_id;	/* Platform-assigned static local Region-ID */
+	u8 remote_region_id;	/* Platform-assigned static remote Region-ID */
+	u32 reserved1;		/* Reserved */
+	/* Region-ID Programming Registers[] */
+};
+
+/* Values for region_id_flags above */
+#define ACPI_MRRM_VALID_REGION_ID_FLAGS_LOCAL   (1<<0)
+#define ACPI_MRRM_VALID_REGION_ID_FLAGS_REMOTE  (1<<1)
+
 /*******************************************************************************
  *
  * MSDM - Microsoft Data Management table
-- 
2.48.1

[PATCH v4 2/4] ACPI/MRRM: Minimal parse of ACPI MRRM table

[Tony Luck]

The resctrl file system code needs to know how many region tags
are supported. Parse the ACPI MRRM table and save the max_mem_region
value.

Provide a function for resctrl to collect that value.

Signed-off-by: Tony Luck <tony.luck@intel.com>
---
 include/linux/acpi.h     |  9 +++++++++
 drivers/acpi/acpi_mrrm.c | 42 ++++++++++++++++++++++++++++++++++++++++
 arch/x86/Kconfig         |  1 +
 drivers/acpi/Kconfig     |  3 +++
 drivers/acpi/Makefile    |  1 +
 5 files changed, 56 insertions(+)
 create mode 100644 drivers/acpi/acpi_mrrm.c

diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 3f2e93ed9730..c409f4cecb09 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -772,6 +772,10 @@ int acpi_get_local_u64_address(acpi_handle handle, u64 *addr);
 int acpi_get_local_address(acpi_handle handle, u32 *addr);
 const char *acpi_get_subsystem_id(acpi_handle handle);
 
+#ifdef CONFIG_ACPI_MRRM
+int acpi_mrrm_max_mem_region(void);
+#endif
+
 #else	/* !CONFIG_ACPI */
 
 #define acpi_disabled 1
@@ -1092,6 +1096,11 @@ static inline acpi_handle acpi_get_processor_handle(int cpu)
 	return NULL;
 }
 
+static inline int acpi_mrrm_max_mem_region(void)
+{
+	return -ENOENT;
+}
+
 #endif	/* !CONFIG_ACPI */
 
 #ifdef CONFIG_ACPI_HMAT
diff --git a/drivers/acpi/acpi_mrrm.c b/drivers/acpi/acpi_mrrm.c
new file mode 100644
index 000000000000..ab8022e58da5
--- /dev/null
+++ b/drivers/acpi/acpi_mrrm.c
@@ -0,0 +1,42 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2025, Intel Corporation.
+ *
+ * Memory Range and Region Mapping (MRRM) structure
+ */
+
+#define pr_fmt(fmt) "acpi/mrrm: " fmt
+
+#include <linux/acpi.h>
+#include <linux/init.h>
+
+static int max_mem_region = -ENOENT;
+
+/* Access for use by resctrl file system */
+int acpi_mrrm_max_mem_region(void)
+{
+	return max_mem_region;
+}
+
+static __init int acpi_parse_mrrm(struct acpi_table_header *table)
+{
+	struct acpi_table_mrrm *mrrm;
+
+	mrrm = (struct acpi_table_mrrm *)table;
+	if (!mrrm)
+		return -ENODEV;
+
+	max_mem_region = mrrm->max_mem_region;
+
+	return 0;
+}
+
+static __init int mrrm_init(void)
+{
+	int ret;
+
+	ret = acpi_table_parse(ACPI_SIG_MRRM, acpi_parse_mrrm);
+
+	return ret;
+}
+device_initcall(mrrm_init);
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 4b9f378e05f6..0e43069082df 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -38,6 +38,7 @@ config X86_64
 	select ARCH_HAS_ELFCORE_COMPAT
 	select ZONE_DMA32
 	select EXECMEM if DYNAMIC_FTRACE
+	select ACPI_MRRM if ACPI
 
 config FORCE_DYNAMIC_FTRACE
 	def_bool y
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 7f10aa38269d..7bc40c2735ac 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -576,6 +576,9 @@ config ACPI_FFH
 	  Enable this feature if you want to set up and install the FFH Address
 	  Space handler to handle FFH OpRegion in the firmware.
 
+config ACPI_MRRM
+	bool
+
 source "drivers/acpi/pmic/Kconfig"
 
 config ACPI_VIOT
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index 797070fc9a3f..d1b0affb844f 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -66,6 +66,7 @@ acpi-$(CONFIG_ACPI_WATCHDOG)	+= acpi_watchdog.o
 acpi-$(CONFIG_ACPI_PRMT)	+= prmt.o
 acpi-$(CONFIG_ACPI_PCC)		+= acpi_pcc.o
 acpi-$(CONFIG_ACPI_FFH)		+= acpi_ffh.o
+acpi-$(CONFIG_ACPI_MRRM)	+= acpi_mrrm.o
 
 # Address translation
 acpi-$(CONFIG_ACPI_ADXL)	+= acpi_adxl.o
-- 
2.48.1

[PATCH v4 3/4] ACPI/MRRM: Add /sys files to describe memory ranges

[Tony Luck]

Perf and resctrl users need an enumeration of which memory addresses
are bound to which "region" tag.

Parse the ACPI MRRM table and add /sys entries for each memory range
describing base address, length, NUMA node, and which region tags apply
for same-socket and cross-socket access.

Signed-off-by: Tony Luck <tony.luck@intel.com>
---
 drivers/acpi/acpi_mrrm.c | 143 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 142 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/acpi_mrrm.c b/drivers/acpi/acpi_mrrm.c
index ab8022e58da5..f04645a0407f 100644
--- a/drivers/acpi/acpi_mrrm.c
+++ b/drivers/acpi/acpi_mrrm.c
@@ -3,12 +3,16 @@
  * Copyright (c) 2025, Intel Corporation.
  *
  * Memory Range and Region Mapping (MRRM) structure
+ *
+ * Parse and report the platform's MRRM table in /sys.
  */
 
 #define pr_fmt(fmt) "acpi/mrrm: " fmt
 
 #include <linux/acpi.h>
 #include <linux/init.h>
+#include <linux/string.h>
+#include <linux/sysfs.h>
 
 static int max_mem_region = -ENOENT;
 
@@ -18,25 +22,162 @@ int acpi_mrrm_max_mem_region(void)
 	return max_mem_region;
 }
 
+struct mrrm_mem_range_entry {
+	u64 base;
+	u64 length;
+	int node;
+	u8  local_region_id;
+	u8  remote_region_id;
+};
+
+static struct mrrm_mem_range_entry *mrrm_mem_range_entry;
+static u32 mrrm_mem_entry_num;
+
+static int get_node_num(struct mrrm_mem_range_entry *e)
+{
+	unsigned int nid;
+
+	for_each_online_node(nid) {
+		for (int z = 0; z < MAX_NR_ZONES; z++) {
+			struct zone *zone = NODE_DATA(nid)->node_zones + z;
+
+			if (!populated_zone(zone))
+				continue;
+			if (zone_intersects(zone, PHYS_PFN(e->base), PHYS_PFN(e->length)))
+				return zone_to_nid(zone);
+		}
+	}
+
+	return -ENOENT;
+}
+
 static __init int acpi_parse_mrrm(struct acpi_table_header *table)
 {
+	struct acpi_mrrm_mem_range_entry *mre_entry;
 	struct acpi_table_mrrm *mrrm;
+	void *mre, *mrrm_end;
+	int mre_count = 0;
 
 	mrrm = (struct acpi_table_mrrm *)table;
 	if (!mrrm)
 		return -ENODEV;
 
+	if (mrrm->flags & ACPI_MRRM_FLAGS_REGION_ASSIGNMENT_OS)
+		return -EOPNOTSUPP;
+
+	mrrm_end = (void *)mrrm + mrrm->header.length - 1;
+	mre = (void *)mrrm + sizeof(struct acpi_table_mrrm);
+	while (mre < mrrm_end) {
+		mre_entry = mre;
+		mre_count++;
+		mre += mre_entry->header.length;
+	}
+	if (!mre_count) {
+		pr_info(FW_BUG "No ranges listed in MRRM table\n");
+		return -EINVAL;
+	}
+
+	mrrm_mem_range_entry = kmalloc_array(mre_count, sizeof(*mrrm_mem_range_entry),
+					     GFP_KERNEL | __GFP_ZERO);
+	if (!mrrm_mem_range_entry)
+		return -ENOMEM;
+
+	mre = (void *)mrrm + sizeof(struct acpi_table_mrrm);
+	while (mre < mrrm_end) {
+		struct mrrm_mem_range_entry *e;
+
+		mre_entry = mre;
+		e = mrrm_mem_range_entry + mrrm_mem_entry_num;
+
+		e->base = mre_entry->addr_base;
+		e->length = mre_entry->addr_len;
+		e->node = get_node_num(e);
+
+		if (mre_entry->region_id_flags & ACPI_MRRM_VALID_REGION_ID_FLAGS_LOCAL)
+			e->local_region_id = mre_entry->local_region_id;
+		else
+			e->local_region_id = -1;
+		if (mre_entry->region_id_flags & ACPI_MRRM_VALID_REGION_ID_FLAGS_REMOTE)
+			e->remote_region_id = mre_entry->remote_region_id;
+		else
+			e->remote_region_id = -1;
+
+		mrrm_mem_entry_num++;
+		mre += mre_entry->header.length;
+	}
+
 	max_mem_region = mrrm->max_mem_region;
 
 	return 0;
 }
 
+#define RANGE_ATTR(name, fmt)						\
+static ssize_t name##_show(struct kobject *kobj,			\
+			  struct kobj_attribute *attr, char *buf)	\
+{									\
+	struct mrrm_mem_range_entry *mre;				\
+	const char *kname = kobject_name(kobj);				\
+	int n, ret;							\
+									\
+	ret = kstrtoint(kname + 5, 10, &n);				\
+	if (ret)							\
+		return ret;						\
+									\
+	mre = mrrm_mem_range_entry + n;					\
+									\
+	return sysfs_emit(buf, fmt, mre->name);				\
+}									\
+static struct kobj_attribute name##_attr = __ATTR_RO(name)
+
+RANGE_ATTR(base, "0x%llx\n");
+RANGE_ATTR(length, "0x%llx\n");
+RANGE_ATTR(node, "%d\n");
+RANGE_ATTR(local_region_id, "%d\n");
+RANGE_ATTR(remote_region_id, "%d\n");
+
+static struct attribute *memory_range_attrs[] = {
+	&base_attr.attr,
+	&length_attr.attr,
+	&node_attr.attr,
+	&local_region_id_attr.attr,
+	&remote_region_id_attr.attr,
+	NULL
+};
+
+ATTRIBUTE_GROUPS(memory_range);
+
+static __init int add_boot_memory_ranges(void)
+{
+	struct kobject *pkobj, *kobj;
+	int ret = -EINVAL;
+	char *name;
+
+	pkobj = kobject_create_and_add("memory_ranges", acpi_kobj);
+
+	for (int i = 0; i < mrrm_mem_entry_num; i++) {
+		name = kasprintf(GFP_KERNEL, "range%d", i);
+		if (!name)
+			break;
+
+		kobj = kobject_create_and_add(name, pkobj);
+
+		ret = sysfs_create_groups(kobj, memory_range_groups);
+		if (ret)
+			return ret;
+	}
+
+	return ret;
+}
+
 static __init int mrrm_init(void)
 {
 	int ret;
 
 	ret = acpi_table_parse(ACPI_SIG_MRRM, acpi_parse_mrrm);
 
-	return ret;
+	if (ret < 0)
+		return ret;
+
+	return add_boot_memory_ranges();
 }
 device_initcall(mrrm_init);
-- 
2.48.1

[PATCH v4 4/4] ACPI: Add documentation for exposing MRRM data

[Tony Luck]

Initial implementation provides enumeration of the address ranges
NUMA node numbers, and BIOS assigned region IDs for each range.

Signed-off-by: Tony Luck <tony.luck@intel.com>
---
 Documentation/ABI/testing/sysfs-firmware-acpi | 21 +++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-firmware-acpi b/Documentation/ABI/testing/sysfs-firmware-acpi
index 5249ad5a96d9..fffba38f9ce1 100644
--- a/Documentation/ABI/testing/sysfs-firmware-acpi
+++ b/Documentation/ABI/testing/sysfs-firmware-acpi
@@ -248,3 +248,24 @@ Description:
 		  # cat ff_pwr_btn
 		  7	enabled
 
+What:		/sys/firmware/acpi/memory_ranges/rangeX
+Date:		February 2025
+Contact:	Tony Luck <tony.luck@intel.com>
+Description:
+		On systems with the ACPI MRRM table reports the
+		parameters for each range.
+
+		base: Starting system physical address.
+
+		length: Length of this range in bytes.
+
+		node: NUMA node that this range belongs to. Negative numbers
+		indicate that the node number could not be determined (e.g
+		for an address range that is reserved for future hot add of
+		memory).
+
+		local_region_id: ID associated with access by agents
+		local to this range of addresses.
+
+		remote_region_id: ID associated with access by agents
+		non-local to this range of addresses.
-- 
2.48.1

Re: [PATCH v4 3/4] ACPI/MRRM: Add /sys files to describe memory ranges

[Fenghua Yu]

Hi, Tony,

On 4/29/25 13:24, Tony Luck wrote:
> Perf and resctrl users need an enumeration of which memory addresses
> are bound to which "region" tag.
>
> Parse the ACPI MRRM table and add /sys entries for each memory range
> describing base address, length, NUMA node, and which region tags apply
> for same-socket and cross-socket access.
>
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> ---
>   drivers/acpi/acpi_mrrm.c | 143 ++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 142 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/acpi_mrrm.c b/drivers/acpi/acpi_mrrm.c
> index ab8022e58da5..f04645a0407f 100644
> --- a/drivers/acpi/acpi_mrrm.c
> +++ b/drivers/acpi/acpi_mrrm.c
> @@ -3,12 +3,16 @@
>    * Copyright (c) 2025, Intel Corporation.
>    *
>    * Memory Range and Region Mapping (MRRM) structure
> + *
> + * Parse and report the platform's MRRM table in /sys.
>    */
>   
>   #define pr_fmt(fmt) "acpi/mrrm: " fmt
>   
>   #include <linux/acpi.h>
>   #include <linux/init.h>
> +#include <linux/string.h>
> +#include <linux/sysfs.h>
>   
>   static int max_mem_region = -ENOENT;
>   
> @@ -18,25 +22,162 @@ int acpi_mrrm_max_mem_region(void)
>   	return max_mem_region;
>   }
>   
> +struct mrrm_mem_range_entry {
> +	u64 base;
> +	u64 length;
> +	int node;
> +	u8  local_region_id;
> +	u8  remote_region_id;
> +};
> +
> +static struct mrrm_mem_range_entry *mrrm_mem_range_entry;
> +static u32 mrrm_mem_entry_num;
> +
> +static int get_node_num(struct mrrm_mem_range_entry *e)
> +{
> +	unsigned int nid;
> +
> +	for_each_online_node(nid) {
> +		for (int z = 0; z < MAX_NR_ZONES; z++) {
> +			struct zone *zone = NODE_DATA(nid)->node_zones + z;
> +
> +			if (!populated_zone(zone))
> +				continue;
> +			if (zone_intersects(zone, PHYS_PFN(e->base), PHYS_PFN(e->length)))
> +				return zone_to_nid(zone);
> +		}
> +	}
> +
> +	return -ENOENT;
> +}
> +
>   static __init int acpi_parse_mrrm(struct acpi_table_header *table)
>   {
> +	struct acpi_mrrm_mem_range_entry *mre_entry;
>   	struct acpi_table_mrrm *mrrm;
> +	void *mre, *mrrm_end;
> +	int mre_count = 0;
>   
>   	mrrm = (struct acpi_table_mrrm *)table;
>   	if (!mrrm)
>   		return -ENODEV;
>   
> +	if (mrrm->flags & ACPI_MRRM_FLAGS_REGION_ASSIGNMENT_OS)
> +		return -EOPNOTSUPP;
> +
> +	mrrm_end = (void *)mrrm + mrrm->header.length - 1;
> +	mre = (void *)mrrm + sizeof(struct acpi_table_mrrm);
> +	while (mre < mrrm_end) {
> +		mre_entry = mre;
> +		mre_count++;
> +		mre += mre_entry->header.length;
> +	}
> +	if (!mre_count) {
> +		pr_info(FW_BUG "No ranges listed in MRRM table\n");
> +	 	return -EINVAL;
> +	}
> +
> +	mrrm_mem_range_entry = kmalloc_array(mre_count, sizeof(*mrrm_mem_range_entry),
> +					     GFP_KERNEL | __GFP_ZERO);
> +	if (!mrrm_mem_range_entry)
> +		return -ENOMEM;
> +
> +	mre = (void *)mrrm + sizeof(struct acpi_table_mrrm);
> +	while (mre < mrrm_end) {
> +		struct mrrm_mem_range_entry *e;
> +
> +		mre_entry = mre;
> +		e = mrrm_mem_range_entry + mrrm_mem_entry_num;
> +
> +		e->base = mre_entry->addr_base;
> +		e->length = mre_entry->addr_len;
> +		e->node = get_node_num(e);
> +
> +		if (mre_entry->region_id_flags & ACPI_MRRM_VALID_REGION_ID_FLAGS_LOCAL)
> +			e->local_region_id = mre_entry->local_region_id;
> +		else
> +			e->local_region_id = -1;
> +		if (mre_entry->region_id_flags & ACPI_MRRM_VALID_REGION_ID_FLAGS_REMOTE)
> +			e->remote_region_id = mre_entry->remote_region_id;
> +		else
> +			e->remote_region_id = -1;
> +
> +		mrrm_mem_entry_num++;
> +		mre += mre_entry->header.length;
> +	}
> +
>   	max_mem_region = mrrm->max_mem_region;
>   
>   	return 0;
>   }
>   
> +#define RANGE_ATTR(name, fmt)						\
> +static ssize_t name##_show(struct kobject *kobj,			\

"name" is used as a macro parameter. But "name" is also used as a 
variable mre->name in the macro. checkpatch complains this kind of usage.

Maybe change the parameter "name" as something like "range_name" to 
avoid the potential confusion?

> +			  struct kobj_attribute *attr, char *buf)	\
> +{									\
> +	struct mrrm_mem_range_entry *mre;				\
> +	const char *kname = kobject_name(kobj);				\
> +	int n, ret;							\
> +									\
> +	ret = kstrtoint(kname + 5, 10, &n);				\
> +	if (ret)							\
> +		return ret;						\
> +									\
> +	mre = mrrm_mem_range_entry + n;					\
> +									\
> +	return sysfs_emit(buf, fmt, mre->name);				\
> +}									\
> +static struct kobj_attribute name##_attr = __ATTR_RO(name)
> +
> +RANGE_ATTR(base, "0x%llx\n");
> +RANGE_ATTR(length, "0x%llx\n");
> +RANGE_ATTR(node, "%d\n");
> +RANGE_ATTR(local_region_id, "%d\n");
> +RANGE_ATTR(remote_region_id, "%d\n");
> +
> +static struct attribute *memory_range_attrs[] = {
> +	&base_attr.attr,
> +	&length_attr.attr,
> +	&node_attr.attr,
> +	&local_region_id_attr.attr,
> +	&remote_region_id_attr.attr,
> +	NULL
> +};
> +
> +ATTRIBUTE_GROUPS(memory_range);
> +
> +static __init int add_boot_memory_ranges(void)
> +{
> +	struct kobject *pkobj, *kobj;
> +	int ret = -EINVAL;
> +	char *name;
> +
> +	pkobj = kobject_create_and_add("memory_ranges", acpi_kobj);
> +
> +	for (int i = 0; i < mrrm_mem_entry_num; i++) {
> +		name = kasprintf(GFP_KERNEL, "range%d", i);
> +		if (!name)
> +			break;
> +
> +		kobj = kobject_create_and_add(name, pkobj);
> +
> +		ret = sysfs_create_groups(kobj, memory_range_groups);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return ret;
> +}
> +
>   static __init int mrrm_init(void)
>   {
>   	int ret;
>   
>   	ret = acpi_table_parse(ACPI_SIG_MRRM, acpi_parse_mrrm);
>   
This blank line seems redundant. Maybe remove it so that the "if (ret < 
0)" sentence follows the "ret = ...." sentence immediately?
> -	return ret;
> +	if (ret < 0)
> +		return ret;
> +
> +	return add_boot_memory_ranges();
>   }
>   device_initcall(mrrm_init);

Thanks.

-Fenghua

Re: [PATCH v4 4/4] ACPI: Add documentation for exposing MRRM data

[Fenghua Yu]

Hi, Tony,

On 4/29/25 13:24, Tony Luck wrote:
> Initial implementation provides enumeration of the address ranges
> NUMA node numbers, and BIOS assigned region IDs for each range.
>
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> ---
>   Documentation/ABI/testing/sysfs-firmware-acpi | 21 +++++++++++++++++++
>   1 file changed, 21 insertions(+)
>
> diff --git a/Documentation/ABI/testing/sysfs-firmware-acpi b/Documentation/ABI/testing/sysfs-firmware-acpi
> index 5249ad5a96d9..fffba38f9ce1 100644
> --- a/Documentation/ABI/testing/sysfs-firmware-acpi
> +++ b/Documentation/ABI/testing/sysfs-firmware-acpi
> @@ -248,3 +248,24 @@ Description:
>   		  # cat ff_pwr_btn
>   		  7	enabled
>   
> +What:		/sys/firmware/acpi/memory_ranges/rangeX
> +Date:		February 2025
> +Contact:	Tony Luck <tony.luck@intel.com>
> +Description:
> +		On systems with the ACPI MRRM table reports the
> +		parameters for each range.

Is there a need to explain what's "X" here? The "X" is not a number 
directly reported by MRRM, right?

Maybe something like "range ID is enumerated from MRRM starting from 0."?

> +
> +		base: Starting system physical address.
> +
> +		length: Length of this range in bytes.
> +
> +		node: NUMA node that this range belongs to. Negative numbers
> +		indicate that the node number could not be determined (e.g
> +		for an address range that is reserved for future hot add of
> +		memory).
> +
> +		local_region_id: ID associated with access by agents
> +		local to this range of addresses.
> +
> +		remote_region_id: ID associated with access by agents
> +		non-local to this range of addresses.

Thanks.

-Fenghua

Re: [PATCH v4 1/4] ACPICA: Define MRRM ACPI table

[Rafael J. Wysocki]

On Tue, Apr 29, 2025 at 10:24 PM Tony Luck <tony.luck@intel.com> wrote:
>
> Patch for reference, this has already been applied to
> https://github.com/acpica/acpica and will in due course make its way
> into Linux when the next ACPICA release is ported over.
>
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> ---
>  include/acpi/actbl1.h |  7 +++++++
>  include/acpi/actbl2.h | 42 ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 49 insertions(+)
>
> diff --git a/include/acpi/actbl1.h b/include/acpi/actbl1.h
> index 387fc821703a..4cb36392e9e9 100644
> --- a/include/acpi/actbl1.h
> +++ b/include/acpi/actbl1.h
> @@ -110,6 +110,13 @@ struct acpi_whea_header {
>         u64 mask;               /* Bitmask required for this register instruction */
>  };
>
> +/* Larger subtable header (when Length can exceed 255) */
> +
> +struct acpi_subtbl_hdr_16 {
> +       u16 type;
> +       u16 length;
> +};
> +
>  /* https://docs.microsoft.com/en-us/windows-hardware/drivers/ddi/acpitabl/ns-acpitabl-aspt_table */
>  #define ASPT_REVISION_ID 0x01
>  struct acpi_table_aspt {
> diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
> index 2e917a8f8bca..e7423db6e24b 100644
> --- a/include/acpi/actbl2.h
> +++ b/include/acpi/actbl2.h
> @@ -37,6 +37,7 @@
>  #define ACPI_SIG_MCHI           "MCHI" /* Management Controller Host Interface table */
>  #define ACPI_SIG_MPAM           "MPAM" /* Memory System Resource Partitioning and Monitoring Table */
>  #define ACPI_SIG_MPST           "MPST" /* Memory Power State Table */
> +#define ACPI_SIG_MRRM           "MRRM"      /* Memory Range and Region Mapping table */
>  #define ACPI_SIG_MSDM           "MSDM" /* Microsoft Data Management Table */
>  #define ACPI_SIG_NFIT           "NFIT" /* NVDIMM Firmware Interface Table */
>  #define ACPI_SIG_NHLT           "NHLT" /* Non HD Audio Link Table */
> @@ -1736,6 +1737,47 @@ struct acpi_msct_proximity {
>         u64 memory_capacity;    /* In bytes */
>  };
>
> +/*******************************************************************************
> + *
> + * MRRM - Memory Range and Region Mapping (MRRM) table
> + * Conforms to "Intel Resource Director Technology Architecture Specification"
> + * Version 1.1, January 2025
> + *
> + ******************************************************************************/
> +
> +struct acpi_table_mrrm {
> +       struct acpi_table_header header;        /* Common ACPI table header */
> +       u8 max_mem_region;                      /* Max Memory Regions supported */
> +       u8 flags;                               /* Region assignment type */
> +       u8 reserved[26];
> +       u8 memory_range_entry[];
> +};
> +
> +/* Flags */
> +#define ACPI_MRRM_FLAGS_REGION_ASSIGNMENT_OS (1<<0)
> +
> +/*******************************************************************************
> +       *
> +       * Memory Range entry - Memory Range entry in MRRM table
> +       *
> +       ******************************************************************************/
> +
> +struct acpi_mrrm_mem_range_entry {
> +       struct acpi_subtbl_hdr_16 header;
> +       u32 reserved0;          /* Reserved */
> +       u64 addr_base;          /* Base addr of the mem range */
> +       u64 addr_len;           /* Length of the mem range */
> +       u16 region_id_flags;    /* Valid local or remote Region-ID */
> +       u8 local_region_id;     /* Platform-assigned static local Region-ID */
> +       u8 remote_region_id;    /* Platform-assigned static remote Region-ID */
> +       u32 reserved1;          /* Reserved */
> +       /* Region-ID Programming Registers[] */
> +};
> +
> +/* Values for region_id_flags above */
> +#define ACPI_MRRM_VALID_REGION_ID_FLAGS_LOCAL   (1<<0)
> +#define ACPI_MRRM_VALID_REGION_ID_FLAGS_REMOTE  (1<<1)
> +
>  /*******************************************************************************
>   *
>   * MSDM - Microsoft Data Management table
> --

All of the above definitions should be there in linux-next now.

Can you please check if they are there and they are correct?
Alternatively, please check

https://web.git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/log/?h=testing

Re: [PATCH v4 3/4] ACPI/MRRM: Add /sys files to describe memory ranges

[Luck, Tony]

On Sun, May 04, 2025 at 11:23:50PM -0700, Fenghua Yu wrote:
> Hi, Tony,
> > +#define RANGE_ATTR(name, fmt)						\
> > +static ssize_t name##_show(struct kobject *kobj,			\
> 
> "name" is used as a macro parameter. But "name" is also used as a variable
> mre->name in the macro. checkpatch complains this kind of usage.

The checkpatch complaint is that is is used twice. Once as "mre->name"
(as you noted), but also as "__ATTR_RO(name)" two lines after.

Checkpatch is worried that the macor might be invoked with an argument
that has side effects ("foo++", or return from function call "baz()")
which would result in the side-effects happening twice.

It's a false positive in this case because:

1) This macro is only used with a simple argument (and can only ever be
used in that way.
2) It's used for a static initialization of compile time, so a 2nd
reason why side-effects from an argment are not possible.

> Maybe change the parameter "name" as something like "range_name" to avoid
> the potential confusion?
> 
> > +			  struct kobj_attribute *attr, char *buf)	\
> > +{									\
> > +	struct mrrm_mem_range_entry *mre;				\
> > +	const char *kname = kobject_name(kobj);				\
> > +	int n, ret;							\
> > +									\
> > +	ret = kstrtoint(kname + 5, 10, &n);				\
> > +	if (ret)							\
> > +		return ret;						\
> > +									\
> > +	mre = mrrm_mem_range_entry + n;					\
> > +									\
> > +	return sysfs_emit(buf, fmt, mre->name);				\
> > +}									\
> > +static struct kobj_attribute name##_attr = __ATTR_RO(name)
> > +
> > +RANGE_ATTR(base, "0x%llx\n");
> > +RANGE_ATTR(length, "0x%llx\n");
> > +RANGE_ATTR(node, "%d\n");
> > +RANGE_ATTR(local_region_id, "%d\n");
> > +RANGE_ATTR(remote_region_id, "%d\n");

...

> > +
> >   static __init int mrrm_init(void)
> >   {
> >   	int ret;
> >   	ret = acpi_table_parse(ACPI_SIG_MRRM, acpi_parse_mrrm);
> This blank line seems redundant. Maybe remove it so that the "if (ret < 0)"
> sentence follows the "ret = ...." sentence immediately?

Agreed. I will delete in next version.

> > -	return ret;
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	return add_boot_memory_ranges();
> >   }
> >   device_initcall(mrrm_init);
> 
> Thanks.
> 
> -Fenghua

Thanks for the review.

-Tony
> 

Re: [PATCH v4 4/4] ACPI: Add documentation for exposing MRRM data

[Luck, Tony]

On Sun, May 04, 2025 at 11:34:51PM -0700, Fenghua Yu wrote:
> Hi, Tony,
> 
> On 4/29/25 13:24, Tony Luck wrote:
> > Initial implementation provides enumeration of the address ranges
> > NUMA node numbers, and BIOS assigned region IDs for each range.
> > 
> > Signed-off-by: Tony Luck <tony.luck@intel.com>
> > ---
> >   Documentation/ABI/testing/sysfs-firmware-acpi | 21 +++++++++++++++++++
> >   1 file changed, 21 insertions(+)
> > 
> > diff --git a/Documentation/ABI/testing/sysfs-firmware-acpi b/Documentation/ABI/testing/sysfs-firmware-acpi
> > index 5249ad5a96d9..fffba38f9ce1 100644
> > --- a/Documentation/ABI/testing/sysfs-firmware-acpi
> > +++ b/Documentation/ABI/testing/sysfs-firmware-acpi
> > @@ -248,3 +248,24 @@ Description:
> >   		  # cat ff_pwr_btn
> >   		  7	enabled
> > +What:		/sys/firmware/acpi/memory_ranges/rangeX
> > +Date:		February 2025
> > +Contact:	Tony Luck <tony.luck@intel.com>
> > +Description:
> > +		On systems with the ACPI MRRM table reports the
> > +		parameters for each range.
> 
> Is there a need to explain what's "X" here? The "X" is not a number directly
> reported by MRRM, right?
> 
> Maybe something like "range ID is enumerated from MRRM starting from 0."?

I'm not sure about this one. "X" in the ABI documentation files is a
standard notation for "there are several of these with a number for
each". When the number does refer to some physical object, then there
may be a need to describe it. But if it is simply a counter to give a
separate name for each one of some repeating thing ... then I don't
think it helps to add additional explanation.

> 
> > +
> > +		base: Starting system physical address.
> > +
> > +		length: Length of this range in bytes.
> > +
> > +		node: NUMA node that this range belongs to. Negative numbers
> > +		indicate that the node number could not be determined (e.g
> > +		for an address range that is reserved for future hot add of
> > +		memory).
> > +
> > +		local_region_id: ID associated with access by agents
> > +		local to this range of addresses.
> > +
> > +		remote_region_id: ID associated with access by agents
> > +		non-local to this range of addresses.
> 
> Thanks.
> 
> -Fenghua

-Tony

RE: [PATCH v4 1/4] ACPICA: Define MRRM ACPI table

[Luck, Tony]

> > +/* Values for region_id_flags above */
> > +#define ACPI_MRRM_VALID_REGION_ID_FLAGS_LOCAL   (1<<0)
> > +#define ACPI_MRRM_VALID_REGION_ID_FLAGS_REMOTE  (1<<1)
> > +
> >  /*******************************************************************************
> >   *
> >   * MSDM - Microsoft Data Management table
> > --
>
> All of the above definitions should be there in linux-next now.
>
> Can you please check if they are there and they are correct?
> Alternatively, please check
>
> https://web.git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/log/?h=testing

Rafael,

I checked the "testing" branch. The MRRM bits there look good.

Do you want me to post a V5 of this series that drops patch1? 

Fenghua had some comments ... the one about a spurious blank
line in mrrm_init() is real. I'm less sure about documenting the
meaning of "X" in the ABI documentation patch.

-Tony

Re: [PATCH v4 1/4] ACPICA: Define MRRM ACPI table

[Rafael J. Wysocki]

On Mon, May 5, 2025 at 7:12 PM Luck, Tony <tony.luck@intel.com> wrote:
>
> > > +/* Values for region_id_flags above */
> > > +#define ACPI_MRRM_VALID_REGION_ID_FLAGS_LOCAL   (1<<0)
> > > +#define ACPI_MRRM_VALID_REGION_ID_FLAGS_REMOTE  (1<<1)
> > > +
> > >  /*******************************************************************************
> > >   *
> > >   * MSDM - Microsoft Data Management table
> > > --
> >
> > All of the above definitions should be there in linux-next now.
> >
> > Can you please check if they are there and they are correct?
> > Alternatively, please check
> >
> > https://web.git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/log/?h=testing
>
> Rafael,
>
> I checked the "testing" branch. The MRRM bits there look good.
>
> Do you want me to post a V5 of this series that drops patch1?
>
> Fenghua had some comments ... the one about a spurious blank
> line in mrrm_init() is real. I'm less sure about documenting the
> meaning of "X" in the ABI documentation patch.

If you want to update any of the 3 last patches, please send an
update.  Otherwise, let me know and I can pick them up from the
current series.

Thanks!