Re: [PATCH v3 6/7] powerpc/fadump: Do not allow hot-remove memory from fadump reserved area.

[Mahesh Jagannath Salgaonkar <mahesh@linux.vnet.ibm.com>]

On 04/03/2018 08:48 AM, Pingfan Liu wrote:
> I think CMA has protected us from hot-remove, so this patch is not necessary.

Yes, but only if the memory from declared CMA region is allocated using
cma_alloc(). The rest of the memory inside CMA region which hasn't been
cma_allocat-ed can still be hot-removed. Hence this patch is necessary.

fadump allocates only 1 page from fadump CMA region for metadata region.
Rest all memory is free to use by applications and vulnerable to hot-remove.

Thanks,
-Mahesh.

> 
> Regards,
> Pingfan
> 
> On Mon, Apr 2, 2018 at 2:30 PM, Mahesh J Salgaonkar
> <mahesh@linux.vnet.ibm.com> wrote:
>> From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
>>
>> For fadump to work successfully there should not be any holes in reserved
>> memory ranges where kernel has asked firmware to move the content of old
>> kernel memory in event of crash. But this memory area is currently not
>> protected from hot-remove operations. Hence, fadump service can fail to
>> re-register after the hot-remove operation, if hot-removed memory belongs
>> to fadump reserved region. To avoid this make sure that memory from fadump
>> reserved area is not hot-removable if fadump is registered.
>>
>> However, if user still wants to remove that memory, he can do so by
>> manually stopping fadump service before hot-remove operation.
>>
>> Signed-off-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
>> ---
>>  arch/powerpc/include/asm/fadump.h               |    2 +-
>>  arch/powerpc/kernel/fadump.c                    |   10 ++++++++--
>>  arch/powerpc/platforms/pseries/hotplug-memory.c |    7 +++++--
>>  3 files changed, 14 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/fadump.h b/arch/powerpc/include/asm/fadump.h
>> index 44b6ebfe9be6..d16dc77107a8 100644
>> --- a/arch/powerpc/include/asm/fadump.h
>> +++ b/arch/powerpc/include/asm/fadump.h
>> @@ -207,7 +207,7 @@ struct fad_crash_memory_ranges {
>>         unsigned long long      size;
>>  };
>>
>> -extern int is_fadump_boot_memory_area(u64 addr, ulong size);
>> +extern int is_fadump_memory_area(u64 addr, ulong size);
>>  extern int early_init_dt_scan_fw_dump(unsigned long node,
>>                 const char *uname, int depth, void *data);
>>  extern int fadump_reserve_mem(void);
>> diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
>> index 59aaf0357a52..2c3c7e655eec 100644
>> --- a/arch/powerpc/kernel/fadump.c
>> +++ b/arch/powerpc/kernel/fadump.c
>> @@ -162,13 +162,19 @@ int __init early_init_dt_scan_fw_dump(unsigned long node,
>>
>>  /*
>>   * If fadump is registered, check if the memory provided
>> - * falls within boot memory area.
>> + * falls within boot memory area and reserved memory area.
>>   */
>> -int is_fadump_boot_memory_area(u64 addr, ulong size)
>> +int is_fadump_memory_area(u64 addr, ulong size)
>>  {
>> +       u64 d_start = fw_dump.reserve_dump_area_start;
>> +       u64 d_end = d_start + fw_dump.reserve_dump_area_size;
>> +
>>         if (!fw_dump.dump_registered)
>>                 return 0;
>>
>> +       if (((addr + size) > d_start) && (addr <= d_end))
>> +               return 1;
>> +
>>         return (addr + size) > RMA_START && addr <= fw_dump.boot_memory_size;
>>  }
>>
>> diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
>> index c1578f54c626..e4c658cda3a7 100644
>> --- a/arch/powerpc/platforms/pseries/hotplug-memory.c
>> +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
>> @@ -389,8 +389,11 @@ static bool lmb_is_removable(struct drmem_lmb *lmb)
>>         phys_addr = lmb->base_addr;
>>
>>  #ifdef CONFIG_FA_DUMP
>> -       /* Don't hot-remove memory that falls in fadump boot memory area */
>> -       if (is_fadump_boot_memory_area(phys_addr, block_sz))
>> +       /*
>> +        * Don't hot-remove memory that falls in fadump boot memory area
>> +        * and memory that is reserved for capturing old kernel memory.
>> +        */
>> +       if (is_fadump_memory_area(phys_addr, block_sz))
>>                 return false;
>>  #endif
>>
>>
>