linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v6 0/3] x86/boot/KASLR: Parse ACPI table and limit kaslr in immovable memory.
@ 2018-09-10 12:40 Chao Fan
  2018-09-10 12:40 ` [PATCH v6 1/3] x86/boot: Add acpitb.c to parse acpi tables Chao Fan
                   ` (3 more replies)
  0 siblings, 4 replies; 15+ messages in thread
From: Chao Fan @ 2018-09-10 12:40 UTC (permalink / raw)
  To: tglx, mingo, hpa, x86, linux-kernel, keescook, bhe
  Cc: indou.takao, caoj.fnst, douly.fnst, 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 movable memory chosen by KASLR 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 hot-remove, 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 new 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.
I think ACPI code is an independent part, so copy the codes
and functions to 'compressed/' directory, so that kaslr won't
influence the initialization of ACPI.

PATCH 1/3 Add acpitb.c to provide functions to parse ACPI code.
PATCH 2/3 If CONFIG_MEMORY_HOTREMOVE enabled, walk all nodes and
          store the information of immovable memory regions.
PATCH 3/3 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

v5->v6:
Follow Baoquan He's suggestion:
 - Change some log.
 - Add the check for acpi_rsdp
 - Change some code logical to make code clear

Any comments will be welcome.


Chao Fan (3):
  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 | 382 ++++++++++++++++++++++++++++++
 arch/x86/boot/compressed/kaslr.c  |  76 +++++-
 arch/x86/boot/compressed/misc.h   |   8 +
 4 files changed, 459 insertions(+), 11 deletions(-)
 create mode 100644 arch/x86/boot/compressed/acpitb.c

-- 
2.17.1




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

* [PATCH v6 1/3] x86/boot: Add acpitb.c to parse acpi tables
  2018-09-10 12:40 [PATCH v6 0/3] x86/boot/KASLR: Parse ACPI table and limit kaslr in immovable memory Chao Fan
@ 2018-09-10 12:40 ` Chao Fan
  2018-09-10 20:13   ` Rafael J. Wysocki
                     ` (3 more replies)
  2018-09-10 12:40 ` [PATCH v6 2/3] x86/boot/KASLR: Walk srat tables to filter immovable memory Chao Fan
                   ` (2 subsequent siblings)
  3 siblings, 4 replies; 15+ messages in thread
From: Chao Fan @ 2018-09-10 12:40 UTC (permalink / raw)
  To: tglx, mingo, hpa, x86, linux-kernel, keescook, bhe
  Cc: indou.takao, caoj.fnst, douly.fnst, Chao Fan, Rafael J. Wysocki,
	Len Brown, Ard Biesheuvel, linux-efi, linux-acpi

Imitate the ACPI code of parsing ACPI tables to dig and read ACPI
tables. Since some operations are not needed here, functions are
simplified. Functions will be used to dig SRAT tables to get
information of memory, so that KASLR can the memory in immovable node.

And also, these functions won't influence the initialization of
ACPI after start_kernel().

Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Len Brown <lenb@kernel.org>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: linux-efi@vger.kernel.org
Cc: linux-acpi@vger.kernel.org
---
 arch/x86/boot/compressed/Makefile |   4 +
 arch/x86/boot/compressed/acpitb.c | 382 ++++++++++++++++++++++++++++++
 arch/x86/boot/compressed/misc.h   |   8 +
 3 files changed, 394 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 28764dacf018..394d7d93da7a 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..66c515401076
--- /dev/null
+++ b/arch/x86/boot/compressed/acpitb.c
@@ -0,0 +1,382 @@
+// SPDX-License-Identifier: GPL-2.0
+#define BOOT_CTYPE_H
+#include "misc.h"
+
+#include <linux/efi.h>
+#include <asm/efi.h>
+#include <linux/numa.h>
+#include <linux/acpi.h>
+
+extern unsigned long get_cmd_line_ptr(void);
+
+#define STATIC
+#include <linux/decompress/mm.h>
+
+#ifdef CONFIG_MEMORY_HOTREMOVE
+struct mem_vector {
+	unsigned long long start;
+	unsigned long long size;
+};
+/* Store the immovable memory regions */
+struct mem_vector immovable_mem[MAX_NUMNODES*2];
+#endif
+
+#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;
+	}
+}
+
+#ifdef CONFIG_KEXEC
+static bool get_acpi_rsdp(acpi_physical_address *rsdp_addr)
+{
+	char *args = (char *)get_cmd_line_ptr();
+	size_t len = strlen((char *)args);
+	char *tmp_cmdline, *param, *val;
+	unsigned long long addr = 0;
+	char *endptr;
+
+	if (!strstr(args, "acpi_rsdp="))
+		return false;
+
+	tmp_cmdline = malloc(len+1);
+	if (!tmp_cmdline)
+		error("Failed to allocate space for tmp_cmdline");
+
+	memcpy(tmp_cmdline, args, len);
+	tmp_cmdline[len] = 0;
+	args = tmp_cmdline;
+
+	args = skip_spaces(args);
+
+	while (*args) {
+		args = next_arg(args, &param, &val);
+
+		if (!val && strcmp(param, "--") == 0) {
+			warn("Only '--' specified in cmdline");
+			free(tmp_cmdline);
+			return false;
+		}
+
+		if (!strcmp(param, "acpi_rsdp")) {
+			addr = simple_strtoull(val, &endptr, 0);
+
+			if (addr == 0)
+				return false;
+
+			*rsdp_addr = (acpi_physical_address)addr;
+			return true;
+		}
+	}
+	return false;
+}
+#else
+static bool get_acpi_rsdp(acpi_physical_address *rsdp_addr)
+{
+	return false;
+}
+#endif
+
+/*
+ * 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 = get_acpi_rsdp(&pa);
+
+	if (!status || pa == 0)
+		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;
+}
+
+#ifdef CONFIG_MEMORY_HOTREMOVE
+/*
+ * According to ACPI table, filter the immvoable memory regions
+ * and store them in immovable_mem[].
+ */
+void get_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") || strstr(args, "acpi=off"))
+		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
diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
index a1d5918765f3..bb395e5f88e3 100644
--- a/arch/x86/boot/compressed/misc.h
+++ b/arch/x86/boot/compressed/misc.h
@@ -116,3 +116,11 @@ static inline void console_init(void)
 void set_sev_encryption_mask(void);
 
 #endif
+
+/* acpitb.c */
+/* Store the amount of immovable memory regions */
+int num_immovable_mem;
+#ifdef CONFIG_MEMORY_HOTREMOVE
+#define ACPI_MAX_TABLES                128
+void get_immovable_mem(void);
+#endif
-- 
2.17.1




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

* [PATCH v6 2/3] x86/boot/KASLR: Walk srat tables to filter immovable memory
  2018-09-10 12:40 [PATCH v6 0/3] x86/boot/KASLR: Parse ACPI table and limit kaslr in immovable memory Chao Fan
  2018-09-10 12:40 ` [PATCH v6 1/3] x86/boot: Add acpitb.c to parse acpi tables Chao Fan
