From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id B2743C04A94 for ; Mon, 31 Jul 2023 22:11:54 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231803AbjGaWLx (ORCPT ); Mon, 31 Jul 2023 18:11:53 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41414 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231804AbjGaWLs (ORCPT ); Mon, 31 Jul 2023 18:11:48 -0400 Received: from mail.zytor.com (unknown [IPv6:2607:7c80:54:3::138]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0580B1981; Mon, 31 Jul 2023 15:11:41 -0700 (PDT) Received: from [192.168.105.249] ([75.104.94.137]) (authenticated bits=0) by mail.zytor.com (8.17.1/8.17.1) with ESMTPSA id 36VM7txf3103048 (version=TLSv1.3 cipher=TLS_AES_128_GCM_SHA256 bits=128 verify=NO); Mon, 31 Jul 2023 15:08:00 -0700 DKIM-Filter: OpenDKIM Filter v2.11.0 mail.zytor.com 36VM7txf3103048 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=zytor.com; s=2023071101; t=1690841375; bh=X+M4w9IgqyGhFftvL0ts+Txt2pH1wocbrviAaT7WSp0=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=XFcw8mHtRsl4TcZ5Z+9TuwZ7pfjvdNqSpwDLDpFsAfd/pRi+FbwrTEcqslERWxffC cgFKch7rqXk7xeaKLgEYkT1cKGAy6lNFxl6xQJ0fbNDJ7sv9zpYGWXBRa8N2lEo5DR G/GCfDVd/qzoFF9SUxuA7hRU/ROGNY4q6TBtRqRfJXYuVJskDXCrkKAQal43XsJ7PX PlycbEss3tGY6NKNX7yE7ihX3QPEQjBg9VfjApas+qZFkVyB7AoO2WWrlZ+j8SXiEq CQq7y33RknpAiyzIOPtVrS0p+SqzFZP4pjxOjHk1xtmUQ70RjsXjOD9F2NjeIVYHD8 KsFoANCn7EDLw== Message-ID: Date: Mon, 31 Jul 2023 15:07:47 -0700 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.13.0 Subject: Re: [PATCH v9 29/36] x86/fred: FRED entry/exit and dispatch code Content-Language: en-US To: Xin Li , linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-edac@vger.kernel.org, linux-hyperv@vger.kernel.org, kvm@vger.kernel.org, xen-devel@lists.xenproject.org Cc: Jonathan Corbet , Thomas Gleixner , Ingo Molnar , Borislav Petkov , Dave Hansen , x86@kernel.org, Andy Lutomirski , Oleg Nesterov , Tony Luck , "K . Y . Srinivasan" , Haiyang Zhang , Wei Liu , Dexuan Cui , Paolo Bonzini , Wanpeng Li , Vitaly Kuznetsov , Sean Christopherson , Peter Zijlstra , Juergen Gross , Stefano Stabellini , Oleksandr Tyshchenko , Josh Poimboeuf , "Paul E . McKenney" , Catalin Marinas , Randy Dunlap , Steven Rostedt , Kim Phillips , Hyeonggon Yoo <42.hyeyoo@gmail.com>, "Liam R . Howlett" , Sebastian Reichel , "Kirill A . Shutemov" , Suren Baghdasaryan , Pawan Gupta , Jiaxi Chen , Babu Moger , Jim Mattson , Sandipan Das , Lai Jiangshan , Hans de Goede , Reinette Chatre , Daniel Sneddon , Breno Leitao , Nikunj A Dadhania , Brian Gerst , Sami Tolvanen , Alexander Potapenko , Andrew Morton , Arnd Bergmann , "Eric W . Biederman" , Kees Cook , Masami Hiramatsu , Masahiro Yamada , Ze Gao , Fei Li , Conghui , Ashok Raj , "Jason A . Donenfeld" , Mark Rutland , Jacob Pan , Jiapeng Chong , Jane Malalane , David Woodhouse , Boris Ostrovsky , Arnaldo Carvalho de Melo , Yantengsi , Christophe Leroy , Sathvika Vasireddy References: <20230731064119.3870-1-xin3.li@intel.com> From: "H. Peter Anvin" In-Reply-To: <20230731064119.3870-1-xin3.li@intel.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-hyperv@vger.kernel.org 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