Message ID | 20231116191127.3446476-1-ubizjak@gmail.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:9910:0:b0:403:3b70:6f57 with SMTP id i16csp54939vqn; Thu, 16 Nov 2023 11:11:58 -0800 (PST) X-Google-Smtp-Source: AGHT+IHuiodutktB9WAiWzxAp5pZ55p7ZnEykWirDyhwFd9f01zCpdZvyHzo2Xxs2njiaSxPDL+l X-Received: by 2002:a05:6a00:3016:b0:68b:e29c:b69 with SMTP id ay22-20020a056a00301600b0068be29c0b69mr4216923pfb.9.1700161918311; Thu, 16 Nov 2023 11:11:58 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1700161918; cv=none; d=google.com; s=arc-20160816; b=A1QerlhvKxBfMCCeH3GtsYlxga89F+feBRNNjjh6jY4nd3UleTY08rw/lG21bdYgP/ v/CZS1rro8ipDMnQ2slcUzgWc+roKj5W0a/XV0Qr/XoiQ9CZyE1wFeJF6hKODItbhBk/ G8vxK3YYrK72bccalWDCn/AfaXr14uCHKYQvtalQVuZ+d4MkVc6LUgpHqMrI4xqiy75Q Wx9O7irLhsBnj20lOFk0LoJq9k18RMc6yqvTqh7JnnFn2PpfTpIU4qXZ3LYElf4EtC3e 6p3hFVpp1C4EV/o+akuygtkd+HWewwCRVguPVdYEOJQez/xdGhNgqkfHo4JkCzymg7U7 Efng== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=GwQUpHrldieb9q/f898qx9fwKE9eO/IChKWvTMHwjHs=; fh=sWK7YOurtkTyaE7aaZtytOmSvUDe8vhU+5hRY6IWLIo=; b=YvtjJK4Ia//CoimUG1DavcUy7FNejkSCA/taKnHcQyZez5VkKPGl9b7TDyZnbWR49Q Mc2DdyRi1LmX24j/Ae0bA534Vrwrd6W0kG+QTSrFhc7u9TAJQnKh5UjC1c7ix5wij13Y GEeiPJxvyvISp8eHutMAeDS9xtv2/Bo50H+iLj0igBWXPQi0qsGjF1f5Ypb22FCqWMiG HqZnxqniQsfUFhABno+nnYzeq3NdSA9pk0YXHo3tTZwL+e7xdz9ffaa3uCo4ORrkzlTq hdvgcddNOdyupFEkASlxXzzHujHXWZXVRqZZYaQskNfyFff1+dDyG6i2kQRg3FXmvOQp psrg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20230601 header.b=NA5xmWzM; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:2 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from agentk.vger.email (agentk.vger.email. [2620:137:e000::3:2]) by mx.google.com with ESMTPS id s10-20020a056a00194a00b006c33a1be028si159374pfk.87.2023.11.16.11.11.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 16 Nov 2023 11:11:58 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:2 as permitted sender) client-ip=2620:137:e000::3:2; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20230601 header.b=NA5xmWzM; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:2 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by agentk.vger.email (Postfix) with ESMTP id 072D780F9CBC; Thu, 16 Nov 2023 11:11:56 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at agentk.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229437AbjKPTLo (ORCPT <rfc822;jaysivo@gmail.com> + 30 others); Thu, 16 Nov 2023 14:11:44 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:35820 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229510AbjKPTLn (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 16 Nov 2023 14:11:43 -0500 Received: from mail-ej1-x630.google.com (mail-ej1-x630.google.com [IPv6:2a00:1450:4864:20::630]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id CD02CD4E for <linux-kernel@vger.kernel.org>; Thu, 16 Nov 2023 11:11:39 -0800 (PST) Received: by mail-ej1-x630.google.com with SMTP id a640c23a62f3a-9d216597f64so165351666b.3 for <linux-kernel@vger.kernel.org>; Thu, 16 Nov 2023 11:11:39 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1700161898; x=1700766698; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=GwQUpHrldieb9q/f898qx9fwKE9eO/IChKWvTMHwjHs=; b=NA5xmWzMRF1SZmLb55xWj52IAqnlpEQ6IfUPLi5kP3PmUG6gI+dHhk8sLXbpkd/Sdb PHDrOBGbnSQfMQ+cJCdVZJ2SZZE9P9utsiviAP0J9gwv1J11BDCHoVHnOKacVK72ajDN RDMAnu58mCZwv++hegCuP+Uhlkv3vXfBznx0b31KnXsk++Npt4mloxDS9/P4/PUtyGnI 7Mynwee8CxJV3zt7anCV/OhunnDGlwL4nLk5BtvyxbC658tkwoDMQJvY9Ts9wvJQ3Wow lDfENoVPWfIMMjWTaejHxrCSmWqBsjwy+bQ3mzU3fdfqjoxVoMwXX+ZdrWZ6RdKPYUK6 19fg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1700161898; x=1700766698; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=GwQUpHrldieb9q/f898qx9fwKE9eO/IChKWvTMHwjHs=; b=o7Y5qUXITdgK1uHXaaiOKtTiQjFj3SimevrLaIgv1cGVRz7MT3fdGPRuxbMUMuJlGS 1mVejuWaCTnUs3J/tKCj9szQYWIsqR48zISpWFA+C2KdZ/xQNb4gguR7H9mjGszsWcWM UQLoP25W7I1VOOGM5Ee37tBe8/PGAgIMKKjDXkNYqC+sn6GQge5xmECvTSaobTX1fnr+ qBZEaDsBgGFdhV6ZAVVex8CjLyq9qmDK8nOoDKCXJhv5rSXrPgKn8NaYBuqzkxE9LqdU RDF+KwoKjuNbowrORTxARUoRxMsDuahoWsv7lkh9N7+ht4rw3WYoUJhF2yjitHBcqZgP jX8A== X-Gm-Message-State: AOJu0YxvMwZEFaS3mZ0me+6iEisp+122J485A5K9s6idARToUIm0CTsx eKKF0z5fhAtmU1uARhxWnMI= X-Received: by 2002:a17:906:234d:b0:9d3:f436:6809 with SMTP id m13-20020a170906234d00b009d3f4366809mr12531752eja.39.1700161897905; Thu, 16 Nov 2023 11:11:37 -0800 (PST) Received: from localhost.localdomain ([46.248.82.114]) by smtp.gmail.com with ESMTPSA id mb24-20020a170906eb1800b00992e14af9c3sm8899951ejb.143.2023.11.16.11.11.37 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 16 Nov 2023 11:11:37 -0800 (PST) From: Uros Bizjak <ubizjak@gmail.com> To: x86@kernel.org, linux-kernel@vger.kernel.org Cc: Uros Bizjak <ubizjak@gmail.com>, Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@kernel.org>, Borislav Petkov <bp@alien8.de>, Dave Hansen <dave.hansen@linux.intel.com>, "H. Peter Anvin" <hpa@zytor.com>, Peter Zijlstra <peterz@infradead.org> Subject: [PATCH -tip] x86/mm: Use %RIP-relative address in untagged_addr() Date: Thu, 16 Nov 2023 20:10:59 +0100 Message-ID: <20231116191127.3446476-1-ubizjak@gmail.com> X-Mailer: git-send-email 2.41.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-0.6 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on agentk.vger.email Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (agentk.vger.email [0.0.0.0]); Thu, 16 Nov 2023 11:11:56 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1782748983486287515 X-GMAIL-MSGID: 1782748983486287515 |
Series |
[-tip] x86/mm: Use %RIP-relative address in untagged_addr()
|
|
Commit Message
Uros Bizjak
Nov. 16, 2023, 7:10 p.m. UTC
%RIP-relative addresses are nowadays correctly handled in alternative
instructions, so remove misleading comment and improve assembly to
use %RIP-relative address.
Also, explicitly using %gs: prefix will segfault for non-SMP builds.
Use macros from percpu.h which will DTRT with segment prefix register
as far as SMP/non-SMP builds are concerned.
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
---
arch/x86/include/asm/uaccess_64.h | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
Comments
On Thu, Nov 16, 2023 at 08:10:59PM +0100, Uros Bizjak wrote: > %RIP-relative addresses are nowadays correctly handled in alternative > instructions, so remove misleading comment and improve assembly to > use %RIP-relative address. Ha!, it might've been this exact case (and Kirill grumbling) that got me to fix the alternative code :-) > Also, explicitly using %gs: prefix will segfault for non-SMP builds. > Use macros from percpu.h which will DTRT with segment prefix register > as far as SMP/non-SMP builds are concerned. > Signed-off-by: Uros Bizjak <ubizjak@gmail.com> Acked-byL Peter Zijlstra (Intel) <peterz@infradaed.org> > --- > arch/x86/include/asm/uaccess_64.h | 11 ++++------- > 1 file changed, 4 insertions(+), 7 deletions(-) > > diff --git a/arch/x86/include/asm/uaccess_64.h b/arch/x86/include/asm/uaccess_64.h > index f2c02e4469cc..01455c0b070c 100644 > --- a/arch/x86/include/asm/uaccess_64.h > +++ b/arch/x86/include/asm/uaccess_64.h > @@ -11,6 +11,7 @@ > #include <asm/alternative.h> > #include <asm/cpufeatures.h> > #include <asm/page.h> > +#include <asm/percpu.h> > > #ifdef CONFIG_ADDRESS_MASKING > /* > @@ -18,14 +19,10 @@ > */ > static inline unsigned long __untagged_addr(unsigned long addr) > { > - /* > - * Refer tlbstate_untag_mask directly to avoid RIP-relative relocation > - * in alternative instructions. The relocation gets wrong when gets > - * copied to the target place. > - */ > asm (ALTERNATIVE("", > - "and %%gs:tlbstate_untag_mask, %[addr]\n\t", X86_FEATURE_LAM) > - : [addr] "+r" (addr) : "m" (tlbstate_untag_mask)); > + "and " __percpu_arg([mask]) ", %[addr]", X86_FEATURE_LAM) > + : [addr] "+r" (addr) > + : [mask] "m" (__my_cpu_var(tlbstate_untag_mask))); > > return addr; > } > -- > 2.41.0 >
On Fri, Nov 17, 2023 at 10:41:03AM +0100, Peter Zijlstra wrote: > On Thu, Nov 16, 2023 at 08:10:59PM +0100, Uros Bizjak wrote: > > %RIP-relative addresses are nowadays correctly handled in alternative > > instructions, so remove misleading comment and improve assembly to > > use %RIP-relative address. > > Ha!, it might've been this exact case (and Kirill grumbling) that got me > to fix the alternative code :-) Nice! :) > > Also, explicitly using %gs: prefix will segfault for non-SMP builds. > > Use macros from percpu.h which will DTRT with segment prefix register > > as far as SMP/non-SMP builds are concerned. > > > Signed-off-by: Uros Bizjak <ubizjak@gmail.com> > > Acked-byL Peter Zijlstra (Intel) <peterz@infradaed.org> Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
On 11/16/23 11:10, Uros Bizjak wrote: > %RIP-relative addresses are nowadays correctly handled in alternative > instructions, so remove misleading comment and improve assembly to > use %RIP-relative address. > > Also, explicitly using %gs: prefix will segfault for non-SMP builds. > Use macros from percpu.h which will DTRT with segment prefix register > as far as SMP/non-SMP builds are concerned. OK, this is starting to feel silly. One could seriously question the use case for supporting !SMP builds x86-64. It isn't like our performance for SMP builds on UP systems is significantly worse, it is mostly just a matter of code size, and the difference isn't huge, either, especially considering that on systems of the x86-64 era the kernel is a rather small part of system memory (unlike the very early i386 era, for those of us who remember those ancient times.) The number of UP x86-64 systems is really very small (since multicore/SMT became ubiquitous at roughly the same time x86-64 was introduced), and as far as I know none of them lack APIC which is really the most fundamental difference between SMP and !SMP on x86. Why don't we simply have %gs_base == 0 as an invariant for !SMP? If we *REALLY* care to skip SWAPGS on !SMP systems, we could use alternatives to patch out %gs: and lock (wouldn't even have to be explicit: this is the kind of thing that objtool does really well.) We can use alternatives without anything special, since it only matters after we have entered user spae for the first time and would be concurrent with patching out SWAPGS itself. If we really *do* care about UP builds, we could teach objtool to do this patching at compile time for the !SMP builds. Also, didn't we at least use to have a way to mark a function as "init on UP" so that it could be jettisoned with the init code if we find ourselves on a uniprocessor system? -hpa
On Fri, Nov 17, 2023 at 1:16 PM H. Peter Anvin <hpa@zytor.com> wrote: > > On 11/16/23 11:10, Uros Bizjak wrote: > > %RIP-relative addresses are nowadays correctly handled in alternative > > instructions, so remove misleading comment and improve assembly to > > use %RIP-relative address. > > > > Also, explicitly using %gs: prefix will segfault for non-SMP builds. > > Use macros from percpu.h which will DTRT with segment prefix register > > as far as SMP/non-SMP builds are concerned. > > OK, this is starting to feel silly. One could seriously question the use > case for supporting !SMP builds x86-64. It isn't like our performance > for SMP builds on UP systems is significantly worse, it is mostly just a > matter of code size, and the difference isn't huge, either, especially > considering that on systems of the x86-64 era the kernel is a rather > small part of system memory (unlike the very early i386 era, for those > of us who remember those ancient times.) > > The number of UP x86-64 systems is really very small (since > multicore/SMT became ubiquitous at roughly the same time x86-64 was > introduced), and as far as I know none of them lack APIC which is really > the most fundamental difference between SMP and !SMP on x86. > > Why don't we simply have %gs_base == 0 as an invariant for !SMP? The reason is stack protector, which is still stuck at %gs:40. So GSBASE has to point at fixed_percpu_data, even on a UP build. That is corrected by the patch series I recently posted, though. > If we > *REALLY* care to skip SWAPGS on !SMP systems, we could use alternatives > to patch out %gs: and lock (wouldn't even have to be explicit: this is > the kind of thing that objtool does really well.) We can use > alternatives without anything special, since it only matters after we > have entered user spae for the first time and would be concurrent with > patching out SWAPGS itself. There is already support to patch out LOCK prefixes when running an SMP build on a single CPU (.smp_locks section). Patching out the GS prefix would only work if the initial percpu area is not freed. Beyond that I don't think other optimizations are worth the effort, and would get very little testing. Brian Gerst
On 11/17/23 11:43, Brian Gerst wrote:>> >> Why don't we simply have %gs_base == 0 as an invariant for !SMP? > > The reason is stack protector, which is still stuck at %gs:40. So > GSBASE has to point at fixed_percpu_data, even on a UP build. That is > corrected by the patch series I recently posted, though. > Right, that problem is gone. >> If we >> *REALLY* care to skip SWAPGS on !SMP systems, we could use alternativesYep, that is >> to patch out %gs: and lock (wouldn't even have to be explicit: this is >> the kind of thing that objtool does really well.) We can use >> alternatives without anything special, since it only matters after we >> have entered user spae for the first time and would be concurrent with >> patching out SWAPGS itself. > > There is already support to patch out LOCK prefixes when running an > SMP build on a single CPU (.smp_locks section). Patching out the GS > prefix would only work if the initial percpu area is not freed. > Beyond that I don't think other optimizations are worth the effort, > and would get very little testing. Yes, that is basically my point. -hpa
diff --git a/arch/x86/include/asm/uaccess_64.h b/arch/x86/include/asm/uaccess_64.h index f2c02e4469cc..01455c0b070c 100644 --- a/arch/x86/include/asm/uaccess_64.h +++ b/arch/x86/include/asm/uaccess_64.h @@ -11,6 +11,7 @@ #include <asm/alternative.h> #include <asm/cpufeatures.h> #include <asm/page.h> +#include <asm/percpu.h> #ifdef CONFIG_ADDRESS_MASKING /* @@ -18,14 +19,10 @@ */ static inline unsigned long __untagged_addr(unsigned long addr) { - /* - * Refer tlbstate_untag_mask directly to avoid RIP-relative relocation - * in alternative instructions. The relocation gets wrong when gets - * copied to the target place. - */ asm (ALTERNATIVE("", - "and %%gs:tlbstate_untag_mask, %[addr]\n\t", X86_FEATURE_LAM) - : [addr] "+r" (addr) : "m" (tlbstate_untag_mask)); + "and " __percpu_arg([mask]) ", %[addr]", X86_FEATURE_LAM) + : [addr] "+r" (addr) + : [mask] "m" (__my_cpu_var(tlbstate_untag_mask))); return addr; }