diff mbox

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

Message ID 1405067941-27134-2-git-send-email-Bharat.Bhushan@freescale.com
State New, archived
Headers show

Commit Message

Bharat Bhushan July 11, 2014, 8:38 a.m. UTC
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(-)

Comments

Scott Wood July 28, 2014, 9:52 p.m. UTC | #1
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.

-Scott


--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Scott Wood July 30, 2014, 5:47 p.m. UTC | #2
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


--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bharat Bhushan July 30, 2014, 5:57 p.m. UTC | #3
> -----Original Message-----

> From: Wood Scott-B07421

> Sent: Wednesday, July 30, 2014 11:18 PM

> 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 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).


But if EDM is set then guest debug emulation will not start/allowed.


Thanks
-Bharat

> 

> -Scott

>
Scott Wood July 30, 2014, 6:15 p.m. UTC | #4
On Wed, 2014-07-30 at 12:57 -0500, Bhushan Bharat-R65777 wrote:
> 
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Wednesday, July 30, 2014 11:18 PM
> > 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 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).
> 
> But if EDM is set then guest debug emulation will not start/allowed.

I don't mean after the guest tries to write to the registers -- I mean
after the code has been added to KVM to allow it to work.

-Scott


--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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;