[prev in list] [next in list] [prev in thread] [next in thread]
List: kvm
Subject: Re: [PATCH v9 29/36] x86/fred: FRED entry/exit and dispatch code
From: "H. Peter Anvin" <hpa () zytor ! com>
Date: 2023-07-31 22:07:47
Message-ID: da169e64-9dad-18a8-611b-57ff74006285 () zytor ! com
[Download RAW message or body]
On 7/30/23 23:41, Xin Li wrote:
> +static DEFINE_FRED_HANDLER(fred_other_default)
> +{
> + regs->vector = X86_TRAP_UD;
> + fred_emulate_fault(regs);
> +}
> +
> +static DEFINE_FRED_HANDLER(fred_syscall)
> +{
> + regs->orig_ax = regs->ax;
> + regs->ax = -ENOSYS;
> + do_syscall_64(regs, regs->orig_ax);
> +}
> +
> +#if IS_ENABLED(CONFIG_IA32_EMULATION)
> +/*
> + * Emulate SYSENTER if applicable. This is not the preferred system
> + * call in 32-bit mode under FRED, rather int $0x80 is preferred and
> + * exported in the vdso.
> + */
> +static DEFINE_FRED_HANDLER(fred_sysenter)
> +{
> + regs->orig_ax = regs->ax;
> + regs->ax = -ENOSYS;
> + do_fast_syscall_32(regs);
> +}
> +#else
> +#define fred_sysenter fred_other_default
> +#endif
> +
> +static DEFINE_FRED_HANDLER(fred_other)
> +{
> + static const fred_handler user_other_handlers[FRED_NUM_OTHER_VECTORS] =
> + {
> + /*
> + * Vector 0 of the other event type is not used
> + * per FRED spec 5.0.
> + */
> + [0] = fred_other_default,
> + [FRED_SYSCALL] = fred_syscall,
> + [FRED_SYSENTER] = fred_sysenter
> + };
> +
> + user_other_handlers[regs->vector](regs);
> +}
OK, this is wrong.
Dispatching like fred_syscall() is only valid for syscall64, which means
you have to check regs->l is set in addition to the correct regs->vector
to determine validity.
Similarly, sysenter is only valid if regs->l is clear.
The best way is probably to drop the dispatch table here and just do an
if ... else if ... else statement; gcc is smart enough that it will
combine the vector test and the L bit test into a single mask and
compare. This also allows stubs to be inlined.
However, emulating #UD on events other than wrong mode of SYSCALL and
SYSENTER may be a bad idea. It would probably be better to invoke
fred_bad_event() in that case.
Something like this:
+static DEFINE_FRED_HANDLER(fred_other_default)
+{
+ regs->vector = X86_TRAP_UD;
+ fred_emulate_fault(regs);
+}
1) rename this to fred_emulate_ud (since that is what it actually does.)
... then ...
/* The compiler can fold these into a single test */
if (likely(regs->vector == FRED_SYSCALL && regs->l)) {
fred_syscall64(regs);
} else if (likely(regs->vector == FRED_SYSENTER && !regs->l)) {
fred_sysenter32(regs);
} else if (regs->vector == FRED_SYSCALL ||
regs->vector == FRED_SYSENTER) {
/* Invalid SYSCALL or SYSENTER instruction */
fred_emulate_ud(regs);
} else {
/* Unknown event */
fred_bad_event(regs);
}
... or the SYSCALL64 and SYSENTER32 can be inlined with the appropriate
comment (gcc will do so regardless.)
-hpa
-hpa
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic