On 13/03/23 21:55, Laurent Dufour wrote:
On 12/03/2023 19:11:51, Sourabh Jain wrote:
A new member "fdt_index" is added to the kimage_arch struct to hold
the index of the FDT (Flattened Device Tree) segment from the kexec
the segment array.
fdt_index will provide direct access to the FDT segment in the kexec
segment array after the kdump kernel is loaded.
The new attribute will be used in the arch crash hotplug handler
(added in upcoming patches) on every CPU and memory hotplug event.
The fdt_index is populated for both kexec_load and kexec_file_load
case.
Signed-off-by: Sourabh Jain <sourabhjain@xxxxxxxxxxxxx>
---
arch/powerpc/include/asm/kexec.h | 5 +++++
arch/powerpc/kexec/core_64.c | 31 +++++++++++++++++++++++++++++++
2 files changed, 36 insertions(+)
diff --git a/arch/powerpc/include/asm/kexec.h b/arch/powerpc/include/asm/kexec.h
index 8090ad7d97d9d..348eb96e8ca67 100644
--- a/arch/powerpc/include/asm/kexec.h
+++ b/arch/powerpc/include/asm/kexec.h
@@ -103,6 +103,8 @@ void kexec_copy_flush(struct kimage *image);
struct crash_mem;
int update_cpus_node(void *fdt);
int get_crash_memory_ranges(struct crash_mem **mem_ranges);
+int machine_kexec_post_load(struct kimage *image);
+#define machine_kexec_post_load machine_kexec_post_load
Minor comment, when CONFIG_CRASH_HOTPLUG is not set the function is simply
returning 0, why not defining it has an inline in that case?
We can, but if the initialization of fdt_index is taken care during the
allocation
of kimage struct, I will move the fdt_index discovery logic to arch crash
hotplug handler. More on fdt_index initialization in the next comment.
#endif
#if defined(CONFIG_CRASH_DUMP) && defined(CONFIG_PPC_RTAS)
@@ -118,6 +120,9 @@ extern const struct kexec_file_ops kexec_elf64_ops;
struct kimage_arch {
struct crash_mem *exclude_ranges;
+#if defined(CONFIG_CRASH_HOTPLUG)
+ int fdt_index;
+#endif
unsigned long backup_start;
void *backup_buf;
void *fdt;
diff --git a/arch/powerpc/kexec/core_64.c b/arch/powerpc/kexec/core_64.c
index 0b292f93a74cc..531486c973988 100644
--- a/arch/powerpc/kexec/core_64.c
+++ b/arch/powerpc/kexec/core_64.c
@@ -77,6 +77,37 @@ int machine_kexec_prepare(struct kimage *image)
return 0;
}
+int machine_kexec_post_load(struct kimage *kimage)
+{
+#if defined(CONFIG_CRASH_HOTPLUG)
+ int i;
+ void *ptr;
+ unsigned long mem;
+
+ /* Mark fdt_index invalid */
+ kimage->arch.fdt_index = -1;
Why is that not done in the series introducing the generic
crash hotplug update, in do_kimage_alloc_init() ?
do_kimage_alloc_init is a generic function where as fdt_index is a ppc
specific
attribute. If fdt_index is initialized in do_kimage_alloc_init then
other architectures
will have build issues.
This way there is a guarantee that the field will not be used while set by
default to 0.
I agree that until the kernel hits the machine_kexec_post_load function
on the
kdump kernel load path there is no way to identify the fdt_index is holding
a valid index or not.
Since there is no consumer of kimage's fdt_index attribute from the
point of its
allocation to until it is initialized, we don't have any impact of
fdt_index holding
value 0 (which is not valid) for sometime.
But still we can do few things to allow fdt_index attribute consumers to
check
the sanity of fdt_index.
1. Introduce arch specific function call to initialize kimage_arch
struct (basically fdt_index for now).
and call it inside do_kimage_alloc_init and initialize the fdt_index
with -1 there.
2. Add another attribute in kimage_arch struct to indicate the sanity of the
fdt_index attribute. For example fdt_index_valid, if this holds zero then
the fdt_index holds invalid index. (looks inefficient to me)
Not sure is it worth doing but please let me know your opinion.
Thanks,
Sourabh Jain
_______________________________________________
kexec mailing list
kexec@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/kexec
