tflite: ahwb: Use dmabuf with proper synchronization
* Allocate dmabuf with udmabuf if available.
* Use ioctl() w/ DMA_BUF_IOCTL_SYNC when locking/unlocking a dmabuf.
* Check whether imported buffer is a dmabuf.
BUG=b:305999697
TEST=Pass android_hardware_buffer_test on redrix.
Change-Id: Iefafa158924ac34558c5671ef2d2813ace3a4d24
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/tflite/+/5400938
Tested-by: Shik Chen <shik@chromium.org>
Auto-Submit: Shik Chen <shik@chromium.org>
Commit-Queue: ChromeOS Auto Retry <chromeos-auto-retry@chromeos-bot.iam.gserviceaccount.com>
Reviewed-by: Tommy Chiang <ototot@google.com>
diff --git a/common/BUILD.bazel b/common/BUILD.bazel
index 304a2e9..3d731f6 100644
--- a/common/BUILD.bazel
+++ b/common/BUILD.bazel
@@ -68,6 +68,7 @@
deps = [
":android_hardware_buffer",
":libnativewindow.lds",
+ ":log",
],
)
diff --git a/common/android_hardware_buffer.cc b/common/android_hardware_buffer.cc
index f3fb0d4..2d57ebf 100644
--- a/common/android_hardware_buffer.cc
+++ b/common/android_hardware_buffer.cc
@@ -7,6 +7,9 @@
#include "common/android_hardware_buffer.h"
#include <fcntl.h>
+#include <linux/dma-buf.h>
+#include <linux/udmabuf.h>
+#include <sys/ioctl.h>
#include <sys/mman.h>
#include <unistd.h>
@@ -17,6 +20,8 @@
#include <new>
#include <utility>
+#include "common/log.h"
+
namespace {
// TODO(shik): Move this into its own file.
@@ -24,7 +29,8 @@
public:
static const int kInvalidFd = -1;
- explicit ScopedFd(int fd = kInvalidFd) : fd_(fd) {}
+ ScopedFd() : ScopedFd(kInvalidFd) {}
+ explicit ScopedFd(int fd) : fd_(fd) {}
ScopedFd(ScopedFd&& other) { *this = std::move(other); }
@@ -91,6 +97,83 @@
return std::unique_ptr<native_handle_t>(cloned);
}
+bool IsDmaBuf(int fd) {
+ // Do a no-op sync intentionally.
+ dma_buf_sync sync = {.flags = 0};
+ // TODO(shik): Handle EINTR.
+ int ret = ioctl(fd, DMA_BUF_IOCTL_SYNC, &sync);
+
+ // The ioctl() will get ENOTTY when the specified request does not apply to
+ // the kind of object that the file descriptor references.
+ bool not_applicable = ret == -1 && errno == ENOTTY;
+ return !not_applicable;
+}
+
+bool IsUdmabufAvailable() {
+ static bool avail = [] {
+ ScopedFd fd(open("/dev/udmabuf", O_RDWR));
+ if (!fd.is_valid()) {
+ LOGF(INFO) << "/dev/udmabuf is not available";
+ }
+ return fd.is_valid();
+ }();
+ return avail;
+}
+
+ScopedFd AllocateWithMemfd(size_t size) {
+ // TODO(shik): Use a more descriptive name to make debugging easier.
+ ScopedFd fd(memfd_create("ahwb", MFD_CLOEXEC | MFD_ALLOW_SEALING));
+ if (!fd.is_valid()) {
+ PLOGF(ERROR) << "memfd_create() failed";
+ return {};
+ }
+
+ if (ftruncate64(fd, size) != 0) {
+ PLOGF(ERROR) << "ftruncate64() failed";
+ return {};
+ }
+
+ if (fcntl(fd, F_ADD_SEALS, F_SEAL_SHRINK | F_SEAL_GROW) != 0) {
+ PLOGF(ERROR) << "fcntl() failed";
+ return {};
+ }
+
+ if (!IsUdmabufAvailable()) {
+ return fd;
+ }
+
+ ScopedFd udmabuf(open("/dev/udmabuf", O_RDWR));
+ if (!udmabuf.is_valid()) {
+ PLOGF(ERROR) << "open /dev/udmabuf failed";
+ return {};
+ }
+
+ udmabuf_create create = {
+ .memfd = static_cast<__u32>(fd.get()),
+ .flags = UDMABUF_FLAGS_CLOEXEC,
+ .offset = 0,
+ .size = size,
+ };
+ ScopedFd dmabuf_fd(ioctl(udmabuf, UDMABUF_CREATE, &create));
+ if (!dmabuf_fd.is_valid()) {
+ PLOGF(ERROR) << "ioctl() for UDMABUF_CREATE failed";
+ return {};
+ }
+
+ return dmabuf_fd;
+}
+
+uint64_t SyncFlagsFromUsageMask(uint64_t usage) {
+ uint64_t flags = 0;
+ if (usage & AHARDWAREBUFFER_USAGE_CPU_READ_MASK) {
+ flags |= DMA_BUF_SYNC_READ;
+ }
+ if (usage & AHARDWAREBUFFER_USAGE_CPU_WRITE_MASK) {
+ flags |= DMA_BUF_SYNC_WRITE;
+ }
+ return flags;
+}
+
class Allocator {
public:
static Allocator* GetInstance() {
@@ -100,37 +183,40 @@
return instance;
}
- // TODO(shik): Use dma_heap or udmabuf to create a real dmabuf.
+ // TODO(shik): Add another backend using dma_heap.
int Allocate(const AHardwareBuffer_Desc* _Nonnull desc,
AHardwareBuffer* _Nullable* _Nonnull out_buffer) {
if (!IsSupported(desc)) {
+ LOGF(ERROR) << "Unsupported desc";
return -EINVAL;
}
- uint32_t size = desc->width;
- // TODO(shik): Use a more descriptive name to make debugging easier.
- ScopedFd fd(memfd_create("ahwb", MFD_CLOEXEC | MFD_ALLOW_SEALING));
+
+ // Ensure the allocated size is page-aligned.
+ size_t page = sysconf(_SC_PAGESIZE);
+ size_t size = (desc->width + page - 1) / page * page;
+
+ ScopedFd fd = AllocateWithMemfd(size);
if (!fd.is_valid()) {
- return -errno;
+ return -EINVAL;
}
- if (ftruncate64(fd, size) != 0) {
- return -errno;
- }
- if (fcntl(fd, F_ADD_SEALS, F_SEAL_SHRINK | F_SEAL_GROW) != 0) {
- return -errno;
- }
+
void* data = mmap(nullptr, size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
if (data == MAP_FAILED) {
+ PLOGF(ERROR) << "mmap() failed";
return -errno;
}
auto handle = CreateNativeHandle(fd);
+ bool is_dmabuf = IsDmaBuf(fd);
Buffer buffer = {
.fd = std::move(fd),
.data = data,
.size = size,
+ .is_dmabuf = is_dmabuf,
.ref_count = 1,
.desc = *desc,
.handle = std::move(handle),
+ .locked_usage = 0,
};
*out_buffer = reinterpret_cast<AHardwareBuffer*>(data);
buffers_.emplace(*out_buffer, std::move(buffer));
@@ -165,28 +251,80 @@
int32_t fence,
const ARect* _Nullable rect,
void* _Nullable* _Nonnull out_virtual_address) {
- const uint64_t kNonCpuUsageMask = ~(AHARDWAREBUFFER_USAGE_CPU_READ_MASK |
- AHARDWAREBUFFER_USAGE_CPU_WRITE_MASK);
+ const uint64_t kCpuUsageMask = (AHARDWAREBUFFER_USAGE_CPU_READ_MASK |
+ AHARDWAREBUFFER_USAGE_CPU_WRITE_MASK);
auto it = buffers_.find(buffer);
// TODO(shik): Support fence.
- if (it == buffers_.end() || (usage & kNonCpuUsageMask) != 0 || fence >= 0 ||
- rect != nullptr) {
+ if (it == buffers_.end() || fence >= 0 || rect != nullptr) {
return -EINVAL;
}
auto& buf = it->second;
- // TODO(shik): Call proper ioctl() with DMA_BUF_IOCTL_SYNC.
+
+ if (buf.locked_usage != 0) {
+ // TODO(shik): Support multiple concurrent locks if the usages are
+ // compatible. The semantic is a little bit tricky and there is no use
+ // case yet, so simply return an error for now.
+ LOGF(ERROR) << "Buffer is alerady locked";
+ return -EINVAL;
+ }
+
+ if ((usage & kCpuUsageMask) == 0 || (usage & ~kCpuUsageMask) != 0) {
+ LOGF(ERROR) << "Invalid usage mask";
+ return -EINVAL;
+ }
+
+ bool hasRead = (usage & AHARDWAREBUFFER_USAGE_CPU_READ_MASK) != 0;
+ bool canRead = (buf.desc.usage & AHARDWAREBUFFER_USAGE_CPU_READ_MASK) != 0;
+ bool hasWrite = (usage & AHARDWAREBUFFER_USAGE_CPU_WRITE_MASK) != 0;
+ bool canWrite =
+ (buf.desc.usage & AHARDWAREBUFFER_USAGE_CPU_WRITE_MASK) != 0;
+ if ((hasRead && !canRead) || (hasWrite && !canWrite)) {
+ LOGF(ERROR) << "Incompatible usage mask";
+ return -EINVAL;
+ }
+
+ if (buf.is_dmabuf) {
+ dma_buf_sync sync = {
+ .flags = DMA_BUF_SYNC_START | SyncFlagsFromUsageMask(usage),
+ };
+ // TODO(shik): Handle EINTR.
+ int ret = ioctl(buf.fd, DMA_BUF_IOCTL_SYNC, &sync);
+ if (ret != 0) {
+ PLOGF(ERROR) << "ioctl() for DMA_BUF_IOCTL_SYNC failed";
+ return -EINVAL;
+ }
+ }
+
+ buf.locked_usage = usage;
*out_virtual_address = buf.data;
return 0;
}
int Unlock(AHardwareBuffer* _Nonnull buffer, int32_t* _Nullable fence) {
- if (buffers_.find(buffer) == buffers_.end()) {
+ auto it = buffers_.find(buffer);
+ if (it == buffers_.end()) {
return -EINVAL;
}
+ auto& buf = it->second;
+
+ // TODO(shik): Support fence.
if (fence != nullptr) {
*fence = -1;
}
- // TODO(shik): Call proper ioctl() with DMA_BUF_IOCTL_SYNC.
+
+ if (buf.is_dmabuf) {
+ dma_buf_sync sync = {
+ .flags = DMA_BUF_SYNC_END | SyncFlagsFromUsageMask(buf.locked_usage),
+ };
+ // TODO(shik): Handle EINTR.
+ int ret = ioctl(buf.fd, DMA_BUF_IOCTL_SYNC, &sync);
+ if (ret != 0) {
+ PLOGF(ERROR) << "ioctl() for DMA_BUF_IOCTL_SYNC failed";
+ return -EINVAL;
+ }
+ }
+
+ buf.locked_usage = 0;
return 0;
}
@@ -235,19 +373,22 @@
owned_handle->data[0] = fd;
}
- uint32_t size = desc->width;
+ size_t size = desc->width;
void* data = mmap(nullptr, size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
if (data == MAP_FAILED) {
return -errno;
}
+ bool is_dmabuf = IsDmaBuf(fd);
Buffer buffer = {
.fd = ScopedFd(fd),
.data = data,
.size = size,
+ .is_dmabuf = is_dmabuf,
.ref_count = 1,
.desc = *desc,
.handle = std::move(owned_handle),
+ .locked_usage = 0,
};
*out_buffer = reinterpret_cast<AHardwareBuffer*>(data);
buffers_.emplace(*out_buffer, std::move(buffer));
@@ -258,7 +399,8 @@
struct Buffer {
ScopedFd fd;
void* data;
- uint32_t size;
+ size_t size;
+ bool is_dmabuf;
int ref_count;
AHardwareBuffer_Desc desc;
@@ -266,6 +408,10 @@
// ScopedFd. Therefore, we don't need to close it separately when releasing
// native_handle_t, nor do we need to duplicate it (dup) when cloning.
std::unique_ptr<native_handle_t> handle;
+
+ // The usage mask applied when the buffer is locked. The value would be 0
+ // when the buffer is not locked.
+ uint64_t locked_usage;
};
std::map<const AHardwareBuffer*, Buffer> buffers_;
diff --git a/common/android_hardware_buffer_test.cc b/common/android_hardware_buffer_test.cc
index eba1305..202586e 100644
--- a/common/android_hardware_buffer_test.cc
+++ b/common/android_hardware_buffer_test.cc
@@ -244,6 +244,43 @@
ASSERT_EQ(addr, nullptr);
}
+TEST(AHardwareBuffer, LockUsage) {
+ // Allocate a read-only buffer.
+ const AHardwareBuffer_Desc desc = {
+ .width = kSize,
+ .height = 1,
+ .layers = 1,
+ .format = AHARDWAREBUFFER_FORMAT_BLOB,
+ .usage = AHARDWAREBUFFER_USAGE_CPU_READ_OFTEN,
+ .stride = kSize,
+ };
+ AHardwareBuffer* buffer = nullptr;
+ ASSERT_EQ(AHardwareBuffer_allocate(&desc, &buffer), 0);
+ ASSERT_NE(buffer, nullptr);
+
+ // Can be locked for read.
+ void* addr = nullptr;
+ ASSERT_EQ(AHardwareBuffer_lock(buffer, AHARDWAREBUFFER_USAGE_CPU_READ_RARELY,
+ /*fence=*/-1, /*rect=*/nullptr, &addr),
+ 0);
+ ASSERT_NE(addr, nullptr);
+ ASSERT_EQ(AHardwareBuffer_unlock(buffer, /*fence=*/nullptr), 0);
+ addr = nullptr;
+
+ // Cannot be locked for write.
+ ASSERT_NE(AHardwareBuffer_lock(buffer, AHARDWAREBUFFER_USAGE_CPU_WRITE_RARELY,
+ /*fence=*/-1, /*rect=*/nullptr, &addr),
+ 0);
+ ASSERT_EQ(addr, nullptr);
+
+ // Release the buffer. The lock should fail.
+ AHardwareBuffer_release(buffer);
+ ASSERT_NE(AHardwareBuffer_lock(buffer, AHARDWAREBUFFER_USAGE_CPU_READ_RARELY,
+ /*fence=*/-1, /*rect=*/nullptr, &addr),
+ 0);
+ ASSERT_EQ(addr, nullptr);
+}
+
int main(int argc, char** argv) {
testing::InitGoogleTest(&argc, argv);
return RUN_ALL_TESTS();
