From: AKASHI Takahiro <takahiro.akashi@linaro.org>
To: James Morse <james.morse@arm.com>
Cc: catalin.marinas@arm.com, will.deacon@arm.com,
bauerman@linux.vnet.ibm.com, dhowells@redhat.com,
vgoyal@redhat.com, herbert@gondor.apana.org.au,
davem@davemloft.net, akpm@linux-foundation.org,
mpe@ellerman.id.au, dyoung@redhat.com, bhe@redhat.com,
arnd@arndb.de, ard.biesheuvel@linaro.org, julien.thierry@arm.com,
kexec@lists.infradead.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v7 05/11] arm64: kexec_file: create purgatory
Date: Fri, 9 Feb 2018 21:40:56 +0900 [thread overview]
Message-ID: <20180209124055.xlwqdvb33uxsqlzh@fireball> (raw)
In-Reply-To: <5A7B475C.2080008@arm.com>
On Wed, Feb 07, 2018 at 06:37:16PM +0000, James Morse wrote:
> Hi Akashi,
>
> On 04/12/17 02:57, AKASHI Takahiro wrote:
> > This is a basic purgatory, or a kind of glue code between the two kernels,
> > for arm64.
> >
> > Since purgatory is assumed to be relocatable (not executable) object by
> > kexec generic code, arch_kexec_apply_relocations_add() is required in
> > general. Arm64's purgatory, however, is a simple asm and all the references
> > can be resolved as local, no re-linking is needed here.
> >
> > Please note that even if we don't support digest check at purgatory we
>
> (You knew what I was going to ask!)
Yes, definitely.
>
> > need purgatory_sha_regions and purgatory_sha256_digest as they are
> > referenced by generic kexec code.
>
> As somewhere to store the values? If we aren't doing the validation could we add
> something about why not to the commit message? I think its because we only worry
> about memory corruption for kdump, and for kdump we unmap the crash-kernel
> region during normal-operation to prevent it getting corrupted.
>
> As we aren't doing the hash validation, could we hide its core-code behind some
> ARCH_HAS_KEXEC_PURGATORY_HASH, instead of defining dummy symbols and doing
> unnecessary work to fill them in?
Yes, this is one idea.
But as you mentioned below, adding a purgatory for arm64's kexec_file
does make little sense as I've already removed digest check code after
MarkR's comment.
>
> > diff --git a/arch/arm64/purgatory/entry.S b/arch/arm64/purgatory/entry.S
> > new file mode 100644
> > index 000000000000..fe6e968076db
> > --- /dev/null
> > +++ b/arch/arm64/purgatory/entry.S
> > @@ -0,0 +1,55 @@
> > +/*
> > + * kexec core purgatory
> > + */
> > +#include <linux/linkage.h>
> > +#include <uapi/linux/kexec.h>
> > +
> > +#define SHA256_DIGEST_SIZE 32 /* defined in crypto/sha.h */
> > +
> > +.text
> > +
> > +ENTRY(purgatory_start)
> > + /* Start new image. */
> > + ldr x17, __kernel_entry
> > + ldr x0, __dtb_addr
> > + mov x1, xzr
> > + mov x2, xzr
> > + mov x3, xzr
> > + br x17
> > +END(purgatory_start)
>
> Is this what arm64_relocate_new_kernel() drops into? I thought that had the
> kernel boot register values already so we wouldn't need another trampoline for
> kexec_file_load()...
Indeed
> .. but now that I look, it doesn't have the DTB, presumably because for regular
> kexec we don't know where user-space put it.
>
> Could we add some x0_for_kexec that is 0 by default (if that's the ABI), or the
First, I didn't get what you meaned here.
After managing to modify my code, I found that we could re-use
cpu_soft_restart(), especially, the fifth argument, which is currently
contant 0, but we will be able to pass dtb address here.
In turn, we can also use this argument to determine, in relocate_new_kernel(),
whether we should call puragatory (kexec_load) or directly jump into the kernel
(kexec_file_load).
> DTB address for kexec_file_load()? This would avoid this extra trampoline, and
> patching in the values from load_other_segments().
>
> I'd love to avoid an in-kernel purgatory! (its code with funny
> compile/link/relocation requirements that is impossible to debug)
Lovely!
I really appreicated your valuable comments.
and more on other patches comming?
-Takahiro AKASHI
>
> Thanks,
>
> James
next prev parent reply other threads:[~2018-02-09 12:41 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-12-04 2:57 [PATCH v7 00/11] arm64: kexec: add kexec_file_load() support AKASHI Takahiro
2017-12-04 2:57 ` [PATCH v7 01/11] resource: add walk_system_ram_res_rev() AKASHI Takahiro
2017-12-04 2:57 ` [PATCH v7 02/11] kexec_file: factor out arch_kexec_kernel_*() from x86, powerpc AKASHI Takahiro
2017-12-04 2:57 ` [PATCH v7 03/11] kexec_file: factor out crashdump elf header function from x86 AKASHI Takahiro
2018-02-07 18:37 ` James Morse
2018-02-09 12:23 ` AKASHI Takahiro
2017-12-04 2:57 ` [PATCH v7 04/11] asm-generic: add kexec_file_load system call to unistd.h AKASHI Takahiro
2017-12-04 2:57 ` [PATCH v7 05/11] arm64: kexec_file: create purgatory AKASHI Takahiro
2018-02-07 18:37 ` James Morse
2018-02-09 12:40 ` AKASHI Takahiro [this message]
2017-12-04 2:57 ` [PATCH v7 06/11] arm64: kexec_file: load initrd, device-tree and purgatory segments AKASHI Takahiro
2017-12-04 2:57 ` [PATCH v7 07/11] arm64: kexec_file: set up for crash dump adding elf core header AKASHI Takahiro
2017-12-04 2:57 ` [PATCH v7 08/11] arm64: kexec_file: enable KEXEC_FILE config AKASHI Takahiro
2017-12-04 2:57 ` [PATCH v7 09/11] arm64: kexec_file: add Image format support AKASHI Takahiro
2017-12-04 2:58 ` [PATCH v7 10/11] include: pe.h: remove message[] from mz header definition AKASHI Takahiro
2017-12-04 2:58 ` [PATCH v7 11/11] arm64: kexec_file: enable KEXEC_VERIFY_SIG for Image AKASHI Takahiro
2018-02-07 18:37 ` [PATCH v7 00/11] arm64: kexec: add kexec_file_load() support James Morse
2018-02-09 11:49 ` AKASHI Takahiro
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=20180209124055.xlwqdvb33uxsqlzh@fireball \
--to=takahiro.akashi@linaro.org \
--cc=akpm@linux-foundation.org \
--cc=ard.biesheuvel@linaro.org \
--cc=arnd@arndb.de \
--cc=bauerman@linux.vnet.ibm.com \
--cc=bhe@redhat.com \
--cc=catalin.marinas@arm.com \
--cc=davem@davemloft.net \
--cc=dhowells@redhat.com \
--cc=dyoung@redhat.com \
--cc=herbert@gondor.apana.org.au \
--cc=james.morse@arm.com \
--cc=julien.thierry@arm.com \
--cc=kexec@lists.infradead.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mpe@ellerman.id.au \
--cc=vgoyal@redhat.com \
--cc=will.deacon@arm.com \
/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).