Re: [PATCH v4 net-next 07/15] net: enetc: add .ndo_set_vf_mac() to the enetc v4 driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the newly added `.ndo_set_vf_mac` callback directly exposes it: Incorrect MAC address programming on Big Endian architectures due to `__raw_writel()`.
--

commit d4ab065b05df3371bda6a210173bde546f473ad3
Author: Wei Fang <wei.fang@nxp.com>

net: enetc: add .ndo_set_vf_mac() to the enetc v4 driver
    
Add .ndo_set_vf_mac() to the enetc v4 driver to configure the MAC
addresses of VFs.

> diff --git a/drivers/net/ethernet/freescale/enetc/enetc4_pf.c b/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
> index e20989b6f8b49..a44d50b7fd2ad 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
> @@ -579,6 +579,7 @@ static const struct net_device_ops enetc4_ndev_ops = {
>  	.ndo_hwtstamp_get	= enetc_hwtstamp_get,
>  	.ndo_hwtstamp_set	= enetc_hwtstamp_set,
>  	.ndo_set_vf_trust	= enetc_pf_set_vf_trust,
> +	.ndo_set_vf_mac		= enetc_pf_set_vf_mac,

[Severity: High]
This isn't a bug introduced by this patch, but the newly added
.ndo_set_vf_mac callback directly exposes a pre-existing issue in the MAC
address programming.

When the MAC address is set, the driver invokes enetc4_pf_set_si_primary_mac()
via the enetc_pf_set_vf_mac() -> enetc_set_si_hw_addr() call chain:

drivers/net/ethernet/freescale/enetc/enetc4_pf.c:enetc4_pf_set_si_primary_mac() {
	u16 lower = get_unaligned_le16(addr + 4);
	u32 upper = get_unaligned_le32(addr);

	if (si != 0) {
		__raw_writel(upper, hw->port + ENETC4_PSIPMAR0(si));
		__raw_writel(lower, hw->port + ENETC4_PSIPMAR1(si));
    ...
}

Could the use of __raw_writel() cause the MAC address to be programmed with
reversed byte order on Big Endian architectures?

Since get_unaligned_le32() returns a host-endian integer, writing it directly
using __raw_writel() on a Big Endian system will write the bytes in reverse
order compared to what a Little Endian PCI device expects.

Furthermore, will writing the u16 lower value using the 32-bit __raw_writel()
place the MAC bytes in the wrong word offset on Big Endian systems?

This would cause the network interface to drop legitimate unicast traffic.

>  };
>  
>  static struct phylink_pcs *

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909100733.1139689-1-wei.fang@oss.nxp.com?part=7