Module: Mesa Branch: main Commit: 3a949de28c42d8714320e56bd99168148503da7d URL: http://cgit.freedesktop.org/mesa/mesa/commit/?id=3a949de28c42d8714320e56bd99168148503da7d
Author: Vlad Schiller <[email protected]> Date: Thu Aug 10 13:14:38 2023 +0100 pvr: Remove PVR_WINSYS_BO_FLAG_ZERO_ON_ALLOC flag There has been a recent change to the new powervr KMD to always zero buffer objects at allocation time to avoid information leaks. This change was made to address upstream feedback [1]. The result is that the PVR_WINSYS_BO_FLAG_ZERO_ON_ALLOC no longer makes a difference when using this KMD. As the powervr KMD is the one we actually care about, it makes sense to mirror this change when using the downstream pvrsrvkm KMD in order to avoid differences in behaviour between the two KMDs. As this makes the PVR_WINSYS_BO_FLAG_ZERO_ON_ALLOC flag entirely redundant, remove it. [1] https://lists.freedesktop.org/archives/dri-devel/2023-August/418042.html Signed-off-by: Vlad Schiller <[email protected]> Reviewed-by: Frank Binns <[email protected]> Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/24930> --- src/imagination/common/pvr_debug.c | 6 +----- src/imagination/common/pvr_debug.h | 5 ++--- src/imagination/vulkan/pvr_bo.c | 9 +-------- src/imagination/vulkan/pvr_bo.h | 5 ----- src/imagination/vulkan/pvr_job_render.c | 8 ++------ src/imagination/vulkan/winsys/pvr_winsys.h | 5 ----- src/imagination/vulkan/winsys/pvrsrvkm/pvr_srv_bo.c | 17 +++++------------ 7 files changed, 11 insertions(+), 44 deletions(-) diff --git a/src/imagination/common/pvr_debug.c b/src/imagination/common/pvr_debug.c index f92414ff8b3..e910f4b217f 100644 --- a/src/imagination/common/pvr_debug.c +++ b/src/imagination/common/pvr_debug.c @@ -34,8 +34,6 @@ static const struct debug_named_value debug_control[] = { "Dump the contents of the control stream buffer on every job submit." }, { "bo_track", PVR_DEBUG_TRACK_BOS, "Track all buffer objects with at least one reference." }, - { "bo_zero", PVR_DEBUG_ZERO_BOS, - "Zero all buffer objects at allocation to make them deterministic." }, { "vk_desc", PVR_DEBUG_VK_DUMP_DESCRIPTOR_SET_LAYOUT, "Dump descriptor set and pipeline layouts." }, { "info", PVR_DEBUG_INFO, @@ -56,8 +54,6 @@ void pvr_process_debug_variable(void) * implies another it should be set here. */ - if (PVR_IS_DEBUG_SET(DUMP_CONTROL_STREAM)) { + if (PVR_IS_DEBUG_SET(DUMP_CONTROL_STREAM)) PVR_DEBUG_SET(TRACK_BOS); - PVR_DEBUG_SET(ZERO_BOS); - } } diff --git a/src/imagination/common/pvr_debug.h b/src/imagination/common/pvr_debug.h index 3aa08795299..33d22424ab1 100644 --- a/src/imagination/common/pvr_debug.h +++ b/src/imagination/common/pvr_debug.h @@ -36,9 +36,8 @@ extern uint32_t PVR_DEBUG; #define PVR_DEBUG_DUMP_CONTROL_STREAM BITFIELD_BIT(0) #define PVR_DEBUG_TRACK_BOS BITFIELD_BIT(1) -#define PVR_DEBUG_ZERO_BOS BITFIELD_BIT(2) -#define PVR_DEBUG_VK_DUMP_DESCRIPTOR_SET_LAYOUT BITFIELD_BIT(3) -#define PVR_DEBUG_INFO BITFIELD_BIT(4) +#define PVR_DEBUG_VK_DUMP_DESCRIPTOR_SET_LAYOUT BITFIELD_BIT(2) +#define PVR_DEBUG_INFO BITFIELD_BIT(3) void pvr_process_debug_variable(void); diff --git a/src/imagination/vulkan/pvr_bo.c b/src/imagination/vulkan/pvr_bo.c index cce1ae5eaaf..cf3da154537 100644 --- a/src/imagination/vulkan/pvr_bo.c +++ b/src/imagination/vulkan/pvr_bo.c @@ -281,9 +281,6 @@ static uint32_t pvr_bo_alloc_to_winsys_flags(uint64_t flags) if (flags & PVR_BO_ALLOC_FLAG_PM_FW_PROTECT) ws_flags |= PVR_WINSYS_BO_FLAG_PM_FW_PROTECT; - if (flags & PVR_BO_ALLOC_FLAG_ZERO_ON_ALLOC) - ws_flags |= PVR_WINSYS_BO_FLAG_ZERO_ON_ALLOC; - return ws_flags; } @@ -349,9 +346,6 @@ VkResult pvr_bo_alloc(struct pvr_device *device, struct pvr_bo *pvr_bo; VkResult result; - if (PVR_IS_DEBUG_SET(ZERO_BOS)) - flags |= PVR_BO_ALLOC_FLAG_ZERO_ON_ALLOC; - pvr_bo = pvr_bo_alloc_bo(device); if (!pvr_bo) { result = vk_error(device, VK_ERROR_OUT_OF_HOST_MEMORY); @@ -374,8 +368,7 @@ VkResult pvr_bo_alloc(struct pvr_device *device, if (result != VK_SUCCESS) goto err_buffer_destroy; - if (flags & PVR_BO_ALLOC_FLAG_ZERO_ON_ALLOC) - VG(VALGRIND_MAKE_MEM_DEFINED(pvr_bo->bo->map, pvr_bo->bo->size)); + VG(VALGRIND_MAKE_MEM_DEFINED(pvr_bo->bo->map, pvr_bo->bo->size)); } result = device->ws->ops->heap_alloc(heap, size, alignment, &pvr_bo->vma); diff --git a/src/imagination/vulkan/pvr_bo.h b/src/imagination/vulkan/pvr_bo.h index 296dcc8ca4d..bf9c22a5882 100644 --- a/src/imagination/vulkan/pvr_bo.h +++ b/src/imagination/vulkan/pvr_bo.h @@ -116,11 +116,6 @@ struct pvr_suballoc_bo { * firmware processor. */ #define PVR_BO_ALLOC_FLAG_PM_FW_PROTECT BITFIELD_BIT(3U) -/** - * \brief Flag passed to #pvr_bo_alloc() to indicate that the buffer should be - * zeroed at allocation time. - */ -#define PVR_BO_ALLOC_FLAG_ZERO_ON_ALLOC BITFIELD_BIT(4U) VkResult pvr_bo_alloc(struct pvr_device *device, struct pvr_winsys_heap *heap, diff --git a/src/imagination/vulkan/pvr_job_render.c b/src/imagination/vulkan/pvr_job_render.c index 3354d809dbd..e0dd39f5815 100644 --- a/src/imagination/vulkan/pvr_job_render.c +++ b/src/imagination/vulkan/pvr_job_render.c @@ -377,8 +377,6 @@ static VkResult pvr_rt_vheap_rtc_data_init(struct pvr_device *device, struct pvr_rt_dataset *rt_dataset, uint32_t layers) { - const uint64_t bo_flags = PVR_BO_ALLOC_FLAG_GPU_UNCACHED | - PVR_BO_ALLOC_FLAG_ZERO_ON_ALLOC; uint64_t vheap_size; uint32_t alignment; uint64_t rtc_size; @@ -407,7 +405,7 @@ static VkResult pvr_rt_vheap_rtc_data_init(struct pvr_device *device, device->heaps.general_heap, vheap_size + rtc_size, alignment, - bo_flags, + PVR_BO_ALLOC_FLAG_GPU_UNCACHED, &rt_dataset->vheap_rtc_bo); if (result != VK_SUCCESS) return result; @@ -482,8 +480,6 @@ static VkResult pvr_rt_tpc_data_init(struct pvr_device *device, const struct pvr_rt_mtile_info *mtile_info, uint32_t layers) { - const uint64_t bo_flags = PVR_BO_ALLOC_FLAG_GPU_UNCACHED | - PVR_BO_ALLOC_FLAG_ZERO_ON_ALLOC; uint64_t tpc_size; pvr_rt_get_tail_ptr_stride_size(device, @@ -497,7 +493,7 @@ static VkResult pvr_rt_tpc_data_init(struct pvr_device *device, device->heaps.general_heap, tpc_size, PVRX(CR_TE_TPC_ADDR_BASE_ALIGNMENT), - bo_flags, + PVR_BO_ALLOC_FLAG_GPU_UNCACHED, &rt_dataset->tpc_bo); } diff --git a/src/imagination/vulkan/winsys/pvr_winsys.h b/src/imagination/vulkan/winsys/pvr_winsys.h index e232ac90937..92f6538234e 100644 --- a/src/imagination/vulkan/winsys/pvr_winsys.h +++ b/src/imagination/vulkan/winsys/pvr_winsys.h @@ -111,11 +111,6 @@ enum pvr_winsys_bo_type { * accessible to the Parameter Manager unit and firmware processor. */ #define PVR_WINSYS_BO_FLAG_PM_FW_PROTECT BITFIELD_BIT(2U) -/** - * \brief Flag passed to #pvr_winsys_ops.buffer_create to indicate that the - * buffer should be zeroed at allocation time. - */ -#define PVR_WINSYS_BO_FLAG_ZERO_ON_ALLOC BITFIELD_BIT(3U) struct pvr_winsys_bo { struct pvr_winsys *ws; diff --git a/src/imagination/vulkan/winsys/pvrsrvkm/pvr_srv_bo.c b/src/imagination/vulkan/winsys/pvrsrvkm/pvr_srv_bo.c index 5f46885f0f1..0934ca95827 100644 --- a/src/imagination/vulkan/winsys/pvrsrvkm/pvr_srv_bo.c +++ b/src/imagination/vulkan/winsys/pvrsrvkm/pvr_srv_bo.c @@ -123,10 +123,10 @@ static uint64_t pvr_srv_get_alloc_flags(uint32_t ws_flags) * userspace mappings. Check to see if there's any situations where we * wouldn't want this to be the case. */ - uint64_t srv_flags = PVR_SRV_MEMALLOCFLAG_GPU_READABLE | - PVR_SRV_MEMALLOCFLAG_GPU_WRITEABLE | - PVR_SRV_MEMALLOCFLAG_KERNEL_CPU_MAPPABLE | - PVR_SRV_MEMALLOCFLAG_CPU_UNCACHED_WC; + uint64_t srv_flags = + PVR_SRV_MEMALLOCFLAG_GPU_READABLE | PVR_SRV_MEMALLOCFLAG_GPU_WRITEABLE | + PVR_SRV_MEMALLOCFLAG_KERNEL_CPU_MAPPABLE | + PVR_SRV_MEMALLOCFLAG_CPU_UNCACHED_WC | PVR_SRV_MEMALLOCFLAG_ZERO_ON_ALLOC; if (ws_flags & PVR_WINSYS_BO_FLAG_CPU_ACCESS) { srv_flags |= PVR_SRV_MEMALLOCFLAG_CPU_READABLE | @@ -141,9 +141,6 @@ static uint64_t pvr_srv_get_alloc_flags(uint32_t ws_flags) if (ws_flags & PVR_WINSYS_BO_FLAG_PM_FW_PROTECT) srv_flags |= PVR_SRV_MEMALLOCFLAG_DEVICE_FLAG(PM_FW_PROTECT); - if (ws_flags & PVR_WINSYS_BO_FLAG_ZERO_ON_ALLOC) - srv_flags |= PVR_SRV_MEMALLOCFLAG_ZERO_ON_ALLOC; - return srv_flags; } @@ -323,11 +320,7 @@ VkResult pvr_srv_winsys_buffer_map(struct pvr_winsys_bo *bo) return result; } - VG(VALGRIND_MALLOCLIKE_BLOCK(bo->map, - bo->size, - 0, - srv_bo->flags & - PVR_SRV_MEMALLOCFLAG_ZERO_ON_ALLOC)); + VG(VALGRIND_MALLOCLIKE_BLOCK(bo->map, bo->size, 0, true)); buffer_acquire(srv_bo);
