Commit 86c432ca5d6da90a26ac8d3e680f2268b502d9c5
Committed by
David S. Miller
1 parent
883cb07583
Exists in
master
and in
6 other branches
Revert "sfc: Use write-combining to reduce TX latency" and follow-ups
This reverts commits 65f0b417dee94f779ce9b77102b7d73c93723b39, d88d6b05fee3cc78e5b0273eb58c31201dcc6b76, fcfa060468a4edcf776f0c1211d826d5de1668c1, 747df2258b1b9a2e25929ef496262c339c380009 and 867955f5682f7157fdafe8670804b9f8ea077bc7. Depending on the processor model, write-combining may result in reordering that the NIC will not tolerate. This typically results in a DMA error event and reset by the driver, logged as: sfc 0000:0e:00.0: eth2: TX DMA Q reports TX_EV_PKT_ERR. sfc 0000:0e:00.0: eth2: resetting (ALL) Signed-off-by: Ben Hutchings <bhutchings@solarflare.com> Signed-off-by: David S. Miller <davem@davemloft.net>
Showing 7 changed files with 27 additions and 88 deletions Side-by-side Diff
drivers/net/sfc/efx.c
... | ... | @@ -1050,7 +1050,6 @@ |
1050 | 1050 | { |
1051 | 1051 | struct pci_dev *pci_dev = efx->pci_dev; |
1052 | 1052 | dma_addr_t dma_mask = efx->type->max_dma_mask; |
1053 | - bool use_wc; | |
1054 | 1053 | int rc; |
1055 | 1054 | |
1056 | 1055 | netif_dbg(efx, probe, efx->net_dev, "initialising I/O\n"); |
... | ... | @@ -1101,21 +1100,8 @@ |
1101 | 1100 | rc = -EIO; |
1102 | 1101 | goto fail3; |
1103 | 1102 | } |
1104 | - | |
1105 | - /* bug22643: If SR-IOV is enabled then tx push over a write combined | |
1106 | - * mapping is unsafe. We need to disable write combining in this case. | |
1107 | - * MSI is unsupported when SR-IOV is enabled, and the firmware will | |
1108 | - * have removed the MSI capability. So write combining is safe if | |
1109 | - * there is an MSI capability. | |
1110 | - */ | |
1111 | - use_wc = (!EFX_WORKAROUND_22643(efx) || | |
1112 | - pci_find_capability(pci_dev, PCI_CAP_ID_MSI)); | |
1113 | - if (use_wc) | |
1114 | - efx->membase = ioremap_wc(efx->membase_phys, | |
1115 | - efx->type->mem_map_size); | |
1116 | - else | |
1117 | - efx->membase = ioremap_nocache(efx->membase_phys, | |
1118 | - efx->type->mem_map_size); | |
1103 | + efx->membase = ioremap_nocache(efx->membase_phys, | |
1104 | + efx->type->mem_map_size); | |
1119 | 1105 | if (!efx->membase) { |
1120 | 1106 | netif_err(efx, probe, efx->net_dev, |
1121 | 1107 | "could not map memory BAR at %llx+%x\n", |
drivers/net/sfc/io.h
... | ... | @@ -48,9 +48,9 @@ |
48 | 48 | * replacing the low 96 bits with zero does not affect functionality. |
49 | 49 | * - If the host writes to the last dword address of such a register |
50 | 50 | * (i.e. the high 32 bits) the underlying register will always be |
51 | - * written. If the collector and the current write together do not | |
52 | - * provide values for all 128 bits of the register, the low 96 bits | |
53 | - * will be written as zero. | |
51 | + * written. If the collector does not hold values for the low 96 | |
52 | + * bits of the register, they will be written as zero. Writing to | |
53 | + * the last qword does not have this effect and must not be done. | |
54 | 54 | * - If the host writes to the address of any other part of such a |
55 | 55 | * register while the collector already holds values for some other |
56 | 56 | * register, the write is discarded and the collector maintains its |
... | ... | @@ -103,7 +103,6 @@ |
103 | 103 | _efx_writed(efx, value->u32[2], reg + 8); |
104 | 104 | _efx_writed(efx, value->u32[3], reg + 12); |
105 | 105 | #endif |
106 | - wmb(); | |
107 | 106 | mmiowb(); |
108 | 107 | spin_unlock_irqrestore(&efx->biu_lock, flags); |
109 | 108 | } |
... | ... | @@ -126,7 +125,6 @@ |
126 | 125 | __raw_writel((__force u32)value->u32[0], membase + addr); |
127 | 126 | __raw_writel((__force u32)value->u32[1], membase + addr + 4); |
128 | 127 | #endif |
129 | - wmb(); | |
130 | 128 | mmiowb(); |
131 | 129 | spin_unlock_irqrestore(&efx->biu_lock, flags); |
132 | 130 | } |
... | ... | @@ -141,7 +139,6 @@ |
141 | 139 | |
142 | 140 | /* No lock required */ |
143 | 141 | _efx_writed(efx, value->u32[0], reg); |
144 | - wmb(); | |
145 | 142 | } |
146 | 143 | |
147 | 144 | /* Read a 128-bit CSR, locking as appropriate. */ |
... | ... | @@ -152,7 +149,6 @@ |
152 | 149 | |
153 | 150 | spin_lock_irqsave(&efx->biu_lock, flags); |
154 | 151 | value->u32[0] = _efx_readd(efx, reg + 0); |
155 | - rmb(); | |
156 | 152 | value->u32[1] = _efx_readd(efx, reg + 4); |
157 | 153 | value->u32[2] = _efx_readd(efx, reg + 8); |
158 | 154 | value->u32[3] = _efx_readd(efx, reg + 12); |
... | ... | @@ -175,7 +171,6 @@ |
175 | 171 | value->u64[0] = (__force __le64)__raw_readq(membase + addr); |
176 | 172 | #else |
177 | 173 | value->u32[0] = (__force __le32)__raw_readl(membase + addr); |
178 | - rmb(); | |
179 | 174 | value->u32[1] = (__force __le32)__raw_readl(membase + addr + 4); |
180 | 175 | #endif |
181 | 176 | spin_unlock_irqrestore(&efx->biu_lock, flags); |
182 | 177 | |
183 | 178 | |
... | ... | @@ -242,14 +237,12 @@ |
242 | 237 | |
243 | 238 | #ifdef EFX_USE_QWORD_IO |
244 | 239 | _efx_writeq(efx, value->u64[0], reg + 0); |
245 | - _efx_writeq(efx, value->u64[1], reg + 8); | |
246 | 240 | #else |
247 | 241 | _efx_writed(efx, value->u32[0], reg + 0); |
248 | 242 | _efx_writed(efx, value->u32[1], reg + 4); |
243 | +#endif | |
249 | 244 | _efx_writed(efx, value->u32[2], reg + 8); |
250 | 245 | _efx_writed(efx, value->u32[3], reg + 12); |
251 | -#endif | |
252 | - wmb(); | |
253 | 246 | } |
254 | 247 | #define efx_writeo_page(efx, value, reg, page) \ |
255 | 248 | _efx_writeo_page(efx, value, \ |
drivers/net/sfc/mcdi.c
... | ... | @@ -50,20 +50,6 @@ |
50 | 50 | return &nic_data->mcdi; |
51 | 51 | } |
52 | 52 | |
53 | -static inline void | |
54 | -efx_mcdi_readd(struct efx_nic *efx, efx_dword_t *value, unsigned reg) | |
55 | -{ | |
56 | - struct siena_nic_data *nic_data = efx->nic_data; | |
57 | - value->u32[0] = (__force __le32)__raw_readl(nic_data->mcdi_smem + reg); | |
58 | -} | |
59 | - | |
60 | -static inline void | |
61 | -efx_mcdi_writed(struct efx_nic *efx, const efx_dword_t *value, unsigned reg) | |
62 | -{ | |
63 | - struct siena_nic_data *nic_data = efx->nic_data; | |
64 | - __raw_writel((__force u32)value->u32[0], nic_data->mcdi_smem + reg); | |
65 | -} | |
66 | - | |
67 | 53 | void efx_mcdi_init(struct efx_nic *efx) |
68 | 54 | { |
69 | 55 | struct efx_mcdi_iface *mcdi; |
... | ... | @@ -84,8 +70,8 @@ |
84 | 70 | const u8 *inbuf, size_t inlen) |
85 | 71 | { |
86 | 72 | struct efx_mcdi_iface *mcdi = efx_mcdi(efx); |
87 | - unsigned pdu = MCDI_PDU(efx); | |
88 | - unsigned doorbell = MCDI_DOORBELL(efx); | |
73 | + unsigned pdu = FR_CZ_MC_TREG_SMEM + MCDI_PDU(efx); | |
74 | + unsigned doorbell = FR_CZ_MC_TREG_SMEM + MCDI_DOORBELL(efx); | |
89 | 75 | unsigned int i; |
90 | 76 | efx_dword_t hdr; |
91 | 77 | u32 xflags, seqno; |
92 | 78 | |
93 | 79 | |
94 | 80 | |
95 | 81 | |
96 | 82 | |
... | ... | @@ -106,28 +92,29 @@ |
106 | 92 | MCDI_HEADER_SEQ, seqno, |
107 | 93 | MCDI_HEADER_XFLAGS, xflags); |
108 | 94 | |
109 | - efx_mcdi_writed(efx, &hdr, pdu); | |
95 | + efx_writed(efx, &hdr, pdu); | |
110 | 96 | |
111 | 97 | for (i = 0; i < inlen; i += 4) |
112 | - efx_mcdi_writed(efx, (const efx_dword_t *)(inbuf + i), | |
113 | - pdu + 4 + i); | |
98 | + _efx_writed(efx, *((__le32 *)(inbuf + i)), pdu + 4 + i); | |
114 | 99 | |
100 | + /* Ensure the payload is written out before the header */ | |
101 | + wmb(); | |
102 | + | |
115 | 103 | /* ring the doorbell with a distinctive value */ |
116 | - EFX_POPULATE_DWORD_1(hdr, EFX_DWORD_0, 0x45789abc); | |
117 | - efx_mcdi_writed(efx, &hdr, doorbell); | |
104 | + _efx_writed(efx, (__force __le32) 0x45789abc, doorbell); | |
118 | 105 | } |
119 | 106 | |
120 | 107 | static void efx_mcdi_copyout(struct efx_nic *efx, u8 *outbuf, size_t outlen) |
121 | 108 | { |
122 | 109 | struct efx_mcdi_iface *mcdi = efx_mcdi(efx); |
123 | - unsigned int pdu = MCDI_PDU(efx); | |
110 | + unsigned int pdu = FR_CZ_MC_TREG_SMEM + MCDI_PDU(efx); | |
124 | 111 | int i; |
125 | 112 | |
126 | 113 | BUG_ON(atomic_read(&mcdi->state) == MCDI_STATE_QUIESCENT); |
127 | 114 | BUG_ON(outlen & 3 || outlen >= 0x100); |
128 | 115 | |
129 | 116 | for (i = 0; i < outlen; i += 4) |
130 | - efx_mcdi_readd(efx, (efx_dword_t *)(outbuf + i), pdu + 4 + i); | |
117 | + *((__le32 *)(outbuf + i)) = _efx_readd(efx, pdu + 4 + i); | |
131 | 118 | } |
132 | 119 | |
133 | 120 | static int efx_mcdi_poll(struct efx_nic *efx) |
... | ... | @@ -135,7 +122,7 @@ |
135 | 122 | struct efx_mcdi_iface *mcdi = efx_mcdi(efx); |
136 | 123 | unsigned int time, finish; |
137 | 124 | unsigned int respseq, respcmd, error; |
138 | - unsigned int pdu = MCDI_PDU(efx); | |
125 | + unsigned int pdu = FR_CZ_MC_TREG_SMEM + MCDI_PDU(efx); | |
139 | 126 | unsigned int rc, spins; |
140 | 127 | efx_dword_t reg; |
141 | 128 | |
... | ... | @@ -161,7 +148,8 @@ |
161 | 148 | |
162 | 149 | time = get_seconds(); |
163 | 150 | |
164 | - efx_mcdi_readd(efx, ®, pdu); | |
151 | + rmb(); | |
152 | + efx_readd(efx, ®, pdu); | |
165 | 153 | |
166 | 154 | /* All 1's indicates that shared memory is in reset (and is |
167 | 155 | * not a valid header). Wait for it to come out reset before |
... | ... | @@ -188,7 +176,7 @@ |
188 | 176 | respseq, mcdi->seqno); |
189 | 177 | rc = EIO; |
190 | 178 | } else if (error) { |
191 | - efx_mcdi_readd(efx, ®, pdu + 4); | |
179 | + efx_readd(efx, ®, pdu + 4); | |
192 | 180 | switch (EFX_DWORD_FIELD(reg, EFX_DWORD_0)) { |
193 | 181 | #define TRANSLATE_ERROR(name) \ |
194 | 182 | case MC_CMD_ERR_ ## name: \ |
195 | 183 | |
196 | 184 | |
... | ... | @@ -222,21 +210,21 @@ |
222 | 210 | /* Test and clear MC-rebooted flag for this port/function */ |
223 | 211 | int efx_mcdi_poll_reboot(struct efx_nic *efx) |
224 | 212 | { |
225 | - unsigned int addr = MCDI_REBOOT_FLAG(efx); | |
213 | + unsigned int addr = FR_CZ_MC_TREG_SMEM + MCDI_REBOOT_FLAG(efx); | |
226 | 214 | efx_dword_t reg; |
227 | 215 | uint32_t value; |
228 | 216 | |
229 | 217 | if (efx_nic_rev(efx) < EFX_REV_SIENA_A0) |
230 | 218 | return false; |
231 | 219 | |
232 | - efx_mcdi_readd(efx, ®, addr); | |
220 | + efx_readd(efx, ®, addr); | |
233 | 221 | value = EFX_DWORD_FIELD(reg, EFX_DWORD_0); |
234 | 222 | |
235 | 223 | if (value == 0) |
236 | 224 | return 0; |
237 | 225 | |
238 | 226 | EFX_ZERO_DWORD(reg); |
239 | - efx_mcdi_writed(efx, ®, addr); | |
227 | + efx_writed(efx, ®, addr); | |
240 | 228 | |
241 | 229 | if (value == MC_STATUS_DWORD_ASSERT) |
242 | 230 | return -EINTR; |
drivers/net/sfc/nic.c
... | ... | @@ -1936,13 +1936,6 @@ |
1936 | 1936 | |
1937 | 1937 | size = min_t(size_t, table->step, 16); |
1938 | 1938 | |
1939 | - if (table->offset >= efx->type->mem_map_size) { | |
1940 | - /* No longer mapped; return dummy data */ | |
1941 | - memcpy(buf, "\xde\xc0\xad\xde", 4); | |
1942 | - buf += table->rows * size; | |
1943 | - continue; | |
1944 | - } | |
1945 | - | |
1946 | 1939 | for (i = 0; i < table->rows; i++) { |
1947 | 1940 | switch (table->step) { |
1948 | 1941 | case 4: /* 32-bit register or SRAM */ |
drivers/net/sfc/nic.h
... | ... | @@ -143,12 +143,10 @@ |
143 | 143 | /** |
144 | 144 | * struct siena_nic_data - Siena NIC state |
145 | 145 | * @mcdi: Management-Controller-to-Driver Interface |
146 | - * @mcdi_smem: MCDI shared memory mapping. The mapping is always uncacheable. | |
147 | 146 | * @wol_filter_id: Wake-on-LAN packet filter id |
148 | 147 | */ |
149 | 148 | struct siena_nic_data { |
150 | 149 | struct efx_mcdi_iface mcdi; |
151 | - void __iomem *mcdi_smem; | |
152 | 150 | int wol_filter_id; |
153 | 151 | }; |
154 | 152 |
drivers/net/sfc/siena.c
... | ... | @@ -250,26 +250,12 @@ |
250 | 250 | efx_reado(efx, ®, FR_AZ_CS_DEBUG); |
251 | 251 | efx->net_dev->dev_id = EFX_OWORD_FIELD(reg, FRF_CZ_CS_PORT_NUM) - 1; |
252 | 252 | |
253 | - /* Initialise MCDI */ | |
254 | - nic_data->mcdi_smem = ioremap_nocache(efx->membase_phys + | |
255 | - FR_CZ_MC_TREG_SMEM, | |
256 | - FR_CZ_MC_TREG_SMEM_STEP * | |
257 | - FR_CZ_MC_TREG_SMEM_ROWS); | |
258 | - if (!nic_data->mcdi_smem) { | |
259 | - netif_err(efx, probe, efx->net_dev, | |
260 | - "could not map MCDI at %llx+%x\n", | |
261 | - (unsigned long long)efx->membase_phys + | |
262 | - FR_CZ_MC_TREG_SMEM, | |
263 | - FR_CZ_MC_TREG_SMEM_STEP * FR_CZ_MC_TREG_SMEM_ROWS); | |
264 | - rc = -ENOMEM; | |
265 | - goto fail1; | |
266 | - } | |
267 | 253 | efx_mcdi_init(efx); |
268 | 254 | |
269 | 255 | /* Recover from a failed assertion before probing */ |
270 | 256 | rc = efx_mcdi_handle_assertion(efx); |
271 | 257 | if (rc) |
272 | - goto fail2; | |
258 | + goto fail1; | |
273 | 259 | |
274 | 260 | /* Let the BMC know that the driver is now in charge of link and |
275 | 261 | * filter settings. We must do this before we reset the NIC */ |
... | ... | @@ -324,7 +310,6 @@ |
324 | 310 | fail3: |
325 | 311 | efx_mcdi_drv_attach(efx, false, NULL); |
326 | 312 | fail2: |
327 | - iounmap(nic_data->mcdi_smem); | |
328 | 313 | fail1: |
329 | 314 | kfree(efx->nic_data); |
330 | 315 | return rc; |
... | ... | @@ -404,8 +389,6 @@ |
404 | 389 | |
405 | 390 | static void siena_remove_nic(struct efx_nic *efx) |
406 | 391 | { |
407 | - struct siena_nic_data *nic_data = efx->nic_data; | |
408 | - | |
409 | 392 | efx_nic_free_buffer(efx, &efx->irq_status); |
410 | 393 | |
411 | 394 | siena_reset_hw(efx, RESET_TYPE_ALL); |
... | ... | @@ -415,8 +398,7 @@ |
415 | 398 | efx_mcdi_drv_attach(efx, false, NULL); |
416 | 399 | |
417 | 400 | /* Tear down the private nic state */ |
418 | - iounmap(nic_data->mcdi_smem); | |
419 | - kfree(nic_data); | |
401 | + kfree(efx->nic_data); | |
420 | 402 | efx->nic_data = NULL; |
421 | 403 | } |
422 | 404 | |
... | ... | @@ -656,7 +638,8 @@ |
656 | 638 | .default_mac_ops = &efx_mcdi_mac_operations, |
657 | 639 | |
658 | 640 | .revision = EFX_REV_SIENA_A0, |
659 | - .mem_map_size = FR_CZ_MC_TREG_SMEM, /* MC_TREG_SMEM mapped separately */ | |
641 | + .mem_map_size = (FR_CZ_MC_TREG_SMEM + | |
642 | + FR_CZ_MC_TREG_SMEM_STEP * FR_CZ_MC_TREG_SMEM_ROWS), | |
660 | 643 | .txd_ptr_tbl_base = FR_BZ_TX_DESC_PTR_TBL, |
661 | 644 | .rxd_ptr_tbl_base = FR_BZ_RX_DESC_PTR_TBL, |
662 | 645 | .buf_tbl_base = FR_BZ_BUF_FULL_TBL, |
drivers/net/sfc/workarounds.h
... | ... | @@ -38,8 +38,6 @@ |
38 | 38 | #define EFX_WORKAROUND_15783 EFX_WORKAROUND_ALWAYS |
39 | 39 | /* Legacy interrupt storm when interrupt fifo fills */ |
40 | 40 | #define EFX_WORKAROUND_17213 EFX_WORKAROUND_SIENA |
41 | -/* Write combining and sriov=enabled are incompatible */ | |
42 | -#define EFX_WORKAROUND_22643 EFX_WORKAROUND_SIENA | |
43 | 41 | |
44 | 42 | /* Spurious parity errors in TSORT buffers */ |
45 | 43 | #define EFX_WORKAROUND_5129 EFX_WORKAROUND_FALCON_A |