Re: [PATCH V3 05/22] LoongArch: Add build infrastructure

[Arnd Bergmann <arnd@arndb.de>]

On Fri, Sep 17, 2021 at 5:57 AM Huacai Chen <chenhuacai@loongson.cn>
wrote:> --- /dev/null
> +++ b/arch/loongarch/.gitignore
> @@ -0,0 +1,9 @@
> +*.lds
> +*.raw
> +calc_vmlinuz_load_addr
> +elf-entry
> +relocs
> +vmlinux.*
> +vmlinuz.*
> +
> +!kernel/vmlinux.lds.S

Can you double-check that 'make clean' and/or 'make mrproper' remove all the
generated files? This may already be the case, but I don't see how this is
done in your Makefile

> +
> +choice
> +       prompt "Page Table Layout"
> +       default 16KB_2LEVEL if 32BIT
> +       default 16KB_3LEVEL if 64BIT
> +       help
> +         Allows choosing the page table layout, which is a combination
> +         of page size and page table levels. The virtual memory address
> +         space bits are determined by the page table layout.
> +
> +config 4KB_3LEVEL
> +       bool "4KB with 3 levels"
> +       select PAGE_SIZE_4KB
> +       select PGTABLE_3LEVEL
> +       help
> +         This option selects 4KB page size with 3 level page tables, which
> +         support a maximum 39 bits of application virtual memory.
> + ...

Nice, this choice statement looks a lot better than the version you had before.

> +
> +cflags-y += -ffreestanding

I had not noticed this previously, but I think this should not be used here,
as -ffreestanding disables a number of optimizations for compiler builtins.

Did you just copy this from MIPS or do you have a particular reason this
is used here?

> +# Some distribution-specific toolchains might pass the -fstack-check
> +# option during the build, which adds a simple stack-probe at the beginning
> +# of every function.  This stack probe is to ensure that there is enough
> +# stack space, else a SEGV is generated.  This is not desirable for LoongArch
> +# as kernel stacks are small, placed in unmapped virtual memory, and do not
> +# grow when overflowed.
> +#
> +cflags-y += -fno-stack-check

This is already set in the global Makefile and can be removed as well

> +cflags-y += $(call as-option,-Wa$(comma)-mno-fix-loongson3-llsc,)
> +cflags-y += -U_LOONGARCH_ISA -D_LOONGARCH_ISA=_LOONGARCH_ISA_LOONGARCH64
> +
> +load-y                         = 0x9000000000200000
> +
> +drivers-$(CONFIG_PCI)          += arch/loongarch/pci/
> +
> +KBUILD_AFLAGS  += $(cflags-y)
> +KBUILD_CFLAGS  += $(cflags-y)
> +KBUILD_CPPFLAGS += -DVMLINUX_LOAD_ADDRESS=$(load-y)
> +
> +bootvars-y     = VMLINUX_LOAD_ADDRESS=$(load-y) PLATFORM="$(platform-y)"

I would argue that VMLINUX_LOAD_ADDRESS should not be configurable
here, instead all kernels should use the same value.

> diff --git a/arch/loongarch/include/asm/Kbuild b/arch/loongarch/include/asm/Kbuild
> new file mode 100644
> index 000000000000..41a76e675321
> --- /dev/null
> +++ b/arch/loongarch/include/asm/Kbuild
> @@ -0,0 +1,31 @@
> +# SPDX-License-Identifier: GPL-2.0
> +generic-y += dma-contiguous.h
> +generic-y += export.h
> +generic-y += mcs_spinlock.h
> +generic-y += parport.h
> +generic-y += early_ioremap.h
> +generic-y += qrwlock.h
> +generic-y += qspinlock.h

The list is apparently from an older kernel and no longer needed for
files that are listed in include/asm-generic/Kbuild.

Please only list the files that are not already there.

        Arnd