Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 05/11] net: ethernet: mediatek: remove unnecessary spaces from Makefile
From: Bartosz Golaszewski @ 2020-05-20 11:25 UTC (permalink / raw)
  To: Rob Herring, David S . Miller, Matthias Brugger, John Crispin,
	Sean Wang, Mark Lee, Jakub Kicinski, Arnd Bergmann, Fabien Parent,
	Heiner Kallweit, Edwin Peer
  Cc: devicetree, Stephane Le Provost, netdev, linux-kernel,
	Bartosz Golaszewski, linux-mediatek, Andrew Perepech, Pedro Tsai,
	linux-arm-kernel
In-Reply-To: <20200520112523.30995-1-brgl@bgdev.pl>

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

The Makefile formatting in the kernel tree usually doesn't use tabs,
so remove them before we add a second driver.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/net/ethernet/mediatek/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mediatek/Makefile b/drivers/net/ethernet/mediatek/Makefile
index 2d8362f9341b..3362fb7ef859 100644
--- a/drivers/net/ethernet/mediatek/Makefile
+++ b/drivers/net/ethernet/mediatek/Makefile
@@ -3,5 +3,5 @@
 # Makefile for the Mediatek SoCs built-in ethernet macs
 #
 
-obj-$(CONFIG_NET_MEDIATEK_SOC)                 += mtk_eth.o
+obj-$(CONFIG_NET_MEDIATEK_SOC) += mtk_eth.o
 mtk_eth-y := mtk_eth_soc.o mtk_sgmii.o mtk_eth_path.o
-- 
2.25.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v4 06/11] net: ethernet: mtk-eth-mac: new driver
From: Bartosz Golaszewski @ 2020-05-20 11:25 UTC (permalink / raw)
  To: Rob Herring, David S . Miller, Matthias Brugger, John Crispin,
	Sean Wang, Mark Lee, Jakub Kicinski, Arnd Bergmann, Fabien Parent,
	Heiner Kallweit, Edwin Peer
  Cc: devicetree, Stephane Le Provost, netdev, linux-kernel,
	Bartosz Golaszewski, linux-mediatek, Andrew Perepech, Pedro Tsai,
	linux-arm-kernel
In-Reply-To: <20200520112523.30995-1-brgl@bgdev.pl>

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

This adds the driver for the MediaTek Ethernet MAC used on the MT8* SoC
family. For now we only support full-duplex.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/net/ethernet/mediatek/Kconfig       |    6 +
 drivers/net/ethernet/mediatek/Makefile      |    1 +
 drivers/net/ethernet/mediatek/mtk_eth_mac.c | 1668 +++++++++++++++++++
 3 files changed, 1675 insertions(+)
 create mode 100644 drivers/net/ethernet/mediatek/mtk_eth_mac.c

diff --git a/drivers/net/ethernet/mediatek/Kconfig b/drivers/net/ethernet/mediatek/Kconfig
index 5079b8090f16..5c3793076765 100644
--- a/drivers/net/ethernet/mediatek/Kconfig
+++ b/drivers/net/ethernet/mediatek/Kconfig
@@ -14,4 +14,10 @@ config NET_MEDIATEK_SOC
 	  This driver supports the gigabit ethernet MACs in the
 	  MediaTek SoC family.
 
+config NET_MEDIATEK_MAC
+	tristate "MediaTek Ethernet MAC support"
+	select PHYLIB
+	help
+	  This driver supports the ethernet IP on MediaTek MT85** SoCs.
+
 endif #NET_VENDOR_MEDIATEK
diff --git a/drivers/net/ethernet/mediatek/Makefile b/drivers/net/ethernet/mediatek/Makefile
index 3362fb7ef859..f7f5638943a0 100644
--- a/drivers/net/ethernet/mediatek/Makefile
+++ b/drivers/net/ethernet/mediatek/Makefile
@@ -5,3 +5,4 @@
 
 obj-$(CONFIG_NET_MEDIATEK_SOC) += mtk_eth.o
 mtk_eth-y := mtk_eth_soc.o mtk_sgmii.o mtk_eth_path.o
+obj-$(CONFIG_NET_MEDIATEK_MAC) += mtk_eth_mac.o
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_mac.c b/drivers/net/ethernet/mediatek/mtk_eth_mac.c
new file mode 100644
index 000000000000..4dfe7c2c4e3d
--- /dev/null
+++ b/drivers/net/ethernet/mediatek/mtk_eth_mac.c
@@ -0,0 +1,1668 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2020 MediaTek Corporation
+ * Copyright (c) 2020 BayLibre SAS
+ *
+ * Author: Bartosz Golaszewski <bgolaszewski@baylibre.com>
+ */
+
+#include <linux/bits.h>
+#include <linux/clk.h>
+#include <linux/dma-mapping.h>
+#include <linux/etherdevice.h>
+#include <linux/jiffies.h>
+#include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
+#include <linux/mii.h>
+#include <linux/module.h>
+#include <linux/netdevice.h>
+#include <linux/of.h>
+#include <linux/of_mdio.h>
+#include <linux/of_net.h>
+#include <linux/platform_device.h>
+#include <linux/pm.h>
+#include <linux/regmap.h>
+#include <linux/skbuff.h>
+#include <linux/spinlock.h>
+#include <linux/workqueue.h>
+
+#define MTK_MAC_DRVNAME				"mtk_eth_mac"
+
+#define MTK_MAC_WAIT_TIMEOUT			300
+#define MTK_MAC_MAX_FRAME_SIZE			1514
+#define MTK_MAC_SKB_ALIGNMENT			16
+#define MTK_MAC_NAPI_WEIGHT			64
+#define MTK_MAC_HASHTABLE_MC_LIMIT		256
+#define MTK_MAC_HASHTABLE_SIZE_MAX		512
+
+/* Normally we'd use NET_IP_ALIGN but on arm64 its value is 0 and it doesn't
+ * work for this controller.
+ */
+#define MTK_MAC_IP_ALIGN			2
+
+static const char *const mtk_mac_clk_names[] = { "core", "reg", "trans" };
+#define MTK_MAC_NCLKS ARRAY_SIZE(mtk_mac_clk_names)
+
+/* PHY Control Register 0 */
+#define MTK_MAC_REG_PHY_CTRL0			0x0000
+#define MTK_MAC_BIT_PHY_CTRL0_WTCMD		BIT(13)
+#define MTK_MAC_BIT_PHY_CTRL0_RDCMD		BIT(14)
+#define MTK_MAC_BIT_PHY_CTRL0_RWOK		BIT(15)
+#define MTK_MAC_MSK_PHY_CTRL0_PREG		GENMASK(12, 8)
+#define MTK_MAC_OFF_PHY_CTRL0_PREG		8
+#define MTK_MAC_MSK_PHY_CTRL0_RWDATA		GENMASK(31, 16)
+#define MTK_MAC_OFF_PHY_CTRL0_RWDATA		16
+
+/* PHY Control Register 1 */
+#define MTK_MAC_REG_PHY_CTRL1			0x0004
+#define MTK_MAC_BIT_PHY_CTRL1_LINK_ST		BIT(0)
+#define MTK_MAC_BIT_PHY_CTRL1_AN_EN		BIT(8)
+#define MTK_MAC_OFF_PHY_CTRL1_FORCE_SPD		9
+#define MTK_MAC_VAL_PHY_CTRL1_FORCE_SPD_10M	0x00
+#define MTK_MAC_VAL_PHY_CTRL1_FORCE_SPD_100M	0x01
+#define MTK_MAC_VAL_PHY_CTRL1_FORCE_SPD_1000M	0x02
+#define MTK_MAC_BIT_PHY_CTRL1_FORCE_DPX		BIT(11)
+#define MTK_MAC_BIT_PHY_CTRL1_FORCE_FC_RX	BIT(12)
+#define MTK_MAC_BIT_PHY_CTRL1_FORCE_FC_TX	BIT(13)
+
+/* MAC Configuration Register */
+#define MTK_MAC_REG_MAC_CFG			0x0008
+#define MTK_MAC_OFF_MAC_CFG_IPG			10
+#define MTK_MAC_VAL_MAC_CFG_IPG_96BIT		GENMASK(4, 0)
+#define MTK_MAC_BIT_MAC_CFG_MAXLEN_1522		BIT(16)
+#define MTK_MAC_BIT_MAC_CFG_AUTO_PAD		BIT(19)
+#define MTK_MAC_BIT_MAC_CFG_CRC_STRIP		BIT(20)
+#define MTK_MAC_BIT_MAC_CFG_VLAN_STRIP		BIT(22)
+#define MTK_MAC_BIT_MAC_CFG_NIC_PD		BIT(31)
+
+/* Flow-Control Configuration Register */
+#define MTK_MAC_REG_FC_CFG			0x000c
+#define MTK_MAC_BIT_FC_CFG_BP_EN		BIT(7)
+#define MTK_MAC_BIT_FC_CFG_UC_PAUSE_DIR	BIT(8)
+#define MTK_MAC_OFF_FC_CFG_SEND_PAUSE_TH	16
+#define MTK_MAC_MSK_FC_CFG_SEND_PAUSE_TH	GENMASK(27, 16)
+#define MTK_MAC_VAL_FC_CFG_SEND_PAUSE_TH_2K	0x800
+
+/* ARL Configuration Register */
+#define MTK_MAC_REG_ARL_CFG			0x0010
+#define MTK_MAC_BIT_ARL_CFG_HASH_ALG		BIT(0)
+#define MTK_MAC_BIT_ARL_CFG_MISC_MODE		BIT(4)
+
+/* MAC High and Low Bytes Registers */
+#define MTK_MAC_REG_MY_MAC_H			0x0014
+#define MTK_MAC_REG_MY_MAC_L			0x0018
+
+/* Hash Table Control Register */
+#define MTK_MAC_REG_HASH_CTRL			0x001c
+#define MTK_MAC_MSK_HASH_CTRL_HASH_BIT_ADDR	GENMASK(8, 0)
+#define MTK_MAC_BIT_HASH_CTRL_HASH_BIT_DATA	BIT(12)
+#define MTK_MAC_BIT_HASH_CTRL_ACC_CMD		BIT(13)
+#define MTK_MAC_BIT_HASH_CTRL_CMD_START		BIT(14)
+#define MTK_MAC_BIT_HASH_CTRL_BIST_OK		BIT(16)
+#define MTK_MAC_BIT_HASH_CTRL_BIST_DONE		BIT(17)
+#define MTK_MAC_BIT_HASH_CTRL_BIST_EN		BIT(31)
+
+/* TX DMA Control Register */
+#define MTK_MAC_REG_TX_DMA_CTRL			0x0034
+#define MTK_MAC_BIT_TX_DMA_CTRL_START		BIT(0)
+#define MTK_MAC_BIT_TX_DMA_CTRL_STOP		BIT(1)
+#define MTK_MAC_BIT_TX_DMA_CTRL_RESUME		BIT(2)
+
+/* RX DMA Control Register */
+#define MTK_MAC_REG_RX_DMA_CTRL			0x0038
+#define MTK_MAC_BIT_RX_DMA_CTRL_START		BIT(0)
+#define MTK_MAC_BIT_RX_DMA_CTRL_STOP		BIT(1)
+#define MTK_MAC_BIT_RX_DMA_CTRL_RESUME		BIT(2)
+
+/* DMA Address Registers */
+#define MTK_MAC_REG_TX_DPTR			0x003c
+#define MTK_MAC_REG_RX_DPTR			0x0040
+#define MTK_MAC_REG_TX_BASE_ADDR		0x0044
+#define MTK_MAC_REG_RX_BASE_ADDR		0x0048
+
+/* Interrupt Status Register */
+#define MTK_MAC_REG_INT_STS			0x0050
+#define MTK_MAC_REG_INT_STS_PORT_STS_CHG	BIT(2)
+#define MTK_MAC_REG_INT_STS_MIB_CNT_TH		BIT(3)
+#define MTK_MAC_BIT_INT_STS_FNRC		BIT(6)
+#define MTK_MAC_BIT_INT_STS_TNTC		BIT(8)
+
+/* Interrupt Mask Register */
+#define MTK_MAC_REG_INT_MASK			0x0054
+#define MTK_MAC_BIT_INT_MASK_FNRC		BIT(6)
+
+/* Misc. Config Register */
+#define MTK_MAC_REG_TEST1			0x005c
+#define MTK_MAC_BIT_TEST1_RST_HASH_MBIST	BIT(31)
+
+/* Extended Configuration Register */
+#define MTK_MAC_REG_EXT_CFG			0x0060
+#define MTK_MAC_OFF_EXT_CFG_SND_PAUSE_RLS	16
+#define MTK_MAC_MSK_EXT_CFG_SND_PAUSE_RLS	GENMASK(26, 16)
+#define MTK_MAC_VAL_EXT_CFG_SND_PAUSE_RLS_1K	0x400
+
+/* EthSys Configuration Register */
+#define MTK_MAC_REG_SYS_CONF			0x0094
+#define MTK_MAC_BIT_MII_PAD_OUT_ENABLE		BIT(0)
+#define MTK_MAC_BIT_EXT_MDC_MODE		BIT(1)
+#define MTK_MAC_BIT_SWC_MII_MODE		BIT(2)
+
+/* MAC Clock Configuration Register */
+#define MTK_MAC_REG_MAC_CLK_CONF		0x00ac
+#define MTK_MAC_MSK_MAC_CLK_CONF		GENMASK(7, 0)
+#define MTK_MAC_BIT_CLK_DIV_10			0x0a
+
+/* Counter registers. */
+#define MTK_MAC_REG_C_RXOKPKT			0x0100
+#define MTK_MAC_REG_C_RXOKBYTE			0x0104
+#define MTK_MAC_REG_C_RXRUNT			0x0108
+#define MTK_MAC_REG_C_RXLONG			0x010c
+#define MTK_MAC_REG_C_RXDROP			0x0110
+#define MTK_MAC_REG_C_RXCRC			0x0114
+#define MTK_MAC_REG_C_RXARLDROP			0x0118
+#define MTK_MAC_REG_C_RXVLANDROP		0x011c
+#define MTK_MAC_REG_C_RXCSERR			0x0120
+#define MTK_MAC_REG_C_RXPAUSE			0x0124
+#define MTK_MAC_REG_C_TXOKPKT			0x0128
+#define MTK_MAC_REG_C_TXOKBYTE			0x012c
+#define MTK_MAC_REG_C_TXPAUSECOL		0x0130
+#define MTK_MAC_REG_C_TXRTY			0x0134
+#define MTK_MAC_REG_C_TXSKIP			0x0138
+#define MTK_MAC_REG_C_TX_ARP			0x013c
+#define MTK_MAC_REG_C_RX_RERR			0x01d8
+#define MTK_MAC_REG_C_RX_UNI			0x01dc
+#define MTK_MAC_REG_C_RX_MULTI			0x01e0
+#define MTK_MAC_REG_C_RX_BROAD			0x01e4
+#define MTK_MAC_REG_C_RX_ALIGNERR		0x01e8
+#define MTK_MAC_REG_C_TX_UNI			0x01ec
+#define MTK_MAC_REG_C_TX_MULTI			0x01f0
+#define MTK_MAC_REG_C_TX_BROAD			0x01f4
+#define MTK_MAC_REG_C_TX_TIMEOUT		0x01f8
+#define MTK_MAC_REG_C_TX_LATECOL		0x01fc
+#define MTK_MAC_REG_C_RX_LENGTHERR		0x0214
+#define MTK_MAC_REG_C_RX_TWIST			0x0218
+
+/* Ethernet CFG Control */
+#define MTK_PERICFG_REG_NIC_CFG_CON		0x03c4
+#define MTK_PERICFG_MSK_NIC_CFG_CON_CFG_MII	GENMASK(3, 0)
+#define MTK_PERICFG_BIT_NIC_CFG_CON_RMII	BIT(0)
+
+/* Represents the actual structure of descriptors used by the MAC. We can
+ * reuse the same structure for both TX and RX - the layout is the same, only
+ * the flags differ slightly.
+ */
+struct mtk_mac_ring_desc {
+	/* Contains both the status flags as well as packet length. */
+	u32 status;
+	u32 data_ptr;
+	u32 vtag;
+	u32 reserved;
+};
+
+#define MTK_MAC_DESC_MSK_LEN			GENMASK(15, 0)
+#define MTK_MAC_DESC_BIT_RX_CRCE		BIT(24)
+#define MTK_MAC_DESC_BIT_RX_OSIZE		BIT(25)
+#define MTK_MAC_DESC_BIT_INT			BIT(27)
+#define MTK_MAC_DESC_BIT_LS			BIT(28)
+#define MTK_MAC_DESC_BIT_FS			BIT(29)
+#define MTK_MAC_DESC_BIT_EOR			BIT(30)
+#define MTK_MAC_DESC_BIT_COWN			BIT(31)
+
+/* Helper structure for storing data read from/written to descriptors in order
+ * to limit reads from/writes to DMA memory.
+ */
+struct mtk_mac_ring_desc_data {
+	unsigned int len;
+	unsigned int flags;
+	dma_addr_t dma_addr;
+	struct sk_buff *skb;
+};
+
+#define MTK_MAC_RING_NUM_DESCS			128
+#define MTK_MAC_NUM_TX_DESCS			MTK_MAC_RING_NUM_DESCS
+#define MTK_MAC_NUM_RX_DESCS			MTK_MAC_RING_NUM_DESCS
+#define MTK_MAC_NUM_DESCS_TOTAL			(MTK_MAC_RING_NUM_DESCS * 2)
+#define MTK_MAC_DMA_SIZE \
+		(MTK_MAC_NUM_DESCS_TOTAL * sizeof(struct mtk_mac_ring_desc))
+
+struct mtk_mac_ring {
+	struct mtk_mac_ring_desc *descs;
+	struct sk_buff *skbs[MTK_MAC_RING_NUM_DESCS];
+	dma_addr_t dma_addrs[MTK_MAC_RING_NUM_DESCS];
+	unsigned int head;
+	unsigned int tail;
+};
+
+struct mtk_mac_priv {
+	struct net_device *ndev;
+
+	struct regmap *regs;
+	struct regmap *pericfg;
+
+	struct clk_bulk_data clks[MTK_MAC_NCLKS];
+
+	void *ring_base;
+	struct mtk_mac_ring_desc *descs_base;
+	dma_addr_t dma_addr;
+	struct mtk_mac_ring tx_ring;
+	struct mtk_mac_ring rx_ring;
+
+	struct mii_bus *mii;
+	struct napi_struct napi;
+
+	struct device_node *phy_node;
+	phy_interface_t phy_intf;
+	struct phy_device *phydev;
+	unsigned int link;
+	int speed;
+	int duplex;
+	int pause;
+
+	/* Protects against concurrent descriptor access. */
+	spinlock_t lock;
+
+	struct rtnl_link_stats64 stats;
+	struct work_struct stats_work;
+};
+
+static struct device *mtk_mac_get_dev(struct mtk_mac_priv *priv)
+{
+	return priv->ndev->dev.parent;
+}
+
+static const struct regmap_config mtk_mac_regmap_config = {
+	.reg_bits		= 32,
+	.val_bits		= 32,
+	.reg_stride		= 4,
+	.disable_locking	= true,
+};
+
+static void mtk_mac_ring_init(struct mtk_mac_ring *ring,
+			      struct mtk_mac_ring_desc *descs)
+{
+	memset(ring, 0, sizeof(*ring));
+	ring->descs = descs;
+	ring->head = 0;
+	ring->tail = 0;
+}
+
+static int mtk_mac_ring_pop_tail(struct mtk_mac_ring *ring,
+				 struct mtk_mac_ring_desc_data *desc_data)
+{
+	struct mtk_mac_ring_desc *desc = &ring->descs[ring->tail];
+	unsigned int status;
+
+	status = desc->status;
+	dma_rmb(); /* Make sure we read the status bits before checking it. */
+
+	if (!(status & MTK_MAC_DESC_BIT_COWN))
+		return -1;
+
+	desc_data->len = status & MTK_MAC_DESC_MSK_LEN;
+	desc_data->flags = status & ~MTK_MAC_DESC_MSK_LEN;
+	desc_data->dma_addr = ring->dma_addrs[ring->tail];
+	desc_data->skb = ring->skbs[ring->tail];
+
+	ring->dma_addrs[ring->tail] = 0;
+	ring->skbs[ring->tail] = NULL;
+
+	desc->data_ptr = 0;
+	desc->status = MTK_MAC_DESC_BIT_COWN;
+	if (status & MTK_MAC_DESC_BIT_EOR)
+		desc->status |= MTK_MAC_DESC_BIT_EOR;
+
+	ring->tail = (ring->tail + 1) % MTK_MAC_RING_NUM_DESCS;
+
+	return 0;
+}
+
+static void mtk_mac_ring_push_head(struct mtk_mac_ring *ring,
+				   struct mtk_mac_ring_desc_data *desc_data,
+				   unsigned int flags)
+{
+	struct mtk_mac_ring_desc *desc = &ring->descs[ring->head];
+	unsigned int status;
+
+	status = desc->status;
+
+	ring->skbs[ring->head] = desc_data->skb;
+	ring->dma_addrs[ring->head] = desc_data->dma_addr;
+	desc->data_ptr = desc_data->dma_addr;
+
+	status |= desc_data->len;
+	if (flags)
+		status |= flags;
+	desc->status = status;
+
+	/* Flush previous modifications before ownership change. */
+	dma_wmb();
+	desc->status &= ~MTK_MAC_DESC_BIT_COWN;
+
+	ring->head = (ring->head + 1) % MTK_MAC_RING_NUM_DESCS;
+}
+
+static void mtk_mac_ring_push_head_rx(struct mtk_mac_ring *ring,
+				      struct mtk_mac_ring_desc_data *desc_data)
+{
+	mtk_mac_ring_push_head(ring, desc_data, 0);
+}
+
+static void mtk_mac_ring_push_head_tx(struct mtk_mac_ring *ring,
+				      struct mtk_mac_ring_desc_data *desc_data)
+{
+	static const unsigned int flags = MTK_MAC_DESC_BIT_FS |
+					  MTK_MAC_DESC_BIT_LS |
+					  MTK_MAC_DESC_BIT_INT;
+
+	mtk_mac_ring_push_head(ring, desc_data, flags);
+}
+
+static unsigned int mtk_mac_ring_num_used_descs(struct mtk_mac_ring *ring)
+{
+	return abs(ring->head - ring->tail);
+}
+
+static bool mtk_mac_ring_full(struct mtk_mac_ring *ring)
+{
+	return mtk_mac_ring_num_used_descs(ring) == MTK_MAC_RING_NUM_DESCS;
+}
+
+static bool mtk_mac_ring_descs_available(struct mtk_mac_ring *ring)
+{
+	return mtk_mac_ring_num_used_descs(ring) > 0;
+}
+
+static void mtk_mac_lock(struct mtk_mac_priv *priv)
+{
+	spin_lock_bh(&priv->lock);
+}
+
+static void mtk_mac_unlock(struct mtk_mac_priv *priv)
+{
+	spin_unlock_bh(&priv->lock);
+}
+
+static dma_addr_t mtk_mac_dma_map_rx(struct mtk_mac_priv *priv,
+				     struct sk_buff *skb)
+{
+	struct device *dev = mtk_mac_get_dev(priv);
+
+	/* Data pointer for the RX DMA descriptor must be aligned to 4N + 2. */
+	return dma_map_single(dev, skb_tail_pointer(skb) - 2,
+			      skb_tailroom(skb), DMA_FROM_DEVICE);
+}
+
+static void mtk_mac_dma_unmap_rx(struct mtk_mac_priv *priv,
+				 struct mtk_mac_ring_desc_data *desc_data)
+{
+	struct device *dev = mtk_mac_get_dev(priv);
+
+	dma_unmap_single(dev, desc_data->dma_addr,
+			 skb_tailroom(desc_data->skb), DMA_FROM_DEVICE);
+}
+
+static dma_addr_t mtk_mac_dma_map_tx(struct mtk_mac_priv *priv,
+				     struct sk_buff *skb)
+{
+	struct device *dev = mtk_mac_get_dev(priv);
+
+	return dma_map_single(dev, skb->data, skb_headlen(skb), DMA_TO_DEVICE);
+}
+
+static void mtk_mac_dma_unmap_tx(struct mtk_mac_priv *priv,
+				 struct mtk_mac_ring_desc_data *desc_data)
+{
+	struct device *dev = mtk_mac_get_dev(priv);
+
+	return dma_unmap_single(dev, desc_data->dma_addr,
+				skb_headlen(desc_data->skb), DMA_TO_DEVICE);
+}
+
+static void mtk_mac_nic_disable_pd(struct mtk_mac_priv *priv)
+{
+	regmap_update_bits(priv->regs, MTK_MAC_REG_MAC_CFG,
+			   MTK_MAC_BIT_MAC_CFG_NIC_PD, 0);
+}
+
+/* Unmask the three interrupts we care about, mask all others. */
+static void mtk_mac_intr_enable(struct mtk_mac_priv *priv)
+{
+	unsigned int val = MTK_MAC_BIT_INT_STS_TNTC |
+			   MTK_MAC_BIT_INT_STS_FNRC |
+			   MTK_MAC_REG_INT_STS_MIB_CNT_TH;
+
+	regmap_write(priv->regs, MTK_MAC_REG_INT_MASK, ~val);
+}
+
+static void mtk_mac_intr_disable(struct mtk_mac_priv *priv)
+{
+	regmap_write(priv->regs, MTK_MAC_REG_INT_MASK, ~0);
+}
+
+static void mtk_mac_intr_enable_tx(struct mtk_mac_priv *priv)
+{
+	regmap_update_bits(priv->regs, MTK_MAC_REG_INT_MASK,
+			   MTK_MAC_BIT_INT_STS_TNTC, 0);
+}
+
+static void mtk_mac_intr_enable_rx(struct mtk_mac_priv *priv)
+{
+	regmap_update_bits(priv->regs, MTK_MAC_REG_INT_MASK,
+			   MTK_MAC_BIT_INT_STS_FNRC, 0);
+}
+
+static void mtk_mac_intr_enable_stats(struct mtk_mac_priv *priv)
+{
+	regmap_update_bits(priv->regs, MTK_MAC_REG_INT_MASK,
+			   MTK_MAC_REG_INT_STS_MIB_CNT_TH, 0);
+}
+
+static void mtk_mac_intr_disable_tx(struct mtk_mac_priv *priv)
+{
+	regmap_update_bits(priv->regs, MTK_MAC_REG_INT_MASK,
+			   MTK_MAC_BIT_INT_STS_TNTC, MTK_MAC_BIT_INT_STS_TNTC);
+}
+
+static void mtk_mac_intr_disable_rx(struct mtk_mac_priv *priv)
+{
+	regmap_update_bits(priv->regs, MTK_MAC_REG_INT_MASK,
+			   MTK_MAC_BIT_INT_STS_FNRC, MTK_MAC_BIT_INT_STS_FNRC);
+}
+
+static void mtk_mac_intr_disable_stats(struct mtk_mac_priv *priv)
+{
+	regmap_update_bits(priv->regs, MTK_MAC_REG_INT_MASK,
+			   MTK_MAC_REG_INT_STS_MIB_CNT_TH,
+			   MTK_MAC_REG_INT_STS_MIB_CNT_TH);
+}
+
+static unsigned int mtk_mac_intr_read(struct mtk_mac_priv *priv)
+{
+	unsigned int val;
+
+	regmap_read(priv->regs, MTK_MAC_REG_INT_STS, &val);
+
+	return val;
+}
+
+static unsigned int mtk_mac_intr_ack_all(struct mtk_mac_priv *priv)
+{
+	unsigned int val;
+
+	val = mtk_mac_intr_read(priv);
+	regmap_write(priv->regs, MTK_MAC_REG_INT_STS, val);
+
+	return val;
+}
+
+static void mtk_mac_dma_init(struct mtk_mac_priv *priv)
+{
+	struct mtk_mac_ring_desc *desc;
+	unsigned int val;
+	int i;
+
+	priv->descs_base = (struct mtk_mac_ring_desc *)priv->ring_base;
+
+	for (i = 0; i < MTK_MAC_NUM_DESCS_TOTAL; i++) {
+		desc = &priv->descs_base[i];
+
+		memset(desc, 0, sizeof(*desc));
+		desc->status = MTK_MAC_DESC_BIT_COWN;
+		if ((i == MTK_MAC_NUM_TX_DESCS - 1) ||
+		    (i == MTK_MAC_NUM_DESCS_TOTAL - 1))
+			desc->status |= MTK_MAC_DESC_BIT_EOR;
+	}
+
+	mtk_mac_ring_init(&priv->tx_ring, priv->descs_base);
+	mtk_mac_ring_init(&priv->rx_ring,
+			  priv->descs_base + MTK_MAC_NUM_TX_DESCS);
+
+	/* Set DMA pointers. */
+	val = (unsigned int)priv->dma_addr;
+	regmap_write(priv->regs, MTK_MAC_REG_TX_BASE_ADDR, val);
+	regmap_write(priv->regs, MTK_MAC_REG_TX_DPTR, val);
+
+	val += sizeof(struct mtk_mac_ring_desc) * MTK_MAC_NUM_TX_DESCS;
+	regmap_write(priv->regs, MTK_MAC_REG_RX_BASE_ADDR, val);
+	regmap_write(priv->regs, MTK_MAC_REG_RX_DPTR, val);
+}
+
+static void mtk_mac_dma_start(struct mtk_mac_priv *priv)
+{
+	regmap_update_bits(priv->regs, MTK_MAC_REG_TX_DMA_CTRL,
+			   MTK_MAC_BIT_TX_DMA_CTRL_START,
+			   MTK_MAC_BIT_TX_DMA_CTRL_START);
+	regmap_update_bits(priv->regs, MTK_MAC_REG_RX_DMA_CTRL,
+			   MTK_MAC_BIT_RX_DMA_CTRL_START,
+			   MTK_MAC_BIT_RX_DMA_CTRL_START);
+}
+
+static void mtk_mac_dma_stop(struct mtk_mac_priv *priv)
+{
+	regmap_write(priv->regs, MTK_MAC_REG_TX_DMA_CTRL,
+		     MTK_MAC_BIT_TX_DMA_CTRL_STOP);
+	regmap_write(priv->regs, MTK_MAC_REG_RX_DMA_CTRL,
+		     MTK_MAC_BIT_RX_DMA_CTRL_STOP);
+}
+
+static void mtk_mac_dma_disable(struct mtk_mac_priv *priv)
+{
+	int i;
+
+	mtk_mac_dma_stop(priv);
+
+	/* Take back all descriptors. */
+	for (i = 0; i < MTK_MAC_NUM_DESCS_TOTAL; i++)
+		priv->descs_base[i].status |= MTK_MAC_DESC_BIT_COWN;
+}
+
+static void mtk_mac_dma_resume_rx(struct mtk_mac_priv *priv)
+{
+	regmap_update_bits(priv->regs, MTK_MAC_REG_RX_DMA_CTRL,
+			   MTK_MAC_BIT_RX_DMA_CTRL_RESUME,
+			   MTK_MAC_BIT_RX_DMA_CTRL_RESUME);
+}
+
+static void mtk_mac_dma_resume_tx(struct mtk_mac_priv *priv)
+{
+	regmap_update_bits(priv->regs, MTK_MAC_REG_TX_DMA_CTRL,
+			   MTK_MAC_BIT_TX_DMA_CTRL_RESUME,
+			   MTK_MAC_BIT_TX_DMA_CTRL_RESUME);
+}
+
+static void mtk_mac_set_mac_addr(struct net_device *ndev)
+{
+	struct mtk_mac_priv *priv = netdev_priv(ndev);
+	u8 *mac_addr = ndev->dev_addr;
+	unsigned int high, low;
+
+	high = mac_addr[0] << 8 | mac_addr[1] << 0;
+	low = mac_addr[2] << 24 | mac_addr[3] << 16 |
+	      mac_addr[4] << 8 | mac_addr[5];
+
+	regmap_write(priv->regs, MTK_MAC_REG_MY_MAC_H, high);
+	regmap_write(priv->regs, MTK_MAC_REG_MY_MAC_L, low);
+}
+
+static void mtk_mac_reset_counters(struct mtk_mac_priv *priv)
+{
+	static const unsigned int counter_regs[] = {
+		MTK_MAC_REG_C_RXOKPKT,
+		MTK_MAC_REG_C_RXOKBYTE,
+		MTK_MAC_REG_C_RXRUNT,
+		MTK_MAC_REG_C_RXLONG,
+		MTK_MAC_REG_C_RXDROP,
+		MTK_MAC_REG_C_RXCRC,
+		MTK_MAC_REG_C_RXARLDROP,
+		MTK_MAC_REG_C_RXVLANDROP,
+		MTK_MAC_REG_C_RXCSERR,
+		MTK_MAC_REG_C_RXPAUSE,
+		MTK_MAC_REG_C_TXOKPKT,
+		MTK_MAC_REG_C_TXOKBYTE,
+		MTK_MAC_REG_C_TXPAUSECOL,
+		MTK_MAC_REG_C_TXRTY,
+		MTK_MAC_REG_C_TXSKIP,
+		MTK_MAC_REG_C_TX_ARP,
+		MTK_MAC_REG_C_RX_RERR,
+		MTK_MAC_REG_C_RX_UNI,
+		MTK_MAC_REG_C_RX_MULTI,
+		MTK_MAC_REG_C_RX_BROAD,
+		MTK_MAC_REG_C_RX_ALIGNERR,
+		MTK_MAC_REG_C_TX_UNI,
+		MTK_MAC_REG_C_TX_MULTI,
+		MTK_MAC_REG_C_TX_BROAD,
+		MTK_MAC_REG_C_TX_TIMEOUT,
+		MTK_MAC_REG_C_TX_LATECOL,
+		MTK_MAC_REG_C_RX_LENGTHERR,
+		MTK_MAC_REG_C_RX_TWIST,
+	};
+
+	unsigned int i, val;
+
+	for (i = 0; i < ARRAY_SIZE(counter_regs); i++)
+		regmap_read(priv->regs, counter_regs[i], &val);
+}
+
+static void mtk_mac_update_stat(struct mtk_mac_priv *priv,
+				unsigned int reg, u64 *stat)
+{
+	unsigned int val;
+
+	regmap_read(priv->regs, reg, &val);
+	*stat += val;
+}
+
+/* Try to get as many stats as possible from the internal registers instead
+ * of tracking them ourselves.
+ */
+static void mtk_mac_update_stats(struct mtk_mac_priv *priv)
+{
+	struct rtnl_link_stats64 *stats = &priv->stats;
+
+	/* OK packets and bytes. */
+	mtk_mac_update_stat(priv, MTK_MAC_REG_C_RXOKPKT, &stats->rx_packets);
+	mtk_mac_update_stat(priv, MTK_MAC_REG_C_TXOKPKT, &stats->tx_packets);
+	mtk_mac_update_stat(priv, MTK_MAC_REG_C_RXOKBYTE, &stats->rx_bytes);
+	mtk_mac_update_stat(priv, MTK_MAC_REG_C_TXOKBYTE, &stats->tx_bytes);
+
+	/* RX & TX multicast. */
+	mtk_mac_update_stat(priv, MTK_MAC_REG_C_RX_MULTI, &stats->multicast);
+	mtk_mac_update_stat(priv, MTK_MAC_REG_C_TX_MULTI, &stats->multicast);
+
+	/* Collisions. */
+	mtk_mac_update_stat(priv, MTK_MAC_REG_C_TXPAUSECOL, &stats->collisions);
+	mtk_mac_update_stat(priv, MTK_MAC_REG_C_TX_LATECOL, &stats->collisions);
+	mtk_mac_update_stat(priv, MTK_MAC_REG_C_RXRUNT, &stats->collisions);
+
+	/* RX Errors. */
+	mtk_mac_update_stat(priv, MTK_MAC_REG_C_RX_LENGTHERR,
+			    &stats->rx_length_errors);
+	mtk_mac_update_stat(priv, MTK_MAC_REG_C_RXLONG, &stats->rx_over_errors);
+	mtk_mac_update_stat(priv, MTK_MAC_REG_C_RXCRC, &stats->rx_crc_errors);
+	mtk_mac_update_stat(priv, MTK_MAC_REG_C_RX_ALIGNERR,
+			    &stats->rx_frame_errors);
+	mtk_mac_update_stat(priv, MTK_MAC_REG_C_RXDROP, &stats->rx_fifo_errors);
+	/* Sum of the general RX error counter + all of the above. */
+	mtk_mac_update_stat(priv, MTK_MAC_REG_C_RX_RERR, &stats->rx_errors);
+	stats->rx_errors += stats->rx_length_errors;
+	stats->rx_errors += stats->rx_over_errors;
+	stats->rx_errors += stats->rx_crc_errors;
+	stats->rx_errors += stats->rx_frame_errors;
+	stats->rx_errors += stats->rx_fifo_errors;
+}
+
+/* This runs in process context and parallel TX and RX paths executing in
+ * napi context may result in losing some stats data but this should happen
+ * seldom enough to be acceptable.
+ */
+static void mtk_mac_update_stats_work(struct work_struct *work)
+{
+	struct mtk_mac_priv *priv = container_of(work, struct mtk_mac_priv,
+						 stats_work);
+
+	mtk_mac_update_stats(priv);
+	mtk_mac_reset_counters(priv);
+	mtk_mac_intr_enable_stats(priv);
+}
+
+static struct sk_buff *mtk_mac_alloc_skb(struct net_device *ndev)
+{
+	uintptr_t tail, offset;
+	struct sk_buff *skb;
+
+	skb = dev_alloc_skb(MTK_MAC_MAX_FRAME_SIZE);
+	if (!skb)
+		return NULL;
+
+	/* Align to 16 bytes. */
+	tail = (uintptr_t)skb_tail_pointer(skb);
+	if (tail & (MTK_MAC_SKB_ALIGNMENT - 1)) {
+		offset = tail & (MTK_MAC_SKB_ALIGNMENT - 1);
+		skb_reserve(skb, MTK_MAC_SKB_ALIGNMENT - offset);
+	}
+
+	/* Ensure 16-byte alignment of the skb pointer: eth_type_trans() will
+	 * extract the Ethernet header (14 bytes) so we need two more bytes.
+	 */
+	skb_reserve(skb, MTK_MAC_IP_ALIGN);
+
+	return skb;
+}
+
+static int mtk_mac_prepare_rx_skbs(struct net_device *ndev)
+{
+	struct mtk_mac_priv *priv = netdev_priv(ndev);
+	struct mtk_mac_ring *ring = &priv->rx_ring;
+	struct device *dev = mtk_mac_get_dev(priv);
+	struct mtk_mac_ring_desc *desc;
+	struct sk_buff *skb;
+	dma_addr_t dma_addr;
+	int i;
+
+	for (i = 0; i < MTK_MAC_NUM_RX_DESCS; i++) {
+		skb = mtk_mac_alloc_skb(ndev);
+		if (!skb)
+			return -ENOMEM;
+
+		dma_addr = mtk_mac_dma_map_rx(priv, skb);
+		if (dma_mapping_error(dev, dma_addr)) {
+			dev_kfree_skb(skb);
+			return -ENOMEM;
+		}
+
+		desc = &ring->descs[i];
+		desc->data_ptr = dma_addr;
+		desc->status |= skb_tailroom(skb) & MTK_MAC_DESC_MSK_LEN;
+		desc->status &= ~MTK_MAC_DESC_BIT_COWN;
+		ring->skbs[i] = skb;
+		ring->dma_addrs[i] = dma_addr;
+	}
+
+	return 0;
+}
+
+static void
+mtk_mac_ring_free_skbs(struct mtk_mac_priv *priv, struct mtk_mac_ring *ring,
+		       void (*unmap_func)(struct mtk_mac_priv *,
+					  struct mtk_mac_ring_desc_data *))
+{
+	struct mtk_mac_ring_desc_data desc_data;
+	struct mtk_mac_ring_desc *desc;
+	int i;
+
+	for (i = 0; i < MTK_MAC_RING_NUM_DESCS; i++) {
+		if (!ring->dma_addrs[i])
+			continue;
+
+		desc = &ring->descs[i];
+
+		desc_data.dma_addr = ring->dma_addrs[i];
+		desc_data.skb = ring->skbs[i];
+
+		unmap_func(priv, &desc_data);
+		dev_kfree_skb(desc_data.skb);
+	}
+}
+
+static void mtk_mac_free_rx_skbs(struct mtk_mac_priv *priv)
+{
+	struct mtk_mac_ring *ring = &priv->rx_ring;
+
+	mtk_mac_ring_free_skbs(priv, ring, mtk_mac_dma_unmap_rx);
+}
+
+static void mtk_mac_free_tx_skbs(struct mtk_mac_priv *priv)
+{
+	struct mtk_mac_ring *ring = &priv->tx_ring;
+
+	mtk_mac_ring_free_skbs(priv, ring, mtk_mac_dma_unmap_tx);
+}
+
+/* All processing for TX and RX happens in the napi poll callback. */
+static irqreturn_t mtk_mac_handle_irq(int irq, void *data)
+{
+	struct mtk_mac_priv *priv;
+	struct net_device *ndev;
+	bool need_napi = false;
+	unsigned int status;
+
+	ndev = data;
+	priv = netdev_priv(ndev);
+
+	if (netif_running(ndev)) {
+		status = mtk_mac_intr_read(priv);
+
+		if (status & MTK_MAC_BIT_INT_STS_TNTC) {
+			mtk_mac_intr_disable_tx(priv);
+			need_napi = true;
+		}
+
+		if (status & MTK_MAC_BIT_INT_STS_FNRC) {
+			mtk_mac_intr_disable_rx(priv);
+			need_napi = true;
+		}
+
+		if (need_napi)
+			napi_schedule(&priv->napi);
+
+		/* One of the counters reached 0x8000000 - update stats and
+		 * reset all counters.
+		 */
+		if (unlikely(status & MTK_MAC_REG_INT_STS_MIB_CNT_TH)) {
+			mtk_mac_intr_disable_stats(priv);
+			schedule_work(&priv->stats_work);
+		}
+
+		mtk_mac_intr_ack_all(priv);
+	}
+
+	return IRQ_HANDLED;
+}
+
+/* Wait for the completion of any previous command - CMD_START bit must be
+ * cleared by hardware.
+ */
+static int mtk_mac_hash_wait_cmd_start(struct mtk_mac_priv *priv)
+{
+	unsigned int val;
+
+	return regmap_read_poll_timeout_atomic(priv->regs,
+				MTK_MAC_REG_HASH_CTRL, val,
+				!(val & MTK_MAC_BIT_HASH_CTRL_CMD_START),
+				10, MTK_MAC_WAIT_TIMEOUT);
+}
+
+static int mtk_mac_hash_wait_ok(struct mtk_mac_priv *priv)
+{
+	unsigned int val;
+	int ret;
+
+	/* Wait for BIST_DONE bit. */
+	ret = regmap_read_poll_timeout_atomic(priv->regs,
+					MTK_MAC_REG_HASH_CTRL, val,
+					val & MTK_MAC_BIT_HASH_CTRL_BIST_DONE,
+					10, MTK_MAC_WAIT_TIMEOUT);
+	if (ret)
+		return ret;
+
+	/* Check the BIST_OK bit. */
+	regmap_read(priv->regs, MTK_MAC_REG_HASH_CTRL, &val);
+	if (!(val & MTK_MAC_BIT_HASH_CTRL_BIST_OK))
+		return -EIO;
+
+	return 0;
+}
+
+static int mtk_mac_set_hashbit(struct mtk_mac_priv *priv,
+			       unsigned int hash_addr)
+{
+	unsigned int val;
+	int ret;
+
+	ret = mtk_mac_hash_wait_cmd_start(priv);
+	if (ret)
+		return ret;
+
+	val = hash_addr & MTK_MAC_MSK_HASH_CTRL_HASH_BIT_ADDR;
+	val |= MTK_MAC_BIT_HASH_CTRL_ACC_CMD;
+	val |= MTK_MAC_BIT_HASH_CTRL_CMD_START;
+	val |= MTK_MAC_BIT_HASH_CTRL_BIST_EN;
+	val |= MTK_MAC_BIT_HASH_CTRL_HASH_BIT_DATA;
+	regmap_write(priv->regs, MTK_MAC_REG_HASH_CTRL, val);
+
+	return mtk_mac_hash_wait_ok(priv);
+}
+
+static int mtk_mac_reset_hash_table(struct mtk_mac_priv *priv)
+{
+	int ret;
+
+	ret = mtk_mac_hash_wait_cmd_start(priv);
+	if (ret)
+		return ret;
+
+	regmap_update_bits(priv->regs, MTK_MAC_REG_HASH_CTRL,
+			   MTK_MAC_BIT_HASH_CTRL_BIST_EN,
+			   MTK_MAC_BIT_HASH_CTRL_BIST_EN);
+	regmap_update_bits(priv->regs, MTK_MAC_REG_TEST1,
+			   MTK_MAC_BIT_TEST1_RST_HASH_MBIST,
+			   MTK_MAC_BIT_TEST1_RST_HASH_MBIST);
+
+	return mtk_mac_hash_wait_ok(priv);
+}
+
+static void mtk_mac_phy_config(struct mtk_mac_priv *priv)
+{
+	unsigned int val;
+
+	if (priv->speed == SPEED_1000)
+		val = MTK_MAC_VAL_PHY_CTRL1_FORCE_SPD_1000M;
+	else if (priv->speed == SPEED_100)
+		val = MTK_MAC_VAL_PHY_CTRL1_FORCE_SPD_100M;
+	else
+		val = MTK_MAC_VAL_PHY_CTRL1_FORCE_SPD_10M;
+	val <<= MTK_MAC_OFF_PHY_CTRL1_FORCE_SPD;
+
+	val |= MTK_MAC_BIT_PHY_CTRL1_AN_EN;
+	val |= MTK_MAC_BIT_PHY_CTRL1_FORCE_FC_RX;
+	val |= MTK_MAC_BIT_PHY_CTRL1_FORCE_FC_TX;
+	/* Only full-duplex supported for now. */
+	val |= MTK_MAC_BIT_PHY_CTRL1_FORCE_DPX;
+
+	regmap_write(priv->regs, MTK_MAC_REG_PHY_CTRL1, val);
+
+	if (priv->pause) {
+		val = MTK_MAC_VAL_FC_CFG_SEND_PAUSE_TH_2K;
+		val <<= MTK_MAC_OFF_FC_CFG_SEND_PAUSE_TH;
+		val |= MTK_MAC_BIT_FC_CFG_UC_PAUSE_DIR;
+	} else {
+		val = 0;
+	}
+
+	regmap_update_bits(priv->regs, MTK_MAC_REG_FC_CFG,
+			   MTK_MAC_MSK_FC_CFG_SEND_PAUSE_TH |
+			   MTK_MAC_BIT_FC_CFG_UC_PAUSE_DIR, val);
+
+	if (priv->pause) {
+		val = MTK_MAC_VAL_EXT_CFG_SND_PAUSE_RLS_1K;
+		val <<= MTK_MAC_OFF_EXT_CFG_SND_PAUSE_RLS;
+	} else {
+		val = 0;
+	}
+
+	regmap_update_bits(priv->regs, MTK_MAC_REG_EXT_CFG,
+			   MTK_MAC_MSK_EXT_CFG_SND_PAUSE_RLS, val);
+}
+
+static void mtk_mac_adjust_link(struct net_device *ndev)
+{
+	struct mtk_mac_priv *priv = netdev_priv(ndev);
+	struct phy_device *phydev = priv->phydev;
+	bool new_state = false;
+
+	if (phydev->link) {
+		if (!priv->link) {
+			priv->link = phydev->link;
+			new_state = true;
+		}
+
+		if (priv->speed != phydev->speed) {
+			priv->speed = phydev->speed;
+			new_state = true;
+		}
+
+		if (priv->pause != phydev->pause) {
+			priv->pause = phydev->pause;
+			new_state = true;
+		}
+	} else {
+		if (priv->link) {
+			priv->link = phydev->link;
+			new_state = true;
+		}
+	}
+
+	if (new_state) {
+		if (phydev->link)
+			mtk_mac_phy_config(priv);
+
+		phy_print_status(ndev->phydev);
+	}
+}
+
+static void mtk_mac_init_config(struct mtk_mac_priv *priv)
+{
+	unsigned int val;
+
+	val = (MTK_MAC_BIT_MII_PAD_OUT_ENABLE |
+	       MTK_MAC_BIT_EXT_MDC_MODE |
+	       MTK_MAC_BIT_SWC_MII_MODE);
+
+	regmap_write(priv->regs, MTK_MAC_REG_SYS_CONF, val);
+	regmap_update_bits(priv->regs, MTK_MAC_REG_MAC_CLK_CONF,
+			   MTK_MAC_MSK_MAC_CLK_CONF,
+			   MTK_MAC_BIT_CLK_DIV_10);
+}
+
+static void mtk_mac_set_mode_rmii(struct mtk_mac_priv *priv)
+{
+	regmap_update_bits(priv->pericfg, MTK_PERICFG_REG_NIC_CFG_CON,
+			   MTK_PERICFG_MSK_NIC_CFG_CON_CFG_MII,
+			   MTK_PERICFG_BIT_NIC_CFG_CON_RMII);
+}
+
+static int mtk_mac_enable(struct net_device *ndev)
+{
+	struct mtk_mac_priv *priv = netdev_priv(ndev);
+	unsigned int val;
+	int ret;
+
+	mtk_mac_nic_disable_pd(priv);
+	mtk_mac_intr_disable(priv);
+	mtk_mac_dma_stop(priv);
+
+	mtk_mac_set_mac_addr(ndev);
+
+	/* Configure the MAC */
+	val = MTK_MAC_VAL_MAC_CFG_IPG_96BIT;
+	val <<= MTK_MAC_OFF_MAC_CFG_IPG;
+	val |= MTK_MAC_BIT_MAC_CFG_MAXLEN_1522;
+	val |= MTK_MAC_BIT_MAC_CFG_AUTO_PAD;
+	val |= MTK_MAC_BIT_MAC_CFG_CRC_STRIP;
+	regmap_write(priv->regs, MTK_MAC_REG_MAC_CFG, val);
+
+	/* Enable Hash Table BIST and reset it */
+	ret = mtk_mac_reset_hash_table(priv);
+	if (ret)
+		return ret;
+
+	/* Setup the hashing algorithm */
+	regmap_update_bits(priv->regs, MTK_MAC_REG_ARL_CFG,
+			   MTK_MAC_BIT_ARL_CFG_HASH_ALG |
+			   MTK_MAC_BIT_ARL_CFG_MISC_MODE, 0);
+
+	/* Don't strip VLAN tags */
+	regmap_update_bits(priv->regs, MTK_MAC_REG_MAC_CFG,
+			   MTK_MAC_BIT_MAC_CFG_VLAN_STRIP, 0);
+
+	/* Setup DMA */
+	mtk_mac_dma_init(priv);
+
+	ret = mtk_mac_prepare_rx_skbs(ndev);
+	if (ret)
+		goto err_out;
+
+	/* Request the interrupt */
+	ret = request_irq(ndev->irq, mtk_mac_handle_irq,
+			  IRQF_TRIGGER_FALLING, ndev->name, ndev);
+	if (ret)
+		goto err_free_skbs;
+
+	napi_enable(&priv->napi);
+
+	mtk_mac_intr_ack_all(priv);
+	mtk_mac_intr_enable(priv);
+
+	/* Connect to and start PHY */
+	priv->phydev = of_phy_connect(ndev, priv->phy_node,
+				      mtk_mac_adjust_link, 0, priv->phy_intf);
+	if (!priv->phydev) {
+		netdev_err(ndev, "failed to connect to PHY\n");
+		goto err_free_irq;
+	}
+
+	mtk_mac_dma_start(priv);
+	phy_start(priv->phydev);
+	netif_start_queue(ndev);
+
+	return 0;
+
+err_free_irq:
+	free_irq(ndev->irq, ndev);
+err_free_skbs:
+	mtk_mac_free_rx_skbs(priv);
+err_out:
+	return ret;
+}
+
+static void mtk_mac_disable(struct net_device *ndev)
+{
+	struct mtk_mac_priv *priv = netdev_priv(ndev);
+
+	netif_stop_queue(ndev);
+	napi_disable(&priv->napi);
+	mtk_mac_intr_disable(priv);
+	mtk_mac_dma_disable(priv);
+	mtk_mac_intr_ack_all(priv);
+	phy_stop(priv->phydev);
+	phy_disconnect(priv->phydev);
+	free_irq(ndev->irq, ndev);
+	mtk_mac_free_rx_skbs(priv);
+	mtk_mac_free_tx_skbs(priv);
+}
+
+static int mtk_mac_netdev_open(struct net_device *ndev)
+{
+	return mtk_mac_enable(ndev);
+}
+
+static int mtk_mac_netdev_stop(struct net_device *ndev)
+{
+	mtk_mac_disable(ndev);
+
+	return 0;
+}
+
+static int mtk_mac_netdev_ioctl(struct net_device *ndev,
+				struct ifreq *req, int cmd)
+{
+	if (!netif_running(ndev))
+		return -EINVAL;
+
+	return phy_mii_ioctl(ndev->phydev, req, cmd);
+}
+
+static int mtk_mac_netdev_start_xmit(struct sk_buff *skb,
+				     struct net_device *ndev)
+{
+	struct mtk_mac_priv *priv = netdev_priv(ndev);
+	struct mtk_mac_ring *ring = &priv->tx_ring;
+	struct device *dev = mtk_mac_get_dev(priv);
+	struct mtk_mac_ring_desc_data desc_data;
+
+	desc_data.dma_addr = mtk_mac_dma_map_tx(priv, skb);
+	if (dma_mapping_error(dev, desc_data.dma_addr))
+		goto err_drop_packet;
+
+	desc_data.skb = skb;
+	desc_data.len = skb->len;
+
+	mtk_mac_lock(priv);
+
+	mtk_mac_ring_push_head_tx(ring, &desc_data);
+
+	netdev_sent_queue(ndev, skb->len);
+
+	if (mtk_mac_ring_full(ring))
+		netif_stop_queue(ndev);
+
+	mtk_mac_unlock(priv);
+
+	mtk_mac_dma_resume_tx(priv);
+
+	return NETDEV_TX_OK;
+
+err_drop_packet:
+	dev_kfree_skb(skb);
+	ndev->stats.tx_dropped++;
+	return NETDEV_TX_BUSY;
+}
+
+/* Returns the number of bytes sent or a negative number on the first
+ * descriptor owned by DMA.
+ */
+static int mtk_mac_tx_complete_one(struct mtk_mac_priv *priv)
+{
+	struct mtk_mac_ring *ring = &priv->tx_ring;
+	struct mtk_mac_ring_desc_data desc_data;
+	int ret;
+
+	ret = mtk_mac_ring_pop_tail(ring, &desc_data);
+	if (ret)
+		return ret;
+
+	mtk_mac_dma_unmap_tx(priv, &desc_data);
+	ret = desc_data.skb->len;
+	dev_kfree_skb_irq(desc_data.skb);
+
+	return ret;
+}
+
+static void mtk_mac_tx_complete_all(struct mtk_mac_priv *priv)
+{
+	struct mtk_mac_ring *ring = &priv->tx_ring;
+	struct net_device *ndev = priv->ndev;
+	int ret, pkts_compl, bytes_compl;
+	bool wake = false;
+
+	mtk_mac_lock(priv);
+
+	for (pkts_compl = 0, bytes_compl = 0;;
+	     pkts_compl++, bytes_compl += ret, wake = true) {
+		if (!mtk_mac_ring_descs_available(ring))
+			break;
+
+		ret = mtk_mac_tx_complete_one(priv);
+		if (ret < 0)
+			break;
+	}
+
+	netdev_completed_queue(ndev, pkts_compl, bytes_compl);
+
+	if (wake && netif_queue_stopped(ndev))
+		netif_wake_queue(ndev);
+
+	mtk_mac_intr_enable_tx(priv);
+
+	mtk_mac_unlock(priv);
+}
+
+static void mtk_mac_netdev_get_stats64(struct net_device *ndev,
+				       struct rtnl_link_stats64 *stats)
+{
+	struct mtk_mac_priv *priv = netdev_priv(ndev);
+
+	mtk_mac_update_stats(priv);
+
+	memcpy(stats, &priv->stats, sizeof(*stats));
+}
+
+static void mtk_mac_set_rx_mode(struct net_device *ndev)
+{
+	struct mtk_mac_priv *priv = netdev_priv(ndev);
+	struct netdev_hw_addr *hw_addr;
+	unsigned int hash_addr, i;
+	int ret;
+
+	if (ndev->flags & IFF_PROMISC) {
+		regmap_update_bits(priv->regs, MTK_MAC_REG_ARL_CFG,
+				   MTK_MAC_BIT_ARL_CFG_MISC_MODE,
+				   MTK_MAC_BIT_ARL_CFG_MISC_MODE);
+	} else if (netdev_mc_count(ndev) > MTK_MAC_HASHTABLE_MC_LIMIT ||
+		   ndev->flags & IFF_ALLMULTI) {
+		for (i = 0; i < MTK_MAC_HASHTABLE_SIZE_MAX; i++) {
+			ret = mtk_mac_set_hashbit(priv, i);
+			if (ret)
+				goto hash_fail;
+		}
+	} else {
+		/* Clear previous settings. */
+		ret = mtk_mac_reset_hash_table(priv);
+		if (ret)
+			goto hash_fail;
+
+		netdev_for_each_mc_addr(hw_addr, ndev) {
+			hash_addr = (hw_addr->addr[0] & 0x01) << 8;
+			hash_addr += hw_addr->addr[5];
+			ret = mtk_mac_set_hashbit(priv, hash_addr);
+			if (ret)
+				goto hash_fail;
+		}
+	}
+
+	return;
+
+hash_fail:
+	if (ret == -ETIMEDOUT)
+		netdev_err(ndev, "setting hash bit timed out\n");
+	else
+		/* Should be -EIO */
+		netdev_err(ndev, "unable to set hash bit");
+}
+
+static const struct net_device_ops mtk_mac_netdev_ops = {
+	.ndo_open		= mtk_mac_netdev_open,
+	.ndo_stop		= mtk_mac_netdev_stop,
+	.ndo_start_xmit		= mtk_mac_netdev_start_xmit,
+	.ndo_get_stats64	= mtk_mac_netdev_get_stats64,
+	.ndo_set_rx_mode	= mtk_mac_set_rx_mode,
+	.ndo_do_ioctl		= mtk_mac_netdev_ioctl,
+	.ndo_set_mac_address	= eth_mac_addr,
+	.ndo_validate_addr	= eth_validate_addr,
+};
+
+static void mtk_mac_get_drvinfo(struct net_device *dev,
+				struct ethtool_drvinfo *info)
+{
+	strlcpy(info->driver, MTK_MAC_DRVNAME, sizeof(info->driver));
+}
+
+/* TODO Add ethtool stats. */
+static const struct ethtool_ops mtk_mac_ethtool_ops = {
+	.get_drvinfo		= mtk_mac_get_drvinfo,
+	.get_link		= ethtool_op_get_link,
+	.get_link_ksettings	= phy_ethtool_get_link_ksettings,
+	.set_link_ksettings	= phy_ethtool_set_link_ksettings,
+};
+
+static int mtk_mac_receive_packet(struct mtk_mac_priv *priv)
+{
+	struct mtk_mac_ring *ring = &priv->rx_ring;
+	struct device *dev = mtk_mac_get_dev(priv);
+	struct mtk_mac_ring_desc_data desc_data;
+	struct net_device *ndev = priv->ndev;
+	struct sk_buff *curr_skb, *new_skb;
+	dma_addr_t new_dma_addr;
+	int ret;
+
+	mtk_mac_lock(priv);
+	ret = mtk_mac_ring_pop_tail(ring, &desc_data);
+	mtk_mac_unlock(priv);
+	if (ret)
+		return -1;
+
+	curr_skb = desc_data.skb;
+
+	if ((desc_data.flags & MTK_MAC_DESC_BIT_RX_CRCE) ||
+	    (desc_data.flags & MTK_MAC_DESC_BIT_RX_OSIZE)) {
+		/* Error packet -> drop and reuse skb. */
+		new_skb = curr_skb;
+		goto push_new_skb;
+	}
+
+	/* Prepare new skb before receiving the current one. Reuse the current
+	 * skb if we fail at any point.
+	 */
+	new_skb = mtk_mac_alloc_skb(ndev);
+	if (!new_skb) {
+		ndev->stats.rx_dropped++;
+		new_skb = curr_skb;
+		goto push_new_skb;
+	}
+
+	new_dma_addr = mtk_mac_dma_map_rx(priv, new_skb);
+	if (dma_mapping_error(dev, new_dma_addr)) {
+		ndev->stats.rx_dropped++;
+		dev_kfree_skb(new_skb);
+		new_skb = curr_skb;
+		netdev_err(ndev, "DMA mapping error of RX descriptor\n");
+		goto push_new_skb;
+	}
+
+	/* We can't fail anymore at this point: it's safe to unmap the skb. */
+	mtk_mac_dma_unmap_rx(priv, &desc_data);
+
+	skb_put(desc_data.skb, desc_data.len);
+	desc_data.skb->ip_summed = CHECKSUM_NONE;
+	desc_data.skb->protocol = eth_type_trans(desc_data.skb, ndev);
+	desc_data.skb->dev = ndev;
+	netif_receive_skb(desc_data.skb);
+
+push_new_skb:
+	desc_data.dma_addr = new_dma_addr;
+	desc_data.len = skb_tailroom(new_skb);
+	desc_data.skb = new_skb;
+
+	mtk_mac_lock(priv);
+	mtk_mac_ring_push_head_rx(ring, &desc_data);
+	mtk_mac_unlock(priv);
+
+	return 0;
+}
+
+static int mtk_mac_process_rx(struct mtk_mac_priv *priv, int budget)
+{
+	int received, ret;
+
+	for (received = 0, ret = 0; received < budget && ret == 0; received++)
+		ret = mtk_mac_receive_packet(priv);
+
+	mtk_mac_dma_resume_rx(priv);
+
+	return received;
+}
+
+static int mtk_mac_poll(struct napi_struct *napi, int budget)
+{
+	struct mtk_mac_priv *priv;
+	int received = 0;
+
+	priv = container_of(napi, struct mtk_mac_priv, napi);
+
+	/* Clean-up all TX descriptors. */
+	mtk_mac_tx_complete_all(priv);
+	/* Receive up to $budget packets. */
+	received = mtk_mac_process_rx(priv, budget);
+
+	if (received < budget) {
+		napi_complete_done(napi, received);
+		mtk_mac_intr_enable_rx(priv);
+	}
+
+	return received;
+}
+
+static void mtk_mac_mdio_rwok_clear(struct mtk_mac_priv *priv)
+{
+	regmap_write(priv->regs, MTK_MAC_REG_PHY_CTRL0,
+		     MTK_MAC_BIT_PHY_CTRL0_RWOK);
+}
+
+static int mtk_mac_mdio_rwok_wait(struct mtk_mac_priv *priv)
+{
+	unsigned int val;
+
+	return regmap_read_poll_timeout(priv->regs, MTK_MAC_REG_PHY_CTRL0,
+					val, val & MTK_MAC_BIT_PHY_CTRL0_RWOK,
+					10, MTK_MAC_WAIT_TIMEOUT);
+}
+
+static int mtk_mac_mdio_read(struct mii_bus *mii, int phy_id, int regnum)
+{
+	struct mtk_mac_priv *priv = mii->priv;
+	unsigned int val, data;
+	int ret;
+
+	if (regnum & MII_ADDR_C45)
+		return -EOPNOTSUPP;
+
+	mtk_mac_mdio_rwok_clear(priv);
+
+	val = (regnum << MTK_MAC_OFF_PHY_CTRL0_PREG);
+	val &= MTK_MAC_MSK_PHY_CTRL0_PREG;
+	val |= MTK_MAC_BIT_PHY_CTRL0_RDCMD;
+
+	regmap_write(priv->regs, MTK_MAC_REG_PHY_CTRL0, val);
+
+	ret = mtk_mac_mdio_rwok_wait(priv);
+	if (ret)
+		return ret;
+
+	regmap_read(priv->regs, MTK_MAC_REG_PHY_CTRL0, &data);
+
+	data &= MTK_MAC_MSK_PHY_CTRL0_RWDATA;
+	data >>= MTK_MAC_OFF_PHY_CTRL0_RWDATA;
+
+	return data;
+}
+
+static int mtk_mac_mdio_write(struct mii_bus *mii, int phy_id,
+			      int regnum, u16 data)
+{
+	struct mtk_mac_priv *priv = mii->priv;
+	unsigned int val;
+
+	if (regnum & MII_ADDR_C45)
+		return -EOPNOTSUPP;
+
+	mtk_mac_mdio_rwok_clear(priv);
+
+	val = data;
+	val <<= MTK_MAC_OFF_PHY_CTRL0_RWDATA;
+	val &= MTK_MAC_MSK_PHY_CTRL0_RWDATA;
+	regnum <<= MTK_MAC_OFF_PHY_CTRL0_PREG;
+	regnum &= MTK_MAC_MSK_PHY_CTRL0_PREG;
+	val |= regnum;
+	val |= MTK_MAC_BIT_PHY_CTRL0_WTCMD;
+
+	regmap_write(priv->regs, MTK_MAC_REG_PHY_CTRL0, val);
+
+	return mtk_mac_mdio_rwok_wait(priv);
+}
+
+static int mtk_mac_mdio_init(struct net_device *ndev)
+{
+	struct mtk_mac_priv *priv = netdev_priv(ndev);
+	struct device *dev = mtk_mac_get_dev(priv);
+	struct device_node *of_node, *mdio_node;
+	int ret;
+
+	of_node = dev->of_node;
+
+	mdio_node = of_get_child_by_name(of_node, "mdio");
+	if (!mdio_node)
+		return -ENODEV;
+
+	if (!of_device_is_available(mdio_node)) {
+		ret = -ENODEV;
+		goto out_put_node;
+	}
+
+	priv->mii = devm_mdiobus_alloc(dev);
+	if (!priv->mii) {
+		ret = -ENOMEM;
+		goto out_put_node;
+	}
+
+	snprintf(priv->mii->id, MII_BUS_ID_SIZE, "%s", dev_name(dev));
+	priv->mii->name = "mtk-mac-mdio";
+	priv->mii->parent = dev;
+	priv->mii->read = mtk_mac_mdio_read;
+	priv->mii->write = mtk_mac_mdio_write;
+	priv->mii->priv = priv;
+
+	ret = of_mdiobus_register(priv->mii, mdio_node);
+
+out_put_node:
+	of_node_put(mdio_node);
+	return ret;
+}
+
+static int mtk_mac_suspend(struct device *dev)
+{
+	struct mtk_mac_priv *priv;
+	struct net_device *ndev;
+
+	ndev = dev_get_drvdata(dev);
+	priv = netdev_priv(ndev);
+
+	if (netif_running(ndev))
+		mtk_mac_disable(ndev);
+
+	clk_bulk_disable_unprepare(MTK_MAC_NCLKS, priv->clks);
+
+	return 0;
+}
+
+static int mtk_mac_resume(struct device *dev)
+{
+	struct mtk_mac_priv *priv;
+	struct net_device *ndev;
+	int ret;
+
+	ndev = dev_get_drvdata(dev);
+	priv = netdev_priv(ndev);
+
+	ret = clk_bulk_prepare_enable(MTK_MAC_NCLKS, priv->clks);
+	if (ret)
+		return ret;
+
+	if (netif_running(ndev)) {
+		ret = mtk_mac_enable(ndev);
+		if (ret)
+			clk_bulk_disable_unprepare(MTK_MAC_NCLKS, priv->clks);
+	}
+
+	return ret;
+}
+
+static void mtk_mac_clk_disable_unprepare(void *data)
+{
+	struct mtk_mac_priv *priv = data;
+
+	clk_bulk_disable_unprepare(MTK_MAC_NCLKS, priv->clks);
+}
+
+static void mtk_mac_unregister_netdev(void *data)
+{
+	struct net_device *ndev = data;
+
+	unregister_netdev(ndev);
+}
+
+static int mtk_mac_probe(struct platform_device *pdev)
+{
+	struct device_node *of_node;
+	struct mtk_mac_priv *priv;
+	struct net_device *ndev;
+	struct device *dev;
+	void __iomem *base;
+	int ret, i;
+
+	dev = &pdev->dev;
+	of_node = dev->of_node;
+
+	ndev = devm_alloc_etherdev(dev, sizeof(*priv));
+	if (!ndev)
+		return -ENOMEM;
+
+	priv = netdev_priv(ndev);
+	priv->ndev = ndev;
+	SET_NETDEV_DEV(ndev, dev);
+	platform_set_drvdata(pdev, ndev);
+
+	ndev->min_mtu = ETH_ZLEN;
+	ndev->max_mtu = MTK_MAC_MAX_FRAME_SIZE;
+
+	spin_lock_init(&priv->lock);
+	INIT_WORK(&priv->stats_work, mtk_mac_update_stats_work);
+
+	base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	/* We won't be checking the return values of regmap read & write
+	 * functions. They can only fail for mmio if there's a clock attached
+	 * to regmap which is not the case here.
+	 */
+	priv->regs = devm_regmap_init_mmio(dev, base,
+					   &mtk_mac_regmap_config);
+	if (IS_ERR(priv->regs))
+		return PTR_ERR(priv->regs);
+
+	priv->pericfg = syscon_regmap_lookup_by_phandle(of_node,
+							"mediatek,pericfg");
+	if (IS_ERR(priv->pericfg)) {
+		dev_err(dev, "Failed to lookup the PERICFG syscon\n");
+		return PTR_ERR(priv->pericfg);
+	}
+
+	ndev->irq = platform_get_irq(pdev, 0);
+	if (ndev->irq < 0)
+		return ndev->irq;
+
+	for (i = 0; i < MTK_MAC_NCLKS; i++)
+		priv->clks[i].id = mtk_mac_clk_names[i];
+	ret = devm_clk_bulk_get(dev, MTK_MAC_NCLKS, priv->clks);
+	if (ret)
+		return ret;
+
+	ret = clk_bulk_prepare_enable(MTK_MAC_NCLKS, priv->clks);
+	if (ret)
+		return ret;
+
+	ret = devm_add_action_or_reset(dev,
+				       mtk_mac_clk_disable_unprepare, priv);
+	if (ret)
+		return ret;
+
+	ret = of_get_phy_mode(of_node, &priv->phy_intf);
+	if (ret) {
+		return ret;
+	} else if (priv->phy_intf != PHY_INTERFACE_MODE_RMII) {
+		dev_err(dev, "unsupported phy mode: %s\n",
+			phy_modes(priv->phy_intf));
+		return -EINVAL;
+	}
+
+	priv->phy_node = of_parse_phandle(of_node, "phy-handle", 0);
+	if (!priv->phy_node) {
+		dev_err(dev, "failed to retrieve the phy handle from device tree\n");
+		return -ENODEV;
+	}
+
+	mtk_mac_set_mode_rmii(priv);
+
+	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
+	if (ret) {
+		dev_err(dev, "unsupported DMA mask\n");
+		return ret;
+	}
+
+	priv->ring_base = dmam_alloc_coherent(dev, MTK_MAC_DMA_SIZE,
+					      &priv->dma_addr,
+					      GFP_KERNEL | GFP_DMA);
+	if (!priv->ring_base)
+		return -ENOMEM;
+
+	mtk_mac_nic_disable_pd(priv);
+	mtk_mac_init_config(priv);
+
+	ret = mtk_mac_mdio_init(ndev);
+	if (ret)
+		return ret;
+
+	ret = eth_platform_get_mac_address(dev, ndev->dev_addr);
+	if (ret || !is_valid_ether_addr(ndev->dev_addr))
+		eth_hw_addr_random(ndev);
+
+	ndev->netdev_ops = &mtk_mac_netdev_ops;
+	ndev->ethtool_ops = &mtk_mac_ethtool_ops;
+
+	netif_napi_add(ndev, &priv->napi, mtk_mac_poll, MTK_MAC_NAPI_WEIGHT);
+
+	ret = register_netdev(ndev);
+	if (ret)
+		return ret;
+
+	ret = devm_add_action_or_reset(dev, mtk_mac_unregister_netdev, ndev);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static const struct of_device_id mtk_mac_of_match[] = {
+	{ .compatible = "mediatek,mt8516-eth", },
+	{ .compatible = "mediatek,mt8518-eth", },
+	{ .compatible = "mediatek,mt8175-eth", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, mtk_mac_of_match);
+
+static SIMPLE_DEV_PM_OPS(mtk_mac_pm_ops,
+			 mtk_mac_suspend, mtk_mac_resume);
+
+static struct platform_driver mtk_mac_driver = {
+	.driver = {
+		.name = MTK_MAC_DRVNAME,
+		.pm = &mtk_mac_pm_ops,
+		.of_match_table = of_match_ptr(mtk_mac_of_match),
+	},
+	.probe = mtk_mac_probe,
+};
+module_platform_driver(mtk_mac_driver);
+
+MODULE_AUTHOR("Bartosz Golaszewski <bgolaszewski@baylibre.com>");
+MODULE_DESCRIPTION("Mediatek Ethernet MAC Driver");
+MODULE_LICENSE("GPL");
-- 
2.25.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v4 07/11] ARM64: dts: mediatek: add pericfg syscon to mt8516.dtsi
From: Bartosz Golaszewski @ 2020-05-20 11:25 UTC (permalink / raw)
  To: Rob Herring, David S . Miller, Matthias Brugger, John Crispin,
	Sean Wang, Mark Lee, Jakub Kicinski, Arnd Bergmann, Fabien Parent,
	Heiner Kallweit, Edwin Peer
  Cc: devicetree, Stephane Le Provost, netdev, linux-kernel,
	Bartosz Golaszewski, linux-mediatek, Andrew Perepech, Pedro Tsai,
	linux-arm-kernel
In-Reply-To: <20200520112523.30995-1-brgl@bgdev.pl>

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

This adds support for the PERICFG register range as a syscon. This will
soon be used by the MediaTek Ethernet MAC driver for NIC configuration.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 arch/arm64/boot/dts/mediatek/mt8516.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8516.dtsi b/arch/arm64/boot/dts/mediatek/mt8516.dtsi
index 2f8adf042195..8cedaf74ae86 100644
--- a/arch/arm64/boot/dts/mediatek/mt8516.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8516.dtsi
@@ -191,6 +191,11 @@ infracfg: infracfg@10001000 {
 			#clock-cells = <1>;
 		};
 
+		pericfg: pericfg@10003050 {
+			compatible = "mediatek,mt8516-pericfg", "syscon";
+			reg = <0 0x10003050 0 0x1000>;
+		};
+
 		apmixedsys: apmixedsys@10018000 {
 			compatible = "mediatek,mt8516-apmixedsys", "syscon";
 			reg = <0 0x10018000 0 0x710>;
-- 
2.25.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v4 09/11] ARM64: dts: mediatek: add an alias for ethernet0 for pumpkin boards
From: Bartosz Golaszewski @ 2020-05-20 11:25 UTC (permalink / raw)
  To: Rob Herring, David S . Miller, Matthias Brugger, John Crispin,
	Sean Wang, Mark Lee, Jakub Kicinski, Arnd Bergmann, Fabien Parent,
	Heiner Kallweit, Edwin Peer
  Cc: devicetree, Stephane Le Provost, netdev, linux-kernel,
	Bartosz Golaszewski, linux-mediatek, Andrew Perepech, Pedro Tsai,
	linux-arm-kernel
In-Reply-To: <20200520112523.30995-1-brgl@bgdev.pl>

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Add the ethernet0 alias for ethernet so that u-boot can find this node
and fill in the MAC address.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 arch/arm64/boot/dts/mediatek/pumpkin-common.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/mediatek/pumpkin-common.dtsi b/arch/arm64/boot/dts/mediatek/pumpkin-common.dtsi
index a31093d7142b..97d9b000c37e 100644
--- a/arch/arm64/boot/dts/mediatek/pumpkin-common.dtsi
+++ b/arch/arm64/boot/dts/mediatek/pumpkin-common.dtsi
@@ -9,6 +9,7 @@
 / {
 	aliases {
 		serial0 = &uart0;
+		ethernet0 = &ethernet;
 	};
 
 	chosen {
-- 
2.25.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v4 08/11] ARM64: dts: mediatek: add the ethernet node to mt8516.dtsi
From: Bartosz Golaszewski @ 2020-05-20 11:25 UTC (permalink / raw)
  To: Rob Herring, David S . Miller, Matthias Brugger, John Crispin,
	Sean Wang, Mark Lee, Jakub Kicinski, Arnd Bergmann, Fabien Parent,
	Heiner Kallweit, Edwin Peer
  Cc: devicetree, Stephane Le Provost, netdev, linux-kernel,
	Bartosz Golaszewski, linux-mediatek, Andrew Perepech, Pedro Tsai,
	linux-arm-kernel
In-Reply-To: <20200520112523.30995-1-brgl@bgdev.pl>

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Add the Ethernet MAC node to mt8516.dtsi. This defines parameters common
to all the boards based on this SoC.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 arch/arm64/boot/dts/mediatek/mt8516.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8516.dtsi b/arch/arm64/boot/dts/mediatek/mt8516.dtsi
index 8cedaf74ae86..89af661e7f63 100644
--- a/arch/arm64/boot/dts/mediatek/mt8516.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8516.dtsi
@@ -406,6 +406,18 @@ mmc2: mmc@11170000 {
 			status = "disabled";
 		};
 
+		ethernet: ethernet@11180000 {
+			compatible = "mediatek,mt8516-eth";
+			reg = <0 0x11180000 0 0x1000>;
+			mediatek,pericfg = <&pericfg>;
+			interrupts = <GIC_SPI 111 IRQ_TYPE_LEVEL_LOW>;
+			clocks = <&topckgen CLK_TOP_RG_ETH>,
+				 <&topckgen CLK_TOP_66M_ETH>,
+				 <&topckgen CLK_TOP_133M_ETH>;
+			clock-names = "core", "reg", "trans";
+			status = "disabled";
+		};
+
 		rng: rng@1020c000 {
 			compatible = "mediatek,mt8516-rng",
 				     "mediatek,mt7623-rng";
-- 
2.25.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v4 11/11] ARM64: dts: mediatek: enable ethernet on pumpkin boards
From: Bartosz Golaszewski @ 2020-05-20 11:25 UTC (permalink / raw)
  To: Rob Herring, David S . Miller, Matthias Brugger, John Crispin,
	Sean Wang, Mark Lee, Jakub Kicinski, Arnd Bergmann, Fabien Parent,
	Heiner Kallweit, Edwin Peer
  Cc: devicetree, Stephane Le Provost, netdev, linux-kernel,
	Bartosz Golaszewski, linux-mediatek, Andrew Perepech, Pedro Tsai,
	linux-arm-kernel
In-Reply-To: <20200520112523.30995-1-brgl@bgdev.pl>

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Add remaining properties to the ethernet node and enable it.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 .../boot/dts/mediatek/pumpkin-common.dtsi      | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/pumpkin-common.dtsi b/arch/arm64/boot/dts/mediatek/pumpkin-common.dtsi
index 4b1d5f69aba6..dfceffe6950a 100644
--- a/arch/arm64/boot/dts/mediatek/pumpkin-common.dtsi
+++ b/arch/arm64/boot/dts/mediatek/pumpkin-common.dtsi
@@ -167,6 +167,24 @@ &uart0 {
 	status = "okay";
 };
 
+&ethernet {
+	pinctrl-names = "default";
+	pinctrl-0 = <&ethernet_pins_default>;
+	phy-handle = <&eth_phy>;
+	phy-mode = "rmii";
+	mac-address = [00 00 00 00 00 00];
+	status = "okay";
+
+	mdio {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		eth_phy: ethernet-phy@0 {
+			reg = <0>;
+		};
+	};
+};
+
 &usb0 {
 	status = "okay";
 	dr_mode = "peripheral";
-- 
2.25.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v4 10/11] ARM64: dts: mediatek: add ethernet pins for pumpkin boards
From: Bartosz Golaszewski @ 2020-05-20 11:25 UTC (permalink / raw)
  To: Rob Herring, David S . Miller, Matthias Brugger, John Crispin,
	Sean Wang, Mark Lee, Jakub Kicinski, Arnd Bergmann, Fabien Parent,
	Heiner Kallweit, Edwin Peer
  Cc: devicetree, Stephane Le Provost, netdev, linux-kernel,
	Bartosz Golaszewski, linux-mediatek, Andrew Perepech, Pedro Tsai,
	linux-arm-kernel
In-Reply-To: <20200520112523.30995-1-brgl@bgdev.pl>

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Setup the pin control for the Ethernet MAC.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 arch/arm64/boot/dts/mediatek/pumpkin-common.dtsi | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/pumpkin-common.dtsi b/arch/arm64/boot/dts/mediatek/pumpkin-common.dtsi
index 97d9b000c37e..4b1d5f69aba6 100644
--- a/arch/arm64/boot/dts/mediatek/pumpkin-common.dtsi
+++ b/arch/arm64/boot/dts/mediatek/pumpkin-common.dtsi
@@ -219,4 +219,19 @@ gpio_mux_int_n_pin {
 			bias-pull-up;
 		};
 	};
+
+	ethernet_pins_default: ethernet {
+		pins_ethernet {
+			pinmux = <MT8516_PIN_0_EINT0__FUNC_EXT_TXD0>,
+				 <MT8516_PIN_1_EINT1__FUNC_EXT_TXD1>,
+				 <MT8516_PIN_5_EINT5__FUNC_EXT_RXER>,
+				 <MT8516_PIN_6_EINT6__FUNC_EXT_RXC>,
+				 <MT8516_PIN_7_EINT7__FUNC_EXT_RXDV>,
+				 <MT8516_PIN_8_EINT8__FUNC_EXT_RXD0>,
+				 <MT8516_PIN_9_EINT9__FUNC_EXT_RXD1>,
+				 <MT8516_PIN_12_EINT12__FUNC_EXT_TXEN>,
+				 <MT8516_PIN_38_MRG_DI__FUNC_EXT_MDIO>,
+				 <MT8516_PIN_39_MRG_DO__FUNC_EXT_MDC>;
+		};
+	};
 };
