Re: [PATCH v5 11/17] kbuild: Add generic hook for architectures to use before the final vmlinux link

[Hari Bathini <hbathini@linux.ibm.com>]

Hello Masahiro,

On 10/10/24 5:07 pm, Masahiro Yamada wrote:
> On Thu, Oct 10, 2024 at 6:57 PM Hari Bathini <hbathini@linux.ibm.com> wrote:
>>
>>
>> On 09/10/24 8:53 pm, Masahiro Yamada wrote:
>>> On Mon, Sep 16, 2024 at 5:58 AM Hari Bathini <hbathini@linux.ibm.com> wrote:
>>>>
>>>> From: Naveen N Rao <naveen@kernel.org>
>>>>
>>>> On powerpc, we would like to be able to make a pass on vmlinux.o and
>>>> generate a new object file to be linked into vmlinux. Add a generic pass
>>>> in Makefile.vmlinux that architectures can use for this purpose.
>>>>
>>>> Architectures need to select CONFIG_ARCH_WANTS_PRE_LINK_VMLINUX and must
>>>> provide arch/<arch>/tools/Makefile with .arch.vmlinux.o target, which
>>>> will be invoked prior to the final vmlinux link step.
>>>>
>>>> Signed-off-by: Naveen N Rao <naveen@kernel.org>
>>>> Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
>>>> ---
>>>>
>>>> Changes in v5:
>>>> * Intermediate files named .vmlinux.arch.* instead of .arch.vmlinux.*
>>>>
>>>>
>>>>    arch/Kconfig             | 6 ++++++
>>>>    scripts/Makefile.vmlinux | 7 +++++++
>>>>    scripts/link-vmlinux.sh  | 7 ++++++-
>>>>    3 files changed, 19 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/Kconfig b/arch/Kconfig
>>>> index 975dd22a2dbd..ef868ff8156a 100644
>>>> --- a/arch/Kconfig
>>>> +++ b/arch/Kconfig
>>>> @@ -1643,4 +1643,10 @@ config CC_HAS_SANE_FUNCTION_ALIGNMENT
>>>>    config ARCH_NEED_CMPXCHG_1_EMU
>>>>           bool
>>>>
>>>> +config ARCH_WANTS_PRE_LINK_VMLINUX
>>>> +       def_bool n
>>>
>>>
>>> Redundant default. This line should be "bool".
>>>
>>>
>>>
>>>
>>>
>>>
>>>> +       help
>>>> +         An architecture can select this if it provides arch/<arch>/tools/Makefile
>>>> +         with .arch.vmlinux.o target to be linked into vmlinux.
>>>> +
>>>>    endmenu
>>>> diff --git a/scripts/Makefile.vmlinux b/scripts/Makefile.vmlinux
>>>> index 49946cb96844..edf6fae8d960 100644
>>>> --- a/scripts/Makefile.vmlinux
>>>> +++ b/scripts/Makefile.vmlinux
>>>> @@ -22,6 +22,13 @@ targets += .vmlinux.export.o
>>>>    vmlinux: .vmlinux.export.o
>>>>    endif
>>>>
>>>> +ifdef CONFIG_ARCH_WANTS_PRE_LINK_VMLINUX
>>>> +vmlinux: arch/$(SRCARCH)/tools/.vmlinux.arch.o
>>>
>>> If you move this to arch/*/tools/, there is no reason
>>> to make it a hidden file.
>>
>> Thanks for reviewing and the detailed comments, Masahiro.
>>
>>>
>>>
>>> vmlinux: arch/$(SRCARCH)/tools/vmlinux.arch.o
>>>
>>>
>>>
>>>
>>>> +arch/$(SRCARCH)/tools/.vmlinux.arch.o: vmlinux.o
>>>
>>> FORCE is missing.
>>
>>
>> I dropped FORCE as it was rebuilding vmlinux on every invocation
>> of `make` irrespective of whether vmlinux.o changed or not..
> 
> 
> It is because you did not add vmlinux.arch.S to 'targets'
> 
> See my comment in 12/17.
> 
>    targets += vmlinux.arch.S
> 
> 
>> Just curious if the changes you suggested makes FORCE necessary
>> or FORCE was expected even without the other changes you suggested?
> 
> 
> FORCE is necessary.
> 
> arch/powerpc/tools/Makefile must be checked every time.
> 
> 
> When arch/powerpc/tools/ftrace-gen-ool-stubs.sh is changed,
> vmlinux must be relinked.

Thanks for the review and clarifications!
Posted v6 with the changes. Please review:

  
https://lore.kernel.org/all/20241018173632.277333-1-hbathini@linux.ibm.com/

- Hari