Re: [patch V4 part 3 17/29] x86/entry: Convert Invalid Opcode exception to IDTENTRY

Andy Lutomirski
2020-05-14T15:00:20Z

> On May 14, 2020, at 5:33 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
>
> ﻿Andy Lutomirski <luto@kernel.org> writes:
>
>>> On Tue, May 5, 2020 at 7:15 AM Thomas Gleixner <tglx@linutronix.de> wrote:
>>>
>>> From: Thomas Gleixner <tglx@linutronix.de>
>>>
>>> Convert #UD to IDTENTRY:
>>> - Implement the C entry point with DEFINE_IDTENTRY
>>> - Emit the ASM stub with DECLARE_IDTENTRY
>>> - Remove the ASM idtentry in 64bit
>>> - Remove the open coded ASM entry code in 32bit
>>> - Fixup the XEN/PV code
>>> - Fixup the FOOF bug call in fault.c
>>> - Remove the old prototyoes
>>>
>>> No functional change.
>>
>> I think there *is* a functional change:
>>
>>
>>> --- a/arch/x86/mm/fault.c
>>> +++ b/arch/x86/mm/fault.c
>>> @@ -567,7 +567,7 @@ static int is_f00f_bug(struct pt_regs *r
>>> nr = (address - idt_descr.address) >> 3;
>>>
>>> if (nr == 6) {
>>> - do_invalid_op(regs, 0);
>>> + handle_invalid_op(regs);
>>
>> I suspect the old code was wrong and no one noticed because no one has
>> a F00F-buggy machine any more.
>
> I don't think so. It's really just the same thing. The old #UD C
> function was:
>
> void do_invalid_op(struct pt_regs *regs, long error_code)
> {
> do_error_trap(regs, error_code, "invalid opcode", X86_TRAP_UD,
> SIGILL, ILL_ILLOPN, (void __user *)uprobe_get_trap_addr(regs));
> }
>
> after expanding the DO_ERROR() muck.
>
> The new one does:
>
> void handle_invalid_op(struct pt_regs *regs)
> {
> do_error_trap(regs, 0, "invalid opcode", X86_TRAP_UD, SIGILL,
> ILL_ILLOPN, error_get_trap_addr(regs));
> }
>
> which is exactly the same except for the error code being hardcoded to
> zero in handle_invalid_op() because #UD does not have one.
>
>> So maybe document that you fixed up the F00F bug, too. Otherwise:
>
> Not sure what to document :)

Duh, right, never mind. Here I was thinking the old C handlers did all the entry/exit work.

>
> Thanks,
>
> tglx