On Tue, Mar 5, 2024 at 12:42 AM Arnd Bergmann [off-list ref] wrote:

On Tue, Mar 5, 2024, at 03:01, Mina Almasry wrote:

quoted

--- a/arch/alpha/include/uapi/asm/socket.h
+++ b/arch/alpha/include/uapi/asm/socket.h
 #define SO_PEERPIDFD         77
+#define SO_DEVMEM_LINEAR     79
+#define SO_DEVMEM_DMABUF     80
--- a/arch/mips/include/uapi/asm/socket.h
+++ b/arch/mips/include/uapi/asm/socket.h
 #define SO_PEERPIDFD         77
+#define SO_DEVMEM_LINEAR     79
+#define SO_DEVMEM_DMABUF     80
--- a/arch/parisc/include/uapi/asm/socket.h
+++ b/arch/parisc/include/uapi/asm/socket.h
 #define SO_PEERPIDFD         0x404B
+#define SO_DEVMEM_LINEAR     98
+#define SO_DEVMEM_DMABUF     99
--- a/arch/sparc/include/uapi/asm/socket.h
+++ b/arch/sparc/include/uapi/asm/socket.h
 #define SO_PEERPIDFD             0x0056
+#define SO_DEVMEM_LINEAR         0x0058
+#define SO_DEVMEM_DMABUF         0x0059
--- a/include/uapi/asm-generic/socket.h
+++ b/include/uapi/asm-generic/socket.h

@@ -135,6 +135,11 @@
 #define SO_PEERPIDFD         77
+#define SO_DEVMEM_LINEAR     98
+#define SO_DEVMEM_DMABUF     99

These look inconsistent. I can see how you picked the
alpha and mips numbers, but how did you come up with
the generic and parisc ones? Can you follow the existing
scheme instead?

Sorry, yes, this is a bit weird. I'll change this to use the next
available entry rather than leave a gap.

quoted

diff --git a/include/uapi/linux/uio.h b/include/uapi/linux/uio.h
index 059b1a9147f4..ad92e37699da 100644
--- a/include/uapi/linux/uio.h
+++ b/include/uapi/linux/uio.h

@@ -20,6 +20,16 @@ struct iovec
      __kernel_size_t iov_len; /* Must be size_t (1003.1g) */
 };

+struct dmabuf_cmsg {
+     __u64 frag_offset;      /* offset into the dmabuf where the frag starts.
+                              */
+     __u32 frag_size;        /* size of the frag. */
+     __u32 frag_token;       /* token representing this frag for
+                              * DEVMEM_DONTNEED.
+                              */
+     __u32  dmabuf_id;       /* dmabuf id this frag belongs to. */
+};

This structure requires a special compat handler to run
x86-32 binaries on x86-64 because of the different alignment
requirements. Any uapi-visible structures should be defined
to avoid this and just have no holes in them. Maybe extend
one of the __u32 members to __u64 or add another 32-bit padding field?

Honestly the 32-bit fields as-is are somewhat comically large. I don't
think extending the __u32 -> __u64 is preferred because I don't see us
needing that much, so maybe I can add another 32-bit padding field.
Does this look good to you?

struct dmabuf_cmsg {
  __u64 frag_offset;
  __u32 frag_size;
  __u32 frag_token;
  __u32 dmabuf_id;
  __u32 ext; /* reserved for future flags */
};

Another option is to actually compress frag_token & dmabuf_id to be
32-bit combined size if that addresses your concern. I prefer that
less in case they end up being too small for future use cases.

-- 
Thanks,
Mina