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.
I'm okay with this approach, but I don't like name for
enter_hypervisor_from_guest_noirq(). Right now it is doing exactly one
thing - mitigates SSBD. So, maybe more appropriate name will be
something like "mitigate_ssbd()" ?
> 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.
As I pointed in the previous email, enter_hypervisor_from_guest() does
not use on-stack registers as well.
> Fixes: a7898e4c59 ("xen/arm: Add ARCH_WORKAROUND_2 support for guests")
> Reported-by: Andrii Anisov <andrii_ani...@epam.com>
> Signed-off-by: Julien Grall <julien.gr...@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.
That would be appropriate in my opinion.
> ---
> 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
