mirror of
https://github.com/openwrt/openwrt.git
synced 2024-12-25 08:21:14 +00:00
a282a04538
All modifications made by update_kernel.sh run in a fresh clone without any existing toolchains. Build system: x86_64 Build-tested: ipq806x/R7800, ath79/generic, bcm27xx/bcm2711 Run-tested: ipq806x/R7800 No dmesg regressions, everything functional Signed-off-by: John Audia <graysky@archlinux.us>
386 lines
12 KiB
Diff
386 lines
12 KiB
Diff
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, ENETC_MAX_NUM_TXQS)
|
|
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
|
|
@@ -329,7 +329,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))
|
|
@@ -352,90 +352,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
|
|
@@ -987,8 +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);
|
|
+/* 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) },
|