RE: [PATCH 5/5 v2] KVM: PPC: BOOKE: Emulate debug registers and exception

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Tuesday, August 05, 2014 4:23 AM
> To: Bhushan Bharat-R65777
> Cc: agraf@xxxxxxx; kvm-ppc@xxxxxxxxxxxxxxx; kvm@xxxxxxxxxxxxxxx; Yoder Stuart-
> B08248
> Subject: Re: [PATCH 5/5 v2] KVM: PPC: BOOKE: Emulate debug registers and
> exception
> 
> On Mon, 2014-08-04 at 13:32 +0530, Bharat Bhushan wrote:
> > @@ -735,7 +745,27 @@ 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;
> >
> > -	/* Clear guest dbsr (vcpu->arch.dbsr).
> > +	if (vcpu->guest_debug == 0) {
> > +		/*
> > +		 * Debug resources belong to Guest.
> > +		 * Imprecise debug event are not injected
> > +		 */
> > +		if (dbsr & DBSR_IDE)
> > +			return RESUME_GUEST;
> 
> This is incorrect.  DBSR_IDE shouldn't *cause* an injection, but it shouldn't
> inhibit it either.

Will this work ?
		If ((dbsr & DBSR_IDE) && !(dbsr & ~DBSR_IDE))
			Return RESUME_GUEST; 

> 
> > @@ -828,6 +858,8 @@ static void kvmppc_restart_interrupt(struct kvm_vcpu
> *vcpu,
> >  	case BOOKE_INTERRUPT_DEBUG:
> >  		/* Save DBSR before preemption is enabled */
> >  		vcpu->arch.dbsr = mfspr(SPRN_DBSR);
> > +		/* MASK out DBSR_MRR */
> > +		vcpu->arch.dbsr &= ~DBSR_MRR;
> >  		kvmppc_clear_dbsr();
> >  		break;
> >  	}
> 
> DBSR[MRR] can only be set once per host system reset.  There's no need to filter
> it out here; just make sure the host clears it at some point before this point.

Can you please suggest where ? somewhere in KVM initialization ?

> The MRR value doesn't currently survive past kvmppc_clear_dbsr(), so this isn't
> helping to preserve it for the host's benefit...
> 
> > @@ -1858,6 +1890,7 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct
> > kvm_vcpu *vcpu,
> >
> >  	if (!(dbg->control & KVM_GUESTDBG_ENABLE)) {
> >  		vcpu->arch.shadow_dbg_reg.dbcr0 = 0;
> > +		vcpu->arch.dbg_reg.dbcr0 = 0;
> 
> Again, it's not clear why we need shadow debug registers here.  "Just in case we
> implement something that can't be implemented" isn't a good reason to keep
> complexity around.

One reason was that setting EDM in guest visible register, For this we need shadow_reg is used to save/restore state in h/w register (which does not have DBCR0_EDM) but debug_reg have DBCR0_EDM.

Thanks
-Bharat

> 
> -Scott
> 

��.n��������+%������w��{.n�����o�^n�r������&��z�ޗ�zf���h���~����������_��+v���)ߣ�


[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux