[net] octeontx2: Fix klockwork and coverity issues

Message ID 20231101074919.2614608-1-sumang@marvell.com
State New
Headers
Series [net] octeontx2: Fix klockwork and coverity issues |

Commit Message

Suman Ghosh Nov. 1, 2023, 7:49 a.m. UTC
  Fix all klockwork and coverity issues reported on AF and PF/VF driver.

Signed-off-by: Suman Ghosh <sumang@marvell.com>
Signed-off-by: Ratheesh Kannoth <rkannoth@marvell.com>
---
 .../net/ethernet/marvell/octeontx2/af/cgx.c   | 14 ++++-
 .../marvell/octeontx2/af/mcs_rvu_if.c         |  8 ++-
 .../net/ethernet/marvell/octeontx2/af/ptp.c   | 11 +++-
 .../ethernet/marvell/octeontx2/af/rvu_cpt.c   |  2 +-
 .../marvell/octeontx2/af/rvu_debugfs.c        |  8 ++-
 .../ethernet/marvell/octeontx2/af/rvu_nix.c   |  2 +-
 .../ethernet/marvell/octeontx2/af/rvu_npc.c   |  2 +-
 .../marvell/octeontx2/nic/otx2_common.c       |  8 +--
 .../ethernet/marvell/octeontx2/nic/otx2_pf.c  |  3 +
 .../ethernet/marvell/octeontx2/nic/otx2_reg.h | 55 ++++++++++---------
 .../marvell/octeontx2/nic/otx2_txrx.c         |  2 +-
 .../net/ethernet/marvell/octeontx2/nic/qos.c  |  7 ++-
 12 files changed, 77 insertions(+), 45 deletions(-)
  

Comments

Andrew Lunn Nov. 1, 2023, 12:42 p.m. UTC | #1
On Wed, Nov 01, 2023 at 01:19:19PM +0530, Suman Ghosh wrote:
> Fix all klockwork and coverity issues reported on AF and PF/VF driver.
> 
> Signed-off-by: Suman Ghosh <sumang@marvell.com>
> Signed-off-by: Ratheesh Kannoth <rkannoth@marvell.com>

The subject line is:
[net PATCH] octeontx2: Fix klockwork and coverity issues

So you want these fixes backported to net? If so, you need to provide
Fixes: tags.

This patch is way too big. A fix patch generally fixes one thing, and
it documents what it fixes. Or it could be one class of problems, like
uninitialised variables etc. Its good to include the message from the
static analyser in the commit message.

    Andrew

---
pw-bot: cr
  
Suman Ghosh Nov. 3, 2023, 4:39 p.m. UTC | #2
>> Fix all klockwork and coverity issues reported on AF and PF/VF driver.
>>
>> Signed-off-by: Suman Ghosh <sumang@marvell.com>
>> Signed-off-by: Ratheesh Kannoth <rkannoth@marvell.com>
>
>The subject line is:
>[net PATCH] octeontx2: Fix klockwork and coverity issues
>
>So you want these fixes backported to net? If so, you need to provide
>Fixes: tags.
>
>This patch is way too big. A fix patch generally fixes one thing, and it
>documents what it fixes. Or it could be one class of problems, like
>uninitialised variables etc. Its good to include the message from the
>static analyser in the commit message.
>
>    Andrew
[Suman] Yes, backporting to net was the intention. Do we need to put all the fixes tags? Because the fixes are from different commits.
Are there any generic submission steps for klockwork fixes?
>
>---
>pw-bot: cr
  
Andrew Lunn Nov. 3, 2023, 5:06 p.m. UTC | #3
> [Suman] Yes, backporting to net was the intention. Do we need to put all the fixes tags? Because the fixes are from different commits.

When you break it up into multiple patches, the fixes-tag will get
simpler.

> Are there any generic submission steps for klockwork fixes?

Take a look at

https://docs.kernel.org/process/stable-kernel-rules.html

In particular, the "It must fix a real bug that bothers people".

Some issues found by static code tools don't actually bother
people. So you might want some patches to net-next, not net.

     Andrew
  
