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

["Wei Fang (OSS)" <wei.fang@oss.nxp.com>]

> Pre-existing issues:
> - [High] Asymmetrical MAC address register access between set and get
> functions for the PF (si == 0), causing the bootloader's MAC address to be
> ignored and overwritten.
> --
> 
> commit 7332ce3a436cebfb9b0a304898ec0b592d1b0ff9
> 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 17fd9ee27942f..eeb70feeb7737 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 is a pre-existing issue, but while reviewing the MAC address handling in
> this file, I noticed an asymmetrical register access between the set and get
> functions for the PF (when si == 0).
> 
> In enetc4_pf_get_si_primary_mac(), the code unconditionally reads from
> PSIPMAR0:
> 
>     upper = __raw_readl(hw->port + ENETC4_PSIPMAR0(si));
>     lower = __raw_readl(hw->port + ENETC4_PSIPMAR1(si));
> 
> However, the set function enetc4_pf_set_si_primary_mac() correctly writes
> to ENETC4_PMAR0 for si == 0.
> 
> Does this cause the PF (si == 0) to fail to read the primary MAC address
> configured by the bootloader or firmware during enetc4_pf_init()?
> 
> Since enetc_setup_mac_addresses() calls enetc_get_si_hw_addr(), wouldn't this
> read from the wrong register, assume no MAC is present, and generate a
> random
> MAC address that overwrites the bootloader's MAC address?
> 

This is a false positive. For the PF (SI 0), PSIPMAR0/1(0) are read only
registers, the MAC address of PF should be configured by PMAR0/1
registers. The values in PMAR0/1 are reflected in PSIPMAR0/1(0), so
the driver can get the MAC address from PSIPMAR0/1(0).