@ 2018-09-10 12:40 ` Chao Fan
  2018-09-10 12:40 ` [PATCH v6 3/3] x86/boot/KASLR: Limit kaslr to choosing the " Chao Fan
  2018-09-11  1:13 ` [PATCH v6 0/3] x86/boot/KASLR: Parse ACPI table and limit kaslr in " Chao Fan
  3 siblings, 0 replies; 15+ messages in thread
From: Chao Fan @ 2018-09-10 12:40 UTC (permalink / raw)
  To: tglx, mingo, hpa, x86, linux-kernel, keescook, bhe
  Cc: indou.takao, caoj.fnst, douly.fnst, Chao Fan

If CONFIG_MEMORY_HOTREMOVE enabled, 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 | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index d1e19f358b6e..3d839040cc7f 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -101,6 +101,11 @@ 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 */
+extern struct mem_vector immovable_mem[MAX_NUMNODES*2];
+#endif
+
 
 enum mem_avoid_index {
 	MEM_AVOID_ZO_RANGE = 0,
@@ -418,6 +423,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 */
+	get_immovable_mem();
+#endif
+
 #ifdef CONFIG_X86_VERBOSE_BOOTUP
 	/* Make sure video RAM can be used. */
 	add_identity_map(0, PMD_SIZE);
-- 
2.17.1




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

* [PATCH v6 3/3] x86/boot/KASLR: Limit kaslr to choosing the immovable memory
  2018-09-10 12:40 [PATCH v6 0/3] x86/boot/KASLR: Parse ACPI table and limit kaslr in immovable memory Chao Fan
  2018-09-10 12:40 ` [PATCH v6 1/3] x86/boot: Add acpitb.c to parse acpi tables Chao Fan
  2018-09-10 12:40 ` [PATCH v6 2/3] x86/boot/KASLR: Walk srat tables to filter immovable memory Chao Fan
@ 2018-09-10 12:40 ` Chao Fan
  2018-09-11  1:13 ` [PATCH v6 0/3] x86/boot/KASLR: Parse ACPI table and limit kaslr in " Chao Fan
  3 siblings, 0 replies; 15+ messages in thread
From: Chao Fan @ 2018-09-10 12:40 UTC (permalink / raw)
  To: tglx, mingo, hpa, x86, linux-kernel, keescook, bhe
  Cc: indou.takao, caoj.fnst, douly.fnst, Chao Fan

If CONFIG_MEMORY_HOTREMOVE enabled and the amount of immovable
memory regions is not zero. Calculate the intersection between memory
regions from e820/efi memory table and immovable memory regions.

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 3d839040cc7f..3b0b305c049d 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -583,9 +583,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;
@@ -662,6 +662,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)
+{
+	int i;
+	/*
+	 * 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
+	/*
+	 * If immovable memory found, filter the intersection between
+	 * immovable memory and region to slots_count.
+	 * Otherwise, go on old code.
+	 */
+	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
+}
+
 #ifdef CONFIG_EFI
 /*
  * Returns true if mirror region found (and must have been processed
@@ -727,11 +777,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;
 }
@@ -758,11 +805,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




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

* Re: [PATCH v6 1/3] x86/boot: Add acpitb.c to parse acpi tables
  2018-09-10 12:40 ` [PATCH v6 1/3] x86/boot: Add acpitb.c to parse acpi tables Chao Fan
