Skip to contentSkip to conversation

Fix bug: SRS instructions would trap to EL3 in Secure EL1 even if specified mode was not monitor mode.

PATCH In review Ralf-Philipp Weinmann 3 participants 4 messages Mar 3, 2016 – Mar 7, 2016

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Fix bug: SRS instructions would trap to EL3 in Secure EL1 even if specified mode was not monitor mode.

According to the ARMv8 Architecture reference manual [F6.1.203], ALL of the following conditions need to be met for SRS to trap to EL3:
* It is executed at Secure PL1.
* The specified mode is monitor mode.
* EL3 is using AArch64.

---
 target-arm/translate.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
target-arm/translate.c +3 -2
index c29c47f..a7688bb 100644
@@ -7582,7 +7582,8 @@ static void gen_srs(DisasContext *s,
     bool undef = false;
 
     /* SRS is:
-     * - trapped to EL3 if EL3 is AArch64 and we are at Secure EL1
+     * - trapped to EL3 if EL3 is AArch64 and we are at Secure EL1 and 
+     *   mode is monitor mode
      * - UNDEFINED in Hyp mode
      * - UNPREDICTABLE in User or System mode
      * - UNPREDICTABLE if the specified mode is:
@@ -7592,7 +7593,7 @@ static void gen_srs(DisasContext *s,
      * -- Monitor, if we are Non-secure
      * For the UNPREDICTABLE cases we choose to UNDEF.
      */
-    if (s->current_el == 1 && !s->ns) {
+    if (s->current_el == 1 && !s->ns && mode == ARM_CPU_MODE_MON) {
         gen_exception_insn(s, 4, EXCP_UDEF, syn_uncategorized(), 3);
         return;
     }
Signature · 1 line
2.5.4 (Apple Git-61)
#1 lore raw
Quoted 5 lines · Ralf-Philipp Weinmann

According to the ARMv8 Architecture reference manual [F6.1.203], ALL of the following conditions need to be met for SRS to trap to EL3:
* It is executed at Secure PL1.
* The specified mode is monitor mode.
* EL3 is using AArch64.

The code changes in the patch looks good for me. But anyway, you should:
(1) tweak the commit message title according to the requirements [1] and
(2) add your "Singed-off-by:" line [2]

Actually, you'd better read the whole page [3] carefully.

[1] http://wiki.qemu.org/Contribute/SubmitAPatch#Write_a_meaningful_commit_message
[2] http://wiki.qemu.org/Contribute/SubmitAPatch#Patch_emails_must_include_a_Signed-off-by:_line
[3] http://wiki.qemu.org/Contribute/SubmitAPatch

Kind regards,
Sergey

Quoted 27 lines
---
  target-arm/translate.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)
target-arm/translate.c +3 -2
index c29c47f..a7688bb 100644
@@ -7582,7 +7582,8 @@ static void gen_srs(DisasContext *s,
      bool undef = false;
 
      /* SRS is:
-     * - trapped to EL3 if EL3 is AArch64 and we are at Secure EL1
+     * - trapped to EL3 if EL3 is AArch64 and we are at Secure EL1 and
+     *   mode is monitor mode
       * - UNDEFINED in Hyp mode
       * - UNPREDICTABLE in User or System mode
       * - UNPREDICTABLE if the specified mode is:
@@ -7592,7 +7593,7 @@ static void gen_srs(DisasContext *s,
       * -- Monitor, if we are Non-secure
       * For the UNPREDICTABLE cases we choose to UNDEF.
       */
-    if (s->current_el == 1 && !s->ns) {
+    if (s->current_el == 1 && !s->ns && mode == ARM_CPU_MODE_MON) {
          gen_exception_insn(s, 4, EXCP_UDEF, syn_uncategorized(), 3);
          return;
      }
#2 lore raw
Quoted 9 lines · Sergey Fedorov
Quoted 6 lines · Ralf-Philipp Weinmann

According to the ARMv8 Architecture reference manual [F6.1.203], ALL of the following conditions need to be met for SRS to trap to EL3:
* It is executed at Secure PL1.
* The specified mode is monitor mode.
* EL3 is using AArch64.

Quoted 3 lines

The code changes in the patch looks good for me. But anyway, you should:
(1) tweak the commit message title according to the requirements [1] and
(2) add your "Singed-off-by:" line [2]

Ralf-Philipp sent a second version of this email which did have the signed-off-by line, so this patch is already in master (as commit ba63cf47a9304113); I did tweak the commit message as I applied it.

thanks
-- PMM

#3 lore raw
Quoted 8 lines · Peter Maydell
Quoted 7 lines · Sergey Fedorov
Quoted 5 lines · Ralf-Philipp Weinmann

According to the ARMv8 Architecture reference manual [F6.1.203], ALL of the following conditions need to be met for SRS to trap to EL3:
* It is executed at Secure PL1.
* The specified mode is monitor mode.
* EL3 is using AArch64.

Quoted 7 lines
Quoted 3 lines

The code changes in the patch looks good for me. But anyway, you should:
(1) tweak the commit message title according to the requirements [1] and
(2) add your "Singed-off-by:" line [2]

Ralf-Philipp sent a second version of this email which did have the signed-off-by line, so this patch is already in master (as commit ba63cf47a9304113); I did tweak the commit message as I applied it.

Really, not sure how I missed that :)

Regards,
Sergey