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.

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

-Scott


--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to