[PATCH 0/2] KVM: powerpc/booke: Debug interrupt injection to guest

[PATCH 0/2] KVM: powerpc/booke: Debug interrupt injection to guest

[Bharat Bhushan]

First patch add support for debug interrupt at "debug level"
while second patch add supports for allowing userspace to inject
debug interrupt to guest.

Bharat Bhushan (2):
  KVM: powerpc/booke: allow debug interrupt at "debug level"
  KVM : powerpc/booke: Allow debug interrupt injection to guest

 arch/powerpc/kvm/booke.c  | 37 ++++++++++++++++++++++++++++++++++---
 arch/powerpc/kvm/e500mc.c | 10 +++++++++-
 2 files changed, 43 insertions(+), 4 deletions(-)

-- 
1.9.0

[PATCH 1/2] KVM: powerpc/booke: allow debug interrupt at "debug level"

[Bharat Bhushan]

Debug interrupt can be either "critical level" or "debug level".
There are separate set of save/restore registers used for different level.
Example: DSRR0/DSRR1 are used for "debug level" and CSRR0/CSRR1
are used for critical level debug interrupt.

Using CPU_FTR_DEBUG_LVL_EXC to decide which interrupt level to be used.

Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com>
---
 arch/powerpc/kvm/booke.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index ab62109..bb25937 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -424,7 +424,11 @@ static int kvmppc_booke_irqprio_deliver(struct kvm_vcpu *vcpu,
 		allowed = vcpu->arch.shared->msr & MSR_DE;
 		allowed = allowed && !crit;
 		msr_mask = MSR_ME;
-		int_class = INT_CLASS_CRIT;
+		if (cpu_has_feature(CPU_FTR_DEBUG_LVL_EXC))
+			int_class = INT_CLASS_DBG;
+		else
+			int_class = INT_CLASS_CRIT;
+
 		break;
 	}
 
-- 
1.9.0

[PATCH 2/2] KVM : powerpc/booke: Allow debug interrupt injection to guest

[Bharat Bhushan]

This patch allow userspace to inject debug interrupt to guest.

Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com>
---
 arch/powerpc/kvm/booke.c  | 31 +++++++++++++++++++++++++++++--
 arch/powerpc/kvm/e500mc.c | 10 +++++++++-
 2 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index bb25937..63ac38c 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -135,6 +135,11 @@ static void kvmppc_vcpu_sync_fpu(struct kvm_vcpu *vcpu)
 #endif
 }
 
