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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 029DFCA0ED3 for ; Mon, 2 Sep 2024 14:33:02 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=oYPFIRtAZ0HB6EtI9EVtjXAwXiG8sQtRUypG3vd7YV4=; b=AXLxFEOSb1ouRzfck8XAXIcfRI kEJ4zK/w2nylE16AYzPVKdZgN575rW14nLJM9b9vw/nnW8zTspoc6ez7ekb8RCDKZ6evtNPwgw9kq G8fCrrngBij/fmLxHGfYCRNboUKJvoVqQvnzUM2wRW/S4mhThANrMIaJZ3hoowWdjGsfrhEY5srEU x8H4nYHz0+a7udZca3Mqxm288C2Jd5prQL6X40J8FGWqafA+VLMr1uSQ2oAvRB3MfBlOp7B4WgRY5 AD1VIVsOgqw2/iUl5RpyFW+CiroPIWKXglKLifWaOW9OJoYgGNVcDyaFFOYYdea7o6G6XIEtfC+fK 3dk2k0Cw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sl86r-0000000EgUE-3eGA; Mon, 02 Sep 2024 14:32:49 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sl85x-0000000EgEC-3ocf for linux-arm-kernel@lists.infradead.org; Mon, 02 Sep 2024 14:31:55 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 78F25FEC; Mon, 2 Sep 2024 07:32:16 -0700 (PDT) Received: from J2N7QTR9R3 (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 504883F73B; Mon, 2 Sep 2024 07:31:48 -0700 (PDT) Date: Mon, 2 Sep 2024 15:31:43 +0100 From: Mark Rutland To: Adhemerval Zanella Cc: "Jason A . Donenfeld" , Theodore Ts'o , linux-kernel@vger.kernel.org, linux-crypto@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-arch@vger.kernel.org, Catalin Marinas , Will Deacon , Thomas Gleixner , Eric Biggers , Christophe Leroy Subject: Re: [PATCH v3] aarch64: vdso: Wire up getrandom() vDSO implementation Message-ID: References: <20240902125312.3934-1-adhemerval.zanella@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240902125312.3934-1-adhemerval.zanella@linaro.org> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240902_073154_056487_10830B0A X-CRM114-Status: GOOD ( 31.89 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, Sep 02, 2024 at 12:52:57PM +0000, Adhemerval Zanella wrote: > +static __always_inline const struct vdso_rng_data *__arch_get_vdso_rng_data(void) > +{ > + /* > + * If a task belongs to a time namespace then the real VVAR page is mapped > + * with the VVAR_TIMENS_PAGE_OFFSET offset. > + */ This confused me, and I see that it is truncated from the existing commit in arch/arm64/kerne/vdso.c: If a task belongs to a time namespace then a namespace specific VVAR is mapped with the VVAR_DATA_PAGE_OFFSET and the real VVAR page is mapped with the VVAR_TIMENS_PAGE_OFFSET offset. ... and IIUC the "namespace specific VVAR" page doesn't have the RNG data, right? It'd be good to spell that out, e.g. /* * The RNG data is in the real VVAR data page, but if a task * belongs to a time namespsace then VVAR_DATA_PAGE_OFFSET * points to the namespace-specific VVAR page and * VVAR_TIMENS_PAGE_OFFSET points to the real VVAR page. */ It does feel weird that everything else has to work around timer namespaces rather than that being limited to the timer code, so we'll probably want to flip that if we add anything else to the VDSO, or have a separate VVAR_RNG page. > + if (IS_ENABLED(CONFIG_TIME_NS) && _vdso_data->clock_mode == VDSO_CLOCKMODE_TIMENS) > + return (void*)&_vdso_rng_data + VVAR_TIMENS_PAGE_OFFSET * PAGE_SIZE; > + return &_vdso_rng_data; > +} [...] > diff --git a/arch/arm64/kernel/vdso/vgetrandom.c b/arch/arm64/kernel/vdso/vgetrandom.c > new file mode 100644 > index 000000000000..95682d29c4bf > --- /dev/null > +++ b/arch/arm64/kernel/vdso/vgetrandom.c > @@ -0,0 +1,15 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +typeof(__cvdso_getrandom) __kernel_getrandom; > + > +ssize_t __kernel_getrandom(void *buffer, size_t len, unsigned int flags, void *opaque_state, size_t opaque_len) > +{ > + asm goto ( > + ALTERNATIVE("b %[fallback]", "nop", ARM64_HAS_FPSIMD) : : : : fallback); > + return __cvdso_getrandom(buffer, len, flags, opaque_state, opaque_len); > + > +fallback: > + if (unlikely(opaque_len == ~0UL && !buffer && !len && !flags)) > + return -ENOSYS; > + return getrandom_syscall(buffer, len, flags); > +} The asm it pretty painful to read, and AFAICT what you actually want here is alternative_has_cap_likely(), which we could use were that not using alt_cb_patch_nops behind the scenes. I reckon it's worth making the work for the VDSO first (patch below); that way you can make this much nicer: ssize_t __kernel_getrandom(void *buffer, size_t len, unsigned int flags, void *opaque_state, size_t opaque_len) { if (alternative_has_cap_likely(ARM64_HAS_FPSIMD)) { return __cvdso_getrandom(buffer, len, flags, opaque_state, opaque_len); } if (unlikely(opaque_len == ~0UL && !buffer && !len && !flags)) return -ENOSYS; return getrandom_syscall(buffer, len, flags); } ... though the conditions for returning -ENOSYS look very odd to me; why do we care about fast-pathing that specific case rather than forwarding that to the kernel, and does __cvdso_getrandom() handle that correctly? Mark ---->8---- >From b7ee23e4ec47805527c9d7c2ee6b02328fe8437a Mon Sep 17 00:00:00 2001 From: Mark Rutland Date: Mon, 2 Sep 2024 15:08:12 +0100 Subject: [PATCH] arm64: alternative: make alternative_has_cap_likely() VDSO compatible Currently alternative_has_cap_unlikely() can be used in VDSO code, but alternative_has_cap_likely() cannot as it references alt_cb_patch_nops, which is not available when linking the VDSO. This is unfortunate as it would be useful to have alternative_has_cap_likely() available in VDSO code. The use of alt_cb_patch_nops was added in commit: d926079f17bf8aa4 ("arm64: alternatives: add shared NOP callback") ... as removing duplicate NOPs within the kernel Image saved areasonable amount of space. Given the VDSO code will have nowhere near as many alternative branches as the main kernel image, this isn't much of a concern, and a few extra nops isn't a massive problem. Change alternative_has_cap_likely() to only use alt_cb_patch_nops for the main kernel image, and allow duplicate NOPs in VDSO code. Signed-off-by: Mark Rutland Cc: Catalin Marinas Cc: Will Deacon --- arch/arm64/include/asm/alternative-macros.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/arch/arm64/include/asm/alternative-macros.h b/arch/arm64/include/asm/alternative-macros.h index d328f549b1a60..c8c77f9e36d60 100644 --- a/arch/arm64/include/asm/alternative-macros.h +++ b/arch/arm64/include/asm/alternative-macros.h @@ -230,7 +230,11 @@ alternative_has_cap_likely(const unsigned long cpucap) return false; asm goto( +#ifdef BUILD_VDSO + ALTERNATIVE("b %l[l_no]", "nop", %[cpucap]) +#else ALTERNATIVE_CB("b %l[l_no]", %[cpucap], alt_cb_patch_nops) +#endif : : [cpucap] "i" (cpucap) : -- 2.30.2