| Message ID | 1344985483-7440-17-git-send-email-agraf@suse.de |
|---|---|
| State | New, archived |
| Headers | show |
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 <agraf@suse.de> > --- > 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? -Scott -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <agraf@suse.de> >> --- >> 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 :). Alex -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <agraf@suse.de> >>> --- >>> 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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Aug 14, 2012 at 07:10:43PM -0500, 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 <agraf@suse.de> > > --- > > 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? > > -Scott It is good practice to document each memory barrier. -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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; + } + if (kvmppc_core_prepare_to_enter(vcpu)) { /* interrupts got enabled in between, so we are back at square 1 */
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 <agraf@suse.de> --- arch/powerpc/kvm/booke.c | 24 +++++++++++++++++------- 1 files changed, 17 insertions(+), 7 deletions(-)