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.