Kexec Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Michael Ellerman <mpe@ellerman.id.au>
To: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>,
	Dave Young <dyoung@redhat.com>
Cc: Stewart Smith <stewart@linux.vnet.ibm.com>,
	Stephen Rothwell <sfr@canb.auug.org.au>,
	Baoquan He <bhe@redhat.com>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	x86@kernel.org, kexec@lists.infradead.org,
	linux-kernel@vger.kernel.org, Ingo Molnar <mingo@redhat.com>,
	Paul Mackerras <paulus@samba.org>,
	Eric Biederman <ebiederm@xmission.com>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Mimi Zohar <zohar@linux.vnet.ibm.com>,
	linuxppc-dev@lists.ozlabs.org,
	Andrew Morton <akpm@linux-foundation.org>,
	Vivek Goyal <vgoyal@redhat.com>
Subject: Re: [PATCH v10 04/10] kexec_file: Add support for purgatory built as PIE.
Date: Tue, 22 Nov 2016 17:01:10 +1100	[thread overview]
Message-ID: <87ziksuk7t.fsf@concordia.ellerman.id.au> (raw)
In-Reply-To: <5009580.5GxAkTrMYA@morokweng>

Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> writes:
> Am Sonntag, 20. November 2016, 10:45:46 BRST schrieb Dave Young:
>> On 11/10/16 at 01:27am, Thiago Jung Bauermann wrote:
>> > powerpc's purgatory.ro has 12 relocation types when built as
>> > a relocatable object. To implement support for them requires
>> > arch_kexec_apply_relocations_add to duplicate a lot of code with
>> > module_64.c:apply_relocate_add.
>> > 
>> > When built as a Position Independent Executable there are only 4
>> > relocation types in purgatory.ro, so it becomes practical for the powerpc
>> > implementation of kexec_file to have its own relocation implementation.
>> > 
>> > Also, the purgatory is an executable and not an intermediary output from
>> > the compiler so it makes sense conceptually that it is easier to build
>> > it as a PIE than as a partially linked object.
>> > 
>> > Apart from the greatly reduced number of relocations, there are two
>> > differences between a relocatable object and a PIE:
>> > 
>> > 1. __kexec_load_purgatory needs to use the program headers rather than the
>> > 
>> >    section headers to figure out how to load the binary.
>> > 
>> > 2. Symbol values are absolute addresses instead of relative to the
>> > 
>> >    start of the section.
>> > 
>> > This patch adds the support needed in generic code for the differences
>> > above and allows powerpc to load and relocate a position independent
>> > purgatory.
>> 
>> [snip]
>> 
>> The kexec-tools machine_apply_elf_rel is pretty simple for ppc64, it is
>> not that complex. So could you look into simplify your kexec_file
>> implementation?
>
> I can try, but there is one fundamental issue here: powerpc position-dependent 
> code relies more on relocations than x86 position-dependent code does, so 
> there's a limit to how simple it can be made without switching to position-
> independent code. And it will always be more involved than it is on x86.

I think we need to go back to the drawing board on this one.

My hope was that building purgatory as PIE would reduce the amount of
complexity, but instead it's just added more. Sorry for sending you in
that direction.


In general I dislike the level of complexity of the kexec-tools
purgatory, and in particular I'm not comfortable with things like:

diff --git a/arch/powerpc/purgatory/sha256.c b/arch/powerpc/purgatory/sha256.c
new file mode 100644
index 000000000000..6abee1877d56
--- /dev/null
+++ b/arch/powerpc/purgatory/sha256.c
@@ -0,0 +1,6 @@
+#include "../boot/string.h"
+
+/* Avoid including x86's boot/string.h in sha256.c. */
+#define BOOT_STRING_H
+
+#include "../../x86/purgatory/sha256.c"


I think the best way to get this over the line would be to take the
kexec-lite purgatory implementation and use that to begin with. I know
it doesn't have all the features of the kexec-tools version, but it
should work, and we can look at adding the extra features later.

I'll try and get that working tonight.

cheers

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

  parent reply	other threads:[~2016-11-22  6:01 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-10  3:27 [PATCH v10 00/10] kexec_file_load implementation for PowerPC Thiago Jung Bauermann
2016-11-10  3:27 ` [PATCH v10 01/10] kexec_file: Allow arch-specific memory walking for kexec_add_buffer Thiago Jung Bauermann
2016-11-10  3:27 ` [PATCH v10 02/10] kexec_file: Change kexec_add_buffer to take kexec_buf as argument Thiago Jung Bauermann
2016-11-10  3:27 ` [PATCH v10 03/10] kexec_file: Factor out kexec_locate_mem_hole from kexec_add_buffer Thiago Jung Bauermann
2016-11-10  3:27 ` [PATCH v10 04/10] kexec_file: Add support for purgatory built as PIE Thiago Jung Bauermann
2016-11-20  2:45   ` Dave Young
2016-11-21 23:49     ` Thiago Jung Bauermann
2016-11-22  1:32       ` Dave Young
2016-11-22  6:01       ` Michael Ellerman [this message]
2016-11-22  6:16         ` Dave Young
2016-11-22 13:44         ` Thiago Jung Bauermann
2016-11-23  1:32           ` Dave Young
2016-11-23  2:54             ` Thiago Jung Bauermann
2016-11-23  8:45       ` Dave Young
2016-11-10  3:27 ` [PATCH v10 05/10] powerpc: Change places using CONFIG_KEXEC to use CONFIG_KEXEC_CORE instead Thiago Jung Bauermann
2016-11-10  3:27 ` [PATCH v10 06/10] powerpc: Implement kexec_file_load Thiago Jung Bauermann
2016-11-10  3:27 ` [PATCH v10 07/10] powerpc: Add functions to read ELF files of any endianness Thiago Jung Bauermann
2016-11-10  3:27 ` [PATCH v10 08/10] powerpc: Add support for loading ELF kernels with kexec_file_load Thiago Jung Bauermann
2016-11-10  3:27 ` [PATCH v10 09/10] powerpc: Add purgatory for kexec_file_load implementation Thiago Jung Bauermann
2016-11-10  3:27 ` [PATCH v10 10/10] powerpc: Enable CONFIG_KEXEC_FILE in powerpc server defconfigs Thiago Jung Bauermann

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=87ziksuk7t.fsf@concordia.ellerman.id.au \
    --to=mpe@ellerman.id.au \
    --cc=akpm@linux-foundation.org \
    --cc=bauerman@linux.vnet.ibm.com \
    --cc=benh@kernel.crashing.org \
    --cc=bhe@redhat.com \
    --cc=dyoung@redhat.com \
    --cc=ebiederm@xmission.com \
    --cc=hpa@zytor.com \
    --cc=kexec@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mingo@redhat.com \
    --cc=paulus@samba.org \
    --cc=sfr@canb.auug.org.au \
    --cc=stewart@linux.vnet.ibm.com \
    --cc=tglx@linutronix.de \
    --cc=vgoyal@redhat.com \
    --cc=x86@kernel.org \
    --cc=zohar@linux.vnet.ibm.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