Re: [PATCH V2 2/2] arm64/mm: Add remaining TLBI_XXX_MASK macros

[Anshuman Khandual <anshuman.khandual@arm.com>]

On 28/10/25 6:13 PM, Jonathan Cameron wrote:
> On Mon, 27 Oct 2025 07:06:45 +0530
> Anshuman Khandual <anshuman.khandual@arm.com> wrote:
> 
>> On 24/10/25 4:30 PM, Jonathan Cameron wrote:
>>> On Fri, 24 Oct 2025 05:02:07 +0100
>>> Anshuman Khandual <anshuman.khandual@arm.com> wrote:
>>>   
>>>> Add remaining TLBI_XXX_MASK macros and replace current open encoded fields.
>>>> While here replace hard coded page size based shifts but with derived ones
>>>> via ilog2() thus adding some required context.
>>>>
>>>> TLBI_TTL_MASK has been split into separate TLBI_TTL_MASK and TLBI_TG_MASK
>>>> as appropriate because currently it simultaneously contains both page size
>>>> and translation table level information. KVM on arm64 has been updated to
>>>> accommodate these changes to TLBI_TTL_MASK.
>>>>
>>>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>>>> Cc: Will Deacon <will@kernel.org>
>>>> Cc: Marc Zyngier <maz@kernel.org>
>>>> Cc: Oliver Upton <oliver.upton@linux.dev>
>>>> Cc: linux-arm-kernel@lists.infradead.org
>>>> Cc: linux-kernel@vger.kernel.org
>>>> Cc: kvmarm@lists.linux.dev
>>>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>>>> ---
>>>>  arch/arm64/include/asm/tlbflush.h | 26 ++++++++++++++++++--------
>>>>  arch/arm64/kvm/nested.c           |  8 +++++---
>>>>  2 files changed, 23 insertions(+), 11 deletions(-)
>>>>
>>>> diff --git a/arch/arm64/include/asm/tlbflush.h b/arch/arm64/include/asm/tlbflush.h
>>>> index 131096094f5b..cf75fc2a06c3 100644
>>>> --- a/arch/arm64/include/asm/tlbflush.h
>>>> +++ b/arch/arm64/include/asm/tlbflush.h
>>>> @@ -57,9 +57,10 @@
>>>>  /* This macro creates a properly formatted VA operand for the TLBI */
>>>>  #define __TLBI_VADDR(addr, asid)				\
>>>>  	({							\
>>>> -		unsigned long __ta = (addr) >> 12;		\
>>>> -		__ta &= GENMASK_ULL(43, 0);			\
>>>> -		__ta |= (unsigned long)(asid) << 48;		\
>>>> +		unsigned long __ta = (addr) >> ilog2(SZ_4K);	\
>>>> +		__ta &= TLBI_BADDR_MASK;			\
>>>> +		__ta &= ~TLBI_ASID_MASK;			\
>>>> +		__ta |= FIELD_PREP(TLBI_ASID_MASK, asid);	\  
>>> I think you can replace the two lines above with
>>> 		FIELD_MODIFY(TLBI_ASID_MASK, &__ta, asid);
>>>
>>> It's a small reduction in code but I don't mind much either way.  
>>
>> Right. FIELD_MODIFY() might be appropriate in this scenario but
>> there will be some additional code churn needed. I don't have a
>> strong opinion either way.
>>
>> --- a/arch/arm64/include/asm/tlbflush.h
>> +++ b/arch/arm64/include/asm/tlbflush.h
>> @@ -55,13 +55,12 @@
>>  } while (0)
>>
>>  /* This macro creates a properly formatted VA operand for the TLBI */
>> -#define __TLBI_VADDR(addr, asid)                               \
>> -       ({                                                      \
>> -               unsigned long __ta = (addr) >> ilog2(SZ_4K);    \
>> -               __ta &= TLBI_BADDR_MASK;                        \
>> -               __ta &= ~TLBI_ASID_MASK;                        \
>> -               __ta |= FIELD_PREP(TLBI_ASID_MASK, asid);       \
>> -               __ta;                                           \
>> +#define __TLBI_VADDR(addr, asid)                                       \
>> +       ({                                                              \
>> +               unsigned long __ta = (addr) >> ilog2(SZ_4K);            \
>> +               __ta &= TLBI_BADDR_MASK;                                \
>> +               __ta |= FIELD_MODIFY(TLBI_ASID_MASK, &__ta, asid);      \
> 		  FIELD_MODIFY(TLBI_ASID_MASK, &__tab, asid);
> 
> as it's done in place in the second parameter.

Sorry but could not understand your point above.