-- 
2.25.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: BUG: sleeping function called from atomic due to "Balance initial LPI affinity across CPUs"
From: John Garry @ 2020-05-20 11:28 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Stephen Rothwell, Linux Kernel Mailing List,
	Linux Next Mailing List, Qian Cai, Thomas Gleixner, Linux ARM
In-Reply-To: <e07d73938a7534c6c2cd0517de378fcd@kernel.org>

On 20/05/2020 10:51, Marc Zyngier wrote:
> Hi John,
> 
> On 2020-05-20 09:43, John Garry wrote:
>> On 19/05/2020 23:09, Qian Cai wrote:
>>> Reverted the linux-next commit f068a62c548c ("irqchip/gic-v3-its:
>>> Balance initial LPI affinity across CPUs") fixed these warnings during
>>> boot,
>>
>> Thanks for the notice. So we need the following set to see this:
>> CONFIG_CPUMASK_OFFSTACK=y
>> CONFIG_DEBUG_ATOMIC_SLEEP=y
>> CONFIG_DEBUG_PER_CPU_MAPS=y
> 
> Ah, thanks for pointing this out.
> 
>>> its_select_cpu at drivers/irqchip/irq-gic-v3-its.c:1572
>>>
>>> [  332.819381][ T3359] BUG: sleeping function called from invalid
>>> context at mm/slab.h:568
>>> [  332.827405][ T3359] in_atomic(): 1, irqs_disabled(): 128,
>>> non_block: 0, pid: 3359, name: irqbalance
>>> [  332.836455][ T3359] INFO: lockdep is turned off.
>>> [  332.841076][ T3359] irq event stamp: 0
>>> [  332.844836][ T3359] hardirqs last  enabled at (0): 
>>> [<0000000000000000>] 0x0
>>> [  332.851828][ T3359] hardirqs last disabled at (0):
>>> [<ffff9000101ea65c>] copy_process+0x98c/0x1f34
>>> [  332.860710][ T3359] softirqs last  enabled at (0):
>>> [<ffff9000101ea690>] copy_process+0x9c0/0x1f34
>>> [  332.869586][ T3359] softirqs last disabled at (0): 
>>> [<0000000000000000>] 0x0
>>> [  332.876560][ T3359] CPU: 155 PID: 3359 Comm: irqbalance Tainted: G
>>>        W    L    5.7.0-rc6-next-20200519 #1
>>> [  332.886563][ T3359] Hardware name: HPE Apollo 70
>>> /C01_APACHE_MB         , BIOS L50_5.13_1.11 06/18/2019
>>> [  332.897000][ T3359] Call trace:
>>> [  332.900151][ T3359]  dump_backtrace+0x0/0x22c
>>> [  332.904514][ T3359]  show_stack+0x28/0x34
>>> [  332.908543][ T3359]  dump_stack+0x104/0x194
>>> [  332.912738][ T3359]  ___might_sleep+0x314/0x328
>>> [  332.917274][ T3359]  __might_sleep+0x7c/0xe0
>>> [  332.921563][ T3359]  slab_pre_alloc_hook+0x44/0x8c
>>> [  332.926360][ T3359]  __kmalloc_node+0xb0/0x618
>>> [  332.930811][ T3359]  alloc_cpumask_var_node+0x48/0x94
>>
>> We could use GFP_ATOMIC flag at the callsite here, but maybe there is
>> a better solution.
> 
> I don't see one, and I doubt it is worth the hassle to have anything
> but GFP_ATOMIC. The default arm64 config is to have on-stack cpumasks,
> and only DEBUG_PER_CPU_MAPS allows this to be changed.

JFYI, I was able to recreate on my D05:

[11.951739] CPU: 21 PID: 1 Comm: swapper/0 Tainted: G 
W5.7.0-rc6-next-20200519-dirty #16
[11.961262] Hardware name: Huawei Taishan 2280 /D05, BIOS Hisilicon D05 
IT21 Nemo 2.0 RC0 04/18/2018
[11.970435] Call trace:
[11.972890]  dump_backtrace+0x0/0x1b0
[11.976561]  show_stack+0x14/0x1c
[11.979884]  dump_stack+0xc0/0xf0
[11.983207]  ___might_sleep+0x10c/0x12c
[11.987052]  __might_sleep+0x4c/0x80
[11.990637]  __kmalloc_node+0x1ac/0x2dc
[11.994483]  alloc_cpumask_var_node+0x28/0x60
[11.998852]  alloc_cpumask_var+0x10/0x18
[12.002787]  its_select_cpu+0x2c/0x19c
[12.006546]  its_irq_domain_activate+0x38/0xf0
[12.011004]  __irq_domain_activate_irq+0x6c/0xac
[12.011108] sd 0:0:7:0: [sdh] Attached SCSI disk
[12.015636]  __irq_domain_activate_irq+0x34/0xac
[12.015638]  __irq_domain_activate_irq+0x34/0xac
[12.015640]  irq_domain_activate_irq+0x3c/0x4c
[12.015643]  irq_activate+0x20/0x30
[12.037482]  __setup_irq+0x538/0x6ec
[12.041065]  request_threaded_irq+0xdc/0x18c
[12.045350]  usb_add_hcd+0x4b0/0x6c0
[12.048934]  ehci_platform_probe+0x1f4/0x438
[12.053218]  platform_drv_probe+0x4c/0xa0
[12.057239]  really_probe+0xf4/0x35c
[12.060823]  driver_probe_device+0x54/0xb0
[12.064931]  device_driver_attach+0x68/0x70
[12.069126]  __driver_attach+0x9c/0xf8
[12.072884]  bus_for_each_dev+0x50/0xa0
[12.076730]  driver_attach+0x20/0x28
[12.080313]  bus_add_driver+0x148/0x1fc
[12.084159]  driver_register+0x6c/0x124
[12.088005]  __platform_driver_register+0x48/0x50
[12.092727]  ehci_platform_init+0x54/0x64
[12.096747]  do_one_initcall+0x50/0x194
[12.100593]  kernel_init_freeable+0x1e4/0x250
[12.104964]  kernel_init+0x10/0x108
[12.108460]  ret_from_fork+0x10/0x18
[12.112077] ehci-platform PNP0D20:00: irq 361, io mem 0xa7020000

And setting GFP_ATOMIC removes that BUG.

Cheers,
John


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v11 2/3] i2c: npcm7xx: Add Nuvoton NPCM I2C controller driver
From: Avi Fishman @ 2020-05-20 11:37 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: devicetree, Linux Kernel Mailing List, Tomer Maimon, Nancy Yuen,
	Patrick Venture, OpenBMC Maillist, wsa, Brendan Higgins, ofery,
	Tali Perry, kfting, Rob Herring, linux-i2c, Linux ARM,
	Benjamin Fair
