Hi Oleksii, > On 5 Sep 2023, at 15:37, Oleksii <[email protected]> wrote: > > Hi Bertrand, > > On Mon, 2023-09-04 at 13:39 +0000, Bertrand Marquis wrote: >> Hi Oleksii, >> >>> On 1 Sep 2023, at 18:02, Oleksii Kurochko >>> <[email protected]> wrote: >>> >>> Some headers are shared between individual architectures or are >>> empty. >>> To avoid duplication of these headers, asm-generic is introduced. >>> >>> With the following patch, an architecture uses generic headers >>> mentioned in the file arch/$(ARCH)/include/asm/Kbuild. >> >> Kbuild refers to "Kernel build" I guess. >> I am ok to keep that name to keep things simpler when compared to >> Linux scripts but it would be good to mention that in the commit >> message for future reference. > Sure. I'll add that Kbuild refers to "Kernel build" >> >>> >>> To use a generic header is needed to add to >>> arch/$(ARCH)/include/asm/Kbuild : >>> generic-y += <name-of-header-file.h> >>> >>> For each mentioned header in arch/$(ARCH)/include/asm/Kbuild, >>> kbuild will generate the necessary wrapper in >>> arch/$(ARCH)/include/generated/asm. >>> >>> Signed-off-by: Oleksii Kurochko <[email protected]> >>> --- >>> Changes in V4: >>> - introduce asm-generic support >>> - update commit message >>> --- >>> Changes in V3: >>> - Rename stubs dir to asm-generic >>> --- >>> Changes in V2: >>> - Nothing changed. >>> --- >>> xen/Makefile | 10 +++++++++- >>> xen/arch/arm/include/asm/Kbuild | 1 + >>> xen/arch/ppc/include/asm/Kbuild | 1 + >>> xen/arch/riscv/include/asm/Kbuild | 1 + >>> xen/arch/x86/include/asm/Kbuild | 1 + >>> xen/scripts/Makefile.asm-generic | 23 +++++++++++++++++++++++ >>> 6 files changed, 36 insertions(+), 1 deletion(-) >>> create mode 100644 xen/arch/arm/include/asm/Kbuild >>> create mode 100644 xen/arch/ppc/include/asm/Kbuild >>> create mode 100644 xen/arch/riscv/include/asm/Kbuild >>> create mode 100644 xen/arch/x86/include/asm/Kbuild >>> create mode 100644 xen/scripts/Makefile.asm-generic >>> >>> diff --git a/xen/Makefile b/xen/Makefile >>> index f57e5a596c..698d6ddeb8 100644 >>> --- a/xen/Makefile >>> +++ b/xen/Makefile >>> @@ -438,6 +438,7 @@ ifdef building_out_of_srctree >>> endif >>> CFLAGS += -I$(srctree)/include >>> CFLAGS += -I$(srctree)/arch/$(SRCARCH)/include >>> +CFLAGS += -I$(srctree)/arch/$(SRCARCH)/include/generated >> >> Why are we generating files in the source tree ? >> Shouldn't we keep it unmodified ? > I think you are right. > I got myself confused because I do not use out-of-tree compilation so > for me it looked like $(srctree). > > It would be better to use $(objtree) instead.
Ack >> >>> >>> # Note that link order matters! >>> ALL_OBJS-y := common/built_in.o >>> @@ -580,6 +581,7 @@ _clean: >>> rm -f $(TARGET).efi $(TARGET).efi.map $(TARGET).efi.elf >>> $(TARGET).efi.stripped >>> rm -f asm-offsets.s arch/*/include/asm/asm-offsets.h >>> rm -f .banner .allconfig.tmp include/xen/compile.h >>> + rm -rf arch/*/include/generated >>> >> >> You must use the same as for CFLAGS here so $(srctree) would be >> needed. >> (or something else depending on the fix for previous comment) > Thanks. I'll change to the value mentioned in CFLAGS. > >> >>> .PHONY: _distclean >>> _distclean: clean >>> @@ -589,7 +591,7 @@ $(TARGET).gz: $(TARGET) >>> gzip -n -f -9 < $< > [email protected] >>> mv [email protected] $@ >>> >>> -$(TARGET): outputmakefile FORCE >>> +$(TARGET): outputmakefile asm-generic FORCE >>> $(Q)$(MAKE) $(build)=tools >>> $(Q)$(MAKE) $(build)=. include/xen/compile.h >>> $(Q)$(MAKE) $(build)=include all >>> @@ -667,6 +669,12 @@ endif # need-sub-make >>> PHONY += FORCE >>> FORCE: >>> >>> +# Support for using generic headers in asm-generic >>> +PHONY += asm-generic >>> +asm-generic: >>> + $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.asm-generic \ >>> + obj=arch/$(SRCARCH)/include/generated/asm >>> + >>> # Declare the contents of the PHONY variable as phony. We keep >>> that >>> # information in a variable so we can use it in if_changed and >>> friends. >>> .PHONY: $(PHONY) >>> diff --git a/xen/arch/arm/include/asm/Kbuild >>> b/xen/arch/arm/include/asm/Kbuild >>> new file mode 100644 >>> index 0000000000..a4e40e534e >>> --- /dev/null >>> +++ b/xen/arch/arm/include/asm/Kbuild >>> @@ -0,0 +1 @@ >>> +# SPDX-License-Identifier: GPL-2.0-only >>> diff --git a/xen/arch/ppc/include/asm/Kbuild >>> b/xen/arch/ppc/include/asm/Kbuild >>> new file mode 100644 >>> index 0000000000..a4e40e534e >>> --- /dev/null >>> +++ b/xen/arch/ppc/include/asm/Kbuild >>> @@ -0,0 +1 @@ >>> +# SPDX-License-Identifier: GPL-2.0-only >>> diff --git a/xen/arch/riscv/include/asm/Kbuild >>> b/xen/arch/riscv/include/asm/Kbuild >>> new file mode 100644 >>> index 0000000000..a4e40e534e >>> --- /dev/null >>> +++ b/xen/arch/riscv/include/asm/Kbuild >>> @@ -0,0 +1 @@ >>> +# SPDX-License-Identifier: GPL-2.0-only >>> diff --git a/xen/arch/x86/include/asm/Kbuild >>> b/xen/arch/x86/include/asm/Kbuild >>> new file mode 100644 >>> index 0000000000..a4e40e534e >>> --- /dev/null >>> +++ b/xen/arch/x86/include/asm/Kbuild >>> @@ -0,0 +1 @@ >>> +# SPDX-License-Identifier: GPL-2.0-only >>> diff --git a/xen/scripts/Makefile.asm-generic >>> b/xen/scripts/Makefile.asm-generic >>> new file mode 100644 >>> index 0000000000..0aac3f50b8 >>> --- /dev/null >>> +++ b/xen/scripts/Makefile.asm-generic >>> @@ -0,0 +1,23 @@ >>> +# SPDX-License-Identifier: GPL-2.0 >>> +# include/asm-generic contains a lot of files that are used >>> +# verbatim by several architectures. >>> +# >>> +# This Makefile reads the file arch/$(SRCARCH)/include/asm/Kbuild >>> +# and for each file listed in this file with generic-y creates >>> +# a small wrapper file in $(obj) >>> (arch/$(SRCARCH)/include/generated/asm) >>> + >>> +kbuild-file := $(srctree)/arch/$(SRCARCH)/include/asm/Kbuild >>> +include $(kbuild-file) >>> + >>> +include scripts/Kbuild.include >>> + >>> +# Create output directory if not already present >>> +_dummy := $(shell [ -d $(obj) ] || mkdir -p $(obj)) >>> + >>> +quiet_cmd_wrap = WRAP $@ >>> +cmd_wrap = echo "\#include <asm-generic/$*.h>" >$@ >>> + >>> +all: $(patsubst %, $(obj)/%, $(generic-y)) >>> + >>> +$(obj)/%.h: >>> + $(call cmd,wrap) >> >> In fact i think your files are not generated in the srctree so this >> will break if you use out of tree compilation. > Why not? They will be generated in $obj which equal to > obj=arch/$(SRCARCH)/include/generated/asm ( in my case ). Right they are generated in the build tree but if it is not the same as srctree it will not work due to CFLAGS. > > Anyway I think that obj should be equal to what is mentioned in CFLAGS. We generate other files like asm-offsets.h and it could in fact be that both srctree and build tree are already in include path. There is in makefile a rule already removing asm-offsets.h which is generated and this one does not use a prefix so we should follow this standard in fact. So we should not use any prefix (srctree or obj) i think. @anthony: any suggestion here ? Bertrand > > ~ Oleksii
