honeykrisp: map placed memory at the allocation size

src/asahi/lib/agx_device_virtio.c

@@ -190,17 +190,20 @@ agx_virtio_bo_unbind_object(struct agx_device *dev, uint32_t object_handle)
    return 0;
 }
 
-static void
+/* Real mapping primitive, non-static so the placed-map regression can drive
+ * it: the per-length contract it implements belongs to the ops signature. */
+void
 agx_virtio_bo_mmap(struct agx_device *dev, struct agx_bo *bo, void *fixed_addr,
-                   UNUSED size_t length)
+                   size_t length)
 {
-   bo->_map_size = bo->size;
-   bo->_map = vdrm_bo_map(dev->vdrm, bo->handle, bo->size, fixed_addr);
-   if (bo->_map == MAP_FAILED) {
+   size_t map_size = length ?: bo->size;
+   bo->_map_size = map_size;
+   bo->_map = vdrm_bo_map(dev->vdrm, bo->handle, map_size, fixed_addr);
+   if (bo->_map == NULL || bo->_map == MAP_FAILED) {
       bo->_map = NULL;
       bo->_map_size = 0;
       fprintf(stderr, "mmap failed: result=%p size=0x%llx fd=%i\n", bo->_map,
-              (long long)bo->size, dev->fd);
+              (long long)map_size, dev->fd);
    }
 }
 

src/asahi/lib/agx_device_virtio.h

@@ -11,3 +11,6 @@
 int agx_virtio_simple_ioctl(struct agx_device *dev, unsigned cmd, void *_req);
 
 bool agx_virtio_open_device(struct agx_device *dev);
+
+void agx_virtio_bo_mmap(struct agx_device *dev, struct agx_bo *bo,
+                        void *fixed_addr, size_t length);

src/asahi/vulkan/hk_device_memory.c

@@ -18,7 +18,6 @@
 #include "util/os_misc.h"
 
 #include <inttypes.h>
-#include <sys/mman.h>
 
 /* GPU memory is system memory that the kernel cannot evict or swap, and
  * heapBudget is only advisory, so the heap limit alone lets a process (or
@@ -376,15 +375,7 @@ hk_FreeMemory(VkDevice device, VkDeviceMemory _mem,
    if (mem->bo->flags & (AGX_BO_SHAREABLE | AGX_BO_SHARED))
       hk_remove_ext_bo(dev, mem->bo);
 
-   /* Freeing a mapped memory object unmaps it. The shared mapping goes with
-    * the BO; an owned one belongs to this object alone. */
-   if (mem->map_owned)
-      agx_bo_unmap_owned(mem->bo, mem->map, mem->map_size);
-   else if (mem->map && mem->map_placed) {
-      dev->dev.ops.bo_munmap(&dev->dev, mem->bo);
-      mem->bo->_map = NULL;
-      mem->bo->_map_size = 0;
-   }
+   hk_memory_unmap(mem);
 
    agx_bo_unreference(&dev->dev, mem->bo);
 
@@ -446,55 +437,32 @@ hk_MapMemory2KHR(VkDevice device, const VkMemoryMapInfoKHR *pMemoryMapInfo,
                        "A placed mapping must cover exactly the allocation.");
    }
 
-   /* A backend unable to create independent mappings must not relocate a
-    * mapping that another imported memory object may still be using. */
+   /* Preserve the backend alias guard before asking for an independent map. */
    if (fixed_addr && !dev->dev.ops.bo_mmap_independent && mem->bo->_map &&
        p_atomic_read(&mem->bo->refcnt) > 1) {
       *ppData = NULL;
       return vk_errorf(dev, VK_ERROR_MEMORY_MAP_FAILED,
                        "Cannot relocate a shared mapping on this backend.");
    }
 
