anv: Fix a relocation race condition
Previously, we would read the offset from the BO in anv_reloc_list_add to generate the presumed offset and then again in the caller to compute the 64-bit address to write into the buffer. However, if the offset somehow changed between these two points, the presumed offset would no longer match the written offset. This is unlikely to actually ever be a problem in practice because the presumed offset gets recorded first and so if the written address is wrong then the presumed offset is almost certainly wrong and the relocation will trigger. However, it's much safer to simply have anv_reloc_list_add return the 64-bit address. Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
This commit is contained in:
@@ -143,14 +143,21 @@ anv_reloc_list_grow(struct anv_reloc_list *list,
|
|||||||
return VK_SUCCESS;
|
return VK_SUCCESS;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#define READ_ONCE(x) (*(volatile __typeof__(x) *)&(x))
|
||||||
|
|
||||||
VkResult
|
VkResult
|
||||||
anv_reloc_list_add(struct anv_reloc_list *list,
|
anv_reloc_list_add(struct anv_reloc_list *list,
|
||||||
const VkAllocationCallbacks *alloc,
|
const VkAllocationCallbacks *alloc,
|
||||||
uint32_t offset, struct anv_bo *target_bo, uint32_t delta)
|
uint32_t offset, struct anv_bo *target_bo, uint32_t delta,
|
||||||
|
uint64_t *address_u64_out)
|
||||||
{
|
{
|
||||||
struct drm_i915_gem_relocation_entry *entry;
|
struct drm_i915_gem_relocation_entry *entry;
|
||||||
int index;
|
int index;
|
||||||
|
|
||||||
|
uint64_t target_bo_offset = READ_ONCE(target_bo->offset);
|
||||||
|
if (address_u64_out)
|
||||||
|
*address_u64_out = target_bo_offset + delta;
|
||||||
|
|
||||||
if (target_bo->flags & EXEC_OBJECT_PINNED) {
|
if (target_bo->flags & EXEC_OBJECT_PINNED) {
|
||||||
if (list->deps == NULL) {
|
if (list->deps == NULL) {
|
||||||
list->deps = _mesa_pointer_set_create(NULL);
|
list->deps = _mesa_pointer_set_create(NULL);
|
||||||
@@ -172,7 +179,7 @@ anv_reloc_list_add(struct anv_reloc_list *list,
|
|||||||
entry->target_handle = target_bo->gem_handle;
|
entry->target_handle = target_bo->gem_handle;
|
||||||
entry->delta = delta;
|
entry->delta = delta;
|
||||||
entry->offset = offset;
|
entry->offset = offset;
|
||||||
entry->presumed_offset = target_bo->offset;
|
entry->presumed_offset = target_bo_offset;
|
||||||
entry->read_domains = 0;
|
entry->read_domains = 0;
|
||||||
entry->write_domain = 0;
|
entry->write_domain = 0;
|
||||||
VG(VALGRIND_CHECK_MEM_IS_DEFINED(entry, sizeof(*entry)));
|
VG(VALGRIND_CHECK_MEM_IS_DEFINED(entry, sizeof(*entry)));
|
||||||
@@ -241,14 +248,16 @@ uint64_t
|
|||||||
anv_batch_emit_reloc(struct anv_batch *batch,
|
anv_batch_emit_reloc(struct anv_batch *batch,
|
||||||
void *location, struct anv_bo *bo, uint32_t delta)
|
void *location, struct anv_bo *bo, uint32_t delta)
|
||||||
{
|
{
|
||||||
|
uint64_t address_u64 = 0;
|
||||||
VkResult result = anv_reloc_list_add(batch->relocs, batch->alloc,
|
VkResult result = anv_reloc_list_add(batch->relocs, batch->alloc,
|
||||||
location - batch->start, bo, delta);
|
location - batch->start, bo, delta,
|
||||||
|
&address_u64);
|
||||||
if (result != VK_SUCCESS) {
|
if (result != VK_SUCCESS) {
|
||||||
anv_batch_set_error(batch, result);
|
anv_batch_set_error(batch, result);
|
||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
return bo->offset + delta;
|
return address_u64;
|
||||||
}
|
}
|
||||||
|
|
||||||
void
|
void
|
||||||
|
@@ -1317,7 +1317,7 @@ void anv_reloc_list_finish(struct anv_reloc_list *list,
|
|||||||
VkResult anv_reloc_list_add(struct anv_reloc_list *list,
|
VkResult anv_reloc_list_add(struct anv_reloc_list *list,
|
||||||
const VkAllocationCallbacks *alloc,
|
const VkAllocationCallbacks *alloc,
|
||||||
uint32_t offset, struct anv_bo *target_bo,
|
uint32_t offset, struct anv_bo *target_bo,
|
||||||
uint32_t delta);
|
uint32_t delta, uint64_t *address_u64_out);
|
||||||
|
|
||||||
struct anv_batch_bo {
|
struct anv_batch_bo {
|
||||||
/* Link in the anv_cmd_buffer.owned_batch_bos list */
|
/* Link in the anv_cmd_buffer.owned_batch_bos list */
|
||||||
|
@@ -57,17 +57,17 @@ blorp_surface_reloc(struct blorp_batch *batch, uint32_t ss_offset,
|
|||||||
struct blorp_address address, uint32_t delta)
|
struct blorp_address address, uint32_t delta)
|
||||||
{
|
{
|
||||||
struct anv_cmd_buffer *cmd_buffer = batch->driver_batch;
|
struct anv_cmd_buffer *cmd_buffer = batch->driver_batch;
|
||||||
|
uint64_t address_u64 = 0;
|
||||||
VkResult result =
|
VkResult result =
|
||||||
anv_reloc_list_add(&cmd_buffer->surface_relocs, &cmd_buffer->pool->alloc,
|
anv_reloc_list_add(&cmd_buffer->surface_relocs, &cmd_buffer->pool->alloc,
|
||||||
ss_offset, address.buffer, address.offset + delta);
|
ss_offset, address.buffer, address.offset + delta,
|
||||||
|
&address_u64);
|
||||||
if (result != VK_SUCCESS)
|
if (result != VK_SUCCESS)
|
||||||
anv_batch_set_error(&cmd_buffer->batch, result);
|
anv_batch_set_error(&cmd_buffer->batch, result);
|
||||||
|
|
||||||
void *dest = anv_block_pool_map(
|
void *dest = anv_block_pool_map(
|
||||||
&cmd_buffer->device->surface_state_pool.block_pool, ss_offset);
|
&cmd_buffer->device->surface_state_pool.block_pool, ss_offset);
|
||||||
uint64_t val = ((struct anv_bo*)address.buffer)->offset + address.offset +
|
write_reloc(cmd_buffer->device, dest, address_u64, false);
|
||||||
delta;
|
|
||||||
write_reloc(cmd_buffer->device, dest, val, false);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
static uint64_t
|
static uint64_t
|
||||||
|
@@ -207,7 +207,7 @@ add_surface_reloc(struct anv_cmd_buffer *cmd_buffer,
|
|||||||
VkResult result =
|
VkResult result =
|
||||||
anv_reloc_list_add(&cmd_buffer->surface_relocs, &cmd_buffer->pool->alloc,
|
anv_reloc_list_add(&cmd_buffer->surface_relocs, &cmd_buffer->pool->alloc,
|
||||||
state.offset + isl_dev->ss.addr_offset,
|
state.offset + isl_dev->ss.addr_offset,
|
||||||
addr.bo, addr.offset);
|
addr.bo, addr.offset, NULL);
|
||||||
if (result != VK_SUCCESS)
|
if (result != VK_SUCCESS)
|
||||||
anv_batch_set_error(&cmd_buffer->batch, result);
|
anv_batch_set_error(&cmd_buffer->batch, result);
|
||||||
}
|
}
|
||||||
@@ -226,7 +226,9 @@ add_surface_state_relocs(struct anv_cmd_buffer *cmd_buffer,
|
|||||||
anv_reloc_list_add(&cmd_buffer->surface_relocs,
|
anv_reloc_list_add(&cmd_buffer->surface_relocs,
|
||||||
&cmd_buffer->pool->alloc,
|
&cmd_buffer->pool->alloc,
|
||||||
state.state.offset + isl_dev->ss.aux_addr_offset,
|
state.state.offset + isl_dev->ss.aux_addr_offset,
|
||||||
state.aux_address.bo, state.aux_address.offset);
|
state.aux_address.bo,
|
||||||
|
state.aux_address.offset,
|
||||||
|
NULL);
|
||||||
if (result != VK_SUCCESS)
|
if (result != VK_SUCCESS)
|
||||||
anv_batch_set_error(&cmd_buffer->batch, result);
|
anv_batch_set_error(&cmd_buffer->batch, result);
|
||||||
}
|
}
|
||||||
@@ -237,7 +239,9 @@ add_surface_state_relocs(struct anv_cmd_buffer *cmd_buffer,
|
|||||||
&cmd_buffer->pool->alloc,
|
&cmd_buffer->pool->alloc,
|
||||||
state.state.offset +
|
state.state.offset +
|
||||||
isl_dev->ss.clear_color_state_offset,
|
isl_dev->ss.clear_color_state_offset,
|
||||||
state.clear_address.bo, state.clear_address.offset);
|
state.clear_address.bo,
|
||||||
|
state.clear_address.offset,
|
||||||
|
NULL);
|
||||||
if (result != VK_SUCCESS)
|
if (result != VK_SUCCESS)
|
||||||
anv_batch_set_error(&cmd_buffer->batch, result);
|
anv_batch_set_error(&cmd_buffer->batch, result);
|
||||||
}
|
}
|
||||||
|
Reference in New Issue
Block a user