netdev - Re: [PATCH iwl-net] i40e: Fix max frame size check

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <d6114f2b-4ac8-ce15-19f6-483965daf3f3@intel.com>
Date: Wed, 8 Nov 2023 12:38:00 -0800
From: Jesse Brandeburg <jesse.brandeburg@...el.com>
To: Ivan Vecera <ivecera@...hat.com>, <intel-wired-lan@...ts.osuosl.org>
CC: Jacob Keller <jacob.e.keller@...el.com>, Wojciech Drewek
	<wojciech.drewek@...el.com>, Tony Nguyen <anthony.l.nguyen@...el.com>, "David
 S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, "Jakub
 Kicinski" <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, "open
 list:NETWORKING DRIVERS" <netdev@...r.kernel.org>
Subject: Re: [PATCH iwl-net] i40e: Fix max frame size check

On 11/8/2023 7:10 AM, Ivan Vecera wrote:
> Commit 3a2c6ced90e1 ("i40e: Add a check to see if MFS is set") added
> a check for port's MFS (max frame size) value. The value is stored
> in PRTGL_SAH register for each physical port. According datasheet this
> register is defined as:
> 
> PRTGL_SAH[PRT]: (0x001E2140 + 0x4*PRT, PRT=0...3)
> 
> where PRT is physical port number.

<trimmed lkml, and a couple of non-existent intel addresses>

Was there an actual problem here? I suspect if you read all the
registers for each PF's BAR, you'll find that all 4 report the same,
correct value, for the perspective of the BAR they're being read from.

The i40e hardware does this (somewhat non-obvious) for *lots* of port
specific registers, and what happens is no matter which of the 4 you
read the value from, you'll get the right "port specific" value. This is
because the hardware designers decided to make a different "view" on the
register set depending on which PF you access it from. The only time
these offsets matter is when the part is in debug mode or when the
firmware is reading the internal registers (from the internal firmware
register space - which has no aliasing) that rely on the correct offset.

In this case, I think your change won't make any functional difference,
but I can see why you want to make the change as the code doesn't match
the datasheet's definition of the register.

That all said, unless you can prove a problem, I'm relatively sure that
nothing is wrong here in functionality or code. And if you go this
route, there might be a lot of other registers to fix that have the same
aliasing.

I apologize for the confusing manuals and header file, it's complicated
but in practice works really well. Effectively you can't read other
port's registers by accident.

Here was my experiment showing the aliasing on X722. You'll note that
the lower 16 bits are a MAC address that doesn't change no matter which
register you read.

device 20:0.0
0x1e2140 == 0x26008245
0x1e2144 == 0x26008245
0x1e2148 == 0x26008245
0x1e214c == 0x26008245
device 20:0.1
0x1e2140 == 0x26008345
0x1e2144 == 0x26008345
0x1e2148 == 0x26008345
0x1e214c == 0x26008345
device 20:0.2
0x1e2140 == 0x26008445
0x1e2144 == 0x26008445
0x1e2148 == 0x26008445
0x1e214c == 0x26008445
device 20:0.3
0x1e2140 == 0x26008545
0x1e2144 == 0x26008545
0x1e2148 == 0x26008545
0x1e214c == 0x26008545

lspci -d ::0200
20:00.0 Ethernet controller: Intel Corporation Ethernet Connection X722
for 10GBASE-T (rev 04)
20:00.1 Ethernet controller: Intel Corporation Ethernet Connection X722
for 10GBASE-T (rev 04)
20:00.2 Ethernet controller: Intel Corporation Ethernet Connection X722
for 10GbE SFP+ (rev 04)
20:00.3 Ethernet controller: Intel Corporation Ethernet Connection X722
for 10GbE SFP+ (rev 04)

Hope this helps!