Suman Ghosh Nov. 14, 2023, 5:23 a.m. UTC | #4
>
>In particular, the "It must fix a real bug that bothers people".
>
>Some issues found by static code tools don't actually bother people. So
>you might want some patches to net-next, not net.
>
>     Andrew
[Suman] Yes Andrew. I will push it to net-next
  
Christophe JAILLET Nov. 14, 2023, 4:42 p.m. UTC | #5
Le 01/11/2023 à 08:49, Suman Ghosh a écrit :
> Fix all klockwork and coverity issues reported on AF and PF/VF driver.
> 
> Signed-off-by: Suman Ghosh <sumang@marvell.com>
> Signed-off-by: Ratheesh Kannoth <rkannoth@marvell.com>
> ---
>   .../net/ethernet/marvell/octeontx2/af/cgx.c   | 14 ++++-
>   .../marvell/octeontx2/af/mcs_rvu_if.c         |  8 ++-
>   .../net/ethernet/marvell/octeontx2/af/ptp.c   | 11 +++-
>   .../ethernet/marvell/octeontx2/af/rvu_cpt.c   |  2 +-
>   .../marvell/octeontx2/af/rvu_debugfs.c        |  8 ++-
>   .../ethernet/marvell/octeontx2/af/rvu_nix.c   |  2 +-
>   .../ethernet/marvell/octeontx2/af/rvu_npc.c   |  2 +-
>   .../marvell/octeontx2/nic/otx2_common.c       |  8 +--
>   .../ethernet/marvell/octeontx2/nic/otx2_pf.c  |  3 +
>   .../ethernet/marvell/octeontx2/nic/otx2_reg.h | 55 ++++++++++---------
>   .../marvell/octeontx2/nic/otx2_txrx.c         |  2 +-
>   .../net/ethernet/marvell/octeontx2/nic/qos.c  |  7 ++-
>   12 files changed, 77 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cgx.c b/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
> index 6c70c8498690..5a672888577e 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
> @@ -457,12 +457,19 @@ int cgx_lmac_addr_max_entries_get(u8 cgx_id, u8 lmac_id)
>   u64 cgx_lmac_addr_get(u8 cgx_id, u8 lmac_id)
>   {
>   	struct cgx *cgx_dev = cgx_get_pdata(cgx_id);
> -	struct lmac *lmac = lmac_pdata(lmac_id, cgx_dev);
>   	struct mac_ops *mac_ops;
> +	struct lmac *lmac;
>   	int index;
>   	u64 cfg;
>   	int id;
>   
> +	if (!cgx_dev)
> +		return 0;
> +
> +	lmac = lmac_pdata(lmac_id, cgx_dev);
> +	if (!lmac)
> +		return 0;
> +
>   	mac_ops = cgx_dev->mac_ops;
>   
>   	id = get_sequence_id_of_lmac(cgx_dev, lmac_id);
> @@ -955,6 +962,9 @@ int cgx_lmac_pfc_config(void *cgxd, int lmac_id, u8 tx_pause,
>   
>   	/* Write source MAC address which will be filled into PFC packet */
>   	cfg = cgx_lmac_addr_get(cgx->cgx_id, lmac_id);
> +	if (!cfg)
> +		return -ENODEV;
> +
>   	cgx_write(cgx, lmac_id, CGXX_SMUX_SMAC, cfg);
>   
>   	return 0;
> @@ -1617,7 +1627,7 @@ unsigned long cgx_get_lmac_bmap(void *cgxd)
>   static int cgx_lmac_init(struct cgx *cgx)
>   {
>   	struct lmac *lmac;
> -	u64 lmac_list;
> +	u64 lmac_list = 0;

This is not needed.
The static checker is not good enough to see it, that's all :).

>   	int i, err;
>   
>   	/* lmac_list specifies which lmacs are enabled
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/mcs_rvu_if.c b/drivers/net/ethernet/marvell/octeontx2/af/mcs_rvu_if.c
> index dfd23580e3b8..1b0b022f5493 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/mcs_rvu_if.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/mcs_rvu_if.c
> @@ -625,8 +625,8 @@ int rvu_mbox_handler_mcs_free_resources(struct rvu *rvu,
>   {
>   	u16 pcifunc = req->hdr.pcifunc;
>   	struct mcs_rsrc_map *map;
> +	int rc = -EINVAL;
>   	struct mcs *mcs;
> -	int rc = 0;

This should be argumented.

This changes the behavior of the code. Why should we return -EINVAL if 
nothing in the switch below matches?

>   
>   	if (req->mcs_id >= rvu->mcs_blk_cnt)
>   		return MCS_AF_ERR_INVALID_MCSID;
> @@ -675,8 +675,8 @@ int rvu_mbox_handler_mcs_alloc_resources(struct rvu *rvu,
>   {
>   	u16 pcifunc = req->hdr.pcifunc;
>   	struct mcs_rsrc_map *map;
> +	int rsrc_id = -EINVAL, i;
>   	struct mcs *mcs;
> -	int rsrc_id, i;

Same

>   
>   	if (req->mcs_id >= rvu->mcs_blk_cnt)
>   		return MCS_AF_ERR_INVALID_MCSID;
> @@ -737,6 +737,8 @@ int rvu_mbox_handler_mcs_alloc_resources(struct rvu *rvu,
>   			rsp->rsrc_cnt++;
>   		}
>   		break;
> +	default:
> +		goto exit;
>   	}
>   
>   	rsp->rsrc_type = req->rsrc_type;
> @@ -849,7 +851,7 @@ int rvu_mbox_handler_mcs_ctrl_pkt_rule_write(struct rvu *rvu,
>   static void rvu_mcs_set_lmac_bmap(struct rvu *rvu)
>   {
>   	struct mcs *mcs = mcs_get_pdata(0);
> -	unsigned long lmac_bmap;
> +	unsigned long lmac_bmap = 0;
>   	int cgx, lmac, port;
>   
>   	for (port = 0; port < mcs->hw->lmac_cnt; port++) {
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/ptp.c b/drivers/net/ethernet/marvell/octeontx2/af/ptp.c
> index bcc96eed2481..a199b1123ba7 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/ptp.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/ptp.c
> @@ -518,6 +518,7 @@ static int ptp_probe(struct pci_dev *pdev,
>   		     const struct pci_device_id *ent)
>   {
>   	struct ptp *ptp;
> +	void __iomem * const *base;
>   	int err;
>   
>   	ptp = kzalloc(sizeof(*ptp), GFP_KERNEL);
> @@ -536,7 +537,15 @@ static int ptp_probe(struct pci_dev *pdev,
>   	if (err)
>   		goto error_free;
>   
> -	ptp->reg_base = pcim_iomap_table(pdev)[PCI_PTP_BAR_NO];
> +	base = pcim_iomap_table(pdev);
> +	if (!base)
> +		goto error_free;
> +
> +	ptp->reg_base = base[PCI_PTP_BAR_NO];
> +	if (!ptp->reg_base) {
> +		err = -ENODEV;
> +		goto error_free;
> +	}
>   
>   	pci_set_drvdata(pdev, ptp);
>   	if (!first_ptp_block)
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cpt.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cpt.c
> index f047185f38e0..a1a919fcda47 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cpt.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cpt.c
> @@ -43,7 +43,7 @@ static irqreturn_t cpt_af_flt_intr_handler(int vec, void *ptr)
>   	struct rvu *rvu = block->rvu;
>   	int blkaddr = block->addr;
>   	u64 reg, val;
> -	int i, eng;
> +	int i, eng = 0;

Why is this the correct value if nothing else matches in the switch below?

>   	u8 grp;
>   
>   	reg = rvu_read64(rvu, blkaddr, CPT_AF_FLTX_INT(vec));
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_debugfs.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_debugfs.c
> index bd817ee88735..307942ff1b10 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_debugfs.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_debugfs.c
> @@ -519,12 +519,16 @@ RVU_DEBUG_SEQ_FOPS(mcs_rx_secy_stats, mcs_rx_secy_stats_display, NULL);
>   static void rvu_dbg_mcs_init(struct rvu *rvu)
>   {
>   	struct mcs *mcs;
> -	char dname[10];
> +	char *dname = NULL;

=NULL is not needed, but see below.

>   	int i;
>   
>   	if (!rvu->mcs_blk_cnt)
>   		return;
>   
> +	dname = kmalloc_array(rvu->mcs_blk_cnt, sizeof(char), GFP_KERNEL);

Hi,

I think that kmalloc() would be enough here.

More importantly, I think it is wrong.
This dbname buffer is used to store "mcs%d". %d can be up to 
rvu->mcs_blk_cnt, but it does not require that amount of memory.

(if mcs_blk_cnt = 1000, we need 3 + 4 + 1 = 8 byte ("mcs" + "1000" + 
\0), not 1000 bytes.

If mcs_blk_cnt is small, we under-allocate, if it is high we allocate 
way too much memory.

Maybe kasprintf()/kfree() would be a cleaner option.


> +	if (!dname)
> +		return;
> +
>   	rvu->rvu_dbg.mcs_root = debugfs_create_dir("mcs", rvu->rvu_dbg.root);
>   
>   	for (i = 0; i < rvu->mcs_blk_cnt; i++) {
> @@ -568,6 +572,8 @@ static void rvu_dbg_mcs_init(struct rvu *rvu)
>   		debugfs_create_file("port", 0600, rvu->rvu_dbg.mcs_tx, mcs,
>   				    &rvu_dbg_mcs_tx_port_stats_fops);
>   	}
> +
> +	kfree(dname);
>   }
>   

...

(I have not looked the code below that)
  

Patch

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cgx.c b/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
index 6c70c8498690..5a672888577e 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
@@ -457,12 +457,19 @@  int cgx_lmac_addr_max_entries_get(u8 cgx_id, u8 lmac_id)
 u64 cgx_lmac_addr_get(u8 cgx_id, u8 lmac_id)
 {
 	struct cgx *cgx_dev = cgx_get_pdata(cgx_id);
-	struct lmac *lmac = lmac_pdata(lmac_id, cgx_dev);
 	struct mac_ops *mac_ops;
+	struct lmac *lmac;
 	int index;
 	u64 cfg;
 	int id;
 
+	if (!cgx_dev)
+		return 0;
+
+	lmac = lmac_pdata(lmac_id, cgx_dev);
+	if (!lmac)
+		return 0;
+
 	mac_ops = cgx_dev->mac_ops;
 
 	id = get_sequence_id_of_lmac(cgx_dev, lmac_id);
@@ -955,6 +962,9 @@  int cgx_lmac_pfc_config(void *cgxd, int lmac_id, u8 tx_pause,
 
 	/* Write source MAC address which will be filled into PFC packet */
 	cfg = cgx_lmac_addr_get(cgx->cgx_id, lmac_id);
+	if (!cfg)
+		return -ENODEV;
+
 	cgx_write(cgx, lmac_id, CGXX_SMUX_SMAC, cfg);
 
 	return 0;
@@ -1617,7 +1627,7 @@  unsigned long cgx_get_lmac_bmap(void *cgxd)
 static int cgx_lmac_init(struct cgx *cgx)
 {
 	struct lmac *lmac;
-	u64 lmac_list;
+	u64 lmac_list = 0;
 	int i, err;
 
 	/* lmac_list specifies which lmacs are enabled
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/mcs_rvu_if.c b/drivers/net/ethernet/marvell/octeontx2/af/mcs_rvu_if.c
index dfd23580e3b8..1b0b022f5493 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/mcs_rvu_if.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/mcs_rvu_if.c
@@ -625,8 +625,8 @@  int rvu_mbox_handler_mcs_free_resources(struct rvu *rvu,
 {
 	u16 pcifunc = req->hdr.pcifunc;
 	struct mcs_rsrc_map *map;
+	int rc = -EINVAL;
 	struct mcs *mcs;
-	int rc = 0;
 
 	if (req->mcs_id >= rvu->mcs_blk_cnt)
 		return MCS_AF_ERR_INVALID_MCSID;
@@ -675,8 +675,8 @@  int rvu_mbox_handler_mcs_alloc_resources(struct rvu *rvu,
 {
 	u16 pcifunc = req->hdr.pcifunc;
 	struct mcs_rsrc_map *map;
+	int rsrc_id = -EINVAL, i;
 	struct mcs *mcs;
-	int rsrc_id, i;
 
 	if (req->mcs_id >= rvu->mcs_blk_cnt)
 		return MCS_AF_ERR_INVALID_MCSID;
@@ -737,6 +737,8 @@  int rvu_mbox_handler_mcs_alloc_resources(struct rvu *rvu,
 			rsp->rsrc_cnt++;
 		}
 		break;
+	default:
+		goto exit;
 	}
 
 	rsp->rsrc_type = req->rsrc_type;
@@ -849,7 +851,7 @@  int rvu_mbox_handler_mcs_ctrl_pkt_rule_write(struct rvu *rvu,
 static void rvu_mcs_set_lmac_bmap(struct rvu *rvu)
 {
 	struct mcs *mcs = mcs_get_pdata(0);
-	unsigned long lmac_bmap;
+	unsigned long lmac_bmap = 0;
 	int cgx, lmac, port;
 
 	for (port = 0; port < mcs->hw->lmac_cnt; port++) {
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/ptp.c b/drivers/net/ethernet/marvell/octeontx2/af/ptp.c
index bcc96eed2481..a199b1123ba7 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/ptp.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/ptp.c
@@ -518,6 +518,7 @@  static int ptp_probe(struct pci_dev *pdev,
 		     const struct pci_device_id *ent)
 {
 	struct ptp *ptp;
+	void __iomem * const *base;
 	int err;
 
 	ptp = kzalloc(sizeof(*ptp), GFP_KERNEL);
@@ -536,7 +537,15 @@  static int ptp_probe(struct pci_dev *pdev,
 	if (err)
 		goto error_free;
 
-	ptp->reg_base = pcim_iomap_table(pdev)[PCI_PTP_BAR_NO];
+	base = pcim_iomap_table(pdev);
+	if (!base)
+		goto error_free;
+
+	ptp->reg_base = base[PCI_PTP_BAR_NO];
+	if (!ptp->reg_base) {
+		err = -ENODEV;
+		goto error_free;
+	}
 
 	pci_set_drvdata(pdev, ptp);
 	if (!first_ptp_block)
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cpt.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cpt.c
index f047185f38e0..a1a919fcda47 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cpt.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cpt.c
@@ -43,7 +43,7 @@  static irqreturn_t cpt_af_flt_intr_handler(int vec, void *ptr)
 	struct rvu *rvu = block->rvu;
 	int blkaddr = block->addr;
 	u64 reg, val;
-	int i, eng;
+	int i, eng = 0;
 	u8 grp;
 
 	reg = rvu_read64(rvu, blkaddr, CPT_AF_FLTX_INT(vec));
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_debugfs.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_debugfs.c
index bd817ee88735..307942ff1b10 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_debugfs.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_debugfs.c
@@ -519,12 +519,16 @@  RVU_DEBUG_SEQ_FOPS(mcs_rx_secy_stats, mcs_rx_secy_stats_display, NULL);
 static void rvu_dbg_mcs_init(struct rvu *rvu)
 {
 	struct mcs *mcs;
-	char dname[10];
+	char *dname = NULL;
 	int i;
 
 	if (!rvu->mcs_blk_cnt)
 		return;
 
+	dname = kmalloc_array(rvu->mcs_blk_cnt, sizeof(char), GFP_KERNEL);
+	if (!dname)
+		return;
+
 	rvu->rvu_dbg.mcs_root = debugfs_create_dir("mcs", rvu->rvu_dbg.root);
 
 	for (i = 0; i < rvu->mcs_blk_cnt; i++) {
@@ -568,6 +572,8 @@  static void rvu_dbg_mcs_init(struct rvu *rvu)
 		debugfs_create_file("port", 0600, rvu->rvu_dbg.mcs_tx, mcs,
 				    &rvu_dbg_mcs_tx_port_stats_fops);
 	}
+
+	kfree(dname);
 }
 
 #define LMT_MAPTBL_ENTRY_SIZE 16
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
index 23c2f2ed2fb8..2fa2ef970e88 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
@@ -5033,7 +5033,7 @@  static void nix_inline_ipsec_cfg(struct rvu *rvu, struct nix_inline_ipsec_cfg *r
 				 int blkaddr)
 {
 	u8 cpt_idx, cpt_blkaddr;
-	u64 val;
+	u64 val = 0;
 
 	cpt_idx = (blkaddr == BLKADDR_NIX0) ? 0 : 1;
 	if (req->enable) {
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
index 16cfc802e348..b25ecd36ca61 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
@@ -1734,8 +1734,8 @@  static void npc_load_kpu_profile(struct rvu *rvu)
 				rvu->kpu_prfl_addr = NULL;
 			} else {
 				kfree(rvu->kpu_fwdata);
+				rvu->kpu_fwdata = NULL;
 			}
-			rvu->kpu_fwdata = NULL;
 			rvu->kpu_fwdata_sz = 0;
 			if (retry_fwdb) {
 				retry_fwdb = false;
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
index 1a42bfded872..628251e940e8 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
@@ -648,14 +648,14 @@  int otx2_txschq_config(struct otx2_nic *pfvf, int lvl, int prio, bool txschq_for
 	} else if (lvl == NIX_TXSCH_LVL_TL4) {
 		parent = schq_list[NIX_TXSCH_LVL_TL3][prio];
 		req->reg[0] = NIX_AF_TL4X_PARENT(schq);
-		req->regval[0] = parent << 16;
+		req->regval[0] = (u64)parent << 16;
 		req->num_regs++;
 		req->reg[1] = NIX_AF_TL4X_SCHEDULE(schq);
 		req->regval[1] = dwrr_val;
 	} else if (lvl == NIX_TXSCH_LVL_TL3) {
 		parent = schq_list[NIX_TXSCH_LVL_TL2][prio];
 		req->reg[0] = NIX_AF_TL3X_PARENT(schq);
-		req->regval[0] = parent << 16;
+		req->regval[0] = (u64)parent << 16;
 		req->num_regs++;
 		req->reg[1] = NIX_AF_TL3X_SCHEDULE(schq);
 		req->regval[1] = dwrr_val;
@@ -670,11 +670,11 @@  int otx2_txschq_config(struct otx2_nic *pfvf, int lvl, int prio, bool txschq_for
 	} else if (lvl == NIX_TXSCH_LVL_TL2) {
 		parent = schq_list[NIX_TXSCH_LVL_TL1][prio];
 		req->reg[0] = NIX_AF_TL2X_PARENT(schq);
-		req->regval[0] = parent << 16;
+		req->regval[0] = (u64)parent << 16;
 
 		req->num_regs++;
 		req->reg[1] = NIX_AF_TL2X_SCHEDULE(schq);
-		req->regval[1] = TXSCH_TL1_DFLT_RR_PRIO << 24 | dwrr_val;
+		req->regval[1] = (u64)hw->txschq_aggr_lvl_rr_prio << 24 | dwrr_val;
 
 		if (lvl == hw->txschq_link_cfg_lvl) {
 			req->num_regs++;
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
index 6daf4d58c25d..62702ff6f3ea 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
@@ -496,6 +496,9 @@  static void otx2_pfvf_mbox_handler(struct work_struct *work)
 	return;
 
 inval_msg:
+	if (!msg)
+		return;
+
 	otx2_reply_invalid_msg(mbox, vf_idx, 0, msg->id);
 	otx2_mbox_msg_send(mbox, vf_idx);
 }
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_reg.h b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_reg.h
index 45a32e4b49d1..e3aee6e36215 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_reg.h
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_reg.h
@@ -139,33 +139,34 @@ 
 #define	NIX_LF_CINTX_ENA_W1C(a)		(NIX_LFBASE | 0xD50 | (a) << 12)
 
 /* NIX AF transmit scheduler registers */
-#define NIX_AF_SMQX_CFG(a)		(0x700 | (a) << 16)
-#define NIX_AF_TL1X_SCHEDULE(a)		(0xC00 | (a) << 16)
-#define NIX_AF_TL1X_CIR(a)		(0xC20 | (a) << 16)
-#define NIX_AF_TL1X_TOPOLOGY(a)		(0xC80 | (a) << 16)
-#define NIX_AF_TL2X_PARENT(a)		(0xE88 | (a) << 16)
-#define NIX_AF_TL2X_SCHEDULE(a)		(0xE00 | (a) << 16)
-#define NIX_AF_TL2X_TOPOLOGY(a)		(0xE80 | (a) << 16)
-#define NIX_AF_TL2X_CIR(a)              (0xE20 | (a) << 16)
-#define NIX_AF_TL2X_PIR(a)              (0xE30 | (a) << 16)
-#define NIX_AF_TL3X_PARENT(a)		(0x1088 | (a) << 16)
-#define NIX_AF_TL3X_SCHEDULE(a)		(0x1000 | (a) << 16)
-#define NIX_AF_TL3X_SHAPE(a)		(0x1010 | (a) << 16)
-#define NIX_AF_TL3X_CIR(a)		(0x1020 | (a) << 16)
-#define NIX_AF_TL3X_PIR(a)		(0x1030 | (a) << 16)
-#define NIX_AF_TL3X_TOPOLOGY(a)		(0x1080 | (a) << 16)
-#define NIX_AF_TL4X_PARENT(a)		(0x1288 | (a) << 16)
-#define NIX_AF_TL4X_SCHEDULE(a)		(0x1200 | (a) << 16)
-#define NIX_AF_TL4X_SHAPE(a)		(0x1210 | (a) << 16)
-#define NIX_AF_TL4X_CIR(a)		(0x1220 | (a) << 16)
-#define NIX_AF_TL4X_PIR(a)		(0x1230 | (a) << 16)
-#define NIX_AF_TL4X_TOPOLOGY(a)		(0x1280 | (a) << 16)
-#define NIX_AF_MDQX_SCHEDULE(a)		(0x1400 | (a) << 16)
-#define NIX_AF_MDQX_SHAPE(a)		(0x1410 | (a) << 16)
-#define NIX_AF_MDQX_CIR(a)		(0x1420 | (a) << 16)
-#define NIX_AF_MDQX_PIR(a)		(0x1430 | (a) << 16)
-#define NIX_AF_MDQX_PARENT(a)		(0x1480 | (a) << 16)
-#define NIX_AF_TL3_TL2X_LINKX_CFG(a, b)	(0x1700 | (a) << 16 | (b) << 3)
+#define NIX_AF_SMQX_CFG(a)		(0x700 | (u64)(a) << 16)
+#define NIX_AF_TL4X_SDP_LINK_CFG(a)	(0xB10 | (u64)(a) << 16)
+#define NIX_AF_TL1X_SCHEDULE(a)		(0xC00 | (u64)(a) << 16)
+#define NIX_AF_TL1X_CIR(a)		(0xC20 | (u64)(a) << 16)
+#define NIX_AF_TL1X_TOPOLOGY(a)		(0xC80 | (u64)(a) << 16)
+#define NIX_AF_TL2X_PARENT(a)		(0xE88 | (u64)(a) << 16)
+#define NIX_AF_TL2X_SCHEDULE(a)		(0xE00 | (u64)(a) << 16)
+#define NIX_AF_TL2X_TOPOLOGY(a)		(0xE80 | (u64)(a) << 16)
+#define NIX_AF_TL2X_CIR(a)		(0xE20 | (u64)(a) << 16)
+#define NIX_AF_TL2X_PIR(a)		(0xE30 | (u64)(a) << 16)
+#define NIX_AF_TL3X_PARENT(a)		(0x1088 | (u64)(a) << 16)
+#define NIX_AF_TL3X_SCHEDULE(a)		(0x1000 | (u64)(a) << 16)
+#define NIX_AF_TL3X_SHAPE(a)		(0x1010 | (u64)(a) << 16)
+#define NIX_AF_TL3X_CIR(a)		(0x1020 | (u64)(a) << 16)
+#define NIX_AF_TL3X_PIR(a)		(0x1030 | (u64)(a) << 16)
+#define NIX_AF_TL3X_TOPOLOGY(a)		(0x1080 | (u64)(a) << 16)
+#define NIX_AF_TL4X_PARENT(a)		(0x1288 | (u64)(a) << 16)
+#define NIX_AF_TL4X_SCHEDULE(a)		(0x1200 | (u64)(a) << 16)
+#define NIX_AF_TL4X_SHAPE(a)		(0x1210 | (u64)(a) << 16)
+#define NIX_AF_TL4X_CIR(a)		(0x1220 | (u64)(a) << 16)
+#define NIX_AF_TL4X_PIR(a)		(0x1230 | (u64)(a) << 16)
+#define NIX_AF_TL4X_TOPOLOGY(a)		(0x1280 | (u64)(a) << 16)
+#define NIX_AF_MDQX_SCHEDULE(a)		(0x1400 | (u64)(a) << 16)
+#define NIX_AF_MDQX_SHAPE(a)		(0x1410 | (u64)(a) << 16)
+#define NIX_AF_MDQX_CIR(a)		(0x1420 | (u64)(a) << 16)
+#define NIX_AF_MDQX_PIR(a)		(0x1430 | (u64)(a) << 16)
+#define NIX_AF_MDQX_PARENT(a)		(0x1480 | (u64)(a) << 16)
+#define NIX_AF_TL3_TL2X_LINKX_CFG(a, b)	(0x1700 | (u64)(a) << 16 | (b) << 3)
 
 /* LMT LF registers */
 #define LMT_LFBASE			BIT_ULL(RVU_FUNC_BLKADDR_SHIFT)
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c
index 53b2a4ef5298..04a462b3e638 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c
@@ -510,7 +510,7 @@  static int otx2_tx_napi_handler(struct otx2_nic *pfvf,
 
 static void otx2_adjust_adaptive_coalese(struct otx2_nic *pfvf, struct otx2_cq_poll *cq_poll)
 {
-	struct dim_sample dim_sample;
+	struct dim_sample dim_sample = { 0 };
 	u64 rx_frames, rx_bytes;
 
 	rx_frames = OTX2_GET_RX_STATS(RX_BCAST) + OTX2_GET_RX_STATS(RX_MCAST) +
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/qos.c b/drivers/net/ethernet/marvell/octeontx2/nic/qos.c
index 1e77bbf5d22a..7b23120a3e60 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/qos.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/qos.c
@@ -153,7 +153,6 @@  static void __otx2_qos_txschq_cfg(struct otx2_nic *pfvf,
 		num_regs++;
 
 		otx2_config_sched_shaping(pfvf, node, cfg, &num_regs);
-
 	} else if (level == NIX_TXSCH_LVL_TL4) {
 		otx2_config_sched_shaping(pfvf, node, cfg, &num_regs);
 	} else if (level == NIX_TXSCH_LVL_TL3) {
@@ -528,6 +527,7 @@  otx2_qos_sw_create_leaf_node(struct otx2_nic *pfvf,
 	err = otx2_qos_add_child_node(parent, node);
 	if (err) {
 		mutex_unlock(&pfvf->qos.qos_lock);
+		kfree(node);
 		return ERR_PTR(err);
 	}
 	mutex_unlock(&pfvf->qos.qos_lock);
@@ -1028,8 +1028,9 @@  static int otx2_qos_root_add(struct otx2_nic *pfvf, u16 htb_maj_id, u16 htb_defc
 	new_cfg = kzalloc(sizeof(*new_cfg), GFP_KERNEL);
 	if (!new_cfg) {
 		NL_SET_ERR_MSG_MOD(extack, "Memory allocation error");
-		err = -ENOMEM;
-		goto free_root_node;
+		otx2_qos_sw_node_delete(pfvf, root);
+		mutex_destroy(&pfvf->qos.qos_lock);
+		return -ENOMEM;
 	}
 	/* allocate htb root node */
 	new_cfg->schq[root->level] = 1;