Re: [PATCHv6 06/30] x86/traps: Refactor exc_general_protection()

Kirill A. Shutemov
2022-03-17T14:05:28Z

On Thu, Mar 17, 2022 at 01:21:53AM +0100, Thomas Gleixner wrote:
> On Wed, Mar 16 2022 at 05:08, Kirill A. Shutemov wrote:
> > TDX brings a new exception -- Virtualization Exception (#VE). Handling
> > of #VE structurally very similar to handling #GP.
> >
> > Extract two helpers from exc_general_protection() that can be reused for
> > handling #VE.
> >
> > No functional changes.
> >
> > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> > Reviewed-by: Dave Hansen <dave.hansen@linux.intel.com>
>
> Not that I care much, but this is the second instance of something I
> suggested. We have tags for that...

Sorry, will add the tag.

By the other instance you meant common base for SEAMCALL and TDCALL C
wrappers, right? Will fix too.

> > +static bool gp_try_fixup_and_notify(struct pt_regs *regs, int trapnr,
> > + unsigned long error_code, const char *str)
> > +{
> > + int ret;
>
> Adding this to make my suggestion compile is the easy way out, but...
>
> > +
> > + if (fixup_exception(regs, trapnr, error_code, 0))
> > + return true;
> > +
> > + current->thread.error_code = error_code;
> > + current->thread.trap_nr = trapnr;
> > +
> > + /*
> > + * To be potentially processing a kprobe fault and to trust the result
> > + * from kprobe_running(), we have to be non-preemptible.
> > + */
> > + if (!preemptible() && kprobe_running() &&
> > + kprobe_fault_handler(regs, trapnr))
> > + return true;
> > +
> > + ret = notify_die(DIE_GPF, str, regs, error_code, trapnr, SIGSEGV);
> > + return ret == NOTIFY_STOP;
>
> Why not doing the obvious:
>
> return notify_die(DIE_GPF, str, regs, error_code, trapnr, SIGSEGV) == NOTIFY_STOP;
>
> Hmm?

I don't like lines this long (although my variant doesn't look good too).

Will do your way.

--
Kirill A. Shutemov