[PATCH v5 0/4] x86/boot/KASLR: Parse ACPI table and limit kaslr in immovable memory.

[PATCH v5 0/4] x86/boot/KASLR: Parse ACPI table and limit kaslr in immovable memory.

[Chao Fan]

***Background:
People reported that kaslr may randomly chooses some positions
which are located in movable memory regions. This will break memory
hotplug feature and make the memory can't be removed.

***Solutions:
There should be a method to limit kaslr to choosing immovable memory
regions, so there are 2 solutions:
1) Add a kernel parameter to specify the memory regions.
2) Get the information of memory hotremove, then kaslr will know the
   right regions.
In method 2, information about memory hot remove is in ACPI
tables, which will be parsed after 'start_kernel', kaslr can't get
the information.
In method 1, users should know the regions address and specify in
kernel parameter.

In the earliest time, I tried to dig ACPI tabls to solve this problem.
But I didn't splite the code in 'compressed/' and ACPI code, so the patch
is hard to follow so refused by community.
Somebody suggest to add a kernel parameter to specify the
immovable memory so that limit kaslr in these regions. Then I make
a patchset. After several versions, Ingo gave a suggestion:
https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1634024.html
Follow Ingo's suggestion, imitate the ACPI code to parse the acpi
tables, so that the kaslr can get necessary memory information in
ACPI tables.
Since I think ACPI code is independent part, so copy the codes
and functions to 'compressed/' directory, so that kaslr won't
influence the initialization of ACPI.

PATCH 1/4 Reuse the head file of linux/acpi.h, and copy a fcuntion from
          ACPI code.
PATCH 2/4 Functions to parse ACPI code.
PATCH 3/4 If 'CONFIG_MEMORY_HOTREMOVE' specified, walk all nodes and
          store the information of immovable memory regions.
PATCH 4/4 According to the immovable memory regions, filter the
          immovable regions which KASLR can choose.

***Test results:
 - I did a very simple test, and it can get the memory information in
   bios and efi KVM guest machine, and put it by early printk. But no
   more tests, so it's with RFC tag.

v1->v2:
 -  Simplify some code.
Follow Baoquan He's suggestion:
 - Reuse the head file of acpi code.

v2->v3:
 - Test in more conditions, so remove the 'RFC' tag.
 - Change some comments.

v3->v4:
Follow Thomas Gleixner's suggetsion:
 - Put the whole efi related function into #define CONFIG_EFI and return
   false in the other stub.
 - Simplify two functions in head file.

v4->v5:
Follow Dou Liyang's suggestion:
 - Add more comments about some functions based on kernel code.
 - Change some typo in comments.
 - Clean useless variable.
 - Add check for the boundary of array.
 - Add check for 'movable_node' parameter

Any comments will be welcome.


Chao Fan (4):
  x86/boot: Add acpitb.h to help parse acpi tables
  x86/boot: Add acpitb.c to parse acpi tables
  x86/boot/KASLR: Walk srat tables to filter immovable memory
  x86/boot/KASLR: Limit kaslr to choosing the immovable memory

 arch/x86/boot/compressed/Makefile |   4 +
 arch/x86/boot/compressed/acpitb.c | 272 ++++++++++++++++++++++++++++++
 arch/x86/boot/compressed/acpitb.h |   7 +
 arch/x86/boot/compressed/kaslr.c  | 125 ++++++++++++--
 4 files changed, 397 insertions(+), 11 deletions(-)
 create mode 100644 arch/x86/boot/compressed/acpitb.c
 create mode 100644 arch/x86/boot/compressed/acpitb.h

-- 
2.17.1

[PATCH v5 1/4] x86/boot: Add acpitb.h to help parse acpi tables

[Chao Fan]

In order to parse ACPI tables, reuse the head file linux/acpi.h,
so that the files in 'compressed' directory can read ACPI table
by including this head file.

Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com>
---
 arch/x86/boot/compressed/acpitb.h | 7 +++++++
 1 file changed, 7 insertions(+)
 create mode 100644 arch/x86/boot/compressed/acpitb.h

diff --git a/arch/x86/boot/compressed/acpitb.h b/arch/x86/boot/compressed/acpitb.h
new file mode 100644
index 000000000000..f8ab6e5b3e06
--- /dev/null
+++ b/arch/x86/boot/compressed/acpitb.h
@@ -0,0 +1,7 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#include <linux/acpi.h>
+
+#define ACPI_MAX_TABLES                128
+
+/* Function to get ACPI SRAT table pointer. */
+struct acpi_table_header *get_acpi_srat_table(void);
-- 
2.17.1

[PATCH v5 2/4] x86/boot: Add acpitb.c to parse acpi tables

[Chao Fan]

Imitate the ACPI code to parse ACPI tables. Functions are simplified
cause some operations are not needed here.
And also, this method won't influence the initialization of ACPI.

Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com>
---
 arch/x86/boot/compressed/Makefile |   4 +
 arch/x86/boot/compressed/acpitb.c | 272 ++++++++++++++++++++++++++++++
 2 files changed, 276 insertions(+)
 create mode 100644 arch/x86/boot/compressed/acpitb.c

diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
index 169c2feda14a..a382bb50f599 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -83,6 +83,10 @@ ifdef CONFIG_X86_64
 	vmlinux-objs-y += $(obj)/pgtable_64.o
 endif
 
+ifdef CONFIG_MEMORY_HOTREMOVE
+vmlinux-objs-$(CONFIG_RANDOMIZE_BASE) += $(obj)/acpitb.o
+endif
+
 $(obj)/eboot.o: KBUILD_CFLAGS += -fshort-wchar -mno-red-zone
 
 vmlinux-objs-$(CONFIG_EFI_STUB) += $(obj)/eboot.o $(obj)/efi_stub_$(BITS).o \
