Re: [PATCH net 2/4] net: ti: icssg: Use undirected TX tag for native XDP in HSR offload mode

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] DMA Mapping / Page Pool Corruption
--

commit e56804b68dffcb60ec1826c1b1a00d1c6ff8b34b
Author: Meghana Malladi <m-malladi@ti.com>

net: ti: icssg: Use undirected TX tag for native XDP in HSR offload mode

This commit updates emac_xmit_xdp_frame() to set the correct destination
tag and tag insertion flags for XDP TX frames when in HSR offload mode.
This allows the PRU firmware to properly duplicate the frames and insert
the HSR sequence tag.

> diff --git a/drivers/net/ethernet/ti/icssg/icssg_common.c b/drivers/net/ethernet/ti/icssg/icssg_common.c
> index 55a696912811f..ede32f266729e 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_common.c
> +++ b/drivers/net/ethernet/ti/icssg/icssg_common.c

[ ... ]

> @@ -737,9 +738,25 @@ u32 emac_xmit_xdp_frame(struct prueth_emac *emac,
>  
>  	/* set dst tag to indicate internal qid at the firmware which is at
>  	 * bit8..bit15. bit0..bit7 indicates port num for directed
> -	 * packets in case of switch mode operation
> +	 * packets in case of switch mode operation and port num 0
> +	 * for undirected packets in case of HSR offload mode.
> +	 *
> +	 * XDP_TX frames arrive on a slave port with the HSR tag already
> +	 * stripped by the PRU firmware.  Like skb TX via hsr0, they must
> +	 * be sent as undirected so the PRU duplicates them to both ports
> +	 * and re-inserts the HSR sequence tag.
>  	 */
> -	cppi5_desc_set_tags_ids(&first_desc->hdr, 0, (emac->port_id | (q_idx << 8)));
> +	dst_tag_id = emac->port_id | (q_idx << 8);
> +
> +	if (emac->prueth->is_hsr_offload_mode &&
> +	    (ndev->features & NETIF_F_HW_HSR_DUP))
> +		dst_tag_id = PRUETH_UNDIRECTED_PKT_DST_TAG;
> +
> +	if (emac->prueth->is_hsr_offload_mode &&
> +	    (ndev->features & NETIF_F_HW_HSR_TAG_INS))
> +		epib[1] |= PRUETH_UNDIRECTED_PKT_TAG_INS;
> +
> +	cppi5_desc_set_tags_ids(&first_desc->hdr, 0, dst_tag_id);

[Severity: High]
This is a pre-existing issue, but is it possible that prueth_xmit_free()
inadvertently destroys the persistent DMA mapping managed by the page pool
for XDP_TX frames?

In emac_xmit_xdp_frame(), when an XDP frame is transmitted via XDP_TX, it
reuses the page pool's pre-existing DMA address without creating a new mapping:

    if (buff_type == PRUETH_TX_BUFF_TYPE_XDP_TX) {
        page = virt_to_head_page(xdpf->data);
        ...
        buf_dma = page_pool_get_dma_addr(page);
        buf_dma += xdpf->headroom + sizeof(struct xdp_frame);
    } else {
        buf_dma = dma_map_single(...);

However, during TX completion or error handling, prueth_xmit_free()
unconditionally calls dma_unmap_single() on the buffer for PRUETH_SWDATA_XDPF:

    dma_unmap_single(tx_chn->dma_dev, buf_dma, buf_dma_len,
                     DMA_TO_DEVICE);

Could this cause an IOMMU fault or memory corruption when this page is
later recycled and the hardware tries to DMA into the unmapped address?

>  	k3_udma_glue_tx_dma_to_cppi5_addr(tx_chn->tx_chn, &buf_dma);
>  	cppi5_hdesc_attach_buf(first_desc, buf_dma, xdpf->len, buf_dma, xdpf->len);
>  	swdata = cppi5_hdesc_get_swdata(first_desc);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260611185744.2498070-1-m-malladi@ti.com?part=2