[PATCH net-next 2/2] net: axienet: Use iowrite64 to write all 64b descriptor pointers

[Andy Chiu]

Reviewed-by: Greentime Hu <greentime.hu at sifive.com>

On Mon, Jun 13, 2022 at 11:45 AM Andy Chiu <andy.chiu at sifive.com> wrote:
>
> According to commit f735c40ed93c ("net: axienet: Autodetect 64-bit DMA
> capability") and AXI-DMA spec (pg021), on 64-bit capable dma, only
> writing MSB part of tail descriptor pointer causes DMA engine to start
> fetching descriptors. However, we found that it is true only if dma is in
> idle state. In other words, dma would use a tailp even if it only has LSB
> updated, when the dma is running.
>
> The non-atomicity of this behavior could be problematic if enough
> delay were introduced in between the 2 writes. For example, if an
> interrupt comes right after the LSB write and the cpu spends long
> enough time in the handler for the dma to get back into idle state by
> completing descriptors, then the seconcd write to MSB would treat dma
> to start fetching descriptors again. Since the descriptor next to the
> one pointed by current tail pointer is not filled by the kernel yet,
> fetching a null descriptor here causes a dma internal error and halt
> the dma engine down.
>
> We suggest that the dma engine should start process a 64-bit MMIO write
> to the descriptor pointer only if ONE 32-bit part of it is written on all
> states. Or we should restrict the use of 64-bit addressable dma on 32-bit
> platforms, since those devices have no instruction to guarantee the write
> to LSB and MSB part of tail pointer occurs atomically to the dma.
>
> initial condition:
> curp =  x-3;
> tailp = x-2;
> LSB = x;
> MSB = 0;
>
> cpu:                       |dma:
>  iowrite32(LSB, tailp)     |  completes #(x-3) desc, curp = x-3
>  ...                       |  tailp updated
>  => irq                    |  completes #(x-2) desc, curp = x-2
>     ...                    |  completes #(x-1) desc, curp = x-1
>     ...                    |  ...
>     ...                    |  completes #x desc, curp = tailp = x
>  <= irqreturn              |  reaches tailp == curp = x, idle
>  iowrite32(MSB, tailp + 4) |  ...
>                            |  tailp updated, starts fetching...
>                            |  fetches #(x + 1) desc, sees cntrl = 0
>                            |  post Tx error, halt
>
> Signed-off-by: Andy Chiu <andy.chiu at sifive.com>
> Reported-by: Max Hsu <max.hsu at sifive.com>
> ---
>  drivers/net/ethernet/xilinx/xilinx_axienet.h | 21 +++++++++++++++++---
>  1 file changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet.h b/drivers/net/ethernet/xilinx/xilinx_axienet.h
> index 6c95676ba172..97ddc0273b8a 100644
> --- a/drivers/net/ethernet/xilinx/xilinx_axienet.h
> +++ b/drivers/net/ethernet/xilinx/xilinx_axienet.h
> @@ -564,13 +564,28 @@ static inline void axienet_dma_out32(struct axienet_local *lp,
>  }
>
>  #ifdef CONFIG_64BIT
> +/**
> + * axienet_dma_out64 - Memory mapped Axi DMA register write.
> + * @lp:                Pointer to axienet local structure
> + * @reg:       Address offset from the base address of the Axi DMA core
> + * @value:     Value to be written into the Axi DMA register
> + *
> + * This function writes the desired value into the corresponding Axi DMA
> + * register.
> + */
> +static inline void axienet_dma_out64(struct axienet_local *lp,
> +                                    off_t reg, u64 value)
> +{
> +       iowrite64(value, lp->dma_regs + reg);
> +}
> +
>  static void axienet_dma_out_addr(struct axienet_local *lp, off_t reg,
>                                  dma_addr_t addr)
>  {
> -       axienet_dma_out32(lp, reg, lower_32_bits(addr));
> -
>         if (lp->features & XAE_FEATURE_DMA_64BIT)
> -               axienet_dma_out32(lp, reg + 4, upper_32_bits(addr));
> +               axienet_dma_out64(lp, reg, addr);
> +       else
> +               axienet_dma_out32(lp, reg, lower_32_bits(addr));
>  }
>
>  #else /* CONFIG_64BIT */
> --
> 2.36.0
>