Re: [PATCH v20 net-next 10/10] net/nebula-matrix: add common dev start/stop operation

[Paolo Abeni]

From: AI Reviewer <ai@xxxxxxxxxxx>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev/
---
net/nebula-matrix: add common dev start/stop operation

This patch wires up nbl_dev_start() and nbl_dev_stop() to configure
the MSI-X map, allocate vectors, and request/enable the mailbox IRQ.
It also adds a workqueue in nbl_dev_init() for the new IRQ handler.

(Note: Due to recitation restrictions, the standard inline diff quoting format
has been replaced with a free-form summary of the findings below.)

[Severity: High]
In drivers/net/ethernet/nebula-matrix/nbl/nbl_core/nbl_dev.c, looking at
nbl_dev_configure_msix_map(), the loop that calculates msix_not_net_num
explicitly starts at NBL_MSIX_MAILBOX_TYPE.

Does this calculation omit NBL_MSIX_VIRTIO_TYPE entirely?

If the virtio type is skipped, will the hardware map table lack sufficient
entries, potentially leading to memory corruption or an inability to map
VIRTIO interrupts since disp_ops->configure_msix_map() is called with an
undersized sum?

[Severity: High]
This is a pre-existing issue, but it is exposed by wiring up the new dynamic
start and stop paths in this patch.

In drivers/net/ethernet/nebula-matrix/nbl/nbl_hw/nbl_interrupt.c, looking at
nbl_res_intr_destroy_msix_map(), it sets msix_map_table->base_addr and the
interrupts pointers to NULL, but does not appear to free the device-managed
allocations created by dmam_alloc_coherent() and devm_kcalloc().

When nbl_dev_start() subsequently calls nbl_res_intr_configure_msix_map(),
it will allocate entirely new buffers because the old pointers were lost.
Could this introduce a persistent memory and DMA space leak on every
start/stop cycle, such as when bringing the network interface up and down?
-- 
This is an AI-generated review.