From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 8AE022750EF for ; Tue, 15 Apr 2025 06:28:06 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.140.110.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1744698489; cv=none; b=f1gP7aelqe+YAnpeaSuM3PpjikL0UNqKoPiEFdO/8K4moaNduItnZEhGUDBp/Zwz3CV30NgrX5i1jCdszU1Dkrn24w19JgwFsNc8BSqrZnpBG5JFV2LlObYvmTx/ccwnrk50uwI93muZkiTaZmN1zQVGEy/SSFAwEmV/xPFIqdg= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1744698489; c=relaxed/simple; bh=1jFVtsvm+dJBxl+YLRLEeNH9kTC39AeT3qdnKM1EBPI=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=E9yDw/GeYctOc621eA6RV2z8bBJuvvfstE0/j/ZisX0z9u2uuYM+0sSHa4ltBLaDxam2fesegWX9ywPNIYR9zyWKQvMKjcSdSUBFQVoGi/V3nrwhbwpDXOTptJY3BIgOKeDqcidP09ksF6k55Cb2YYGlMnkh2C6vonbGT6MrKnM= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com; spf=pass smtp.mailfrom=arm.com; arc=none smtp.client-ip=217.140.110.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=arm.com 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 D690315A1; Mon, 14 Apr 2025 23:28:03 -0700 (PDT) Received: from [10.163.49.104] (unknown [10.163.49.104]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id AB4F13F694; Mon, 14 Apr 2025 23:28:02 -0700 (PDT) Message-ID: <0eabad93-26ef-4452-bd89-17c153f483f3@arm.com> Date: Tue, 15 Apr 2025 11:57:57 +0530 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] arm64/mm: Re-organise setting up FEAT_S1PIE registers PIRE0_EL1 and PIR_EL1 To: Ryan Roberts , Ard Biesheuvel Cc: linux-arm-kernel@lists.infradead.org, Catalin Marinas , Will Deacon , Mark Rutland , linux-kernel@vger.kernel.org References: <20250410074024.1545768-1-anshuman.khandual@arm.com> <6e6305fd-3b93-43ec-8114-e81b2926adfc@arm.com> <16602b97-2f49-4612-9e9a-d6d0ed964fd3@arm.com> <5d975762-7678-419f-8e2f-40547c079276@arm.com> Content-Language: en-US From: Anshuman Khandual In-Reply-To: <5d975762-7678-419f-8e2f-40547c079276@arm.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit On 4/14/25 18:01, Ryan Roberts wrote: > On 14/04/2025 13:28, Ard Biesheuvel wrote: >> On Mon, 14 Apr 2025 at 14:04, Ryan Roberts wrote: >>> >>> On 14/04/2025 10:41, Ard Biesheuvel wrote: >>>> On Mon, 14 Apr 2025 at 09:52, Ryan Roberts wrote: >>>>> >>>>> On 10/04/2025 08:40, Anshuman Khandual wrote: >>>>>> mov_q cannot really move PIE_E[0|1] macros into a general purpose register >>>>>> as expected if those macro constants contain some 128 bit layout elements, >>>>>> required for D128 page tables. Fix this problem via first loading up these >>>>>> macro constants into a given memory location and then subsequently setting >>>>>> up registers PIRE0_EL1 and PIR_EL1 by retrieving the memory stored values. >>>>> >>>>> From memory, the primary issue is that for D128, PIE_E[0|1] are defined in terms >>>>> of 128-bit types with shifting and masking, which the assembler can't do? It >>>>> would be good to spell this out. >>>>> >>>>>> >>>>>> Cc: Catalin Marinas >>>>>> Cc: Will Deacon >>>>>> Cc: Mark Rutland >>>>>> Cc: Ard Biesheuvel >>>>>> Cc: Ryan Roberts >>>>>> Cc: linux-arm-kernel@lists.infradead.org >>>>>> Cc: linux-kernel@vger.kernel.org >>>>>> Signed-off-by: Anshuman Khandual >>>>>> --- >>>>>> This patch applies on v6.15-rc1 >>>>>> >>>>>> arch/arm64/kernel/head.S | 3 +++ >>>>>> arch/arm64/kernel/pi/map_range.c | 6 ++++++ >>>>>> arch/arm64/kernel/pi/pi.h | 1 + >>>>>> arch/arm64/mm/mmu.c | 1 + >>>>>> arch/arm64/mm/proc.S | 5 +++-- >>>>>> 5 files changed, 14 insertions(+), 2 deletions(-) >>>>>> >>>>>> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S >>>>>> index 2ce73525de2c..4950d9cc638a 100644 >>>>>> --- a/arch/arm64/kernel/head.S >>>>>> +++ b/arch/arm64/kernel/head.S >>>>>> @@ -126,6 +126,9 @@ SYM_CODE_START(primary_entry) >>>>>> * On return, the CPU will be ready for the MMU to be turned on and >>>>>> * the TCR will have been set. >>>>>> */ >>>>>> + adr_l x0, pir_data >>>>>> + bl __pi_load_pir_data >>>>> >>>>> Using C code to pre-calculate the values into global variables that the assembly >>>>> code then loads and stuffs into the PIR registers feels hacky. I wonder if we >>>>> can instead pre-calculate into asm-offsets.h? e.g. add the following to >>>>> asm-offsets.c: >>>>> >>>>> DEFINE(PIE_E0_ASM, PIE_E0); >>>>> DEFINE(PIE_E1_ASM, PIE_E1); >>>>> >>>>> Which will generate the asm-offsets.h header with PIE_E[0|1]_ASM with the >>>>> pre-calculated values that you can then use in proc.S? >>>>> >>>> >>>> There is another issue, which is that mov_q tries to be smart, and >>>> emit fewer than 4 MOVZ/MOVK instructions if possible. So the .if >>>> directive evaluates the argument, which does not work with symbolic >>>> constants. >>> >>> I'm not quite understanding the detail here; what do you mean by "symbolic >>> constants"? asm-offsets.h will provide something like: >>> >>> #define PIE_E0_ASM 1234567890 >>> >>> The current code is using a hash-define and that's working fine: >>> >>> mov_q x0, PIE_E0 >>> >>> >>> Won't the C preprocessor just substitute and everything will work out? >>> >> >> Yeah, you're right. I was experimenting with something like >> >> .set .Lpie_e0, PIE_E0_ASM >> mov_q xN, .Lpie_e0 >> >> where this problem does exist, but we can just use PIE_E0_ASM directly >> and things should work as expected. > > Ahh great, sounds like this should be pretty simple then! Following change works both on current and with D128 page tables. --- a/arch/arm64/kernel/asm-offsets.c +++ b/arch/arm64/kernel/asm-offsets.c @@ -182,5 +182,7 @@ int main(void) #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS DEFINE(FTRACE_OPS_DIRECT_CALL, offsetof(struct ftrace_ops, direct_call)); #endif + DEFINE(PIE_E0_ASM, PIE_E0); + DEFINE(PIE_E1_ASM, PIE_E1); return 0; } diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S index 737c99d79833..f45494425d09 100644 --- a/arch/arm64/mm/proc.S +++ b/arch/arm64/mm/proc.S @@ -536,9 +536,9 @@ alternative_else_nop_endif #define PTE_MAYBE_NG 0 #define PTE_MAYBE_SHARED 0 - mov_q x0, PIE_E0 + mov_q x0, PIE_E0_ASM msr REG_PIRE0_EL1, x0 - mov_q x0, PIE_E1 + mov_q x0, PIE_E1_ASM msr REG_PIR_EL1, x0 #undef PTE_MAYBE_NG