From 98895d0417b6a4595134152294df15833014400d Mon Sep 17 00:00:00 2001 From: X512 Date: Mon, 25 Sep 2023 16:27:44 +0900 Subject: [PATCH] riscv64/mmu: use struct bitfield for flags and std::atomic Fixes some PTE concurrent access bugs. Change-Id: I09ec56861fae389a8a3e228b17a3921b85202c8b Reviewed-on: https://review.haiku-os.org/c/haiku/+/6949 Reviewed-by: Alex von Gluck IV --- .../system/arch/riscv64/arch_cpu_defs.h | 27 +-- .../platform/efi/arch/riscv64/arch_mmu.cpp | 83 ++++--- src/system/boot/platform/riscv/mmu.cpp | 84 ++++--- .../arch/riscv64/RISCV64VMTranslationMap.cpp | 228 +++++++++--------- .../arch/riscv64/RISCV64VMTranslationMap.h | 4 +- src/system/kernel/arch/riscv64/arch_vm.cpp | 31 ++- .../arch/riscv64/arch_vm_translation_map.cpp | 43 ++-- 7 files changed, 270 insertions(+), 230 deletions(-) diff --git a/headers/private/system/arch/riscv64/arch_cpu_defs.h b/headers/private/system/arch/riscv64/arch_cpu_defs.h index 827a383c9c..870ba0499c 100644 --- a/headers/private/system/arch/riscv64/arch_cpu_defs.h +++ b/headers/private/system/arch/riscv64/arch_cpu_defs.h @@ -124,23 +124,20 @@ enum { pteIdxBits = 9, }; -enum { - pteValid = 0, - pteRead = 1, - pteWrite = 2, - pteExec = 3, - pteUser = 4, - pteGlobal = 5, - pteAccessed = 6, - pteDirty = 7, -}; - union Pte { struct { - uint64 flags: 8; - uint64 rsw: 2; - uint64 ppn: 44; - uint64 reserved: 10; + uint64 isValid: 1; + uint64 isRead: 1; + uint64 isWrite: 1; + uint64 isExec: 1; + uint64 isUser: 1; + uint64 isGlobal: 1; + uint64 isAccessed: 1; + uint64 isDirty: 1; + + uint64 rsw: 2; + uint64 ppn: 44; + uint64 reserved: 10; }; uint64 val; }; diff --git a/src/system/boot/platform/efi/arch/riscv64/arch_mmu.cpp b/src/system/boot/platform/efi/arch/riscv64/arch_mmu.cpp index 1da7552f8e..de4b451f46 100644 --- a/src/system/boot/platform/efi/arch/riscv64/arch_mmu.cpp +++ b/src/system/boot/platform/efi/arch/riscv64/arch_mmu.cpp @@ -63,15 +63,15 @@ WritePteFlags(uint32 flags) if ((1 << i) & flags) { if (first) first = false; else dprintf(", "); switch (i) { - case pteValid: dprintf("valid"); break; - case pteRead: dprintf("read"); break; - case pteWrite: dprintf("write"); break; - case pteExec: dprintf("exec"); break; - case pteUser: dprintf("user"); break; - case pteGlobal: dprintf("global"); break; - case pteAccessed: dprintf("accessed"); break; - case pteDirty: dprintf("dirty"); break; - default: dprintf("%" B_PRIu32, i); + case 0: dprintf("valid"); break; + case 1: dprintf("read"); break; + case 2: dprintf("write"); break; + case 3: dprintf("exec"); break; + case 4: dprintf("user"); break; + case 5: dprintf("global"); break; + case 6: dprintf("accessed"); break; + case 7: dprintf("dirty"); break; + default: dprintf("%" B_PRIu32, i); } } } @@ -106,8 +106,8 @@ DumpPageTableInt(Pte* pte, uint64_t virtAdr, uint32_t level, uint64& firstVirt, uint64& firstFlags, uint64& len) { for (uint32 i = 0; i < pteCount; i++) { - if (((1 << pteValid) & pte[i].flags) != 0) { - if ((((1 << pteRead) | (1 << pteWrite) | (1 << pteExec)) & pte[i].flags) == 0) { + if (pte[i].isValid) { + if (!pte[i].isRead && !pte[i].isWrite && !pte[i].isExec) { if (level == 0) panic("internal page table on level 0"); @@ -119,7 +119,7 @@ DumpPageTableInt(Pte* pte, uint64_t virtAdr, uint32_t level, uint64& firstVirt, SignExtendVirtAdr(virtAdr + ((uint64_t)i << (pageBits + pteIdxBits*level))), pte[i].ppn * B_PAGE_SIZE, 1 << (pageBits + pteIdxBits*level), - pte[i].flags, + pte[i].val & 0xff, firstVirt, firstPhys, firstFlags, len); } } @@ -151,14 +151,19 @@ LookupPte(addr_t virtAdr, bool alloc) Pte *pte = (Pte*)VirtFromPhys(sPageTable); for (int level = 2; level > 0; level --) { pte += VirtAdrPte(virtAdr, level); - if (((1 << pteValid) & pte->flags) == 0) { + if (!pte->isValid) { if (!alloc) return NULL; - pte->ppn = mmu_allocate_page() / B_PAGE_SIZE; - if (pte->ppn == 0) + uint64 ppn = mmu_allocate_page() / B_PAGE_SIZE; + if (ppn == 0) return NULL; - memset((Pte*)VirtFromPhys(B_PAGE_SIZE * pte->ppn), 0, B_PAGE_SIZE); - pte->flags |= (1 << pteValid) | (IS_KERNEL_ADDRESS(virtAdr) ? (1 << pteGlobal) : 0); + memset((Pte*)VirtFromPhys(B_PAGE_SIZE * ppn), 0, B_PAGE_SIZE); + Pte newPte { + .isValid = true, + .isGlobal = IS_KERNEL_ADDRESS(virtAdr), + .ppn = ppn + }; + pte->val = newPte.val; } pte = (Pte*)VirtFromPhys(B_PAGE_SIZE * pte->ppn); } @@ -174,10 +179,15 @@ Map(addr_t virtAdr, phys_addr_t physAdr, uint64 flags) Pte* pte = LookupPte(virtAdr, true); if (pte == NULL) panic("can't allocate page table"); - pte->ppn = physAdr / B_PAGE_SIZE; - pte->flags = (1 << pteValid) | (1 << pteAccessed) | (1 << pteDirty) - | (IS_KERNEL_ADDRESS(virtAdr) ? (1 << pteGlobal) : 0) - | flags; + Pte newPte { + .isValid = true, + .isGlobal = IS_KERNEL_ADDRESS(virtAdr), + .isAccessed = true, + .isDirty = true, + }; + newPte.val |= flags; + + pte->val = newPte.val; } @@ -230,11 +240,16 @@ PreallocKernelRange() Pte* root = (Pte*)VirtFromPhys(sPageTable); for (uint64 i = VirtAdrPte(KERNEL_BASE, 2); i <= VirtAdrPte(KERNEL_TOP, 2); i++) { - Pte *pte = &root[i]; - pte->ppn = mmu_allocate_page() / B_PAGE_SIZE; - if (pte->ppn == 0) panic("can't alloc early physical page"); + Pte* pte = &root[i]; + uint64 ppn = mmu_allocate_page() / B_PAGE_SIZE; + if (ppn == 0) panic("can't alloc early physical page"); memset(VirtFromPhys(B_PAGE_SIZE * pte->ppn), 0, B_PAGE_SIZE); - pte->flags |= (1 << pteValid) | (1 << pteGlobal); + Pte newPte { + .isValid = true, + .isGlobal = true, + .ppn = ppn + }; + pte->val = newPte.val; } } @@ -366,7 +381,7 @@ arch_mmu_generate_post_efi_page_tables(size_t memoryMapSize, efi_memory_descript gKernelArgs.arch_args.physMap.start = KERNEL_TOP + 1 - physMemRange.size; gKernelArgs.arch_args.physMap.size = physMemRange.size; MapRange(gKernelArgs.arch_args.physMap.start, physMemRange.start, physMemRange.size, - (1 << pteRead) | (1 << pteWrite)); + Pte {.isRead = true, .isWrite = true}.val); // Boot loader TRACE("Boot loader:\n"); @@ -376,7 +391,7 @@ arch_mmu_generate_post_efi_page_tables(size_t memoryMapSize, efi_memory_descript case EfiLoaderCode: case EfiLoaderData: MapRange(entry->VirtualStart, entry->PhysicalStart, entry->NumberOfPages * B_PAGE_SIZE, - (1 << pteRead) | (1 << pteWrite) | (1 << pteExec)); + Pte {.isRead = true, .isWrite = true, .isExec = true}.val); break; default: ; @@ -393,7 +408,7 @@ arch_mmu_generate_post_efi_page_tables(size_t memoryMapSize, efi_memory_descript efi_memory_descriptor* entry = &memoryMap[i]; if ((entry->Attribute & EFI_MEMORY_RUNTIME) != 0) MapRange(entry->VirtualStart, entry->PhysicalStart, entry->NumberOfPages * B_PAGE_SIZE, - (1 << pteRead) | (1 << pteWrite) | (1 << pteExec)); + Pte {.isRead = true, .isWrite = true, .isExec = true}.val); } // Memory regions @@ -403,22 +418,22 @@ arch_mmu_generate_post_efi_page_tables(size_t memoryMapSize, efi_memory_descript phys_addr_t physAdr; size_t size; while (mmu_next_region(&cookie, &virtAdr, &physAdr, &size)) { - MapRange(virtAdr, physAdr, size, (1 << pteRead) | (1 << pteWrite) | (1 << pteExec)); + MapRange(virtAdr, physAdr, size, Pte {.isRead = true, .isWrite = true, .isExec = true}.val); } // Devices TRACE("Devices:\n"); - MapAddrRange(gKernelArgs.arch_args.clint, (1 << pteRead) | (1 << pteWrite)); - MapAddrRange(gKernelArgs.arch_args.htif, (1 << pteRead) | (1 << pteWrite)); - MapAddrRange(gKernelArgs.arch_args.plic, (1 << pteRead) | (1 << pteWrite)); + MapAddrRange(gKernelArgs.arch_args.clint, Pte {.isRead = true, .isWrite = true}.val); + MapAddrRange(gKernelArgs.arch_args.htif, Pte {.isRead = true, .isWrite = true}.val); + MapAddrRange(gKernelArgs.arch_args.plic, Pte {.isRead = true, .isWrite = true}.val); if (strcmp(gKernelArgs.arch_args.uart.kind, "") != 0) { MapRange(gKernelArgs.arch_args.uart.regs.start, gKernelArgs.arch_args.uart.regs.start, gKernelArgs.arch_args.uart.regs.size, - (1 << pteRead) | (1 << pteWrite)); + Pte {.isRead = true, .isWrite = true}.val); MapAddrRange(gKernelArgs.arch_args.uart.regs, - (1 << pteRead) | (1 << pteWrite)); + Pte {.isRead = true, .isWrite = true}.val); } sort_address_ranges(gKernelArgs.virtual_allocated_range, diff --git a/src/system/boot/platform/riscv/mmu.cpp b/src/system/boot/platform/riscv/mmu.cpp index ca1446fa0c..84eb16c1f2 100644 --- a/src/system/boot/platform/riscv/mmu.cpp +++ b/src/system/boot/platform/riscv/mmu.cpp @@ -54,15 +54,15 @@ WritePteFlags(uint32 flags) if ((1 << i) & flags) { if (first) first = false; else dprintf(", "); switch (i) { - case pteValid: dprintf("valid"); break; - case pteRead: dprintf("read"); break; - case pteWrite: dprintf("write"); break; - case pteExec: dprintf("exec"); break; - case pteUser: dprintf("user"); break; - case pteGlobal: dprintf("global"); break; - case pteAccessed: dprintf("accessed"); break; - case pteDirty: dprintf("dirty"); break; - default: dprintf("%" B_PRIu32, i); + case 0: dprintf("valid"); break; + case 1: dprintf("read"); break; + case 2: dprintf("write"); break; + case 3: dprintf("exec"); break; + case 4: dprintf("user"); break; + case 5: dprintf("global"); break; + case 6: dprintf("accessed"); break; + case 7: dprintf("dirty"); break; + default: dprintf("%" B_PRIu32, i); } } } @@ -139,14 +139,19 @@ LookupPte(addr_t virtAdr, bool alloc) Pte *pte = (Pte*)VirtFromPhys(sPageTable); for (int level = 2; level > 0; level--) { pte += VirtAdrPte(virtAdr, level); - if (!((1 << pteValid) & pte->flags)) { + if (!pte->isValid) { if (!alloc) return NULL; - pte->ppn = AllocPhysPage() / B_PAGE_SIZE; - if (pte->ppn == 0) + uint64 ppn = AllocPhysPage() / B_PAGE_SIZE; + if (ppn == 0) return NULL; - memset((Pte*)VirtFromPhys(B_PAGE_SIZE * pte->ppn), 0, B_PAGE_SIZE); - pte->flags |= (1 << pteValid) | (IS_KERNEL_ADDRESS(virtAdr) ? (1 << pteGlobal) : 0); + memset((Pte*)VirtFromPhys(B_PAGE_SIZE * ppn), 0, B_PAGE_SIZE); + Pte newPte { + .isValid = true, + .isGlobal = IS_KERNEL_ADDRESS(virtAdr), + .ppn = ppn + }; + pte->val = newPte.val; } pte = (Pte*)VirtFromPhys(B_PAGE_SIZE * pte->ppn); } @@ -163,10 +168,16 @@ Map(addr_t virtAdr, phys_addr_t physAdr, uint64 flags) if (pte == NULL) panic("can't allocate page table"); - pte->ppn = physAdr / B_PAGE_SIZE; - pte->flags = (1 << pteValid) | (1 << pteAccessed) | (1 << pteDirty) - | (IS_KERNEL_ADDRESS(virtAdr) ? (1 << pteGlobal) : 0) - | flags; + Pte newPte { + .isValid = true, + .isGlobal = IS_KERNEL_ADDRESS(virtAdr), + .isAccessed = true, + .isDirty = true, + .ppn = physAdr / B_PAGE_SIZE + }; + newPte.val |= flags; + + pte->val = newPte.val; } @@ -215,10 +226,15 @@ PreallocKernelRange() for (uint64 i = VirtAdrPte(KERNEL_BASE, 2); i <= VirtAdrPte(KERNEL_TOP, 2); i++) { Pte* pte = &root[i]; - pte->ppn = AllocPhysPage() / B_PAGE_SIZE; - if (pte->ppn == 0) panic("can't alloc early physical page"); + uint64 ppn = AllocPhysPage() / B_PAGE_SIZE; + if (ppn == 0) panic("can't alloc early physical page"); memset(VirtFromPhys(B_PAGE_SIZE * pte->ppn), 0, B_PAGE_SIZE); - pte->flags |= (1 << pteValid) | (1 << pteGlobal); + Pte newPte { + .isValid = true, + .isGlobal = true, + .ppn = ppn + }; + pte->val = newPte.val; } } @@ -239,32 +255,30 @@ SetupPageTable() MapRange(gKernelArgs.arch_args.physMap.start, gKernelArgs.physical_memory_range[0].start, gKernelArgs.arch_args.physMap.size, - (1 << pteRead) | (1 << pteWrite)); + Pte {.isRead = true, .isWrite = true}.val); // Boot loader MapRangeIdentity((addr_t)gMemBase, &gStackEnd - gMemBase, - (1 << pteRead) | (1 << pteWrite) | (1 << pteExec)); + Pte {.isRead = true, .isWrite = true, .isExec = true}.val); // Memory regions MemoryRegion* region; for (region = sRegions; region != NULL; region = region->next) { - uint64 flags = 0; - if ((region->protection & B_READ_AREA) != 0) - flags |= (1 << pteRead); - if ((region->protection & B_WRITE_AREA) != 0) - flags |= (1 << pteWrite); - if ((region->protection & B_EXECUTE_AREA) != 0) - flags |= (1 << pteExec); - MapRange(region->virtAdr, region->physAdr, region->size, flags); + Pte flags { + .isRead = (region->protection & B_READ_AREA) != 0, + .isWrite = (region->protection & B_WRITE_AREA) != 0, + .isExec = (region->protection & B_EXECUTE_AREA) != 0 + }; + MapRange(region->virtAdr, region->physAdr, region->size, flags.val); } // Devices - MapAddrRange(gKernelArgs.arch_args.clint, (1 << pteRead) | (1 << pteWrite)); - MapAddrRange(gKernelArgs.arch_args.htif, (1 << pteRead) | (1 << pteWrite)); - MapAddrRange(gKernelArgs.arch_args.plic, (1 << pteRead) | (1 << pteWrite)); + MapAddrRange(gKernelArgs.arch_args.clint, Pte {.isRead = true, .isWrite = true}.val); + MapAddrRange(gKernelArgs.arch_args.htif, Pte {.isRead = true, .isWrite = true}.val); + MapAddrRange(gKernelArgs.arch_args.plic, Pte {.isRead = true, .isWrite = true}.val); if (strcmp(gKernelArgs.arch_args.uart.kind, "") != 0) { MapAddrRange(gKernelArgs.arch_args.uart.regs, - (1 << pteRead) | (1 << pteWrite)); + Pte {.isRead = true, .isWrite = true}.val); } } diff --git a/src/system/kernel/arch/riscv64/RISCV64VMTranslationMap.cpp b/src/system/kernel/arch/riscv64/RISCV64VMTranslationMap.cpp index 4a01174ca0..e9e86328ed 100644 --- a/src/system/kernel/arch/riscv64/RISCV64VMTranslationMap.cpp +++ b/src/system/kernel/arch/riscv64/RISCV64VMTranslationMap.cpp @@ -122,7 +122,7 @@ FreePageTable(page_num_t ppn, bool isKernel, uint32 level = 2) end = VirtAdrPte(USER_TOP, 2); } for (uint64 i = beg; i <= end; i++) { - if ((1 << pteValid) & pte[i].flags) + if (pte[i].isValid) FreePageTable(pte[i].ppn, isKernel, level - 1); } } @@ -150,7 +150,7 @@ GetPageTableSize(page_num_t ppn, bool isKernel, uint32 level = 2) end = VirtAdrPte(USER_TOP, 2); } for (uint64 i = beg; i <= end; i++) { - if ((1 << pteValid) & pte[i].flags) + if (pte[i].isValid) size += GetPageTableSize(pte[i].ppn, isKernel, level - 1); } return size; @@ -160,7 +160,7 @@ GetPageTableSize(page_num_t ppn, bool isKernel, uint32 level = 2) //#pragma mark RISCV64VMTranslationMap -Pte* +std::atomic* RISCV64VMTranslationMap::LookupPte(addr_t virtAdr, bool alloc, vm_page_reservation* reservation) { @@ -185,26 +185,31 @@ RISCV64VMTranslationMap::LookupPte(addr_t virtAdr, bool alloc, i <= VirtAdrPte(KERNEL_TOP, 2); i++) { Pte *pte = &userPageTable[i]; pte->ppn = kernelPageTable[i].ppn; - pte->flags |= (1 << pteValid); + pte->isValid = true; } } } - Pte *pte = (Pte*)VirtFromPhys(fPageTable); + auto pte = (std::atomic*)VirtFromPhys(fPageTable); for (int level = 2; level > 0; level--) { pte += VirtAdrPte(virtAdr, level); - if (!((1 << pteValid) & pte->flags)) { + if (!pte->load().isValid) { if (!alloc) return NULL; vm_page* page = vm_page_allocate_page(reservation, PAGE_STATE_WIRED | VM_PAGE_ALLOC_CLEAR); - pte->ppn = page->physical_page_number; - if (pte->ppn == 0) + page_num_t ppn = page->physical_page_number; + if (ppn == 0) return NULL; DEBUG_PAGE_ACCESS_END(page); fPageTableSize++; - pte->flags |= (1 << pteValid) | (fIsKernel ? (1 << pteGlobal) : 0); + Pte newPte { + .isValid = true, + .isGlobal = fIsKernel, + .ppn = ppn + }; + pte->store(newPte); } - pte = (Pte*)VirtFromPhys(B_PAGE_SIZE * pte->ppn); + pte = (std::atomic*)VirtFromPhys(B_PAGE_SIZE * pte->load().ppn); } pte += VirtAdrPte(virtAdr, 0); return pte; @@ -214,12 +219,15 @@ RISCV64VMTranslationMap::LookupPte(addr_t virtAdr, bool alloc, phys_addr_t RISCV64VMTranslationMap::LookupAddr(addr_t virtAdr) { - Pte* pte = LookupPte(virtAdr, false, NULL); - if (pte == NULL || !((1 << pteValid) & pte->flags)) + std::atomic* pte = LookupPte(virtAdr, false, NULL); + if (pte == NULL) return 0; - if (fIsKernel != (((1 << pteUser) & pte->flags) == 0)) + Pte pteVal = pte->load(); + if (!pteVal.isValid) return 0; - return pte->ppn * B_PAGE_SIZE; + if (fIsKernel != !pteVal.isUser) + return 0; + return pteVal.ppn * B_PAGE_SIZE; } @@ -313,36 +321,38 @@ RISCV64VMTranslationMap::Map(addr_t virtualAddress, phys_addr_t physicalAddress, ThreadCPUPinner pinner(thread_get_current_thread()); - Pte* pte = LookupPte(virtualAddress, true, reservation); + std::atomic* pte = LookupPte(virtualAddress, true, reservation); if (pte == NULL) panic("can't allocate page table"); - Pte newPte; - newPte.ppn = physicalAddress / B_PAGE_SIZE; - newPte.flags = (1 << pteValid) | (fIsKernel ? (1 << pteGlobal) : 0); + Pte newPte { + .isValid = true, + .isGlobal = fIsKernel, + .ppn = physicalAddress / B_PAGE_SIZE + }; if ((attributes & B_USER_PROTECTION) != 0) { - newPte.flags |= (1 << pteUser); + newPte.isUser = true; if ((attributes & B_READ_AREA) != 0) - newPte.flags |= (1 << pteRead); + newPte.isRead = true; if ((attributes & B_WRITE_AREA) != 0) - newPte.flags |= (1 << pteWrite); + newPte.isWrite = true; if ((attributes & B_EXECUTE_AREA) != 0) { - newPte.flags |= (1 << pteExec); + newPte.isExec = true; fInvalidCode = true; } } else { if ((attributes & B_KERNEL_READ_AREA) != 0) - newPte.flags |= (1 << pteRead); + newPte.isRead = true; if ((attributes & B_KERNEL_WRITE_AREA) != 0) - newPte.flags |= (1 << pteWrite); + newPte.isWrite = true; if ((attributes & B_KERNEL_EXECUTE_AREA) != 0) { - newPte.flags |= (1 << pteExec); + newPte.isExec = true; fInvalidCode = true; } } - *pte = newPte; + pte->store(newPte); // Note: We don't need to invalidate the TLB for this address, as previously // the entry was not present and the TLB doesn't cache those entries. @@ -362,11 +372,11 @@ RISCV64VMTranslationMap::Unmap(addr_t start, addr_t end) ThreadCPUPinner pinner(thread_get_current_thread()); for (addr_t page = start; page < end; page += B_PAGE_SIZE) { - Pte* pte = LookupPte(page, false, NULL); + std::atomic* pte = LookupPte(page, false, NULL); if (pte != NULL) { fMapCount--; - Pte oldPte{.val = (uint64)atomic_get_and_set64((int64*)&pte->val, 0)}; - if ((oldPte.flags & (1 << pteAccessed)) != 0) + Pte oldPte = pte->exchange({}); + if (oldPte.isAccessed) InvalidatePage(page); } } @@ -406,24 +416,23 @@ RISCV64VMTranslationMap::UnmapPage(VMArea* area, addr_t address, ThreadCPUPinner pinner(thread_get_current_thread()); - Pte* pte = LookupPte(address, false, NULL); - if (pte == NULL || ((1 << pteValid) & pte->flags) == 0) + std::atomic* pte = LookupPte(address, false, NULL); + if (pte == NULL || !pte->load().isValid) return B_ENTRY_NOT_FOUND; RecursiveLocker locker(fLock); - Pte oldPte{.val = (uint64)atomic_get_and_set64((int64*)&pte->val, 0)}; + Pte oldPte = pte->exchange({}); fMapCount--; pinner.Unlock(); - if ((oldPte.flags & (1 << pteAccessed)) != 0) + if (oldPte.isAccessed) InvalidatePage(address); Flush(); locker.Detach(); // PageUnmapped takes ownership - PageUnmapped(area, oldPte.ppn, ((1 << pteAccessed) & oldPte.flags) != 0, - ((1 << pteDirty) & oldPte.flags) != 0, updatePageQueue); + PageUnmapped(area, oldPte.ppn, oldPte.isAccessed, oldPte.isDirty, updatePageQueue); return B_OK; } @@ -446,17 +455,17 @@ RISCV64VMTranslationMap::UnmapPages(VMArea* area, addr_t base, size_t size, ThreadCPUPinner pinner(thread_get_current_thread()); for (addr_t start = base; start < end; start += B_PAGE_SIZE) { - Pte* pte = LookupPte(start, false, NULL); + std::atomic* pte = LookupPte(start, false, NULL); if (pte == NULL) continue; - Pte oldPte{.val = (uint64)atomic_get_and_set64((int64*)&pte->val, 0)}; - if ((oldPte.flags & (1 << pteValid)) == 0) + Pte oldPte = pte->exchange({}); + if (!oldPte.isValid) continue; fMapCount--; - if ((oldPte.flags & (1 << pteAccessed)) != 0) + if (oldPte.isAccessed) InvalidatePage(start); if (area->cache_type != CACHE_TYPE_DEVICE) { @@ -470,10 +479,8 @@ RISCV64VMTranslationMap::UnmapPages(VMArea* area, addr_t base, size_t size, DEBUG_PAGE_ACCESS_START(page); // transfer the accessed/dirty flags to the page - if ((oldPte.flags & (1 << pteAccessed)) != 0) - page->accessed = true; - if ((oldPte.flags & (1 << pteDirty)) != 0) - page->modified = true; + page->accessed = oldPte.isAccessed; + page->modified = oldPte.isDirty; // remove the mapping object/decrement the wired_count of the // page @@ -572,27 +579,26 @@ RISCV64VMTranslationMap::UnmapArea(VMArea* area, bool deletingAddressSpace, + ((page->cache_offset * B_PAGE_SIZE) - area->cache_offset); - Pte* pte = LookupPte(address, false, NULL); - if (pte == NULL - || ((1 << pteValid) & pte->flags) == 0) { + std::atomic* pte = LookupPte(address, false, NULL); + if (pte == NULL || !pte->load().isValid) { panic("page %p has mapping for area %p " "(%#" B_PRIxADDR "), but has no " "page table", page, area, address); continue; } - Pte oldPte{.val = (uint64)atomic_get_and_set64((int64*)&pte->val, 0)}; + Pte oldPte = pte->exchange({}); // transfer the accessed/dirty flags to the page and // invalidate the mapping, if necessary - if (((1 << pteAccessed) & oldPte.flags) != 0) { + if (oldPte.isAccessed) { page->accessed = true; if (!deletingAddressSpace) InvalidatePage(address); } - if (((1 << pteDirty) & oldPte.flags) != 0) + if (oldPte.isDirty) page->modified = true; if (pageFullyUnmapped) { @@ -642,32 +648,32 @@ RISCV64VMTranslationMap::Query(addr_t virtualAddress, if (fPageTable == 0) return B_OK; - Pte* pte = LookupPte(virtualAddress, false, NULL); - if (pte == 0) + std::atomic* pte = LookupPte(virtualAddress, false, NULL); + if (pte == NULL) return B_OK; - Pte pteVal = *pte; + Pte pteVal = pte->load(); *_physicalAddress = pteVal.ppn * B_PAGE_SIZE; - if (((1 << pteValid) & pteVal.flags) != 0) + if (pteVal.isValid) *_flags |= PAGE_PRESENT; - if (((1 << pteDirty) & pteVal.flags) != 0) + if (pteVal.isDirty) *_flags |= PAGE_MODIFIED; - if (((1 << pteAccessed) & pteVal.flags) != 0) + if (pteVal.isAccessed) *_flags |= PAGE_ACCESSED; - if (((1 << pteUser) & pteVal.flags) != 0) { - if (((1 << pteRead) & pteVal.flags) != 0) + if (pteVal.isUser) { + if (pteVal.isRead) *_flags |= B_READ_AREA; - if (((1 << pteWrite) & pteVal.flags) != 0) + if (pteVal.isWrite) *_flags |= B_WRITE_AREA; - if (((1 << pteExec) & pteVal.flags) != 0) + if (pteVal.isExec) *_flags |= B_EXECUTE_AREA; } else { - if (((1 << pteRead) & pteVal.flags) != 0) + if (pteVal.isRead) *_flags |= B_KERNEL_READ_AREA; - if (((1 << pteWrite) & pteVal.flags) != 0) + if (pteVal.isWrite) *_flags |= B_KERNEL_WRITE_AREA; - if (((1 << pteExec) & pteVal.flags) != 0) + if (pteVal.isExec) *_flags |= B_KERNEL_EXECUTE_AREA; } @@ -693,41 +699,38 @@ status_t RISCV64VMTranslationMap::Protect(addr_t base, addr_t top, for (addr_t page = base; page < top; page += B_PAGE_SIZE) { - Pte* pte = LookupPte(page, false, NULL); - if (pte == NULL || ((1 << pteValid) & pte->flags) == 0) { + std::atomic* pte = LookupPte(page, false, NULL); + if (pte == NULL || !pte->load().isValid) { TRACE("attempt to protect not mapped page: 0x%" B_PRIxADDR "\n", page); continue; } - Pte oldPte = *pte; - Pte newPte = oldPte; - newPte.flags &= (1 << pteValid) | (1 << pteGlobal) - | (1 << pteAccessed) | (1 << pteDirty); + Pte oldPte {}; + Pte newPte {}; + while (true) { + oldPte = pte->load(); - if ((attributes & B_USER_PROTECTION) != 0) { - newPte.flags |= (1 << pteUser); - if ((attributes & B_READ_AREA) != 0) - newPte.flags |= (1 << pteRead); - if ((attributes & B_WRITE_AREA) != 0) - newPte.flags |= (1 << pteWrite); - if ((attributes & B_EXECUTE_AREA) != 0) { - newPte.flags |= (1 << pteExec); - fInvalidCode = true; - } - } else { - if ((attributes & B_KERNEL_READ_AREA) != 0) - newPte.flags |= (1 << pteRead); - if ((attributes & B_KERNEL_WRITE_AREA) != 0) - newPte.flags |= (1 << pteWrite); - if ((attributes & B_KERNEL_EXECUTE_AREA) != 0) { - newPte.flags |= (1 << pteExec); - fInvalidCode = true; + newPte = oldPte; + if ((attributes & B_USER_PROTECTION) != 0) { + newPte.isUser = true; + newPte.isRead = (attributes & B_READ_AREA) != 0; + newPte.isWrite = (attributes & B_WRITE_AREA) != 0; + newPte.isExec = (attributes & B_EXECUTE_AREA) != 0; + } else { + newPte.isUser = false; + newPte.isRead = (attributes & B_KERNEL_READ_AREA) != 0; + newPte.isWrite = (attributes & B_KERNEL_WRITE_AREA) != 0; + newPte.isExec = (attributes & B_KERNEL_EXECUTE_AREA) != 0; } + + if (pte->compare_exchange_strong(oldPte, newPte)) + break; } - *pte = newPte; - if ((oldPte.flags & (1 << pteAccessed)) != 0) + fInvalidCode = newPte.isExec; + + if (oldPte.isAccessed) InvalidatePage(page); } @@ -752,11 +755,14 @@ RISCV64VMTranslationMap::ProtectArea(VMArea* area, uint32 attributes) } -static inline uint32 +static inline uint64 ConvertAccessedFlags(uint32 flags) { - return ((flags & PAGE_MODIFIED) ? (1 << pteDirty ) : 0) - | ((flags & PAGE_ACCESSED) ? (1 << pteAccessed) : 0); + Pte pteFlags { + .isAccessed = (flags & PAGE_ACCESSED) != 0, + .isDirty = (flags & PAGE_MODIFIED) != 0 + }; + return pteFlags.val; } @@ -766,11 +772,11 @@ RISCV64VMTranslationMap::SetFlags(addr_t address, uint32 flags) // Only called from interrupt handler with interrupts disabled for CPUs that don't support // setting accessed/modified flags by hardware. - Pte* pte = LookupPte(address, false, NULL); - if (pte == NULL || ((1 << pteValid) & pte->flags) == 0) + std::atomic* pte = LookupPte(address, false, NULL); + if (pte == NULL || !pte->load().isValid) return; - pte->flags |= ConvertAccessedFlags(flags); + *(std::atomic*)pte |= ConvertAccessedFlags(flags); if (IS_KERNEL_ADDRESS(address)) FlushTlbPage(address); @@ -786,11 +792,11 @@ RISCV64VMTranslationMap::ClearFlags(addr_t address, uint32 flags) { ThreadCPUPinner pinner(thread_get_current_thread()); - Pte* pte = LookupPte(address, false, NULL); - if (pte == NULL || ((1 << pteValid) & pte->flags) == 0) + std::atomic* pte = LookupPte(address, false, NULL); + if (pte == NULL || !pte->load().isValid) return B_OK; - pte->flags &= ~ConvertAccessedFlags(flags); + *(std::atomic*)pte &= ~ConvertAccessedFlags(flags); InvalidatePage(address); return B_OK; } @@ -807,35 +813,33 @@ RISCV64VMTranslationMap::ClearAccessedAndModified(VMArea* area, addr_t address, RecursiveLocker locker(fLock); ThreadCPUPinner pinner(thread_get_current_thread()); - Pte* pte = LookupPte(address, false, NULL); - if (pte == NULL || ((1 << pteValid) & pte->flags) == 0) + std::atomic* pte = LookupPte(address, false, NULL); + if (pte == NULL || !pte->load().isValid) return false; - Pte oldPte; + Pte oldPte {}; if (unmapIfUnaccessed) { for (;;) { - oldPte = *pte; - if (((1 << pteValid) & oldPte.flags) == 0) + oldPte = pte->load(); + if (!oldPte.isValid) return false; - if (((1 << pteAccessed) & oldPte.flags) != 0) { - oldPte.val = atomic_and64((int64*)&pte->val, - ~((1 << pteAccessed) | (1 << pteDirty))); + if (oldPte.isAccessed) { + oldPte.val = ((std::atomic*)pte)->fetch_and( + ~Pte {.isAccessed = true, .isDirty = true}.val); break; } - if (atomic_test_and_set64((int64*)&pte->val, 0, oldPte.val) - == (int64)oldPte.val) { + if (pte->compare_exchange_strong(oldPte, {})) break; - } } } else { - oldPte.val = atomic_and64((int64*)&pte->val, - ~((1 << pteAccessed) | (1 << pteDirty))); + oldPte.val = ((std::atomic*)pte)->fetch_and( + ~Pte {.isAccessed = true, .isDirty = true}.val); } pinner.Unlock(); - _modified = ((1 << pteDirty) & oldPte.flags) != 0; - if (((1 << pteAccessed) & oldPte.flags) != 0) { + _modified = oldPte.isDirty; + if (oldPte.isAccessed) { InvalidatePage(address); Flush(); return true; diff --git a/src/system/kernel/arch/riscv64/RISCV64VMTranslationMap.h b/src/system/kernel/arch/riscv64/RISCV64VMTranslationMap.h index df9e02ba60..56477f7d82 100644 --- a/src/system/kernel/arch/riscv64/RISCV64VMTranslationMap.h +++ b/src/system/kernel/arch/riscv64/RISCV64VMTranslationMap.h @@ -9,6 +9,8 @@ #define _RISCV64VMTRANSLATIONMAP_H_ +#include + #include #include #include @@ -97,7 +99,7 @@ struct RISCV64VMTranslationMap: public VMTranslationMap { inline void InvalidatePage(addr_t address); private: - Pte* LookupPte(addr_t virtAdr, bool alloc, + std::atomic* LookupPte(addr_t virtAdr, bool alloc, vm_page_reservation* reservation); phys_addr_t LookupAddr(addr_t virtAdr); diff --git a/src/system/kernel/arch/riscv64/arch_vm.cpp b/src/system/kernel/arch/riscv64/arch_vm.cpp index 87b58be985..a93dd0b24d 100644 --- a/src/system/kernel/arch/riscv64/arch_vm.cpp +++ b/src/system/kernel/arch/riscv64/arch_vm.cpp @@ -36,7 +36,7 @@ LookupPte(phys_addr_t pageTable, addr_t virtAdr) Pte *pte = (Pte*)VirtFromPhys(pageTable); for (int level = 2; level > 0; level --) { pte += VirtAdrPte(virtAdr, level); - if (!((1 << pteValid) & pte->flags)) { + if (!pte->isValid) { return NULL; } // TODO: Handle superpages (RWX=0 when not at lowest level) @@ -61,28 +61,28 @@ WritePteFlags(uint32 flags) dprintf(", "); switch (i) { - case pteValid: + case 0: dprintf("valid"); break; - case pteRead: + case 1: dprintf("read"); break; - case pteWrite: + case 2: dprintf("write"); break; - case pteExec: + case 3: dprintf("exec"); break; - case pteUser: + case 4: dprintf("user"); break; - case pteGlobal: + case 5: dprintf("global"); break; - case pteAccessed: + case 6: dprintf("accessed"); break; - case pteDirty: + case 7: dprintf("dirty"); break; default: @@ -140,9 +140,8 @@ static void DumpPageTableInt(Pte* pte, uint64_t virtAdr, uint32_t level, PageTableDumper& dumper) { for (uint32 i = 0; i < pteCount; i++) { - if (((1 << pteValid) & pte[i].flags) != 0) { - if ((((1 << pteRead) | (1 << pteWrite) - | (1 << pteExec)) & pte[i].flags) == 0) { + if (pte[i].isValid) { + if (!pte[i].isRead && !pte[i].isWrite && !pte[i].isExec) { if (level == 0) kprintf(" internal page table on level 0\n"); @@ -154,7 +153,7 @@ DumpPageTableInt(Pte* pte, uint64_t virtAdr, uint32_t level, PageTableDumper& du dumper.Write(SignExtendVirtAdr(virtAdr + ((uint64_t)i << (pageBits + pteIdxBits*level))), pte[i].ppn * B_PAGE_SIZE, 1 << (pageBits + pteIdxBits * level), - pte[i].flags); + pte[i].val & 0xff); } } } @@ -215,10 +214,10 @@ DumpPageTable(int argc, char** argv) } else { for (; size > 0; base += B_PAGE_SIZE, size -= B_PAGE_SIZE) { Pte* pte = LookupPte(satp.ppn * B_PAGE_SIZE, base); - if (pte == NULL || (pte->flags & (1 << pteValid)) == 0) + if (pte == NULL || !pte->isValid) continue; - dumper.Write(base, pte->ppn * B_PAGE_SIZE, B_PAGE_SIZE, pte->flags); + dumper.Write(base, pte->ppn * B_PAGE_SIZE, B_PAGE_SIZE, pte->val & 0xff); } } @@ -265,7 +264,7 @@ DumpVirtPage(int argc, char** argv) } PageTableDumper dumper; - dumper.Write(virt, pte->ppn * B_PAGE_SIZE, B_PAGE_SIZE, pte->flags); + dumper.Write(virt, pte->ppn * B_PAGE_SIZE, B_PAGE_SIZE, pte->val & 0xff); return 0; } diff --git a/src/system/kernel/arch/riscv64/arch_vm_translation_map.cpp b/src/system/kernel/arch/riscv64/arch_vm_translation_map.cpp index bf4bc0d952..165571d386 100644 --- a/src/system/kernel/arch/riscv64/arch_vm_translation_map.cpp +++ b/src/system/kernel/arch/riscv64/arch_vm_translation_map.cpp @@ -48,14 +48,19 @@ LookupPte(addr_t virtAdr, bool alloc, kernel_args* args, Pte *pte = (Pte*)VirtFromPhys(sPageTable); for (int level = 2; level > 0; level --) { pte += VirtAdrPte(virtAdr, level); - if (!((1 << pteValid) & pte->flags)) { + if (!pte->isValid) { if (!alloc) return NULL; - pte->ppn = get_free_page(args); - if (pte->ppn == 0) + page_num_t ppn = get_free_page(args); + if (ppn == 0) return NULL; - memset((Pte*)VirtFromPhys(B_PAGE_SIZE * pte->ppn), 0, B_PAGE_SIZE); - pte->flags |= (1 << pteValid) | (1 << pteGlobal); + memset((Pte*)VirtFromPhys(B_PAGE_SIZE * ppn), 0, B_PAGE_SIZE); + Pte newPte { + .isValid = true, + .isGlobal = true, + .ppn = ppn + }; + pte->val = newPte.val; } pte = (Pte*)VirtFromPhys(B_PAGE_SIZE * pte->ppn); } @@ -72,10 +77,16 @@ Map(addr_t virtAdr, phys_addr_t physAdr, uint64 flags, kernel_args* args, Pte* pte = LookupPte(virtAdr, true, args, get_free_page); if (pte == NULL) panic("can't allocate page table"); - pte->ppn = physAdr / B_PAGE_SIZE; - pte->flags = (1 << pteValid) | (1 << pteAccessed) | (1 << pteDirty) - | (1 << pteGlobal) // we map only kernel pages here so always set global flag - | flags; + Pte newPte { + .isValid = true, + .isGlobal = true, // we map only kernel pages here so always set global flag + .isAccessed = true, + .isDirty = true, + .ppn = physAdr / B_PAGE_SIZE + }; + newPte.val |= flags; + + pte->val = newPte.val; FlushTlbPage(virtAdr); } @@ -155,14 +166,12 @@ arch_vm_translation_map_early_map(kernel_args *args, phys_addr_t (*get_free_page)(kernel_args *)) { //dprintf("early_map(%#" B_PRIxADDR ", %#" B_PRIxADDR ")\n", virtAdr, physAdr); - uint64 flags = 0; - if ((attributes & B_KERNEL_READ_AREA) != 0) - flags |= (1 << pteRead); - if ((attributes & B_KERNEL_WRITE_AREA) != 0) - flags |= (1 << pteWrite); - if ((attributes & B_KERNEL_EXECUTE_AREA) != 0) - flags |= (1 << pteExec); - Map(virtAdr, physAdr, flags, args, get_free_page); + Pte flags { + .isRead = (attributes & B_KERNEL_READ_AREA) != 0, + .isWrite = (attributes & B_KERNEL_WRITE_AREA) != 0, + .isExec = (attributes & B_KERNEL_EXECUTE_AREA) != 0, + }; + Map(virtAdr, physAdr, flags.val, args, get_free_page); return B_OK; }