Re: [PATCH v3 net 1/9] net: enetc: fix incorrect mailbox message status returned to VFs

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] `__raw_writel` and `__raw_writew` are used for MAC address registers instead of standard I/O accessors, causing incorrect byte ordering on Big-Endian architectures.
- [High] The mailbox message structures passed over DMA use native `u16` instead of explicitly sized and endian-annotated types, causing communication failures between differing architectures.
--

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

net: enetc: fix incorrect mailbox message status returned to VFs

This commit corrects the mailbox message status returned to VFs in two
cases. It ensures that unsupported command types return a failure status
instead of retaining the default success status. It also returns a failure
when a VF attempts to override a MAC address that has already been set by
the PF.

> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.c b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
> index a12fd54a475f6..27d4bb65e017f 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
> @@ -493,11 +493,13 @@ static u16 enetc_msg_pf_set_vf_primary_mac_addr(struct enetc_pf *pf,
>  		return ENETC_MSG_CMD_STATUS_FAIL;
>  
>  	addr = cmd->mac.sa_data;
> -	if (vf_state->flags & ENETC_VF_FLAG_PF_SET_MAC)
> +	if (vf_state->flags & ENETC_VF_FLAG_PF_SET_MAC) {
>  		dev_warn(dev, "Attempt to override PF set mac addr for VF%d\n",
>  			 vf_id);
> -	else
> -		enetc_pf_set_primary_mac_addr(&pf->si->hw, vf_id + 1, addr);
> +		return ENETC_MSG_CMD_STATUS_FAIL;
> +	}
> +
> +	enetc_pf_set_primary_mac_addr(&pf->si->hw, vf_id + 1, addr);

[Severity: High]
This is a pre-existing issue, but does enetc_pf_set_primary_mac_addr() handle
endianness correctly on Big-Endian architectures?

Looking at the implementation:

static void enetc_pf_set_primary_mac_addr(struct enetc_hw *hw, int si,
					  const u8 *addr)
{
	u32 upper = get_unaligned_le32(addr);
	...
	__raw_writel(upper, hw->port + ENETC_PSIPMAR0(si));
	...
}

Using __raw_writel() instead of writel() bypasses byte-swapping. Since
get_unaligned_le32() converts the MAC bytes into a CPU-native 32-bit integer,
writing it directly with __raw_writel() on a Big-Endian system will reverse
the bytes compared to what a Little-Endian device expects.

Could this cause the hardware to be programmed with a corrupted MAC address,
and should it use writel() or the driver's standard I/O accessors instead?

>  
>  	return ENETC_MSG_CMD_STATUS_OK;
>  }
> @@ -509,7 +511,6 @@ void enetc_msg_handle_rxmsg(struct enetc_pf *pf, int vf_id, u16 *status)
>  	struct enetc_msg_cmd_header *cmd_hdr;
>  	u16 cmd_type;
>  
> -	*status = ENETC_MSG_CMD_STATUS_OK;
>  	cmd_hdr = (struct enetc_msg_cmd_header *)msg->vaddr;
>  	cmd_type = cmd_hdr->type;

[Severity: High]
This is a pre-existing issue, but will reading cmd_hdr->type cause
communication failures if the VF guest and host PF have different endianness?

The mailbox message structures passed over DMA use native u16 types:

struct enetc_msg_cmd_header {
	u16 type;
	u16 id;
};

If a Little-Endian guest runs on a Big-Endian host, the PF will misinterpret
the command types because it reads them in its native byte order.

Do these fields need to be explicitly sized and endian-annotated (e.g., __le16)
with le16_to_cpu() conversions applied when reading them to ensure cross-endian
configurations function correctly?

[ ... ]

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