[PATCH] arm64/mm/hotplug: Warn when memory limit has been reduced

[PATCH] arm64/mm/hotplug: Warn when memory limit has been reduced

[Anshuman Khandual]

If the max memory limit has been reduced with 'mem=' kernel command line
option, there might be UEFI memory map described memory beyond that limit
which could be hot removed. This might be problematic for subsequent kexec
kernel which could just access such removed memory.

Memory offline notifier exists because there is no other way to block the
removal of boot memory, only the offlining (which isn't actually a problem)
But with 'mem=', there is no chance to stop such boot memory being offlined
as it where never in use by the kernel. As 'mem=' is a debug only option on
arm64 platform, just warn for such a situation and move on.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: James Morse <james.morse@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
This applies on v5.15-rc1

 arch/arm64/include/asm/memory.h |  1 +
 arch/arm64/mm/init.c            |  9 +++++++--
 arch/arm64/mm/mmu.c             | 12 ++++++++++++
 3 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index f1745a843414..361d4e01a864 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -353,6 +353,7 @@ static inline void *phys_to_virt(phys_addr_t x)
 })
 
 void dump_mem_limit(void);
+bool has_mem_limit_reduced(void);
 #endif /* !ASSEMBLY */
 
 /*
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 37a81754d9b6..cf21edfc8b0f 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -211,6 +211,11 @@ EXPORT_SYMBOL(pfn_is_map_memory);
 
 static phys_addr_t memory_limit = PHYS_ADDR_MAX;
 
+bool has_mem_limit_reduced(void)
+{
+	return memory_limit != PHYS_ADDR_MAX;
+}
+
 /*
  * Limit the memory size that was specified via FDT.
  */
@@ -285,7 +290,7 @@ void __init arm64_memblock_init(void)
 	 * high up in memory, add back the kernel region that must be accessible
 	 * via the linear mapping.
 	 */
-	if (memory_limit != PHYS_ADDR_MAX) {
+	if (has_mem_limit_reduced()) {
 		memblock_mem_limit_remove_map(memory_limit);
 		memblock_add(__pa_symbol(_text), (u64)(_end - _text));
 	}
@@ -461,7 +466,7 @@ void free_initmem(void)
 
 void dump_mem_limit(void)
 {
-	if (memory_limit != PHYS_ADDR_MAX) {
+	if (has_mem_limit_reduced()) {
 		pr_emerg("Memory Limit: %llu MB\n", memory_limit >> 20);
 	} else {
 		pr_emerg("Memory Limit: none\n");
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index cfd9deb347c3..7ac39ee876c3 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -1627,6 +1627,18 @@ static int __init prevent_bootmem_remove_init(void)
 	if (!IS_ENABLED(CONFIG_MEMORY_HOTREMOVE))
 		return ret;
 
+	if (has_mem_limit_reduced()) {
+		/*
+		 * Physical memory limit has been reduced via the 'mem=' kernel
+		 * command line option. Memory beyond reduced limit could now be
+		 * removed and reassigned (guest ?) transparently to the kernel.
+		 * This might cause subsequent kexec kernel to crash or at least
+		 * corrupt the memory when accessing UEFI memory map enumerated
+		 * boot memory which might have been repurposed.
+		 */
+		pr_warn("Memory limit reduced, kexec might be problematic\n");
+	}
+
 	validate_bootmem_online();
 	ret = register_memory_notifier(&prevent_bootmem_remove_nb);
 	if (ret)
-- 
2.20.1

[PATCH] arm64/mm/hotplug: Warn when memory limit has been reduced

[Anshuman Khandual]

If the max memory limit has been reduced with 'mem=' kernel command line
option, there might be UEFI memory map described memory beyond that limit
which could be hot removed. This might be problematic for subsequent kexec
kernel which could just access such removed memory.

Memory offline notifier exists because there is no other way to block the
removal of boot memory, only the offlining (which isn't actually a problem)
But with 'mem=', there is no chance to stop such boot memory being offlined
as it where never in use by the kernel. As 'mem=' is a debug only option on
arm64 platform, just warn for such a situation and move on.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: James Morse <james.morse@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
This applies on v5.15-rc1

 arch/arm64/include/asm/memory.h |  1 +
 arch/arm64/mm/init.c            |  9 +++++++--
 arch/arm64/mm/mmu.c             | 12 ++++++++++++
 3 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index f1745a843414..361d4e01a864 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -353,6 +353,7 @@ static inline void *phys_to_virt(phys_addr_t x)
 })
 
 void dump_mem_limit(void);
+bool has_mem_limit_reduced(void);
 #endif /* !ASSEMBLY */
 
 /*
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 37a81754d9b6..cf21edfc8b0f 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -211,6 +211,11 @@ EXPORT_SYMBOL(pfn_is_map_memory);
 
 static phys_addr_t memory_limit = PHYS_ADDR_MAX;
 
+bool has_mem_limit_reduced(void)
+{
+	return memory_limit != PHYS_ADDR_MAX;
+}
+
 /*
  * Limit the memory size that was specified via FDT.
  */
@@ -285,7 +290,7 @@ void __init arm64_memblock_init(void)
 	 * high up in memory, add back the kernel region that must be accessible
 	 * via the linear mapping.
 	 */
-	if (memory_limit != PHYS_ADDR_MAX) {
+	if (has_mem_limit_reduced()) {
 		memblock_mem_limit_remove_map(memory_limit);
 		memblock_add(__pa_symbol(_text), (u64)(_end - _text));
 	}
@@ -461,7 +466,7 @@ void free_initmem(void)
 
 void dump_mem_limit(void)
 {
-	if (memory_limit != PHYS_ADDR_MAX) {
+	if (has_mem_limit_reduced()) {
 		pr_emerg("Memory Limit: %llu MB\n", memory_limit >> 20);
 	} else {
 		pr_emerg("Memory Limit: none\n");
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index cfd9deb347c3..7ac39ee876c3 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -1627,6 +1627,18 @@ static int __init prevent_bootmem_remove_init(void)
 	if (!IS_ENABLED(CONFIG_MEMORY_HOTREMOVE))
 		return ret;
 
+	if (has_mem_limit_reduced()) {
+		/*
+		 * Physical memory limit has been reduced via the 'mem=' kernel
+		 * command line option. Memory beyond reduced limit could now be
+		 * removed and reassigned (guest ?) transparently to the kernel.
+		 * This might cause subsequent kexec kernel to crash or at least
+		 * corrupt the memory when accessing UEFI memory map enumerated
+		 * boot memory which might have been repurposed.
+		 */
+		pr_warn("Memory limit reduced, kexec might be problematic\n");
+	}
+
 	validate_bootmem_online();
 	ret = register_memory_notifier(&prevent_bootmem_remove_nb);
 	if (ret)
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64/mm/hotplug: Warn when memory limit has been reduced

[Catalin Marinas]

On Tue, Sep 14, 2021 at 12:21:10PM +0530, Anshuman Khandual wrote:
> If the max memory limit has been reduced with 'mem=' kernel command line
> option, there might be UEFI memory map described memory beyond that limit
> which could be hot removed. This might be problematic for subsequent kexec
> kernel which could just access such removed memory.
> 
> Memory offline notifier exists because there is no other way to block the
> removal of boot memory, only the offlining (which isn't actually a problem)
> But with 'mem=', there is no chance to stop such boot memory being offlined
> as it where never in use by the kernel. As 'mem=' is a debug only option on
> arm64 platform, just warn for such a situation and move on.

Just to make sure I understand, is the memory beyond the mem= limit
considered online by the core code and it can be subsequently offlined?
Looking at walk_system_ram_range(), it doesn't seem to care about the
removed memblock ranges. Would such memory beyond the mem= limit need to
have been onlined first?

> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index cfd9deb347c3..7ac39ee876c3 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -1627,6 +1627,18 @@ static int __init prevent_bootmem_remove_init(void)
>  	if (!IS_ENABLED(CONFIG_MEMORY_HOTREMOVE))
>  		return ret;
>  
> +	if (has_mem_limit_reduced()) {
> +		/*
> +		 * Physical memory limit has been reduced via the 'mem=' kernel
> +		 * command line option. Memory beyond reduced limit could now be
> +		 * removed and reassigned (guest ?) transparently to the kernel.
> +		 * This might cause subsequent kexec kernel to crash or at least
> +		 * corrupt the memory when accessing UEFI memory map enumerated
> +		 * boot memory which might have been repurposed.
> +		 */
> +		pr_warn("Memory limit reduced, kexec might be problematic\n");
> +	}

I'd actually move the warning to hotplug notifier callback rather than
the init function. I'd also make it pr_warn_once().

-- 
Catalin

Re: [PATCH] arm64/mm/hotplug: Warn when memory limit has been reduced

[Catalin Marinas]

On Tue, Sep 14, 2021 at 12:21:10PM +0530, Anshuman Khandual wrote:
> If the max memory limit has been reduced with 'mem=' kernel command line
> option, there might be UEFI memory map described memory beyond that limit
> which could be hot removed. This might be problematic for subsequent kexec
> kernel which could just access such removed memory.
> 
> Memory offline notifier exists because there is no other way to block the
> removal of boot memory, only the offlining (which isn't actually a problem)
> But with 'mem=', there is no chance to stop such boot memory being offlined
> as it where never in use by the kernel. As 'mem=' is a debug only option on
> arm64 platform, just warn for such a situation and move on.

Just to make sure I understand, is the memory beyond the mem= limit
considered online by the core code and it can be subsequently offlined?
Looking at walk_system_ram_range(), it doesn't seem to care about the
removed memblock ranges. Would such memory beyond the mem= limit need to
have been onlined first?

> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index cfd9deb347c3..7ac39ee876c3 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -1627,6 +1627,18 @@ static int __init prevent_bootmem_remove_init(void)
>  	if (!IS_ENABLED(CONFIG_MEMORY_HOTREMOVE))
>  		return ret;
>  
> +	if (has_mem_limit_reduced()) {
> +		/*
> +		 * Physical memory limit has been reduced via the 'mem=' kernel
> +		 * command line option. Memory beyond reduced limit could now be
> +		 * removed and reassigned (guest ?) transparently to the kernel.
> +		 * This might cause subsequent kexec kernel to crash or at least
> +		 * corrupt the memory when accessing UEFI memory map enumerated
> +		 * boot memory which might have been repurposed.
> +		 */
> +		pr_warn("Memory limit reduced, kexec might be problematic\n");
> +	}

I'd actually move the warning to hotplug notifier callback rather than
the init function. I'd also make it pr_warn_once().

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64/mm/hotplug: Warn when memory limit has been reduced

[Anshuman Khandual]

On 9/16/21 8:04 PM, Catalin Marinas wrote:
> On Tue, Sep 14, 2021 at 12:21:10PM +0530, Anshuman Khandual wrote:
>> If the max memory limit has been reduced with 'mem=' kernel command line
>> option, there might be UEFI memory map described memory beyond that limit
>> which could be hot removed. This might be problematic for subsequent kexec
>> kernel which could just access such removed memory.
>>
>> Memory offline notifier exists because there is no other way to block the
>> removal of boot memory, only the offlining (which isn't actually a problem)
>> But with 'mem=', there is no chance to stop such boot memory being offlined
>> as it where never in use by the kernel. As 'mem=' is a debug only option on
>> arm64 platform, just warn for such a situation and move on.
> 
> Just to make sure I understand, is the memory beyond the mem= limit
> considered online by the core code and it can be subsequently offlined?

No, those memory (beyond the mem= limit) would never be part of the memblock
itself during boot, and hence cannot be onlined or used. But these can come
back into the kernel via a hot add operation after boot, it may be onlined
and possibly removed afterwards. But the reason for not being able to stop
such removal is different. Agreed, the commit message is bit misleading.

> Looking at walk_system_ram_range(), it doesn't seem to care about the
> removed memblock ranges. Would such memory beyond the mem= limit need to
> have been onlined first?

The problem here is bit different and the description in the commit message
is not very clear. Lets consider the following scenario.

1. UEFI boot memory map has got X amount of memory
2. The kernel boots with just (X - Y) memory via mem=X-Y option
3. Platform hot adds Y after the boot
4. Memory ranges in Y gets onlined (and repurposed)
5. Platform hot removes Y subsequently

Because the kernel originally booted with (mem=X-Y) option, all the memory
sections in Y, never got marked with SECTION_IS_EARLY. Subsequent hot add
operation would create memblock entries but could not mark SECTION_IS_EARLY
either. Hence those sections even after being onlined, can now be removed.
The existing notifier can not prevent this because these sections dont have
SECTION_IS_EARLY flag. The intent here is to warn users about the inability
of the notifier to deal with boot memory that comes back later via hot plug
after being carved out first via the 'mem=' command line option. The kexec
problem is just an side effect.

Interestingly, the kexec problem would still persist even without the memory
hotplug (and hot remove) being enabled. What if the platform just choose to
re-purposes Y for some thing else and then the kernel does a kexec afterwards
? The new kernel would seamlessly try to use Y causing further problem.

Another warning would be needed during memory init for any potential kexec
problems, regardless whether hotplug is enabled or not. A separate patch might
be required for that.

But this proposal here is intended to just warn the user that memory hotremove
notifier could not prevent potential boot memory being removed, if mem= option
has already reduced the boot memory range during boot. Probably the warning
message and documentation here need some changes to reflect it better.

> 
>> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
>> index cfd9deb347c3..7ac39ee876c3 100644
>> --- a/arch/arm64/mm/mmu.c
>> +++ b/arch/arm64/mm/mmu.c
>> @@ -1627,6 +1627,18 @@ static int __init prevent_bootmem_remove_init(void)
>>  	if (!IS_ENABLED(CONFIG_MEMORY_HOTREMOVE))
>>  		return ret;
>>  
>> +	if (has_mem_limit_reduced()) {
>> +		/*
>> +		 * Physical memory limit has been reduced via the 'mem=' kernel
>> +		 * command line option. Memory beyond reduced limit could now be
>> +		 * removed and reassigned (guest ?) transparently to the kernel.
>> +		 * This might cause subsequent kexec kernel to crash or at least
>> +		 * corrupt the memory when accessing UEFI memory map enumerated
>> +		 * boot memory which might have been repurposed.
>> +		 */
>> +		pr_warn("Memory limit reduced, kexec might be problematic\n");
>> +	}
> 
> I'd actually move the warning to hotplug notifier callback rather than
> the init function. I'd also make it pr_warn_once().
> 
Okay, will change.

Re: [PATCH] arm64/mm/hotplug: Warn when memory limit has been reduced

[Anshuman Khandual]

On 9/16/21 8:04 PM, Catalin Marinas wrote:
> On Tue, Sep 14, 2021 at 12:21:10PM +0530, Anshuman Khandual wrote:
>> If the max memory limit has been reduced with 'mem=' kernel command line
>> option, there might be UEFI memory map described memory beyond that limit
>> which could be hot removed. This might be problematic for subsequent kexec
>> kernel which could just access such removed memory.
>>
>> Memory offline notifier exists because there is no other way to block the
>> removal of boot memory, only the offlining (which isn't actually a problem)
>> But with 'mem=', there is no chance to stop such boot memory being offlined
>> as it where never in use by the kernel. As 'mem=' is a debug only option on
>> arm64 platform, just warn for such a situation and move on.
> 
> Just to make sure I understand, is the memory beyond the mem= limit
> considered online by the core code and it can be subsequently offlined?

No, those memory (beyond the mem= limit) would never be part of the memblock
itself during boot, and hence cannot be onlined or used. But these can come
back into the kernel via a hot add operation after boot, it may be onlined
and possibly removed afterwards. But the reason for not being able to stop
such removal is different. Agreed, the commit message is bit misleading.

> Looking at walk_system_ram_range(), it doesn't seem to care about the
> removed memblock ranges. Would such memory beyond the mem= limit need to
> have been onlined first?

The problem here is bit different and the description in the commit message
is not very clear. Lets consider the following scenario.

1. UEFI boot memory map has got X amount of memory
2. The kernel boots with just (X - Y) memory via mem=X-Y option
3. Platform hot adds Y after the boot
4. Memory ranges in Y gets onlined (and repurposed)
5. Platform hot removes Y subsequently

Because the kernel originally booted with (mem=X-Y) option, all the memory
sections in Y, never got marked with SECTION_IS_EARLY. Subsequent hot add
operation would create memblock entries but could not mark SECTION_IS_EARLY
either. Hence those sections even after being onlined, can now be removed.
The existing notifier can not prevent this because these sections dont have
SECTION_IS_EARLY flag. The intent here is to warn users about the inability
of the notifier to deal with boot memory that comes back later via hot plug
after being carved out first via the 'mem=' command line option. The kexec
problem is just an side effect.

Interestingly, the kexec problem would still persist even without the memory
hotplug (and hot remove) being enabled. What if the platform just choose to
re-purposes Y for some thing else and then the kernel does a kexec afterwards
? The new kernel would seamlessly try to use Y causing further problem.

Another warning would be needed during memory init for any potential kexec
problems, regardless whether hotplug is enabled or not. A separate patch might
be required for that.

But this proposal here is intended to just warn the user that memory hotremove
notifier could not prevent potential boot memory being removed, if mem= option
has already reduced the boot memory range during boot. Probably the warning
message and documentation here need some changes to reflect it better.

> 
>> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
>> index cfd9deb347c3..7ac39ee876c3 100644
>> --- a/arch/arm64/mm/mmu.c
>> +++ b/arch/arm64/mm/mmu.c
>> @@ -1627,6 +1627,18 @@ static int __init prevent_bootmem_remove_init(void)
>>  	if (!IS_ENABLED(CONFIG_MEMORY_HOTREMOVE))
>>  		return ret;
>>  
>> +	if (has_mem_limit_reduced()) {
>> +		/*
>> +		 * Physical memory limit has been reduced via the 'mem=' kernel
>> +		 * command line option. Memory beyond reduced limit could now be
>> +		 * removed and reassigned (guest ?) transparently to the kernel.
>> +		 * This might cause subsequent kexec kernel to crash or at least
>> +		 * corrupt the memory when accessing UEFI memory map enumerated
>> +		 * boot memory which might have been repurposed.
>> +		 */
>> +		pr_warn("Memory limit reduced, kexec might be problematic\n");
>> +	}
> 
> I'd actually move the warning to hotplug notifier callback rather than
> the init function. I'd also make it pr_warn_once().
> 
Okay, will change.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel