From 50bb52021c0007c7a9188725e7f22efcbe97e57e Mon Sep 17 00:00:00 2001 From: Owen Anderson Date: Mon, 2 Sep 2024 06:51:14 +0000 Subject: [PATCH] arm64: Fixes to the page fault handler. * Improve atomicity of PTE updates. * Centralize a few constants. Change-Id: Iaeeb82e9dc7f7ca97d13ba816fb72c6475754310 Reviewed-on: https://review.haiku-os.org/c/haiku/+/8171 Reviewed-by: waddlesplash Reviewed-by: Fredrik Holmqvist Tested-by: Commit checker robot --- .../arch/arm64/arch_vm_translation_map.h | 4 +++ .../arch/arm64/VMSAv8TranslationMap.cpp | 23 ------------ .../kernel/arch/arm64/VMSAv8TranslationMap.h | 23 ++++++++++++ src/system/kernel/arch/arm64/arch_int.cpp | 36 ++++++++++--------- .../arch/arm64/arch_vm_translation_map.cpp | 7 ---- 5 files changed, 47 insertions(+), 46 deletions(-) diff --git a/headers/private/kernel/arch/arm64/arch_vm_translation_map.h b/headers/private/kernel/arch/arm64/arch_vm_translation_map.h index cf0dd01124..4997c73513 100644 --- a/headers/private/kernel/arch/arm64/arch_vm_translation_map.h +++ b/headers/private/kernel/arch/arm64/arch_vm_translation_map.h @@ -5,6 +5,10 @@ #ifndef _KERNEL_ARCH_ARM64_ARCH_VM_TRANSLATION_MAP_H_ #define _KERNEL_ARCH_ARM64_ARCH_VM_TRANSLATION_MAP_H_ +// The base address of TTBR*_EL1 is in bits [47:1] of the register, and the +// low bit is implicitly zero. +static constexpr uint64_t kTtbrBasePhysAddrMask = (((1UL << 47) - 1) << 1); + void arch_vm_install_empty_table_ttbr0(void); #endif /* _KERNEL_ARCH_ARM64_ARCH_VM_TRANSLATION_MAP_H_ */ diff --git a/src/system/kernel/arch/arm64/VMSAv8TranslationMap.cpp b/src/system/kernel/arch/arm64/VMSAv8TranslationMap.cpp index 544bf5029c..20a8d79a9c 100644 --- a/src/system/kernel/arch/arm64/VMSAv8TranslationMap.cpp +++ b/src/system/kernel/arch/arm64/VMSAv8TranslationMap.cpp @@ -10,29 +10,6 @@ #include #include - -static constexpr uint64_t kPteAddrMask = (((1UL << 36) - 1) << 12); -static constexpr uint64_t kPteAttrMask = ~(kPteAddrMask | 0x3); -static constexpr uint64_t kPteTLBCompatMask = (kPteAddrMask | (0x3 << 2) | (0x3 << 8)); - -static constexpr uint64_t kPteValidMask = 0x1; -static constexpr uint64_t kPteTypeMask = 0x3; -static constexpr uint64_t kPteTypeL012Table = 0x3; -static constexpr uint64_t kPteTypeL12Block = 0x1; -static constexpr uint64_t kPteTypeL3Page = 0x3; - -static constexpr uint64_t kAttrSWDBM = (1UL << 55); -static constexpr uint64_t kAttrUXN = (1UL << 54); -static constexpr uint64_t kAttrPXN = (1UL << 53); -static constexpr uint64_t kAttrDBM = (1UL << 51); -static constexpr uint64_t kAttrNG = (1UL << 11); -static constexpr uint64_t kAttrAF = (1UL << 10); -static constexpr uint64_t kAttrSHInnerShareable = (3UL << 8); -static constexpr uint64_t kAttrAPReadOnly = (1UL << 7); -static constexpr uint64_t kAttrAPUserAccess = (1UL << 6); - -static constexpr uint64_t kTLBIMask = ((1UL << 44) - 1); - uint32_t VMSAv8TranslationMap::fHwFeature; uint64_t VMSAv8TranslationMap::fMair; diff --git a/src/system/kernel/arch/arm64/VMSAv8TranslationMap.h b/src/system/kernel/arch/arm64/VMSAv8TranslationMap.h index 6c17f78467..a31e86e0db 100644 --- a/src/system/kernel/arch/arm64/VMSAv8TranslationMap.h +++ b/src/system/kernel/arch/arm64/VMSAv8TranslationMap.h @@ -10,6 +10,29 @@ #include +static constexpr uint64_t kPteAddrMask = (((1UL << 36) - 1) << 12); +static constexpr uint64_t kPteAttrMask = ~(kPteAddrMask | 0x3); +static constexpr uint64_t kPteTLBCompatMask = (kPteAddrMask | (0x3 << 2) | (0x3 << 8)); + +static constexpr uint64_t kPteValidMask = 0x1; +static constexpr uint64_t kPteTypeMask = 0x3; +static constexpr uint64_t kPteTypeL012Table = 0x3; +static constexpr uint64_t kPteTypeL12Block = 0x1; +static constexpr uint64_t kPteTypeL3Page = 0x3; + +static constexpr uint64_t kAttrSWDBM = (1UL << 55); +static constexpr uint64_t kAttrUXN = (1UL << 54); +static constexpr uint64_t kAttrPXN = (1UL << 53); +static constexpr uint64_t kAttrDBM = (1UL << 51); +static constexpr uint64_t kAttrNG = (1UL << 11); +static constexpr uint64_t kAttrAF = (1UL << 10); +static constexpr uint64_t kAttrSHInnerShareable = (3UL << 8); +static constexpr uint64_t kAttrAPReadOnly = (1UL << 7); +static constexpr uint64_t kAttrAPUserAccess = (1UL << 6); + +static constexpr uint64_t kTLBIMask = ((1UL << 44) - 1); + + struct VMSAv8TranslationMap : public VMTranslationMap { public: VMSAv8TranslationMap( diff --git a/src/system/kernel/arch/arm64/arch_int.cpp b/src/system/kernel/arch/arm64/arch_int.cpp index afc3b4b938..07b480c687 100644 --- a/src/system/kernel/arch/arm64/arch_int.cpp +++ b/src/system/kernel/arch/arm64/arch_int.cpp @@ -127,13 +127,6 @@ arch_int_init_post_device_manager(struct kernel_args *args) static int page_bits = 12; -static constexpr uint64_t kPteAddrMask = (((1UL << 36) - 1) << 12); -static constexpr uint64_t kPteAttrMask = ~(kPteAddrMask | 0x3); -static constexpr uint64_t kAttrSWDBM = (1UL << 55); -static constexpr uint64_t kAttrAF = (1UL << 10); -static constexpr uint64_t kAttrAP2 = (1UL << 7); - - static uint64_t* TableFromPa(phys_addr_t pa) { @@ -154,21 +147,31 @@ fixup_entry(phys_addr_t ptPa, int level, addr_t va, bool wr) int index = (va >> shift) & tableMask; uint64_t *pte = &TableFromPa(ptPa)[index]; + uint64_t oldPte = atomic_get64((int64*)pte); - int type = *pte & 0x3; - uint64_t addr = *pte & kPteAddrMask; + int type = oldPte & kPteTypeMask; + uint64_t addr = oldPte & kPteAddrMask; - if ((level == 3 && type == 0x3) || (level < 3 && type == 0x1)) { - if (!wr && (*pte & kAttrAF) == 0) { - atomic_or64((int64*)pte, kAttrAF); + if ((level == 3 && type == kPteTypeL3Page) || (level < 3 && type == kPteTypeL12Block)) { + if (!wr && (oldPte & kAttrAF) == 0) { + uint64_t newPte = oldPte | kAttrAF; + if ((uint64_t)atomic_test_and_set64((int64*)pte, newPte, oldPte) != oldPte) + return true; // If something changed, handle it by taking another fault + asm("dsb ishst"); + asm("isb"); return true; } - if (wr && (*pte & kAttrSWDBM) != 0 && (*pte & kAttrAP2) != 0) { - atomic_and64((int64*)pte, ~kAttrAP2); - asm("tlbi vaae1is, %0 \n dsb ish"::"r"(va >> page_bits)); + if (wr && (oldPte & kAttrSWDBM) != 0 && (oldPte & kAttrAPReadOnly) != 0) { + uint64_t newPte = oldPte & ~kAttrAPReadOnly; + if ((uint64_t)atomic_test_and_set64((int64*)pte, newPte, oldPte) != oldPte) + return true; + asm("dsb ishst"); + asm("tlbi vaae1is, %0" :: "r" ((va >> 12) & kTLBIMask)); + asm("dsb ish"); + asm("isb"); return true; } - } else if (level < 3 && type == 0x3) { + } else if (level < 3 && type == kPteTypeL012Table) { return fixup_entry(addr, level + 1, va, wr); } @@ -251,6 +254,7 @@ do_sync_handler(iframe * frame) ptPa = READ_SPECIALREG(TTBR1_EL1); else ptPa = READ_SPECIALREG(TTBR0_EL1); + ptPa &= kTtbrBasePhysAddrMask; switch (frame->esr & ISS_DATA_DFSC_MASK) { case ISS_DATA_DFSC_TF_L0: diff --git a/src/system/kernel/arch/arm64/arch_vm_translation_map.cpp b/src/system/kernel/arch/arm64/arch_vm_translation_map.cpp index cc4fd1c117..002e1d01c1 100644 --- a/src/system/kernel/arch/arm64/arch_vm_translation_map.cpp +++ b/src/system/kernel/arch/arm64/arch_vm_translation_map.cpp @@ -15,10 +15,6 @@ static char sPhysicalPageMapperData[sizeof(PMAPPhysicalPageMapper)]; -// The base address of TTBR*_EL1 is in bits [47:1] of the register, and the -// low bit is implicitly zero. -static constexpr uint64_t kTtbrBasePhysAddrMask = (((1UL << 47) - 1) << 1); - // Physical pointer to an empty page table, which is used for break-before-make // when updating TTBR0_EL1. static phys_addr_t sEmptyTable; @@ -145,9 +141,6 @@ arch_vm_translation_map_init_post_area(kernel_args* args) // TODO: reuse some bits from VMSAv8TranslationMap -static constexpr uint64_t kPteAddrMask = (((1UL << 36) - 1) << 12); -static constexpr uint64_t kPteAttrMask = ~(kPteAddrMask | 0x3); - static uint64_t page_bits = 12; static uint64_t tsz = 16;