| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <ZEj2UYNavsn0xE/D@corigine.com>
Date: Wed, 26 Apr 2023 12:00:49 +0200
From: Simon Horman <simon.horman@...igine.com>
To: Sai Krishna <saikrishnag@...vell.com>
Cc: davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
pabeni@...hat.com, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org, leon@...nel.org,
sgoutham@...vell.com, gakula@...vell.com, lcherian@...vell.com,
jerinj@...vell.com, hkelam@...vell.com, sbhatta@...vell.com,
Ratheesh Kannoth <rkannoth@...vell.com>
Subject: Re: [net PATCH v4 09/10] octeontx2-af: Skip PFs if not enabled
On Wed, Apr 26, 2023 at 01:13:44PM +0530, Sai Krishna wrote:
> From: Ratheesh Kannoth <rkannoth@...vell.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@...vell.com>
> Signed-off-by: Sunil Kovvuri Goutham <sgoutham@...vell.com>
> Signed-off-by: Sai Krishna <saikrishnag@...vell.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
>
Powered by blists - more mailing lists