Messages in this thread | | | From | Thomas Gleixner <> | Subject | Re: [patch 01/10] x86/fpu/signal: Clarify exception handling in restore_fpregs_from_user() | Date | Tue, 31 Aug 2021 00:01:35 +0200 |
| |
On Mon, Aug 30 2021 at 14:26, Linus Torvalds wrote: > On Mon, Aug 30, 2021 at 2:07 PM Al Viro <viro@zeniv.linux.org.uk> wrote: >> >> Incidentally, why do we bother with negation in those? Why not have >> user_insn(), XSTATE_OP() and kernel_insn_err() return 0 or trap >> number...
Correct.
> I really wish we didn't have that odd _ASM_EXTABLE_FAULT/ > ex_handler_fault() special case at all. > > It's *very* confusing, and it actually seems to be mis-used. It looks > like the "copy_mc_fragile" code uses it by mistake, and doesn't > actually want that "modify %%rax" behavior of that exception handler > AT ALL. > > If I read that code correctly, it almost by mistake doesn't actually > care, and will overwrite %%rax with the right result, but it doesn't > look like the "fault code in %eax" was ever *intentional*. There's no > mention of it. > > Maybe I'm misreading that code, but I look at it and just go "Whaa?"
Ooops. I never looked at that usage site. It indeed does not make use of that information. The original __mcsafe_copy() made use of it, but that got removed/replaced long ago.
The other user is SGX which actually uses the trap number in EAX for failure analysis.
> The code in user_insn() clearly *does* use that fault number (and, as > you say, inverts it for some reason), but I wonder how much it really > cares? Could we get rid of it, and just set a fixed error code? > > I only checked one user, but that one didn't actually care which fault > it was, it only cared about fault-vs-no-fault.
The usage sites of user_insn() and XSTATE_OP() need to distinguish:
- success - fail due to #PF (which can be tried to handle) - fail due to some other exception (#GP, #MC)
I found that _ASM_EXTABLE_FAULT() mechanism pretty conveniant for this and the negation was just me being lazy after I discovered that X86_TRAP_PF == EFAULT. It turned out not to be a brilliant idea, but at the time it looked great...
So yes, the negation does not matter, but the ability to check whether the fail was caused by #PF or not matters.
Thanks,
tglx
| |