In-Reply-To: <20200520102452.GP1634618@smile.fi.intel.com>

Thanks Andy,
Question below:

On Wed, May 20, 2020 at 1:24 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Wed, May 20, 2020 at 12:51:12PM +0300, Tali Perry wrote:
> > Add Nuvoton NPCM BMC I2C controller driver.
>
> ...
>
> > +#ifdef CONFIG_DEBUG_FS
>
> Why?!

It is made to save code size if CONFIG_DEBUG_FS is not defined?
We see a lot of kernel code that is doing it.
So could you elaborate what is the problem?

>
> > +#include <linux/debugfs.h>
> > +#endif
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH 0/5] net: provide a devres variant of register_netdev()
From: Bartosz Golaszewski @ 2020-05-20 11:44 UTC (permalink / raw)
  To: Jonathan Corbet, David S . Miller, Matthias Brugger, John Crispin,
	Sean Wang, Mark Lee, Jakub Kicinski, Arnd Bergmann, Fabien Parent,
	Heiner Kallweit, Edwin Peer
  Cc: devicetree, Stephane Le Provost, netdev, linux-kernel,
	Bartosz Golaszewski, linux-mediatek, Andrew Perepech, Pedro Tsai,
	linux-arm-kernel

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

This series applies on top of my mtk-eth-mac series[1].

