[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <79355a26-c901-40a4-8a90-096da1380d9f@quicinc.com>
Date: Tue, 17 Jun 2025 00:19:11 +0800
From: Luo Jie <quic_luoj@...cinc.com>
To: Will Deacon <will@...nel.org>
CC: Markus Elfring <Markus.Elfring@....de>, <cocci@...ia.fr>,
LKML
<linux-kernel@...r.kernel.org>,
<linux-arm-kernel@...ts.infradead.org>, <kvmarm@...ts.linux.dev>,
Andrew Lunn <andrew@...n.ch>,
Catalin Marinas
<catalin.marinas@....com>,
Joey Gouly <joey.gouly@....com>, Julia Lawall
<Julia.Lawall@...ia.fr>,
Kiran Kumar C.S.K <quic_kkumarcs@...cinc.com>,
"Lei
Wei" <quic_leiwei@...cinc.com>, Marc Zyngier <maz@...nel.org>,
Nicolas Palix
<nicolas.palix@...g.fr>,
Oliver Upton <oliver.upton@...ux.dev>,
Pavithra R
<quic_pavir@...cinc.com>,
Rasmus Villemoes <linux@...musvillemoes.dk>,
Suruchi Agarwal <quic_suruchia@...cinc.com>,
Suzuki Poulose
<suzuki.poulose@....com>,
Yury Norov <yury.norov@...il.com>, Zenghui Yu
<yuzenghui@...wei.com>,
<quic_linchen@...cinc.com>
Subject: Re: [cocci] [PATCH v4 2/5] arm64: tlb: Convert the opencoded field
modify
On 6/16/2025 6:41 PM, Will Deacon wrote:
> On Mon, Jun 16, 2025 at 06:37:41PM +0800, Luo Jie wrote:
>>
>>
>> On 6/13/2025 4:15 AM, Markus Elfring wrote:
>>> I see further refinement possibilities for such a change description.
>>>
>>>
>>>> Replace below code with the wrapper FIELD_MODIFY(MASK, ®, val)
>>>> - reg &= ~MASK;
>>>> - reg |= FIELD_PREP(MASK, val);
>>>
>>> * How do you think about to omit leading minus characters?
>>>
>>> * Subsequent blank line?
>>>
>>>
>>>> More information about semantic patching is available at
>>>> http://coccinelle.lip6.fr/
>>>
>>> I suggest to omit this information here (and in similar patches).
>>>
>>> Regards,
>>> Markus
>>
>> Thank you for your suggestions. The current commit message was generated
>> by the following patch mode command:
>> ```
>> make coccicheck MODE=patch COCCI=scripts/coccinelle/misc/field_modify.cocci
>> V=1
>> ```
>> However, as I understand, the discussion on the ARM patches (between
>> Russel/Marc/Yury) has concluded that the ARM arch changes may not be
>> adding value over the current code, so I will drop the ARM patches
>> in the next version.
>
> Well, hang on a second. From what I can tell, the objections haven't
> been specific to arch/arm{,64}/. You haven't really explained why this
> new helper is needed and what value it brings over the existing set of
> functionality.
>
> So maybe start there, rather than dropping the parts that attracted the
> comments to start with?
>
> Will
FIELD_MODIFY is similar to uxx_replace_bits() for in-memory bit field
modification, but with the added advantage of strict parameter type
checking at compile time.
Previous discussions (in patch series v3) among Russell, Marc, and Yury
focused on whether there is any added advantage of using FIELD_MODIFY()
(possibility of size overflow checking) for the specific cases being
modified by the patch series inside arm64. For the one case where enum
was being used, it was originally thought that there may be a
possibility of size overflow due to 32bit size used by the compiler for
the enum. However it was identified that the kernel's compiler already
uses 64bit types for enum if the number of bits used with the enum is
more than 32 (the code in this case used more than 32 bits).So, it seems
that there is no additional benefit of using FIELD_MODIFY() in the
specific cases in arm64/.
Please note that the current ARM64 code using FIELD_PREP() also supports
strict parameter checking at compile time. Both FIELD_PREP and
FIELD_MODIFY utilize `__BF_FIELD_CHECK()` to support this compile-time
parameter validation.
Powered by blists - more mailing lists