Re: [PATCH v2 67/94] arm64: Add level-hinted TLB invalidation helper

From: Marc Zyngier <maz@kernel.org>
To: Zhenyu Ye <yezhenyu2@huawei.com>
Cc: kvm@vger.kernel.org, Suzuki K Poulose <suzuki.poulose@arm.com>,
	Jintack Lim <jintack@cs.columbia.edu>,
	Andre Przywara <andre.przywara@arm.com>,
	Christoffer Dall <christoffer.dall@arm.com>,
	Dave Martin <Dave.Martin@arm.com>,
	James Morse <james.morse@arm.com>,
	Julien Thierry <julien.thierry.kdev@gmail.com>,
	Alexandru Elisei <alexandru.elisei@arm.com>,
	kvmarm@lists.cs.columbia.edu,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v2 67/94] arm64: Add level-hinted TLB invalidation helper
Date: Wed, 25 Mar 2020 11:28:03 +0000	[thread overview]
Message-ID: <ff18c6737eed7baa29adee5de4f044c5@kernel.org> (raw)
In-Reply-To: <b4120382-d175-0c2f-249e-cc77a09709db@huawei.com>

Zhenyu,

On 2020-03-25 10:38, Zhenyu Ye wrote:
> Hi Marc,
> 
> On 2020/2/12 1:49, Marc Zyngier wrote:
>> Add a level-hinted TLB invalidation helper that only gets used if
>> ARMv8.4-TTL gets detected.
>> 
>> Signed-off-by: Marc Zyngier <maz@kernel.org>
>> ---
>>  arch/arm64/include/asm/tlbflush.h | 30 ++++++++++++++++++++++++++++++
>>  1 file changed, 30 insertions(+)
>> 
>> diff --git a/arch/arm64/include/asm/tlbflush.h 
>> b/arch/arm64/include/asm/tlbflush.h
>> index bc3949064725..a3f70778a325 100644
>> --- a/arch/arm64/include/asm/tlbflush.h
>> +++ b/arch/arm64/include/asm/tlbflush.h
>> @@ -10,6 +10,7 @@
>> 
>>  #ifndef __ASSEMBLY__
>> 
>> +#include <linux/bitfield.h>
>>  #include <linux/mm_types.h>
>>  #include <linux/sched.h>
>>  #include <asm/cputype.h>
>> @@ -59,6 +60,35 @@
>>  		__ta;						\
>>  	})
>> 
>> +#define TLBI_TTL_MASK	GENMASK_ULL(47, 44)
>> +
>> +#define __tlbi_level(op, addr, level)					\
>> +	do {								\
>> +		u64 arg = addr;						\
>> +									\
>> +		if (cpus_have_const_cap(ARM64_HAS_ARMv8_4_TTL) &&	\
>> +		    level) {						\
>> +			u64 ttl = level;				\
>> +									\
>> +			switch (PAGE_SIZE) {				\
>> +			case SZ_4K:					\
>> +				ttl |= 1 << 2;				\
>> +				break;					\
>> +			case SZ_16K:					\
>> +				ttl |= 2 << 2;				\
>> +				break;					\
>> +			case SZ_64K:					\
>> +				ttl |= 3 << 2;				\
>> +				break;					\
>> +			}						\
> 
> Can we define a macro here to replace the switch? It will be more
> clearly and efficient. Such as:
> 
> #define __TG ((PAGE_SHIFT - 12) / 2 + 1)
> ttl |= __TG << 2;

Let me rephrase this: a convoluted formula with magic numbers in it
is clearer than the values documented in the specification (Table 
D5-53)?
I have the exact opposite view.

As for efficency, you do realize that the compiler always discards two 
third
of this code on any possible configuration, right?

I think the code above is the clearest way to express table D5-53, and
the only missing bit is a reference to that table.

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel