* [PATCH] accel: ethosu: Don't read the U65 rounding mode as a storage mode
@ 2026-08-24 15:26 Tomeu Vizoso
2026-08-24 15:39 ` sashiko-bot
2026-08-25 0:36 ` Rob Herring
0 siblings, 2 replies; 3+ messages in thread
From: Tomeu Vizoso @ 2026-08-24 15:26 UTC (permalink / raw)
To: Rob Herring (Arm), Tomeu Vizoso, Oded Gabbay, Thomas Zimmermann,
Frank Li
Cc: dri-devel, linux-kernel
Bits 15:14 of NPU_SET_{IFM,OFM}_PRECISION select the activation storage
mode on U85 only. On U65 the same field holds the rounding mode, and the
command stream parser has read it as a storage mode since the driver was
added.
That went unnoticed while unknown values fell through the switch, but
now that they are rejected, every U65 command stream that asks for
natural rounding (2) fails CMDSTREAM_BO_CREATE with -EINVAL. Mesa emits
it for average pooling, concatenation, split, unpack, strided slice, LUT
and argmax, which is 72 failures of the Teflon test suite on an i.MX93.
Truncating rounding (1) is misread as well: it picks the two-tile
address path and computes a bogus feature map size from tile bases the
command stream never set.
Read the field as a storage mode only on the hardware where it is one.
Fixes: 5a5e9c0228e6 ("accel: Add Arm Ethos-U NPU driver")
Fixes: 6b7e0066294d ("accel: ethosu: Handle U85 internal chaining buffer")
Assisted-by: Claude:claude-opus-5
Signed-off-by: Tomeu Vizoso <tomeu@tomeuvizoso.net>
---
drivers/accel/ethosu/ethosu_gem.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/accel/ethosu/ethosu_gem.c b/drivers/accel/ethosu/ethosu_gem.c
index d50fed64d4d9..fa37a190e9ff 100644
--- a/drivers/accel/ethosu/ethosu_gem.c
+++ b/drivers/accel/ethosu/ethosu_gem.c
@@ -204,7 +204,7 @@ static u64 feat_matrix_length(struct ethosu_device *edev,
struct feat_matrix *fm,
u32 x, u32 y, u32 c, bool ofm)
{
- u32 element_size, storage = fm->precision >> 14;
+ u32 element_size, storage = ethosu_is_u65(edev) ? 0 : fm->precision >> 14;
int tile = 0;
u64 addr;
--
2.54.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] accel: ethosu: Don't read the U65 rounding mode as a storage mode
2026-08-24 15:26 [PATCH] accel: ethosu: Don't read the U65 rounding mode as a storage mode Tomeu Vizoso
@ 2026-08-24 15:39 ` sashiko-bot
2026-08-25 0:36 ` Rob Herring
1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-08-24 15:39 UTC (permalink / raw)
To: Tomeu Vizoso; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing default error case in the feature matrix format switch allows out-of-bounds DMA.
--
--- Patch [1]: [PATCH] accel: ethosu: Don't read the U65 rounding mode as a storage mode ---
commit 1513ddc6b4e0eb1db33e857fa9c9f18ec8c80157
Author: Tomeu Vizoso <tomeu.vizoso@gmail.com>
accel: ethosu: Don't read the U65 rounding mode as a storage mode
This commit modifies the ethosu driver to stop reading bits 15:14 of
the precision field as a storage mode on U65 NPUs. On U65, this field
holds the rounding mode instead, so the storage mode is now forced to 0
for these devices to prevent command stream parsing failures.
> diff --git a/drivers/accel/ethosu/ethosu_gem.c b/drivers/accel/ethosu/ethosu_gem.c
> index d50fed64d4d93..fa37a190e9fff 100644
> --- a/drivers/accel/ethosu/ethosu_gem.c
> +++ b/drivers/accel/ethosu/ethosu_gem.c
> @@ -204,7 +204,7 @@ static u64 feat_matrix_length(struct ethosu_device *edev,
> struct feat_matrix *fm,
> u32 x, u32 y, u32 c, bool ofm)
> {
> - u32 element_size, storage = fm->precision >> 14;
> + u32 element_size, storage = ethosu_is_u65(edev) ? 0 : fm->precision >> 14;
[Severity: High]
This is a pre-existing issue, but by forcing storage to 0 on U65, does this
inadvertently allow uninitialized precision values to bypass storage
validation?
If a command stream omits the NPU_SET_IFM_PRECISION command, fm->precision is
initialized to 0xffff. Because storage is now forced to 0, this bypasses the
earlier switch (storage) check which would have rejected it as 3.
Later in feat_matrix_length(), there is a switch for the format:
switch ((fm->precision >> 6) & 0x3) { // format
case 0: //nhwc:
element_size = BIT((fm->precision >> (ofm ? 1 : 2)) & 0x3);
addr += x * fm->stride_x + c * element_size;
break;
case 1: //nhcwb16:
element_size = BIT((fm->precision >> (ofm ? 1 : 2)) & 0x3);
addr += (c / 16) * fm->stride_c + (16 * x + (c & 0xf)) * element_size;
break;
}
Because there is no default error case, formats 2 and 3 fall through without
adding the width and depth (x and c) components to the required length.
Does this result in an under-calculated buffer length that bypasses the
driver's GEM size validation and could allow out-of-bounds DMA accesses?
> int tile = 0;
> u64 addr;
>
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260824152612.751007-1-tomeu@tomeuvizoso.net?part=1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] accel: ethosu: Don't read the U65 rounding mode as a storage mode
2026-08-24 15:26 [PATCH] accel: ethosu: Don't read the U65 rounding mode as a storage mode Tomeu Vizoso
2026-08-24 15:39 ` sashiko-bot
@ 2026-08-25 0:36 ` Rob Herring
1 sibling, 0 replies; 3+ messages in thread
From: Rob Herring @ 2026-08-25 0:36 UTC (permalink / raw)
To: Tomeu Vizoso
Cc: Tomeu Vizoso, Oded Gabbay, Thomas Zimmermann, Frank Li, dri-devel,
linux-kernel
On Mon, Aug 24, 2026 at 05:26:11PM +0200, Tomeu Vizoso wrote:
> Bits 15:14 of NPU_SET_{IFM,OFM}_PRECISION select the activation storage
> mode on U85 only. On U65 the same field holds the rounding mode, and the
> command stream parser has read it as a storage mode since the driver was
> added.
>
> That went unnoticed while unknown values fell through the switch, but
> now that they are rejected, every U65 command stream that asks for
> natural rounding (2) fails CMDSTREAM_BO_CREATE with -EINVAL. Mesa emits
> it for average pooling, concatenation, split, unpack, strided slice, LUT
> and argmax, which is 72 failures of the Teflon test suite on an i.MX93.
> Truncating rounding (1) is misread as well: it picks the two-tile
> address path and computes a bogus feature map size from tile bases the
> command stream never set.
>
> Read the field as a storage mode only on the hardware where it is one.
>
> Fixes: 5a5e9c0228e6 ("accel: Add Arm Ethos-U NPU driver")
> Fixes: 6b7e0066294d ("accel: ethosu: Handle U85 internal chaining buffer")
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Tomeu Vizoso <tomeu@tomeuvizoso.net>
> ---
> drivers/accel/ethosu/ethosu_gem.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Applied.
I guess I failed to update the i.MX93 kernel with my fix. Sorry about
that.
Rob
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-08-25 0:36 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-24 15:26 [PATCH] accel: ethosu: Don't read the U65 rounding mode as a storage mode Tomeu Vizoso
2026-08-24 15:39 ` sashiko-bot
2026-08-25 0:36 ` Rob Herring
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.