* [PATCH v2 1/3] powerpc/fadump: make is_fadump_active() visible for exporting vmcore
@ 2023-09-05 18:36 Hari Bathini
2023-09-05 18:36 ` [PATCH v2 2/3] vmcore: allow fadump to export vmcore even if is_kdump_kernel() is false Hari Bathini
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Hari Bathini @ 2023-09-05 18:36 UTC (permalink / raw)
To: lkml, linuxppc-dev, Kexec-ml
Cc: Sourabh Jain, Dave Young, Mahesh J Salgaonkar, Baoquan He
Include asm/fadump.h in asm/kexec.h to make it visible while exporting
vmcore. Also, update is_fadump_active() to return boolean instead of
integer for better readability. The change will be used in the next
patch to ensure vmcore is exported when fadump is active.
Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
---
Changes in v2:
* New patch based on Baoquan's suggestion to use is_fadump_active()
instead of introducing new function is_crashdump_kernel().
arch/powerpc/include/asm/fadump.h | 4 ++--
arch/powerpc/include/asm/kexec.h | 8 ++++++--
arch/powerpc/kernel/fadump.c | 4 ++--
3 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/include/asm/fadump.h b/arch/powerpc/include/asm/fadump.h
index 526a6a647312..27b74a7e2162 100644
--- a/arch/powerpc/include/asm/fadump.h
+++ b/arch/powerpc/include/asm/fadump.h
@@ -15,13 +15,13 @@ extern int crashing_cpu;
extern int is_fadump_memory_area(u64 addr, ulong size);
extern int setup_fadump(void);
-extern int is_fadump_active(void);
+extern bool is_fadump_active(void);
extern int should_fadump_crash(void);
extern void crash_fadump(struct pt_regs *, const char *);
extern void fadump_cleanup(void);
#else /* CONFIG_FA_DUMP */
-static inline int is_fadump_active(void) { return 0; }
+static inline bool is_fadump_active(void) { return false; }
static inline int should_fadump_crash(void) { return 0; }
static inline void crash_fadump(struct pt_regs *regs, const char *str) { }
static inline void fadump_cleanup(void) { }
diff --git a/arch/powerpc/include/asm/kexec.h b/arch/powerpc/include/asm/kexec.h
index a1ddba01e7d1..b760ef459234 100644
--- a/arch/powerpc/include/asm/kexec.h
+++ b/arch/powerpc/include/asm/kexec.h
@@ -51,6 +51,7 @@
#ifndef __ASSEMBLY__
#include <asm/reg.h>
+#include <asm/fadump.h>
typedef void (*crash_shutdown_t)(void);
@@ -99,10 +100,13 @@ void relocate_new_kernel(unsigned long indirection_page, unsigned long reboot_co
void kexec_copy_flush(struct kimage *image);
-#if defined(CONFIG_CRASH_DUMP) && defined(CONFIG_PPC_RTAS)
+#if defined(CONFIG_CRASH_DUMP)
+#define is_fadump_active is_fadump_active
+#if defined(CONFIG_PPC_RTAS)
void crash_free_reserved_phys_range(unsigned long begin, unsigned long end);
#define crash_free_reserved_phys_range crash_free_reserved_phys_range
-#endif
+#endif /* CONFIG_PPC_RTAS */
+#endif /* CONFIG_CRASH_DUMP */
#ifdef CONFIG_KEXEC_FILE
extern const struct kexec_file_ops kexec_elf64_ops;
diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
index 3ff2da7b120b..5682a65e8326 100644
--- a/arch/powerpc/kernel/fadump.c
+++ b/arch/powerpc/kernel/fadump.c
@@ -187,9 +187,9 @@ int should_fadump_crash(void)
return 1;
}
-int is_fadump_active(void)
+bool is_fadump_active(void)
{
- return fw_dump.dump_active;
+ return !!fw_dump.dump_active;
}
/*
--
2.41.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 2/3] vmcore: allow fadump to export vmcore even if is_kdump_kernel() is false
2023-09-05 18:36 [PATCH v2 1/3] powerpc/fadump: make is_fadump_active() visible for exporting vmcore Hari Bathini
@ 2023-09-05 18:36 ` Hari Bathini
2023-09-11 7:13 ` Michael Ellerman
2023-09-05 18:36 ` [PATCH v2 3/3] powerpc/fadump: make is_kdump_kernel() return false when fadump is active Hari Bathini
2023-09-07 5:42 ` [PATCH v2 1/3] powerpc/fadump: make is_fadump_active() visible for exporting vmcore Baoquan He
2 siblings, 1 reply; 8+ messages in thread
From: Hari Bathini @ 2023-09-05 18:36 UTC (permalink / raw)
To: lkml, linuxppc-dev, Kexec-ml
Cc: Sourabh Jain, Dave Young, Mahesh J Salgaonkar, Baoquan He
Currently, is_kdump_kernel() returns true when elfcorehdr_addr is set.
While elfcorehdr_addr is set for kexec based kernel dump mechanism,
alternate dump capturing methods like fadump [1] also set it to export
the vmcore. Since, is_kdump_kernel() is used to restrict resources in
crash dump capture kernel and such restrictions are not desirable for
fadump, allow is_kdump_kernel() to be defined differently for fadump
case. With that change, include is_fadump_active() check in functions
is_vmcore_usable() & vmcore_unusable() to be able to export vmcore for
fadump case too.
[1] https://docs.kernel.org/powerpc/firmware-assisted-dump.html
Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
---
Changes in v2:
* is_fadump_active() check added to is_vmcore_usable() as suggested
by Baoquan.
include/linux/crash_dump.h | 19 +++++++++++++++++--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/include/linux/crash_dump.h b/include/linux/crash_dump.h
index 0f3a656293b0..de8a9fabfb6f 100644
--- a/include/linux/crash_dump.h
+++ b/include/linux/crash_dump.h
@@ -50,6 +50,7 @@ void vmcore_cleanup(void);
#define vmcore_elf64_check_arch(x) (elf_check_arch(x) || vmcore_elf_check_arch_cross(x))
#endif
+#ifndef is_kdump_kernel
/*
* is_kdump_kernel() checks whether this kernel is booting after a panic of
* previous kernel or not. This is determined by checking if previous kernel
@@ -64,6 +65,19 @@ static inline bool is_kdump_kernel(void)
{
return elfcorehdr_addr != ELFCORE_ADDR_MAX;
}
+#endif
+
+#ifndef is_fadump_active
+/*
+ * If f/w assisted dump capturing mechanism (fadump), instead of kexec based
+ * dump capturing mechanism (kdump) is exporting the vmcore, then this function
+ * will be defined in arch specific code to return true, when appropriate.
+ */
+static inline bool is_fadump_active(void)
+{
+ return false;
+}
+#endif
/* is_vmcore_usable() checks if the kernel is booting after a panic and
* the vmcore region is usable.
@@ -75,7 +89,8 @@ static inline bool is_kdump_kernel(void)
static inline int is_vmcore_usable(void)
{
- return is_kdump_kernel() && elfcorehdr_addr != ELFCORE_ADDR_ERR ? 1 : 0;
+ return (is_kdump_kernel() || is_fadump_active())
+ && elfcorehdr_addr != ELFCORE_ADDR_ERR ? 1 : 0;
}
/* vmcore_unusable() marks the vmcore as unusable,
@@ -84,7 +99,7 @@ static inline int is_vmcore_usable(void)
static inline void vmcore_unusable(void)
{
- if (is_kdump_kernel())
+ if (is_kdump_kernel() || is_fadump_active())
elfcorehdr_addr = ELFCORE_ADDR_ERR;
}
--
2.41.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 3/3] powerpc/fadump: make is_kdump_kernel() return false when fadump is active
2023-09-05 18:36 [PATCH v2 1/3] powerpc/fadump: make is_fadump_active() visible for exporting vmcore Hari Bathini
2023-09-05 18:36 ` [PATCH v2 2/3] vmcore: allow fadump to export vmcore even if is_kdump_kernel() is false Hari Bathini
@ 2023-09-05 18:36 ` Hari Bathini
2023-09-07 5:42 ` [PATCH v2 1/3] powerpc/fadump: make is_fadump_active() visible for exporting vmcore Baoquan He
2 siblings, 0 replies; 8+ messages in thread
From: Hari Bathini @ 2023-09-05 18:36 UTC (permalink / raw)
To: lkml, linuxppc-dev, Kexec-ml
Cc: Sourabh Jain, Dave Young, Mahesh J Salgaonkar, Baoquan He
Currently, is_kdump_kernel() returns true in crash dump capture kernel
for both kdump and fadump crash dump capturing methods, as both these
methods set elfcorehdr_addr. Some restrictions enforced for crash dump
capture kernel, based on is_kdump_kernel(), are specifically meant for
kdump case and not desirable for fadump - eg. IO queues restriction in
device drivers. So, define is_kdump_kernel() to return false when f/w
assisted dump is active.
Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
---
arch/powerpc/include/asm/kexec.h | 2 ++
arch/powerpc/kernel/crash_dump.c | 11 +++++++++++
2 files changed, 13 insertions(+)
diff --git a/arch/powerpc/include/asm/kexec.h b/arch/powerpc/include/asm/kexec.h
index b760ef459234..f0b9c3fd0618 100644
--- a/arch/powerpc/include/asm/kexec.h
+++ b/arch/powerpc/include/asm/kexec.h
@@ -101,6 +101,8 @@ void relocate_new_kernel(unsigned long indirection_page, unsigned long reboot_co
void kexec_copy_flush(struct kimage *image);
#if defined(CONFIG_CRASH_DUMP)
+bool is_kdump_kernel(void);
+#define is_kdump_kernel is_kdump_kernel
#define is_fadump_active is_fadump_active
#if defined(CONFIG_PPC_RTAS)
void crash_free_reserved_phys_range(unsigned long begin, unsigned long end);
diff --git a/arch/powerpc/kernel/crash_dump.c b/arch/powerpc/kernel/crash_dump.c
index 9a3b85bfc83f..6d8e616ce3ce 100644
--- a/arch/powerpc/kernel/crash_dump.c
+++ b/arch/powerpc/kernel/crash_dump.c
@@ -92,6 +92,17 @@ ssize_t copy_oldmem_page(struct iov_iter *iter, unsigned long pfn,
return csize;
}
+/*
+ * Return true only when kexec based kernel dump capturing method is used.
+ * This ensures all restritions applied for kdump case are not automatically
+ * applied for fadump case.
+ */
+bool is_kdump_kernel(void)
+{
+ return !is_fadump_active() && elfcorehdr_addr != ELFCORE_ADDR_MAX;
+}
+EXPORT_SYMBOL_GPL(is_kdump_kernel);
+
#ifdef CONFIG_PPC_RTAS
/*
* The crashkernel region will almost always overlap the RTAS region, so
--
2.41.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/3] powerpc/fadump: make is_fadump_active() visible for exporting vmcore
2023-09-05 18:36 [PATCH v2 1/3] powerpc/fadump: make is_fadump_active() visible for exporting vmcore Hari Bathini
2023-09-05 18:36 ` [PATCH v2 2/3] vmcore: allow fadump to export vmcore even if is_kdump_kernel() is false Hari Bathini
2023-09-05 18:36 ` [PATCH v2 3/3] powerpc/fadump: make is_kdump_kernel() return false when fadump is active Hari Bathini
@ 2023-09-07 5:42 ` Baoquan He
2023-09-08 5:49 ` Hari Bathini
2 siblings, 1 reply; 8+ messages in thread
From: Baoquan He @ 2023-09-07 5:42 UTC (permalink / raw)
To: Hari Bathini
Cc: linuxppc-dev, Kexec-ml, lkml, Sourabh Jain, Mahesh J Salgaonkar,
Dave Young
On 09/06/23 at 12:06am, Hari Bathini wrote:
> Include asm/fadump.h in asm/kexec.h to make it visible while exporting
> vmcore. Also, update is_fadump_active() to return boolean instead of
> integer for better readability. The change will be used in the next
> patch to ensure vmcore is exported when fadump is active.
>
> Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
Thanks, Hari. The whole series looks good to me.
Acked-by: Baoquan He <bhe@redhat.com>
Since it's a power specific change, should be picked into powerpc tree?
Thanks
Baoquan
> ---
>
> Changes in v2:
> * New patch based on Baoquan's suggestion to use is_fadump_active()
> instead of introducing new function is_crashdump_kernel().
>
>
> arch/powerpc/include/asm/fadump.h | 4 ++--
> arch/powerpc/include/asm/kexec.h | 8 ++++++--
> arch/powerpc/kernel/fadump.c | 4 ++--
> 3 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/fadump.h b/arch/powerpc/include/asm/fadump.h
> index 526a6a647312..27b74a7e2162 100644
> --- a/arch/powerpc/include/asm/fadump.h
> +++ b/arch/powerpc/include/asm/fadump.h
> @@ -15,13 +15,13 @@ extern int crashing_cpu;
>
> extern int is_fadump_memory_area(u64 addr, ulong size);
> extern int setup_fadump(void);
> -extern int is_fadump_active(void);
> +extern bool is_fadump_active(void);
> extern int should_fadump_crash(void);
> extern void crash_fadump(struct pt_regs *, const char *);
> extern void fadump_cleanup(void);
>
> #else /* CONFIG_FA_DUMP */
> -static inline int is_fadump_active(void) { return 0; }
> +static inline bool is_fadump_active(void) { return false; }
> static inline int should_fadump_crash(void) { return 0; }
> static inline void crash_fadump(struct pt_regs *regs, const char *str) { }
> static inline void fadump_cleanup(void) { }
> diff --git a/arch/powerpc/include/asm/kexec.h b/arch/powerpc/include/asm/kexec.h
> index a1ddba01e7d1..b760ef459234 100644
> --- a/arch/powerpc/include/asm/kexec.h
> +++ b/arch/powerpc/include/asm/kexec.h
> @@ -51,6 +51,7 @@
>
> #ifndef __ASSEMBLY__
> #include <asm/reg.h>
> +#include <asm/fadump.h>
>
> typedef void (*crash_shutdown_t)(void);
>
> @@ -99,10 +100,13 @@ void relocate_new_kernel(unsigned long indirection_page, unsigned long reboot_co
>
> void kexec_copy_flush(struct kimage *image);
>
> -#if defined(CONFIG_CRASH_DUMP) && defined(CONFIG_PPC_RTAS)
> +#if defined(CONFIG_CRASH_DUMP)
> +#define is_fadump_active is_fadump_active
> +#if defined(CONFIG_PPC_RTAS)
> void crash_free_reserved_phys_range(unsigned long begin, unsigned long end);
> #define crash_free_reserved_phys_range crash_free_reserved_phys_range
> -#endif
> +#endif /* CONFIG_PPC_RTAS */
> +#endif /* CONFIG_CRASH_DUMP */
>
> #ifdef CONFIG_KEXEC_FILE
> extern const struct kexec_file_ops kexec_elf64_ops;
> diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
> index 3ff2da7b120b..5682a65e8326 100644
> --- a/arch/powerpc/kernel/fadump.c
> +++ b/arch/powerpc/kernel/fadump.c
> @@ -187,9 +187,9 @@ int should_fadump_crash(void)
> return 1;
> }
>
> -int is_fadump_active(void)
> +bool is_fadump_active(void)
> {
> - return fw_dump.dump_active;
> + return !!fw_dump.dump_active;
> }
>
> /*
> --
> 2.41.0
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/3] powerpc/fadump: make is_fadump_active() visible for exporting vmcore
2023-09-07 5:42 ` [PATCH v2 1/3] powerpc/fadump: make is_fadump_active() visible for exporting vmcore Baoquan He
@ 2023-09-08 5:49 ` Hari Bathini
0 siblings, 0 replies; 8+ messages in thread
From: Hari Bathini @ 2023-09-08 5:49 UTC (permalink / raw)
To: Baoquan He, Michael Ellerman
Cc: linuxppc-dev, Kexec-ml, lkml, Sourabh Jain, Mahesh J Salgaonkar,
Dave Young
Thanks, Baoquan.
On 07/09/23 11:12 am, Baoquan He wrote:
> On 09/06/23 at 12:06am, Hari Bathini wrote:
>> Include asm/fadump.h in asm/kexec.h to make it visible while exporting
>> vmcore. Also, update is_fadump_active() to return boolean instead of
>> integer for better readability. The change will be used in the next
>> patch to ensure vmcore is exported when fadump is active.
>>
>> Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
>
> Thanks, Hari. The whole series looks good to me.
>
> Acked-by: Baoquan He <bhe@redhat.com>
>
> Since it's a power specific change, should be picked into powerpc tree?
Michael, would you mind taking the series via powerpc tree..
Thanks
Hari
>> ---
>>
>> Changes in v2:
>> * New patch based on Baoquan's suggestion to use is_fadump_active()
>> instead of introducing new function is_crashdump_kernel().
>>
>>
>> arch/powerpc/include/asm/fadump.h | 4 ++--
>> arch/powerpc/include/asm/kexec.h | 8 ++++++--
>> arch/powerpc/kernel/fadump.c | 4 ++--
>> 3 files changed, 10 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/fadump.h b/arch/powerpc/include/asm/fadump.h
>> index 526a6a647312..27b74a7e2162 100644
>> --- a/arch/powerpc/include/asm/fadump.h
>> +++ b/arch/powerpc/include/asm/fadump.h
>> @@ -15,13 +15,13 @@ extern int crashing_cpu;
>>
>> extern int is_fadump_memory_area(u64 addr, ulong size);
>> extern int setup_fadump(void);
>> -extern int is_fadump_active(void);
>> +extern bool is_fadump_active(void);
>> extern int should_fadump_crash(void);
>> extern void crash_fadump(struct pt_regs *, const char *);
>> extern void fadump_cleanup(void);
>>
>> #else /* CONFIG_FA_DUMP */
>> -static inline int is_fadump_active(void) { return 0; }
>> +static inline bool is_fadump_active(void) { return false; }
>> static inline int should_fadump_crash(void) { return 0; }
>> static inline void crash_fadump(struct pt_regs *regs, const char *str) { }
>> static inline void fadump_cleanup(void) { }
>> diff --git a/arch/powerpc/include/asm/kexec.h b/arch/powerpc/include/asm/kexec.h
>> index a1ddba01e7d1..b760ef459234 100644
>> --- a/arch/powerpc/include/asm/kexec.h
>> +++ b/arch/powerpc/include/asm/kexec.h
>> @@ -51,6 +51,7 @@
>>
>> #ifndef __ASSEMBLY__
>> #include <asm/reg.h>
>> +#include <asm/fadump.h>
>>
>> typedef void (*crash_shutdown_t)(void);
>>
>> @@ -99,10 +100,13 @@ void relocate_new_kernel(unsigned long indirection_page, unsigned long reboot_co
>>
>> void kexec_copy_flush(struct kimage *image);
>>
>> -#if defined(CONFIG_CRASH_DUMP) && defined(CONFIG_PPC_RTAS)
>> +#if defined(CONFIG_CRASH_DUMP)
>> +#define is_fadump_active is_fadump_active
>> +#if defined(CONFIG_PPC_RTAS)
>> void crash_free_reserved_phys_range(unsigned long begin, unsigned long end);
>> #define crash_free_reserved_phys_range crash_free_reserved_phys_range
>> -#endif
>> +#endif /* CONFIG_PPC_RTAS */
>> +#endif /* CONFIG_CRASH_DUMP */
>>
>> #ifdef CONFIG_KEXEC_FILE
>> extern const struct kexec_file_ops kexec_elf64_ops;
>> diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
>> index 3ff2da7b120b..5682a65e8326 100644
>> --- a/arch/powerpc/kernel/fadump.c
>> +++ b/arch/powerpc/kernel/fadump.c
>> @@ -187,9 +187,9 @@ int should_fadump_crash(void)
>> return 1;
>> }
>>
>> -int is_fadump_active(void)
>> +bool is_fadump_active(void)
>> {
>> - return fw_dump.dump_active;
>> + return !!fw_dump.dump_active;
>> }
>>
>> /*
>> --
>> 2.41.0
>>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/3] vmcore: allow fadump to export vmcore even if is_kdump_kernel() is false
2023-09-05 18:36 ` [PATCH v2 2/3] vmcore: allow fadump to export vmcore even if is_kdump_kernel() is false Hari Bathini
@ 2023-09-11 7:13 ` Michael Ellerman
2023-09-11 10:31 ` Baoquan He
0 siblings, 1 reply; 8+ messages in thread
From: Michael Ellerman @ 2023-09-11 7:13 UTC (permalink / raw)
To: Hari Bathini, lkml, linuxppc-dev, Kexec-ml
Cc: Sourabh Jain, Dave Young, Mahesh J Salgaonkar, Baoquan He
Hari Bathini <hbathini@linux.ibm.com> writes:
> Currently, is_kdump_kernel() returns true when elfcorehdr_addr is set.
> While elfcorehdr_addr is set for kexec based kernel dump mechanism,
> alternate dump capturing methods like fadump [1] also set it to export
> the vmcore. Since, is_kdump_kernel() is used to restrict resources in
> crash dump capture kernel and such restrictions are not desirable for
> fadump, allow is_kdump_kernel() to be defined differently for fadump
> case. With that change, include is_fadump_active() check in functions
> is_vmcore_usable() & vmcore_unusable() to be able to export vmcore for
> fadump case too.
...
> diff --git a/include/linux/crash_dump.h b/include/linux/crash_dump.h
> index 0f3a656293b0..de8a9fabfb6f 100644
> --- a/include/linux/crash_dump.h
> +++ b/include/linux/crash_dump.h
> @@ -50,6 +50,7 @@ void vmcore_cleanup(void);
> #define vmcore_elf64_check_arch(x) (elf_check_arch(x) || vmcore_elf_check_arch_cross(x))
> #endif
>
> +#ifndef is_kdump_kernel
> /*
> * is_kdump_kernel() checks whether this kernel is booting after a panic of
> * previous kernel or not. This is determined by checking if previous kernel
> @@ -64,6 +65,19 @@ static inline bool is_kdump_kernel(void)
> {
> return elfcorehdr_addr != ELFCORE_ADDR_MAX;
> }
> +#endif
> +
> +#ifndef is_fadump_active
> +/*
> + * If f/w assisted dump capturing mechanism (fadump), instead of kexec based
> + * dump capturing mechanism (kdump) is exporting the vmcore, then this function
> + * will be defined in arch specific code to return true, when appropriate.
> + */
> +static inline bool is_fadump_active(void)
> +{
> + return false;
> +}
> +#endif
>
> /* is_vmcore_usable() checks if the kernel is booting after a panic and
> * the vmcore region is usable.
> @@ -75,7 +89,8 @@ static inline bool is_kdump_kernel(void)
>
> static inline int is_vmcore_usable(void)
> {
> - return is_kdump_kernel() && elfcorehdr_addr != ELFCORE_ADDR_ERR ? 1 : 0;
> + return (is_kdump_kernel() || is_fadump_active())
> + && elfcorehdr_addr != ELFCORE_ADDR_ERR ? 1 : 0;
> }
>
> /* vmcore_unusable() marks the vmcore as unusable,
> @@ -84,7 +99,7 @@ static inline int is_vmcore_usable(void)
>
> static inline void vmcore_unusable(void)
> {
> - if (is_kdump_kernel())
> + if (is_kdump_kernel() || is_fadump_active())
> elfcorehdr_addr = ELFCORE_ADDR_ERR;
> }
I think it would be cleaner to decouple is_vmcore_usable() and
vmcore_usable() from is_kdump_kernel().
ie, make them operate solely based on the value of elforehdr_addr:
static inline int is_vmcore_usable(void)
{
elfcorehdr_addr != ELFCORE_ADDR_ERR && \
elfcorehdr_addr != ELFCORE_ADDR_MAX;
}
static inline void vmcore_unusable(void)
{
elfcorehdr_addr = ELFCORE_ADDR_ERR;
}
Then all we need on powerpc is a way to override is_kdump_kernel().
cheers
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/3] vmcore: allow fadump to export vmcore even if is_kdump_kernel() is false
2023-09-11 7:13 ` Michael Ellerman
@ 2023-09-11 10:31 ` Baoquan He
2023-09-12 8:31 ` Hari Bathini
0 siblings, 1 reply; 8+ messages in thread
From: Baoquan He @ 2023-09-11 10:31 UTC (permalink / raw)
To: Michael Ellerman
Cc: linuxppc-dev, Kexec-ml, lkml, Sourabh Jain, Mahesh J Salgaonkar,
Dave Young, Hari Bathini
On 09/11/23 at 05:13pm, Michael Ellerman wrote:
> Hari Bathini <hbathini@linux.ibm.com> writes:
> > Currently, is_kdump_kernel() returns true when elfcorehdr_addr is set.
> > While elfcorehdr_addr is set for kexec based kernel dump mechanism,
> > alternate dump capturing methods like fadump [1] also set it to export
> > the vmcore. Since, is_kdump_kernel() is used to restrict resources in
> > crash dump capture kernel and such restrictions are not desirable for
> > fadump, allow is_kdump_kernel() to be defined differently for fadump
> > case. With that change, include is_fadump_active() check in functions
> > is_vmcore_usable() & vmcore_unusable() to be able to export vmcore for
> > fadump case too.
> ...
> > diff --git a/include/linux/crash_dump.h b/include/linux/crash_dump.h
> > index 0f3a656293b0..de8a9fabfb6f 100644
> > --- a/include/linux/crash_dump.h
> > +++ b/include/linux/crash_dump.h
> > @@ -50,6 +50,7 @@ void vmcore_cleanup(void);
> > #define vmcore_elf64_check_arch(x) (elf_check_arch(x) || vmcore_elf_check_arch_cross(x))
> > #endif
> >
> > +#ifndef is_kdump_kernel
> > /*
> > * is_kdump_kernel() checks whether this kernel is booting after a panic of
> > * previous kernel or not. This is determined by checking if previous kernel
> > @@ -64,6 +65,19 @@ static inline bool is_kdump_kernel(void)
> > {
> > return elfcorehdr_addr != ELFCORE_ADDR_MAX;
> > }
> > +#endif
> > +
> > +#ifndef is_fadump_active
> > +/*
> > + * If f/w assisted dump capturing mechanism (fadump), instead of kexec based
> > + * dump capturing mechanism (kdump) is exporting the vmcore, then this function
> > + * will be defined in arch specific code to return true, when appropriate.
> > + */
> > +static inline bool is_fadump_active(void)
> > +{
> > + return false;
> > +}
> > +#endif
> >
> > /* is_vmcore_usable() checks if the kernel is booting after a panic and
> > * the vmcore region is usable.
> > @@ -75,7 +89,8 @@ static inline bool is_kdump_kernel(void)
> >
> > static inline int is_vmcore_usable(void)
> > {
> > - return is_kdump_kernel() && elfcorehdr_addr != ELFCORE_ADDR_ERR ? 1 : 0;
> > + return (is_kdump_kernel() || is_fadump_active())
> > + && elfcorehdr_addr != ELFCORE_ADDR_ERR ? 1 : 0;
> > }
> >
> > /* vmcore_unusable() marks the vmcore as unusable,
> > @@ -84,7 +99,7 @@ static inline int is_vmcore_usable(void)
> >
> > static inline void vmcore_unusable(void)
> > {
> > - if (is_kdump_kernel())
> > + if (is_kdump_kernel() || is_fadump_active())
> > elfcorehdr_addr = ELFCORE_ADDR_ERR;
> > }
>
> I think it would be cleaner to decouple is_vmcore_usable() and
> vmcore_usable() from is_kdump_kernel().
>
> ie, make them operate solely based on the value of elforehdr_addr:
>
> static inline int is_vmcore_usable(void)
> {
> elfcorehdr_addr != ELFCORE_ADDR_ERR && \
> elfcorehdr_addr != ELFCORE_ADDR_MAX;
Agree. I fell into the blind corner of thinking earlier. Above change
is better.
> }
>
> static inline void vmcore_unusable(void)
> {
> elfcorehdr_addr = ELFCORE_ADDR_ERR;
> }
>
>
> Then all we need on powerpc is a way to override is_kdump_kernel().
>
> cheers
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/3] vmcore: allow fadump to export vmcore even if is_kdump_kernel() is false
2023-09-11 10:31 ` Baoquan He
@ 2023-09-12 8:31 ` Hari Bathini
0 siblings, 0 replies; 8+ messages in thread
From: Hari Bathini @ 2023-09-12 8:31 UTC (permalink / raw)
To: Baoquan He, Michael Ellerman
Cc: linuxppc-dev, Kexec-ml, lkml, Sourabh Jain, Mahesh J Salgaonkar,
Dave Young
On 11/09/23 4:01 pm, Baoquan He wrote:
> On 09/11/23 at 05:13pm, Michael Ellerman wrote:
>> Hari Bathini <hbathini@linux.ibm.com> writes:
>>> Currently, is_kdump_kernel() returns true when elfcorehdr_addr is set.
>>> While elfcorehdr_addr is set for kexec based kernel dump mechanism,
>>> alternate dump capturing methods like fadump [1] also set it to export
>>> the vmcore. Since, is_kdump_kernel() is used to restrict resources in
>>> crash dump capture kernel and such restrictions are not desirable for
>>> fadump, allow is_kdump_kernel() to be defined differently for fadump
>>> case. With that change, include is_fadump_active() check in functions
>>> is_vmcore_usable() & vmcore_unusable() to be able to export vmcore for
>>> fadump case too.
>> ...
>>> diff --git a/include/linux/crash_dump.h b/include/linux/crash_dump.h
>>> index 0f3a656293b0..de8a9fabfb6f 100644
>>> --- a/include/linux/crash_dump.h
>>> +++ b/include/linux/crash_dump.h
>>> @@ -50,6 +50,7 @@ void vmcore_cleanup(void);
>>> #define vmcore_elf64_check_arch(x) (elf_check_arch(x) || vmcore_elf_check_arch_cross(x))
>>> #endif
>>>
>>> +#ifndef is_kdump_kernel
>>> /*
>>> * is_kdump_kernel() checks whether this kernel is booting after a panic of
>>> * previous kernel or not. This is determined by checking if previous kernel
>>> @@ -64,6 +65,19 @@ static inline bool is_kdump_kernel(void)
>>> {
>>> return elfcorehdr_addr != ELFCORE_ADDR_MAX;
>>> }
>>> +#endif
>>> +
>>> +#ifndef is_fadump_active
>>> +/*
>>> + * If f/w assisted dump capturing mechanism (fadump), instead of kexec based
>>> + * dump capturing mechanism (kdump) is exporting the vmcore, then this function
>>> + * will be defined in arch specific code to return true, when appropriate.
>>> + */
>>> +static inline bool is_fadump_active(void)
>>> +{
>>> + return false;
>>> +}
>>> +#endif
>>>
>>> /* is_vmcore_usable() checks if the kernel is booting after a panic and
>>> * the vmcore region is usable.
>>> @@ -75,7 +89,8 @@ static inline bool is_kdump_kernel(void)
>>>
>>> static inline int is_vmcore_usable(void)
>>> {
>>> - return is_kdump_kernel() && elfcorehdr_addr != ELFCORE_ADDR_ERR ? 1 : 0;
>>> + return (is_kdump_kernel() || is_fadump_active())
>>> + && elfcorehdr_addr != ELFCORE_ADDR_ERR ? 1 : 0;
>>> }
>>>
>>> /* vmcore_unusable() marks the vmcore as unusable,
>>> @@ -84,7 +99,7 @@ static inline int is_vmcore_usable(void)
>>>
>>> static inline void vmcore_unusable(void)
>>> {
>>> - if (is_kdump_kernel())
>>> + if (is_kdump_kernel() || is_fadump_active())
>>> elfcorehdr_addr = ELFCORE_ADDR_ERR;
>>> }
>>
>> I think it would be cleaner to decouple is_vmcore_usable() and
>> vmcore_usable() from is_kdump_kernel().
>>
>> ie, make them operate solely based on the value of elforehdr_addr:
>>
>> static inline int is_vmcore_usable(void)
>> {
>> elfcorehdr_addr != ELFCORE_ADDR_ERR && \
>> elfcorehdr_addr != ELFCORE_ADDR_MAX;
>
> Agree. I fell into the blind corner of thinking earlier. Above change
> is better.
Thanks for the reviews.
Posted v3.
- Hari
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-09-12 9:07 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-05 18:36 [PATCH v2 1/3] powerpc/fadump: make is_fadump_active() visible for exporting vmcore Hari Bathini
2023-09-05 18:36 ` [PATCH v2 2/3] vmcore: allow fadump to export vmcore even if is_kdump_kernel() is false Hari Bathini
2023-09-11 7:13 ` Michael Ellerman
2023-09-11 10:31 ` Baoquan He
2023-09-12 8:31 ` Hari Bathini
2023-09-05 18:36 ` [PATCH v2 3/3] powerpc/fadump: make is_kdump_kernel() return false when fadump is active Hari Bathini
2023-09-07 5:42 ` [PATCH v2 1/3] powerpc/fadump: make is_fadump_active() visible for exporting vmcore Baoquan He
2023-09-08 5:49 ` Hari Bathini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).