Using devres helpers allows to shrink the probing code, avoid memory leaks in
error paths make sure the order in which resources are freed is the exact
opposite of their allocation. This series proposes to add a devres variant
of register_netdev() that will only work with net_device structures whose
memory is also managed.

First we add the missing documentation entry for the only other networking
devres helper: devm_alloc_etherdev().

Next we move devm_alloc_etherdev() into a separate source file.

We then use a proxy structure in devm_alloc_etherdev() to improve readability.

Last: we implement devm_register_netdev() and use it in mtk-eth-mac driver.

[1] https://lkml.org/lkml/2020/5/20/507

Bartosz Golaszewski (5):
  Documentation: devres: add a missing section for networking helpers
  net: move devres helpers into a separate source file
  net: devres: define a separate devres structure for
    devm_alloc_etherdev()
  net: devres: provide devm_register_netdev()
  net: ethernet: mtk_eth_mac: use devm_register_netdev()

 .../driver-api/driver-model/devres.rst        |  5 +
 drivers/net/ethernet/mediatek/mtk_eth_mac.c   | 17 +---
 include/linux/netdevice.h                     |  2 +
 net/Makefile                                  |  2 +-
 net/devres.c                                  | 95 +++++++++++++++++++
 net/ethernet/eth.c                            | 28 ------
 6 files changed, 104 insertions(+), 45 deletions(-)
 create mode 100644 net/devres.c

-- 
2.25.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH 1/5] Documentation: devres: add a missing section for networking helpers
From: Bartosz Golaszewski @ 2020-05-20 11:44 UTC (permalink / raw)
  To: Jonathan Corbet, David S . Miller, Matthias Brugger, John Crispin,
	Sean Wang, Mark Lee, Jakub Kicinski, Arnd Bergmann, Fabien Parent,
	Heiner Kallweit, Edwin Peer
  Cc: devicetree, Stephane Le Provost, netdev, linux-kernel,
	Bartosz Golaszewski, linux-mediatek, Andrew Perepech, Pedro Tsai,
	linux-arm-kernel
In-Reply-To: <20200520114415.13041-1-brgl@bgdev.pl>

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Add a new section for networking devres helpers to devres.rst and list
the two existing devm functions.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 Documentation/driver-api/driver-model/devres.rst | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/driver-api/driver-model/devres.rst b/Documentation/driver-api/driver-model/devres.rst
index 46c13780994c..50df28d20fa7 100644
--- a/Documentation/driver-api/driver-model/devres.rst
+++ b/Documentation/driver-api/driver-model/devres.rst
@@ -372,6 +372,10 @@ MUX
   devm_mux_chip_register()
   devm_mux_control_get()
 
+NET
+  devm_alloc_etherdev()
+  devm_alloc_etherdev_mqs()
+
 PER-CPU MEM
   devm_alloc_percpu()
   devm_free_percpu()
-- 
2.25.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH 2/5] net: move devres helpers into a separate source file
From: Bartosz Golaszewski @ 2020-05-20 11:44 UTC (permalink / raw)
  To: Jonathan Corbet, David S . Miller, Matthias Brugger, John Crispin,
	Sean Wang, Mark Lee, Jakub Kicinski, Arnd Bergmann, Fabien Parent,
	Heiner Kallweit, Edwin Peer
  Cc: devicetree, Stephane Le Provost, netdev, linux-kernel,
	Bartosz Golaszewski, linux-mediatek, Andrew Perepech, Pedro Tsai,
	linux-arm-kernel
In-Reply-To: <20200520114415.13041-1-brgl@bgdev.pl>

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

There's currently only a single devres helper in net/ - devm variant
of alloc_etherdev. Let's move it to net/devres.c with the intention of
assing a second one: devm_register_netdev(). This new routine will need
to know the address of the release function of devm_alloc_etherdev() so
that it can verify (using devres_find()) that the struct net_device
that's being passed to it is also resource managed.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 net/Makefile       |  2 +-
 net/devres.c       | 36 ++++++++++++++++++++++++++++++++++++
 net/ethernet/eth.c | 28 ----------------------------
 3 files changed, 37 insertions(+), 29 deletions(-)
 create mode 100644 net/devres.c

diff --git a/net/Makefile b/net/Makefile
index 07ea48160874..5744bf1997fd 100644
--- a/net/Makefile
+++ b/net/Makefile
@@ -6,7 +6,7 @@
 # Rewritten to use lists instead of if-statements.
 #
 
-obj-$(CONFIG_NET)		:= socket.o core/
+obj-$(CONFIG_NET)		:= devres.o socket.o core/
 
 tmp-$(CONFIG_COMPAT) 		:= compat.o
 obj-$(CONFIG_NET)		+= $(tmp-y)
diff --git a/net/devres.c b/net/devres.c
new file mode 100644
index 000000000000..c1465d9f9019
--- /dev/null
+++ b/net/devres.c
@@ -0,0 +1,36 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * This file contains all networking devres helpers.
+ */
+
+#include <linux/device.h>
+#include <linux/etherdevice.h>
+#include <linux/netdevice.h>
+
+static void devm_free_netdev(struct device *dev, void *res)
+{
+	free_netdev(*(struct net_device **)res);
+}
+
+struct net_device *devm_alloc_etherdev_mqs(struct device *dev, int sizeof_priv,
+					   unsigned int txqs, unsigned int rxqs)
+{
+	struct net_device **dr;
+	struct net_device *netdev;
+
+	dr = devres_alloc(devm_free_netdev, sizeof(*dr), GFP_KERNEL);
+	if (!dr)
+		return NULL;
+
+	netdev = alloc_etherdev_mqs(sizeof_priv, txqs, rxqs);
+	if (!netdev) {
+		devres_free(dr);
+		return NULL;
+	}
+
+	*dr = netdev;
+	devres_add(dev, dr);
+
+	return netdev;
+}
+EXPORT_SYMBOL(devm_alloc_etherdev_mqs);
diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
index c8b903302ff2..dac65180c4ef 100644
--- a/net/ethernet/eth.c
+++ b/net/ethernet/eth.c
@@ -400,34 +400,6 @@ struct net_device *alloc_etherdev_mqs(int sizeof_priv, unsigned int txqs,
 }
 EXPORT_SYMBOL(alloc_etherdev_mqs);
 
-static void devm_free_netdev(struct device *dev, void *res)
-{
-	free_netdev(*(struct net_device **)res);
-}
-
-struct net_device *devm_alloc_etherdev_mqs(struct device *dev, int sizeof_priv,
-					   unsigned int txqs, unsigned int rxqs)
-{
-	struct net_device **dr;
-	struct net_device *netdev;
-
-	dr = devres_alloc(devm_free_netdev, sizeof(*dr), GFP_KERNEL);
-	if (!dr)
-		return NULL;
-
-	netdev = alloc_etherdev_mqs(sizeof_priv, txqs, rxqs);
-	if (!netdev) {
-		devres_free(dr);
-		return NULL;
-	}
-
-	*dr = netdev;
-	devres_add(dev, dr);
-
-	return netdev;
-}
-EXPORT_SYMBOL(devm_alloc_etherdev_mqs);
-
 ssize_t sysfs_format_mac(char *buf, const unsigned char *addr, int len)
 {
 	return scnprintf(buf, PAGE_SIZE, "%*phC\n", len, addr);
-- 
2.25.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH 3/5] net: devres: define a separate devres structure for devm_alloc_etherdev()
From: Bartosz Golaszewski @ 2020-05-20 11:44 UTC (permalink / raw)
  To: Jonathan Corbet, David S . Miller, Matthias Brugger, John Crispin,
	Sean Wang, Mark Lee, Jakub Kicinski, Arnd Bergmann, Fabien Parent,
	Heiner Kallweit, Edwin Peer
  Cc: devicetree, Stephane Le Provost, netdev, linux-kernel,
	Bartosz Golaszewski, linux-mediatek, Andrew Perepech, Pedro Tsai,
	linux-arm-kernel
In-Reply-To: <20200520114415.13041-1-brgl@bgdev.pl>

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Not using a proxy structure to store struct net_device doesn't save
anything in terms of compiled code size or memory usage but significantly
decreases the readability of the code with all the pointer casting.

Define struct net_device_devres and use it in devm_alloc_etherdev_mqs().

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 net/devres.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/net/devres.c b/net/devres.c
index c1465d9f9019..b97b0c5a8216 100644
--- a/net/devres.c
+++ b/net/devres.c
@@ -7,30 +7,34 @@
 #include <linux/etherdevice.h>
 #include <linux/netdevice.h>
 
-static void devm_free_netdev(struct device *dev, void *res)
+struct net_device_devres {
+	struct net_device *ndev;
+};
+
+static void devm_free_netdev(struct device *dev, void *this)
 {
-	free_netdev(*(struct net_device **)res);
+	struct net_device_devres *res = this;
+
+	free_netdev(res->ndev);
 }
 
 struct net_device *devm_alloc_etherdev_mqs(struct device *dev, int sizeof_priv,
 					   unsigned int txqs, unsigned int rxqs)
 {
-	struct net_device **dr;
-	struct net_device *netdev;
+	struct net_device_devres *dr;
 
 	dr = devres_alloc(devm_free_netdev, sizeof(*dr), GFP_KERNEL);
 	if (!dr)
 		return NULL;
 
-	netdev = alloc_etherdev_mqs(sizeof_priv, txqs, rxqs);
-	if (!netdev) {
+	dr->ndev = alloc_etherdev_mqs(sizeof_priv, txqs, rxqs);
+	if (!dr->ndev) {
 		devres_free(dr);
 		return NULL;
 	}
 
-	*dr = netdev;
 	devres_add(dev, dr);
 
-	return netdev;
+	return dr->ndev;
 }
 EXPORT_SYMBOL(devm_alloc_etherdev_mqs);
-- 
2.25.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH 4/5] net: devres: provide devm_register_netdev()
From: Bartosz Golaszewski @ 2020-05-20 11:44 UTC (permalink / raw)
  To: Jonathan Corbet, David S . Miller, Matthias Brugger, John Crispin,
	Sean Wang, Mark Lee, Jakub Kicinski, Arnd Bergmann, Fabien Parent,
	Heiner Kallweit, Edwin Peer
  Cc: devicetree, Stephane Le Provost, netdev, linux-kernel,
	Bartosz Golaszewski, linux-mediatek, Andrew Perepech, Pedro Tsai,
	linux-arm-kernel
In-Reply-To: <20200520114415.13041-1-brgl@bgdev.pl>

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Provide devm_register_netdev() - a device resource managed variant
of register_netdev(). This new helper will only work for net_device
structs that are also already managed by devres.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 .../driver-api/driver-model/devres.rst        |  1 +
 include/linux/netdevice.h                     |  2 +
 net/devres.c                                  | 55 +++++++++++++++++++
 3 files changed, 58 insertions(+)

diff --git a/Documentation/driver-api/driver-model/devres.rst b/Documentation/driver-api/driver-model/devres.rst
index 50df28d20fa7..fc242ed4bde5 100644
--- a/Documentation/driver-api/driver-model/devres.rst
+++ b/Documentation/driver-api/driver-model/devres.rst
@@ -375,6 +375,7 @@ MUX
 NET
   devm_alloc_etherdev()
   devm_alloc_etherdev_mqs()
+  devm_register_netdev()
 
 PER-CPU MEM
   devm_alloc_percpu()
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 130a668049ab..c4ad728993dd 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -4208,6 +4208,8 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
 int register_netdev(struct net_device *dev);
 void unregister_netdev(struct net_device *dev);
 
+int devm_register_netdev(struct device *dev, struct net_device *ndev);
+
 /* General hardware address lists handling functions */
 int __hw_addr_sync(struct netdev_hw_addr_list *to_list,
 		   struct netdev_hw_addr_list *from_list, int addr_len);
diff --git a/net/devres.c b/net/devres.c
index b97b0c5a8216..57a6a88d11f6 100644
--- a/net/devres.c
+++ b/net/devres.c
@@ -38,3 +38,58 @@ struct net_device *devm_alloc_etherdev_mqs(struct device *dev, int sizeof_priv,
 	return dr->ndev;
 }
 EXPORT_SYMBOL(devm_alloc_etherdev_mqs);
+
+static void devm_netdev_release(struct device *dev, void *this)
+{
+	struct net_device_devres *res = this;
+
+	unregister_netdev(res->ndev);
+}
+
+static int netdev_devres_match(struct device *dev, void *this, void *match_data)
+{
+	struct net_device_devres *res = this;
+	struct net_device *ndev = match_data;
+
+	return ndev == res->ndev;
+}
+
+/**
+ *	devm_register_netdev - resource managed variant of register_netdev()
+ *	@dev: managing device for this netdev - usually the parent device
+ *	@ndev: device to register
+ *
+ *	This is a devres variant of register_netdev() for which the unregister
+ *	function will be call automatically when the managing device is
+ *	detached. Note: the net_device used must also be resource managed by
+ *	the same struct device.
+ */
+int devm_register_netdev(struct device *dev, struct net_device *ndev)
+{
+	struct net_device_devres *dr;
+	int ret;
+
+	/* struct net_device must itself be managed. For now a managed netdev
+	 * can only be allocated by devm_alloc_etherdev_mqs() so the check is
+	 * straightforward.
+	 */
+	if (WARN_ON(!devres_find(dev, devm_free_netdev,
+				 netdev_devres_match, ndev)))
+		return -EINVAL;
+
+	dr = devres_alloc(devm_netdev_release, sizeof(*dr), GFP_KERNEL);
+	if (!dr)
+		return -ENOMEM;
+
+	ret = register_netdev(ndev);
+	if (ret) {
+		devres_free(dr);
+		return ret;
+	}
+
+	dr->ndev = ndev;
+	devres_add(ndev->dev.parent, dr);
+
+	return 0;
+}
+EXPORT_SYMBOL(devm_register_netdev);
-- 
2.25.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH 5/5] net: ethernet: mtk_eth_mac: use devm_register_netdev()
From: Bartosz Golaszewski @ 2020-05-20 11:44 UTC (permalink / raw)
  To: Jonathan Corbet, David S . Miller, Matthias Brugger, John Crispin,
	Sean Wang, Mark Lee, Jakub Kicinski, Arnd Bergmann, Fabien Parent,
	Heiner Kallweit, Edwin Peer
  Cc: devicetree, Stephane Le Provost, netdev, linux-kernel,
	Bartosz Golaszewski, linux-mediatek, Andrew Perepech, Pedro Tsai,
	linux-arm-kernel
In-Reply-To: <20200520114415.13041-1-brgl@bgdev.pl>

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Use the new devres variant of register_netdev() in the mtk-eth-mac
driver and shrink the code by a couple lines.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/net/ethernet/mediatek/mtk_eth_mac.c | 17 +----------------
 1 file changed, 1 insertion(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_mac.c b/drivers/net/ethernet/mediatek/mtk_eth_mac.c
index 4dfe7c2c4e3d..2919ce27efeb 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_mac.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_mac.c
@@ -1513,13 +1513,6 @@ static void mtk_mac_clk_disable_unprepare(void *data)
 	clk_bulk_disable_unprepare(MTK_MAC_NCLKS, priv->clks);
 }
 
