[PATCH v2 5/5] KVM: arm64: uapi: Add kvm_debug_exit_arch.hsr_high

[Alexandru Elisei]

Hi,

On Thu, Apr 21, 2022 at 01:58:52PM +0100, Marc Zyngier wrote:
> On Thu, 21 Apr 2022 11:05:47 +0100,
> Alexandru Elisei <alexandru.elisei at arm.com> wrote:
> > 
> > diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
> > index 93d92130d36c..fd5b6773e3a2 100644
> > --- a/arch/arm64/kvm/handle_exit.c
> > +++ b/arch/arm64/kvm/handle_exit.c
> > @@ -121,6 +121,8 @@ static int kvm_handle_guest_debug(struct kvm_vcpu *vcpu)
> >  
> >  	run->exit_reason = KVM_EXIT_DEBUG;
> >  	run->debug.arch.hsr = lower_32_bits(esr);
> > +	run->debug.arch.hsr_high = upper_32_bits(esr);
> > +	run->flags |= KVM_DEBUG_ARCH_HSR_HIGH_VALID;
> 
> Who will eventually clear this flag? I'm concerned that it could be
> misinterpreted by other userspace paths, as once you get a debug exit
> on this vcpu, it will always be set.
> 
> Probably only a matter of clearing flags on all the other exit paths.

I missed this part, I was under the impression that kvm_run->flags was
already cleared on every exit (that's why it's bitwise OR'ed with the
flag).

kvm_arch_vcpu_ioctl_run() always sets exit_reason = KVM_EXIT_UNKNOWN, so I
guess if we want to be consistent, kvm_run->flags should be cleared at the
same time. Unless you want KVM to clear flags for all exit reasons *except*
KVM_EXIT_UNKNOWN.

I prefer clearing flags in kvm_arch_vcpu_ioctl_run() as that looks to me
like the least error prone way to do it, and if in the future an exit
reason wants to preserve flags across KVM_RUN ioctls we can add a check for
that particular situation.

> 
> Also, please document the flag in the API file (only a couple of x86
> flags are there so far).

Sure thing, will do.

Thanks,
Alex