Re: [PATCH v3 6/6] MIPS: kexec: Use prepare method from generic platform as default option

[Dengcheng Zhu <dzhu@wavecomp.com>]

Hi Paul,


On 07/24/2018 04:36 PM, Paul Burton wrote:
> Hi Dengcheng,
>
> On Mon, Jul 23, 2018 at 07:48:19AM -0700, Dengcheng Zhu wrote:
>> The kexec_prepare method for the generic platform should be applicable to
>> other platforms.
> ...only for platforms which use the UHI boot protocol, which is what
> generic_kexec_prepare() sets up kexec_args for. On any platforms which
> don't use this boot protocol the new kernel wound find unexpected
> arguments & in the worst case do something crazy with them.
>
>> For those otherwise, like Octeon, they will use their own
>> _machine_kexec_prepare().
>>
>> Without the default prepare work, platforms other than the generic one will
>> not be able to automatically set up command line correctly for the new
>> kernel.
> So even with this patch they still can't unless they happen to use the
> UHI boot protocol.
>
> If we ever have multiple in-tree platforms which make use of the UHI
> boot protocol & want to use kexec then I'd be fine with us moving
> generic_kexec_prepare() & renaming it something like uhi_kexec_prepare()
> but I prefer that we don't just presume the boot protocol for any
> platform that doesn't set _machine_kexec_prepare.

Thanks for pointing this out. How about something like the following in
kernel/machine_kexec.c?

--------------------------
#ifdef CONFIG_UHI_BOOT
static int uhi_machine_kexec_prepare(struct kimage *kimage)
{
     <the code of generic_kexec_prepare()>
}

int (*_machine_kexec_prepare)(struct kimage *) = uhi_machine_kexec_prepare;
#else
int (*_machine_kexec_prepare)(struct kimage *) = NULL;
#endif
--------------------------

In this way, any platform (config MIPS_GENERIC at this moment) can do
"select UHI_BOOT" if applicable. So this will be a one-line change to
UHI platforms that have not been upstreamed.



Regards,

Dengcheng

---------------------------------------------------------------------------

*From:* Paul Burton <mailto:paul.burton@mips.com>
*Sent:* Tuesday, July 24, 2018 4:36PM
*To:* Dengcheng Zhu <mailto:dzhu@wavecomp.com>
*Cc:* Pburton <mailto:pburton@wavecomp.com>, Ralf 
<mailto:ralf@linux-mips.org>, Linux-mips 
<mailto:linux-mips@linux-mips.org>, Rachel.mozes 
<mailto:rachel.mozes@intel.com>
*Subject:* Re: [PATCH v3 6/6] MIPS: kexec: Use prepare method from generic 
platform as default option

Hi Dengcheng,

On Mon, Jul 23, 2018 at 07:48:19AM -0700, Dengcheng Zhu wrote:

> The kexec_prepare method for the generic platform should be applicable to
> other platforms.

...only for platforms which use the UHI boot protocol, which is what
generic_kexec_prepare() sets up kexec_args for. On any platforms which
don't use this boot protocol the new kernel wound find unexpected
arguments & in the worst case do something crazy with them.

> For those otherwise, like Octeon, they will use their own
> _machine_kexec_prepare().
>
> Without the default prepare work, platforms other than the generic one will
> not be able to automatically set up command line correctly for the new
> kernel.

So even with this patch they still can't unless they happen to use the
UHI boot protocol.

If we ever have multiple in-tree platforms which make use of the UHI
boot protocol & want to use kexec then I'd be fine with us moving
generic_kexec_prepare() & renaming it something like uhi_kexec_prepare()
but I prefer that we don't just presume the boot protocol for any
platform that doesn't set _machine_kexec_prepare.

Hopefully anyone using the UHI boot protocol is doing so because they
want to fit in with the generic kernel platform so this won't happen
anyway (just need to get EVA supported by the generic platform & then
there'd be no reason at all not to use it).

Interestingly there's a patch in patchwork from last year which aimed at
making the default to pass arguments the current kernel was given to the
new kernel [1]. That would seem like a saner default since we'd be
making no presumptions about what those arguments should actually be,
but I don't think that's safe either because if any of those arguments
are pointers we have no guarantees that we haven't overwritten the
memory they're pointing at.

Thanks,
     Paul

[1] https://patchwork.linux-mips.org/patch/15397/