-static void mtk_mac_unregister_netdev(void *data)
-{
-	struct net_device *ndev = data;
-
-	unregister_netdev(ndev);
-}
-
 static int mtk_mac_probe(struct platform_device *pdev)
 {
 	struct device_node *of_node;
@@ -1631,15 +1624,7 @@ static int mtk_mac_probe(struct platform_device *pdev)
 
 	netif_napi_add(ndev, &priv->napi, mtk_mac_poll, MTK_MAC_NAPI_WEIGHT);
 
-	ret = register_netdev(ndev);
-	if (ret)
-		return ret;
-
-	ret = devm_add_action_or_reset(dev, mtk_mac_unregister_netdev, ndev);
-	if (ret)
-		return ret;
-
-	return 0;
+	return devm_register_netdev(dev, ndev);
 }
 
 static const struct of_device_id mtk_mac_of_match[] = {
-- 
2.25.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: [PATCH v4 2/4] kasan: record and print the free track
From: Dmitry Vyukov @ 2020-05-20 11:46 UTC (permalink / raw)
  To: Walter Wu
  Cc: wsd_upstream, linux-mediatek, LKML, kasan-dev, Linux-MM,
	Alexander Potapenko, Andrey Ryabinin, Linux ARM
In-Reply-To: <1589974955.3182.8.camel@mtksdccf07>

On Wed, May 20, 2020 at 1:42 PM Walter Wu <walter-zh.wu@mediatek.com> wrote:
> > > > > > > > > > > On Tue, May 19, 2020 at 4:25 AM Walter Wu <walter-zh.wu@mediatek.com> wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > Move free track from slub alloc meta-data to slub free meta-data in
> > > > > > > > > > > > order to make struct kasan_free_meta size is 16 bytes. It is a good
> > > > > > > > > > > > size because it is the minimal redzone size and a good number of
> > > > > > > > > > > > alignment.
> > > > > > > > > > > >
> > > > > > > > > > > > For free track in generic KASAN, we do the modification in struct
> > > > > > > > > > > > kasan_alloc_meta and kasan_free_meta:
> > > > > > > > > > > > - remove free track from kasan_alloc_meta.
> > > > > > > > > > > > - add free track into kasan_free_meta.
> > > > > > > > > > > >
> > > > > > > > > > > > [1]https://bugzilla.kernel.org/show_bug.cgi?id=198437
> > > > > > > > > > > >
> > > > > > > > > > > > Signed-off-by: Walter Wu <walter-zh.wu@mediatek.com>
> > > > > > > > > > > > Suggested-by: Dmitry Vyukov <dvyukov@google.com>
> > > > > > > > > > > > Cc: Andrey Ryabinin <aryabinin@virtuozzo.com>
> > > > > > > > > > > > Cc: Dmitry Vyukov <dvyukov@google.com>
> > > > > > > > > > > > Cc: Alexander Potapenko <glider@google.com>
> > > > > > > > > > > > ---
> > > > > > > > > > > >  mm/kasan/common.c  | 22 ++--------------------
> > > > > > > > > > > >  mm/kasan/generic.c | 18 ++++++++++++++++++
> > > > > > > > > > > >  mm/kasan/kasan.h   |  7 +++++++
> > > > > > > > > > > >  mm/kasan/report.c  | 20 --------------------
> > > > > > > > > > > >  mm/kasan/tags.c    | 37 +++++++++++++++++++++++++++++++++++++
> > > > > > > > > > > >  5 files changed, 64 insertions(+), 40 deletions(-)
> > > > > > > > > > > >
> > > > > > > > > > > > diff --git a/mm/kasan/common.c b/mm/kasan/common.c
> > > > > > > > > > > > index 8bc618289bb1..47b53912f322 100644
> > > > > > > > > > > > --- a/mm/kasan/common.c
> > > > > > > > > > > > +++ b/mm/kasan/common.c
> > > > > > > > > > > > @@ -51,7 +51,7 @@ depot_stack_handle_t kasan_save_stack(gfp_t flags)
> > > > > > > > > > > >         return stack_depot_save(entries, nr_entries, flags);
> > > > > > > > > > > >  }
> > > > > > > > > > > >
> > > > > > > > > > > > -static inline void set_track(struct kasan_track *track, gfp_t flags)
> > > > > > > > > > > > +void kasan_set_track(struct kasan_track *track, gfp_t flags)
> > > > > > > > > > > >  {
> > > > > > > > > > > >         track->pid = current->pid;
> > > > > > > > > > > >         track->stack = kasan_save_stack(flags);
> > > > > > > > > > > > @@ -299,24 +299,6 @@ struct kasan_free_meta *get_free_info(struct kmem_cache *cache,
> > > > > > > > > > > >         return (void *)object + cache->kasan_info.free_meta_offset;
> > > > > > > > > > > >  }
> > > > > > > > > > > >
> > > > > > > > > > > > -
> > > > > > > > > > > > -static void kasan_set_free_info(struct kmem_cache *cache,
> > > > > > > > > > > > -               void *object, u8 tag)
> > > > > > > > > > > > -{
> > > > > > > > > > > > -       struct kasan_alloc_meta *alloc_meta;
> > > > > > > > > > > > -       u8 idx = 0;
> > > > > > > > > > > > -
> > > > > > > > > > > > -       alloc_meta = get_alloc_info(cache, object);
> > > > > > > > > > > > -
> > > > > > > > > > > > -#ifdef CONFIG_KASAN_SW_TAGS_IDENTIFY
> > > > > > > > > > > > -       idx = alloc_meta->free_track_idx;
> > > > > > > > > > > > -       alloc_meta->free_pointer_tag[idx] = tag;
> > > > > > > > > > > > -       alloc_meta->free_track_idx = (idx + 1) % KASAN_NR_FREE_STACKS;
> > > > > > > > > > > > -#endif
> > > > > > > > > > > > -
> > > > > > > > > > > > -       set_track(&alloc_meta->free_track[idx], GFP_NOWAIT);
> > > > > > > > > > > > -}
> > > > > > > > > > > > -
> > > > > > > > > > > >  void kasan_poison_slab(struct page *page)
> > > > > > > > > > > >  {
> > > > > > > > > > > >         unsigned long i;
> > > > > > > > > > > > @@ -492,7 +474,7 @@ static void *__kasan_kmalloc(struct kmem_cache *cache, const void *object,
> > > > > > > > > > > >                 KASAN_KMALLOC_REDZONE);
> > > > > > > > > > > >
> > > > > > > > > > > >         if (cache->flags & SLAB_KASAN)
> > > > > > > > > > > > -               set_track(&get_alloc_info(cache, object)->alloc_track, flags);
> > > > > > > > > > > > +               kasan_set_track(&get_alloc_info(cache, object)->alloc_track, flags);
> > > > > > > > > > > >
> > > > > > > > > > > >         return set_tag(object, tag);
> > > > > > > > > > > >  }
> > > > > > > > > > > > diff --git a/mm/kasan/generic.c b/mm/kasan/generic.c
> > > > > > > > > > > > index 3372bdcaf92a..763d8a13e0ac 100644
> > > > > > > > > > > > --- a/mm/kasan/generic.c
> > > > > > > > > > > > +++ b/mm/kasan/generic.c
> > > > > > > > > > > > @@ -344,3 +344,21 @@ void kasan_record_aux_stack(void *addr)
> > > > > > > > > > > >         alloc_info->aux_stack[1] = alloc_info->aux_stack[0];
> > > > > > > > > > > >         alloc_info->aux_stack[0] = kasan_save_stack(GFP_NOWAIT);
> > > > > > > > > > > >  }
> > > > > > > > > > > > +
> > > > > > > > > > > > +void kasan_set_free_info(struct kmem_cache *cache,
> > > > > > > > > > > > +                               void *object, u8 tag)
> > > > > > > > > > > > +{
> > > > > > > > > > > > +       struct kasan_free_meta *free_meta;
> > > > > > > > > > > > +
> > > > > > > > > > > > +       free_meta = get_free_info(cache, object);
> > > > > > > > > > > > +       kasan_set_track(&free_meta->free_track, GFP_NOWAIT);
> > > > > > > > > > > > +}
> > > > > > > > > > > > +
> > > > > > > > > > > > +struct kasan_track *kasan_get_free_track(struct kmem_cache *cache,
> > > > > > > > > > > > +                               void *object, u8 tag)
> > > > > > > > > > > > +{
> > > > > > > > > > > > +       struct kasan_free_meta *free_meta;
> > > > > > > > > > > > +
> > > > > > > > > > > > +       free_meta = get_free_info(cache, object);
> > > > > > > > > > > > +       return &free_meta->free_track;
> > > > > > > > > > > > +}
> > > > > > > > > > > > diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
> > > > > > > > > > > > index a7391bc83070..ad897ec36545 100644
> > > > > > > > > > > > --- a/mm/kasan/kasan.h
> > > > > > > > > > > > +++ b/mm/kasan/kasan.h
> > > > > > > > > > > > @@ -127,6 +127,9 @@ struct kasan_free_meta {
> > > > > > > > > > > >          * Otherwise it might be used for the allocator freelist.
> > > > > > > > > > > >          */
> > > > > > > > > > > >         struct qlist_node quarantine_link;
> > > > > > > > > > > > +#ifdef CONFIG_KASAN_GENERIC
> > > > > > > > > > > > +       struct kasan_track free_track;
> > > > > > > > > > > > +#endif
> > > > > > > > > > > >  };
> > > > > > > > > > > >
> > > > > > > > > > > >  struct kasan_alloc_meta *get_alloc_info(struct kmem_cache *cache,
> > > > > > > > > > > > @@ -168,6 +171,10 @@ void kasan_report_invalid_free(void *object, unsigned long ip);
> > > > > > > > > > > >  struct page *kasan_addr_to_page(const void *addr);
> > > > > > > > > > > >
> > > > > > > > > > > >  depot_stack_handle_t kasan_save_stack(gfp_t flags);
> > > > > > > > > > > > +void kasan_set_track(struct kasan_track *track, gfp_t flags);
> > > > > > > > > > > > +void kasan_set_free_info(struct kmem_cache *cache, void *object, u8 tag);
> > > > > > > > > > > > +struct kasan_track *kasan_get_free_track(struct kmem_cache *cache,
> > > > > > > > > > > > +                               void *object, u8 tag);
> > > > > > > > > > > >
> > > > > > > > > > > >  #if defined(CONFIG_KASAN_GENERIC) && \
> > > > > > > > > > > >         (defined(CONFIG_SLAB) || defined(CONFIG_SLUB))
> > > > > > > > > > > > diff --git a/mm/kasan/report.c b/mm/kasan/report.c
> > > > > > > > > > > > index 6f8f2bf8f53b..96d2657fe70f 100644
> > > > > > > > > > > > --- a/mm/kasan/report.c
> > > > > > > > > > > > +++ b/mm/kasan/report.c
> > > > > > > > > > > > @@ -159,26 +159,6 @@ static void describe_object_addr(struct kmem_cache *cache, void *object,
> > > > > > > > > > > >                 (void *)(object_addr + cache->object_size));
> > > > > > > > > > > >  }
> > > > > > > > > > > >
> > > > > > > > > > > > -static struct kasan_track *kasan_get_free_track(struct kmem_cache *cache,
> > > > > > > > > > > > -               void *object, u8 tag)
> > > > > > > > > > > > -{
> > > > > > > > > > > > -       struct kasan_alloc_meta *alloc_meta;
> > > > > > > > > > > > -       int i = 0;
> > > > > > > > > > > > -
> > > > > > > > > > > > -       alloc_meta = get_alloc_info(cache, object);
> > > > > > > > > > > > -
> > > > > > > > > > > > -#ifdef CONFIG_KASAN_SW_TAGS_IDENTIFY
> > > > > > > > > > > > -       for (i = 0; i < KASAN_NR_FREE_STACKS; i++) {
> > > > > > > > > > > > -               if (alloc_meta->free_pointer_tag[i] == tag)
> > > > > > > > > > > > -                       break;
> > > > > > > > > > > > -       }
> > > > > > > > > > > > -       if (i == KASAN_NR_FREE_STACKS)
> > > > > > > > > > > > -               i = alloc_meta->free_track_idx;
> > > > > > > > > > > > -#endif
> > > > > > > > > > > > -
> > > > > > > > > > > > -       return &alloc_meta->free_track[i];
> > > > > > > > > > > > -}
> > > > > > > > > > > > -
> > > > > > > > > > > >  #ifdef CONFIG_KASAN_GENERIC
> > > > > > > > > > > >  static void print_stack(depot_stack_handle_t stack)
> > > > > > > > > > > >  {
> > > > > > > > > > > > diff --git a/mm/kasan/tags.c b/mm/kasan/tags.c
> > > > > > > > > > > > index 25b7734e7013..201dee5d6ae0 100644
> > > > > > > > > > > > --- a/mm/kasan/tags.c
> > > > > > > > > > > > +++ b/mm/kasan/tags.c
> > > > > > > > > > > > @@ -162,3 +162,40 @@ void __hwasan_tag_memory(unsigned long addr, u8 tag, unsigned long size)
> > > > > > > > > > > >         kasan_poison_shadow((void *)addr, size, tag);
> > > > > > > > > > > >  }
> > > > > > > > > > > >  EXPORT_SYMBOL(__hwasan_tag_memory);
> > > > > > > > > > > > +
> > > > > > > > > > > > +void kasan_set_free_info(struct kmem_cache *cache,
> > > > > > > > > > > > +                               void *object, u8 tag)
> > > > > > > > > > > > +{
> > > > > > > > > > > > +       struct kasan_alloc_meta *alloc_meta;
> > > > > > > > > > > > +       u8 idx = 0;
> > > > > > > > > > > > +
> > > > > > > > > > > > +       alloc_meta = get_alloc_info(cache, object);
> > > > > > > > > > > > +
> > > > > > > > > > > > +#ifdef CONFIG_KASAN_SW_TAGS_IDENTIFY
> > > > > > > > > > > > +       idx = alloc_meta->free_track_idx;
> > > > > > > > > > > > +       alloc_meta->free_pointer_tag[idx] = tag;
> > > > > > > > > > > > +       alloc_meta->free_track_idx = (idx + 1) % KASAN_NR_FREE_STACKS;
> > > > > > > > > > > > +#endif
> > > > > > > > > > > > +
> > > > > > > > > > > > +       kasan_set_track(&alloc_meta->free_track[idx], GFP_NOWAIT);
> > > > > > > > > > > > +}
> > > > > > > > > > > > +
> > > > > > > > > > > > +struct kasan_track *kasan_get_free_track(struct kmem_cache *cache,
> > > > > > > > > > > > +                               void *object, u8 tag)
> > > > > > > > > > > > +{
> > > > > > > > > > > > +       struct kasan_alloc_meta *alloc_meta;
> > > > > > > > > > > > +       int i = 0;
> > > > > > > > > > > > +
> > > > > > > > > > > > +       alloc_meta = get_alloc_info(cache, object);
> > > > > > > > > > > > +
> > > > > > > > > > > > +#ifdef CONFIG_KASAN_SW_TAGS_IDENTIFY
> > > > > > > > > > > > +       for (i = 0; i < KASAN_NR_FREE_STACKS; i++) {
> > > > > > > > > > > > +               if (alloc_meta->free_pointer_tag[i] == tag)
> > > > > > > > > > > > +                       break;
> > > > > > > > > > > > +       }
> > > > > > > > > > > > +       if (i == KASAN_NR_FREE_STACKS)
> > > > > > > > > > > > +               i = alloc_meta->free_track_idx;
> > > > > > > > > > > > +#endif
> > > > > > > > > > > > +
> > > > > > > > > > > > +       return &alloc_meta->free_track[i];
> > > > > > > > > > > > +}
> > > > > > > > > > >
> > > > > > > > > > > Hi Walter,
> > > > > > > > > > >
> > > > > > > > > > > FTR I've uploaded this for review purposes here:
> > > > > > > > > > > https://linux-review.googlesource.com/c/linux/kernel/git/torvalds/linux/+/2458
> > > > > > > > > > >
> > > > > > > > > > > Diff from the previous version is available as:
> > > > > > > > > > > https://linux-review.googlesource.com/c/linux/kernel/git/torvalds/linux/+/2458/1..2
> > > > > > > > > > >
> > > > > > > > > > > I've tested this locally and with syzkaller. This is :
> > > > > > > > > > >
> > > > > > > > > > > [   80.583021][    C3] Freed by task 0:
> > > > > > > > > > > [   80.583480][    C3]  kasan_save_stack+0x1b/0x40 mm/kasan/common.c:49
> > > > > > > > > > > [   80.584056][    C3]  kasan_set_track+0x1c/0x30 mm/kasan/common.c:57
> > > > > > > > > > > [   80.584617][    C3]  kasan_set_free_info+0x1b/0x30 mm/kasan/generic.c:354
> > > > > > > > > > > [   80.585221][    C3]  __kasan_slab_free+0xd8/0x120 mm/kasan/common.c:438
> > > > > > > > > > > [   80.585814][    C3]  __cache_free mm/slab.c:3426 [inline]
> > > > > > > > > > > [   80.585814][    C3]  kfree+0x10b/0x2b0 mm/slab.c:3757
> > > > > > > > > > > [   80.586291][    C3]  kasan_rcu_reclaim+0x16/0x43 [test_kasan]
> > > > > > > > > > > [   80.587009][    C3]  rcu_do_batch kernel/rcu/tree.c:2207 [inline]
> > > > > > > > > > > [   80.587009][    C3]  rcu_core+0x59f/0x1370 kernel/rcu/tree.c:2434
> > > > > > > > > > > [   80.587537][    C3]  __do_softirq+0x26c/0x9fa kernel/softirq.c:292
> > > > > > > > > > > [   80.588085][    C3]
> > > > > > > > > > > [   80.588367][    C3] Last one call_rcu() call stack:
> > > > > > > > > > > [   80.589052][    C3]  kasan_save_stack+0x1b/0x40 mm/kasan/common.c:49
> > > > > > > > > > > [   80.589622][    C3]  kasan_record_aux_stack+0x82/0xb0 mm/kasan/generic.c:345
> > > > > > > > > > > [   80.590254][    C3]  __call_rcu kernel/rcu/tree.c:2672 [inline]
> > > > > > > > > > > [   80.590254][    C3]  call_rcu+0x14f/0x7f0 kernel/rcu/tree.c:2746
> > > > > > > > > > > [   80.590782][    C3]  kasan_rcu_uaf+0xe4/0xeb [test_kasan]
> > > > > > > > > > > [   80.591697][    C3]  kmalloc_tests_init+0xbc/0x1097 [test_kasan]
> > > > > > > > > > > [   80.592900][    C3]  do_one_initcall+0x10a/0x7d0 init/main.c:1196
> > > > > > > > > > > [   80.593494][    C3]  do_init_module+0x1e6/0x6d0 kernel/module.c:3539
> > > > > > > > > > > [   80.594066][    C3]  load_module+0x7464/0x9450 kernel/module.c:3890
> > > > > > > > > > > [   80.594626][    C3]  __do_sys_init_module+0x1e3/0x220 kernel/module.c:3953
> > > > > > > > > > > [   80.595265][    C3]  do_syscall_64+0xf6/0x7d0 arch/x86/entry/common.c:295
> > > > > > > > > > > [   80.595822][    C3]  entry_SYSCALL_64_after_hwframe+0x49/0xb3
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > Overall this looks very good to me.
> > > > > > > > > > > But there is one aspect that bothers me. In the previous patch you had
> > > > > > > > > > > code that returned NULL from kasan_get_free_track() if the object is
> > > > > > > > > > > live (which means free meta is not available, it's occupied by object
> > > > > > > > > > > data). Now you dropped that code, but I think we still need it.
> > > > > > > > > > > Otherwise we cast user object data to free meta and print the free
> > > > > > > > > > > stack/pid from whatever garbage is there. This may lead to very
> > > > > > > > > > > confusing output and potentially to crashes in stackdepot.
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > Yes, I totally agree with you. In the previous email I thought that
> > > > > > > > > > there is a problem with free track, but I did not point it out. Thank
> > > > > > > > > > you for pointing this problem. As you mentioned, we should fix it.
> > > > > > > > > >
> > > > > > > > > > > What do you think about this patch on top of your patches?
> > > > > > > > > > > https://linux-review.googlesource.com/c/linux/kernel/git/torvalds/linux/+/2478
> > > > > > > > > > > This way we very precisely mark the period of time when the object has
> > > > > > > > > > > free track live and set.
> > > > > > > > > > > If it looks good to you, feel free to incorporate it into your series.
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > Thank you for providing good idea solution.
> > > > > > > > > >
> > > > > > > > > > I saw this patch, that is a great patch. I think it can fix the issue
> > > > > > > > > > which has garbage stack. it should work as described below.
> > > > > > > > > >
> > > > > > > > > > 1). When object is live, then don't print free stack.
> > > > > > > > > > 2). When object is NOT alive, after free object:
> > > > > > > > > > 2a). when object is in quarantine, then it can print free stack
> > > > > > > > > > 2b). when object is NOT in quarantine, then it can NOT print free stack.
> > > > > > > > > >
> > > > > > > > > > I have a question about 2), why we don't directly use
> > > > > > > > > > KASAN_KMALLOC_FREE? if we directly use it, then 2b) can print free
> > > > > > > > > > stack? 2b) may has use-after-free? so that it may need free stack.
> > > > > > > > >
> > > > > > >
> > > > > > > About 2b), I see another question. When do qlink_free(), it will be
> > > > > > > written KASAN_KMALLOC_FREE from KASAN_KMALLOC_FREETRACK? if we don't
> > > > > > > write shadow memory, it is still KASAN_KMALLOC_FREETRACK, then 2b) will
> > > > > > > have free stack? Because I see you add KASAN_KMALLOC_FREETRACK to get
> > > > > > > use-after-free in get_shadow_bug_type(). so should it not write
> > > > > > > KASAN_KMALLOC_FREE?
> > > > > >
> > > > > > It may or may not work.
> > > > > > The potential problem is that when qlink_free calls ___cache_free,
> > > > > > slab/slub may start using object memory for its own purposes, e.g.
> > > > > > store the next link. This next link may overwrite part of free meta.
> > > > > > It actually may work because the slab/slib next link is likely to
> > > > > > overlap with kasan_free_meta.quarantine_link only. And we may have
> > > > > > kasan_free_meta.free_track intact while KASAN_KMALLOC_FREE is set. But
> > > > > > this needs careful checking for both slab and slub and if they may use
> > > > > > more than 1 word in some configurations.
> > > > > >
> > > > >
> > > > > This problem looks like existing, even without this change? currently
> > > > > KASAN may get wrong free stack?
> > > >
> > > > No, we should not have this problem now. Currently free track is
> > > > stored in alloc meta. Alloc meta does not overlap with the object.
> > > > It's only free meta that overlaps with the object and slab metadata at
> > > > different periods of the block lifetime. Schematically what we have
> > > > is:
> > > >
> > > > struct block_t {
> > > >   alloc_meta kasan_alloc_meta;
> > > >   union {
> > > >     user_data char[N];
> > > >     slab_meta slab_meta;
> > > >     free_meta kasan_free_meta;
> > > >   };
> > > > }
> > > >
> > > > free_meta shared storage space with 2 other things.
> > > >
> > >
> > > Ah...I forget it is stored in alloc mata, Yes, it should not have this
> > > problem.
> > >
> > > Thanks for your detailed explanation
> > >
> > > > > Regardless of whether the shadow memory content is
> > > > > KASAN_KMALLOC_FREETRACK or KASAN_KMALLOC_FREE, it may have this problem?
> > > >
> > > > KASAN_KMALLOC_FREETRACK is set only when nobody else uses the storage.
> > > >
> > >
> > > Ok, I will use KASAN_KMALLOC_FREE. If you have any concerns, please tell me.
> > > Thanks.
> >
> > You mean KASAN_KMALLOC_FREETRACK?
> >
>
> Yes, sorry, I made you misunderstand what I meant. I mean that qlink_free()
> write KASAN_KMALLOC_FREE.

Good.


> > Or, you checked that using KASAN_KMALLOC_FREE is safe and will not
> > cause any bad overlap?
> >
>
> This item you refer to, maybe it can be done in the future.
>
> >
> > > > > But because of kasan_get_free_track() have conditions to get free track,
> > > > > so that if shadow memory content is KASAN_KMALLOC_FREE, then it will
> > > > > avoid this problem and always print right free stack.
> > > >
> > > >
> > > >
> > > > > > > > > We can't use KASAN_KMALLOC_FREE because of this part:
> > > > > > > > >
> > > > > > > > > static bool __kasan_slab_free(struct kmem_cache *cache, void *object,
> > > > > > > > >                   unsigned long ip, bool quarantine)
> > > > > > > > > {
> > > > > > > > > ...
> > > > > > > > >     kasan_poison_shadow(object, rounded_up_size, KASAN_KMALLOC_FREE);
> > > > > > > > >
> > > > > > > > >     if ((IS_ENABLED(CONFIG_KASAN_GENERIC) && !quarantine) ||
> > > > > > > > >             unlikely(!(cache->flags & SLAB_KASAN)))
> > > > > > > > >         return false;
> > > > > > > > >
> > > > > > > > >     kasan_set_free_info(cache, object, tag);
> > > > > > > > > ...
> > > > > > > > >
> > > > > > > >
> > > > > > > > Ok, I see. When return false, then the shadow memory content has
> > > > > > > > KASAN_KMALLOC_FREE, but it doesn't set free stack, so that we need to
> > > > > > > > avoid this situation. Thank for you reminder.
> > > > > > > >
> > > > > > > > >
> > > > > > > > > We may set KASAN_KMALLOC_FREE, but not set the track (or even have
> > > > > > > > > memory for the track!).
> > > > > > > > > The object may not have free meta allocated at all, e.g. very large
> > > > > > > > > object with ctor (no place to store meta), or it may be in a mempool:
> > > > > > > > > https://elixir.bootlin.com/linux/v5.7-rc6/source/mm/mempool.c#L109
> > > > > > > > > and mempool may be using the object memory itself (for its own next
> > > > > > > > > link or something).
> > > > > > > > >
> > > > > > > > > KASAN_KMALLOC_FREETRACK very explicitly tracks the exact condition we
> > > > > > > > > want: we have meta info live now and we have free track set.
> >
>
> --
> You received this message because you are subscribed to the Google Groups "kasan-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kasan-dev+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/1589974955.3182.8.camel%40mtksdccf07.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v11 2/3] i2c: npcm7xx: Add Nuvoton NPCM I2C controller driver
From: Andy Shevchenko @ 2020-05-20 11:49 UTC (permalink / raw)
  To: Avi Fishman
  Cc: devicetree, Linux Kernel Mailing List, Tomer Maimon, Nancy Yuen,
	Patrick Venture, OpenBMC Maillist, wsa, Brendan Higgins, ofery,
	Tali Perry, kfting, Rob Herring, linux-i2c, Linux ARM,
	Benjamin Fair
In-Reply-To: <CAKKbWA5L_n7iC6-d22Am62SOoDBwNWO87+sXtRbwxwuVdjmRYA@mail.gmail.com>

On Wed, May 20, 2020 at 02:37:13PM +0300, Avi Fishman wrote:
> On Wed, May 20, 2020 at 1:24 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > On Wed, May 20, 2020 at 12:51:12PM +0300, Tali Perry wrote:
> > > Add Nuvoton NPCM BMC I2C controller driver.
> >
> > ...
> >
> > > +#ifdef CONFIG_DEBUG_FS
> >
> > Why?!
> 
> It is made to save code size if CONFIG_DEBUG_FS is not defined?

Nope (in cases I have commented on). Try again.

> We see a lot of kernel code that is doing it.

Cargo cult, okay. So, somebody should try to understand what they are doing.

> So could you elaborate what is the problem?

Problem 1: ugly code.
Problem 2: some of the code is not guarded (seems never been tested with disabled debugfs).
Problem 3: it's not needed.

> > > +#include <linux/debugfs.h>
> > > +#endif

-- 
With Best Regards,
Andy Shevchenko



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] arm64/cpufeature: Move BUG_ON() inside get_arm64_ftr_reg()
From: Catalin Marinas @ 2020-05-20 11:49 UTC (permalink / raw)
  To: Anshuman Khandual
  Cc: mark.rutland, Suzuki K Poulose, linux-kernel, Mark Brown,
	Will Deacon, linux-arm-kernel
In-Reply-To: <1589937774-20479-1-git-send-email-anshuman.khandual@arm.com>

On Wed, May 20, 2020 at 06:52:54AM +0530, Anshuman Khandual wrote:
> There is no way to proceed when requested register could not be searched in
> arm64_ftr_reg[]. Requesting for a non present register would be an error as
> well. Hence lets just BUG_ON() when the search fails in get_arm64_ftr_reg()
> rather than checking for return value and doing the same in some individual
> callers.
> 
> But there are some callers that dont BUG_ON() upon search failure. It adds
> an argument 'failsafe' that provides required switch between callers based
> on whether they could proceed or not.
> 
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> 
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>

BTW, there should be no empty line between the Cc block and the SoB.

The patch looks fine. Just a note that the patch transforms a current
WARN_ON in a BUG_ON but that's fine by me.

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 04/15] PCI: brcmstb: Add compatibily of other chips
From: Nicolas Saenz Julienne @ 2020-05-20 11:51 UTC (permalink / raw)
  To: Jim Quinlan
  Cc: Rob Herring, Lorenzo Pieralisi,
	open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS, open list,
	Florian Fainelli, maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
	Bjorn Helgaas,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE
