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

[Subbaraya Sundeep <sbhatta@marvell.com>]

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.
> 
> > 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
> 
> [ ... ]
> 
> > +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;
> > +}
> 
> [ ... ]
> 
> > 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?
> 
> > +
> >  	/* Init Auras and pools used by NIX RQ, for free buffer ptrs */
> >  	err = otx2_rq_aura_pool_init(pf);
> >  	if (err) {
> 
> [ ... ]