linux-kernel - Re: [PATCH net-next v3 04/14] net: ethernet: qualcomm: Initialize PPE buffer management for IPQ9574

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: <4c24723a-1773-4034-95fb-47748f6e8982@quicinc.com>
Date: Tue, 11 Mar 2025 18:44:40 +0800
From: Jie Luo <quic_luoj@...cinc.com>
To: Andrew Lunn <andrew@...n.ch>
CC: Andrew Lunn <andrew+netdev@...n.ch>,
        "David S. Miller"
	<davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski
	<kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>, Rob Herring
	<robh@...nel.org>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        Conor Dooley
	<conor+dt@...nel.org>, Lei Wei <quic_leiwei@...cinc.com>,
        Suruchi Agarwal
	<quic_suruchia@...cinc.com>,
        Pavithra R <quic_pavir@...cinc.com>,
        "Simon
 Horman" <horms@...nel.org>, Jonathan Corbet <corbet@....net>,
        Kees Cook
	<kees@...nel.org>,
        "Gustavo A. R. Silva" <gustavoars@...nel.org>,
        "Philipp
 Zabel" <p.zabel@...gutronix.de>,
        <linux-arm-msm@...r.kernel.org>, <netdev@...r.kernel.org>,
        <devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <linux-doc@...r.kernel.org>, <linux-hardening@...r.kernel.org>,
        <quic_kkumarcs@...cinc.com>, <quic_linchen@...cinc.com>,
        <srinivas.kandagatla@...aro.org>, <bartosz.golaszewski@...aro.org>,
        <john@...ozen.org>
Subject: Re: [PATCH net-next v3 04/14] net: ethernet: qualcomm: Initialize PPE
 buffer management for IPQ9574



On 3/6/2025 11:29 PM, Andrew Lunn wrote:
>> Thanks for the suggestion. Just to clarify, we preferred
>> u32p_replace_bits() over FIELD_PREP() because the former does
>> a clear-and-set operation against a given mask, where as with
>> FIELD_PREP(), we need to clear the bits first before we use the
>> macro and then set it. Due to this, we preferred using
>> u32_replace_bits() since it made the macro definitions to modify
>> the registers simpler. Given this, would it be acceptable to
>> document u32p_replace_bits() better, as it is already being used
>> by other drivers as well?
> 
> I suggest you submit a patch to those who maintain that file and see
> what they say.
> 
> But maybe also look at how others are using u32p_replace_bits() and
> should it be wrapped up in a macro? FIELD_MOD()? These macros do a lot
> of build time checking that you are not overflowing the type. It would
> be good to have that to catch bugs at build time, rather than years
> later at runtime.
> 
>        Andrew

OK, understand. I will submit the patch by adding the FIELD_MODIFY()
with required build time checking included.

Below is a draft of the macro, please take a look if possible before
it is posted to maintainers. I will update the driver to use this macro
if it can be accepted. Thanks.

/** 
  
  

  * FIELD_MODIFY() - modify a bitfield element 
  
  

  * @_mask: shifted mask defining the field's length and position 
  
  

  * @_reg_p: point to the memory that should be updated 
  
  

  * @_val: value to store in the bitfield 
  
  

  * 
  
  

  * FIELD_MODIFY() modifies the set of bits in @_reg_p specified
  * by @_mask, by replacing them with the bitfield value passed
  * in as @_val. 
  
  
  
  

  */ 
  
  

#define FIELD_MODIFY(_mask, _reg_p, _val)              		      \ 
  
  

         ({                                  			      \ 
  
  

                 __BF_FIELD_CHECK(_mask, *_reg_p, _val, "FIELD_MODIFY: 
"); \ 
  

                 *_reg_p &= ~(_mask);                                  \ 
  
  

                 *_reg_p |= (_val) << __bf_shf(_mask) & (_mask);       \ 
  
  

         })