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

[Daniel Zahka]

On Sun Sep 27, 2026 at 8:01 PM EDT, netdev-bot+sashiko wrote:
> 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:

These are in the range of our 64MiB BAR0.