Fix: Process & Kernel PDs going out of sync

Author: coderarjob

docs/notes/TODO.md

@@ -156,13 +156,13 @@
 | ::: unittests how are we going to mock, so for unittests something else |           |            |
 | ::: need to be done.                                                    |           |            |
 |-------------------------------------------------------------------------|-----------|------------|
-| [ ] Process management                                                  | 11 Apr 24 |            |
+| [X] Process management                                                  | 11 Apr 24 |            |
 | ::: [X] Process Killing: Need a way to pass exit code to parent process | 21 Apr 24 | 11 Nov 24  |
 | Possible solution.                                                      |           |            |
 | When a process ending it would add SIGCHILD signal for its parent       | 21 Apr 24 |            |
 | and the scheduler will make sure that the parent gets the message.      |           |            |
 | Need a way to avoid creation of ZOMBIE processes.                       |           |            |
-| ::: [ ] Process Exit: Copy the Kernel PDEs from PD of the process to    | 27 Apr 24 |            |
+| ::: [X] Process Exit: Copy the Kernel PDEs from PD of the process to    | 27 Apr 24 | 26 Dec 24  |
 | ::: PD of the Kernel. At the start we copy the Kernel PDEs but are not  |           |            |
 | ::: copying it back. The Kernel memory mapping might have changed by    |           |            |
 | ::: now, so this copying is needed.                                     |           |            |

include/paging.h

@@ -42,7 +42,8 @@ typedef enum PagingMapFlags {
 typedef enum PagingOperationFlags {
     PG_NEWPD_FLAG_COPY_KERNEL_PAGES = (1 << 0),
     PG_NEWPD_FLAG_RECURSIVE_MAP     = (1 << 1),
-    PG_DELPD_FLAG_KEEP_KERNEL_PAGES = (1 << 2)
+    PG_DELPD_FLAG_KEEP_KERNEL_PAGES = (1 << 2),
+    PG_NEWPD_FLAG_CREATE_NEW        = (1 << 3),
 } PagingOperationFlags;
 
 // Physical start of the page frame 'pf'.
@@ -60,7 +61,8 @@ bool kpg_unmap (PageDirectory pd, PTR va);
 void* kpg_temporaryMap (Physical pa);
 void kpg_temporaryUnmap();
 bool kpg_doesMappingExists (PageDirectory pd, PTR va, Physical* pa);
-bool kpg_createNewPageDirectory (Physical* newPD, PagingOperationFlags flags);
-bool kpg_deletePageDirectory(Physical pd, PagingOperationFlags flags);
+bool kpg_setupPageDirectory (Physical* const pd, PagingOperationFlags flags,
+                             Physical const* const kernelPD);
+bool kpg_deletePageDirectory (Physical pd, PagingOperationFlags flags);
 
 #endif // PAGING_H

include/process.h

@@ -76,3 +76,4 @@ UINT kprocess_getCurrentPID();
 bool kprocess_popEvent (UINT pid, KProcessEvent* ev);
 bool kprocess_pushEvent (UINT pid, UINT eventID, UINT eventData);
 KProcessSections* kprocess_getCurrentProcessDataSection();
+void kprocess_syncPD();

include/vmm.h

@@ -32,6 +32,7 @@ PTR kvmm_findFree (VMemoryManager* vmm, SIZE szPages);
 PTR kvmm_memmap (VMemoryManager* vmm, PTR va, Physical const* const pa, SIZE szPages,
                  VMemoryMemMapFlags flags, Physical* const outPA);
 bool kvmm_checkbounds (VMemoryManager* vmm, PTR addr);
+bool kvmm_isPageDirectoryDirty (VMemoryManager* const vmm);
 
 #if defined(DEBUG) && defined(PORT_E9_ENABLED)
 void kvmm_printVASList (VMemoryManager* vmm);

include/vmm_struct.h

@@ -22,6 +22,7 @@ struct VMemoryManager {
     PTR end;
     bool isStaticAllocated;
     Physical parentProcessPD;
+    bool isPageDirectoryDirty; // Process's PageDirectory has changed. Will be reset once read.
     KernelPhysicalMemoryRegions physicalRegion;
     ListNode head;
 };

src/kernel/vmm.c

@@ -188,7 +188,7 @@ static VMemoryAddressSpace* find_vas (VMemoryManager const* const vmm, PTR start
     return NULL;
 }
 
-static bool commitVirtualPages (VMemoryManager const* const vmm, PTR vaStart,
+static bool commitVirtualPages (VMemoryManager* const vmm, PTR vaStart,
                                 Physical const* const paStart, SIZE numPages,
                                 const VMemoryAddressSpace* const vas, Physical* const outPA)
 {
@@ -223,6 +223,7 @@ static bool commitVirtualPages (VMemoryManager const* const vmm, PTR vaStart,
     }
 
     kpg_temporaryUnmap();
+    vmm->isPageDirectoryDirty = true; // PD has changed.
     return true;
 }
 
@@ -503,11 +504,21 @@ bool kvmm_commitPage (VMemoryManager* vmm, PTR va)
     return true;
 }
 
-Physical kvmm_getPageDirectory (const VMemoryManager* vmm)
+Physical kvmm_getPageDirectory (const VMemoryManager* const vmm)
 {
     FUNC_ENTRY ("vmm: %x", vmm);
 
     k_assert (vmm != NULL, "VMM not provided");
 
     return vmm->parentProcessPD;
 }
+
+bool kvmm_isPageDirectoryDirty (VMemoryManager* const vmm)
+{
+    FUNC_ENTRY ("vmm: %x", vmm);
+    k_assert (vmm != NULL, "VMM not provided");
+
+    bool isDirty   = vmm->isPageDirectoryDirty;
+    vmm->isPageDirectoryDirty = false;
+    return isDirty;
+}

src/kernel/x86/interrupts.c

@@ -139,11 +139,25 @@ void page_fault_handler (InterruptFrame *frame, UINT errorcode)
 
     // Page faults can occur for various reasons, it could be permissions or if page frame is not
     // present. Commit a physical page only it there is not one already allocated.
-    VMemoryManager* context = kvmm_checkbounds (g_kstate.context, fault_addr)
-                                  ? g_kstate.context
-                                  : kprocess_getCurrentContext();
+    VMemoryManager* context = kprocess_getCurrentContext();
+    if (kvmm_checkbounds (g_kstate.context, fault_addr)) {
+        context = g_kstate.context;
+
+        // Attempt 1: Check if Kernel PD has changed and a sync is pending.
+        if (kvmm_isPageDirectoryDirty (context)) {
+            INFO ("Kernel and Process PD out of sync. Syncing..");
+            kprocess_syncPD();
+            return; // then retry.
+        }
+    }
 
+    // Attempt 2 : Process PD is sync with the Kernel PD. Actually commit the faulting virtual
+    // address.
     if (err->Present == false && kvmm_commitPage (context, fault_addr)) {
+        // Kernel PD has changed. Sync all Page Directories to it.
+        if (context == g_kstate.context) {
+            kprocess_syncPD();
+        }
         return; // then retry
     }
 

src/kernel/x86/paging.c

@@ -425,35 +425,52 @@ bool kpg_doesMappingExists (PageDirectory pd, PTR va, Physical* pa)
  * @Input   flags       Flags that determine the setup of the Page directory.
  * @return              True if success, false otherwise.
  **************************************************************************************************/
-bool kpg_createNewPageDirectory (Physical* newPD, PagingOperationFlags flags)
+bool kpg_setupPageDirectory (Physical* const pd, PagingOperationFlags flags,
+                             Physical const* const kernelPD)
 {
-    FUNC_ENTRY ("newPD return addr: %px, Flags: %x", newPD, flags);
+    FUNC_ENTRY ("PD addr: %px, Flags: %x, kernel PD: %px", pd, flags, kernelPD);
 
-    if (kpmm_alloc (newPD, 1, PMM_REGION_ANY) == false) {
-        RETURN_ERROR (ERROR_PASSTHROUGH, false); // PMM alloc failure
-    }
+    k_assert (pd != NULL, "Invalid address.");
 
-    INFO ("New PD physical location: %px", newPD->val);
+    PageDirectory l_pd = NULL;
+    // ---------------------------------------------------------------
+    // Create a new, empty Page Directory.
+    // ---------------------------------------------------------------
+    if (BIT_ISSET (flags, PG_NEWPD_FLAG_CREATE_NEW)) {
+        if (kpmm_alloc (pd, 1, PMM_REGION_ANY) == false) {
+            RETURN_ERROR (ERROR_PASSTHROUGH, false); // PMM alloc failure
+        }
+        INFO ("New PD physical location: %px", pd->val);
 
-    PageDirectory pd = s_internal_temporaryMap (*newPD);
-    k_memset (pd, 0, CONFIG_PAGE_FRAME_SIZE_BYTES);
+        l_pd = s_internal_temporaryMap (*pd);
+        k_memset (l_pd, 0, CONFIG_PAGE_FRAME_SIZE_BYTES);
+    } else {
+        l_pd = s_internal_temporaryMap (*pd);
+    }
 
-    // Temporary map this PD and copy kernel page table entries
+    // ---------------------------------------------------------------
+    // Copy kernel pages tables to the provided PD.
+    // ---------------------------------------------------------------
     if (BIT_ISSET (flags, PG_NEWPD_FLAG_COPY_KERNEL_PAGES)) {
-        PageDirectory currentPD = kpg_getcurrentpd();
+        k_assert (kernelPD != NULL, "Invalid address.");
+
+        PageDirectory l_kernelPD = kpg_temporaryMap (*kernelPD);
         for (UINT pdi = KERNEL_PDE_INDEX; pdi < 1024; pdi++) {
-            pd[pdi] = currentPD[pdi];
+            l_pd[pdi] = l_kernelPD[pdi];
         }
+        kpg_temporaryUnmap();
+    }
 
-        if (BIT_ISSET (flags, PG_NEWPD_FLAG_RECURSIVE_MAP)) {
-            pd[RECURSIVE_PDE_INDEX].pageTableFrame = PHYSICAL_TO_PAGEFRAME (newPD->val);
-        }
+    // ---------------------------------------------------------------
+    // Recursive map
+    // ---------------------------------------------------------------
+    if (BIT_ISSET (flags, PG_NEWPD_FLAG_RECURSIVE_MAP)) {
+        l_pd[RECURSIVE_PDE_INDEX].pageTableFrame = PHYSICAL_TO_PAGEFRAME (pd->val);
     }
-    s_internal_temporaryUnmap();
 
-    return true; // Success
+    s_internal_temporaryUnmap();
+    return true;
 }
-
 /***************************************************************************************************
  * Deletes a Page Directory and its Page tables.
  *

src/kernel/x86/process.c

@@ -208,8 +208,11 @@ static bool s_createProcessPageDirectory (KProcessInfo* pinfo)
     if (BIT_ISUNSET (pinfo->flags, PROCESS_FLAGS_THREAD)) {
         // Create physical memory for Page directory of the new non-thread process
         Physical newPD;
-        if (!kpg_createNewPageDirectory (&newPD, PG_NEWPD_FLAG_COPY_KERNEL_PAGES |
-                                                     PG_NEWPD_FLAG_RECURSIVE_MAP)) {
+        Physical kernelPD = kvmm_getPageDirectory (g_kstate.context);
+        if (!kpg_setupPageDirectory (&newPD,
+                                     PG_NEWPD_FLAG_CREATE_NEW | PG_NEWPD_FLAG_COPY_KERNEL_PAGES |
+                                         PG_NEWPD_FLAG_RECURSIVE_MAP,
+                                     &kernelPD)) {
             RETURN_ERROR (ERROR_PASSTHROUGH, false); // Cannot create new process.
         }
 
@@ -829,3 +832,27 @@ bool kprocess_pushEvent (UINT pid, UINT eventID, UINT eventData)
     enqueue (&pinfo->eventsQueueHead, &e->eventQueueNode);
     return true;
 }
+
+void kprocess_syncPD()
+{
+    FUNC_ENTRY();
+
+    if (!KERNEL_PHASE_CHECK (KERNEL_PHASE_STATE_KERNEL_READY)) {
+        return;
+    }
+
+    Physical kernelPD = kvmm_getPageDirectory (g_kstate.context);
+
+    ListNode* node  = NULL;
+    KProcessInfo* p = NULL;
+    list_for_each (&schedulerQueueHead, node)
+    {
+        p           = LIST_ITEM (node, KProcessInfo, schedulerQueueNode);
+        Physical pd = kvmm_getPageDirectory (p->context);
+        if (!kpg_setupPageDirectory (&pd,
+                                     PG_NEWPD_FLAG_COPY_KERNEL_PAGES | PG_NEWPD_FLAG_RECURSIVE_MAP,
+                                     &kernelPD)) {
+            FATAL_BUG(); // No reason it should fail.
+        }
+    }
+}