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 62968CA0EED for ; Tue, 19 Aug 2025 08:43:53 +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:Content-Type:MIME-Version: References:In-Reply-To:Subject:Cc:To:From:Message-ID: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=EAoQdpzwxuti772MPMT9IU0xVBIlyeQ7u38tA+p9XEs=; b=Y8Kd2RPX7I+pqxC5qP/ZulFGyQ VyxUrfQkYOtyBujseBaJcAjf0tqxeHrUev9HEpUNZTZsWqCbIyL1ggTDxnN7aoTO3OWtc2YkL6j3Z 04EW6i5wBic/VgQmviK78VQRjU+q2KY14hbVCuyz+lViOdf4k93Bn1qyXrrj/q9vV0uqNjl2aZ+O0 tgkAshbZoNBgQjfATj/+s+d9ngXoX9EVx9rdtOvW0rIvq/1x/W2kWzBMgcwDi3BINn9PGcIcBDbM+ A76FL/iCYJ2S314t4Wa/CIIQ84LgUysoG+HJTETMIOmyctlFmzTtg9TSRI1YsUo0rEELjVMia/VON y2JJt89g==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1uoHwY-00000009q6k-2duV; Tue, 19 Aug 2025 08:43:46 +0000 Received: from dfw.source.kernel.org ([139.178.84.217]) by bombadil.infradead.org with esmtps (Exim 4.98.2 #2 (Red Hat Linux)) id 1uoHSC-00000009ltU-2ho6 for linux-arm-kernel@lists.infradead.org; Tue, 19 Aug 2025 08:12:25 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id D8B375C5BCF; Tue, 19 Aug 2025 08:12:23 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 7CD04C4CEF1; Tue, 19 Aug 2025 08:12:23 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1755591143; bh=/dfQAZayWIHQFfzoYaLoa9ZkINRtrj3kEglQs3WnKZM=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=Oh1vjBjXaqQbcJHw3DjqK6XlLzbF/z1jr+sMGRuWk+icJ8e1Uo5CFIT+NTudIih/e 2nstt53rKHr03zEdKBOezlJQCXYYBeeRyta5adtumTVNKP54qnB38VNmu/joFhnSF0 0Aqy0S+Uol1BCfeVQkIQUd48uY8T276Py30Pa8f35g+Mr920cqdQX+WWAI/T2JvEXb 7nWBifR+388oDoXODW/RogsRMVbqFOt/w3jYDXht1kY7V/vFi8D5xLdhF0GySzs+Wd /LfM7ComIUQIQyg+97rkSvZaZyDzT5hElHtYy1sWSviQibf6nptwi+ITnNlWLqUD+3 EvTGlH/zW0f1w== Received: from host86-149-246-145.range86-149.btcentralplus.com ([86.149.246.145] helo=lobster-girl.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1uoHS9-008szj-4s; Tue, 19 Aug 2025 09:12:21 +0100 Date: Tue, 19 Aug 2025 09:12:17 +0100 Message-ID: <87v7mjk9we.wl-maz@kernel.org> From: Marc Zyngier To: Anshuman Khandual Cc: linux-arm-kernel@lists.infradead.org, Catalin Marinas , Will Deacon , Oliver Upton , Mark Brown , Ryan Roberts , kvmarm@lists.linux.dev, linux-kernel@vger.kernel.org Subject: Re: [PATCH 2/4] arm64/sysreg: Replace TCR_EL1 field macros In-Reply-To: References: <20250818045759.672408-1-anshuman.khandual@arm.com> <20250818045759.672408-3-anshuman.khandual@arm.com> <87jz30my30.wl-maz@kernel.org> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/30.1 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII X-SA-Exim-Connect-IP: 86.149.246.145 X-SA-Exim-Rcpt-To: anshuman.khandual@arm.com, linux-arm-kernel@lists.infradead.org, catalin.marinas@arm.com, will@kernel.org, oliver.upton@linux.dev, broonie@kernel.org, ryan.roberts@arm.com, kvmarm@lists.linux.dev, linux-kernel@vger.kernel.org X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20250819_011224_810559_A723B05F X-CRM114-Status: GOOD ( 33.11 ) 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 Tue, 19 Aug 2025 07:46:50 +0100, Anshuman Khandual wrote: > > > > On 18/08/25 9:16 PM, Marc Zyngier wrote: > > On Mon, 18 Aug 2025 05:57:57 +0100, > > Anshuman Khandual wrote: > >> > >> This just replaces all used TCR_EL1 field macros with tools sysreg variant > >> based fields and subsequently drops them from the header (pgtable-hwdef.h). > >> While here, also drop all the unused TCR_XXX macros from the header. > >> > >> Cc: Catalin Marinas > >> Cc: Will Deacon > >> Cc: Marc Zyngier > >> Cc: Mark Brown > >> Cc: kvmarm@lists.linux.dev > >> Cc: linux-arm-kernel@lists.infradead.org > >> Cc: linux-kernel@vger.kernel.org > >> Signed-off-by: Anshuman Khandual > >> --- > >> arch/arm64/include/asm/assembler.h | 6 +- > >> arch/arm64/include/asm/cputype.h | 2 +- > >> arch/arm64/include/asm/kvm_arm.h | 28 +++--- > >> arch/arm64/include/asm/kvm_nested.h | 6 +- > >> arch/arm64/include/asm/mmu_context.h | 4 +- > >> arch/arm64/include/asm/pgtable-hwdef.h | 107 +++------------------ > >> arch/arm64/include/asm/pgtable-prot.h | 2 +- > >> arch/arm64/kernel/cpufeature.c | 4 +- > >> arch/arm64/kernel/pi/map_kernel.c | 8 +- > >> arch/arm64/kernel/vmcore_info.c | 2 +- > >> arch/arm64/kvm/arm.c | 6 +- > >> arch/arm64/kvm/at.c | 48 ++++----- > >> arch/arm64/kvm/hyp/include/hyp/switch.h | 2 +- > >> arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h | 2 +- > >> arch/arm64/kvm/hyp/nvhe/switch.c | 2 +- > >> arch/arm64/kvm/hyp/nvhe/tlb.c | 2 +- > >> arch/arm64/kvm/hyp/vhe/tlb.c | 2 +- > >> arch/arm64/kvm/nested.c | 8 +- > >> arch/arm64/kvm/pauth.c | 12 +-- > >> arch/arm64/mm/proc.S | 29 +++--- > >> tools/arch/arm64/include/asm/cputype.h | 2 +- > >> 21 files changed, 101 insertions(+), 183 deletions(-) > > > > [...] > > > >> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c > >> index 888f7c7abf54..b47d6d530e57 100644 > >> --- a/arch/arm64/kvm/arm.c > >> +++ b/arch/arm64/kvm/arm.c > >> @@ -2000,10 +2000,10 @@ static void __init cpu_prepare_hyp_mode(int cpu, u32 hyp_va_bits) > >> > >> tcr = read_sysreg(tcr_el1); > >> if (cpus_have_final_cap(ARM64_KVM_HVHE)) { > >> - tcr &= ~(TCR_HD | TCR_HA | TCR_A1 | TCR_T0SZ_MASK); > >> - tcr |= TCR_EPD1_MASK; > >> + tcr &= ~(TCR_EL1_HD | TCR_EL1_HA | TCR_EL1_A1 | TCR_EL1_T0SZ_MASK); > >> + tcr |= TCR_EL1_EPD1_MASK; > > > > Except that none of that code is about EL1. At all. > > > >> } else { > >> - unsigned long ips = FIELD_GET(TCR_IPS_MASK, tcr); > >> + unsigned long ips = FIELD_GET(TCR_EL1_IPS_MASK, tcr); > >> > >> tcr &= TCR_EL2_MASK; > >> tcr |= TCR_EL2_RES1 | FIELD_PREP(TCR_EL2_PS_MASK, ips); > >> diff --git a/arch/arm64/kvm/at.c b/arch/arm64/kvm/at.c > >> index 0e5610533949..5f0f10ef38f0 100644 > >> --- a/arch/arm64/kvm/at.c > >> +++ b/arch/arm64/kvm/at.c > >> @@ -134,8 +134,8 @@ static int setup_s1_walk(struct kvm_vcpu *vcpu, struct s1_walk_info *wi, > >> tbi = (wi->regime == TR_EL2 ? > >> FIELD_GET(TCR_EL2_TBI, tcr) : > >> (va55 ? > >> - FIELD_GET(TCR_TBI1, tcr) : > >> - FIELD_GET(TCR_TBI0, tcr))); > >> + FIELD_GET(TCR_EL1_TBI1, tcr) : > >> + FIELD_GET(TCR_EL1_TBI0, tcr))); > > > > This is the reason number one why I dislike this patch. > > > > Here, we deal with both the EL1&0 *and* the EL2&0 translation > > regimes. And I left the original definition *on purpose* so that > > nobody would read this code as being EL1-only. Now, you will glance > > over it with warm fuzzy feeling that you know what this is about -- > > purely EL1. And that's what bugs are made of. > > > > Of course, nothing changed functionally. But is it better? No. > > Just wondering - will it be better to use TCR_EL1/TCR_EL2 definitions > conditionally for EL1&0 and EL2&0 translation regimes as applicable > ? Write the code, look at the result, realise this is totally useless. Because TCR_EL1 and TCR_EL2 *WHEN E2H==1* are designed to have the same layout. > Could there any other better method here ? Because the current > situation where there are some custom TCR macros, some tools sysreg > generated macros, and then those macros getting used in an adhoc > manner in different places, is not very consistent either. The better way is to leave this stuff alone. Honestly, I don't see any improvement in repainting the KVM code to make it less readable. If anything, define the old macros in terms of the new ones, and move them to be KVM-private. M. -- Jazz isn't dead. It just smells funny.