@ 2018-09-10 20:13   ` Rafael J. Wysocki
  2018-09-11  1:27     ` Chao Fan
  2018-09-11  8:52   ` [RFC PATCH] x86/boot: get_acpi_srat_table() can be static kbuild test robot
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 15+ messages in thread
From: Rafael J. Wysocki @ 2018-09-10 20:13 UTC (permalink / raw)
  To: fanc.fnst
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	the arch/x86 maintainers, Linux Kernel Mailing List, Kees Cook,
	bhe, indou.takao, Cao jin, Dou Liyang, Rafael J. Wysocki,
	Len Brown, Ard Biesheuvel, linux-efi, ACPI Devel Maling List

On Mon, Sep 10, 2018 at 2:41 PM Chao Fan <fanc.fnst@cn.fujitsu.com> wrote:
>
> Imitate the ACPI code of parsing ACPI tables to dig and read ACPI
> tables.

Why?

> Since some operations are not needed here,

What operations?

> functions are simplified. Functions will be used to dig SRAT tables to get
> information of memory, so that KASLR can the memory in immovable node.

So do you want to parse SRAT only or do something in addition to that?

> And also, these functions won't influence the initialization of
> ACPI after start_kernel().

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

* Re: [PATCH v6 0/3] x86/boot/KASLR: Parse ACPI table and limit kaslr in immovable memory.
  2018-09-10 12:40 [PATCH v6 0/3] x86/boot/KASLR: Parse ACPI table and limit kaslr in immovable memory Chao Fan
                   ` (2 preceding siblings ...)
  2018-09-10 12:40 ` [PATCH v6 3/3] x86/boot/KASLR: Limit kaslr to choosing the " Chao Fan
@ 2018-09-11  1:13 ` Chao Fan
  2018-09-11  7:28   ` Rafael J. Wysocki
  3 siblings, 1 reply; 15+ messages in thread
From: Chao Fan @ 2018-09-11  1:13 UTC (permalink / raw)
  To: tglx, mingo, hpa, x86, linux-kernel, keescook, bhe, rafael
  Cc: indou.takao, caoj.fnst, douly.fnst

sorry, the cover letter forwand to rafael.

Thanks,
Chao Fan

On Mon, Sep 10, 2018 at 08:40:12PM +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 movable memory chosen by KASLR 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 hot-remove, 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 new 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.
>I think ACPI code is an independent part, so copy the codes
>and functions to 'compressed/' directory, so that kaslr won't
>influence the initialization of ACPI.
>
>PATCH 1/3 Add acpitb.c to provide functions to parse ACPI code.
>PATCH 2/3 If CONFIG_MEMORY_HOTREMOVE enabled, walk all nodes and
>          store the information of immovable memory regions.
>PATCH 3/3 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
>
>v5->v6:
>Follow Baoquan He's suggestion:
> - Change some log.
> - Add the check for acpi_rsdp
> - Change some code logical to make code clear
>
>Any comments will be welcome.
>
>
>Chao Fan (3):
>  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 | 382 ++++++++++++++++++++++++++++++
> arch/x86/boot/compressed/kaslr.c  |  76 +++++-
> arch/x86/boot/compressed/misc.h   |   8 +
> 4 files changed, 459 insertions(+), 11 deletions(-)
> create mode 100644 arch/x86/boot/compressed/acpitb.c
>
>-- 
>2.17.1
>



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

* Re: [PATCH v6 1/3] x86/boot: Add acpitb.c to parse acpi tables
  2018-09-10 20:13   ` Rafael J. Wysocki
@ 2018-09-11  1:27     ` Chao Fan
  2018-09-11  7:41       ` Rafael J. Wysocki
  0 siblings, 1 reply; 15+ messages in thread
From: Chao Fan @ 2018-09-11  1:27 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	the arch/x86 maintainers, Linux Kernel Mailing List, Kees Cook,
	bhe, indou.takao, Cao jin, Dou Liyang, Rafael J. Wysocki,
	Len Brown, Ard Biesheuvel, linux-efi, ACPI Devel Maling List

On Mon, Sep 10, 2018 at 10:13:49PM +0200, Rafael J. Wysocki wrote:
>On Mon, Sep 10, 2018 at 2:41 PM Chao Fan <fanc.fnst@cn.fujitsu.com> wrote:
>>
>> Imitate the ACPI code of parsing ACPI tables to dig and read ACPI
>> tables.
>
>Why?

Sorry for that, I have sent the cover letter to you.

>
>> Since some operations are not needed here,
>
>What operations?

Such as some operations related to the map between physical address
and virtual address. Here in compressed period, use physical address
directely.

>
>> functions are simplified. Functions will be used to dig SRAT tables to get
>> information of memory, so that KASLR can the memory in immovable node.
>
>So do you want to parse SRAT only or do something in addition to that?

Only read and get memory information, won't change SRAT.
So ACPI code will not be changed.

Thanks,
Chao Fan

>
>> And also, these functions won't influence the initialization of
>> ACPI after start_kernel().
>
>



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

* Re: [PATCH v6 0/3] x86/boot/KASLR: Parse ACPI table and limit kaslr in immovable memory.
  2018-09-11  1:13 ` [PATCH v6 0/3] x86/boot/KASLR: Parse ACPI table and limit kaslr in " Chao Fan
@ 2018-09-11  7:28   ` Rafael J. Wysocki
  2018-09-11  7:53     ` Chao Fan
  0 siblings, 1 reply; 15+ messages in thread
From: Rafael J. Wysocki @ 2018-09-11  7:28 UTC (permalink / raw)
  To: fanc.fnst
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	the arch/x86 maintainers, Linux Kernel Mailing List, Kees Cook,
	bhe, Rafael J. Wysocki, indou.takao, Cao jin, Dou Liyang

On Tue, Sep 11, 2018 at 3:13 AM Chao Fan <fanc.fnst@cn.fujitsu.com> wrote:
>
> sorry, the cover letter forwand to rafael.

Thanks, but the relevant information still needs to go to the
changelog of the patch adding the extra ACPI tables parsing code.
Your cover letter will not be there in the git log.

I also would suggest to CC the entire series to linux-acpi along with
the cover letter.

Thanks,
Rafael

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

* Re: [PATCH v6 1/3] x86/boot: Add acpitb.c to parse acpi tables
  2018-09-11  1:27     ` Chao Fan
@ 2018-09-11  7:41       ` Rafael J. Wysocki
  2018-09-11  7:52         ` Chao Fan
  0 siblings, 1 reply; 15+ messages in thread
From: Rafael J. Wysocki @ 2018-09-11  7:41 UTC (permalink / raw)
  To: fanc.fnst
  Cc: Rafael J. Wysocki, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	the arch/x86 maintainers, Linux Kernel Mailing List, Kees Cook,
	bhe, indou.takao, Cao jin, Dou Liyang, Rafael J. Wysocki,
	Len Brown, Ard Biesheuvel, linux-efi, ACPI Devel Maling List

On Tue, Sep 11, 2018 at 3:28 AM Chao Fan <fanc.fnst@cn.fujitsu.com> wrote:
>
> On Mon, Sep 10, 2018 at 10:13:49PM +0200, Rafael J. Wysocki wrote:
> >On Mon, Sep 10, 2018 at 2:41 PM Chao Fan <fanc.fnst@cn.fujitsu.com> wrote:
> >>
> >> Imitate the ACPI code of parsing ACPI tables to dig and read ACPI
> >> tables.
> >
> >Why?
>
> Sorry for that, I have sent the cover letter to you.

Thanks, but please add the explanation here too.

> >
> >> Since some operations are not needed here,
> >
> >What operations?
>
> Such as some operations related to the map between physical address
> and virtual address. Here in compressed period, use physical address
> directely.

This information also needs to go into the changelog of the patch.

> >
> >> functions are simplified. Functions will be used to dig SRAT tables to get
> >> information of memory, so that KASLR can the memory in immovable node.
> >
> >So do you want to parse SRAT only or do something in addition to that?
>
> Only read and get memory information, won't change SRAT.
> So ACPI code will not be changed.

I meant whether or not SRAT is the only table you need to look into.
If so, that also needs to be said in the patch changelog.

Thanks,
Rafael

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

* Re: [PATCH v6 1/3] x86/boot: Add acpitb.c to parse acpi tables
  2018-09-11  7:41       ` Rafael J. Wysocki
@ 2018-09-11  7:52         ` Chao Fan
  0 siblings, 0 replies; 15+ messages in thread
From: Chao Fan @ 2018-09-11  7:52 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	the arch/x86 maintainers, Linux Kernel Mailing List, Kees Cook,
	bhe, indou.takao, Cao jin, Dou Liyang, Rafael J. Wysocki,
	Len Brown, Ard Biesheuvel, linux-efi, ACPI Devel Maling List

On Tue, Sep 11, 2018 at 09:41:36AM +0200, Rafael J. Wysocki wrote:
>On Tue, Sep 11, 2018 at 3:28 AM Chao Fan <fanc.fnst@cn.fujitsu.com> wrote:
>>
>> On Mon, Sep 10, 2018 at 10:13:49PM +0200, Rafael J. Wysocki wrote:
>> >On Mon, Sep 10, 2018 at 2:41 PM Chao Fan <fanc.fnst@cn.fujitsu.com> wrote:
>> >>
>> >> Imitate the ACPI code of parsing ACPI tables to dig and read ACPI
>> >> tables.
>> >
>> >Why?
>>
>> Sorry for that, I have sent the cover letter to you.
>
>Thanks, but please add the explanation here too.

OK, I will add more.

>
>> >
>> >> Since some operations are not needed here,
>> >
>> >What operations?
>>
>> Such as some operations related to the map between physical address
>> and virtual address. Here in compressed period, use physical address
>> directely.
>
>This information also needs to go into the changelog of the patch.

Sorry for that.

>
>> >
>> >> functions are simplified. Functions will be used to dig SRAT tables to get
>> >> information of memory, so that KASLR can the memory in immovable node.
>> >
>> >So do you want to parse SRAT only or do something in addition to that?
>>
>> Only read and get memory information, won't change SRAT.
>> So ACPI code will not be changed.
>
>I meant whether or not SRAT is the only table you need to look into.
>If so, that also needs to be said in the patch changelog.

Yes, you are right, here only SRAT table is needed.

Thanks,
Chao Fan

>
>Thanks,
>Rafael
>
>



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

* Re: [PATCH v6 0/3] x86/boot/KASLR: Parse ACPI table and limit kaslr in immovable memory.
  2018-09-11  7:28   ` Rafael J. Wysocki
@ 2018-09-11  7:53     ` Chao Fan
  0 siblings, 0 replies; 15+ messages in thread
