Re: [net-next PATCH v2 4/4] octeontx2-pf: cn20k: Use unified Halo context

On 2026-03-20 at 22:20:37, Simon Horman (horms@kernel.org) wrote:

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
octeontx2-pf: cn20k: Use unified Halo context

Use unified Halo context present in CN20K hardware for
octeontx2 netdevs instead of aura and pool contexts.

quoted

diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/cn20k.c b/drivers/net/ethernet/marvell/octeontx2/nic/cn20k.c
--- a/drivers/net/ethernet/marvell/octeontx2/nic/cn20k.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/cn20k.c

[ ... ]

quoted

+int cn20k_npa_alloc_dpc(struct otx2_nic *nic)
+{
+	struct npa_cn20k_dpc_alloc_req *req;
+	struct npa_cn20k_dpc_alloc_rsp *rsp;
+	int err;
+
+	req = otx2_mbox_alloc_msg_npa_cn20k_dpc_alloc(&nic->mbox);
+	if (!req)
+		return -ENOMEM;
+
+	/* Count successful ALLOC requests only */
+	req->dpc_conf = 1ULL << 4;
+
+	err = otx2_sync_mbox_msg(&nic->mbox);
+	if (err)
+		return err;
+
+	rsp = (struct npa_cn20k_dpc_alloc_rsp *)otx2_mbox_get_rsp(&nic->mbox.mbox,
+								  0, &req->hdr);
+	if (IS_ERR(rsp))
+		return PTR_ERR(rsp);
+
+	nic->npa_dpc = rsp->cntr_id;
+
+	return 0;
+}

[ ... ]

quoted

diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c

@@ -1651,6 +1651,9 @@ int otx2_init_hw_resources(struct otx2_nic *pf)
 	if (!is_otx2_lbkvf(pf->pdev))
 		otx2_nix_config_bp(pf, true);

+	if (is_cn20k(pf->pdev))
+		cn20k_npa_alloc_dpc(pf);

                ^^^^^^^^^^^^^^^^^^^^^

Should the return value of cn20k_npa_alloc_dpc() be checked here?

DPC counters are for debugging only we can proceed if counter was not
allocated.

Looking at cn20k_npa_alloc_dpc(), it can return -ENOMEM or errors from
otx2_sync_mbox_msg() or PTR_ERR() from otx2_mbox_get_rsp(). If the DPC
counter allocation fails, pf->npa_dpc remains 0 (from struct
zero-initialization).

Then cn20k_halo_aq_init() configures all NPA auras with
aq->halo.op_dpc_set = pfvf->npa_dpc (= 0), referencing an unallocated
DPC performance counter. On teardown, cn20k_npa_free_dpc() sends a mbox
message to free counter ID 0, which was never allocated.

Agreed. Will add npa_dpc_valid flag and based on that only will use
pfvf->npa_dpc to fix this.

Thanks,
Sundeep

Every other error-returning mbox operation in this function checks the
return value and jumps to an error label on failure. Should this follow
the same pattern?

quoted

+
 	/* Init Auras and pools used by NIX RQ, for free buffer ptrs */
 	err = otx2_rq_aura_pool_init(pf);
 	if (err) {

[ ... ]