Re: [net PATCH v3 09/10] octeontx2-af: Skip PFs if not enabled

[Sai Krishna Gajula]

> -----Original Message-----
> From: Simon Horman <simon.horman@xxxxxxxxxxxx>
> Sent: Wednesday, April 19, 2023 4:21 PM
> To: Sai Krishna Gajula <saikrishnag@xxxxxxxxxxx>
> Cc: davem@xxxxxxxxxxxxx; edumazet@xxxxxxxxxx; kuba@xxxxxxxxxx;
> pabeni@xxxxxxxxxx; netdev@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; leon@xxxxxxxxxx; Sunil Kovvuri Goutham
> <sgoutham@xxxxxxxxxxx>; Geethasowjanya Akula <gakula@xxxxxxxxxxx>;
> Linu Cherian <lcherian@xxxxxxxxxxx>; Jerin Jacob Kollanukkaran
> <jerinj@xxxxxxxxxxx>; Hariprasad Kelam <hkelam@xxxxxxxxxxx>;
> Subbaraya Sundeep Bhatta <sbhatta@xxxxxxxxxxx>; Ratheesh Kannoth
> <rkannoth@xxxxxxxxxxx>
> Subject: Re: [net PATCH v3 09/10] octeontx2-af: Skip PFs if not enabled
> 
> On Wed, Apr 19, 2023 at 11:50:17AM +0530, Sai Krishna wrote:
> > From: Ratheesh Kannoth <rkannoth@xxxxxxxxxxx>
> >
> > Skip mbox initialization of disabled PFs. Firmware configures PFs and
> > allocate mbox resources etc. Linux should configure particular PFs,
> > which ever are enabled by firmware.
> >
> > Fixes: 9bdc47a6e328 ("octeontx2-af: Mbox communication support btw AF
> > and it's VFs")
> > Signed-off-by: Ratheesh Kannoth <rkannoth@xxxxxxxxxxx>
> > Signed-off-by: Sunil Kovvuri Goutham <sgoutham@xxxxxxxxxxx>
> > Signed-off-by: Sai Krishna <saikrishnag@xxxxxxxxxxx>
> 
> ...
> 
> > @@ -2343,8 +2349,27 @@ static int rvu_mbox_init(struct rvu *rvu, struct
> mbox_wq_info *mw,
> >  	int err = -EINVAL, i, dir, dir_up;
> >  	void __iomem *reg_base;
> >  	struct rvu_work *mwork;
> > +	unsigned long *pf_bmap;
> >  	void **mbox_regions;
> >  	const char *name;
> > +	u64 cfg;
> > +
> > +	pf_bmap = kcalloc(BITS_TO_LONGS(num), sizeof(long),
> GFP_KERNEL);
> 
> Sorry for not noticing this earlier, but maybe bitmap_alloc() is appropriate
> here.
> 
We will provide v4 patch with using bitmap_alloc()/bitmap_zalloc() .

> > +	if (!pf_bmap)
> > +		return -ENOMEM;
> > +
> > +	/* RVU VFs */
> > +	if (type == TYPE_AFVF)
> > +		bitmap_set(pf_bmap, 0, num);
> > +
> > +	if (type == TYPE_AFPF) {
> > +		/* Mark enabled PFs in bitmap */
> > +		for (i = 0; i < num; i++) {
> > +			cfg = rvu_read64(rvu, BLKADDR_RVUM,
> RVU_PRIV_PFX_CFG(i));
> > +			if (cfg & BIT_ULL(20))
> > +				set_bit(i, pf_bmap);
> > +		}
> > +	}
> >
> >  	mbox_regions = kcalloc(num, sizeof(void *), GFP_KERNEL);
> >  	if (!mbox_regions)
> 
> 		I think pf_bmap is leaked here.

We will provide v4 patch with fixing pf_bmap leak.

> 
> > @@ -2356,7 +2381,7 @@ static int rvu_mbox_init(struct rvu *rvu, struct
> mbox_wq_info *mw,
> >  		dir = MBOX_DIR_AFPF;
> >  		dir_up = MBOX_DIR_AFPF_UP;
> >  		reg_base = rvu->afreg_base;
> > -		err = rvu_get_mbox_regions(rvu, mbox_regions, num,
> TYPE_AFPF);
> > +		err = rvu_get_mbox_regions(rvu, mbox_regions, num,
> TYPE_AFPF,
> > +pf_bmap);
> >  		if (err)
> >  			goto free_regions;
> >  		break;
> > @@ -2365,7 +2390,7 @@ static int rvu_mbox_init(struct rvu *rvu, struct
> mbox_wq_info *mw,
> >  		dir = MBOX_DIR_PFVF;
> >  		dir_up = MBOX_DIR_PFVF_UP;
> >  		reg_base = rvu->pfreg_base;
> > -		err = rvu_get_mbox_regions(rvu, mbox_regions, num,
> TYPE_AFVF);
> > +		err = rvu_get_mbox_regions(rvu, mbox_regions, num,
> TYPE_AFVF,
> > +pf_bmap);
> >  		if (err)
> >  			goto free_regions;
> >  		break;
> > @@ -2396,16 +2421,19 @@ static int rvu_mbox_init(struct rvu *rvu, struct
> mbox_wq_info *mw,
> >  	}
> >
> >  	err = otx2_mbox_regions_init(&mw->mbox, mbox_regions, rvu-
> >pdev,
> > -				     reg_base, dir, num);
> > +				     reg_base, dir, num, pf_bmap);
> >  	if (err)
> >  		goto exit;
> >
> >  	err = otx2_mbox_regions_init(&mw->mbox_up, mbox_regions, rvu-
> >pdev,
> > -				     reg_base, dir_up, num);
> > +				     reg_base, dir_up, num, pf_bmap);
> >  	if (err)
> >  		goto exit;
> >
> >  	for (i = 0; i < num; i++) {
> > +		if (!test_bit(i, pf_bmap))
> > +			continue;
> > +
> >  		mwork = &mw->mbox_wrk[i];
> >  		mwork->rvu = rvu;
> >  		INIT_WORK(&mwork->work, mbox_handler); @@ -2415,6
> +2443,7 @@ static
> > int rvu_mbox_init(struct rvu *rvu, struct mbox_wq_info *mw,
> >  		INIT_WORK(&mwork->work, mbox_up_handler);
> >  	}
> >  	kfree(mbox_regions);
> > +	kfree(pf_bmap);
> >  	return 0;
> >
> >  exit:
> > @@ -2424,6 +2453,7 @@ static int rvu_mbox_init(struct rvu *rvu, struct
> mbox_wq_info *mw,
> >  		iounmap((void __iomem *)mbox_regions[num]);
> >  free_regions:
> >  	kfree(mbox_regions);
> > +	kfree(pf_bmap);
> >  	return err;
> >  }
> >
> > --
> > 2.25.1
> >