Re: [kvm-unit-tests PATCH 2/2] arm64: Check if the configured translation granule is supported

[Nikos Nikoleris <nikos.nikoleris@arm.com>]

On 03/11/2020 17:36, Andrew Jones wrote:
> On Tue, Nov 03, 2020 at 05:03:15PM +0000, Alexandru Elisei wrote:
>>> +}
>>> +
>>> +static inline bool system_supports_granule(size_t granule)
>>> +{
>>> +	u64 mmfr0 = get_id_aa64mmfr0_el1();
>>> +
>>> +	return ((granule == SZ_4K && ((mmfr0 >> 28) & 0xf) == 0) ||
>>> +		(granule == SZ_64K && ((mmfr0 >> 24) & 0xf) == 0) ||
>>> +		(granule == SZ_16K && ((mmfr0 >> 20) & 0xf) == 1));
>>> +}
>>
>> Or we can turn it into a switch statement and keep all the field defines. Either
>> way looks good to me (funny how tgran16 stands out).
>>
> 
> Keeping the defines is probably a good idea. Whether the function uses
> a switch or an expression like above doesn't matter to me much. Keeping
> LOC down in the lib/ code is a goal of kvm-unit-tests, but so is
> readabilty. If the switch looks better, then let's go that way.
> 

I liked Drew's version in that it was very concise. The new version will 
be much longer. If you think it's more readable I'll use that instead.

diff --git a/lib/arm64/asm/processor.h b/lib/arm64/asm/processor.h
index 02665b8..430ded3 100644
--- a/lib/arm64/asm/processor.h
+++ b/lib/arm64/asm/processor.h
@@ -117,5 +117,38 @@ static inline u64 get_ctr(void)

  extern u32 dcache_line_size;

+static inline unsigned long get_id_aa64mmfr0_el1(void)
+{
+       return read_sysreg(id_aa64mmfr0_el1);
+}
+
+#define ID_AA64MMFR0_TGRAN4_SHIFT      28
+#define ID_AA64MMFR0_TGRAN64_SHIFT     24
+#define ID_AA64MMFR0_TGRAN16_SHIFT     20
+
+#define ID_AA64MMFR0_TGRAN4_SUPPORTED  0x0
+#define ID_AA64MMFR0_TGRAN64_SUPPORTED 0x0
+#define ID_AA64MMFR0_TGRAN16_SUPPORTED 0x1
+
+static inline bool system_supports_granule(size_t granule)
+{
+       u32 shift;
+       u32 val;
+       u64 mmfr0 = get_id_aa64mmfr0_el1();
+       if (granule == SZ_4K) {
+               shift = ID_AA64MMFR0_TGRAN4_SHIFT;
+               val = ID_AA64MMFR0_TGRAN4_SUPPORTED;
+       } else if (granule == SZ_16K) {
+               shift = ID_AA64MMFR0_TGRAN16_SHIFT;
+               val = ID_AA64MMFR0_TGRAN16_SUPPORTED;
+       } else {
+               assert(granule == SZ_64K);
+               shift = ID_AA64MMFR0_TGRAN64_SHIFT;
+               val = ID_AA64MMFR0_TGRAN64_SUPPORTED;
+       }
+
+       return ((mmfr0 >> shift) & 0xf) == val;
+}
+
  #endif /* !__ASSEMBLY__ */
  #endif /* _ASMARM64_PROCESSOR_H_ */

Thanks,

Nikos

> Thanks,
> drew
>