From: Chao Fan @ 2018-09-11  7:53 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	the arch/x86 maintainers, Linux Kernel Mailing List, Kees Cook,
	bhe, indou.takao, Cao jin, Dou Liyang

On Tue, Sep 11, 2018 at 09:28:44AM +0200, Rafael J. Wysocki wrote:
>On Tue, Sep 11, 2018 at 3:13 AM Chao Fan <fanc.fnst@cn.fujitsu.com> wrote:
>>
>> sorry, the cover letter forwand to rafael.
>
>Thanks, but the relevant information still needs to go to the
>changelog of the patch adding the extra ACPI tables parsing code.
>Your cover letter will not be there in the git log.
>
>I also would suggest to CC the entire series to linux-acpi along with
>the cover letter.

OK, I will add more in next version.

Thanks,
Chao Fan

>
>Thanks,
>Rafael
>
>



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

* [RFC PATCH] x86/boot: get_acpi_srat_table() can be static
  2018-09-10 12:40 ` [PATCH v6 1/3] x86/boot: Add acpitb.c to parse acpi tables Chao Fan
  2018-09-10 20:13   ` Rafael J. Wysocki
@ 2018-09-11  8:52   ` kbuild test robot
  2018-09-11  8:55     ` Chao Fan
  2018-09-11  8:52   ` [PATCH v6 1/3] x86/boot: Add acpitb.c to parse acpi tables kbuild test robot
  2018-09-12  7:08   ` kbuild test robot
  3 siblings, 1 reply; 15+ messages in thread
From: kbuild test robot @ 2018-09-11  8:52 UTC (permalink / raw)
  To: Chao Fan
  Cc: kbuild-all, tglx, mingo, hpa, x86, linux-kernel, keescook, bhe,
	indou.takao, caoj.fnst, douly.fnst, Chao Fan, Rafael J. Wysocki,
	Len Brown, Ard Biesheuvel, linux-efi, linux-acpi


Fixes: cc1a4e2a5474 ("x86/boot: Add acpitb.c to parse acpi tables")
Signed-off-by: kbuild test robot <fengguang.wu@intel.com>
---
 acpitb.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/boot/compressed/acpitb.c b/arch/x86/boot/compressed/acpitb.c
index 66c5154..4ddd732 100644
--- a/arch/x86/boot/compressed/acpitb.c
+++ b/arch/x86/boot/compressed/acpitb.c
@@ -279,7 +279,7 @@ static acpi_physical_address get_rsdp_addr(void)
 	return pa;
 }
 
