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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAK8P3a0E3fRJU9VCRp7MQz-jBC0nHi1DwjPGtymEg3vJJoDqrg@mail.gmail.com>
Date:   Fri, 17 Sep 2021 09:53:18 +0200
From:   Arnd Bergmann <arnd@...db.de>
To:     Huacai Chen <chenhuacai@...ngson.cn>
Cc:     Arnd Bergmann <arnd@...db.de>, Andy Lutomirski <luto@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Peter Zijlstra <peterz@...radead.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        David Airlie <airlied@...ux.ie>,
        Jonathan Corbet <corbet@....net>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        linux-arch <linux-arch@...r.kernel.org>,
        "open list:DOCUMENTATION" <linux-doc@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Xuefeng Li <lixuefeng@...ngson.cn>,
        Yanteng Si <siyanteng@...ngson.cn>,
        Huacai Chen <chenhuacai@...il.com>,
        Jiaxun Yang <jiaxun.yang@...goat.com>
Subject: Re: [PATCH V3 05/22] LoongArch: Add build infrastructure

On Fri, Sep 17, 2021 at 5:57 AM Huacai Chen <chenhuacai@...ngson.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