Re: [v26,net-next,02/10] net/nebula-matrix: add core driver architecture and HW layer initialization

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 · Medium: 1 · Low: 0

- [Medium] In `nbl_hw_init_leonis()` (drivers/net/ethernet/nebula-
  matrix/nbl/nbl_hw/nbl_hw_leonis/nbl_hw_leonis.c) two error paths print…

--

net/nebula-matrix: add core driver architecture and HW layer initialization

This adds the nbl_adapter/nbl_core/nbl_common_info device context, the PCI
probe/remove entry points with basic capability parsing, and the Leonis
hardware layer that requests the BARs and ioremaps them.  One question
below about the BAR size error paths.

> diff --git a/drivers/net/ethernet/nebula-matrix/nbl/nbl_hw/nbl_hw_leonis/nbl_hw_leonis.c b/drivers/net/ethernet/nebula-matrix/nbl/nbl_hw/nbl_hw_leonis/nbl_hw_leonis.c
> new file mode 100644
> index 000000000000..a67d1a674466
> --- /dev/null
> +++ b/drivers/net/ethernet/nebula-matrix/nbl/nbl_hw/nbl_hw_leonis/nbl_hw_leonis.c
> @@ -0,0 +1,152 @@

[ ... ]

> +int nbl_hw_init_leonis(struct nbl_adapter *adapter)
> +{
> +	resource_size_t expect_sz = NBL_MEM_BAR_TOTAL_SIZE;
> +	struct nbl_common_info *common = &adapter->common;
> +	struct pci_dev *pdev = common->pdev;
> +	struct nbl_hw_mgt *hw_mgt = NULL;
> +	resource_size_t bar_len;

[ ... ]

> +		if (bar_len < NBL_MEM_BAR_TOTAL_SIZE) {
> +			dev_err(&pdev->dev,
> +				"MEMORY BAR len %pr smaller than expected %pr\n",
> +				&bar_len, &expect_sz);

[Severity: Medium]
Should these two be %pa rather than %pr?

Both bar_len and expect_sz are plain resource_size_t locals on the stack,
but %pr is the struct resource specifier.  vsprintf hands it to
resource_string(), which dereferences the argument as a struct resource:

lib/vsprintf.c:resource_string() {
	...
	if (res->flags & IORESOURCE_IO) {
	...
		p = hex_range(p, pend, res->start, res->end, *specp);
	...
}

struct resource places flags at offset 24 on 64-bit, so does this read
roughly 32 bytes past the end of each 8-byte local and print adjacent
kernel stack contents (both the decoded resource type and the second hex
value) into dmesg?

Nothing warns at build time because every %p variant just consumes a
void *.  Documentation/core-api/printk-formats.rst lists %pa[p] for
phys_addr_t / resource_size_t, so %pa &bar_len (or a cast to
unsigned long long with %llu) would seem to be what is wanted here.

The path is reached from nbl_probe() -> nbl_core_init() ->
nbl_hw_init_leonis() whenever common->has_ctrl is set (function 0, not a
VF) and pci_resource_len(pdev, NBL_MEMORY_BAR) is below 64MiB.

> +			ret = -EINVAL;
> +			goto setup_mgt_fail;
> +		}

[ ... ]

> +	} else {
> +		if (bar_len < NBL_REG_NET_ONLY_LEN) {
> +			dev_err(&pdev->dev,
> +				"MEMORY BAR len %pr too small for net only reg space\n",
> +				&bar_len);

[Severity: Medium]
Same %pr versus %pa question on this second site in nbl_hw_init_leonis(),
which fires on a non-ctrl function when the MEMORY BAR is reported smaller
than 8KiB.

This one still appears unchanged at the end of the series, so no later
patch in the set seems to address it.

> +			ret = -EINVAL;
> +			goto setup_mgt_fail;
> +		}

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260831021408.2325-1-illusion.wang%40nebula-matrix.com