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 <[email protected]>

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;
 };

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,

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);
 	}
 }
 

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<Pte>*
 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<Pte>*)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<Pte>*)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>* 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>* 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>* 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>* 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>* 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>* 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>* 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>* 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>* pte = LookupPte(address, false, NULL);
+	if (pte == NULL || !pte->load().isValid)
 		return;
 
-	pte->flags |= ConvertAccessedFlags(flags);
+	*(std::atomic<uint64>*)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>* pte = LookupPte(address, false, NULL);
+	if (pte == NULL || !pte->load().isValid)
 		return B_OK;
 
-	pte->flags &= ~ConvertAccessedFlags(flags);
+	*(std::atomic<uint64>*)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>* 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<uint64>*)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<uint64>*)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;

src/system/kernel/arch/riscv64/RISCV64VMTranslationMap.h

@@ -9,6 +9,8 @@
 #define _RISCV64VMTRANSLATIONMAP_H_
 
 
+#include <atomic>
+
 #include <vm/VMTranslationMap.h>
 #include <arch_cpu_defs.h>
 #include <kernel/smp.h>
@@ -97,7 +99,7 @@ struct RISCV64VMTranslationMap: public VMTranslationMap {
 	inline	void				InvalidatePage(addr_t address);
 
 private:
-			Pte*				LookupPte(addr_t virtAdr, bool alloc,
+			std::atomic<Pte>*	LookupPte(addr_t virtAdr, bool alloc,
 									vm_page_reservation* reservation);
 			phys_addr_t			LookupAddr(addr_t virtAdr);
 

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;
 }

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;
 }