Re: [PATCH 1/6] KVM: PPC: BOOKE: No need to set DBCR0_EDM in guest visible register

[Scott Wood <scottwood@freescale.com>]

On Wed, 2014-07-30 at 00:21 -0500, Bhushan Bharat-R65777 wrote:
> 
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Tuesday, July 29, 2014 3:22 AM
> > To: Bhushan Bharat-R65777
> > Cc: agraf@suse.de; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; Yoder Stuart-
> > B08248
> > Subject: Re: [PATCH 1/6] KVM: PPC: BOOKE: No need to set DBCR0_EDM in guest
> > visible register
> > 
> > On Fri, 2014-07-11 at 14:08 +0530, Bharat Bhushan wrote:
> > > This is not used and  even I do not remember why this was added in
> > > first place.
> > >
> > > Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com>
> > > ---
> > >  arch/powerpc/kvm/booke.c | 2 --
> > >  1 file changed, 2 deletions(-)
> > >
> > > diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index
> > > ab62109..a5ee42c 100644
> > > --- a/arch/powerpc/kvm/booke.c
> > > +++ b/arch/powerpc/kvm/booke.c
> > > @@ -1804,8 +1804,6 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu
> > *vcpu,
> > >  	kvm_guest_protect_msr(vcpu, MSR_DE, true);
> > >  	vcpu->guest_debug = dbg->control;
> > >  	vcpu->arch.shadow_dbg_reg.dbcr0 = 0;
> > > -	/* Set DBCR0_EDM in guest visible DBCR0 register. */
> > > -	vcpu->arch.dbg_reg.dbcr0 = DBCR0_EDM;
> > >
> > >  	if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP)
> > >  		vcpu->arch.shadow_dbg_reg.dbcr0 |= DBCR0_IDM | DBCR0_IC;
> > 
> > This was intended to let the guest know that the host owns the debug resources,
> > by analogy to what a JTAG debugger would do.
> > 
> > The Power ISA has this "Virtualized Implementation Note":
> > 
> >         It is the responsibility of the hypervisor to ensure that
> >         DBCR0[EDM] is consistent with usage of DEP.
> 
> Ok, That means that if MSRP_DEP is set then set DBCR0_EDM  and if MSRP_DEP is clear then clear DBCR0_EDM, right?
> We need to implement above mentioned this.

We should probably clear EDM only when guest debug emulation is working
and enabled (i.e. not until at least patch 6/6).

-Scott