From: James Morse <james.morse@arm.com>
To: Pavel Tatashin <pasha.tatashin@soleen.com>
Cc: jmorris@namei.org, sashal@kernel.org, ebiederm@xmission.com,
kexec@lists.infradead.org, linux-kernel@vger.kernel.org,
corbet@lwn.net, catalin.marinas@arm.com, will@kernel.org,
linux-arm-kernel@lists.infradead.org, maz@kernel.org,
vladimir.murzin@arm.com, matthias.bgg@gmail.com,
bhsharma@redhat.com, linux-mm@kvack.org, mark.rutland@arm.com,
steve.capper@arm.com, rfontana@redhat.com, tglx@linutronix.de,
selindag@gmail.com
Subject: Re: [PATCH v9 11/18] arm64: kexec: arm64_relocate_new_kernel clean-ups
Date: Thu, 7 May 2020 17:22:13 +0100 [thread overview]
Message-ID: <dc47bee6-69ee-2ece-12d4-258bd964b954@arm.com> (raw)
In-Reply-To: <20200326032420.27220-12-pasha.tatashin@soleen.com>
Hi Pavel,
On 26/03/2020 03:24, Pavel Tatashin wrote:
> Remove excessive empty lines from arm64_relocate_new_kernel.
To make it harder to read? Or just for the churn ...
> Also, use comments on the same lines with instructions where
> appropriate.
Churn,
> Change ENDPROC to END it never returns.
It might be more useful to convert this to the new style annotations, which should be a
separate patch. See Documentation/asm-annotations.rst
> copy_page(dest, src, tmps...)
> Increments dest and src by PAGE_SIZE, so no need to store dest
> prior to calling copy_page and increment it after. Also, src is not
> used after a copy, not need to copy either.
This bit sounds like cleanup, but I can't isolate it from the noise below....
> Call raw_dcache_line_size() only when relocation is actually going to
> happen.
Why?
The pattern in this code is to setup register that don't change at the top, then do all
the work. I think this was an attempt to make it more readable.
Nothing branches back to that label, so this is fine, its just less obviously correct.
> Since '.align 3' is intended to align globals at the end of the file,
> move it there.
Please don't make noisy changes to whitespace and comments, its never worth it.
Thanks,
James
> diff --git a/arch/arm64/kernel/relocate_kernel.S b/arch/arm64/kernel/relocate_kernel.S
> index c1d7db71a726..e9c974ea4717 100644
> --- a/arch/arm64/kernel/relocate_kernel.S
> +++ b/arch/arm64/kernel/relocate_kernel.S
> @@ -8,7 +8,6 @@
>
> #include <linux/kexec.h>
> #include <linux/linkage.h>
> -
> #include <asm/assembler.h>
> #include <asm/kexec.h>
> #include <asm/page.h>
> @@ -17,25 +16,21 @@
> /*
> * arm64_relocate_new_kernel - Put a 2nd stage image in place and boot it.
> *
> - * The memory that the old kernel occupies may be overwritten when coping the
> + * The memory that the old kernel occupies may be overwritten when copying the
> * new image to its final location. To assure that the
> * arm64_relocate_new_kernel routine which does that copy is not overwritten,
> * all code and data needed by arm64_relocate_new_kernel must be between the
> * symbols arm64_relocate_new_kernel and arm64_relocate_new_kernel_end. The
> * machine_kexec() routine will copy arm64_relocate_new_kernel to the kexec
> - * control_code_page, a special page which has been set up to be preserved
> - * during the copy operation.
> + * safe memory that has been set up to be preserved during the copy operation.
> */
> ENTRY(arm64_relocate_new_kernel)
> -
> /* Setup the list loop variables. */
> mov x18, x2 /* x18 = dtb address */
> mov x17, x1 /* x17 = kimage_start */
> mov x16, x0 /* x16 = kimage_head */
> - raw_dcache_line_size x15, x0 /* x15 = dcache line size */
> mov x14, xzr /* x14 = entry ptr */
> mov x13, xzr /* x13 = copy dest */
> -
> /* Clear the sctlr_el2 flags. */
> mrs x0, CurrentEL
> cmp x0, #CurrentEL_EL2
> @@ -46,14 +41,11 @@ ENTRY(arm64_relocate_new_kernel)
> pre_disable_mmu_workaround
> msr sctlr_el2, x0
> isb
> -1:
> -
> - /* Check if the new image needs relocation. */
> +1: /* Check if the new image needs relocation. */
> tbnz x16, IND_DONE_BIT, .Ldone
> -
> + raw_dcache_line_size x15, x1 /* x15 = dcache line size */
> .Lloop:
> and x12, x16, PAGE_MASK /* x12 = addr */
> -
> /* Test the entry flags. */
> .Ltest_source:
> tbz x16, IND_SOURCE_BIT, .Ltest_indirection
> @@ -69,34 +61,18 @@ ENTRY(arm64_relocate_new_kernel)
> b.lo 2b
> dsb sy
>
> - mov x20, x13
> - mov x21, x12
> - copy_page x20, x21, x0, x1, x2, x3, x4, x5, x6, x7
> -
> - /* dest += PAGE_SIZE */
> - add x13, x13, PAGE_SIZE
> + copy_page x13, x12, x0, x1, x2, x3, x4, x5, x6, x7
> b .Lnext
> -
> .Ltest_indirection:
> tbz x16, IND_INDIRECTION_BIT, .Ltest_destination
> -
> - /* ptr = addr */
> - mov x14, x12
> + mov x14, x12 /* ptr = addr */
> b .Lnext
> -
> .Ltest_destination:
> tbz x16, IND_DESTINATION_BIT, .Lnext
> -
> - /* dest = addr */
> - mov x13, x12
> -
> + mov x13, x12 /* dest = addr */
> .Lnext:
> - /* entry = *ptr++ */
> - ldr x16, [x14], #8
> -
> - /* while (!(entry & DONE)) */
> - tbz x16, IND_DONE_BIT, .Lloop
> -
> + ldr x16, [x14], #8 /* entry = *ptr++ */
> + tbz x16, IND_DONE_BIT, .Lloop /* while (!(entry & DONE)) */
> .Ldone:
> /* wait for writes from copy_page to finish */
> dsb nsh
> @@ -110,16 +86,12 @@ ENTRY(arm64_relocate_new_kernel)
> mov x2, xzr
> mov x3, xzr
> br x17
> -
> -ENDPROC(arm64_relocate_new_kernel)
> -
> .ltorg
> -
> -.align 3 /* To keep the 64-bit values below naturally aligned. */
> +END(arm64_relocate_new_kernel)
>
> .Lcopy_end:
> .org KEXEC_CONTROL_PAGE_SIZE
> -
> +.align 3 /* To keep the 64-bit values below naturally aligned. */
> /*
> * arm64_relocate_new_kernel_size - Number of bytes to copy to the
> * control_code_page.
>
next prev parent reply other threads:[~2020-05-07 16:22 UTC|newest]
Thread overview: 45+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-26 3:24 [PATCH v9 00/18] arm64: MMU enabled kexec relocation Pavel Tatashin
2020-03-26 3:24 ` [PATCH v9 01/18] arm64: kexec: make dtb_mem always enabled Pavel Tatashin
2020-04-29 17:00 ` James Morse
2021-01-23 0:17 ` Pavel Tatashin
2020-03-26 3:24 ` [PATCH v9 02/18] arm64: hibernate: move page handling function to new trans_pgd.c Pavel Tatashin
2020-04-29 17:00 ` James Morse
2021-01-23 0:18 ` Pavel Tatashin
2020-03-26 3:24 ` [PATCH v9 03/18] arm64: trans_pgd: make trans_pgd_map_page generic Pavel Tatashin
2020-04-29 17:01 ` James Morse
2021-01-22 21:52 ` Pavel Tatashin
2020-03-26 3:24 ` [PATCH v9 04/18] arm64: trans_pgd: pass allocator trans_pgd_create_copy Pavel Tatashin
2020-04-29 17:01 ` James Morse
2021-01-23 0:20 ` Pavel Tatashin
2020-03-26 3:24 ` [PATCH v9 05/18] arm64: trans_pgd: pass NULL instead of init_mm to *_populate functions Pavel Tatashin
2020-04-29 17:01 ` James Morse
2021-01-23 0:22 ` Pavel Tatashin
2020-03-26 3:24 ` [PATCH v9 06/18] arm64: mm: Always update TCR_EL1 from __cpu_set_tcr_t0sz() Pavel Tatashin
2020-03-26 3:24 ` [PATCH v9 07/18] arm64: trans_pgd: hibernate: idmap the single page that holds the copy page routines Pavel Tatashin
2020-04-29 17:01 ` James Morse
2021-01-23 0:35 ` Pavel Tatashin
2020-03-26 3:24 ` [PATCH v9 08/18] arm64: kexec: move relocation function setup Pavel Tatashin
2020-04-29 17:01 ` James Morse
2021-01-23 1:01 ` Pavel Tatashin
2020-03-26 3:24 ` [PATCH v9 09/18] arm64: kexec: call kexec_image_info only once Pavel Tatashin
2020-04-29 17:01 ` James Morse
2020-03-26 3:24 ` [PATCH v9 10/18] arm64: kexec: cpu_soft_restart change argument types Pavel Tatashin
2020-04-29 17:01 ` James Morse
2021-01-23 1:14 ` Pavel Tatashin
2020-03-26 3:24 ` [PATCH v9 11/18] arm64: kexec: arm64_relocate_new_kernel clean-ups Pavel Tatashin
2020-05-07 16:22 ` James Morse [this message]
2020-03-26 3:24 ` [PATCH v9 12/18] arm64: kexec: arm64_relocate_new_kernel don't use x0 as temp Pavel Tatashin
2020-05-07 16:22 ` James Morse
2020-03-26 3:24 ` [PATCH v9 13/18] arm64: kexec: add expandable argument to relocation function Pavel Tatashin
2020-05-07 16:22 ` James Morse
2021-01-23 2:49 ` Pavel Tatashin
2020-03-26 3:24 ` [PATCH v9 14/18] arm64: kexec: offset for " Pavel Tatashin
2020-05-07 16:22 ` James Morse
2020-03-26 3:24 ` [PATCH v9 15/18] arm64: kexec: kexec EL2 vectors Pavel Tatashin
2020-04-29 17:35 ` Marc Zyngier
2021-01-25 19:07 ` Pavel Tatashin
2020-05-07 16:21 ` James Morse
2020-03-26 3:24 ` [PATCH v9 16/18] arm64: kexec: configure trans_pgd page table for kexec Pavel Tatashin
2020-05-07 16:22 ` James Morse
2020-03-26 3:24 ` [PATCH v9 17/18] arm64: kexec: enable MMU during kexec relocation Pavel Tatashin
2020-03-26 3:24 ` [PATCH v9 18/18] arm64: kexec: remove head from relocation argument Pavel Tatashin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=dc47bee6-69ee-2ece-12d4-258bd964b954@arm.com \
--to=james.morse@arm.com \
--cc=bhsharma@redhat.com \
--cc=catalin.marinas@arm.com \
--cc=corbet@lwn.net \
--cc=ebiederm@xmission.com \
--cc=jmorris@namei.org \
--cc=kexec@lists.infradead.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mark.rutland@arm.com \
--cc=matthias.bgg@gmail.com \
--cc=maz@kernel.org \
--cc=pasha.tatashin@soleen.com \
--cc=rfontana@redhat.com \
--cc=sashal@kernel.org \
--cc=selindag@gmail.com \
--cc=steve.capper@arm.com \
--cc=tglx@linutronix.de \
--cc=vladimir.murzin@arm.com \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).