[PATCH] target/arm: Use correct entrypoint for SVC taken from Hyp to Hyp

[PATCH] target/arm: Use correct entrypoint for SVC taken from Hyp to Hyp

[Peter Maydell]

The exception caused by an SVC instruction may be taken to AArch32
Hyp mode for two reasons:
 * HCR.TGE indicates that exceptions from EL0 should trap to EL2
 * we were already in Hyp mode

The entrypoint in the vector table to be used differs in these two
cases: for an exception routed to Hyp mode from EL0, we enter at the
common 0x14 "hyp trap" entrypoint.  For SVC from Hyp mode to Hyp
mode, we enter at the 0x08 (svc/hvc trap) entrypoint.
In the v8A Arm ARM pseudocode this is done in AArch32.TakeSVCException.

QEMU incorrectly routed both of these exceptions to the 0x14
entrypoint.  Correct the entrypoint for SVC from Hyp to Hyp by making
use of the existing logic which handles "normal entrypoint for
Hyp-to-Hyp, otherwise 0x14" for traps like UNDEF and data/prefetch
aborts (reproduced here since it's outside the visible context
in the diff for this commit):

    if (arm_current_el(env) != 2 && addr < 0x14) {
        addr = 0x14;
    }

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/helper.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index cfca0f5ba6d..a898e400f6c 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -9655,7 +9655,7 @@ static void arm_cpu_do_interrupt_aarch32_hyp(CPUState *cs)
      * separately here.
      *
      * The vector table entry used is always the 0x14 Hyp mode entry point,
-     * unless this is an UNDEF/HVC/abort taken from Hyp to Hyp.
+     * unless this is an UNDEF/SVC/HVC/abort taken from Hyp to Hyp.
      * The offset applied to the preferred return address is always zero
      * (see DDI0487C.a section G1.12.3).
      * PSTATE A/I/F masks are set based only on the SCR.EA/IRQ/FIQ values.
@@ -9669,7 +9669,7 @@ static void arm_cpu_do_interrupt_aarch32_hyp(CPUState *cs)
         addr = 0x04;
         break;
     case EXCP_SWI:
-        addr = 0x14;
+        addr = 0x08;
         break;
     case EXCP_BKPT:
         /* Fall through to prefetch abort.  */
-- 
2.25.1

Re: [PATCH] target/arm: Use correct entrypoint for SVC taken from Hyp to Hyp

[Peter Maydell]

Ping for code review? This is a nice short easy one :-)

thanks
-- PMM

On Mon, 17 Jan 2022 at 13:19, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> The exception caused by an SVC instruction may be taken to AArch32
> Hyp mode for two reasons:
>  * HCR.TGE indicates that exceptions from EL0 should trap to EL2
>  * we were already in Hyp mode
>
> The entrypoint in the vector table to be used differs in these two
> cases: for an exception routed to Hyp mode from EL0, we enter at the
> common 0x14 "hyp trap" entrypoint.  For SVC from Hyp mode to Hyp
> mode, we enter at the 0x08 (svc/hvc trap) entrypoint.
> In the v8A Arm ARM pseudocode this is done in AArch32.TakeSVCException.
>
> QEMU incorrectly routed both of these exceptions to the 0x14
> entrypoint.  Correct the entrypoint for SVC from Hyp to Hyp by making
> use of the existing logic which handles "normal entrypoint for
> Hyp-to-Hyp, otherwise 0x14" for traps like UNDEF and data/prefetch
> aborts (reproduced here since it's outside the visible context
> in the diff for this commit):
>
>     if (arm_current_el(env) != 2 && addr < 0x14) {
>         addr = 0x14;
>     }
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target/arm/helper.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index cfca0f5ba6d..a898e400f6c 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -9655,7 +9655,7 @@ static void arm_cpu_do_interrupt_aarch32_hyp(CPUState *cs)
>       * separately here.
>       *
>       * The vector table entry used is always the 0x14 Hyp mode entry point,
> -     * unless this is an UNDEF/HVC/abort taken from Hyp to Hyp.
> +     * unless this is an UNDEF/SVC/HVC/abort taken from Hyp to Hyp.
>       * The offset applied to the preferred return address is always zero
>       * (see DDI0487C.a section G1.12.3).
>       * PSTATE A/I/F masks are set based only on the SCR.EA/IRQ/FIQ values.
> @@ -9669,7 +9669,7 @@ static void arm_cpu_do_interrupt_aarch32_hyp(CPUState *cs)
>          addr = 0x04;
>          break;
>      case EXCP_SWI:
> -        addr = 0x14;
> +        addr = 0x08;
>          break;
>      case EXCP_BKPT:
>          /* Fall through to prefetch abort.  */
> --
> 2.25.1
>