In-Reply-To: <20200519203419.12369-5-james.quinlan@broadcom.com>


[-- Attachment #1.1: Type: text/plain, Size: 4630 bytes --]

Hi Jim,

On Tue, 2020-05-19 at 16:34 -0400, Jim Quinlan wrote:
> From: Jim Quinlan <jquinlan@broadcom.com>
> 
> Add in compatibility strings and code for three Broadcom STB chips.
> Some of the register locations, shifts, and masks are different
> for certain chips, requiring the use of different constants based
> on of_id.
> 
> We would like to add the following at this time to the match list
> but we need to wait until the end of this patchset so that
> everything works.
> 
>     { .compatible = "brcm,bcm7211-pcie", .data = &generic_cfg },
>     { .compatible = "brcm,bcm7278-pcie", .data = &bcm7278_cfg },
>     { .compatible = "brcm,bcm7216-pcie", .data = &bcm7278_cfg },
>     { .compatible = "brcm,bcm7445-pcie", .data = &generic_cfg },
> 
> Signed-off-by: Jim Quinlan <jquinlan@broadcom.com>
> ---
>  drivers/pci/controller/pcie-brcmstb.c | 103 +++++++++++++++++++++++---
>  1 file changed, 91 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/pci/controller/pcie-brcmstb.c
> b/drivers/pci/controller/pcie-brcmstb.c
> index 73020b4ff090..c1cf4ea7d3d9 100644
> --- a/drivers/pci/controller/pcie-brcmstb.c
> +++ b/drivers/pci/controller/pcie-brcmstb.c
> @@ -120,9 +120,8 @@
>  #define  PCIE_EXT_SLOT_SHIFT				15
>  #define  PCIE_EXT_FUNC_SHIFT				12
>  
> -#define PCIE_RGR1_SW_INIT_1				0x9210
>  #define  PCIE_RGR1_SW_INIT_1_PERST_MASK			0x1
> -#define  PCIE_RGR1_SW_INIT_1_INIT_MASK			0x2
> +#define  PCIE_RGR1_SW_INIT_1_PERST_SHIFT		0x0
>  
>  /* PCIe parameters */
>  #define BRCM_NUM_PCIE_OUT_WINS		0x4
> @@ -152,6 +151,69 @@
>  #define SSC_STATUS_SSC_MASK		0x400
>  #define SSC_STATUS_PLL_LOCK_MASK	0x800
>  
> +#define IDX_ADDR(pcie)	\
> +	(pcie->reg_offsets[EXT_CFG_INDEX])
> +#define DATA_ADDR(pcie)	\
> +	(pcie->reg_offsets[EXT_CFG_DATA])
> +#define PCIE_RGR1_SW_INIT_1(pcie) \
> +	(pcie->reg_offsets[RGR1_SW_INIT_1])
> +
> +enum {
> +	RGR1_SW_INIT_1,
> +	EXT_CFG_INDEX,
> +	EXT_CFG_DATA,
> +};
> +
> +enum {
> +	RGR1_SW_INIT_1_INIT_MASK,
> +	RGR1_SW_INIT_1_INIT_SHIFT,
> +};
> +
> +enum pcie_type {
> +	GENERIC,
> +	BCM7278,
> +};
> +
> +struct pcie_cfg_data {
> +	const int *reg_field_info;
> +	const int *offsets;
> +	const enum pcie_type type;
> +};
> +
> +static const int pcie_reg_field_info[] = {
> +	[RGR1_SW_INIT_1_INIT_MASK] = 0x2,
> +	[RGR1_SW_INIT_1_INIT_SHIFT] = 0x1,
> +};
> +
> +static const int pcie_reg_field_info_bcm7278[] = {
> +	[RGR1_SW_INIT_1_INIT_MASK] = 0x1,
> +	[RGR1_SW_INIT_1_INIT_SHIFT] = 0x0,
> +};
> +
> +static const int pcie_offsets[] = {
> +	[RGR1_SW_INIT_1] = 0x9210,
> +	[EXT_CFG_INDEX]  = 0x9000,
> +	[EXT_CFG_DATA]   = 0x9004,
> +};
> +
> +static const struct pcie_cfg_data generic_cfg = {
> +	.reg_field_info	= pcie_reg_field_info,
> +	.offsets	= pcie_offsets,
> +	.type		= GENERIC,
> +};
> +
> +static const int pcie_offset_bcm7278[] = {
> +	[RGR1_SW_INIT_1] = 0xc010,
> +	[EXT_CFG_INDEX] = 0x9000,
> +	[EXT_CFG_DATA] = 0x9004,
> +};
> +
> +static const struct pcie_cfg_data bcm7278_cfg = {
> +	.reg_field_info = pcie_reg_field_info_bcm7278,
> +	.offsets	= pcie_offset_bcm7278,
> +	.type		= BCM7278,
> +};

It's not essential, but if v2 is due I'd suggest factoring out the bcm2728
specific structures above, and moving them to patch #15. This will keep a
clearer division between the patch introducing the infrastructure and the one
adding the support for a new device.

> +
>  struct brcm_msi {
>  	struct device		*dev;
>  	void __iomem		*base;
> @@ -176,6 +238,9 @@ struct brcm_pcie {
>  	int			gen;
>  	u64			msi_target_addr;
>  	struct brcm_msi		*msi;
> +	const int		*reg_offsets;
> +	const int		*reg_field_info;
> +	enum pcie_type		type;
>  };
>  
>  /*
> @@ -602,20 +667,21 @@ static struct pci_ops brcm_pcie_ops = {
>  
>  static inline void brcm_pcie_bridge_sw_init_set(struct brcm_pcie *pcie, u32
> val)
>  {
> -	u32 tmp;
> +	u32 tmp, mask =  pcie->reg_field_info[RGR1_SW_INIT_1_INIT_MASK];
> +	u32 shift = pcie->reg_field_info[RGR1_SW_INIT_1_INIT_SHIFT];

I don't think you need shift here, IIUC u32p_replace_bits() will take care of
all the masking and shifting internally, moreover, you'd be able to drop the
shift entry from reg_field_info.

> -	tmp = readl(pcie->base + PCIE_RGR1_SW_INIT_1);
> -	u32p_replace_bits(&tmp, val, PCIE_RGR1_SW_INIT_1_INIT_MASK);
> -	writel(tmp, pcie->base + PCIE_RGR1_SW_INIT_1);
> +	tmp = readl(pcie->base + PCIE_RGR1_SW_INIT_1(pcie));
> +	tmp = (tmp & ~mask) | ((val << shift) & mask);
> +	writel(tmp, pcie->base + PCIE_RGR1_SW_INIT_1(pcie));
>  }

Regards,
Nicolas


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v4 2/4] kasan: record and print the free track
From: Walter Wu @ 2020-05-20 11:42 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: wsd_upstream, linux-mediatek, LKML, kasan-dev, Linux-MM,
	Alexander Potapenko, Andrey Ryabinin, Linux ARM
In-Reply-To: <CACT4Y+aRybP+aKEF7XC5rZsdyj-Sj=tASUBnq7TqzTLH1ukkOw@mail.gmail.com>

On Wed, 2020-05-20 at 13:15 +0200, 'Dmitry Vyukov' via kasan-dev wrote:
> On Wed, May 20, 2020 at 12:15 PM Walter Wu <walter-zh.wu@mediatek.com> wrote:
> >
> > On Wed, 2020-05-20 at 11:44 +0200, 'Dmitry Vyukov' via kasan-dev wrote:
> > > On Wed, May 20, 2020 at 11:17 AM Walter Wu <walter-zh.wu@mediatek.com> wrote:
> > > > > > On Wed, 2020-05-20 at 13:14 +0800, Walter Wu wrote:
> > > > > > > > On Wed, May 20, 2020 at 6:03 AM Walter Wu <walter-zh.wu@mediatek.com> wrote:
> > > > > > > > >
> > > > > > > > > > On Tue, May 19, 2020 at 4:25 AM Walter Wu <walter-zh.wu@mediatek.com> wrote:
> > > > > > > > > > >
> > > > > > > > > > > Move free track from slub alloc meta-data to slub free meta-data in
> > > > > > > > > > > order to make struct kasan_free_meta size is 16 bytes. It is a good
> > > > > > > > > > > size because it is the minimal redzone size and a good number of
> > > > > > > > > > > alignment.
> > > > > > > > > > >
> > > > > > > > > > > For free track in generic KASAN, we do the modification in struct
> > > > > > > > > > > kasan_alloc_meta and kasan_free_meta:
> > > > > > > > > > > - remove free track from kasan_alloc_meta.
> > > > > > > > > > > - add free track into kasan_free_meta.
> > > > > > > > > > >
> > > > > > > > > > > [1]https://bugzilla.kernel.org/show_bug.cgi?id=198437
> > > > > > > > > > >
> > > > > > > > > > > Signed-off-by: Walter Wu <walter-zh.wu@mediatek.com>
> > > > > > > > > > > Suggested-by: Dmitry Vyukov <dvyukov@google.com>
> > > > > > > > > > > Cc: Andrey Ryabinin <aryabinin@virtuozzo.com>
> > > > > > > > > > > Cc: Dmitry Vyukov <dvyukov@google.com>
> > > > > > > > > > > Cc: Alexander Potapenko <glider@google.com>
> > > > > > > > > > > ---
> > > > > > > > > > >  mm/kasan/common.c  | 22 ++--------------------
> > > > > > > > > > >  mm/kasan/generic.c | 18 ++++++++++++++++++
> > > > > > > > > > >  mm/kasan/kasan.h   |  7 +++++++
> > > > > > > > > > >  mm/kasan/report.c  | 20 --------------------
> > > > > > > > > > >  mm/kasan/tags.c    | 37 +++++++++++++++++++++++++++++++++++++
> > > > > > > > > > >  5 files changed, 64 insertions(+), 40 deletions(-)
> > > > > > > > > > >
> > > > > > > > > > > diff --git a/mm/kasan/common.c b/mm/kasan/common.c
> > > > > > > > > > > index 8bc618289bb1..47b53912f322 100644
> > > > > > > > > > > --- a/mm/kasan/common.c
> > > > > > > > > > > +++ b/mm/kasan/common.c
> > > > > > > > > > > @@ -51,7 +51,7 @@ depot_stack_handle_t kasan_save_stack(gfp_t flags)
> > > > > > > > > > >         return stack_depot_save(entries, nr_entries, flags);
> > > > > > > > > > >  }
> > > > > > > > > > >
> > > > > > > > > > > -static inline void set_track(struct kasan_track *track, gfp_t flags)
> > > > > > > > > > > +void kasan_set_track(struct kasan_track *track, gfp_t flags)
> > > > > > > > > > >  {
> > > > > > > > > > >         track->pid = current->pid;
> > > > > > > > > > >         track->stack = kasan_save_stack(flags);
> > > > > > > > > > > @@ -299,24 +299,6 @@ struct kasan_free_meta *get_free_info(struct kmem_cache *cache,
> > > > > > > > > > >         return (void *)object + cache->kasan_info.free_meta_offset;
> > > > > > > > > > >  }
> > > > > > > > > > >
> > > > > > > > > > > -
> > > > > > > > > > > -static void kasan_set_free_info(struct kmem_cache *cache,
> > > > > > > > > > > -               void *object, u8 tag)
> > > > > > > > > > > -{
> > > > > > > > > > > -       struct kasan_alloc_meta *alloc_meta;
> > > > > > > > > > > -       u8 idx = 0;
> > > > > > > > > > > -
> > > > > > > > > > > -       alloc_meta = get_alloc_info(cache, object);
> > > > > > > > > > > -
> > > > > > > > > > > -#ifdef CONFIG_KASAN_SW_TAGS_IDENTIFY
> > > > > > > > > > > -       idx = alloc_meta->free_track_idx;
> > > > > > > > > > > -       alloc_meta->free_pointer_tag[idx] = tag;
> > > > > > > > > > > -       alloc_meta->free_track_idx = (idx + 1) % KASAN_NR_FREE_STACKS;
> > > > > > > > > > > -#endif
> > > > > > > > > > > -
> > > > > > > > > > > -       set_track(&alloc_meta->free_track[idx], GFP_NOWAIT);
> > > > > > > > > > > -}
> > > > > > > > > > > -
> > > > > > > > > > >  void kasan_poison_slab(struct page *page)
> > > > > > > > > > >  {
> > > > > > > > > > >         unsigned long i;
> > > > > > > > > > > @@ -492,7 +474,7 @@ static void *__kasan_kmalloc(struct kmem_cache *cache, const void *object,
> > > > > > > > > > >                 KASAN_KMALLOC_REDZONE);
> > > > > > > > > > >
> > > > > > > > > > >         if (cache->flags & SLAB_KASAN)
> > > > > > > > > > > -               set_track(&get_alloc_info(cache, object)->alloc_track, flags);
> > > > > > > > > > > +               kasan_set_track(&get_alloc_info(cache, object)->alloc_track, flags);
> > > > > > > > > > >
> > > > > > > > > > >         return set_tag(object, tag);
> > > > > > > > > > >  }
> > > > > > > > > > > diff --git a/mm/kasan/generic.c b/mm/kasan/generic.c
> > > > > > > > > > > index 3372bdcaf92a..763d8a13e0ac 100644
> > > > > > > > > > > --- a/mm/kasan/generic.c
> > > > > > > > > > > +++ b/mm/kasan/generic.c
> > > > > > > > > > > @@ -344,3 +344,21 @@ void kasan_record_aux_stack(void *addr)
> > > > > > > > > > >         alloc_info->aux_stack[1] = alloc_info->aux_stack[0];
> > > > > > > > > > >         alloc_info->aux_stack[0] = kasan_save_stack(GFP_NOWAIT);
> > > > > > > > > > >  }
> > > > > > > > > > > +
> > > > > > > > > > > +void kasan_set_free_info(struct kmem_cache *cache,
> > > > > > > > > > > +                               void *object, u8 tag)
> > > > > > > > > > > +{
> > > > > > > > > > > +       struct kasan_free_meta *free_meta;
> > > > > > > > > > > +
> > > > > > > > > > > +       free_meta = get_free_info(cache, object);
> > > > > > > > > > > +       kasan_set_track(&free_meta->free_track, GFP_NOWAIT);
> > > > > > > > > > > +}
> > > > > > > > > > > +
> > > > > > > > > > > +struct kasan_track *kasan_get_free_track(struct kmem_cache *cache,
> > > > > > > > > > > +                               void *object, u8 tag)
> > > > > > > > > > > +{
> > > > > > > > > > > +       struct kasan_free_meta *free_meta;
> > > > > > > > > > > +
> > > > > > > > > > > +       free_meta = get_free_info(cache, object);
> > > > > > > > > > > +       return &free_meta->free_track;
> > > > > > > > > > > +}
> > > > > > > > > > > diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
> > > > > > > > > > > index a7391bc83070..ad897ec36545 100644
> > > > > > > > > > > --- a/mm/kasan/kasan.h
> > > > > > > > > > > +++ b/mm/kasan/kasan.h
> > > > > > > > > > > @@ -127,6 +127,9 @@ struct kasan_free_meta {
> > > > > > > > > > >          * Otherwise it might be used for the allocator freelist.
> > > > > > > > > > >          */
> > > > > > > > > > >         struct qlist_node quarantine_link;
> > > > > > > > > > > +#ifdef CONFIG_KASAN_GENERIC
> > > > > > > > > > > +       struct kasan_track free_track;
> > > > > > > > > > > +#endif
> > > > > > > > > > >  };
> > > > > > > > > > >
> > > > > > > > > > >  struct kasan_alloc_meta *get_alloc_info(struct kmem_cache *cache,
> > > > > > > > > > > @@ -168,6 +171,10 @@ void kasan_report_invalid_free(void *object, unsigned long ip);
> > > > > > > > > > >  struct page *kasan_addr_to_page(const void *addr);
> > > > > > > > > > >
> > > > > > > > > > >  depot_stack_handle_t kasan_save_stack(gfp_t flags);
> > > > > > > > > > > +void kasan_set_track(struct kasan_track *track, gfp_t flags);
> > > > > > > > > > > +void kasan_set_free_info(struct kmem_cache *cache, void *object, u8 tag);
> > > > > > > > > > > +struct kasan_track *kasan_get_free_track(struct kmem_cache *cache,
> > > > > > > > > > > +                               void *object, u8 tag);
> > > > > > > > > > >
> > > > > > > > > > >  #if defined(CONFIG_KASAN_GENERIC) && \
> > > > > > > > > > >         (defined(CONFIG_SLAB) || defined(CONFIG_SLUB))
> > > > > > > > > > > diff --git a/mm/kasan/report.c b/mm/kasan/report.c
> > > > > > > > > > > index 6f8f2bf8f53b..96d2657fe70f 100644
> > > > > > > > > > > --- a/mm/kasan/report.c
> > > > > > > > > > > +++ b/mm/kasan/report.c
> > > > > > > > > > > @@ -159,26 +159,6 @@ static void describe_object_addr(struct kmem_cache *cache, void *object,
> > > > > > > > > > >                 (void *)(object_addr + cache->object_size));
> > > > > > > > > > >  }
> > > > > > > > > > >
> > > > > > > > > > > -static struct kasan_track *kasan_get_free_track(struct kmem_cache *cache,
> > > > > > > > > > > -               void *object, u8 tag)
> > > > > > > > > > > -{
> > > > > > > > > > > -       struct kasan_alloc_meta *alloc_meta;
> > > > > > > > > > > -       int i = 0;
> > > > > > > > > > > -
> > > > > > > > > > > -       alloc_meta = get_alloc_info(cache, object);
> > > > > > > > > > > -
> > > > > > > > > > > -#ifdef CONFIG_KASAN_SW_TAGS_IDENTIFY
> > > > > > > > > > > -       for (i = 0; i < KASAN_NR_FREE_STACKS; i++) {
> > > > > > > > > > > -               if (alloc_meta->free_pointer_tag[i] == tag)
> > > > > > > > > > > -                       break;
> > > > > > > > > > > -       }
> > > > > > > > > > > -       if (i == KASAN_NR_FREE_STACKS)
> > > > > > > > > > > -               i = alloc_meta->free_track_idx;
> > > > > > > > > > > -#endif
> > > > > > > > > > > -
> > > > > > > > > > > -       return &alloc_meta->free_track[i];
> > > > > > > > > > > -}
> > > > > > > > > > > -
> > > > > > > > > > >  #ifdef CONFIG_KASAN_GENERIC
> > > > > > > > > > >  static void print_stack(depot_stack_handle_t stack)
> > > > > > > > > > >  {
> > > > > > > > > > > diff --git a/mm/kasan/tags.c b/mm/kasan/tags.c
> > > > > > > > > > > index 25b7734e7013..201dee5d6ae0 100644
> > > > > > > > > > > --- a/mm/kasan/tags.c
> > > > > > > > > > > +++ b/mm/kasan/tags.c
> > > > > > > > > > > @@ -162,3 +162,40 @@ void __hwasan_tag_memory(unsigned long addr, u8 tag, unsigned long size)
> > > > > > > > > > >         kasan_poison_shadow((void *)addr, size, tag);
> > > > > > > > > > >  }
> > > > > > > > > > >  EXPORT_SYMBOL(__hwasan_tag_memory);
> > > > > > > > > > > +
> > > > > > > > > > > +void kasan_set_free_info(struct kmem_cache *cache,
> > > > > > > > > > > +                               void *object, u8 tag)
> > > > > > > > > > > +{
> > > > > > > > > > > +       struct kasan_alloc_meta *alloc_meta;
> > > > > > > > > > > +       u8 idx = 0;
> > > > > > > > > > > +
> > > > > > > > > > > +       alloc_meta = get_alloc_info(cache, object);
> > > > > > > > > > > +
> > > > > > > > > > > +#ifdef CONFIG_KASAN_SW_TAGS_IDENTIFY
> > > > > > > > > > > +       idx = alloc_meta->free_track_idx;
> > > > > > > > > > > +       alloc_meta->free_pointer_tag[idx] = tag;
> > > > > > > > > > > +       alloc_meta->free_track_idx = (idx + 1) % KASAN_NR_FREE_STACKS;
> > > > > > > > > > > +#endif
> > > > > > > > > > > +
> > > > > > > > > > > +       kasan_set_track(&alloc_meta->free_track[idx], GFP_NOWAIT);
> > > > > > > > > > > +}
> > > > > > > > > > > +
> > > > > > > > > > > +struct kasan_track *kasan_get_free_track(struct kmem_cache *cache,
> > > > > > > > > > > +                               void *object, u8 tag)
> > > > > > > > > > > +{
> > > > > > > > > > > +       struct kasan_alloc_meta *alloc_meta;
> > > > > > > > > > > +       int i = 0;
> > > > > > > > > > > +
> > > > > > > > > > > +       alloc_meta = get_alloc_info(cache, object);
> > > > > > > > > > > +
> > > > > > > > > > > +#ifdef CONFIG_KASAN_SW_TAGS_IDENTIFY
> > > > > > > > > > > +       for (i = 0; i < KASAN_NR_FREE_STACKS; i++) {
> > > > > > > > > > > +               if (alloc_meta->free_pointer_tag[i] == tag)
> > > > > > > > > > > +                       break;
> > > > > > > > > > > +       }
> > > > > > > > > > > +       if (i == KASAN_NR_FREE_STACKS)
> > > > > > > > > > > +               i = alloc_meta->free_track_idx;
> > > > > > > > > > > +#endif
> > > > > > > > > > > +
> > > > > > > > > > > +       return &alloc_meta->free_track[i];
> > > > > > > > > > > +}
> > > > > > > > > >
> > > > > > > > > > Hi Walter,
> > > > > > > > > >
> > > > > > > > > > FTR I've uploaded this for review purposes here:
> > > > > > > > > > https://linux-review.googlesource.com/c/linux/kernel/git/torvalds/linux/+/2458
> > > > > > > > > >
> > > > > > > > > > Diff from the previous version is available as:
> > > > > > > > > > https://linux-review.googlesource.com/c/linux/kernel/git/torvalds/linux/+/2458/1..2
> > > > > > > > > >
> > > > > > > > > > I've tested this locally and with syzkaller. This is :
> > > > > > > > > >
> > > > > > > > > > [   80.583021][    C3] Freed by task 0:
> > > > > > > > > > [   80.583480][    C3]  kasan_save_stack+0x1b/0x40 mm/kasan/common.c:49
> > > > > > > > > > [   80.584056][    C3]  kasan_set_track+0x1c/0x30 mm/kasan/common.c:57
> > > > > > > > > > [   80.584617][    C3]  kasan_set_free_info+0x1b/0x30 mm/kasan/generic.c:354
> > > > > > > > > > [   80.585221][    C3]  __kasan_slab_free+0xd8/0x120 mm/kasan/common.c:438
> > > > > > > > > > [   80.585814][    C3]  __cache_free mm/slab.c:3426 [inline]
> > > > > > > > > > [   80.585814][    C3]  kfree+0x10b/0x2b0 mm/slab.c:3757
> > > > > > > > > > [   80.586291][    C3]  kasan_rcu_reclaim+0x16/0x43 [test_kasan]
> > > > > > > > > > [   80.587009][    C3]  rcu_do_batch kernel/rcu/tree.c:2207 [inline]
> > > > > > > > > > [   80.587009][    C3]  rcu_core+0x59f/0x1370 kernel/rcu/tree.c:2434
> > > > > > > > > > [   80.587537][    C3]  __do_softirq+0x26c/0x9fa kernel/softirq.c:292
> > > > > > > > > > [   80.588085][    C3]
> > > > > > > > > > [   80.588367][    C3] Last one call_rcu() call stack:
> > > > > > > > > > [   80.589052][    C3]  kasan_save_stack+0x1b/0x40 mm/kasan/common.c:49
> > > > > > > > > > [   80.589622][    C3]  kasan_record_aux_stack+0x82/0xb0 mm/kasan/generic.c:345
> > > > > > > > > > [   80.590254][    C3]  __call_rcu kernel/rcu/tree.c:2672 [inline]
> > > > > > > > > > [   80.590254][    C3]  call_rcu+0x14f/0x7f0 kernel/rcu/tree.c:2746
> > > > > > > > > > [   80.590782][    C3]  kasan_rcu_uaf+0xe4/0xeb [test_kasan]
> > > > > > > > > > [   80.591697][    C3]  kmalloc_tests_init+0xbc/0x1097 [test_kasan]
> > > > > > > > > > [   80.592900][    C3]  do_one_initcall+0x10a/0x7d0 init/main.c:1196
> > > > > > > > > > [   80.593494][    C3]  do_init_module+0x1e6/0x6d0 kernel/module.c:3539
> > > > > > > > > > [   80.594066][    C3]  load_module+0x7464/0x9450 kernel/module.c:3890
> > > > > > > > > > [   80.594626][    C3]  __do_sys_init_module+0x1e3/0x220 kernel/module.c:3953
> > > > > > > > > > [   80.595265][    C3]  do_syscall_64+0xf6/0x7d0 arch/x86/entry/common.c:295
> > > > > > > > > > [   80.595822][    C3]  entry_SYSCALL_64_after_hwframe+0x49/0xb3
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > Overall this looks very good to me.
> > > > > > > > > > But there is one aspect that bothers me. In the previous patch you had
> > > > > > > > > > code that returned NULL from kasan_get_free_track() if the object is
> > > > > > > > > > live (which means free meta is not available, it's occupied by object
> > > > > > > > > > data). Now you dropped that code, but I think we still need it.
> > > > > > > > > > Otherwise we cast user object data to free meta and print the free
> > > > > > > > > > stack/pid from whatever garbage is there. This may lead to very
> > > > > > > > > > confusing output and potentially to crashes in stackdepot.
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > Yes, I totally agree with you. In the previous email I thought that
> > > > > > > > > there is a problem with free track, but I did not point it out. Thank
> > > > > > > > > you for pointing this problem. As you mentioned, we should fix it.
> > > > > > > > >
> > > > > > > > > > What do you think about this patch on top of your patches?
> > > > > > > > > > https://linux-review.googlesource.com/c/linux/kernel/git/torvalds/linux/+/2478
> > > > > > > > > > This way we very precisely mark the period of time when the object has
> > > > > > > > > > free track live and set.
> > > > > > > > > > If it looks good to you, feel free to incorporate it into your series.
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > Thank you for providing good idea solution.
> > > > > > > > >
> > > > > > > > > I saw this patch, that is a great patch. I think it can fix the issue
> > > > > > > > > which has garbage stack. it should work as described below.
> > > > > > > > >
> > > > > > > > > 1). When object is live, then don't print free stack.
> > > > > > > > > 2). When object is NOT alive, after free object:
> > > > > > > > > 2a). when object is in quarantine, then it can print free stack
> > > > > > > > > 2b). when object is NOT in quarantine, then it can NOT print free stack.
> > > > > > > > >
> > > > > > > > > I have a question about 2), why we don't directly use
> > > > > > > > > KASAN_KMALLOC_FREE? if we directly use it, then 2b) can print free
> > > > > > > > > stack? 2b) may has use-after-free? so that it may need free stack.
> > > > > > > >
> > > > > >
> > > > > > About 2b), I see another question. When do qlink_free(), it will be
> > > > > > written KASAN_KMALLOC_FREE from KASAN_KMALLOC_FREETRACK? if we don't
> > > > > > write shadow memory, it is still KASAN_KMALLOC_FREETRACK, then 2b) will
> > > > > > have free stack? Because I see you add KASAN_KMALLOC_FREETRACK to get
> > > > > > use-after-free in get_shadow_bug_type(). so should it not write
> > > > > > KASAN_KMALLOC_FREE?
> > > > >
> > > > > It may or may not work.
> > > > > The potential problem is that when qlink_free calls ___cache_free,
> > > > > slab/slub may start using object memory for its own purposes, e.g.
> > > > > store the next link. This next link may overwrite part of free meta.
> > > > > It actually may work because the slab/slib next link is likely to
> > > > > overlap with kasan_free_meta.quarantine_link only. And we may have
> > > > > kasan_free_meta.free_track intact while KASAN_KMALLOC_FREE is set. But
> > > > > this needs careful checking for both slab and slub and if they may use
> > > > > more than 1 word in some configurations.
> > > > >
> > > >
> > > > This problem looks like existing, even without this change? currently
> > > > KASAN may get wrong free stack?
> > >
> > > No, we should not have this problem now. Currently free track is
> > > stored in alloc meta. Alloc meta does not overlap with the object.
> > > It's only free meta that overlaps with the object and slab metadata at
> > > different periods of the block lifetime. Schematically what we have
> > > is:
> > >
> > > struct block_t {
> > >   alloc_meta kasan_alloc_meta;
> > >   union {
> > >     user_data char[N];
> > >     slab_meta slab_meta;
> > >     free_meta kasan_free_meta;
> > >   };
> > > }
> > >
> > > free_meta shared storage space with 2 other things.
> > >
> >
> > Ah...I forget it is stored in alloc mata, Yes, it should not have this
> > problem.
> >
> > Thanks for your detailed explanation
> >
> > > > Regardless of whether the shadow memory content is
> > > > KASAN_KMALLOC_FREETRACK or KASAN_KMALLOC_FREE, it may have this problem?
> > >
> > > KASAN_KMALLOC_FREETRACK is set only when nobody else uses the storage.
> > >
> >
> > Ok, I will use KASAN_KMALLOC_FREE. If you have any concerns, please tell me.
> > Thanks.
> 
> You mean KASAN_KMALLOC_FREETRACK?
> 

