Skip to content

Commit

Permalink
KVM: arm64: Use debug_owner to track if debug regs need save/restore
Browse files Browse the repository at this point in the history
Use the debug owner to determine if the debug regs are in use instead of
keeping around the DEBUG_DIRTY flag. Debug registers are now
saved/restored after the first trap, regardless of whether it was a read
or a write. This also shifts the point at which KVM becomes lazy to
vcpu_put() rather than the next exception taken from the guest.

Tested-by: James Clark <james.clark@linaro.org>
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
Link: https://lore.kernel.org/r/20241219224116.3941496-12-oliver.upton@linux.dev
Signed-off-by: Marc Zyngier <maz@kernel.org>
  • Loading branch information
Oliver Upton authored and Marc Zyngier committed Dec 20, 2024
1 parent 803602b commit beb470d
Show file tree
Hide file tree
Showing 5 changed files with 7 additions and 55 deletions.
4 changes: 2 additions & 2 deletions arch/arm64/include/asm/kvm_host.h
Original file line number Diff line number Diff line change
Expand Up @@ -917,8 +917,6 @@ struct kvm_vcpu_arch {
#define EXCEPT_AA64_EL2_IRQ __vcpu_except_flags(5)
#define EXCEPT_AA64_EL2_FIQ __vcpu_except_flags(6)
#define EXCEPT_AA64_EL2_SERR __vcpu_except_flags(7)
/* Guest debug is live */
#define DEBUG_DIRTY __vcpu_single_flag(iflags, BIT(4))

/* Physical CPU not in supported_cpus */
#define ON_UNSUPPORTED_CPU __vcpu_single_flag(sflags, BIT(0))
Expand Down Expand Up @@ -1356,6 +1354,8 @@ void kvm_debug_set_guest_ownership(struct kvm_vcpu *vcpu);
((vcpu)->arch.debug_owner != VCPU_DEBUG_FREE)
#define kvm_host_owns_debug_regs(vcpu) \
((vcpu)->arch.debug_owner == VCPU_DEBUG_HOST_OWNED)
#define kvm_guest_owns_debug_regs(vcpu) \
((vcpu)->arch.debug_owner == VCPU_DEBUG_GUEST_OWNED)

int kvm_arm_vcpu_arch_set_attr(struct kvm_vcpu *vcpu,
struct kvm_device_attr *attr);
Expand Down
19 changes: 3 additions & 16 deletions arch/arm64/kvm/debug.c
Original file line number Diff line number Diff line change
Expand Up @@ -86,15 +86,9 @@ static void kvm_arm_setup_mdcr_el2(struct kvm_vcpu *vcpu)
vcpu->arch.mdcr_el2 |= MDCR_EL2_TDE;

/*
* Trap debug register access when one of the following is true:
* - Userspace is using the hardware to debug the guest
* (KVM_GUESTDBG_USE_HW is set).
* - The guest is not using debug (DEBUG_DIRTY clear).
* - The guest has enabled the OS Lock (debug exceptions are blocked).
* Trap debug registers if the guest doesn't have ownership of them.
*/
if ((vcpu->guest_debug & KVM_GUESTDBG_USE_HW) ||
!vcpu_get_flag(vcpu, DEBUG_DIRTY) ||
kvm_vcpu_os_lock_enabled(vcpu))
if (!kvm_guest_owns_debug_regs(vcpu))
vcpu->arch.mdcr_el2 |= MDCR_EL2_TDA;

/* Write MDCR_EL2 directly if we're already at EL2 */
Expand Down Expand Up @@ -127,8 +121,7 @@ void kvm_arm_vcpu_init_debug(struct kvm_vcpu *vcpu)
* debug related registers.
*
* Additionally, KVM only traps guest accesses to the debug registers if
* the guest is not actively using them (see the DEBUG_DIRTY
* flag on vcpu->arch.iflags). Since the guest must not interfere
* the guest is not actively using them. Since the guest must not interfere
* with the hardware state when debugging the guest, we must ensure that
* trapping is enabled whenever we are debugging the guest using the
* debug registers.
Expand Down Expand Up @@ -195,8 +188,6 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu)
mdscr |= DBG_MDSCR_MDE;
vcpu_write_sys_reg(vcpu, mdscr, MDSCR_EL1);

vcpu_set_flag(vcpu, DEBUG_DIRTY);

/*
* The OS Lock blocks debug exceptions in all ELs when it is
* enabled. If the guest has enabled the OS Lock, constrain its
Expand All @@ -211,10 +202,6 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu)
vcpu_write_sys_reg(vcpu, mdscr, MDSCR_EL1);
}
}

/* If KDE or MDE are set, perform a full save/restore cycle. */
if (vcpu_read_sys_reg(vcpu, MDSCR_EL1) & (DBG_MDSCR_KDE | DBG_MDSCR_MDE))
vcpu_set_flag(vcpu, DEBUG_DIRTY);
}

void kvm_arm_clear_debug(struct kvm_vcpu *vcpu)
Expand Down
2 changes: 0 additions & 2 deletions arch/arm64/kvm/hyp/include/hyp/debug-sr.h
Original file line number Diff line number Diff line change
Expand Up @@ -176,8 +176,6 @@ static inline void __debug_switch_to_host_common(struct kvm_vcpu *vcpu)

__debug_save_state(guest_dbg, guest_ctxt);
__debug_restore_state(host_dbg, host_ctxt);

vcpu_clear_flag(vcpu, DEBUG_DIRTY);
}

#endif /* __ARM64_KVM_HYP_DEBUG_SR_H__ */
4 changes: 2 additions & 2 deletions arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ static inline void __sysreg32_save_state(struct kvm_vcpu *vcpu)
__vcpu_sys_reg(vcpu, DACR32_EL2) = read_sysreg(dacr32_el2);
__vcpu_sys_reg(vcpu, IFSR32_EL2) = read_sysreg(ifsr32_el2);

if (has_vhe() || vcpu_get_flag(vcpu, DEBUG_DIRTY))
if (has_vhe() || kvm_debug_regs_in_use(vcpu))
__vcpu_sys_reg(vcpu, DBGVCR32_EL2) = read_sysreg(dbgvcr32_el2);
}

Expand All @@ -300,7 +300,7 @@ static inline void __sysreg32_restore_state(struct kvm_vcpu *vcpu)
write_sysreg(__vcpu_sys_reg(vcpu, DACR32_EL2), dacr32_el2);
write_sysreg(__vcpu_sys_reg(vcpu, IFSR32_EL2), ifsr32_el2);

if (has_vhe() || vcpu_get_flag(vcpu, DEBUG_DIRTY))
if (has_vhe() || kvm_debug_regs_in_use(vcpu))
write_sysreg(__vcpu_sys_reg(vcpu, DBGVCR32_EL2), dbgvcr32_el2);
}

Expand Down
33 changes: 0 additions & 33 deletions arch/arm64/kvm/sys_regs.c
Original file line number Diff line number Diff line change
Expand Up @@ -621,40 +621,11 @@ static bool trap_dbgauthstatus_el1(struct kvm_vcpu *vcpu,
}
}

/*
* We want to avoid world-switching all the DBG registers all the
* time:
*
* - If we've touched any debug register, it is likely that we're
* going to touch more of them. It then makes sense to disable the
* traps and start doing the save/restore dance
* - If debug is active (DBG_MDSCR_KDE or DBG_MDSCR_MDE set), it is
* then mandatory to save/restore the registers, as the guest
* depends on them.
*
* For this, we use a DIRTY bit, indicating the guest has modified the
* debug registers, used as follow:
*
* On guest entry:
* - If the dirty bit is set (because we're coming back from trapping),
* disable the traps, save host registers, restore guest registers.
* - If debug is actively in use (DBG_MDSCR_KDE or DBG_MDSCR_MDE set),
* set the dirty bit, disable the traps, save host registers,
* restore guest registers.
* - Otherwise, enable the traps
*
* On guest exit:
* - If the dirty bit is set, save guest registers, restore host
* registers and clear the dirty bit. This ensure that the host can
* now use the debug registers.
*/
static bool trap_debug_regs(struct kvm_vcpu *vcpu,
struct sys_reg_params *p,
const struct sys_reg_desc *r)
{
access_rw(vcpu, p, r);
if (p->is_write)
vcpu_set_flag(vcpu, DEBUG_DIRTY);

kvm_debug_set_guest_ownership(vcpu);
return true;
Expand All @@ -665,9 +636,6 @@ static bool trap_debug_regs(struct kvm_vcpu *vcpu,
*
* A 32 bit write to a debug register leave top bits alone
* A 32 bit read from a debug register only returns the bottom bits
*
* All writes will set the DEBUG_DIRTY flag to ensure the hyp code
* switches between host and guest values in future.
*/
static void reg_to_dbg(struct kvm_vcpu *vcpu,
struct sys_reg_params *p,
Expand All @@ -684,7 +652,6 @@ static void reg_to_dbg(struct kvm_vcpu *vcpu,
*dbg_reg = val;

kvm_debug_set_guest_ownership(vcpu);
vcpu_set_flag(vcpu, DEBUG_DIRTY);
}

static void dbg_to_reg(struct kvm_vcpu *vcpu,
Expand Down

0 comments on commit beb470d

Please sign in to comment.