linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] staging: axis-fifo: fix maximum TX packet length check
@ 2025-08-02 20:59 Ovidiu Panait
  2025-08-17 11:12 ` Greg KH
  0 siblings, 1 reply; 3+ messages in thread
From: Ovidiu Panait @ 2025-08-02 20:59 UTC (permalink / raw)
  To: gshahrouzi, gregkh, linux-kernel; +Cc: Ovidiu Panait

Since commit 2ca34b508774 ("staging: axis-fifo: Correct handling of
tx_fifo_depth for size validation"), write() operations with packets
larger than 'tx_fifo_depth - 4' words are no longer rejected with -EINVAL.

Fortunately, the packets are not actually getting transmitted to hardware,
otherwise they would be raising a 'Transmit Packet Overrun Error'
interrupt, which requires a reset of the TX circuit to recover from.

Instead, the request times out inside wait_event_interruptible_timeout()
and always returns -EAGAIN, since the wake up condition can never be true
for these packets. But still, they unnecessarily block other tasks from
writing to the FIFO and the EAGAIN return code signals userspace to retry
the write() call, even though it will always fail and time out.

According to the AXI4-Stream FIFO reference manual (PG080), the maximum
valid packet length is 'tx_fifo_depth - 4' words, so attempting to send
larger packets is invalid and should not be happening in the first place:

> The maximum packet that can be transmitted is limited by the size of
> the FIFO, which is (C_TX_FIFO_DEPTH–4)*(data interface width/8) bytes.

Therefore, bring back the old behavior and outright reject packets larger
than 'tx_fifo_depth - 4' with -EINVAL. Add a comment to explain why the
check is necessary. The dev_err() message was removed to avoid cluttering
the dmesg log if an invalid packet is received from userspace.

Fixes: 2ca34b508774 ("staging: axis-fifo: Correct handling of tx_fifo_depth for size validation")
Signed-off-by: Ovidiu Panait <ovidiu.panait.oss@gmail.com>
---
 drivers/staging/axis-fifo/axis-fifo.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/axis-fifo/axis-fifo.c b/drivers/staging/axis-fifo/axis-fifo.c
index 25c0fa8c415d..0f244f80c368 100644
--- a/drivers/staging/axis-fifo/axis-fifo.c
+++ b/drivers/staging/axis-fifo/axis-fifo.c
@@ -322,11 +322,17 @@ static ssize_t axis_fifo_write(struct file *f, const char __user *buf,
 		return -EINVAL;
 	}
 
-	if (words_to_write > fifo->tx_fifo_depth) {
-		dev_err(fifo->dt_device, "tried to write more words [%u] than slots in the fifo buffer [%u]\n",
-			words_to_write, fifo->tx_fifo_depth);
+	/*
+	 * In 'Store-and-Forward' mode, the maximum packet that can be
+	 * transmitted is limited by the size of the FIFO, which is
+	 * (C_TX_FIFO_DEPTH–4)*(data interface width/8) bytes.
+	 *
+	 * Do not attempt to send a packet larger than 'tx_fifo_depth - 4',
+	 * otherwise a 'Transmit Packet Overrun Error' interrupt will be
+	 * raised, which requires a reset of the TX circuit to recover.
+	 */
+	if (words_to_write > (fifo->tx_fifo_depth - 4))
 		return -EINVAL;
-	}
 
 	if (fifo->write_flags & O_NONBLOCK) {
 		/*
-- 
2.50.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] staging: axis-fifo: fix maximum TX packet length check
  2025-08-02 20:59 [PATCH] staging: axis-fifo: fix maximum TX packet length check Ovidiu Panait
@ 2025-08-17 11:12 ` Greg KH
  2025-08-17 16:08   ` Ovidiu Panait
  0 siblings, 1 reply; 3+ messages in thread
From: Greg KH @ 2025-08-17 11:12 UTC (permalink / raw)
  To: Ovidiu Panait; +Cc: gshahrouzi, linux-kernel

On Sat, Aug 02, 2025 at 11:59:43PM +0300, Ovidiu Panait wrote:
> Since commit 2ca34b508774 ("staging: axis-fifo: Correct handling of
> tx_fifo_depth for size validation"), write() operations with packets
> larger than 'tx_fifo_depth - 4' words are no longer rejected with -EINVAL.
> 
> Fortunately, the packets are not actually getting transmitted to hardware,
> otherwise they would be raising a 'Transmit Packet Overrun Error'
> interrupt, which requires a reset of the TX circuit to recover from.
> 
> Instead, the request times out inside wait_event_interruptible_timeout()
> and always returns -EAGAIN, since the wake up condition can never be true
> for these packets. But still, they unnecessarily block other tasks from
> writing to the FIFO and the EAGAIN return code signals userspace to retry
> the write() call, even though it will always fail and time out.
> 
> According to the AXI4-Stream FIFO reference manual (PG080), the maximum
> valid packet length is 'tx_fifo_depth - 4' words, so attempting to send
> larger packets is invalid and should not be happening in the first place:
> 
> > The maximum packet that can be transmitted is limited by the size of
> > the FIFO, which is (C_TX_FIFO_DEPTH–4)*(data interface width/8) bytes.
> 
> Therefore, bring back the old behavior and outright reject packets larger
> than 'tx_fifo_depth - 4' with -EINVAL. Add a comment to explain why the
> check is necessary. The dev_err() message was removed to avoid cluttering
> the dmesg log if an invalid packet is received from userspace.
> 
> Fixes: 2ca34b508774 ("staging: axis-fifo: Correct handling of tx_fifo_depth for size validation")
> Signed-off-by: Ovidiu Panait <ovidiu.panait.oss@gmail.com>

Any specific reason you didn't cc: the staging list?  Or add cc: stable
to this commit?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] staging: axis-fifo: fix maximum TX packet length check
  2025-08-17 11:12 ` Greg KH
