linux-kernel - Re: [PATCH] spi: microchip-core: prevent RX overflows when transmit size

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: <3e8c08fb-ec3a-4d00-a3e9-43c299dcf942@quicinc.com>
Date: Wed, 29 Jan 2025 18:57:34 +0530
From: Mukesh Kumar Savaliya <quic_msavaliy@...cinc.com>
To: Conor Dooley <conor@...nel.org>, <linux-spi@...r.kernel.org>
CC: Conor Dooley <conor.dooley@...rochip.com>, <stable@...r.kernel.org>,
        Daire
 McNamara <daire.mcnamara@...rochip.com>,
        Mark Brown <broonie@...nel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] spi: microchip-core: prevent RX overflows when transmit
 size > FIFO size



On 1/14/2025 10:43 PM, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@...rochip.com>
Not sure why this "From:" is coming here, should be not coming.
> 
> When the size of a transfer exceeds the size of the FIFO (32 bytes), RX
> overflows will be generated and receive data will be corrupted and
> warnings will be produced. For example, here's an error generated by a
> transfer of 36 bytes:
> 
why warning on RX generated when TZ size is > 32 bytes ?
Also you should be taking 32 bytes in a one go and then send remaining 4 
bytes. I am thinking if your TX is faster than the RX receiving ?
>    spi_master spi0: mchp_corespi_interrupt: RX OVERFLOW: rxlen: 4, txlen: 0
> 
> I am not entirely sure how this happens, as rxlen being 4 means that 32
> of 36 bytes have been read from the RX FIFO so there should be
> sufficient room for 4 more bytes but timing is likely a factor as simply
> adding a delay in the transmit path is enough to avoid the overflows.
I suggest to try to chunk in 32 bytes size.
> 
> CC: stable@...r.kernel.org
> Fixes: 9ac8d17694b6 ("spi: add support for microchip fpga spi controllers")
> Signed-off-by: Conor Dooley <conor.dooley@...rochip.com>
> ---
> Been sitting on this one for a bit, original reporter claims the problem
> isn't fixed, but it fixed the issue on my setup so I am sending the patch
> as it's an improvement on the status quo at the very least.
> 
> CC: Conor Dooley <conor.dooley@...rochip.com>
> CC: Daire McNamara <daire.mcnamara@...rochip.com>
> CC: Mark Brown <broonie@...nel.org>
> CC: linux-spi@...r.kernel.org
> CC: linux-kernel@...r.kernel.org
> ---
>   drivers/spi/spi-microchip-core.c | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/spi/spi-microchip-core.c b/drivers/spi/spi-microchip-core.c
> index 5b6af55855ef..3582fe8d3fc4 100644
> --- a/drivers/spi/spi-microchip-core.c
> +++ b/drivers/spi/spi-microchip-core.c
> @@ -221,6 +221,13 @@ static inline void mchp_corespi_write_fifo(struct mchp_corespi *spi)
>   	while ((i < fifo_max) && !(mchp_corespi_read(spi, REG_STATUS) & STATUS_TXFIFO_FULL)) {
>   		u32 word;
>   
> +		/*
> +		 * If the transfer is larger than FIFO_DEPTH, spin until space
> +		 * is made in the RX FIFO to avoid losing data to RX overflows
> +		 */
what if you transfer first 32 bytes and then remaining ?
> +		while (mchp_corespi_read(spi, REG_STATUS) & STATUS_RXFIFO_FULL)
> +			;
> +
>   		if (spi->n_bytes == 4)
>   			word = spi->tx_buf ? *((u32 *)spi->tx_buf) : 0xaa;
>   		else if (spi->n_bytes == 2)