> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Friday, August 01, 2014 2:16 AM
> To: Bhushan Bharat-R65777
> Cc: ag...@suse.de; kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Yoder
> Stuart-
> B08248
> Subject: Re: [PATCH 6/6] KVM: PPC: BOOKE: Emulate debug registers and
> exception
>
> On Thu, 2014-07-31 at 01:15 -0500, Bhushan Bharat-R65777 wrote:
> >
> > > -----Original Message-----
> > > From: Wood Scott-B07421
> > > Sent: Thursday, July 31, 2014 8:18 AM
> > > To: Bhushan Bharat-R65777
> > > Cc: ag...@suse.de; kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Yoder
> Stuart-
> > > B08248
> > > Subject: Re: [PATCH 6/6] KVM: PPC: BOOKE: Emulate debug registers and
> exception
> > >
> > > On Wed, 2014-07-30 at 01:43 -0500, Bhushan Bharat-R65777 wrote:
> > > >
> > > > > -----Original Message-----
> > > > > From: Wood Scott-B07421
> > > > > Sent: Tuesday, July 29, 2014 3:58 AM
> > > > > To: Bhushan Bharat-R65777
> > > > > Cc: ag...@suse.de; kvm-ppc@vger.kernel.org; k...@vger.kernel.org;
> > > > > Yoder Stuart-
> > > > > B08248
> > > > > Subject: Re: [PATCH 6/6] KVM: PPC: BOOKE: Emulate debug registers
> > > > > and exception
> > > > >
> > > > > Userspace might be interested in
> > > > > the raw value,
> > > >
> > > > With the current design, If userspace is interested then it will not
> > > > get the DBSR.
> > >
> > > Oh, because DBSR isn't currently implemented in sregs or one reg?
> >
> > That is one reason. Another is that if we give dbsr visibility to
> > userspace then userspace have to clear dbsr in handling KVM_EXIT_DEBUG.
>
> Right -- since I didn't realize DBSR wasn't already exposed, I thought
> userspace already had this responsibility.
>
> > > It looked like it was removing dbsr visibility and the requirement for
> userspace
> > > to clear dbsr. I guess the old way was that the value in
> > > vcpu->arch.dbsr didn't matter until the next debug exception, when it
> > > would be overwritten by the new SPRN_DBSR?
> >
> > But that means old dbsr will be visibility to userspace, which is even bad
> than not visible, no?
> >
> > Also this can lead to old dbsr visible to guest once userspace releases
> > debug resources, but this can be solved by clearing dbsr in
> > kvm_arch_vcpu_ioctl_set_guest_debug() -> " if (!(dbg->control &
> > KVM_GUESTDBG_ENABLE)) { }".
>
> I wasn't suggesting that you keep it that way, just clarifying my
> understanding of the current code.
>
>
> >
> > >
> > > > > > + case SPRN_DBCR2:
> > > > > > + /*
> > > > > > + * If userspace is debugging guest then guest
> > > > > > + * can not access debug registers.
> > > > > > + */
> > > > > > + if (vcpu->guest_debug)
> > > > > > + break;
> > > > > > +
> > > > > > + debug_inst = true;
> > > > > > + vcpu->arch.dbg_reg.dbcr2 = spr_val;
> > > > > > + vcpu->arch.shadow_dbg_reg.dbcr2 = spr_val;
> > > > > > break;
> > > > >
> > > > > In what circumstances can the architected and shadow registers differ?
> > > >
> > > > As of now they are same. But I think that if we want to implement other
> > > features like "Freeze Timer (FT)" then they can be different.
> > >
> > > I don't think we can possibly implement Freeze Timer.
> >
> > May be, but in my opinion we should keep this open.
>
> We're not talking about API here -- the implementation should be kept
> simple if there's no imminent need for shadow registers.
>
> > > > I am not sure what we should in that case ?
> > > >
> > > > As we are currently emulating a subset of debug events (IAC, DAC, IC,
> > > > BT and TIE --- DBCR0 emulation) then we should expose status of those
> > > > events in guest dbsr and rest should be cleared ?
> > >
> > > I'm not saying they need to be exposed to the guest, but I don't see where
> you
> > > filter out bits like these.
> >
> > I am trying to get what all bits should be filtered out, all bits
> > except IACx, DACx, IC, BT and TIE (same as event set filtering done
> > when setting DBCR0) ?
> >
> > i.e IDE, UDE, MRR, IRPT, RET, CIRPT, CRET should be filtered out?
>
> Bits like IRPT and RET don't really matter, as you shouldn't see them
> happen. Likewise MRR if you're sure you've cleared it since boot.
We can clear MRR bits when update vcpu->arch->dbsr with SPRM_DBSR in kvm debug
handler
> But
> IDE could be set any time an asynchronous exception happens. I don't
> think you should filter it out, but instead make sure that it doesn't
> cause an exception to be delivered.
So this means that in kvmpp_handle_debug() if DBSR_IDE is set then do not
inject debug interrupt
and
on dbsr write emulation, deque the debug interrupt even if DBSR_IDE is set.
case SPRN_DBSR:
vcpu->arch.dbsr &= ~spr_val;
if (!(vcpu->arch.dbsr & ~DBSR_IDE))
kvmppc_core_dequeue_debug(vcpu);
break;
or
vcpu->arch.dbsr &= ~(spr_val | DBSR_IDE);
if (!vcpu->arch.dbsr)
kvmppc_core_dequeue_debug(vcpu);
break;
Thanks
-Bharat
>
> -Scott
>
