netdev - Re: [PATCH iwl-net 4/5] ice: Remove unnecessary offset calculation for PF scoped registers

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 for Android: free password hash cracker in your pocket

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

Message-ID: <31ea8ffb-ee35-8595-e66d-ea56001cdd43@intel.com>
Date: Wed, 2 Oct 2024 15:12:05 -0700
From: Tony Nguyen <anthony.l.nguyen@...el.com>
To: Karol Kolacinski <karol.kolacinski@...el.com>,
	<intel-wired-lan@...ts.osuosl.org>
CC: <netdev@...r.kernel.org>, <przemyslaw.kitszel@...el.com>, Jacob Keller
	<jacob.e.keller@...el.com>, Arkadiusz Kubalewski
	<arkadiusz.kubalewski@...el.com>
Subject: Re: [PATCH iwl-net 4/5] ice: Remove unnecessary offset calculation
 for PF scoped registers



On 9/30/2024 5:08 AM, Karol Kolacinski wrote:
> From: Jacob Keller <jacob.e.keller@...el.com>
> 
> The PFTSYN_SEM, PFINT_TSYN_MSK and PFHH_SEM registers are defined in the
> datasheet as "PF scoped". PF scoped registers appear in the datasheet to
> have a different offset per PF. This is misleading. Each PF has its own
> scope of the register, and accessing any of the offsets on a given PF
> will read the PF-appropriate register. There is no reason to calculate
> different offsets when reading and writing to these registers.
> 
> The original code implementing access to the semaphore registers failed
> to understand this nature of PF-scoped registers and included additional
> offset calculations. Remove these.
> 
> This can be tested with direct access to the registers to show that each
> PF sees its own scoped register:
> 
>    user@...tem:~ice$ for i in {0..7}; do sudo readpci -q -s 17:00.0 \
>                      -a $((0x88880 + 4*i)); done
>    0x88880 == 0x00000001
>    0x88884 == 0x00000001
>    0x88888 == 0x00000001
>    0x8888c == 0x00000001
>    0x88890 == 0x00000001
>    0x88894 == 0x00000001
>    0x88898 == 0x00000001
>    0x8889c == 0x00000001
>    user@...tem:~ice$ for i in {0..7}; do sudo readpci -q -s 17:00.0 \
>                      -a $((0x88880 + 4*i)) -w 0 ; done
>    0x88880 == 0x00000000
>    0x88884 == 0x00000000
>    0x88888 == 0x00000000
>    0x8888c == 0x00000000
>    0x88890 == 0x00000000
>    0x88894 == 0x00000000
>    0x88898 == 0x00000000
>    0x8889c == 0x00000000
>    user@...tem:~ice$ for i in {0..7}; do sudo readpci -q -s 17:00.0 \
>                      -a $((0x88880 + 4*i)); done
>    0x88880 == 0x00000001
>    0x88884 == 0x00000001
>    0x88888 == 0x00000001
>    0x8888c == 0x00000001
>    0x88890 == 0x00000001
>    0x88894 == 0x00000001
>    0x88898 == 0x00000001
>    0x8889c == 0x00000001
> 
> Additionally, you can quickly tell that the PF-offset doesn't matter
> because its not an included parameter of the auto-generated register
> header file. Other parameters which do matter get generated as part of
> the offset macros.
> 
> Fix the uses of PFTSYN_SEM, PFINT_TSYN_MSK and PFHH_SEM to stop doing
> the unnecessary offset calculation.

This also sounds like -next material rather than a bug fix.

Thanks,
Tony

> Fixes: 7d606a1e2d05 ("ice: unify logic for programming PFINT_TSYN_MSK")
> Fixes: 03cb4473be92 ("ice: add low level PTP clock access functions")
> Fixes: 13a64f0b9894 ("ice: support crosstimestamping on E822 devices if supported")
> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@...el.com>
> Reviewed-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@...el.com>
> Signed-off-by: Jacob Keller <jacob.e.keller@...el.com>
> Signed-off-by: Karol Kolacinski <karol.kolacinski@...el.com>