diff mbox series

[net,v4,09/10] octeontx2-af: Skip PFs if not enabled

Message ID 20230426074345.750135-10-saikrishnag@marvell.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series octeontx2: Miscellaneous fixes | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net
netdev/fixes_present success Fixes tag present in non-next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 18 this patch: 18
netdev/cc_maintainers fail 2 blamed authors not CCed: mkallio@marvell.com tduszynski@marvell.com; 2 maintainers not CCed: mkallio@marvell.com tduszynski@marvell.com
netdev/build_clang success Errors and warnings before: 21 this patch: 21
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 39 this patch: 39
netdev/checkpatch warning WARNING: line length of 81 exceeds 80 columns WARNING: line length of 87 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Sai Krishna Gajula April 26, 2023, 7:43 a.m. UTC
From: Ratheesh Kannoth <rkannoth@marvell.com>

Fiwmware enables set of PFs and allocate mbox resources based on the
number of enabled PFs. Current driver tries to initialize the mbox
resources even for disabled PFs, which may waste the resources.
This patch fixes the issue by skipping the mbox initialization of
disabled PFs.

Fixes: 9bdc47a6e328 ("octeontx2-af: Mbox communication support btw AF and it's VFs")
Signed-off-by: Ratheesh Kannoth <rkannoth@marvell.com>
Signed-off-by: Sunil Kovvuri Goutham <sgoutham@marvell.com>
Signed-off-by: Sai Krishna <saikrishnag@marvell.com>
---
 .../net/ethernet/marvell/octeontx2/af/mbox.c  |  5 ++-
 .../net/ethernet/marvell/octeontx2/af/mbox.h  |  3 +-
 .../net/ethernet/marvell/octeontx2/af/rvu.c   | 44 ++++++++++++++++---
 3 files changed, 44 insertions(+), 8 deletions(-)

Comments

Simon Horman April 26, 2023, 10 a.m. UTC | #1
On Wed, Apr 26, 2023 at 01:13:44PM +0530, Sai Krishna wrote:
> From: Ratheesh Kannoth <rkannoth@marvell.com>
> 
> Fiwmware enables set of PFs and allocate mbox resources based on the
> number of enabled PFs. Current driver tries to initialize the mbox
> resources even for disabled PFs, which may waste the resources.
> This patch fixes the issue by skipping the mbox initialization of
> disabled PFs.

FWIIW, this feels more like an enhancement than a fix to me.

> Fixes: 9bdc47a6e328 ("octeontx2-af: Mbox communication support btw AF and it's VFs")
> Signed-off-by: Ratheesh Kannoth <rkannoth@marvell.com>
> Signed-off-by: Sunil Kovvuri Goutham <sgoutham@marvell.com>
> Signed-off-by: Sai Krishna <saikrishnag@marvell.com>
> ---
>  .../net/ethernet/marvell/octeontx2/af/mbox.c  |  5 ++-
>  .../net/ethernet/marvell/octeontx2/af/mbox.h  |  3 +-
>  .../net/ethernet/marvell/octeontx2/af/rvu.c   | 44 ++++++++++++++++---
>  3 files changed, 44 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/mbox.c b/drivers/net/ethernet/marvell/octeontx2/af/mbox.c
> index 2898931d5260..9690ac01f02c 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/mbox.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/mbox.c
> @@ -157,7 +157,7 @@ EXPORT_SYMBOL(otx2_mbox_init);
>   */
>  int otx2_mbox_regions_init(struct otx2_mbox *mbox, void **hwbase,
>  			   struct pci_dev *pdev, void *reg_base,
> -			   int direction, int ndevs)
> +			   int direction, int ndevs, unsigned long *pf_bmap)
>  {
>  	struct otx2_mbox_dev *mdev;
>  	int devid, err;
> @@ -169,6 +169,9 @@ int otx2_mbox_regions_init(struct otx2_mbox *mbox, void **hwbase,
>  	mbox->hwbase = hwbase[0];
>  
>  	for (devid = 0; devid < ndevs; devid++) {
> +		if (!test_bit(devid, pf_bmap))
> +			continue;
> +
>  		mdev = &mbox->dev[devid];
>  		mdev->mbase = hwbase[devid];
>  		mdev->hwbase = hwbase[devid];
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> index 0ce533848536..26636a4d7dcc 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> @@ -96,9 +96,10 @@ void otx2_mbox_destroy(struct otx2_mbox *mbox);
>  int otx2_mbox_init(struct otx2_mbox *mbox, void __force *hwbase,
>  		   struct pci_dev *pdev, void __force *reg_base,
>  		   int direction, int ndevs);
> +
>  int otx2_mbox_regions_init(struct otx2_mbox *mbox, void __force **hwbase,
>  			   struct pci_dev *pdev, void __force *reg_base,
> -			   int direction, int ndevs);
> +			   int direction, int ndevs, unsigned long *bmap);
>  void otx2_mbox_msg_send(struct otx2_mbox *mbox, int devid);
>  int otx2_mbox_wait_for_rsp(struct otx2_mbox *mbox, int devid);
>  int otx2_mbox_busy_poll_for_rsp(struct otx2_mbox *mbox, int devid);
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
> index 8683ce57ed3f..242089b6f199 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
> @@ -2282,7 +2282,7 @@ static inline void rvu_afvf_mbox_up_handler(struct work_struct *work)
>  }
>  
>  static int rvu_get_mbox_regions(struct rvu *rvu, void **mbox_addr,
> -				int num, int type)
> +				int num, int type, unsigned long *pf_bmap)
>  {
>  	struct rvu_hwinfo *hw = rvu->hw;
>  	int region;
> @@ -2294,6 +2294,9 @@ static int rvu_get_mbox_regions(struct rvu *rvu, void **mbox_addr,
>  	 */
>  	if (type == TYPE_AFVF) {
>  		for (region = 0; region < num; region++) {
> +			if (!test_bit(region, pf_bmap))
> +				continue;
> +
>  			if (hw->cap.per_pf_mbox_regs) {
>  				bar4 = rvu_read64(rvu, BLKADDR_RVUM,
>  						  RVU_AF_PFX_BAR4_ADDR(0)) +
> @@ -2315,6 +2318,9 @@ static int rvu_get_mbox_regions(struct rvu *rvu, void **mbox_addr,
>  	 * RVU_AF_PF_BAR4_ADDR register.
>  	 */
>  	for (region = 0; region < num; region++) {
> +		if (!test_bit(region, pf_bmap))
> +			continue;
> +
>  		if (hw->cap.per_pf_mbox_regs) {
>  			bar4 = rvu_read64(rvu, BLKADDR_RVUM,
>  					  RVU_AF_PFX_BAR4_ADDR(region));
> @@ -2343,12 +2349,33 @@ 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 = bitmap_zalloc(num, GFP_KERNEL);
> +	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)
> +	if (!mbox_regions) {
> +		bitmap_free(pf_bmap);
>  		return -ENOMEM;

Maybe it is more idiomatic to use

	goto free_bitmap;

> +	}
>  
>  	switch (type) {
>  	case TYPE_AFPF:
> @@ -2356,7 +2383,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 +2392,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 +2423,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 +2445,7 @@ static int rvu_mbox_init(struct rvu *rvu, struct mbox_wq_info *mw,
>  		INIT_WORK(&mwork->work, mbox_up_handler);
>  	}
>  	kfree(mbox_regions);
> +	bitmap_free(pf_bmap);
>  	return 0;

Also, this could avoid duplication using:

	goto free_regions;

>  exit:
> @@ -2424,6 +2455,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);

free_bitmap:

> +	bitmap_free(pf_bmap);
>  	return err;
>  }
>  
> -- 
> 2.25.1
>
Ratheesh Kannoth May 2, 2023, 3:10 a.m. UTC | #2
Hi Simon,

Thanks for your review. Please find replies inline. 

-Ratheesh

> -----Original Message-----
> From: Simon Horman <simon.horman@corigine.com>
> Sent: Wednesday, April 26, 2023 3:31 PM
> To: Sai Krishna Gajula <saikrishnag@marvell.com>
> Cc: davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> pabeni@redhat.com; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; leon@kernel.org; Sunil Kovvuri Goutham
> <sgoutham@marvell.com>; Geethasowjanya Akula <gakula@marvell.com>;
> Linu Cherian <lcherian@marvell.com>; Jerin Jacob Kollanukkaran
> <jerinj@marvell.com>; Hariprasad Kelam <hkelam@marvell.com>;
> Subbaraya Sundeep Bhatta <sbhatta@marvell.com>; Ratheesh Kannoth
> <rkannoth@marvell.com>
> Subject: [EXT] Re: [net PATCH v4 09/10] octeontx2-af: Skip PFs if not enabled
> 
> External Email
> 
> ----------------------------------------------------------------------
> On Wed, Apr 26, 2023 at 01:13:44PM +0530, Sai Krishna wrote:
> > From: Ratheesh Kannoth <rkannoth@marvell.com>
> >
> > Fiwmware enables set of PFs and allocate mbox resources based on the
> > number of enabled PFs. Current driver tries to initialize the mbox
> > resources even for disabled PFs, which may waste the resources.
> > This patch fixes the issue by skipping the mbox initialization of
> > disabled PFs.
> 
> FWIIW, this feels more like an enhancement than a fix to me.
[Ratheesh Kannoth] I agree, commit message convey this change as enhancement. But this 
Code change fixes a crash in driver.  We will modify the commit message as below in next 
Patchset version. 
" Firmware enables PFs and allocate mbox resources for each of the PFs.
Currently PF driver configures mbox resources without checking whether
PF is enabled or not. This results in crash. This patch fixes this issue
by skipping disabled PF's mbox initialization. "
> 
> > Fixes: 9bdc47a6e328 ("octeontx2-af: Mbox communication support btw AF
> > and it's VFs")
> > Signed-off-by: Ratheesh Kannoth <rkannoth@marvell.com>
> > Signed-off-by: Sunil Kovvuri Goutham <sgoutham@marvell.com>
> > Signed-off-by: Sai Krishna <saikrishnag@marvell.com>
> > ---
> >  .../net/ethernet/marvell/octeontx2/af/mbox.c  |  5 ++-
> > .../net/ethernet/marvell/octeontx2/af/mbox.h  |  3 +-
> >  .../net/ethernet/marvell/octeontx2/af/rvu.c   | 44 ++++++++++++++++---
> >  3 files changed, 44 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/marvell/octeontx2/af/mbox.c
> > b/drivers/net/ethernet/marvell/octeontx2/af/mbox.c
> > index 2898931d5260..9690ac01f02c 100644
> > --- a/drivers/net/ethernet/marvell/octeontx2/af/mbox.c
> > +++ b/drivers/net/ethernet/marvell/octeontx2/af/mbox.c
> > @@ -157,7 +157,7 @@ EXPORT_SYMBOL(otx2_mbox_init);
> >   */
> >  int otx2_mbox_regions_init(struct otx2_mbox *mbox, void **hwbase,
> >  			   struct pci_dev *pdev, void *reg_base,
> > -			   int direction, int ndevs)
> > +			   int direction, int ndevs, unsigned long *pf_bmap)
> >  {
> >  	struct otx2_mbox_dev *mdev;
> >  	int devid, err;
> > @@ -169,6 +169,9 @@ int otx2_mbox_regions_init(struct otx2_mbox
> *mbox, void **hwbase,
> >  	mbox->hwbase = hwbase[0];
> >
> >  	for (devid = 0; devid < ndevs; devid++) {
> > +		if (!test_bit(devid, pf_bmap))
> > +			continue;
> > +
> >  		mdev = &mbox->dev[devid];
> >  		mdev->mbase = hwbase[devid];
> >  		mdev->hwbase = hwbase[devid];
> > diff --git a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> > b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> > index 0ce533848536..26636a4d7dcc 100644
> > --- a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> > +++ b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> > @@ -96,9 +96,10 @@ void otx2_mbox_destroy(struct otx2_mbox *mbox);
> > int otx2_mbox_init(struct otx2_mbox *mbox, void __force *hwbase,
> >  		   struct pci_dev *pdev, void __force *reg_base,
> >  		   int direction, int ndevs);
> > +
> >  int otx2_mbox_regions_init(struct otx2_mbox *mbox, void __force
> **hwbase,
> >  			   struct pci_dev *pdev, void __force *reg_base,
> > -			   int direction, int ndevs);
> > +			   int direction, int ndevs, unsigned long *bmap);
> >  void otx2_mbox_msg_send(struct otx2_mbox *mbox, int devid);  int
> > otx2_mbox_wait_for_rsp(struct otx2_mbox *mbox, int devid);  int
> > otx2_mbox_busy_poll_for_rsp(struct otx2_mbox *mbox, int devid); diff
> > --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
> > b/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
> > index 8683ce57ed3f..242089b6f199 100644
> > --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
> > +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
> > @@ -2282,7 +2282,7 @@ static inline void
> > rvu_afvf_mbox_up_handler(struct work_struct *work)  }
> >
> >  static int rvu_get_mbox_regions(struct rvu *rvu, void **mbox_addr,
> > -				int num, int type)
> > +				int num, int type, unsigned long *pf_bmap)
> >  {
> >  	struct rvu_hwinfo *hw = rvu->hw;
> >  	int region;
> > @@ -2294,6 +2294,9 @@ static int rvu_get_mbox_regions(struct rvu *rvu,
> void **mbox_addr,
> >  	 */
> >  	if (type == TYPE_AFVF) {
> >  		for (region = 0; region < num; region++) {
> > +			if (!test_bit(region, pf_bmap))
> > +				continue;
> > +
> >  			if (hw->cap.per_pf_mbox_regs) {
> >  				bar4 = rvu_read64(rvu, BLKADDR_RVUM,
> >
> RVU_AF_PFX_BAR4_ADDR(0)) +
> > @@ -2315,6 +2318,9 @@ static int rvu_get_mbox_regions(struct rvu *rvu,
> void **mbox_addr,
> >  	 * RVU_AF_PF_BAR4_ADDR register.
> >  	 */
> >  	for (region = 0; region < num; region++) {
> > +		if (!test_bit(region, pf_bmap))
> > +			continue;
> > +
> >  		if (hw->cap.per_pf_mbox_regs) {
> >  			bar4 = rvu_read64(rvu, BLKADDR_RVUM,
> >  					  RVU_AF_PFX_BAR4_ADDR(region));
> @@ -2343,12 +2349,33 @@ 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 = bitmap_zalloc(num, GFP_KERNEL);
> > +	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)
> > +	if (!mbox_regions) {
> > +		bitmap_free(pf_bmap);
> >  		return -ENOMEM;
> 
> Maybe it is more idiomatic to use
> 
> 	goto free_bitmap;
> 
[Ratheesh Kannoth] Agree. Will update the code. 
> > +	}
> >
> >  	switch (type) {
> >  	case TYPE_AFPF:
> > @@ -2356,7 +2383,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 +2392,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 +2423,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
> +2445,7 @@ static
> > int rvu_mbox_init(struct rvu *rvu, struct mbox_wq_info *mw,
> >  		INIT_WORK(&mwork->work, mbox_up_handler);
> >  	}
> >  	kfree(mbox_regions);
> > +	bitmap_free(pf_bmap);
> >  	return 0;
> 
> Also, this could avoid duplication using:
> 
> 	goto free_regions;
> 
> >  exit:
> > @@ -2424,6 +2455,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);
> 
> free_bitmap:
> 
> > +	bitmap_free(pf_bmap);
> >  	return err;
> >  }
> >
> > --
> > 2.25.1
> >
Simon Horman May 2, 2023, 11:55 a.m. UTC | #3
On Tue, May 02, 2023 at 03:10:53AM +0000, Ratheesh Kannoth wrote:
> Hi Simon,
> 
> Thanks for your review. Please find replies inline. 
> 
> -Ratheesh
> 
> > -----Original Message-----
> > From: Simon Horman <simon.horman@corigine.com>
> > Sent: Wednesday, April 26, 2023 3:31 PM
> > To: Sai Krishna Gajula <saikrishnag@marvell.com>
> > Cc: davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> > pabeni@redhat.com; netdev@vger.kernel.org; linux-
> > kernel@vger.kernel.org; leon@kernel.org; Sunil Kovvuri Goutham
> > <sgoutham@marvell.com>; Geethasowjanya Akula <gakula@marvell.com>;
> > Linu Cherian <lcherian@marvell.com>; Jerin Jacob Kollanukkaran
> > <jerinj@marvell.com>; Hariprasad Kelam <hkelam@marvell.com>;
> > Subbaraya Sundeep Bhatta <sbhatta@marvell.com>; Ratheesh Kannoth
> > <rkannoth@marvell.com>
> > Subject: [EXT] Re: [net PATCH v4 09/10] octeontx2-af: Skip PFs if not enabled
> > 
> > External Email
> > 
> > ----------------------------------------------------------------------
> > On Wed, Apr 26, 2023 at 01:13:44PM +0530, Sai Krishna wrote:
> > > From: Ratheesh Kannoth <rkannoth@marvell.com>
> > >
> > > Fiwmware enables set of PFs and allocate mbox resources based on the
> > > number of enabled PFs. Current driver tries to initialize the mbox
> > > resources even for disabled PFs, which may waste the resources.
> > > This patch fixes the issue by skipping the mbox initialization of
> > > disabled PFs.
> > 
> > FWIIW, this feels more like an enhancement than a fix to me.
>
> [Ratheesh Kannoth] I agree, commit message convey this change as
> enhancement. But this Code change fixes a crash in driver.  We will
> modify the commit message as below in next Patchset version.  " Firmware
> enables PFs and allocate mbox resources for each of the PFs.  Currently
> PF driver configures mbox resources without checking whether PF is
> enabled or not. This results in crash. This patch fixes this issue by
> skipping disabled PF's mbox initialization. "

Thanks, much appreciated.
diff mbox series

Patch

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/mbox.c b/drivers/net/ethernet/marvell/octeontx2/af/mbox.c
index 2898931d5260..9690ac01f02c 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/mbox.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/mbox.c
@@ -157,7 +157,7 @@  EXPORT_SYMBOL(otx2_mbox_init);
  */
 int otx2_mbox_regions_init(struct otx2_mbox *mbox, void **hwbase,
 			   struct pci_dev *pdev, void *reg_base,
-			   int direction, int ndevs)
+			   int direction, int ndevs, unsigned long *pf_bmap)
 {
 	struct otx2_mbox_dev *mdev;
 	int devid, err;
@@ -169,6 +169,9 @@  int otx2_mbox_regions_init(struct otx2_mbox *mbox, void **hwbase,
 	mbox->hwbase = hwbase[0];
 
 	for (devid = 0; devid < ndevs; devid++) {
+		if (!test_bit(devid, pf_bmap))
+			continue;
+
 		mdev = &mbox->dev[devid];
 		mdev->mbase = hwbase[devid];
 		mdev->hwbase = hwbase[devid];
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
index 0ce533848536..26636a4d7dcc 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
+++ b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
@@ -96,9 +96,10 @@  void otx2_mbox_destroy(struct otx2_mbox *mbox);
 int otx2_mbox_init(struct otx2_mbox *mbox, void __force *hwbase,
 		   struct pci_dev *pdev, void __force *reg_base,
 		   int direction, int ndevs);
+
 int otx2_mbox_regions_init(struct otx2_mbox *mbox, void __force **hwbase,
 			   struct pci_dev *pdev, void __force *reg_base,
-			   int direction, int ndevs);
+			   int direction, int ndevs, unsigned long *bmap);
 void otx2_mbox_msg_send(struct otx2_mbox *mbox, int devid);
 int otx2_mbox_wait_for_rsp(struct otx2_mbox *mbox, int devid);
 int otx2_mbox_busy_poll_for_rsp(struct otx2_mbox *mbox, int devid);
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
index 8683ce57ed3f..242089b6f199 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
@@ -2282,7 +2282,7 @@  static inline void rvu_afvf_mbox_up_handler(struct work_struct *work)
 }
 
 static int rvu_get_mbox_regions(struct rvu *rvu, void **mbox_addr,
-				int num, int type)
+				int num, int type, unsigned long *pf_bmap)
 {
 	struct rvu_hwinfo *hw = rvu->hw;
 	int region;
@@ -2294,6 +2294,9 @@  static int rvu_get_mbox_regions(struct rvu *rvu, void **mbox_addr,
 	 */
 	if (type == TYPE_AFVF) {
 		for (region = 0; region < num; region++) {
+			if (!test_bit(region, pf_bmap))
+				continue;
+
 			if (hw->cap.per_pf_mbox_regs) {
 				bar4 = rvu_read64(rvu, BLKADDR_RVUM,
 						  RVU_AF_PFX_BAR4_ADDR(0)) +
@@ -2315,6 +2318,9 @@  static int rvu_get_mbox_regions(struct rvu *rvu, void **mbox_addr,
 	 * RVU_AF_PF_BAR4_ADDR register.
 	 */
 	for (region = 0; region < num; region++) {
+		if (!test_bit(region, pf_bmap))
+			continue;
+
 		if (hw->cap.per_pf_mbox_regs) {
 			bar4 = rvu_read64(rvu, BLKADDR_RVUM,
 					  RVU_AF_PFX_BAR4_ADDR(region));
@@ -2343,12 +2349,33 @@  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 = bitmap_zalloc(num, GFP_KERNEL);
+	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)
+	if (!mbox_regions) {
+		bitmap_free(pf_bmap);
 		return -ENOMEM;
+	}
 
 	switch (type) {
 	case TYPE_AFPF:
@@ -2356,7 +2383,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 +2392,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 +2423,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 +2445,7 @@  static int rvu_mbox_init(struct rvu *rvu, struct mbox_wq_info *mw,
 		INIT_WORK(&mwork->work, mbox_up_handler);
 	}
 	kfree(mbox_regions);
+	bitmap_free(pf_bmap);
 	return 0;
 
 exit:
@@ -2424,6 +2455,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);
+	bitmap_free(pf_bmap);
 	return err;
 }