123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121122123124125126127128129130131132133134135136137138139140141142143144145146147148149150151152153154155156157158159160161162163164165166167168169170171172173174175176177178179180181182183184185186187188189190191192193194195196197198199200201202203204205206207208209210211212213214215216217218219220221222223224225226227228229230231232233234235236237238239240241242243244245246247248249250251252253254255256257258259260261262263264265266267268269270271272273274275276277278279280281282283284285286287288289290291292293294295296297298299300301302303304305306307308309310311312313314315316317318319320321322323324325326327328329330331332333334335336337338339340341342343344345346347348349350351352353354355356357358359360361362363364365366367368369370371372373374375376377378379380381382383384385386387388389390391392393394395396397398399400401402403404405406407408409 |
- From 0f62a794b41ad14a962c70445844d61a8097805e Mon Sep 17 00:00:00 2001
- From: Alex Marginean <[email protected]>
- Date: Tue, 20 Aug 2019 12:34:22 +0300
- Subject: [PATCH] enetc: WA for MDIO register access issue
- Due to a hardware issue access to MDIO registers concurrent with other
- ENETC register access may lead to the MDIO access being dropped or
- corrupted. The workaround introduces locking for all register access in
- ENETC space. To reduce performance impact, code except MDIO uses per-cpu
- locks, MDIO code having to acquire all per-CPU locks to perform an access.
- To further reduce the performance impact, datapath functions acquire the
- per-cpu lock fewer times and use _hot accessors. All the rest of the code
- uses the _wa accessors which lock every time a register is accessed.
- Signed-off-by: Alex Marginean <[email protected]>
- ---
- drivers/net/ethernet/freescale/enetc/enetc.c | 85 ++++++++++++++----
- drivers/net/ethernet/freescale/enetc/enetc_hw.h | 105 +++++++++++++++++++++-
- drivers/net/ethernet/freescale/enetc/enetc_mdio.c | 4 +-
- drivers/net/ethernet/freescale/enetc/enetc_pf.c | 3 +
- 4 files changed, 176 insertions(+), 21 deletions(-)
- --- a/drivers/net/ethernet/freescale/enetc/enetc.c
- +++ b/drivers/net/ethernet/freescale/enetc/enetc.c
- @@ -20,8 +20,13 @@ netdev_tx_t enetc_xmit(struct sk_buff *s
- {
- struct enetc_ndev_priv *priv = netdev_priv(ndev);
- struct enetc_bdr *tx_ring;
- + unsigned long flags;
- + /* pointer to per-cpu ENETC lock for register access issue WA */
- + spinlock_t *lock;
- int count;
-
- + lock = this_cpu_ptr(&enetc_gregs);
- +
- tx_ring = priv->tx_ring[skb->queue_mapping];
-
- if (unlikely(skb_shinfo(skb)->nr_frags > ENETC_MAX_SKB_FRAGS))
- @@ -34,7 +39,12 @@ netdev_tx_t enetc_xmit(struct sk_buff *s
- return NETDEV_TX_BUSY;
- }
-
- + spin_lock_irqsave(lock, flags);
- +
- count = enetc_map_tx_buffs(tx_ring, skb, priv->active_offloads);
- +
- + spin_unlock_irqrestore(lock, flags);
- +
- if (unlikely(!count))
- goto drop_packet_err;
-
- @@ -228,7 +238,7 @@ static int enetc_map_tx_buffs(struct ene
- tx_ring->next_to_use = i;
-
- /* let H/W know BD ring has been updated */
- - enetc_wr_reg(tx_ring->tpir, i); /* includes wmb() */
- + enetc_wr_reg_hot(tx_ring->tpir, i); /* includes wmb() */
-
- return count;
-
- @@ -249,13 +259,21 @@ dma_err:
- static irqreturn_t enetc_msix(int irq, void *data)
- {
- struct enetc_int_vector *v = data;
- + unsigned long flags;
- + /* pointer to per-cpu ENETC lock for register access issue WA */
- + spinlock_t *lock;
- int i;
-
- + lock = this_cpu_ptr(&enetc_gregs);
- + spin_lock_irqsave(lock, flags);
- +
- /* disable interrupts */
- - enetc_wr_reg(v->rbier, 0);
- + enetc_wr_reg_hot(v->rbier, 0);
-
- for_each_set_bit(i, &v->tx_rings_map, ENETC_MAX_NUM_TXQS)
- - enetc_wr_reg(v->tbier_base + ENETC_BDR_OFF(i), 0);
- + enetc_wr_reg_hot(v->tbier_base + ENETC_BDR_OFF(i), 0);
- +
- + spin_unlock_irqrestore(lock, flags);
-
- napi_schedule_irqoff(&v->napi);
-
- @@ -271,6 +289,9 @@ static int enetc_poll(struct napi_struct
- struct enetc_int_vector
- *v = container_of(napi, struct enetc_int_vector, napi);
- bool complete = true;
- + unsigned long flags;
- + /* pointer to per-cpu ENETC lock for register access issue WA */
- + spinlock_t *lock;
- int work_done;
- int i;
-
- @@ -287,19 +308,24 @@ static int enetc_poll(struct napi_struct
-
- napi_complete_done(napi, work_done);
-
- + lock = this_cpu_ptr(&enetc_gregs);
- + spin_lock_irqsave(lock, flags);
- +
- /* enable interrupts */
- - enetc_wr_reg(v->rbier, ENETC_RBIER_RXTIE);
- + enetc_wr_reg_hot(v->rbier, ENETC_RBIER_RXTIE);
-
- for_each_set_bit(i, &v->tx_rings_map, ENETC_MAX_NUM_TXQS)
- - enetc_wr_reg(v->tbier_base + ENETC_BDR_OFF(i),
- - ENETC_TBIER_TXTIE);
- + enetc_wr_reg_hot(v->tbier_base + ENETC_BDR_OFF(i),
- + ENETC_TBIER_TXTIE);
- +
- + spin_unlock_irqrestore(lock, flags);
-
- return work_done;
- }
-
- static int enetc_bd_ready_count(struct enetc_bdr *tx_ring, int ci)
- {
- - int pi = enetc_rd_reg(tx_ring->tcir) & ENETC_TBCIR_IDX_MASK;
- + int pi = enetc_rd_reg_hot(tx_ring->tcir) & ENETC_TBCIR_IDX_MASK;
-
- return pi >= ci ? pi - ci : tx_ring->bd_count - ci + pi;
- }
- @@ -337,9 +363,18 @@ static bool enetc_clean_tx_ring(struct e
- bool do_tstamp;
- u64 tstamp = 0;
-
- + unsigned long flags;
- + /* pointer to per-cpu ENETC lock for register access issue WA */
- + spinlock_t *lock;
- +
- + lock = this_cpu_ptr(&enetc_gregs);
- +
- i = tx_ring->next_to_clean;
- tx_swbd = &tx_ring->tx_swbd[i];
- +
- + spin_lock_irqsave(lock, flags);
- bds_to_clean = enetc_bd_ready_count(tx_ring, i);
- + spin_unlock_irqrestore(lock, flags);
-
- do_tstamp = false;
-
- @@ -382,16 +417,20 @@ static bool enetc_clean_tx_ring(struct e
- tx_swbd = tx_ring->tx_swbd;
- }
-
- + spin_lock_irqsave(lock, flags);
- +
- /* BD iteration loop end */
- if (is_eof) {
- tx_frm_cnt++;
- /* re-arm interrupt source */
- - enetc_wr_reg(tx_ring->idr, BIT(tx_ring->index) |
- - BIT(16 + tx_ring->index));
- + enetc_wr_reg_hot(tx_ring->idr, BIT(tx_ring->index) |
- + BIT(16 + tx_ring->index));
- }
-
- if (unlikely(!bds_to_clean))
- bds_to_clean = enetc_bd_ready_count(tx_ring, i);
- +
- + spin_unlock_irqrestore(lock, flags);
- }
-
- tx_ring->next_to_clean = i;
- @@ -470,13 +509,14 @@ static int enetc_refill_rx_ring(struct e
- rx_ring->next_to_alloc = i; /* keep track from page reuse */
- rx_ring->next_to_use = i;
- /* update ENETC's consumer index */
- - enetc_wr_reg(rx_ring->rcir, i);
- + enetc_wr_reg_hot(rx_ring->rcir, i);
- }
-
- return j;
- }
-
- #ifdef CONFIG_FSL_ENETC_HW_TIMESTAMPING
- +/* Must be called with &enetc_gregs spinlock held */
- static void enetc_get_rx_tstamp(struct net_device *ndev,
- union enetc_rx_bd *rxbd,
- struct sk_buff *skb)
- @@ -488,8 +528,8 @@ static void enetc_get_rx_tstamp(struct n
- u64 tstamp;
-
- if (le16_to_cpu(rxbd->r.flags) & ENETC_RXBD_FLAG_TSTMP) {
- - lo = enetc_rd(hw, ENETC_SICTR0);
- - hi = enetc_rd(hw, ENETC_SICTR1);
- + lo = enetc_rd_reg_hot(hw->reg + ENETC_SICTR0);
- + hi = enetc_rd_reg_hot(hw->reg + ENETC_SICTR1);
- tstamp_lo = le32_to_cpu(rxbd->r.tstamp);
- if (lo <= tstamp_lo)
- hi -= 1;
- @@ -627,6 +667,12 @@ static int enetc_clean_rx_ring(struct en
- int rx_frm_cnt = 0, rx_byte_cnt = 0;
- int cleaned_cnt, i;
-
- + unsigned long flags;
- + /* pointer to per-cpu ENETC lock for register access issue WA */
- + spinlock_t *lock;
- +
- + lock = this_cpu_ptr(&enetc_gregs);
- +
- cleaned_cnt = enetc_bd_unused(rx_ring);
- /* next descriptor to process */
- i = rx_ring->next_to_clean;
- @@ -637,6 +683,8 @@ static int enetc_clean_rx_ring(struct en
- u32 bd_status;
- u16 size;
-
- + spin_lock_irqsave(lock, flags);
- +
- if (cleaned_cnt >= ENETC_RXBD_BUNDLE) {
- int count = enetc_refill_rx_ring(rx_ring, cleaned_cnt);
-
- @@ -645,15 +693,19 @@ static int enetc_clean_rx_ring(struct en
-
- rxbd = ENETC_RXBD(*rx_ring, i);
- bd_status = le32_to_cpu(rxbd->r.lstatus);
- - if (!bd_status)
- + if (!bd_status) {
- + spin_unlock_irqrestore(lock, flags);
- break;
- + }
-
- - enetc_wr_reg(rx_ring->idr, BIT(rx_ring->index));
- + enetc_wr_reg_hot(rx_ring->idr, BIT(rx_ring->index));
- dma_rmb(); /* for reading other rxbd fields */
- size = le16_to_cpu(rxbd->r.buf_len);
- skb = enetc_map_rx_buff_to_skb(rx_ring, i, size);
- - if (!skb)
- + if (!skb) {
- + spin_unlock_irqrestore(lock, flags);
- break;
- + }
-
- enetc_get_offloads(rx_ring, rxbd, skb);
-
- @@ -667,6 +719,7 @@ static int enetc_clean_rx_ring(struct en
-
- if (unlikely(bd_status &
- ENETC_RXBD_LSTATUS(ENETC_RXBD_ERR_MASK))) {
- + spin_unlock_irqrestore(lock, flags);
- dev_kfree_skb(skb);
- while (!(bd_status & ENETC_RXBD_LSTATUS_F)) {
- dma_rmb();
- @@ -710,6 +763,8 @@ static int enetc_clean_rx_ring(struct en
-
- enetc_process_skb(rx_ring, skb);
-
- + spin_unlock_irqrestore(lock, flags);
- +
- napi_gro_receive(napi, skb);
-
- rx_frm_cnt++;
- --- a/drivers/net/ethernet/freescale/enetc/enetc_hw.h
- +++ b/drivers/net/ethernet/freescale/enetc/enetc_hw.h
- @@ -325,8 +325,15 @@ struct enetc_hw {
- };
-
- /* general register accessors */
- -#define enetc_rd_reg(reg) ioread32((reg))
- -#define enetc_wr_reg(reg, val) iowrite32((val), (reg))
- +#define enetc_rd_reg(reg) enetc_rd_reg_wa((reg))
- +#define enetc_wr_reg(reg, val) enetc_wr_reg_wa((reg), (val))
- +
- +/* accessors for data-path, due to MDIO issue on LS1028 these should be called
- + * only under enetc_gregs per-cpu lock
- + */
- +#define enetc_rd_reg_hot(reg) ioread32((reg))
- +#define enetc_wr_reg_hot(reg, val) iowrite32((val), (reg))
- +
- #ifdef ioread64
- #define enetc_rd_reg64(reg) ioread64((reg))
- #else
- @@ -345,12 +352,102 @@ static inline u64 enetc_rd_reg64(void __
- }
- #endif
-
- +extern DEFINE_PER_CPU(spinlock_t, enetc_gregs);
- +
- +static inline u32 enetc_rd_reg_wa(void *reg)
- +{
- + unsigned long flags;
- + /* pointer to per-cpu ENETC lock for register access issue WA */
- + spinlock_t *lock;
- + u32 val;
- +
- + lock = this_cpu_ptr(&enetc_gregs);
- + spin_lock_irqsave(lock, flags);
- + val = ioread32(reg);
- + spin_unlock_irqrestore(lock, flags);
- +
- + return val;
- +}
- +
- +static inline void enetc_wr_reg_wa(void *reg, u32 val)
- +{
- + unsigned long flags;
- + /* pointer to per-cpu ENETC lock for register access issue WA */
- + spinlock_t *lock;
- +
- + lock = this_cpu_ptr(&enetc_gregs);
- + spin_lock_irqsave(lock, flags);
- + iowrite32(val, reg);
- + spin_unlock_irqrestore(lock, flags);
- +}
- +
- +/* NR_CPUS=256 in ARM64 defconfig and using it as array size triggers stack
- + * frame warnings for the functions below. Use a custom define of 2 for now,
- + * LS1028 has just two cores.
- + */
- +#define ENETC_NR_CPU_LOCKS 2
- +
- +static inline u32 enetc_rd_reg_wa_single(void *reg)
- +{
- + u32 val;
- + int cpu;
- + /* per-cpu ENETC lock array for register access issue WA */
- + spinlock_t *lock[ENETC_NR_CPU_LOCKS];
- + unsigned long flags;
- +
- + local_irq_save(flags);
- + preempt_disable();
- +
- + for_each_online_cpu(cpu) {
- + lock[cpu] = per_cpu_ptr(&enetc_gregs, cpu);
- + spin_lock(lock[cpu]);
- + }
- +
- + val = ioread32(reg);
- +
- + for_each_online_cpu(cpu)
- + spin_unlock(lock[cpu]);
- + local_irq_restore(flags);
- +
- + preempt_enable();
- +
- + return val;
- +}
- +
- +static inline void enetc_wr_reg_wa_single(void *reg, u32 val)
- +{
- + int cpu;
- + /* per-cpu ENETC lock array for register access issue WA */
- + spinlock_t *lock[ENETC_NR_CPU_LOCKS];
- + unsigned long flags;
- +
- + local_irq_save(flags);
- + preempt_disable();
- +
- + for_each_online_cpu(cpu) {
- + lock[cpu] = per_cpu_ptr(&enetc_gregs, cpu);
- + spin_lock(lock[cpu]);
- + }
- +
- + iowrite32(val, reg);
- +
- + for_each_online_cpu(cpu)
- + spin_unlock(lock[cpu]);
- + local_irq_restore(flags);
- +
- + preempt_enable();
- +}
- +
- #define enetc_rd(hw, off) enetc_rd_reg((hw)->reg + (off))
- #define enetc_wr(hw, off, val) enetc_wr_reg((hw)->reg + (off), val)
- #define enetc_rd64(hw, off) enetc_rd_reg64((hw)->reg + (off))
- /* port register accessors - PF only */
- -#define enetc_port_rd(hw, off) enetc_rd_reg((hw)->port + (off))
- -#define enetc_port_wr(hw, off, val) enetc_wr_reg((hw)->port + (off), val)
- +#define enetc_port_rd(hw, off) enetc_rd_reg_wa((hw)->port + (off))
- +#define enetc_port_wr(hw, off, val) enetc_wr_reg_wa((hw)->port + (off), val)
- +#define enetc_port_rd_single(hw, off) enetc_rd_reg_wa_single(\
- + (hw)->port + (off))
- +#define enetc_port_wr_single(hw, off, val) enetc_wr_reg_wa_single(\
- + (hw)->port + (off), val)
- /* global register accessors - PF only */
- #define enetc_global_rd(hw, off) enetc_rd_reg((hw)->global + (off))
- #define enetc_global_wr(hw, off, val) enetc_wr_reg((hw)->global + (off), val)
- --- a/drivers/net/ethernet/freescale/enetc/enetc_mdio.c
- +++ b/drivers/net/ethernet/freescale/enetc/enetc_mdio.c
- @@ -16,13 +16,13 @@
-
- static inline u32 _enetc_mdio_rd(struct enetc_mdio_priv *mdio_priv, int off)
- {
- - return enetc_port_rd(mdio_priv->hw, mdio_priv->mdio_base + off);
- + return enetc_port_rd_single(mdio_priv->hw, mdio_priv->mdio_base + off);
- }
-
- static inline void _enetc_mdio_wr(struct enetc_mdio_priv *mdio_priv, int off,
- u32 val)
- {
- - enetc_port_wr(mdio_priv->hw, mdio_priv->mdio_base + off, val);
- + enetc_port_wr_single(mdio_priv->hw, mdio_priv->mdio_base + off, val);
- }
-
- #define enetc_mdio_rd(mdio_priv, off) \
- --- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
- +++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
- @@ -987,6 +987,9 @@ static void enetc_pf_remove(struct pci_d
- enetc_pci_remove(pdev);
- }
-
- +DEFINE_PER_CPU(spinlock_t, enetc_gregs);
- +EXPORT_PER_CPU_SYMBOL(enetc_gregs);
- +
- static const struct pci_device_id enetc_pf_id_table[] = {
- { PCI_DEVICE(PCI_VENDOR_ID_FREESCALE, ENETC_DEV_ID_PF) },
- { 0, } /* End of table. */
|