Re: [PATCH v2 1/4] KVM: arm64: Leave KVM_ARM64_DEBUG_DIRTY updates to the host

From: Dave Martin <Dave.Martin@arm.com>
To: Andrew Scull <ascull@google.com>
Cc: maz@kernel.org, kernel-team@android.com, kvmarm@lists.cs.columbia.edu
Subject: Re: [PATCH v2 1/4] KVM: arm64: Leave KVM_ARM64_DEBUG_DIRTY updates to the host
Date: Wed, 22 Jul 2020 17:24:23 +0100	[thread overview]
Message-ID: <20200722162423.GO30452@arm.com> (raw)
In-Reply-To: <20200713210505.2959828-2-ascull@google.com>

On Mon, Jul 13, 2020 at 10:05:02PM +0100, Andrew Scull wrote:
> Move the clearing of KVM_ARM64_DEBUG_DIRTY from being one of the last
> things hyp does before exiting to the host to being one of the first
> things the host does after hyp exits.
> 
> This means the host always manages the state of the bit and hyp simply
> respects that in the context switch.
> 
> No functional change.
> 
> Signed-off-by: Andrew Scull <ascull@google.com>

Seems reasonable, though we have to map the vcpu arch flags into hyp
anyway.  For FPSIMD we do maintain these flags from hyp, in order to
void mapping in host-specific stuff (the thread flags).

So maybe this change isn't that useful?

I don't have a strong opinion though.  If this change fits in better
with the broader KVM work you're doing, I don't see a problem with it.

So, FWIW:

Reviewed-by: Dave Martin <Dave.Martin@arm.com>

> ---
>  arch/arm64/include/asm/kvm_host.h         | 2 +-
>  arch/arm64/kvm/debug.c                    | 2 ++
>  arch/arm64/kvm/hyp/include/hyp/debug-sr.h | 2 --
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index e1a32c0707bb..b06f24b5f443 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -404,7 +404,7 @@ struct kvm_vcpu_arch {
>  })
>  
>  /* vcpu_arch flags field values: */
> -#define KVM_ARM64_DEBUG_DIRTY		(1 << 0)
> +#define KVM_ARM64_DEBUG_DIRTY		(1 << 0) /* vcpu is using debug */
>  #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 */
> diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
> index 7a7e425616b5..e9932618a362 100644
> --- a/arch/arm64/kvm/debug.c
> +++ b/arch/arm64/kvm/debug.c
> @@ -209,6 +209,8 @@ void kvm_arm_clear_debug(struct kvm_vcpu *vcpu)
>  {
>  	trace_kvm_arm_clear_debug(vcpu->guest_debug);
>  
> +	vcpu->arch.flags &= ~KVM_ARM64_DEBUG_DIRTY;
> +
>  	if (vcpu->guest_debug) {
>  		restore_guest_debug_regs(vcpu);
>  
> diff --git a/arch/arm64/kvm/hyp/include/hyp/debug-sr.h b/arch/arm64/kvm/hyp/include/hyp/debug-sr.h
> index 0297dc63988c..50ca5d048017 100644
> --- a/arch/arm64/kvm/hyp/include/hyp/debug-sr.h
> +++ b/arch/arm64/kvm/hyp/include/hyp/debug-sr.h
> @@ -161,8 +161,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->arch.flags &= ~KVM_ARM64_DEBUG_DIRTY;
>  }
>  
>  #endif /* __ARM64_KVM_HYP_DEBUG_SR_H__ */
> -- 
> 2.27.0.383.g050319c2ae-goog
> 
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm