gralloc4: Remove bases from handle

Bug: 213170949
Test: Boot to home
Test: VtsHalGraphicsMapperV4_0TargetTest
Merged-In: I0a0a071c5fc6e304c025a53217cd798452b0c2c3
Change-Id: I0a0a071c5fc6e304c025a53217cd798452b0c2c3

8 files changed, 74 insertions, 83 deletions

diff --git a/gralloc4/src/allocator/mali_gralloc_ion.cpp b/gralloc4/src/allocator/mali_gralloc_ion.cpp
index 2d63a70..986af11 100644
--- a/gralloc4/src/allocator/mali_gralloc_ion.cpp
+++ b/gralloc4/src/allocator/mali_gralloc_ion.cpp
@@ -299,20 +299,8 @@ void mali_gralloc_ion_free(private_handle_t * const hnd)
 {
 	for (int i = 0; i < hnd->fd_count; i++)
 	{
-		void* mapped_addr = reinterpret_cast<void*>(hnd->bases[i]);
-
-		/* Buffer might be unregistered already so we need to assure we have a valid handle */
-		if (mapped_addr != nullptr)
-		{
-			if (munmap(mapped_addr, hnd->alloc_sizes[i]) != 0)
-			{
-				/* TODO: more detailed error logs */
-				MALI_GRALLOC_LOGE("Failed to munmap handle %p", hnd);
-			}
-		}
 		close(hnd->fds[i]);
 		hnd->fds[i] = -1;
-		hnd->bases[i] = 0;
 	}
 	delete hnd;
 }
@@ -480,15 +468,17 @@ int mali_gralloc_ion_allocate(const gralloc_buffer_descriptor_t *descriptors,
 	return 0;
 }
 
-int mali_gralloc_ion_map(private_handle_t *hnd)
+std::array<void*, MAX_BUFFER_FDS> mali_gralloc_ion_map(private_handle_t *hnd)
 {
-	uint64_t usage = hnd->producer_usage | hnd->consumer_usage;
+	std::array<void*, MAX_BUFFER_FDS> vaddrs;
+	vaddrs.fill(nullptr);
 
+	uint64_t usage = hnd->producer_usage | hnd->consumer_usage;
 	/* Do not allow cpu access to secure buffers */
 	if (usage & (GRALLOC_USAGE_PROTECTED | GRALLOC_USAGE_NOZEROED)
 			&& !(usage & GRALLOC_USAGE_PRIVATE_NONSECURE))
 	{
-		return 0;
+		return vaddrs;
 	}
 
 	for (int fidx = 0; fidx < hnd->fd_count; fidx++) {
@@ -505,38 +495,38 @@ int mali_gralloc_ion_map(private_handle_t *hnd)
 
 			for (int cidx = 0; cidx < fidx; fidx++)
 			{
-				munmap((void*)hnd->bases[cidx], hnd->alloc_sizes[cidx]);
-				hnd->bases[cidx] = 0;
+				munmap((void*)vaddrs[cidx], hnd->alloc_sizes[cidx]);
+				vaddrs[cidx] = 0;
 			}
 
-			return -err;
+			return vaddrs;
 		}
 
-		hnd->bases[fidx] = uintptr_t(mappedAddress);
+		vaddrs[fidx] = mappedAddress;
 	}
 
-	return 0;
+	return vaddrs;
 }
 
-void mali_gralloc_ion_unmap(private_handle_t *hnd)
+void mali_gralloc_ion_unmap(private_handle_t *hnd, std::array<void*, MAX_BUFFER_FDS>& vaddrs)
 {
 	for (int i = 0; i < hnd->fd_count; i++)
 	{
 		int err = 0;
 
-		if (hnd->bases[i])
+		if (vaddrs[i])
 		{
-			err = munmap((void*)hnd->bases[i], hnd->alloc_sizes[i]);
+			err = munmap(vaddrs[i], hnd->alloc_sizes[i]);
 		}
 
 		if (err)
 		{
 			MALI_GRALLOC_LOGE("Could not munmap base:%p size:%" PRIu64 " '%s'",
-					(void*)hnd->bases[i], hnd->alloc_sizes[i], strerror(errno));
+					(void*)vaddrs[i], hnd->alloc_sizes[i], strerror(errno));
 		}
 		else
 		{
-			hnd->bases[i] = 0;
+			vaddrs[i] = 0;
 		}
 	}
 
diff --git a/gralloc4/src/allocator/mali_gralloc_ion.h b/gralloc4/src/allocator/mali_gralloc_ion.h
index 5f55c2f..d826650 100644
--- a/gralloc4/src/allocator/mali_gralloc_ion.h
+++ b/gralloc4/src/allocator/mali_gralloc_ion.h
@@ -30,8 +30,8 @@ int mali_gralloc_ion_sync_start(const private_handle_t * const hnd,
                                 const bool read, const bool write);
 int mali_gralloc_ion_sync_end(const private_handle_t * const hnd,
                               const bool read, const bool write);
-int mali_gralloc_ion_map(private_handle_t *hnd);
-void mali_gralloc_ion_unmap(private_handle_t *hnd);
+std::array<void*, MAX_BUFFER_FDS> mali_gralloc_ion_map(private_handle_t *hnd);
+void mali_gralloc_ion_unmap(private_handle_t *hnd, std::array<void*, MAX_BUFFER_FDS>& vaddrs);
 int mali_gralloc_attr_allocate(void);
 
 #endif /* MALI_GRALLOC_ION_H_ */
diff --git a/gralloc4/src/core/mali_gralloc_bufferaccess.cpp b/gralloc4/src/core/mali_gralloc_bufferaccess.cpp
index adda46b..344ab2a 100644
--- a/gralloc4/src/core/mali_gralloc_bufferaccess.cpp
+++ b/gralloc4/src/core/mali_gralloc_bufferaccess.cpp
@@ -221,9 +221,16 @@ int mali_gralloc_lock(buffer_handle_t buffer,
 			return -EINVAL;
 		}
 
-		mali_gralloc_reference_map(buffer);
+		if (mali_gralloc_reference_map(buffer) != 0) {
+			return -EINVAL;
+		}
 
-		*vaddr = (void *)hnd->bases[0];
+		std::optional<void*> buf_addr = mali_gralloc_reference_get_buf_addr(buffer);
+		if (!buf_addr.has_value()) {
+			MALI_GRALLOC_LOGE("BUG: Invalid buffer address on a just mapped buffer");
+			return -EINVAL;
+		}
+		*vaddr = buf_addr.value();
 
 		buffer_sync(hnd, get_tx_direction(usage));
 	}
diff --git a/gralloc4/src/core/mali_gralloc_reference.cpp b/gralloc4/src/core/mali_gralloc_reference.cpp
index b84cbb4..84f4c14 100644
--- a/gralloc4/src/core/mali_gralloc_reference.cpp
+++ b/gralloc4/src/core/mali_gralloc_reference.cpp
@@ -35,7 +35,7 @@ private:
     // should become the only place where address mapping is maintained and can be
     // queried from.
     struct MappedData {
-        void *bases[MAX_BUFFER_FDS] = {};
+        std::array<void *, MAX_BUFFER_FDS> bases;
         size_t alloc_sizes[MAX_BUFFER_FDS] = {};
         uint64_t ref_count = 0;
     };
