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

[Pingfan Liu]

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.


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
>