Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Fri, 26 Apr 2019 11:40:13 -0700
From:   Jakub Kicinski <jakub.kicinski@netronome.com>
To:     Esben Haabendal <esben@geanix.com>
Cc:     netdev@vger.kernel.org, "David S. Miller" <davem@davemloft.net>,
        Michal Simek <michal.simek@xilinx.com>,
        Yang Wei <yang.wei9@zte.com.cn>,
        YueHaibing <yuehaibing@huawei.com>,
        Luis Chamberlain <mcgrof@kernel.org>,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 03/12] net: ll_temac: Fix support for 64-bit platforms
Message-ID: <20190426114013.333e62c8@cakuba.netronome.com>
In-Reply-To: <20190426073231.4008-4-esben@geanix.com>
References: <20190426073231.4008-1-esben@geanix.com>
        <20190426073231.4008-4-esben@geanix.com>
Organization: Netronome Systems, Ltd.
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, 26 Apr 2019 09:32:22 +0200, Esben Haabendal wrote:
> The use of buffer descriptor APP4 field (32-bit) for storing skb pointer
> obviously does not work on 64-bit platforms.
> As APP3 is also unused, we can use that to store the other half of 64-bit
> pointer values.
> 
> Contrary to what is hinted at in commit message of commit 15bfe05c8d63
> ("net: ethernet: xilinx: Mark XILINX_LL_TEMAC broken on 64-bit")
> there are no other pointers stored in cdmac_bd.
> 
> Signed-off-by: Esben Haabendal <esben@geanix.com>

This is a bit strange, the driver stores the host's virtual address into
the HW descriptor?

> diff --git a/drivers/net/ethernet/xilinx/Kconfig b/drivers/net/ethernet/xilinx/Kconfig
> index da4ec57..6d68c8a 100644
> --- a/drivers/net/ethernet/xilinx/Kconfig
> +++ b/drivers/net/ethernet/xilinx/Kconfig
> @@ -34,7 +34,6 @@ config XILINX_AXI_EMAC
>  config XILINX_LL_TEMAC
>  	tristate "Xilinx LL TEMAC (LocalLink Tri-mode Ethernet MAC) driver"
>  	depends on (PPC || MICROBLAZE)
> -	depends on !64BIT || BROKEN
>  	select PHYLIB
>  	---help---
>  	  This driver supports the Xilinx 10/100/1000 LocalLink TEMAC
> diff --git a/drivers/net/ethernet/xilinx/ll_temac_main.c b/drivers/net/ethernet/xilinx/ll_temac_main.c
> index 4594fe3..a365bfd 100644
> --- a/drivers/net/ethernet/xilinx/ll_temac_main.c
> +++ b/drivers/net/ethernet/xilinx/ll_temac_main.c
> @@ -619,11 +619,39 @@ static void temac_adjust_link(struct net_device *ndev)
>  	mutex_unlock(&lp->indirect_mutex);
>  }
>  
> +#ifdef CONFIG_64BIT
> +
> +void ptr_to_txbd(void *p, struct cdmac_bd *bd)
> +{
> +	bd->app3 = (u32)(((u64)p) >> 32);
> +	bd->app4 = (u32)((u64)p & 0xFFFFFFFF);
> +}
> +
> +void *ptr_from_txbd(struct cdmac_bd *bd)
> +{
> +	return (void *)(((u64)(bd->app3) << 32) | bd->app4);
> +}
> +
> +#else
> +
> +void ptr_to_txbd(void *p, struct cmdac_bd *bd)
> +{
> +	bd->app4 = (u32)p;
> +}
> +
> +void *ptr_from_txbd(struct cdmac_bd *bd)
> +{
> +	return (void *)(bd->app4);
> +}
> +
> +#endif
> +
>  static void temac_start_xmit_done(struct net_device *ndev)
>  {
>  	struct temac_local *lp = netdev_priv(ndev);
>  	struct cdmac_bd *cur_p;
>  	unsigned int stat = 0;
> +	struct sk_buff *skb;
>  
>  	cur_p = &lp->tx_bd_v[lp->tx_bd_ci];
>  	stat = cur_p->app0;
> @@ -631,8 +659,9 @@ static void temac_start_xmit_done(struct net_device *ndev)
>  	while (stat & STS_CTRL_APP0_CMPLT) {
>  		dma_unmap_single(ndev->dev.parent, cur_p->phys, cur_p->len,
>  				 DMA_TO_DEVICE);
> -		if (cur_p->app4)
> -			dev_consume_skb_irq((struct sk_buff *)cur_p->app4);
> +		skb = (struct sk_buff *)ptr_from_txbd(cur_p);
> +		if (skb)
> +			dev_consume_skb_irq(skb);
>  		cur_p->app0 = 0;
>  		cur_p->app1 = 0;
>  		cur_p->app2 = 0;
> @@ -711,7 +740,7 @@ temac_start_xmit(struct sk_buff *skb, struct net_device *ndev)
>  	cur_p->len = skb_headlen(skb);
>  	cur_p->phys = dma_map_single(ndev->dev.parent, skb->data,
>  				     skb_headlen(skb), DMA_TO_DEVICE);
> -	cur_p->app4 = (unsigned long)skb;
> +	ptr_to_txbd((void *)skb, cur_p);
>  
>  	for (ii = 0; ii < num_frag; ii++) {
>  		lp->tx_bd_tail++;