ipc: Support client->server "in_handles"

doc/changes/ipc/mr.413.md

@@ -1 +1,4 @@
-Generalize handling of native-platform handles in IPC code, and de-duplicate code between server and client.
+---
+- mr.427
+---
+Generalize handling of native-platform handles in IPC code, allow bi-directional handle transfer, and de-duplicate code between server and client.

src/xrt/ipc/ipc_server_client.c

@@ -37,9 +37,9 @@
 
 xrt_result_t
 ipc_handle_instance_get_shm_fd(volatile struct ipc_client_state *ics,
-                               size_t max_num_handles,
+                               uint32_t max_num_handles,
                                xrt_shmem_handle_t *out_handles,
-                               size_t *out_num_handles)
+                               uint32_t *out_num_handles)
 {
 	assert(max_num_handles >= 1);
 
@@ -276,9 +276,9 @@ ipc_handle_swapchain_create(volatile struct ipc_client_state *ics,
                             uint32_t *out_id,
                             uint32_t *out_num_images,
                             uint64_t *out_size,
-                            size_t max_num_fds,
+                            uint32_t max_num_handles,
                             xrt_graphics_buffer_handle_t *out_handles,
-                            size_t *out_num_handles)
+                            uint32_t *out_num_handles)
 {
 	// Our handle is just the index for now.
 	uint32_t index = 0;
@@ -315,7 +315,7 @@ ipc_handle_swapchain_create(volatile struct ipc_client_state *ics,
 
 	// Sanity checking.
 	assert(num_images <= IPC_MAX_SWAPCHAIN_FDS);
-	assert(num_images <= max_num_fds);
+	assert(num_images <= max_num_handles);
 
 	*out_id = index;
 	*out_size = xscn->images[0].size;
@@ -349,7 +349,6 @@ xrt_result_t
 ipc_handle_swapchain_acquire_image(volatile struct ipc_client_state *ics,
                                    uint32_t id,
                                    uint32_t *out_index)
-
 {
 	//! @todo Look up the index.
 	uint32_t sc_index = id;

src/xrt/ipc/ipc_utils.c

@@ -19,6 +19,7 @@
 #include <stdio.h>
 #include <string.h>
 #include <stdint.h>
+#include <assert.h>
 
 /*!
  * Debug level logging.
@@ -132,8 +133,13 @@ ipc_receive_fds(struct ipc_message_channel *imc,
                 void *out_data,
                 size_t size,
                 int *out_handles,
-                size_t num_handles)
+                uint32_t num_handles)
 {
+	assert(imc != NULL);
+	assert(out_data != NULL);
+	assert(size != 0);
+	assert(out_handles != NULL);
+	assert(num_handles != 0);
 	union imcontrol_buf u;
 	const size_t fds_size = sizeof(int) * num_handles;
 	const size_t cmsg_size = CMSG_SPACE(fds_size);
@@ -175,8 +181,13 @@ ipc_send_fds(struct ipc_message_channel *imc,
              const void *data,
              size_t size,
              const int *handles,
-             size_t num_handles)
+             uint32_t num_handles)
 {
+	assert(imc != NULL);
+	assert(data != NULL);
+	assert(size != 0);
+	assert(handles != NULL);
+	assert(num_handles != 0);
 	union imcontrol_buf u;
 	size_t cmsg_size = CMSG_SPACE(sizeof(int) * num_handles);
 
@@ -221,7 +232,7 @@ ipc_receive_handles_shmem(struct ipc_message_channel *imc,
                           void *out_data,
                           size_t size,
                           xrt_shmem_handle_t *out_handles,
-                          size_t num_handles)
+                          uint32_t num_handles)
 {
 	return ipc_receive_fds(imc, out_data, size, out_handles, num_handles);
 }
@@ -232,7 +243,7 @@ ipc_send_handles_shmem(struct ipc_message_channel *imc,
                        const void *data,
                        size_t size,
                        const xrt_shmem_handle_t *handles,
-                       size_t num_handles)
+                       uint32_t num_handles)
 {
 	return ipc_send_fds(imc, data, size, handles, num_handles);
 }
@@ -297,7 +308,7 @@ ipc_receive_handles_graphics_buffer(struct ipc_message_channel *imc,
                                     void *out_data,
                                     size_t size,
                                     xrt_graphics_buffer_handle_t *out_handles,
-                                    size_t num_handles)
+                                    uint32_t num_handles)
 {
 	return ipc_receive_fds(imc, out_data, size, out_handles, num_handles);
 }
@@ -308,7 +319,7 @@ ipc_send_handles_graphics_buffer(struct ipc_message_channel *imc,
                                  const void *data,
                                  size_t size,
                                  const xrt_graphics_buffer_handle_t *handles,
-                                 size_t num_handles)
+                                 uint32_t num_handles)
 {
 	return ipc_send_fds(imc, data, size, handles, num_handles);
 }

src/xrt/ipc/ipc_utils.h

@@ -14,6 +14,7 @@
 
 #include <stddef.h>
 #include <stdbool.h>
+#include <stdint.h>
 
 #ifdef __cplusplus
 extern "C" {
@@ -39,6 +40,13 @@ ipc_message_channel_close(struct ipc_message_channel *imc);
  *
  * There are other functions if you have handles, not just scalar/aggregate
  * data.
+ *
+ * @param imc Message channel to use
+ * @param[in] data Pointer to the data buffer to send. Must not be
+ * null: use a filler message if necessary.
+ * @param[in] size Size of data pointed-to by @p data, must be greater than 0
+ *
+ * @public @memberof ipc_message_channel
  */
 xrt_result_t
 ipc_send(struct ipc_message_channel *imc, const void *data, size_t size);
@@ -48,6 +56,13 @@ ipc_send(struct ipc_message_channel *imc, const void *data, size_t size);
  *
  * There are other functions if you have handles, not just scalar/aggregate
  * data.
+ *
+ * @param imc Message channel to use
+ * @param[out] out_data Pointer to the buffer to fill with data. Must not be
+ * null.
+ * @param[in] size Maximum size to read, must be greater than 0
+ *
+ * @public @memberof ipc_message_channel
  */
 xrt_result_t
 ipc_receive(struct ipc_message_channel *imc, void *out_data, size_t size);
@@ -58,26 +73,51 @@ ipc_receive(struct ipc_message_channel *imc, void *out_data, size_t size);
  * functions.
  * @{
  */
+#ifdef XRT_OS_UNIX
 /*!
  * Receive a message along with a known number of file descriptors over the IPC
  * channel.
+ *
+ * @param imc Message channel to use
+ * @param[out] out_data Pointer to the buffer to fill with data. Must not be
+ * null.
+ * @param[in] size Maximum size to read, must be greater than 0
+ * @param[out] out_handles Array of file descriptors to populate.  Must not be
+ * null.
+ * @param[in] num_handles Number of elements to receive into @p out_handles,
+ * must be greater than 0 and must match the value provided at the other end.
+ *
+ * @public @memberof ipc_message_channel
  */
 xrt_result_t
 ipc_receive_fds(struct ipc_message_channel *imc,
                 void *out_data,
                 size_t size,
                 int *out_handles,
-                size_t num_handles);
+                uint32_t num_handles);
 
 /*!
  * Send a message along with file descriptors over the IPC channel.
+ *
+ * @param imc Message channel to use
+ * @param[in] data Pointer to the data buffer to send. Must not be
+ * null: use a filler message if necessary.
+ * @param[in] size Size of data pointed-to by @p data, must be greater than 0
+ * @param[out] handles Array of file descriptors to send.  Must not be
+ * null.
+ * @param[in] num_handles Number of elements in @p handles, must be greater
+ * than 0. If this is variable, it must also be separately transmitted ahead of
+ * time, because the receiver must have the same value in its receive call.
+ *
+ * @public @memberof ipc_message_channel
  */
 xrt_result_t
 ipc_send_fds(struct ipc_message_channel *imc,
              const void *data,
              size_t size,
              const int *handles,
-             size_t num_handles);
+             uint32_t num_handles);
+#endif // XRT_OS_UNIX
 /*!
  * @}
  */
@@ -88,20 +128,53 @@ ipc_send_fds(struct ipc_message_channel *imc,
  * data.
  * @{
  */
+
+/*!
+ * Receive a message along with a known number of shared memory handles over the
+ * IPC channel.
+ *
+ * @param imc Message channel to use
+ * @param[out] out_data Pointer to the buffer to fill with data. Must not be
+ * null.
+ * @param[in] size Maximum size to read, must be greater than 0
+ * @param[out] out_handles Array of shared memory handles to populate.  Must not
+ * be null.
+ * @param[in] num_handles Number of elements to receive into @p out_handles,
+ * must be greater than 0 and must match the value provided at the other end.
+ *
+ * @public @memberof ipc_message_channel
+ * @relatesalso xrt_shmem_handle_t
+ */
 xrt_result_t
 ipc_receive_handles_shmem(struct ipc_message_channel *imc,
                           void *out_data,
                           size_t size,
                           xrt_shmem_handle_t *out_handles,
-                          size_t num_handles);
+                          uint32_t num_handles);
 
 
+/*!
+ * Send a message along with shared memory handles over the IPC channel.
+ *
+ * @param imc Message channel to use
+ * @param[in] data Pointer to the data buffer to send. Must not be
+ * null: use a filler message if necessary.
+ * @param[in] size Size of data pointed-to by @p data, must be greater than 0
+ * @param[out] handles Array of shared memory handles to send.  Must not be
+ * null.
+ * @param[in] num_handles Number of elements in @p handles, must be greater
+ * than 0. If this is variable, it must also be separately transmitted ahead of
+ * time, because the receiver must have the same value in its receive call.
+ *
+ * @public @memberof ipc_message_channel
+ * @relatesalso xrt_shmem_handle_t
+ */
 xrt_result_t
 ipc_send_handles_shmem(struct ipc_message_channel *imc,
                        const void *data,
                        size_t size,
                        const xrt_shmem_handle_t *handles,
-                       size_t num_handles);
+                       uint32_t num_handles);
 /*!
  * @}
  */
@@ -112,20 +185,53 @@ ipc_send_handles_shmem(struct ipc_message_channel *imc,
  * message data.
  * @{
  */
+/*!
+ * Receive a message along with a known number of graphics buffer handles over
+ * the IPC channel.
+ *
+ * @param imc Message channel to use
+ * @param[out] out_data Pointer to the buffer to fill with data. Must not be
+ * null.
+ * @param[in] size Maximum size to read, must be greater than 0
+ * @param[out] out_handles Array of graphics buffer handles to populate.  Must
+ * not be null.
+ * @param[in] num_handles Number of elements to receive into @p out_handles,
+ * must be greater than 0 and must match the value provided at the other end.
+ *
+ * @public @memberof ipc_message_channel
+ * @relatesalso xrt_graphics_buffer_handle_t
+ */
 xrt_result_t
 ipc_receive_handles_graphics_buffer(struct ipc_message_channel *imc,
                                     void *out_data,
                                     size_t size,
                                     xrt_graphics_buffer_handle_t *out_handles,
-                                    size_t num_handles);
+                                    uint32_t num_handles);
 
 
+/*!
+ * Send a message along with native graphics buffer handles over the IPC
+ * channel.
+ *
+ * @param imc Message channel to use
+ * @param[in] data Pointer to the data buffer to send. Must not be
+ * null: use a filler message if necessary.
+ * @param[in] size Size of data pointed-to by @p data, must be greater than 0
+ * @param[out] handles Array of graphics buffer handles to send.  Must not be
+ * null.
+ * @param[in] num_handles Number of elements in @p handles, must be greater
+ * than 0. If this is variable, it must also be separately transmitted ahead of
+ * time, because the receiver must have the same value in its receive call.
+ *
+ * @public @memberof ipc_message_channel
+ * @relatesalso xrt_graphics_buffer_handle_t
+ */
 xrt_result_t
 ipc_send_handles_graphics_buffer(struct ipc_message_channel *imc,
                                  const void *data,
                                  size_t size,
                                  const xrt_graphics_buffer_handle_t *handles,
-                                 size_t num_handles);
+                                 uint32_t num_handles);
 /*!
  * @}
  */

src/xrt/ipc/ipcproto/common.py

@@ -102,6 +102,7 @@ class Arg:
 
 class HandleType:
     """A native handle type requiring special treatment."""
+
     # Keep this synchronized with the definition in the JSON Schema.
     HANDLE_RE = re.compile(r"xrt_([a-z_]+)_handle_t")
 
@@ -130,6 +131,11 @@ class HandleType:
         """Get the name of the count argument."""
         return 'num_'+self.argstem
 
+    @property
+    def count_arg_type(self):
+        """Get the type of the count argument."""
+        return "uint32_t"
+
     @property
     def arg_names(self):
         """Get the argument names for the client proxy."""
@@ -140,7 +146,7 @@ class HandleType:
     def arg_decls(self):
         """Get the argument declarations for the client proxy."""
         types = (self.typename + ' *',
-                 'size_t ')
+                 self.count_arg_type + ' ')
         return (x + y for x, y in zip(types, self.arg_names))
 
     @property
@@ -153,9 +159,9 @@ class HandleType:
     @property
     def handler_arg_decls(self):
         """Get the argument declarations for the server handler."""
-        types = ('size_t ',
+        types = (self.count_arg_type + ' ',
                  self.typename + ' *',
-                 'size_t *')
+                 self.count_arg_type + ' *')
         return (x + y for x, y in zip(types, self.handler_arg_names))
 
 
@@ -178,6 +184,8 @@ class Call:
         """Write declaration of ipc_call_CALLNAME."""
         args = ["struct ipc_connection *ipc_c"]
         args.extend(arg.get_func_argument_in() for arg in self.in_args)
+        if self.in_handles:
+            args.extend(self.in_handles.arg_decls)
         args.extend(arg.get_func_argument_out() for arg in self.out_args)
         if self.out_handles:
             args.extend(self.out_handles.arg_decls)
@@ -190,14 +198,22 @@ class Call:
         args.extend(arg.get_func_argument_out() for arg in self.out_args)
         if self.out_handles:
             args.extend(self.out_handles.handler_arg_decls)
+        if self.in_handles:
+            args.extend(self.in_handles.arg_decls)
         write_decl(f, 'xrt_result_t', 'ipc_handle_' + self.name, args)
 
+    @property
+    def needs_msg_struct(self):
+        """Decide whether this call needs a msg struct."""
+        return self.in_args or self.in_handles
+
     def __init__(self, name, data):
         """Construct a call from call name and call data dictionary."""
         self.id = None
         self.name = name
         self.in_args = []
         self.out_args = []
+        self.in_handles = None
         self.out_handles = None
         for key, val in data.items():
             if key == 'id':
@@ -208,6 +224,8 @@ class Call:
                 self.out_args = Arg.parse_array(val)
             elif key == 'out_handles':
                 self.out_handles = HandleType(val)
+            elif key == 'in_handles':
+                self.in_handles = HandleType(val)
             else:
                 raise RuntimeError("Unrecognized key")
         if not self.id:

src/xrt/ipc/proto.py

@@ -3,9 +3,11 @@
 # SPDX-License-Identifier: BSL-1.0
 """Generate code from a JSON file describing the IPC protocol."""
 
-from ipcproto.common import Proto, write_invocation, write_result_handler
 import argparse
 
+from ipcproto.common import (Proto, write_decl, write_invocation,
+                             write_result_handler)
+
 header = '''// Copyright 2020, Collabora, Ltd.
 // SPDX-License-Identifier: BSL-1.0
 /*!
@@ -72,12 +74,15 @@ ipc_cmd_to_str(ipc_command_t id)
 
     for call in p.calls:
         # Should we emit a msg struct.
-        if call.in_args:
+        if call.needs_msg_struct:
             f.write("\nstruct ipc_" + call.name + "_msg\n")
             f.write("{\n")
             f.write("\tenum ipc_command cmd;\n")
             for arg in call.in_args:
                 f.write("\t" + arg.get_struct_field() + ";\n")
+            if call.in_handles:
+                f.write("\t%s %s;\n" % (call.in_handles.count_arg_type,
+                                        call.in_handles.count_arg_name))
             f.write("};\n")
         # Should we emit a reply struct.
         if call.out_args:
@@ -110,7 +115,7 @@ def generate_client_c(file, p):
         f.write("\n{\n")
 
         # Message struct
-        if call.in_args:
+        if call.needs_msg_struct:
             f.write("\tstruct ipc_" + call.name + "_msg _msg = {\n")
         else:
             f.write("\tstruct ipc_command_msg _msg = {\n")
@@ -120,6 +125,9 @@ def generate_client_c(file, p):
                 f.write("\t    ." + arg.name + " = *" + arg.name + ",\n")
             else:
                 f.write("\t    ." + arg.name + " = " + arg.name + ",\n")
+        if call.in_handles:
+            f.write("\t    ." + call.in_handles.count_arg_name +
+                    " = " + call.in_handles.count_arg_name + ",\n")
         f.write("\t};\n")
 
         # Reply struct
@@ -133,12 +141,40 @@ def generate_client_c(file, p):
 \tos_mutex_lock(&ipc_c->mutex);
 """)
         cleanup = "os_mutex_unlock(&ipc_c->mutex);"
+
+        # Prepare initial sending
         func = 'ipc_send'
         args = ['&ipc_c->imc', '&_msg', 'sizeof(_msg)']
+        f.write("\n\t// Send our request")
         write_invocation(f, 'xrt_result_t ret', func, args, indent="\t")
         f.write(';')
         write_result_handler(f, 'ret', cleanup, indent="\t")
 
+        if call.in_handles:
+            # Must send these in a second message
+            # since the server doesn't know how many to expect.
+            f.write("\n\t// Send our handles separately\n")
+            f.write("\n\t// We need this message data as filler only\n")
+            f.write("\tstruct ipc_command_msg _handle_msg = {\n")
+            f.write("\t    .cmd = " + str(call.id) + ",\n")
+            f.write("\t};\n")
+            write_invocation(
+                f,
+                'ret',
+                'ipc_send_handles_' + call.in_handles.stem,
+                (
+                    '&ipc_c->imc',
+                    "&_handle_msg",
+                    "sizeof(_handle_msg)",
+                    call.in_handles.arg_name,
+                    call.in_handles.count_arg_name
+                ),
+                indent="\t"
+            )
+            f.write(';')
+            write_result_handler(f, 'ret', cleanup, indent="\t")
+
+        f.write("\n\t// Await the reply")
         func = 'ipc_receive'
         args = ['&ipc_c->imc', '&_reply', 'sizeof(_reply)']
         if call.out_handles:
@@ -208,7 +244,7 @@ ipc_dispatch(volatile struct ipc_client_state *ics, ipc_command_t *ipc_command)
 
     for call in p.calls:
         f.write("\tcase " + call.id + ": {\n")
-        if call.in_args:
+        if call.needs_msg_struct:
             f.write(
                 "\t\tstruct ipc_{}_msg *msg =\n".format(call.name))
             f.write(
@@ -221,9 +257,36 @@ ipc_dispatch(volatile struct ipc_client_state *ics, ipc_command_t *ipc_command)
         if call.out_handles:
             f.write("\t\t%s %s[MAX_HANDLES] = {0};\n" % (
                 call.out_handles.typename, call.out_handles.arg_name))
-            f.write("\t\tsize_t %s = {0};\n" % call.out_handles.count_arg_name)
+            f.write("\t\t%s %s = {0};\n" % (
+                call.out_handles.count_arg_type,
+                call.out_handles.count_arg_name))
         f.write("\n")
 
+        if call.in_handles:
+            # We need to fetch these handles separately
+            f.write("\t\t%s in_%s[MAX_HANDLES] = {0};\n" % (
+                call.in_handles.typename, call.in_handles.arg_name))
+            f.write("\t\tstruct ipc_command_msg _handle_msg = {0};\n")
+            write_invocation(
+                f,
+                'xrt_result_t receive_handle_result',
+                'ipc_receive_handles_' + call.in_handles.stem,
+                (
+                    "(struct ipc_message_channel *)&ics->imc",
+                    "&_handle_msg",
+                    "sizeof(_handle_msg)",
+                    "in_" + call.in_handles.arg_name,
+                    "msg->"+call.in_handles.count_arg_name
+                ),
+                indent="\t\t"
+            )
+            f.write(";")
+            write_result_handler(f, "receive_handle_result",
+                                 indent="\t\t")
+            f.write("\t\tif (_handle_msg.cmd != %s) {\n" % str(call.id))
+            f.write("\t\t\treturn XRT_ERROR_IPC_FAILURE;\n")
+            f.write("\t\t}\n")
+
         # Write call to ipc_handle_CALLNAME
         args = ["ics"]
         for arg in call.in_args:
@@ -235,6 +298,10 @@ ipc_dispatch(volatile struct ipc_client_state *ics, ipc_command_t *ipc_command)
             args.extend(("MAX_HANDLES",
                          call.out_handles.arg_name,
                          "&" + call.out_handles.count_arg_name))
+
+        if call.in_handles:
+            args.extend(("&in_%s[0]" % call.in_handles.arg_name,
+                         "msg->"+call.in_handles.count_arg_name))
         write_invocation(f, 'reply.result', 'ipc_handle_' +
                          call.name, args, indent="\t\t")
         f.write(";\n")
@@ -281,10 +348,19 @@ def generate_server_header(file, p):
 
 
 // clang-format off
-
-xrt_result_t
-ipc_dispatch(volatile struct ipc_client_state *ics, ipc_command_t *ipc_command);
 ''')
+    # This decl is constant, but we must write it here
+    # because it depends on a generated enum.
+    write_decl(
+        f,
+        "xrt_result_t",
+        "ipc_dispatch",
+        [
+            "volatile struct ipc_client_state *ics",
+            "ipc_command_t *ipc_command"
+        ]
+    )
+    f.write(";\n")
 
     for call in p.calls:
         call.write_handler_decl(f)

src/xrt/ipc/proto.schema.json

@@ -97,6 +97,19 @@
                     }
                 }
             },
+            "in_handles": {
+                "$id": "#/call/properties/in_handles",
+                "type": "object",
+                "title": "Handles supplied to call",
+                "required": [
+                    "type"
+                ],
+                "properties": {
+                    "type": {
+                        "$ref": "#/definitions/handle"
+                    }
+                }
+            },
             "in": {
                 "title": "Input parameters",
                 "$ref": "#/definitions/param_list"