Re: [PATCH 1/4] RISC-V: Add support for riscv kexec/kdump on kexec-tools

["Björn Töpel" <bjorn@kernel.org>]

Simon!

Simon Horman <horms@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 <horms@kernel.org>
> Subject: [PATCH] workflow: Add riscv64
>
> Add riscv64 to matrix of build architectures.
>
> Signed-off-by: Simon Horman <horms@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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv