Re: [PATCH 1/2] kexec: Default __NR_kexec_file_load is set to undefined

[Philipp Rudo <prudo@redhat.com>]

Hi Youling,

On Tue, 28 Feb 2023 10:02:16 +0800
Youling Tang <tangyouling@loongson.cn> wrote:

> Hi, Philipp
> 
> On 02/27/2023 11:19 PM, Philipp Rudo wrote:
> > Hi Youling,
> >
> > On Fri, 24 Feb 2023 17:51:07 +0800
> > Youling Tang <tangyouling@loongson.cn> wrote:
> >  
> >> The initial reason is that after the merger of 29fe5067ed07
> >> ("kexec: make -a the default"), kexec cannot be used on LoongArch,
> >> MIPS .etc architectures. We need to add "-c" for normal use. The
> >> current kexec_file_load system call is not implemented in
> >> architectures such as LoongArch, so it needs to pass kexec_load.
> >> So we need to set __NR_kexec_file_load to undefined in unsupported
> >> architectures. This will return EFALLBACK via is_kexec_file_load_implemented,
> >> and then via kexec_load.
> >>
> >> Signed-off-by: Youling Tang <tangyouling@loongson.cn>
> >> ---
> >>  kexec/kexec-syscall.h | 2 --
> >>  1 file changed, 2 deletions(-)
> >>
> >> diff --git a/kexec/kexec-syscall.h b/kexec/kexec-syscall.h
> >> index be6ccd5..ea77936 100644
> >> --- a/kexec/kexec-syscall.h
> >> +++ b/kexec/kexec-syscall.h
> >> @@ -59,9 +59,7 @@
> >>  #endif
> >>  #endif /*ifndef __NR_kexec_load*/
> >>
> >> -#ifdef __arm__
> >>  #undef __NR_kexec_file_load
> >> -#endif
> >>
> >>  #ifndef __NR_kexec_file_load  
> >
> > I don't think this will work as intended.  
> 
> Works fine in LoongArch after applying this patch.

I believe you. The problem is that it changes the behavior on other
architectures.

> >
> > On the top of the file sys/syscall.h gets included. In there
> > architectures that support kexec_file_load define __NR_kexec_file_load.
> > This also means that if an architecture doesn't support kexec_file_load
> > __NR_kexec_file_load shouldn't be defined in the first place. Thus I
> > suggest that you find out why sys/syscall.h defines
> > __NR_kexec_file_load for LoongArch even when the system call is not
> > supported and fix it there.  
> Yes, in the kernel, LoongArch uses the generic syscall number (in
> include/uapi/asm-generic/unistd.h), so __NR_kexec_file_load is
> defined as 294.

Ok, I see. I have expected that it is wrapped in some ifdef
CONFIG_KEXEC_FILE. But apparently that is not the case. Thanks for the
clarification.

> I think the simpler way is to modify it in kexec-tools as follows,
> #ifdef __loongarch__
> #undef __NR_kexec_file_load
> #endif

Yeah, looks like this is the best solution. Only thing you could do is
to merge it with the arm special handling. I.e. so it reads

#if defined(__arm__) || defined(__longarch__)
#undef __NR_kexec_file_load
#endif

But that is more a matter of taste. So you and Simon need to agree what
you like more.

Thanks
Philipp


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