diff --git a/arch/x86/boot/compressed/acpitb.c b/arch/x86/boot/compressed/acpitb.c
new file mode 100644
index 000000000000..a1ec0d9313fe
--- /dev/null
+++ b/arch/x86/boot/compressed/acpitb.c
@@ -0,0 +1,272 @@
+// SPDX-License-Identifier: GPL-2.0
+#define BOOT_CTYPE_H
+#include "misc.h"
+#include "acpitb.h"
+
+#include <linux/efi.h>
+#include <asm/efi.h>
+#include <linux/numa.h>
+
+extern unsigned long get_cmd_line_ptr(void);
+
+#ifdef CONFIG_EFI
+/* Search EFI table for rsdp table. */
+static bool efi_get_rsdp_addr(acpi_physical_address *rsdp_addr)
+{
+	efi_system_table_t *systab;
+	bool find_rsdp = false;
+	bool efi_64 = false;
+	void *config_tables;
+	struct efi_info *e;
+	char *sig;
+	int size;
+	int i;
+
+	e = &boot_params->efi_info;
+	sig = (char *)&e->efi_loader_signature;
+
+	if (!strncmp(sig, EFI64_LOADER_SIGNATURE, 4))
+		efi_64 = true;
+	else if (!strncmp(sig, EFI32_LOADER_SIGNATURE, 4))
+		efi_64 = false;
+	else {
+		debug_putstr("Wrong EFI loader signature.\n");
+		return false;
+	}
+
+	/* Get systab from boot params. Based on efi_init(). */
+#ifdef CONFIG_X86_32
+	if (e->efi_systab_hi || e->efi_memmap_hi) {
+		debug_putstr("Table located above 4GB, disabling EFI.\n");
+		return false;
+	}
+	systab = (efi_system_table_t *)e->efi_systab;
+#else
+	systab = (efi_system_table_t *)(
+			e->efi_systab | ((__u64)e->efi_systab_hi<<32));
+#endif
+
+	if (systab == NULL)
+		return false;
+
+	/*
+	 * Get EFI tables from systab. Based on efi_config_init() and
+	 * efi_config_parse_tables(). Only dig the useful tables but not
+	 * do the initialization jobs.
+	 */
+	size = efi_64 ? sizeof(efi_config_table_64_t) :
+			sizeof(efi_config_table_32_t);
+
+	for (i = 0; i < systab->nr_tables; i++) {
+		efi_guid_t guid;
+		unsigned long table;
+
+		config_tables = (void *)(systab->tables + size * i);
+		if (efi_64) {
+			efi_config_table_64_t *tmp_table;
+
+			tmp_table = (efi_config_table_64_t *)config_tables;
+			guid = tmp_table->guid;
+			table = tmp_table->table;
+#ifndef CONFIG_64BIT
+			if (table >> 32) {
+				debug_putstr("Table located above 4G, disabling EFI.\n");
+				return false;
+			}
+#endif
+		} else {
+			efi_config_table_32_t *tmp_table;
+
+			tmp_table = (efi_config_table_32_t *)config_tables;
+			guid = tmp_table->guid;
+			table = tmp_table->table;
+		}
+
+		/*
+		 * Get rsdp from EFI tables.
+		 * If ACPI20 table found, use it and return true.
+		 * If ACPI20 table not found, but ACPI table found,
+		 * use the ACPI table and return true.
+		 * If neither ACPI table nor ACPI20 table found,
+		 * return false.
+		 */
+		if (!(efi_guidcmp(guid, ACPI_TABLE_GUID))) {
+			*rsdp_addr = (acpi_physical_address)table;
+			find_rsdp = true;
+		} else if (!(efi_guidcmp(guid, ACPI_20_TABLE_GUID))) {
+			*rsdp_addr = (acpi_physical_address)table;
+			return true;
+		}
+	}
+	return find_rsdp;
+}
+#else
+static bool efi_get_rsdp_addr(acpi_physical_address *rsdp_addr)
+{
+	return false;
+}
+#endif
+
+static u8 checksum(u8 *buffer, u32 length)
+{
+	u8 sum = 0;
+	u8 *end = buffer + length;
+
+	while (buffer < end)
+		sum = (u8)(sum + *(buffer++));
+
+	return sum;
+}
+
+/*
+ * Used to search a block of memory for the RSDP signature.
+ * Return Pointer to the RSDP if found, otherwise NULL.
+ * Based on acpi_tb_scan_memory_for_rsdp().
+ */
+static u8 *scan_mem_for_rsdp(u8 *start_address, u32 length)
+{
+	struct acpi_table_rsdp *rsdp;
+	u8 *end_address;
+	u8 *mem_rover;
+
+	end_address = start_address + length;
+
+	/* Search from given start address for the requested length */
+	for (mem_rover = start_address; mem_rover < end_address;
+	     mem_rover += ACPI_RSDP_SCAN_STEP) {
+		/* The RSDP signature and checksum must both be correct */
+		rsdp = (struct acpi_table_rsdp *)mem_rover;
+		if (!ACPI_VALIDATE_RSDP_SIG(rsdp->signature))
+			continue;
+		if (checksum((u8 *) rsdp, ACPI_RSDP_CHECKSUM_LENGTH) != 0)
+			continue;
+		if ((rsdp->revision >= 2) &&
+		    (checksum((u8 *) rsdp, ACPI_RSDP_XCHECKSUM_LENGTH) != 0))
+			continue;
+
+		/* Sig and checksum valid, we have found a real RSDP */
+		return mem_rover;
+	}
+	return NULL;
+}
+
+/*
+ * Used to search RSDP physical address.
+ * Based on acpi_find_root_pointer(). Since only use physical address
+ * in this period, so there is no need to do the memory map jobs.
+ */
+static void bios_get_rsdp_addr(acpi_physical_address *rsdp_addr)
+{
+	struct acpi_table_rsdp *rsdp;
+	u8 *table_ptr;
+	u8 *mem_rover;
+	u32 address;
+
+	/* Get the location of the Extended BIOS Data Area (EBDA) */
+	table_ptr = (u8 *)ACPI_EBDA_PTR_LOCATION;
+	*(u32 *)(void *)&address = *(u16 *)(void *)table_ptr;
+	address <<= 4;
+	table_ptr = (u8 *)(acpi_physical_address)address;
+
+	/*
+	 * Search EBDA paragraphs (EBDA is required to be a minimum of
+	 * 1K length)
+	 */
+	if (address > 0x400) {
+		mem_rover = scan_mem_for_rsdp(table_ptr, ACPI_EBDA_WINDOW_SIZE);
+
+		if (mem_rover) {
+			address += (u32)ACPI_PTR_DIFF(mem_rover, table_ptr);
+			*rsdp_addr = (acpi_physical_address)address;
+			return;
+		}
+	}
+
+	table_ptr = (u8 *)ACPI_HI_RSDP_WINDOW_BASE;
+	mem_rover = scan_mem_for_rsdp(table_ptr, ACPI_HI_RSDP_WINDOW_SIZE);
+
+	/*
+	 * Search upper memory: 16-byte boundaries in E0000h-FFFFFh
+	 */
+	if (mem_rover) {
+		address = (u32)(ACPI_HI_RSDP_WINDOW_BASE +
+				ACPI_PTR_DIFF(mem_rover, table_ptr));
+		*rsdp_addr = (acpi_physical_address)address;
+		return;
+	}
+}
+
+/*
+ * Used to dig rsdp table from EFI table or BIOS.
+ * If rsdp table found in EFI table, use it. Or search BIOS.
+ * Based on acpi_os_get_root_pointer().
+ */
+static acpi_physical_address get_rsdp_addr(void)
+{
+	acpi_physical_address pa = 0;
+	bool status = false;
+
+	status = efi_get_rsdp_addr(&pa);
+
+	if (!status || pa == 0)
+		bios_get_rsdp_addr(&pa);
+
+	return pa;
+}
+
+struct acpi_table_header *get_acpi_srat_table(void)
+{
+	char *args = (char *)get_cmd_line_ptr();
+	acpi_physical_address acpi_table;
+	acpi_physical_address root_table;
+	struct acpi_table_header *header;
+	struct acpi_table_rsdp *rsdp;
+	char *signature;
+	u8 *entry;
+	u32 count;
+	u32 size;
+	int i, j;
+	u32 len;
+
+	rsdp = (struct acpi_table_rsdp *)get_rsdp_addr();
+	if (!rsdp)
+		return NULL;
+
+	/* Get rsdt or xsdt from rsdp. */
+	if (!strstr(args, "acpi=rsdt") &&
+	    rsdp->xsdt_physical_address && rsdp->revision > 1) {
+		root_table = rsdp->xsdt_physical_address;
+		size = ACPI_XSDT_ENTRY_SIZE;
+	} else {
+		root_table = rsdp->rsdt_physical_address;
+		size = ACPI_RSDT_ENTRY_SIZE;
+	}
+
+	/* Get ACPI root table from rsdt or xsdt.*/
+	header = (struct acpi_table_header *)root_table;
+	len = header->length;
+	count = (u32)((len - sizeof(struct acpi_table_header)) / size);
+	entry = ACPI_ADD_PTR(u8, header, sizeof(struct acpi_table_header));
+
+	for (i = 0; i < count; i++) {
+		u64 address64;
+
+		if (size == ACPI_RSDT_ENTRY_SIZE)
+			acpi_table = ((acpi_physical_address)
+				      (*ACPI_CAST_PTR(u32, entry)));
+		else {
+			*(u64 *)(void *)&address64 = *(u64 *)(void *)entry;
+			acpi_table = (acpi_physical_address) address64;
+		}
+
+		if (acpi_table) {
+			header = (struct acpi_table_header *)acpi_table;
+			signature = header->signature;
+
+			if (!strncmp(signature, "SRAT", 4))
+				return header;
+		}
+		entry += size;
+	}
+	return NULL;
+}
-- 
2.17.1

[PATCH v5 3/4] x86/boot/KASLR: Walk srat tables to filter immovable memory

[Chao Fan]

If 'CONFIG_MEMORY_HOTREMOVE' specified, walk the acpi srat memory
tables, store the immovable memory regions, so that kaslr can get
the information abouth where can be selected or not.
If 'CONFIG_MEMORY_HOTREMOVE' not specified, go on the old code.

Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com>
---
 arch/x86/boot/compressed/kaslr.c | 59 ++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index 302517929932..720878f967a3 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -31,6 +31,7 @@
 
 #include "misc.h"
 #include "error.h"
+#include "acpitb.h"
 #include "../string.h"
 
 #include <generated/compile.h>
@@ -104,6 +105,14 @@ static bool memmap_too_large;
 /* Store memory limit specified by "mem=nn[KMG]" or "memmap=nn[KMG]" */
 static unsigned long long mem_limit = ULLONG_MAX;
 
+#ifdef CONFIG_MEMORY_HOTREMOVE
+/* Store the immovable memory regions */
+static struct mem_vector immovable_mem[MAX_NUMNODES*2];
+
+/* Store the amount of immovable memory regions */
+static int num_immovable_mem;
+#endif
+
 
 enum mem_avoid_index {
 	MEM_AVOID_ZO_RANGE = 0,
@@ -298,6 +307,51 @@ static int handle_mem_options(void)
 	return 0;
 }
 
+#ifdef CONFIG_MEMORY_HOTREMOVE
+/*
+ * According to ACPI table, filter the immvoable memory regions
+ * and store them in immovable_mem[].
+ */
+static void handle_immovable_mem(void)
+{
+	char *args = (char *)get_cmd_line_ptr();
+	struct acpi_table_header *table_header;
+	struct acpi_subtable_header *table;
+	struct acpi_srat_mem_affinity *ma;
+	unsigned long table_end;
+	int i = 0;
+
+	if (!strstr(args, "movable_node"))
+		return;
+
+	table_header = get_acpi_srat_table();
+	if (!table_header)
+		return;
+
+	table_end = (unsigned long)table_header + table_header->length;
+
+	table = (struct acpi_subtable_header *)
+		((unsigned long)table_header + sizeof(struct acpi_table_srat));
+
+	while (((unsigned long)table) + table->length < table_end) {
+		if (table->type == 1) {
+			ma = (struct acpi_srat_mem_affinity *)table;
+			if (!(ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE)) {
+				immovable_mem[i].start = ma->base_address;
+				immovable_mem[i].size = ma->length;
+				i++;
+			}
+
+			if (i >= MAX_NUMNODES*2)
+				break;
+		}
+		table = (struct acpi_subtable_header *)
+			((unsigned long)table + table->length);
+	}
+	num_immovable_mem = i;
+}
+#endif
+
 /*
  * In theory, KASLR can put the kernel anywhere in the range of [16M, 64T).
  * The mem_avoid array is used to store the ranges that need to be avoided
@@ -421,6 +475,11 @@ static void mem_avoid_init(unsigned long input, unsigned long input_size,
 	/* Mark the memmap regions we need to avoid */
 	handle_mem_options();
 
+#ifdef CONFIG_MEMORY_HOTREMOVE
+	/* Mark the immovable regions we need to choose */
+	handle_immovable_mem();
+#endif
+
 #ifdef CONFIG_X86_VERBOSE_BOOTUP
 	/* Make sure video RAM can be used. */
 	add_identity_map(0, PMD_SIZE);
-- 
2.17.1

[PATCH v5 4/4] x86/boot/KASLR: Limit kaslr to choosing the immovable memory

[Chao Fan]

If 'CONFIG_MEMORY_HOTREMOVE' specified and the account of immovable
memory regions is not zero. Calculate the intersection between memory
regions from e820/efi memory table and immovable memory regions.
Or go on the old code.

Rename process_mem_region to slots_count to match slots_fetch_random,
and name new function as process_mem_region.

Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com>
---
 arch/x86/boot/compressed/kaslr.c | 66 ++++++++++++++++++++++++++------
 1 file changed, 55 insertions(+), 11 deletions(-)

diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index 720878f967a3..9c6e24a23a2d 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -635,9 +635,9 @@ static unsigned long slots_fetch_random(void)
 	return 0;
 }
 
-static void process_mem_region(struct mem_vector *entry,
-			       unsigned long minimum,
-			       unsigned long image_size)
+static void slots_count(struct mem_vector *entry,
+			unsigned long minimum,
+			unsigned long image_size)
 {
 	struct mem_vector region, overlap;
 	struct slot_area slot_area;
@@ -714,6 +714,56 @@ static void process_mem_region(struct mem_vector *entry,
 	}
 }
 
+static bool process_mem_region(struct mem_vector *region,
+			       unsigned long long minimum,
+			       unsigned long long image_size)
+{
+#ifdef CONFIG_MEMORY_HOTREMOVE
+	/*
+	 * If immovable memory found, filter the intersection between
+	 * immovable memory and region to slots_count.
+	 * Otherwise, go on old code.
+	 */
+	if (num_immovable_mem > 0) {
+		int i;
+
+		for (i = 0; i < num_immovable_mem; i++) {
+			struct mem_vector entry;
+			unsigned long long start, end, entry_end, region_end;
+
+			start = immovable_mem[i].start;
+			end = start + immovable_mem[i].size;
+			region_end = region->start + region->size;
+
+			entry.start = clamp(region->start, start, end);
+			entry_end = clamp(region_end, start, end);
+
+			if (entry.start + image_size < entry_end) {
+				entry.size = entry_end - entry.start;
+				slots_count(&entry, minimum, image_size);
+
+				if (slot_area_index == MAX_SLOT_AREA) {
+					debug_putstr("Aborted e820/efi memmap scan (slot_areas full)!\n");
+					return 1;
+				}
+			}
+		}
+		return 0;
+	}
+#endif
+	/*
+	 * If no immovable memory found, or MEMORY_HOTREMOVE disabled,
+	 * walk all the regions, so use region directely.
+	 */
+	slots_count(region, minimum, image_size);
+
+	if (slot_area_index == MAX_SLOT_AREA) {
+		debug_putstr("Aborted e820/efi memmap scan (slot_areas full)!\n");
+		return 1;
+	}
+	return 0;
+}
+
 #ifdef CONFIG_EFI
 /*
  * Returns true if mirror region found (and must have been processed
@@ -779,11 +829,8 @@ process_efi_entries(unsigned long minimum, unsigned long image_size)
 
 		region.start = md->phys_addr;
 		region.size = md->num_pages << EFI_PAGE_SHIFT;
-		process_mem_region(&region, minimum, image_size);
-		if (slot_area_index == MAX_SLOT_AREA) {
-			debug_putstr("Aborted EFI scan (slot_areas full)!\n");
+		if (process_mem_region(&region, minimum, image_size))
 			break;
-		}
 	}
 	return true;
 }
@@ -810,11 +857,8 @@ static void process_e820_entries(unsigned long minimum,
 			continue;
 		region.start = entry->addr;
 		region.size = entry->size;
-		process_mem_region(&region, minimum, image_size);
-		if (slot_area_index == MAX_SLOT_AREA) {
-			debug_putstr("Aborted e820 scan (slot_areas full)!\n");
+		if (process_mem_region(&region, minimum, image_size))
 			break;
-		}
 	}
 }
 
-- 
2.17.1

Re: [PATCH v5 0/4] x86/boot/KASLR: Parse ACPI table and limit kaslr in immovable memory.

[Chao Fan]

Hi all,

No reply for 2 weeks, any comments?

Thanks,
Chao Fan

On Tue, Aug 07, 2018 at 02:49:56PM +0800, Chao Fan wrote:
>***Background:
>People reported that kaslr may randomly chooses some positions
>which are located in movable memory regions. This will break memory
>hotplug feature and make the memory can't be removed.
>
>***Solutions:
>There should be a method to limit kaslr to choosing immovable memory
>regions, so there are 2 solutions:
>1) Add a kernel parameter to specify the memory regions.
>2) Get the information of memory hotremove, then kaslr will know the
>   right regions.
>In method 2, information about memory hot remove is in ACPI
>tables, which will be parsed after 'start_kernel', kaslr can't get
>the information.
>In method 1, users should know the regions address and specify in
>kernel parameter.
>
>In the earliest time, I tried to dig ACPI tabls to solve this problem.
>But I didn't splite the code in 'compressed/' and ACPI code, so the patch
>is hard to follow so refused by community.
>Somebody suggest to add a kernel parameter to specify the
>immovable memory so that limit kaslr in these regions. Then I make
>a patchset. After several versions, Ingo gave a suggestion:
>https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1634024.html
>Follow Ingo's suggestion, imitate the ACPI code to parse the acpi
>tables, so that the kaslr can get necessary memory information in
>ACPI tables.
>Since I think ACPI code is independent part, so copy the codes
>and functions to 'compressed/' directory, so that kaslr won't
>influence the initialization of ACPI.
>
>PATCH 1/4 Reuse the head file of linux/acpi.h, and copy a fcuntion from
>          ACPI code.
>PATCH 2/4 Functions to parse ACPI code.
>PATCH 3/4 If 'CONFIG_MEMORY_HOTREMOVE' specified, walk all nodes and
>          store the information of immovable memory regions.
>PATCH 4/4 According to the immovable memory regions, filter the
>          immovable regions which KASLR can choose.
>
>***Test results:
> - I did a very simple test, and it can get the memory information in
>   bios and efi KVM guest machine, and put it by early printk. But no
>   more tests, so it's with RFC tag.
>
>v1->v2:
> -  Simplify some code.
>Follow Baoquan He's suggestion:
> - Reuse the head file of acpi code.
>
>v2->v3:
> - Test in more conditions, so remove the 'RFC' tag.
> - Change some comments.
>
>v3->v4:
>Follow Thomas Gleixner's suggetsion:
> - Put the whole efi related function into #define CONFIG_EFI and return
>   false in the other stub.
> - Simplify two functions in head file.
>
>v4->v5:
>Follow Dou Liyang's suggestion:
> - Add more comments about some functions based on kernel code.
> - Change some typo in comments.
> - Clean useless variable.
> - Add check for the boundary of array.
> - Add check for 'movable_node' parameter
>
>Any comments will be welcome.
>
>
>Chao Fan (4):
>  x86/boot: Add acpitb.h to help parse acpi tables
>  x86/boot: Add acpitb.c to parse acpi tables
>  x86/boot/KASLR: Walk srat tables to filter immovable memory
>  x86/boot/KASLR: Limit kaslr to choosing the immovable memory
>
> arch/x86/boot/compressed/Makefile |   4 +
> arch/x86/boot/compressed/acpitb.c | 272 ++++++++++++++++++++++++++++++
> arch/x86/boot/compressed/acpitb.h |   7 +
> arch/x86/boot/compressed/kaslr.c  | 125 ++++++++++++--
> 4 files changed, 397 insertions(+), 11 deletions(-)
> create mode 100644 arch/x86/boot/compressed/acpitb.c
> create mode 100644 arch/x86/boot/compressed/acpitb.h
>
>-- 
>2.17.1
>

Re: [PATCH v5 3/4] x86/boot/KASLR: Walk srat tables to filter immovable memory

[Baoquan He]

On 08/07/18 at 02:49pm, Chao Fan wrote:
> If 'CONFIG_MEMORY_HOTREMOVE' specified, walk the acpi srat memory
> tables, store the immovable memory regions, so that kaslr can get
> the information abouth where can be selected or not.
> If 'CONFIG_MEMORY_HOTREMOVE' not specified, go on the old code.
> 
> Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com>
> ---
>  arch/x86/boot/compressed/kaslr.c | 59 ++++++++++++++++++++++++++++++++
>  1 file changed, 59 insertions(+)
> 
> diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
> index 302517929932..720878f967a3 100644
> --- a/arch/x86/boot/compressed/kaslr.c
> +++ b/arch/x86/boot/compressed/kaslr.c
> @@ -31,6 +31,7 @@
>  
>  #include "misc.h"
>  #include "error.h"
> +#include "acpitb.h"
>  #include "../string.h"
>  
>  #include <generated/compile.h>
> @@ -104,6 +105,14 @@ static bool memmap_too_large;
>  /* Store memory limit specified by "mem=nn[KMG]" or "memmap=nn[KMG]" */
>  static unsigned long long mem_limit = ULLONG_MAX;
>  
> +#ifdef CONFIG_MEMORY_HOTREMOVE
> +/* Store the immovable memory regions */
> +static struct mem_vector immovable_mem[MAX_NUMNODES*2];
> +
> +/* Store the amount of immovable memory regions */
> +static int num_immovable_mem;
> +#endif
> +
>  
>  enum mem_avoid_index {
>  	MEM_AVOID_ZO_RANGE = 0,
> @@ -298,6 +307,51 @@ static int handle_mem_options(void)
>  	return 0;
>  }
>  
> +#ifdef CONFIG_MEMORY_HOTREMOVE
> +/*
> + * According to ACPI table, filter the immvoable memory regions
> + * and store them in immovable_mem[].
> + */
> +static void handle_immovable_mem(void)
> +{
> +	char *args = (char *)get_cmd_line_ptr();
> +	struct acpi_table_header *table_header;
> +	struct acpi_subtable_header *table;
> +	struct acpi_srat_mem_affinity *ma;
> +	unsigned long table_end;
> +	int i = 0;
> +
> +	if (!strstr(args, "movable_node"))

If 'acpi=off' specified, better return too here.

> +		return;
> +
> +	table_header = get_acpi_srat_table();
> +	if (!table_header)
> +		return;
> +
> +	table_end = (unsigned long)table_header + table_header->length;
> +
> +	table = (struct acpi_subtable_header *)
> +		((unsigned long)table_header + sizeof(struct acpi_table_srat));
> +
> +	while (((unsigned long)table) + table->length < table_end) {
> +		if (table->type == 1) {
> +			ma = (struct acpi_srat_mem_affinity *)table;
> +			if (!(ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE)) {
> +				immovable_mem[i].start = ma->base_address;
> +				immovable_mem[i].size = ma->length;
> +				i++;
> +			}
> +
> +			if (i >= MAX_NUMNODES*2)
> +				break;
> +		}
> +		table = (struct acpi_subtable_header *)
> +			((unsigned long)table + table->length);
> +	}
> +	num_immovable_mem = i;
> +}
> +#endif
> +
>  /*
>   * In theory, KASLR can put the kernel anywhere in the range of [16M, 64T).
>   * The mem_avoid array is used to store the ranges that need to be avoided
> @@ -421,6 +475,11 @@ static void mem_avoid_init(unsigned long input, unsigned long input_size,
>  	/* Mark the memmap regions we need to avoid */
>  	handle_mem_options();
>  
> +#ifdef CONFIG_MEMORY_HOTREMOVE
> +	/* Mark the immovable regions we need to choose */
> +	handle_immovable_mem();
> +#endif
> +
>  #ifdef CONFIG_X86_VERBOSE_BOOTUP
>  	/* Make sure video RAM can be used. */
>  	add_identity_map(0, PMD_SIZE);
> -- 
> 2.17.1
> 
> 
> 

