Module: Mesa Branch: staging/23.0 Commit: 656b086fcfc7616d9a2c1cebb1196f3c0009fc92 URL: http://cgit.freedesktop.org/mesa/mesa/commit/?id=656b086fcfc7616d9a2c1cebb1196f3c0009fc92
Author: José Roberto de Souza <[email protected]> Date: Mon Jan 23 12:09:56 2023 -0800 intel/ds: Fix crash when allocating more intel_ds_queues than u_vector was initialized u_vector_add() don't keep the returned pointers valid. After the initial size allocated in u_vector_init() is reached it will allocate a bigger buffer and copy data from older buffer to the new one and free the old buffer, making all the previous pointers returned by u_vector_add() invalid and crashing the application when trying to access it. This is reproduced when running dEQP-VK.synchronization.signal_order.timeline_semaphore.* in DG2 SKUs that has 4 CCS engines, INTEL_COMPUTE_CLASS=1 is set and of course perfetto build is enabled. To fix this issue here I'm moving the storage/allocation of struct intel_ds_queue to struct anv_queue/iris_batch and using struct list_head to maintain a chain of intel_ds_queue of the intel_ds_device. This allows us to append or remove queues dynamically in future if necessary. Fixes: e760c5b37be9 ("anv: add perfetto source") Reviewed-by: Lionel Landwerlin <[email protected]> Signed-off-by: José Roberto de Souza <[email protected]> Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20977> (cherry picked from commit 8092bc2158ebb8a5f85e0ec569387c5dcd0d1627) Conflicts: src/intel/ds/intel_driver_ds.cc src/intel/vulkan/anv_utrace.c src/intel/vulkan_hasvk/anv_utrace.c --- .pick_status.json | 2 +- src/gallium/drivers/iris/iris_batch.c | 6 ++-- src/gallium/drivers/iris/iris_batch.h | 2 +- src/gallium/drivers/iris/iris_utrace.c | 7 ++--- src/intel/ds/intel_driver_ds.cc | 22 +++++++------- src/intel/ds/intel_driver_ds.h | 12 +++++--- src/intel/vulkan/anv_batch_chain.c | 4 +-- src/intel/vulkan/anv_private.h | 2 +- src/intel/vulkan/anv_utrace.c | 51 ++++++++++++++++---------------- src/intel/vulkan_hasvk/anv_batch_chain.c | 4 +-- src/intel/vulkan_hasvk/anv_private.h | 2 +- src/intel/vulkan_hasvk/anv_utrace.c | 9 +++--- 12 files changed, 61 insertions(+), 62 deletions(-) diff --git a/.pick_status.json b/.pick_status.json index 06d2b9f75e7..70e669dcc39 100644 --- a/.pick_status.json +++ b/.pick_status.json @@ -2785,7 +2785,7 @@ "description": "intel/ds: Fix crash when allocating more intel_ds_queues than u_vector was initialized", "nominated": true, "nomination_type": 1, - "resolution": 0, + "resolution": 1, "main_sha": null, "because_sha": "e760c5b37be938427a9c88182ea99f7f66721ca3" }, diff --git a/src/gallium/drivers/iris/iris_batch.c b/src/gallium/drivers/iris/iris_batch.c index f2f9d14b67c..054c27e0797 100644 --- a/src/gallium/drivers/iris/iris_batch.c +++ b/src/gallium/drivers/iris/iris_batch.c @@ -1047,10 +1047,10 @@ _iris_batch_flush(struct iris_batch *batch, const char *file, int line) } - uint64_t start_ts = intel_ds_begin_submit(batch->ds); - uint64_t submission_id = batch->ds->submission_id; + uint64_t start_ts = intel_ds_begin_submit(&batch->ds); + uint64_t submission_id = batch->ds.submission_id; int ret = submit_batch(batch); - intel_ds_end_submit(batch->ds, start_ts); + intel_ds_end_submit(&batch->ds, start_ts); /* When batch submission fails, our end-of-batch syncobj remains * unsignalled, and in fact is not even considered submitted. diff --git a/src/gallium/drivers/iris/iris_batch.h b/src/gallium/drivers/iris/iris_batch.h index a03d6bb3d89..d9c77cfae95 100644 --- a/src/gallium/drivers/iris/iris_batch.h +++ b/src/gallium/drivers/iris/iris_batch.h @@ -196,7 +196,7 @@ struct iris_batch { struct u_trace trace; /** Batch wrapper structure for perfetto */ - struct intel_ds_queue *ds; + struct intel_ds_queue ds; }; void iris_init_batches(struct iris_context *ice, int priority); diff --git a/src/gallium/drivers/iris/iris_utrace.c b/src/gallium/drivers/iris/iris_utrace.c index 16ac6eea589..415a4f416e5 100644 --- a/src/gallium/drivers/iris/iris_utrace.c +++ b/src/gallium/drivers/iris/iris_utrace.c @@ -95,7 +95,7 @@ iris_utrace_delete_flush_data(struct u_trace_context *utctx, void iris_utrace_flush(struct iris_batch *batch, uint64_t submission_id) { struct intel_ds_flush_data *flush_data = malloc(sizeof(*flush_data)); - intel_ds_flush_data_init(flush_data, batch->ds, submission_id); + intel_ds_flush_data_init(flush_data, &batch->ds, submission_id); u_trace_flush(&batch->trace, flush_data, false); } @@ -122,9 +122,8 @@ void iris_utrace_init(struct iris_context *ice) iris_utrace_delete_flush_data); for (int i = 0; i < IRIS_BATCH_COUNT; i++) { - ice->batches[i].ds = - intel_ds_device_add_queue(&ice->ds, "%s", - iris_batch_name_to_string(i)); + intel_ds_device_init_queue(&ice->ds, &ice->batches[i].ds, "%s", + iris_batch_name_to_string(i)); } } diff --git a/src/intel/ds/intel_driver_ds.cc b/src/intel/ds/intel_driver_ds.cc index f221b6bc516..d7f97fbf64c 100644 --- a/src/intel/ds/intel_driver_ds.cc +++ b/src/intel/ds/intel_driver_ds.cc @@ -190,12 +190,10 @@ static void send_descriptors(IntelRenderpassDataSource::TraceContext &ctx, struct intel_ds_device *device) { - struct intel_ds_queue *queue; - PERFETTO_LOG("Sending renderstage descriptors"); device->event_id = 0; - u_vector_foreach(queue, &device->queues) { + list_for_each_entry_safe(struct intel_ds_queue, queue, &device->queues, link) { for (uint32_t s = 0; s < ARRAY_SIZE(queue->stages); s++) { queue->stages[s].start_ns = 0; } @@ -227,7 +225,7 @@ send_descriptors(IntelRenderpassDataSource::TraceContext &ctx, } /* Emit all the IID picked at device/queue creation. */ - u_vector_foreach(queue, &device->queues) { + list_for_each_entry_safe(struct intel_ds_queue, queue, &device->queues, link) { for (unsigned s = 0; s < INTEL_DS_QUEUE_STAGE_N_STAGES; s++) { { /* We put the stage number in there so that all rows are order @@ -534,29 +532,27 @@ intel_ds_device_init(struct intel_ds_device *device, device->info = *devinfo; device->iid = get_iid(); device->api = api; - u_vector_init(&device->queues, 4, sizeof(struct intel_ds_queue)); + list_inithead(&device->queues); } void intel_ds_device_fini(struct intel_ds_device *device) { u_trace_context_fini(&device->trace_context); - u_vector_finish(&device->queues); } struct intel_ds_queue * -intel_ds_device_add_queue(struct intel_ds_device *device, - const char *fmt_name, - ...) +intel_ds_device_init_queue(struct intel_ds_device *device, + struct intel_ds_queue *queue, + const char *fmt_name, + ...) { - struct intel_ds_queue *queue = - (struct intel_ds_queue *) u_vector_add(&device->queues); va_list ap; memset(queue, 0, sizeof(*queue)); queue->device = device; - queue->queue_id = u_vector_length(&device->queues) - 1; + queue->queue_id = list_length(&device->queues) - 1; va_start(ap, fmt_name); vsnprintf(queue->name, sizeof(queue->name), fmt_name, ap); @@ -567,6 +563,8 @@ intel_ds_device_add_queue(struct intel_ds_device *device, queue->stages[s].stage_iid = get_iid(); } + list_add(&queue->link, &device->queues); + return queue; } diff --git a/src/intel/ds/intel_driver_ds.h b/src/intel/ds/intel_driver_ds.h index 243c161785e..7f844b7eca3 100644 --- a/src/intel/ds/intel_driver_ds.h +++ b/src/intel/ds/intel_driver_ds.h @@ -109,7 +109,7 @@ struct intel_ds_device { struct u_trace_context trace_context; /* List of intel_ds_queue */ - struct u_vector queues; + struct list_head queues; }; struct intel_ds_stage { @@ -124,6 +124,8 @@ struct intel_ds_stage { }; struct intel_ds_queue { + struct list_head link; + /* Device this queue belongs to */ struct intel_ds_device *device; @@ -160,9 +162,11 @@ void intel_ds_device_init(struct intel_ds_device *device, enum intel_ds_api api); void intel_ds_device_fini(struct intel_ds_device *device); -struct intel_ds_queue *intel_ds_device_add_queue(struct intel_ds_device *device, - const char *fmt_name, - ...); +struct intel_ds_queue * +intel_ds_device_init_queue(struct intel_ds_device *device, + struct intel_ds_queue *queue, + const char *fmt_name, + ...); void intel_ds_flush_data_init(struct intel_ds_flush_data *data, struct intel_ds_queue *queue, diff --git a/src/intel/vulkan/anv_batch_chain.c b/src/intel/vulkan/anv_batch_chain.c index 5cca2a5536b..02aaab7ef52 100644 --- a/src/intel/vulkan/anv_batch_chain.c +++ b/src/intel/vulkan/anv_batch_chain.c @@ -2040,14 +2040,14 @@ anv_queue_submit(struct vk_queue *vk_queue, return VK_SUCCESS; } - uint64_t start_ts = intel_ds_begin_submit(queue->ds); + uint64_t start_ts = intel_ds_begin_submit(&queue->ds); pthread_mutex_lock(&device->mutex); result = anv_queue_submit_locked(queue, submit); /* Take submission ID under lock */ pthread_mutex_unlock(&device->mutex); - intel_ds_end_submit(queue->ds, start_ts); + intel_ds_end_submit(&queue->ds, start_ts); return result; } diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h index 4cce89582f8..72eeb342ab6 100644 --- a/src/intel/vulkan/anv_private.h +++ b/src/intel/vulkan/anv_private.h @@ -1056,7 +1056,7 @@ struct anv_queue { /** Synchronization object for debug purposes (DEBUG_SYNC) */ struct vk_sync *sync; - struct intel_ds_queue * ds; + struct intel_ds_queue ds; }; struct nir_xfb_info; diff --git a/src/intel/vulkan/anv_utrace.c b/src/intel/vulkan/anv_utrace.c index 13e42e4aaf3..3f081c4050b 100644 --- a/src/intel/vulkan/anv_utrace.c +++ b/src/intel/vulkan/anv_utrace.c @@ -1,25 +1,25 @@ /* - * Copyright © 2021 Intel Corporation - * - * Permission is hereby granted, free of charge, to any person obtaining a - * copy of this software and associated documentation files (the "Software"), - * to deal in the Software without restriction, including without limitation - * the rights to use, copy, modify, merge, publish, distribute, sublicense, - * and/or sell copies of the Software, and to permit persons to whom the - * Software is furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice (including the next - * paragraph) shall be included in all copies or substantial portions of the - * Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING - * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS - * IN THE SOFTWARE. - */ +* Copyright © 2021 Intel Corporation +* +* Permission is hereby granted, free of charge, to any person obtaining a +* copy of this software and associated documentation files (the "Software"), +* to deal in the Software without restriction, including without limitation +* the rights to use, copy, modify, merge, publish, distribute, sublicense, +* and/or sell copies of the Software, and to permit persons to whom the +* Software is furnished to do so, subject to the following conditions: +* +* The above copyright notice and this permission notice (including the next +* paragraph) shall be included in all copies or substantial portions of the +* Software. +* +* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL +* THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS +* IN THE SOFTWARE. +*/ #include "anv_private.h" @@ -111,7 +111,7 @@ anv_device_utrace_flush_cmd_buffers(struct anv_queue *queue, if (!flush) return vk_error(device, VK_ERROR_OUT_OF_HOST_MEMORY); - intel_ds_flush_data_init(&flush->ds, queue->ds, queue->ds->submission_id); + intel_ds_flush_data_init(&flush->ds, &queue->ds, queue->ds.submission_id); result = vk_sync_create(&device->vk, &device->physical->sync_syncobj_type, 0, 0, &flush->sync); @@ -284,10 +284,9 @@ anv_device_utrace_init(struct anv_device *device) for (uint32_t q = 0; q < device->queue_count; q++) { struct anv_queue *queue = &device->queues[q]; - queue->ds = - intel_ds_device_add_queue(&device->ds, "%s%u", - intel_engines_class_to_string(queue->family->engine_class), - queue->index_in_family); + intel_ds_device_init_queue(&device->ds, &queue->ds, "%s%u", + intel_engines_class_to_string(queue->family->engine_class), + queue->vk.index_in_family); } } diff --git a/src/intel/vulkan_hasvk/anv_batch_chain.c b/src/intel/vulkan_hasvk/anv_batch_chain.c index ea4678e6b7c..a13c1670c63 100644 --- a/src/intel/vulkan_hasvk/anv_batch_chain.c +++ b/src/intel/vulkan_hasvk/anv_batch_chain.c @@ -2385,14 +2385,14 @@ anv_queue_submit(struct vk_queue *vk_queue, return VK_SUCCESS; } - uint64_t start_ts = intel_ds_begin_submit(queue->ds); + uint64_t start_ts = intel_ds_begin_submit(&queue->ds); pthread_mutex_lock(&device->mutex); result = anv_queue_submit_locked(queue, submit); /* Take submission ID under lock */ pthread_mutex_unlock(&device->mutex); - intel_ds_end_submit(queue->ds, start_ts); + intel_ds_end_submit(&queue->ds, start_ts); return result; } diff --git a/src/intel/vulkan_hasvk/anv_private.h b/src/intel/vulkan_hasvk/anv_private.h index e53440253c8..7bc7ef995c7 100644 --- a/src/intel/vulkan_hasvk/anv_private.h +++ b/src/intel/vulkan_hasvk/anv_private.h @@ -974,7 +974,7 @@ struct anv_queue { /** Synchronization object for debug purposes (DEBUG_SYNC) */ struct vk_sync *sync; - struct intel_ds_queue * ds; + struct intel_ds_queue ds; }; struct nir_xfb_info; diff --git a/src/intel/vulkan_hasvk/anv_utrace.c b/src/intel/vulkan_hasvk/anv_utrace.c index d74d6f6daee..d78ab07efba 100644 --- a/src/intel/vulkan_hasvk/anv_utrace.c +++ b/src/intel/vulkan_hasvk/anv_utrace.c @@ -111,7 +111,7 @@ anv_device_utrace_flush_cmd_buffers(struct anv_queue *queue, if (!flush) return vk_error(device, VK_ERROR_OUT_OF_HOST_MEMORY); - intel_ds_flush_data_init(&flush->ds, queue->ds, queue->ds->submission_id); + intel_ds_flush_data_init(&flush->ds, &queue->ds, queue->ds.submission_id); result = vk_sync_create(&device->vk, &device->physical->sync_syncobj_type, 0, 0, &flush->sync); @@ -284,10 +284,9 @@ anv_device_utrace_init(struct anv_device *device) for (uint32_t q = 0; q < device->queue_count; q++) { struct anv_queue *queue = &device->queues[q]; - queue->ds = - intel_ds_device_add_queue(&device->ds, "%s%u", - intel_engines_class_to_string(queue->family->engine_class), - queue->index_in_family); + intel_ds_device_init_queue(&device->ds, &queue->ds, "%s%u", + intel_engines_class_to_string(queue->family->engine_class), + queue->vk.index_in_family); } }
