linux-kernel - Re: [PATCH net v2 2/3] octeon_ep: ensure dbell BADDR updation

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <0572ccee-d6ab-4921-b44e-c8641622940f@redhat.com>
Date: Mon, 29 Dec 2025 17:47:21 +0100
From: Paolo Abeni <pabeni@...hat.com>
To: Vimlesh Kumar <vimleshk@...vell.com>, netdev@...r.kernel.org,
 linux-kernel@...r.kernel.org
Cc: sedara@...vell.com, srasheed@...vell.com, hgani@...vell.com,
 Veerasenareddy Burru <vburru@...vell.com>,
 Andrew Lunn <andrew+netdev@...n.ch>, "David S. Miller"
 <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>
Subject: Re: [PATCH net v2 2/3] octeon_ep: ensure dbell BADDR updation

On 12/19/25 11:07 AM, Vimlesh Kumar wrote:
> @@ -327,11 +328,13 @@ static void octep_setup_iq_regs_cnxk_pf(struct octep_device *oct, int iq_no)
>  }
>  
>  /* Setup registers for a hardware Rx Queue  */
> -static void octep_setup_oq_regs_cnxk_pf(struct octep_device *oct, int oq_no)
> +static int octep_setup_oq_regs_cnxk_pf(struct octep_device *oct, int oq_no)
>  {
>  	u64 reg_val;
>  	u64 oq_ctl = 0ULL;
> +	u64 reg_ba_val;
>  	u32 time_threshold = 0;
> +	unsigned long t_out_jiffies;
>  	struct octep_oq *oq = oct->oq[oq_no];

Ouch, the above variable declaration follows the exact opposite of the
mandated style, please use the reverse christmas tree order.

Similar nit on the next patch.

>  
>  	oq_no += CFG_GET_PORTS_PF_SRN(oct->conf);
> @@ -343,6 +346,33 @@ static void octep_setup_oq_regs_cnxk_pf(struct octep_device *oct, int oq_no)
>  			reg_val = octep_read_csr64(oct, CNXK_SDP_R_OUT_CONTROL(oq_no));
>  		} while (!(reg_val & CNXK_R_OUT_CTL_IDLE));
>  	}
> +	octep_write_csr64(oct, CNXK_SDP_R_OUT_WMARK(oq_no),  oq->max_count);
> +	/* Wait for WMARK to get applied */
> +	usleep_range(10, 15);
> +
> +	octep_write_csr64(oct, CNXK_SDP_R_OUT_SLIST_BADDR(oq_no),
> +			  oq->desc_ring_dma);
> +	octep_write_csr64(oct, CNXK_SDP_R_OUT_SLIST_RSIZE(oq_no),
> +			  oq->max_count);
> +	reg_ba_val = octep_read_csr64(oct, CNXK_SDP_R_OUT_SLIST_BADDR(oq_no));
> +
> +	if (reg_ba_val != oq->desc_ring_dma) {
> +		t_out_jiffies = jiffies + 10 * HZ;
> +		do {
> +			if (reg_ba_val == ULLONG_MAX)
> +				return -EFAULT;
> +			octep_write_csr64(oct,
> +					  CNXK_SDP_R_OUT_SLIST_BADDR(oq_no),
> +					  oq->desc_ring_dma);
> +			octep_write_csr64(oct,
> +					  CNXK_SDP_R_OUT_SLIST_RSIZE(oq_no),
> +					  oq->max_count);
> +			reg_ba_val =
> +			octep_read_csr64(oct,
> +					 CNXK_SDP_R_OUT_SLIST_BADDR(oq_no));
> +		} while ((reg_ba_val != oq->desc_ring_dma) &&
> +			  time_before(jiffies, t_out_jiffies));

If a timeout happens - !time_before(jiffies, t_out_jiffies) - execution
proceed even if reg_ba_val != oq->desc_ring_dma; I guess you should
error out istead?!? Otherwise you should at least add a comment for
future memory explaining why that is correct.

Similar question on the next patch.

/P