On Mon, Jan 11, 2021 at 03:00:05PM +0200, Mikko Perttunen wrote:
Add the userspace interface header, specifying interfaces for allocating and accessing syncpoints from userspace, and for creating sync_file based fences based on syncpoint thresholds.
Signed-off-by: Mikko Perttunen mperttunen@nvidia.com
include/uapi/linux/host1x.h | 134 ++++++++++++++++++++++++++++++++++++ 1 file changed, 134 insertions(+) create mode 100644 include/uapi/linux/host1x.h
What's the number of these syncpoints that we expect userspace to create? There's a limited amount of open file descriptors available by default, so this needs to be kept reasonably low.
diff --git a/include/uapi/linux/host1x.h b/include/uapi/linux/host1x.h new file mode 100644 index 000000000000..9c8fb9425cb2 --- /dev/null +++ b/include/uapi/linux/host1x.h @@ -0,0 +1,134 @@ +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ +/* Copyright (c) 2020 NVIDIA Corporation */
+#ifndef _UAPI__LINUX_HOST1X_H +#define _UAPI__LINUX_HOST1X_H
+#include <linux/ioctl.h> +#include <linux/types.h>
+#if defined(__cplusplus) +extern "C" { +#endif
+struct host1x_allocate_syncpoint {
- /**
* @fd: [out]** New file descriptor representing the allocated syncpoint.*/- __s32 fd;
- __u32 reserved[3];
+};
+struct host1x_syncpoint_info {
- /**
* @id: [out]** System-global ID of the syncpoint.*/- __u32 id;
- __u32 reserved[3];
+};
Given that this has only out parameters, I expect this will be called on the FD returned by HOST1X_IOCTL_ALLOCATE_SYNCPOINT? It might be worth pointing that out explicitly in a comment.
+struct host1x_syncpoint_increment {
- /**
* @count: [in]** Number of times to increment the syncpoint. The syncpoint can* be observed at in-between values, but each increment is atomic.*/- __u32 count;
+};
This seems like it would have to be called on the FD as well...
+struct host1x_read_syncpoint {
- /**
* @id: [in]** ID of the syncpoint to read.*/- __u32 id;
- /**
* @value: [out]** Current value of the syncpoint.*/- __u32 value;
+};
... but then, all of a sudden you seem to switch things around and allow reading the value of an arbitrary syncpoint specified by ID.
Now, I suspect that's because reading the syncpoint is harmless and does not allow abuse, whereas incrementing could be abused if allowed on an arbitrary syncpoint ID. But I think it's worth spelling all that out in some documentation to make this clear from a security point of view and from a usability point of view for people trying to figure out how to use these interfaces.
+struct host1x_create_fence {
- /**
* @id: [in]** ID of the syncpoint to create a fence for.*/- __u32 id;
- /**
* @threshold: [in]** When the syncpoint reaches this value, the fence will be signaled.* The syncpoint is considered to have reached the threshold when the* following condition is true:** ((value - threshold) & 0x80000000U) == 0U**/- __u32 threshold;
- /**
* @fence_fd: [out]** New sync_file file descriptor containing the created fence.*/- __s32 fence_fd;
- __u32 reserved[1];
+};
Again this takes an arbitrary syncpoint ID as input, so I expect that the corresponding IOCTL will have to be called on the host1x device node? Again, I think it would be good to either point that out for each structure or IOCTL, or alternatively maybe reorder these such that this becomes clearer.
+struct host1x_fence_extract_fence {
- __u32 id;
- __u32 threshold;
+};
+struct host1x_fence_extract {
- /**
* @fence_fd: [in]** sync_file file descriptor*/- __s32 fence_fd;
- /**
* @num_fences: [in,out]** In: size of the `fences_ptr` array counted in elements.* Out: required size of the `fences_ptr` array counted in elements.*/- __u32 num_fences;
- /**
* @fences_ptr: [in]** Pointer to array of `struct host1x_fence_extract_fence`.*/- __u64 fences_ptr;
- __u32 reserved[2];
+};
For the others it's pretty clear to me what the purpose is, but I'm at a complete loss with this one. What's the use-case for this?
In general I think it'd make sense to add a bit more documentation about how all these IOCTLs are meant to be used to give people a better understanding of why these are needed.
Thierry
+#define HOST1X_IOCTL_ALLOCATE_SYNCPOINT _IOWR('X', 0x00, struct host1x_allocate_syncpoint) +#define HOST1X_IOCTL_READ_SYNCPOINT _IOR ('X', 0x01, struct host1x_read_syncpoint) +#define HOST1X_IOCTL_CREATE_FENCE _IOWR('X', 0x02, struct host1x_create_fence) +#define HOST1X_IOCTL_SYNCPOINT_INFO _IOWR('X', 0x03, struct host1x_syncpoint_info) +#define HOST1X_IOCTL_SYNCPOINT_INCREMENT _IOWR('X', 0x04, struct host1x_syncpoint_increment) +#define HOST1X_IOCTL_FENCE_EXTRACT _IOWR('X', 0x05, struct host1x_fence_extract)
+#if defined(__cplusplus) +} +#endif
+#endif
2.30.0