DANGER DANGER. Switching to global critical region.
This changes almost all locks held by guest threads to use a single global critical region. This emulates the behavior on the PPC of disabling interrupts (by calls like KeRaiseIrqlToDpcLevel or masking interrupts), and prevents deadlocks from occuring when threads are suspended or otherwise blocked. This has performance implications and a pass is needed to ensure the locking is as granular as possible. It could also break everything because it's fundamentally unsound. We'll see.
This commit is contained in:
@@ -129,7 +129,7 @@ void* X64CodeCache::PlaceGuestCode(uint32_t guest_address, void* machine_code,
|
||||
uint8_t* code_address = nullptr;
|
||||
UnwindReservation unwind_reservation;
|
||||
{
|
||||
std::lock_guard<xe::mutex> allocation_lock(allocation_mutex_);
|
||||
auto global_lock = global_critical_region_.Acquire();
|
||||
|
||||
low_mark = generated_code_offset_;
|
||||
|
||||
@@ -192,7 +192,7 @@ uint32_t X64CodeCache::PlaceData(const void* data, size_t length) {
|
||||
size_t high_mark;
|
||||
uint8_t* data_address = nullptr;
|
||||
{
|
||||
std::lock_guard<xe::mutex> allocation_lock(allocation_mutex_);
|
||||
auto global_lock = global_critical_region_.Acquire();
|
||||
|
||||
// Reserve code.
|
||||
// Always move the code to land on 16b alignment.
|
||||
|
||||
@@ -12,7 +12,6 @@
|
||||
|
||||
#include <atomic>
|
||||
#include <memory>
|
||||
#include <mutex>
|
||||
#include <string>
|
||||
#include <utility>
|
||||
#include <vector>
|
||||
@@ -91,9 +90,9 @@ class X64CodeCache : public CodeCache {
|
||||
std::wstring file_name_;
|
||||
xe::memory::FileMappingHandle mapping_ = nullptr;
|
||||
|
||||
// Must be held when manipulating the offsets or counts of anything, to keep
|
||||
// the tables consistent and ordered.
|
||||
xe::mutex allocation_mutex_;
|
||||
// NOTE: the global critical region must be held when manipulating the offsets
|
||||
// or counts of anything, to keep the tables consistent and ordered.
|
||||
xe::global_critical_region global_critical_region_;
|
||||
|
||||
// Value that the indirection table will be initialized with upon commit.
|
||||
uint32_t indirection_default_value_ = 0xFEEDF00D;
|
||||
|
||||
@@ -18,7 +18,7 @@ namespace cpu {
|
||||
EntryTable::EntryTable() = default;
|
||||
|
||||
EntryTable::~EntryTable() {
|
||||
std::lock_guard<xe::mutex> guard(lock_);
|
||||
auto global_lock = global_critical_region_.Acquire();
|
||||
for (auto it : map_) {
|
||||
Entry* entry = it.second;
|
||||
delete entry;
|
||||
@@ -26,7 +26,7 @@ EntryTable::~EntryTable() {
|
||||
}
|
||||
|
||||
Entry* EntryTable::Get(uint32_t address) {
|
||||
std::lock_guard<xe::mutex> guard(lock_);
|
||||
auto global_lock = global_critical_region_.Acquire();
|
||||
const auto& it = map_.find(address);
|
||||
Entry* entry = it != map_.end() ? it->second : nullptr;
|
||||
if (entry) {
|
||||
@@ -42,7 +42,7 @@ Entry::Status EntryTable::GetOrCreate(uint32_t address, Entry** out_entry) {
|
||||
// TODO(benvanik): replace with a map with wait-free for find.
|
||||
// https://github.com/facebook/folly/blob/master/folly/AtomicHashMap.h
|
||||
|
||||
lock_.lock();
|
||||
auto global_lock = global_critical_region_.Acquire();
|
||||
const auto& it = map_.find(address);
|
||||
Entry* entry = it != map_.end() ? it->second : nullptr;
|
||||
Entry::Status status;
|
||||
@@ -51,10 +51,10 @@ Entry::Status EntryTable::GetOrCreate(uint32_t address, Entry** out_entry) {
|
||||
if (entry->status == Entry::STATUS_COMPILING) {
|
||||
// Still compiling, so spin.
|
||||
do {
|
||||
lock_.unlock();
|
||||
global_lock.unlock();
|
||||
// TODO(benvanik): sleep for less time?
|
||||
xe::threading::Sleep(std::chrono::microseconds(10));
|
||||
lock_.lock();
|
||||
global_lock.lock();
|
||||
} while (entry->status == Entry::STATUS_COMPILING);
|
||||
}
|
||||
status = entry->status;
|
||||
@@ -68,13 +68,13 @@ Entry::Status EntryTable::GetOrCreate(uint32_t address, Entry** out_entry) {
|
||||
map_[address] = entry;
|
||||
status = Entry::STATUS_NEW;
|
||||
}
|
||||
lock_.unlock();
|
||||
global_lock.unlock();
|
||||
*out_entry = entry;
|
||||
return status;
|
||||
}
|
||||
|
||||
std::vector<Function*> EntryTable::FindWithAddress(uint32_t address) {
|
||||
std::lock_guard<xe::mutex> guard(lock_);
|
||||
auto global_lock = global_critical_region_.Acquire();
|
||||
std::vector<Function*> fns;
|
||||
for (auto& it : map_) {
|
||||
Entry* entry = it.second;
|
||||
|
||||
@@ -10,7 +10,6 @@
|
||||
#ifndef XENIA_CPU_ENTRY_TABLE_H_
|
||||
#define XENIA_CPU_ENTRY_TABLE_H_
|
||||
|
||||
#include <mutex>
|
||||
#include <unordered_map>
|
||||
#include <vector>
|
||||
|
||||
@@ -46,8 +45,8 @@ class EntryTable {
|
||||
std::vector<Function*> FindWithAddress(uint32_t address);
|
||||
|
||||
private:
|
||||
xe::global_critical_region global_critical_region_;
|
||||
// TODO(benvanik): replace with a better data structure.
|
||||
xe::mutex lock_;
|
||||
std::unordered_map<uint32_t, Entry*> map_;
|
||||
};
|
||||
|
||||
|
||||
@@ -9,6 +9,7 @@
|
||||
|
||||
#include "xenia/cpu/frontend/ppc_frontend.h"
|
||||
|
||||
#include "xenia/base/atomic.h"
|
||||
#include "xenia/cpu/frontend/ppc_context.h"
|
||||
#include "xenia/cpu/frontend/ppc_disasm.h"
|
||||
#include "xenia/cpu/frontend/ppc_emit.h"
|
||||
@@ -71,20 +72,20 @@ void EnterGlobalLock(PPCContext* ppc_context, void* arg0, void* arg1) {
|
||||
auto global_mutex = reinterpret_cast<xe::recursive_mutex*>(arg0);
|
||||
auto global_lock_count = reinterpret_cast<int32_t*>(arg1);
|
||||
global_mutex->lock();
|
||||
*global_lock_count = *global_lock_count + 1;
|
||||
xe::atomic_inc(global_lock_count);
|
||||
}
|
||||
|
||||
// Leaves the global lock. Safe to recursion.
|
||||
void LeaveGlobalLock(PPCContext* ppc_context, void* arg0, void* arg1) {
|
||||
auto global_mutex = reinterpret_cast<xe::recursive_mutex*>(arg0);
|
||||
auto global_lock_count = reinterpret_cast<int32_t*>(arg1);
|
||||
*global_lock_count = *global_lock_count - 1;
|
||||
assert_true(*global_lock_count >= 0);
|
||||
auto new_lock_count = xe::atomic_dec(global_lock_count);
|
||||
assert_true(new_lock_count >= 0);
|
||||
global_mutex->unlock();
|
||||
}
|
||||
|
||||
bool PPCFrontend::Initialize() {
|
||||
void* arg0 = reinterpret_cast<void*>(processor_->global_mutex());
|
||||
void* arg0 = reinterpret_cast<void*>(&xe::global_critical_region::mutex());
|
||||
void* arg1 = reinterpret_cast<void*>(&builtins_.global_lock_count);
|
||||
builtins_.check_global_lock =
|
||||
processor_->DefineBuiltin("CheckGlobalLock", CheckGlobalLock, arg0, arg1);
|
||||
|
||||
@@ -11,9 +11,7 @@
|
||||
#define XENIA_CPU_FRONTEND_PPC_FRONTEND_H_
|
||||
|
||||
#include <memory>
|
||||
#include <mutex>
|
||||
|
||||
#include "xenia/base/mutex.h"
|
||||
#include "xenia/base/type_pool.h"
|
||||
#include "xenia/cpu/frontend/context_info.h"
|
||||
#include "xenia/cpu/function.h"
|
||||
|
||||
@@ -13,7 +13,6 @@
|
||||
#include <memory>
|
||||
#include <vector>
|
||||
|
||||
#include "xenia/base/mutex.h"
|
||||
#include "xenia/cpu/debug_info.h"
|
||||
#include "xenia/cpu/frontend/ppc_context.h"
|
||||
#include "xenia/cpu/symbol.h"
|
||||
|
||||
@@ -113,9 +113,9 @@ uintptr_t MMIOHandler::AddPhysicalWriteWatch(uint32_t guest_address,
|
||||
entry->callback = callback;
|
||||
entry->callback_context = callback_context;
|
||||
entry->callback_data = callback_data;
|
||||
write_watch_mutex_.lock();
|
||||
global_critical_region_.mutex().lock();
|
||||
write_watches_.push_back(entry);
|
||||
write_watch_mutex_.unlock();
|
||||
global_critical_region_.mutex().unlock();
|
||||
|
||||
// Make the desired range read only under all address spaces.
|
||||
xe::memory::Protect(physical_membase_ + entry->address, entry->length,
|
||||
@@ -154,12 +154,12 @@ void MMIOHandler::CancelWriteWatch(uintptr_t watch_handle) {
|
||||
ClearWriteWatch(entry);
|
||||
|
||||
// Remove from table.
|
||||
write_watch_mutex_.lock();
|
||||
global_critical_region_.mutex().lock();
|
||||
auto it = std::find(write_watches_.begin(), write_watches_.end(), entry);
|
||||
if (it != write_watches_.end()) {
|
||||
write_watches_.erase(it);
|
||||
}
|
||||
write_watch_mutex_.unlock();
|
||||
global_critical_region_.mutex().unlock();
|
||||
|
||||
delete entry;
|
||||
}
|
||||
@@ -170,7 +170,7 @@ bool MMIOHandler::CheckWriteWatch(void* thread_state, uint64_t fault_address) {
|
||||
physical_address &= 0x1FFFFFFF;
|
||||
}
|
||||
std::list<WriteWatchEntry*> pending_invalidates;
|
||||
write_watch_mutex_.lock();
|
||||
global_critical_region_.mutex().lock();
|
||||
for (auto it = write_watches_.begin(); it != write_watches_.end();) {
|
||||
auto entry = *it;
|
||||
if (entry->address <= physical_address &&
|
||||
@@ -186,7 +186,7 @@ bool MMIOHandler::CheckWriteWatch(void* thread_state, uint64_t fault_address) {
|
||||
}
|
||||
++it;
|
||||
}
|
||||
write_watch_mutex_.unlock();
|
||||
global_critical_region_.mutex().unlock();
|
||||
if (pending_invalidates.empty()) {
|
||||
// Rethrow access violation - range was not being watched.
|
||||
return false;
|
||||
|
||||
@@ -12,7 +12,6 @@
|
||||
|
||||
#include <list>
|
||||
#include <memory>
|
||||
#include <mutex>
|
||||
#include <vector>
|
||||
|
||||
#include "xenia/base/mutex.h"
|
||||
@@ -90,8 +89,8 @@ class MMIOHandler {
|
||||
|
||||
std::vector<MMIORange> mapped_ranges_;
|
||||
|
||||
xe::global_critical_region global_critical_region_;
|
||||
// TODO(benvanik): data structure magic.
|
||||
xe::mutex write_watch_mutex_;
|
||||
std::list<WriteWatchEntry*> write_watches_;
|
||||
|
||||
static MMIOHandler* global_handler_;
|
||||
|
||||
@@ -29,7 +29,7 @@ Module::~Module() = default;
|
||||
bool Module::ContainsAddress(uint32_t address) { return true; }
|
||||
|
||||
Symbol* Module::LookupSymbol(uint32_t address, bool wait) {
|
||||
lock_.lock();
|
||||
auto global_lock = global_critical_region_.Acquire();
|
||||
const auto it = map_.find(address);
|
||||
Symbol* symbol = it != map_.end() ? it->second : nullptr;
|
||||
if (symbol) {
|
||||
@@ -37,10 +37,10 @@ Symbol* Module::LookupSymbol(uint32_t address, bool wait) {
|
||||
// Some other thread is declaring the symbol - wait.
|
||||
if (wait) {
|
||||
do {
|
||||
lock_.unlock();
|
||||
global_lock.unlock();
|
||||
// TODO(benvanik): sleep for less time?
|
||||
xe::threading::Sleep(std::chrono::microseconds(100));
|
||||
lock_.lock();
|
||||
global_lock.lock();
|
||||
} while (symbol->status() == Symbol::Status::kDeclaring);
|
||||
} else {
|
||||
// Immediate request, just return.
|
||||
@@ -48,31 +48,31 @@ Symbol* Module::LookupSymbol(uint32_t address, bool wait) {
|
||||
}
|
||||
}
|
||||
}
|
||||
lock_.unlock();
|
||||
global_lock.unlock();
|
||||
return symbol;
|
||||
}
|
||||
|
||||
Symbol::Status Module::DeclareSymbol(Symbol::Type type, uint32_t address,
|
||||
Symbol** out_symbol) {
|
||||
*out_symbol = nullptr;
|
||||
lock_.lock();
|
||||
auto global_lock = global_critical_region_.Acquire();
|
||||
auto it = map_.find(address);
|
||||
Symbol* symbol = it != map_.end() ? it->second : nullptr;
|
||||
Symbol::Status status;
|
||||
if (symbol) {
|
||||
// If we exist but are the wrong type, die.
|
||||
if (symbol->type() != type) {
|
||||
lock_.unlock();
|
||||
global_lock.unlock();
|
||||
return Symbol::Status::kFailed;
|
||||
}
|
||||
// If we aren't ready yet spin and wait.
|
||||
if (symbol->status() == Symbol::Status::kDeclaring) {
|
||||
// Still declaring, so spin.
|
||||
do {
|
||||
lock_.unlock();
|
||||
global_lock.unlock();
|
||||
// TODO(benvanik): sleep for less time?
|
||||
xe::threading::Sleep(std::chrono::microseconds(100));
|
||||
lock_.lock();
|
||||
global_lock.lock();
|
||||
} while (symbol->status() == Symbol::Status::kDeclaring);
|
||||
}
|
||||
status = symbol->status();
|
||||
@@ -90,7 +90,7 @@ Symbol::Status Module::DeclareSymbol(Symbol::Type type, uint32_t address,
|
||||
list_.emplace_back(symbol);
|
||||
status = Symbol::Status::kNew;
|
||||
}
|
||||
lock_.unlock();
|
||||
global_lock.unlock();
|
||||
*out_symbol = symbol;
|
||||
|
||||
// Get debug info from providers, if this is new.
|
||||
@@ -117,7 +117,7 @@ Symbol::Status Module::DeclareVariable(uint32_t address, Symbol** out_symbol) {
|
||||
}
|
||||
|
||||
Symbol::Status Module::DefineSymbol(Symbol* symbol) {
|
||||
lock_.lock();
|
||||
auto global_lock = global_critical_region_.Acquire();
|
||||
Symbol::Status status;
|
||||
if (symbol->status() == Symbol::Status::kDeclared) {
|
||||
// Declared but undefined, so request caller define it.
|
||||
@@ -126,16 +126,16 @@ Symbol::Status Module::DefineSymbol(Symbol* symbol) {
|
||||
} else if (symbol->status() == Symbol::Status::kDefining) {
|
||||
// Still defining, so spin.
|
||||
do {
|
||||
lock_.unlock();
|
||||
global_lock.unlock();
|
||||
// TODO(benvanik): sleep for less time?
|
||||
xe::threading::Sleep(std::chrono::microseconds(100));
|
||||
lock_.lock();
|
||||
global_lock.lock();
|
||||
} while (symbol->status() == Symbol::Status::kDefining);
|
||||
status = symbol->status();
|
||||
} else {
|
||||
status = symbol->status();
|
||||
}
|
||||
lock_.unlock();
|
||||
global_lock.unlock();
|
||||
return status;
|
||||
}
|
||||
|
||||
@@ -148,7 +148,7 @@ Symbol::Status Module::DefineVariable(Symbol* symbol) {
|
||||
}
|
||||
|
||||
void Module::ForEachFunction(std::function<void(Function*)> callback) {
|
||||
std::lock_guard<xe::mutex> guard(lock_);
|
||||
auto global_lock = global_critical_region_.Acquire();
|
||||
for (auto& symbol : list_) {
|
||||
if (symbol->type() == Symbol::Type::kFunction) {
|
||||
Function* info = static_cast<Function*>(symbol.get());
|
||||
@@ -159,7 +159,7 @@ void Module::ForEachFunction(std::function<void(Function*)> callback) {
|
||||
|
||||
void Module::ForEachSymbol(size_t start_index, size_t end_index,
|
||||
std::function<void(Symbol*)> callback) {
|
||||
std::lock_guard<xe::mutex> guard(lock_);
|
||||
auto global_lock = global_critical_region_.Acquire();
|
||||
start_index = std::min(start_index, list_.size());
|
||||
end_index = std::min(end_index, list_.size());
|
||||
for (size_t i = start_index; i <= end_index; ++i) {
|
||||
@@ -169,7 +169,7 @@ void Module::ForEachSymbol(size_t start_index, size_t end_index,
|
||||
}
|
||||
|
||||
size_t Module::QuerySymbolCount() {
|
||||
std::lock_guard<xe::mutex> guard(lock_);
|
||||
auto global_lock = global_critical_region_.Acquire();
|
||||
return list_.size();
|
||||
}
|
||||
|
||||
|
||||
@@ -12,7 +12,6 @@
|
||||
|
||||
#include <functional>
|
||||
#include <memory>
|
||||
#include <mutex>
|
||||
#include <string>
|
||||
#include <unordered_map>
|
||||
#include <vector>
|
||||
@@ -64,8 +63,8 @@ class Module {
|
||||
Symbol** out_symbol);
|
||||
Symbol::Status DefineSymbol(Symbol* symbol);
|
||||
|
||||
xe::global_critical_region global_critical_region_;
|
||||
// TODO(benvanik): replace with a better data structure.
|
||||
xe::mutex lock_;
|
||||
std::unordered_map<uint32_t, Symbol*> map_;
|
||||
std::vector<std::unique_ptr<Symbol>> list_;
|
||||
};
|
||||
|
||||
@@ -60,7 +60,7 @@ Processor::Processor(xe::Memory* memory, ExportResolver* export_resolver,
|
||||
|
||||
Processor::~Processor() {
|
||||
{
|
||||
std::lock_guard<xe::mutex> guard(modules_lock_);
|
||||
auto global_lock = global_critical_region_.Acquire();
|
||||
modules_.clear();
|
||||
}
|
||||
|
||||
@@ -126,13 +126,13 @@ bool Processor::Setup() {
|
||||
}
|
||||
|
||||
bool Processor::AddModule(std::unique_ptr<Module> module) {
|
||||
std::lock_guard<xe::mutex> guard(modules_lock_);
|
||||
auto global_lock = global_critical_region_.Acquire();
|
||||
modules_.push_back(std::move(module));
|
||||
return true;
|
||||
}
|
||||
|
||||
Module* Processor::GetModule(const char* name) {
|
||||
std::lock_guard<xe::mutex> guard(modules_lock_);
|
||||
auto global_lock = global_critical_region_.Acquire();
|
||||
for (const auto& module : modules_) {
|
||||
if (module->name() == name) {
|
||||
return module.get();
|
||||
@@ -142,7 +142,7 @@ Module* Processor::GetModule(const char* name) {
|
||||
}
|
||||
|
||||
std::vector<Module*> Processor::GetModules() {
|
||||
std::lock_guard<xe::mutex> guard(modules_lock_);
|
||||
auto global_lock = global_critical_region_.Acquire();
|
||||
std::vector<Module*> clone(modules_.size());
|
||||
for (const auto& module : modules_) {
|
||||
clone.push_back(module.get());
|
||||
@@ -215,7 +215,7 @@ Function* Processor::LookupFunction(uint32_t address) {
|
||||
// Find the module that contains the address.
|
||||
Module* code_module = nullptr;
|
||||
{
|
||||
std::lock_guard<xe::mutex> guard(modules_lock_);
|
||||
auto global_lock = global_critical_region_.Acquire();
|
||||
// TODO(benvanik): sort by code address (if contiguous) so can bsearch.
|
||||
// TODO(benvanik): cache last module low/high, as likely to be in there.
|
||||
for (const auto& module : modules_) {
|
||||
@@ -335,7 +335,7 @@ uint64_t Processor::ExecuteInterrupt(ThreadState* thread_state,
|
||||
// Hold the global lock during interrupt dispatch.
|
||||
// This will block if any code is in a critical region (has interrupts
|
||||
// disabled) or if any other interrupt is executing.
|
||||
std::lock_guard<xe::recursive_mutex> lock(global_mutex_);
|
||||
auto global_lock = global_critical_region_.Acquire();
|
||||
|
||||
PPCContext* context = thread_state->context();
|
||||
assert_true(arg_count <= 5);
|
||||
|
||||
@@ -86,7 +86,6 @@ class Processor {
|
||||
uint64_t ExecuteInterrupt(ThreadState* thread_state, uint32_t address,
|
||||
uint64_t args[], size_t arg_count);
|
||||
|
||||
xe::recursive_mutex* global_mutex() { return &global_mutex_; }
|
||||
Irql RaiseIrql(Irql new_value);
|
||||
void LowerIrql(Irql old_value);
|
||||
|
||||
@@ -104,13 +103,12 @@ class Processor {
|
||||
ExportResolver* export_resolver_ = nullptr;
|
||||
|
||||
EntryTable entry_table_;
|
||||
xe::mutex modules_lock_;
|
||||
xe::global_critical_region global_critical_region_;
|
||||
std::vector<std::unique_ptr<Module>> modules_;
|
||||
Module* builtin_module_ = nullptr;
|
||||
uint32_t next_builtin_address_ = 0xFFFF0000u;
|
||||
|
||||
Irql irql_;
|
||||
xe::recursive_mutex global_mutex_;
|
||||
};
|
||||
|
||||
} // namespace cpu
|
||||
|
||||
@@ -90,7 +90,7 @@ ThreadState::ThreadState(Processor* processor, uint32_t thread_id,
|
||||
std::memset(context_, 0, sizeof(PPCContext));
|
||||
|
||||
// Stash pointers to common structures that callbacks may need.
|
||||
context_->global_mutex = processor_->global_mutex();
|
||||
context_->global_mutex = &xe::global_critical_region::mutex();
|
||||
context_->virtual_membase = memory_->virtual_membase();
|
||||
context_->physical_membase = memory_->physical_membase();
|
||||
context_->processor = processor_;
|
||||
|
||||
Reference in New Issue
Block a user