Skip to content

Commit

Permalink
KVM: arm64: Optimise FPSIMD handling to reduce guest/host thrashing
Browse files Browse the repository at this point in the history
This patch refactors KVM to align the host and guest FPSIMD
save/restore logic with each other for arm64.  This reduces the
number of redundant save/restore operations that must occur, and
reduces the common-case IRQ blackout time during guest exit storms
by saving the host state lazily and optimising away the need to
restore the host state before returning to the run loop.

Four hooks are defined in order to enable this:

 * kvm_arch_vcpu_run_map_fp():
   Called on PID change to map necessary bits of current to Hyp.

 * kvm_arch_vcpu_load_fp():
   Set up FP/SIMD for entering the KVM run loop (parse as
   "vcpu_load fp").

 * kvm_arch_vcpu_ctxsync_fp():
   Get FP/SIMD into a safe state for re-enabling interrupts after a
   guest exit back to the run loop.

   For arm64 specifically, this involves updating the host kernel's
   FPSIMD context tracking metadata so that kernel-mode NEON use
   will cause the vcpu's FPSIMD state to be saved back correctly
   into the vcpu struct.  This must be done before re-enabling
   interrupts because kernel-mode NEON may be used by softirqs.

 * kvm_arch_vcpu_put_fp():
   Save guest FP/SIMD state back to memory and dissociate from the
   CPU ("vcpu_put fp").

Also, the arm64 FPSIMD context switch code is updated to enable it
to save back FPSIMD state for a vcpu, not just current.  A few
helpers drive this:

 * fpsimd_bind_state_to_cpu(struct user_fpsimd_state *fp):
   mark this CPU as having context fp (which may belong to a vcpu)
   currently loaded in its registers.  This is the non-task
   equivalent of the static function fpsimd_bind_to_cpu() in
   fpsimd.c.

 * task_fpsimd_save():
   exported to allow KVM to save the guest's FPSIMD state back to
   memory on exit from the run loop.

 * fpsimd_flush_state():
   invalidate any context's FPSIMD state that is currently loaded.
   Used to disassociate the vcpu from the CPU regs on run loop exit.

These changes allow the run loop to enable interrupts (and thus
softirqs that may use kernel-mode NEON) without having to save the
guest's FPSIMD state eagerly.

Some new vcpu_arch fields are added to make all this work.  Because
host FPSIMD state can now be saved back directly into current's
thread_struct as appropriate, host_cpu_context is no longer used
for preserving the FPSIMD state.  However, it is still needed for
preserving other things such as the host's system registers.  To
avoid ABI churn, the redundant storage space in host_cpu_context is
not removed for now.

arch/arm is not addressed by this patch and continues to use its
current save/restore logic.  It could provide implementations of
the helpers later if desired.

Signed-off-by: Dave Martin <[email protected]>
Reviewed-by: Marc Zyngier <[email protected]>
Reviewed-by: Christoffer Dall <[email protected]>
Reviewed-by: Alex Bennée <[email protected]>
Acked-by: Catalin Marinas <[email protected]>
Signed-off-by: Marc Zyngier <[email protected]>
  • Loading branch information
Dave Martin authored and Marc Zyngier committed May 25, 2018
1 parent fa89d31 commit e6b673b
Show file tree
Hide file tree
Showing 9 changed files with 192 additions and 31 deletions.
8 changes: 8 additions & 0 deletions arch/arm/include/asm/kvm_host.h
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,14 @@ int kvm_arm_vcpu_arch_get_attr(struct kvm_vcpu *vcpu,
int kvm_arm_vcpu_arch_has_attr(struct kvm_vcpu *vcpu,
struct kvm_device_attr *attr);

/*
* VFP/NEON switching is all done by the hyp switch code, so no need to
* coordinate with host context handling for this state:
*/
static inline void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu) {}
static inline void kvm_arch_vcpu_ctxsync_fp(struct kvm_vcpu *vcpu) {}
static inline void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu) {}

/* All host FP/SIMD state is restored on guest exit, so nothing to save: */
static inline void kvm_fpsimd_flush_cpu_state(void) {}

Expand Down
6 changes: 6 additions & 0 deletions arch/arm64/include/asm/fpsimd.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ struct task_struct;
extern void fpsimd_save_state(struct user_fpsimd_state *state);
extern void fpsimd_load_state(struct user_fpsimd_state *state);

extern void fpsimd_save(void);

extern void fpsimd_thread_switch(struct task_struct *next);
extern void fpsimd_flush_thread(void);

Expand All @@ -49,7 +51,11 @@ extern void fpsimd_preserve_current_state(void);
extern void fpsimd_restore_current_state(void);
extern void fpsimd_update_current_state(struct user_fpsimd_state const *state);

extern void fpsimd_bind_task_to_cpu(void);
extern void fpsimd_bind_state_to_cpu(struct user_fpsimd_state *state);

extern void fpsimd_flush_task_state(struct task_struct *target);
extern void fpsimd_flush_cpu_state(void);
extern void sve_flush_cpu_state(void);