-struct acpi_table_header *get_acpi_srat_table(void)
+static struct acpi_table_header *get_acpi_srat_table(void)
 {
 	char *args = (char *)get_cmd_line_ptr();
 	acpi_physical_address acpi_table;

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

* Re: [PATCH v6 1/3] x86/boot: Add acpitb.c to parse acpi tables
  2018-09-10 12:40 ` [PATCH v6 1/3] x86/boot: Add acpitb.c to parse acpi tables Chao Fan
  2018-09-10 20:13   ` Rafael J. Wysocki
  2018-09-11  8:52   ` [RFC PATCH] x86/boot: get_acpi_srat_table() can be static kbuild test robot
@ 2018-09-11  8:52   ` kbuild test robot
  2018-09-12  7:08   ` kbuild test robot
  3 siblings, 0 replies; 15+ messages in thread
From: kbuild test robot @ 2018-09-11  8:52 UTC (permalink / raw)
  To: Chao Fan
  Cc: kbuild-all, tglx, mingo, hpa, x86, linux-kernel, keescook, bhe,
	indou.takao, caoj.fnst, douly.fnst, Chao Fan, Rafael J. Wysocki,
	Len Brown, Ard Biesheuvel, linux-efi, linux-acpi

[-- Attachment #1: Type: text/plain, Size: 6851 bytes --]

Hi Chao,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on tip/x86/core]
[also build test WARNING on v4.19-rc3 next-20180910]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Chao-Fan/x86-boot-KASLR-Parse-ACPI-table-and-limit-kaslr-in-immovable-memory/20180911-043350
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   arch/x86/boot/compressed/misc.h:121:5: sparse: symbol 'num_immovable_mem' was not declared. Should it be static?
   arch/x86/boot/compressed/acpitb.c:21:19: sparse: symbol 'immovable_mem' was not declared. Should it be static?
   arch/x86/boot/compressed/acpitb.c:225:17: sparse: undefined identifier 'error'
   arch/x86/boot/compressed/acpitb.c:237:25: sparse: undefined identifier 'warn'
   arch/x86/boot/compressed/acpitb.c:282:26: sparse: symbol 'get_acpi_srat_table' was not declared. Should it be static?
   arch/x86/boot/compressed/acpitb.c:225:22: sparse: call with no type!
   arch/x86/boot/compressed/acpitb.c:237:29: sparse: call with no type!
   arch/x86/boot/compressed/acpitb.c: In function 'get_acpi_rsdp':
>> arch/x86/boot/compressed/acpitb.c:225:3: warning: implicit declaration of function 'error' [-Wimplicit-function-declaration]
      error("Failed to allocate space for tmp_cmdline");
      ^~~~~
>> arch/x86/boot/compressed/acpitb.c:237:4: warning: implicit declaration of function 'warn'; did you mean '__warn'? [-Wimplicit-function-declaration]
       warn("Only '--' specified in cmdline");
       ^~~~
       __warn

sparse warnings: (new ones prefixed by >>)

   arch/x86/boot/compressed/misc.h:121:5: sparse: symbol 'num_immovable_mem' was not declared. Should it be static?
   arch/x86/boot/compressed/acpitb.c:225:17: sparse: undefined identifier 'error'
   arch/x86/boot/compressed/acpitb.c:237:25: sparse: undefined identifier 'warn'
>> arch/x86/boot/compressed/acpitb.c:282:26: sparse: symbol 'get_acpi_srat_table' was not declared. Should it be static?
>> arch/x86/boot/compressed/acpitb.c:225:22: sparse: call with no type!
   arch/x86/boot/compressed/acpitb.c:237:29: sparse: call with no type!
   arch/x86/boot/compressed/acpitb.c: In function 'get_acpi_rsdp':
   arch/x86/boot/compressed/acpitb.c:225:3: warning: implicit declaration of function 'error' [-Wimplicit-function-declaration]
      error("Failed to allocate space for tmp_cmdline");
      ^~~~~
   arch/x86/boot/compressed/acpitb.c:237:4: warning: implicit declaration of function 'warn'; did you mean '__warn'? [-Wimplicit-function-declaration]
       warn("Only '--' specified in cmdline");
       ^~~~
       __warn

Please review and possibly fold the followup patch.

vim +/error +225 arch/x86/boot/compressed/acpitb.c

   210	
   211	#ifdef CONFIG_KEXEC
   212	static bool get_acpi_rsdp(acpi_physical_address *rsdp_addr)
   213	{
   214		char *args = (char *)get_cmd_line_ptr();
   215		size_t len = strlen((char *)args);
   216		char *tmp_cmdline, *param, *val;
   217		unsigned long long addr = 0;
   218		char *endptr;
   219	
   220		if (!strstr(args, "acpi_rsdp="))
   221			return false;
   222	
   223		tmp_cmdline = malloc(len+1);
   224		if (!tmp_cmdline)
 > 225			error("Failed to allocate space for tmp_cmdline");
   226	
   227		memcpy(tmp_cmdline, args, len);
   228		tmp_cmdline[len] = 0;
   229		args = tmp_cmdline;
   230	
   231		args = skip_spaces(args);
   232	
   233		while (*args) {
   234			args = next_arg(args, &param, &val);
   235	
   236			if (!val && strcmp(param, "--") == 0) {
 > 237				warn("Only '--' specified in cmdline");
   238				free(tmp_cmdline);
   239				return false;
   240			}
   241	
   242			if (!strcmp(param, "acpi_rsdp")) {
   243				addr = simple_strtoull(val, &endptr, 0);
   244	
   245				if (addr == 0)
   246					return false;
   247	
   248				*rsdp_addr = (acpi_physical_address)addr;
   249				return true;
   250			}
   251		}
   252		return false;
   253	}
   254	#else
   255	static bool get_acpi_rsdp(acpi_physical_address *rsdp_addr)
   256	{
   257		return false;
   258	}
   259	#endif
   260	
   261	/*
   262	 * Used to dig rsdp table from EFI table or BIOS.
   263	 * If rsdp table found in EFI table, use it. Or search BIOS.
   264	 * Based on acpi_os_get_root_pointer().
   265	 */
   266	static acpi_physical_address get_rsdp_addr(void)
   267	{
   268		acpi_physical_address pa = 0;
   269		bool status = false;
   270	
   271		status = get_acpi_rsdp(&pa);
   272	
   273		if (!status || pa == 0)
   274			status = efi_get_rsdp_addr(&pa);
   275	
   276		if (!status || pa == 0)
   277			bios_get_rsdp_addr(&pa);
   278	
   279		return pa;
   280	}
   281	
 > 282	struct acpi_table_header *get_acpi_srat_table(void)
   283	{
   284		char *args = (char *)get_cmd_line_ptr();
   285		acpi_physical_address acpi_table;
   286		acpi_physical_address root_table;
   287		struct acpi_table_header *header;
   288		struct acpi_table_rsdp *rsdp;
   289		char *signature;
   290		u8 *entry;
   291		u32 count;
   292		u32 size;
   293		int i, j;
   294		u32 len;
   295	
   296		rsdp = (struct acpi_table_rsdp *)get_rsdp_addr();
   297		if (!rsdp)
   298			return NULL;
   299	
   300		/* Get rsdt or xsdt from rsdp. */
   301		if (!strstr(args, "acpi=rsdt") &&
   302		    rsdp->xsdt_physical_address && rsdp->revision > 1) {
   303			root_table = rsdp->xsdt_physical_address;
   304			size = ACPI_XSDT_ENTRY_SIZE;
   305		} else {
   306			root_table = rsdp->rsdt_physical_address;
   307			size = ACPI_RSDT_ENTRY_SIZE;
   308		}
   309	
   310		/* Get ACPI root table from rsdt or xsdt.*/
   311		header = (struct acpi_table_header *)root_table;
   312		len = header->length;
   313		count = (u32)((len - sizeof(struct acpi_table_header)) / size);
   314		entry = ACPI_ADD_PTR(u8, header, sizeof(struct acpi_table_header));
   315	
   316		for (i = 0; i < count; i++) {
   317			u64 address64;
   318	
   319			if (size == ACPI_RSDT_ENTRY_SIZE)
   320				acpi_table = ((acpi_physical_address)
   321					      (*ACPI_CAST_PTR(u32, entry)));
   322			else {
   323				*(u64 *)(void *)&address64 = *(u64 *)(void *)entry;
   324				acpi_table = (acpi_physical_address) address64;
   325			}
   326	
   327			if (acpi_table) {
   328				header = (struct acpi_table_header *)acpi_table;
   329				signature = header->signature;
   330	
   331				if (!strncmp(signature, "SRAT", 4))
   332					return header;
   333			}
   334			entry += size;
   335		}
   336		return NULL;
   337	}
   338	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 65704 bytes --]

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

* Re: [RFC PATCH] x86/boot: get_acpi_srat_table() can be static
  2018-09-11  8:52   ` [RFC PATCH] x86/boot: get_acpi_srat_table() can be static kbuild test robot
@ 2018-09-11  8:55     ` Chao Fan
  0 siblings, 0 replies; 15+ messages in thread
From: Chao Fan @ 2018-09-11  8:55 UTC (permalink / raw)
  To: kbuild test robot
  Cc: kbuild-all, tglx, mingo, hpa, x86, linux-kernel, keescook, bhe,
	indou.takao, caoj.fnst, douly.fnst, Rafael J. Wysocki, Len Brown,
	Ard Biesheuvel, linux-efi, linux-acpi

On Tue, Sep 11, 2018 at 04:52:26PM +0800, kbuild test robot wrote:
>
>Fixes: cc1a4e2a5474 ("x86/boot: Add acpitb.c to parse acpi tables")
>Signed-off-by: kbuild test robot <fengguang.wu@intel.com>
>---
> acpitb.c |    2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/arch/x86/boot/compressed/acpitb.c b/arch/x86/boot/compressed/acpitb.c
>index 66c5154..4ddd732 100644
>--- a/arch/x86/boot/compressed/acpitb.c
>+++ b/arch/x86/boot/compressed/acpitb.c
>@@ -279,7 +279,7 @@ static acpi_physical_address get_rsdp_addr(void)
> 	return pa;
> }
> 
>-struct acpi_table_header *get_acpi_srat_table(void)
>+static struct acpi_table_header *get_acpi_srat_table(void)

Good catch, will add it.

Thanks,
Chao Fan

> {
> 	char *args = (char *)get_cmd_line_ptr();
> 	acpi_physical_address acpi_table;
>
>



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

* Re: [PATCH v6 1/3] x86/boot: Add acpitb.c to parse acpi tables
  2018-09-10 12:40 ` [PATCH v6 1/3] x86/boot: Add acpitb.c to parse acpi tables Chao Fan
                     ` (2 preceding siblings ...)
  2018-09-11  8:52   ` [PATCH v6 1/3] x86/boot: Add acpitb.c to parse acpi tables kbuild test robot
@ 2018-09-12  7:08   ` kbuild test robot
  3 siblings, 0 replies; 15+ messages in thread
From: kbuild test robot @ 2018-09-12  7:08 UTC (permalink / raw)
  To: Chao Fan
  Cc: kbuild-all, tglx, mingo, hpa, x86, linux-kernel, keescook, bhe,
	indou.takao, caoj.fnst, douly.fnst, Chao Fan, Rafael J. Wysocki,
	Len Brown, Ard Biesheuvel, linux-efi, linux-acpi

[-- Attachment #1: Type: text/plain, Size: 5117 bytes --]

Hi Chao,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on tip/x86/core]
[also build test WARNING on v4.19-rc3 next-20180911]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Chao-Fan/x86-boot-KASLR-Parse-ACPI-table-and-limit-kaslr-in-immovable-memory/20180911-043350
config: i386-randconfig-x0-09121328 (attached as .config)
compiler: gcc-5 (Debian 5.5.0-3) 5.4.1 20171010
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All warnings (new ones prefixed by >>):

   arch/x86/boot/compressed/acpitb.c: In function 'bios_get_rsdp_addr':
>> arch/x86/boot/compressed/acpitb.c:181:14: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
     table_ptr = (u8 *)(acpi_physical_address)address;
                 ^
   arch/x86/boot/compressed/acpitb.c: In function 'get_acpi_rsdp':
   arch/x86/boot/compressed/acpitb.c:225:3: warning: implicit declaration of function 'error' [-Wimplicit-function-declaration]
      error("Failed to allocate space for tmp_cmdline");
      ^
>> arch/x86/boot/compressed/acpitb.c:237:4: warning: implicit declaration of function 'warn' [-Wimplicit-function-declaration]
       warn("Only '--' specified in cmdline");
       ^
   arch/x86/boot/compressed/acpitb.c: In function 'get_acpi_srat_table':
   arch/x86/boot/compressed/acpitb.c:296:9: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
     rsdp = (struct acpi_table_rsdp *)get_rsdp_addr();
            ^
   arch/x86/boot/compressed/acpitb.c:311:11: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
     header = (struct acpi_table_header *)root_table;
              ^
   arch/x86/boot/compressed/acpitb.c:328:13: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
       header = (struct acpi_table_header *)acpi_table;
                ^

vim +181 arch/x86/boot/compressed/acpitb.c

   164	
   165	/*
   166	 * Used to search RSDP physical address.
   167	 * Based on acpi_find_root_pointer(). Since only use physical address
   168	 * in this period, so there is no need to do the memory map jobs.
   169	 */
   170	static void bios_get_rsdp_addr(acpi_physical_address *rsdp_addr)
   171	{
   172		struct acpi_table_rsdp *rsdp;
   173		u8 *table_ptr;
   174		u8 *mem_rover;
   175		u32 address;
   176	
   177		/* Get the location of the Extended BIOS Data Area (EBDA) */
   178		table_ptr = (u8 *)ACPI_EBDA_PTR_LOCATION;
   179		*(u32 *)(void *)&address = *(u16 *)(void *)table_ptr;
   180		address <<= 4;
 > 181		table_ptr = (u8 *)(acpi_physical_address)address;
   182	
   183		/*
   184		 * Search EBDA paragraphs (EBDA is required to be a minimum of
   185		 * 1K length)
   186		 */
   187		if (address > 0x400) {
   188			mem_rover = scan_mem_for_rsdp(table_ptr, ACPI_EBDA_WINDOW_SIZE);
   189	
   190			if (mem_rover) {
   191				address += (u32)ACPI_PTR_DIFF(mem_rover, table_ptr);
   192				*rsdp_addr = (acpi_physical_address)address;
   193				return;
   194			}
   195		}
   196	
   197		table_ptr = (u8 *)ACPI_HI_RSDP_WINDOW_BASE;
   198		mem_rover = scan_mem_for_rsdp(table_ptr, ACPI_HI_RSDP_WINDOW_SIZE);
   199	
   200		/*
   201		 * Search upper memory: 16-byte boundaries in E0000h-FFFFFh
   202		 */
   203		if (mem_rover) {
   204			address = (u32)(ACPI_HI_RSDP_WINDOW_BASE +
   205					ACPI_PTR_DIFF(mem_rover, table_ptr));
   206			*rsdp_addr = (acpi_physical_address)address;
   207			return;
   208		}
   209	}
   210	
   211	#ifdef CONFIG_KEXEC
   212	static bool get_acpi_rsdp(acpi_physical_address *rsdp_addr)
   213	{
   214		char *args = (char *)get_cmd_line_ptr();
   215		size_t len = strlen((char *)args);
   216		char *tmp_cmdline, *param, *val;
   217		unsigned long long addr = 0;
   218		char *endptr;
   219	
   220		if (!strstr(args, "acpi_rsdp="))
   221			return false;
   222	
   223		tmp_cmdline = malloc(len+1);
   224		if (!tmp_cmdline)
   225			error("Failed to allocate space for tmp_cmdline");
   226	
   227		memcpy(tmp_cmdline, args, len);
   228		tmp_cmdline[len] = 0;
   229		args = tmp_cmdline;
   230	
   231		args = skip_spaces(args);
   232	
   233		while (*args) {
   234			args = next_arg(args, &param, &val);
   235	
   236			if (!val && strcmp(param, "--") == 0) {
 > 237				warn("Only '--' specified in cmdline");
   238				free(tmp_cmdline);
   239				return false;
   240			}
   241	
   242			if (!strcmp(param, "acpi_rsdp")) {
   243				addr = simple_strtoull(val, &endptr, 0);
   244	
   245				if (addr == 0)
   246					return false;
   247	
   248				*rsdp_addr = (acpi_physical_address)addr;
   249				return true;
   250			}
   251		}
   252		return false;
   253	}
   254	#else
   255	static bool get_acpi_rsdp(acpi_physical_address *rsdp_addr)
   256	{
   257		return false;
   258	}
   259	#endif
   260	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 27945 bytes --]

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

end of thread, other threads:[~2018-09-12  7:09 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-10 12:40 [PATCH v6 0/3] x86/boot/KASLR: Parse ACPI table and limit kaslr in immovable memory Chao Fan
2018-09-10 12:40 ` [PATCH v6 1/3] x86/boot: Add acpitb.c to parse acpi tables Chao Fan
2018-09-10 20:13   ` Rafael J. Wysocki
2018-09-11  1:27     ` Chao Fan
2018-09-11  7:41       ` Rafael J. Wysocki
2018-09-11  7:52         ` Chao Fan
2018-09-11  8:52   ` [RFC PATCH] x86/boot: get_acpi_srat_table() can be static kbuild test robot
2018-09-11  8:55     ` Chao Fan
2018-09-11  8:52   ` [PATCH v6 1/3] x86/boot: Add acpitb.c to parse acpi tables kbuild test robot
2018-09-12  7:08   ` kbuild test robot
2018-09-10 12:40 ` [PATCH v6 2/3] x86/boot/KASLR: Walk srat tables to filter immovable memory Chao Fan
2018-09-10 12:40 ` [PATCH v6 3/3] x86/boot/KASLR: Limit kaslr to choosing the " Chao Fan
2018-09-11  1:13 ` [PATCH v6 0/3] x86/boot/KASLR: Parse ACPI table and limit kaslr in " Chao Fan
2018-09-11  7:28   ` Rafael J. Wysocki
2018-09-11  7:53     ` Chao Fan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).