[PATCH v4 07/12] ppc64/kexec_file: add support to relocate purgatory

[Michael Ellerman]

Hari Bathini <hbathini at linux.ibm.com> writes:
> On 22/07/20 9:55 am, Michael Ellerman wrote:
>> Hari Bathini <hbathini at linux.ibm.com> writes:
>>> Right now purgatory implementation is only minimal. But if purgatory
>>> code is to be enhanced to copy memory to the backup region and verify
>>> sha256 digest, relocations may have to be applied to the purgatory.
>>> So, add support to relocate purgatory in kexec_file_load system call
>>> by setting up TOC pointer and applying RELA relocations as needed.
>>>
>>> Reported-by: kernel test robot <lkp at intel.com>
>>> [lkp: In v1, 'struct mem_sym' was declared in parameter list]
>>> Signed-off-by: Hari Bathini <hbathini at linux.ibm.com>
>>> ---
>>>
>>> * Michael, can you share your opinion on the below:
>>>     - https://lore.kernel.org/patchwork/patch/1272027/
>>>     - My intention in cover note.
>> 
>> It seems like a lot of complexity for little benefit.
>> 
>> AFAICS your final purgatory_64.c is only 36 lines, and all it does is a
>> single (open coded) memcpy().
>> 
>> It seems like we could write that in not many more lines of assembler
>> and avoid all this code.
>
> Hi Michael,
>
> I am not sure if you would agree with me on this, but I am looking at the
> purgatory code as work in progress. As mentioned in the cover note, I intend
> to add log messaging, sha256 verification into purgatory. And also change it
> to position independent executable after moving common purgatory code (sha256
> verification) to arch-independent code.

I've never understood the desire to put more logic into purgatory. It's
the absolute worst place to detect that something's gone wrong, because
we have no facilities in there to do anything useful. We don't even know
what platform we're on.

> When I initially took this up, I wanted to add all the above changes too, but
> cut down on it, in the interest of time, first to get kdump (kexec -s -p)
> working in v5.9 merge window.
>
> But as the logic in patches 07/12 & 08/12 has been tested in kexec-tools code
> a lot of times and there are unlikely to be any changes to them except for
> __kexec_do_relocs() function (afaics), when -PIE would be used, I submitted them.
> With patch 09/12, I tried for a change that uses relocations while is minimal
> for now.
>
> Would you prefer it to be absolutely minimal by dropping patches 7 & 8 for
> now and writing the backup data copy code in assembler?

Yes please.

cheers