@@ -94,7 +94,8 @@ private:
     }
 
     int map_locked(buffer_handle_t handle) REQUIRES(lock) {
-        private_handle_t *hnd = (private_handle_t *)handle;
+        private_handle_t *hnd =
+                reinterpret_cast<private_handle_t *>(const_cast<native_handle *>(handle));
         auto it = buffer_map.find(hnd);
 
         if (it == buffer_map.end()) {
@@ -116,13 +117,12 @@ private:
             return -EINVAL;
         }
 
-        int error = mali_gralloc_ion_map(hnd);
-        if (error != 0) {
-            return error;
+        data.bases = mali_gralloc_ion_map(hnd);
+        if (data.bases[0] == nullptr) {
+            return -EINVAL;
         }
 
         for (auto i = 0; i < MAX_BUFFER_FDS; i++) {
-            data.bases[i] = reinterpret_cast<void *>(hnd->bases[i]);
             data.alloc_sizes[i] = hnd->alloc_sizes[i];
         }
 
@@ -135,7 +135,7 @@ private:
             return -EINVAL;
         }
 
-        const auto *hnd = (private_handle_t *)handle;
+        const auto *hnd = reinterpret_cast<private_handle_t *>(const_cast<native_handle *>(handle));
         auto it = buffer_map.find(hnd);
         if (it == buffer_map.end()) {
             MALI_GRALLOC_LOGE("Reference unimported buffer %p, returning error", handle);
@@ -145,8 +145,7 @@ private:
         auto &data = *(it->second.get());
         if (data.bases[0] != nullptr) {
             for (auto i = 0; i < MAX_BUFFER_FDS; i++) {
-                if (data.bases[i] != reinterpret_cast<void *>(hnd->bases[i]) ||
-                    data.alloc_sizes[i] != hnd->alloc_sizes[i]) {
+                if (data.alloc_sizes[i] != hnd->alloc_sizes[i]) {
                     MALI_GRALLOC_LOGE(
                             "Validation failed: Buffer attributes inconsistent with mapper");
                     return -EINVAL;
@@ -154,7 +153,7 @@ private:
             }
         } else {
             for (auto i = 0; i < MAX_BUFFER_FDS; i++) {
-                if (hnd->bases[i] != 0 || data.bases[i] != nullptr) {
+                if (data.bases[i] != nullptr) {
                     MALI_GRALLOC_LOGE("Validation failed: Expected nullptr for unmapped buffer");
                     return -EINVAL;
                 }
@@ -177,7 +176,8 @@ public:
         }
         std::lock_guard<std::mutex> _l(lock);
 
-        private_handle_t *hnd = (private_handle_t *)handle;
+        private_handle_t *hnd =
+                reinterpret_cast<private_handle_t *>(const_cast<native_handle *>(handle));
 
         auto it = buffer_map.find(hnd);
         if (it == buffer_map.end()) {
@@ -189,10 +189,6 @@ public:
                 MALI_GRALLOC_LOGE("Failed to create buffer data mapping");
                 return -EINVAL;
             }
-
-            for (int i = 0; i < MAX_BUFFER_FDS; i++) {
-                hnd->bases[i] = 0;
-            }
         } else if (it->second->ref_count == 0) {
             MALI_GRALLOC_LOGE("BUG: Import counter of an imported buffer is 0, expect errors");
         }
@@ -224,7 +220,8 @@ public:
             return error;
         }
 
-        private_handle_t *hnd = (private_handle_t *)handle;
+        private_handle_t *hnd =
+                reinterpret_cast<private_handle_t *>(const_cast<native_handle *>(handle));
         auto it = buffer_map.find(hnd);
         if (it == buffer_map.end()) {
             MALI_GRALLOC_LOGE("Trying to release a non-imported buffer");
@@ -240,7 +237,7 @@ public:
         data.ref_count--;
         if (data.ref_count == 0) {
             if (data.bases[0] != nullptr) {
-                mali_gralloc_ion_unmap(hnd);
+                mali_gralloc_ion_unmap(hnd, data.bases);
             }
 
             /* TODO: Make this unmapping of shared meta fd into a function? */
@@ -258,6 +255,31 @@ public:
         std::lock_guard<std::mutex> _l(lock);
         return validate_locked(handle);
     }
+
+    std::optional<void *> get_buf_addr(buffer_handle_t handle) {
+        std::lock_guard<std::mutex> _l(lock);
+
+        auto error = validate_locked(handle);
+        if (error != 0) {
+            return {};
+        }
+
+        private_handle_t *hnd =
+                reinterpret_cast<private_handle_t *>(const_cast<native_handle *>(handle));
+        auto it = buffer_map.find(hnd);
+        if (it == buffer_map.end()) {
+            MALI_GRALLOC_LOGE("BUG: Cannot find a validated buffer");
+            return {};
+        }
+
+        const auto &data = *(it->second.get());
+        if (data.bases[0] == nullptr) {
+            MALI_GRALLOC_LOGE("BUG: Called %s for an un-mapped buffer", __FUNCTION__);
+            return {};
+        }
+
+        return data.bases[0];
+    }
 };
 
 int mali_gralloc_reference_retain(buffer_handle_t handle) {
@@ -275,3 +297,7 @@ int mali_gralloc_reference_release(buffer_handle_t handle) {
 int mali_gralloc_reference_validate(buffer_handle_t handle) {
     return BufferManager::getInstance().validate(handle);
 }
+
+std::optional<void *> mali_gralloc_reference_get_buf_addr(buffer_handle_t handle) {
+    return BufferManager::getInstance().get_buf_addr(handle);
+}
diff --git a/gralloc4/src/core/mali_gralloc_reference.h b/gralloc4/src/core/mali_gralloc_reference.h
index acf8e82..257bb6b 100644
--- a/gralloc4/src/core/mali_gralloc_reference.h
+++ b/gralloc4/src/core/mali_gralloc_reference.h
@@ -19,11 +19,14 @@
 #ifndef MALI_GRALLOC_REFERENCE_H_
 #define MALI_GRALLOC_REFERENCE_H_
 
-#include "gralloc_priv.h"
+#include <cutils/native_handle.h>
+#include <optional>
 
 int mali_gralloc_reference_retain(buffer_handle_t handle);
 int mali_gralloc_reference_release(buffer_handle_t handle);
 int mali_gralloc_reference_validate(buffer_handle_t handle);
 int mali_gralloc_reference_map(buffer_handle_t handle);
 
+std::optional<void*> mali_gralloc_reference_get_buf_addr(buffer_handle_t handle);
+
 #endif /* MALI_GRALLOC_REFERENCE_H_ */
diff --git a/gralloc4/src/hidl_common/Mapper.cpp b/gralloc4/src/hidl_common/Mapper.cpp
index f6ebe6f..aa8e6d2 100644
--- a/gralloc4/src/hidl_common/Mapper.cpp
+++ b/gralloc4/src/hidl_common/Mapper.cpp
@@ -245,14 +245,6 @@ static Error unlockBuffer(buffer_handle_t bufferHandle,
 	}
 
 	auto private_handle = private_handle_t::dynamicCast(bufferHandle);
-#if 0
-	if (!private_handle->cpu_write && !private_handle->cpu_read)
-	{
-		MALI_GRALLOC_LOGW("Attempt to call unlock*() on an unlocked buffer (%p)", bufferHandle);
-
-		/* TODO: handle simulatneous locks differently. May be keep a global lock count per buffer? */
-	}
-#endif
 
 	const int result = mali_gralloc_unlock(bufferHandle);
 	if (result)
diff --git a/gralloc4/src/libGralloc4Wrapper/wrapper.cpp b/gralloc4/src/libGralloc4Wrapper/wrapper.cpp
index c9d3381..7d286e0 100644
--- a/gralloc4/src/libGralloc4Wrapper/wrapper.cpp
+++ b/gralloc4/src/libGralloc4Wrapper/wrapper.cpp
@@ -195,15 +195,6 @@ int freeImportedHandle(void *handle)
 
     const private_handle_t *hnd = static_cast<private_handle_t *>(handle);
 
-    struct UnmapWork { void *base; size_t alloc_size; };
-    std::vector<UnmapWork> work(hnd->fd_count);
-
-    for (size_t i = 0; i < hnd->fd_count; ++i)
-    {
-        work[i].base = reinterpret_cast<void*>(hnd->bases[i]);
-        work[i].alloc_size = hnd->alloc_sizes[i];
-    }
-
     static android::sp<IMapper> mapper = IMapper::getService();
     if (!mapper)
     {
@@ -217,20 +208,6 @@ int freeImportedHandle(void *handle)
         return -1;
     }
 
-    {
-        bool unmapFailed = false;
-        for (const UnmapWork &w : work)
-        {
-            if (!w.base) { continue; }
-            if (int ret = munmap(w.base, w.alloc_size); ret)
-            {
-                ALOGE("libGralloc4Wrapper: %s couldn't unmap address %p (size %zu): %s", __func__, w.base, w.alloc_size, strerror(ret));
-                unmapFailed = true;
-            }
-        }
-        if (unmapFailed) { return -1; }
-    }
-
     return 0;
 }
 
diff --git a/gralloc4/src/mali_gralloc_buffer.h b/gralloc4/src/mali_gralloc_buffer.h
index c4db9fc..11c41f5 100644
--- a/gralloc4/src/mali_gralloc_buffer.h
+++ b/gralloc4/src/mali_gralloc_buffer.h
@@ -236,7 +236,6 @@ struct private_handle_t
 	// locally mapped shared attribute area
 
 	int ion_handles[MAX_BUFFER_FDS];
-	uint64_t bases[MAX_BUFFER_FDS];
 	uint64_t alloc_sizes[MAX_BUFFER_FDS];
 
 	void *attr_base __attribute__((aligned (8))) DEFAULT_INITIALIZER(nullptr);
@@ -292,7 +291,6 @@ struct private_handle_t
 		if (_alloc_sizes)
 			memcpy(alloc_sizes, _alloc_sizes, sizeof(alloc_sizes));
 
-		memset(bases, 0, sizeof(bases));
 		memset(ion_handles, 0, sizeof(ion_handles));
 	}
 
@@ -406,7 +404,6 @@ struct private_handle_t
 			"alloc_format(0x%" PRIx64 ") "
 			"alloc_sizes(%" PRIu64 " %" PRIu64 " %" PRIu64 ") "
 			"layer_count(%d) "
-			"bases(%p %p %p %p) "
 			"\n",
 			str,
 			numInts, numFds, fd_count,
@@ -421,8 +418,7 @@ struct private_handle_t
 			plane_info[2].size, plane_info[2].byte_stride, plane_info[2].alloc_width, plane_info[2].alloc_height,
 			alloc_format,
 			alloc_sizes[0], alloc_sizes[1], alloc_sizes[2],
-			layer_count,
-			(void*)bases[0], (void*)bases[1], (void*)bases[2], attr_base
+			layer_count
 		);
 	}