linux-kernel - Re: [PATCH] x86/xen: simplify sysenter and syscall setup

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <bd3e990b-e2a7-bbf0-7e2f-259e6a353393@citrix.com>
Date:   Fri, 21 Oct 2022 08:06:11 +0000
From:   Andrew Cooper <Andrew.Cooper3@...rix.com>
To:     Borislav Petkov <bp@...en8.de>, Juergen Gross <jgross@...e.com>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "x86@...nel.org" <x86@...nel.org>,
        Boris Ostrovsky <boris.ostrovsky@...cle.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        "H. Peter Anvin" <hpa@...or.com>,
        "xen-devel@...ts.xenproject.org" <xen-devel@...ts.xenproject.org>,
        "Peter Zijlstra (Intel)" <peterz@...radead.org>
Subject: Re: [PATCH] x86/xen: simplify sysenter and syscall setup

On 20/10/2022 12:39, Borislav Petkov wrote:
> On Thu, Oct 20, 2022 at 01:36:19PM +0200, Juergen Gross wrote:
>> xen_enable_sysenter() and xen_enable_syscall() can be simplified a lot.
>>
>> Signed-off-by: Juergen Gross <jgross@...e.com>
>> ---
>>  arch/x86/xen/setup.c | 23 ++++++-----------------
>>  1 file changed, 6 insertions(+), 17 deletions(-)
>>
>> diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
>> index cfa99e8f054b..0f33ed6d3a7b 100644
>> --- a/arch/x86/xen/setup.c
>> +++ b/arch/x86/xen/setup.c
>> @@ -910,17 +910,9 @@ static int register_callback(unsigned type, const void *func)
>>  
>>  void xen_enable_sysenter(void)
>>  {
>> -	int ret;
>> -	unsigned sysenter_feature;
>> -
>> -	sysenter_feature = X86_FEATURE_SYSENTER32;
>> -
>> -	if (!boot_cpu_has(sysenter_feature))
>> -		return;
>> -
>> -	ret = register_callback(CALLBACKTYPE_sysenter, xen_entry_SYSENTER_compat);
>> -	if(ret != 0)
>> -		setup_clear_cpu_cap(sysenter_feature);
>> +	if (boot_cpu_has(X86_FEATURE_SYSENTER32) &&
> Can you switch that and below to cpu_feature_enabled() while at it, pls?

Why?

This function (should) be called on the BSP only (because Xen's API lets
this be specified when starting APs).

Whether it's once, or one per cpu, it doesn't matter.

cpu_feature_enabled() puts in an out-of-line thunk (which is what
actually gets used), and a patchable code section.

Text patching will happen at least once to orphan the out-of-line thunk,
probably after the last time it gets used, then then maybe again later
to clear the feature.  Even if you had several million CPUs, there's no
way the overhead of cpu_feature_enabled() is worth it here.

~Andrew