On 27/09/2019 12:56, Volodymyr Babchuk wrote:
Julien,
Hi...
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()" ?
If I wanted to call it mitigate_ssbd() I would have implemented completely
differently. The reason it is like that is because we may need more code to be
added here in the future (I have Andrii's series in mind). So I would rather
avoid a further renaming later on and some rework.
Regarding the name, this is a split of enter_hypervisor_from_guest(). Hence, why
the first path is the same. The noirq merely help the user to know what to
expect. This is better of yet an __ version. Feel free to suggest a better suffix.
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.
I am well aware of this, hence my comment here in the commit message ;). The
reason it is like that is because I wanted to keep the prototype the same for
all functions called from the entry path (this includes do_trap_*).
[...]
---
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
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel