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-...@vger.kernel.org; kvm@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-...@vger.kernel.org; kvm@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. 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.
-Scott
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