Re: [PATCH v5 3/4] x86/boot/KASLR: Walk srat tables to filter immovable memory

[Chao Fan]

On Thu, Aug 23, 2018 at 03:25:35PM +0800, Baoquan He wrote:
[...]
>> +static void handle_immovable_mem(void)
>> +{
>> +	char *args = (char *)get_cmd_line_ptr();
>> +	struct acpi_table_header *table_header;
>> +	struct acpi_subtable_header *table;
>> +	struct acpi_srat_mem_affinity *ma;
>> +	unsigned long table_end;
>> +	int i = 0;
>> +
>> +	if (!strstr(args, "movable_node"))
>
>If 'acpi=off' specified, better return too here.

Thanks, you are right.
Yes, I will think about it and add it to a suitable place.
May the position where we try to get the acpi table is better.
Or just here is also OK.

Thanks,
Chao Fan

>

Re: [PATCH v5 3/4] x86/boot/KASLR: Walk srat tables to filter immovable memory

[Baoquan He]

On 08/07/18 at 02:49pm, Chao Fan wrote:
> If 'CONFIG_MEMORY_HOTREMOVE' specified, walk the acpi srat memory
> tables, store the immovable memory regions, so that kaslr can get
> the information abouth where can be selected or not.
> If 'CONFIG_MEMORY_HOTREMOVE' not specified, go on the old code.
Just adjust the patch log a little bit, just for your reference.

If 'CONFIG_MEMORY_HOTREMOVE' specified, walk through the acpi srat memory
tables and store those immovable memory regions so that kaslr can get
where to choose for randomization.

> 
> Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com>
> ---
>  arch/x86/boot/compressed/kaslr.c | 59 ++++++++++++++++++++++++++++++++
>  1 file changed, 59 insertions(+)
> 
> diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
> index 302517929932..720878f967a3 100644
> --- a/arch/x86/boot/compressed/kaslr.c
> +++ b/arch/x86/boot/compressed/kaslr.c
> @@ -31,6 +31,7 @@
>  
>  #include "misc.h"
>  #include "error.h"
> +#include "acpitb.h"
>  #include "../string.h"
>  
>  #include <generated/compile.h>
> @@ -104,6 +105,14 @@ static bool memmap_too_large;
>  /* Store memory limit specified by "mem=nn[KMG]" or "memmap=nn[KMG]" */
>  static unsigned long long mem_limit = ULLONG_MAX;
>  
> +#ifdef CONFIG_MEMORY_HOTREMOVE
> +/* Store the immovable memory regions */
> +static struct mem_vector immovable_mem[MAX_NUMNODES*2];
> +
> +/* Store the amount of immovable memory regions */
> +static int num_immovable_mem;
> +#endif
> +
>  
>  enum mem_avoid_index {
>  	MEM_AVOID_ZO_RANGE = 0,
> @@ -298,6 +307,51 @@ static int handle_mem_options(void)
>  	return 0;
>  }
>  
> +#ifdef CONFIG_MEMORY_HOTREMOVE
> +/*
> + * According to ACPI table, filter the immvoable memory regions
> + * and store them in immovable_mem[].
> + */
> +static void handle_immovable_mem(void)

Can we change this function like below, passed in the immovable_mem and
the array lengty, the value of num_immovable_mem will be passed back?

static void handle_immovable_mem(char* region, int max, int *num)
{
}

Like this, you don't need patch 1/4 to make a header file, just put this
function handle_immovable_mem() into acpitb.c since it's handling acpi
tables. And its name can be get_immovable_mem().


> +{
> +	char *args = (char *)get_cmd_line_ptr();
> +	struct acpi_table_header *table_header;
> +	struct acpi_subtable_header *table;
> +	struct acpi_srat_mem_affinity *ma;
> +	unsigned long table_end;
> +	int i = 0;
> +
> +	if (!strstr(args, "movable_node"))
> +		return;
> +
> +	table_header = get_acpi_srat_table();
> +	if (!table_header)
> +		return;
> +
> +	table_end = (unsigned long)table_header + table_header->length;
> +
> +	table = (struct acpi_subtable_header *)
> +		((unsigned long)table_header + sizeof(struct acpi_table_srat));
> +
> +	while (((unsigned long)table) + table->length < table_end) {
> +		if (table->type == 1) {
> +			ma = (struct acpi_srat_mem_affinity *)table;
> +			if (!(ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE)) {
> +				immovable_mem[i].start = ma->base_address;
> +				immovable_mem[i].size = ma->length;
> +				i++;
> +			}
> +
> +			if (i >= MAX_NUMNODES*2)
> +				break;
> +		}
> +		table = (struct acpi_subtable_header *)
> +			((unsigned long)table + table->length);
> +	}
> +	num_immovable_mem = i;
> +}
> +#endif
> +
>  /*
>   * In theory, KASLR can put the kernel anywhere in the range of [16M, 64T).
>   * The mem_avoid array is used to store the ranges that need to be avoided
> @@ -421,6 +475,11 @@ static void mem_avoid_init(unsigned long input, unsigned long input_size,
>  	/* Mark the memmap regions we need to avoid */
>  	handle_mem_options();
>  
> +#ifdef CONFIG_MEMORY_HOTREMOVE
> +	/* Mark the immovable regions we need to choose */
> +	handle_immovable_mem();
> +#endif
> +
>  #ifdef CONFIG_X86_VERBOSE_BOOTUP
>  	/* Make sure video RAM can be used. */
>  	add_identity_map(0, PMD_SIZE);
> -- 
> 2.17.1
> 
> 
> 

Re: [PATCH v5 3/4] x86/boot/KASLR: Walk srat tables to filter immovable memory

[Chao Fan]

On Mon, Aug 27, 2018 at 10:13:49AM +0800, Baoquan He wrote:
>On 08/07/18 at 02:49pm, Chao Fan wrote:
>> If 'CONFIG_MEMORY_HOTREMOVE' specified, walk the acpi srat memory
>> tables, store the immovable memory regions, so that kaslr can get
>> the information abouth where can be selected or not.
>> If 'CONFIG_MEMORY_HOTREMOVE' not specified, go on the old code.
>Just adjust the patch log a little bit, just for your reference.
>
>If 'CONFIG_MEMORY_HOTREMOVE' specified, walk through the acpi srat memory
>tables and store those immovable memory regions so that kaslr can get
>where to choose for randomization.

OK, I will change more patch log in next version.

>
>> 

[...]

>> +#ifdef CONFIG_MEMORY_HOTREMOVE
>> +/*
>> + * According to ACPI table, filter the immvoable memory regions
>> + * and store them in immovable_mem[].
>> + */
>> +static void handle_immovable_mem(void)
>
>Can we change this function like below, passed in the immovable_mem and
>the array lengty, the value of num_immovable_mem will be passed back?
>
>static void handle_immovable_mem(char* region, int max, int *num)
>{
>}
>
>Like this, you don't need patch 1/4 to make a header file, just put this
>function handle_immovable_mem() into acpitb.c since it's handling acpi
>tables. And its name can be get_immovable_mem().

That makes sense.
But if we change like this, we can put the whole function to acpitb.c.
It will be like:
static void handle_immovable_mem(int *num)
{
}
Because the value of immovable_mem is filled in handle_immovable_mem,
it's better to put immovable_mem in acpitb.c also, then extern in
kaslr.c.

Thanks,
Chao Fan

Re: [PATCH v5 3/4] x86/boot/KASLR: Walk srat tables to filter immovable memory

[Baoquan He]

On 08/27/18 at 10:56am, Chao Fan wrote:

> >> +#ifdef CONFIG_MEMORY_HOTREMOVE
> >> +/*
> >> + * According to ACPI table, filter the immvoable memory regions
> >> + * and store them in immovable_mem[].
> >> + */
> >> +static void handle_immovable_mem(void)
> >
> >Can we change this function like below, passed in the immovable_mem and
> >the array lengty, the value of num_immovable_mem will be passed back?
> >
> >static void handle_immovable_mem(char* region, int max, int *num)
> >{
> >}
> >
> >Like this, you don't need patch 1/4 to make a header file, just put this
> >function handle_immovable_mem() into acpitb.c since it's handling acpi
> >tables. And its name can be get_immovable_mem().
> 
> That makes sense.
> But if we change like this, we can put the whole function to acpitb.c.
> It will be like:
> static void handle_immovable_mem(int *num)
> {
> }
> Because the value of immovable_mem is filled in handle_immovable_mem,
> it's better to put immovable_mem in acpitb.c also, then extern in
> kaslr.c.

Yes, it's fine to me. Thanks.

> 
> 

Re: [PATCH v5 4/4] x86/boot/KASLR: Limit kaslr to choosing the immovable memory

[Baoquan He]

On 08/07/18 at 02:50pm, Chao Fan wrote:
> If 'CONFIG_MEMORY_HOTREMOVE' specified and the account of immovable
> memory regions is not zero. Calculate the intersection between memory
> regions from e820/efi memory table and immovable memory regions.
> Or go on the old code.
> 
> Rename process_mem_region to slots_count to match slots_fetch_random,
> and name new function as process_mem_region.
> 
> Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com>
> ---
>  arch/x86/boot/compressed/kaslr.c | 66 ++++++++++++++++++++++++++------
>  1 file changed, 55 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
> index 720878f967a3..9c6e24a23a2d 100644
> --- a/arch/x86/boot/compressed/kaslr.c
> +++ b/arch/x86/boot/compressed/kaslr.c
> @@ -635,9 +635,9 @@ static unsigned long slots_fetch_random(void)
>  	return 0;
>  }
>  
> -static void process_mem_region(struct mem_vector *entry,
> -			       unsigned long minimum,
> -			       unsigned long image_size)
> +static void slots_count(struct mem_vector *entry,
> +			unsigned long minimum,
> +			unsigned long image_size)
>  {
>  	struct mem_vector region, overlap;
>  	struct slot_area slot_area;
> @@ -714,6 +714,56 @@ static void process_mem_region(struct mem_vector *entry,
>  	}
>  }
>  
> +static bool process_mem_region(struct mem_vector *region,
> +			       unsigned long long minimum,
> +			       unsigned long long image_size)
> +{
> +#ifdef CONFIG_MEMORY_HOTREMOVE
> +	/*
> +	 * If immovable memory found, filter the intersection between
> +	 * immovable memory and region to slots_count.
> +	 * Otherwise, go on old code.
> +	 */
> +	if (num_immovable_mem > 0) {

Is it possible to take num_immovable_mem out from the #ifdef
CONFIG_MEMORY_HOTREMOVE region so that you can check it earlier to see
if the old way need be taken? This way, we can reduce one level of
indentation in the for loop. Just personal thought.

static bool process_mem_region(struct mem_vector *region,
                            unsigned long long minimum,
                            unsigned long long image_size)
{
	if (!num_immovable_mem) {
		slots_count(region, minimum, image_size);
		
		if (slot_area_index == MAX_SLOT_AREA) {
			debug_putstr("Aborted e820/efi memmap scan (slot_areas full)!\n");
			return 1;
		}
		return 0;
	}


#ifdef CONFIG_MEMORY_HOTREMOVE
	for (i = 0; i < num_immovable_mem; i++) {
		......
	}
#endif
}

> +		int i;
> +
> +		for (i = 0; i < num_immovable_mem; i++) {
> +			struct mem_vector entry;
> +			unsigned long long start, end, entry_end, region_end;
> +
> +			start = immovable_mem[i].start;
> +			end = start + immovable_mem[i].size;
> +			region_end = region->start + region->size;
> +
> +			entry.start = clamp(region->start, start, end);
> +			entry_end = clamp(region_end, start, end);
> +
> +			if (entry.start + image_size < entry_end) {
> +				entry.size = entry_end - entry.start;
> +				slots_count(&entry, minimum, image_size);
> +
> +				if (slot_area_index == MAX_SLOT_AREA) {
> +					debug_putstr("Aborted e820/efi memmap scan (slot_areas full)!\n");
> +					return 1;
> +				}
> +			}
> +		}
> +		return 0;
> +	}
> +#endif
> +	/*
> +	 * If no immovable memory found, or MEMORY_HOTREMOVE disabled,
> +	 * walk all the regions, so use region directely.
> +	 */
> +	slots_count(region, minimum, image_size);
> +
> +	if (slot_area_index == MAX_SLOT_AREA) {
> +		debug_putstr("Aborted e820/efi memmap scan (slot_areas full)!\n");
> +		return 1;
> +	}
> +	return 0;
> +}
> +
>  #ifdef CONFIG_EFI
>  /*
>   * Returns true if mirror region found (and must have been processed
> @@ -779,11 +829,8 @@ process_efi_entries(unsigned long minimum, unsigned long image_size)
>  
>  		region.start = md->phys_addr;
>  		region.size = md->num_pages << EFI_PAGE_SHIFT;
> -		process_mem_region(&region, minimum, image_size);
> -		if (slot_area_index == MAX_SLOT_AREA) {
> -			debug_putstr("Aborted EFI scan (slot_areas full)!\n");
> +		if (process_mem_region(&region, minimum, image_size))
>  			break;
> -		}
>  	}
>  	return true;
>  }
> @@ -810,11 +857,8 @@ static void process_e820_entries(unsigned long minimum,
>  			continue;
>  		region.start = entry->addr;
>  		region.size = entry->size;
> -		process_mem_region(&region, minimum, image_size);
> -		if (slot_area_index == MAX_SLOT_AREA) {
> -			debug_putstr("Aborted e820 scan (slot_areas full)!\n");
> +		if (process_mem_region(&region, minimum, image_size))
>  			break;
> -		}
>  	}
>  }
>  
> -- 
> 2.17.1
> 
> 
> 

Re: [PATCH v5 4/4] x86/boot/KASLR: Limit kaslr to choosing the immovable memory

[Baoquan He]

On 08/07/18 at 02:50pm, Chao Fan wrote:
> If 'CONFIG_MEMORY_HOTREMOVE' specified and the account of immovable
If CONFIG_MEMORY_HOTREMOVE is enabled, 
> memory regions is not zero. Calculate the intersection between memory
> regions from e820/efi memory table and immovable memory regions.
> Or go on the old code.
> 
> Rename process_mem_region to slots_count to match slots_fetch_random,
> and name new function as process_mem_region.
> 
> Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com>
> ---
>  arch/x86/boot/compressed/kaslr.c | 66 ++++++++++++++++++++++++++------
>  1 file changed, 55 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
> index 720878f967a3..9c6e24a23a2d 100644
> --- a/arch/x86/boot/compressed/kaslr.c
> +++ b/arch/x86/boot/compressed/kaslr.c
> @@ -635,9 +635,9 @@ static unsigned long slots_fetch_random(void)
>  	return 0;
>  }
>  
> -static void process_mem_region(struct mem_vector *entry,
> -			       unsigned long minimum,
> -			       unsigned long image_size)
> +static void slots_count(struct mem_vector *entry,
> +			unsigned long minimum,
> +			unsigned long image_size)
>  {
>  	struct mem_vector region, overlap;
>  	struct slot_area slot_area;
> @@ -714,6 +714,56 @@ static void process_mem_region(struct mem_vector *entry,
>  	}
>  }
>  
> +static bool process_mem_region(struct mem_vector *region,
> +			       unsigned long long minimum,
> +			       unsigned long long image_size)
> +{

Is it possible to take num_immovable_mem definition out from #ifdef
CONFIG_MEMORY_HOTREMOVE block and check it here like below? This way,
one level of indentation can be reduced in the for loop, and code is
more readable.


static bool process_mem_region(struct mem_vector *region,
			       unsigned long long minimum,
			       unsigned long long image_size)
{

	/*
	 * If no immovable memory found, or MEMORY_HOTREMOVE disabled,
	 * walk all the regions, so use region directely.
	 */
	if (num_immovable_mem > 0) {
		slots_count(region, minimum, image_size);
		
		if (slot_area_index == MAX_SLOT_AREA) {
			debug_putstr("Aborted e820/efi memmap scan (slot_areas full)!\n");
			return 1;
		}
		return 0;
	}

