Re: [PATCH 06/15] KVM: arm64: Advance debug_owner state machine for sysreg traps

[Marc Zyngier <maz@kernel.org>]

On Fri, 08 Nov 2024 22:24:10 +0000,
Oliver Upton <oliver.upton@linux.dev> wrote:
> 
> Preserve lazy restore of the breakpoint/watchpoint registers by updating
> the debug owner after the first trap, assuming the host isn't using
> them.
> 
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> ---
>  arch/arm64/include/asm/kvm_host.h | 1 +
>  arch/arm64/kvm/debug.c            | 9 +++++++++
>  arch/arm64/kvm/sys_regs.c         | 2 ++
>  3 files changed, 12 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index f954b082dc4f..d76e4e44b65b 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -1352,6 +1352,7 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu);
>  void kvm_arm_clear_debug(struct kvm_vcpu *vcpu);
>  void kvm_arm_reset_debug_ptr(struct kvm_vcpu *vcpu);
>  void kvm_vcpu_load_debug(struct kvm_vcpu *vcpu);
> +void kvm_handle_debug_access(struct kvm_vcpu *vcpu);
>  
>  #define kvm_vcpu_os_lock_enabled(vcpu)		\
>  	(!!(__vcpu_sys_reg(vcpu, OSLSR_EL1) & OSLSR_EL1_OSLK))
> diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
> index e2798a4eec47..c2bf1b296b14 100644
> --- a/arch/arm64/kvm/debug.c
> +++ b/arch/arm64/kvm/debug.c
> @@ -336,3 +336,12 @@ void kvm_vcpu_load_debug(struct kvm_vcpu *vcpu)
>  			vcpu->arch.debug_owner = VCPU_DEBUG_FREE;
>  	}
>  }
> +
> +void kvm_handle_debug_access(struct kvm_vcpu *vcpu)
> +{
> +	if (kvm_host_owns_debug_regs(vcpu))
> +		return;
> +
> +	WARN_ON_ONCE(vcpu->arch.debug_owner == VCPU_DEBUG_GUEST_OWNED);
> +	vcpu->arch.debug_owner = VCPU_DEBUG_GUEST_OWNED;
> +}

I think it would make more sense to merge this patch with the previous
one, so that we get a better idea of the ownership flow before getting
in the meat of the debug surgery.

Another thing is that kvm_handle_debug_access() is very generic and
doesn't quite says what this does. How about something such as
kvm_debug_set_guest_ownership(), or any variation on the same theme?

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.