Re: [PATCH v3 10/16] KVM: arm64: Use debug_owner to track if debug regs need save/restore

[Oliver Upton <oliver.upton@linux.dev>]

On Wed, Dec 18, 2024 at 06:11:54PM +0000, Marc Zyngier wrote:
> On Mon, 09 Dec 2024 18:09:20 +0000,
> Oliver Upton <oliver.upton@linux.dev> wrote:
> > 
> > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> > index f84306b11261..5b8982bcc0bb 100644
> > --- a/arch/arm64/kvm/sys_regs.c
> > +++ b/arch/arm64/kvm/sys_regs.c
> > @@ -621,40 +621,12 @@ 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;
> 
> Sorry for stating the same thing again, but this is a repeat of [1].
> Should I add this on top?

Argh! No, thanks for catching this (again). I really ought to make sure
I squash fixes in when I say I do...

-- 
Thanks,
Oliver