#ifdef CONFIG_MEMORY_HOTREMOVE
	for (i = 0; i < num_immovable_mem; i++) {
		...
	}
#endif
}

> +#ifdef CONFIG_MEMORY_HOTREMOVE
> +	/*
> +	 * If immovable memory found, filter the intersection between
> +	 * immovable memory and region to slots_count.
> +	 * Otherwise, go on old code.
> +	 */
> +	if (num_immovable_mem > 0) {
> +		int i;
> +
> +		for (i = 0; i < num_immovable_mem; i++) {
> +			struct mem_vector entry;
> +			unsigned long long start, end, entry_end, region_end;
> +
> +			start = immovable_mem[i].start;
> +			end = start + immovable_mem[i].size;
> +			region_end = region->start + region->size;
> +
> +			entry.start = clamp(region->start, start, end);
> +			entry_end = clamp(region_end, start, end);
> +
> +			if (entry.start + image_size < entry_end) {
> +				entry.size = entry_end - entry.start;
> +				slots_count(&entry, minimum, image_size);
> +
> +				if (slot_area_index == MAX_SLOT_AREA) {
> +					debug_putstr("Aborted e820/efi memmap scan (slot_areas full)!\n");
> +					return 1;
> +				}
> +			}
> +		}
> +		return 0;
> +	}
> +#endif
> +	/*
> +	 * If no immovable memory found, or MEMORY_HOTREMOVE disabled,
> +	 * walk all the regions, so use region directely.
> +	 */
> +	slots_count(region, minimum, image_size);
> +
> +	if (slot_area_index == MAX_SLOT_AREA) {
> +		debug_putstr("Aborted e820/efi memmap scan (slot_areas full)!\n");
> +		return 1;
> +	}
> +	return 0;
> +}
> +
>  #ifdef CONFIG_EFI
>  /*
>   * Returns true if mirror region found (and must have been processed
> @@ -779,11 +829,8 @@ process_efi_entries(unsigned long minimum, unsigned long image_size)
>  
>  		region.start = md->phys_addr;
>  		region.size = md->num_pages << EFI_PAGE_SHIFT;
> -		process_mem_region(&region, minimum, image_size);
> -		if (slot_area_index == MAX_SLOT_AREA) {
> -			debug_putstr("Aborted EFI scan (slot_areas full)!\n");
> +		if (process_mem_region(&region, minimum, image_size))
>  			break;
> -		}
>  	}
>  	return true;
>  }
> @@ -810,11 +857,8 @@ static void process_e820_entries(unsigned long minimum,
>  			continue;
>  		region.start = entry->addr;
>  		region.size = entry->size;
> -		process_mem_region(&region, minimum, image_size);
> -		if (slot_area_index == MAX_SLOT_AREA) {
> -			debug_putstr("Aborted e820 scan (slot_areas full)!\n");
> +		if (process_mem_region(&region, minimum, image_size))
>  			break;
> -		}
>  	}
>  }
>  
> -- 
> 2.17.1
> 
> 
> 

Re: [PATCH v5 4/4] x86/boot/KASLR: Limit kaslr to choosing the immovable memory

[Baoquan He]

On 08/27/18 at 01:35pm, Baoquan He wrote:
 
> Is it possible to take num_immovable_mem out from the #ifdef
> CONFIG_MEMORY_HOTREMOVE region so that you can check it earlier to see
> if the old way need be taken? This way, we can reduce one level of
> indentation in the for loop. Just personal thought.

Sorry, there's something wrong with my offlineimap, I thought it was not
sent out, so replied twice. Please ignore this one.

> 
> static bool process_mem_region(struct mem_vector *region,
>                             unsigned long long minimum,
>                             unsigned long long image_size)
> {
> 	if (!num_immovable_mem) {
> 		slots_count(region, minimum, image_size);
> 		
> 		if (slot_area_index == MAX_SLOT_AREA) {
> 			debug_putstr("Aborted e820/efi memmap scan (slot_areas full)!\n");
> 			return 1;
> 		}
> 		return 0;
> 	}
> 
> 
> #ifdef CONFIG_MEMORY_HOTREMOVE
> 	for (i = 0; i < num_immovable_mem; i++) {
> 		......
> 	}
> #endif
> }
> 
> > +		int i;
> > +
> > +		for (i = 0; i < num_immovable_mem; i++) {
> > +			struct mem_vector entry;
> > +			unsigned long long start, end, entry_end, region_end;
> > +
> > +			start = immovable_mem[i].start;
> > +			end = start + immovable_mem[i].size;
> > +			region_end = region->start + region->size;
> > +
> > +			entry.start = clamp(region->start, start, end);
> > +			entry_end = clamp(region_end, start, end);
> > +
> > +			if (entry.start + image_size < entry_end) {
> > +				entry.size = entry_end - entry.start;
> > +				slots_count(&entry, minimum, image_size);
> > +
> > +				if (slot_area_index == MAX_SLOT_AREA) {
> > +					debug_putstr("Aborted e820/efi memmap scan (slot_areas full)!\n");
> > +					return 1;
> > +				}
> > +			}
> > +		}
> > +		return 0;
> > +	}
> > +#endif
> > +	/*
> > +	 * If no immovable memory found, or MEMORY_HOTREMOVE disabled,
> > +	 * walk all the regions, so use region directely.
> > +	 */
> > +	slots_count(region, minimum, image_size);
> > +
> > +	if (slot_area_index == MAX_SLOT_AREA) {
> > +		debug_putstr("Aborted e820/efi memmap scan (slot_areas full)!\n");
> > +		return 1;
> > +	}
> > +	return 0;
> > +}
> > +
> >  #ifdef CONFIG_EFI
> >  /*
> >   * Returns true if mirror region found (and must have been processed
> > @@ -779,11 +829,8 @@ process_efi_entries(unsigned long minimum, unsigned long image_size)
> >  
> >  		region.start = md->phys_addr;
> >  		region.size = md->num_pages << EFI_PAGE_SHIFT;
> > -		process_mem_region(&region, minimum, image_size);
> > -		if (slot_area_index == MAX_SLOT_AREA) {
> > -			debug_putstr("Aborted EFI scan (slot_areas full)!\n");
> > +		if (process_mem_region(&region, minimum, image_size))
> >  			break;
> > -		}
> >  	}
> >  	return true;
> >  }
> > @@ -810,11 +857,8 @@ static void process_e820_entries(unsigned long minimum,
> >  			continue;
> >  		region.start = entry->addr;
> >  		region.size = entry->size;
> > -		process_mem_region(&region, minimum, image_size);
> > -		if (slot_area_index == MAX_SLOT_AREA) {
> > -			debug_putstr("Aborted e820 scan (slot_areas full)!\n");
> > +		if (process_mem_region(&region, minimum, image_size))
> >  			break;
> > -		}
> >  	}
> >  }
> >  
> > -- 
> > 2.17.1
> > 
> > 
> > 

Re: [PATCH v5 4/4] x86/boot/KASLR: Limit kaslr to choosing the immovable memory

[Chao Fan]

On Mon, Aug 27, 2018 at 01:56:07PM +0800, Baoquan He wrote:
>On 08/07/18 at 02:50pm, Chao Fan wrote:
>> If 'CONFIG_MEMORY_HOTREMOVE' specified and the account of immovable
>If CONFIG_MEMORY_HOTREMOVE is enabled, 
>> memory regions is not zero. Calculate the intersection between memory
>> regions from e820/efi memory table and immovable memory regions.
>> Or go on the old code.
>> 
>> Rename process_mem_region to slots_count to match slots_fetch_random,
>> and name new function as process_mem_region.
>> 
>> Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com>
>> ---
>>  arch/x86/boot/compressed/kaslr.c | 66 ++++++++++++++++++++++++++------
>>  1 file changed, 55 insertions(+), 11 deletions(-)
>> 
>> diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
>> index 720878f967a3..9c6e24a23a2d 100644
>> --- a/arch/x86/boot/compressed/kaslr.c
>> +++ b/arch/x86/boot/compressed/kaslr.c
>> @@ -635,9 +635,9 @@ static unsigned long slots_fetch_random(void)
>>  	return 0;
>>  }
>>  
>> -static void process_mem_region(struct mem_vector *entry,
>> -			       unsigned long minimum,
>> -			       unsigned long image_size)
>> +static void slots_count(struct mem_vector *entry,
>> +			unsigned long minimum,
>> +			unsigned long image_size)
>>  {
>>  	struct mem_vector region, overlap;
>>  	struct slot_area slot_area;
>> @@ -714,6 +714,56 @@ static void process_mem_region(struct mem_vector *entry,
>>  	}
>>  }
>>  
>> +static bool process_mem_region(struct mem_vector *region,
>> +			       unsigned long long minimum,
>> +			       unsigned long long image_size)
>> +{
>
>Is it possible to take num_immovable_mem definition out from #ifdef
>CONFIG_MEMORY_HOTREMOVE block and check it here like below? This way,
>one level of indentation can be reduced in the for loop, and code is
>more readable.
>

I think there is a mistake.

The logical is:
if (#ifdef CONFIG_MEMORY_HOTREMOVE) && (num_immovable_mem > 0)
	then A;
else
	then B;

But below is:
if (num_immovable_mem > 0)
	then B;
else if (#ifdef CONFIG_MEMORY_HOTREMOVE)
	then A;
else
	nothing;

The precondition of the loop is (num_immovable_mem > 0), because
there is only one condition that we need go the A code:
CONFIG_MEMORY_HOTREMOVE is defined, and memory information in srat
found.

But there is many conditions we go the B code:
1. CONFIG_MEMORY_HOTREMOVE is not defined.
2. CONFIG_MEMORY_HOTREMOVE defined, but we didn't get the right acpi tables
3. CONFIG_MEMORY_HOTREMOVE defined, or there is only one node in this machine.

Yes, the code is hard to read, but you have changed the logical, there
is a compromise method, I don't know whether is better:

#ifdef CONFIG_MEMORY_HOTREMOVE
	if (num_immovable_mem == 0)
		goto B;

	for (i = 0; i < num_immovable_mem; i++) {
		...
	}
#endif

B:
	slots_count(region, minimum, image_size);
	
	if (slot_area_index == MAX_SLOT_AREA) {
		debug_putstr("Aborted e820/efi memmap scan (slot_areas full)!\n");
		return 1;
	}
	return 0;
	

>
>static bool process_mem_region(struct mem_vector *region,
>			       unsigned long long minimum,
>			       unsigned long long image_size)
>{
>
>	/*
>	 * If no immovable memory found, or MEMORY_HOTREMOVE disabled,
>	 * walk all the regions, so use region directely.
>	 */
>	if (num_immovable_mem > 0) {
>		slots_count(region, minimum, image_size);
>		
>		if (slot_area_index == MAX_SLOT_AREA) {
>			debug_putstr("Aborted e820/efi memmap scan (slot_areas full)!\n");
>			return 1;
>		}
>		return 0;
>	}
>
>#ifdef CONFIG_MEMORY_HOTREMOVE
>	for (i = 0; i < num_immovable_mem; i++) {
>		...
>	}
>#endif
>}
>

