On 08/14/2012 07:13 PM, Alexander Graf wrote: > > On 15.08.2012, at 02:10, Scott Wood wrote: > >> On 08/14/2012 06:04 PM, Alexander Graf wrote: >>> We need a central place to check for pending requests in. Add one that >>> only does the timer check we already do in a different place. >>> >>> Later, this central function can be extended by more checks. >>> >>> Signed-off-by: Alexander Graf <[email protected]> >>> --- >>> arch/powerpc/kvm/booke.c | 24 +++++++++++++++++------- >>> 1 files changed, 17 insertions(+), 7 deletions(-) >>> >>> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c >>> index 1d4ce9a..bcf87fe 100644 >>> --- a/arch/powerpc/kvm/booke.c >>> +++ b/arch/powerpc/kvm/booke.c >>> @@ -419,13 +419,6 @@ static void kvmppc_core_check_exceptions(struct >>> kvm_vcpu *vcpu) >>> unsigned long *pending = &vcpu->arch.pending_exceptions; >>> unsigned int priority; >>> >>> - if (vcpu->requests) { >>> - if (kvm_check_request(KVM_REQ_PENDING_TIMER, vcpu)) { >>> - smp_mb(); >>> - update_timer_ints(vcpu); >>> - } >>> - } >>> - >>> priority = __ffs(*pending); >>> while (priority < BOOKE_IRQPRIO_MAX) { >>> if (kvmppc_booke_irqprio_deliver(vcpu, priority)) >>> @@ -461,6 +454,14 @@ int kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu) >>> return r; >>> } >>> >>> +static void kvmppc_check_requests(struct kvm_vcpu *vcpu) >>> +{ >>> + if (vcpu->requests) { >>> + if (kvm_check_request(KVM_REQ_PENDING_TIMER, vcpu)) >>> + update_timer_ints(vcpu); >>> + } >>> +} >>> + >>> /* >>> * Common checks before entering the guest world. Call with interrupts >>> * disabled. >>> @@ -485,6 +486,15 @@ static int kvmppc_prepare_to_enter(struct kvm_vcpu >>> *vcpu) >>> break; >>> } >>> >>> + smp_mb(); >>> + if (vcpu->requests) { >>> + /* Make sure we process requests preemptable */ >>> + local_irq_enable(); >>> + kvmppc_check_requests(vcpu); >>> + local_irq_disable(); >>> + continue; >>> + } >> >> What previous memory access is the smp_mb() ordering against? > > Against itself really (see the continue?). I might have just gotten this > wrong though :).
Ordering is already ensured by the hardware in that case since it's the same address -- and the stuff inside the if statement is more than enough to ensure the compiler doesn't cache an old value of vcpu->requests. That said, I think the problem is not that the smp_mb() shouldn't be there, but that there should be a vcpu->mode setting before it. -Scott -- 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
