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 <vlad-radu.schiller@imgtec.com>
Reviewed-by: Frank Binns <frank.binns@imgtec.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/24930>

diff --git a/src/imagination/common/pvr_debug.c b/src/imagination/common/pvr_debug.c
index f92414f..e910f4b 100644 (file)
--- 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 3aa0879..33d2242 100644 (file)
--- 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 cce1ae5..cf3da15 100644 (file)
--- 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 296dcc8..bf9c22a 100644 (file)
--- 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 3354d80..e0dd39f 100644 (file)
--- 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 e232ac9..92f6538 100644 (file)
--- 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 5f46885..0934ca9 100644 (file)
--- 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);