/* Maximum VL that SVE VL-agnostic software can transparently support */
Expand Down
21 changes: 21 additions & 0 deletions arch/arm64/include/asm/kvm_host.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include <asm/kvm.h>
#include <asm/kvm_asm.h>
#include <asm/kvm_mmio.h>
#include <asm/thread_info.h>

#define __KVM_HAVE_ARCH_INTC_INITIALIZED

Expand Down Expand Up @@ -238,6 +239,10 @@ struct kvm_vcpu_arch {

/* Pointer to host CPU context */
kvm_cpu_context_t *host_cpu_context;

struct thread_info *host_thread_info; /* hyp VA */
struct user_fpsimd_state *host_fpsimd_state; /* hyp VA */

struct {
/* {Break,watch}point registers */
struct kvm_guest_debug_arch regs;
Expand Down Expand Up @@ -295,6 +300,9 @@ struct kvm_vcpu_arch {

/* vcpu_arch flags field values: */
#define KVM_ARM64_DEBUG_DIRTY (1 << 0)
#define KVM_ARM64_FP_ENABLED (1 << 1) /* guest FP regs loaded */
#define KVM_ARM64_FP_HOST (1 << 2) /* host FP regs loaded */
#define KVM_ARM64_HOST_SVE_IN_USE (1 << 3) /* backup for host TIF_SVE */

#define vcpu_gp_regs(v) (&(v)->arch.ctxt.gp_regs)

Expand Down Expand Up @@ -423,6 +431,19 @@ static inline void __cpu_init_stage2(void)
"PARange is %d bits, unsupported configuration!", parange);
}

/* Guest/host FPSIMD coordination helpers */
int kvm_arch_vcpu_run_map_fp(struct kvm_vcpu *vcpu);
void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu);
void kvm_arch_vcpu_ctxsync_fp(struct kvm_vcpu *vcpu);
void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu);

#ifdef CONFIG_KVM /* Avoid conflicts with core headers if CONFIG_KVM=n */
static inline int kvm_arch_vcpu_run_pid_change(struct kvm_vcpu *vcpu)
{
return kvm_arch_vcpu_run_map_fp(vcpu);
}
#endif

/*
* All host FP/SIMD state is restored on guest exit, so nothing needs
* doing here except in the SVE case:
Expand Down
19 changes: 15 additions & 4 deletions arch/arm64/kernel/fpsimd.c
Original file line number Diff line number Diff line change
Expand Up @@ -265,10 +265,10 @@ static void task_fpsimd_load(void)
*
* Softirqs (and preemption) must be disabled.
*/
static void fpsimd_save(void)
void fpsimd_save(void)
{
struct user_fpsimd_state *st = __this_cpu_read(fpsimd_last_state.st);
/* set by fpsimd_bind_task_to_cpu() */
/* set by fpsimd_bind_task_to_cpu() or fpsimd_bind_state_to_cpu() */

WARN_ON(!in_softirq() && !irqs_disabled());

Expand Down Expand Up @@ -986,7 +986,7 @@ void fpsimd_signal_preserve_current_state(void)
* Associate current's FPSIMD context with this cpu
* Preemption must be disabled when calling this function.
*/
static void fpsimd_bind_task_to_cpu(void)
void fpsimd_bind_task_to_cpu(void)
{
struct fpsimd_last_state_struct *last =
this_cpu_ptr(&fpsimd_last_state);
Expand All @@ -1006,6 +1006,17 @@ static void fpsimd_bind_task_to_cpu(void)
}
}

void fpsimd_bind_state_to_cpu(struct user_fpsimd_state *st)
{
struct fpsimd_last_state_struct *last =
this_cpu_ptr(&fpsimd_last_state);

WARN_ON(!in_softirq() && !irqs_disabled());

last->st = st;
last->sve_in_use = false;
}

