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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <dgn3qq2q35rjem3lm33k646tl7ucxghwnvttplr4fdk35aftfx@2tzxh6ku5m3d>
Date: Wed, 8 Jan 2025 22:35:36 +0530
From: Mahesh J Salgaonkar <mahesh@...ux.ibm.com>
To: Sourabh Jain <sourabhjain@...ux.ibm.com>
Cc: linuxppc-dev@...ts.ozlabs.org, Andrew Morton <akpm@...ux-foundation.org>,
        Baoquan he <bhe@...hat.com>, Hari Bathini <hbathini@...ux.ibm.com>,
        Madhavan Srinivasan <maddy@...ux.ibm.com>,
        Michael Ellerman <mpe@...erman.id.au>, kexec@...ts.infradead.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH RESEND v1 4/5] powerpc/crash: use generic crashkernel
 reservation

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
> 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@...ux-foundation.org>
> Cc: Baoquan he <bhe@...hat.com>
> Cc: Hari Bathini <hbathini@...ux.ibm.com>
> CC: Madhavan Srinivasan <maddy@...ux.ibm.com>
> Cc: Michael Ellerman <mpe@...erman.id.au>
> Cc: kexec@...ts.infradead.org
> Cc: linux-kernel@...r.kernel.org
> Signed-off-by: Sourabh Jain <sourabhjain@...ux.ibm.com>
> ---
>  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);

Do we really need to rename this ? it is still called from powepc arch
and not from the common code.

>  #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@...ux.ibm.com>

Thanks,
-Mahesh.