[boot-wrapper PATCH 0/5] Misc cleanups

[boot-wrapper PATCH 0/5] Misc cleanups

[Mark Rutland]

These are a few cleanup patches I've split out of my ongoing bootwrapper
rework, and intend to apply by the end of this week.

My WIP cleanup branch with subsequent patches can be found at:

  git://git.kernel.org/pub/scm/linux/kernel/git/mark/boot-wrapper-aarch64.git cleanup

Thanks,
Mark.

Mark Rutland (5):
  Remove unused Set/Way cache helpers
  aarch32: simplify _switch_monitor
  GICv3: initialize without RMW
  Rename kernel *_RESET values to *_KERNEL
  Rename `CNTFRQ` -> `COUNTER_FREQ`

 Makefile.am                       |  4 ++--
 arch/aarch32/boot.S               | 25 +++++++------------------
 arch/aarch32/include/asm/cpu.h    | 30 ++----------------------------
 arch/aarch32/include/asm/gic-v3.h |  7 -------
 arch/aarch64/boot.S               |  6 +++---
 arch/aarch64/include/asm/cpu.h    | 32 +++-----------------------------
 arch/aarch64/include/asm/gic-v3.h |  7 -------
 common/gic-v3.c                   |  8 +++-----
 8 files changed, 20 insertions(+), 99 deletions(-)

-- 
2.11.0


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

[boot-wrapper PATCH 1/5] Remove unused Set/Way cache helpers

[Mark Rutland]

We removed the Set/Way cache maintenance in commit:

  864182b26c20a39d ("Remove cache maintenance")

... but forgot to remove the arch helpers which are now unused.

Remove the unused helpers.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
---
 arch/aarch32/include/asm/cpu.h | 26 --------------------------
 arch/aarch64/include/asm/cpu.h | 26 --------------------------
 2 files changed, 52 deletions(-)

diff --git a/arch/aarch32/include/asm/cpu.h b/arch/aarch32/include/asm/cpu.h
index 878be67..a7993f0 100644
--- a/arch/aarch32/include/asm/cpu.h
+++ b/arch/aarch32/include/asm/cpu.h
@@ -56,32 +56,6 @@ static inline uint32_t read_id_pfr1(void)
 	return val;
 }
 
-static inline uint32_t read_clidr(void)
-{
-	uint32_t val;
-
-	asm volatile ("mrc	p15, 1, %0, c0, c0, 1" : "=r" (val));
-	return val;
-}
-
-static inline uint32_t read_ccsidr(void)
-{
-	uint32_t val;
-
-	asm volatile ("mrc	p15, 1, %0, c0, c0, 0" : "=r" (val));
-	return val;
-}
-
-static inline void write_csselr(uint32_t val)
-{
-	asm volatile ("mcr	p15, 2, %0, c0, c0, 0" : : "r" (val));
-}
-
-static inline void dccisw(uint32_t val)
-{
-	asm volatile ("mcr	p15, 0, %0, c7, c14, 2" : : "r" (val));
-}
-
 static inline void iciallu(void)
 {
 	uint32_t val = 0;
diff --git a/arch/aarch64/include/asm/cpu.h b/arch/aarch64/include/asm/cpu.h
index ccb5397..1cddbb8 100644
--- a/arch/aarch64/include/asm/cpu.h
+++ b/arch/aarch64/include/asm/cpu.h
@@ -66,32 +66,6 @@ static inline uint64_t read_id_aa64pfr0(void)
 	return val;
 }
 
-static inline uint32_t read_clidr(void)
-{
-	uint32_t val;
-
-	asm volatile ("mrs	%0, clidr_el1" : "=r" (val));
-	return val;
-}
-
-static inline uint32_t read_ccsidr(void)
-{
-	uint32_t val;
-
-	asm volatile ("mrs	%0, ccsidr_el1" : "=r" (val));
-	return val;
-}
-
-static inline void write_csselr(uint32_t val)
-{
-	asm volatile ("msr	csselr_el1, %0" : : "r" (val));
-}
-
-static inline void dccisw(uint32_t val)
-{
-	asm volatile ("dc	cisw, %0" : : "r" (val));
-}
-
 static inline void iciallu(void)
 {
 	asm volatile ("ic	iallu");
-- 
2.11.0


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

[boot-wrapper PATCH 2/5] aarch32: simplify _switch_monitor

[Mark Rutland]

If we're lucky enough to have been booted into secure PL1, we can switch
to monitor mode with an exception return rather than an SMC call, which
avoids the need for boot-time vectors.

Note that while all Secure PL1 register state is accessible in Secure
SVC mode, we must switch to Monitor mode before we set SCR.NS=1.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
---
 arch/aarch32/boot.S | 19 ++++---------------
 1 file changed, 4 insertions(+), 15 deletions(-)

diff --git a/arch/aarch32/boot.S b/arch/aarch32/boot.S
index 82927de..e397cb0 100644
--- a/arch/aarch32/boot.S
+++ b/arch/aarch32/boot.S
@@ -37,9 +37,10 @@ ASM_FUNC(_start)
 	b	start_no_el3
 
 _switch_monitor:
-	ldr	r0, =boot_vectors
-	bl	setup_vector
-	smc	#0
+	adr	lr, _monitor
+	ldr	r0, =(PSR_A | PSR_I | PSR_F | PSR_MON)
+	msr	spsr, r0
+	movs	pc, lr
 
 _monitor:
 	/* Move the stack to Monitor mode*/
@@ -102,18 +103,6 @@ ASM_FUNC(jump_kernel)
 	msr	spsr_cxf, r4
 	movs	pc, lr
 
-	.section .vectors
-	.align 6
-boot_vectors:
-	b	.
-	b	.
-	b	_monitor
-	b	.
-	b	.
-	b	.
-	b	.
-	b	.
-
 	.section .data
 	.align 2
 flag_no_el3:
-- 
2.11.0


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

[boot-wrapper PATCH 3/5] GICv3: initialize without RMW

[Mark Rutland]

There's no need to perform an RMW sequence to initialize ICC_SRE_EL3, as
there are no bits that we need to preserve, and generally we should
reset registers to specific values such that RESx bits aren't configured
to UNKNOWN values that could be problematic in future architecture
versions.

Instead, let's initialize ICC_SRE_EL3 with a constant value. Since the
`DIB` and `DFB` fields are RAO/WI in some configurations and we have no
reason to initialize these to 0, we always initialize these to 1, in
addition to `SRE` and `SRE_Enable`.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Alexandru Elisei <alexandru.elisei@arm.com>
Cc: Andre Przywara <andre.przywara@arm.com>
---
 arch/aarch32/include/asm/gic-v3.h | 7 -------
 arch/aarch64/include/asm/gic-v3.h | 7 -------
 common/gic-v3.c                   | 8 +++-----
 3 files changed, 3 insertions(+), 19 deletions(-)

diff --git a/arch/aarch32/include/asm/gic-v3.h b/arch/aarch32/include/asm/gic-v3.h
index ec9a327..65f38de 100644
--- a/arch/aarch32/include/asm/gic-v3.h
+++ b/arch/aarch32/include/asm/gic-v3.h
@@ -9,13 +9,6 @@
 #ifndef __ASM_AARCH32_GICV3_H
 #define __ASM_AARCH32_GICV3_H
 
-static inline uint32_t gic_read_icc_sre(void)
-{
-	uint32_t val;
-	asm volatile ("mrc p15, 6, %0, c12, c12, 5" : "=r" (val));
-	return val;
-}
-
 static inline void gic_write_icc_sre(uint32_t val)
 {
 	asm volatile ("mcr p15, 6, %0, c12, c12, 5" : : "r" (val));
diff --git a/arch/aarch64/include/asm/gic-v3.h b/arch/aarch64/include/asm/gic-v3.h
index e743c02..5b32380 100644
--- a/arch/aarch64/include/asm/gic-v3.h
+++ b/arch/aarch64/include/asm/gic-v3.h
@@ -15,13 +15,6 @@
 #define ICC_CTLR_EL3	"S3_6_C12_C12_4"
 #define ICC_PMR_EL1	"S3_0_C4_C6_0"
 
-static inline uint32_t gic_read_icc_sre(void)
-{
-	uint32_t val;
-	asm volatile ("mrs %0, " ICC_SRE_EL3 : "=r" (val));
-	return val;
-}
-
 static inline void gic_write_icc_sre(uint32_t val)
 {
 	asm volatile ("msr " ICC_SRE_EL3 ", %0" : : "r" (val));
diff --git a/common/gic-v3.c b/common/gic-v3.c
index 62f9676..6207007 100644
--- a/common/gic-v3.c
+++ b/common/gic-v3.c
@@ -42,6 +42,8 @@
 #define GICR_TYPER_Last			(1 << 4)
 
 #define ICC_SRE_SRE			(1 << 0)
+#define ICC_SRE_DFB			(1 << 1)
+#define ICC_SRE_DIB			(1 << 2)
 #define ICC_SRE_Enable			(1 << 3)
 
 void gic_secure_init_primary(void)
@@ -101,8 +103,6 @@ void gic_secure_init_primary(void)
 
 void gic_secure_init(void)
 {
-	uint32_t sre;
-
 	/*
 	 * If GICv3 is not available, skip initialisation. The OS will probably
 	 * fail with a warning, but this should be easier to debug than a
@@ -114,9 +114,7 @@ void gic_secure_init(void)
 	if (this_cpu_logical_id() == 0)
 		gic_secure_init_primary();
 
-	sre = gic_read_icc_sre();
-	sre |= ICC_SRE_Enable | ICC_SRE_SRE;
-	gic_write_icc_sre(sre);
+	gic_write_icc_sre(ICC_SRE_Enable | ICC_SRE_DIB | ICC_SRE_DFB | ICC_SRE_SRE);
 	isb();
 
 	gic_write_icc_ctlr(0);
-- 
2.11.0


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

[boot-wrapper PATCH 4/5] Rename kernel *_RESET values to *_KERNEL

[Mark Rutland]

Our *_RESET constants are used to initalize state for the kernel rather
than the bootwrapper itself, so for clarity we should use a *_KERNEL
suffix rather than a _RESET suffix.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
---
 arch/aarch32/boot.S            | 4 ++--
 arch/aarch32/include/asm/cpu.h | 4 ++--
 arch/aarch64/boot.S            | 4 ++--
 arch/aarch64/include/asm/cpu.h | 6 +++---
 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/arch/aarch32/boot.S b/arch/aarch32/boot.S
index e397cb0..08bf932 100644
--- a/arch/aarch32/boot.S
+++ b/arch/aarch32/boot.S
@@ -76,10 +76,10 @@ ASM_FUNC(jump_kernel)
 	push	{r0 - r3}
 	mov	r5, sp
 
-	ldr	r0, =HSCTLR_RESET
+	ldr	r0, =HSCTLR_KERNEL
 	mcr	p15, 4, r0, c1, c0, 0		@ HSCTLR
 
-	ldr	r0, =SCTLR_RESET
+	ldr	r0, =SCTLR_KERNEL
 	mcr	p15, 0, r0, c1, c0, 0		@ SCTLR
 
 	/* Reset our stack pointer */
diff --git a/arch/aarch32/include/asm/cpu.h b/arch/aarch32/include/asm/cpu.h
index a7993f0..105cae5 100644
--- a/arch/aarch32/include/asm/cpu.h
+++ b/arch/aarch32/include/asm/cpu.h
@@ -13,8 +13,8 @@
 #define MPIDR_INVALID		(-1)
 
 /* Only RES1 bits and CP15 barriers for the kernel */
-#define HSCTLR_RESET		(3 << 28 | 3 << 22 | 1 << 18 | 1 << 16 | 1 << 11 | 3 << 4)
-#define SCTLR_RESET		(3 << 22 | 1 << 11 | 1 << 5 | 3 << 4)
+#define HSCTLR_KERNEL		(3 << 28 | 3 << 22 | 1 << 18 | 1 << 16 | 1 << 11 | 3 << 4)
+#define SCTLR_KERNEL		(3 << 22 | 1 << 11 | 1 << 5 | 3 << 4)
 
 #define PSR_SVC			0x13
 #define PSR_HYP			0x1a
diff --git a/arch/aarch64/boot.S b/arch/aarch64/boot.S
index d8d7ccd..587a25f 100644
--- a/arch/aarch64/boot.S
+++ b/arch/aarch64/boot.S
@@ -140,10 +140,10 @@ ASM_FUNC(jump_kernel)
 	mov	x22, x3
 	mov	x23, x4
 
-	ldr	x0, =SCTLR_EL1_RESET
+	ldr	x0, =SCTLR_EL1_KERNEL
 	msr	sctlr_el1, x0
 
-	ldr	x0, =SCTLR_EL2_RESET
+	ldr	x0, =SCTLR_EL2_KERNEL
 	msr	sctlr_el2, x0
 
 	cpuid	x0, x1
diff --git a/arch/aarch64/include/asm/cpu.h b/arch/aarch64/include/asm/cpu.h
index 1cddbb8..63eb1c3 100644
--- a/arch/aarch64/include/asm/cpu.h
+++ b/arch/aarch64/include/asm/cpu.h
@@ -17,7 +17,7 @@
  * RES1 bits,  little-endian, caches and MMU off, no alignment checking,
  * no WXN.
  */
-#define SCTLR_EL2_RESET		(3 << 28 | 3 << 22 | 1 << 18 | 1 << 16 | 1 << 11 | 3 << 4)
+#define SCTLR_EL2_KERNEL	(3 << 28 | 3 << 22 | 1 << 18 | 1 << 16 | 1 << 11 | 3 << 4)
 
 #define SPSR_A			(1 << 8)	/* System Error masked */
 #define SPSR_D			(1 << 9)	/* Debug masked */
@@ -37,10 +37,10 @@
 
 #ifdef KERNEL_32
 /* 32-bit kernel decompressor uses CP15 barriers */
-#define SCTLR_EL1_RESET		(SCTLR_EL1_RES1 | SCTLR_EL1_CP15BEN)
+#define SCTLR_EL1_KERNEL	(SCTLR_EL1_RES1 | SCTLR_EL1_CP15BEN)
 #define SPSR_KERNEL		(SPSR_A | SPSR_I | SPSR_F | SPSR_HYP)
 #else
-#define SCTLR_EL1_RESET		SCTLR_EL1_RES1
+#define SCTLR_EL1_KERNEL	SCTLR_EL1_RES1
 #define SPSR_KERNEL		(SPSR_A | SPSR_D | SPSR_I | SPSR_F | SPSR_EL2H)
 #endif
 
-- 
2.11.0


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

[boot-wrapper PATCH 5/5] Rename `CNTFRQ` -> `COUNTER_FREQ`

[Mark Rutland]

To avoid any confusuion between the CNTFRQ/CNTFRQ_EL0 register and the
vallue it will be progrmamed with, rename the `CNTFRQ` constant to
`COUNTER_FREQ.

In future patches this will allow us to use `CNTFRQ` as a macro for the
AArch32 CP15 register encoding.

There should be no functional change as a result of this patch.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
---
 Makefile.am         | 4 ++--
 arch/aarch32/boot.S | 2 +-
 arch/aarch64/boot.S | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 3591d23..d0a68df 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -13,12 +13,12 @@ SCRIPT_DIR	:= $(top_srcdir)/scripts
 PHYS_OFFSET	:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/findmem.pl $(KERNEL_DTB))
 UART_BASE	:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/findbase.pl $(KERNEL_DTB) 0 'arm,pl011')
 SYSREGS_BASE	:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/findbase.pl $(KERNEL_DTB) 0 'arm,vexpress-sysreg' 2> /dev/null)
-CNTFRQ		:= 24000000
+COUNTER_FREQ	:= 24000000
 
 CPU_IDS		:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/findcpuids.pl $(KERNEL_DTB))
 NR_CPUS         := $(shell echo $(CPU_IDS) | tr ',' ' ' | wc -w)
 
-DEFINES		= -DCNTFRQ=$(CNTFRQ)
+DEFINES		= -DCOUNTER_FREQ=$(COUNTER_FREQ)
 DEFINES		+= -DCPU_IDS=$(CPU_IDS)
 DEFINES		+= -DNR_CPUS=$(NR_CPUS)
 DEFINES		+= $(if $(SYSREGS_BASE), -DSYSREGS_BASE=$(SYSREGS_BASE), )
diff --git a/arch/aarch32/boot.S b/arch/aarch32/boot.S
index 08bf932..4add338 100644
--- a/arch/aarch32/boot.S
+++ b/arch/aarch32/boot.S
@@ -54,7 +54,7 @@ _monitor:
 	mov	r0, #(1 << 10 | 1 << 11)	@ Enable NS access to CPACR
 	mcr	p15, 0, r0, c1, c1, 2		@ NSACR
 
-	ldr	r0, =CNTFRQ
+	ldr	r0, =COUNTER_FREQ
 	mcr	p15, 0, r0, c14, c0, 0		@ CNTFRQ
 
 	bl	gic_secure_init
diff --git a/arch/aarch64/boot.S b/arch/aarch64/boot.S
index 587a25f..bfbb6ec 100644
--- a/arch/aarch64/boot.S
+++ b/arch/aarch64/boot.S
@@ -118,7 +118,7 @@ ASM_FUNC(_start)
 	msr	ZCR_EL3, x0			// for EL2.
 
 1:
-	ldr	x0, =CNTFRQ
+	ldr	x0, =COUNTER_FREQ
 	msr	cntfrq_el0, x0
 
 	bl	gic_secure_init
-- 
2.11.0


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

Re: [boot-wrapper PATCH 1/5] Remove unused Set/Way cache helpers

[Andre Przywara]

On 8/24/21 2:48 PM, Mark Rutland wrote:
> We removed the Set/Way cache maintenance in commit:
> 
>    864182b26c20a39d ("Remove cache maintenance")
> 
> ... but forgot to remove the arch helpers which are now unused.
> 
> Remove the unused helpers.

Indeed, no users in the tree.

> Signed-off-by: Mark Rutland <mark.rutland@arm.com>

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

Thanks,
Andre

> ---
>   arch/aarch32/include/asm/cpu.h | 26 --------------------------
>   arch/aarch64/include/asm/cpu.h | 26 --------------------------
>   2 files changed, 52 deletions(-)
> 
> diff --git a/arch/aarch32/include/asm/cpu.h b/arch/aarch32/include/asm/cpu.h
> index 878be67..a7993f0 100644
> --- a/arch/aarch32/include/asm/cpu.h
> +++ b/arch/aarch32/include/asm/cpu.h
> @@ -56,32 +56,6 @@ static inline uint32_t read_id_pfr1(void)
>   	return val;
>   }
>   
> -static inline uint32_t read_clidr(void)
> -{
> -	uint32_t val;
> -
> -	asm volatile ("mrc	p15, 1, %0, c0, c0, 1" : "=r" (val));
> -	return val;
> -}
> -
> -static inline uint32_t read_ccsidr(void)
> -{
> -	uint32_t val;
> -
> -	asm volatile ("mrc	p15, 1, %0, c0, c0, 0" : "=r" (val));
> -	return val;
> -}
> -
> -static inline void write_csselr(uint32_t val)
> -{
> -	asm volatile ("mcr	p15, 2, %0, c0, c0, 0" : : "r" (val));
> -}
> -
> -static inline void dccisw(uint32_t val)
> -{
> -	asm volatile ("mcr	p15, 0, %0, c7, c14, 2" : : "r" (val));
> -}
> -
>   static inline void iciallu(void)
>   {
>   	uint32_t val = 0;
> diff --git a/arch/aarch64/include/asm/cpu.h b/arch/aarch64/include/asm/cpu.h
> index ccb5397..1cddbb8 100644
> --- a/arch/aarch64/include/asm/cpu.h
> +++ b/arch/aarch64/include/asm/cpu.h
> @@ -66,32 +66,6 @@ static inline uint64_t read_id_aa64pfr0(void)
>   	return val;
>   }
>   
> -static inline uint32_t read_clidr(void)
> -{
> -	uint32_t val;
> -
> -	asm volatile ("mrs	%0, clidr_el1" : "=r" (val));
> -	return val;
> -}
> -
> -static inline uint32_t read_ccsidr(void)
> -{
> -	uint32_t val;
> -
> -	asm volatile ("mrs	%0, ccsidr_el1" : "=r" (val));
> -	return val;
> -}
> -
> -static inline void write_csselr(uint32_t val)
> -{
> -	asm volatile ("msr	csselr_el1, %0" : : "r" (val));
> -}
> -
> -static inline void dccisw(uint32_t val)
> -{
> -	asm volatile ("dc	cisw, %0" : : "r" (val));
> -}
> -
>   static inline void iciallu(void)
>   {
>   	asm volatile ("ic	iallu");
> 


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

Re: [boot-wrapper PATCH 2/5] aarch32: simplify _switch_monitor

[Andre Przywara]

On 8/24/21 2:48 PM, Mark Rutland wrote:
> If we're lucky enough to have been booted into secure PL1, we can switch
> to monitor mode with an exception return rather than an SMC call, which
> avoids the need for boot-time vectors.
> 
> Note that while all Secure PL1 register state is accessible in Secure
> SVC mode, we must switch to Monitor mode before we set SCR.NS=1.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>

I don't know much about the subtle differences between smc, movs pc and 
eret, but from what I read this is indeed an easier way to get to MON.

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

Cheers,
Andre

> ---
>   arch/aarch32/boot.S | 19 ++++---------------
>   1 file changed, 4 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/aarch32/boot.S b/arch/aarch32/boot.S
> index 82927de..e397cb0 100644
> --- a/arch/aarch32/boot.S
> +++ b/arch/aarch32/boot.S
> @@ -37,9 +37,10 @@ ASM_FUNC(_start)
>   	b	start_no_el3
>   
>   _switch_monitor:
> -	ldr	r0, =boot_vectors
> -	bl	setup_vector
> -	smc	#0
> +	adr	lr, _monitor
> +	ldr	r0, =(PSR_A | PSR_I | PSR_F | PSR_MON)
> +	msr	spsr, r0
> +	movs	pc, lr
>   
>   _monitor:
>   	/* Move the stack to Monitor mode*/
> @@ -102,18 +103,6 @@ ASM_FUNC(jump_kernel)
>   	msr	spsr_cxf, r4
>   	movs	pc, lr
>   
> -	.section .vectors
> -	.align 6
> -boot_vectors:
> -	b	.
> -	b	.
> -	b	_monitor
> -	b	.
> -	b	.
> -	b	.
> -	b	.
> -	b	.
> -
>   	.section .data
>   	.align 2
>   flag_no_el3:
> 


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

Re: [boot-wrapper PATCH 3/5] GICv3: initialize without RMW

[Andre Przywara]

On 8/24/21 2:48 PM, Mark Rutland wrote:
> There's no need to perform an RMW sequence to initialize ICC_SRE_EL3, as
> there are no bits that we need to preserve, and generally we should
> reset registers to specific values such that RESx bits aren't configured
> to UNKNOWN values that could be problematic in future architecture
> versions.
> 
> Instead, let's initialize ICC_SRE_EL3 with a constant value. Since the
> `DIB` and `DFB` fields are RAO/WI in some configurations and we have no
> reason to initialize these to 0, we always initialize these to 1, in
> addition to `SRE` and `SRE_Enable`.

Indeed, actually the architectural reset value is 0 (bypass enabled), 
and we seem to be just saved by the fact that the model implements them 
as RAO/WI. So forcing a value is the right thing to do.

> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Alexandru Elisei <alexandru.elisei@arm.com>
> Cc: Andre Przywara <andre.przywara@arm.com>

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

Cheers,
Andre

> ---
>   arch/aarch32/include/asm/gic-v3.h | 7 -------
>   arch/aarch64/include/asm/gic-v3.h | 7 -------
>   common/gic-v3.c                   | 8 +++-----
>   3 files changed, 3 insertions(+), 19 deletions(-)
> 
> diff --git a/arch/aarch32/include/asm/gic-v3.h b/arch/aarch32/include/asm/gic-v3.h
> index ec9a327..65f38de 100644
> --- a/arch/aarch32/include/asm/gic-v3.h
> +++ b/arch/aarch32/include/asm/gic-v3.h
> @@ -9,13 +9,6 @@
>   #ifndef __ASM_AARCH32_GICV3_H
>   #define __ASM_AARCH32_GICV3_H
>   
> -static inline uint32_t gic_read_icc_sre(void)
> -{
> -	uint32_t val;
> -	asm volatile ("mrc p15, 6, %0, c12, c12, 5" : "=r" (val));
> -	return val;
> -}
> -
>   static inline void gic_write_icc_sre(uint32_t val)
>   {
>   	asm volatile ("mcr p15, 6, %0, c12, c12, 5" : : "r" (val));
> diff --git a/arch/aarch64/include/asm/gic-v3.h b/arch/aarch64/include/asm/gic-v3.h
> index e743c02..5b32380 100644
> --- a/arch/aarch64/include/asm/gic-v3.h
> +++ b/arch/aarch64/include/asm/gic-v3.h
> @@ -15,13 +15,6 @@
>   #define ICC_CTLR_EL3	"S3_6_C12_C12_4"
>   #define ICC_PMR_EL1	"S3_0_C4_C6_0"
>   
> -static inline uint32_t gic_read_icc_sre(void)
> -{
> -	uint32_t val;
> -	asm volatile ("mrs %0, " ICC_SRE_EL3 : "=r" (val));
> -	return val;
> -}
> -
>   static inline void gic_write_icc_sre(uint32_t val)
>   {
>   	asm volatile ("msr " ICC_SRE_EL3 ", %0" : : "r" (val));
> diff --git a/common/gic-v3.c b/common/gic-v3.c
> index 62f9676..6207007 100644
> --- a/common/gic-v3.c
> +++ b/common/gic-v3.c
> @@ -42,6 +42,8 @@
>   #define GICR_TYPER_Last			(1 << 4)
>   
>   #define ICC_SRE_SRE			(1 << 0)
> +#define ICC_SRE_DFB			(1 << 1)
> +#define ICC_SRE_DIB			(1 << 2)
>   #define ICC_SRE_Enable			(1 << 3)
>   
>   void gic_secure_init_primary(void)
> @@ -101,8 +103,6 @@ void gic_secure_init_primary(void)
>   
>   void gic_secure_init(void)
>   {
> -	uint32_t sre;
> -
>   	/*
>   	 * If GICv3 is not available, skip initialisation. The OS will probably
>   	 * fail with a warning, but this should be easier to debug than a
> @@ -114,9 +114,7 @@ void gic_secure_init(void)
>   	if (this_cpu_logical_id() == 0)
>   		gic_secure_init_primary();
>   
> -	sre = gic_read_icc_sre();
> -	sre |= ICC_SRE_Enable | ICC_SRE_SRE;
> -	gic_write_icc_sre(sre);
> +	gic_write_icc_sre(ICC_SRE_Enable | ICC_SRE_DIB | ICC_SRE_DFB | ICC_SRE_SRE);
>   	isb();
>   
>   	gic_write_icc_ctlr(0);
> 


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

Re: [boot-wrapper PATCH 4/5] Rename kernel *_RESET values to *_KERNEL

[Andre Przywara]

On 8/24/21 2:48 PM, Mark Rutland wrote:
> Our *_RESET constants are used to initalize state for the kernel rather
> than the bootwrapper itself, so for clarity we should use a *_KERNEL
> suffix rather than a _RESET suffix.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>

It's indeed only renaming:

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

Cheers,
Andre

> ---
>   arch/aarch32/boot.S            | 4 ++--
>   arch/aarch32/include/asm/cpu.h | 4 ++--
>   arch/aarch64/boot.S            | 4 ++--
>   arch/aarch64/include/asm/cpu.h | 6 +++---
>   4 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/aarch32/boot.S b/arch/aarch32/boot.S
> index e397cb0..08bf932 100644
> --- a/arch/aarch32/boot.S
> +++ b/arch/aarch32/boot.S
> @@ -76,10 +76,10 @@ ASM_FUNC(jump_kernel)
>   	push	{r0 - r3}
>   	mov	r5, sp
>   
> -	ldr	r0, =HSCTLR_RESET
> +	ldr	r0, =HSCTLR_KERNEL
>   	mcr	p15, 4, r0, c1, c0, 0		@ HSCTLR
>   
> -	ldr	r0, =SCTLR_RESET
> +	ldr	r0, =SCTLR_KERNEL
>   	mcr	p15, 0, r0, c1, c0, 0		@ SCTLR
>   
>   	/* Reset our stack pointer */
> diff --git a/arch/aarch32/include/asm/cpu.h b/arch/aarch32/include/asm/cpu.h
> index a7993f0..105cae5 100644
> --- a/arch/aarch32/include/asm/cpu.h
> +++ b/arch/aarch32/include/asm/cpu.h
> @@ -13,8 +13,8 @@
>   #define MPIDR_INVALID		(-1)
>   
>   /* Only RES1 bits and CP15 barriers for the kernel */
> -#define HSCTLR_RESET		(3 << 28 | 3 << 22 | 1 << 18 | 1 << 16 | 1 << 11 | 3 << 4)
> -#define SCTLR_RESET		(3 << 22 | 1 << 11 | 1 << 5 | 3 << 4)
> +#define HSCTLR_KERNEL		(3 << 28 | 3 << 22 | 1 << 18 | 1 << 16 | 1 << 11 | 3 << 4)
> +#define SCTLR_KERNEL		(3 << 22 | 1 << 11 | 1 << 5 | 3 << 4)
>   
>   #define PSR_SVC			0x13
>   #define PSR_HYP			0x1a
> diff --git a/arch/aarch64/boot.S b/arch/aarch64/boot.S
> index d8d7ccd..587a25f 100644
> --- a/arch/aarch64/boot.S
> +++ b/arch/aarch64/boot.S
> @@ -140,10 +140,10 @@ ASM_FUNC(jump_kernel)
>   	mov	x22, x3
>   	mov	x23, x4
>   
> -	ldr	x0, =SCTLR_EL1_RESET
> +	ldr	x0, =SCTLR_EL1_KERNEL
>   	msr	sctlr_el1, x0
>   
> -	ldr	x0, =SCTLR_EL2_RESET
> +	ldr	x0, =SCTLR_EL2_KERNEL
>   	msr	sctlr_el2, x0
>   
>   	cpuid	x0, x1
> diff --git a/arch/aarch64/include/asm/cpu.h b/arch/aarch64/include/asm/cpu.h
> index 1cddbb8..63eb1c3 100644
> --- a/arch/aarch64/include/asm/cpu.h
> +++ b/arch/aarch64/include/asm/cpu.h
> @@ -17,7 +17,7 @@
>    * RES1 bits,  little-endian, caches and MMU off, no alignment checking,
>    * no WXN.
>    */
> -#define SCTLR_EL2_RESET		(3 << 28 | 3 << 22 | 1 << 18 | 1 << 16 | 1 << 11 | 3 << 4)
> +#define SCTLR_EL2_KERNEL	(3 << 28 | 3 << 22 | 1 << 18 | 1 << 16 | 1 << 11 | 3 << 4)
>   
>   #define SPSR_A			(1 << 8)	/* System Error masked */
>   #define SPSR_D			(1 << 9)	/* Debug masked */
> @@ -37,10 +37,10 @@
>   
>   #ifdef KERNEL_32
>   /* 32-bit kernel decompressor uses CP15 barriers */
> -#define SCTLR_EL1_RESET		(SCTLR_EL1_RES1 | SCTLR_EL1_CP15BEN)
> +#define SCTLR_EL1_KERNEL	(SCTLR_EL1_RES1 | SCTLR_EL1_CP15BEN)
>   #define SPSR_KERNEL		(SPSR_A | SPSR_I | SPSR_F | SPSR_HYP)
>   #else
> -#define SCTLR_EL1_RESET		SCTLR_EL1_RES1
> +#define SCTLR_EL1_KERNEL	SCTLR_EL1_RES1
>   #define SPSR_KERNEL		(SPSR_A | SPSR_D | SPSR_I | SPSR_F | SPSR_EL2H)
>   #endif
>   
> 


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

Re: [boot-wrapper PATCH 5/5] Rename `CNTFRQ` -> `COUNTER_FREQ`

[Andre Przywara]

On 8/24/21 2:49 PM, Mark Rutland wrote:
> To avoid any confusuion between the CNTFRQ/CNTFRQ_EL0 register and the
> vallue it will be progrmamed with, rename the `CNTFRQ` constant to
> `COUNTER_FREQ.
> 
> In future patches this will allow us to use `CNTFRQ` as a macro for the
> AArch32 CP15 register encoding.
> 
> There should be no functional change as a result of this patch.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>

Confirmed to be just renaming:

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

Cheers,
Andre

> ---
>   Makefile.am         | 4 ++--
>   arch/aarch32/boot.S | 2 +-
>   arch/aarch64/boot.S | 2 +-
>   3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index 3591d23..d0a68df 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -13,12 +13,12 @@ SCRIPT_DIR	:= $(top_srcdir)/scripts
>   PHYS_OFFSET	:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/findmem.pl $(KERNEL_DTB))
>   UART_BASE	:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/findbase.pl $(KERNEL_DTB) 0 'arm,pl011')
>   SYSREGS_BASE	:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/findbase.pl $(KERNEL_DTB) 0 'arm,vexpress-sysreg' 2> /dev/null)
> -CNTFRQ		:= 24000000
> +COUNTER_FREQ	:= 24000000
>   
>   CPU_IDS		:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/findcpuids.pl $(KERNEL_DTB))
>   NR_CPUS         := $(shell echo $(CPU_IDS) | tr ',' ' ' | wc -w)
>   
> -DEFINES		= -DCNTFRQ=$(CNTFRQ)
> +DEFINES		= -DCOUNTER_FREQ=$(COUNTER_FREQ)
>   DEFINES		+= -DCPU_IDS=$(CPU_IDS)
>   DEFINES		+= -DNR_CPUS=$(NR_CPUS)
>   DEFINES		+= $(if $(SYSREGS_BASE), -DSYSREGS_BASE=$(SYSREGS_BASE), )
> diff --git a/arch/aarch32/boot.S b/arch/aarch32/boot.S
> index 08bf932..4add338 100644
> --- a/arch/aarch32/boot.S
> +++ b/arch/aarch32/boot.S
> @@ -54,7 +54,7 @@ _monitor:
>   	mov	r0, #(1 << 10 | 1 << 11)	@ Enable NS access to CPACR
>   	mcr	p15, 0, r0, c1, c1, 2		@ NSACR
>   
> -	ldr	r0, =CNTFRQ
> +	ldr	r0, =COUNTER_FREQ
>   	mcr	p15, 0, r0, c14, c0, 0		@ CNTFRQ
>   
>   	bl	gic_secure_init
> diff --git a/arch/aarch64/boot.S b/arch/aarch64/boot.S
> index 587a25f..bfbb6ec 100644
> --- a/arch/aarch64/boot.S
> +++ b/arch/aarch64/boot.S
> @@ -118,7 +118,7 @@ ASM_FUNC(_start)
>   	msr	ZCR_EL3, x0			// for EL2.
>   
>   1:
> -	ldr	x0, =CNTFRQ
> +	ldr	x0, =COUNTER_FREQ
>   	msr	cntfrq_el0, x0
>   
>   	bl	gic_secure_init
> 


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

