[PATCH iwl-next 1/2] ixgbe: e610: add missing endianness conversion

[PATCH iwl-next 1/2] ixgbe: e610: add missing endianness conversion

[Piotr Kwapulinski]

Fix a possible ACI issue on big-endian platforms.

Fixes: 46761fd52a88 ("ixgbe: Add support for E610 FW Admin Command Interface")
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Signed-off-by: Piotr Kwapulinski <piotr.kwapulinski@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
index c2f8189..f494e90 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
@@ -113,7 +113,8 @@ static int ixgbe_aci_send_cmd_execute(struct ixgbe_hw *hw,
 
 	/* Descriptor is written to specific registers */
 	for (i = 0; i < IXGBE_ACI_DESC_SIZE_IN_DWORDS; i++)
-		IXGBE_WRITE_REG(hw, IXGBE_PF_HIDA(i), raw_desc[i]);
+		IXGBE_WRITE_REG(hw, IXGBE_PF_HIDA(i),
+				le32_to_cpu(raw_desc[i]));
 
 	/* SW has to set PF_HICR.C bit and clear PF_HICR.SV and
 	 * PF_HICR_EV
@@ -145,7 +146,7 @@ static int ixgbe_aci_send_cmd_execute(struct ixgbe_hw *hw,
 	if ((hicr & IXGBE_PF_HICR_SV)) {
 		for (i = 0; i < IXGBE_ACI_DESC_SIZE_IN_DWORDS; i++) {
 			raw_desc[i] = IXGBE_READ_REG(hw, IXGBE_PF_HIDA(i));
-			raw_desc[i] = raw_desc[i];
+			raw_desc[i] = cpu_to_le32(raw_desc[i]);
 		}
 	}
 
@@ -153,7 +154,7 @@ static int ixgbe_aci_send_cmd_execute(struct ixgbe_hw *hw,
 	if ((hicr & IXGBE_PF_HICR_EV) && !(hicr & IXGBE_PF_HICR_C)) {
 		for (i = 0; i < IXGBE_ACI_DESC_SIZE_IN_DWORDS; i++) {
 			raw_desc[i] = IXGBE_READ_REG(hw, IXGBE_PF_HIDA_2(i));
-			raw_desc[i] = raw_desc[i];
+			raw_desc[i] = cpu_to_le32(raw_desc[i]);
 		}
 	}
 
-- 
2.47.1

Re: [Intel-wired-lan] [PATCH iwl-next 1/2] ixgbe: e610: add missing endianness conversion

[Paul Menzel]

Dear Piotr,


Thank you for your patch.

Am 16.01.26 um 13:23 schrieb Piotr Kwapulinski:
> Fix a possible ACI issue on big-endian platforms.

Please elaborate, why this is needed, and `raw_desc[i]` needs to be 
converted.

For the summary/title, you could also be more specific. Something like:

 > ixgbe: e610: Convert ACI descriptor buffer(?) to little endian

> Fixes: 46761fd52a88 ("ixgbe: Add support for E610 FW Admin Command Interface")
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> Signed-off-by: Piotr Kwapulinski <piotr.kwapulinski@intel.com>
> ---
>   drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
> index c2f8189..f494e90 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
> @@ -113,7 +113,8 @@ static int ixgbe_aci_send_cmd_execute(struct ixgbe_hw *hw,
>   
>   	/* Descriptor is written to specific registers */
>   	for (i = 0; i < IXGBE_ACI_DESC_SIZE_IN_DWORDS; i++)
> -		IXGBE_WRITE_REG(hw, IXGBE_PF_HIDA(i), raw_desc[i]);
> +		IXGBE_WRITE_REG(hw, IXGBE_PF_HIDA(i),
> +				le32_to_cpu(raw_desc[i]));
>   
>   	/* SW has to set PF_HICR.C bit and clear PF_HICR.SV and
>   	 * PF_HICR_EV
> @@ -145,7 +146,7 @@ static int ixgbe_aci_send_cmd_execute(struct ixgbe_hw *hw,
>   	if ((hicr & IXGBE_PF_HICR_SV)) {
>   		for (i = 0; i < IXGBE_ACI_DESC_SIZE_IN_DWORDS; i++) {
>   			raw_desc[i] = IXGBE_READ_REG(hw, IXGBE_PF_HIDA(i));
> -			raw_desc[i] = raw_desc[i];
> +			raw_desc[i] = cpu_to_le32(raw_desc[i]);
>   		}
>   	}
>   
> @@ -153,7 +154,7 @@ static int ixgbe_aci_send_cmd_execute(struct ixgbe_hw *hw,
>   	if ((hicr & IXGBE_PF_HICR_EV) && !(hicr & IXGBE_PF_HICR_C)) {
>   		for (i = 0; i < IXGBE_ACI_DESC_SIZE_IN_DWORDS; i++) {
>   			raw_desc[i] = IXGBE_READ_REG(hw, IXGBE_PF_HIDA_2(i));
> -			raw_desc[i] = raw_desc[i];
> +			raw_desc[i] = cpu_to_le32(raw_desc[i]);
>   		}
>   	}


Kind regards,

Paul

Re: [PATCH iwl-next 1/2] ixgbe: e610: add missing endianness conversion

[kernel test robot]

Hi Piotr,

kernel test robot noticed the following build warnings:

[auto build test WARNING on tnguy-next-queue/dev-queue]

url:    https://github.com/intel-lab-lkp/linux/commits/Piotr-Kwapulinski/ixgbe-e610-add-missing-endianness-conversion/20260116-200705
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue.git dev-queue
patch link:    https://lore.kernel.org/r/20260116122353.78235-1-piotr.kwapulinski%40intel.com
patch subject: [PATCH iwl-next 1/2] ixgbe: e610: add missing endianness conversion
config: x86_64-randconfig-r111-20260117 (https://download.01.org/0day-ci/archive/20260117/202601172130.UkaUgjIE-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260117/202601172130.UkaUgjIE-lkp@intel.com/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 <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202601172130.UkaUgjIE-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c:116:17: sparse: sparse: cast to restricted __le32
>> drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c:149:37: sparse: sparse: incorrect type in assignment (different base types) @@     expected unsigned int [usertype] @@     got restricted __le32 [usertype] @@
   drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c:149:37: sparse:     expected unsigned int [usertype]
   drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c:149:37: sparse:     got restricted __le32 [usertype]
   drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c:157:37: sparse: sparse: incorrect type in assignment (different base types) @@     expected unsigned int [usertype] @@     got restricted __le32 [usertype] @@
   drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c:157:37: sparse:     expected unsigned int [usertype]
   drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c:157:37: sparse:     got restricted __le32 [usertype]

vim +116 drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c

    35	
    36	/**
    37	 * ixgbe_aci_send_cmd_execute - execute sending FW Admin Command to FW Admin
    38	 * Command Interface
    39	 * @hw: pointer to the HW struct
    40	 * @desc: descriptor describing the command
    41	 * @buf: buffer to use for indirect commands (NULL for direct commands)
    42	 * @buf_size: size of buffer for indirect commands (0 for direct commands)
    43	 *
    44	 * Admin Command is sent using CSR by setting descriptor and buffer in specific
    45	 * registers.
    46	 *
    47	 * Return: the exit code of the operation.
    48	 * * - 0 - success.
    49	 * * - -EIO - CSR mechanism is not enabled.
    50	 * * - -EBUSY - CSR mechanism is busy.
    51	 * * - -EINVAL - buf_size is too big or
    52	 * invalid argument buf or buf_size.
    53	 * * - -ETIME - Admin Command X command timeout.
    54	 * * - -EIO - Admin Command X invalid state of HICR register or
    55	 * Admin Command failed because of bad opcode was returned or
    56	 * Admin Command failed with error Y.
    57	 */
    58	static int ixgbe_aci_send_cmd_execute(struct ixgbe_hw *hw,
    59					      struct libie_aq_desc *desc,
    60					      void *buf, u16 buf_size)
    61	{
    62		u16 opcode, buf_tail_size = buf_size % 4;
    63		u32 *raw_desc = (u32 *)desc;
    64		u32 hicr, i, buf_tail = 0;
    65		bool valid_buf = false;
    66	
    67		hw->aci.last_status = LIBIE_AQ_RC_OK;
    68	
    69		/* It's necessary to check if mechanism is enabled */
    70		hicr = IXGBE_READ_REG(hw, IXGBE_PF_HICR);
    71	
    72		if (!(hicr & IXGBE_PF_HICR_EN))
    73			return -EIO;
    74	
    75		if (hicr & IXGBE_PF_HICR_C) {
    76			hw->aci.last_status = LIBIE_AQ_RC_EBUSY;
    77			return -EBUSY;
    78		}
    79	
    80		opcode = le16_to_cpu(desc->opcode);
    81	
    82		if (buf_size > IXGBE_ACI_MAX_BUFFER_SIZE)
    83			return -EINVAL;
    84	
    85		if (buf)
    86			desc->flags |= cpu_to_le16(LIBIE_AQ_FLAG_BUF);
    87	
    88		if (desc->flags & cpu_to_le16(LIBIE_AQ_FLAG_BUF)) {
    89			if ((buf && !buf_size) ||
    90			    (!buf && buf_size))
    91				return -EINVAL;
    92			if (buf && buf_size)
    93				valid_buf = true;
    94		}
    95	
    96		if (valid_buf) {
    97			if (buf_tail_size)
    98				memcpy(&buf_tail, buf + buf_size - buf_tail_size,
    99				       buf_tail_size);
   100	
   101			if (((buf_size + 3) & ~0x3) > LIBIE_AQ_LG_BUF)
   102				desc->flags |= cpu_to_le16(LIBIE_AQ_FLAG_LB);
   103	
   104			desc->datalen = cpu_to_le16(buf_size);
   105	
   106			if (desc->flags & cpu_to_le16(LIBIE_AQ_FLAG_RD)) {
   107				for (i = 0; i < buf_size / 4; i++)
   108					IXGBE_WRITE_REG(hw, IXGBE_PF_HIBA(i), ((u32 *)buf)[i]);
   109				if (buf_tail_size)
   110					IXGBE_WRITE_REG(hw, IXGBE_PF_HIBA(i), buf_tail);
   111			}
   112		}
   113	
   114		/* Descriptor is written to specific registers */
   115		for (i = 0; i < IXGBE_ACI_DESC_SIZE_IN_DWORDS; i++)
 > 116			IXGBE_WRITE_REG(hw, IXGBE_PF_HIDA(i),
   117					le32_to_cpu(raw_desc[i]));
   118	
   119		/* SW has to set PF_HICR.C bit and clear PF_HICR.SV and
   120		 * PF_HICR_EV
   121		 */
   122		hicr = (IXGBE_READ_REG(hw, IXGBE_PF_HICR) | IXGBE_PF_HICR_C) &
   123		       ~(IXGBE_PF_HICR_SV | IXGBE_PF_HICR_EV);
   124		IXGBE_WRITE_REG(hw, IXGBE_PF_HICR, hicr);
   125	
   126	#define MAX_SLEEP_RESP_US 1000
   127	#define MAX_TMOUT_RESP_SYNC_US 100000000
   128	
   129		/* Wait for sync Admin Command response */
   130		read_poll_timeout(IXGBE_READ_REG, hicr,
   131				  (hicr & IXGBE_PF_HICR_SV) ||
   132				  !(hicr & IXGBE_PF_HICR_C),
   133				  MAX_SLEEP_RESP_US, MAX_TMOUT_RESP_SYNC_US, true, hw,
   134				  IXGBE_PF_HICR);
   135	
   136	#define MAX_TMOUT_RESP_ASYNC_US 150000000
   137	
   138		/* Wait for async Admin Command response */
   139		read_poll_timeout(IXGBE_READ_REG, hicr,
   140				  (hicr & IXGBE_PF_HICR_EV) ||
   141				  !(hicr & IXGBE_PF_HICR_C),
   142				  MAX_SLEEP_RESP_US, MAX_TMOUT_RESP_ASYNC_US, true, hw,
   143				  IXGBE_PF_HICR);
   144	
   145		/* Read sync Admin Command response */
   146		if ((hicr & IXGBE_PF_HICR_SV)) {
   147			for (i = 0; i < IXGBE_ACI_DESC_SIZE_IN_DWORDS; i++) {
   148				raw_desc[i] = IXGBE_READ_REG(hw, IXGBE_PF_HIDA(i));
 > 149				raw_desc[i] = cpu_to_le32(raw_desc[i]);
   150			}
   151		}
   152	
   153		/* Read async Admin Command response */
   154		if ((hicr & IXGBE_PF_HICR_EV) && !(hicr & IXGBE_PF_HICR_C)) {
   155			for (i = 0; i < IXGBE_ACI_DESC_SIZE_IN_DWORDS; i++) {
   156				raw_desc[i] = IXGBE_READ_REG(hw, IXGBE_PF_HIDA_2(i));
   157				raw_desc[i] = cpu_to_le32(raw_desc[i]);
   158			}
   159		}
   160	
   161		/* Handle timeout and invalid state of HICR register */
   162		if (hicr & IXGBE_PF_HICR_C)
   163			return -ETIME;
   164	
   165		if (!(hicr & IXGBE_PF_HICR_SV) && !(hicr & IXGBE_PF_HICR_EV))
   166			return -EIO;
   167	
   168		/* For every command other than 0x0014 treat opcode mismatch
   169		 * as an error. Response to 0x0014 command read from HIDA_2
   170		 * is a descriptor of an event which is expected to contain
   171		 * different opcode than the command.
   172		 */
   173		if (desc->opcode != cpu_to_le16(opcode) &&
   174		    opcode != ixgbe_aci_opc_get_fw_event)
   175			return -EIO;
   176	
   177		if (desc->retval) {
   178			hw->aci.last_status = (enum libie_aq_err)
   179				le16_to_cpu(desc->retval);
   180			return -EIO;
   181		}
   182	
   183		/* Write a response values to a buf */
   184		if (valid_buf) {
   185			for (i = 0; i < buf_size / 4; i++)
   186				((u32 *)buf)[i] = IXGBE_READ_REG(hw, IXGBE_PF_HIBA(i));
   187			if (buf_tail_size) {
   188				buf_tail = IXGBE_READ_REG(hw, IXGBE_PF_HIBA(i));
   189				memcpy(buf + buf_size - buf_tail_size, &buf_tail,
   190				       buf_tail_size);
   191			}
   192		}
   193	
   194		return 0;
   195	}
   196	

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

Re: [PATCH iwl-next 1/2] ixgbe: e610: add missing endianness conversion

[Dan Carpenter]

On Fri, Jan 16, 2026 at 01:23:53PM +0100, Piotr Kwapulinski wrote:
> Fix a possible ACI issue on big-endian platforms.
> 
> Fixes: 46761fd52a88 ("ixgbe: Add support for E610 FW Admin Command Interface")
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> Signed-off-by: Piotr Kwapulinski <piotr.kwapulinski@intel.com>
> ---

Intel has a lot of code which assume that it will only run on little
endian systems...  Which is probably a fair assumption, honestly.
For example:
drivers/platform/x86/intel/uncore-frequency/uncore-frequency-common.c:90 store_attr() warn: passing casted pointer '&input' to 'kstrtobool()' 32 vs 1.
drivers/net/ethernet/intel/i40e/i40e_common.c:4345 i40e_led_get_reg() warn: passing casted pointer 'reg_val' to 'i40e_read_phy_register_clause45()' 32 vs 16.
I seem them on occasion when I'm reviewing static checker warnings but
I ignore them because Intel chips are little endian.

I don't have a problem with fixing Sparse endianness warnings, but the
commit message should really say that it doesn't affect real life.

regards,
dan carpenter

RE: [Intel-wired-lan] [PATCH iwl-next 1/2] ixgbe: e610: add missing endianness conversion

[Kwapulinski, Piotr]

>-----Original Message-----
>From: Paul Menzel <pmenzel@molgen.mpg.de> 
>Sent: Friday, January 16, 2026 2:18 PM
>To: Kwapulinski, Piotr <piotr.kwapulinski@intel.com>
>Cc: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; dan.carpenter@linaro.org; horms@kernel.org; Loktionov, Aleksandr <aleksandr.loktionov@intel.com>
>Subject: Re: [Intel-wired-lan] [PATCH iwl-next 1/2] ixgbe: e610: add missing endianness conversion
>
>Dear Piotr,
>
>
>Thank you for your patch.
>
>Am 16.01.26 um 13:23 schrieb Piotr Kwapulinski:
>> Fix a possible ACI issue on big-endian platforms.
>
>Please elaborate, why this is needed, and `raw_desc[i]` needs to be converted.
>
>For the summary/title, you could also be more specific. Something like:
Will update. Thank you.
Piotr

[...]

RE: [PATCH iwl-next 1/2] ixgbe: e610: add missing endianness conversion

[Kwapulinski, Piotr]

>-----Original Message-----
>From: Dan Carpenter <dan.carpenter@linaro.org> 
>Sent: Sunday, January 18, 2026 1:57 PM
>To: Kwapulinski, Piotr <piotr.kwapulinski@intel.com>
>Cc: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; horms@kernel.org; Loktionov, Aleksandr <aleksandr.loktionov@intel.com>
>Subject: Re: [PATCH iwl-next 1/2] ixgbe: e610: add missing endianness conversion
>
>On Fri, Jan 16, 2026 at 01:23:53PM +0100, Piotr Kwapulinski wrote:
>> Fix a possible ACI issue on big-endian platforms.
>> 
>> Fixes: 46761fd52a88 ("ixgbe: Add support for E610 FW Admin Command 
>> Interface")
>> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
>> Signed-off-by: Piotr Kwapulinski <piotr.kwapulinski@intel.com>
>> ---
>
>Intel has a lot of code which assume that it will only run on little endian systems...  Which is probably a fair assumption, honestly.
>For example:
>drivers/platform/x86/intel/uncore-frequency/uncore-frequency-common.c:90 store_attr() warn: passing casted pointer '&input' to 'kstrtobool()' 32 vs 1.
>drivers/net/ethernet/intel/i40e/i40e_common.c:4345 i40e_led_get_reg() warn: passing casted pointer 'reg_val' to 'i40e_read_phy_register_clause45()' 32 vs 16.
>I seem them on occasion when I'm reviewing static checker warnings but I ignore them because Intel chips are little endian.
>
>I don't have a problem with fixing Sparse endianness warnings, but the commit message should really say that it doesn't affect real life.
Will update. Thank you.
Piotr
[...]