+static int kvmppc_core_pending_debug(struct kvm_vcpu *vcpu)
+{
+	return test_bit(BOOKE_IRQPRIO_DEBUG, &vcpu->arch.pending_exceptions);
+}
+
 static void kvmppc_vcpu_sync_debug(struct kvm_vcpu *vcpu)
 {
 	/* Synchronize guest's desire to get debug interrupts into shadow MSR */
@@ -143,8 +148,11 @@ static void kvmppc_vcpu_sync_debug(struct kvm_vcpu *vcpu)
 	vcpu->arch.shadow_msr |= vcpu->arch.shared->msr & MSR_DE;
 #endif
 
-	/* Force enable debug interrupts when user space wants to debug */
-	if (vcpu->guest_debug) {
+	/*
+	 * Force enable debug interrupts when user space wants to debug
+	 * and there is no debug interrupt pending for guest to handle.
+	 */
+	if (vcpu->guest_debug && !kvmppc_core_pending_debug(vcpu)) {
 #ifdef CONFIG_KVM_BOOKE_HV
 		/*
 		 * Since there is no shadow MSR, sync MSR_DE into the guest
@@ -264,6 +272,16 @@ static void kvmppc_core_dequeue_watchdog(struct kvm_vcpu *vcpu)
 	clear_bit(BOOKE_IRQPRIO_WATCHDOG, &vcpu->arch.pending_exceptions);
 }
 
+static void kvmppc_core_queue_debug(struct kvm_vcpu *vcpu)
+{
+	kvmppc_booke_queue_irqprio(vcpu, BOOKE_IRQPRIO_DEBUG);
+}
+
+static void kvmppc_core_dequeue_debug(struct kvm_vcpu *vcpu)
+{
+	clear_bit(BOOKE_IRQPRIO_DEBUG, &vcpu->arch.pending_exceptions);
+}
+
 static void set_guest_srr(struct kvm_vcpu *vcpu, unsigned long srr0, u32 srr1)
 {
 #ifdef CONFIG_KVM_BOOKE_HV
@@ -1332,6 +1350,7 @@ static void get_sregs_base(struct kvm_vcpu *vcpu,
 	sregs->u.e.dec = kvmppc_get_dec(vcpu, tb);
 	sregs->u.e.tb = tb;
 	sregs->u.e.vrsave = vcpu->arch.vrsave;
+	sregs->u.e.dbsr = vcpu->arch.dbsr;
 }
 
 static int set_sregs_base(struct kvm_vcpu *vcpu,
@@ -1356,6 +1375,14 @@ static int set_sregs_base(struct kvm_vcpu *vcpu,
 	if (sregs->u.e.update_special & KVM_SREGS_E_UPDATE_TSR)
 		kvmppc_set_tsr(vcpu, sregs->u.e.tsr);
 
+	if (sregs->u.e.update_special & KVM_SREGS_E_UPDATE_DBSR) {
+		vcpu->arch.dbsr = sregs->u.e.dbsr;
+		if (vcpu->arch.dbsr)
+			kvmppc_core_queue_debug(vcpu);
+		else
+			kvmppc_core_dequeue_debug(vcpu);
+	}
+
 	return 0;
 }
 
diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c
index 17e4562..ea724f2 100644
--- a/arch/powerpc/kvm/e500mc.c
+++ b/arch/powerpc/kvm/e500mc.c
@@ -212,7 +212,7 @@ static int kvmppc_core_get_sregs_e500mc(struct kvm_vcpu *vcpu,
 	struct kvmppc_vcpu_e500 *vcpu_e500 = to_e500(vcpu);
 
 	sregs->u.e.features |= KVM_SREGS_E_ARCH206_MMU | KVM_SREGS_E_PM |
-			       KVM_SREGS_E_PC;
+			       KVM_SREGS_E_PC | KVM_SREGS_E_ED;
 	sregs->u.e.impl_id = KVM_SREGS_E_IMPL_FSL;
 
 	sregs->u.e.impl.fsl.features = 0;
@@ -220,6 +220,9 @@ static int kvmppc_core_get_sregs_e500mc(struct kvm_vcpu *vcpu,
 	sregs->u.e.impl.fsl.hid0 = vcpu_e500->hid0;
 	sregs->u.e.impl.fsl.mcar = vcpu_e500->mcar;
 
+	sregs->u.e.dsrr0 = vcpu->arch.dsrr0;
+	sregs->u.e.dsrr1 = vcpu->arch.dsrr1;
+
 	kvmppc_get_sregs_e500_tlb(vcpu, sregs);
 
 	sregs->u.e.ivor_high[3] =
@@ -261,6 +264,11 @@ static int kvmppc_core_set_sregs_e500mc(struct kvm_vcpu *vcpu,
 			sregs->u.e.ivor_high[5];
 	}
 
+	if (sregs->u.e.features & KVM_SREGS_E_ED) {
+		vcpu->arch.dsrr0 = sregs->u.e.dsrr0;
+		vcpu->arch.dsrr1 = sregs->u.e.dsrr1;
+	}
+
 	return kvmppc_set_sregs_ivor(vcpu, sregs);
 }
 
-- 
1.9.0

Re: [PATCH 2/2] KVM : powerpc/booke: Allow debug interrupt injection to guest

[Scott Wood]

On Fri, 2014-06-27 at 11:55 +0530, Bharat Bhushan wrote:
> This patch allow userspace to inject debug interrupt to guest.
> 
> Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com>

Could you describe how userspace plans to make use of this, and go into
more detail about the changes you're making?

> ---
>  arch/powerpc/kvm/booke.c  | 31 +++++++++++++++++++++++++++++--
>  arch/powerpc/kvm/e500mc.c | 10 +++++++++-
>  2 files changed, 38 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
> index bb25937..63ac38c 100644
> --- a/arch/powerpc/kvm/booke.c
> +++ b/arch/powerpc/kvm/booke.c
> @@ -135,6 +135,11 @@ static void kvmppc_vcpu_sync_fpu(struct kvm_vcpu *vcpu)
>  #endif
>  }
>  
> +static int kvmppc_core_pending_debug(struct kvm_vcpu *vcpu)
> +{
> +	return test_bit(BOOKE_IRQPRIO_DEBUG, &vcpu->arch.pending_exceptions);
> +}
> +
>  static void kvmppc_vcpu_sync_debug(struct kvm_vcpu *vcpu)
>  {
>  	/* Synchronize guest's desire to get debug interrupts into shadow MSR */
> @@ -143,8 +148,11 @@ static void kvmppc_vcpu_sync_debug(struct kvm_vcpu *vcpu)
>  	vcpu->arch.shadow_msr |= vcpu->arch.shared->msr & MSR_DE;
>  #endif
>  
> -	/* Force enable debug interrupts when user space wants to debug */
> -	if (vcpu->guest_debug) {
> +	/*
> +	 * Force enable debug interrupts when user space wants to debug
> +	 * and there is no debug interrupt pending for guest to handle.
> +	 */
> +	if (vcpu->guest_debug && !kvmppc_core_pending_debug(vcpu)) {

Are you trying to allow the guest to be simultaneously debugged by
itself and by host userspace?  How does this work?

>  #ifdef CONFIG_KVM_BOOKE_HV
>  		/*
>  		 * Since there is no shadow MSR, sync MSR_DE into the guest
> @@ -264,6 +272,16 @@ static void kvmppc_core_dequeue_watchdog(struct kvm_vcpu *vcpu)
>  	clear_bit(BOOKE_IRQPRIO_WATCHDOG, &vcpu->arch.pending_exceptions);
>  }
>  
> +static void kvmppc_core_queue_debug(struct kvm_vcpu *vcpu)
> +{
> +	kvmppc_booke_queue_irqprio(vcpu, BOOKE_IRQPRIO_DEBUG);
> +}
> +
> +static void kvmppc_core_dequeue_debug(struct kvm_vcpu *vcpu)
> +{
> +	clear_bit(BOOKE_IRQPRIO_DEBUG, &vcpu->arch.pending_exceptions);
> +}

Is there currently no support for a guest debugging itself (i.e.
guest_debug unset) on e500v2?

>  static void set_guest_srr(struct kvm_vcpu *vcpu, unsigned long srr0, u32 srr1)
>  {
>  #ifdef CONFIG_KVM_BOOKE_HV
> @@ -1332,6 +1350,7 @@ static void get_sregs_base(struct kvm_vcpu *vcpu,
>  	sregs->u.e.dec = kvmppc_get_dec(vcpu, tb);
>  	sregs->u.e.tb = tb;
>  	sregs->u.e.vrsave = vcpu->arch.vrsave;
> +	sregs->u.e.dbsr = vcpu->arch.dbsr;
>  }
>  
>  static int set_sregs_base(struct kvm_vcpu *vcpu,
> @@ -1356,6 +1375,14 @@ static int set_sregs_base(struct kvm_vcpu *vcpu,
>  	if (sregs->u.e.update_special & KVM_SREGS_E_UPDATE_TSR)
>  		kvmppc_set_tsr(vcpu, sregs->u.e.tsr);
>  
> +	if (sregs->u.e.update_special & KVM_SREGS_E_UPDATE_DBSR) {
> +		vcpu->arch.dbsr = sregs->u.e.dbsr;
> +		if (vcpu->arch.dbsr)
> +			kvmppc_core_queue_debug(vcpu);
> +		else
> +			kvmppc_core_dequeue_debug(vcpu);
> +	}
> +
>  	return 0;
>  }

one reg?
 
> diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c
> index 17e4562..ea724f2 100644
> --- a/arch/powerpc/kvm/e500mc.c
> +++ b/arch/powerpc/kvm/e500mc.c
> @@ -212,7 +212,7 @@ static int kvmppc_core_get_sregs_e500mc(struct kvm_vcpu *vcpu,
>  	struct kvmppc_vcpu_e500 *vcpu_e500 = to_e500(vcpu);
>  
>  	sregs->u.e.features |= KVM_SREGS_E_ARCH206_MMU | KVM_SREGS_E_PM |
> -			       KVM_SREGS_E_PC;
> +			       KVM_SREGS_E_PC | KVM_SREGS_E_ED;
>
>  	sregs->u.e.impl_id = KVM_SREGS_E_IMPL_FSL;
>  
>  	sregs->u.e.impl.fsl.features = 0;
> @@ -220,6 +220,9 @@ static int kvmppc_core_get_sregs_e500mc(struct kvm_vcpu *vcpu,
>  	sregs->u.e.impl.fsl.hid0 = vcpu_e500->hid0;
>  	sregs->u.e.impl.fsl.mcar = vcpu_e500->mcar;
>  
> +	sregs->u.e.dsrr0 = vcpu->arch.dsrr0;
> +	sregs->u.e.dsrr1 = vcpu->arch.dsrr1;
> +
>  	kvmppc_get_sregs_e500_tlb(vcpu, sregs);
>  
>  	sregs->u.e.ivor_high[3] =
> @@ -261,6 +264,11 @@ static int kvmppc_core_set_sregs_e500mc(struct kvm_vcpu *vcpu,
>  			sregs->u.e.ivor_high[5];
>  	}
>  
> +	if (sregs->u.e.features & KVM_SREGS_E_ED) {
> +		vcpu->arch.dsrr0 = sregs->u.e.dsrr0;
> +		vcpu->arch.dsrr1 = sregs->u.e.dsrr1;
> +	}

SPRG9?

-Scott

RE: [PATCH 2/2] KVM : powerpc/booke: Allow debug interrupt injection to guest

[Bharat.Bhushan]

> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Friday, June 27, 2014 11:53 PM
> To: Bhushan Bharat-R65777
> Cc: agraf@suse.de; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org
> Subject: Re: [PATCH 2/2] KVM : powerpc/booke: Allow debug interrupt injection to
> guest
> 
> On Fri, 2014-06-27 at 11:55 +0530, Bharat Bhushan wrote:
> > This patch allow userspace to inject debug interrupt to guest.
> >
> > Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com>
> 
> Could you describe how userspace plans to make use of this, and go into more
> detail about the changes you're making?

When a debug interrupt happens in guest then we switch to host userspace (QEMU) and if QEMU is not able to handle a debug interrupt then it injects the debug interrupt to guest. QEMU uses SET_SREGS (not a one_reg interface), with DBSR have proper values, for injecting the debug interrupt. In SET_SREGS handling for DBSR register, KVM injects debug interrupt to guest.

> 
> > ---
> >  arch/powerpc/kvm/booke.c  | 31 +++++++++++++++++++++++++++++--
> > arch/powerpc/kvm/e500mc.c | 10 +++++++++-
> >  2 files changed, 38 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index
> > bb25937..63ac38c 100644
> > --- a/arch/powerpc/kvm/booke.c
> > +++ b/arch/powerpc/kvm/booke.c
> > @@ -135,6 +135,11 @@ static void kvmppc_vcpu_sync_fpu(struct kvm_vcpu
> > *vcpu)  #endif  }
> >
> > +static int kvmppc_core_pending_debug(struct kvm_vcpu *vcpu) {
> > +	return test_bit(BOOKE_IRQPRIO_DEBUG,
> > +&vcpu->arch.pending_exceptions); }
> > +
> >  static void kvmppc_vcpu_sync_debug(struct kvm_vcpu *vcpu)  {
> >  	/* Synchronize guest's desire to get debug interrupts into shadow
> > MSR */ @@ -143,8 +148,11 @@ static void kvmppc_vcpu_sync_debug(struct kvm_vcpu
> *vcpu)
> >  	vcpu->arch.shadow_msr |= vcpu->arch.shared->msr & MSR_DE;  #endif
> >
> > -	/* Force enable debug interrupts when user space wants to debug */
> > -	if (vcpu->guest_debug) {
> > +	/*
> > +	 * Force enable debug interrupts when user space wants to debug
> > +	 * and there is no debug interrupt pending for guest to handle.
> > +	 */
> > +	if (vcpu->guest_debug && !kvmppc_core_pending_debug(vcpu)) {
> 
> Are you trying to allow the guest to be simultaneously debugged by itself and by
> host userspace?  How does this work?

Not actually, Currently we are not partitioning debug resources between host userspace and guest. In fact we do not emulate debug registers for guest. But we want host userspace to pass the interrupt to guest if it is not able to handle. 

> 
> >  #ifdef CONFIG_KVM_BOOKE_HV
> >  		/*
> >  		 * Since there is no shadow MSR, sync MSR_DE into the guest @@
> > -264,6 +272,16 @@ static void kvmppc_core_dequeue_watchdog(struct kvm_vcpu
> *vcpu)
> >  	clear_bit(BOOKE_IRQPRIO_WATCHDOG, &vcpu->arch.pending_exceptions);
> > }
> >
> > +static void kvmppc_core_queue_debug(struct kvm_vcpu *vcpu) {
> > +	kvmppc_booke_queue_irqprio(vcpu, BOOKE_IRQPRIO_DEBUG); }
> > +
> > +static void kvmppc_core_dequeue_debug(struct kvm_vcpu *vcpu) {
> > +	clear_bit(BOOKE_IRQPRIO_DEBUG, &vcpu->arch.pending_exceptions); }
> 
> Is there currently no support for a guest debugging itself (i.e.
> guest_debug unset) on e500v2?

Yes, It is not yet supported (IACx/DACx/DBCR/DBSR/DSRRx are not yet emulated). 

> 
> >  static void set_guest_srr(struct kvm_vcpu *vcpu, unsigned long srr0,
> > u32 srr1)  {  #ifdef CONFIG_KVM_BOOKE_HV @@ -1332,6 +1350,7 @@ static
> > void get_sregs_base(struct kvm_vcpu *vcpu,
> >  	sregs->u.e.dec = kvmppc_get_dec(vcpu, tb);
> >  	sregs->u.e.tb = tb;
> >  	sregs->u.e.vrsave = vcpu->arch.vrsave;
> > +	sregs->u.e.dbsr = vcpu->arch.dbsr;
> >  }
> >
> >  static int set_sregs_base(struct kvm_vcpu *vcpu, @@ -1356,6 +1375,14
> > @@ static int set_sregs_base(struct kvm_vcpu *vcpu,
> >  	if (sregs->u.e.update_special & KVM_SREGS_E_UPDATE_TSR)
> >  		kvmppc_set_tsr(vcpu, sregs->u.e.tsr);
> >
> > +	if (sregs->u.e.update_special & KVM_SREGS_E_UPDATE_DBSR) {
> > +		vcpu->arch.dbsr = sregs->u.e.dbsr;
> > +		if (vcpu->arch.dbsr)
> > +			kvmppc_core_queue_debug(vcpu);
> > +		else
> > +			kvmppc_core_dequeue_debug(vcpu);
> > +	}
> > +
> >  	return 0;
> >  }
> 
> one reg?

We are using SREGS but if required we can use one_reg.

> 
> > diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c
> > index 17e4562..ea724f2 100644
> > --- a/arch/powerpc/kvm/e500mc.c
> > +++ b/arch/powerpc/kvm/e500mc.c
> > @@ -212,7 +212,7 @@ static int kvmppc_core_get_sregs_e500mc(struct kvm_vcpu
> *vcpu,
> >  	struct kvmppc_vcpu_e500 *vcpu_e500 = to_e500(vcpu);
> >
> >  	sregs->u.e.features |= KVM_SREGS_E_ARCH206_MMU | KVM_SREGS_E_PM |
> > -			       KVM_SREGS_E_PC;
> > +			       KVM_SREGS_E_PC | KVM_SREGS_E_ED;
> >
> >  	sregs->u.e.impl_id = KVM_SREGS_E_IMPL_FSL;
> >
> >  	sregs->u.e.impl.fsl.features = 0;
> > @@ -220,6 +220,9 @@ static int kvmppc_core_get_sregs_e500mc(struct kvm_vcpu
> *vcpu,
> >  	sregs->u.e.impl.fsl.hid0 = vcpu_e500->hid0;
> >  	sregs->u.e.impl.fsl.mcar = vcpu_e500->mcar;
> >
> > +	sregs->u.e.dsrr0 = vcpu->arch.dsrr0;
> > +	sregs->u.e.dsrr1 = vcpu->arch.dsrr1;
> > +
> >  	kvmppc_get_sregs_e500_tlb(vcpu, sregs);
> >
> >  	sregs->u.e.ivor_high[3] =
> > @@ -261,6 +264,11 @@ static int kvmppc_core_set_sregs_e500mc(struct kvm_vcpu
> *vcpu,
> >  			sregs->u.e.ivor_high[5];
> >  	}
> >
> > +	if (sregs->u.e.features & KVM_SREGS_E_ED) {
> > +		vcpu->arch.dsrr0 = sregs->u.e.dsrr0;
> > +		vcpu->arch.dsrr1 = sregs->u.e.dsrr1;
> > +	}
> 
> SPRG9?

Yes we need to emulate this register as well, and other DEBUG registers.
So the question is, should this patch be sent with the patchset when we emulates other debug registers for guest?

> 
> -Scott
> 

Re: [PATCH 2/2] KVM : powerpc/booke: Allow debug interrupt injection to guest

[Scott Wood]

On Sun, 2014-06-29 at 23:38 -0500, Bhushan Bharat-R65777 wrote:
> 
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Friday, June 27, 2014 11:53 PM
> > To: Bhushan Bharat-R65777
> > Cc: agraf@suse.de; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org
> > Subject: Re: [PATCH 2/2] KVM : powerpc/booke: Allow debug interrupt injection to
> > guest
> > 
> > On Fri, 2014-06-27 at 11:55 +0530, Bharat Bhushan wrote:
> > > -	/* Force enable debug interrupts when user space wants to debug */
> > > -	if (vcpu->guest_debug) {
> > > +	/*
> > > +	 * Force enable debug interrupts when user space wants to debug
> > > +	 * and there is no debug interrupt pending for guest to handle.
> > > +	 */
> > > +	if (vcpu->guest_debug && !kvmppc_core_pending_debug(vcpu)) {
> > 
> > Are you trying to allow the guest to be simultaneously debugged by itself and by
> > host userspace?  How does this work?
> 
> Not actually, Currently we are not partitioning debug resources between
> host userspace and guest. In fact we do not emulate debug registers for
> guest. But we want host userspace to pass the interrupt to guest if it
> is not able to handle. 

I don't understand the logic here.  A debug interrupt should be injected
when the programming model in the guest says that a debug interrupt
should happen.  How can that occur currently?  If the guest didn't set
up the debug registers and QEMU still can't handle the debug interrupt,
that's a bug in QEMU (or KVM, or the hardware...).  Injecting the
interrupt into the guest just adds another bug on top of that.

> > >  #ifdef CONFIG_KVM_BOOKE_HV
> > >  		/*
> > >  		 * Since there is no shadow MSR, sync MSR_DE into the guest @@
> > > -264,6 +272,16 @@ static void kvmppc_core_dequeue_watchdog(struct kvm_vcpu
> > *vcpu)
> > >  	clear_bit(BOOKE_IRQPRIO_WATCHDOG, &vcpu->arch.pending_exceptions);
> > > }
> > >
> > > +static void kvmppc_core_queue_debug(struct kvm_vcpu *vcpu) {
> > > +	kvmppc_booke_queue_irqprio(vcpu, BOOKE_IRQPRIO_DEBUG); }
> > > +
> > > +static void kvmppc_core_dequeue_debug(struct kvm_vcpu *vcpu) {
> > > +	clear_bit(BOOKE_IRQPRIO_DEBUG, &vcpu->arch.pending_exceptions); }
> > 
> > Is there currently no support for a guest debugging itself (i.e.
> > guest_debug unset) on e500v2?
> 
> Yes, It is not yet supported (IACx/DACx/DBCR/DBSR/DSRRx are not yet emulated). 

How is it useful to inject a debug exception into the guest, until these
things are emulated?

> > > @@ static int set_sregs_base(struct kvm_vcpu *vcpu,
> > >  	if (sregs->u.e.update_special & KVM_SREGS_E_UPDATE_TSR)
> > >  		kvmppc_set_tsr(vcpu, sregs->u.e.tsr);
> > >
> > > +	if (sregs->u.e.update_special & KVM_SREGS_E_UPDATE_DBSR) {
> > > +		vcpu->arch.dbsr = sregs->u.e.dbsr;
> > > +		if (vcpu->arch.dbsr)
> > > +			kvmppc_core_queue_debug(vcpu);
> > > +		else
> > > +			kvmppc_core_dequeue_debug(vcpu);
> > > +	}
> > > +
> > >  	return 0;
> > >  }
> > 
> > one reg?
> 
> We are using SREGS but if required we can use one_reg.

I thought we were preferring one reg over sregs for new functionality.

-Scott

RE: [PATCH 2/2] KVM : powerpc/booke: Allow debug interrupt injection to guest

[Bharat.Bhushan]

> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Tuesday, July 01, 2014 1:56 AM
> To: Bhushan Bharat-R65777
> Cc: agraf@suse.de; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org
> Subject: Re: [PATCH 2/2] KVM : powerpc/booke: Allow debug interrupt injection to
> guest
> 
> On Sun, 2014-06-29 at 23:38 -0500, Bhushan Bharat-R65777 wrote:
> >
> > > -----Original Message-----
> > > From: Wood Scott-B07421
> > > Sent: Friday, June 27, 2014 11:53 PM
> > > To: Bhushan Bharat-R65777
> > > Cc: agraf@suse.de; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org
> > > Subject: Re: [PATCH 2/2] KVM : powerpc/booke: Allow debug interrupt
> > > injection to guest
> > >
> > > On Fri, 2014-06-27 at 11:55 +0530, Bharat Bhushan wrote:
> > > > -	/* Force enable debug interrupts when user space wants to debug */
> > > > -	if (vcpu->guest_debug) {
> > > > +	/*
> > > > +	 * Force enable debug interrupts when user space wants to debug
> > > > +	 * and there is no debug interrupt pending for guest to handle.
> > > > +	 */
> > > > +	if (vcpu->guest_debug && !kvmppc_core_pending_debug(vcpu)) {
> > >
> > > Are you trying to allow the guest to be simultaneously debugged by
> > > itself and by host userspace?  How does this work?
> >
> > Not actually, Currently we are not partitioning debug resources
> > between host userspace and guest. In fact we do not emulate debug
> > registers for guest. But we want host userspace to pass the interrupt
> > to guest if it is not able to handle.
> 
> I don't understand the logic here.  A debug interrupt should be injected when
> the programming model in the guest says that a debug interrupt should happen.
> How can that occur currently?  If the guest didn't set up the debug registers
> and QEMU still can't handle the debug interrupt, that's a bug in QEMU (or KVM,
> or the hardware...).  Injecting the interrupt into the guest just adds another
> bug on top of that.

Ok, Till we add support for guest to used debug resource, can we say that userspace will still try to inject debug interrupt (as it does not know guest capability) to guest but KVM will
 - clear guest dbsr
 - ratelimited_printk()

Suggestions please ?

Thanks
-Bharat

> 
> > > >  #ifdef CONFIG_KVM_BOOKE_HV
> > > >  		/*
> > > >  		 * Since there is no shadow MSR, sync MSR_DE into the guest
> @@
> > > > -264,6 +272,16 @@ static void kvmppc_core_dequeue_watchdog(struct
> > > > kvm_vcpu
> > > *vcpu)
> > > >  	clear_bit(BOOKE_IRQPRIO_WATCHDOG,
> > > > &vcpu->arch.pending_exceptions); }
> > > >
> > > > +static void kvmppc_core_queue_debug(struct kvm_vcpu *vcpu) {
> > > > +	kvmppc_booke_queue_irqprio(vcpu, BOOKE_IRQPRIO_DEBUG); }
> > > > +
> > > > +static void kvmppc_core_dequeue_debug(struct kvm_vcpu *vcpu) {
> > > > +	clear_bit(BOOKE_IRQPRIO_DEBUG, &vcpu->arch.pending_exceptions);
> > > > +}
> > >
> > > Is there currently no support for a guest debugging itself (i.e.
> > > guest_debug unset) on e500v2?
> >
> > Yes, It is not yet supported (IACx/DACx/DBCR/DBSR/DSRRx are not yet emulated).
> 
> How is it useful to inject a debug exception into the guest, until these things
> are emulated?
> 
> > > > @@ static int set_sregs_base(struct kvm_vcpu *vcpu,
> > > >  	if (sregs->u.e.update_special & KVM_SREGS_E_UPDATE_TSR)
> > > >  		kvmppc_set_tsr(vcpu, sregs->u.e.tsr);
> > > >
> > > > +	if (sregs->u.e.update_special & KVM_SREGS_E_UPDATE_DBSR) {
> > > > +		vcpu->arch.dbsr = sregs->u.e.dbsr;
> > > > +		if (vcpu->arch.dbsr)
> > > > +			kvmppc_core_queue_debug(vcpu);
> > > > +		else
> > > > +			kvmppc_core_dequeue_debug(vcpu);
> > > > +	}
> > > > +
> > > >  	return 0;
> > > >  }
> > >
> > > one reg?
> >
> > We are using SREGS but if required we can use one_reg.
> 
> I thought we were preferring one reg over sregs for new functionality.
> 
> -Scott
> 

Re: [PATCH 2/2] KVM : powerpc/booke: Allow debug interrupt injection to guest

[Alexander Graf]

> Am 30.06.2014 um 22:25 schrieb Scott Wood <scottwood@freescale.com>:
> 
>> On Sun, 2014-06-29 at 23:38 -0500, Bhushan Bharat-R65777 wrote:
>> 
>>> -----Original Message-----
>>> From: Wood Scott-B07421
>>> Sent: Friday, June 27, 2014 11:53 PM
>>> To: Bhushan Bharat-R65777
>>> Cc: agraf@suse.de; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org
>>> Subject: Re: [PATCH 2/2] KVM : powerpc/booke: Allow debug interrupt injection to
>>> guest
>>> 
>>>> On Fri, 2014-06-27 at 11:55 +0530, Bharat Bhushan wrote:
>>>> -    /* Force enable debug interrupts when user space wants to debug */
>>>> -    if (vcpu->guest_debug) {
>>>> +    /*
>>>> +     * Force enable debug interrupts when user space wants to debug
>>>> +     * and there is no debug interrupt pending for guest to handle.
>>>> +     */
>>>> +    if (vcpu->guest_debug && !kvmppc_core_pending_debug(vcpu)) {
>>> 
>>> Are you trying to allow the guest to be simultaneously debugged by itself and by
>>> host userspace?  How does this work?
>> 
>> Not actually, Currently we are not partitioning debug resources between
>> host userspace and guest. In fact we do not emulate debug registers for
>> guest. But we want host userspace to pass the interrupt to guest if it
>> is not able to handle.
> 
> I don't understand the logic here.  A debug interrupt should be injected
> when the programming model in the guest says that a debug interrupt
> should happen.  How can that occur currently?  If the guest didn't set
> up the debug registers and QEMU still can't handle the debug interrupt,
> that's a bug in QEMU (or KVM, or the hardware...).  Injecting the
> interrupt into the guest just adds another bug on top of that.

I don't think QEMU should be aware of these limitations.

> 
>>>> #ifdef CONFIG_KVM_BOOKE_HV
>>>>        /*
>>>>         * Since there is no shadow MSR, sync MSR_DE into the guest @@
>>>> -264,6 +272,16 @@ static void kvmppc_core_dequeue_watchdog(struct kvm_vcpu
>>> *vcpu)
>>>>    clear_bit(BOOKE_IRQPRIO_WATCHDOG, &vcpu->arch.pending_exceptions);
>>>> }
>>>> 
>>>> +static void kvmppc_core_queue_debug(struct kvm_vcpu *vcpu) {
>>>> +    kvmppc_booke_queue_irqprio(vcpu, BOOKE_IRQPRIO_DEBUG); }
>>>> +
>>>> +static void kvmppc_core_dequeue_debug(struct kvm_vcpu *vcpu) {
>>>> +    clear_bit(BOOKE_IRQPRIO_DEBUG, &vcpu->arch.pending_exceptions); }
>>> 
>>> Is there currently no support for a guest debugging itself (i.e.
>>> guest_debug unset) on e500v2?
>> 
>> Yes, It is not yet supported (IACx/DACx/DBCR/DBSR/DSRRx are not yet emulated).
> 
> How is it useful to inject a debug exception into the guest, until these
> things are emulated?

We don't have to touch QEMU later then ;). But I agree that it would make a lot of sense to enable guest debugging along the way - it can't be that hard, no?

> 
>>>> @@ static int set_sregs_base(struct kvm_vcpu *vcpu,
>>>>    if (sregs->u.e.update_special & KVM_SREGS_E_UPDATE_TSR)
>>>>        kvmppc_set_tsr(vcpu, sregs->u.e.tsr);
>>>> 
>>>> +    if (sregs->u.e.update_special & KVM_SREGS_E_UPDATE_DBSR) {
>>>> +        vcpu->arch.dbsr = sregs->u.e.dbsr;
>>>> +        if (vcpu->arch.dbsr)
>>>> +            kvmppc_core_queue_debug(vcpu);
>>>> +        else
>>>> +            kvmppc_core_dequeue_debug(vcpu);
>>>> +    }
>>>> +
>>>>    return 0;
>>>> }
>>> 
>>> one reg?
>> 
>> We are using SREGS but if required we can use one_reg.
> 
> I thought we were preferring one reg over sregs for new functionality.

I'm personally torn on this one. The problem here is that the sregs fields and values are already reserved. For anything we don't have an API for yet, yes, one_reg only. IIUC we have the API here, but were lacking the implementation.


Alex

RE: [PATCH 2/2] KVM : powerpc/booke: Allow debug interrupt injection to guest

[Bharat.Bhushan]

> -----Original Message-----
> From: Alexander Graf [mailto:agraf@suse.de]
> Sent: Tuesday, July 01, 2014 11:53 AM
> To: Wood Scott-B07421
> Cc: Bhushan Bharat-R65777; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org
> Subject: Re: [PATCH 2/2] KVM : powerpc/booke: Allow debug interrupt injection to
> guest
> 
> 
> 
> > Am 30.06.2014 um 22:25 schrieb Scott Wood <scottwood@freescale.com>:
> >
> >> On Sun, 2014-06-29 at 23:38 -0500, Bhushan Bharat-R65777 wrote:
> >>
> >>> -----Original Message-----
> >>> From: Wood Scott-B07421
> >>> Sent: Friday, June 27, 2014 11:53 PM
> >>> To: Bhushan Bharat-R65777
> >>> Cc: agraf@suse.de; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org
> >>> Subject: Re: [PATCH 2/2] KVM : powerpc/booke: Allow debug interrupt
> >>> injection to guest
> >>>
> >>>> On Fri, 2014-06-27 at 11:55 +0530, Bharat Bhushan wrote:
> >>>> -    /* Force enable debug interrupts when user space wants to debug */
> >>>> -    if (vcpu->guest_debug) {
> >>>> +    /*
> >>>> +     * Force enable debug interrupts when user space wants to debug
> >>>> +     * and there is no debug interrupt pending for guest to handle.
> >>>> +     */
> >>>> +    if (vcpu->guest_debug && !kvmppc_core_pending_debug(vcpu)) {
> >>>
> >>> Are you trying to allow the guest to be simultaneously debugged by
> >>> itself and by host userspace?  How does this work?
> >>
> >> Not actually, Currently we are not partitioning debug resources
> >> between host userspace and guest. In fact we do not emulate debug
> >> registers for guest. But we want host userspace to pass the interrupt
> >> to guest if it is not able to handle.
> >
> > I don't understand the logic here.  A debug interrupt should be
> > injected when the programming model in the guest says that a debug
> > interrupt should happen.  How can that occur currently?  If the guest
> > didn't set up the debug registers and QEMU still can't handle the
> > debug interrupt, that's a bug in QEMU (or KVM, or the hardware...).
> > Injecting the interrupt into the guest just adds another bug on top of that.
> 
> I don't think QEMU should be aware of these limitations.
> 
> >
> >>>> #ifdef CONFIG_KVM_BOOKE_HV
> >>>>        /*
> >>>>         * Since there is no shadow MSR, sync MSR_DE into the guest
> >>>> @@
> >>>> -264,6 +272,16 @@ static void kvmppc_core_dequeue_watchdog(struct
> >>>> kvm_vcpu
> >>> *vcpu)
> >>>>    clear_bit(BOOKE_IRQPRIO_WATCHDOG,
> >>>> &vcpu->arch.pending_exceptions); }
> >>>>
> >>>> +static void kvmppc_core_queue_debug(struct kvm_vcpu *vcpu) {
> >>>> +    kvmppc_booke_queue_irqprio(vcpu, BOOKE_IRQPRIO_DEBUG); }
> >>>> +
> >>>> +static void kvmppc_core_dequeue_debug(struct kvm_vcpu *vcpu) {
> >>>> +    clear_bit(BOOKE_IRQPRIO_DEBUG,
> >>>> +&vcpu->arch.pending_exceptions); }
> >>>
> >>> Is there currently no support for a guest debugging itself (i.e.
> >>> guest_debug unset) on e500v2?
> >>
> >> Yes, It is not yet supported (IACx/DACx/DBCR/DBSR/DSRRx are not yet
> emulated).
> >
> > How is it useful to inject a debug exception into the guest, until
> > these things are emulated?
> 
> We don't have to touch QEMU later then ;). But I agree that it would make a lot
> of sense to enable guest debugging along the way - it can't be that hard, no?

Copy pasting my response in another email:

"
Ok, Till we add support for guest to used debug resource, can we say that userspace will still try to inject debug interrupt (as it does not know guest capability) to guest but KVM will:
 - clear guest dbsr
 - ratelimited_printk()
"

Thanks
-Bharat

> 
> >
> >>>> @@ static int set_sregs_base(struct kvm_vcpu *vcpu,
> >>>>    if (sregs->u.e.update_special & KVM_SREGS_E_UPDATE_TSR)
> >>>>        kvmppc_set_tsr(vcpu, sregs->u.e.tsr);
> >>>>
> >>>> +    if (sregs->u.e.update_special & KVM_SREGS_E_UPDATE_DBSR) {
> >>>> +        vcpu->arch.dbsr = sregs->u.e.dbsr;
> >>>> +        if (vcpu->arch.dbsr)
> >>>> +            kvmppc_core_queue_debug(vcpu);
> >>>> +        else
> >>>> +            kvmppc_core_dequeue_debug(vcpu);
> >>>> +    }
> >>>> +
> >>>>    return 0;
> >>>> }
> >>>
> >>> one reg?
> >>
> >> We are using SREGS but if required we can use one_reg.
> >
> > I thought we were preferring one reg over sregs for new functionality.
> 
> I'm personally torn on this one. The problem here is that the sregs fields and
> values are already reserved. For anything we don't have an API for yet, yes,
> one_reg only. IIUC we have the API here, but were lacking the implementation.
> 
> 
> Alex

Re: [PATCH 2/2] KVM : powerpc/booke: Allow debug interrupt injection to guest

[Alexander Graf]

On 01.07.14 12:06, Bharat.Bhushan@freescale.com wrote:
>
>> -----Original Message-----
>> From: Alexander Graf [mailto:agraf@suse.de]
>> Sent: Tuesday, July 01, 2014 11:53 AM
>> To: Wood Scott-B07421
>> Cc: Bhushan Bharat-R65777; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org
>> Subject: Re: [PATCH 2/2] KVM : powerpc/booke: Allow debug interrupt injection to
>> guest
>>
>>
>>
>>> Am 30.06.2014 um 22:25 schrieb Scott Wood <scottwood@freescale.com>:
>>>
>>>> On Sun, 2014-06-29 at 23:38 -0500, Bhushan Bharat-R65777 wrote:
>>>>
>>>>> -----Original Message-----
>>>>> From: Wood Scott-B07421
>>>>> Sent: Friday, June 27, 2014 11:53 PM
>>>>> To: Bhushan Bharat-R65777
>>>>> Cc: agraf@suse.de; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org
>>>>> Subject: Re: [PATCH 2/2] KVM : powerpc/booke: Allow debug interrupt
>>>>> injection to guest
>>>>>
>>>>>> On Fri, 2014-06-27 at 11:55 +0530, Bharat Bhushan wrote:
>>>>>> -    /* Force enable debug interrupts when user space wants to debug */
>>>>>> -    if (vcpu->guest_debug) {
>>>>>> +    /*
>>>>>> +     * Force enable debug interrupts when user space wants to debug
>>>>>> +     * and there is no debug interrupt pending for guest to handle.
>>>>>> +     */
>>>>>> +    if (vcpu->guest_debug && !kvmppc_core_pending_debug(vcpu)) {
>>>>> Are you trying to allow the guest to be simultaneously debugged by
>>>>> itself and by host userspace?  How does this work?
>>>> Not actually, Currently we are not partitioning debug resources
>>>> between host userspace and guest. In fact we do not emulate debug
>>>> registers for guest. But we want host userspace to pass the interrupt
>>>> to guest if it is not able to handle.
>>> I don't understand the logic here.  A debug interrupt should be
>>> injected when the programming model in the guest says that a debug
>>> interrupt should happen.  How can that occur currently?  If the guest
>>> didn't set up the debug registers and QEMU still can't handle the
>>> debug interrupt, that's a bug in QEMU (or KVM, or the hardware...).
>>> Injecting the interrupt into the guest just adds another bug on top of that.
>> I don't think QEMU should be aware of these limitations.
>>
>>>>>> #ifdef CONFIG_KVM_BOOKE_HV
>>>>>>         /*
>>>>>>          * Since there is no shadow MSR, sync MSR_DE into the guest
>>>>>> @@
>>>>>> -264,6 +272,16 @@ static void kvmppc_core_dequeue_watchdog(struct
>>>>>> kvm_vcpu
>>>>> *vcpu)
>>>>>>     clear_bit(BOOKE_IRQPRIO_WATCHDOG,
>>>>>> &vcpu->arch.pending_exceptions); }
>>>>>>
>>>>>> +static void kvmppc_core_queue_debug(struct kvm_vcpu *vcpu) {
>>>>>> +    kvmppc_booke_queue_irqprio(vcpu, BOOKE_IRQPRIO_DEBUG); }
>>>>>> +
>>>>>> +static void kvmppc_core_dequeue_debug(struct kvm_vcpu *vcpu) {
>>>>>> +    clear_bit(BOOKE_IRQPRIO_DEBUG,
>>>>>> +&vcpu->arch.pending_exceptions); }
>>>>> Is there currently no support for a guest debugging itself (i.e.
>>>>> guest_debug unset) on e500v2?
>>>> Yes, It is not yet supported (IACx/DACx/DBCR/DBSR/DSRRx are not yet
>> emulated).
>>> How is it useful to inject a debug exception into the guest, until
>>> these things are emulated?
>> We don't have to touch QEMU later then ;). But I agree that it would make a lot
>> of sense to enable guest debugging along the way - it can't be that hard, no?
> Copy pasting my response in another email:
>
> "
> Ok, Till we add support for guest to used debug resource, can we say that userspace will still try to inject debug interrupt (as it does not know guest capability) to guest but KVM will:
>   - clear guest dbsr

Can we fake the value of DBSR that the guest sees?


Alex

RE: [PATCH 2/2] KVM : powerpc/booke: Allow debug interrupt injection to guest

[Bharat.Bhushan]

> -----Original Message-----
> From: Alexander Graf [mailto:agraf@suse.de]
> Sent: Tuesday, July 01, 2014 3:42 PM
> To: Bhushan Bharat-R65777; Wood Scott-B07421
> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org
> Subject: Re: [PATCH 2/2] KVM : powerpc/booke: Allow debug interrupt injection to
> guest
> 
> 
> On 01.07.14 12:06, Bharat.Bhushan@freescale.com wrote:
> >
> >> -----Original Message-----
> >> From: Alexander Graf [mailto:agraf@suse.de]
> >> Sent: Tuesday, July 01, 2014 11:53 AM
> >> To: Wood Scott-B07421
> >> Cc: Bhushan Bharat-R65777; kvm-ppc@vger.kernel.org;
> >> kvm@vger.kernel.org
> >> Subject: Re: [PATCH 2/2] KVM : powerpc/booke: Allow debug interrupt
> >> injection to guest
> >>
> >>
> >>
> >>> Am 30.06.2014 um 22:25 schrieb Scott Wood <scottwood@freescale.com>:
> >>>
> >>>> On Sun, 2014-06-29 at 23:38 -0500, Bhushan Bharat-R65777 wrote:
> >>>>
> >>>>> -----Original Message-----
> >>>>> From: Wood Scott-B07421
> >>>>> Sent: Friday, June 27, 2014 11:53 PM
> >>>>> To: Bhushan Bharat-R65777
> >>>>> Cc: agraf@suse.de; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org
> >>>>> Subject: Re: [PATCH 2/2] KVM : powerpc/booke: Allow debug
> >>>>> interrupt injection to guest
> >>>>>
> >>>>>> On Fri, 2014-06-27 at 11:55 +0530, Bharat Bhushan wrote:
> >>>>>> -    /* Force enable debug interrupts when user space wants to debug */
> >>>>>> -    if (vcpu->guest_debug) {
> >>>>>> +    /*
> >>>>>> +     * Force enable debug interrupts when user space wants to debug
> >>>>>> +     * and there is no debug interrupt pending for guest to handle.
> >>>>>> +     */
> >>>>>> +    if (vcpu->guest_debug && !kvmppc_core_pending_debug(vcpu)) {
> >>>>> Are you trying to allow the guest to be simultaneously debugged by
> >>>>> itself and by host userspace?  How does this work?
> >>>> Not actually, Currently we are not partitioning debug resources
> >>>> between host userspace and guest. In fact we do not emulate debug
> >>>> registers for guest. But we want host userspace to pass the
> >>>> interrupt to guest if it is not able to handle.
> >>> I don't understand the logic here.  A debug interrupt should be
> >>> injected when the programming model in the guest says that a debug
> >>> interrupt should happen.  How can that occur currently?  If the
> >>> guest didn't set up the debug registers and QEMU still can't handle
> >>> the debug interrupt, that's a bug in QEMU (or KVM, or the hardware...).
> >>> Injecting the interrupt into the guest just adds another bug on top of that.
> >> I don't think QEMU should be aware of these limitations.
> >>
> >>>>>> #ifdef CONFIG_KVM_BOOKE_HV
> >>>>>>         /*
> >>>>>>          * Since there is no shadow MSR, sync MSR_DE into the
> >>>>>> guest @@
> >>>>>> -264,6 +272,16 @@ static void kvmppc_core_dequeue_watchdog(struct
> >>>>>> kvm_vcpu
> >>>>> *vcpu)
> >>>>>>     clear_bit(BOOKE_IRQPRIO_WATCHDOG,
> >>>>>> &vcpu->arch.pending_exceptions); }
> >>>>>>
> >>>>>> +static void kvmppc_core_queue_debug(struct kvm_vcpu *vcpu) {
> >>>>>> +    kvmppc_booke_queue_irqprio(vcpu, BOOKE_IRQPRIO_DEBUG); }
> >>>>>> +
> >>>>>> +static void kvmppc_core_dequeue_debug(struct kvm_vcpu *vcpu) {
> >>>>>> +    clear_bit(BOOKE_IRQPRIO_DEBUG,
> >>>>>> +&vcpu->arch.pending_exceptions); }
> >>>>> Is there currently no support for a guest debugging itself (i.e.
> >>>>> guest_debug unset) on e500v2?
> >>>> Yes, It is not yet supported (IACx/DACx/DBCR/DBSR/DSRRx are not yet
> >> emulated).
> >>> How is it useful to inject a debug exception into the guest, until
> >>> these things are emulated?
> >> We don't have to touch QEMU later then ;). But I agree that it would
> >> make a lot of sense to enable guest debugging along the way - it can't be
> that hard, no?
> > Copy pasting my response in another email:
> >
> > "
> > Ok, Till we add support for guest to used debug resource, can we say that
> userspace will still try to inject debug interrupt (as it does not know guest
> capability) to guest but KVM will:
> >   - clear guest dbsr
> 
> Can we fake the value of DBSR that the guest sees?

Yes; we can hack the dbsr emulation with a comment :)
But why we want that?

Thanks
-Bharat

> 
> 
> Alex

Re: [PATCH 2/2] KVM : powerpc/booke: Allow debug interrupt injection to guest

[Alexander Graf]

On 01.07.14 12:30, Bharat.Bhushan@freescale.com wrote:
>
>> -----Original Message-----
>> From: Alexander Graf [mailto:agraf@suse.de]
>> Sent: Tuesday, July 01, 2014 3:42 PM
>> To: Bhushan Bharat-R65777; Wood Scott-B07421
>> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org
>> Subject: Re: [PATCH 2/2] KVM : powerpc/booke: Allow debug interrupt injection to
>> guest
>>
>>
>> On 01.07.14 12:06, Bharat.Bhushan@freescale.com wrote:
>>>> -----Original Message-----
>>>> From: Alexander Graf [mailto:agraf@suse.de]
>>>> Sent: Tuesday, July 01, 2014 11:53 AM
>>>> To: Wood Scott-B07421
>>>> Cc: Bhushan Bharat-R65777; kvm-ppc@vger.kernel.org;
>>>> kvm@vger.kernel.org
>>>> Subject: Re: [PATCH 2/2] KVM : powerpc/booke: Allow debug interrupt
>>>> injection to guest
>>>>
>>>>
>>>>
>>>>> Am 30.06.2014 um 22:25 schrieb Scott Wood <scottwood@freescale.com>:
>>>>>
>>>>>> On Sun, 2014-06-29 at 23:38 -0500, Bhushan Bharat-R65777 wrote:
>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Wood Scott-B07421
>>>>>>> Sent: Friday, June 27, 2014 11:53 PM
>>>>>>> To: Bhushan Bharat-R65777
>>>>>>> Cc: agraf@suse.de; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org
>>>>>>> Subject: Re: [PATCH 2/2] KVM : powerpc/booke: Allow debug
>>>>>>> interrupt injection to guest
>>>>>>>
>>>>>>>> On Fri, 2014-06-27 at 11:55 +0530, Bharat Bhushan wrote:
>>>>>>>> -    /* Force enable debug interrupts when user space wants to debug */
>>>>>>>> -    if (vcpu->guest_debug) {
>>>>>>>> +    /*
>>>>>>>> +     * Force enable debug interrupts when user space wants to debug
>>>>>>>> +     * and there is no debug interrupt pending for guest to handle.
>>>>>>>> +     */
>>>>>>>> +    if (vcpu->guest_debug && !kvmppc_core_pending_debug(vcpu)) {
>>>>>>> Are you trying to allow the guest to be simultaneously debugged by
>>>>>>> itself and by host userspace?  How does this work?
>>>>>> Not actually, Currently we are not partitioning debug resources
>>>>>> between host userspace and guest. In fact we do not emulate debug
>>>>>> registers for guest. But we want host userspace to pass the
>>>>>> interrupt to guest if it is not able to handle.
>>>>> I don't understand the logic here.  A debug interrupt should be
>>>>> injected when the programming model in the guest says that a debug
>>>>> interrupt should happen.  How can that occur currently?  If the
>>>>> guest didn't set up the debug registers and QEMU still can't handle
>>>>> the debug interrupt, that's a bug in QEMU (or KVM, or the hardware...).
>>>>> Injecting the interrupt into the guest just adds another bug on top of that.
>>>> I don't think QEMU should be aware of these limitations.
>>>>
>>>>>>>> #ifdef CONFIG_KVM_BOOKE_HV
>>>>>>>>          /*
>>>>>>>>           * Since there is no shadow MSR, sync MSR_DE into the
>>>>>>>> guest @@
>>>>>>>> -264,6 +272,16 @@ static void kvmppc_core_dequeue_watchdog(struct
>>>>>>>> kvm_vcpu
>>>>>>> *vcpu)
>>>>>>>>      clear_bit(BOOKE_IRQPRIO_WATCHDOG,
>>>>>>>> &vcpu->arch.pending_exceptions); }
>>>>>>>>
>>>>>>>> +static void kvmppc_core_queue_debug(struct kvm_vcpu *vcpu) {
>>>>>>>> +    kvmppc_booke_queue_irqprio(vcpu, BOOKE_IRQPRIO_DEBUG); }
>>>>>>>> +
>>>>>>>> +static void kvmppc_core_dequeue_debug(struct kvm_vcpu *vcpu) {
>>>>>>>> +    clear_bit(BOOKE_IRQPRIO_DEBUG,
>>>>>>>> +&vcpu->arch.pending_exceptions); }
>>>>>>> Is there currently no support for a guest debugging itself (i.e.
>>>>>>> guest_debug unset) on e500v2?
>>>>>> Yes, It is not yet supported (IACx/DACx/DBCR/DBSR/DSRRx are not yet
>>>> emulated).
>>>>> How is it useful to inject a debug exception into the guest, until
>>>>> these things are emulated?
>>>> We don't have to touch QEMU later then ;). But I agree that it would
>>>> make a lot of sense to enable guest debugging along the way - it can't be
>> that hard, no?
>>> Copy pasting my response in another email:
>>>
>>> "
>>> Ok, Till we add support for guest to used debug resource, can we say that
>> userspace will still try to inject debug interrupt (as it does not know guest
>> capability) to guest but KVM will:
>>>    - clear guest dbsr
>> Can we fake the value of DBSR that the guest sees?
> Yes; we can hack the dbsr emulation with a comment :)
> But why we want that?

I guess I don't fully grasp what you're proposing :).


Alex

Re: [PATCH 2/2] KVM : powerpc/booke: Allow debug interrupt injection to guest

[Scott Wood]

On Tue, 2014-07-01 at 08:23 +0200, Alexander Graf wrote:
> 
> > Am 30.06.2014 um 22:25 schrieb Scott Wood <scottwood@freescale.com>:
> > 
> >> On Sun, 2014-06-29 at 23:38 -0500, Bhushan Bharat-R65777 wrote:
> >> 
> >>> -----Original Message-----
> >>> From: Wood Scott-B07421
> >>> Sent: Friday, June 27, 2014 11:53 PM
> >>> To: Bhushan Bharat-R65777
> >>> Cc: agraf@suse.de; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org
> >>> Subject: Re: [PATCH 2/2] KVM : powerpc/booke: Allow debug interrupt injection to
> >>> guest
> >>> 
> >>>> On Fri, 2014-06-27 at 11:55 +0530, Bharat Bhushan wrote:
> >>>> -    /* Force enable debug interrupts when user space wants to debug */
> >>>> -    if (vcpu->guest_debug) {
> >>>> +    /*
> >>>> +     * Force enable debug interrupts when user space wants to debug
> >>>> +     * and there is no debug interrupt pending for guest to handle.
> >>>> +     */
> >>>> +    if (vcpu->guest_debug && !kvmppc_core_pending_debug(vcpu)) {
> >>> 
> >>> Are you trying to allow the guest to be simultaneously debugged by itself and by
> >>> host userspace?  How does this work?
> >> 
> >> Not actually, Currently we are not partitioning debug resources between
> >> host userspace and guest. In fact we do not emulate debug registers for
> >> guest. But we want host userspace to pass the interrupt to guest if it
> >> is not able to handle.
> > 
> > I don't understand the logic here.  A debug interrupt should be injected
> > when the programming model in the guest says that a debug interrupt
> > should happen.  How can that occur currently?  If the guest didn't set
> > up the debug registers and QEMU still can't handle the debug interrupt,
> > that's a bug in QEMU (or KVM, or the hardware...).  Injecting the
> > interrupt into the guest just adds another bug on top of that.
> 
> I don't think QEMU should be aware of these limitations.

OK, but we should at least have some idea of how the whole thing is
supposed to work, in order to determine if this is the correct behavior
for QEMU.  I thought the model was that debug resources are either owned
by QEMU or by the guest, and in the latter case, QEMU would never see
the debug exception to begin with.

> >>> one reg?
> >> 
> >> We are using SREGS but if required we can use one_reg.
> > 
> > I thought we were preferring one reg over sregs for new functionality.
> 
> I'm personally torn on this one. The problem here is that the sregs
> fields and values are already reserved. For anything we don't have an
> API for yet, yes, one_reg only. IIUC we have the API here, but were
> lacking the implementation.

>From a QEMU perspective, are we going to want to update this at the same
time as everything else, or would jumping through sregs hoops be a
nuisance?  Is the long term goal to have one reg support for everything?

-Scott

Re: [PATCH 2/2] KVM : powerpc/booke: Allow debug interrupt injection to guest

[Alexander Graf]

On 01.07.14 16:58, Scott Wood wrote:
> On Tue, 2014-07-01 at 08:23 +0200, Alexander Graf wrote:
>>> Am 30.06.2014 um 22:25 schrieb Scott Wood <scottwood@freescale.com>:
>>>
>>>> On Sun, 2014-06-29 at 23:38 -0500, Bhushan Bharat-R65777 wrote:
>>>>
>>>>> -----Original Message-----
>>>>> From: Wood Scott-B07421
>>>>> Sent: Friday, June 27, 2014 11:53 PM
>>>>> To: Bhushan Bharat-R65777
>>>>> Cc: agraf@suse.de; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org
>>>>> Subject: Re: [PATCH 2/2] KVM : powerpc/booke: Allow debug interrupt injection to
>>>>> guest
>>>>>
>>>>>> On Fri, 2014-06-27 at 11:55 +0530, Bharat Bhushan wrote:
>>>>>> -    /* Force enable debug interrupts when user space wants to debug */
>>>>>> -    if (vcpu->guest_debug) {
>>>>>> +    /*
>>>>>> +     * Force enable debug interrupts when user space wants to debug
>>>>>> +     * and there is no debug interrupt pending for guest to handle.
>>>>>> +     */
>>>>>> +    if (vcpu->guest_debug && !kvmppc_core_pending_debug(vcpu)) {
>>>>> Are you trying to allow the guest to be simultaneously debugged by itself and by
>>>>> host userspace?  How does this work?
>>>> Not actually, Currently we are not partitioning debug resources between
>>>> host userspace and guest. In fact we do not emulate debug registers for
>>>> guest. But we want host userspace to pass the interrupt to guest if it
>>>> is not able to handle.
>>> I don't understand the logic here.  A debug interrupt should be injected
>>> when the programming model in the guest says that a debug interrupt
>>> should happen.  How can that occur currently?  If the guest didn't set
>>> up the debug registers and QEMU still can't handle the debug interrupt,
>>> that's a bug in QEMU (or KVM, or the hardware...).  Injecting the
>>> interrupt into the guest just adds another bug on top of that.
>> I don't think QEMU should be aware of these limitations.
> OK, but we should at least have some idea of how the whole thing is
> supposed to work, in order to determine if this is the correct behavior
> for QEMU.  I thought the model was that debug resources are either owned
> by QEMU or by the guest, and in the latter case, QEMU would never see
> the debug exception to begin with.

That's bad for a number of reasons. For starters it's different from how 
x86 handles debug registers - and I hate to be different just for the 
sake of being different. So if we do want to declare that debug 
registers are owned by either QEMU or the guest, we should change the 
semantics for all architectures.

The second problem I see is that we're disabling use cases for the sake 
of correctness. When I use gdbstub, I usually don't want anything in the 
way of debugging something - like if I want to debug the guest debugging 
itself for example.

So overall, I think the x86 approach is a nice middle ground between 
usability, performance and functionality.

>
>>>>> one reg?
>>>> We are using SREGS but if required we can use one_reg.
>>> I thought we were preferring one reg over sregs for new functionality.
>> I'm personally torn on this one. The problem here is that the sregs
>> fields and values are already reserved. For anything we don't have an
>> API for yet, yes, one_reg only. IIUC we have the API here, but were
>> lacking the implementation.
>  From a QEMU perspective, are we going to want to update this at the same
> time as everything else, or would jumping through sregs hoops be a
> nuisance?  Is the long term goal to have one reg support for everything?

Because we need to maintain backwards compatibility, I don't think we'll 
actually have any benefit from one reg support for everything.

I think we're in a path that is slow enough already to not worry about 
performance. I think we're good to just call the sregs setter on demand 
when we want to change single registers.


Alex

Re: [PATCH 2/2] KVM : powerpc/booke: Allow debug interrupt injection to guest

[Scott Wood]

On Tue, 2014-07-01 at 17:04 +0200, Alexander Graf wrote:
> On 01.07.14 16:58, Scott Wood wrote:
> > On Tue, 2014-07-01 at 08:23 +0200, Alexander Graf wrote:
> >> I don't think QEMU should be aware of these limitations.
> > OK, but we should at least have some idea of how the whole thing is
> > supposed to work, in order to determine if this is the correct behavior
> > for QEMU.  I thought the model was that debug resources are either owned
> > by QEMU or by the guest, and in the latter case, QEMU would never see
> > the debug exception to begin with.
> 
> That's bad for a number of reasons. For starters it's different from how 
> x86 handles debug registers - and I hate to be different just for the 
> sake of being different.

How does it work on x86?

> So if we do want to declare that debug registers are owned by either
> QEMU or the guest, we should change the semantics for all
> architectures.

If we want to say that ownership of the registers is shared, we need a
plan for how that would actually work.

> The second problem I see is that we're disabling use cases for the sake 
> of correctness. When I use gdbstub, I usually don't want anything in the 
> way of debugging something - like if I want to debug the guest debugging 
> itself for example.

I don't have a problem with making it possible for userspace to forcibly
claim ownership of the debug registers -- but I don't want to advertise
that won't mess up the guest debugging that you're trying to debug,
without some reason to believe that.

> So overall, I think the x86 approach is a nice middle ground between 
> usability, performance and functionality.

You mean don't document anything other than the mechanism to get/set the
registers and hope everything works out? :-)

> >>>>> one reg?
> >>>> We are using SREGS but if required we can use one_reg.
> >>> I thought we were preferring one reg over sregs for new functionality.
> >> I'm personally torn on this one. The problem here is that the sregs
> >> fields and values are already reserved. For anything we don't have an
> >> API for yet, yes, one_reg only. IIUC we have the API here, but were
> >> lacking the implementation.
> >  From a QEMU perspective, are we going to want to update this at the same
> > time as everything else, or would jumping through sregs hoops be a
> > nuisance?  Is the long term goal to have one reg support for everything?
> 
> Because we need to maintain backwards compatibility, I don't think we'll 
> actually have any benefit from one reg support for everything.

We don't need to maintain backwards compatibility with interfaces that
were never implemented, and even where we do need to maintain
compatibility, that doesn't mean the caller needs to use the old,
cumbersome interface (don't assume the caller is QEMU, or that all new
features of QEMU need to support old kernels).

> I think we're in a path that is slow enough already to not worry about 
> performance.

It's not just about performance, but simplicity of use, and consistency
of API.

Oh, and it looks like there already exist one reg definitions and
implementations for most of the debug registers.

-Scott

Re: [PATCH 2/2] KVM : powerpc/booke: Allow debug interrupt injection to guest

[Alexander Graf]

On 01.07.14 17:35, Scott Wood wrote:
> On Tue, 2014-07-01 at 17:04 +0200, Alexander Graf wrote:
>> On 01.07.14 16:58, Scott Wood wrote:
>>> On Tue, 2014-07-01 at 08:23 +0200, Alexander Graf wrote:
>>>> I don't think QEMU should be aware of these limitations.
>>> OK, but we should at least have some idea of how the whole thing is
>>> supposed to work, in order to determine if this is the correct behavior
>>> for QEMU.  I thought the model was that debug resources are either owned
>>> by QEMU or by the guest, and in the latter case, QEMU would never see
>>> the debug exception to begin with.
>> That's bad for a number of reasons. For starters it's different from how
>> x86 handles debug registers - and I hate to be different just for the
>> sake of being different.
> How does it work on x86?

It overwrites more-or-less random breakpoints with its own ones, but 
leaves the others intact ;).

>
>> So if we do want to declare that debug registers are owned by either
>> QEMU or the guest, we should change the semantics for all
>> architectures.
> If we want to say that ownership of the registers is shared, we need a
> plan for how that would actually work.

I think you're overengineering here :). When do people actually use 
gdbstub? Usually when they want to debug a broken guest. We can either

   * overengineer heavily and reduce the number of registers available 
to the guest to always have spares
   * overengineer a bit and turn off guest debugging completely when we 
use gdbstub
   * just leave as much alive as we can, hoping that it helps with the 
debugging

Option 3 is what x86 does - and I think it's a reasonable approach. This 
is not an interface that needs to be 100% consistent and bullet proof, 
it's a best effort to enable you to debug as much as possible.

>
>> The second problem I see is that we're disabling use cases for the sake
>> of correctness. When I use gdbstub, I usually don't want anything in the
>> way of debugging something - like if I want to debug the guest debugging
>> itself for example.
> I don't have a problem with making it possible for userspace to forcibly
> claim ownership of the debug registers -- but I don't want to advertise
> that won't mess up the guest debugging that you're trying to debug,
> without some reason to believe that.

It will mes up guest debugging, but only up to the extent that's 
necessary. We can even try to find unused breakpoint registers before we 
start killing guest ones.

>
>> So overall, I think the x86 approach is a nice middle ground between
>> usability, performance and functionality.
> You mean don't document anything other than the mechanism to get/set the
> registers and hope everything works out? :-)

There's certainly some lack of documentation here ;). Otherwise we 
probably wouldn't have this conversation :).

>
>>>>>>> one reg?
>>>>>> We are using SREGS but if required we can use one_reg.
>>>>> I thought we were preferring one reg over sregs for new functionality.
>>>> I'm personally torn on this one. The problem here is that the sregs
>>>> fields and values are already reserved. For anything we don't have an
>>>> API for yet, yes, one_reg only. IIUC we have the API here, but were
>>>> lacking the implementation.
>>>   From a QEMU perspective, are we going to want to update this at the same
>>> time as everything else, or would jumping through sregs hoops be a
>>> nuisance?  Is the long term goal to have one reg support for everything?
>> Because we need to maintain backwards compatibility, I don't think we'll
>> actually have any benefit from one reg support for everything.
> We don't need to maintain backwards compatibility with interfaces that
> were never implemented, and even where we do need to maintain
> compatibility, that doesn't mean the caller needs to use the old,
> cumbersome interface (don't assume the caller is QEMU, or that all new
> features of QEMU need to support old kernels).

Yes, that's why I'm torn for this particular case. I thought you were 
asking about the general line of thinking. In this concrete case I think 
the only thing we get from using SREGS over ONE_REG is that we don't 
have to wait for the patches to trickle into kvm-next to be able to 
apply the QEMU patches ;).

 From an esthetic point of view, I would prefer ONE_REG too.

>
>> I think we're in a path that is slow enough already to not worry about
>> performance.
> It's not just about performance, but simplicity of use, and consistency
> of API.
>
> Oh, and it looks like there already exist one reg definitions and
> implementations for most of the debug registers.

For BookE? Where?


Alex

Re: [PATCH 2/2] KVM : powerpc/booke: Allow debug interrupt injection to guest

[Scott Wood]

On Tue, 2014-07-01 at 18:22 +0200, Alexander Graf wrote:
> On 01.07.14 17:35, Scott Wood wrote:
> > On Tue, 2014-07-01 at 17:04 +0200, Alexander Graf wrote:
> >> On 01.07.14 16:58, Scott Wood wrote:
> >>> On Tue, 2014-07-01 at 08:23 +0200, Alexander Graf wrote:
> >>>> I don't think QEMU should be aware of these limitations.
> >>> OK, but we should at least have some idea of how the whole thing is
> >>> supposed to work, in order to determine if this is the correct behavior
> >>> for QEMU.  I thought the model was that debug resources are either owned
> >>> by QEMU or by the guest, and in the latter case, QEMU would never see
> >>> the debug exception to begin with.
> >> That's bad for a number of reasons. For starters it's different from how
> >> x86 handles debug registers - and I hate to be different just for the
> >> sake of being different.
> > How does it work on x86?
> 
> It overwrites more-or-less random breakpoints with its own ones, but 
> leaves the others intact ;).

Are you talking about software breakpoints or management of hardware
debug registers?

> >> So if we do want to declare that debug registers are owned by either
> >> QEMU or the guest, we should change the semantics for all
> >> architectures.
> > If we want to say that ownership of the registers is shared, we need a
> > plan for how that would actually work.
> 
> I think you're overengineering here :). When do people actually use 
> gdbstub? Usually when they want to debug a broken guest. We can either
> 
>    * overengineer heavily and reduce the number of registers available 
> to the guest to always have spares
>    * overengineer a bit and turn off guest debugging completely when we 
> use gdbstub
>    * just leave as much alive as we can, hoping that it helps with the 
> debugging
> 
> Option 3 is what x86 does - and I think it's a reasonable approach. This 
> is not an interface that needs to be 100% consistent and bullet proof, 
> it's a best effort to enable you to debug as much as possible.

I'm not insisting on 100% -- just hoping for some explanation/discussion
about how it's intended to work for the cases where it can.

How will MSR[DE] and MSRP[DEP] be handled?

How would I go about telling QEMU/KVM that I don't want this shared
mode, because I don't want guest to interfere with the debugging I'm
trying to do from QEMU?

Will guest accesses to debug registers cause a userspace exit when
guest_debug is enabled?

> >> I think we're in a path that is slow enough already to not worry about
> >> performance.
> > It's not just about performance, but simplicity of use, and consistency
> > of API.
> >
> > Oh, and it looks like there already exist one reg definitions and
> > implementations for most of the debug registers.
> 
> For BookE? Where?

arch/powerpc/kvm/booke.c: KVM_REG_PPC_IACn, KVM_REG_PPC_DACn

-Scott

RE: [PATCH 2/2] KVM : powerpc/booke: Allow debug interrupt injection to guest

[Bharat.Bhushan]

> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Tuesday, July 01, 2014 10:11 PM
> To: Alexander Graf
> Cc: Bhushan Bharat-R65777; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org
> Subject: Re: [PATCH 2/2] KVM : powerpc/booke: Allow debug interrupt injection to
> guest
> 
> On Tue, 2014-07-01 at 18:22 +0200, Alexander Graf wrote:
> > On 01.07.14 17:35, Scott Wood wrote:
> > > On Tue, 2014-07-01 at 17:04 +0200, Alexander Graf wrote:
> > >> On 01.07.14 16:58, Scott Wood wrote:
> > >>> On Tue, 2014-07-01 at 08:23 +0200, Alexander Graf wrote:
> > >>>> I don't think QEMU should be aware of these limitations.
> > >>> OK, but we should at least have some idea of how the whole thing
> > >>> is supposed to work, in order to determine if this is the correct
> > >>> behavior for QEMU.  I thought the model was that debug resources
> > >>> are either owned by QEMU or by the guest, and in the latter case,
> > >>> QEMU would never see the debug exception to begin with.
> > >> That's bad for a number of reasons. For starters it's different
> > >> from how
> > >> x86 handles debug registers - and I hate to be different just for
> > >> the sake of being different.
> > > How does it work on x86?
> >
> > It overwrites more-or-less random breakpoints with its own ones, but
> > leaves the others intact ;).
> 
> Are you talking about software breakpoints or management of hardware debug
> registers?
> 
> > >> So if we do want to declare that debug registers are owned by
> > >> either QEMU or the guest, we should change the semantics for all
> > >> architectures.
> > > If we want to say that ownership of the registers is shared, we need
> > > a plan for how that would actually work.
> >
> > I think you're overengineering here :). When do people actually use
> > gdbstub? Usually when they want to debug a broken guest. We can either
> >
> >    * overengineer heavily and reduce the number of registers available
> > to the guest to always have spares
> >    * overengineer a bit and turn off guest debugging completely when
> > we use gdbstub
> >    * just leave as much alive as we can, hoping that it helps with the
> > debugging
> >
> > Option 3 is what x86 does - and I think it's a reasonable approach.
> > This is not an interface that needs to be 100% consistent and bullet
> > proof, it's a best effort to enable you to debug as much as possible.
> 
> I'm not insisting on 100% -- just hoping for some explanation/discussion about
> how it's intended to work for the cases where it can.
> 
> How will MSR[DE] and MSRP[DEP] be handled?
> 
> How would I go about telling QEMU/KVM that I don't want this shared mode,
> because I don't want guest to interfere with the debugging I'm trying to do from
> QEMU?
> 
> Will guest accesses to debug registers cause a userspace exit when guest_debug
> is enabled?
> 
> > >> I think we're in a path that is slow enough already to not worry
> > >> about performance.
> > > It's not just about performance, but simplicity of use, and
> > > consistency of API.
> > >
> > > Oh, and it looks like there already exist one reg definitions and
> > > implementations for most of the debug registers.
> >
> > For BookE? Where?
> 
> arch/powerpc/kvm/booke.c: KVM_REG_PPC_IACn, KVM_REG_PPC_DACn

I tried to quickly prototype what I think we want to do (this is not tested)

-----------------------------------------------------------------------
diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
index e8b3982..746b5c6 100644
--- a/arch/powerpc/include/asm/kvm_ppc.h
+++ b/arch/powerpc/include/asm/kvm_ppc.h
@@ -179,6 +179,9 @@ extern int kvmppc_xics_get_xive(struct kvm *kvm, u32 irq, u32 *server,
 extern int kvmppc_xics_int_on(struct kvm *kvm, u32 irq);
 extern int kvmppc_xics_int_off(struct kvm *kvm, u32 irq);
 
+void kvmppc_core_dequeue_debug(struct kvm_vcpu *vcpu);
+void kvmppc_core_queue_debug(struct kvm_vcpu *vcpu);
+
 /*
  * Cuts out inst bits with ordering according to spec.
  * That means the leftmost bit is zero. All given bits are included.
diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index 9f13056..0b7e4e4 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -235,6 +235,16 @@ void kvmppc_core_dequeue_dec(struct kvm_vcpu *vcpu)
 	clear_bit(BOOKE_IRQPRIO_DECREMENTER, &vcpu->arch.pending_exceptions);
 }
 
+void kvmppc_core_queue_debug(struct kvm_vcpu *vcpu)
+{
+	kvmppc_booke_queue_irqprio(vcpu, BOOKE_IRQPRIO_DEBUG);
+}
+ 
+void kvmppc_core_dequeue_debug(struct kvm_vcpu *vcpu)
+{
+	clear_bit(BOOKE_IRQPRIO_DEBUG, &vcpu->arch.pending_exceptions);
+}
+
 void kvmppc_core_queue_external(struct kvm_vcpu *vcpu,
                                 struct kvm_interrupt *irq)
 {
@@ -841,6 +851,20 @@ static int kvmppc_handle_debug(struct kvm_run *run, struct kvm_vcpu *vcpu)
 	struct debug_reg *dbg_reg = &(vcpu->arch.shadow_dbg_reg);
 	u32 dbsr = vcpu->arch.dbsr;
 
+	/* Userspace (QEMU) is not using debug resource, so inject debug interrupt
+	 * directly to guest debug.
+	 */
+	if (vcpu->guest_debug == 0) {
+		if (dbsr && (vcpu->arch.shared->msr & MSR_DE))
+			kvmppc_core_queue_debug(vcpu);
+
+		/* Inject a program interrupt if trap debug is not allowed */
+		if ((dbsr & DBSR_TIE) && !(vcpu->arch.shared->msr & MSR_DE))
+			kvmppc_core_queue_program(vcpu, ESR_PTR);
+
+		return RESUME_GUEST;
+	}
+
 	run->debug.arch.status = 0;
 	run->debug.arch.address = vcpu->arch.pc;
 
@@ -1920,7 +1944,7 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
 	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;
+//	vcpu->arch.dbg_reg.dbcr0 = DBCR0_EDM;
 
 	if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP)
 		vcpu->arch.shadow_dbg_reg.dbcr0 |= DBCR0_IDM | DBCR0_IC;
diff --git a/arch/powerpc/kvm/booke_emulate.c b/arch/powerpc/kvm/booke_emulate.c
index aaff1b7..ef8de7f 100644
--- a/arch/powerpc/kvm/booke_emulate.c
+++ b/arch/powerpc/kvm/booke_emulate.c
@@ -26,6 +26,7 @@
 #define OP_19_XOP_RFMCI   38
 #define OP_19_XOP_RFI     50
 #define OP_19_XOP_RFCI    51
+#define OP_19_XOP_RFDI    39
 
 #define OP_31_XOP_MFMSR   83
 #define OP_31_XOP_WRTEE   131
@@ -38,10 +39,24 @@ static void kvmppc_emul_rfi(struct kvm_vcpu *vcpu)
 	kvmppc_set_msr(vcpu, vcpu->arch.shared->srr1);
 }
 
+static void kvmppc_emul_rfdi(struct kvm_vcpu *vcpu)
+{
+	vcpu->arch.pc = vcpu->arch.dsrr0;
+	/* Force MSR_DE when guest does not own debug facilities */
+	if (vcpu->guest_debug)
+		kvmppc_set_msr(vcpu, vcpu->arch.dsrr1 | MSR_DE);
+	else
+		kvmppc_set_msr(vcpu, vcpu->arch.dsrr1);
+}
+
 static void kvmppc_emul_rfci(struct kvm_vcpu *vcpu)
 {
 	vcpu->arch.pc = vcpu->arch.csrr0;
-	kvmppc_set_msr(vcpu, vcpu->arch.csrr1);
+	/* Force MSR_DE when guest does not own debug facilities */
+	if (vcpu->guest_debug)
+		kvmppc_set_msr(vcpu, vcpu->arch.csrr1 | MSR_DE);
+	else
+		kvmppc_set_msr(vcpu, vcpu->arch.csrr1);
 }
 
 static void kvmppc_emul_rfmci(struct kvm_vcpu *vcpu)
@@ -78,6 +93,12 @@ int kvmppc_booke_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu,
 			*advance = 0;
 			break;
 
+		case OP_19_XOP_RFDI:
+			kvmppc_emul_rfdi(vcpu);
+//			kvmppc_set_exit_type(vcpu, EMULATED_RFDI_EXITS);
+			*advance = 0;
+			break;
+
 		default:
 			emulated = EMULATE_FAIL;
 			break;
@@ -131,6 +152,7 @@ int kvmppc_booke_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu,
 int kvmppc_booke_emulate_mtspr(struct kvm_vcpu *vcpu, int sprn, ulong spr_val)
 {
 	int emulated = EMULATE_DONE;
+	bool debug_inst = false;
 
 	switch (sprn) {
 	case SPRN_DEAR:
@@ -145,21 +167,74 @@ int kvmppc_booke_emulate_mtspr(struct kvm_vcpu *vcpu, int sprn, ulong spr_val)
 	case SPRN_CSRR1:
 		vcpu->arch.csrr1 = spr_val;
 		break;
-	case SPRN_DBCR0:
-		vcpu->arch.dbg_reg.dbcr0 = spr_val;
+	case SPRN_DSRR0:
+		vcpu->arch.dsrr0 = spr_val;
 		break;
-	case SPRN_DBCR1:
-		vcpu->arch.dbg_reg.dbcr1 = spr_val;
+	case SPRN_DSRR1:
+		vcpu->arch.dsrr1 = spr_val;
+		break;
+	case SPRN_IAC1:
+		debug_inst = true;
+		vcpu->arch.dbg_reg.iac1 = spr_val;
+		vcpu->arch.shadow_dbg_reg.iac1 = spr_val;
+		break;
+	case SPRN_IAC2:
+		debug_inst = true;
+		vcpu->arch.dbg_reg.iac2 = spr_val;
+		vcpu->arch.shadow_dbg_reg.iac2 = spr_val;
+		break;
+#ifndef CONFIG_PPC_FSL_BOOK3E
+	case SPRN_IAC3:
+		debug_inst = true;
+		vcpu->arch.dbg_reg.iac3 = spr_val;
+		vcpu->arch.shadow_dbg_reg.iac3 = spr_val;
 		break;
+	case SPRN_IAC4:
+		debug_inst = true;
+		vcpu->arch.dbg_reg.iac4 = spr_val;
+		vcpu->arch.shadow_dbg_reg.iac4 = spr_val;
+		break;
+#endif
+	case SPRN_DAC1:
+		debug_inst = true;
+		vcpu->arch.dbg_reg.dac1 = spr_val;
+		vcpu->arch.shadow_dbg_reg.dac1 = spr_val;
+		break;
+	case SPRN_DAC2:
+		debug_inst = true;
+		vcpu->arch.dbg_reg.dac2 = spr_val;
+		vcpu->arch.shadow_dbg_reg.dac2 = spr_val;
+		break;
+ 	case SPRN_DBCR0:
+		debug_inst = true;
+		spr_val &= (DBCR0_IDM | DBCR0_IC | DBCR0_BT | DBCR0_TIE |
+			DBCR0_IAC1 | DBCR0_IAC2 | DBCR0_IAC3 | DBCR0_IAC4  |
+			DBCR0_DAC1R | DBCR0_DAC1W | DBCR0_DAC2R | DBCR0_DAC2W);
+
+ 		vcpu->arch.dbg_reg.dbcr0 = spr_val;
+		vcpu->arch.shadow_dbg_reg.dbcr0 = spr_val;
+ 		break;
+ 	case SPRN_DBCR1:
+		debug_inst = true;
+ 		vcpu->arch.dbg_reg.dbcr1 = spr_val;
+		vcpu->arch.shadow_dbg_reg.dbcr1 = spr_val;
+		break;
+	case SPRN_DBCR2:
+		debug_inst = true;
+		vcpu->arch.dbg_reg.dbcr2 = spr_val;
+		vcpu->arch.shadow_dbg_reg.dbcr2 = spr_val;
+ 		break;
+ 	case SPRN_DBSR:
+ 		vcpu->arch.dbsr &= ~spr_val;
+		if (vcpu->arch.dbsr == 0)
+			kvmppc_core_dequeue_debug(vcpu);
+ 		break;
 	case SPRN_MCSRR0:
 		vcpu->arch.mcsrr0 = spr_val;
 		break;
 	case SPRN_MCSRR1:
 		vcpu->arch.mcsrr1 = spr_val;
 		break;
-	case SPRN_DBSR:
-		vcpu->arch.dbsr &= ~spr_val;
-		break;
 	case SPRN_TSR:
 		kvmppc_clr_tsr_bits(vcpu, spr_val);
 		break;
@@ -271,6 +346,10 @@ int kvmppc_booke_emulate_mtspr(struct kvm_vcpu *vcpu, int sprn, ulong spr_val)
 		emulated = EMULATE_FAIL;
 	}
 
+	if (debug_inst) {
+		switch_booke_debug_regs(&vcpu->arch.shadow_dbg_reg);
+		current->thread.debug = vcpu->arch.shadow_dbg_reg;
+	}
 	return emulated;
 }
 
@@ -297,12 +376,41 @@ int kvmppc_booke_emulate_mfspr(struct kvm_vcpu *vcpu, int sprn, ulong *spr_val)
 	case SPRN_CSRR1:
 		*spr_val = vcpu->arch.csrr1;
 		break;
+	case SPRN_DSRR0:
+		*spr_val = vcpu->arch.dsrr0;
+		break;
+	case SPRN_DSRR1:
+		*spr_val = vcpu->arch.dsrr1;
+		break;
+	case SPRN_IAC1:
+		*spr_val = vcpu->arch.dbg_reg.iac1;
+		break;
+	case SPRN_IAC2:
+		*spr_val = vcpu->arch.dbg_reg.iac2;
+		break;
+#ifndef CONFIG_PPC_FSL_BOOK3E
+	case SPRN_IAC3:
+		*spr_val = vcpu->arch.dbg_reg.iac3;
+		break;
+	case SPRN_IAC4:
+		*spr_val = vcpu->arch.dbg_reg.iac4;
+		break;
+#endif
+	case SPRN_DAC1:
+		*spr_val = vcpu->arch.dbg_reg.dac1;
+		break;
+	case SPRN_DAC2:
+		*spr_val = vcpu->arch.dbg_reg.dac2;
+		break;
 	case SPRN_DBCR0:
 		*spr_val = vcpu->arch.dbg_reg.dbcr0;
 		break;
 	case SPRN_DBCR1:
 		*spr_val = vcpu->arch.dbg_reg.dbcr1;
 		break;
+	case SPRN_DBCR2:
+		*spr_val = vcpu->arch.dbg_reg.dbcr2;
+		break;
 	case SPRN_MCSRR0:
 		*spr_val = vcpu->arch.mcsrr0;
 		break;
diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c
index bbf7cdd..560b576 100644
--- a/arch/powerpc/kvm/e500mc.c
+++ b/arch/powerpc/kvm/e500mc.c
@@ -249,7 +249,7 @@ int kvmppc_core_vcpu_setup(struct kvm_vcpu *vcpu)
 #ifdef CONFIG_64BIT
 	vcpu->arch.shadow_epcr |= SPRN_EPCR_ICM;
 #endif
-	vcpu->arch.shadow_msrp = MSRP_UCLEP | MSRP_DEP | MSRP_PMMP;
+	vcpu->arch.shadow_msrp = MSRP_UCLEP | MSRP_PMMP;
 
 	vcpu->arch.pvr = mfspr(SPRN_PVR);
 	vcpu_e500->svr = mfspr(SPRN_SVR);


------------------

Thanks
-Bharat

> 
> -Scott
> 

RE: [PATCH 2/2] KVM : powerpc/booke: Allow debug interrupt injection to guest

[Bharat.Bhushan]

> -----Original Message-----
> From: Bhushan Bharat-R65777
> Sent: Wednesday, July 02, 2014 5:07 PM
> To: Wood Scott-B07421; Alexander Graf
> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org
> Subject: RE: [PATCH 2/2] KVM : powerpc/booke: Allow debug interrupt injection to
> guest
> 
> 
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Tuesday, July 01, 2014 10:11 PM
> > To: Alexander Graf
> > Cc: Bhushan Bharat-R65777; kvm-ppc@vger.kernel.org;
> > kvm@vger.kernel.org
> > Subject: Re: [PATCH 2/2] KVM : powerpc/booke: Allow debug interrupt
> > injection to guest
> >
> > On Tue, 2014-07-01 at 18:22 +0200, Alexander Graf wrote:
> > > On 01.07.14 17:35, Scott Wood wrote:
> > > > On Tue, 2014-07-01 at 17:04 +0200, Alexander Graf wrote:
> > > >> On 01.07.14 16:58, Scott Wood wrote:
> > > >>> On Tue, 2014-07-01 at 08:23 +0200, Alexander Graf wrote:
> > > >>>> I don't think QEMU should be aware of these limitations.
> > > >>> OK, but we should at least have some idea of how the whole thing
> > > >>> is supposed to work, in order to determine if this is the
> > > >>> correct behavior for QEMU.  I thought the model was that debug
> > > >>> resources are either owned by QEMU or by the guest, and in the
> > > >>> latter case, QEMU would never see the debug exception to begin with.
> > > >> That's bad for a number of reasons. For starters it's different
> > > >> from how
> > > >> x86 handles debug registers - and I hate to be different just for
> > > >> the sake of being different.
> > > > How does it work on x86?
> > >
> > > It overwrites more-or-less random breakpoints with its own ones, but
> > > leaves the others intact ;).
> >
> > Are you talking about software breakpoints or management of hardware
> > debug registers?
> >
> > > >> So if we do want to declare that debug registers are owned by
> > > >> either QEMU or the guest, we should change the semantics for all
> > > >> architectures.
> > > > If we want to say that ownership of the registers is shared, we
> > > > need a plan for how that would actually work.
> > >
> > > I think you're overengineering here :). When do people actually use
> > > gdbstub? Usually when they want to debug a broken guest. We can
> > > either
> > >
> > >    * overengineer heavily and reduce the number of registers
> > > available to the guest to always have spares
> > >    * overengineer a bit and turn off guest debugging completely when
> > > we use gdbstub
> > >    * just leave as much alive as we can, hoping that it helps with
> > > the debugging
> > >
> > > Option 3 is what x86 does - and I think it's a reasonable approach.
> > > This is not an interface that needs to be 100% consistent and bullet
> > > proof, it's a best effort to enable you to debug as much as possible.
> >
> > I'm not insisting on 100% -- just hoping for some
> > explanation/discussion about how it's intended to work for the cases where it
> can.
> >
> > How will MSR[DE] and MSRP[DEP] be handled?
> >
> > How would I go about telling QEMU/KVM that I don't want this shared
> > mode, because I don't want guest to interfere with the debugging I'm
> > trying to do from QEMU?
> >
> > Will guest accesses to debug registers cause a userspace exit when
> > guest_debug is enabled?
> >
> > > >> I think we're in a path that is slow enough already to not worry
> > > >> about performance.
> > > > It's not just about performance, but simplicity of use, and
> > > > consistency of API.
> > > >
> > > > Oh, and it looks like there already exist one reg definitions and
> > > > implementations for most of the debug registers.
> > >
> > > For BookE? Where?
> >
> > arch/powerpc/kvm/booke.c: KVM_REG_PPC_IACn, KVM_REG_PPC_DACn
> 
> I tried to quickly prototype what I think we want to do (this is not tested)

Hi Scott,

There is one problem which is stopping us to share debug resource between qemu and guest, looking for suggestions:
- As qemu is also using debug resource,  We have to set MSR_DE and set MSRP_DEP (guest will not be able to clear MSR_DE). So qemu set debug events will always cause the debug interrupts.
- Now guest is also using debug resources and for some reason if guest wants to clear MSR_DE (disable debug interrupt) But it will not be able to disable as MSRP_DEP is set and KVM will not come to know guest willingness to disable MSR_DE.
- If the debug interrupts occurs then we will exit to QEMU and this may not a QEMU set event so it will inject interrupt to guest (using one-reg or set-sregs)
- Now KVM, when handling one-reg/sregs request to inject debug interrupt, do not know whether guest can handle the debug interrupt or not (as guest might have tried to set/clear MSR_DE).

Thanks
-Bharat

> 
> -----------------------------------------------------------------------
> diff --git a/arch/powerpc/include/asm/kvm_ppc.h
> b/arch/powerpc/include/asm/kvm_ppc.h
> index e8b3982..746b5c6 100644
> --- a/arch/powerpc/include/asm/kvm_ppc.h
> +++ b/arch/powerpc/include/asm/kvm_ppc.h
> @@ -179,6 +179,9 @@ extern int kvmppc_xics_get_xive(struct kvm *kvm, u32 irq,
> u32 *server,  extern int kvmppc_xics_int_on(struct kvm *kvm, u32 irq);  extern
> int kvmppc_xics_int_off(struct kvm *kvm, u32 irq);
> 
> +void kvmppc_core_dequeue_debug(struct kvm_vcpu *vcpu); void
> +kvmppc_core_queue_debug(struct kvm_vcpu *vcpu);
> +
>  /*
>   * Cuts out inst bits with ordering according to spec.
>   * That means the leftmost bit is zero. All given bits are included.
> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index
> 9f13056..0b7e4e4 100644
> --- a/arch/powerpc/kvm/booke.c
> +++ b/arch/powerpc/kvm/booke.c
> @@ -235,6 +235,16 @@ void kvmppc_core_dequeue_dec(struct kvm_vcpu *vcpu)
>  	clear_bit(BOOKE_IRQPRIO_DECREMENTER, &vcpu->arch.pending_exceptions);  }
> 
> +void kvmppc_core_queue_debug(struct kvm_vcpu *vcpu) {
> +	kvmppc_booke_queue_irqprio(vcpu, BOOKE_IRQPRIO_DEBUG); }
> +
> +void kvmppc_core_dequeue_debug(struct kvm_vcpu *vcpu) {
> +	clear_bit(BOOKE_IRQPRIO_DEBUG, &vcpu->arch.pending_exceptions); }
> +
>  void kvmppc_core_queue_external(struct kvm_vcpu *vcpu,
>                                  struct kvm_interrupt *irq)  { @@ -841,6 +851,20
> @@ static int kvmppc_handle_debug(struct kvm_run *run, struct kvm_vcpu *vcpu)
>  	struct debug_reg *dbg_reg = &(vcpu->arch.shadow_dbg_reg);
>  	u32 dbsr = vcpu->arch.dbsr;
> 
> +	/* Userspace (QEMU) is not using debug resource, so inject debug interrupt
> +	 * directly to guest debug.
> +	 */
> +	if (vcpu->guest_debug == 0) {
> +		if (dbsr && (vcpu->arch.shared->msr & MSR_DE))
> +			kvmppc_core_queue_debug(vcpu);
> +
> +		/* Inject a program interrupt if trap debug is not allowed */
> +		if ((dbsr & DBSR_TIE) && !(vcpu->arch.shared->msr & MSR_DE))
> +			kvmppc_core_queue_program(vcpu, ESR_PTR);
> +
> +		return RESUME_GUEST;
> +	}
> +
>  	run->debug.arch.status = 0;
>  	run->debug.arch.address = vcpu->arch.pc;
> 
> @@ -1920,7 +1944,7 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu
> *vcpu,
>  	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;
> +//	vcpu->arch.dbg_reg.dbcr0 = DBCR0_EDM;
> 
>  	if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP)
>  		vcpu->arch.shadow_dbg_reg.dbcr0 |= DBCR0_IDM | DBCR0_IC; diff --git
> a/arch/powerpc/kvm/booke_emulate.c b/arch/powerpc/kvm/booke_emulate.c
> index aaff1b7..ef8de7f 100644
> --- a/arch/powerpc/kvm/booke_emulate.c
> +++ b/arch/powerpc/kvm/booke_emulate.c
> @@ -26,6 +26,7 @@
>  #define OP_19_XOP_RFMCI   38
>  #define OP_19_XOP_RFI     50
>  #define OP_19_XOP_RFCI    51
> +#define OP_19_XOP_RFDI    39
> 
>  #define OP_31_XOP_MFMSR   83
>  #define OP_31_XOP_WRTEE   131
> @@ -38,10 +39,24 @@ static void kvmppc_emul_rfi(struct kvm_vcpu *vcpu)
>  	kvmppc_set_msr(vcpu, vcpu->arch.shared->srr1);  }
> 
> +static void kvmppc_emul_rfdi(struct kvm_vcpu *vcpu) {
> +	vcpu->arch.pc = vcpu->arch.dsrr0;
> +	/* Force MSR_DE when guest does not own debug facilities */
> +	if (vcpu->guest_debug)
> +		kvmppc_set_msr(vcpu, vcpu->arch.dsrr1 | MSR_DE);
> +	else
> +		kvmppc_set_msr(vcpu, vcpu->arch.dsrr1); }
> +
>  static void kvmppc_emul_rfci(struct kvm_vcpu *vcpu)  {
>  	vcpu->arch.pc = vcpu->arch.csrr0;
> -	kvmppc_set_msr(vcpu, vcpu->arch.csrr1);
> +	/* Force MSR_DE when guest does not own debug facilities */
> +	if (vcpu->guest_debug)
> +		kvmppc_set_msr(vcpu, vcpu->arch.csrr1 | MSR_DE);
> +	else
> +		kvmppc_set_msr(vcpu, vcpu->arch.csrr1);
>  }
> 
>  static void kvmppc_emul_rfmci(struct kvm_vcpu *vcpu) @@ -78,6 +93,12 @@ int
> kvmppc_booke_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu,
>  			*advance = 0;
>  			break;
> 
> +		case OP_19_XOP_RFDI:
> +			kvmppc_emul_rfdi(vcpu);
> +//			kvmppc_set_exit_type(vcpu, EMULATED_RFDI_EXITS);
> +			*advance = 0;
> +			break;
> +
>  		default:
>  			emulated = EMULATE_FAIL;
>  			break;
> @@ -131,6 +152,7 @@ int kvmppc_booke_emulate_op(struct kvm_run *run, struct
> kvm_vcpu *vcpu,  int kvmppc_booke_emulate_mtspr(struct kvm_vcpu *vcpu, int sprn,
> ulong spr_val)  {
>  	int emulated = EMULATE_DONE;
> +	bool debug_inst = false;
> 
>  	switch (sprn) {
>  	case SPRN_DEAR:
> @@ -145,21 +167,74 @@ int kvmppc_booke_emulate_mtspr(struct kvm_vcpu *vcpu, int
> sprn, ulong spr_val)
>  	case SPRN_CSRR1:
>  		vcpu->arch.csrr1 = spr_val;
>  		break;
> -	case SPRN_DBCR0:
> -		vcpu->arch.dbg_reg.dbcr0 = spr_val;
> +	case SPRN_DSRR0:
> +		vcpu->arch.dsrr0 = spr_val;
>  		break;
> -	case SPRN_DBCR1:
> -		vcpu->arch.dbg_reg.dbcr1 = spr_val;
> +	case SPRN_DSRR1:
> +		vcpu->arch.dsrr1 = spr_val;
> +		break;
> +	case SPRN_IAC1:
> +		debug_inst = true;
> +		vcpu->arch.dbg_reg.iac1 = spr_val;
> +		vcpu->arch.shadow_dbg_reg.iac1 = spr_val;
> +		break;
> +	case SPRN_IAC2:
> +		debug_inst = true;
> +		vcpu->arch.dbg_reg.iac2 = spr_val;
> +		vcpu->arch.shadow_dbg_reg.iac2 = spr_val;
> +		break;
> +#ifndef CONFIG_PPC_FSL_BOOK3E
> +	case SPRN_IAC3:
> +		debug_inst = true;
> +		vcpu->arch.dbg_reg.iac3 = spr_val;
> +		vcpu->arch.shadow_dbg_reg.iac3 = spr_val;
>  		break;
> +	case SPRN_IAC4:
> +		debug_inst = true;
> +		vcpu->arch.dbg_reg.iac4 = spr_val;
> +		vcpu->arch.shadow_dbg_reg.iac4 = spr_val;
> +		break;
> +#endif
> +	case SPRN_DAC1:
> +		debug_inst = true;
> +		vcpu->arch.dbg_reg.dac1 = spr_val;
> +		vcpu->arch.shadow_dbg_reg.dac1 = spr_val;
> +		break;
> +	case SPRN_DAC2:
> +		debug_inst = true;
> +		vcpu->arch.dbg_reg.dac2 = spr_val;
> +		vcpu->arch.shadow_dbg_reg.dac2 = spr_val;
> +		break;
> + 	case SPRN_DBCR0:
> +		debug_inst = true;
> +		spr_val &= (DBCR0_IDM | DBCR0_IC | DBCR0_BT | DBCR0_TIE |
> +			DBCR0_IAC1 | DBCR0_IAC2 | DBCR0_IAC3 | DBCR0_IAC4  |
> +			DBCR0_DAC1R | DBCR0_DAC1W | DBCR0_DAC2R | DBCR0_DAC2W);
> +
> + 		vcpu->arch.dbg_reg.dbcr0 = spr_val;
> +		vcpu->arch.shadow_dbg_reg.dbcr0 = spr_val;
> + 		break;
> + 	case SPRN_DBCR1:
> +		debug_inst = true;
> + 		vcpu->arch.dbg_reg.dbcr1 = spr_val;
> +		vcpu->arch.shadow_dbg_reg.dbcr1 = spr_val;
> +		break;
> +	case SPRN_DBCR2:
> +		debug_inst = true;
> +		vcpu->arch.dbg_reg.dbcr2 = spr_val;
> +		vcpu->arch.shadow_dbg_reg.dbcr2 = spr_val;
> + 		break;
> + 	case SPRN_DBSR:
> + 		vcpu->arch.dbsr &= ~spr_val;
> +		if (vcpu->arch.dbsr == 0)
> +			kvmppc_core_dequeue_debug(vcpu);
> + 		break;
>  	case SPRN_MCSRR0:
>  		vcpu->arch.mcsrr0 = spr_val;
>  		break;
>  	case SPRN_MCSRR1:
>  		vcpu->arch.mcsrr1 = spr_val;
>  		break;
> -	case SPRN_DBSR:
> -		vcpu->arch.dbsr &= ~spr_val;
> -		break;
>  	case SPRN_TSR:
>  		kvmppc_clr_tsr_bits(vcpu, spr_val);
>  		break;
> @@ -271,6 +346,10 @@ int kvmppc_booke_emulate_mtspr(struct kvm_vcpu *vcpu, int
> sprn, ulong spr_val)
>  		emulated = EMULATE_FAIL;
>  	}
> 
> +	if (debug_inst) {
> +		switch_booke_debug_regs(&vcpu->arch.shadow_dbg_reg);
> +		current->thread.debug = vcpu->arch.shadow_dbg_reg;
> +	}
>  	return emulated;
>  }
> 
> @@ -297,12 +376,41 @@ int kvmppc_booke_emulate_mfspr(struct kvm_vcpu *vcpu, int
> sprn, ulong *spr_val)
>  	case SPRN_CSRR1:
>  		*spr_val = vcpu->arch.csrr1;
>  		break;
> +	case SPRN_DSRR0:
> +		*spr_val = vcpu->arch.dsrr0;
> +		break;
> +	case SPRN_DSRR1:
> +		*spr_val = vcpu->arch.dsrr1;
> +		break;
> +	case SPRN_IAC1:
> +		*spr_val = vcpu->arch.dbg_reg.iac1;
> +		break;
> +	case SPRN_IAC2:
> +		*spr_val = vcpu->arch.dbg_reg.iac2;
> +		break;
> +#ifndef CONFIG_PPC_FSL_BOOK3E
> +	case SPRN_IAC3:
> +		*spr_val = vcpu->arch.dbg_reg.iac3;
> +		break;
> +	case SPRN_IAC4:
> +		*spr_val = vcpu->arch.dbg_reg.iac4;
> +		break;
> +#endif
> +	case SPRN_DAC1:
> +		*spr_val = vcpu->arch.dbg_reg.dac1;
> +		break;
> +	case SPRN_DAC2:
> +		*spr_val = vcpu->arch.dbg_reg.dac2;
> +		break;
>  	case SPRN_DBCR0:
>  		*spr_val = vcpu->arch.dbg_reg.dbcr0;
>  		break;
>  	case SPRN_DBCR1:
>  		*spr_val = vcpu->arch.dbg_reg.dbcr1;
>  		break;
> +	case SPRN_DBCR2:
> +		*spr_val = vcpu->arch.dbg_reg.dbcr2;
> +		break;
>  	case SPRN_MCSRR0:
>  		*spr_val = vcpu->arch.mcsrr0;
>  		break;
> diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c index
> bbf7cdd..560b576 100644
> --- a/arch/powerpc/kvm/e500mc.c
> +++ b/arch/powerpc/kvm/e500mc.c
> @@ -249,7 +249,7 @@ int kvmppc_core_vcpu_setup(struct kvm_vcpu *vcpu)  #ifdef
> CONFIG_64BIT
>  	vcpu->arch.shadow_epcr |= SPRN_EPCR_ICM;  #endif
> -	vcpu->arch.shadow_msrp = MSRP_UCLEP | MSRP_DEP | MSRP_PMMP;
> +	vcpu->arch.shadow_msrp = MSRP_UCLEP | MSRP_PMMP;
> 
>  	vcpu->arch.pvr = mfspr(SPRN_PVR);
>  	vcpu_e500->svr = mfspr(SPRN_SVR);
> 
> 
> ------------------
> 
> Thanks
> -Bharat
> 
> >
> > -Scott
> >

Re: [PATCH 2/2] KVM : powerpc/booke: Allow debug interrupt injection to guest

[Alexander Graf]

On 02.07.14 19:28, Bharat.Bhushan@freescale.com wrote:
>
>> -----Original Message-----
>> From: Bhushan Bharat-R65777
>> Sent: Wednesday, July 02, 2014 5:07 PM
>> To: Wood Scott-B07421; Alexander Graf
>> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org
>> Subject: RE: [PATCH 2/2] KVM : powerpc/booke: Allow debug interrupt injection to
>> guest
>>
>>
>>> -----Original Message-----
>>> From: Wood Scott-B07421
>>> Sent: Tuesday, July 01, 2014 10:11 PM
>>> To: Alexander Graf
>>> Cc: Bhushan Bharat-R65777; kvm-ppc@vger.kernel.org;
>>> kvm@vger.kernel.org
>>> Subject: Re: [PATCH 2/2] KVM : powerpc/booke: Allow debug interrupt
>>> injection to guest
>>>
>>> On Tue, 2014-07-01 at 18:22 +0200, Alexander Graf wrote:
>>>> On 01.07.14 17:35, Scott Wood wrote:
>>>>> On Tue, 2014-07-01 at 17:04 +0200, Alexander Graf wrote:
>>>>>> On 01.07.14 16:58, Scott Wood wrote:
>>>>>>> On Tue, 2014-07-01 at 08:23 +0200, Alexander Graf wrote:
>>>>>>>> I don't think QEMU should be aware of these limitations.
>>>>>>> OK, but we should at least have some idea of how the whole thing
>>>>>>> is supposed to work, in order to determine if this is the
>>>>>>> correct behavior for QEMU.  I thought the model was that debug
>>>>>>> resources are either owned by QEMU or by the guest, and in the
>>>>>>> latter case, QEMU would never see the debug exception to begin with.
>>>>>> That's bad for a number of reasons. For starters it's different
>>>>>> from how
>>>>>> x86 handles debug registers - and I hate to be different just for
>>>>>> the sake of being different.
>>>>> How does it work on x86?
>>>> It overwrites more-or-less random breakpoints with its own ones, but
>>>> leaves the others intact ;).
>>> Are you talking about software breakpoints or management of hardware
>>> debug registers?
>>>
>>>>>> So if we do want to declare that debug registers are owned by
>>>>>> either QEMU or the guest, we should change the semantics for all
>>>>>> architectures.
>>>>> If we want to say that ownership of the registers is shared, we
>>>>> need a plan for how that would actually work.
>>>> I think you're overengineering here :). When do people actually use
>>>> gdbstub? Usually when they want to debug a broken guest. We can
>>>> either
>>>>
>>>>     * overengineer heavily and reduce the number of registers
>>>> available to the guest to always have spares
>>>>     * overengineer a bit and turn off guest debugging completely when
>>>> we use gdbstub
>>>>     * just leave as much alive as we can, hoping that it helps with
>>>> the debugging
>>>>
>>>> Option 3 is what x86 does - and I think it's a reasonable approach.
>>>> This is not an interface that needs to be 100% consistent and bullet
>>>> proof, it's a best effort to enable you to debug as much as possible.
>>> I'm not insisting on 100% -- just hoping for some
>>> explanation/discussion about how it's intended to work for the cases where it
>> can.
>>> How will MSR[DE] and MSRP[DEP] be handled?
>>>
>>> How would I go about telling QEMU/KVM that I don't want this shared
>>> mode, because I don't want guest to interfere with the debugging I'm
>>> trying to do from QEMU?
>>>
>>> Will guest accesses to debug registers cause a userspace exit when
>>> guest_debug is enabled?
>>>
>>>>>> I think we're in a path that is slow enough already to not worry
>>>>>> about performance.
>>>>> It's not just about performance, but simplicity of use, and
>>>>> consistency of API.
>>>>>
>>>>> Oh, and it looks like there already exist one reg definitions and
>>>>> implementations for most of the debug registers.
>>>> For BookE? Where?
>>> arch/powerpc/kvm/booke.c: KVM_REG_PPC_IACn, KVM_REG_PPC_DACn
>> I tried to quickly prototype what I think we want to do (this is not tested)
> Hi Scott,
>
> There is one problem which is stopping us to share debug resource between qemu and guest, looking for suggestions:
> - As qemu is also using debug resource,  We have to set MSR_DE and set MSRP_DEP (guest will not be able to clear MSR_DE). So qemu set debug events will always cause the debug interrupts.
> - Now guest is also using debug resources and for some reason if guest wants to clear MSR_DE (disable debug interrupt) But it will not be able to disable as MSRP_DEP is set and KVM will not come to know guest willingness to disable MSR_DE.
> - If the debug interrupts occurs then we will exit to QEMU and this may not a QEMU set event so it will inject interrupt to guest (using one-reg or set-sregs)
> - Now KVM, when handling one-reg/sregs request to inject debug interrupt, do not know whether guest can handle the debug interrupt or not (as guest might have tried to set/clear MSR_DE).

Yeah, and with this everything falls apart. I guess we can't share 
hardware debug resources after all then. So yes, let's declare all 
hardware debug facilities QEMU owned as soon as QEMU starts using gdbstub.


Alex