[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <80c7af9b-ac6a-42fe-981f-c4b6f2cc9d7f@arm.com>
Date: Wed, 19 Nov 2025 06:37:00 +0530
From: Anshuman Khandual <anshuman.khandual@....com>
To: Mark Rutland <mark.rutland@....com>
Cc: linux-arm-kernel@...ts.infradead.org,
Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org>,
Ryan Roberts <ryan.roberts@....com>, Ard Biesheuvel <ardb@...nel.org>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 0/6] arm64/mm: TTBRx_EL1 related changes
On 14/11/25 3:25 PM, Mark Rutland wrote:
> On Thu, Nov 13, 2025 at 02:48:44PM +0530, Anshuman Khandual wrote:
>> On 03/11/25 10:56 AM, Anshuman Khandual wrote:
>>> This series contains some TTBRx_EL1 related changes, aimed at standardizing
>>> TTBRx_EL1 register field accesses via tools sysreg format and also explains
>>> 52 PA specific handling methods via a new macro along with in code comments
>>>
>>> This series applies on v6.18-rc4
>>>
>>> Cc: Catalin Marinas <catalin.marinas@....com>
>>> Cc: Will Deacon <will@...nel.org>
>>> Cc: Ryan Roberts <ryan.roberts@....com>
>>> Cc: Ard Biesheuvel <ardb@...nel.org>
>>> Cc: linux-arm-kernel@...ts.infradead.org
>>> Cc: linux-kernel@...r.kernel.org
>>>
>>> Anshuman Khandual (6):
>>> arm64/mm: Directly use TTBRx_EL1_ASID_MASK
>>> arm64/mm: Directly use TTBRx_EL1_CnP
>>> arm64/mm: Represent TTBR_BADDR_MASK_52 with TTBRx_EL1_BADDR_MASK
>>> arm64/mm: Ensure correct 48 bit PA gets into TTBRx_EL1
>>> arm64/mm: Describe 52 PA folding into TTBRx_EL1
>>> arm64/mm: Describe TTBR1_BADDR_4852_OFFSET
>>>
>>> arch/arm64/include/asm/asm-uaccess.h | 2 +-
>>> arch/arm64/include/asm/assembler.h | 3 ++-
>>> arch/arm64/include/asm/mmu_context.h | 2 +-
>>> arch/arm64/include/asm/pgtable-hwdef.h | 23 ++++++++++++++++++++---
>>> arch/arm64/include/asm/pgtable.h | 5 +++--
>>> arch/arm64/include/asm/uaccess.h | 6 +++---
>>> arch/arm64/kernel/entry.S | 2 +-
>>> arch/arm64/kernel/mte.c | 4 ++--
>>> arch/arm64/mm/context.c | 8 ++++----
>>> arch/arm64/mm/mmu.c | 2 +-
>>> 10 files changed, 38 insertions(+), 19 deletions(-)
>>
>> Gentle ping. Beside [PATCH 4/6] (which can be dropped as indicated by Mark)
>> any concerns regarding reset of these changes here ?
>
> Overall I don;t think this series actually improves anything; it just
> shuffles things around, and leaves conversions half-done. I don't think
> we must take this as-is.
>
> For patches 1 and 2, the changes would be fine if we were also getting
> rid of TTBR_ASID_MASK and TTBR_CNP_BIT, but we don't, apparently because
> those are still used by KVM. It feels like those two patches should be
> split into a separate series that *only* moves code over to generate
> sysreg definitions, also updates KVM, and removes the unused legacy
> definitions.
Sure thing. I will spin out a separate series as suggested.
>
> For patch 3, I think the change makes the code harder to read, and
> harder to understand, because there's no context to explain why we're
> masking out a single bit. I don't think this is actually an improvement.
We are masking out a single bit here as per the ARM DDI 0487
L.B in the page number D24-8540.
------------------------------------------------------------
When TTBR0_EL1.BADDR represents a 52-bit addresses, all of the following apply:
• Bits A[51:48] of the stage 1 translation table base address bits are in register bits[5:2].
• Register bit[1] is RES0.
• The smallest permitted value of x is 6.
• When x>6, register bits[(x-1):6] are RES0.
------------------------------------------------------------
Is not deriving the applicable address mask in the 52 bit PA
context from the original base address mask better than hard
coded mask GENMASK_ULL(47, 2) ? Just wondering - would it be
better to add some more context in the comment above it ?
> See below for related notes for patch 5.
>
> For patch 4, as above, I think the patch can be dropped.
Agreed.
>
> For patch 5, this could be OK, but we should define
> TTBR_BADDR_52_PA_PIVOT as (51 - 5) and avoid the magic number entirely.
Agreed.
> IMO it'd be nicer to just extract and re-insert the bits; I think our
> current logic is unnecessarily micro-optimized so that this can be
> implemented with a shifted-OR + AND, whereas I think we could burn a
> temporary register and use BFX + BFI + AND, and that would be clearer as
> to *which* bits we're trying to move.
Will that only be applicable for phys_to_ttbr()'s assembly variant
and the macro definition in <asm/pgtable.h> which does the right
shifted OR followed by AND still remains unchanged ?
>
> For patch 6, I guess this is fine; I don't have a strong feeling either
> way.
IMHO it will be great to have this patch included.
Powered by blists - more mailing lists