Re: [boot-wrapper PATCH 0/5] Misc cleanups

[Marc Zyngier]

On Tue, 24 Aug 2021 14:48:55 +0100,
Mark Rutland <mark.rutland@arm.com> wrote:
> 
> These are a few cleanup patches I've split out of my ongoing bootwrapper
> rework, and intend to apply by the end of this week.
> 
> My WIP cleanup branch with subsequent patches can be found at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/mark/boot-wrapper-aarch64.git cleanup
> 
> Thanks,
> Mark.
> 
> Mark Rutland (5):
>   Remove unused Set/Way cache helpers
>   aarch32: simplify _switch_monitor
>   GICv3: initialize without RMW
>   Rename kernel *_RESET values to *_KERNEL
>   Rename `CNTFRQ` -> `COUNTER_FREQ`
> 
>  Makefile.am                       |  4 ++--
>  arch/aarch32/boot.S               | 25 +++++++------------------
>  arch/aarch32/include/asm/cpu.h    | 30 ++----------------------------
>  arch/aarch32/include/asm/gic-v3.h |  7 -------
>  arch/aarch64/boot.S               |  6 +++---
>  arch/aarch64/include/asm/cpu.h    | 32 +++-----------------------------
>  arch/aarch64/include/asm/gic-v3.h |  7 -------
>  common/gic-v3.c                   |  8 +++-----
>  8 files changed, 20 insertions(+), 99 deletions(-)

For the whole series:

Acked-by: Marc Zyngier <maz@kernel.org>

	M.

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

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