[Qemu-devel] [PATCH] Fix bug: SRS instructions would trap to EL3 in Secure EL1 even if specified mode was not monitor mode.

[Qemu-devel] [PATCH] Fix bug: SRS instructions would trap to EL3 in Secure EL1 even if specified mode was not monitor mode.

[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.
---
 target-arm/translate.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/target-arm/translate.c b/target-arm/translate.c
index c29c47f..a7688bb 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -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.5.4 (Apple Git-61)

Re: [Qemu-devel] [Qemu-arm] [PATCH] Fix bug: SRS instructions would trap to EL3 in Secure EL1 even if specified mode was not monitor mode.

[Sergey Fedorov]

On 23.02.2016 01:25, Ralf-Philipp Weinmann wrote:
> 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

> ---
>   target-arm/translate.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index c29c47f..a7688bb 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -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;
>       }

Re: [Qemu-devel] [Qemu-arm] [PATCH] Fix bug: SRS instructions would trap to EL3 in Secure EL1 even if specified mode was not monitor mode.

[Peter Maydell]

On 7 March 2016 at 02:04, Sergey Fedorov <serge.fdrv@gmail.com> wrote:
> On 23.02.2016 01:25, Ralf-Philipp Weinmann wrote:
>>
>> 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]

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

Re: [Qemu-devel] [Qemu-arm] [PATCH] Fix bug: SRS instructions would trap to EL3 in Secure EL1 even if specified mode was not monitor mode.

[Sergey Fedorov]

On 07.03.2016 00:14, Peter Maydell wrote:
> On 7 March 2016 at 02:04, Sergey Fedorov <serge.fdrv@gmail.com> wrote:
>> On 23.02.2016 01:25, Ralf-Philipp Weinmann wrote:
>>> 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]
> 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