Re: [PATCH v30 net-next 2/8] net/nebula-matrix: add common resource implementation

[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] Baseline: the out-of-bounds MMIO hazard is not reachable, but the
  comment is wrong.

--

net/nebula-matrix: add common resource implementation

This patch adds a chip-agnostic resource layer for the Nebula Matrix
driver. It builds read-only PF topology, SR-IOV BDF, Ethernet port and
VSI tables on the control PF at probe time. It also adds several new
hw_ops, including flush_write, get_real_bus, get_fw_eth_map and
get_board_info.

> 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
> index eeff6216e4aa..4d3477f70bcc 100644
> --- 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
> @@ -97,6 +97,32 @@ static void nbl_hw_rd_regs_lock(struct nbl_hw_mgt *hw_mgt, u64 reg, u32 *data,
>  	spin_unlock(&hw_mgt->reg_lock);
>  }
>  
> +/*
> + * Only call this when has_ctrl=true, which maps enough space
> + * (bar_len - 8192) to cover NBL_HW_DUMMY_REG (0x1300904).
> + * The flow/design guarantees this is only called in the
> + * has_ctrl path.
> + */
> +static void nbl_flush_writes(struct nbl_hw_mgt *hw_mgt)
> +{
> +	nbl_hw_rd32(hw_mgt, NBL_HW_DUMMY_REG);
> +}

[Severity: Low]
Is the "(bar_len - 8192)" part of this comment accurate? The has_ctrl
path in nbl_hw_init_leonis() does not size the mapping from bar_len:

    hw_size = PAGE_ALIGN_DOWN(NBL_MEM_BAR_TOTAL_SIZE -
                              NBL_RDMA_NOTIFY_LEN);
    hw_mgt->hw_addr =
        pcim_iomap(pdev, NBL_MEMORY_BAR,
                   hw_size);

The size is fixed and does not depend on bar_len. It is also smaller
than 64 MiB - 8 KiB when PAGE_SIZE is larger than 8 KiB.

The has_ctrl-only rule is also written only in this comment. The
flush_write member of struct nbl_hw_ops in nbl_def_hw.h says nothing
about it, and nbl_hw_setup_ops() installs the op for every PF. That
includes net-only PFs, which map only NBL_REG_NET_ONLY_LEN (8 KiB). For
those PFs, NBL_HW_DUMMY_REG (0x1300904) is well outside the mapping.

This patch adds no callers. Later in the series, the flush_write callers
in nbl_interrupt.c all seem to sit behind has_ctrl checks, so the
out-of-bounds read does not look reachable. Could the comment be
corrected? Could the has_ctrl requirement also be documented on the
flush_write op in nbl_def_hw.h, or enforced in the code?

[ ... ]

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