Re: [bootwrapper PATCH v2 03/13] aarch64: add system register accessors

From: Andre Przywara <andre.przywara@arm.com>
To: Mark Rutland <mark.rutland@arm.com>
Cc: linux-arm-kernel@lists.infradead.org, Jaxson.Han@arm.com,
	robin.murphy@arm.com, vladimir.murzin@arm.com, Wei.Chen@arm.com
Subject: Re: [bootwrapper PATCH v2 03/13] aarch64: add system register accessors
Date: Fri, 14 Jan 2022 15:32:27 +0000	[thread overview]
Message-ID: <20220114153227.3f2a2d09@donnerap.cambridge.arm.com> (raw)
In-Reply-To: <20220114105653.3003399-4-mark.rutland@arm.com>

On Fri, 14 Jan 2022 10:56:43 +0000
Mark Rutland <mark.rutland@arm.com> wrote:

Hi Mark,

> We open code the use of mrs/msr for specific registers, which is
> somewhat tedious. Add macros to do this generically, along with a helper
> to extract a specific register field. Existing C usage is converted to
> the new helpers, and register definitions moved to a common location.
> 
> There should be no functional change as a result of this patch.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>

Stared at and verified the architecture bits used in here:

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre

> ---
>  arch/aarch64/include/asm/cpu.h    | 41 ++++++++++++++++++++++---------
>  arch/aarch64/include/asm/gic-v3.h | 10 +++-----
>  2 files changed, 32 insertions(+), 19 deletions(-)
> 
> diff --git a/arch/aarch64/include/asm/cpu.h b/arch/aarch64/include/asm/cpu.h
> index 63eb1c3..1053414 100644
> --- a/arch/aarch64/include/asm/cpu.h
> +++ b/arch/aarch64/include/asm/cpu.h
> @@ -9,10 +9,14 @@
>  #ifndef __ASM_AARCH64_CPU_H
>  #define __ASM_AARCH64_CPU_H
>  
> +#include <bits.h>
> +
>  #define MPIDR_ID_BITS		0xff00ffffff
>  
>  #define CURRENTEL_EL3		(3 << 2)
>  
> +#define ID_AA64PFR0_EL1_GIC	BITS(27, 24)
> +
>  /*
>   * RES1 bits,  little-endian, caches and MMU off, no alignment checking,
>   * no WXN.
> @@ -29,6 +33,12 @@
>  
>  #define CPTR_EL3_EZ		(1 << 8)
>  
> +#define ICC_SRE_EL2		S3_4_C12_C9_5
> +#define ICC_SRE_EL3		S3_6_C12_C12_5
> +#define ICC_CTLR_EL1		S3_0_C12_C12_4
> +#define ICC_CTLR_EL3		S3_6_C12_C12_4
> +#define ICC_PMR_EL1		S3_0_C4_C6_0
> +
>  #define ZCR_EL3			s3_6_c1_c2_0
>  #define ZCR_EL3_LEN_MASK	0x1ff
>  
> @@ -50,20 +60,27 @@
>  
>  #define sevl()		asm volatile ("sevl\n" : : : "memory")
>  
> -static inline unsigned long read_mpidr(void)
> -{
> -	unsigned long mpidr;
> +#define __str(def)	#def
>  
> -	asm volatile ("mrs	%0, mpidr_el1\n" : "=r" (mpidr));
> -	return mpidr & MPIDR_ID_BITS;
> -}
> +#define mrs(reg)							\
> +({									\
> +	unsigned long __mrs_val;					\
> +	asm volatile("mrs %0, " __str(reg) : "=r" (__mrs_val));		\
> +	__mrs_val;							\
> +})
>  
> -static inline uint64_t read_id_aa64pfr0(void)
> -{
> -	uint64_t val;
> +#define msr(reg, val)							\
> +do {									\
> +	unsigned long __msr_val = val;					\
> +	asm volatile("msr " __str(reg) ", %0" : : "r" (__msr_val));	\
> +} while (0)
> +
> +#define mrs_field(reg, field) \
> +	BITS_EXTRACT(mrs(reg), (reg##_##field))
>  
> -	asm volatile ("mrs	%0, id_aa64pfr0_el1\n" : "=r" (val));
> -	return val;
> +static inline unsigned long read_mpidr(void)
> +{
> +	return mrs(mpidr_el1) & MPIDR_ID_BITS;
>  }
>  
>  static inline void iciallu(void)
> @@ -73,7 +90,7 @@ static inline void iciallu(void)
>  
>  static inline int has_gicv3_sysreg(void)
>  {
> -	return !!((read_id_aa64pfr0() >> 24) & 0xf);
> +	return !!mrs_field(ID_AA64PFR0_EL1, GIC);
>  }
>  
>  #endif /* !__ASSEMBLY__ */
> diff --git a/arch/aarch64/include/asm/gic-v3.h b/arch/aarch64/include/asm/gic-v3.h
> index 5b32380..2447480 100644
> --- a/arch/aarch64/include/asm/gic-v3.h
> +++ b/arch/aarch64/include/asm/gic-v3.h
> @@ -9,20 +9,16 @@
>  #ifndef __ASM_AARCH64_GICV3_H
>  #define __ASM_AARCH64_GICV3_H
>  
> -#define ICC_SRE_EL2	"S3_4_C12_C9_5"
> -#define ICC_SRE_EL3	"S3_6_C12_C12_5"
> -#define ICC_CTLR_EL1	"S3_0_C12_C12_4"
> -#define ICC_CTLR_EL3	"S3_6_C12_C12_4"
> -#define ICC_PMR_EL1	"S3_0_C4_C6_0"
> +#include <asm/cpu.h>
>  
>  static inline void gic_write_icc_sre(uint32_t val)
>  {
> -	asm volatile ("msr " ICC_SRE_EL3 ", %0" : : "r" (val));
> +	msr(ICC_SRE_EL3, val);
>  }
>  
>  static inline void gic_write_icc_ctlr(uint32_t val)
>  {
> -	asm volatile ("msr " ICC_CTLR_EL3 ", %0" : : "r" (val));
> +	msr(ICC_CTLR_EL3, val);
>  }
>  
>  #endif

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