GPUMemoryManager: Improve safety of memory reads.

This commit is contained in:
Fernando Sahmkow 2020-04-08 12:08:06 -04:00
parent f9d5718c4b
commit e00d992848
3 changed files with 47 additions and 55 deletions

View File

@ -53,7 +53,7 @@ public:
if (!is_written && !IsRegionWritten(cpu_addr, cpu_addr + size - 1)) { if (!is_written && !IsRegionWritten(cpu_addr, cpu_addr + size - 1)) {
auto& memory_manager = system.GPU().MemoryManager(); auto& memory_manager = system.GPU().MemoryManager();
if (use_fast_cbuf) { if (use_fast_cbuf) {
if (Tegra::MemoryManager::IsGranularRange(gpu_addr, size)) { if (memory_manager.IsGranularRange(gpu_addr, size)) {
const auto host_ptr = memory_manager.GetPointer(gpu_addr); const auto host_ptr = memory_manager.GetPointer(gpu_addr);
return ConstBufferUpload(host_ptr, size); return ConstBufferUpload(host_ptr, size);
} else { } else {
@ -62,7 +62,7 @@ public:
return ConstBufferUpload(staging_buffer.data(), size); return ConstBufferUpload(staging_buffer.data(), size);
} }
} else { } else {
if (Tegra::MemoryManager::IsGranularRange(gpu_addr, size)) { if (memory_manager.IsGranularRange(gpu_addr, size)) {
const auto host_ptr = memory_manager.GetPointer(gpu_addr); const auto host_ptr = memory_manager.GetPointer(gpu_addr);
return StreamBufferUpload(host_ptr, size, alignment); return StreamBufferUpload(host_ptr, size, alignment);
} else { } else {
@ -228,7 +228,7 @@ private:
auto& memory_manager = system.GPU().MemoryManager(); auto& memory_manager = system.GPU().MemoryManager();
const VAddr cpu_addr_end = cpu_addr + size; const VAddr cpu_addr_end = cpu_addr + size;
MapInterval new_map = CreateMap(cpu_addr, cpu_addr_end, gpu_addr); MapInterval new_map = CreateMap(cpu_addr, cpu_addr_end, gpu_addr);
if (Tegra::MemoryManager::IsGranularRange(gpu_addr, size)) { if (memory_manager.IsGranularRange(gpu_addr, size)) {
u8* host_ptr = memory_manager.GetPointer(gpu_addr); u8* host_ptr = memory_manager.GetPointer(gpu_addr);
UploadBlockData(block, block->GetOffset(cpu_addr), size, host_ptr); UploadBlockData(block, block->GetOffset(cpu_addr), size, host_ptr);
} else { } else {

View File

@ -139,11 +139,11 @@ T MemoryManager::Read(GPUVAddr addr) const {
return {}; return {};
} }
const u8* page_pointer{page_table.pointers[addr >> page_bits]}; const u8* page_pointer{GetPointer(addr)};
if (page_pointer) { if (page_pointer) {
// NOTE: Avoid adding any extra logic to this fast-path block // NOTE: Avoid adding any extra logic to this fast-path block
T value; T value;
std::memcpy(&value, &page_pointer[addr & page_mask], sizeof(T)); std::memcpy(&value, page_pointer, sizeof(T));
return value; return value;
} }
@ -166,10 +166,10 @@ void MemoryManager::Write(GPUVAddr addr, T data) {
return; return;
} }
u8* page_pointer{page_table.pointers[addr >> page_bits]}; u8* page_pointer{GetPointer(addr)};
if (page_pointer) { if (page_pointer) {
// NOTE: Avoid adding any extra logic to this fast-path block // NOTE: Avoid adding any extra logic to this fast-path block
std::memcpy(&page_pointer[addr & page_mask], &data, sizeof(T)); std::memcpy(page_pointer, &data, sizeof(T));
return; return;
} }
@ -200,9 +200,12 @@ u8* MemoryManager::GetPointer(GPUVAddr addr) {
return {}; return {};
} }
u8* const page_pointer{page_table.pointers[addr >> page_bits]}; auto& memory = system.Memory();
if (page_pointer != nullptr) {
return page_pointer + (addr & page_mask); const VAddr const page_addr{page_table.backing_addr[addr >> page_bits]};
if (page_addr != 0) {
return memory.GetPointer(page_addr + (addr & page_mask));
} }
LOG_ERROR(HW_GPU, "Unknown GetPointer @ 0x{:016X}", addr); LOG_ERROR(HW_GPU, "Unknown GetPointer @ 0x{:016X}", addr);
@ -214,9 +217,12 @@ const u8* MemoryManager::GetPointer(GPUVAddr addr) const {
return {}; return {};
} }
const u8* const page_pointer{page_table.pointers[addr >> page_bits]}; const auto& memory = system.Memory();
if (page_pointer != nullptr) {
return page_pointer + (addr & page_mask); const VAddr const page_addr{page_table.backing_addr[addr >> page_bits]};
if (page_addr != 0) {
return memory.GetPointer(page_addr + (addr & page_mask));
} }
LOG_ERROR(HW_GPU, "Unknown GetPointer @ 0x{:016X}", addr); LOG_ERROR(HW_GPU, "Unknown GetPointer @ 0x{:016X}", addr);
@ -237,17 +243,19 @@ void MemoryManager::ReadBlock(GPUVAddr src_addr, void* dest_buffer, const std::s
std::size_t page_index{src_addr >> page_bits}; std::size_t page_index{src_addr >> page_bits};
std::size_t page_offset{src_addr & page_mask}; std::size_t page_offset{src_addr & page_mask};
auto& memory = system.Memory();
while (remaining_size > 0) { while (remaining_size > 0) {
const std::size_t copy_amount{ const std::size_t copy_amount{
std::min(static_cast<std::size_t>(page_size) - page_offset, remaining_size)}; std::min(static_cast<std::size_t>(page_size) - page_offset, remaining_size)};
switch (page_table.attributes[page_index]) { switch (page_table.attributes[page_index]) {
case Common::PageType::Memory: { case Common::PageType::Memory: {
const u8* src_ptr{page_table.pointers[page_index] + page_offset}; const VAddr src_addr{page_table.backing_addr[page_index] + page_offset};
// Flush must happen on the rasterizer interface, such that memory is always synchronous // Flush must happen on the rasterizer interface, such that memory is always synchronous
// when it is read (even when in asynchronous GPU mode). Fixes Dead Cells title menu. // when it is read (even when in asynchronous GPU mode). Fixes Dead Cells title menu.
rasterizer.FlushRegion(page_table.backing_addr[page_index] + page_offset, copy_amount); rasterizer.FlushRegion(src_addr, copy_amount);
std::memcpy(dest_buffer, src_ptr, copy_amount); memory.ReadBlockUnsafe(src_addr, dest_buffer, copy_amount);
break; break;
} }
default: default:
@ -267,13 +275,15 @@ void MemoryManager::ReadBlockUnsafe(GPUVAddr src_addr, void* dest_buffer,
std::size_t page_index{src_addr >> page_bits}; std::size_t page_index{src_addr >> page_bits};
std::size_t page_offset{src_addr & page_mask}; std::size_t page_offset{src_addr & page_mask};
auto& memory = system.Memory();
while (remaining_size > 0) { while (remaining_size > 0) {
const std::size_t copy_amount{ const std::size_t copy_amount{
std::min(static_cast<std::size_t>(page_size) - page_offset, remaining_size)}; std::min(static_cast<std::size_t>(page_size) - page_offset, remaining_size)};
const u8* page_pointer = page_table.pointers[page_index]; const u8* page_pointer = page_table.pointers[page_index];
if (page_pointer) { if (page_pointer) {
const u8* src_ptr{page_pointer + page_offset}; const VAddr src_addr{page_table.backing_addr[page_index] + page_offset};
std::memcpy(dest_buffer, src_ptr, copy_amount); memory.ReadBlockUnsafe(src_addr, dest_buffer, copy_amount);
} else { } else {
std::memset(dest_buffer, 0, copy_amount); std::memset(dest_buffer, 0, copy_amount);
} }
@ -289,18 +299,19 @@ void MemoryManager::WriteBlock(GPUVAddr dest_addr, const void* src_buffer, const
std::size_t page_index{dest_addr >> page_bits}; std::size_t page_index{dest_addr >> page_bits};
std::size_t page_offset{dest_addr & page_mask}; std::size_t page_offset{dest_addr & page_mask};
auto& memory = system.Memory();
while (remaining_size > 0) { while (remaining_size > 0) {
const std::size_t copy_amount{ const std::size_t copy_amount{
std::min(static_cast<std::size_t>(page_size) - page_offset, remaining_size)}; std::min(static_cast<std::size_t>(page_size) - page_offset, remaining_size)};
switch (page_table.attributes[page_index]) { switch (page_table.attributes[page_index]) {
case Common::PageType::Memory: { case Common::PageType::Memory: {
u8* dest_ptr{page_table.pointers[page_index] + page_offset}; const VAddr dest_addr{page_table.backing_addr[page_index] + page_offset};
// Invalidate must happen on the rasterizer interface, such that memory is always // Invalidate must happen on the rasterizer interface, such that memory is always
// synchronous when it is written (even when in asynchronous GPU mode). // synchronous when it is written (even when in asynchronous GPU mode).
rasterizer.InvalidateRegion(page_table.backing_addr[page_index] + page_offset, rasterizer.InvalidateRegion(dest_addr, copy_amount);
copy_amount); memory.WriteBlockUnsafe(dest_addr, src_buffer, copy_amount);
std::memcpy(dest_ptr, src_buffer, copy_amount);
break; break;
} }
default: default:
@ -320,13 +331,15 @@ void MemoryManager::WriteBlockUnsafe(GPUVAddr dest_addr, const void* src_buffer,
std::size_t page_index{dest_addr >> page_bits}; std::size_t page_index{dest_addr >> page_bits};
std::size_t page_offset{dest_addr & page_mask}; std::size_t page_offset{dest_addr & page_mask};
auto& memory = system.Memory();
while (remaining_size > 0) { while (remaining_size > 0) {
const std::size_t copy_amount{ const std::size_t copy_amount{
std::min(static_cast<std::size_t>(page_size) - page_offset, remaining_size)}; std::min(static_cast<std::size_t>(page_size) - page_offset, remaining_size)};
u8* page_pointer = page_table.pointers[page_index]; u8* page_pointer = page_table.pointers[page_index];
if (page_pointer) { if (page_pointer) {
u8* dest_ptr{page_pointer + page_offset}; const VAddr dest_addr{page_table.backing_addr[page_index] + page_offset};
std::memcpy(dest_ptr, src_buffer, copy_amount); memory.WriteBlockUnsafe(dest_addr, src_buffer, copy_amount);
} }
page_index++; page_index++;
page_offset = 0; page_offset = 0;
@ -336,33 +349,9 @@ void MemoryManager::WriteBlockUnsafe(GPUVAddr dest_addr, const void* src_buffer,
} }
void MemoryManager::CopyBlock(GPUVAddr dest_addr, GPUVAddr src_addr, const std::size_t size) { void MemoryManager::CopyBlock(GPUVAddr dest_addr, GPUVAddr src_addr, const std::size_t size) {
std::size_t remaining_size{size}; std::vector<u8> tmp_buffer(size);
std::size_t page_index{src_addr >> page_bits}; ReadBlock(src_addr, tmp_buffer.data(), size);
std::size_t page_offset{src_addr & page_mask}; WriteBlock(dest_addr, tmp_buffer.data(), size);
while (remaining_size > 0) {
const std::size_t copy_amount{
std::min(static_cast<std::size_t>(page_size) - page_offset, remaining_size)};
switch (page_table.attributes[page_index]) {
case Common::PageType::Memory: {
// Flush must happen on the rasterizer interface, such that memory is always synchronous
// when it is copied (even when in asynchronous GPU mode).
const u8* src_ptr{page_table.pointers[page_index] + page_offset};
rasterizer.FlushRegion(page_table.backing_addr[page_index] + page_offset, copy_amount);
WriteBlock(dest_addr, src_ptr, copy_amount);
break;
}
default:
UNREACHABLE();
}
page_index++;
page_offset = 0;
dest_addr += static_cast<VAddr>(copy_amount);
src_addr += static_cast<VAddr>(copy_amount);
remaining_size -= copy_amount;
}
} }
void MemoryManager::CopyBlockUnsafe(GPUVAddr dest_addr, GPUVAddr src_addr, const std::size_t size) { void MemoryManager::CopyBlockUnsafe(GPUVAddr dest_addr, GPUVAddr src_addr, const std::size_t size) {
@ -371,6 +360,12 @@ void MemoryManager::CopyBlockUnsafe(GPUVAddr dest_addr, GPUVAddr src_addr, const
WriteBlockUnsafe(dest_addr, tmp_buffer.data(), size); WriteBlockUnsafe(dest_addr, tmp_buffer.data(), size);
} }
bool MemoryManager::IsGranularRange(GPUVAddr gpu_addr, std::size_t size) {
const VAddr addr = page_table.backing_addr[gpu_addr >> page_bits];
const std::size_t page = (addr & Memory::PAGE_MASK) + size;
return page <= Memory::PAGE_SIZE;
}
void MemoryManager::MapPages(GPUVAddr base, u64 size, u8* memory, Common::PageType type, void MemoryManager::MapPages(GPUVAddr base, u64 size, u8* memory, Common::PageType type,
VAddr backing_addr) { VAddr backing_addr) {
LOG_DEBUG(HW_GPU, "Mapping {} onto {:016X}-{:016X}", fmt::ptr(memory), base * page_size, LOG_DEBUG(HW_GPU, "Mapping {} onto {:016X}-{:016X}", fmt::ptr(memory), base * page_size,

View File

@ -97,10 +97,7 @@ public:
void WriteBlockUnsafe(GPUVAddr dest_addr, const void* src_buffer, std::size_t size); void WriteBlockUnsafe(GPUVAddr dest_addr, const void* src_buffer, std::size_t size);
void CopyBlockUnsafe(GPUVAddr dest_addr, GPUVAddr src_addr, std::size_t size); void CopyBlockUnsafe(GPUVAddr dest_addr, GPUVAddr src_addr, std::size_t size);
static bool IsGranularRange(GPUVAddr gpu_addr, std::size_t size) { bool IsGranularRange(GPUVAddr gpu_addr, std::size_t size);
const std::size_t page = (gpu_addr & page_mask) + size;
return page <= page_size;
}
private: private:
using VMAMap = std::map<GPUVAddr, VirtualMemoryArea>; using VMAMap = std::map<GPUVAddr, VirtualMemoryArea>;