Received-SPF: pass (google.com: domain of linux-kernel+bounces-24439-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <64deebbd-93d0-47dc-835e-f719655e076c@suse.de>
Date: Fri, 12 Jan 2024 11:56:18 +0300
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] net: ethernet: ravb: fix dma mapping failure handling
Content-Language: en-US
To: Nikita Yushchenko <nikita.yoush@cogentembedded.com>,
 "David S. Miller" <davem@davemloft.net>, Jakub Kicinski <kuba@kernel.org>,
 Paolo Abeni <pabeni@redhat.com>
Cc: Sergey Shtylyov <s.shtylyov@omp.ru>,
 Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>,
 Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>,
 Wolfram Sang <wsa+renesas@sang-engineering.com>,
 =?UTF-8?Q?Uwe_Kleine-K=C3=B6nig?= <u.kleine-koenig@pengutronix.de>,
 netdev@vger.kernel.org, linux-renesas-soc@vger.kernel.org,
 linux-kernel@vger.kernel.org
References: <20240112050639.405784-1-nikita.yoush@cogentembedded.com>
From: Denis Kirjanov <dkirjanov@suse.de>
In-Reply-To: <20240112050639.405784-1-nikita.yoush@cogentembedded.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit


On 1/12/24 08:06, Nikita Yushchenko wrote:
> dma_mapping_error() depends on getting full 64-bit dma_addr_t and does
> not work correctly if 32-bit value is passed instead.
> 
> Fix handling of dma_map_single() failures on Rx ring entries:
> - do not store return value of dma_map_signle() in 32-bit variable,
> - do not use dma_mapping_error() against 32-bit descriptor field when
>   checking if unmap is needed, check for zero size instead.

Hmm, something is wrong here since you're mixing DMA api and forced 32bit values.
if dma uses 32bit addresses then dma_addr_t need only be 32 bits wide


> 
> Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
> ---
>  drivers/net/ethernet/renesas/ravb_main.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> index 8649b3e90edb..4d4b5d44c4e7 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> @@ -256,8 +256,7 @@ static void ravb_rx_ring_free_gbeth(struct net_device *ndev, int q)
>  	for (i = 0; i < priv->num_rx_ring[q]; i++) {
>  		struct ravb_rx_desc *desc = &priv->gbeth_rx_ring[i];
>  
> -		if (!dma_mapping_error(ndev->dev.parent,
> -				       le32_to_cpu(desc->dptr)))
> +		if (le16_to_cpu(desc->ds_cc) != 0)
>  			dma_unmap_single(ndev->dev.parent,
>  					 le32_to_cpu(desc->dptr),
>  					 GBETH_RX_BUFF_MAX,
> @@ -281,8 +280,7 @@ static void ravb_rx_ring_free_rcar(struct net_device *ndev, int q)
>  	for (i = 0; i < priv->num_rx_ring[q]; i++) {
>  		struct ravb_ex_rx_desc *desc = &priv->rx_ring[q][i];
>  
> -		if (!dma_mapping_error(ndev->dev.parent,
> -				       le32_to_cpu(desc->dptr)))
> +		if (le16_to_cpu(desc->ds_cc) != 0)
>  			dma_unmap_single(ndev->dev.parent,
>  					 le32_to_cpu(desc->dptr),
>  					 RX_BUF_SZ,
> @@ -1949,7 +1947,7 @@ static netdev_tx_t ravb_start_xmit(struct sk_buff *skb, struct net_device *ndev)
>  	struct ravb_tstamp_skb *ts_skb;
>  	struct ravb_tx_desc *desc;
>  	unsigned long flags;
> -	u32 dma_addr;
> +	dma_addr_t dma_addr;
>  	void *buffer;
>  	u32 entry;
>  	u32 len;