@ 2025-08-17 16:08   ` Ovidiu Panait
  0 siblings, 0 replies; 3+ messages in thread
From: Ovidiu Panait @ 2025-08-17 16:08 UTC (permalink / raw)
  To: Greg KH; +Cc: gshahrouzi, linux-kernel



On 8/17/25 2:12 PM, Greg KH wrote:
> On Sat, Aug 02, 2025 at 11:59:43PM +0300, Ovidiu Panait wrote:
>> Since commit 2ca34b508774 ("staging: axis-fifo: Correct handling of
>> tx_fifo_depth for size validation"), write() operations with packets
>> larger than 'tx_fifo_depth - 4' words are no longer rejected with -EINVAL.
>>
>> Fortunately, the packets are not actually getting transmitted to hardware,
>> otherwise they would be raising a 'Transmit Packet Overrun Error'
>> interrupt, which requires a reset of the TX circuit to recover from.
>>
>> Instead, the request times out inside wait_event_interruptible_timeout()
>> and always returns -EAGAIN, since the wake up condition can never be true
>> for these packets. But still, they unnecessarily block other tasks from
>> writing to the FIFO and the EAGAIN return code signals userspace to retry
>> the write() call, even though it will always fail and time out.
>>
>> According to the AXI4-Stream FIFO reference manual (PG080), the maximum
>> valid packet length is 'tx_fifo_depth - 4' words, so attempting to send
>> larger packets is invalid and should not be happening in the first place:
>>
>>> The maximum packet that can be transmitted is limited by the size of
>>> the FIFO, which is (C_TX_FIFO_DEPTH–4)*(data interface width/8) bytes.
>>
>> Therefore, bring back the old behavior and outright reject packets larger
>> than 'tx_fifo_depth - 4' with -EINVAL. Add a comment to explain why the
>> check is necessary. The dev_err() message was removed to avoid cluttering
>> the dmesg log if an invalid packet is received from userspace.
>>
>> Fixes: 2ca34b508774 ("staging: axis-fifo: Correct handling of tx_fifo_depth for size validation")
>> Signed-off-by: Ovidiu Panait <ovidiu.panait.oss@gmail.com>
> 
> Any specific reason you didn't cc: the staging list?  Or add cc: stable
> to this commit?
> 

It was a copy-paste error. I'll resend this with the cc: stable tag
included.

Thanks,
Ovidiu

> thanks,
> 
> greg k-h


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-08-17 16:09 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-02 20:59 [PATCH] staging: axis-fifo: fix maximum TX packet length check Ovidiu Panait
2025-08-17 11:12 ` Greg KH
2025-08-17 16:08   ` Ovidiu Panait

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).