[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <904f2004-147c-4037-a6dd-20264bc79dc6@intel.com>
Date: Thu, 31 Jul 2025 10:02:04 -0700
From: Jacob Keller <jacob.e.keller@...el.com>
To: Jedrzej Jagielski <jedrzej.jagielski@...el.com>,
<intel-wired-lan@...ts.osuosl.org>
CC: <anthony.l.nguyen@...el.com>, <netdev@...r.kernel.org>, "Aleksandr
Loktionov" <aleksandr.loktionov@...el.com>
Subject: Re: [Intel-wired-lan] [PATCH iwl-next v1] ixgbe: reduce number of
reads when getting OROM data
On 7/31/2025 5:50 AM, Jedrzej Jagielski wrote:
> Currently, during locating the CIVD section, the ixgbe driver loops
> over the OROM area and at each iteration reads only OROM-datastruct-size
> amount of data. This results in many small reads and is inefficient.
>
> Optimize this by reading the entire OROM bank into memory once before
> entering the loop. This significantly reduces the probing time.
>
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@...el.com>
> Signed-off-by: Jedrzej Jagielski <jedrzej.jagielski@...el.com>
> ---
Nice. I recall doing something similar in ice.
Reviewed-by: Jacob Keller <jacob.e.keller@...el.com>
> drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c | 58 +++++++++++++------
> 1 file changed, 39 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
> index 87b03c1992a8..048b2aae155a 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
> @@ -3006,50 +3006,70 @@ static int ixgbe_get_nvm_srev(struct ixgbe_hw *hw,
> * Searches through the Option ROM flash contents to locate the CIVD data for
> * the image.
> *
> - * Return: the exit code of the operation.
> + * Return: -ENOMEM when cannot allocate memory, -EDOM for checksum violation,
> + * -ENODATA when cannot find proper data, -EIO for faulty read or
> + * 0 on success.
> + *
> + * On success @civd stores collected data.
> */
> static int
> ixgbe_get_orom_civd_data(struct ixgbe_hw *hw, enum ixgbe_bank_select bank,
> struct ixgbe_orom_civd_info *civd)
> {
> - struct ixgbe_orom_civd_info tmp;
> + u32 orom_size = hw->flash.banks.orom_size;
> + u8 *orom_data;
> u32 offset;
> int err;
>
> + orom_data = kzalloc(orom_size, GFP_KERNEL);
> + if (!orom_data)
> + return -ENOMEM;
> +
> + err = ixgbe_read_flash_module(hw, bank,
> + IXGBE_E610_SR_1ST_OROM_BANK_PTR, 0,
> + orom_data, orom_size);
> + if (err) {
> + err = -EIO;
> + goto cleanup;
> + }
> +
> /* The CIVD section is located in the Option ROM aligned to 512 bytes.
> * The first 4 bytes must contain the ASCII characters "$CIV".
> * A simple modulo 256 sum of all of the bytes of the structure must
> * equal 0.
> */
> - for (offset = 0; (offset + SZ_512) <= hw->flash.banks.orom_size;
> - offset += SZ_512) {
> + for (offset = 0; (offset + SZ_512) <= orom_size; offset += SZ_512) {
> + struct ixgbe_orom_civd_info *tmp;
> u8 sum = 0;
> u32 i;
>
> - err = ixgbe_read_flash_module(hw, bank,
> - IXGBE_E610_SR_1ST_OROM_BANK_PTR,
> - offset,
> - (u8 *)&tmp, sizeof(tmp));
> - if (err)
> - return err;
> + BUILD_BUG_ON(sizeof(*tmp) > SZ_512);
> +
> + tmp = (struct ixgbe_orom_civd_info *)&orom_data[offset];
>
> /* Skip forward until we find a matching signature */
> - if (memcmp(IXGBE_OROM_CIV_SIGNATURE, tmp.signature,
> - sizeof(tmp.signature)))
> + if (memcmp(IXGBE_OROM_CIV_SIGNATURE, tmp->signature,
> + sizeof(tmp->signature)))
> continue;
>
> /* Verify that the simple checksum is zero */
> - for (i = 0; i < sizeof(tmp); i++)
> - sum += ((u8 *)&tmp)[i];
> + for (i = 0; i < sizeof(*tmp); i++)
> + sum += ((u8 *)tmp)[i];
>
> - if (sum)
> - return -EDOM;
> + if (sum) {
> + err = -EDOM;
> + goto cleanup;
> + }
>
> - *civd = tmp;
> - return 0;
> + *civd = *tmp;
> + err = 0;
> + goto cleanup;
> }
>
> - return -ENODATA;
> + err = -ENODATA;
> +cleanup:
> + kfree(orom_data);
> + return err;
> }
>
> /**
Download attachment "OpenPGP_signature.asc" of type "application/pgp-signature" (237 bytes)
Powered by blists - more mailing lists