Message ID | 152265064158.23251.10213309407765206304.stgit@jupiter.in.ibm.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | powerpc/fadump: Improvements and fixes for firmware-assisted dump. | expand |
I think CMA has protected us from hot-remove, so this patch is not necessary. 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 > >
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 >> >> >
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