Re: [PATCH v5 4/4] x86/boot/KASLR: Limit kaslr to choosing the immovable memory

[Chao Fan]

On Mon, Aug 27, 2018 at 02:28:54PM +0800, Chao Fan wrote:
>On Mon, Aug 27, 2018 at 01:56:07PM +0800, Baoquan He wrote:
>>On 08/07/18 at 02:50pm, Chao Fan wrote:
[...]
>>
>>Is it possible to take num_immovable_mem definition out from #ifdef
>>CONFIG_MEMORY_HOTREMOVE block and check it here like below? This way,
>>one level of indentation can be reduced in the for loop, and code is
>>more readable.
>>
>
>I think there is a mistake.
>
>The logical is:
>if (#ifdef CONFIG_MEMORY_HOTREMOVE) && (num_immovable_mem > 0)
>	then A;
>else
>	then B;
>
>But below is:

Sorry for that, here the 'below' means the code in your mail.

Thanks,
Chao Fan

>if (num_immovable_mem > 0)
>	then B;
>else if (#ifdef CONFIG_MEMORY_HOTREMOVE)
>	then A;
>else
>	nothing;
>
>The precondition of the loop is (num_immovable_mem > 0), because
>there is only one condition that we need go the A code:
>CONFIG_MEMORY_HOTREMOVE is defined, and memory information in srat
>found.
>
>But there is many conditions we go the B code:
>1. CONFIG_MEMORY_HOTREMOVE is not defined.
>2. CONFIG_MEMORY_HOTREMOVE defined, but we didn't get the right acpi tables
>3. CONFIG_MEMORY_HOTREMOVE defined, or there is only one node in this machine.
>
>Yes, the code is hard to read, but you have changed the logical, there
>is a compromise method, I don't know whether is better:
>
>#ifdef CONFIG_MEMORY_HOTREMOVE
>	if (num_immovable_mem == 0)
>		goto B;
>
>	for (i = 0; i < num_immovable_mem; i++) {
>		...
>	}
>#endif
>
>B:
>	slots_count(region, minimum, image_size);
>	
>	if (slot_area_index == MAX_SLOT_AREA) {
>		debug_putstr("Aborted e820/efi memmap scan (slot_areas full)!\n");
>		return 1;
>	}
>	return 0;
>	
>
>>
>>static bool process_mem_region(struct mem_vector *region,
>>			       unsigned long long minimum,
>>			       unsigned long long image_size)
>>{
>>
>>	/*
>>	 * If no immovable memory found, or MEMORY_HOTREMOVE disabled,
>>	 * walk all the regions, so use region directely.
>>	 */
>>	if (num_immovable_mem > 0) {
>>		slots_count(region, minimum, image_size);
>>		
>>		if (slot_area_index == MAX_SLOT_AREA) {
>>			debug_putstr("Aborted e820/efi memmap scan (slot_areas full)!\n");
>>			return 1;
>>		}
>>		return 0;
>>	}
>>
>>#ifdef CONFIG_MEMORY_HOTREMOVE
>>	for (i = 0; i < num_immovable_mem; i++) {
>>		...
>>	}
>>#endif
>>}
>>

Re: [PATCH v5 4/4] x86/boot/KASLR: Limit kaslr to choosing the immovable memory

[Baoquan He]

On 08/27/18 at 02:28pm, Chao Fan wrote:
> >Is it possible to take num_immovable_mem definition out from #ifdef
> >CONFIG_MEMORY_HOTREMOVE block and check it here like below? This way,
> >one level of indentation can be reduced in the for loop, and code is
> >more readable.
> >
> 
> I think there is a mistake.
> 
> The logical is:
> if (#ifdef CONFIG_MEMORY_HOTREMOVE) && (num_immovable_mem > 0)
> 	then A;
> else
> 	then B;
> 
> But below is:
> if (num_immovable_mem > 0)
> 	then B;
> else if (#ifdef CONFIG_MEMORY_HOTREMOVE)
> 	then A;
> else
> 	nothing;
> 
> The precondition of the loop is (num_immovable_mem > 0), because
> there is only one condition that we need go the A code:
> CONFIG_MEMORY_HOTREMOVE is defined, and memory information in srat
> found.


Yes, we are saying the same thing. if num_immovable_mem == 0, it covers
all the cases you listed at below. Here I assume you have taken
num_immovable_mem definition out.

#ifdef CONFIG_MEMORY_HOTREMOVE
/* Store the immovable memory regions */
static struct mem_vector immovable_mem[MAX_NUMNODES*2];
#endif

/* Store the amount of immovable memory regions */
static int num_immovable_mem;

> 
> But there is many conditions we go the B code:
> 1. CONFIG_MEMORY_HOTREMOVE is not defined.
> 2. CONFIG_MEMORY_HOTREMOVE defined, but we didn't get the right acpi tables
> 3. CONFIG_MEMORY_HOTREMOVE defined, or there is only one node in this machine.
> 
> Yes, the code is hard to read, but you have changed the logical, there
> is a compromise method, I don't know whether is better:
> 
> #ifdef CONFIG_MEMORY_HOTREMOVE
> 	if (num_immovable_mem == 0)
> 		goto B;
> 
> 	for (i = 0; i < num_immovable_mem; i++) {
> 		...
> 	}
> #endif
> 
> B:
> 	slots_count(region, minimum, image_size);
> 	
> 	if (slot_area_index == MAX_SLOT_AREA) {
> 		debug_putstr("Aborted e820/efi memmap scan (slot_areas full)!\n");
> 		return 1;
> 	}
> 	return 0;
> 	
> 
> >
> >static bool process_mem_region(struct mem_vector *region,
> >			       unsigned long long minimum,
> >			       unsigned long long image_size)
> >{
> >
> >	/*
> >	 * If no immovable memory found, or MEMORY_HOTREMOVE disabled,
> >	 * walk all the regions, so use region directely.
> >	 */
> >	if (num_immovable_mem > 0) {
> >		slots_count(region, minimum, image_size);
> >		
> >		if (slot_area_index == MAX_SLOT_AREA) {
> >			debug_putstr("Aborted e820/efi memmap scan (slot_areas full)!\n");
> >			return 1;
> >		}
> >		return 0;
> >	}
> >
> >#ifdef CONFIG_MEMORY_HOTREMOVE
> >	for (i = 0; i < num_immovable_mem; i++) {
> >		...
> >	}
> >#endif
> >}
> >
> 
> 

Re: [PATCH v5 4/4] x86/boot/KASLR: Limit kaslr to choosing the immovable memory

[Baoquan He]

On 08/27/18 at 01:56pm, Baoquan He wrote:
> On 08/07/18 at 02:50pm, Chao Fan wrote:
> > If 'CONFIG_MEMORY_HOTREMOVE' specified and the account of immovable
> If CONFIG_MEMORY_HOTREMOVE is enabled, 
> > memory regions is not zero. Calculate the intersection between memory
> > regions from e820/efi memory table and immovable memory regions.
> > Or go on the old code.
> > 
> > Rename process_mem_region to slots_count to match slots_fetch_random,
> > and name new function as process_mem_region.
> > 
> > Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com>
> > ---
> >  arch/x86/boot/compressed/kaslr.c | 66 ++++++++++++++++++++++++++------
> >  1 file changed, 55 insertions(+), 11 deletions(-)
> > 
> > diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
> > index 720878f967a3..9c6e24a23a2d 100644
> > --- a/arch/x86/boot/compressed/kaslr.c
> > +++ b/arch/x86/boot/compressed/kaslr.c
> > @@ -635,9 +635,9 @@ static unsigned long slots_fetch_random(void)
> >  	return 0;
> >  }
> >  
> > -static void process_mem_region(struct mem_vector *entry,
> > -			       unsigned long minimum,
> > -			       unsigned long image_size)
> > +static void slots_count(struct mem_vector *entry,
> > +			unsigned long minimum,
> > +			unsigned long image_size)
> >  {
> >  	struct mem_vector region, overlap;
> >  	struct slot_area slot_area;
> > @@ -714,6 +714,56 @@ static void process_mem_region(struct mem_vector *entry,
> >  	}
> >  }
> >  
> > +static bool process_mem_region(struct mem_vector *region,
> > +			       unsigned long long minimum,
> > +			       unsigned long long image_size)
> > +{
> 
> Is it possible to take num_immovable_mem definition out from #ifdef
> CONFIG_MEMORY_HOTREMOVE block and check it here like below? This way,
> one level of indentation can be reduced in the for loop, and code is
> more readable.
> 
> 
> static bool process_mem_region(struct mem_vector *region,
> 			       unsigned long long minimum,
> 			       unsigned long long image_size)
> {
> 
> 	/*
> 	 * If no immovable memory found, or MEMORY_HOTREMOVE disabled,
> 	 * walk all the regions, so use region directely.
> 	 */
> 	if (num_immovable_mem > 0) {
Sorry, I meant if (!num_immovable_mem) {


> 		slots_count(region, minimum, image_size);
> 		
> 		if (slot_area_index == MAX_SLOT_AREA) {
> 			debug_putstr("Aborted e820/efi memmap scan (slot_areas full)!\n");
> 			return 1;
> 		}
> 		return 0;
> 	}
> 
> #ifdef CONFIG_MEMORY_HOTREMOVE
> 	for (i = 0; i < num_immovable_mem; i++) {
> 		...
> 	}
> #endif
> }
> 
> > +#ifdef CONFIG_MEMORY_HOTREMOVE
> > +	/*
> > +	 * If immovable memory found, filter the intersection between
> > +	 * immovable memory and region to slots_count.
> > +	 * Otherwise, go on old code.
> > +	 */
> > +	if (num_immovable_mem > 0) {
> > +		int i;
> > +
> > +		for (i = 0; i < num_immovable_mem; i++) {
> > +			struct mem_vector entry;
> > +			unsigned long long start, end, entry_end, region_end;
> > +
> > +			start = immovable_mem[i].start;
> > +			end = start + immovable_mem[i].size;
> > +			region_end = region->start + region->size;
> > +
> > +			entry.start = clamp(region->start, start, end);
> > +			entry_end = clamp(region_end, start, end);
> > +
> > +			if (entry.start + image_size < entry_end) {
> > +				entry.size = entry_end - entry.start;
> > +				slots_count(&entry, minimum, image_size);
> > +
> > +				if (slot_area_index == MAX_SLOT_AREA) {
> > +					debug_putstr("Aborted e820/efi memmap scan (slot_areas full)!\n");
> > +					return 1;
> > +				}
> > +			}
> > +		}
> > +		return 0;
> > +	}
> > +#endif
> > +	/*
> > +	 * If no immovable memory found, or MEMORY_HOTREMOVE disabled,
> > +	 * walk all the regions, so use region directely.
> > +	 */
> > +	slots_count(region, minimum, image_size);
> > +
> > +	if (slot_area_index == MAX_SLOT_AREA) {
> > +		debug_putstr("Aborted e820/efi memmap scan (slot_areas full)!\n");
> > +		return 1;
> > +	}
> > +	return 0;
> > +}
> > +
> >  #ifdef CONFIG_EFI
> >  /*
> >   * Returns true if mirror region found (and must have been processed
> > @@ -779,11 +829,8 @@ process_efi_entries(unsigned long minimum, unsigned long image_size)
> >  
> >  		region.start = md->phys_addr;
> >  		region.size = md->num_pages << EFI_PAGE_SHIFT;
> > -		process_mem_region(&region, minimum, image_size);
> > -		if (slot_area_index == MAX_SLOT_AREA) {
> > -			debug_putstr("Aborted EFI scan (slot_areas full)!\n");
> > +		if (process_mem_region(&region, minimum, image_size))
> >  			break;
> > -		}
> >  	}
> >  	return true;
> >  }
> > @@ -810,11 +857,8 @@ static void process_e820_entries(unsigned long minimum,
> >  			continue;
> >  		region.start = entry->addr;
> >  		region.size = entry->size;
> > -		process_mem_region(&region, minimum, image_size);
> > -		if (slot_area_index == MAX_SLOT_AREA) {
> > -			debug_putstr("Aborted e820 scan (slot_areas full)!\n");
> > +		if (process_mem_region(&region, minimum, image_size))
> >  			break;
> > -		}
> >  	}
> >  }
> >  
> > -- 
> > 2.17.1
> > 
> > 
> > 

Re: [PATCH v5 2/4] x86/boot: Add acpitb.c to parse acpi tables

[Baoquan He]

Hi Chao,

On 08/07/18 at 02:49pm, Chao Fan wrote:

I think better add acpi/efi maintainers to CC when you post.


> +static acpi_physical_address get_rsdp_addr(void)
> +{
> +	acpi_physical_address pa = 0;
> +	bool status = false;


Here, I think you should make get_rsdp_addr() consistent with its
counterpart acpi_os_get_root_pointer(). In acpi_os_get_root_pointer(),
we take care of the case that acpi_rsdp passed in a value.
> +
> +	status = efi_get_rsdp_addr(&pa);
> +
> +	if (!status || pa == 0)
> +		bios_get_rsdp_addr(&pa);
> +
> +	return pa;
> +}
> +
> +struct acpi_table_header *get_acpi_srat_table(void)
> +{
> +	char *args = (char *)get_cmd_line_ptr();
> +	acpi_physical_address acpi_table;
> +	acpi_physical_address root_table;
> +	struct acpi_table_header *header;
> +	struct acpi_table_rsdp *rsdp;
> +	char *signature;
> +	u8 *entry;
> +	u32 count;
> +	u32 size;
> +	int i, j;
> +	u32 len;
> +
> +	rsdp = (struct acpi_table_rsdp *)get_rsdp_addr();
> +	if (!rsdp)
> +		return NULL;
> +
> +	/* Get rsdt or xsdt from rsdp. */
> +	if (!strstr(args, "acpi=rsdt") &&
> +	    rsdp->xsdt_physical_address && rsdp->revision > 1) {
> +		root_table = rsdp->xsdt_physical_address;
> +		size = ACPI_XSDT_ENTRY_SIZE;
> +	} else {
> +		root_table = rsdp->rsdt_physical_address;
> +		size = ACPI_RSDT_ENTRY_SIZE;
> +	}
> +
> +	/* Get ACPI root table from rsdt or xsdt.*/
> +	header = (struct acpi_table_header *)root_table;
> +	len = header->length;
> +	count = (u32)((len - sizeof(struct acpi_table_header)) / size);
> +	entry = ACPI_ADD_PTR(u8, header, sizeof(struct acpi_table_header));
> +
> +	for (i = 0; i < count; i++) {
> +		u64 address64;
> +
> +		if (size == ACPI_RSDT_ENTRY_SIZE)
> +			acpi_table = ((acpi_physical_address)
> +				      (*ACPI_CAST_PTR(u32, entry)));
> +		else {
> +			*(u64 *)(void *)&address64 = *(u64 *)(void *)entry;
> +			acpi_table = (acpi_physical_address) address64;
> +		}
> +
> +		if (acpi_table) {
> +			header = (struct acpi_table_header *)acpi_table;
> +			signature = header->signature;
> +
> +			if (!strncmp(signature, "SRAT", 4))
> +				return header;
> +		}
> +		entry += size;
> +	}
> +	return NULL;
> +}
> -- 
> 2.17.1
> 
> 
>