D3D12: Remove the 24K extra memory for texture corruption

2D array texture may corrupt on some Intel devices, making out-of-bound
texture access and memory information leak. It's a critical security
issue. Intel driver team suggested the 24K extra memory approach in
order to mitigate the security issue before.

However, the texture corruption issue (and even the correctness issue)
can be worked around via allocating a few extra layers. And patches
have already been merged in Dawn, with a lot tests for verification. The
24K extra memory for each texture is actually incorrect and unnecessary.
So this patch removes relevant code in Dawn.

This patch mainly reverts some code of this patch below:
https://dawn-review.googlesource.com/c/dawn/+/96220

Bug: dawn:1507
Change-Id: Ic3239115ad4c74bdee928577ccbb20f1e35d13c3
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/114641
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Yunchao He <yunchao.he@intel.com>
This commit is contained in:
Yunchao He 2022-12-16 16:46:12 +00:00 committed by Dawn LUCI CQ
parent 7c3e9a6dd2
commit 5082727e55
2 changed files with 14 additions and 65 deletions

View File

@ -25,8 +25,6 @@
#include "dawn/native/d3d12/ResidencyManagerD3D12.h"
#include "dawn/native/d3d12/UtilsD3D12.h"
static constexpr uint32_t kExtraMemoryToMitigateTextureCorruption = 24576u;
namespace dawn::native::d3d12 {
namespace {
MemorySegment GetMemorySegment(Device* device, D3D12_HEAP_TYPE heapType) {
@ -475,8 +473,6 @@ ResultOrError<ResourceHeapAllocation> ResourceAllocatorManager::CreatePlacedReso
mDevice->GetD3D12Device()->GetResourceAllocationInfo(0, 1, &resourceDescriptor);
}
resourceInfo.SizeInBytes += GetResourcePadding(resourceDescriptor);
// If d3d tells us the resource size is invalid, treat the error as OOM.
// Otherwise, creating the resource could cause a device loss (too large).
// This is because NextPowerOfTwo(UINT64_MAX) overflows and proceeds to
@ -499,21 +495,6 @@ ResultOrError<ResourceHeapAllocation> ResourceAllocatorManager::CreatePlacedReso
Heap* heap = ToBackend(allocation.GetResourceHeap());
ComPtr<ID3D12Resource> placedResource;
DAWN_TRY_ASSIGN(placedResource,
CreatePlacedResourceInHeap(heap, allocation.GetOffset(), resourceDescriptor,
optimizedClearValue, initialUsage));
return ResourceHeapAllocation{allocation.GetInfo(), allocation.GetOffset(),
std::move(placedResource), heap};
}
ResultOrError<ComPtr<ID3D12Resource>> ResourceAllocatorManager::CreatePlacedResourceInHeap(
Heap* heap,
const uint64_t offset,
const D3D12_RESOURCE_DESC& resourceDescriptor,
const D3D12_CLEAR_VALUE* optimizedClearValue,
D3D12_RESOURCE_STATES initialUsage) {
ComPtr<ID3D12Resource> placedResource;
// Before calling CreatePlacedResource, we must ensure the target heap is resident.
// CreatePlacedResource will fail if it is not.
DAWN_TRY(mDevice->GetResidencyManager()->LockAllocation(heap));
@ -525,16 +506,19 @@ ResultOrError<ComPtr<ID3D12Resource>> ResourceAllocatorManager::CreatePlacedReso
// within the same command-list and does not require additional synchronization (aliasing
// barrier).
// https://docs.microsoft.com/en-us/windows/win32/api/d3d12/nf-d3d12-id3d12device-createplacedresource
DAWN_TRY(
CheckOutOfMemoryHRESULT(mDevice->GetD3D12Device()->CreatePlacedResource(
heap->GetD3D12Heap(), offset, &resourceDescriptor, initialUsage,
optimizedClearValue, IID_PPV_ARGS(&placedResource)),
"ID3D12Device::CreatePlacedResource"));
ComPtr<ID3D12Resource> placedResource;
DAWN_TRY(CheckOutOfMemoryHRESULT(
mDevice->GetD3D12Device()->CreatePlacedResource(
heap->GetD3D12Heap(), allocation.GetOffset(), &resourceDescriptor, initialUsage,
optimizedClearValue, IID_PPV_ARGS(&placedResource)),
"ID3D12Device::CreatePlacedResource"));
// After CreatePlacedResource has finished, the heap can be unlocked from residency. This
// will insert it into the residency LRU.
mDevice->GetResidencyManager()->UnlockAllocation(heap);
return std::move(placedResource);
return ResourceHeapAllocation{allocation.GetInfo(), allocation.GetOffset(),
std::move(placedResource), heap};
}
ResultOrError<ResourceHeapAllocation> ResourceAllocatorManager::CreateCommittedResource(
@ -556,9 +540,6 @@ ResultOrError<ResourceHeapAllocation> ResourceAllocatorManager::CreateCommittedR
D3D12_RESOURCE_ALLOCATION_INFO resourceInfo =
mDevice->GetD3D12Device()->GetResourceAllocationInfo(0, 1, &resourceDescriptor);
uint64_t extraMemory = GetResourcePadding(resourceDescriptor);
resourceInfo.SizeInBytes += extraMemory;
if (resourceInfo.SizeInBytes == 0 ||
resourceInfo.SizeInBytes == std::numeric_limits<uint64_t>::max()) {
return DAWN_OUT_OF_MEMORY_ERROR("Resource allocation size was invalid.");
@ -577,23 +558,11 @@ ResultOrError<ResourceHeapAllocation> ResourceAllocatorManager::CreateCommittedR
// Note: Heap flags are inferred by the resource descriptor and do not need to be explicitly
// provided to CreateCommittedResource.
ComPtr<ID3D12Resource> committedResource;
if (extraMemory > 0) {
const ResourceHeapKind resourceHeapKind = GetResourceHeapKind(
resourceDescriptor.Dimension, heapType, resourceDescriptor.Flags, mResourceHeapTier);
std::unique_ptr<ResourceHeapBase> heapBase;
DAWN_TRY_ASSIGN(heapBase, mPooledHeapAllocators[resourceHeapKind]->AllocateResourceHeap(
resourceInfo.SizeInBytes));
Heap* heap = ToBackend(heapBase.get());
DAWN_TRY_ASSIGN(committedResource,
CreatePlacedResourceInHeap(heap, 0, resourceDescriptor, optimizedClearValue,
initialUsage));
} else {
DAWN_TRY(CheckOutOfMemoryHRESULT(
mDevice->GetD3D12Device()->CreateCommittedResource(
&heapProperties, D3D12_HEAP_FLAG_NONE, &resourceDescriptor, initialUsage,
optimizedClearValue, IID_PPV_ARGS(&committedResource)),
"ID3D12Device::CreateCommittedResource"));
}
DAWN_TRY(CheckOutOfMemoryHRESULT(
mDevice->GetD3D12Device()->CreateCommittedResource(
&heapProperties, D3D12_HEAP_FLAG_NONE, &resourceDescriptor, initialUsage,
optimizedClearValue, IID_PPV_ARGS(&committedResource)),
"ID3D12Device::CreateCommittedResource"));
// When using CreateCommittedResource, D3D12 creates an implicit heap that contains the
// resource allocation. Because Dawn's memory residency management occurs at the resource
@ -614,17 +583,6 @@ ResultOrError<ResourceHeapAllocation> ResourceAllocatorManager::CreateCommittedR
/*offset*/ 0, std::move(committedResource), heap};
}
uint64_t ResourceAllocatorManager::GetResourcePadding(
const D3D12_RESOURCE_DESC& resourceDescriptor) const {
// If we are allocating memory for a 2D array texture on D3D12 backend, we need to allocate
// extra memory on some devices, see crbug.com/dawn/949 for details.
if (mDevice->IsToggleEnabled(Toggle::D3D12AllocateExtraMemoryFor2DArrayColorTexture) &&
resourceDescriptor.Dimension == D3D12_RESOURCE_DIMENSION_TEXTURE2D &&
resourceDescriptor.DepthOrArraySize > 1) {
return kExtraMemoryToMitigateTextureCorruption;
}
return 0;
}
void ResourceAllocatorManager::DestroyPool() {
for (auto& alloc : mPooledHeapAllocators) {
alloc->DestroyPool();

View File

@ -87,15 +87,6 @@ class ResourceAllocatorManager {
const D3D12_CLEAR_VALUE* optimizedClearValue,
D3D12_RESOURCE_STATES initialUsage);
ResultOrError<ComPtr<ID3D12Resource>> CreatePlacedResourceInHeap(
Heap* heap,
const uint64_t offset,
const D3D12_RESOURCE_DESC& resourceDescriptor,
const D3D12_CLEAR_VALUE* optimizedClearValue,
D3D12_RESOURCE_STATES initialUsage);
uint64_t GetResourcePadding(const D3D12_RESOURCE_DESC& resourceDescriptor) const;
void DestroyPool();
Device* mDevice;