linux-next: build failure after merge of the arm64 tree

linux-next: build failure after merge of the arm64 tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 993 bytes --]

Hi Catalin,

Yesterday's linux-next overnight builds (most arm builds) failed like
this:

./arch/arm/kernel/vmlinux.lds:677: undefined symbol `__hyp_idmap_size' referenced in expression

See, for example,
http://kisskb.ellerman.id.au/kisskb/buildresult/12391163/ . Have a look
at http://kisskb.ellerman.id.au/kisskb/head/8631/ to see the full
carnage :-(

This has been happening for a couple of days and then before that we got:

/opt/cross/gcc-4.6.3-nolibc/arm-unknown-linux-gnueabi/bin/arm-unknown-linux-gnueabi-ld:./arch/arm/kernel/vmlinux.lds:544: syntax error

We are using gcc 4.6.3 and binutils 2.22 to do these builds.

I have reverted

12eb3e833961	ARM: kvm: assert on HYP section boundaries not actual code size
e60a1fec44a2	ARM: kvm: implement replacement for ld's LOG2CEIL()
06f75a1f6200	ARM, arm64: kvm: get rid of the bounce page

from linux-next today.  Please address this ASAP.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

linux-next: build failure after merge of the arm64 tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 925 bytes --]

Hi all,

After merging the arm64 tree, today's linux-next build (x86_64
allmodconfig) failed like this:

drivers/perf/amlogic/meson_g12_ddr_pmu.c: In function 'dmc_g12_get_freq_quick':
drivers/perf/amlogic/meson_g12_ddr_pmu.c:135:15: error: implicit declaration of function 'readl' [-Werror=implicit-function-declaration]
  135 |         val = readl(info->pll_reg);
      |               ^~~~~
drivers/perf/amlogic/meson_g12_ddr_pmu.c: In function 'dmc_g12_counter_enable':
drivers/perf/amlogic/meson_g12_ddr_pmu.c:204:9: error: implicit declaration of function 'writel' [-Werror=implicit-function-declaration]
  204 |         writel(clock_count, info->ddr_reg[0] + DMC_MON_G12_TIMER);
      |         ^~~~~~

Caused by commit

  2016e2113d35 ("perf/amlogic: Add support for Amlogic meson G12 SoC DDR PMU driver")

I have used the arm64 tree from next-20221121 for today.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: build failure after merge of the arm64 tree

[Jiucheng Xu]

Hi Stephen,

Could you please share your compiler information, commit id and other

environments info for me to reproduce the building error?

Thanks,

Jiucheng

On 2022/11/22 7:41, Stephen Rothwell wrote:
> Hi all,
>
> After merging the arm64 tree, today's linux-next build (x86_64
> allmodconfig) failed like this:
>
> drivers/perf/amlogic/meson_g12_ddr_pmu.c: In function 'dmc_g12_get_freq_quick':
> drivers/perf/amlogic/meson_g12_ddr_pmu.c:135:15: error: implicit declaration of function 'readl' [-Werror=implicit-function-declaration]
>    135 |         val = readl(info->pll_reg);
>        |               ^~~~~
> drivers/perf/amlogic/meson_g12_ddr_pmu.c: In function 'dmc_g12_counter_enable':
> drivers/perf/amlogic/meson_g12_ddr_pmu.c:204:9: error: implicit declaration of function 'writel' [-Werror=implicit-function-declaration]
>    204 |         writel(clock_count, info->ddr_reg[0] + DMC_MON_G12_TIMER);
>        |         ^~~~~~
>
> Caused by commit
>
>    2016e2113d35 ("perf/amlogic: Add support for Amlogic meson G12 SoC DDR PMU driver")
>
> I have used the arm64 tree from next-20221121 for today.
>
-- 
Thanks,
Jiucheng

Re: linux-next: build failure after merge of the arm64 tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 991 bytes --]

Hi,

On Tue, 22 Nov 2022 12:17:24 +0800 Jiucheng Xu <jiucheng.xu@amlogic.com> wrote:
>
> Could you please share your compiler information, commit id and other
> 
> environments info for me to reproduce the building error?

$ x86_64-linux-gnu-gcc --version
x86_64-linux-gnu-gcc (Debian 12.2.0-3) 12.2.0

The head of the arm64 tree that I merged was

  27131b99f024 ("Merge branches 'for-next/acpi', 'for-next/asm-const', 'for-next/cpufeature', 'for-next/dynamic-scs', 'for-next/errata', 'for-next/fpsimd', 'for-next/insn', 'for-next/kbuild', 'for-next/kdump', 'for-next/mm', 'for-next/perf', 'for-next/selftests', 'for-next/stacks', 'for-next/trivial', 'for-next/uaccess' and 'for-next/undef-traps' into for-next/core")

This is a cross build, host ppc64le, target x86_64 allmodconfig.

drivers/perf/amlogic/meson_g12_ddr_pmu.c needs to include linux/io.h
(and your test builds probably got it (or asm/io.h) via some other
include file).

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

linux-next: build failure after merge of the arm64 tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 644 bytes --]

Hi all,

After merging the arm64 tree, today's linux-next build (x86_64
allmodconfig) failed like this:

drivers/perf/arm_cspmu/arm_cspmu.c: In function 'arm_cspmu_get_cpus':
drivers/perf/arm_cspmu/arm_cspmu.c:1114:29: error: implicit declaration of function 'get_acpi_id_for_cpu' [-Werror=implicit-function-declaration]
 1114 |                             get_acpi_id_for_cpu(cpu)) {
      |                             ^~~~~~~~~~~~~~~~~~~

Caused by commit

  e37dfd65731d ("perf: arm_cspmu: Add support for ARM CoreSight PMU driver")

I have used the arm64 tree from next-20221115 for today.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

RE: linux-next: build failure after merge of the arm64 tree

[Besar Wicaksono]

> -----Original Message-----
> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Sent: Tuesday, November 15, 2022 4:04 PM
> To: Catalin Marinas <catalin.marinas@arm.com>; Will Deacon
> <will@kernel.org>
> Cc: Besar Wicaksono <bwicaksono@nvidia.com>; Linux Kernel Mailing List
> <linux-kernel@vger.kernel.org>; Linux Next Mailing List <linux-
> next@vger.kernel.org>
> Subject: linux-next: build failure after merge of the arm64 tree
> 
> Hi all,
> 
> After merging the arm64 tree, today's linux-next build (x86_64
> allmodconfig) failed like this:
> 
> drivers/perf/arm_cspmu/arm_cspmu.c: In function 'arm_cspmu_get_cpus':
> drivers/perf/arm_cspmu/arm_cspmu.c:1114:29: error: implicit declaration of
> function 'get_acpi_id_for_cpu' [-Werror=implicit-function-declaration]
>  1114 |                             get_acpi_id_for_cpu(cpu)) {
>       |                             ^~~~~~~~~~~~~~~~~~~
> 

+CC Suzuki for awareness.

This function is used for searching CPU with matching ACPI Processor ID in PPTT.
I will check if adding PPTT dependency to arm_cspmu would resolve this issue.

Regards,
Besar

> Caused by commit
> 
>   e37dfd65731d ("perf: arm_cspmu: Add support for ARM CoreSight PMU
> driver")
> 
> I have used the arm64 tree from next-20221115 for today.
> 
> --
> Cheers,
> Stephen Rothwell

Re: linux-next: build failure after merge of the arm64 tree

[Suzuki K Poulose]

On 15/11/2022 23:52, Besar Wicaksono wrote:
> 
> 
>> -----Original Message-----
>> From: Stephen Rothwell <sfr@canb.auug.org.au>
>> Sent: Tuesday, November 15, 2022 4:04 PM
>> To: Catalin Marinas <catalin.marinas@arm.com>; Will Deacon
>> <will@kernel.org>
>> Cc: Besar Wicaksono <bwicaksono@nvidia.com>; Linux Kernel Mailing List
>> <linux-kernel@vger.kernel.org>; Linux Next Mailing List <linux-
>> next@vger.kernel.org>
>> Subject: linux-next: build failure after merge of the arm64 tree
>>
>> Hi all,
>>
>> After merging the arm64 tree, today's linux-next build (x86_64
>> allmodconfig) failed like this:
>>
>> drivers/perf/arm_cspmu/arm_cspmu.c: In function 'arm_cspmu_get_cpus':
>> drivers/perf/arm_cspmu/arm_cspmu.c:1114:29: error: implicit declaration of
>> function 'get_acpi_id_for_cpu' [-Werror=implicit-function-declaration]
>>   1114 |                             get_acpi_id_for_cpu(cpu)) {
>>        |                             ^~~~~~~~~~~~~~~~~~~
>>
> 
> +CC Suzuki for awareness.
> 
> This function is used for searching CPU with matching ACPI Processor ID in PPTT.
> I will check if adding PPTT dependency to arm_cspmu would resolve this issue.

Please make this dependent on ARM64. That should resolve the failure.
As such the helper is available with ARM64 && ACPI.

Suzuki

> 
> Regards,
> Besar
> 
>> Caused by commit
>>
>>    e37dfd65731d ("perf: arm_cspmu: Add support for ARM CoreSight PMU
>> driver")
>>
>> I have used the arm64 tree from next-20221115 for today.
>>
>> --
>> Cheers,
>> Stephen Rothwell

linux-next: build failure after merge of the arm64 tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 423 bytes --]

Hi all,

After merging the arm64 tree, today's linux-next build (powerpc
ppc64_defconfig) was just spinning in make - it executing some scripts,
but it was hard to catch just what.

Apparently caused by commit

  5cf896fb6be3 ("arm64: Add support for relocating the kernel with RELR relocations")

I have not idea why, but reverting the above commit allows to build
to finish.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: build failure after merge of the arm64 tree

[Peter Collingbourne]

On Tue, Aug 6, 2019 at 4:50 PM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Hi all,
>
> After merging the arm64 tree, today's linux-next build (powerpc
> ppc64_defconfig) was just spinning in make - it executing some scripts,
> but it was hard to catch just what.
>
> Apparently caused by commit
>
>   5cf896fb6be3 ("arm64: Add support for relocating the kernel with RELR relocations")
>
> I have not idea why, but reverting the above commit allows to build
> to finish.

Okay, I can reproduce with:

$ make ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnu- defconfig
*** Default configuration is based on 'ppc64_defconfig'
#
# No change to .config
#
$ make ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnu- -j72
scripts/kconfig/conf  --syncconfig Kconfig
scripts/kconfig/conf  --syncconfig Kconfig
scripts/kconfig/conf  --syncconfig Kconfig
[...]

And confirmed that backing out my patch fixes it.

The problem seems to come from the use of $(NM) in the Kconfig file.
If I apply this diff:

diff --git a/init/Kconfig b/init/Kconfig
index d96127ebc44e0..177a95b323230 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -31,7 +31,7 @@ config CC_HAS_ASM_GOTO
        def_bool $(success,$(srctree)/scripts/gcc-goto.sh $(CC))

 config TOOLS_SUPPORT_RELR
-       def_bool $(success,env "CC=$(CC)" "LD=$(LD)" "NM=$(NM)"
"OBJCOPY=$(OBJCOPY)" $(srctree)/scripts/tools-support-relr.sh)
+       def_bool $(success,$(NM))

 config CC_HAS_WARN_MAYBE_UNINITIALIZED
        def_bool $(cc-option,-Wmaybe-uninitialized)

I still see the hang. Replacing $(NM) with something else makes it go away.

That leads me to ask what is special about $(NM) + powerpc? It turns
out to be this fragment of arch/powerpc/Makefile:

ifdef CONFIG_PPC64
new_nm := $(shell if $(NM) --help 2>&1 | grep -- '--synthetic' >
/dev/null; then echo y; else echo n; fi)

ifeq ($(new_nm),y)
NM              := $(NM) --synthetic
endif
endif

We're setting NM to something else based on a config option, which I
presume sets up some sort of circular dependency that confuses
Kconfig. Removing this fragment of the makefile (or appending
--synthetic unconditionally) also makes the problem go away.

We should at least able to remove the test for a new-enough binutils.
According to changes.rst we require binutils 2.21 which was released
in 2011, and support for --synthetic was added to binutils in 2004:
https://github.com/bminor/binutils-gdb/commit/0873df2aec48685715d2c5041c1f6f4ed43976c1

But why are we passing --synthetic at all on ppc64? This behaviour
seems to come from this commit from 2004:
https://github.com/mpe/linux-fullhistory/commit/0e32679a4ea48a634d94e97355d47512ef14d71f
which states: "On new toolchains we need to use nm --synthetic or we
miss code symbols."

But I only see a couple of missing symbols if I compare the output of
nm with and without --synthetic on a powerpc64 kernel:

$ diff -u <(powerpc-linux-gnu-nm --synthetic vmlinux)
<(powerpc-linux-gnu-nm  vmlinux)
--- /dev/fd/63 2019-08-06 18:48:56.127020621 -0700
+++ /dev/fd/62 2019-08-06 18:48:56.131020636 -0700
@@ -74840,7 +74840,6 @@
 c000000001901b10 D LZ4_decompress_fast
 c0000000007819a0 T .LZ4_decompress_fast_continue
 c000000001901b70 D LZ4_decompress_fast_continue
-c0000000007811e0 t .LZ4_decompress_fast_extDict
 c000000001901b40 d LZ4_decompress_fast_extDict
 c000000000781960 T .LZ4_decompress_fast_usingDict
 c000000001901b58 D LZ4_decompress_fast_usingDict
@@ -74856,7 +74855,6 @@
 c000000001901bd0 D LZ4_decompress_safe_usingDict
 c0000000007822b0 T .LZ4_decompress_safe_withPrefix64k
 c000000001901b88 D LZ4_decompress_safe_withPrefix64k
-c000000000780c60 t .LZ4_decompress_safe_withSmallPrefix
 c000000001901b28 d LZ4_decompress_safe_withSmallPrefix
 c00000000077fbe0 T .LZ4_setStreamDecode
 c000000001901ac8 D LZ4_setStreamDecode

I guess the problem was worse back in 2004. It almost certainly didn't
involve these particular symbols because they were added in commit
2209fda323e2fd2a2d0885595fd5097717f8d2aa from 2018. So I guess we have
a couple of possible quick fixes (assuming that the Kconfig issue
can't be solved somehow): either stop passing --synthetic on powerpc
and lose a couple of symbols in 64-bit kernels, or start passing it
unconditionally on powerpc (it doesn't seem to make a difference to
the nm output on a ppc64_defconfig kernel with CONFIG_PPC64=n). I'm
cc'ing the powerpc maintainers for their opinion on what to do. While
this is being resolved we should probably back out my patch from
-next.

Peter

Re: linux-next: build failure after merge of the arm64 tree

[Will Deacon]

Hi Peter,

On Tue, Aug 06, 2019 at 07:34:36PM -0700, Peter Collingbourne wrote:
> On Tue, Aug 6, 2019 at 4:50 PM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> > After merging the arm64 tree, today's linux-next build (powerpc
> > ppc64_defconfig) was just spinning in make - it executing some scripts,
> > but it was hard to catch just what.
> >
> > Apparently caused by commit
> >
> >   5cf896fb6be3 ("arm64: Add support for relocating the kernel with RELR relocations")
> >
> > I have not idea why, but reverting the above commit allows to build
> > to finish.
> 
> Okay, I can reproduce with:

Likewise.

> That leads me to ask what is special about $(NM) + powerpc? It turns
> out to be this fragment of arch/powerpc/Makefile:
> 
> ifdef CONFIG_PPC64
> new_nm := $(shell if $(NM) --help 2>&1 | grep -- '--synthetic' >
> /dev/null; then echo y; else echo n; fi)
> 
> ifeq ($(new_nm),y)
> NM              := $(NM) --synthetic
> endif
> endif
> 
> We're setting NM to something else based on a config option, which I
> presume sets up some sort of circular dependency that confuses
> Kconfig. Removing this fragment of the makefile (or appending
> --synthetic unconditionally) also makes the problem go away.

Yes, I think you're right. The lack of something like KBUILD_NMFLAGS means
that architectures are forced to override NM entirely if they want to pass
any specific options. Making that conditional on a Kconfig option appears
to send the entire thing recursive.

> So I guess we have a couple of possible quick fixes (assuming that the
> Kconfig issue can't be solved somehow): either stop passing --synthetic on
> powerpc and lose a couple of symbols in 64-bit kernels, or start passing
> it unconditionally on powerpc (it doesn't seem to make a difference to the
> nm output on a ppc64_defconfig kernel with CONFIG_PPC64=n). I'm cc'ing the
> powerpc maintainers for their opinion on what to do. While this is being
> resolved we should probably back out my patch from -next.

Although Alpha, Itanic and PowerPC all override NM, only PowerPC does it
conditionally so I agree with you that passing '--synthetic' unconditionally
would resolve the problem and is certainly my preferred approach if mpe is
ok with it.

In the meantime, it seems a shame to revert your patch, so I'll bodge it
as below and we can revert the bodge if PowerPC manages to remove the
conditional NM override. Sound ok to you?

Cheers,

Will

--->8

diff --git a/init/Kconfig b/init/Kconfig
index d96127ebc44e..a38027a06b79 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -31,7 +31,7 @@ config CC_HAS_ASM_GOTO
 	def_bool $(success,$(srctree)/scripts/gcc-goto.sh $(CC))
 
 config TOOLS_SUPPORT_RELR
-	def_bool $(success,env "CC=$(CC)" "LD=$(LD)" "NM=$(NM)" "OBJCOPY=$(OBJCOPY)" $(srctree)/scripts/tools-support-relr.sh)
+	def_bool $(success,env "CC=$(CC)" "LD=$(LD)" "NM=$(CROSS_COMPILE)nm" "OBJCOPY=$(OBJCOPY)" $(srctree)/scripts/tools-support-relr.sh)
 
 config CC_HAS_WARN_MAYBE_UNINITIALIZED
 	def_bool $(cc-option,-Wmaybe-uninitialized)

Re: linux-next: build failure after merge of the arm64 tree

[Masahiro Yamada]

Hi.

On Wed, Aug 7, 2019 at 8:46 PM Will Deacon <will@kernel.org> wrote:
>
> Hi Peter,
>
> On Tue, Aug 06, 2019 at 07:34:36PM -0700, Peter Collingbourne wrote:
> > On Tue, Aug 6, 2019 at 4:50 PM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> > > After merging the arm64 tree, today's linux-next build (powerpc
> > > ppc64_defconfig) was just spinning in make - it executing some scripts,
> > > but it was hard to catch just what.
> > >
> > > Apparently caused by commit
> > >
> > >   5cf896fb6be3 ("arm64: Add support for relocating the kernel with RELR relocations")
> > >
> > > I have not idea why, but reverting the above commit allows to build
> > > to finish.
> >
> > Okay, I can reproduce with:
>
> Likewise.
>
> > That leads me to ask what is special about $(NM) + powerpc? It turns
> > out to be this fragment of arch/powerpc/Makefile:
> >
> > ifdef CONFIG_PPC64
> > new_nm := $(shell if $(NM) --help 2>&1 | grep -- '--synthetic' >
> > /dev/null; then echo y; else echo n; fi)
> >
> > ifeq ($(new_nm),y)
> > NM              := $(NM) --synthetic
> > endif
> > endif
> >
> > We're setting NM to something else based on a config option, which I
> > presume sets up some sort of circular dependency that confuses
> > Kconfig. Removing this fragment of the makefile (or appending
> > --synthetic unconditionally) also makes the problem go away.

Exactly. This makes a circular dependency.
Kconfig determines the environment variable 'NM' has been changed,
and re-runs.



> Yes, I think you're right. The lack of something like KBUILD_NMFLAGS means
> that architectures are forced to override NM entirely if they want to pass
> any specific options. Making that conditional on a Kconfig option appears
> to send the entire thing recursive.

Adding KBUILD_NMFLAGS is probably the right thing to do.
(Is there somebody who wants to volunteer for this?)

But, for this particular case, I vote for
the entire removal of --synthetic.

This dates back to 2004, and the commit message
did not clearly explain why it was needed.

The PowerPC maintainers should re-evaluate
whether or not this flag is necessary.

ppc32 is working without --synthetic,
so probably ppc64 would be ...



>
> > So I guess we have a couple of possible quick fixes (assuming that the
> > Kconfig issue can't be solved somehow): either stop passing --synthetic on
> > powerpc and lose a couple of symbols in 64-bit kernels, or start passing
> > it unconditionally on powerpc (it doesn't seem to make a difference to the
> > nm output on a ppc64_defconfig kernel with CONFIG_PPC64=n). I'm cc'ing the
> > powerpc maintainers for their opinion on what to do. While this is being
> > resolved we should probably back out my patch from -next.
>
> Although Alpha, Itanic and PowerPC all override NM, only PowerPC does it
> conditionally so I agree with you that passing '--synthetic' unconditionally
> would resolve the problem and is certainly my preferred approach if mpe is
> ok with it.
>
> In the meantime, it seems a shame to revert your patch, so I'll bodge it
> as below and we can revert the bodge if PowerPC manages to remove the
> conditional NM override. Sound ok to you?
>
> Cheers,
>
> Will
>
> --->8
>
> diff --git a/init/Kconfig b/init/Kconfig
> index d96127ebc44e..a38027a06b79 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -31,7 +31,7 @@ config CC_HAS_ASM_GOTO
>         def_bool $(success,$(srctree)/scripts/gcc-goto.sh $(CC))
>
>  config TOOLS_SUPPORT_RELR
> -       def_bool $(success,env "CC=$(CC)" "LD=$(LD)" "NM=$(NM)" "OBJCOPY=$(OBJCOPY)" $(srctree)/scripts/tools-support-relr.sh)
> +       def_bool $(success,env "CC=$(CC)" "LD=$(LD)" "NM=$(CROSS_COMPILE)nm" "OBJCOPY=$(OBJCOPY)" $(srctree)/scripts/tools-support-relr.sh)
>
>  config CC_HAS_WARN_MAYBE_UNINITIALIZED
>         def_bool $(cc-option,-Wmaybe-uninitialized)

Maybe,

def_bool $(success,env "CC=$(CC)" "LD=$(LD)" "OBJCOPY=$(OBJCOPY)"
$(srctree)/scripts/tools-support-relr.sh)

will work.


Or more simply

def_bool $(success,$(srctree)/scripts/tools-support-relr.sh)



CC, LD, OBJCOPY, NM are environment variables,
so I think tools-support-relr.sh can directly use them.


However, this bypasses the detection of environment variable changes.
If a user passes NM= from the command line, Kconfig will no longer
notice the change of NM.



-- 
Best Regards
Masahiro Yamada

Re: linux-next: build failure after merge of the arm64 tree

[Will Deacon]

Hello Masahiro,

On Wed, Aug 07, 2019 at 11:43:02PM +0900, Masahiro Yamada wrote:
> On Wed, Aug 7, 2019 at 8:46 PM Will Deacon <will@kernel.org> wrote:
> > On Tue, Aug 06, 2019 at 07:34:36PM -0700, Peter Collingbourne wrote:
> > > We're setting NM to something else based on a config option, which I
> > > presume sets up some sort of circular dependency that confuses
> > > Kconfig. Removing this fragment of the makefile (or appending
> > > --synthetic unconditionally) also makes the problem go away.
> 
> Exactly. This makes a circular dependency.
> Kconfig determines the environment variable 'NM' has been changed,
> and re-runs.
> 
> > Yes, I think you're right. The lack of something like KBUILD_NMFLAGS means
> > that architectures are forced to override NM entirely if they want to pass
> > any specific options. Making that conditional on a Kconfig option appears
> > to send the entire thing recursive.
> 
> Adding KBUILD_NMFLAGS is probably the right thing to do.
> (Is there somebody who wants to volunteer for this?)

I don't think so ;) We don't do this for many other tools, and there's only
this one case that really seems to require it.

> But, for this particular case, I vote for
> the entire removal of --synthetic.
> 
> This dates back to 2004, and the commit message
> did not clearly explain why it was needed.
> 
> The PowerPC maintainers should re-evaluate
> whether or not this flag is necessary.
> 
> ppc32 is working without --synthetic,
> so probably ppc64 would be ...

Again, this is up to the PPC maintainers.

> > diff --git a/init/Kconfig b/init/Kconfig
> > index d96127ebc44e..a38027a06b79 100644
> > --- a/init/Kconfig
> > +++ b/init/Kconfig
> > @@ -31,7 +31,7 @@ config CC_HAS_ASM_GOTO
> >         def_bool $(success,$(srctree)/scripts/gcc-goto.sh $(CC))
> >
> >  config TOOLS_SUPPORT_RELR
> > -       def_bool $(success,env "CC=$(CC)" "LD=$(LD)" "NM=$(NM)" "OBJCOPY=$(OBJCOPY)" $(srctree)/scripts/tools-support-relr.sh)
> > +       def_bool $(success,env "CC=$(CC)" "LD=$(LD)" "NM=$(CROSS_COMPILE)nm" "OBJCOPY=$(OBJCOPY)" $(srctree)/scripts/tools-support-relr.sh)
> >
> >  config CC_HAS_WARN_MAYBE_UNINITIALIZED
> >         def_bool $(cc-option,-Wmaybe-uninitialized)
> 
> Maybe,
> 
> def_bool $(success,env "CC=$(CC)" "LD=$(LD)" "OBJCOPY=$(OBJCOPY)"
> $(srctree)/scripts/tools-support-relr.sh)
> 
> will work.

Oh yes, I prefer that. I've included the updated patch below, and I'll
put it into -next shortly so that we resolve the build breakage. Hopefully
we'll have a better fix once the ozlabs folks wake up.

> Or more simply
> 
> def_bool $(success,$(srctree)/scripts/tools-support-relr.sh)
> 
> CC, LD, OBJCOPY, NM are environment variables,
> so I think tools-support-relr.sh can directly use them.

Maybe, but the other scripts invoked here tend to pass $(CC) through as
an argument, and that helps with your observation below...:

> However, this bypasses the detection of environment variable changes.
> If a user passes NM= from the command line, Kconfig will no longer
> notice the change of NM.

... but since my proposal also breaks this, then I think your idea of just
dropping $NM for now is best.

Will

--->8

From 71c67a31f09fa8fdd1495dffd96a5f0d4cef2ede Mon Sep 17 00:00:00 2001
From: Will Deacon <will@kernel.org>
Date: Wed, 7 Aug 2019 12:48:33 +0100
Subject: [PATCH] init/Kconfig: Fix infinite Kconfig recursion on PPC

Commit 5cf896fb6be3 ("arm64: Add support for relocating the kernel with
RELR relocations") introduced CONFIG_TOOLS_SUPPORT_RELR, which checks
for RELR support in the toolchain as part of the kernel configuration.
During this procedure, "$(NM)" is invoked to see if it supports the new
relocation format, however PowerPC conditionally overrides this variable
in the architecture Makefile in order to pass '--synthetic' when
targetting PPC64.

This conditional override causes Kconfig to recurse forever, since
CONFIG_TOOLS_SUPPORT_RELR cannot be determined without $(NM) being
defined, but that in turn depends on CONFIG_PPC64:

  $ make ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnu-
  scripts/kconfig/conf  --syncconfig Kconfig
  scripts/kconfig/conf  --syncconfig Kconfig
  scripts/kconfig/conf  --syncconfig Kconfig
  [...]

In this particular case, it looks like PowerPC may be able to pass
'--synthetic' unconditionally to nm or even drop it altogether. While
that is being resolved, let's just bodge the RELR check by picking up
$(NM) directly from the environment in whatever state it happens to be
in.

Cc: Peter Collingbourne <pcc@google.com>
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Suggested-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Signed-off-by: Will Deacon <will@kernel.org>
---
 init/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/init/Kconfig b/init/Kconfig
index d96127ebc44e..8b4596edda4e 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -31,7 +31,7 @@ config CC_HAS_ASM_GOTO
 	def_bool $(success,$(srctree)/scripts/gcc-goto.sh $(CC))
 
 config TOOLS_SUPPORT_RELR
-	def_bool $(success,env "CC=$(CC)" "LD=$(LD)" "NM=$(NM)" "OBJCOPY=$(OBJCOPY)" $(srctree)/scripts/tools-support-relr.sh)
+	def_bool $(success,env "CC=$(CC)" "LD=$(LD)" "OBJCOPY=$(OBJCOPY)" $(srctree)/scripts/tools-support-relr.sh)
 
 config CC_HAS_WARN_MAYBE_UNINITIALIZED
 	def_bool $(cc-option,-Wmaybe-uninitialized)
-- 
2.17.1

Re: linux-next: build failure after merge of the arm64 tree

[Peter Collingbourne]

On Wed, Aug 7, 2019 at 8:25 AM Will Deacon <will@kernel.org> wrote:
>
> Hello Masahiro,
>
> On Wed, Aug 07, 2019 at 11:43:02PM +0900, Masahiro Yamada wrote:
> > On Wed, Aug 7, 2019 at 8:46 PM Will Deacon <will@kernel.org> wrote:
> > > On Tue, Aug 06, 2019 at 07:34:36PM -0700, Peter Collingbourne wrote:
> > > > We're setting NM to something else based on a config option, which I
> > > > presume sets up some sort of circular dependency that confuses
> > > > Kconfig. Removing this fragment of the makefile (or appending
> > > > --synthetic unconditionally) also makes the problem go away.
> >
> > Exactly. This makes a circular dependency.
> > Kconfig determines the environment variable 'NM' has been changed,
> > and re-runs.
> >
> > > Yes, I think you're right. The lack of something like KBUILD_NMFLAGS means
> > > that architectures are forced to override NM entirely if they want to pass
> > > any specific options. Making that conditional on a Kconfig option appears
> > > to send the entire thing recursive.
> >
> > Adding KBUILD_NMFLAGS is probably the right thing to do.
> > (Is there somebody who wants to volunteer for this?)
>
> I don't think so ;) We don't do this for many other tools, and there's only
> this one case that really seems to require it.
>
> > But, for this particular case, I vote for
> > the entire removal of --synthetic.
> >
> > This dates back to 2004, and the commit message
> > did not clearly explain why it was needed.
> >
> > The PowerPC maintainers should re-evaluate
> > whether or not this flag is necessary.
> >
> > ppc32 is working without --synthetic,
> > so probably ppc64 would be ...
>
> Again, this is up to the PPC maintainers.
>
> > > diff --git a/init/Kconfig b/init/Kconfig
> > > index d96127ebc44e..a38027a06b79 100644
> > > --- a/init/Kconfig
> > > +++ b/init/Kconfig
> > > @@ -31,7 +31,7 @@ config CC_HAS_ASM_GOTO
> > >         def_bool $(success,$(srctree)/scripts/gcc-goto.sh $(CC))
> > >
> > >  config TOOLS_SUPPORT_RELR
> > > -       def_bool $(success,env "CC=$(CC)" "LD=$(LD)" "NM=$(NM)" "OBJCOPY=$(OBJCOPY)" $(srctree)/scripts/tools-support-relr.sh)
> > > +       def_bool $(success,env "CC=$(CC)" "LD=$(LD)" "NM=$(CROSS_COMPILE)nm" "OBJCOPY=$(OBJCOPY)" $(srctree)/scripts/tools-support-relr.sh)
> > >
> > >  config CC_HAS_WARN_MAYBE_UNINITIALIZED
> > >         def_bool $(cc-option,-Wmaybe-uninitialized)
> >
> > Maybe,
> >
> > def_bool $(success,env "CC=$(CC)" "LD=$(LD)" "OBJCOPY=$(OBJCOPY)"
> > $(srctree)/scripts/tools-support-relr.sh)
> >
> > will work.
>
> Oh yes, I prefer that. I've included the updated patch below, and I'll
> put it into -next shortly so that we resolve the build breakage. Hopefully
> we'll have a better fix once the ozlabs folks wake up.
>
> > Or more simply
> >
> > def_bool $(success,$(srctree)/scripts/tools-support-relr.sh)
> >
> > CC, LD, OBJCOPY, NM are environment variables,
> > so I think tools-support-relr.sh can directly use them.
>
> Maybe, but the other scripts invoked here tend to pass $(CC) through as
> an argument, and that helps with your observation below...:
>
> > However, this bypasses the detection of environment variable changes.
> > If a user passes NM= from the command line, Kconfig will no longer
> > notice the change of NM.
>
> ... but since my proposal also breaks this, then I think your idea of just
> dropping $NM for now is best.

Thanks Will, that sounds good to me as well. I verified that it fixes
the hang in ppc64 on my end, and that we can still produce RELR
relocations on arm64 by passing in llvm-nm as $NM.

> Will
>
> --->8
>
> From 71c67a31f09fa8fdd1495dffd96a5f0d4cef2ede Mon Sep 17 00:00:00 2001
> From: Will Deacon <will@kernel.org>
> Date: Wed, 7 Aug 2019 12:48:33 +0100
> Subject: [PATCH] init/Kconfig: Fix infinite Kconfig recursion on PPC
>
> Commit 5cf896fb6be3 ("arm64: Add support for relocating the kernel with
> RELR relocations") introduced CONFIG_TOOLS_SUPPORT_RELR, which checks
> for RELR support in the toolchain as part of the kernel configuration.
> During this procedure, "$(NM)" is invoked to see if it supports the new
> relocation format, however PowerPC conditionally overrides this variable
> in the architecture Makefile in order to pass '--synthetic' when
> targetting PPC64.
>
> This conditional override causes Kconfig to recurse forever, since
> CONFIG_TOOLS_SUPPORT_RELR cannot be determined without $(NM) being
> defined, but that in turn depends on CONFIG_PPC64:
>
>   $ make ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnu-
>   scripts/kconfig/conf  --syncconfig Kconfig
>   scripts/kconfig/conf  --syncconfig Kconfig
>   scripts/kconfig/conf  --syncconfig Kconfig
>   [...]
>
> In this particular case, it looks like PowerPC may be able to pass
> '--synthetic' unconditionally to nm or even drop it altogether. While
> that is being resolved, let's just bodge the RELR check by picking up
> $(NM) directly from the environment in whatever state it happens to be
> in.
>
> Cc: Peter Collingbourne <pcc@google.com>
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Suggested-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> Signed-off-by: Will Deacon <will@kernel.org>

Tested-by: Peter Collingbourne <pcc@google.com>
Reviewed-by: Peter Collingbourne <pcc@google.com>

Peter

Re: linux-next: build failure after merge of the arm64 tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 2049 bytes --]

Hi all,

On Wed, 7 Aug 2019 09:33:07 -0700 Peter Collingbourne <pcc@google.com> wrote:
>
> On Wed, Aug 7, 2019 at 8:25 AM Will Deacon <will@kernel.org> wrote:
> >
> > From 71c67a31f09fa8fdd1495dffd96a5f0d4cef2ede Mon Sep 17 00:00:00 2001
> > From: Will Deacon <will@kernel.org>
> > Date: Wed, 7 Aug 2019 12:48:33 +0100
> > Subject: [PATCH] init/Kconfig: Fix infinite Kconfig recursion on PPC
> >
> > Commit 5cf896fb6be3 ("arm64: Add support for relocating the kernel with
> > RELR relocations") introduced CONFIG_TOOLS_SUPPORT_RELR, which checks
> > for RELR support in the toolchain as part of the kernel configuration.
> > During this procedure, "$(NM)" is invoked to see if it supports the new
> > relocation format, however PowerPC conditionally overrides this variable
> > in the architecture Makefile in order to pass '--synthetic' when
> > targetting PPC64.
> >
> > This conditional override causes Kconfig to recurse forever, since
> > CONFIG_TOOLS_SUPPORT_RELR cannot be determined without $(NM) being
> > defined, but that in turn depends on CONFIG_PPC64:
> >
> >   $ make ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnu-
> >   scripts/kconfig/conf  --syncconfig Kconfig
> >   scripts/kconfig/conf  --syncconfig Kconfig
> >   scripts/kconfig/conf  --syncconfig Kconfig
> >   [...]
> >
> > In this particular case, it looks like PowerPC may be able to pass
> > '--synthetic' unconditionally to nm or even drop it altogether. While
> > that is being resolved, let's just bodge the RELR check by picking up
> > $(NM) directly from the environment in whatever state it happens to be
> > in.
> >
> > Cc: Peter Collingbourne <pcc@google.com>
> > Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> > Suggested-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> > Signed-off-by: Will Deacon <will@kernel.org>  
> 
> Tested-by: Peter Collingbourne <pcc@google.com>
> Reviewed-by: Peter Collingbourne <pcc@google.com>

Thanks for sorting this out (even temporarily).

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: build failure after merge of the arm64 tree

[Michael Ellerman]

Will Deacon <will@kernel.org> writes:
> On Tue, Aug 06, 2019 at 07:34:36PM -0700, Peter Collingbourne wrote:
>> On Tue, Aug 6, 2019 at 4:50 PM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>> > After merging the arm64 tree, today's linux-next build (powerpc
>> > ppc64_defconfig) was just spinning in make - it executing some scripts,
>> > but it was hard to catch just what.
>> >
>> > Apparently caused by commit
>> >
>> >   5cf896fb6be3 ("arm64: Add support for relocating the kernel with RELR relocations")
>> >
>> > I have not idea why, but reverting the above commit allows to build
>> > to finish.
>> 
>> Okay, I can reproduce with:
>
> Likewise.
>
>> That leads me to ask what is special about $(NM) + powerpc? It turns
>> out to be this fragment of arch/powerpc/Makefile:
>> 
>> ifdef CONFIG_PPC64
>> new_nm := $(shell if $(NM) --help 2>&1 | grep -- '--synthetic' >
>> /dev/null; then echo y; else echo n; fi)
>> 
>> ifeq ($(new_nm),y)
>> NM              := $(NM) --synthetic
>> endif
>> endif
>> 
>> We're setting NM to something else based on a config option, which I
>> presume sets up some sort of circular dependency that confuses
>> Kconfig. Removing this fragment of the makefile (or appending
>> --synthetic unconditionally) also makes the problem go away.
>
> Yes, I think you're right. The lack of something like KBUILD_NMFLAGS means
> that architectures are forced to override NM entirely if they want to pass
> any specific options. Making that conditional on a Kconfig option appears
> to send the entire thing recursive.
>
>> So I guess we have a couple of possible quick fixes (assuming that the
>> Kconfig issue can't be solved somehow): either stop passing --synthetic on
>> powerpc and lose a couple of symbols in 64-bit kernels, or start passing
>> it unconditionally on powerpc (it doesn't seem to make a difference to the
>> nm output on a ppc64_defconfig kernel with CONFIG_PPC64=n). I'm cc'ing the
>> powerpc maintainers for their opinion on what to do. While this is being
>> resolved we should probably back out my patch from -next.
>
> Although Alpha, Itanic and PowerPC all override NM, only PowerPC does it
> conditionally so I agree with you that passing '--synthetic' unconditionally
> would resolve the problem and is certainly my preferred approach if mpe is
> ok with it.

I'd rather we keep passing --synthetic, otherwise there's the potential
that symbols go missing that were previously visible.

I think we can keep the new_nm check, but drop the dependency on
CONFIG_PPC64, and that will fix it. Worst case is we start passing
--synthetic on ppc32, but that's probably not a problem.

This seems to fix it for me, and 32-bit builds fine.

Do you want me to send a proper patch for this, or do you want to squash
it into the original series?

diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index c345b79414a9..403f7e193833 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -39,13 +39,11 @@ endif
 uname := $(shell uname -m)
 KBUILD_DEFCONFIG := $(if $(filter ppc%,$(uname)),$(uname),ppc64)_defconfig
 
-ifdef CONFIG_PPC64
 new_nm := $(shell if $(NM) --help 2>&1 | grep -- '--synthetic' > /dev/null; then echo y; else echo n; fi)
 
 ifeq ($(new_nm),y)
 NM             := $(NM) --synthetic
 endif
-endif
 
 # BITS is used as extension for files which are available in a 32 bit
 # and a 64 bit version to simplify shared Makefiles.


cheers

Re: linux-next: build failure after merge of the arm64 tree

[Will Deacon]

Hi Michael,

On Fri, Aug 16, 2019 at 02:52:40PM +1000, Michael Ellerman wrote:
> Will Deacon <will@kernel.org> writes:
> > Although Alpha, Itanic and PowerPC all override NM, only PowerPC does it
> > conditionally so I agree with you that passing '--synthetic' unconditionally
> > would resolve the problem and is certainly my preferred approach if mpe is
> > ok with it.
> 
> I'd rather we keep passing --synthetic, otherwise there's the potential
> that symbols go missing that were previously visible.

Yup -- that was my suggestion above.

> I think we can keep the new_nm check, but drop the dependency on
> CONFIG_PPC64, and that will fix it. Worst case is we start passing
> --synthetic on ppc32, but that's probably not a problem.
> 
> This seems to fix it for me, and 32-bit builds fine.

Brill, thanks for confirming!

> Do you want me to send a proper patch for this, or do you want to squash
> it into the original series?

I'd prefer not to rebase the arm64 queue, so if you send this as a proper
patch, please, then I can queue it on top before reverting the hack we
currently have.

Will

Re: linux-next: build failure after merge of the arm64 tree

[Michael Ellerman]

Will Deacon <will@kernel.org> writes:
> Hi Michael,
>
> On Fri, Aug 16, 2019 at 02:52:40PM +1000, Michael Ellerman wrote:
>> Will Deacon <will@kernel.org> writes:
>> > Although Alpha, Itanic and PowerPC all override NM, only PowerPC does it
>> > conditionally so I agree with you that passing '--synthetic' unconditionally
>> > would resolve the problem and is certainly my preferred approach if mpe is
>> > ok with it.
>> 
>> I'd rather we keep passing --synthetic, otherwise there's the potential
>> that symbols go missing that were previously visible.
>
> Yup -- that was my suggestion above.
>
>> I think we can keep the new_nm check, but drop the dependency on
>> CONFIG_PPC64, and that will fix it. Worst case is we start passing
>> --synthetic on ppc32, but that's probably not a problem.
>> 
>> This seems to fix it for me, and 32-bit builds fine.
>
> Brill, thanks for confirming!
>
>> Do you want me to send a proper patch for this, or do you want to squash
>> it into the original series?
>
> I'd prefer not to rebase the arm64 queue, so if you send this as a proper
> patch, please, then I can queue it on top before reverting the hack we
> currently have.

Cool, just sent a patch.

cheers

linux-next: build failure after merge of the arm64 tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 826 bytes --]

Hi Catalin,

After merging the arm64 tree, today's linux-next build ()
failed like this:

arch/powerpc/kernel/sys_ppc32.c:146:17: error: conflicting types for 'compat_sys_sendfile'
include/linux/compat.h:593:16: note: previous declaration of 'compat_sys_sendfile' was here
arch/powerpc/kernel/sys_ppc32.c:231:17: error: conflicting types for 'compat_sys_sched_rr_get_interval'
include/linux/compat.h:596:16: note: previous declaration of 'compat_sys_sched_rr_get_interval' was here

Caused by commits 2a23f0dba0c6 ("fs: Add generic compat_sys_sendfile
implementation") and a0c6fc489bf6 ("compat: Add generic
compat_sys_sched_rr_get_interval implementation").  Grep is your friend.

I have used the arm64 tree from next-20120914 for today.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: linux-next: build failure after merge of the arm64 tree

[Catalin Marinas]

Hi Stephen,

On Mon, Sep 17, 2012 at 02:24:05AM +0100, Stephen Rothwell wrote:
> After merging the arm64 tree, today's linux-next build ()
> failed like this:
> 
> arch/powerpc/kernel/sys_ppc32.c:146:17: error: conflicting types for 'compat_sys_sendfile'
> include/linux/compat.h:593:16: note: previous declaration of 'compat_sys_sendfile' was here
> arch/powerpc/kernel/sys_ppc32.c:231:17: error: conflicting types for 'compat_sys_sched_rr_get_interval'
> include/linux/compat.h:596:16: note: previous declaration of 'compat_sys_sched_rr_get_interval' was here
> 
> Caused by commits 2a23f0dba0c6 ("fs: Add generic compat_sys_sendfile
> implementation") and a0c6fc489bf6 ("compat: Add generic
> compat_sys_sched_rr_get_interval implementation").  Grep is your friend.

Thanks for this. It was an attempt to add generic compat_sys_sendfile()
and compat_sys_sched_rr_get_interval() implementations to be shared by
other architectures (patches for the other architectures to be pushed
separately). I'll drop them for now as PowerPC has different prototypes
and push the patches separately.

-- 
Catalin