Simon! Simon Horman <ho...@kernel.org> writes:
> Thanks Björn, > > Overall I am happy with this patchset. But there are a few minor points > I'd like addressed, which relate strictly to build coverage and the CI. > I can handle these myself if you prefer. It's really up to you! What do you prefer? I can spin a v2 if that's easier for you (and then I'll try to address all the things you pointed out below)! Björn > Firstly, it would be really great if the following could be included at the > end of the series, so there is some build coverage of RISC-V in the CI. > (Feel free to rewrite or attribute however you see fit - it is a trival one > liner.) > > From: Simon Horman <ho...@kernel.org> > Subject: [PATCH] workflow: Add riscv64 > > Add riscv64 to matrix of build architectures. > > Signed-off-by: Simon Horman <ho...@kernel.org> > --- > .github/workflows/build.yml | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml > index fd9ea6c8eca7..d1b3e74391ff 100644 > --- a/.github/workflows/build.yml > +++ b/.github/workflows/build.yml > @@ -19,6 +19,7 @@ jobs: > - powerpc > - powerpc64 > - powerpc64le > + - riscv64 > - sh4 > - s390x > - x86_64-x32 > > ... > >> diff --git a/kexec/arch/riscv/Makefile b/kexec/arch/riscv/Makefile >> new file mode 100644 >> index 000000000000..f26cc9025e77 >> --- /dev/null >> +++ b/kexec/arch/riscv/Makefile >> @@ -0,0 +1,35 @@ >> +# >> +# kexec riscv >> +# >> +riscv_KEXEC_SRCS = kexec/arch/riscv/kexec-riscv.c >> +riscv_KEXEC_SRCS += kexec/arch/riscv/kexec-elf-riscv.c >> +riscv_KEXEC_SRCS += kexec/arch/riscv/crashdump-riscv.c >> + >> +riscv_MEM_REGIONS = kexec/mem_regions.c >> + >> +riscv_DT_OPS += kexec/dt-ops.c >> + >> +riscv_ARCH_REUSE_INITRD = >> + >> +riscv_CPPFLAGS += -I $(srcdir)/kexec/ >> + >> +dist += kexec/arch/riscv/Makefile $(riscv_KEXEC_SRCS) >> \ > > kexec/arch/riscv/iomem.h also needs to be added to dist > so that it shows up in the distribution tarball. > >> + kexec/arch/riscv/kexec-riscv.h \ >> + kexec/arch/riscv/include/arch/options.h > > Also, it would be nice to sort these lines alphabetically. > >> + >> +ifdef HAVE_LIBFDT >> + >> +LIBS += -lfdt >> + >> +else >> + >> +include $(srcdir)/kexec/libfdt/Makefile.libfdt >> + >> +libfdt_SRCS += $(LIBFDT_SRCS:%=kexec/libfdt/%) >> + >> +riscv_CPPFLAGS += -I$(srcdir)/kexec/libfdt >> + >> +riscv_KEXEC_SRCS += $(libfdt_SRCS) >> + >> +endif >> + > > ... > >> diff --git a/kexec/dt-ops.c b/kexec/dt-ops.c >> index 0a96b75f65aa..3e285ab2043b 100644 >> --- a/kexec/dt-ops.c >> +++ b/kexec/dt-ops.c >> @@ -4,9 +4,11 @@ >> #include <libfdt.h> >> #include <stdio.h> >> #include <stdlib.h> >> +#include <stdbool.h> >> >> #include "kexec.h" >> #include "dt-ops.h" >> +#include "mem_regions.h" > > As dt-ops.c will now use symbols that are implemented in mem_regions.c > when the former is compiled the latter now must be compiled too. > > In practice it is only mips where that is not the case. > And I believe that can be addressed by squashing the following into > this patch. > > diff --git a/kexec/arch/mips/Makefile b/kexec/arch/mips/Makefile > index 1fe788608fbe..fa87fbdb7897 100644 > --- a/kexec/arch/mips/Makefile > +++ b/kexec/arch/mips/Makefile > @@ -11,6 +11,8 @@ mips_FS2DT_INCLUDE = \ > -include $(srcdir)/kexec/arch/mips/crashdump-mips.h \ > -include $(srcdir)/kexec/arch/mips/kexec-mips.h > > +mips_MEM_REGIONS = kexec/mem_regions.c > + > mips_DT_OPS += kexec/dt-ops.c > > include $(srcdir)/kexec/libfdt/Makefile.libfdt > > FTR, I believe this is the same mem_regions_alloc_and_add issue that > I flagged in the thread [2]. > > And at the link below you can see a build failure with this patch, but not > the fix above, applied: > > https://github.com/horms/kexec-tools/actions/runs/14591381633/job/40927219243
