Commit beb470d9 authored by Oliver Upton's avatar Oliver Upton Committed by Marc Zyngier
Browse files

KVM: arm64: Use debug_owner to track if debug regs need save/restore



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: default avatarJames Clark <james.clark@linaro.org>
Signed-off-by: default avatarOliver Upton <oliver.upton@linux.dev>
Link: https://lore.kernel.org/r/20241219224116.3941496-12-oliver.upton@linux.dev


Signed-off-by: default avatarMarc Zyngier <maz@kernel.org>
parent 803602b0
Loading
Loading
Loading
Loading
+2 −2
Original line number Diff line number Diff line
@@ -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))
@@ -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);
+3 −16
Original line number Diff line number Diff line
@@ -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 */
@@ -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.
@@ -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
@@ -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)
+0 −2
Original line number Diff line number Diff line
@@ -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__ */
+2 −2
Original line number Diff line number Diff line
@@ -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);
}

@@ -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);
}

+0 −33
Original line number Diff line number Diff line
@@ -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;
@@ -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,
@@ -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,