| Message ID | 20200326032420.27220-9-pasha.tatashin@soleen.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: MMU enabled kexec relocation | expand |
Hi Pavel, On 26/03/2020 03:24, Pavel Tatashin wrote: > Currently, kernel relocation function is configured in machine_kexec() > at the time of kexec reboot by using control_code_page. > > This operation, however, is more logical to be done during kexec_load, > and thus remove from reboot time. Move, setup of this function to > newly added machine_kexec_post_load(). This would avoid the need to special-case the cache maintenance, so its a good cleanup... > Because once MMU is enabled, kexec control page will contain more than > relocation kernel, but also vector table, add pointer to the actual > function within this page arch.kern_reloc. Currently, it equals to the > beginning of page, we will add offsets later, when vector table is > added. If the vector table always comes second, wouldn't this be extra work to hold the value 0? You can control the layout of this relocation code, as it has to be written in assembly. I don't get why this would be necessary. > diff --git a/arch/arm64/kernel/machine_kexec.c b/arch/arm64/kernel/machine_kexec.c > index ae1bad0156cd..ec71a153cc2d 100644 > --- a/arch/arm64/kernel/machine_kexec.c > +++ b/arch/arm64/kernel/machine_kexec.c > @@ -58,6 +59,17 @@ void machine_kexec_cleanup(struct kimage *kimage) > /* Empty routine needed to avoid build errors. */ > } > > +int machine_kexec_post_load(struct kimage *kimage) > +{ > + void *reloc_code = page_to_virt(kimage->control_code_page); > + > + memcpy(reloc_code, arm64_relocate_new_kernel, > + arm64_relocate_new_kernel_size); > + kimage->arch.kern_reloc = __pa(reloc_code); Could we move the two cache maintenance calls for this area in here too. Keeping it next to the modification makes it clearer why it is required. In this case we can use flush_icache_range() instead of its __variant because this now happens much earlier. > + return 0; > +} Regardless, Reviewed-by: James Morse <james.morse@arm.com> Thanks, James
On Wed, Apr 29, 2020 at 1:01 PM James Morse <james.morse@arm.com> wrote: > > Hi Pavel, > > On 26/03/2020 03:24, Pavel Tatashin wrote: > > Currently, kernel relocation function is configured in machine_kexec() > > at the time of kexec reboot by using control_code_page. > > > > This operation, however, is more logical to be done during kexec_load, > > and thus remove from reboot time. Move, setup of this function to > > newly added machine_kexec_post_load(). > > This would avoid the need to special-case the cache maintenance, so its a good cleanup... Yes, the computation should be moved from kexec-reboot to kexec-load when possible. > > > > Because once MMU is enabled, kexec control page will contain more than > > relocation kernel, but also vector table, add pointer to the actual > > function within this page arch.kern_reloc. Currently, it equals to the > > beginning of page, we will add offsets later, when vector table is > > added. > > If the vector table always comes second, wouldn't this be extra work to hold the value 0? > You can control the layout of this relocation code, as it has to be written in assembly. > I don't get why this would be necessary. > > > > diff --git a/arch/arm64/kernel/machine_kexec.c b/arch/arm64/kernel/machine_kexec.c > > index ae1bad0156cd..ec71a153cc2d 100644 > > --- a/arch/arm64/kernel/machine_kexec.c > > +++ b/arch/arm64/kernel/machine_kexec.c > > @@ -58,6 +59,17 @@ void machine_kexec_cleanup(struct kimage *kimage) > > /* Empty routine needed to avoid build errors. */ > > } > > > > +int machine_kexec_post_load(struct kimage *kimage) > > +{ > > + void *reloc_code = page_to_virt(kimage->control_code_page); > > + > > + memcpy(reloc_code, arm64_relocate_new_kernel, > > + arm64_relocate_new_kernel_size); > > + kimage->arch.kern_reloc = __pa(reloc_code); > > Could we move the two cache maintenance calls for this area in here too. Keeping it next > to the modification makes it clearer why it is required. Yes, I moved it. > > In this case we can use flush_icache_range() instead of its __variant because this now > happens much earlier. True. > > > > + return 0; > > +} > > Regardless, > Reviewed-by: James Morse <james.morse@arm.com> Thank you for your review. Pasha > > > Thanks, > > James
diff --git a/arch/arm64/include/asm/kexec.h b/arch/arm64/include/asm/kexec.h index 61530ec3a9b1..9befcd87e9a8 100644 --- a/arch/arm64/include/asm/kexec.h +++ b/arch/arm64/include/asm/kexec.h @@ -95,6 +95,7 @@ static inline void crash_post_resume(void) {} struct kimage_arch { void *dtb; phys_addr_t dtb_mem; + phys_addr_t kern_reloc; /* Core ELF header buffer */ void *elf_headers; unsigned long elf_headers_mem; diff --git a/arch/arm64/kernel/machine_kexec.c b/arch/arm64/kernel/machine_kexec.c index ae1bad0156cd..ec71a153cc2d 100644 --- a/arch/arm64/kernel/machine_kexec.c +++ b/arch/arm64/kernel/machine_kexec.c @@ -42,6 +42,7 @@ static void _kexec_image_info(const char *func, int line, pr_debug(" start: %lx\n", kimage->start); pr_debug(" head: %lx\n", kimage->head); pr_debug(" nr_segments: %lu\n", kimage->nr_segments); + pr_debug(" kern_reloc: %pa\n", &kimage->arch.kern_reloc); for (i = 0; i < kimage->nr_segments; i++) { pr_debug(" segment[%lu]: %016lx - %016lx, 0x%lx bytes, %lu pages\n", @@ -58,6 +59,17 @@ void machine_kexec_cleanup(struct kimage *kimage) /* Empty routine needed to avoid build errors. */ } +int machine_kexec_post_load(struct kimage *kimage) +{ + void *reloc_code = page_to_virt(kimage->control_code_page); + + memcpy(reloc_code, arm64_relocate_new_kernel, + arm64_relocate_new_kernel_size); + kimage->arch.kern_reloc = __pa(reloc_code); + + return 0; +} + /** * machine_kexec_prepare - Prepare for a kexec reboot. * @@ -143,8 +155,7 @@ static void kexec_segment_flush(const struct kimage *kimage) */ void machine_kexec(struct kimage *kimage) { - phys_addr_t reboot_code_buffer_phys; - void *reboot_code_buffer; + void *reboot_code_buffer = page_to_virt(kimage->control_code_page); bool in_kexec_crash = (kimage == kexec_crash_image); bool stuck_cpus = cpus_are_stuck_in_kernel(); @@ -155,18 +166,8 @@ void machine_kexec(struct kimage *kimage) WARN(in_kexec_crash && (stuck_cpus || smp_crash_stop_failed()), "Some CPUs may be stale, kdump will be unreliable.\n"); - reboot_code_buffer_phys = page_to_phys(kimage->control_code_page); - reboot_code_buffer = phys_to_virt(reboot_code_buffer_phys); - kexec_image_info(kimage); - /* - * Copy arm64_relocate_new_kernel to the reboot_code_buffer for use - * after the kernel is shut down. - */ - memcpy(reboot_code_buffer, arm64_relocate_new_kernel, - arm64_relocate_new_kernel_size); - /* Flush the reboot_code_buffer in preparation for its execution. */ __flush_dcache_area(reboot_code_buffer, arm64_relocate_new_kernel_size); @@ -202,7 +203,7 @@ void machine_kexec(struct kimage *kimage) * userspace (kexec-tools). * In kexec_file case, the kernel starts directly without purgatory. */ - cpu_soft_restart(reboot_code_buffer_phys, kimage->head, kimage->start, + cpu_soft_restart(kimage->arch.kern_reloc, kimage->head, kimage->start, kimage->arch.dtb_mem); BUG(); /* Should never get here. */
Currently, kernel relocation function is configured in machine_kexec() at the time of kexec reboot by using control_code_page. This operation, however, is more logical to be done during kexec_load, and thus remove from reboot time. Move, setup of this function to newly added machine_kexec_post_load(). Because once MMU is enabled, kexec control page will contain more than relocation kernel, but also vector table, add pointer to the actual function within this page arch.kern_reloc. Currently, it equals to the beginning of page, we will add offsets later, when vector table is added. Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com> --- arch/arm64/include/asm/kexec.h | 1 + arch/arm64/kernel/machine_kexec.c | 27 ++++++++++++++------------- 2 files changed, 15 insertions(+), 13 deletions(-)