Yes, sorry, I made you misunderstand what I meant. I mean that qlink_free()
write KASAN_KMALLOC_FREE. 

> Or, you checked that using KASAN_KMALLOC_FREE is safe and will not
> cause any bad overlap?
> 

This item you refer to, maybe it can be done in the future.

> 
> > > > But because of kasan_get_free_track() have conditions to get free track,
> > > > so that if shadow memory content is KASAN_KMALLOC_FREE, then it will
> > > > avoid this problem and always print right free stack.
> > >
> > >
> > >
> > > > > > > > We can't use KASAN_KMALLOC_FREE because of this part:
> > > > > > > >
> > > > > > > > static bool __kasan_slab_free(struct kmem_cache *cache, void *object,
> > > > > > > >                   unsigned long ip, bool quarantine)
> > > > > > > > {
> > > > > > > > ...
> > > > > > > >     kasan_poison_shadow(object, rounded_up_size, KASAN_KMALLOC_FREE);
> > > > > > > >
> > > > > > > >     if ((IS_ENABLED(CONFIG_KASAN_GENERIC) && !quarantine) ||
> > > > > > > >             unlikely(!(cache->flags & SLAB_KASAN)))
> > > > > > > >         return false;
> > > > > > > >
> > > > > > > >     kasan_set_free_info(cache, object, tag);
> > > > > > > > ...
> > > > > > > >
> > > > > > >
> > > > > > > Ok, I see. When return false, then the shadow memory content has
> > > > > > > KASAN_KMALLOC_FREE, but it doesn't set free stack, so that we need to
> > > > > > > avoid this situation. Thank for you reminder.
> > > > > > >
> > > > > > > >
> > > > > > > > We may set KASAN_KMALLOC_FREE, but not set the track (or even have
> > > > > > > > memory for the track!).
> > > > > > > > The object may not have free meta allocated at all, e.g. very large
> > > > > > > > object with ctor (no place to store meta), or it may be in a mempool:
> > > > > > > > https://elixir.bootlin.com/linux/v5.7-rc6/source/mm/mempool.c#L109
> > > > > > > > and mempool may be using the object memory itself (for its own next
> > > > > > > > link or something).
> > > > > > > >
> > > > > > > > KASAN_KMALLOC_FREETRACK very explicitly tracks the exact condition we
> > > > > > > > want: we have meta info live now and we have free track set.
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v2 1/2] hwrng: iproc-rng200 - Set the quality value
From: Stephan Mueller @ 2020-05-20 11:53 UTC (permalink / raw)
  To: Lukasz Stelmach
  Cc: Florian Fainelli, Herbert Xu, Scott Branden, Matthias Brugger,
	Greg Kroah-Hartman, Matt Mackall, linux-kernel,
	Krzysztof Kozlowski, linux-samsung-soc, Bartlomiej Zolnierkiewicz,
	Kukjin Kim, Arnd Bergmann, Stefan Wahren, Ray Jui,
	bcm-kernel-feedback-list, Markus Elfring, linux-arm-kernel,
	linux-crypto
In-Reply-To: <dleftjh7wa3my6.fsf%l.stelmach@samsung.com>

Am Mittwoch, 20. Mai 2020, 12:44:33 CEST schrieb Lukasz Stelmach:

Hi Lukasz,

> It was <2020-05-20 śro 11:18>, when Stephan Mueller wrote:
> > Am Mittwoch, 20. Mai 2020, 11:10:32 CEST schrieb Lukasz Stelmach:
> >> It was <2020-05-20 śro 08:23>, when Stephan Mueller wrote:
> >>> Am Dienstag, 19. Mai 2020, 23:25:51 CEST schrieb Łukasz Stelmach:
> >>>> The value was estimaded with ea_iid[1] using on 10485760 bytes read
> >>>> from the RNG via /dev/hwrng. The min-entropy value calculated using
> >>>> the most common value estimate (NIST SP 800-90P[2], section 6.3.1)
> >>>> was 7.964464.
> >>> 
> >>> I am sorry, but I think I did not make myself clear: testing random
> >>> numbers post-processing with the statistical tools does NOT give any
> >>> idea about the entropy rate. Thus, all that was calculated is the
> >>> proper implementation of the post-processing operation and not the
> >>> actual noise source.
> >>> 
> >>> What needs to happen is that we need access to raw, unconditioned
> >>> data from the noise source that is analyzed with the statistical
> >>> methods.
> >> 
> >> I did understand you and I assure you the data I tested were obtained
> >> directly from RNGs. As I pointed before[1], that is how /dev/hwrng
> >> works[2].
> > 
> > I understand that /dev/hwrng pulls the data straight from the
> > hardware. But the data from the hardware usually is not obtained
> > straight from the noise source.
> > 
> > Typically you have a noise source (e.g. a ring oscillator) whose data
> > is digitized then fed into a compression function like an LFSR or a
> > hash. Then a cryptographic operation like a CBC-MAC, hash or even a
> > DRBG is applied to that data when the caller wants to have random
> > numbers.
> 
> I do understand your point (but not entirely, see below). [opinion]
> However, I am really not sure that this is a "typical" setting for a HW
> RNG, at least not among RNGs supported by Linux. Otherwise there would
> be no hw_random framework and no rngd(8) which are suppsed to
> post-process imperfectly random data from HW. [/opinion]

The hw_random framework only makes these hardware RNG accessible for in-kernel 
as well as user space use.
> 
> > In order to estimate entropy, we need the raw unconditioned data from
> > the, say, ring oscillator and not from the (cryptographic) output
> > operation.
> 
> Can you tell, why it matters in this case? If I understand correctly,
> the quality field describes not the randomness created by the noise
> generator but the one delivered by the driver to other software
> components.

The quality field is used by add_hwgenerator_randomness to increase the Linux 
RNG entropy estimator accordingly. This is the issue.

And giving an entropy rate based on post-processed data is meaningless.

The concern is, for example, that you use a DRBG that you seeded with, say, a 
zero buffer. You get perfect random data from it that no statistical test can 
disprove. Yet we know this data stream has zero entropy. Thus, we need to get 
to the source and assess its entropy.

> 
> > That said, the illustrated example is typical for hardware RNGs. Yet
> > it is never guaranteed to work that way. Thus, if you can point to
> > architecture documentation of your specific hardware RNGs showing that
> > the data read from the hardware is pure unconditioned noise data, then
> > I have no objections to the patch.
> 
> I can tell for sure that this is the case for exynos-trng[1].

So you are saying that the output for the exynos-trng is straight from a ring 
oscillator without any post-processing of any kind?

If this is the case, I would like to suggest you add that statement to the git 
commit message with that reference. If so, then I would withdraw my objection.

> There is a
> post-processor which I have forgotten about since writing the driver,
> because from the very beginning I didn't intend to use it. I knew there
> is the software framework for post-processing and simply didn't bother.
> 
> With regards to iproc-rng200 I cannot be sure.
> 
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/dri
> vers/char/hw_random/exynos-trng.c?h=v5.6#n100
> 
> Kind regards,


Ciao
Stephan



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v2 1/2] hwrng: iproc-rng200 - Set the quality value
From: Krzysztof Kozlowski @ 2020-05-20 12:00 UTC (permalink / raw)
  To: Stephan Mueller
  Cc: Florian Fainelli, Herbert Xu, Arnd Bergmann, Matthias Brugger,
	Greg Kroah-Hartman, Matt Mackall, Lukasz Stelmach, Scott Branden,
	linux-kernel@vger.kernel.org, linux-samsung-soc@vger.kernel.org,
	Bartlomiej Zolnierkiewicz, Kukjin Kim, bcm-kernel-feedback-list,
	Stefan Wahren, Ray Jui, Markus Elfring, linux-arm-kernel,
	linux-crypto
In-Reply-To: <2041475.ybOAuNAZB8@tauon.chronox.de>

On Wed, 20 May 2020 at 13:53, Stephan Mueller <smueller@chronox.de> wrote:
> > > That said, the illustrated example is typical for hardware RNGs. Yet
> > > it is never guaranteed to work that way. Thus, if you can point to
> > > architecture documentation of your specific hardware RNGs showing that
> > > the data read from the hardware is pure unconditioned noise data, then
> > > I have no objections to the patch.
> >
> > I can tell for sure that this is the case for exynos-trng[1].
>
> So you are saying that the output for the exynos-trng is straight from a ring
> oscillator without any post-processing of any kind?

Hi,

I think we will never be able to state this because the manual is
quite limited in sharing internals. What the driver does and probably
Lukasz wanted to say is that there is "post processing" block and
feature which can be disabled. The manual is saying the TRNG block
generates random data from thermal noise but not how much in a direct
way. There could be some simple post-processing or not (except the one
able to on/off). Also manual says this post processing block is there
to remove statistical weakness from the TRNG block. To me it does not
prove enough that raw data is really raw...

Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v3 0/6] arm64: add the time namespace support
From: Vincenzo Frascino @ 2020-05-20 12:02 UTC (permalink / raw)
  To: Andrei Vagin, Catalin Marinas, Will Deacon
  Cc: Mark Rutland, Thomas Gleixner, linux-kernel, linux-arm-kernel,
	Dmitry Safonov
In-Reply-To: <20200416052618.804515-1-avagin@gmail.com>

Hi Andrei,

On 4/16/20 6:26 AM, Andrei Vagin wrote:
> Allocate the time namespace page among VVAR pages and add the logic
> to handle faults on VVAR properly.
> 
> If a task belongs to a time namespace then the VVAR page which contains
> the system wide VDSO data is replaced with a namespace specific page
> which has the same layout as the VVAR page. That page has vdso_data->seq
> set to 1 to enforce the slow path and vdso_data->clock_mode set to
> VCLOCK_TIMENS to enforce the time namespace handling path.
> 
> The extra check in the case that vdso_data->seq is odd, e.g. a concurrent
> update of the VDSO data is in progress, is not really affecting regular
> tasks which are not part of a time namespace as the task is spin waiting
> for the update to finish and vdso_data->seq to become even again.
> 
> If a time namespace task hits that code path, it invokes the corresponding
> time getter function which retrieves the real VVAR page, reads host time
> and then adds the offset for the requested clock which is stored in the
> special VVAR page.
> 
> v2: Code cleanups suggested by Vincenzo.
> v3: use OPTIMIZER_HIDE_VAR() instead of inline assembly in
>     __arch_get_timens_vdso_data.
> 

Nit: If you re-post, I would remove the OPTIMIZER_HIDE_VAR() reference because
it does not reflect the current status of the patches.

I tested it again with your latest change in the test code and it works for me
(thank you for sending a patch for the test as well).

With this:

Reviewed-by: Vincenzo Frascino <vincenzo.frascino@arm.com>

> Cc: Vincenzo Frascino <vincenzo.frascino@arm.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Dmitry Safonov <dima@arista.com>
> 
> v3 on github (if someone prefers `git pull` to `git am`):
> https://github.com/avagin/linux-task-diag/tree/arm64/timens-v3
> 
> Andrei Vagin (6):
>   arm64/vdso: use the fault callback to map vvar pages
>   arm64/vdso: Zap vvar pages when switching to a time namespace
>   arm64/vdso: Add time napespace page
>   arm64/vdso: Handle faults on timens page
>   arm64/vdso: Restrict splitting VVAR VMA
>   arm64: enable time namespace support
> 
>  arch/arm64/Kconfig                            |   1 +
>  .../include/asm/vdso/compat_gettimeofday.h    |  11 ++
>  arch/arm64/include/asm/vdso/gettimeofday.h    |   8 ++
>  arch/arm64/kernel/vdso.c                      | 134 ++++++++++++++++--
>  arch/arm64/kernel/vdso/vdso.lds.S             |   3 +-
>  arch/arm64/kernel/vdso32/vdso.lds.S           |   3 +-
>  include/vdso/datapage.h                       |   1 +
>  7 files changed, 147 insertions(+), 14 deletions(-)
> 

-- 
Regards,
Vincenzo

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH] bus: arm-integrator-lm: Fix an IS_ERR() vs NULL check
From: Dan Carpenter @ 2020-05-20 12:08 UTC (permalink / raw)
  To: Linus Walleij; +Cc: kernel-janitors, linux-kernel, linux-arm-kernel

The of_find_matching_node() function returns NULL on error, it never
returns error pointers.  This doesn't really impact runtime very much
because if "syscon" is NULL then syscon_node_to_regmap() will return
-EINVAL.  The only runtime difference is that now it returns -ENODEV.

Fixes: ccea5e8a5918 ("bus: Add driver for Integrator/AP logic modules")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
The first patch which added this file doesn't give a good hint what the
subsystem prefix should be so I just guessed "bus: arm-integrator-lm:".

 drivers/bus/arm-integrator-lm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/bus/arm-integrator-lm.c b/drivers/bus/arm-integrator-lm.c
index 669ea7e1f92e..845b6c43fef8 100644
--- a/drivers/bus/arm-integrator-lm.c
+++ b/drivers/bus/arm-integrator-lm.c
@@ -78,10 +78,10 @@ static int integrator_ap_lm_probe(struct platform_device *pdev)
 
 	/* Look up the system controller */
 	syscon = of_find_matching_node(NULL, integrator_ap_syscon_match);
-	if (IS_ERR(syscon)) {
+	if (!syscon) {
 		dev_err(dev,
 			"could not find Integrator/AP system controller\n");
-		return PTR_ERR(syscon);
+		return -ENODEV;
 	}
 	map = syscon_node_to_regmap(syscon);
 	if (IS_ERR(map)) {
-- 
2.26.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: [PATCH v2 1/2] hwrng: iproc-rng200 - Set the quality value
From: Stephan Mueller @ 2020-05-20 12:11 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Florian Fainelli, Herbert Xu, Arnd Bergmann, Matthias Brugger,
	Greg Kroah-Hartman, Matt Mackall, Lukasz Stelmach, Scott Branden,
	linux-kernel@vger.kernel.org, linux-samsung-soc@vger.kernel.org,
	Bartlomiej Zolnierkiewicz, Kukjin Kim, bcm-kernel-feedback-list,
	Stefan Wahren, Ray Jui, Markus Elfring, linux-arm-kernel,
	linux-crypto
In-Reply-To: <CAJKOXPeBkZ3R2wT9-A8LWkFx0W9KY70VW7JNjwp0RMDRc7hkTg@mail.gmail.com>

Am Mittwoch, 20. Mai 2020, 14:00:01 CEST schrieb Krzysztof Kozlowski:

Hi Krzysztof,

> On Wed, 20 May 2020 at 13:53, Stephan Mueller <smueller@chronox.de> wrote:
> > > > That said, the illustrated example is typical for hardware RNGs. Yet
> > > > it is never guaranteed to work that way. Thus, if you can point to
> > > > architecture documentation of your specific hardware RNGs showing that
> > > > the data read from the hardware is pure unconditioned noise data, then
> > > > I have no objections to the patch.
> > > 
> > > I can tell for sure that this is the case for exynos-trng[1].
> > 
> > So you are saying that the output for the exynos-trng is straight from a
> > ring oscillator without any post-processing of any kind?
> 
> Hi,
> 
> I think we will never be able to state this because the manual is
> quite limited in sharing internals. What the driver does and probably
> Lukasz wanted to say is that there is "post processing" block and
> feature which can be disabled. The manual is saying the TRNG block
> generates random data from thermal noise but not how much in a direct
> way. There could be some simple post-processing or not (except the one
> able to on/off). Also manual says this post processing block is there
> to remove statistical weakness from the TRNG block. To me it does not
> prove enough that raw data is really raw...

Unterstood, but can't that statement be added to the commit message?
> 
> Best regards,
> Krzysztof


Ciao
Stephan



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox