netdev - Re: [net v3 PATCH] octeontx2-pf: Avoid typecasts by simplifying otx2_atomic64

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <aDgHEPfNQvziIqpr@e6bae70a73d4>
Date: Thu, 29 May 2025 07:04:48 +0000
From: Subbaraya Sundeep <sbhatta@...vell.com>
To: Simon Horman <horms@...nel.org>
CC: <andrew+netdev@...n.ch>, <davem@...emloft.net>, <edumazet@...gle.com>,
        <kuba@...nel.org>, <pabeni@...hat.com>, <saikrishnag@...vell.com>,
        <gakula@...vell.com>, <hkelam@...vell.com>, <sgoutham@...vell.com>,
        <lcherian@...vell.com>, <bbhushan2@...vell.com>, <jerinj@...vell.com>,
        <netdev@...r.kernel.org>
Subject: Re: [net v3 PATCH] octeontx2-pf: Avoid typecasts by simplifying
 otx2_atomic64_add macro

Hi Simon,

On 2025-05-28 at 15:03:33, Simon Horman (horms@...nel.org) wrote:
> On Wed, May 28, 2025 at 10:10:42AM +0530, Subbaraya Sundeep wrote:
> > Just because otx2_atomic64_add is using u64 pointer as argument
> > all callers has to typecast __iomem void pointers which inturn
> > causing sparse warnings. Fix those by changing otx2_atomic64_add
> > argument to void pointer.
> > 
> > Fixes: caa2da34fd25 ("octeontx2-pf: Initialize and config queues")
> > Signed-off-by: Subbaraya Sundeep <sbhatta@...vell.com>
> > ---
> > v3:
> >  Make otx2_atomic64_add as nop for architectures other than ARM64
> >  to fix sparse warnings
> > v2:
> >  Fixed x86 build error of void pointer dereference reported by
> >  kernel test robot
> 
> Sorry, I seem to have made some some comments on v2 after v3 was posted.
> 
> 1) I'm wondering if you considered changing the type of the 2nd parameter
>    of otx2_atomic64_add to u64 __iomem * and, correspondingly, the type of

My intention is to fix sparse warnings (no __force) and avoid typecasts
so that code is correct and looks cleaner. If I change 2nd param of
otx2_atomics64_add as u64 __iomem * then I still have to use
__force to make sparse happy. This way only otx2_atomic64_add looks odd
internally with assembly stuff.

>    the local variables updated by this patch. Perhaps that isn't so clean
>    for some reason. But if it can be done cleanly it does seem slightly
>    nicer to me.
>
> 2) I wonder if this is more of a clean-up for net-next (once it re-opens,
>    no Fixes tag) than a fix.
> 
Sure. Will post as net-next material later.

Thanks,
Sundeep

> > diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
> 
> ...
> 
> > @@ -747,7 +748,11 @@ static inline u64 otx2_atomic64_add(u64 incr, u64 *ptr)
> >  
> >  #else
> >  #define otx2_write128(lo, hi, addr)		writeq((hi) | (lo), addr)
> > -#define otx2_atomic64_add(incr, ptr)		({ *ptr += incr; })
> > +
> > +static inline u64 otx2_atomic64_add(u64 incr, void __iomem *addr)
> > +{
> > +	return 0;
> 
> Is it intentional that no increment is occurring here,
> whereas there was one in the macro version this replaces?
> 
> > +}
> >  #endif
> 
> ...