[RFC][PATCH] apparent big-endian bugs in dwc-xlgmac
flat view
STALE3235d
From: Al Viro <viro@ZenIV.linux.org.uk>
Date: 2017-12-10 04:54:03
Subsystem:
networking drivers, synopsys designware enterprise ethernet driver, the rest · Maintainers:
Andrew Lunn, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Jose Abreu, Linus Torvalds
In xlgmac_dev_xmit():
/* Mark it as a CONTEXT descriptor */
dma_desc->desc3 = XLGMAC_SET_REG_BITS_LE(
dma_desc->desc3,
TX_CONTEXT_DESC3_CTXT_POS,
TX_CONTEXT_DESC3_CTXT_LEN,
1);
Looking at XLGMAC_SET_REG_BITS_LE() we see this:
#define XLGMAC_SET_REG_BITS_LE(var, pos, len, val) ({ \
typeof(var) _var = (var); \
typeof(pos) _pos = (pos); \
typeof(len) _len = (len); \
typeof(val) _val = (val); \
_val = (_val << _pos) & GENMASK(_pos + _len - 1, _pos); \
_var = (_var & ~GENMASK(_pos + _len - 1, _pos)) | _val; \
cpu_to_le32(_var); \
})
That thing assumes var to be host-endian and has a little-endian result.
Unfortunately, we feed it a little-endian and store the result back into
the same place. That might work if the original values *was* host-endian
and we wanted to end up with little-endian. However, that is immediately
followed by
/* Indicate this descriptor contains the MSS */
dma_desc->desc3 = XLGMAC_SET_REG_BITS_LE(
dma_desc->desc3,
TX_CONTEXT_DESC3_TCMSSV_POS,
TX_CONTEXT_DESC3_TCMSSV_LEN,
1);
where we operate on the now definitely little-endian value. That really
can't be right. I don't have the hardware in question, so I can't test
that, but it smells like this needs something like diff below, making
XLGMAC_SET_REG_BITS_LE take le32 and return le32. GET side of things
is le32 -> u32; definition looks correct, but slightly misannotated.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---diff --git a/drivers/net/ethernet/synopsys/dwc-xlgmac.h b/drivers/net/ethernet/synopsys/dwc-xlgmac.h
index cab3e40a86b9..e95c4c250e16 100644
--- a/drivers/net/ethernet/synopsys/dwc-xlgmac.h
+++ b/drivers/net/ethernet/synopsys/dwc-xlgmac.h@@ -106,7 +106,7 @@ #define XLGMAC_GET_REG_BITS_LE(var, pos, len) ({ \ typeof(pos) _pos = (pos); \ typeof(len) _len = (len); \ - typeof(var) _var = le32_to_cpu((var)); \ + u32 _var = le32_to_cpu((var)); \ ((_var) & GENMASK(_pos + _len - 1, _pos)) >> (_pos); \ })
@@ -125,8 +125,8 @@ typeof(len) _len = (len); \ typeof(val) _val = (val); \ _val = (_val << _pos) & GENMASK(_pos + _len - 1, _pos); \ - _var = (_var & ~GENMASK(_pos + _len - 1, _pos)) | _val; \ - cpu_to_le32(_var); \ + (_var & ~cpu_to_le32(GENMASK(_pos + _len - 1, _pos))) | \ + cpu_to_le32(_val); \ }) struct xlgmac_pdata;