Subject: Re: [PATCH net-next 01/18] net: axienet: Fix casting of pointers to u32

From: Andrew Lunn <[email-masked]>
Date: Tue, 4 Jun 2019 04:05:03 +0200

On Mon, Jun 03, 2019 at 03:57:00PM -0600, Robert Hancock wrote:
> This driver was casting skb pointers to u32 and storing them as such in
> the DMA buffer descriptor, which is obviously broken on 64-bit. The area
> of the buffer descriptor being used is not accessed by the hardware and
> has sufficient room for a 32 or 64-bit pointer, so just store the skb
> pointer as such.
> 
> Signed-off-by: Robert Hancock <[email-masked]>
> ---
>  drivers/net/ethernet/xilinx/xilinx_axienet.h      | 11 +++-------
>  drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 26 ++++++++++++-----------
>  2 files changed, 17 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet.h b/drivers/net/ethernet/xilinx/xilinx_axienet.h
> index 011adae..e09dc14 100644
> --- a/drivers/net/ethernet/xilinx/xilinx_axienet.h
> +++ b/drivers/net/ethernet/xilinx/xilinx_axienet.h
> @@ -356,9 +356,6 @@
>   * @app2:         MM2S/S2MM User Application Field 2.
>   * @app3:         MM2S/S2MM User Application Field 3.
>   * @app4:         MM2S/S2MM User Application Field 4.
> - * @sw_id_offset: MM2S/S2MM Sw ID
> - * @reserved5:    Reserved and not used
> - * @reserved6:    Reserved and not used
>   */
>  struct axidma_bd {
>  	u32 next;	/* Physical address of next buffer descriptor */
> @@ -373,11 +370,9 @@ struct axidma_bd {
>  	u32 app1;	/* TX start << 16 | insert */
>  	u32 app2;	/* TX csum seed */
>  	u32 app3;
> -	u32 app4;
> -	u32 sw_id_offset;
> -	u32 reserved5;
> -	u32 reserved6;
> -};
> +	u32 app4;   /* Last field used by HW */
> +	struct sk_buff *skb;
> +} __aligned(XAXIDMA_BD_MINIMUM_ALIGNMENT);

Hi Robert

Is the memory for the descriptor non-cachable? I expect so.  You may
get slightly better performance if you were to keep an shadow array in
normal RAM. But this is O.K. as well.

Reviewed-by: Andrew Lunn <[email-masked]>

    Andrew