[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <da169e64-9dad-18a8-611b-57ff74006285@zytor.com>
Date: Mon, 31 Jul 2023 15:07:47 -0700
From: "H. Peter Anvin" <hpa@...or.com>
To: Xin Li <xin3.li@...el.com>, linux-doc@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-edac@...r.kernel.org,
linux-hyperv@...r.kernel.org, kvm@...r.kernel.org,
xen-devel@...ts.xenproject.org
Cc: Jonathan Corbet <corbet@....net>,
Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
Andy Lutomirski <luto@...nel.org>,
Oleg Nesterov <oleg@...hat.com>,
Tony Luck <tony.luck@...el.com>,
"K . Y . Srinivasan" <kys@...rosoft.com>,
Haiyang Zhang <haiyangz@...rosoft.com>,
Wei Liu <wei.liu@...nel.org>, Dexuan Cui <decui@...rosoft.com>,
Paolo Bonzini <pbonzini@...hat.com>,
Wanpeng Li <wanpengli@...cent.com>,
Vitaly Kuznetsov <vkuznets@...hat.com>,
Sean Christopherson <seanjc@...gle.com>,
Peter Zijlstra <peterz@...radead.org>,
Juergen Gross <jgross@...e.com>,
Stefano Stabellini <sstabellini@...nel.org>,
Oleksandr Tyshchenko <oleksandr_tyshchenko@...m.com>,
Josh Poimboeuf <jpoimboe@...nel.org>,
"Paul E . McKenney" <paulmck@...nel.org>,
Catalin Marinas <catalin.marinas@....com>,
Randy Dunlap <rdunlap@...radead.org>,
Steven Rostedt <rostedt@...dmis.org>,
Kim Phillips <kim.phillips@....com>,
Hyeonggon Yoo <42.hyeyoo@...il.com>,
"Liam R . Howlett" <Liam.Howlett@...cle.com>,
Sebastian Reichel <sebastian.reichel@...labora.com>,
"Kirill A . Shutemov" <kirill.shutemov@...ux.intel.com>,
Suren Baghdasaryan <surenb@...gle.com>,
Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>,
Jiaxi Chen <jiaxi.chen@...ux.intel.com>,
Babu Moger <babu.moger@....com>,
Jim Mattson <jmattson@...gle.com>,
Sandipan Das <sandipan.das@....com>,
Lai Jiangshan <jiangshanlai@...il.com>,
Hans de Goede <hdegoede@...hat.com>,
Reinette Chatre <reinette.chatre@...el.com>,
Daniel Sneddon <daniel.sneddon@...ux.intel.com>,
Breno Leitao <leitao@...ian.org>,
Nikunj A Dadhania <nikunj@....com>,
Brian Gerst <brgerst@...il.com>,
Sami Tolvanen <samitolvanen@...gle.com>,
Alexander Potapenko <glider@...gle.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Arnd Bergmann <arnd@...db.de>,
"Eric W . Biederman" <ebiederm@...ssion.com>,
Kees Cook <keescook@...omium.org>,
Masami Hiramatsu <mhiramat@...nel.org>,
Masahiro Yamada <masahiroy@...nel.org>,
Ze Gao <zegao2021@...il.com>, Fei Li <fei1.li@...el.com>,
Conghui <conghui.chen@...el.com>,
Ashok Raj <ashok.raj@...el.com>,
"Jason A . Donenfeld" <Jason@...c4.com>,
Mark Rutland <mark.rutland@....com>,
Jacob Pan <jacob.jun.pan@...ux.intel.com>,
Jiapeng Chong <jiapeng.chong@...ux.alibaba.com>,
Jane Malalane <jane.malalane@...rix.com>,
David Woodhouse <dwmw@...zon.co.uk>,
Boris Ostrovsky <boris.ostrovsky@...cle.com>,
Arnaldo Carvalho de Melo <acme@...hat.com>,
Yantengsi <siyanteng@...ngson.cn>,
Christophe Leroy <christophe.leroy@...roup.eu>,
Sathvika Vasireddy <sv@...ux.ibm.com>
Subject: Re: [PATCH v9 29/36] x86/fred: FRED entry/exit and dispatch code
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
Powered by blists - more mailing lists