From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 57BF838B12A for ; Tue, 4 Aug 2026 14:56:22 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785855383; cv=none; b=ULfUhGNb3CGpWDzhbdQtWdujH+jGALG51nTMMVlQPfCnoYiNnioT2GBuuujI5DfJqf9h3ChDD1BsjEQp8pgfgHbLM/76xLbBVTKs1jYDL1Pw3Mgdz6DfJXVwpjTeLeF1tHm+JIAZW2N5KSp3o9vMn5VT/v5eroedqwNYGtwTp00= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785855383; c=relaxed/simple; bh=hvvoH7MThmay3Qtreho5hupRlIi+nqqnGWaTUKUgIGs=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=IgzAnUg7JqIyPO0uyV0ecDjgk53LpLjOsP5/3DZ6lrSuQh0Z2j5+iaVmjc3sozOsmeLNiJbEGOgINFpK7/ocyLfEN6882MiAhTkBzxjZVuTujwneBMZoCNerxPhjVnEml9EKTaU8VG81HyW37SNDwo0duffQgp5WrOLRf2jfZeA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=GLJCs16b; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="GLJCs16b" Received: by smtp.kernel.org (Postfix) with ESMTPSA id BB6B51F00A3F; Tue, 4 Aug 2026 14:56:20 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1785855382; bh=msMgqHZX3Cpzlz49ZY5uD/h6mDTlWrDvHYB6tj0rLc8=; h=Date:From:To:Cc:Subject:References:In-Reply-To; b=GLJCs16bnOBXeMbwwfgHRRXEbz9KGNqyxVQnpsGdSi+XcHFpyStBYsWuwDA5uG75O /Kry99689RRLmSW5ApGSYhvtDde2LJLXba3zZovhd8FqPOVb0yzc2LBXe3BsyIYlQN Kty2G9QoVffdyEPyjZM57t+tifyRtISEnh+l5PBEj9KYhrv53aoQMbFtNZHVNuKfn+ 44MMMVATrgGzhIkmLasUbYJbz91AXQPKWIFnmoH3sF+CZ+X2yS6u66Jw99ZJWPiKwr OND+j9iMiupn28GTKvWWXKfU6EFbajZHOotqW1c1R/U1WMIgf+iJYBc+KA1Zs9zPdh d+Feyi4kKPGYg== Date: Tue, 4 Aug 2026 15:56:17 +0100 From: Will Deacon To: Aman Priyadarshi Cc: catalin.marinas@arm.com, Jason@zx2c4.com, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Ard Biesheuvel Subject: Re: [PATCH] arm64: archrandom: avoid trapping ID register read in __cpu_has_rng() Message-ID: References: <20260720140615.99343-1-amanp@apple.com> <97FF53B9-993A-4E73-9C6B-0DA514AD0778@apple.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <97FF53B9-993A-4E73-9C6B-0DA514AD0778@apple.com> On Fri, Jul 31, 2026 at 06:26:44PM +0100, Aman Priyadarshi wrote: > > On 31 Jul 2026, at 15:43, Will Deacon wrote: > > On Mon, Jul 20, 2026 at 03:06:15PM +0100, Aman Priyadarshi wrote: > >> diff --git a/arch/arm64/include/asm/archrandom.h b/arch/arm64/include/asm/archrandom.h > >> index 8babfbe31f95..8067e9a35641 100644 > >> --- a/arch/arm64/include/asm/archrandom.h > >> +++ b/arch/arm64/include/asm/archrandom.h > >> @@ -61,8 +61,22 @@ static inline bool __arm64_rndrrs(unsigned long *v) > >> > >> static __always_inline bool __cpu_has_rng(void) > >> { > >> - if (unlikely(!system_capabilities_finalized() && !preemptible())) > >> - return this_cpu_has_cap(ARM64_HAS_RNG); > >> + if (unlikely(!system_capabilities_finalized() && !preemptible())) { > >> + /* > >> + * Until the ARM64_HAS_RNG alternative is patched we can't use > >> + * the static-branch form, so consult the feature register > >> + * directly. Don't use this_cpu_has_cap() here: it reads > >> + * ID_AA64ISAR0_EL1 from hardware on every call, under > >> + * virtualization each ID register read traps to the hypervisor > >> + * (HCR_EL2.TID3) -- producing a storm of vmexits during boot. > >> + * The sanitised value is cached in memory. > >> + */ > >> + u64 isar0 = read_sanitised_ftr_reg(SYS_ID_AA64ISAR0_EL1); > >> + > >> + return cpuid_feature_extract_unsigned_field(isar0, > >> + ID_AA64ISAR0_EL1_RNDR_SHIFT) >= > >> + ID_AA64ISAR0_EL1_RNDR_IMP; > >> + } > > > > You can probably rewrite this a little more cleanly along the lines of > > the (not even compile-tested) diff below. I was about to do that, but > > then I got a bit confused by the whole thing. The preemptible() check is > > presumably not needed if we're accessing the in-memory feature registers > > rather than the per-CPU id registers, but then how do you handle races > > with concurrent updates to the "safe value" made by CPUs concurrently > > coming online? > > I kept preemptible() check for this exact reason: the updates made by CPUs > concurrently coming online will always take the downgrade path (a secondary > CPU can clear RNDR, never set it), and therefore by taking the non-preemptible > branch I can guarantee that the pinned CPU supports the said feature. > I agree, this assumes that a secondary CPU folds its own ID registers into sys_val > before it can ever be a randomness consumer, but looking at the code that seems > the case, please feel free to correct me. > Besides, in my opinion, it's hard to argue correctness of this code without > preemptible() check. My point is that this change introduces a data race on 'reg->sys_val' for the ID_AA64ISAR0_EL1 entry in the arm64_ftr_regs array. Will