| 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
| ||
|
Message-ID: <41bd4ca9-2882-4203-8c68-71f5a0d7e20b@intel.com>
Date: Wed, 4 Dec 2024 22:38:19 +0100
From: Przemek Kitszel <przemyslaw.kitszel@...el.com>
To: Piotr Kwapulinski <piotr.kwapulinski@...el.com>
CC: <netdev@...r.kernel.org>, Stefan Wegrzyn <stefan.wegrzyn@...el.com>,
<intel-wired-lan@...ts.osuosl.org>, Jedrzej Jagielski
<jedrzej.jagielski@...el.com>
Subject: Re: [PATCH iwl-next v11 5/8] ixgbe: Add support for EEPROM dump in
E610 device
On 12/4/24 15:31, Piotr Kwapulinski wrote:
> Add low level support for EEPROM dump for the specified network device.
>
> Co-developed-by: Stefan Wegrzyn <stefan.wegrzyn@...el.com>
> Signed-off-by: Stefan Wegrzyn <stefan.wegrzyn@...el.com>
> Signed-off-by: Piotr Kwapulinski <piotr.kwapulinski@...el.com>
> ---
> drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c | 93 +++++++++++++++++++
> drivers/net/ethernet/intel/ixgbe/ixgbe_e610.h | 5 +
> .../ethernet/intel/ixgbe/ixgbe_type_e610.h | 8 ++
> 3 files changed, 106 insertions(+)
just nitpicks, so:
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@...el.com>
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
> index 0542b4b..503a047 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
> @@ -2070,6 +2070,38 @@ int ixgbe_enter_lplu_e610(struct ixgbe_hw *hw)
> return ixgbe_aci_set_phy_cfg(hw, &phy_cfg);
> }
>
> +/**
> + * ixgbe_init_eeprom_params_E610 - Initialize EEPROM params
as bot set (also for our internal version of the patch), this should be
"e610"
> + * @hw: pointer to hardware structure
> + *
> + * Initialize the EEPROM parameters ixgbe_eeprom_info within the ixgbe_hw
> + * struct in order to set up EEPROM access.
> + *
> + * Return: the operation exit code
missing dot (.) at the end
> + */
> +int ixgbe_init_eeprom_params_e610(struct ixgbe_hw *hw)
> +{
> + struct ixgbe_eeprom_info *eeprom = &hw->eeprom;
> + u32 gens_stat;
> + u8 sr_size;
> +
> + if (eeprom->type != ixgbe_eeprom_uninitialized)
> + return 0;
> +
> + eeprom->type = ixgbe_flash;
> +
> + gens_stat = IXGBE_READ_REG(hw, GLNVM_GENS);
> + sr_size = FIELD_GET(GLNVM_GENS_SR_SIZE_M, gens_stat);
> +
> + /* Switching to words (sr_size contains power of 2). */
> + eeprom->word_size = BIT(sr_size) * IXGBE_SR_WORDS_IN_1KB;
> +
> + hw_dbg(hw, "Eeprom params: type = %d, size = %d\n", eeprom->type,
> + eeprom->word_size);
> +
> + return 0;
> +}
> +
> /**
> * ixgbe_aci_get_netlist_node - get a node handle
> * @hw: pointer to the hw struct
> @@ -2316,6 +2348,34 @@ int ixgbe_read_flat_nvm(struct ixgbe_hw *hw, u32 offset, u32 *length,
> return err;
> }
>
> +/**
> + * ixgbe_read_sr_buf_aci - Read Shadow RAM buffer via ACI
> + * @hw: pointer to the HW structure
> + * @offset: offset of the Shadow RAM words to read (0x000000 - 0x001FFF)
> + * @words: (in) number of words to read; (out) number of words actually read
> + * @data: words read from the Shadow RAM
> + *
> + * Read 16 bit words (data buf) from the Shadow RAM. Acquire/release the NVM
> + * ownership.
> + *
> + * Return: the operation exit code
ditto dot
> + */
> +int ixgbe_read_sr_buf_aci(struct ixgbe_hw *hw, u16 offset, u16 *words,
> + u16 *data)
> +{
> + u32 bytes = *words * 2, i;
I would declare @i in the for loop below
> + int err;
> +
> + err = ixgbe_read_flat_nvm(hw, offset * 2, &bytes, (u8 *)data, true);
shouldn't we exit early here?
> +
> + *words = bytes / 2;
> +
> + for (i = 0; i < *words; i++)
> + data[i] = le16_to_cpu(((__le16 *)data)[i]);
> +
> + return err;
> +}
> +
> /**
> * ixgbe_read_ee_aci_e610 - Read EEPROM word using the admin command.
> * @hw: pointer to hardware structure
> @@ -2349,6 +2409,39 @@ int ixgbe_read_ee_aci_e610(struct ixgbe_hw *hw, u16 offset, u16 *data)
> return err;
> }
>
> +/**
> + * ixgbe_read_ee_aci_buffer_e610 - Read EEPROM words via ACI
> + * @hw: pointer to hardware structure
> + * @offset: offset of words in the EEPROM to read
> + * @words: number of words to read
> + * @data: words to read from the EEPROM
> + *
> + * Read 16 bit words from the EEPROM via the ACI. Initialize the EEPROM params
> + * prior to the read. Acquire/release the NVM ownership.
> + *
> + * Return: the operation exit code
ditto dot
> + */
> +int ixgbe_read_ee_aci_buffer_e610(struct ixgbe_hw *hw, u16 offset,
> + u16 words, u16 *data)
> +{
> + int err;
> +
> + if (hw->eeprom.type == ixgbe_eeprom_uninitialized) {
> + err = hw->eeprom.ops.init_params(hw);
> + if (err)
> + return err;
> + }
> +
> + err = ixgbe_acquire_nvm(hw, IXGBE_RES_READ);
> + if (err)
> + return err;
> +
> + err = ixgbe_read_sr_buf_aci(hw, offset, &words, data);
> + ixgbe_release_nvm(hw);
> +
> + return err;
> +}
> +
> /**
> * ixgbe_validate_eeprom_checksum_e610 - Validate EEPROM checksum
> * @hw: pointer to hardware structure
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.h
> index 412ddd1..9cfcfee 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.h
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.h
> @@ -56,6 +56,7 @@ int ixgbe_identify_module_e610(struct ixgbe_hw *hw);
> int ixgbe_setup_phy_link_e610(struct ixgbe_hw *hw);
> int ixgbe_set_phy_power_e610(struct ixgbe_hw *hw, bool on);
> int ixgbe_enter_lplu_e610(struct ixgbe_hw *hw);
> +int ixgbe_init_eeprom_params_e610(struct ixgbe_hw *hw);
> int ixgbe_aci_get_netlist_node(struct ixgbe_hw *hw,
> struct ixgbe_aci_cmd_get_link_topo *cmd,
> u8 *node_part_number, u16 *node_handle);
> @@ -69,7 +70,11 @@ int ixgbe_nvm_validate_checksum(struct ixgbe_hw *hw);
> int ixgbe_read_sr_word_aci(struct ixgbe_hw *hw, u16 offset, u16 *data);
> int ixgbe_read_flat_nvm(struct ixgbe_hw *hw, u32 offset, u32 *length,
> u8 *data, bool read_shadow_ram);
> +int ixgbe_read_sr_buf_aci(struct ixgbe_hw *hw, u16 offset, u16 *words,
> + u16 *data);
> int ixgbe_read_ee_aci_e610(struct ixgbe_hw *hw, u16 offset, u16 *data);
> +int ixgbe_read_ee_aci_buffer_e610(struct ixgbe_hw *hw, u16 offset,
> + u16 words, u16 *data);
> int ixgbe_validate_eeprom_checksum_e610(struct ixgbe_hw *hw, u16 *checksum_val);
>
> #endif /* _IXGBE_E610_H_ */
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type_e610.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_type_e610.h
> index ecc3fc8..9dba8b5 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type_e610.h
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type_e610.h
> @@ -12,11 +12,19 @@
> /* Checksum and Shadow RAM pointers */
> #define E610_SR_SW_CHECKSUM_WORD 0x3F
>
> +/* Shadow RAM related */
> +#define IXGBE_SR_WORDS_IN_1KB 512
> +
> /* Firmware Status Register (GL_FWSTS) */
> #define GL_FWSTS 0x00083048 /* Reset Source: POR */
> #define GL_FWSTS_EP_PF0 BIT(24)
> #define GL_FWSTS_EP_PF1 BIT(25)
>
> +/* Global NVM General Status Register */
> +#define GLNVM_GENS 0x000B6100 /* Reset Source: POR */
> +#define GLNVM_GENS_SR_SIZE_S 5
unused define, please remove
> +#define GLNVM_GENS_SR_SIZE_M GENMASK(7, 5)
> +
> /* Flash Access Register */
> #define IXGBE_GLNVM_FLA 0x000B6108 /* Reset Source: POR */
> #define IXGBE_GLNVM_FLA_LOCKED_S 6
Powered by blists - more mailing lists