I have another try, which drops the changes of the prototype of the
probe method. Instead, introducing the 'probe2' interface and
migrating all arm64 image probe methods to that interface.
I will do more test on the new series and sent it out soon
Thanks,
Pingfan
On Thu, Jun 1, 2023 at 9:45 AM Pingfan Liu <piliu@xxxxxxxxxx> wrote:
>
> On Thu, May 25, 2023 at 11:32 AM Pingfan Liu <piliu@xxxxxxxxxx> wrote:
> >
> > On Wed, May 24, 2023 at 7:53 PM Simon Horman <horms@xxxxxxxxxx> 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@xxxxxxxxxx>
> > > > To: kexec@xxxxxxxxxxxxxxxxxxx
> > > > Cc: horms@xxxxxxxxxxxx
> > > > Cc: ardb@xxxxxxxxxx
> > > > Cc: jeremy.linton@xxxxxxx
> > >
> > > 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
> > >
_______________________________________________
kexec mailing list
kexec@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/kexec
