Re: [PATCH RESEND v1 4/5] powerpc/crash: use generic crashkernel reservation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello Mahesh,

On 08/01/25 22:35, Mahesh J Salgaonkar wrote:
On 2025-01-08 15:44:57 Wed, Sourabh Jain wrote:
Commit 0ab97169aa05 ("crash_core: add generic function to do
reservation") added a generic function to reserve crashkernel memory.
So let's use the same function on powerpc and remove the
architecture-specific code that essentially does the same thing.

The generic crashkernel reservation also provides a way to split the
crashkernel reservation into high and low memory reservations, which can
be enabled for powerpc in the future.

Along with moving to the generic crashkernel reservation, the code
related to finding the base address for the crashkernel has been
separated into its own function name get_crash_base() for better
readability and maintainability.

To prevent crashkernel memory from being added to iomem_resource, the Mahesh.
function arch_add_crash_res_to_iomem() has been introduced. For further
details on why this should not be done for the PowerPC architecture,
please refer to the previous commit titled "crash: let arch decide crash
memory export to iomem_resource.

Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
Cc: Baoquan he <bhe@xxxxxxxxxx>
Cc: Hari Bathini <hbathini@xxxxxxxxxxxxx>
CC: Madhavan Srinivasan <maddy@xxxxxxxxxxxxx>
Cc: Michael Ellerman <mpe@xxxxxxxxxxxxxx>
Cc: kexec@xxxxxxxxxxxxxxxxxxx
Cc: linux-kernel@xxxxxxxxxxxxxxx
Signed-off-by: Sourabh Jain <sourabhjain@xxxxxxxxxxxxx>
---
  arch/powerpc/Kconfig                     |  3 +
  arch/powerpc/include/asm/crash_reserve.h | 18 +++++
  arch/powerpc/include/asm/kexec.h         |  4 +-
  arch/powerpc/kernel/prom.c               |  2 +-
  arch/powerpc/kexec/core.c                | 90 ++++++++++--------------
  5 files changed, 63 insertions(+), 54 deletions(-)
  create mode 100644 arch/powerpc/include/asm/crash_reserve.h
[...]
@@ -113,9 +113,9 @@ int setup_new_fdt_ppc64(const struct kimage *image, void *fdt, struct crash_mem
#ifdef CONFIG_CRASH_RESERVE
  int __init overlaps_crashkernel(unsigned long start, unsigned long size);
-extern void reserve_crashkernel(void);
+extern void arch_reserve_crashkernel(void); Mahesh. Mahesh
Do we really need to rename this ? it is still called from powepc arch
and not from the common code.

You are right, we don’t. However, all architectures (x86, RISC-V, LoongArch, ARM64) that use the generic crash kernel reservation have named their architecture-specific function `arch_reserve_crashkernel()`.
So, I did the same for PowerPC, and this helps sometimes.

Maybe I should justify the name change in the commit to avoid confusion.

Please let me know your opinion.


  #else
-static inline void reserve_crashkernel(void) {}
+static inline void arch_reserve_crashkernel(void) {}
  static inline int overlaps_crashkernel(unsigned long start, unsigned long size) { return 0; }
  #endif
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index e0059842a1c6..9ed9dde7d231 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -860,7 +860,7 @@ void __init early_init_devtree(void *params)
  	 */
  	if (fadump_reserve_mem() == 0)
  #endif
-		reserve_crashkernel();
+		arch_reserve_crashkernel();
  	early_reserve_mem();
if (memory_limit > memblock_phys_mem_size())
Rest looks good to me.

Reviewed-by: Mahesh Salgaonkar <mahesh@xxxxxxxxxxxxx>

Thank you for reviewing this.

- Sourabh Jain




[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux