diff mbox series

[3/3] arm64: memory: Give hotplug memory a different resource name

Message ID 20200326180730.4754-4-james.morse@arm.com (mailing list archive)
State New, archived
Headers show
Series kexec/memory_hotplug: Prevent removal and accidental use | expand

Commit Message

James Morse March 26, 2020, 6:07 p.m. UTC
If kexec chooses to place the kernel in a memory region that was
added after boot, we fail to boot as the kernel is running from a
location that is not described as memory by the UEFI memory map or
the original DT.

To prevent unaware user-space kexec from doing this accidentally,
give these regions a different name.

Signed-off-by: James Morse <james.morse@arm.com>
---
This is a change in behaviour as seen by user-space, because memory hot-add
has already been merged.

 arch/arm64/include/asm/memory.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

Comments

David Hildenbrand March 30, 2020, 7:01 p.m. UTC | #1
On 26.03.20 19:07, James Morse wrote:
> If kexec chooses to place the kernel in a memory region that was
> added after boot, we fail to boot as the kernel is running from a
> location that is not described as memory by the UEFI memory map or
> the original DT.
> 
> To prevent unaware user-space kexec from doing this accidentally,
> give these regions a different name.
> 
> Signed-off-by: James Morse <james.morse@arm.com>
> ---
> This is a change in behaviour as seen by user-space, because memory hot-add
> has already been merged.
> 
>  arch/arm64/include/asm/memory.h | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
> index 2be67b232499..ef1686518469 100644
> --- a/arch/arm64/include/asm/memory.h
> +++ b/arch/arm64/include/asm/memory.h
> @@ -166,6 +166,17 @@
>  #define IOREMAP_MAX_ORDER	(PMD_SHIFT)
>  #endif
>  
> +/*
> + * Memory hotplug allows new regions of 'System RAM' to be added to the system.
> + * These aren't described as memory by the UEFI memory map, or DT memory node.
> + * If we kexec from one of these regions, the new kernel boots from a location
> + * that isn't described as RAM.
> + *
> + * Give these resources a different name, so unaware kexec doesn't do this by
> + * accident.
> + */
> +#define MEMORY_HOTPLUG_RES_NAME "System RAM (hotplug)"
> +
>  #ifndef __ASSEMBLY__
>  extern u64			vabits_actual;
>  #define PAGE_END		(_PAGE_END(vabits_actual))
> 

(While I am familiar with makedumpfile in the crash kernel, I am not yet
familiar with kexec, so bare with me)


Looking at kexec:arch/arm64/crashdump-arm64.c

load_crashdump_segments() -> crash_get_memory_ranges() ->
kexec_iomem_for_each_line() -> iomem_range_callback()


#define SYSTEM_RAM "System RAM\n"

...
} else if (strncmp(str, SYSTEM_RAM, strlen(SYSTEM_RAM)) == 0) {
	return mem_regions_add(&system_memory_rgns, ...);
}


The hotplugged memory will no longer be detected as a crashdump segment,
consequently (AFAIU) not be described in the elf header, and therefore
also no longer dumped (e.g., by makedumpfile).

I assume you'll have to adapt kexec-tools to still consider this memory
for dumping, correct? Or am I missing something?
Eric W. Biederman April 15, 2020, 8:37 p.m. UTC | #2
James Morse <james.morse@arm.com> writes:

> If kexec chooses to place the kernel in a memory region that was
> added after boot, we fail to boot as the kernel is running from a
> location that is not described as memory by the UEFI memory map or
> the original DT.
>
> To prevent unaware user-space kexec from doing this accidentally,
> give these regions a different name.

Please fix the problem and don't hack around it.

Eric
James Morse April 22, 2020, 12:14 p.m. UTC | #3
Hi Eric,

On 15/04/2020 21:37, Eric W. Biederman wrote:
> James Morse <james.morse@arm.com> writes:
> 
>> If kexec chooses to place the kernel in a memory region that was
>> added after boot, we fail to boot as the kernel is running from a
>> location that is not described as memory by the UEFI memory map or
>> the original DT.
>>
>> To prevent unaware user-space kexec from doing this accidentally,
>> give these regions a different name.
> 
> Please fix the problem and don't hack around it.

The problem is firmware didn't describe memory that wasn't present at boot.

arm64 relies on the firmware description of memory well before it can go poking around in
ACPI to find out where extra memory was added to the system.

We already need kexec to not overwrite in-memory structures left by firmware. (like, the
memory map). We do this by naming them reserved in /proc/iomem.

Doing the same for hotadded memory means existing kexec user-space can't do this
accidentally. The shape of /proc/iomem is the only trick in the book for arm64's kexec
userspace, as its the only thing it looks at.



Thanks,

James
diff mbox series

Patch

diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index 2be67b232499..ef1686518469 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -166,6 +166,17 @@ 
 #define IOREMAP_MAX_ORDER	(PMD_SHIFT)
 #endif
 
+/*
+ * Memory hotplug allows new regions of 'System RAM' to be added to the system.
+ * These aren't described as memory by the UEFI memory map, or DT memory node.
+ * If we kexec from one of these regions, the new kernel boots from a location
+ * that isn't described as RAM.
+ *
+ * Give these resources a different name, so unaware kexec doesn't do this by
+ * accident.
+ */
+#define MEMORY_HOTPLUG_RES_NAME "System RAM (hotplug)"
+
 #ifndef __ASSEMBLY__
 extern u64			vabits_actual;
 #define PAGE_END		(_PAGE_END(vabits_actual))