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: <CAMdnO-K49gp3GtM5EjBsBzcLNJJn60jXo-Kxn-zKn25MjVeZaQ@mail.gmail.com>
Date: Thu, 8 Aug 2024 17:41:37 -0700
From: Jitendra Vegiraju <jitendra.vegiraju@...adcom.com>
To: Serge Semin <fancer.lancer@...il.com>
Cc: netdev@...r.kernel.org, alexandre.torgue@...s.st.com, joabreu@...opsys.com, 
	davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com, 
	mcoquelin.stm32@...il.com, bcm-kernel-feedback-list@...adcom.com, 
	richardcochran@...il.com, ast@...nel.org, daniel@...earbox.net, 
	hawk@...nel.org, john.fastabend@...il.com, linux-kernel@...r.kernel.org, 
	linux-stm32@...md-mailman.stormreply.com, 
	linux-arm-kernel@...ts.infradead.org, bpf@...r.kernel.org, andrew@...n.ch, 
	linux@...linux.org.uk, horms@...nel.org, florian.fainelli@...adcom.com
Subject: Re: [PATCH net-next v3 1/3] net: stmmac: Add basic dwxgmac4 support
 to stmmac core

Resending the message since my earlier email turned-in to HTML because of an
attempt to draw asciiart.
On Tue, Aug 6, 2024 at 2:56 PM Serge Semin <fancer.lancer@...il.com> wrote:
>
> On Thu, Aug 01, 2024 at 08:18:20PM -0700, jitendra.vegiraju@...adcom.com wrote:
> > From: Jitendra Vegiraju <jitendra.vegiraju@...adcom.com>
> >
> > Adds support for DWC_xgmac version 4.00a in stmmac core module.
> > This version adds enhancements to DMA architecture for virtualization
> > scalability. This is realized by decoupling physical DMA channels (PDMA)
> > from Virtual DMA channels (VDMA). The  VDMAs are software abastractions
> > that map to PDMAs for frame transmission and reception.
> >
> > The virtualization enhancements are currently not being used and hence
> > a fixed mapping of VDMA to PDMA is configured in the init functions.
> > Because of the new init functions, a new instance of struct stmmac_dma_ops
> > dwxgmac400_dma_ops is added.
> > Most of the other dma operation functions in existing dwxgamc2_dma.c file
> > can be reused.
>
> As we figured out (didn't we?) that it's actually the DW 25GMAC, then
> it should be taken into account across the entire series.
>
Yes, indeed its 25GMAC IP.
We got confirmation that we used an early adopter version of the 25GMAC IP.
The 25GMAC always come with HDMA DMA engine.
The synopsis revision id is 4.xx and device id is 0x55.
> >
> > Signed-off-by: Jitendra Vegiraju <jitendra.vegiraju@...adcom.com>
> > ---
> >  drivers/net/ethernet/stmicro/stmmac/Makefile  |   2 +-
> >  .../ethernet/stmicro/stmmac/dwxgmac2_dma.c    |  31 ++++
> >  .../net/ethernet/stmicro/stmmac/dwxgmac4.c    | 142 ++++++++++++++++++
> >  .../net/ethernet/stmicro/stmmac/dwxgmac4.h    |  84 +++++++++++
> >  4 files changed, 258 insertions(+), 1 deletion(-)
> >  create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwxgmac4.c
> >  create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwxgmac4.h
> >
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
> > index c2f0e91f6bf8..2f637612513d 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/Makefile
> > +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
> > @@ -6,7 +6,7 @@ stmmac-objs:= stmmac_main.o stmmac_ethtool.o stmmac_mdio.o ring_mode.o        \
> >             mmc_core.o stmmac_hwtstamp.o stmmac_ptp.o dwmac4_descs.o  \
> >             dwmac4_dma.o dwmac4_lib.o dwmac4_core.o dwmac5.o hwif.o \
> >             stmmac_tc.o dwxgmac2_core.o dwxgmac2_dma.o dwxgmac2_descs.o \
> > -           stmmac_xdp.o stmmac_est.o \
>
> > +           stmmac_xdp.o stmmac_est.o dwxgmac4.o \
>
> dw25gmac.o?
We will rename all applicable reference to 25mac as you suggest for
the entire patch series.

>
> >             $(stmmac-y)
> >
> >  stmmac-$(CONFIG_STMMAC_SELFTESTS) += stmmac_selftests.o
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
> > index dd2ab6185c40..c15f5247aaa8 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
> > @@ -7,6 +7,7 @@
> >  #include <linux/iopoll.h>
> >  #include "stmmac.h"
> >  #include "dwxgmac2.h"
>
> > +#include "dwxgmac4.h"
>
> "dw25gmac.h"?
>
> >
> >  static int dwxgmac2_dma_reset(void __iomem *ioaddr)
> >  {
> > @@ -641,3 +642,33 @@ const struct stmmac_dma_ops dwxgmac210_dma_ops = {
> >       .enable_sph = dwxgmac2_enable_sph,
> >       .enable_tbs = dwxgmac2_enable_tbs,
> >  };
> > +
>
> > +const struct stmmac_dma_ops dwxgmac400_dma_ops = {
>
> dw25gmac_dma_ops?
>
> > +     .reset = dwxgmac2_dma_reset,
>
> > +     .init = dwxgmac4_dma_init,
> > +     .init_chan = dwxgmac2_dma_init_chan,
> > +     .init_rx_chan = dwxgmac4_dma_init_rx_chan,
> > +     .init_tx_chan = dwxgmac4_dma_init_tx_chan,
>
> dw25gmac_dma_init, dw25gmac_dma_init_rx_chan, dw25gmac_dma_init_tx_chan?
>
> > +     .axi = dwxgmac2_dma_axi,
> > +     .dump_regs = dwxgmac2_dma_dump_regs,
> > +     .dma_rx_mode = dwxgmac2_dma_rx_mode,
> > +     .dma_tx_mode = dwxgmac2_dma_tx_mode,
> > +     .enable_dma_irq = dwxgmac2_enable_dma_irq,
> > +     .disable_dma_irq = dwxgmac2_disable_dma_irq,
> > +     .start_tx = dwxgmac2_dma_start_tx,
> > +     .stop_tx = dwxgmac2_dma_stop_tx,
> > +     .start_rx = dwxgmac2_dma_start_rx,
> > +     .stop_rx = dwxgmac2_dma_stop_rx,
> > +     .dma_interrupt = dwxgmac2_dma_interrupt,
> > +     .get_hw_feature = dwxgmac2_get_hw_feature,
> > +     .rx_watchdog = dwxgmac2_rx_watchdog,
> > +     .set_rx_ring_len = dwxgmac2_set_rx_ring_len,
> > +     .set_tx_ring_len = dwxgmac2_set_tx_ring_len,
> > +     .set_rx_tail_ptr = dwxgmac2_set_rx_tail_ptr,
> > +     .set_tx_tail_ptr = dwxgmac2_set_tx_tail_ptr,
> > +     .enable_tso = dwxgmac2_enable_tso,
> > +     .qmode = dwxgmac2_qmode,
> > +     .set_bfsize = dwxgmac2_set_bfsize,
> > +     .enable_sph = dwxgmac2_enable_sph,
> > +     .enable_tbs = dwxgmac2_enable_tbs,
> > +};
>
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac4.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac4.c
> > new file mode 100644
> > index 000000000000..9c8748122dc6
> > --- /dev/null
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac4.c
>
> dw25gmac.c?
>
> > @@ -0,0 +1,142 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (c) 2024 Broadcom Corporation
> > + */
> > +#include "dwxgmac2.h"
>
> > +#include "dwxgmac4.h"
>
> dw25gmac.h?
>
> > +
> > +static int rd_dma_ch_ind(void __iomem *ioaddr, u8 mode, u32 channel)
> > +{
> > +     u32 reg_val = 0;
> > +     u32 val = 0;
> > +
> > +     reg_val |= mode << XGMAC4_MSEL_SHIFT & XGMAC4_MODE_SELECT;
> > +     reg_val |= channel << XGMAC4_AOFF_SHIFT & XGMAC4_ADDR_OFFSET;
> > +     reg_val |= XGMAC4_CMD_TYPE | XGMAC4_OB;
> > +     writel(reg_val, ioaddr + XGMAC4_DMA_CH_IND_CONTROL);
> > +     val = readl(ioaddr + XGMAC4_DMA_CH_IND_DATA);
> > +     return val;
> > +}
> > +
> > +static void wr_dma_ch_ind(void __iomem *ioaddr, u8 mode, u32 channel, u32 val)
> > +{
> > +     u32 reg_val = 0;
> > +
> > +     writel(val, ioaddr + XGMAC4_DMA_CH_IND_DATA);
> > +     reg_val |= mode << XGMAC4_MSEL_SHIFT & XGMAC4_MODE_SELECT;
> > +     reg_val |= channel << XGMAC4_AOFF_SHIFT & XGMAC4_ADDR_OFFSET;
> > +     reg_val |= XGMAC_OB;
> > +     writel(reg_val, ioaddr + XGMAC4_DMA_CH_IND_CONTROL);
> > +}
> > +
>
> > +static void xgmac4_tp2tc_map(void __iomem *ioaddr, u8 pdma_ch, u32 tc_num)
> > +{
> > +     u32 val = 0;
> > +
> > +     val = rd_dma_ch_ind(ioaddr, MODE_TXEXTCFG, pdma_ch);
> > +     val &= ~XGMAC4_TP2TCMP;
> > +     val |= tc_num << XGMAC4_TP2TCMP_SHIFT & XGMAC4_TP2TCMP;
> > +     wr_dma_ch_ind(ioaddr, MODE_TXEXTCFG, pdma_ch, val);
> > +}
> > +
> > +static void xgmac4_rp2tc_map(void __iomem *ioaddr, u8 pdma_ch, u32 tc_num)
> > +{
> > +     u32 val = 0;
> > +
> > +     val = rd_dma_ch_ind(ioaddr, MODE_RXEXTCFG, pdma_ch);
> > +     val &= ~XGMAC4_RP2TCMP;
> > +     val |= tc_num << XGMAC4_RP2TCMP_SHIFT & XGMAC4_RP2TCMP;
> > +     wr_dma_ch_ind(ioaddr, MODE_RXEXTCFG, pdma_ch, val);
> > +}
>
> What does "tc" stand for? Traffic control? If it's a kind of queue
> then what about implementing the stmmac_ops::map_mtl_to_dma interface
> method?
>
TC stands for "traffic class".
The VDMA to PDMA mapping within HDMA has traffic classification block between
the two blocks to allow control over DMA resource utilization.
VDMA -- TC --- PDMA
Tx VDMAs and RX VDMAs are mapped onto a TC based on TD2TCMP and RD2TCMP.
Multiple VDMAs can be mapped to the same TC.
TCs are further mapped onto PDMAs based on TP2TCMP and RP2TCMP fields
> > +
> > +void dwxgmac4_dma_init(void __iomem *ioaddr,
> > +                    struct stmmac_dma_cfg *dma_cfg, int atds)
> > +{
> > +     u32 value;
> > +     u32 i;
> > +
> > +     value = readl(ioaddr + XGMAC_DMA_SYSBUS_MODE);
> > +
> > +     if (dma_cfg->aal)
> > +             value |= XGMAC_AAL;
> > +
> > +     if (dma_cfg->eame)
> > +             value |= XGMAC_EAME;
> > +
> > +     writel(value, ioaddr + XGMAC_DMA_SYSBUS_MODE);
> > +
> > +     for (i = 0; i < VDMA_TOTAL_CH_COUNT; i++) {
>
> > +             value = rd_dma_ch_ind(ioaddr, MODE_TXDESCCTRL, i);
> > +             value &= ~XGMAC4_TXDCSZ;
> > +             value |= 0x3;
> > +             value &= ~XGMAC4_TDPS;
> > +             value |= (3 << XGMAC4_TDPS_SHIFT) & XGMAC4_TDPS;
> > +             wr_dma_ch_ind(ioaddr, MODE_TXDESCCTRL, i, value);
> > +
> > +             value = rd_dma_ch_ind(ioaddr, MODE_RXDESCCTRL, i);
> > +             value &= ~XGMAC4_RXDCSZ;
> > +             value |= 0x3;
> > +             value &= ~XGMAC4_RDPS;
> > +             value |= (3 << XGMAC4_RDPS_SHIFT) & XGMAC4_RDPS;
> > +             wr_dma_ch_ind(ioaddr, MODE_RXDESCCTRL, i, value);
>
> I know that the TDPS/RDPS means Tx/Rx Descriptor Pre-fetch threshold
> Size. What does the TXDCSZ/RXDCSZ config mean?
>
> Most importantly why are these parameters hardcoded to 3? It
> doesn't seem right.
>
TXDCSZ/RXDCSZ are descriptor cache sizes.
I missed defining macros for valid values, for example 3 indicates 256 bytes.
Will fix it in the next update.

> > +     }
> > +
>
> > +     for (i = 0; i < PDMA_TX_CH_COUNT; i++) {
> > +             value = rd_dma_ch_ind(ioaddr, MODE_TXEXTCFG, i);
> > +             value &= ~(XGMAC4_TXPBL | XGMAC4_TPBLX8_MODE);
> > +             if (dma_cfg->pblx8)
> > +                     value |= XGMAC4_TPBLX8_MODE;
> > +             value |= (dma_cfg->pbl << XGMAC4_TXPBL_SHIFT) & XGMAC4_TXPBL;
> > +             wr_dma_ch_ind(ioaddr, MODE_TXEXTCFG, i, value);
> > +             xgmac4_tp2tc_map(ioaddr, i, i);
> > +     }
> > +
> > +     for (i = 0; i < PDMA_RX_CH_COUNT; i++) {
> > +             value = rd_dma_ch_ind(ioaddr, MODE_RXEXTCFG, i);
> > +             value &= ~(XGMAC4_RXPBL | XGMAC4_RPBLX8_MODE);
> > +             if (dma_cfg->pblx8)
> > +                     value |= XGMAC4_RPBLX8_MODE;
> > +             value |= (dma_cfg->pbl << XGMAC4_RXPBL_SHIFT) & XGMAC4_RXPBL;
> > +             wr_dma_ch_ind(ioaddr, MODE_RXEXTCFG, i, value);
> > +             if (i < PDMA_MAX_TC_COUNT)
> > +                     xgmac4_rp2tc_map(ioaddr, i, i);
> > +             else
> > +                     xgmac4_rp2tc_map(ioaddr, i, PDMA_MAX_TC_COUNT - 1);
> > +     }
>
> Shouldn't these initialization be done on the per-channel basis only
> for only activated queues
> plat_stmmacenet_data::{rx_queues_to_use,tx_queues_to_use} (the STMMAC
> driver one-on-one maps queues and DMA-channels if no custom mapping
> was specified)?
>
These are VDMA to PDMA mappings internal HDMA.
Even though it provides flexibility, a default any to any configuration is used.

> > +}
> > +
> > +void dwxgmac4_dma_init_tx_chan(struct stmmac_priv *priv,
> > +                            void __iomem *ioaddr,
> > +                            struct stmmac_dma_cfg *dma_cfg,
> > +                            dma_addr_t dma_addr, u32 chan)
> > +{
> > +     u32 value;
> > +
> > +     value = readl(ioaddr + XGMAC_DMA_CH_TX_CONTROL(chan));
> > +     value &= ~XGMAC4_TVDMA2TCMP;
> > +     value |= (chan << XGMAC4_TVDMA2TCMP_SHIFT) & XGMAC4_TVDMA2TCMP;
> > +     writel(value, ioaddr + XGMAC_DMA_CH_TX_CONTROL(chan));
> > +
> > +     writel(upper_32_bits(dma_addr),
> > +            ioaddr + XGMAC_DMA_CH_TxDESC_HADDR(chan));
> > +     writel(lower_32_bits(dma_addr),
> > +            ioaddr + XGMAC_DMA_CH_TxDESC_LADDR(chan));
> > +}
> > +
> > +void dwxgmac4_dma_init_rx_chan(struct stmmac_priv *priv,
> > +                            void __iomem *ioaddr,
> > +                            struct stmmac_dma_cfg *dma_cfg,
> > +                            dma_addr_t dma_addr, u32 chan)
> > +{
> > +     u32 value;
> > +
> > +     value = readl(ioaddr + XGMAC_DMA_CH_RX_CONTROL(chan));
> > +     value &= ~XGMAC4_RVDMA2TCMP;
> > +     value |= (chan << XGMAC4_RVDMA2TCMP_SHIFT) & XGMAC4_RVDMA2TCMP;
> > +     writel(value, ioaddr + XGMAC_DMA_CH_RX_CONTROL(chan));
> > +
> > +     writel(upper_32_bits(dma_addr),
> > +            ioaddr + XGMAC_DMA_CH_RxDESC_HADDR(chan));
> > +     writel(lower_32_bits(dma_addr),
> > +            ioaddr + XGMAC_DMA_CH_RxDESC_LADDR(chan));
> > +}
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac4.h b/drivers/net/ethernet/stmicro/stmmac/dwxgmac4.h
> > new file mode 100644
> > index 000000000000..0ce1856b0b34
> > --- /dev/null
>
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac4.h
>
> dw25gmac.h?
Will rename all applicable references to 25gmac.
>
> > @@ -0,0 +1,84 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/* Copyright (c) 2024 Broadcom Corporation
> > + * XGMAC4 definitions.
> > + */
> > +#ifndef __STMMAC_DWXGMAC4_H__
> > +#define __STMMAC_DWXGMAC4_H__
> > +
> > +/* DMA Indirect Registers*/
>
> > +#define XGMAC4_DMA_CH_IND_CONTROL    0X00003080
>
> XXVGMAC_*?
>
Thanks, Will use this prefix.

> > +#define XGMAC4_MODE_SELECT           GENMASK(27, 24)
> > +#define XGMAC4_MSEL_SHIFT            24
> > +enum dma_ch_ind_modes {
> > +     MODE_TXEXTCFG    = 0x0,   /* Tx Extended Config */
> > +     MODE_RXEXTCFG    = 0x1,   /* Rx Extended Config */
> > +     MODE_TXDBGSTS    = 0x2,   /* Tx Debug Status */
> > +     MODE_RXDBGSTS    = 0x3,   /* Rx Debug Status */
> > +     MODE_TXDESCCTRL  = 0x4,   /* Tx Descriptor control */
> > +     MODE_RXDESCCTRL  = 0x5,   /* Rx Descriptor control */
> > +};
> > +
> > +#define XGMAC4_ADDR_OFFSET           GENMASK(14, 8)
> > +#define XGMAC4_AOFF_SHIFT            8
> > +#define XGMAC4_AUTO_INCR             GENMASK(5, 4)
> > +#define XGMAC4_AUTO_SHIFT            4
> > +#define XGMAC4_CMD_TYPE                      BIT(1)
> > +#define XGMAC4_OB                    BIT(0)
> > +#define XGMAC4_DMA_CH_IND_DATA               0X00003084
> > +
> > +/* TX Config definitions */
> > +#define XGMAC4_TXPBL                 GENMASK(29, 24)
> > +#define XGMAC4_TXPBL_SHIFT           24
> > +#define XGMAC4_TPBLX8_MODE           BIT(19)
> > +#define XGMAC4_TP2TCMP                       GENMASK(18, 16)
> > +#define XGMAC4_TP2TCMP_SHIFT         16
> > +#define XGMAC4_ORRQ                  GENMASK(13, 8)
> > +/* RX Config definitions */
> > +#define XGMAC4_RXPBL                 GENMASK(29, 24)
> > +#define XGMAC4_RXPBL_SHIFT           24
> > +#define XGMAC4_RPBLX8_MODE           BIT(19)
> > +#define XGMAC4_RP2TCMP                       GENMASK(18, 16)
> > +#define XGMAC4_RP2TCMP_SHIFT         16
> > +#define XGMAC4_OWRQ                  GENMASK(13, 8)
> > +/* Tx Descriptor control */
> > +#define XGMAC4_TXDCSZ                        GENMASK(2, 0)
> > +#define XGMAC4_TDPS                  GENMASK(5, 3)
> > +#define XGMAC4_TDPS_SHIFT            3
> > +/* Rx Descriptor control */
> > +#define XGMAC4_RXDCSZ                        GENMASK(2, 0)
> > +#define XGMAC4_RDPS                  GENMASK(5, 3)
> > +#define XGMAC4_RDPS_SHIFT            3
> > +
> > +/* DWCXG_DMA_CH(#i) Registers*/
> > +#define XGMAC4_DSL                   GENMASK(20, 18)
> > +#define XGMAC4_MSS                   GENMASK(13, 0)
> > +#define XGMAC4_TFSEL                 GENMASK(30, 29)
> > +#define XGMAC4_TQOS                  GENMASK(27, 24)
> > +#define XGMAC4_IPBL                  BIT(15)
> > +#define XGMAC4_TVDMA2TCMP            GENMASK(6, 4)
> > +#define XGMAC4_TVDMA2TCMP_SHIFT              4
> > +#define XGMAC4_RPF                   BIT(31)
> > +#define XGMAC4_RVDMA2TCMP            GENMASK(30, 28)
> > +#define XGMAC4_RVDMA2TCMP_SHIFT              28
> > +#define XGMAC4_RQOS                  GENMASK(27, 24)
> > +
>
> > +/* PDMA Channel count */
> > +#define PDMA_TX_CH_COUNT             8
> > +#define PDMA_RX_CH_COUNT             10
> > +#define PDMA_MAX_TC_COUNT            8
> > +
> > +/* VDMA channel count */
> > +#define VDMA_TOTAL_CH_COUNT          32
>
> These seems like the vendor-specific constant. What are the actual DW
> 25GMAC constraints?
>
These are the limits for this device.
We can read these limits from hardware, I will fix it in the next patch.

> -Serge(y)
>
> > +
> > +void dwxgmac4_dma_init(void __iomem *ioaddr,
> > +                    struct stmmac_dma_cfg *dma_cfg, int atds);
> > +
> > +void dwxgmac4_dma_init_tx_chan(struct stmmac_priv *priv,
> > +                            void __iomem *ioaddr,
> > +                            struct stmmac_dma_cfg *dma_cfg,
> > +                            dma_addr_t dma_addr, u32 chan);
> > +void dwxgmac4_dma_init_rx_chan(struct stmmac_priv *priv,
> > +                            void __iomem *ioaddr,
> > +                            struct stmmac_dma_cfg *dma_cfg,
> > +                            dma_addr_t dma_addr, u32 chan);
> > +#endif /* __STMMAC_DWXGMAC4_H__ */
> > --
> > 2.34.1
> >
> >

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