Re: [PATCH] target/arm: Use correct entrypoint for SVC taken from Hyp to Hyp

[Edgar E. Iglesias]

[-- Attachment #1: Type: text/plain, Size: 2696 bytes --]

On Thu, Jan 27, 2022 at 6:14 PM Peter Maydell <peter.maydell@linaro.org>
wrote:

> Ping for code review? This is a nice short easy one :-)
>

Looks good comparing with the Pseudocode. I tried reading the chapters
about these exceptions but couldn't find a clear description :-)

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>



>
> thanks
> -- PMM
>
> On Mon, 17 Jan 2022 at 13:19, Peter Maydell <peter.maydell@linaro.org>
> wrote:
> >
> > The exception caused by an SVC instruction may be taken to AArch32
> > Hyp mode for two reasons:
> >  * HCR.TGE indicates that exceptions from EL0 should trap to EL2
> >  * we were already in Hyp mode
> >
> > The entrypoint in the vector table to be used differs in these two
> > cases: for an exception routed to Hyp mode from EL0, we enter at the
> > common 0x14 "hyp trap" entrypoint.  For SVC from Hyp mode to Hyp
> > mode, we enter at the 0x08 (svc/hvc trap) entrypoint.
> > In the v8A Arm ARM pseudocode this is done in AArch32.TakeSVCException.
> >
> > QEMU incorrectly routed both of these exceptions to the 0x14
> > entrypoint.  Correct the entrypoint for SVC from Hyp to Hyp by making
> > use of the existing logic which handles "normal entrypoint for
> > Hyp-to-Hyp, otherwise 0x14" for traps like UNDEF and data/prefetch
> > aborts (reproduced here since it's outside the visible context
> > in the diff for this commit):
> >
> >     if (arm_current_el(env) != 2 && addr < 0x14) {
> >         addr = 0x14;
> >     }
> >
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> >  target/arm/helper.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/target/arm/helper.c b/target/arm/helper.c
> > index cfca0f5ba6d..a898e400f6c 100644
> > --- a/target/arm/helper.c
> > +++ b/target/arm/helper.c
> > @@ -9655,7 +9655,7 @@ static void
> arm_cpu_do_interrupt_aarch32_hyp(CPUState *cs)
> >       * separately here.
> >       *
> >       * The vector table entry used is always the 0x14 Hyp mode entry
> point,
> > -     * unless this is an UNDEF/HVC/abort taken from Hyp to Hyp.
> > +     * unless this is an UNDEF/SVC/HVC/abort taken from Hyp to Hyp.
> >       * The offset applied to the preferred return address is always zero
> >       * (see DDI0487C.a section G1.12.3).
> >       * PSTATE A/I/F masks are set based only on the SCR.EA/IRQ/FIQ
> values.
> > @@ -9669,7 +9669,7 @@ static void
> arm_cpu_do_interrupt_aarch32_hyp(CPUState *cs)
> >          addr = 0x04;
> >          break;
> >      case EXCP_SWI:
> > -        addr = 0x14;
> > +        addr = 0x08;
> >          break;
> >      case EXCP_BKPT:
> >          /* Fall through to prefetch abort.  */
> > --
> > 2.25.1
> >
>
>

[-- Attachment #2: Type: text/html, Size: 3856 bytes --]

Re: [PATCH] target/arm: Use correct entrypoint for SVC taken from Hyp to Hyp

[Richard Henderson]

On 1/18/22 00:19, Peter Maydell wrote:
> The exception caused by an SVC instruction may be taken to AArch32
> Hyp mode for two reasons:
>   * HCR.TGE indicates that exceptions from EL0 should trap to EL2
>   * we were already in Hyp mode
> 
> The entrypoint in the vector table to be used differs in these two
> cases: for an exception routed to Hyp mode from EL0, we enter at the
> common 0x14 "hyp trap" entrypoint.  For SVC from Hyp mode to Hyp
> mode, we enter at the 0x08 (svc/hvc trap) entrypoint.
> In the v8A Arm ARM pseudocode this is done in AArch32.TakeSVCException.
> 
> QEMU incorrectly routed both of these exceptions to the 0x14
> entrypoint.  Correct the entrypoint for SVC from Hyp to Hyp by making
> use of the existing logic which handles "normal entrypoint for
> Hyp-to-Hyp, otherwise 0x14" for traps like UNDEF and data/prefetch
> aborts (reproduced here since it's outside the visible context
> in the diff for this commit):
> 
>      if (arm_current_el(env) != 2 && addr < 0x14) {
>          addr = 0x14;
>      }
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   target/arm/helper.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~