/*
* Load the userland FPSIMD state of 'current' from memory, but only if the
* FPSIMD state already held in the registers is /not/ the most recent FPSIMD
Expand Down Expand Up @@ -1058,7 +1069,7 @@ void fpsimd_flush_task_state(struct task_struct *t)
t->thread.fpsimd_cpu = NR_CPUS;
}

static inline void fpsimd_flush_cpu_state(void)
void fpsimd_flush_cpu_state(void)
{
__this_cpu_write(fpsimd_last_state.st, NULL);
set_thread_flag(TIF_FOREIGN_FPSTATE);
Expand Down
1 change: 1 addition & 0 deletions arch/arm64/kvm/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ config KVM
select HAVE_KVM_IRQ_ROUTING
select IRQ_BYPASS_MANAGER
select HAVE_KVM_IRQ_BYPASS
select HAVE_KVM_VCPU_RUN_PID_CHANGE
---help---
Support hosting virtualized guest machines.
We don't support KVM with 16K page tables yet, due to the multiple
Expand Down
2 changes: 1 addition & 1 deletion arch/arm64/kvm/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/psci.o $(KVM)/arm/perf.o
kvm-$(CONFIG_KVM_ARM_HOST) += inject_fault.o regmap.o va_layout.o
kvm-$(CONFIG_KVM_ARM_HOST) += hyp.o hyp-init.o handle_exit.o
kvm-$(CONFIG_KVM_ARM_HOST) += guest.o debug.o reset.o sys_regs.o sys_regs_generic_v8.o
kvm-$(CONFIG_KVM_ARM_HOST) += vgic-sys-reg-v3.o
kvm-$(CONFIG_KVM_ARM_HOST) += vgic-sys-reg-v3.o fpsimd.o
kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/aarch32.o

kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic.o
Expand Down
111 changes: 111 additions & 0 deletions arch/arm64/kvm/fpsimd.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
// SPDX-License-Identifier: GPL-2.0
/*
* arch/arm64/kvm/fpsimd.c: Guest/host FPSIMD context coordination helpers
*
* Copyright 2018 Arm Limited
* Author: Dave Martin <[email protected]>
*/
#include <linux/bottom_half.h>
#include <linux/sched.h>
#include <linux/thread_info.h>
#include <linux/kvm_host.h>
#include <asm/kvm_asm.h>
#include <asm/kvm_host.h>
#include <asm/kvm_mmu.h>

/*
* Called on entry to KVM_RUN unless this vcpu previously ran at least
* once and the most recent prior KVM_RUN for this vcpu was called from
* the same task as current (highly likely).
*
* This is guaranteed to execute before kvm_arch_vcpu_load_fp(vcpu),
* such that on entering hyp the relevant parts of current are already
* mapped.
*/
int kvm_arch_vcpu_run_map_fp(struct kvm_vcpu *vcpu)
{
int ret;

struct thread_info *ti = &current->thread_info;
struct user_fpsimd_state *fpsimd = &current->thread.uw.fpsimd_state;

/*
* Make sure the host task thread flags and fpsimd state are
* visible to hyp:
*/
ret = create_hyp_mappings(ti, ti + 1, PAGE_HYP);
if (ret)
goto error;

ret = create_hyp_mappings(fpsimd, fpsimd + 1, PAGE_HYP);
if (ret)
goto error;

vcpu->arch.host_thread_info = kern_hyp_va(ti);
vcpu->arch.host_fpsimd_state = kern_hyp_va(fpsimd);
error:
return ret;
}

/*
* Prepare vcpu for saving the host's FPSIMD state and loading the guest's.
* The actual loading is done by the FPSIMD access trap taken to hyp.
*
* Here, we just set the correct metadata to indicate that the FPSIMD
* state in the cpu regs (if any) belongs to current on the host.
*
* TIF_SVE is backed up here, since it may get clobbered with guest state.
* This flag is restored by kvm_arch_vcpu_put_fp(vcpu).
*/
void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu)
{
BUG_ON(system_supports_sve());
BUG_ON(!current->mm);

vcpu->arch.flags &= ~(KVM_ARM64_FP_ENABLED | KVM_ARM64_HOST_SVE_IN_USE);
vcpu->arch.flags |= KVM_ARM64_FP_HOST;
if (test_thread_flag(TIF_SVE))
vcpu->arch.flags |= KVM_ARM64_HOST_SVE_IN_USE;
}

/*
* If the guest FPSIMD state was loaded, update the host's context
* tracking data mark the CPU FPSIMD regs as dirty and belonging to vcpu
* so that they will be written back if the kernel clobbers them due to
* kernel-mode NEON before re-entry into the guest.
*/
void kvm_arch_vcpu_ctxsync_fp(struct kvm_vcpu *vcpu)
{
WARN_ON_ONCE(!irqs_disabled());

if (vcpu->arch.flags & KVM_ARM64_FP_ENABLED) {
fpsimd_bind_state_to_cpu(&vcpu->arch.ctxt.gp_regs.fp_regs);
clear_thread_flag(TIF_FOREIGN_FPSTATE);
clear_thread_flag(TIF_SVE);
}
}

/*
* Write back the vcpu FPSIMD regs if they are dirty, and invalidate the
* cpu FPSIMD regs so that they can't be spuriously reused if this vcpu
* disappears and another task or vcpu appears that recycles the same
* struct fpsimd_state.
*/
void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu)
{
local_bh_disable();

update_thread_flag(TIF_SVE,
vcpu->arch.flags & KVM_ARM64_HOST_SVE_IN_USE);

if (vcpu->arch.flags & KVM_ARM64_FP_ENABLED) {
/* Clean guest FP state to memory and invalidate cpu view */
fpsimd_save();
fpsimd_flush_cpu_state();
} else if (!test_thread_flag(TIF_FOREIGN_FPSTATE)) {
/* Ensure user trap controls are correctly restored */
fpsimd_bind_task_to_cpu();
}

local_bh_enable();
}
Loading

0 comments on commit e6b673b

Please sign in to comment.