Re: [Xen-devel] [PATCH RFC for-4.13 04/10] xen/arm: Ensure the SSBD workaround is re-enabled right after exiting a guest

[Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>]

Hi Julien,

Julien Grall writes:

> At the moment, SSBD workaround is re-enabled for Xen after interrupts
> are unmasked. This means we may end up to execute some part of the
> hypervisor if an interrupt is received before the workaround is
> re-enabled.
>
> As the rest of enter_hypervisor_from_guest() does not require to have
> interrupts masked, the function is now split in two parts:
>     1) enter_hypervisor_from_guest_noirq() called with interrupts
>        masked.

To summarize our discussion in this mail thread: providing that you'll
rename enter_hypervisor_from_guest_noirq to
enter_hypervisor_from_guest_preirq():

Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>

>     2) enter_hypervisor_from_guest() called with interrupts unmasked.
>
> Note that while enter_hypervisor_from_guest_noirq() does not use the
> on-stack context registers, it is still passed as parameter to match the
> rest of the C functions called from the entry path.
>
> Fixes: a7898e4c59 ("xen/arm: Add ARCH_WORKAROUND_2 support for guests")
> Reported-by: Andrii Anisov <andrii_anisov@epam.com>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
>
> ---
>
> Note the Arm32 code has not been changed yet. I am also open on turn
> both enter_hypervisor_from_guest_noirq() and enter_hypervisor_from()
> to functions not taking any parameters.
> ---
>  xen/arch/arm/arm64/entry.S |  2 ++
>  xen/arch/arm/traps.c       | 16 +++++++++++++---
>  2 files changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
> index 9eafae516b..458d12f188 100644
> --- a/xen/arch/arm/arm64/entry.S
> +++ b/xen/arch/arm/arm64/entry.S
> @@ -173,6 +173,8 @@
>          ALTERNATIVE("bl check_pending_vserror; cbnz x0, 1f",
>                      "nop; nop",
>                      SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT)
> +        mov     x0, sp
> +        bl      enter_hypervisor_from_guest_noirq
>          msr     daifclr, \iflags
>          mov     x0, sp
>          bl      enter_hypervisor_from_guest
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 20ba34ec91..5848dd8399 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -2007,16 +2007,26 @@ static inline bool needs_ssbd_flip(struct vcpu *v)
>  }
>
>  /*
> - * Actions that needs to be done after exiting the guest and before any
> - * request from it is handled.
> + * Actions that needs to be done after exiting the guest and before the
> + * interrupts are unmasked.
>   */
> -void enter_hypervisor_from_guest(struct cpu_user_regs *regs)
> +void enter_hypervisor_from_guest_noirq(struct cpu_user_regs *regs)
>  {
>      struct vcpu *v = current;
>
>      /* If the guest has disabled the workaround, bring it back on. */
>      if ( needs_ssbd_flip(v) )
>          arm_smccc_1_1_smc(ARM_SMCCC_ARCH_WORKAROUND_2_FID, 1, NULL);
> +}
> +
> +/*
> + * Actions that needs to be done after exiting the guest and before any
> + * request from it is handled. Depending on the exception trap, this may
> + * be called with interrupts unmasked.
> + */
> +void enter_hypervisor_from_guest(struct cpu_user_regs *regs)
> +{
> +    struct vcpu *v = current;
>
>      /*
>       * If we pended a virtual abort, preserve it until it gets cleared.


--
Volodymyr Babchuk at EPAM
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel