Re: [PATCH v2 04/24] arm64: Move MAIR_EL1_SET to asm/memory.h

[Marc Zyngier]

On Mon, 16 Nov 2020 20:42:58 +0000,
David Brazdil <dbrazdil@xxxxxxxxxx> wrote:
> 
> KVM currently initializes MAIR_EL2 to the value of MAIR_EL1. In
> preparation for initializing MAIR_EL2 before MAIR_EL1, move the constant
> into a shared header file. Since it is used for EL1 and EL2, rename to
> MAIR_ELx_SET.
> 
> Signed-off-by: David Brazdil <dbrazdil@xxxxxxxxxx>
> ---
>  arch/arm64/include/asm/memory.h | 29 ++++++++++++++---------------
>  arch/arm64/include/asm/sysreg.h | 30 ++++++++++++++++++++++++++++++
>  arch/arm64/mm/proc.S            | 15 +--------------
>  3 files changed, 45 insertions(+), 29 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
> index cd61239bae8c..8ae8fd883a0c 100644
> --- a/arch/arm64/include/asm/memory.h
> +++ b/arch/arm64/include/asm/memory.h
> @@ -13,6 +13,7 @@
>  #include <linux/const.h>
>  #include <linux/sizes.h>
>  #include <asm/page-def.h>
> +#include <asm/sysreg.h>
>  
>  /*
>   * Size of the PCI I/O space. This must remain a power of two so that
> @@ -124,21 +125,6 @@
>   */
>  #define SEGMENT_ALIGN		SZ_64K
>  
> -/*
> - * Memory types available.
> - *
> - * IMPORTANT: MT_NORMAL must be index 0 since vm_get_page_prot() may 'or' in
> - *	      the MT_NORMAL_TAGGED memory type for PROT_MTE mappings. Note
> - *	      that protection_map[] only contains MT_NORMAL attributes.
> - */
> -#define MT_NORMAL		0
> -#define MT_NORMAL_TAGGED	1
> -#define MT_NORMAL_NC		2
> -#define MT_NORMAL_WT		3
> -#define MT_DEVICE_nGnRnE	4
> -#define MT_DEVICE_nGnRE		5
> -#define MT_DEVICE_GRE		6
> -
>  /*
>   * Memory types for Stage-2 translation
>   */
> @@ -152,6 +138,19 @@
>  #define MT_S2_FWB_NORMAL	6
>  #define MT_S2_FWB_DEVICE_nGnRE	1
>  
> +/*
> + * Default MAIR_EL1. MT_NORMAL_TAGGED is initially mapped as Normal memory and
> + * changed during __cpu_setup to Normal Tagged if the system supports MTE.
> + */
> +#define MAIR_ELx_SET							\
> +	(MAIR_ATTRIDX(MAIR_ATTR_DEVICE_nGnRnE, MT_DEVICE_nGnRnE) |	\
> +	 MAIR_ATTRIDX(MAIR_ATTR_DEVICE_nGnRE, MT_DEVICE_nGnRE) |	\
> +	 MAIR_ATTRIDX(MAIR_ATTR_DEVICE_GRE, MT_DEVICE_GRE) |		\
> +	 MAIR_ATTRIDX(MAIR_ATTR_NORMAL_NC, MT_NORMAL_NC) |		\
> +	 MAIR_ATTRIDX(MAIR_ATTR_NORMAL, MT_NORMAL) |			\
> +	 MAIR_ATTRIDX(MAIR_ATTR_NORMAL_WT, MT_NORMAL_WT) |		\
> +	 MAIR_ATTRIDX(MAIR_ATTR_NORMAL, MT_NORMAL_TAGGED))
> +
>  #ifdef CONFIG_ARM64_4K_PAGES
>  #define IOREMAP_MAX_ORDER	(PUD_SHIFT)
>  #else
> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> index e2ef4c2edf06..24e773414cb4 100644
> --- a/arch/arm64/include/asm/sysreg.h
> +++ b/arch/arm64/include/asm/sysreg.h
> @@ -635,6 +635,34 @@
>  /* Position the attr at the correct index */
>  #define MAIR_ATTRIDX(attr, idx)		((attr) << ((idx) * 8))
>  
> +/*
> + * Memory types available.
> + *
> + * IMPORTANT: MT_NORMAL must be index 0 since vm_get_page_prot() may 'or' in
> + *	      the MT_NORMAL_TAGGED memory type for PROT_MTE mappings. Note
> + *	      that protection_map[] only contains MT_NORMAL attributes.
> + */
> +#define MT_NORMAL		0
> +#define MT_NORMAL_TAGGED	1
> +#define MT_NORMAL_NC		2
> +#define MT_NORMAL_WT		3
> +#define MT_DEVICE_nGnRnE	4
> +#define MT_DEVICE_nGnRE		5
> +#define MT_DEVICE_GRE		6
> +
> +/*
> + * Default MAIR_ELx. MT_NORMAL_TAGGED is initially mapped as Normal memory and
> + * changed during __cpu_setup to Normal Tagged if the system supports MTE.
> + */
> +#define MAIR_ELx_SET							\
> +	(MAIR_ATTRIDX(MAIR_ATTR_DEVICE_nGnRnE, MT_DEVICE_nGnRnE) |	\
> +	 MAIR_ATTRIDX(MAIR_ATTR_DEVICE_nGnRE, MT_DEVICE_nGnRE) |	\
> +	 MAIR_ATTRIDX(MAIR_ATTR_DEVICE_GRE, MT_DEVICE_GRE) |		\
> +	 MAIR_ATTRIDX(MAIR_ATTR_NORMAL_NC, MT_NORMAL_NC) |		\
> +	 MAIR_ATTRIDX(MAIR_ATTR_NORMAL, MT_NORMAL) |			\
> +	 MAIR_ATTRIDX(MAIR_ATTR_NORMAL_WT, MT_NORMAL_WT) |		\
> +	 MAIR_ATTRIDX(MAIR_ATTR_NORMAL, MT_NORMAL_TAGGED))
> +
>  /* id_aa64isar0 */
>  #define ID_AA64ISAR0_RNDR_SHIFT		60
>  #define ID_AA64ISAR0_TLB_SHIFT		56
> @@ -992,6 +1020,7 @@
>  /* Safe value for MPIDR_EL1: Bit31:RES1, Bit30:U:0, Bit24:MT:0 */
>  #define SYS_MPIDR_SAFE_VAL	(BIT(31))
>  
> +#ifndef LINKER_SCRIPT

