This patch adds support for firmware update to the in-tree igb driver and it is actually a port from the out-of-tree igb driver.
In-band firmware update is one of the essential system maintenance tasks. To simplify this task, the Intel online firmware update
utility provides a common interface that works across different Intel NICs running the igb/ixgbe/i40e drivers. Unfortunately, the
in-tree igb and ixgbe drivers are unable to support this firmware update utility, causing problems for enterprise users who do not
or cannot use out-of-distro drivers due to security and various other reasons (e.g. commercial Linux distros do not provide technical
support for out-of-distro drivers). As a result, getting this feature into the in-tree igb driver is highly desirable.
Signed-off-by: Richard chien <[email protected]>
---
.../net/ethernet/intel/igb/e1000_defines.h | 1 +
drivers/net/ethernet/intel/igb/e1000_hw.h | 59 +++
drivers/net/ethernet/intel/igb/e1000_nvm.c | 51 +++
drivers/net/ethernet/intel/igb/e1000_nvm.h | 4 +
drivers/net/ethernet/intel/igb/e1000_regs.h | 9 +
drivers/net/ethernet/intel/igb/igb_ethtool.c | 378 ++++++++++++------
drivers/net/ethernet/intel/igb/igb_main.c | 34 ++
7 files changed, 424 insertions(+), 112 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/e1000_defines.h b/drivers/net/ethernet/intel/igb/e1000_defines.h
index fa0289284..2fcf7621a 100644
--- a/drivers/net/ethernet/intel/igb/e1000_defines.h
+++ b/drivers/net/ethernet/intel/igb/e1000_defines.h
@@ -481,6 +481,7 @@
#define E1000_RAH_POOL_1 0x00040000
/* Error Codes */
+#define E1000_SUCCESS 0
#define E1000_ERR_NVM 1
#define E1000_ERR_PHY 2
#define E1000_ERR_CONFIG 3
diff --git a/drivers/net/ethernet/intel/igb/e1000_hw.h b/drivers/net/ethernet/intel/igb/e1000_hw.h
index 44111f65a..bbdbb7198 100644
--- a/drivers/net/ethernet/intel/igb/e1000_hw.h
+++ b/drivers/net/ethernet/intel/igb/e1000_hw.h
@@ -292,6 +292,35 @@ struct e1000_host_mng_command_info {
#include "e1000_nvm.h"
#include "e1000_mbx.h"
+/* NVM Update commands */
+#define E1000_NVMUPD_CMD_REG_READ 0x0000000B
+#define E1000_NVMUPD_CMD_REG_WRITE 0x0000000C
+
+/* NVM Update features API */
+#define E1000_NVMUPD_FEATURES_API_VER_MAJOR 0
+#define E1000_NVMUPD_FEATURES_API_VER_MINOR 0
+#define E1000_NVMUPD_FEATURES_API_FEATURES_ARRAY_LEN 12
+#define E1000_NVMUPD_EXEC_FEATURES 0xE
+#define E1000_NVMUPD_FEATURE_FLAT_NVM_SUPPORT (1 << 0)
+#define E1000_NVMUPD_FEATURE_REGISTER_ACCESS_SUPPORT (1 << 1)
+
+#define E1000_NVMUPD_MOD_PNT_MASK 0xFF
+
+struct e1000_nvm_access {
+ u32 command;
+ u32 config;
+ u32 offset; /* in bytes */
+ u32 data_size; /* in bytes */
+ u8 data[1];
+};
+
+struct e1000_nvm_features {
+ u8 major;
+ u8 minor;
+ u16 size;
+ u8 features[E1000_NVMUPD_FEATURES_API_FEATURES_ARRAY_LEN];
+};
+
struct e1000_mac_operations {
s32 (*check_for_link)(struct e1000_hw *);
s32 (*reset_hw)(struct e1000_hw *);
@@ -539,6 +568,8 @@ struct e1000_hw {
u16 vendor_id;
u8 revision_id;
+ /* NVM Update features */
+ struct e1000_nvm_features nvmupd_features;
};
struct net_device *igb_get_hw_dev(struct e1000_hw *hw);
@@ -551,4 +582,32 @@ s32 igb_write_pcie_cap_reg(struct e1000_hw *hw, u32 reg, u16 *value);
void igb_read_pci_cfg(struct e1000_hw *hw, u32 reg, u16 *value);
void igb_write_pci_cfg(struct e1000_hw *hw, u32 reg, u16 *value);
+
+
+u32 e1000_read_reg(struct e1000_hw *hw, u32 reg);
+
+#define E1000_WRITE_REG(hw, reg, val) \
+do { \
+ u8 __iomem *hw_addr = READ_ONCE((hw)->hw_addr); \
+ if (!E1000_REMOVED(hw_addr)) \
+ writel((val), &hw_addr[(reg)]); \
+} while (0)
+
+#define E1000_READ_REG(x, y) e1000_read_reg(x, y)
+#define E1000_READ_REG8(h, r) readb(READ_ONCE(h->hw_addr) + r)
+
+#define E1000_WRITE_FLASH_REG(a, reg, value) ( \
+ writel((value), ((a)->flash_address + reg)))
+
+//#define E1000_READ_FLASH_REG(a, reg) (readl((a)->flash_address + reg))
+
+//#define E1000_READ_FLASH_REG16(a, reg) (readw((a)->flash_address + reg))
+
+
+#define E1000_READ_FLASH_REG(a, reg) (readl((a)->flash_address + reg))
+
+#define E1000_READ_FLASH_REG8(a, reg) ( \
+ readb(READ_ONCE((a)->flash_address) + reg))
+
+
#endif /* _E1000_IGB_HW_H_ */
diff --git a/drivers/net/ethernet/intel/igb/e1000_nvm.c b/drivers/net/ethernet/intel/igb/e1000_nvm.c
index 2dcd64d6d..e3635f3fd 100644
--- a/drivers/net/ethernet/intel/igb/e1000_nvm.c
+++ b/drivers/net/ethernet/intel/igb/e1000_nvm.c
@@ -778,3 +778,54 @@ void igb_get_fw_version(struct e1000_hw *hw, struct e1000_fw_version *fw_vers)
| eeprom_verl;
}
}
+
+/**
+ * e1000_read_nvm - Reads NVM (EEPROM)
+ * @hw: pointer to the HW structure
+ * @offset: the word offset to read
+ * @words: number of 16-bit words to read
+ * @data: pointer to the properly sized buffer for the data.
+ *
+ * Reads 16-bit chunks of data from the NVM (EEPROM). This is a function
+ * pointer entry point called by drivers.
+ **/
+s32 e1000_read_nvm(struct e1000_hw *hw, u16 offset, u16 words, u16 *data)
+{
+ if (hw->nvm.ops.read)
+ return hw->nvm.ops.read(hw, offset, words, data);
+
+ return -E1000_ERR_CONFIG;
+}
+
+/**
+ * e1000_write_nvm - Writes to NVM (EEPROM)
+ * @hw: pointer to the HW structure
+ * @offset: the word offset to read
+ * @words: number of 16-bit words to write
+ * @data: pointer to the properly sized buffer for the data.
+ *
+ * Writes 16-bit chunks of data to the NVM (EEPROM). This is a function
+ * pointer entry point called by drivers.
+ **/
+s32 e1000_write_nvm(struct e1000_hw *hw, u16 offset, u16 words, u16 *data)
+{
+ if (hw->nvm.ops.write)
+ return hw->nvm.ops.write(hw, offset, words, data);
+
+ return E1000_SUCCESS;
+}
+
+/**
+ * e1000_update_nvm_checksum - Updates NVM (EEPROM) checksum
+ * @hw: pointer to the HW structure
+ *
+ * Updates the NVM checksum. Currently no func pointer exists and all
+ * implementations are handled in the generic version of this function.
+ **/
+s32 e1000_update_nvm_checksum(struct e1000_hw *hw)
+{
+ if (hw->nvm.ops.update)
+ return hw->nvm.ops.update(hw);
+
+ return -E1000_ERR_CONFIG;
+}
diff --git a/drivers/net/ethernet/intel/igb/e1000_nvm.h b/drivers/net/ethernet/intel/igb/e1000_nvm.h
index 091cddf4a..6584a0a7a 100644
--- a/drivers/net/ethernet/intel/igb/e1000_nvm.h
+++ b/drivers/net/ethernet/intel/igb/e1000_nvm.h
@@ -33,4 +33,8 @@ struct e1000_fw_version {
};
void igb_get_fw_version(struct e1000_hw *hw, struct e1000_fw_version *fw_vers);
+s32 e1000_read_nvm(struct e1000_hw *hw, u16 offset, u16 words, u16 *data);
+s32 e1000_write_nvm(struct e1000_hw *hw, u16 offset, u16 words, u16 *data);
+s32 e1000_update_nvm_checksum(struct e1000_hw *hw);
+
#endif
diff --git a/drivers/net/ethernet/intel/igb/e1000_regs.h b/drivers/net/ethernet/intel/igb/e1000_regs.h
index eb9f6da92..eae551959 100644
--- a/drivers/net/ethernet/intel/igb/e1000_regs.h
+++ b/drivers/net/ethernet/intel/igb/e1000_regs.h
@@ -9,8 +9,10 @@
#define E1000_EECD 0x00010 /* EEPROM/Flash Control - RW */
#define E1000_EERD 0x00014 /* EEPROM Read - RW */
#define E1000_CTRL_EXT 0x00018 /* Extended Device Control - RW */
+#define E1000_FLA 0x0001C /* Flash Access - RW */
#define E1000_MDIC 0x00020 /* MDI Control - RW */
#define E1000_MDICNFG 0x00E04 /* MDI Config - RW */
+#define E1000_REGISTER_SET_SIZE 0x20000 /* CSR Size */
#define E1000_SCTL 0x00024 /* SerDes Control - RW */
#define E1000_FCAL 0x00028 /* Flow Control Address Low - RW */
#define E1000_FCAH 0x0002C /* Flow Control Address High -RW */
@@ -49,6 +51,7 @@
#define E1000_EEMNGCTL_I210 0x12030 /* MNG EEprom Control */
#define E1000_EEARBC_I210 0x12024 /* EEPROM Auto Read Bus Control */
#define E1000_EEWR 0x0102C /* EEPROM Write Register - RW */
+#define E1000_FLOP 0x0103C /* FLASH Opcode Register */
#define E1000_I2CCMD 0x01028 /* SFPI2C Command Register - RW */
#define E1000_FRTIMER 0x01048 /* Free Running Timer - RW */
#define E1000_TCPTIMER 0x0104C /* TCP Timer - RW */
@@ -66,6 +69,7 @@
#define E1000_MPHY_ADDR_CTRL 0x0024 /* GbE MPHY Address Control */
#define E1000_MPHY_DATA 0x0E10 /* GBE MPHY Data */
#define E1000_MPHY_STAT 0x0E0C /* GBE MPHY Statistics */
+#define E1000_I350_BARCTRL 0x5BFC /* BAR ctrl reg */
/* IEEE 1588 TIMESYNCH */
#define E1000_TSYNCRXCTL 0x0B620 /* Rx Time Sync Control register - RW */
@@ -116,6 +120,7 @@
#define E1000_DMCRTRH 0x05DD0 /* Receive Packet Rate Threshold */
#define E1000_DMCCNT 0x05DD4 /* Current Rx Count */
#define E1000_FCRTC 0x02170 /* Flow Control Rx high watermark */
+#define E1000_PCIEMISC 0x05BB8 /* PCIE misc config register */
/* TX Rate Limit Registers */
#define E1000_RTTDQSEL 0x3604 /* Tx Desc Plane Queue Select - WO */
@@ -390,6 +395,7 @@ do { \
#define E1000_O2BSPC 0x0415C /* OS2BMC packets transmitted by host */
#define E1000_SRWR 0x12018 /* Shadow Ram Write Register - RW */
+#define E1000_EEC_REG 0x12010
#define E1000_I210_FLMNGCTL 0x12038
#define E1000_I210_FLMNGDATA 0x1203C
#define E1000_I210_FLMNGCNT 0x12040
@@ -400,6 +406,9 @@ do { \
#define E1000_I210_FLA 0x1201C
+#define E1000_SHADOWINF 0x12068
+#define E1000_FLFWUPDATE 0x12108
+
#define E1000_I210_DTXMXPKTSZ 0x355C
#define E1000_I210_TXDCTL(_n) (0x0E028 + ((_n) * 0x40))
diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
index 61d72250c..ebed72a3e 100644
--- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
+++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
@@ -724,128 +724,282 @@ static void igb_get_regs(struct net_device *netdev,
regs_buff[739] = rd32(E1000_I210_RR2DCDELAY);
}
-static int igb_get_eeprom_len(struct net_device *netdev)
-{
- struct igb_adapter *adapter = netdev_priv(netdev);
- return adapter->hw.nvm.word_size * 2;
+static u8 igb_nvmupd_get_module(u32 val)
+{
+ return (u8)(val & E1000_NVMUPD_MOD_PNT_MASK);
+}
+
+static int igb_nvmupd_validate_offset(struct igb_adapter *adapter, u32 offset)
+{
+ if (offset >= E1000_REGISTER_SET_SIZE)
+ return 0;
+
+ switch (offset) {
+ case E1000_CTRL:
+ case E1000_STATUS:
+ case E1000_EECD:
+ case E1000_EERD:
+ case E1000_CTRL_EXT:
+ case E1000_FLA:
+ case E1000_FLOP:
+ case E1000_SWSM:
+ case E1000_FWSM:
+ case E1000_SW_FW_SYNC:
+ case E1000_IOVTCL:
+ case E1000_I350_BARCTRL:
+ case E1000_THSTAT:
+ case E1000_EEC_REG:
+ case E1000_SRWR:
+ case E1000_I210_FLA:
+ case E1000_I210_FLSWCTL:
+ case E1000_I210_FLSWDATA:
+ case E1000_I210_FLSWCNT:
+ case E1000_SHADOWINF:
+ case E1000_FLFWUPDATE:
+ case E1000_RAL(0):
+ case E1000_RAL(1):
+ case E1000_RAL(2):
+ case E1000_RAL(3):
+ case E1000_RAL(4):
+ case E1000_RAL(5):
+ case E1000_RAL(6):
+ case E1000_RAL(7):
+ case E1000_RAL(8):
+ case E1000_RAL(9):
+ case E1000_RAL(10):
+ case E1000_RAL(11):
+ case E1000_RAL(12):
+ case E1000_RAL(13):
+ case E1000_RAL(14):
+ case E1000_RAL(15):
+ case E1000_RAH(0):
+ case E1000_RAH(1):
+ case E1000_RAH(2):
+ case E1000_RAH(3):
+ case E1000_RAH(4):
+ case E1000_RAH(5):
+ case E1000_RAH(6):
+ case E1000_RAH(7):
+ case E1000_RAH(8):
+ case E1000_RAH(9):
+ case E1000_RAH(10):
+ case E1000_RAH(11):
+ case E1000_RAH(12):
+ case E1000_RAH(13):
+ case E1000_RAH(14):
+ case E1000_RAH(15):
+ return 0;
+ default:
+ dev_warn(&adapter->pdev->dev, "Bad offset: %x\n", offset);
+ return -ENOTTY;
+ }
+}
+
+static int igb_nvmupd_command(struct e1000_hw *hw,
+ struct e1000_nvm_access *nvm,
+ u8 *bytes)
+{
+ struct igb_adapter *adapter = hw->back;
+ resource_size_t bar0_len;
+ int ret_val = 0;
+ u32 command;
+ u8 module;
+
+ bar0_len = pci_resource_len(adapter->pdev, 0);
+ command = nvm->command;
+ module = igb_nvmupd_get_module(nvm->config);
+
+ switch (command) {
+ case E1000_NVMUPD_CMD_REG_READ:
+ switch (module) {
+ case E1000_NVMUPD_EXEC_FEATURES:
+ if (nvm->data_size == hw->nvmupd_features.size)
+ memcpy(bytes, &hw->nvmupd_features,
+ hw->nvmupd_features.size);
+ else
+ ret_val = -ENOMEM;
+ break;
+ default:
+ if (igb_nvmupd_validate_offset(adapter, nvm->offset))
+ return -ENOTTY;
+ if (nvm->offset >= bar0_len) {
+ if (hw->mac.type == e1000_82576 &&
+ hw->flash_address) {
+ if (nvm->data_size == 1)
+ *bytes = E1000_READ_FLASH_REG8(
+ hw,
+ nvm->offset - bar0_len);
+ else
+ *((u32 *)bytes) =
+ E1000_READ_FLASH_REG(hw,
+ nvm->offset - bar0_len);
+ } else
+ ret_val = -EFAULT;
+ } else if (nvm->data_size == 1)
+ *bytes = E1000_READ_REG8(hw, nvm->offset);
+ else
+ *((u32 *)bytes) = E1000_READ_REG(hw,
+ nvm->offset);
+ break;
+ }
+ break;
+ case E1000_NVMUPD_CMD_REG_WRITE:
+ if (igb_nvmupd_validate_offset(adapter, nvm->offset))
+ return -ENOTTY;
+ if (nvm->offset >= bar0_len) {
+ if (hw->mac.type == e1000_82576 && hw->flash_address)
+ E1000_WRITE_FLASH_REG(hw,
+ nvm->offset - bar0_len,
+ *((u32 *)bytes));
+ else
+ ret_val = -EFAULT;
+ } else
+ E1000_WRITE_REG(hw, nvm->offset, *((u32 *)bytes));
+ break;
+ }
+
+ return ret_val;
}
-static int igb_get_eeprom(struct net_device *netdev,
- struct ethtool_eeprom *eeprom, u8 *bytes)
+static int igb_get_eeprom_len(struct net_device *netdev)
{
- struct igb_adapter *adapter = netdev_priv(netdev);
- struct e1000_hw *hw = &adapter->hw;
- u16 *eeprom_buff;
- int first_word, last_word;
- int ret_val = 0;
- u16 i;
-
- if (eeprom->len == 0)
- return -EINVAL;
-
- eeprom->magic = hw->vendor_id | (hw->device_id << 16);
-
- first_word = eeprom->offset >> 1;
- last_word = (eeprom->offset + eeprom->len - 1) >> 1;
+ struct igb_adapter *adapter = netdev_priv(netdev);
+ struct pci_dev *pdev = adapter->pdev;
- eeprom_buff = kmalloc_array(last_word - first_word + 1, sizeof(u16),
- GFP_KERNEL);
- if (!eeprom_buff)
- return -ENOMEM;
-
- if (hw->nvm.type == e1000_nvm_eeprom_spi)
- ret_val = hw->nvm.ops.read(hw, first_word,
- last_word - first_word + 1,
- eeprom_buff);
- else {
- for (i = 0; i < last_word - first_word + 1; i++) {
- ret_val = hw->nvm.ops.read(hw, first_word + i, 1,
- &eeprom_buff[i]);
- if (ret_val)
- break;
- }
- }
-
- /* Device's eeprom is always little-endian, word addressable */
- for (i = 0; i < last_word - first_word + 1; i++)
- le16_to_cpus(&eeprom_buff[i]);
+ if (adapter->hw.mac.type == e1000_82576)
+ return pci_resource_len(pdev, 0) + pci_resource_len(pdev, 1);
- memcpy(bytes, (u8 *)eeprom_buff + (eeprom->offset & 1),
- eeprom->len);
- kfree(eeprom_buff);
+ return pci_resource_len(pdev, 0);
+}
- return ret_val;
+static int igb_get_eeprom(struct net_device *netdev,
+ struct ethtool_eeprom *eeprom, u8 *bytes)
+{
+ struct igb_adapter *adapter = netdev_priv(netdev);
+ struct e1000_hw *hw = &adapter->hw;
+ u16 *eeprom_buff;
+ int first_word, last_word;
+ int ret_val = 0;
+ struct e1000_nvm_access *nvm;
+ u32 magic;
+ u16 i;
+
+ if (eeprom->len == 0)
+ return -EINVAL;
+
+ magic = hw->vendor_id | (hw->device_id << 16);
+ if (eeprom->magic && eeprom->magic != magic) {
+ nvm = (struct e1000_nvm_access *)eeprom;
+ ret_val = igb_nvmupd_command(hw, nvm, bytes);
+ return ret_val;
+ }
+
+ /* normal ethtool get_eeprom support */
+ eeprom->magic = hw->vendor_id | (hw->device_id << 16);
+
+ first_word = eeprom->offset >> 1;
+ last_word = (eeprom->offset + eeprom->len - 1) >> 1;
+
+ eeprom_buff = kmalloc(sizeof(u16) *
+ (last_word - first_word + 1), GFP_KERNEL);
+ if (!eeprom_buff)
+ return -ENOMEM;
+
+ if (hw->nvm.type == e1000_nvm_eeprom_spi)
+ ret_val = e1000_read_nvm(hw, first_word,
+ last_word - first_word + 1,
+ eeprom_buff);
+ else {
+ for (i = 0; i < last_word - first_word + 1; i++) {
+ ret_val = e1000_read_nvm(hw, first_word + i, 1,
+ &eeprom_buff[i]);
+ if (ret_val)
+ break;
+ }
+ }
+
+ /* Device's eeprom is always little-endian, word addressable */
+ for (i = 0; i < last_word - first_word + 1; i++)
+ eeprom_buff[i] = le16_to_cpu(eeprom_buff[i]);
+
+ memcpy(bytes, (u8 *)eeprom_buff + (eeprom->offset & 1),
+ eeprom->len);
+ kfree(eeprom_buff);
+
+ return ret_val;
}
static int igb_set_eeprom(struct net_device *netdev,
- struct ethtool_eeprom *eeprom, u8 *bytes)
-{
- struct igb_adapter *adapter = netdev_priv(netdev);
- struct e1000_hw *hw = &adapter->hw;
- u16 *eeprom_buff;
- void *ptr;
- int max_len, first_word, last_word, ret_val = 0;
- u16 i;
-
- if (eeprom->len == 0)
- return -EOPNOTSUPP;
-
- if ((hw->mac.type >= e1000_i210) &&
- !igb_get_flash_presence_i210(hw)) {
- return -EOPNOTSUPP;
- }
-
- if (eeprom->magic != (hw->vendor_id | (hw->device_id << 16)))
- return -EFAULT;
-
- max_len = hw->nvm.word_size * 2;
-
- first_word = eeprom->offset >> 1;
- last_word = (eeprom->offset + eeprom->len - 1) >> 1;
- eeprom_buff = kmalloc(max_len, GFP_KERNEL);
- if (!eeprom_buff)
- return -ENOMEM;
-
- ptr = (void *)eeprom_buff;
-
- if (eeprom->offset & 1) {
- /* need read/modify/write of first changed EEPROM word
- * only the second byte of the word is being modified
- */
- ret_val = hw->nvm.ops.read(hw, first_word, 1,
- &eeprom_buff[0]);
- ptr++;
- }
- if (((eeprom->offset + eeprom->len) & 1) && (ret_val == 0)) {
- /* need read/modify/write of last changed EEPROM word
- * only the first byte of the word is being modified
- */
- ret_val = hw->nvm.ops.read(hw, last_word, 1,
- &eeprom_buff[last_word - first_word]);
- if (ret_val)
- goto out;
- }
-
- /* Device's eeprom is always little-endian, word addressable */
- for (i = 0; i < last_word - first_word + 1; i++)
- le16_to_cpus(&eeprom_buff[i]);
-
- memcpy(ptr, bytes, eeprom->len);
-
- for (i = 0; i < last_word - first_word + 1; i++)
- cpu_to_le16s(&eeprom_buff[i]);
-
- ret_val = hw->nvm.ops.write(hw, first_word,
- last_word - first_word + 1, eeprom_buff);
-
- /* Update the checksum if nvm write succeeded */
- if (ret_val == 0)
- hw->nvm.ops.update(hw);
-
- igb_set_fw_version(adapter);
+ struct ethtool_eeprom *eeprom, u8 *bytes)
+{
+ struct igb_adapter *adapter = netdev_priv(netdev);
+ struct e1000_hw *hw = &adapter->hw;
+ u16 *eeprom_buff;
+ void *ptr;
+ int max_len, first_word, last_word, ret_val = 0;
+ struct e1000_nvm_access *nvm;
+ u32 magic;
+ u16 i;
+
+ if (eeprom->len == 0)
+ return -EOPNOTSUPP;
+
+ magic = hw->vendor_id | (hw->device_id << 16);
+ if (eeprom->magic && eeprom->magic != magic) {
+ nvm = (struct e1000_nvm_access *)eeprom;
+ ret_val = igb_nvmupd_command(hw, nvm, bytes);
+ return ret_val;
+ }
+
+ /* normal ethtool get_eeprom support */
+ if (eeprom->magic != (hw->vendor_id | (hw->device_id << 16)))
+ return -EFAULT;
+
+ max_len = hw->nvm.word_size * 2;
+
+ first_word = eeprom->offset >> 1;
+ last_word = (eeprom->offset + eeprom->len - 1) >> 1;
+ eeprom_buff = kmalloc(max_len, GFP_KERNEL);
+ if (!eeprom_buff)
+ return -ENOMEM;
+
+ ptr = (void *)eeprom_buff;
+
+ if (eeprom->offset & 1) {
+ /* need read/modify/write of first changed EEPROM word */
+ /* only the second byte of the word is being modified */
+ ret_val = e1000_read_nvm(hw, first_word, 1,
+ &eeprom_buff[0]);
+ ptr++;
+ }
+ if (((eeprom->offset + eeprom->len) & 1) && (ret_val == 0)) {
+ /* need read/modify/write of last changed EEPROM word */
+ /* only the first byte of the word is being modified */
+ ret_val = e1000_read_nvm(hw, last_word, 1,
+ &eeprom_buff[last_word - first_word]);
+ if (ret_val)
+ goto out;
+ }
+
+ /* Device's eeprom is always little-endian, word addressable */
+ for (i = 0; i < last_word - first_word + 1; i++)
+ le16_to_cpus(&eeprom_buff[i]);
+
+ memcpy(ptr, bytes, eeprom->len);
+
+ for (i = 0; i < last_word - first_word + 1; i++)
+ cpu_to_le16s(&eeprom_buff[i]);
+
+ ret_val = e1000_write_nvm(hw, first_word,
+ last_word - first_word + 1, eeprom_buff);
+
+ /* Update the checksum if write succeeded.
+ * and flush shadow RAM for 82573 controllers */
+ if (ret_val == 0)
+ e1000_update_nvm_checksum(hw);
out:
- kfree(eeprom_buff);
- return ret_val;
+ kfree(eeprom_buff);
+ return ret_val;
}
static void igb_get_drvinfo(struct net_device *netdev,
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index fce2930ae..06b97ed9a 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -1955,6 +1955,28 @@ static void igb_setup_tx_mode(struct igb_adapter *adapter)
"enabled" : "disabled");
}
+u32 e1000_read_reg(struct e1000_hw *hw, u32 reg)
+{
+ struct igb_adapter *igb = container_of(hw, struct igb_adapter, hw);
+ u8 __iomem *hw_addr = READ_ONCE(hw->hw_addr);
+ u32 value = 0;
+
+ if (E1000_REMOVED(hw_addr))
+ return ~value;
+
+ value = readl(&hw_addr[reg]);
+
+ /* reads should not return all F's */
+ if (!(~value) && (!reg || !(~readl(hw_addr)))) {
+ struct net_device *netdev = igb->netdev;
+
+ hw->hw_addr = NULL;
+ netdev_err(netdev, "PCIe link lost\n");
+ }
+
+ return value;
+}
+
/**
* igb_configure - configure the hardware for RX and TX
* @adapter: private board structure
@@ -4091,6 +4113,18 @@ static int igb_sw_init(struct igb_adapter *adapter)
adapter->flags &= ~IGB_FLAG_DMAC;
set_bit(__IGB_DOWN, &adapter->state);
+
+ /* NVM Update features structure initialization */
+ hw->nvmupd_features.major = E1000_NVMUPD_FEATURES_API_VER_MAJOR;
+ hw->nvmupd_features.minor = E1000_NVMUPD_FEATURES_API_VER_MINOR;
+ hw->nvmupd_features.size = sizeof(hw->nvmupd_features);
+ memset(hw->nvmupd_features.features, 0x0,
+ E1000_NVMUPD_FEATURES_API_FEATURES_ARRAY_LEN *
+ sizeof(*hw->nvmupd_features.features));
+
+ hw->nvmupd_features.features[0] =
+ E1000_NVMUPD_FEATURE_REGISTER_ACCESS_SUPPORT;
+
return 0;
}
--
2.40.1
> This patch adds support for firmware update to the in-tree igb driver and it is actually a port from the out-of-tree igb driver.
> In-band firmware update is one of the essential system maintenance tasks. To simplify this task, the Intel online firmware update
…
Please improve such a change description also according to word wrapping
because of more desirable text line lengths.
…
> +++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
> @@ -724,128 +724,282 @@ static void igb_get_regs(struct net_device *netdev,
> regs_buff[739] = rd32(E1000_I210_RR2DCDELAY);
> }
>
> -static int igb_get_eeprom_len(struct net_device *netdev)
> -{
> - struct igb_adapter *adapter = netdev_priv(netdev);
> - return adapter->hw.nvm.word_size * 2;
> +static u8 igb_nvmupd_get_module(u32 val)
> +{
> + return (u8)(val & E1000_NVMUPD_MOD_PNT_MASK);
> +}
…
Would you like to reconsider the indentation once more for your change approach?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst?h=v6.10-rc2#n18
Regards,
Markus
Hi Richard,
kernel test robot noticed the following build warnings:
[auto build test WARNING on tnguy-next-queue/dev-queue]
[also build test WARNING on tnguy-net-queue/dev-queue linus/master v6.10-rc3 next-20240607]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Richard-chien/igb-Add-support-for-firmware-update/20240609-162047
base: https://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue.git dev-queue
patch link: https://lore.kernel.org/r/20240609081526.5621-1-richard.chien%40hpe.com
patch subject: [PATCH] igb: Add support for firmware update
config: alpha-randconfig-r112-20240610 (https://download.01.org/0day-ci/archive/20240610/[email protected]/config)
compiler: alpha-linux-gcc (GCC) 13.2.0
reproduce: (https://download.01.org/0day-ci/archive/20240610/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
sparse warnings: (new ones prefixed by >>)
>> drivers/net/ethernet/intel/igb/igb_ethtool.c:923:34: sparse: sparse: cast to restricted __le16
drivers/net/ethernet/intel/igb/igb_ethtool.c: note: in included file (through include/linux/mmzone.h, include/linux/gfp.h, include/linux/stackdepot.h, ...):
include/linux/page-flags.h:240:46: sparse: sparse: self-comparison always evaluates to false
include/linux/page-flags.h:240:46: sparse: sparse: self-comparison always evaluates to false
vim +923 drivers/net/ethernet/intel/igb/igb_ethtool.c
874
875 static int igb_get_eeprom(struct net_device *netdev,
876 struct ethtool_eeprom *eeprom, u8 *bytes)
877 {
878 struct igb_adapter *adapter = netdev_priv(netdev);
879 struct e1000_hw *hw = &adapter->hw;
880 u16 *eeprom_buff;
881 int first_word, last_word;
882 int ret_val = 0;
883 struct e1000_nvm_access *nvm;
884 u32 magic;
885 u16 i;
886
887 if (eeprom->len == 0)
888 return -EINVAL;
889
890 magic = hw->vendor_id | (hw->device_id << 16);
891 if (eeprom->magic && eeprom->magic != magic) {
892 nvm = (struct e1000_nvm_access *)eeprom;
893 ret_val = igb_nvmupd_command(hw, nvm, bytes);
894 return ret_val;
895 }
896
897 /* normal ethtool get_eeprom support */
898 eeprom->magic = hw->vendor_id | (hw->device_id << 16);
899
900 first_word = eeprom->offset >> 1;
901 last_word = (eeprom->offset + eeprom->len - 1) >> 1;
902
903 eeprom_buff = kmalloc(sizeof(u16) *
904 (last_word - first_word + 1), GFP_KERNEL);
905 if (!eeprom_buff)
906 return -ENOMEM;
907
908 if (hw->nvm.type == e1000_nvm_eeprom_spi)
909 ret_val = e1000_read_nvm(hw, first_word,
910 last_word - first_word + 1,
911 eeprom_buff);
912 else {
913 for (i = 0; i < last_word - first_word + 1; i++) {
914 ret_val = e1000_read_nvm(hw, first_word + i, 1,
915 &eeprom_buff[i]);
916 if (ret_val)
917 break;
918 }
919 }
920
921 /* Device's eeprom is always little-endian, word addressable */
922 for (i = 0; i < last_word - first_word + 1; i++)
> 923 eeprom_buff[i] = le16_to_cpu(eeprom_buff[i]);
924
925 memcpy(bytes, (u8 *)eeprom_buff + (eeprom->offset & 1),
926 eeprom->len);
927 kfree(eeprom_buff);
928
929 return ret_val;
930 }
931
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On Sun, Jun 09, 2024 at 04:15:26PM +0800, Richard chien wrote:
> This patch adds support for firmware update to the in-tree igb driver and it is actually a port from the out-of-tree igb driver.
> In-band firmware update is one of the essential system maintenance tasks. To simplify this task, the Intel online firmware update
> utility provides a common interface that works across different Intel NICs running the igb/ixgbe/i40e drivers. Unfortunately, the
> in-tree igb and ixgbe drivers are unable to support this firmware update utility, causing problems for enterprise users who do not
> or cannot use out-of-distro drivers due to security and various other reasons (e.g. commercial Linux distros do not provide technical
> support for out-of-distro drivers). As a result, getting this feature into the in-tree igb driver is highly desirable.
I don't really follow what this code is doing, but it seems to make
ethtool -e and -E dual purpose? Please could you show examples of how
this is used.
Thanks
Andrew
On 6/9/2024 1:15 AM, Richard chien wrote:
> This patch adds support for firmware update to the in-tree igb driver and it is actually a port from the out-of-tree igb driver.
> In-band firmware update is one of the essential system maintenance tasks. To simplify this task, the Intel online firmware update
> utility provides a common interface that works across different Intel NICs running the igb/ixgbe/i40e drivers. Unfortunately, the
> in-tree igb and ixgbe drivers are unable to support this firmware update utility, causing problems for enterprise users who do not
> or cannot use out-of-distro drivers due to security and various other reasons (e.g. commercial Linux distros do not provide technical
> support for out-of-distro drivers). As a result, getting this feature into the in-tree igb driver is highly desirable.
>
> Signed-off-by: Richard chien <[email protected]>
The motivation to support updating flash with in-kernel drivers is good.
> static int igb_set_eeprom(struct net_device *netdev,
> + struct ethtool_eeprom *eeprom, u8 *bytes)
> +{
> + struct igb_adapter *adapter = netdev_priv(netdev);
> + struct e1000_hw *hw = &adapter->hw;
> + u16 *eeprom_buff;
> + void *ptr;
> + int max_len, first_word, last_word, ret_val = 0;
> + struct e1000_nvm_access *nvm;
> + u32 magic;
> + u16 i;
> +
> + if (eeprom->len == 0)
> + return -EOPNOTSUPP;
> +
> + magic = hw->vendor_id | (hw->device_id << 16);
> + if (eeprom->magic && eeprom->magic != magic) {
> + nvm = (struct e1000_nvm_access *)eeprom;
> + ret_val = igb_nvmupd_command(hw, nvm, bytes);
> + return ret_val;
> + }
> +
However, this implementation is wrong. It is exposing the
ETHTOOL_GEEPROM and ETHTOOL_SEEPROM interface and abusing it to
implement a non-standard interface that is custom to the out-of-tree
Intel drivers to support the flash update utility.
This implementation was widely rejected when discovered in i40e and in
submissions for the ice driver. It abuses the ETHTOOL_GEEPROM and
ETHTOOL_SEEPROM interface in order to allow tools to access the
hardware. The use violates the documented behavior of the ethtool
interface and breaks the intended functionality of ETHTOOL_GEEPROM and
ETHTOOL_SEEPROM.
The correct way to implement flash update is via the devlink dev flash
interface, using request_firmware, and implementing the entire update
process in the driver. The common portions of this could be done in a
shared module.
Attempting to support the broken legacy update that is supported by the
out-of-tree drivers is a non-starter for upstream. We (Intel) have known
this for some time, and this is why the patches and support have never
been published.
> However, this implementation is wrong. It is exposing the
> ETHTOOL_GEEPROM and ETHTOOL_SEEPROM interface and abusing it to
> implement a non-standard interface that is custom to the out-of-tree Intel
> drivers to support the flash update utility.
>
> This implementation was widely rejected when discovered in i40e and in
> submissions for the ice driver. It abuses the ETHTOOL_GEEPROM and
> ETHTOOL_SEEPROM interface in order to allow tools to access the hardware.
> The use violates the documented behavior of the ethtool interface and breaks
> the intended functionality of ETHTOOL_GEEPROM and ETHTOOL_SEEPROM.
Thank you for your detailed explanation.
> The correct way to implement flash update is via the devlink dev flash
> interface, using request_firmware, and implementing the entire update
> process in the driver. The common portions of this could be done in a shared
> module.
In that case, does Intel have a plan to implement this mechanism
in in-kernel drivers?
> Attempting to support the broken legacy update that is supported by the out-
> of-tree drivers is a non-starter for upstream. We (Intel) have known this for
> some time, and this is why the patches and support have never been
> published.
Although the utility in question has been enhanced to perform firmware
update against Intel 1G/10G NICs by using the /dev/mem, this method
would not work when the secure boot is enabled. Considering out-of-band
firmware update (via the BMC) is not supported for Intel 1G/10G NICs, it
would be desirable to have the support for the devlink dev flash interface
in in-kernel drivers (igb & ixgbe).
Thanks
Richard
On 6/11/2024 12:55 AM, Chien, Richard (Options Engineering) wrote:
>> However, this implementation is wrong. It is exposing the
>> ETHTOOL_GEEPROM and ETHTOOL_SEEPROM interface and abusing it to
>> implement a non-standard interface that is custom to the out-of-tree Intel
>> drivers to support the flash update utility.
>>
>> This implementation was widely rejected when discovered in i40e and in
>> submissions for the ice driver. It abuses the ETHTOOL_GEEPROM and
>> ETHTOOL_SEEPROM interface in order to allow tools to access the hardware.
>> The use violates the documented behavior of the ethtool interface and breaks
>> the intended functionality of ETHTOOL_GEEPROM and ETHTOOL_SEEPROM.
>
> Thank you for your detailed explanation.
>
>> The correct way to implement flash update is via the devlink dev flash
>> interface, using request_firmware, and implementing the entire update
>> process in the driver. The common portions of this could be done in a shared
>> module.
>
> In that case, does Intel have a plan to implement this mechanism
> in in-kernel drivers?
>
I'm not aware of active plans right now :(
>> Attempting to support the broken legacy update that is supported by the out-
>> of-tree drivers is a non-starter for upstream. We (Intel) have known this for
>> some time, and this is why the patches and support have never been
>> published.
>
> Although the utility in question has been enhanced to perform firmware
> update against Intel 1G/10G NICs by using the /dev/mem, this method
> would not work when the secure boot is enabled. Considering out-of-band
> firmware update (via the BMC) is not supported for Intel 1G/10G NICs, it
> would be desirable to have the support for the devlink dev flash interface
> in in-kernel drivers (igb & ixgbe).
>
Yes it would be great. I've tried explaining why the existing support
can't go upstream, and why this would be valuable. Unfortunately it
hasn't led to action internally. I'm also not sure if all of the flash
update logic is in anything released under public open source license,
which makes it challenging for someone outside in the community to work
on this.
> Thanks
> Richard
>