diff mbox series

ixgbe: Add support for firmware update

Message ID 20240609085735.6253-1-richard.chien@hpe.com
State Rejected
Headers show
Series ixgbe: Add support for firmware update | expand

Commit Message

Richard chien June 9, 2024, 8:57 a.m. UTC
This patch adds support for firmware update to the in-tree ixgbe driver and it is actually a port
from the out-of-tree ixgbe 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 ixgbe driver is highly desirable.

Signed-off-by: Richard chien <richard.chien@hpe.com>
---
 .../net/ethernet/intel/ixgbe/ixgbe_ethtool.c  | 360 +++++++++++++-----
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  11 +
 drivers/net/ethernet/intel/ixgbe/ixgbe_type.h |  37 ++
 3 files changed, 317 insertions(+), 91 deletions(-)

Comments

Andrew Lunn June 10, 2024, 7:36 p.m. UTC | #1
On Sun, Jun 09, 2024 at 04:57:35PM +0800, Richard chien wrote:
> This patch adds support for firmware update to the in-tree ixgbe driver and it is actually a port
> from the out-of-tree ixgbe 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 ixgbe driver is highly desirable.
> 
> Signed-off-by: Richard chien <richard.chien@hpe.com>

How about you work on one driver at a time, to learn about the
processes for submitting to the Linux kernel.

https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html#netdev-faq

https://docs.kernel.org/process/submitting-patches.html

https://www.kernel.org/doc/html/latest/process/coding-style.html

I would also think about why Intel has not submitted this code before?
Maybe because it does things the wrong way? Please look at how other
Ethernet drivers support firmware. Is it the same? It might be you
need to throw away this code and reimplement it to mainline standards,
maybe using devlink flash, or ethtool -f.

One additional question. Is the firmware part of linux-firmware? Given
this is Intel, i expect the firmware is distributeable, but have they
distributed it?

	Andrew
Chien, Richard (Options Engineering) June 11, 2024, 7:12 a.m. UTC | #2
> I would also think about why Intel has not submitted this code before?
> Maybe because it does things the wrong way? Please look at how other
> Ethernet drivers support firmware. Is it the same? It might be you need to
> throw away this code and reimplement it to mainline standards, maybe using
> devlink flash, or ethtool -f.

See Jacob's reply for details.
 
> One additional question. Is the firmware part of linux-firmware? Given this is
> Intel, i expect the firmware is distributeable, but have they distributed it?

It is the Intel 10G NIC firmware embedded into HPE firmware update packages and redistributed to the end user.

Thanks,
Richard
diff mbox series

Patch

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
index 6e6e6f184..3ce5c662a 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
@@ -993,114 +993,292 @@  static void ixgbe_get_regs(struct net_device *netdev,
 
 static int ixgbe_get_eeprom_len(struct net_device *netdev)
 {
-	struct ixgbe_adapter *adapter = netdev_priv(netdev);
-	return adapter->hw.eeprom.word_size * 2;
+        struct ixgbe_adapter *adapter = netdev_priv(netdev);
+
+        return pci_resource_len(adapter->pdev, 0);
+}
+
+static u8 ixgbe_nvmupd_get_module(u32 val)
+{
+        return (u8)(val & IXGBE_NVMUPD_MOD_PNT_MASK);
+}
+
+static int ixgbe_nvmupd_validate_offset(struct ixgbe_adapter *adapter,
+                                        u32 offset)
+{
+        struct net_device *netdev = adapter->netdev;
+
+        switch (offset) {
+        case IXGBE_STATUS:
+        case IXGBE_ESDP:
+        case IXGBE_MSCA:
+        case IXGBE_MSRWD:
+        case IXGBE_EEC_8259X:
+        case IXGBE_FLA_8259X:
+        case IXGBE_FLOP:
+        case IXGBE_SWSM_8259X:
+        case IXGBE_FWSM_8259X:
+        case IXGBE_FACTPS_8259X:
+        case IXGBE_GSSR:
+        case IXGBE_HICR:
+        case IXGBE_FWSTS:
+                return 0;
+        default:
+                if ((offset >= IXGBE_MAVTV(0) && offset <= IXGBE_MAVTV(7)) ||
+                    (offset >= IXGBE_RAL(0) && offset <= IXGBE_RAH(15)))
+                        return 0;
+        }
+
+        switch (adapter->hw.mac.type) {
+        case ixgbe_mac_82599EB:
+                switch (offset) {
+                case IXGBE_AUTOC:
+                case IXGBE_EERD:
+                case IXGBE_BARCTRL:
+                        return 0;
+                default:
+                        if (offset >= 0x00020000 &&
+                            offset <= ixgbe_get_eeprom_len(netdev))
+                                return 0;
+                }
+                break;
+        case ixgbe_mac_X540:
+                switch (offset) {
+                case IXGBE_EERD:
+                case IXGBE_EEWR:
+                case IXGBE_SRAMREL:
+                case IXGBE_BARCTRL:
+                        return 0;
+                default:
+                        if ((offset >= 0x00020000 &&
+                             offset <= ixgbe_get_eeprom_len(netdev)))
+                                return 0;
+                }
+                break;
+        case ixgbe_mac_X550:
+                switch (offset) {
+                case IXGBE_EEWR:
+                case IXGBE_SRAMREL:
+                case IXGBE_PHYCTL_82599:
+                case IXGBE_FWRESETCNT:
+                        return 0;
+                default:
+                        if (offset >= IXGBE_FLEX_MNG_PTR(0) &&
+                            offset <= IXGBE_FLEX_MNG_PTR(447))
+                                return 0;
+                }
+                break;
+        case ixgbe_mac_X550EM_x:
+                switch (offset) {
+                case IXGBE_PHYCTL_82599:
+                case IXGBE_NW_MNG_IF_SEL:
+                case IXGBE_FWRESETCNT:
+                case IXGBE_I2CCTL_X550:
+                        return 0;
+                default:
+                        if ((offset >= IXGBE_FLEX_MNG_PTR(0) &&
+                             offset <= IXGBE_FLEX_MNG_PTR(447)) ||
+                            (offset >= IXGBE_FUSES0_GROUP(0) &&
+                             offset <= IXGBE_FUSES0_GROUP(7)))
+                                return 0;
+                }
+                break;
+        case ixgbe_mac_x550em_a:
+                switch (offset) {
+                case IXGBE_PHYCTL_82599:
+                case IXGBE_NW_MNG_IF_SEL:
+                case IXGBE_FWRESETCNT:
+                case IXGBE_I2CCTL_X550:
+                case IXGBE_FLA_X550EM_a:
+                case IXGBE_SWSM_X550EM_a:
+                case IXGBE_FWSM_X550EM_a:
+                case IXGBE_SWFW_SYNC_X550EM_a:
+                case IXGBE_FACTPS_X550EM_a:
+                case IXGBE_EEC_X550EM_a:
+                        return 0;
+                default:
+                        if (offset >= IXGBE_FLEX_MNG_PTR(0) &&
+                            offset <= IXGBE_FLEX_MNG_PTR(447))
+                                return 0;
+                }
+        default:
+                break;
+        }
+
+        return -ENOTTY;
+}
+
+static int ixgbe_nvmupd_command(struct ixgbe_hw *hw,
+                                struct ixgbe_nvm_access *nvm,
+                                u8 *bytes)
+{
+        u32 command;
+        int ret_val = 0;
+        u8 module;
+
+        command = nvm->command;
+        module = ixgbe_nvmupd_get_module(nvm->config);
+
+        switch (command) {
+        case IXGBE_NVMUPD_CMD_REG_READ:
+                switch (module) {
+                case IXGBE_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 (ixgbe_nvmupd_validate_offset(hw->back, nvm->offset))
+                                return -ENOTTY;
+
+                        if (nvm->data_size == 1)
+                                *((u8 *)bytes) = IXGBE_R8_Q(hw, nvm->offset);
+                        else
+                                *((u32 *)bytes) = IXGBE_R32_Q(hw, nvm->offset);
+                break;
+                }
+        break;
+        case IXGBE_NVMUPD_CMD_REG_WRITE:
+                if (ixgbe_nvmupd_validate_offset(hw->back, nvm->offset))
+                        return -ENOTTY;
+
+                IXGBE_WRITE_REG(hw, nvm->offset, *((u32 *)bytes));
+        break;
+        }
+
+        return ret_val;
 }
 
 static int ixgbe_get_eeprom(struct net_device *netdev,
-			    struct ethtool_eeprom *eeprom, u8 *bytes)
+                            struct ethtool_eeprom *eeprom, u8 *bytes)
 {
-	struct ixgbe_adapter *adapter = netdev_priv(netdev);
-	struct ixgbe_hw *hw = &adapter->hw;
-	u16 *eeprom_buff;
-	int first_word, last_word, eeprom_len;
-	int ret_val = 0;
-	u16 i;
+        struct ixgbe_adapter *adapter = netdev_priv(netdev);
+        struct ixgbe_hw *hw = &adapter->hw;
+        u16 *eeprom_buff;
+        int first_word, last_word, eeprom_len;
+        struct ixgbe_nvm_access *nvm;
+        u32 magic;
+        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;
-	eeprom_len = last_word - first_word + 1;
-
-	eeprom_buff = kmalloc_array(eeprom_len, sizeof(u16), GFP_KERNEL);
-	if (!eeprom_buff)
-		return -ENOMEM;
+        //WARN("ixgbe_get_eeprom() invoked, bytes=%u\n", bytes);
 
-	ret_val = hw->eeprom.ops.read_buffer(hw, first_word, eeprom_len,
-					     eeprom_buff);
+        if (eeprom->len == 0)
+                return -EINVAL;
 
-	/* Device's eeprom is always little-endian, word addressable */
-	for (i = 0; i < eeprom_len; i++)
-		le16_to_cpus(&eeprom_buff[i]);
+        magic = hw->vendor_id | (hw->device_id << 16);
+        if (eeprom->magic && eeprom->magic != magic) {
+                nvm = (struct ixgbe_nvm_access *)eeprom;
+                ret_val = ixgbe_nvmupd_command(hw, nvm, bytes);
+                return ret_val;
+        }
 
-	memcpy(bytes, (u8 *)eeprom_buff + (eeprom->offset & 1), eeprom->len);
-	kfree(eeprom_buff);
+        /* normal ethtool get_eeprom support */
+        eeprom->magic = hw->vendor_id | (hw->device_id << 16);
 
-	return ret_val;
-}
+        first_word = eeprom->offset >> 1;
+        last_word = (eeprom->offset + eeprom->len - 1) >> 1;
+        eeprom_len = last_word - first_word + 1;
 
