| Message ID | 156630275779.8896.7854485220030978790.stgit@hbathini.in.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add FADump support on PowerNV platform | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch next (c9633332103e55bc73d80d07ead28b95a22a85a3) |
| snowpatch_ozlabs/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 57 lines checked |
Hari Bathini <hbathini@linux.ibm.com> writes: > Firmware uses 32-bit field for region size while copying/backing-up Which firmware exactly is imposing that limit? > memory during MPIPL. So, the maximum copy size for a region would > be a page less than 4GB (aligned to pagesize) but FADump capture > kernel usually needs more memory than that to be preserved to avoid > running into out of memory errors. > > So, request firmware to copy multiple kernel boot memory regions > instead of just one (which worked fine for pseries as 64-bit field > was used for size there). > > Signed-off-by: Hari Bathini <hbathini@linux.ibm.com> > --- > arch/powerpc/platforms/powernv/opal-fadump.c | 35 +++++++++++++++++++++----- > 1 file changed, 28 insertions(+), 7 deletions(-) > > diff --git a/arch/powerpc/platforms/powernv/opal-fadump.c b/arch/powerpc/platforms/powernv/opal-fadump.c > index 91fb909..a755705 100644 > --- a/arch/powerpc/platforms/powernv/opal-fadump.c > +++ b/arch/powerpc/platforms/powernv/opal-fadump.c > @@ -28,6 +28,8 @@ static int opal_fadump_unregister(struct fw_dump *fadump_conf); > static void opal_fadump_update_config(struct fw_dump *fadump_conf, > const struct opal_fadump_mem_struct *fdm) > { > + pr_debug("Boot memory regions count: %d\n", fdm->region_cnt); > + > /* > * The destination address of the first boot memory region is the > * destination address of boot memory regions. > @@ -50,16 +52,35 @@ static void opal_fadump_init_metadata(struct opal_fadump_mem_struct *fdm) > > static ulong opal_fadump_init_mem_struct(struct fw_dump *fadump_conf) > { > - ulong addr = fadump_conf->reserve_dump_area_start; > + ulong src_addr, dest_addr; > + int max_copy_size, cur_size, size; > > opal_fdm = __va(fadump_conf->kernel_metadata); > opal_fadump_init_metadata(opal_fdm); > > - opal_fdm->region_cnt = 1; > - opal_fdm->rgn[0].src = RMA_START; > - opal_fdm->rgn[0].dest = addr; > - opal_fdm->rgn[0].size = fadump_conf->boot_memory_size; > - addr += fadump_conf->boot_memory_size; > + /* > + * Firmware currently supports only 32-bit value for size, "currently" implies it could change in future? If it does we assume it will only increase, and we're happy that old kernels will continue to use the 32-bit limit? > + * align it to pagesize and request firmware to copy multiple > + * kernel boot memory regions. > + */ > + max_copy_size = _ALIGN_DOWN(U32_MAX, PAGE_SIZE); > + > + /* Boot memory regions */ > + src_addr = RMA_START; I'm not convinced using RMA_START actually makes things any clearer, given that it's #defined as 0, and we even have a BUILD_BUG_ON() to make sure it's never anything else. eg: src_addr = 0; > + dest_addr = fadump_conf->reserve_dump_area_start; > + size = fadump_conf->boot_memory_size; > + while (size) { > + cur_size = size > max_copy_size ? max_copy_size : size; > + > + opal_fdm->rgn[opal_fdm->region_cnt].src = src_addr; > + opal_fdm->rgn[opal_fdm->region_cnt].dest = dest_addr; > + opal_fdm->rgn[opal_fdm->region_cnt].size = cur_size; > + > + opal_fdm->region_cnt++; > + dest_addr += cur_size; > + src_addr += cur_size; > + size -= cur_size; > + } > > /* > * Kernel metadata is passed to f/w and retrieved in capture kerenl. > @@ -70,7 +91,7 @@ static ulong opal_fadump_init_mem_struct(struct fw_dump *fadump_conf) > > opal_fadump_update_config(fadump_conf, opal_fdm); > > - return addr; > + return dest_addr; > } > > static ulong opal_fadump_get_metadata_size(void) cheers
On 04/09/19 5:00 PM, Michael Ellerman wrote: > Hari Bathini <hbathini@linux.ibm.com> writes: >> Firmware uses 32-bit field for region size while copying/backing-up > > Which firmware exactly is imposing that limit? I think the MDST/MDRT tables in the f/w. Vasant, which component is that? >> + /* >> + * Firmware currently supports only 32-bit value for size, > > "currently" implies it could change in future? > > If it does we assume it will only increase, and we're happy that old > kernels will continue to use the 32-bit limit? I am not aware of any plans to make it 64-bit. Let me just say f/w supports only 32-bit to get rid of that ambiguity.. - Hari
Hari Bathini <hbathini@linux.ibm.com> writes: > On 04/09/19 5:00 PM, Michael Ellerman wrote: >> Hari Bathini <hbathini@linux.ibm.com> writes: >>> Firmware uses 32-bit field for region size while copying/backing-up >> >> Which firmware exactly is imposing that limit? > > I think the MDST/MDRT tables in the f/w. Vasant, which component is that? > >>> + /* >>> + * Firmware currently supports only 32-bit value for size, >> >> "currently" implies it could change in future? >> >> If it does we assume it will only increase, and we're happy that old >> kernels will continue to use the 32-bit limit? > > I am not aware of any plans to make it 64-bit. Let me just say f/w supports > only 32-bit to get rid of that ambiguity.. OK. As long as everyone is aware that the kernel has no support for it increasing it, without code changes. cheers
diff --git a/arch/powerpc/platforms/powernv/opal-fadump.c b/arch/powerpc/platforms/powernv/opal-fadump.c index 91fb909..a755705 100644 --- a/arch/powerpc/platforms/powernv/opal-fadump.c +++ b/arch/powerpc/platforms/powernv/opal-fadump.c @@ -28,6 +28,8 @@ static int opal_fadump_unregister(struct fw_dump *fadump_conf); static void opal_fadump_update_config(struct fw_dump *fadump_conf, const struct opal_fadump_mem_struct *fdm) { + pr_debug("Boot memory regions count: %d\n", fdm->region_cnt); + /* * The destination address of the first boot memory region is the * destination address of boot memory regions. @@ -50,16 +52,35 @@ static void opal_fadump_init_metadata(struct opal_fadump_mem_struct *fdm) static ulong opal_fadump_init_mem_struct(struct fw_dump *fadump_conf) { - ulong addr = fadump_conf->reserve_dump_area_start; + ulong src_addr, dest_addr; + int max_copy_size, cur_size, size; opal_fdm = __va(fadump_conf->kernel_metadata); opal_fadump_init_metadata(opal_fdm); - opal_fdm->region_cnt = 1; - opal_fdm->rgn[0].src = RMA_START; - opal_fdm->rgn[0].dest = addr; - opal_fdm->rgn[0].size = fadump_conf->boot_memory_size; - addr += fadump_conf->boot_memory_size; + /* + * Firmware currently supports only 32-bit value for size, + * align it to pagesize and request firmware to copy multiple + * kernel boot memory regions. + */ + max_copy_size = _ALIGN_DOWN(U32_MAX, PAGE_SIZE); + + /* Boot memory regions */ + src_addr = RMA_START; + dest_addr = fadump_conf->reserve_dump_area_start; + size = fadump_conf->boot_memory_size; + while (size) { + cur_size = size > max_copy_size ? max_copy_size : size; + + opal_fdm->rgn[opal_fdm->region_cnt].src = src_addr; + opal_fdm->rgn[opal_fdm->region_cnt].dest = dest_addr; + opal_fdm->rgn[opal_fdm->region_cnt].size = cur_size; + + opal_fdm->region_cnt++; + dest_addr += cur_size; + src_addr += cur_size; + size -= cur_size; + } /* * Kernel metadata is passed to f/w and retrieved in capture kerenl. @@ -70,7 +91,7 @@ static ulong opal_fadump_init_mem_struct(struct fw_dump *fadump_conf) opal_fadump_update_config(fadump_conf, opal_fdm); - return addr; + return dest_addr; } static ulong opal_fadump_get_metadata_size(void)
Firmware uses 32-bit field for region size while copying/backing-up memory during MPIPL. So, the maximum copy size for a region would be a page less than 4GB (aligned to pagesize) but FADump capture kernel usually needs more memory than that to be preserved to avoid running into out of memory errors. So, request firmware to copy multiple kernel boot memory regions instead of just one (which worked fine for pseries as 64-bit field was used for size there). Signed-off-by: Hari Bathini <hbathini@linux.ibm.com> --- arch/powerpc/platforms/powernv/opal-fadump.c | 35 +++++++++++++++++++++----- 1 file changed, 28 insertions(+), 7 deletions(-)