Re: [PATCH net-next v2 2/8] eth: mpnic: add register init for the device

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] mpnic_dev_init() writes BAR0 CSRs up to MPNIC_OB_ATTR_RDE_P
  (dword 0x9a001a, byte 0x2680068 + 8, about 38.5 MiB into BAR0), and…

--

eth: mpnic: add register init for the device

This commit adds one-time CSR initialization to mpnic_probe() through a
new mpnic_dev_init() in mpnic_init.c. The new code sets static TXB, RXB,
TQS, AXI and relaxed-ordering configuration. It also starts SRAM zeroing
on several blocks and polls until that finishes.

> diff --git a/drivers/net/ethernet/meta/mpnic/mpnic_pci.c b/drivers/net/ethernet/meta/mpnic/mpnic_pci.c
> index 96393e7812418..68a64377e6c5a 100644
> --- a/drivers/net/ethernet/meta/mpnic/mpnic_pci.c
> +++ b/drivers/net/ethernet/meta/mpnic/mpnic_pci.c

[ ... ]

> @@ -108,6 +112,10 @@ static int mpnic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	pci_set_master(pdev);
>  	pci_save_state(pdev);
>  
> +	err = mpnic_dev_init(mpd);
> +	if (err)
> +		goto err_free_mpd;
> +

[Severity: Low]
Should mpnic_probe() check the size of BAR0 before mpnic_dev_init() runs?

mpnic_probe() maps BAR0 with pcim_iomap_region() but never compares
pci_resource_len(pdev, 0) with the highest CSR offset the driver uses.
Neither mpnic_wr64() nor mpnic_rd64() puts an upper limit on reg:

    if (csr)
        writeq(val, csr + reg);

The highest register mpnic_dev_init() writes is MPNIC_OB_ATTR_RDE_P, from
mpnic_ro_init(). Its byte offset is 0x2680068, about 38.5 MiB into BAR0.
mpnic_desc_sram_init() also issues 4096 writeq() calls at offsets
computed from the loop index:

    for (i = 0; i < MPNIC_MAX_TXQS * 2; i++)
        mpnic_wr64(mpd, MPNIC_TDF_DESC_CFG(i), ...);

A device that matches the PCI ID but reports a smaller BAR0 would make
these accesses land outside the ioremap'd region during probe.

The PCI core normally trusts BAR sizes reported by the device, and fbnic
maps its BARs without a length check too. So this would be defensive
hardening rather than a fix for a known failure. Would a check against
the largest CSR offset in mpnic_probe() be worth adding?

>  	return 0;
>  
>  err_free_mpd:

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260924-linux-mpnic-v2-0-4badc9b58b9e%40gmail.com