Re: [PATCH net-next v3 03/13] net: microchip: fdma: add PCIe ATU support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Writing a potentially unaligned host DMA address to the ATU target register causes hardware truncation, leading to corrupted DMA memory accesses.
- [High] ATU hardware state is not cleared during initialization, which can lead to wild DMAs from stale window configurations.
--

commit e1449c4cacf5f519b9e9335c915ef4cb12b7ad20
Author: Daniel Machon <daniel.machon@microchip.com>

net: microchip: fdma: add PCIe ATU support

This commit adds PCIe Address Translation Unit (ATU) support for the
microchip fdma driver. It divides the outbound address space into six
regions and maps coherent DMA memory allocations to these regions for PCIe
endpoint operation.

> diff --git a/drivers/net/ethernet/microchip/fdma/fdma_pci.c b/drivers/net/ethernet/microchip/fdma/fdma_pci.c
> new file mode 100644
> index 0000000000000..1bd41eaa58a4b
> --- /dev/null
> +++ b/drivers/net/ethernet/microchip/fdma/fdma_pci.c

[ ... ]

> +/* Map a host DMA address into a free outbound region. */
> +struct fdma_pci_atu_region *
> +fdma_pci_atu_region_map(struct fdma_pci_atu *atu, u64 target_addr, int size)
> +{
> +	struct fdma_pci_atu_region *region;

[ ... ]

> +	region->target_addr = target_addr;
> +	region->in_use = true;
> +
> +	/* Enable first, according to datasheet section 3.24.7.4.1 */
> +	fdma_pci_atu_region_enable(region);
> +	fdma_pci_atu_configure_translation(region);
> +
> +	return region;
> +}

Could this lead to corrupted DMA memory accesses due to hardware truncation?

The target_addr passed here comes from fdma_alloc_coherent() which uses
dma_alloc_coherent() and only guarantees PAGE_SIZE alignment (e.g., 4KB).

However, the ATU region size is explicitly aligned to 64KB
(FDMA_PCI_ATU_REGION_ALIGN). PCIe ATUs typically use bitwise substitution
for the address offset and ignore or truncate the lower bits of the target
address register.

If the allocated DMA address is 4KB-aligned but not 64KB-aligned, and the
ATU hardware truncates the lower bits, will this cause a mismatch since
fdma_pci_atu_translate_addr() computes the descriptor address assuming
the exact, untruncated target_addr was used?

[ ... ]

> +/* Initialize ATU, dividing the OB space into equally sized regions. */
> +void fdma_pci_atu_init(struct fdma_pci_atu *atu, void __iomem *addr)
> +{
> +	struct fdma_pci_atu_region *regions = atu->regions;
> +	u32 region_size = fdma_pci_atu_region_size();
> +
> +	atu->addr = addr;
> +
> +	for (int i = 0; i < FDMA_PCI_ATU_REGION_MAX; i++) {
> +		regions[i].base_addr =
> +			FDMA_PCI_ATU_OB_START + (i * region_size);
> +		regions[i].limit_addr =
> +			regions[i].base_addr + region_size - 1;
> +		regions[i].idx = i;
> +		regions[i].atu = atu;
> +	}
> +}

Could this leave the hardware in an inconsistent state and lead to wild DMAs
from stale window configurations?

This initializes the software tracking array, but does not appear to issue
any writes to the hardware to clear or disable the ATU windows.

In scenarios like driver unbind/bind or kexec, the ATU hardware might
retain its previously configured target addresses and enable bits. Should
all ATU regions be explicitly disabled during initialization to prevent
residual traffic from using stale host physical addresses?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260504-lan966x-pci-fdma-v3-0-a56f5740d870@microchip.com?part=3