-static int ixgbe_set_eeprom(struct net_device *netdev,
-			    struct ethtool_eeprom *eeprom, u8 *bytes)
-{
-	struct ixgbe_adapter *adapter = netdev_priv(netdev);
-	struct ixgbe_hw *hw = &adapter->hw;
-	u16 *eeprom_buff;
-	void *ptr;
-	int max_len, first_word, last_word, ret_val = 0;
-	u16 i;
+        eeprom_buff = kmalloc(sizeof(u16) * eeprom_len, GFP_KERNEL);
+        if (!eeprom_buff)
+                return -ENOMEM;
 
-	if (eeprom->len == 0)
-		return -EINVAL;
+        ret_val = hw->eeprom.ops.read_buffer(hw, first_word, eeprom_len,
+                                           eeprom_buff);
 
-	if (eeprom->magic != (hw->vendor_id | (hw->device_id << 16)))
-		return -EINVAL;
+        /* Device's eeprom is always little-endian, word addressable */
+        for (i = 0; i < eeprom_len; i++)
+                le16_to_cpus(&eeprom_buff[i]);
 
-	max_len = hw->eeprom.word_size * 2;
+        memcpy(bytes, (u8 *)eeprom_buff + (eeprom->offset & 1), eeprom->len);
+        kfree(eeprom_buff);
 
-	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 = 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->eeprom.ops.read(hw, first_word, &eeprom_buff[0]);
-		if (ret_val)
-			goto err;
-
-		ptr++;
-	}
-	if ((eeprom->offset + eeprom->len) & 1) {
-		/*
-		 * need read/modify/write of last changed EEPROM word
-		 * only the first byte of the word is being modified
-		 */
-		ret_val = hw->eeprom.ops.read(hw, last_word,
-					  &eeprom_buff[last_word - first_word]);
-		if (ret_val)
-			goto err;
-	}
-
-	/* 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->eeprom.ops.write_buffer(hw, first_word,
-					      last_word - first_word + 1,
-					      eeprom_buff);
+        return ret_val;
+}
 
-	/* Update the checksum */
-	if (ret_val == 0)
-		hw->eeprom.ops.update_checksum(hw);
+static int ixgbe_set_eeprom(struct net_device *netdev,
+                            struct ethtool_eeprom *eeprom, u8 *bytes)
+{
+        struct ixgbe_adapter *adapter = netdev_priv(netdev);
+        struct ixgbe_hw *hw = &adapter->hw;
+        int max_len, first_word, last_word, ret_val = 0;
+        struct ixgbe_nvm_access *nvm;
+        u32 magic;
+        u16 *eeprom_buff, i;
+        void *ptr;
+
+        //WARN("ixgbe_set_eeprom() invoked, bytes=%u\n", bytes);
+
+        if (eeprom->len == 0) 
+                return -EINVAL;
+
+        magic = hw->vendor_id | (hw->device_id << 16);
+        if (eeprom->magic && eeprom->magic != magic) {
+                nvm = (struct ixgbe_nvm_access *)eeprom;
+                ret_val = ixgbe_nvmupd_command(hw, nvm, bytes);
+                return ret_val;
+        }
+
+        /* normal ethtool set_eeprom support */
+
+        if (eeprom->magic != (hw->vendor_id | (hw->device_id << 16)))
+                return -EINVAL;
+
+        max_len = hw->eeprom.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 = 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->eeprom.ops.read(hw, first_word, &eeprom_buff[0]);
+                if (ret_val)
+                        goto err;
+
+                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->eeprom.ops.read(hw, last_word,
+                                          &eeprom_buff[last_word - first_word]);
+                if (ret_val)
+                        goto err;
+        }
+
+        /* 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->eeprom.ops.write_buffer(hw, first_word,
+                                            last_word - first_word + 1,
+                                            eeprom_buff);
+
+        /* Update the checksum */
+        if (ret_val == 0)
+                hw->eeprom.ops.update_checksum(hw);
 
 err:
