netdev - Re: [PATCH iwl-net] ice: fix Get Tx Topology AQ command error on E830

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <a5f27387-463f-4532-b4bc-0fc81b9a96de@intel.com>
Date: Fri, 21 Feb 2025 14:41:10 -0800
From: Jacob Keller <jacob.e.keller@...el.com>
To: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
CC: Tony Nguyen <anthony.l.nguyen@...el.com>, Przemek Kitszel
	<przemyslaw.kitszel@...el.com>, <intel-wired-lan@...ts.osuosl.org>,
	<netdev@...r.kernel.org>, Paul Greenwalt <paul.greenwalt@...el.com>
Subject: Re: [PATCH iwl-net] ice: fix Get Tx Topology AQ command error on E830



On 2/20/2025 11:29 PM, Michal Swiatkowski wrote:
> On Thu, Feb 20, 2025 at 02:45:41PM -0800, Jacob Keller wrote:
>>
>>
>> On 2/19/2025 1:37 AM, Michal Swiatkowski wrote:
>>> On Tue, Feb 18, 2025 at 04:46:34PM -0800, Jacob Keller wrote:
>>>> From: Paul Greenwalt <paul.greenwalt@...el.com>
>>>>
>>>> With E830 Get Tx Topology AQ command (opcode 0x0418) returns an error when
>>>> setting the AQ command read flag, and since the get command is a direct
>>>> command there is no need to set the read flag.
>>>>
>>>> Fix this by only setting read flag on set command.
>>>
>>> Why it isn't true for other hw? I mean, why not:
>>> if (set)
>>> 	RD_FLAG
>>> else 
>>> 	NOT_RD_FLAG
>>> Other hw needs RD flag in case of get too?
>>>
>>
>> From what I understand, we didn't anticipate this flow changing. E810
>> and E822 hardware require FLAG_RD for both get and set, while E825-C and
>> E830 expect FLAG_RD only for set, but not for get.
>>
> 
> Thanks for explanation. Seems resonable from driver perspective and not
> so reasonable from firmware, but maybe this difference is somehow
> important.
> 
> Thanks,
> Michal
> 

Yea, its unfortunate that this changed at all, but we can't really go
back on released firmware versions.

The RD_FLAG is described in the datasheet as:

Flags.RD: Set by driver to indicate that FW needs to read indirect buffer.

It is intended to indicate the presence of the indirect buffer vs a
direct command. I believe what has likely happened is that this is
interpreted in a strict manner so missing the flag causes the commands
to fail. Its quite weird that older firmware required it for a direct
command, so someone probably changed this thinking thats strange..

I haven't been able to dig up more info about this or why it was changed.

Thanks,
Jake