-   /* Bound a placed mapping by the memory object, not the BO: the BO cache
-    * may have handed us one up to twice as large, and the application has
-    * reserved only mem->vk.size at this address.
-    *
-    * An exported allocation and its imports into this device are distinct
-    * memory objects over one agx_bo. Each maps, unmaps, frees and reserves
-    * independently, so each gets a mapping of its own rather than the BO's
-    * shared one, which unmapping any of them would otherwise tear down
-    * under the others. A BO only one memory object can reach keeps the
-    * cached mapping. */
-   if (mem->bo->flags & (AGX_BO_SHARED | AGX_BO_SHAREABLE)) {
-      mem->map = agx_bo_map_owned(mem->bo, fixed_addr, mem->vk.size,
-                                  &mem->map_size);
-      mem->map_owned = mem->map != NULL;
-   }
-   if (!mem->map_owned)
-      mem->map = agx_bo_map_placed(mem->bo, fixed_addr, mem->vk.size);
-   if (mem->map == NULL) {
+   /* Application maps must never replace the stable mapping retained by
+    * image host-copy or decoder users, even when that mapping is absent yet.
+    */
+   size_t map_size = fixed_addr ? mem->vk.size : mem->bo->size;
+   if (hk_memory_map_from_bo(mem, fixed_addr, map_size) == NULL) {
       *ppData = NULL;
       return vk_errorf(dev, VK_ERROR_MEMORY_MAP_FAILED,
                        "failed to map memory BO handle %u",
                        mem->bo->uapi_handle);
    }
 
    if (fixed_addr && mem->map != fixed_addr) {
-      if (mem->map_owned)
-         agx_bo_unmap_owned(mem->bo, mem->map, mem->map_size);
-      else {
-         dev->dev.ops.bo_munmap(&dev->dev, mem->bo);
-         mem->bo->_map = NULL;
-         mem->bo->_map_size = 0;
-      }
-      mem->map = NULL;
-      mem->map_owned = false;
-      mem->map_size = 0;
+      hk_memory_unmap(mem);
       *ppData = NULL;
       return vk_errorf(dev, VK_ERROR_MEMORY_MAP_FAILED,
                        "Placed mapping returned an unexpected address.");
    }
-   mem->map_placed = fixed_addr != NULL;
+
    *ppData = mem->map + offset;
 
    return VK_SUCCESS;
@@ -509,54 +477,15 @@ hk_UnmapMemory2KHR(VkDevice device,
    if (mem == NULL || mem->map == NULL)
       return VK_SUCCESS;
 
-   if (mem->map_owned) {
-      if (pMemoryUnmapInfo->flags & VK_MEMORY_UNMAP_RESERVE_BIT_EXT) {
-         VK_FROM_HANDLE(hk_device, dev, device);
-
-         /* Exactly this object's mapping, as below. */
-         void *err = mmap(mem->map, mem->map_size, PROT_NONE,
-                          MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0);
-         if (err == MAP_FAILED) {
-            return vk_errorf(dev, VK_ERROR_MEMORY_MAP_FAILED,
-                             "Failed to map over original mapping");
-         }
-      } else {
-         agx_bo_unmap_owned(mem->bo, mem->map, mem->map_size);
-      }
-      mem->map = NULL;
-      mem->map_size = 0;
-      mem->map_owned = false;
-      mem->map_placed = false;
-      return VK_SUCCESS;
-   }
-
    if (pMemoryUnmapInfo->flags & VK_MEMORY_UNMAP_RESERVE_BIT_EXT) {
       VK_FROM_HANDLE(hk_device, dev, device);
-
-      /* Reserve exactly what was mapped. A placed map covers the
-       * VkDeviceMemory's allocationSize, not the cached BO's size, and
-       * PROT_NONE'ing the difference lands on address space the application
-       * reserved for itself -- the guard pages dEQP-VK.memory.map_placed.*
-       * fills and reads back. */
-      void *err = mmap(mem->bo->_map, agx_bo_map_size(mem->bo), PROT_NONE,
-                       MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0);
-      if (err == MAP_FAILED) {
+      if (!hk_memory_unmap_reserve(mem))
          return vk_errorf(dev, VK_ERROR_MEMORY_MAP_FAILED,
                           "Failed to map over original mapping");
-      }
-      mem->bo->_map = NULL;
-      mem->bo->_map_size = 0;
    } else {
-      if (mem->bo->_map) {
-         munmap(mem->bo->_map, agx_bo_map_size(mem->bo));
-         mem->bo->_map = NULL;
-         mem->bo->_map_size = 0;
-      }
+      hk_memory_unmap(mem);
    }
 
-   mem->map = NULL;
-   mem->map_placed = false;
-
    return VK_SUCCESS;
 }
 

src/asahi/vulkan/hk_device_memory.h

@@ -9,10 +9,14 @@
 
 #include "hk_private.h"
 
+#include "asahi/lib/agx_device.h"
+
 #include "vk_device_memory.h"
 
 #include "util/list.h"
 
+#include <sys/mman.h>
+
 struct hk_device;
 struct hk_image_plane;
 
@@ -31,10 +35,45 @@ struct hk_device_memory {
    /* map is this object's own mapping of map_size bytes rather than the
     * BO's shared one: the BO may be aliased by other memory objects. */
    size_t map_size;
-   bool map_owned;
-   bool map_placed;
 };
 
+
+/* Application mappings belong to one VkDeviceMemory, never the shared BO. */
+static inline void *
+hk_memory_map_from_bo(struct hk_device_memory *mem, void *fixed_addr,
+                      size_t length)
+{
+   mem->map = agx_bo_map_owned(mem->bo, fixed_addr, length, &mem->map_size);
+   return mem->map;
+}
+
+static inline void
+hk_memory_unmap(struct hk_device_memory *mem)
+{
+   if (mem->map) {
+      agx_bo_unmap_owned(mem->bo, mem->map, mem->map_size);
+      mem->map = NULL;
+      mem->map_size = 0;
+   }
+}
+
+static inline bool
+hk_memory_unmap_reserve(struct hk_device_memory *mem)
+{
+   if (!mem->map)
+      return true;
+
+   void *err = mmap(mem->map, mem->map_size, PROT_NONE,
+                    MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0);
+   if (err == MAP_FAILED)
+      return false;
+
+   mem->map = NULL;
+   mem->map_size = 0;
+   return true;
+}
+
+
 VK_DEFINE_NONDISP_HANDLE_CASTS(hk_device_memory, vk.base, VkDeviceMemory,
                                VK_OBJECT_TYPE_DEVICE_MEMORY)
 

src/asahi/vulkan/meson.build

@@ -454,6 +454,21 @@ hk_deps = [
   idep_libagx,
   idep_libagx_shaders_h,
 ]
+if with_tests
+  test('hk_placed_map_lifetime',
+    executable('hk-placed-map-lifetime',
+      files('tests/test-placed-map.c'),
+      c_args : [c_msvc_compat_args, no_override_init_args,
+                '-Wno-c2x-extensions'],
+      gnu_symbol_visibility : 'hidden',
+      include_directories : [
+        inc_gallium, inc_gallium_aux, inc_include, inc_src,
+        inc_asahi, inc_virtio_gpu, inc_virtio_vdrm,
+      ],
+      dependencies : [hk_deps, idep_mesautil],
+      link_with : [libasahi_lib, libasahi_layout, libasahi_compiler]),
+    suite : ['asahi'])
+endif
 
 libhk = static_library(
   'hk',