Re: [PATCH 03/11] accel: ethosu: Ensure cmd stream ends with a stop op

[Frank Li]

On Thu, Aug 27, 2026 at 03:33:02PM -0500, Rob Herring (Arm) wrote:
> While the QSIZE register setting should prevent an out of bounds access
> of the command stream, it is not clear whether the h/w generates an
> interrupt in this case as is required (to prevent a timeout). As a stop op
> is expected end of the command stream, let's just ensure it is present. A
> stop op in the middle of the command stream also makes no sense.
>
> Fixes: 5a5e9c0228e6 ("accel: Add Arm Ethos-U NPU driver")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Rob Herring (Arm) <robh@xxxxxxxxxx>
> ---

Reviewed-by: Frank Li <Frank.Li@xxxxxxx>

>  drivers/accel/ethosu/ethosu_device.h | 1 +
>  drivers/accel/ethosu/ethosu_gem.c    | 9 +++++++++
>  2 files changed, 10 insertions(+)
>
> diff --git a/drivers/accel/ethosu/ethosu_device.h b/drivers/accel/ethosu/ethosu_device.h
> index d4458eac8447..1eca8590e68d 100644
> --- a/drivers/accel/ethosu/ethosu_device.h
> +++ b/drivers/accel/ethosu/ethosu_device.h
> @@ -87,6 +87,7 @@ struct gen_pool;
>  #define PMU_EV_TYPE_IDLE	0x20
>
>  enum ethosu_cmds {
> +	NPU_OP_STOP = 0x0,
>  	NPU_OP_CONV = 0x2,
>  	NPU_OP_DEPTHWISE = 0x3,
>  	NPU_OP_POOL = 0x5,
> diff --git a/drivers/accel/ethosu/ethosu_gem.c b/drivers/accel/ethosu/ethosu_gem.c
> index d50fed64d4d9..eda9f42239be 100644
> --- a/drivers/accel/ethosu/ethosu_gem.c
> +++ b/drivers/accel/ethosu/ethosu_gem.c
> @@ -390,6 +390,7 @@ static int ethosu_gem_cmdstream_copy_and_validate(struct drm_device *ddev,
>  	struct ethosu_validated_cmdstream_info __free(kfree) *info = kzalloc_obj(*info);
>  	struct ethosu_device *edev = to_ethosu_device(ddev);
>  	u32 *bocmds = bo->base.vaddr;
> +	bool ends_with_stop = false;
>  	struct cmd_state st;
>  	int i, ret;
>
> @@ -426,6 +427,11 @@ static int ethosu_gem_cmdstream_copy_and_validate(struct drm_device *ddev,
>  		}
>
>  		switch (cmd) {
> +		case NPU_OP_STOP:
> +			if (i != size / 4 - 1)
> +				return -EINVAL;
> +			ends_with_stop = true;
> +			break;
>  		case NPU_OP_DMA_START:
>  			srclen = dma_length(info, &st.dma, &st.dma.src);
>  			dstlen = dma_length(info, &st.dma, &st.dma.dst);
> @@ -688,6 +694,9 @@ static int ethosu_gem_cmdstream_copy_and_validate(struct drm_device *ddev,
>  		}
>  	}
>
> +	if (!ends_with_stop)
> +		return -EINVAL;
> +
>  	for (i = 0; i < NPU_BASEP_REGION_MAX; i++) {
>  		if (!info->region_size[i])
>  			continue;
>
> --
> 2.53.0
>