This is terribly ugly. Why is this included by the linker script? Does
it actually define __ASSEMBLY__?

>  #ifdef __ASSEMBLY__
>  
>  	.irp	num,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30
> @@ -1109,5 +1138,6 @@
>  })
>  
>  #endif
> +#endif	/* LINKER_SCRIPT */
>  
>  #endif	/* __ASM_SYSREG_H */
> diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
> index 23c326a06b2d..e3b9aa372b96 100644
> --- a/arch/arm64/mm/proc.S
> +++ b/arch/arm64/mm/proc.S
> @@ -45,19 +45,6 @@
>  #define TCR_KASAN_FLAGS 0
>  #endif
>  
> -/*
> - * Default MAIR_EL1. MT_NORMAL_TAGGED is initially mapped as Normal memory and
> - * changed during __cpu_setup to Normal Tagged if the system supports MTE.
> - */
> -#define MAIR_EL1_SET							\
> -	(MAIR_ATTRIDX(MAIR_ATTR_DEVICE_nGnRnE, MT_DEVICE_nGnRnE) |	\
> -	 MAIR_ATTRIDX(MAIR_ATTR_DEVICE_nGnRE, MT_DEVICE_nGnRE) |	\
> -	 MAIR_ATTRIDX(MAIR_ATTR_DEVICE_GRE, MT_DEVICE_GRE) |		\
> -	 MAIR_ATTRIDX(MAIR_ATTR_NORMAL_NC, MT_NORMAL_NC) |		\
> -	 MAIR_ATTRIDX(MAIR_ATTR_NORMAL, MT_NORMAL) |			\
> -	 MAIR_ATTRIDX(MAIR_ATTR_NORMAL_WT, MT_NORMAL_WT) |		\
> -	 MAIR_ATTRIDX(MAIR_ATTR_NORMAL, MT_NORMAL_TAGGED))
> -
>  #ifdef CONFIG_CPU_PM
>  /**
>   * cpu_do_suspend - save CPU registers context
> @@ -425,7 +412,7 @@ SYM_FUNC_START(__cpu_setup)
>  	/*
>  	 * Memory region attributes
>  	 */
> -	mov_q	x5, MAIR_EL1_SET
> +	mov_q	x5, MAIR_ELx_SET
>  #ifdef CONFIG_ARM64_MTE
>  	/*
>  	 * Update MAIR_EL1, GCR_EL1 and TFSR*_EL1 if MTE is supported
> -- 
> 2.29.2.299.gdc1121823c-goog
> 
> 

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.