Re: [PATCH v5 07/20] x86/kexec: Invoke copy of relocate_kernel() instead of the original

[David Woodhouse <dwmw2@infradead.org>]

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

On Wed, 2024-12-18 at 16:20 -0800, Josh Poimboeuf wrote:
> On Wed, Dec 18, 2024 at 11:27:27PM +0100, David Woodhouse wrote:
> > On Wed, 2024-12-18 at 13:23 -0800, Josh Poimboeuf wrote:
> > > 
> > > The linker script does place it in .data, but objtool runs on the object
> > > file before linking, where it's still in an executable section
> > > (.text..relocate_kernel).
> > > 
> > > How about something like below?
> > > 
> > >   - move text to .data..relocate_kernel
> > >   - remove objtool annotations
> > >   - replace the alternative with a runtime check
> > 
> > That leaves me unable to use 'objdump -S
> > arch/x86/kernel/relocate_kernel_64.o' so I hate it :)
> 
> Well, it's already written in assembly, there's not much benefit in
> disassembling it ;-)  But you can still force gdb to do so with
> something like "x/50i <addr>".

I've spent a lot of time recently wanting to know precisely which
instruction it was that faulted (or that objtool is complaining about),
so I *have* wanted to disassemble it. But yeah, in the general case
that shouldn't be something we're doing all the time.

Perhaps you're right, and we should just rip out all the annotations
and move it to .data..relocate_kernel. But I'll give it a bit more
effort to see if we can make objtool happy and keep the ALTERNATIVE
working without having to do that manually...


> Isn't that easier than putting in all these hacks to make objtool happy
> (and continue to keep it happy over the coming years), when it doesn't
> need to care about this code in the first place?

Less worried about the 'continue' part. This code doesn't get much
churn (not until I just came along and blitzed it, at least).

> Anyway, what I think you're looking for is UNWIND_HINT_UNDEFINED.  In
> fact all the unwind annotations in that file should be UNDEFINED since
> the hints are all referring to the wrong addresses (because copied code)
> and the ORC unwinder isn't reachable for most of that code anyway.
> 
> Also, it's fine to make relocate_kernel() a proper function with
> SYM_FUNC_END, you'd just need to add the following line afterwards:
> 
>   STACK_FRAME_NON_STANDARD relocate_kernel
> 

Thanks, I'll take a look. The next thing objtool is actually
complaining about is that relocate_kernel() itself is not reachable
(since it's never invoked in its original location):

vmlinux.o: warning: objtool: relocate_kernel+0x0: unreachable instruction

I'm not sure why objtool has only just started whining about that with
the very last patches at the top of my tree; it's been true ever since
the commit which executes from the relocated copy.

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]