diff options
Diffstat (limited to 'target/linux/layerscape/patches-5.4/701-net-0236-enetc-Replace-enetc_gregs-with-a-readers-writer-lock.patch')
-rw-r--r-- | target/linux/layerscape/patches-5.4/701-net-0236-enetc-Replace-enetc_gregs-with-a-readers-writer-lock.patch | 385 |
1 files changed, 385 insertions, 0 deletions
diff --git a/target/linux/layerscape/patches-5.4/701-net-0236-enetc-Replace-enetc_gregs-with-a-readers-writer-lock.patch b/target/linux/layerscape/patches-5.4/701-net-0236-enetc-Replace-enetc_gregs-with-a-readers-writer-lock.patch new file mode 100644 index 0000000000..979c2da5c3 --- /dev/null +++ b/target/linux/layerscape/patches-5.4/701-net-0236-enetc-Replace-enetc_gregs-with-a-readers-writer-lock.patch @@ -0,0 +1,385 @@ +From 659f899773f9f3fdc8325f61acc1017dd838126c Mon Sep 17 00:00:00 2001 +From: Vladimir Oltean <vladimir.oltean@nxp.com> +Date: Thu, 14 Nov 2019 17:30:50 +0200 +Subject: [PATCH] enetc: Replace enetc_gregs with a readers-writer lock + +The LS1028A MDIO errata tells us that any MDIO register access must not +be concurrent with any other ENETC register access. + +That has been handled so far by a number of per-CPU spinlocks over the +ENETC register map. This came as an optimization over a single spinlock, +because the regular register accesses can still be concurrent with one +another, as long as they aren't concurrent with MDIO. + +But this logic is broken in RT, because the enetc_rd_reg_wa and +enetc_wr_reg_wa functions can be preempted in any context, and when they +resume they may not run on the same CPU. + +This renders the logic to take the per-CPU spinlock pointless, since the +spinlock may not be the correct one (corresponding to this CPU) after +preemption has occurred. + +The following splat is telling us the same thing: + +[ 19.073928] BUG: using smp_processor_id() in preemptible [00000000] code: systemd-network/3423 +[ 19.073932] caller is debug_smp_processor_id+0x1c/0x30 +[ 19.073935] CPU: 1 PID: 3423 Comm: systemd-network Not tainted 4.19.68-rt26 #1 +[ 19.073936] Hardware name: LS1028A RDB Board (DT) +[ 19.073938] Call trace: +[ 19.073940] dump_backtrace+0x0/0x1a0 +[ 19.073942] show_stack+0x24/0x30 +[ 19.073945] dump_stack+0x9c/0xdc +[ 19.073948] check_preemption_disabled+0xe0/0x100 +[ 19.073951] debug_smp_processor_id+0x1c/0x30 +[ 19.073954] enetc_open+0x1b0/0xbc0 +[ 19.073957] __dev_open+0xdc/0x160 +[ 19.073960] __dev_change_flags+0x160/0x1d0 +[ 19.073963] dev_change_flags+0x34/0x70 +[ 19.073966] do_setlink+0x2a0/0xcd0 +[ 19.073969] rtnl_setlink+0xe4/0x140 +[ 19.073972] rtnetlink_rcv_msg+0x18c/0x500 +[ 19.073975] netlink_rcv_skb+0x60/0x120 +[ 19.073978] rtnetlink_rcv+0x28/0x40 +[ 19.073982] netlink_unicast+0x194/0x210 +[ 19.073985] netlink_sendmsg+0x194/0x330 +[ 19.073987] sock_sendmsg+0x34/0x50 +[ 19.073990] __sys_sendto+0xe4/0x150 +[ 19.073992] __arm64_sys_sendto+0x30/0x40 +[ 19.073996] el0_svc_common+0xa4/0x1a0 +[ 19.073999] el0_svc_handler+0x38/0x80 +[ 19.074002] el0_svc+0x8/0xc + +But there already exists a spinlock optimized for the single writer, +multiple readers case: the rwlock_t. The writer in this case is the MDIO +access code (irrelevant whether that MDIO access is a register read or +write), and the reader is everybody else. + +This patch also fixes two more existing bugs in the errata workaround: +- The MDIO access code was not unlocking the per-CPU spinlocks in the + reverse order of their locking order. +- The per-CPU spinlock array was not initialized. + +Fixes: 5ec0d668d62e ("enetc: WA for MDIO register access issue") +Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> +Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com> +--- + drivers/net/ethernet/freescale/enetc/enetc.c | 59 ++++++---------------- + drivers/net/ethernet/freescale/enetc/enetc_hw.h | 67 ++++--------------------- + drivers/net/ethernet/freescale/enetc/enetc_pf.c | 5 +- + 3 files changed, 30 insertions(+), 101 deletions(-) + +--- a/drivers/net/ethernet/freescale/enetc/enetc.c ++++ b/drivers/net/ethernet/freescale/enetc/enetc.c +@@ -20,13 +20,8 @@ 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)) +@@ -39,11 +34,9 @@ netdev_tx_t enetc_xmit(struct sk_buff *s + return NETDEV_TX_BUSY; + } + +- spin_lock_irqsave(lock, flags); +- ++ read_lock(&enetc_mdio_lock); + count = enetc_map_tx_buffs(tx_ring, skb, priv->active_offloads); +- +- spin_unlock_irqrestore(lock, flags); ++ read_unlock(&enetc_mdio_lock); + + if (unlikely(!count)) + goto drop_packet_err; +@@ -259,13 +252,9 @@ 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); ++ read_lock(&enetc_mdio_lock); + + /* disable interrupts */ + enetc_wr_reg_hot(v->rbier, 0); +@@ -273,7 +262,7 @@ static irqreturn_t enetc_msix(int irq, v + for_each_set_bit(i, &v->tx_rings_map, v->count_tx_rings) + enetc_wr_reg_hot(v->tbier_base + ENETC_BDR_OFF(i), 0); + +- spin_unlock_irqrestore(lock, flags); ++ read_unlock(&enetc_mdio_lock); + + napi_schedule_irqoff(&v->napi); + +@@ -289,9 +278,6 @@ 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; + +@@ -308,8 +294,7 @@ static int enetc_poll(struct napi_struct + + napi_complete_done(napi, work_done); + +- lock = this_cpu_ptr(&enetc_gregs); +- spin_lock_irqsave(lock, flags); ++ read_lock(&enetc_mdio_lock); + + /* enable interrupts */ + enetc_wr_reg_hot(v->rbier, ENETC_RBIER_RXTIE); +@@ -318,7 +303,7 @@ static int enetc_poll(struct napi_struct + enetc_wr_reg_hot(v->tbier_base + ENETC_BDR_OFF(i), + ENETC_TBIER_TXTIE); + +- spin_unlock_irqrestore(lock, flags); ++ read_unlock(&enetc_mdio_lock); + + return work_done; + } +@@ -363,18 +348,12 @@ 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); ++ read_lock(&enetc_mdio_lock); + bds_to_clean = enetc_bd_ready_count(tx_ring, i); +- spin_unlock_irqrestore(lock, flags); ++ read_unlock(&enetc_mdio_lock); + + do_tstamp = false; + +@@ -417,7 +396,7 @@ static bool enetc_clean_tx_ring(struct e + tx_swbd = tx_ring->tx_swbd; + } + +- spin_lock_irqsave(lock, flags); ++ read_lock(&enetc_mdio_lock); + + /* BD iteration loop end */ + if (is_eof) { +@@ -430,7 +409,7 @@ static bool enetc_clean_tx_ring(struct e + if (unlikely(!bds_to_clean)) + bds_to_clean = enetc_bd_ready_count(tx_ring, i); + +- spin_unlock_irqrestore(lock, flags); ++ read_unlock(&enetc_mdio_lock); + } + + tx_ring->next_to_clean = i; +@@ -516,7 +495,7 @@ static int enetc_refill_rx_ring(struct e + } + + #ifdef CONFIG_FSL_ENETC_HW_TIMESTAMPING +-/* Must be called with &enetc_gregs spinlock held */ ++/* Must be called with the read-side enetc_mdio_lock held */ + static void enetc_get_rx_tstamp(struct net_device *ndev, + union enetc_rx_bd *rxbd, + struct sk_buff *skb) +@@ -667,12 +646,6 @@ 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; +@@ -683,7 +656,7 @@ static int enetc_clean_rx_ring(struct en + u32 bd_status; + u16 size; + +- spin_lock_irqsave(lock, flags); ++ read_lock(&enetc_mdio_lock); + + if (cleaned_cnt >= ENETC_RXBD_BUNDLE) { + int count = enetc_refill_rx_ring(rx_ring, cleaned_cnt); +@@ -694,7 +667,7 @@ 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) { +- spin_unlock_irqrestore(lock, flags); ++ read_unlock(&enetc_mdio_lock); + break; + } + +@@ -703,7 +676,7 @@ static int enetc_clean_rx_ring(struct en + size = le16_to_cpu(rxbd->r.buf_len); + skb = enetc_map_rx_buff_to_skb(rx_ring, i, size); + if (!skb) { +- spin_unlock_irqrestore(lock, flags); ++ read_unlock(&enetc_mdio_lock); + break; + } + +@@ -719,7 +692,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); ++ read_unlock(&enetc_mdio_lock); + dev_kfree_skb(skb); + while (!(bd_status & ENETC_RXBD_LSTATUS_F)) { + dma_rmb(); +@@ -763,7 +736,7 @@ static int enetc_clean_rx_ring(struct en + + enetc_process_skb(rx_ring, skb); + +- spin_unlock_irqrestore(lock, flags); ++ read_unlock(&enetc_mdio_lock); + + napi_gro_receive(napi, skb); + +--- a/drivers/net/ethernet/freescale/enetc/enetc_hw.h ++++ b/drivers/net/ethernet/freescale/enetc/enetc_hw.h +@@ -325,7 +325,7 @@ struct enetc_hw { + #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 ++ * only under the rwlock_t enetc_mdio_lock + */ + #define enetc_rd_reg_hot(reg) ioread32((reg)) + #define enetc_wr_reg_hot(reg, val) iowrite32((val), (reg)) +@@ -348,90 +348,45 @@ static inline u64 enetc_rd_reg64(void __ + } + #endif + +-extern DEFINE_PER_CPU(spinlock_t, enetc_gregs); ++extern rwlock_t enetc_mdio_lock; + + 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); ++ read_lock(&enetc_mdio_lock); + val = ioread32(reg); +- spin_unlock_irqrestore(lock, flags); ++ read_unlock(&enetc_mdio_lock); + + 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); ++ read_lock(&enetc_mdio_lock); + iowrite32(val, reg); +- spin_unlock_irqrestore(lock, flags); ++ read_unlock(&enetc_mdio_lock); + } + +-/* 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; ++ u32 val; + +- local_irq_save(flags); +- preempt_disable(); +- +- for_each_online_cpu(cpu) { +- lock[cpu] = per_cpu_ptr(&enetc_gregs, cpu); +- spin_lock(lock[cpu]); +- } +- ++ write_lock_irqsave(&enetc_mdio_lock, flags); + val = ioread32(reg); +- +- for_each_online_cpu(cpu) +- spin_unlock(lock[cpu]); +- local_irq_restore(flags); +- +- preempt_enable(); ++ write_unlock_irqrestore(&enetc_mdio_lock, flags); + + 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]); +- } +- ++ write_lock_irqsave(&enetc_mdio_lock, flags); + iowrite32(val, reg); +- +- for_each_online_cpu(cpu) +- spin_unlock(lock[cpu]); +- local_irq_restore(flags); +- +- preempt_enable(); ++ write_unlock_irqrestore(&enetc_mdio_lock, flags); + } + + #define enetc_rd(hw, off) enetc_rd_reg((hw)->reg + (off)) +--- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c ++++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c +@@ -986,8 +986,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); ++/* Lock for MDIO access errata on LS1028A */ ++DEFINE_RWLOCK(enetc_mdio_lock); ++EXPORT_SYMBOL_GPL(enetc_mdio_lock); + + static const struct pci_device_id enetc_pf_id_table[] = { + { PCI_DEVICE(PCI_VENDOR_ID_FREESCALE, ENETC_DEV_ID_PF) }, |