[PATCHv2 1/6] kexec: Change the image probe's prototype

Wed May 31 18:45:12 PDT 2023

On Thu, May 25, 2023 at 11:32 AM Pingfan Liu <piliu at redhat.com> wrote:
>
> On Wed, May 24, 2023 at 7:53 PM Simon Horman <horms at kernel.org> wrote:
> >
> > On Tue, May 16, 2023 at 03:31:19PM +0800, Pingfan Liu wrote:
> > > As more complicated kernel format occurs such as zboot, where the
> > > compressed kernel is stored as a payload. The straight forward
> > > decompression can not meet the demand.
> > >
> > > A new image probe method is expected to read in the kernel file and decide
> > > how to unfold the content by itself.
> > >
> > > This patch aims to change the image probe's prototype from
> > >             typedef int (probe_t)(const char *kernel_buf, off_t kernel_size);
> > > to
> > >             typedef int (probe_t)(const char *kernel_buf, off_t kernel_size, struct kexec_info *info);
> > >
> > > Later, info can be used to return both the file descriptor and the
> > > parsed kernel buffer.
> > >
> > > In case your are curious, the remaing part of the log describes the
> > > process of the substitution of the new prototype, which can be divided
> > > into three groups.
> > >
> > > 1. change function pointer and its callsites:
> > >   sed -i 's/(probe_t)(const char \*kernel_buf, off_t kernel_size);/(probe_t)(const char \*kernel, off_t kernel_size, struct kexec_info \*info);/g' kexec/kexec.h
> > >   sed -i 's/\.probe(\([^,]*\), \([^)]*\))/\.probe(\1, \2, NULL)/g' kexec/kexec.c
> > >
> > > 2. change the function declaration and definition of each 'probe'
> > >    instance by coccinelle because they may cross lines
> > >
> > > The cocci file looks like:
> > >
> > > @ rule1 @
> > > identifier fn =~ "_probe";
> > > identifier buf, size;
> > > typedef off_t;
> > > @@
> > >
> > > -int fn(const char *buf, off_t size)
> > > +int fn(const char *buf, off_t size, struct kexec_info *info)
> > > {
> > >         ...
> > > }
> > >
> > > @ rule2 @
> > > identifier fn =~ "_probe";
> > > identifier buf, size;
> > > typedef off_t;
> > > @@
> > >
> > > +int fn(const char *buf, off_t size, struct kexec_info *info);
> > > -int fn(const char *buf, off_t size);
> > >
> > > Then running the command
> > > spatch --sp-file cocci/define.cocci --dir kexec --include-headers > ../define.patch
> > > git apply --directory=kexec ../define.patch
> > >
> > > 3. change the direct calls to the probe instances
> > >
> > > Originally I planned to achieve this by coccinelle, but failed similar
> > > to [1]. I have tried using "-I and --include" option for coccinelle, but it
> > > still did not work.
> > >
> > > Checking the direct callsite by "git grep "_probe(" | grep -v const"
> > > Fortunatelly, it turns out that only a few direct callsites exist, which
> > > lies in i386, and easy to be amended manually.
> > >
> > > Anyway, just FYI, the cocci file looks like:
> > > @ rule1 @
> > > identifier fn =~ "_probe";
> > > identifier buf, size;
> > > identifier info;
> > > typedef off_t;
> > > @@
> > >
> > > int fn(const char *buf, off_t size, struct kexec_info *info);
> > >
> > > /* change the direct callsite of any probe */
> > > @ rule2 @
> > > identifier rule1.fn;
> > > expression E1, E2;
> > > @@
> > >
> > >  fn(E1, E2
> > > +  ,NULL
> > >    )
> > >
> > > Then running the command:
> > > spatch --sp-file cocci/direct.cocci --dir kexec --include-headers
> > >
> > > [1]: https://lore.kernel.org/all/alpine.DEB.2.22.394.2202280705080.3112@hadrien/T/
> > >
> > > Signed-off-by: Pingfan Liu <piliu at redhat.com>
> > > To: kexec at lists.infradead.org
> > > Cc: horms at verge.net.au
> > > Cc: ardb at kernel.org
> > > Cc: jeremy.linton at arm.com
> >
> > Unfortunately I am seeing a build failure on (at least) sh4 with this change:
> >
> >  ../../kexec/arch/sh/kexec-zImage-sh.c:54:5: error: conflicting types for ‘zImage_sh_probe’
> >     54 | int zImage_sh_probe(const char *buf, off_t UNUSED(len))
> >        |     ^~~~~~~~~~~~~~~
>
> It may be because the macro UNUSED() is not tackled correctly by coccinelle.
>
> And there are several other places, which use UNUSED()
> git grep UNUSED | grep probe
> kexec/arch/arm/kexec-zImage-arm.c:138:int zImage_arm_probe(const char
> *UNUSED(buf), off_t UNUSED(len))
> kexec/arch/s390/kexec-image.c:219:image_s390_probe(const char
> *UNUSED(kernel_buf), off_t UNUSED(kernel_size))
> kexec/arch/sh/kexec-netbsd-sh.c:38:int netbsd_sh_probe(const char
> *buf, off_t UNUSED(len))
> kexec/arch/sh/kexec-zImage-sh.c:54:int zImage_sh_probe(const char
> *buf, off_t UNUSED(len))
>
> I will see how to handle them.
>

I'm having trouble figuring out how to make Coccinelle recognize
"UNUSED()" even though it can recognize "attribute(unused)".  So I
fixed these issues manually.

I have do cross-compiling for "arm/arm64/m68k/mips/m68k/ppc64/s390/sh"
using the toolchains on https://toolchains.bootlin.com/

And I open a pull request [1] for [PATCHv3 0/6] arm64: zboot support.
The only change from v2->v3 is to fix the compiling issue in [1/6]

[1]: https://github.com/horms/kexec-tools/pull/4

Thanks,

Pingfan

>
> Thanks,
>
> Pingfan
> >  In file included from ../../kexec/arch/sh/kexec-zImage-sh.c:27:
> >  ../../kexec/arch/sh/kexec-sh.h:11:5: note: previous declaration of ‘zImage_sh_probe’ was here
> >     11 | int zImage_sh_probe(const char *buf, off_t len, struct kexec_info *info);
> >        |     ^~~~~~~~~~~~~~~
> >  ../../kexec/arch/sh/kexec-zImage-sh.c:29:18: warning: ‘probe_debug’ defined but not used [-Wunused-const-variable=]
> >     29 | static const int probe_debug = 0;
> >        |                  ^~~~~~~~~~~
> >  make[1]: *** [Makefile:124: kexec/arch/sh/kexec-zImage-sh.o] Error 1
> >  make[1]: *** Waiting for unfinished jobs....
> >  ../../kexec/arch/sh/kexec-netbsd-sh.c:28:18: warning: ‘probe_debug’ defined but not used [-Wunused-const-variable=]
> >     28 | static const int probe_debug = 0;
> >        |                  ^~~~~~~~~~~
> >
> > Link: https://github.com/horms/kexec-tools/actions/runs/5068250493/jobs/9100319093#step:5:215
> >