linux-kernel - Re: [PATCH v7 04/10] accel/rocket: Add job submission IOCTL

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAL_JsqJtKCXE0MLaBN5-r+w+2jVwt8UMwHtr-nP0PoBmfNodpg@mail.gmail.com>
Date: Mon, 30 Jun 2025 15:56:02 -0500
From: Rob Herring <robh@...nel.org>
To: Tomeu Vizoso <tomeu@...euvizoso.net>
Cc: Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Heiko Stuebner <heiko@...ech.de>, Oded Gabbay <ogabbay@...nel.org>, Jonathan Corbet <corbet@....net>, 
	Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>, 
	Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>, 
	Sumit Semwal <sumit.semwal@...aro.org>, Christian König <christian.koenig@....com>, 
	Sebastian Reichel <sebastian.reichel@...labora.com>, 
	Nicolas Frattaroli <nicolas.frattaroli@...labora.com>, Kever Yang <kever.yang@...k-chips.com>, 
	Robin Murphy <robin.murphy@....com>, Daniel Stone <daniel@...ishbar.org>, Da Xue <da@...re.computer>, 
	Jeff Hugo <jeff.hugo@....qualcomm.com>, devicetree@...r.kernel.org, 
	linux-arm-kernel@...ts.infradead.org, linux-rockchip@...ts.infradead.org, 
	linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org, 
	linux-doc@...r.kernel.org, linux-media@...r.kernel.org, 
	linaro-mm-sig@...ts.linaro.org
Subject: Re: [PATCH v7 04/10] accel/rocket: Add job submission IOCTL

On Fri, Jun 6, 2025 at 1:29 AM Tomeu Vizoso <tomeu@...euvizoso.net> wrote:
>
> Using the DRM GPU scheduler infrastructure, with a scheduler for each
> core.
>
> Userspace can decide for a series of tasks to be executed sequentially
> in the same core, so SRAM locality can be taken advantage of.
>
> The job submission code was initially based on Panfrost.
>
> v2:
> - Remove hardcoded number of cores
> - Misc. style fixes (Jeffrey Hugo)
> - Repack IOCTL struct (Jeffrey Hugo)
>
> v3:
> - Adapt to a split of the register block in the DT bindings (Nicolas
>   Frattaroli)
> - Make use of GPL-2.0-only for the copyright notice (Jeff Hugo)
> - Use drm_* logging functions (Thomas Zimmermann)
> - Rename reg i/o macros (Thomas Zimmermann)
> - Add padding to ioctls and check for zero (Jeff Hugo)
> - Improve error handling (Nicolas Frattaroli)
>
> v6:
> - Use mutexes guard (Markus Elfring)
> - Use u64_to_user_ptr (Jeff Hugo)
> - Drop rocket_fence (Rob Herring)
>
> v7:
> - Assign its own IOMMU domain to each client, for isolation (Daniel
>   Stone and Robin Murphy)
>
> Signed-off-by: Tomeu Vizoso <tomeu@...euvizoso.net>
> ---

[...]

> --- a/include/uapi/drm/rocket_accel.h
> +++ b/include/uapi/drm/rocket_accel.h
> @@ -12,8 +12,10 @@ extern "C" {
>  #endif
>
>  #define DRM_ROCKET_CREATE_BO                   0x00
> +#define DRM_ROCKET_SUBMIT                      0x01
>
>  #define DRM_IOCTL_ROCKET_CREATE_BO             DRM_IOWR(DRM_COMMAND_BASE + DRM_ROCKET_CREATE_BO, struct drm_rocket_create_bo)
> +#define DRM_IOCTL_ROCKET_SUBMIT                        DRM_IOW(DRM_COMMAND_BASE + DRM_ROCKET_SUBMIT, struct drm_rocket_submit)
>
>  /**
>   * struct drm_rocket_create_bo - ioctl argument for creating Rocket BOs.
> @@ -37,6 +39,68 @@ struct drm_rocket_create_bo {
>         __u64 offset;
>  };
>
> +/**
> + * struct drm_rocket_task - A task to be run on the NPU
> + *
> + * A task is the smallest unit of work that can be run on the NPU.
> + */
> +struct drm_rocket_task {
> +       /** Input: DMA address to NPU mapping of register command buffer */
> +       __u64 regcmd;
> +
> +       /** Input: Number of commands in the register command buffer */
> +       __u32 regcmd_count;
> +
> +       /** Reserved, must be zero. */
> +       __u32 reserved;
> +};
> +
> +/**
> + * struct drm_rocket_job - A job to be run on the NPU
> + *
> + * The kernel will schedule the execution of this job taking into account its
> + * dependencies with other jobs. All tasks in the same job will be executed
> + * sequentially on the same core, to benefit from memory residency in SRAM.
> + */
> +struct drm_rocket_job {
> +       /** Input: Pointer to an array of struct drm_rocket_task. */
> +       __u64 tasks;
> +
> +       /** Input: Pointer to a u32 array of the BOs that are read by the job. */
> +       __u64 in_bo_handles;
> +
> +       /** Input: Pointer to a u32 array of the BOs that are written to by the job. */
> +       __u64 out_bo_handles;
> +
> +       /** Input: Number of tasks passed in. */
> +       __u32 task_count;
> +
> +       /** Input: Number of input BO handles passed in (size is that times 4). */
> +       __u32 in_bo_handle_count;
> +
> +       /** Input: Number of output BO handles passed in (size is that times 4). */
> +       __u32 out_bo_handle_count;
> +
> +       /** Reserved, must be zero. */
> +       __u32 reserved;
> +};
> +
> +/**
> + * struct drm_rocket_submit - ioctl argument for submitting commands to the NPU.
> + *
> + * The kernel will schedule the execution of these jobs in dependency order.
> + */
> +struct drm_rocket_submit {
> +       /** Input: Pointer to an array of struct drm_rocket_job. */
> +       __u64 jobs;
> +
> +       /** Input: Number of jobs passed in. */
> +       __u32 job_count;

Isn't there a problem if you need to expand drm_rocket_job beyond
using the 1 reserved field? You can't add to the struct because then
you don't know the size here. So you have to modify drm_rocket_submit
to modify drm_rocket_job. Maybe better if you plan for that now rather
than later by making the size explicit.

Though etnaviv at least has similar issues.

Rob

> +
> +       /** Reserved, must be zero. */
> +       __u32 reserved;
> +};