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

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

> > >  #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