Refactor Check and CheckRegion functions in virtual.hpp

TODO.md

@@ -47,5 +47,5 @@
 	- Create a separate list for processes that are waiting for a resource or a signal, etc.
 	- Use all cores to schedule threads.
 
-- [ ] Improve Remap() function.
+- [x] Improve Remap() function.
 	- Remove Unmap & Map logic. Remove all flags directly.

arch/amd64/memory/vmm.cpp

@@ -22,54 +22,72 @@
 
 namespace Memory
 {
-	bool Virtual::Check(void *VirtualAddress, PTFlag Flag, MapType Type)
+	bool Virtual::Check(void *VirtualAddress, PTFlag Flag)
 	{
-		// 0x1000 aligned
 		uintptr_t Address = (uintptr_t)VirtualAddress;
 		Address &= 0xFFFFFFFFFFFFF000;
 
 		PageMapIndexer Index = PageMapIndexer(Address);
-		PageMapLevel4 *PML4 = &this->pTable->Entries[Index.PMLIndex];
 
 		PageDirectoryPointerTableEntryPtr *PDPTE = nullptr;
 		PageDirectoryEntryPtr *PDE = nullptr;
 		PageTableEntryPtr *PTE = nullptr;
 
-		if ((PML4->raw & Flag) > 0)
+		PageMapLevel4 *PML4 = &this->pTable->Entries[Index.PMLIndex];
+		if (!PML4->Present)
 		{
+			debug("PML4 not present for %#lx", VirtualAddress);
+			return false;
+		}
+
 		PDPTE = (PageDirectoryPointerTableEntryPtr *)((uintptr_t)PML4->GetAddress() << 12);
-			if (PDPTE)
+		if (!PDPTE)
 		{
-				if ((PDPTE->Entries[Index.PDPTEIndex].Present))
+			debug("Failed to get PDPTE for %#lx", VirtualAddress);
+			return false;
+		}
+
+		if (PDPTE->Entries[Index.PDPTEIndex].PageSize)
 		{
-					if (Type == MapType::OneGiB && PDPTE->Entries[Index.PDPTEIndex].PageSize)
+			bool result = PDPTE->Entries[Index.PDPTEIndex].raw & Flag;
-						return true;
+			if (!result)
+				debug("Failed to check %#lx for %#lx (raw: %#lx)", VirtualAddress, Flag,
+					  PDPTE->Entries[Index.PDPTEIndex].raw);
+			return result;
+		}
 
 		PDE = (PageDirectoryEntryPtr *)((uintptr_t)PDPTE->Entries[Index.PDPTEIndex].GetAddress() << 12);
-					if (PDE)
+		if (!PDE)
 		{
-						if (Type == MapType::TwoMiB && PDE->Entries[Index.PDEIndex].PageSize)
+			debug("Failed to get PDE for %#lx", VirtualAddress);
-							return true;
-
-						if ((PDE->Entries[Index.PDEIndex].Present))
-						{
-							PTE = (PageTableEntryPtr *)((uintptr_t)PDE->Entries[Index.PDEIndex].GetAddress() << 12);
-							if (PTE)
-							{
-								if ((PTE->Entries[Index.PTEIndex].Present))
-									return true;
-							}
-						}
-					}
-				}
-			}
-		}
 			return false;
 		}
 
+		if (PDE->Entries[Index.PDEIndex].PageSize)
+		{
+			bool result = PDE->Entries[Index.PDEIndex].raw & Flag;
+			if (!result)
+				debug("Failed to check %#lx for %#lx (raw: %#lx)", VirtualAddress, Flag,
+					  PDE->Entries[Index.PDEIndex].raw);
+			return result;
+		}
+
+		PTE = (PageTableEntryPtr *)((uintptr_t)PDE->Entries[Index.PDEIndex].GetAddress() << 12);
+		if (!PTE)
+		{
+			debug("Failed to get PTE for %#lx", VirtualAddress);
+			return false;
+		}
+
+		bool result = PTE->Entries[Index.PTEIndex].raw & Flag;
+		if (!result)
+			debug("Failed to check %#lx for %#lx (raw: %#lx)", VirtualAddress, Flag,
+				  PTE->Entries[Index.PTEIndex].raw);
+		return result;
+	}
+
 	void *Virtual::GetPhysical(void *VirtualAddress)
 	{
-		// 0x1000 aligned
 		uintptr_t Address = (uintptr_t)VirtualAddress;
 		Address &= 0xFFFFFFFFFFFFF000;
 
@@ -114,46 +132,41 @@ namespace Memory
 
 	Virtual::MapType Virtual::GetMapType(void *VirtualAddress)
 	{
-		// 0x1000 aligned
 		uintptr_t Address = (uintptr_t)VirtualAddress;
 		Address &= 0xFFFFFFFFFFFFF000;
 
 		PageMapIndexer Index = PageMapIndexer(Address);
-		PageMapLevel4 *PML4 = &this->pTable->Entries[Index.PMLIndex];
 
 		PageDirectoryPointerTableEntryPtr *PDPTE = nullptr;
 		PageDirectoryEntryPtr *PDE = nullptr;
 		PageTableEntryPtr *PTE = nullptr;
 
-		if (PML4->Present)
+		PageMapLevel4 *PML4 = &this->pTable->Entries[Index.PMLIndex];
-		{
+		if (!PML4->Present)
+			goto ReturnError;
+
 		PDPTE = (PageDirectoryPointerTableEntryPtr *)((uintptr_t)PML4->GetAddress() << 12);
-			if (PDPTE)
+		if (!PDPTE || !PDPTE->Entries[Index.PDPTEIndex].Present)
-			{
+			goto ReturnError;
-				if (PDPTE->Entries[Index.PDPTEIndex].Present)
+
-				{
 		if (PDPTE->Entries[Index.PDPTEIndex].PageSize)
 			return MapType::OneGiB;
 
 		PDE = (PageDirectoryEntryPtr *)((uintptr_t)PDPTE->Entries[Index.PDPTEIndex].GetAddress() << 12);
-					if (PDE)
+		if (!PDE || !PDE->Entries[Index.PDEIndex].Present)
-					{
+			goto ReturnError;
-						if (PDE->Entries[Index.PDEIndex].Present)
+
-						{
 		if (PDE->Entries[Index.PDEIndex].PageSize)
 			return MapType::TwoMiB;
 
 		PTE = (PageTableEntryPtr *)((uintptr_t)PDE->Entries[Index.PDEIndex].GetAddress() << 12);
-							if (PTE)
+		if (!PTE)
-							{
+			goto ReturnError;
+
 		if (PTE->Entries[Index.PTEIndex].Present)
 			return MapType::FourKiB;
-							}
+
-						}
+	ReturnError:
-					}
-				}
-			}
-		}
 		return MapType::NoMapType;
 	}
 
@@ -364,7 +377,7 @@ namespace Memory
 		(byte & 0x02 ? '1' : '0'), \
 		(byte & 0x01 ? '1' : '0')
 
-		if (!this->Check(VirtualAddress, (PTFlag)Flags, Type)) // quick workaround just to see where it fails
+		if (!this->Check(VirtualAddress, (PTFlag)Flags)) // quick workaround just to see where it fails
 			warn("Failed to map v:%#lx p:%#lx with flags: " BYTE_TO_BINARY_PATTERN, VirtualAddress, PhysicalAddress, BYTE_TO_BINARY(Flags));
 #endif
 	}
@@ -426,4 +439,86 @@ namespace Memory
 		PTEPtr->Entries[Index.PTEIndex] = PTE;
 		CPU::x64::invlpg(VirtualAddress);
 	}
+
+	void Virtual::Remap(void *VirtualAddress, void *PhysicalAddress, uint64_t Flags, MapType Type)
+	{
+		SmartLock(this->MemoryLock);
+		if (unlikely(!this->pTable))
+		{
+			error("No page table");
+			return;
+		}
+
+		Flags |= PTFlag::P;
+
+		PageMapIndexer Index = PageMapIndexer((uintptr_t)VirtualAddress);
+		// Clear any flags that are not 1 << 0 (Present) - 1 << 5 (Accessed) because rest are for page table entries only
+		uint64_t DirectoryFlags = Flags & 0x3F;
+
+		PageMapLevel4 *PML4 = &this->pTable->Entries[Index.PMLIndex];
+		PageDirectoryPointerTableEntryPtr *PDPTEPtr = nullptr;
+		if (!PML4->Present)
+		{
+			PDPTEPtr = (PageDirectoryPointerTableEntryPtr *)KernelAllocator.RequestPages(TO_PAGES(sizeof(PageDirectoryPointerTableEntryPtr) + 1));
+			memset(PDPTEPtr, 0, sizeof(PageDirectoryPointerTableEntryPtr));
+			PML4->Present = true;
+			PML4->SetAddress((uintptr_t)PDPTEPtr >> 12);
+		}
+		else
+			PDPTEPtr = (PageDirectoryPointerTableEntryPtr *)(PML4->GetAddress() << 12);
+		PML4->raw |= DirectoryFlags;
+
+		PageDirectoryPointerTableEntry *PDPTE = &PDPTEPtr->Entries[Index.PDPTEIndex];
+		if (Type == MapType::OneGiB)
+		{
+			PDPTE->raw &= 0xFFF;
+			PDPTE->raw |= Flags;
+			PDPTE->PageSize = true;
+			PDPTE->SetAddress((uintptr_t)PhysicalAddress >> 12);
+			debug("Mapped 1GB page at %p to %p", VirtualAddress, PhysicalAddress);
+			return;
+		}
+
+		PageDirectoryEntryPtr *PDEPtr = nullptr;
+		if (!PDPTE->Present)
+		{
+			PDEPtr = (PageDirectoryEntryPtr *)KernelAllocator.RequestPages(TO_PAGES(sizeof(PageDirectoryEntryPtr) + 1));
+			memset(PDEPtr, 0, sizeof(PageDirectoryEntryPtr));
+			PDPTE->Present = true;
+			PDPTE->SetAddress((uintptr_t)PDEPtr >> 12);
+		}
+		else
+			PDEPtr = (PageDirectoryEntryPtr *)(PDPTE->GetAddress() << 12);
+		PDPTE->raw |= DirectoryFlags;
+
+		PageDirectoryEntry *PDE = &PDEPtr->Entries[Index.PDEIndex];
+		if (Type == MapType::TwoMiB)
+		{
+			PDE->raw &= 0xFFF;
+			PDE->raw |= Flags;
+			PDE->PageSize = true;
+			PDE->SetAddress((uintptr_t)PhysicalAddress >> 12);
+			debug("Mapped 2MB page at %p to %p", VirtualAddress, PhysicalAddress);
+			return;
+		}
+
+		PageTableEntryPtr *PTEPtr = nullptr;
+		if (!PDE->Present)
+		{
+			PTEPtr = (PageTableEntryPtr *)KernelAllocator.RequestPages(TO_PAGES(sizeof(PageTableEntryPtr) + 1));
+			memset(PTEPtr, 0, sizeof(PageTableEntryPtr));
+			PDE->Present = true;
+			PDE->SetAddress((uintptr_t)PTEPtr >> 12);
+		}
+		else
+			PTEPtr = (PageTableEntryPtr *)(PDE->GetAddress() << 12);
+		PDE->raw |= DirectoryFlags;
+
+		PageTableEntry *PTE = &PTEPtr->Entries[Index.PTEIndex];
+		PTE->raw &= 0xFFF;
+		PTE->raw |= Flags;
+		PTE->Present = true;
+		PTE->SetAddress((uintptr_t)PhysicalAddress >> 12);
+		CPU::x64::invlpg(VirtualAddress);
+	}
 }

include/memory/virtual.hpp

@@ -59,12 +59,9 @@ namespace Memory
 		 *
 		 * @param VirtualAddress Virtual address of the page
 		 * @param Flag Flag to check
-		 * @param Type Type of the page. Check MapType enum.
 		 * @return true if page has the specified flag, false otherwise.
 		 */
-		bool Check(void *VirtualAddress,
+		bool Check(void *VirtualAddress, PTFlag Flag = PTFlag::P);
-				   PTFlag Flag = PTFlag::P,
-				   MapType Type = MapType::FourKiB);
 
 		/**
 		 * @brief Check if the region has the specified flag.
@@ -72,17 +69,14 @@ namespace Memory
 		 * @param VirtualAddress Virtual address of the region.
 		 * @param Length Length of the region.
 		 * @param Flag Flag to check.
-		 * @param Type Type of the page. Check MapType enum.
 		 * @return true if the region has the specified flag, false otherwise.
 		 */
-		bool CheckRegion(void *VirtualAddress,
+		bool CheckRegion(void *VirtualAddress, size_t Length,
-						 size_t Length,
+						 PTFlag Flag = PTFlag::P)
-						 PTFlag Flag = PTFlag::P,
-						 MapType Type = MapType::FourKiB)
 		{
 			for (size_t i = 0; i < Length; i += PAGE_SIZE_4K)
 			{
-				if (!this->Check((void *)((uintptr_t)VirtualAddress + i), Flag, Type))
+				if (!this->Check((void *)((uintptr_t)VirtualAddress + i), Flag))
 				{
 					debug("address %#lx for pt %#lx has flag(s) %#lx",
 						  (uintptr_t)VirtualAddress + i, this->pTable, Flag);
@@ -293,11 +287,7 @@ namespace Memory
 		 * @param Flags Flags of the page. Check PTFlag enum.
 		 * @param Type Type of the page. Check MapType enum.
 		 */
-		__always_inline inline void Remap(void *VirtualAddress, void *PhysicalAddress, uint64_t Flags, MapType Type = MapType::FourKiB)
+		void Remap(void *VirtualAddress, void *PhysicalAddress, uint64_t Flags, MapType Type = MapType::FourKiB);
-		{
-			this->Unmap(VirtualAddress, Type);
-			this->Map(VirtualAddress, PhysicalAddress, Flags, Type);
-		}
 
 		/**
 		 * @brief Construct a new Virtual object