linux-kernel - Re: [net-next PATCH] net: phy: aquantia: make mailbox interface4 lsw addr mask more specific

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: <20231121150859.7f934627@kernel.org>
Date:   Tue, 21 Nov 2023 15:08:59 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Christian Marangi <ansuelsmth@...il.com>
Cc:     Andrew Lunn <andrew@...n.ch>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Paolo Abeni <pabeni@...hat.com>,
        Robert Marko <robimarko@...il.com>, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org, kernel test robot <lkp@...el.com>
Subject: Re: [net-next PATCH] net: phy: aquantia: make mailbox interface4
 lsw addr mask more specific

On Mon, 20 Nov 2023 20:35:04 +0100 Christian Marangi wrote:
> It seems some arch (s390) require a more specific mask for FIELD_PREP
> and doesn't like using GENMASK(15, 2) for u16 values.
> 
> Fix the compilation error by adding the additional mask for the BITS
> that the PHY ignore and AND the passed addr with the real mask that the
> PHY will parse for the mailbox interface 4 addr to make sure extra
> values are correctly removed.

Ah. Um. Pff. Erm. I'm not sure.

Endianness is not my strong suit but this code:

	/* PHY expect addr in LE */
	addr = (__force u32)cpu_to_le32(addr); 

	/* ... use (u16)(addr)       */
	/* ... use (u16)(addr >> 16) */

does not make sense to me.

You're operating on register values here, there is no endian.
Endian only exists when you store or load from memory. IOW, this:

	addr = 0x12345678;
	print((u16)addr);
	print(addr >> 16);

will print the same exact thing regardless of the CPU endian.

Why did you put the byte swap in there?
-- 
pw-bot: cr