Received-SPF: pass (google.com: domain of linux-kernel+bounces-185963-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
From: Barry Song <21cnbao@gmail.com>
To: sumit.semwal@linaro.org,
	linux-media@vger.kernel.org,
	linaro-mm-sig@lists.linaro.org,
	dri-devel@lists.freedesktop.org
Cc: linux-kernel@vger.kernel.org,
	benjamin.gaignard@collabora.com,
	Brian.Starkey@arm.com,
	jstultz@google.com,
	tjmercier@google.com,
	christian.koenig@amd.com,
	Barry Song <v-songbaohua@oppo.com>
Subject: [RFC PATCH] dma-buf: align fd_flags and heap_flags with dma_heap_allocation_data
Date: Wed, 22 May 2024 21:01:58 +1200
Message-Id: <20240522090158.121797-1-21cnbao@gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

From: Barry Song <v-songbaohua@oppo.com>

dma_heap_allocation_data defines the UAPI as follows:

 struct dma_heap_allocation_data {
         __u64 len;
         __u32 fd;
         __u32 fd_flags;
         __u64 heap_flags;
 };

However, dma_heap_buffer_alloc() casts them into unsigned int. It's unclear
whether this is intentional or what the purpose is, but it can be quite
confusing for users.

Adding to the confusion, dma_heap_ops.allocate defines both of these as
unsigned long. Fortunately, since dma_heap_ops is not part of the UAPI,
it is less of a concern.

struct dma_heap_ops {
        struct dma_buf *(*allocate)(struct dma_heap *heap,
                                    unsigned long len,
                                    unsigned long fd_flags,
                                    unsigned long heap_flags);
};

I am sending this RFC in hopes of clarifying these confusions.

If the goal is to constrain both flags to 32 bits while ensuring the struct
is aligned to 64 bits, it would have been more suitable to define
dma_heap_allocation_data accordingly from the beginning, like so:

 struct dma_heap_allocation_data {
         __u64 len;
         __u32 fd;
         __u32 fd_flags;
         __u32 heap_flags;
	 __u32 padding;
 };

Signed-off-by: Barry Song <v-songbaohua@oppo.com>
---
 drivers/dma-buf/dma-heap.c    | 4 ++--
 include/uapi/linux/dma-heap.h | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
index 84ae708fafe7..2298ca5e112e 100644
--- a/drivers/dma-buf/dma-heap.c
+++ b/drivers/dma-buf/dma-heap.c
@@ -50,8 +50,8 @@ static struct class *dma_heap_class;
 static DEFINE_XARRAY_ALLOC(dma_heap_minors);
 
 static int dma_heap_buffer_alloc(struct dma_heap *heap, size_t len,
-				 unsigned int fd_flags,
-				 unsigned int heap_flags)
+				 u32 fd_flags,
+				 u64 heap_flags)
 {
 	struct dma_buf *dmabuf;
 	int fd;
diff --git a/include/uapi/linux/dma-heap.h b/include/uapi/linux/dma-heap.h
index 6f84fa08e074..a4cf716a49fa 100644
--- a/include/uapi/linux/dma-heap.h
+++ b/include/uapi/linux/dma-heap.h
@@ -19,7 +19,7 @@
 #define DMA_HEAP_VALID_FD_FLAGS (O_CLOEXEC | O_ACCMODE)
 
 /* Currently no heap flags */
-#define DMA_HEAP_VALID_HEAP_FLAGS (0)
+#define DMA_HEAP_VALID_HEAP_FLAGS (0ULL)
 
 /**
  * struct dma_heap_allocation_data - metadata passed from userspace for
-- 
2.34.1