2024-06-09 09:00:12

by Richard chien

[permalink] [raw]
Subject: [PATCH] ixgbe: Add support for firmware update

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 <[email protected]>
---
.../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(-)

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 {
--
2.40.1



2024-06-09 11:24:37

by Markus Elfring

[permalink] [raw]
Subject: Re: [PATCH] ixgbe: Add support for firmware update

> 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


Please improve such a change description also according to word wrapping
because of more desirable text line lengths.



> +++ 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_set_eeprom(struct net_device *netdev,
> + struct ethtool_eeprom *eeprom, u8 *bytes)

> err:
> - kfree(eeprom_buff);
> - return ret_val;
> + kfree(eeprom_buff);
> + return ret_val;
> }

Please keep these statements unmodified.

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

2024-06-09 22:35:00

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] ixgbe: Add support for firmware update

Hi Richard,

kernel test robot noticed the following build warnings:

[auto build test WARNING on tnguy-net-queue/dev-queue]
[also build test WARNING on linus/master v6.10-rc3 next-20240607]
[cannot apply to tnguy-next-queue/dev-queue]
[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/ixgbe-Add-support-for-firmware-update/20240609-170239
base: https://git.kernel.org/pub/scm/linux/kernel/git/tnguy/net-queue.git dev-queue
patch link: https://lore.kernel.org/r/20240609085735.6253-1-richard.chien%40hpe.com
patch subject: [PATCH] ixgbe: Add support for firmware update
config: x86_64-rhel-8.3-rust (https://download.01.org/0day-ci/archive/20240610/[email protected]/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (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]/

All warnings (new ones prefixed by >>):

>> drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c:1104:9: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough]
1104 | default:
| ^
drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c:1104:9: note: insert 'break;' to avoid fall-through
1104 | default:
| ^
| break;
1 warning generated.


vim +1104 drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c

1005
1006 static int ixgbe_nvmupd_validate_offset(struct ixgbe_adapter *adapter,
1007 u32 offset)
1008 {
1009 struct net_device *netdev = adapter->netdev;
1010
1011 switch (offset) {
1012 case IXGBE_STATUS:
1013 case IXGBE_ESDP:
1014 case IXGBE_MSCA:
1015 case IXGBE_MSRWD:
1016 case IXGBE_EEC_8259X:
1017 case IXGBE_FLA_8259X:
1018 case IXGBE_FLOP:
1019 case IXGBE_SWSM_8259X:
1020 case IXGBE_FWSM_8259X:
1021 case IXGBE_FACTPS_8259X:
1022 case IXGBE_GSSR:
1023 case IXGBE_HICR:
1024 case IXGBE_FWSTS:
1025 return 0;
1026 default:
1027 if ((offset >= IXGBE_MAVTV(0) && offset <= IXGBE_MAVTV(7)) ||
1028 (offset >= IXGBE_RAL(0) && offset <= IXGBE_RAH(15)))
1029 return 0;
1030 }
1031
1032 switch (adapter->hw.mac.type) {
1033 case ixgbe_mac_82599EB:
1034 switch (offset) {
1035 case IXGBE_AUTOC:
1036 case IXGBE_EERD:
1037 case IXGBE_BARCTRL:
1038 return 0;
1039 default:
1040 if (offset >= 0x00020000 &&
1041 offset <= ixgbe_get_eeprom_len(netdev))
1042 return 0;
1043 }
1044 break;
1045 case ixgbe_mac_X540:
1046 switch (offset) {
1047 case IXGBE_EERD:
1048 case IXGBE_EEWR:
1049 case IXGBE_SRAMREL:
1050 case IXGBE_BARCTRL:
1051 return 0;
1052 default:
1053 if ((offset >= 0x00020000 &&
1054 offset <= ixgbe_get_eeprom_len(netdev)))
1055 return 0;
1056 }
1057 break;
1058 case ixgbe_mac_X550:
1059 switch (offset) {
1060 case IXGBE_EEWR:
1061 case IXGBE_SRAMREL:
1062 case IXGBE_PHYCTL_82599:
1063 case IXGBE_FWRESETCNT:
1064 return 0;
1065 default:
1066 if (offset >= IXGBE_FLEX_MNG_PTR(0) &&
1067 offset <= IXGBE_FLEX_MNG_PTR(447))
1068 return 0;
1069 }
1070 break;
1071 case ixgbe_mac_X550EM_x:
1072 switch (offset) {
1073 case IXGBE_PHYCTL_82599:
1074 case IXGBE_NW_MNG_IF_SEL:
1075 case IXGBE_FWRESETCNT:
1076 case IXGBE_I2CCTL_X550:
1077 return 0;
1078 default:
1079 if ((offset >= IXGBE_FLEX_MNG_PTR(0) &&
1080 offset <= IXGBE_FLEX_MNG_PTR(447)) ||
1081 (offset >= IXGBE_FUSES0_GROUP(0) &&
1082 offset <= IXGBE_FUSES0_GROUP(7)))
1083 return 0;
1084 }
1085 break;
1086 case ixgbe_mac_x550em_a:
1087 switch (offset) {
1088 case IXGBE_PHYCTL_82599:
1089 case IXGBE_NW_MNG_IF_SEL:
1090 case IXGBE_FWRESETCNT:
1091 case IXGBE_I2CCTL_X550:
1092 case IXGBE_FLA_X550EM_a:
1093 case IXGBE_SWSM_X550EM_a:
1094 case IXGBE_FWSM_X550EM_a:
1095 case IXGBE_SWFW_SYNC_X550EM_a:
1096 case IXGBE_FACTPS_X550EM_a:
1097 case IXGBE_EEC_X550EM_a:
1098 return 0;
1099 default:
1100 if (offset >= IXGBE_FLEX_MNG_PTR(0) &&
1101 offset <= IXGBE_FLEX_MNG_PTR(447))
1102 return 0;
1103 }
> 1104 default:
1105 break;
1106 }
1107
1108 return -ENOTTY;
1109 }
1110

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2024-06-10 19:36:29

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH] ixgbe: Add support for firmware update

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 <[email protected]>

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

Subject: RE: [PATCH] ixgbe: Add support for firmware update

> 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