Re: [GIT PULL] objtool/urgent for v5.15-rc4

From: Linus Torvalds <torvalds@linux-foundation.org>
To: Borislav Petkov <bp@suse.de>, Paolo Bonzini <pbonzini@redhat.com>,
	Sean Christopherson <seanjc@google.com>,
	Vitaly Kuznetsov <vkuznets@redhat.com>,
	Wanpeng Li <wanpengli@tencent.com>,
	Jim Mattson <jmattson@google.com>, Joerg Roedel <joro@8bytes.org>,
	"Peter Zijlstra (Intel)" <peterz@infradead.org>,
	Josh Poimboeuf <jpoimboe@redhat.com>,
	Nathan Chancellor <nathan@kernel.org>
Cc: x86-ml <x86@kernel.org>, lkml <linux-kernel@vger.kernel.org>,
	KVM list <kvm@vger.kernel.org>
Subject: Re: [GIT PULL] objtool/urgent for v5.15-rc4
Date: Sun, 3 Oct 2021 12:10:38 -0700	[thread overview]
Message-ID: <CAHk-=wjtJ532TqnLN+CLqZJXx=MWHjQqi0-fR8PSQ-nGZ_iMvg@mail.gmail.com> (raw)
In-Reply-To: <CAHk-=wiZwq-0LknKhXN4M+T8jbxn_2i9mcKpO+OaBSSq_Eh7tg@mail.gmail.com>

Replying to myself just to add more proper people to the cc.

I'm also wondering how I could possibly be the only person who saw the warning.

I don't think I am, and I think that people who signed off on commit
24ff65257375 ("objtool: Teach get_alt_entry() about more relocation
types") and claimed to have "tested" it, clearly didn't actually do
so.

PeterZ/Josh/Nathan: see the thread at

   https://lore.kernel.org/lkml/CAHk-=wiZwq-0LknKhXN4M+T8jbxn_2i9mcKpO+OaBSSq_Eh7tg@mail.gmail.com/

if you need more context, but I suspect you can figure it out just
from this email too.

              Linus

On Sun, Oct 3, 2021 at 12:02 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Sun, Oct 3, 2021 at 11:38 AM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > Looking at the kvm code, that kvm_fastop_exception thing is some funky sh*t.
> >
> > I _think_ the problem is that 'kvm_fastop_exception' is done with bare
> > asm at the top-level and that triggers some odd interaction with other
> > section data, but I really don't know.
>
> No, it's the fact that it is marked as a global function (why?) that
> it then causes problems.
>
> Now, I don't actually see why it would cause problems (the same way I
> don't see why it's marked global). But removing that
>
>      ".global kvm_fastop_exception \n"
>
> works.
>
> I suspect it makes the linker do the relocation for us before objtool
> runs, because now that it's a local name, there is no worry about
> multiply defined symbols of the same name or anything like that.
>
> I also suspect that the reason for the warning is that the symbol type
> has never been declared, so it's not marked as a STT_FUNC in the
> relocation information.
>
> So independently of this kvm_fastop_exception issue, I'd suggest the
> attached patch for objtool to make the warning more informative for
> people who try to debug this.
>
> So I have a fix ("remove the global declaration"), but I really don't
> like how random this is.
>
> I also tried to instead keep the symbol global, and just mark
> kvm_fastop_exception as a function (and add the proper size
> annotation), but that only causes more objtool warnings for the
> (generated asm) functions that *use* that symbol. Because they also
> don't seem to be properly annotated.
>
> Again, removing the global annotation works around the problem, but
> the real underlying issue does seem to be that "funky sh*t" going on
> in arch/x86/kvm/emulate.c.
>
> So I'd like more people to look at this.
>
> In the meantime, I think the exception handling for kvm
> divide/multiply emulation is badly broken right now. Hmm?
>
>                 Linus