* [PATCH v2][Newlib] arm: Restrict processor mode change when in hypervisor mode.
@ 2023-02-23 17:10 Srinath Parvathaneni
2023-03-02 13:05 ` Richard Earnshaw
0 siblings, 1 reply; 2+ messages in thread
From: Srinath Parvathaneni @ 2023-02-23 17:10 UTC (permalink / raw)
To: newlib; +Cc: Richard Earnshaw, nd
[-- Attachment #1: Type: text/plain, Size: 987 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 when the current processor
mode is hypervisor mode, changing to any other mode using CPSR will result in an
illegal instruction as per Arm-arm and simulator throws undefined instruction
exception.
This patch prevent the change of hypervisor mode to any other mode in _stack_init
function in crt0.S files.
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-02-23 Srinath Parvathaneni <srinath.parvathaneni@arm.com>
* arm/crt0.S (_stack_init): Add check for hypervisor mode.
newlib/ChangeLog:
2023-02-23 Srinath Parvathaneni <srinath.parvathaneni@arm.com>
* libc/sys/arm/crt0.S (_stack_init): Add check for hypervisor mode.
[-- Attachment #2: newlib.diff --]
[-- Type: text/plain, Size: 3378 bytes --]
diff --git a/libgloss/arm/crt0.S b/libgloss/arm/crt0.S
index 78515180bf06f1da37669e4c7e6608c76e1b096d..9d1b649864b1c920b2c1bab7b6b069d52d708c53 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
@@ -149,12 +149,11 @@
#if (__ARM_ARCH_PROFILE != 'M')
/* Following code is compatible for both ARM and Thumb ISA. */
mrs r4, CPSR
- /* Test mode bits - in User of all are 0. */
- tst r4, #(CPSR_M_MASK)
- /* "eq" means r4 AND #0x0F is 0. */
+ mov r3, sp
+ ands r1, r4, #(CPSR_M_MASK)
+ beq .Lskip_cpu_modes
+ cmp r1, #(CPSR_M_HYP)
beq .Lskip_cpu_modes
-
- mov r3, sp /* Save input SP value. */
/* FIQ mode, interrupts disabled. */
mov r1, #(CPSR_M_FIQ|CPSR_M_32BIT|CPSR_I_MASK|CPSR_F_MASK)
diff --git a/newlib/libc/sys/arm/crt0.S b/newlib/libc/sys/arm/crt0.S
index 6b01d8a88b77f44b1ba495aa1f69156f12749527..582cd789897b8a2239581618462047155b55d166 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
@@ -149,12 +149,11 @@
#if (__ARM_ARCH_PROFILE != 'M')
/* Following code is compatible for both ARM and Thumb ISA. */
mrs r4, CPSR
- /* Test mode bits - in User of all are 0. */
- tst r4, #(CPSR_M_MASK)
- /* "eq" means r4 AND #0x0F is 0. */
+ mov r3, sp
+ ands r1, r4, #(CPSR_M_MASK)
+ beq .Lskip_cpu_modes
+ cmp r1, #(CPSR_M_HYP)
beq .Lskip_cpu_modes
-
- mov r3, sp /* Save input SP value. */
/* FIQ mode, interrupts disabled. */
mov r1, #(CPSR_M_FIQ|CPSR_M_32BIT|CPSR_I_MASK|CPSR_F_MASK)
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH v2][Newlib] arm: Restrict processor mode change when in hypervisor mode.
2023-02-23 17:10 [PATCH v2][Newlib] arm: Restrict processor mode change when in hypervisor mode Srinath Parvathaneni
@ 2023-03-02 13:05 ` Richard Earnshaw
0 siblings, 0 replies; 2+ messages in thread
From: Richard Earnshaw @ 2023-03-02 13:05 UTC (permalink / raw)
To: Srinath Parvathaneni, newlib; +Cc: Richard Earnshaw, nd
On 23/02/2023 17:10, 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 when the current
> processor
> mode is hypervisor mode, changing to any other mode using CPSR will
> result in an
> illegal instruction as per Arm-arm and simulator throws undefined
> instruction
> exception.
The reference to a simulator here is confusing. You might have found
this on one, but it's a reflection of the way the architecture is
specified and not specific to running under simulation.
> This patch prevent the change of hypervisor mode to any other mode in
> _stack_init
> function in crt0.S files.
>
> 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-02-23 Srinath Parvathaneni <srinath.parvathaneni@arm.com>
>
> * arm/crt0.S (_stack_init): Add check for hypervisor mode.
>
> newlib/ChangeLog:
>
> 2023-02-23 Srinath Parvathaneni <srinath.parvathaneni@arm.com>
>
> * libc/sys/arm/crt0.S (_stack_init): Add check for hypervisor
> mode.
- /* Test mode bits - in User of all are 0. */
- tst r4, #(CPSR_M_MASK)
- /* "eq" means r4 AND #0x0F is 0. */
+ mov r3, sp
+ ands r1, r4, #(CPSR_M_MASK)
+ beq .Lskip_cpu_modes
+ cmp r1, #(CPSR_M_HYP)
You don't mention anywhere why you've moved the instruction that copies
SP into R3 before the user-mode check. I think it's probably right, but
I think really that's a separate issue that deserves a separate patch
(even though it's pretty trivial).
So please can you resend as two patches, the first fixing the SP->r3
copy and the second for the hypervisor mode issue.
R.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-03-02 13:05 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-23 17:10 [PATCH v2][Newlib] arm: Restrict processor mode change when in hypervisor mode Srinath Parvathaneni
2023-03-02 13:05 ` Richard Earnshaw
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).