| Message ID | 1323444412-18482-2-git-send-email-agraf@suse.de |
|---|---|
| State | New, archived |
| Headers | show |
On 12/09/2011 09:26 AM, Alexander Graf wrote: > When entering the guest, we want to make sure we're not getting preempted > away, so let's disable preemption on entry, but enable it again while handling > guest exits. > > Reported-by: Jörg Sommer <joerg@alea.gnuu.de> > Signed-off-by: Alexander Graf <agraf@suse.de> > --- > arch/powerpc/kvm/book3s_pr.c | 7 +++++++ > 1 files changed, 7 insertions(+), 0 deletions(-) > > diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c > index 726512b..8e4f800 100644 > --- a/arch/powerpc/kvm/book3s_pr.c > +++ b/arch/powerpc/kvm/book3s_pr.c > @@ -519,6 +519,7 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu, > run->ready_for_interrupt_injection = 1; > > trace_kvm_book3s_exit(exit_nr, vcpu); > + preempt_enable(); > kvm_resched(vcpu); > switch (exit_nr) { > case BOOK3S_INTERRUPT_INST_STORAGE: > @@ -763,6 +764,8 @@ program_interrupt: > run->exit_reason = KVM_EXIT_INTR; > r = -EINTR; > } else { > + preempt_disable(); > + > /* In case an interrupt came in that was triggered > * from userspace (like DEC), we need to check what > * to inject now! */ Shouldn't you really have interrupts disabled here, as booke does? Otherwise an interrupt (including an IPI kick) could send you a signal or guest exception after you check. Likewise for other guest entry points. -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 09.12.2011, at 19:19, Scott Wood wrote: > On 12/09/2011 09:26 AM, Alexander Graf wrote: >> When entering the guest, we want to make sure we're not getting preempted >> away, so let's disable preemption on entry, but enable it again while handling >> guest exits. >> >> Reported-by: Jörg Sommer <joerg@alea.gnuu.de> >> Signed-off-by: Alexander Graf <agraf@suse.de> >> --- >> arch/powerpc/kvm/book3s_pr.c | 7 +++++++ >> 1 files changed, 7 insertions(+), 0 deletions(-) >> >> diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c >> index 726512b..8e4f800 100644 >> --- a/arch/powerpc/kvm/book3s_pr.c >> +++ b/arch/powerpc/kvm/book3s_pr.c >> @@ -519,6 +519,7 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu, >> run->ready_for_interrupt_injection = 1; >> >> trace_kvm_book3s_exit(exit_nr, vcpu); >> + preempt_enable(); >> kvm_resched(vcpu); >> switch (exit_nr) { >> case BOOK3S_INTERRUPT_INST_STORAGE: >> @@ -763,6 +764,8 @@ program_interrupt: >> run->exit_reason = KVM_EXIT_INTR; >> r = -EINTR; >> } else { >> + preempt_disable(); >> + >> /* In case an interrupt came in that was triggered >> * from userspace (like DEC), we need to check what >> * to inject now! */ > > Shouldn't you really have interrupts disabled here, as booke does? Ah, thanks for the reminder. Yeah, we probably want to disable interrupts in parallel to checking for signals (basically from one signal check point to world switch). I'm just not 100% sure how to easily sync the C and asm code on the first entry though. Doing local_irq_disable in C and undoing it in asm could become ugly with lazy interrupt disabling. 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 12/09/2011 05:18 PM, Alexander Graf wrote: > > On 09.12.2011, at 19:19, Scott Wood wrote: > >> Shouldn't you really have interrupts disabled here, as booke does? > > Ah, thanks for the reminder. Yeah, we probably want to disable > interrupts in parallel to checking for signals (basically from one > signal check point to world switch). I'm just not 100% sure how to > easily sync the C and asm code on the first entry though. Doing > local_irq_disable in C and undoing it in asm could become ugly with > lazy interrupt disabling. Lots of things get ugly with lazy interrupt disabling. :-( There should be other examples of handling the lazy EE stuff in asm code. After you hard-disable, you should just need to poke the right fields in the PACA with the state you plan to end up in after the rfi, similar to exception return. -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
diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c index 726512b..8e4f800 100644 --- a/arch/powerpc/kvm/book3s_pr.c +++ b/arch/powerpc/kvm/book3s_pr.c @@ -519,6 +519,7 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu, run->ready_for_interrupt_injection = 1; trace_kvm_book3s_exit(exit_nr, vcpu); + preempt_enable(); kvm_resched(vcpu); switch (exit_nr) { case BOOK3S_INTERRUPT_INST_STORAGE: @@ -763,6 +764,8 @@ program_interrupt: run->exit_reason = KVM_EXIT_INTR; r = -EINTR; } else { + preempt_disable(); + /* In case an interrupt came in that was triggered * from userspace (like DEC), we need to check what * to inject now! */ @@ -925,6 +928,8 @@ int kvmppc_vcpu_run(struct kvm_run *kvm_run, struct kvm_vcpu *vcpu) #endif ulong ext_msr; + preempt_disable(); + /* Check if we can run the vcpu at all */ if (!vcpu->arch.sane) { kvm_run->exit_reason = KVM_EXIT_INTERNAL_ERROR; @@ -1006,6 +1011,8 @@ int kvmppc_vcpu_run(struct kvm_run *kvm_run, struct kvm_vcpu *vcpu) current->thread.used_vsr = used_vsr; #endif + preempt_enable(); + return ret; }
When entering the guest, we want to make sure we're not getting preempted away, so let's disable preemption on entry, but enable it again while handling guest exits. Reported-by: Jörg Sommer <joerg@alea.gnuu.de> Signed-off-by: Alexander Graf <agraf@suse.de> --- arch/powerpc/kvm/book3s_pr.c | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-)