| 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
| ||
|
Message-ID: <79ecc981-f2ac-44b0-8e36-70e9d01df3ba@linux.ibm.com>
Date: Thu, 23 Jan 2025 16:15:27 +0530
From: Hari Bathini <hbathini@...ux.ibm.com>
To: Sourabh Jain <sourabhjain@...ux.ibm.com>, linuxppc-dev@...ts.ozlabs.org
Cc: Andrew Morton <akpm@...ux-foundation.org>, Baoquan he <bhe@...hat.com>,
Madhavan Srinivasan <maddy@...ux.ibm.com>,
Michael Ellerman <mpe@...erman.id.au>, kexec@...ts.infradead.org,
linux-kernel@...r.kernel.org, Mahesh Salgaonkar <mahesh@...ux.ibm.com>
Subject: Re: [PATCH v2 5/6] powerpc/crash: use generic crashkernel reservation
Hi Sourabh,
On 21/01/25 5:24 pm, 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
> Reviewed-by: Mahesh Salgaonkar <mahesh@...ux.ibm.com>
> 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
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index db9f7b2d07bf..880d35fadf40 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -718,6 +718,9 @@ config ARCH_SUPPORTS_CRASH_HOTPLUG
> def_bool y
> depends on PPC64
>
> +config ARCH_HAS_GENERIC_CRASHKERNEL_RESERVATION
> + def_bool CRASH_RESERVE
> +
> config FA_DUMP
> bool "Firmware-assisted dump"
> depends on CRASH_DUMP && PPC64 && (PPC_RTAS || PPC_POWERNV)
> diff --git a/arch/powerpc/include/asm/crash_reserve.h b/arch/powerpc/include/asm/crash_reserve.h
> new file mode 100644
> index 000000000000..f5e60721de41
> --- /dev/null
> +++ b/arch/powerpc/include/asm/crash_reserve.h
> @@ -0,0 +1,18 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _ASM_POWERPC_CRASH_RESERVE_H
> +#define _ASM_POWERPC_CRASH_RESERVE_H
> +
> +/* crash kernel regions are Page size agliged */
> +#define CRASH_ALIGN PAGE_SIZE
> +
> +#ifdef CONFIG_ARCH_HAS_GENERIC_CRASHKERNEL_RESERVATION
> +
> +static inline bool arch_add_crash_res_to_iomem(void)
> +{
> + return false;
> +}
> +#define arch_add_crash_res_to_iomem arch_add_crash_res_to_iomem
This is probably not needed if commit c40dd2f766440 can be reverted.
Othewise..
Acked-by: Hari Bathini <hbathini@...ux.ibm.com>
> +#endif
> +
> +#endif /* _ASM_POWERPC_CRASH_RESERVE_H */
> +
> diff --git a/arch/powerpc/include/asm/kexec.h b/arch/powerpc/include/asm/kexec.h
> index 270ee93a0f7d..64741558071f 100644
> --- a/arch/powerpc/include/asm/kexec.h
> +++ b/arch/powerpc/include/asm/kexec.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);
> #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())
> diff --git a/arch/powerpc/kexec/core.c b/arch/powerpc/kexec/core.c
> index 4945b33322ae..b21cfa814492 100644
> --- a/arch/powerpc/kexec/core.c
> +++ b/arch/powerpc/kexec/core.c
> @@ -80,38 +80,20 @@ void machine_kexec(struct kimage *image)
> }
>
> #ifdef CONFIG_CRASH_RESERVE
> -void __init reserve_crashkernel(void)
> -{
> - unsigned long long crash_size, crash_base, total_mem_sz;
> - int ret;
>
> - total_mem_sz = memory_limit ? memory_limit : memblock_phys_mem_size();
> - /* use common parsing */
> - ret = parse_crashkernel(boot_command_line, total_mem_sz,
> - &crash_size, &crash_base, NULL, NULL);
> - if (ret == 0 && crash_size > 0) {
> - crashk_res.start = crash_base;
> - crashk_res.end = crash_base + crash_size - 1;
> - }
> -
> - if (crashk_res.end == crashk_res.start) {
> - crashk_res.start = crashk_res.end = 0;
> - return;
> - }
> -
> - /* We might have got these values via the command line or the
> - * device tree, either way sanitise them now. */
> -
> - crash_size = resource_size(&crashk_res);
> +static unsigned long long __init get_crash_base(unsigned long long crash_base)
> +{
>
> #ifndef CONFIG_NONSTATIC_KERNEL
> - if (crashk_res.start != KDUMP_KERNELBASE)
> + if (crash_base != KDUMP_KERNELBASE)
> printk("Crash kernel location must be 0x%x\n",
> KDUMP_KERNELBASE);
>
> - crashk_res.start = KDUMP_KERNELBASE;
> + return KDUMP_KERNELBASE;
> #else
> - if (!crashk_res.start) {
> + unsigned long long crash_base_align;
> +
> + if (!crash_base) {
> #ifdef CONFIG_PPC64
> /*
> * On the LPAR platform place the crash kernel to mid of
> @@ -123,45 +105,51 @@ void __init reserve_crashkernel(void)
> * kernel starts at 128MB offset on other platforms.
> */
> if (firmware_has_feature(FW_FEATURE_LPAR))
> - crashk_res.start = min_t(u64, ppc64_rma_size / 2, SZ_512M);
> + crash_base = min_t(u64, ppc64_rma_size / 2, SZ_512M);
> else
> - crashk_res.start = min_t(u64, ppc64_rma_size / 2, SZ_128M);
> + crash_base = min_t(u64, ppc64_rma_size / 2, SZ_128M);
> #else
> - crashk_res.start = KDUMP_KERNELBASE;
> + crash_base = KDUMP_KERNELBASE;
> #endif
> }
>
> - crash_base = PAGE_ALIGN(crashk_res.start);
> - if (crash_base != crashk_res.start) {
> - printk("Crash kernel base must be aligned to 0x%lx\n",
> - PAGE_SIZE);
> - crashk_res.start = crash_base;
> - }
> + crash_base_align = PAGE_ALIGN(crash_base);
> + if (crash_base != crash_base_align)
> + pr_warn("Crash kernel base must be aligned to 0x%lx\n", PAGE_SIZE);
>
> + return crash_base_align;
> #endif
> - crash_size = PAGE_ALIGN(crash_size);
> - crashk_res.end = crashk_res.start + crash_size - 1;
> +}
>
> - /* The crash region must not overlap the current kernel */
> - if (overlaps_crashkernel(__pa(_stext), _end - _stext)) {
> - printk(KERN_WARNING
> - "Crash kernel can not overlap current kernel\n");
> - crashk_res.start = crashk_res.end = 0;
> +void __init arch_reserve_crashkernel(void)
> +{
> + unsigned long long crash_size, crash_base, crash_end;
> + unsigned long long kernel_start, kernel_size;
> + unsigned long long total_mem_sz;
> + int ret;
> +
> + total_mem_sz = memory_limit ? memory_limit : memblock_phys_mem_size();
> +
> + /* use common parsing */
> + ret = parse_crashkernel(boot_command_line, total_mem_sz, &crash_size,
> + &crash_base, NULL, NULL);
> +
> + if (ret)
> return;
> - }
>
> - printk(KERN_INFO "Reserving %ldMB of memory at %ldMB "
> - "for crashkernel (System RAM: %ldMB)\n",
> - (unsigned long)(crash_size >> 20),
> - (unsigned long)(crashk_res.start >> 20),
> - (unsigned long)(total_mem_sz >> 20));
> + crash_base = get_crash_base(crash_base);
> + crash_end = crash_base + crash_size - 1;
>
> - if (!memblock_is_region_memory(crashk_res.start, crash_size) ||
> - memblock_reserve(crashk_res.start, crash_size)) {
> - pr_err("Failed to reserve memory for crashkernel!\n");
> - crashk_res.start = crashk_res.end = 0;
> + kernel_start = __pa(_stext);
> + kernel_size = _end - _stext;
> +
> + /* The crash region must not overlap the current kernel */
> + if ((kernel_start + kernel_size > crash_base) && (kernel_start <= crash_end)) {
> + pr_warn("Crash kernel can not overlap current kernel\n");
> return;
> }
> +
> + reserve_crashkernel_generic(crash_size, crash_base, 0, false);
> }
>
> int __init overlaps_crashkernel(unsigned long start, unsigned long size)
Powered by blists - more mailing lists