-	kfree(eeprom_buff);
-	return ret_val;
+        kfree(eeprom_buff);
+        return ret_val;
 }
 
 static void ixgbe_get_drvinfo(struct net_device *netdev,
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 094653e81..ac2405105 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -6519,6 +6519,17 @@  static int ixgbe_sw_init(struct ixgbe_adapter *adapter,
 	if (nr_cpu_ids > IXGBE_MAX_XDP_QS)
 		static_branch_enable(&ixgbe_xdp_locking_key);
 
+        /* NVM Update features structure initialization */
+        hw->nvmupd_features.major = IXGBE_NVMUPD_FEATURES_API_VER_MAJOR;
+        hw->nvmupd_features.minor = IXGBE_NVMUPD_FEATURES_API_VER_MINOR;
+        hw->nvmupd_features.size = sizeof(hw->nvmupd_features);
+        memset(hw->nvmupd_features.features, 0x0,
+               IXGBE_NVMUPD_FEATURES_API_FEATURES_ARRAY_LEN *
+               sizeof(*hw->nvmupd_features.features));
+
+        hw->nvmupd_features.features[0] =
+                IXGBE_NVMUPD_FEATURE_REGISTER_ACCESS_SUPPORT;
+
 	return 0;
 }
 
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
index 346e3d911..5c71e67d2 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
@@ -129,6 +129,8 @@ 
 #define IXGBE_GRC_X550EM_x	IXGBE_GRC_8259X
 #define IXGBE_GRC_X550EM_a	0x15F64
 #define IXGBE_GRC(_hw)		IXGBE_BY_MAC((_hw), GRC)
+#define IXGBE_SRAMREL           0x10210
+#define IXGBE_FWRESETCNT        0x15F40
 
 /* General Receive Control */
 #define IXGBE_GRC_MNG  0x00000001 /* Manageability Enable */
@@ -936,6 +938,7 @@  struct ixgbe_nvm_version {
 #define IXGBE_SWSR      0x15F10
 #define IXGBE_HFDR      0x15FE8
 #define IXGBE_FLEX_MNG  0x15800 /* 0x15800 - 0x15EFC */
+#define IXGBE_FLEX_MNG_PTR(_i)  (IXGBE_FLEX_MNG + ((_i) * 4))
 
 #define IXGBE_HICR_EN              0x01  /* Enable bit - RO */
 /* Driver sets this bit when done to put command in RAM */
@@ -3390,6 +3393,38 @@  struct ixgbe_hw_stats {
 	u64 o2bspc;
 };
 
+/* NVM Update commands */
+#define IXGBE_NVMUPD_CMD_REG_READ	0x0000000B
+#define IXGBE_NVMUPD_CMD_REG_WRITE	0x0000000C 
+
+#define IXGBE_R32_Q(h, r) ixgbe_read_reg(h, r)
+#define IXGBE_R8_Q(h, r) readb(READ_ONCE(h->hw_addr) + r)
+
+/* NVM Update features API */
+#define IXGBE_NVMUPD_FEATURES_API_VER_MAJOR             0
+#define IXGBE_NVMUPD_FEATURES_API_VER_MINOR             0
+#define IXGBE_NVMUPD_FEATURES_API_FEATURES_ARRAY_LEN    12
+#define IXGBE_NVMUPD_EXEC_FEATURES                      0xe
+#define IXGBE_NVMUPD_FEATURE_FLAT_NVM_SUPPORT           BIT(0)
+#define IXGBE_NVMUPD_FEATURE_REGISTER_ACCESS_SUPPORT    BIT(1)
+
+#define IXGBE_NVMUPD_MOD_PNT_MASK                       0xFF
+
+struct ixgbe_nvm_access {
+        u32 command;
+        u32 config; 
+        u32 offset;     /* in bytes */
+        u32 data_size;  /* in bytes */
+        u8 data[1];
+};
+
+struct ixgbe_nvm_features {
+        u8 major;
+        u8 minor;
+        u16 size;
+        u8 features[IXGBE_NVMUPD_FEATURES_API_FEATURES_ARRAY_LEN];
+}; 
+
 /* forward declaration */
 struct ixgbe_hw;
 
@@ -3654,6 +3689,8 @@  struct ixgbe_hw {
 	bool				allow_unsupported_sfp;
 	bool				wol_enabled;
 	bool				need_crosstalk_fix;
+       /* NVM Update features */
+        struct ixgbe_nvm_features nvmupd_features;
 };
 
 struct ixgbe_info {