KVM: arm64: Use debug_owner to track if debug regs need save/restore
authorOliver Upton <oliver.upton@linux.dev>
Thu, 19 Dec 2024 22:41:08 +0000 (14:41 -0800)
committerMarc Zyngier <maz@kernel.org>
Fri, 20 Dec 2024 09:01:25 +0000 (09:01 +0000)
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>
arch/arm64/include/asm/kvm_host.h
arch/arm64/kvm/debug.c
arch/arm64/kvm/hyp/include/hyp/debug-sr.h
arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
arch/arm64/kvm/sys_regs.c

index 905b84e59c24de531b8fe5c2875935775f069700..a6796a952af7accd3bc9d8771d89b6126a941319 100644 (file)
@@ -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);
index f39004c52d3345db39b11e909aadfa4d2612b450..a4ae17c31fa870ceb1344f6551c7856aa250dc66 100644 (file)
@@ -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)
index 9e3051225039cfcf12b1c1cac71df9d870983d4b..c14586d8736144407f6c46f8504009dce94ee134 100644 (file)
@@ -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__ */
index a651c43ad679fcc5a13ab7a619e252d96fd46281..a998b2f6abcbbb9de8864dabc77a2bc6ea5bde76 100644 (file)
@@ -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);
 }
 
index 8c5771572133a06a27d5aba35437ef25989d987a..66db77fb6201e106810ff555d14402c679ac4e0d 100644 (file)
@@ -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,