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

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

[sashiko-bot]

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

Pre-existing issues:
- [High] The v4 driver's MAC address programming functions (`enetc4_pf_set_si_primary_mac` and `enetc4_pf_get_si_primary_mac`) use `__raw_writel` and `__raw_readl` coupled with LE-unaligned helpers, which silently breaks the MAC address configuration on Big Endian architectures.
--

commit fb7fbde8e308b4c501381b6a15d58e74e9aff89a
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 4b69c19b23194..d310506a6fd4b 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
> @@ -575,6 +575,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 is a pre-existing issue, but I'd like to ask about the endianness handling
in the underlying v4 hardware accessors used for MAC configuration.

Looking at enetc4_pf_set_si_primary_mac() and enetc4_pf_get_si_primary_mac()
in drivers/net/ethernet/freescale/enetc/enetc4_pf.c, they use __raw_writel()
and __raw_readl():

    __raw_writel(lower, hw->port + ENETC4_PSIPMAR1(si));
    ...
    lower = __raw_readl(hw->port + ENETC4_PSIPMAR1(si));

Does this silently break MAC address configuration on Big Endian architectures?

get_unaligned_le32() reads the first 4 bytes of the MAC address and returns
them as a host-endian 32-bit integer. __raw_writel() then writes this value
directly to memory without byte swapping. On a Big Endian system, this causes
the bytes to be written in reverse order to the little-endian PCIe device.

For the remaining 16 bits, get_unaligned_le16() returns a host-endian 16-bit
value which is passed into __raw_writel(). The value is zero-extended to
32 bits. On a Big Endian system, writing this directly via __raw_writel()
places the non-zero MAC bytes at offset +2 and +3 instead of offset +0 and +1,
effectively programming 00:00 into the device's MAC address.

The read function enetc4_pf_get_si_primary_mac() is also broken on Big Endian:
__raw_readl() places the MAC bytes into the upper 16 bits of the 32-bit return
value. When assigned to a u16 variable, the value is truncated and the MAC
address is discarded, resulting in 00:00.

Should the driver use standard writel() / readl() or the driver's
enetc_port_wr() to ensure correct host-to-little-endian conversion?

>  };
>  
>  static struct phylink_pcs *

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