Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 19 Apr 2023 17:59:00 +0200
From:   Borislav Petkov <bp@alien8.de>
To:     Josh Poimboeuf <jpoimboe@kernel.org>
Cc:     Linus Torvalds <torvalds@linux-foundation.org>,
        Peter Zijlstra <peterz@infradead.org>, x86-ml <x86@kernel.org>,
        lkml <linux-kernel@vger.kernel.org>
Subject: Re: [GIT PULL] x86/urgent for v6.3-rc7
Message-ID: <20230419155900.GCZEAPxNiOUP31q+/H@fat_crate.local>
References: <20230416122913.GCZDvqGVe9TPa5LPRm@fat_crate.local>
 <CAHk-=wjOZgMu2VYBHMt-yFvurAqWsxLG8wh59km=B245CXRKxA@mail.gmail.com>
 <20230417090412.GAZD0LjH5ZIaGUdoHH@fat_crate.local>
 <20230418012435.fhjxd6moaz6tmnep@treble>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20230418012435.fhjxd6moaz6tmnep@treble>
Precedence: bulk

On Mon, Apr 17, 2023 at 06:24:35PM -0700, Josh Poimboeuf wrote:
> I implemented it in the patch below.

Ha, amazing! Thanks man. :-)

> It was pretty straightforward
> since this case always(?) involves two relocations within a single
> instruction, like:
> 
>  3d9:	48 c7 05 00 00 00 00 00 00 00 00 	movq   $0x0,0x0(%rip)        # 3e4 <x86_wallclock_init+0xa4>	3dc: R_X86_64_PC32	x86_platform+0x8
> 	3e0: R_X86_64_32S	.init.text+0x280
> 
> So objtool warns if the instruction's first relocation is non-init and
> the second one is init.

Sounds plausible to me.

> There are 9 "disallowed store" warnings.  There would have been more,
> but I added per-symbol rate-limiting to keep the total number of
> warnings down.  For example, instead of the x86_wallclock_init() issue
> discovered above, it shows a similar warning from elsewhere:
> 
>   vmlinux.o: warning: objtool: xen_init_apic+0xab: disallowed store of init symbol xen_apic_check to non-init data x86_platform

Obviously a good catch.

However, the only call site is in apic_intr_mode_init() which itself is
__init. So yeah, strictly speaking nothing wrong.

*IF* something calls it later, when __init is gone, then boom.

Dunno, maybe

a) track call sites too but maybe expensive and too much

or

b) make those warnings debug messages in case someone wants to run objtool
in debug mode, feels really bored and wants to fix potential issues.

Or someone has a better idea.

> Anyway, this is kind of experimental.  If any of these warnings don't
> turn out to be useful I could drop some or all them.

Right, I can certainly see potential and as said, since we're already
doing objtool massaging of object files, thought this should be pretty
easy to do. As you've shown. ;-)

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette