> -----Original Message----- > From: [email protected] > [mailto:[email protected]] On Behalf Of Alexander Graf > Sent: Tuesday, September 06, 2011 6:45 AM > To: Wood Scott-B07421 > Cc: [email protected] > Subject: Re: [PATCH 5/5] KVM: PPC: booke: Improve timer > register emulation > > > On 27.08.2011, at 01:31, Scott Wood wrote: > > > From: Liu Yu <[email protected]> > > > > Decrementers are now properly driven by TCR/TSR, and the guest > > has full read/write access to these registers. > > > > The decrementer keeps ticking (and setting the TSR bit) > regardless of > > whether the interrupts are enabled with TCR. > > > > The decrementer stops at zero, rather than going negative. > > > > Signed-off-by: Liu Yu <[email protected]> > > [scott: added dequeue in kvmppc_booke_irqprio_deliver, and > dec stop-at-zero] > > Signed-off-by: Scott Wood <[email protected]> > > --- > > arch/powerpc/include/asm/kvm_host.h | 2 +- > > arch/powerpc/include/asm/kvm_ppc.h | 11 +++++ > > arch/powerpc/kvm/book3s.c | 8 ++++ > > arch/powerpc/kvm/booke.c | 80 > +++++++++++++++++++++++++++-------- > > arch/powerpc/kvm/booke.h | 4 ++ > > arch/powerpc/kvm/booke_emulate.c | 11 ++++- > > arch/powerpc/kvm/emulate.c | 45 ++++++++----------- > > arch/powerpc/kvm/powerpc.c | 20 +-------- > > 8 files changed, 114 insertions(+), 67 deletions(-) > > > > diff --git a/arch/powerpc/include/asm/kvm_host.h > b/arch/powerpc/include/asm/kvm_host.h > > index 3305af4..ea08c79 100644 > > --- a/arch/powerpc/include/asm/kvm_host.h > > +++ b/arch/powerpc/include/asm/kvm_host.h > > @@ -334,7 +334,7 @@ struct kvm_vcpu_arch { > > u32 tbl; > > u32 tbu; > > u32 tcr; > > - u32 tsr; > > + ulong tsr; /* we need to perform set/clr_bits() which > requires ulong */ > > u32 ivor[64]; > > ulong ivpr; > > u32 pvr; > > diff --git a/arch/powerpc/include/asm/kvm_ppc.h > b/arch/powerpc/include/asm/kvm_ppc.h > > index bdaa6c8..ddaa615 100644 > > --- a/arch/powerpc/include/asm/kvm_ppc.h > > +++ b/arch/powerpc/include/asm/kvm_ppc.h > > @@ -66,6 +66,7 @@ extern int > kvmppc_emulate_instruction(struct kvm_run *run, > > extern int kvmppc_emulate_mmio(struct kvm_run *run, struct > kvm_vcpu *vcpu); > > extern void kvmppc_emulate_dec(struct kvm_vcpu *vcpu); > > extern u32 kvmppc_get_dec(struct kvm_vcpu *vcpu, u64 tb); > > +extern void kvmppc_decrementer_func(unsigned long data); > > > > /* Core-specific hooks */ > > > > @@ -197,4 +198,14 @@ int kvm_vcpu_ioctl_config_tlb(struct > kvm_vcpu *vcpu, > > int kvm_vcpu_ioctl_dirty_tlb(struct kvm_vcpu *vcpu, > > struct kvm_dirty_tlb *cfg); > > > > +static inline void kvmppc_wakeup_vcpu(struct kvm_vcpu *vcpu) > > +{ > > + if (waitqueue_active(&vcpu->wq)) { > > + wake_up_interruptible(&vcpu->wq); > > + vcpu->stat.halt_wakeup++; > > + } else if (vcpu->cpu != -1) { > > + smp_send_reschedule(vcpu->cpu); > > + } > > +} > > + > > #endif /* __POWERPC_KVM_PPC_H__ */ > > diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c > > index f68a34d..b057856 100644 > > --- a/arch/powerpc/kvm/book3s.c > > +++ b/arch/powerpc/kvm/book3s.c > > @@ -514,3 +514,11 @@ out: > > mutex_unlock(&kvm->slots_lock); > > return r; > > } > > + > > +void kvmppc_decrementer_func(unsigned long data) > > +{ > > + struct kvm_vcpu *vcpu = (struct kvm_vcpu *)data; > > + > > + kvmppc_core_queue_dec(vcpu); > > + kvmppc_wakeup_vcpu(vcpu); > > +} > > diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c > > index 0ed62c1..502f9ff 100644 > > --- a/arch/powerpc/kvm/booke.c > > +++ b/arch/powerpc/kvm/booke.c > > @@ -205,7 +205,8 @@ void > kvmppc_core_dequeue_external(struct kvm_vcpu *vcpu, > > static int kvmppc_booke_irqprio_deliver(struct kvm_vcpu *vcpu, > > unsigned int priority) > > { > > - int allowed = 0; > > + int allowed = 1; > > + int dequeue = 0; > > ulong uninitialized_var(msr_mask); > > bool update_esr = false, update_dear = false; > > ulong crit_raw = vcpu->arch.shared->critical; > > @@ -258,10 +259,15 @@ static int > kvmppc_booke_irqprio_deliver(struct kvm_vcpu *vcpu, > > allowed = vcpu->arch.shared->msr & MSR_ME; > > msr_mask = 0; > > break; > > - case BOOKE_IRQPRIO_EXTERNAL: > > case BOOKE_IRQPRIO_DECREMENTER: > > + if (!(vcpu->arch.tcr & TCR_DIE)) { > > + allowed = 0; > > + dequeue = 1; > > + } > > + /* fall through */ > > + case BOOKE_IRQPRIO_EXTERNAL: > > case BOOKE_IRQPRIO_FIT: > > - allowed = vcpu->arch.shared->msr & MSR_EE; > > + allowed = allowed && vcpu->arch.shared->msr & MSR_EE; > > allowed = allowed && !crit; > > msr_mask = MSR_CE|MSR_ME|MSR_DE; > > break; > > @@ -269,6 +275,8 @@ static int > kvmppc_booke_irqprio_deliver(struct kvm_vcpu *vcpu, > > allowed = vcpu->arch.shared->msr & MSR_DE; > > msr_mask = MSR_ME; > > break; > > + default: > > + allowed = 0; > > } > > > > if (allowed) { > > @@ -283,6 +291,9 @@ static int > kvmppc_booke_irqprio_deliver(struct kvm_vcpu *vcpu, > > > > if (!keep_irq) > > clear_bit(priority, > &vcpu->arch.pending_exceptions); > > + } else if (dequeue) { > > + /* Should only happen due to races with masking */ > > + clear_bit(priority, &vcpu->arch.pending_exceptions); > > } > > > > return allowed; > > @@ -721,25 +732,16 @@ static int set_sregs_base(struct > kvm_vcpu *vcpu, > > vcpu->arch.shared->esr = sregs->u.e.esr; > > vcpu->arch.shared->dar = sregs->u.e.dear; > > vcpu->arch.vrsave = sregs->u.e.vrsave; > > - vcpu->arch.tcr = sregs->u.e.tcr; > > + kvmppc_set_tcr(vcpu, sregs->u.e.tcr); > > > > - if (sregs->u.e.update_special & KVM_SREGS_E_UPDATE_DEC) > > + if (sregs->u.e.update_special & KVM_SREGS_E_UPDATE_DEC) { > > vcpu->arch.dec = sregs->u.e.dec; > > - > > - kvmppc_emulate_dec(vcpu); > > + kvmppc_emulate_dec(vcpu); > > + } > > > > if (sregs->u.e.update_special & KVM_SREGS_E_UPDATE_TSR) { > > - /* > > - * FIXME: existing KVM timer handling is incomplete. > > - * TSR cannot be read by the guest, and its value in > > - * vcpu->arch is always zero. For now, just handle > > - * the case where the caller is trying to inject a > > - * decrementer interrupt. > > - */ > > - > > - if ((sregs->u.e.tsr & TSR_DIS) && > > - (vcpu->arch.tcr & TCR_DIE)) > > - kvmppc_core_queue_dec(vcpu); > > + kvmppc_clr_tsr_bits(vcpu, ~0); > > + kvmppc_set_tsr_bits(vcpu, sregs->u.e.tsr); > > } > > > > return 0; > > @@ -895,6 +897,48 @@ void kvmppc_core_destroy_vm(struct kvm *kvm) > > { > > } > > > > +void kvmppc_set_tcr(struct kvm_vcpu *vcpu, u32 new_tcr) > > +{ > > + vcpu->arch.tcr = new_tcr; > > + smp_wmb(); > > + > > + /* > > + * Since TCR changed, we need to check > > + * if blocked interrupts are deliverable. > > + */ > > + if ((new_tcr & TCR_DIE) && (vcpu->arch.tsr & TSR_DIS)) { > > + kvmppc_core_queue_dec(vcpu); > > + kvmppc_wakeup_vcpu(vcpu); > > + } > > +} > > + > > +void kvmppc_set_tsr_bits(struct kvm_vcpu *vcpu, u32 tsr_bits) > > +{ > > + set_bits(tsr_bits, &vcpu->arch.tsr); > > I'm still missing the point why you need set_bits/clear_bits. > Can't you just use |= and &= ~ like everyone else? :) >
Because set_bits/clear_bits is atomic. set/clr_tsr_bits() may be called in different threads so that on different cores. We need to make sure modification to be atomic to prevent missing changes. Thanks, Yu -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
