netdev - [PATCH net-next] r8169: improve interrupt handling

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <25f6212f-5c76-365f-79a2-faae269877d7@gmail.com>
Date:   Fri, 23 Feb 2018 22:32:51 +0100
From:   Heiner Kallweit <hkallweit1@...il.com>
To:     Realtek linux nic maintainers <nic_swsd@...ltek.com>,
        David Miller <davem@...emloft.net>
Cc:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: [PATCH net-next] r8169: improve interrupt handling

This patch improves few aspects of interrupt handling:
- update to current interrupt allocation API
  (use pci_alloc_irq_vectors() instead of deprecated pci_enable_msi())
- this implicitly will allocate a MSI-X interrupt if available
- get rid of flag RTL_FEATURE_MSI

Last but not least it enables a feature which was (I presume accidently)
disabled before. There are members of the RTL8169 family supporting MSI
(e.g. RTL8169SB), however MSI never got enabled because RTL_CFG_0 was
missing flag RTL_FEATURE_MSI.
An indicator for "accidently" is that statement "cfg2 |= MSIEnable;"
in rtl_try_msi() is dead code. cfg2 is used for chip versions <= 06
only and for all these chip versions RTL_FEATURE_MSI isn't set.

The patch works fine on a RTL8168evl (chip version 34).

The previously accidently disabled MSI support for the few members of
the old RTL8169 family supporting MSI isn't tested. The RTL8169SB
(chip version 04) I have at least still works in a PCI slot w/o MSI.

Signed-off-by: Heiner Kallweit <hkallweit1@...il.com>
---
 drivers/net/ethernet/realtek/r8169.c | 44 ++++++++++++++++--------------------
 1 file changed, 20 insertions(+), 24 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 96db3283e..4730db990 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -736,8 +736,7 @@ struct ring_info {
 };
 
 enum features {
-	RTL_FEATURE_MSI		= (1 << 0),
-	RTL_FEATURE_GMII	= (1 << 1),
+	RTL_FEATURE_GMII	= (1 << 0),
 };
 
 struct rtl8169_counters {
@@ -7847,7 +7846,7 @@ static int rtl8169_close(struct net_device *dev)
 
 	cancel_work_sync(&tp->wk.work);
 
-	free_irq(pdev->irq, dev);
+	pci_free_irq(pdev, 0, dev);
 
 	dma_free_coherent(&pdev->dev, R8169_RX_RING_BYTES, tp->RxDescArray,
 			  tp->RxPhyAddr);
@@ -7903,9 +7902,8 @@ static int rtl_open(struct net_device *dev)
 
 	rtl_request_firmware(tp);
 
-	retval = request_irq(pdev->irq, rtl8169_interrupt,
-			     (tp->features & RTL_FEATURE_MSI) ? 0 : IRQF_SHARED,
-			     dev->name, dev);
+	retval = pci_request_irq(pdev, 0, rtl8169_interrupt, NULL, dev,
+				 dev->name);
 	if (retval < 0)
 		goto err_release_fw_2;
 
@@ -8253,7 +8251,7 @@ static const struct rtl_cfg_info {
 		.region		= 2,
 		.align		= 8,
 		.event_slow	= SYSErr | LinkChg | RxOverflow,
-		.features	= RTL_FEATURE_GMII | RTL_FEATURE_MSI,
+		.features	= RTL_FEATURE_GMII,
 		.coalesce_info	= rtl_coalesce_info_8168_8136,
 		.default_ver	= RTL_GIGA_MAC_VER_11,
 	},
@@ -8263,32 +8261,32 @@ static const struct rtl_cfg_info {
 		.align		= 8,
 		.event_slow	= SYSErr | LinkChg | RxOverflow | RxFIFOOver |
 				  PCSTimeout,
-		.features	= RTL_FEATURE_MSI,
 		.coalesce_info	= rtl_coalesce_info_8168_8136,
 		.default_ver	= RTL_GIGA_MAC_VER_13,
 	}
 };
 
 /* Cfg9346_Unlock assumed. */
-static unsigned rtl_try_msi(struct rtl8169_private *tp,
-			    const struct rtl_cfg_info *cfg)
+static void rtl_alloc_irq(struct rtl8169_private *tp)
 {
 	void __iomem *ioaddr = tp->mmio_addr;
-	unsigned msi = 0;
 	u8 cfg2;
+	int ret;
 
-	cfg2 = RTL_R8(Config2) & ~MSIEnable;
-	if (cfg->features & RTL_FEATURE_MSI) {
-		if (pci_enable_msi(tp->pci_dev)) {
-			netif_info(tp, hw, tp->dev, "no MSI. Back to INTx.\n");
-		} else {
-			cfg2 |= MSIEnable;
-			msi = RTL_FEATURE_MSI;
-		}
+	ret = pci_alloc_irq_vectors(tp->pci_dev, 1, 1, PCI_IRQ_ALL_TYPES);
+	if (ret < 0) {
+		netif_err(tp, drv, tp->dev, "failed to allocate irq!\n");
+		return;
 	}
-	if (tp->mac_version <= RTL_GIGA_MAC_VER_06)
+
+	if (tp->mac_version <= RTL_GIGA_MAC_VER_06) {
+		RTL_W8(Cfg9346, Cfg9346_Unlock);
+		cfg2 = RTL_R8(Config2) & ~MSIEnable;
+		if (pci_dev_msi_enabled(tp->pci_dev))
+			cfg2 |= MSIEnable;
 		RTL_W8(Config2, cfg2);
-	return msi;
+		RTL_W8(Cfg9346, Cfg9346_Lock);
+	}
 }
 
 DECLARE_RTL_COND(rtl_link_list_ready_cond)
@@ -8497,9 +8495,7 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	chipset = tp->mac_version;
 	tp->txd_version = rtl_chip_infos[chipset].txd_version;
 
-	RTL_W8(Cfg9346, Cfg9346_Unlock);
-	tp->features |= rtl_try_msi(tp, cfg);
-	RTL_W8(Cfg9346, Cfg9346_Lock);
+	rtl_alloc_irq(tp);
 
 	/* override BIOS settings, use userspace tools to enable WOL */
 	__rtl8169_set_wol(tp, 0);
-- 
2.16.2