Skip to content

Commit

Permalink
Kernel: Make ProcessPagingScope restore CR3 properly
Browse files Browse the repository at this point in the history
Instead of restoring CR3 to the current process's paging scope when a
ProcessPagingScope goes out of scope, we now restore exactly whatever
the CR3 value was when we created the ProcessPagingScope.

This fixes breakage in situations where a process ends up with nested
ProcessPagingScopes. This was making profiling very fragile, and with
this change it's now possible to profile g++! :^)
  • Loading branch information
awesomekling committed Jan 19, 2020
1 parent ad3f931 commit 6eab7b3
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 7 deletions.
8 changes: 4 additions & 4 deletions Kernel/Process.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -734,7 +734,8 @@ int Process::do_exec(NonnullRefPtr<FileDescription> main_program_description, Ve
#ifdef MM_DEBUG
dbgprintf("Process %u exec: PD=%x created\n", pid(), m_page_directory.ptr());
#endif
ProcessPagingScope paging_scope(*this);

MM.enter_process_paging_scope(*this);

Region* region { nullptr };

Expand Down Expand Up @@ -775,11 +776,10 @@ int Process::do_exec(NonnullRefPtr<FileDescription> main_program_description, Ve
m_regions.append(move(executable_region));

ArmedScopeGuard rollback_regions_guard([&]() {
m_page_directory = move(old_page_directory);
ASSERT(&current->process() == this);
MM.enter_process_paging_scope(*this);
executable_region = m_regions.take_first();
m_page_directory = move(old_page_directory);
m_regions = move(old_regions);
MM.enter_process_paging_scope(*this);
});

loader = make<ELFLoader>(region->vaddr().as_ptr(), loader_metadata.size);
Expand Down
7 changes: 6 additions & 1 deletion Kernel/VM/MemoryManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -682,10 +682,15 @@ void MemoryManager::dump_kernel_regions()
ProcessPagingScope::ProcessPagingScope(Process& process)
{
ASSERT(current);
asm("movl %%cr3, %%eax"
: "=a"(m_previous_cr3));
MM.enter_process_paging_scope(process);
}

ProcessPagingScope::~ProcessPagingScope()
{
MM.enter_process_paging_scope(current->process());
InterruptDisabler disabler;
current->tss().cr3 = m_previous_cr3;
asm volatile("movl %%eax, %%cr3" ::"a"(m_previous_cr3)
: "memory");
}
8 changes: 6 additions & 2 deletions Kernel/VM/MemoryManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -190,9 +190,13 @@ class MemoryManager {
bool m_quickmap_in_use { false };
};

struct ProcessPagingScope {
ProcessPagingScope(Process&);
class ProcessPagingScope {
public:
explicit ProcessPagingScope(Process&);
~ProcessPagingScope();

private:
u32 m_previous_cr3 { 0 };
};

template<typename Callback>
Expand Down

0 comments on commit 6eab7b3

Please sign in to comment.