Messages in this thread | | | Date | Tue, 18 Dec 2018 11:17:10 +0800 | From | Chao Fan <> | Subject | Re: [PATCH v14 4/5] x86/boot: Parse SRAT address from RSDP and store immovable memory |
| |
On Mon, Dec 17, 2018 at 06:41:49PM +0100, Ingo Molnar wrote: > >* Chao Fan <fanc.fnst@cn.fujitsu.com> wrote: > >> SRAT should be parsed by RSDP to fix the conflict between KASLR >> and memory-hotremove, then find the immovable memory regions and store >> them in an array called immovable_mem[]. With immovable_mem[], KASLR >> can avoid to extract kernel to specific regions. >> >> Since 'RANDOMIZE_BASE' && 'MEMORY_HOTREMOVE' is needed, introduce >> 'CONFIG_EARLY_PARSE_RSDP' to make ifdeffery clear. >> >> Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com> >> --- >> arch/x86/Kconfig | 12 +++ >> arch/x86/boot/compressed/Makefile | 2 + >> arch/x86/boot/compressed/acpi.c | 128 ++++++++++++++++++++++++++++++ >> arch/x86/boot/compressed/kaslr.c | 4 - >> arch/x86/boot/compressed/misc.h | 19 +++++ >> 5 files changed, 161 insertions(+), 4 deletions(-) >> >> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig >> index ba7e3464ee92..333c383478b7 100644 >> --- a/arch/x86/Kconfig >> +++ b/arch/x86/Kconfig >> @@ -2149,6 +2149,18 @@ config X86_NEED_RELOCS >> def_bool y >> depends on RANDOMIZE_BASE || (X86_32 && RELOCATABLE) >> >> +config EARLY_SRAT_PARSE >> + bool "Early SRAT table parsing" >> + def_bool y >> + depends on RANDOMIZE_BASE && MEMORY_HOTREMOVE >> + help >> + This option enables early SRAT parsing in compressed boot stage >> + so that memory hot-remove ranges do not overlap with KASLR >> + chosen ranges. Kernel won't be extracted in hot-removable >> + memory, so that make sure memory-hotremove works well with >> + KASLR enabled. >> + Say Y if you want to use both KASLR and memory-hotremove. > >So why would we want to make this a config option, instead of enabling it >unconditionally?
Well, it can make ifdeffery more clear, and Boris suggested to do that. If there is no KASLR enabled or MEMORY-HOTREMOVE enabled, the code is not needed, so it's not good to enable it unconditionally.
> >How reliable are the hot-removable memory markings in various firmware >versions?
But we can only get the information from firmware, I can't figure out other information. And ACPI also gets the table from here.
> >> +/* Compute SRAT table from RSDP. */ >> +static struct acpi_table_header *get_acpi_srat_table(void) >> +{ >> + acpi_physical_address acpi_table; >> + acpi_physical_address root_table; >> + struct acpi_table_header *header; >> + struct acpi_table_rsdp *rsdp; >> + u32 num_entries; >> + char arg[10]; > >The '10' is just a magic number attached to a meaningless local variable >name. Please explain the limit in the code, and the role of the variable >if it's non-obvious from the name. Or better, try to find a more obvious >name? >
Yes, the '10' is magic number, the meaning is detail. Here, the '10' is used to store the result of 'acpi=' in cmdline. Well, see Documentation/admin-guide/kernel-parameters.txt: acpi= [HW,ACPI,X86,ARM64] Advanced Configuration and Power Interface Format: { force | on | off | strict | noirq | rsdt | copy_dsdt } 'copy_dsdt' is the longest result, its size is '9', plus '\0' is '10'. So I set it as '10'.
And I see the usage like this is: char arg[5]; in pti_check_boottime_disable() of arch/x86/mm/pti.c, since the longest result of 'pti=' in cmdline is 'auto', so it's '5' here.
And: char arg[32]; in fpu__init_parse_early_param() of arch/x86/kernel/fpu/init.c.
And so on. All usage of cmdline_find_option() needs a string, they are often named as arg[] and with a number which can cover the longest result.
Thanks, Chao Fan
>Thanks, > > Ingo > >
| |