Re: [PATCH 09/11] accel: ethosu: Reject unsupported commands

[Frank Li <Frank.li@oss.nxp.com>]

On Thu, Aug 27, 2026 at 03:33:08PM -0500, Rob Herring (Arm) wrote:
> The command-stream validator does not model U85 branches, indexed DMA,
> or OFM transposes. A branch can bypass the linear validation state,
> indexed DMA accesses an unchecked index buffer, and a transpose changes
> the feature-map address calculation.
>
> Reject those commands and configurations, as well as the reserved DMA
> stride mode and feature-map formats. Reject command-stream IRQs because
> they can signal job completion before later commands finish.
>
> Fixes: 5a5e9c0228e6 ("accel: Add Arm Ethos-U NPU driver")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/accel/ethosu/ethosu_device.h |  4 ++++
>  drivers/accel/ethosu/ethosu_gem.c    | 17 +++++++++++++++++
>  2 files changed, 21 insertions(+)
>
> diff --git a/drivers/accel/ethosu/ethosu_device.h b/drivers/accel/ethosu/ethosu_device.h
> index 1eca8590e68d..c330048dbcca 100644
> --- a/drivers/accel/ethosu/ethosu_device.h
> +++ b/drivers/accel/ethosu/ethosu_device.h
> @@ -86,14 +86,18 @@ struct gen_pool;
>  #define PMU_EV_TYPE_CYCLES	0x11
>  #define PMU_EV_TYPE_IDLE	0x20
>
> +#define NPU_DMA_REGION_INDEX_MODE	BIT(11)
> +
>  enum ethosu_cmds {
>  	NPU_OP_STOP = 0x0,
> +	NPU_OP_IRQ = 0x1,
>  	NPU_OP_CONV = 0x2,
>  	NPU_OP_DEPTHWISE = 0x3,
>  	NPU_OP_POOL = 0x5,
>  	NPU_OP_ELEMENTWISE = 0x6,
>  	NPU_OP_RESIZE = 0x7,	// U85 only
>  	NPU_OP_DMA_START = 0x10,
> +	NPU_OP_BRANCH = 0x4100,	// U85 only
>  	NPU_SET_IFM_PAD_TOP = 0x100,
>  	NPU_SET_IFM_PAD_LEFT = 0x101,
>  	NPU_SET_IFM_PAD_RIGHT = 0x102,
> diff --git a/drivers/accel/ethosu/ethosu_gem.c b/drivers/accel/ethosu/ethosu_gem.c
> index 3d1f4121db4f..2aafbfe95a8c 100644
> --- a/drivers/accel/ethosu/ethosu_gem.c
> +++ b/drivers/accel/ethosu/ethosu_gem.c
> @@ -450,6 +450,9 @@ static int ethosu_gem_cmdstream_copy_and_validate(struct drm_device *ddev,
>  		}
>
>  		switch (cmd) {
> +		case NPU_OP_BRANCH:
> +		case NPU_OP_IRQ:
> +			return -EINVAL;
>  		case NPU_OP_STOP:
>  			if (i != size / 4 - 1)
>  				return -EINVAL;
> @@ -522,6 +525,8 @@ static int ethosu_gem_cmdstream_copy_and_validate(struct drm_device *ddev,
>  			st.ifm.depth = param;
>  			break;
>  		case NPU_SET_IFM_PRECISION:
> +			if (((param >> 6) & 0x3) > 1)
> +				return -EINVAL;
>  			st.ifm.precision = param;
>  			break;
>  		case NPU_SET_IFM_BROADCAST:
> @@ -565,6 +570,10 @@ static int ethosu_gem_cmdstream_copy_and_validate(struct drm_device *ddev,
>  			st.ofm.depth = param;
>  			break;
>  		case NPU_SET_OFM_PRECISION:
> +			if (((param >> 6) & 0x3) > 1)
> +				return -EINVAL;
> +			if (!ethosu_is_u65(edev) && (param & GENMASK(13, 11)))
> +				return -EINVAL;
>  			st.ofm.precision = param;
>  			break;
>  		case NPU_SET_OFM_REGION:
> @@ -599,6 +608,8 @@ static int ethosu_gem_cmdstream_copy_and_validate(struct drm_device *ddev,
>  			st.ifm2.broadcast = param;
>  			break;
>  		case NPU_SET_IFM2_PRECISION:
> +			if (((param >> 6) & 0x3) > 1)
> +				return -EINVAL;
>  			st.ifm2.precision = param;
>  			break;
>  		case NPU_SET_IFM2_REGION:
> @@ -673,13 +684,19 @@ static int ethosu_gem_cmdstream_copy_and_validate(struct drm_device *ddev,
>  			break;
>
>  		case NPU_SET_DMA0_SRC_REGION:
> +			if (param & NPU_DMA_REGION_INDEX_MODE)
> +				return -EINVAL;
>  			if (param & 0x100)
>  				st.dma.src.region = -1;
>  			else
>  				st.dma.src.region = param & 0x7;
>  			st.dma.mode = (param >> 9) & 0x3;
> +			if (st.dma.mode == 3)
> +				return -EINVAL;
>  			break;
>  		case NPU_SET_DMA0_DST_REGION:
> +			if (param & NPU_DMA_REGION_INDEX_MODE)
> +				return -EINVAL;
>  			if (param & 0x100)
>  				st.dma.dst.region = -1;
>  			else
>
> --
> 2.53.0
>