| Message ID | 1537524123-9578-8-git-send-email-paulus@ozlabs.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | KVM: PPC: Book3S HV: Nested HV virtualization | expand |
On Fri, Sep 21, 2018 at 08:01:38PM +1000, Paul Mackerras wrote: > Currently kvmppc_handle_exit_hv() is called with the vcore lock held > because it is called within a for_each_runnable_thread loop. > However, we already unlock the vcore within kvmppc_handle_exit_hv() > under certain circumstances, and this is safe because (a) any vcpus > that become runnable and are added to the runnable set by > kvmppc_run_vcpu() have their vcpu->arch.trap == 0 and can't actually > run in the guest (because the vcore state is VCORE_EXITING), and > (b) for_each_runnable_thread is safe against addition or removal > of vcpus from the runnable set. > > Therefore, in order to simplify things for following patches, let's > drop the vcore lock in the for_each_runnable_thread loop, so > kvmppc_handle_exit_hv() gets called without the vcore lock held. > > Signed-off-by: Paul Mackerras <paulus@ozlabs.org> Reviewed-by: David Gibson <david@gibson.dropbear.id.au> > --- > arch/powerpc/kvm/book3s_hv.c | 19 ++++++++++--------- > 1 file changed, 10 insertions(+), 9 deletions(-) > > diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c > index 49a686c..0e17593 100644 > --- a/arch/powerpc/kvm/book3s_hv.c > +++ b/arch/powerpc/kvm/book3s_hv.c > @@ -1084,7 +1084,6 @@ static int kvmppc_emulate_doorbell_instr(struct kvm_vcpu *vcpu) > return RESUME_GUEST; > } > > -/* Called with vcpu->arch.vcore->lock held */ > static int kvmppc_handle_exit_hv(struct kvm_run *run, struct kvm_vcpu *vcpu, > struct task_struct *tsk) > { > @@ -1205,10 +1204,7 @@ static int kvmppc_handle_exit_hv(struct kvm_run *run, struct kvm_vcpu *vcpu, > swab32(vcpu->arch.emul_inst) : > vcpu->arch.emul_inst; > if (vcpu->guest_debug & KVM_GUESTDBG_USE_SW_BP) { > - /* Need vcore unlocked to call kvmppc_get_last_inst */ > - spin_unlock(&vcpu->arch.vcore->lock); > r = kvmppc_emulate_debug_inst(run, vcpu); > - spin_lock(&vcpu->arch.vcore->lock); > } else { > kvmppc_core_queue_program(vcpu, SRR1_PROGILL); > r = RESUME_GUEST; > @@ -1224,12 +1220,8 @@ static int kvmppc_handle_exit_hv(struct kvm_run *run, struct kvm_vcpu *vcpu, > case BOOK3S_INTERRUPT_H_FAC_UNAVAIL: > r = EMULATE_FAIL; > if (((vcpu->arch.hfscr >> 56) == FSCR_MSGP_LG) && > - cpu_has_feature(CPU_FTR_ARCH_300)) { > - /* Need vcore unlocked to call kvmppc_get_last_inst */ > - spin_unlock(&vcpu->arch.vcore->lock); > + cpu_has_feature(CPU_FTR_ARCH_300)) > r = kvmppc_emulate_doorbell_instr(vcpu); > - spin_lock(&vcpu->arch.vcore->lock); > - } > if (r == EMULATE_FAIL) { > kvmppc_core_queue_program(vcpu, SRR1_PROGILL); > r = RESUME_GUEST; > @@ -2599,6 +2591,14 @@ static void post_guest_process(struct kvmppc_vcore *vc, bool is_master) > spin_lock(&vc->lock); > now = get_tb(); > for_each_runnable_thread(i, vcpu, vc) { > + /* > + * It's safe to unlock the vcore in the loop here, because > + * for_each_runnable_thread() is safe against removal of > + * the vcpu, and the vcore state is VCORE_EXITING here, > + * so any vcpus becoming runnable will have their arch.trap > + * set to zero and can't actually run in the guest. > + */ > + spin_unlock(&vc->lock); > /* cancel pending dec exception if dec is positive */ > if (now < vcpu->arch.dec_expires && > kvmppc_core_pending_dec(vcpu)) > @@ -2614,6 +2614,7 @@ static void post_guest_process(struct kvmppc_vcore *vc, bool is_master) > vcpu->arch.ret = ret; > vcpu->arch.trap = 0; > > + spin_lock(&vc->lock); > if (is_kvmppc_resume_guest(vcpu->arch.ret)) { > if (vcpu->arch.pending_exceptions) > kvmppc_core_prepare_to_enter(vcpu);
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c index 49a686c..0e17593 100644 --- a/arch/powerpc/kvm/book3s_hv.c +++ b/arch/powerpc/kvm/book3s_hv.c @@ -1084,7 +1084,6 @@ static int kvmppc_emulate_doorbell_instr(struct kvm_vcpu *vcpu) return RESUME_GUEST; } -/* Called with vcpu->arch.vcore->lock held */ static int kvmppc_handle_exit_hv(struct kvm_run *run, struct kvm_vcpu *vcpu, struct task_struct *tsk) { @@ -1205,10 +1204,7 @@ static int kvmppc_handle_exit_hv(struct kvm_run *run, struct kvm_vcpu *vcpu, swab32(vcpu->arch.emul_inst) : vcpu->arch.emul_inst; if (vcpu->guest_debug & KVM_GUESTDBG_USE_SW_BP) { - /* Need vcore unlocked to call kvmppc_get_last_inst */ - spin_unlock(&vcpu->arch.vcore->lock); r = kvmppc_emulate_debug_inst(run, vcpu); - spin_lock(&vcpu->arch.vcore->lock); } else { kvmppc_core_queue_program(vcpu, SRR1_PROGILL); r = RESUME_GUEST; @@ -1224,12 +1220,8 @@ static int kvmppc_handle_exit_hv(struct kvm_run *run, struct kvm_vcpu *vcpu, case BOOK3S_INTERRUPT_H_FAC_UNAVAIL: r = EMULATE_FAIL; if (((vcpu->arch.hfscr >> 56) == FSCR_MSGP_LG) && - cpu_has_feature(CPU_FTR_ARCH_300)) { - /* Need vcore unlocked to call kvmppc_get_last_inst */ - spin_unlock(&vcpu->arch.vcore->lock); + cpu_has_feature(CPU_FTR_ARCH_300)) r = kvmppc_emulate_doorbell_instr(vcpu); - spin_lock(&vcpu->arch.vcore->lock); - } if (r == EMULATE_FAIL) { kvmppc_core_queue_program(vcpu, SRR1_PROGILL); r = RESUME_GUEST; @@ -2599,6 +2591,14 @@ static void post_guest_process(struct kvmppc_vcore *vc, bool is_master) spin_lock(&vc->lock); now = get_tb(); for_each_runnable_thread(i, vcpu, vc) { + /* + * It's safe to unlock the vcore in the loop here, because + * for_each_runnable_thread() is safe against removal of + * the vcpu, and the vcore state is VCORE_EXITING here, + * so any vcpus becoming runnable will have their arch.trap + * set to zero and can't actually run in the guest. + */ + spin_unlock(&vc->lock); /* cancel pending dec exception if dec is positive */ if (now < vcpu->arch.dec_expires && kvmppc_core_pending_dec(vcpu)) @@ -2614,6 +2614,7 @@ static void post_guest_process(struct kvmppc_vcore *vc, bool is_master) vcpu->arch.ret = ret; vcpu->arch.trap = 0; + spin_lock(&vc->lock); if (is_kvmppc_resume_guest(vcpu->arch.ret)) { if (vcpu->arch.pending_exceptions) kvmppc_core_prepare_to_enter(vcpu);
Currently kvmppc_handle_exit_hv() is called with the vcore lock held because it is called within a for_each_runnable_thread loop. However, we already unlock the vcore within kvmppc_handle_exit_hv() under certain circumstances, and this is safe because (a) any vcpus that become runnable and are added to the runnable set by kvmppc_run_vcpu() have their vcpu->arch.trap == 0 and can't actually run in the guest (because the vcore state is VCORE_EXITING), and (b) for_each_runnable_thread is safe against addition or removal of vcpus from the runnable set. Therefore, in order to simplify things for following patches, let's drop the vcore lock in the for_each_runnable_thread loop, so kvmppc_handle_exit_hv() gets called without the vcore lock held. Signed-off-by: Paul Mackerras <paulus@ozlabs.org> --- arch/powerpc/kvm/book3s_hv.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-)