[PATCH v3][Newlib] arm: Restrict processor mode change when in hypervisor mode.

[PATCH v3][Newlib] arm: Restrict processor mode change when in hypervisor mode.

[Srinath Parvathaneni]

[-- Attachment #1.1: Type: text/plain, Size: 1137 bytes --]

Hi All,

In _stack_init function of crt0.S file, when the current mode is not user mode,
all the processor modes are parsed and the corresponding stack limit are set for
these modes for all A-profile and R-profile CPU's. But if a CPU implements EL2
as its highest exception level, then the core (or a model of it) will start in that
mode. For example, with Cortex-R52 the processor starts by default is hypervisor
mode (EL2), changing to any other mode using CPSR will result in an illegal
instruction as per Arm-arm.
This patch prevent the change of Hypervisor mode to any other EL1 modes in
_stack_init function in crt0.S files and does not set the stack limit for those
EL1 modes.

Regression tested on arm-none-eabi target for newlib and newlib-nano and found
no regressions.

Ok for newlib master?

Regards,
Srinath.

libgloss/ChangeLog:

2023-03-03  Srinath Parvathaneni<srinath.parvathaneni@arm.com>

         * arm/crt0.S (_stack_init): Add check for hypervisor mode.

newlib/ChangeLog:

2023-03-03  Srinath Parvathaneni<srinath.parvathaneni@arm.com>

         * libc/sys/arm/crt0.S (_stack_init): Add check for hypervisor mode.

[-- Attachment #2: newlib.patch --]
[-- Type: text/plain, Size: 3156 bytes --]

diff --git a/libgloss/arm/crt0.S b/libgloss/arm/crt0.S
index 79ae1e2511783d6b834bd1ac278316ae04ea2ec8..cd1a20d61708cd91462db8435ddd6b7a45746f66 100644
--- a/libgloss/arm/crt0.S
+++ b/libgloss/arm/crt0.S
@@ -122,10 +122,10 @@
 *   +-----+ <- SP_svc         of getting in and out of secure state are not as
 *   |     |                   simple as writing to the CPSR mode bits.
 *   | IRQ | -= 0x2000       - Mode switch via CPSR is not allowed once in
-*   |     |                   non-privileged mode, so we take care not to enter
-* ^ +-----+ <- SP_und         "User" to set up its SP, and also skip most
-* s |     |                   operations if already in that mode.
-* t | UND | -= 0x1000
+*   |     |                   non-privileged mode or in hypervisor mode, so we
+* ^ +-----+ <- SP_und         take care not to enter "User" or "Hypervisor" mode
+* s |     |                   to set up its SP, and also skip most operations if
+* t | UND | -= 0x1000         already in these modes.
 * a |     |                Input parameters:
 * c +-----+ <- SP_und       - sp - Initialized SP
 * k |     |                 - r2 - May contain SL value from semihosting
@@ -150,9 +150,9 @@
 	/* Following code is compatible for both ARM and Thumb ISA.  */
 	mrs	r4, CPSR
 	mov	r3, sp /* Save input SP value.  */
-	/* Test mode bits - in User of all are 0.  */
-	tst	r4, #(CPSR_M_MASK)
-	/* "eq" means r4 AND #0x0F is 0.  */
+	ands	r1, r4, #(CPSR_M_MASK)
+	beq	.Lskip_cpu_modes
+	cmp	r1, #(CPSR_M_HYP)
 	beq	.Lskip_cpu_modes
 
 	/* FIQ mode, interrupts disabled.  */
diff --git a/newlib/libc/sys/arm/crt0.S b/newlib/libc/sys/arm/crt0.S
index 121246cfaf1ea197c271e434fe0c05a3aba8fb6a..90d5be393d02ede7c5aa505b6b9c6060ed74b40a 100644
--- a/newlib/libc/sys/arm/crt0.S
+++ b/newlib/libc/sys/arm/crt0.S
@@ -122,10 +122,10 @@
 *   +-----+ <- SP_svc         of getting in and out of secure state are not as
 *   |     |                   simple as writing to the CPSR mode bits.
 *   | IRQ | -= 0x2000       - Mode switch via CPSR is not allowed once in
-*   |     |                   non-privileged mode, so we take care not to enter
-* ^ +-----+ <- SP_und         "User" to set up its SP, and also skip most
-* s |     |                   operations if already in that mode.
-* t | UND | -= 0x1000
+*   |     |                   non-privileged mode or in hypervisor mode, so we
+* ^ +-----+ <- SP_und         take care not to enter "User" or "Hypervisor" mode
+* s |     |                   to set up its SP, and also skip most operations if
+* t | UND | -= 0x1000         already in these modes.
 * a |     |                Input parameters:
 * c +-----+ <- SP_und       - sp - Initialized SP
 * k |     |                 - r2 - May contain SL value from semihosting
@@ -150,9 +150,9 @@
 	/* Following code is compatible for both ARM and Thumb ISA.  */
 	mrs	r4, CPSR
 	mov	r3, sp /* Save input SP value.  */
-	/* Test mode bits - in User of all are 0.  */
-	tst	r4, #(CPSR_M_MASK)
-	/* "eq" means r4 AND #0x0F is 0.  */
+	ands	r1, r4, #(CPSR_M_MASK)
+	beq	.Lskip_cpu_modes
+	cmp	r1, #(CPSR_M_HYP)
 	beq	.Lskip_cpu_modes
 
 	/* FIQ mode, interrupts disabled.  */

Re: [PATCH v3][Newlib] arm: Restrict processor mode change when in hypervisor mode.

[Richard Earnshaw]

Pushed (with tweaks to the commit message).

R.

On 03/03/2023 13:12, Srinath Parvathaneni wrote:
> Hi All,
> 
> In _stack_init function of crt0.S file, when the current mode is not 
> user mode,
> all the processor modes are parsed and the corresponding stack limit are 
> set for
> these modes for all A-profile and R-profile CPU's. But if a CPU 
> implements EL2
> as its highest exception level, then the core (or a model of it) will 
> start in that
> mode. For example, with Cortex-R52 the processor starts by default is 
> hypervisor
> mode (EL2), changing to any other mode using CPSR will result in an illegal
> instruction as per Arm-arm.
> This patch prevent the change of Hypervisor mode to any other EL1 modes in
> _stack_init function in crt0.S files and does not set the stack limit 
> for those
> EL1 modes.
> 
> Regression tested on arm-none-eabi target for newlib and newlib-nano and 
> found
> no regressions.
> 
> Ok for newlib master?
> 
> Regards,
> Srinath.
> 
> libgloss/ChangeLog:
> 
> 2023-03-03  Srinath Parvathaneni<srinath.parvathaneni@arm.com>
> 
>          * arm/crt0.S (_stack_init): Add check for hypervisor mode.
> 
> newlib/ChangeLog:
> 
> 2023-03-03  Srinath Parvathaneni<srinath.parvathaneni@arm.com>
> 
>          * libc/sys/arm/crt0.S (_stack_init): Add check for hypervisor 
> mode.