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 <serge.fdrv@gmail.com>]

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;
>       }