| Message ID | 20130806042337.GT19254@iris.ozlabs.ibm.com |
|---|---|
| State | New, archived |
| Headers | show |
On 06.08.2013, at 06:23, Paul Mackerras wrote: > When we are running a PR KVM guest on POWER8, we have to disable the > new POWER8 feature of taking interrupts with relocation on, that is, > of taking interrupts without disabling the MMU, because the SLB does > not contain the normal kernel SLB entries while in the guest. > Currently we disable relocation-on interrupts when a PR guest is > created, and leave it disabled until there are no more PR guests in > existence. > > This defers the disabling of relocation-on interrupts until the first It would've been nice to see the original patch on kvm-ppc@vger. > time a PR KVM guest vcpu is run. The reason is that in future we will > support both PR and HV guests in the same kernel, and this will avoid > disabling relocation-on interrupts unnecessarily for guests which turn > out to be HV guests, as we will not know at VM creation time whether > it will be a PR or a HV guest. > > Signed-off-by: Paul Mackerras <paulus@samba.org> > --- > arch/powerpc/include/asm/kvm_host.h | 1 + > arch/powerpc/kvm/book3s_pr.c | 71 ++++++++++++++++++++++++++----------- > 2 files changed, 52 insertions(+), 20 deletions(-) > > diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h > index 4d83972..c012db2 100644 > --- a/arch/powerpc/include/asm/kvm_host.h > +++ b/arch/powerpc/include/asm/kvm_host.h > @@ -264,6 +264,7 @@ struct kvm_arch { > #endif /* CONFIG_KVM_BOOK3S_64_HV */ > #ifdef CONFIG_KVM_BOOK3S_PR > struct mutex hpt_mutex; > + bool relon_disabled; > #endif > #ifdef CONFIG_PPC_BOOK3S_64 > struct list_head spapr_tce_tables; > diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c > index 5b06a70..2759ddc 100644 > --- a/arch/powerpc/kvm/book3s_pr.c > +++ b/arch/powerpc/kvm/book3s_pr.c > @@ -1197,6 +1197,47 @@ void kvmppc_core_vcpu_free(struct kvm_vcpu *vcpu) > kmem_cache_free(kvm_vcpu_cache, vcpu); > } > > +/* > + * On POWER8, we have to disable relocation-on interrupts while > + * we are in the guest, since the guest doesn't have the normal > + * kernel SLB contents. Since disabling relocation-on interrupts > + * is a fairly heavy-weight operation, we do it once when starting > + * the first guest vcpu and leave it disabled until the last guest > + * has been destroyed. > + */ > +static unsigned int kvm_global_user_count = 0; > +static DEFINE_SPINLOCK(kvm_global_user_count_lock); > + > +static void disable_relon_interrupts(struct kvm *kvm) > +{ > + mutex_lock(&kvm->lock); > + if (!kvm->arch.relon_disabled) { > + if (firmware_has_feature(FW_FEATURE_SET_MODE)) { Is this the same as the endianness setting rtas call? If so, would a PR guest in an HV guest that provides only endianness setting but no relocation-on setting confuse any of this code? Alex > + spin_lock(&kvm_global_user_count_lock); > + if (++kvm_global_user_count == 1) > + pSeries_disable_reloc_on_exc(); > + spin_unlock(&kvm_global_user_count_lock); > + } > + /* order disabling above with setting relon_disabled */ > + smp_mb(); > + kvm->arch.relon_disabled = true; > + } > + mutex_unlock(&kvm->lock); > +} > + > +static void enable_relon_interrupts(struct kvm *kvm) > +{ > + if (kvm->arch.relon_disabled && > + firmware_has_feature(FW_FEATURE_SET_MODE)) { > + spin_lock(&kvm_global_user_count_lock); > + BUG_ON(kvm_global_user_count == 0); > + if (--kvm_global_user_count == 0) > + pSeries_enable_reloc_on_exc(); > + spin_unlock(&kvm_global_user_count_lock); > + } > + kvm->arch.relon_disabled = false; > +} > + > int kvmppc_vcpu_run(struct kvm_run *kvm_run, struct kvm_vcpu *vcpu) > { > int ret; > @@ -1234,6 +1275,9 @@ int kvmppc_vcpu_run(struct kvm_run *kvm_run, struct kvm_vcpu *vcpu) > goto out; > } > > + if (!vcpu->kvm->arch.relon_disabled) > + disable_relon_interrupts(vcpu->kvm); > + > /* Save FPU state in stack */ > if (current->thread.regs->msr & MSR_FP) > giveup_fpu(current); > @@ -1400,9 +1444,6 @@ void kvmppc_core_flush_memslot(struct kvm *kvm, struct kvm_memory_slot *memslot) > { > } > > -static unsigned int kvm_global_user_count = 0; > -static DEFINE_SPINLOCK(kvm_global_user_count_lock); > - > int kvmppc_core_init_vm(struct kvm *kvm) > { > #ifdef CONFIG_PPC64 > @@ -1411,28 +1452,18 @@ int kvmppc_core_init_vm(struct kvm *kvm) > #endif > mutex_init(&kvm->arch.hpt_mutex); > > - if (firmware_has_feature(FW_FEATURE_SET_MODE)) { > - spin_lock(&kvm_global_user_count_lock); > - if (++kvm_global_user_count == 1) > - pSeries_disable_reloc_on_exc(); > - spin_unlock(&kvm_global_user_count_lock); > - } > + /* > + * If we don't have relocation-on interrupts at all, > + * then we can consider them to be already disabled. > + */ > + kvm->arch.relon_disabled = !firmware_has_feature(FW_FEATURE_SET_MODE); > + > return 0; > } > > void kvmppc_core_destroy_vm(struct kvm *kvm) > { > -#ifdef CONFIG_PPC64 > - WARN_ON(!list_empty(&kvm->arch.spapr_tce_tables)); > -#endif > - > - if (firmware_has_feature(FW_FEATURE_SET_MODE)) { > - spin_lock(&kvm_global_user_count_lock); > - BUG_ON(kvm_global_user_count == 0); > - if (--kvm_global_user_count == 0) > - pSeries_enable_reloc_on_exc(); > - spin_unlock(&kvm_global_user_count_lock); > - } > + enable_relon_interrupts(kvm); > } > > static int kvmppc_book3s_init(void) > -- > 1.8.3.1 > -- 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 Fri, Aug 30, 2013 at 06:30:50PM +0200, Alexander Graf wrote: > > On 06.08.2013, at 06:23, Paul Mackerras wrote: > > > When we are running a PR KVM guest on POWER8, we have to disable the > > new POWER8 feature of taking interrupts with relocation on, that is, > > of taking interrupts without disabling the MMU, because the SLB does > > not contain the normal kernel SLB entries while in the guest. > > Currently we disable relocation-on interrupts when a PR guest is > > created, and leave it disabled until there are no more PR guests in > > existence. > > > > This defers the disabling of relocation-on interrupts until the first > > It would've been nice to see the original patch on kvm-ppc@vger. Here are the headers from my copy of the original mail: > Date: Tue, 6 Aug 2013 14:23:37 +1000 > From: Paul Mackerras <paulus@samba.org> > To: Alexander Graf <agraf@suse.de>, Benjamin Herrenschmidt <benh@kernel.crashing.org> > Cc: kvm-ppc@vger.kernel.org, kvm@vger.kernel.org > Subject: [PATCH 14/23] KVM: PPC: Book3S PR: Delay disabling relocation-on interrupts So as far as I can see, I *did* cc it to kvm-ppc@vger. > > + if (!kvm->arch.relon_disabled) { > > + if (firmware_has_feature(FW_FEATURE_SET_MODE)) { > > Is this the same as the endianness setting rtas call? If so, would a PR guest in an HV guest that provides only endianness setting but no relocation-on setting confuse any of this code? It is the same hcall, but since the interrupts-with-relocation-on function was defined in the first PAPR version that has H_SET_MODE, we shouldn't ever hit that situation. In any case, if we did happen to run under a (non PAPR-compliant) hypervisor that implemented H_SET_MODE but not the relocation-on setting, then we couldn't have enabled relocation-on interrupts in the first place, so it wouldn't matter. Paul. -- 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 31.08.2013, at 00:55, Paul Mackerras wrote: > On Fri, Aug 30, 2013 at 06:30:50PM +0200, Alexander Graf wrote: >> >> On 06.08.2013, at 06:23, Paul Mackerras wrote: >> >>> When we are running a PR KVM guest on POWER8, we have to disable the >>> new POWER8 feature of taking interrupts with relocation on, that is, >>> of taking interrupts without disabling the MMU, because the SLB does >>> not contain the normal kernel SLB entries while in the guest. >>> Currently we disable relocation-on interrupts when a PR guest is >>> created, and leave it disabled until there are no more PR guests in >>> existence. >>> >>> This defers the disabling of relocation-on interrupts until the first >> >> It would've been nice to see the original patch on kvm-ppc@vger. > > Here are the headers from my copy of the original mail: > >> Date: Tue, 6 Aug 2013 14:23:37 +1000 >> From: Paul Mackerras <paulus@samba.org> >> To: Alexander Graf <agraf@suse.de>, Benjamin Herrenschmidt <benh@kernel.crashing.org> >> Cc: kvm-ppc@vger.kernel.org, kvm@vger.kernel.org >> Subject: [PATCH 14/23] KVM: PPC: Book3S PR: Delay disabling relocation-on interrupts > > So as far as I can see, I *did* cc it to kvm-ppc@vger. Oh, sorry to not be more explicit here. I meant the one that actually introduced the relocation-on handling: https://lists.ozlabs.org/pipermail/linuxppc-dev/2012-December/102355.html I can't find any trace of that in my inbox, even though it clearly touches KVM PPC code. > >>> + if (!kvm->arch.relon_disabled) { >>> + if (firmware_has_feature(FW_FEATURE_SET_MODE)) { >> >> Is this the same as the endianness setting rtas call? If so, would a PR guest in an HV guest that provides only endianness setting but no relocation-on setting confuse any of this code? > > It is the same hcall, but since the interrupts-with-relocation-on > function was defined in the first PAPR version that has H_SET_MODE, > we shouldn't ever hit that situation. In any case, if we did happen > to run under a (non PAPR-compliant) hypervisor that implemented > H_SET_MODE but not the relocation-on setting, then we couldn't have > enabled relocation-on interrupts in the first place, so it wouldn't > matter. Well, I think Anton's patches do exactly that: https://lists.nongnu.org/archive/html/qemu-ppc/2013-08/msg00253.html I really just want to double-check that we're not shooting ourselves in the foot here. 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 Sat, Aug 31, 2013 at 01:13:07AM +0200, Alexander Graf wrote: > > Oh, sorry to not be more explicit here. I meant the one that actually introduced the relocation-on handling: > > https://lists.ozlabs.org/pipermail/linuxppc-dev/2012-December/102355.html > > I can't find any trace of that in my inbox, even though it clearly touches KVM PPC code. True, Ian should have cc'd it to kvm-ppc@vger, I'll mention it to him. > > > > >>> + if (!kvm->arch.relon_disabled) { > >>> + if (firmware_has_feature(FW_FEATURE_SET_MODE)) { > >> > >> Is this the same as the endianness setting rtas call? If so, would a PR guest in an HV guest that provides only endianness setting but no relocation-on setting confuse any of this code? > > > > It is the same hcall, but since the interrupts-with-relocation-on > > function was defined in the first PAPR version that has H_SET_MODE, > > we shouldn't ever hit that situation. In any case, if we did happen > > to run under a (non PAPR-compliant) hypervisor that implemented > > H_SET_MODE but not the relocation-on setting, then we couldn't have > > enabled relocation-on interrupts in the first place, so it wouldn't > > matter. > > Well, I think Anton's patches do exactly that: > > https://lists.nongnu.org/archive/html/qemu-ppc/2013-08/msg00253.html > > I really just want to double-check that we're not shooting ourselves in the foot here. I still think there's no real problem, since there would be no other way to enable relocation-on interrupts other than H_SET_MODE. So if H_SET_MODE can't control that setting, then it must be disabled already. However, we should also make sure that H_SET_MODE supports changing the relocation-on setting when it first goes in. I'm going to want that soon anyway since I'm working on POWER8 KVM support at the moment. Paul. -- 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/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h index 4d83972..c012db2 100644 --- a/arch/powerpc/include/asm/kvm_host.h +++ b/arch/powerpc/include/asm/kvm_host.h @@ -264,6 +264,7 @@ struct kvm_arch { #endif /* CONFIG_KVM_BOOK3S_64_HV */ #ifdef CONFIG_KVM_BOOK3S_PR struct mutex hpt_mutex; + bool relon_disabled; #endif #ifdef CONFIG_PPC_BOOK3S_64 struct list_head spapr_tce_tables; diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c index 5b06a70..2759ddc 100644 --- a/arch/powerpc/kvm/book3s_pr.c +++ b/arch/powerpc/kvm/book3s_pr.c @@ -1197,6 +1197,47 @@ void kvmppc_core_vcpu_free(struct kvm_vcpu *vcpu) kmem_cache_free(kvm_vcpu_cache, vcpu); } +/* + * On POWER8, we have to disable relocation-on interrupts while + * we are in the guest, since the guest doesn't have the normal + * kernel SLB contents. Since disabling relocation-on interrupts + * is a fairly heavy-weight operation, we do it once when starting + * the first guest vcpu and leave it disabled until the last guest + * has been destroyed. + */ +static unsigned int kvm_global_user_count = 0; +static DEFINE_SPINLOCK(kvm_global_user_count_lock); + +static void disable_relon_interrupts(struct kvm *kvm) +{ + mutex_lock(&kvm->lock); + if (!kvm->arch.relon_disabled) { + if (firmware_has_feature(FW_FEATURE_SET_MODE)) { + spin_lock(&kvm_global_user_count_lock); + if (++kvm_global_user_count == 1) + pSeries_disable_reloc_on_exc(); + spin_unlock(&kvm_global_user_count_lock); + } + /* order disabling above with setting relon_disabled */ + smp_mb(); + kvm->arch.relon_disabled = true; + } + mutex_unlock(&kvm->lock); +} + +static void enable_relon_interrupts(struct kvm *kvm) +{ + if (kvm->arch.relon_disabled && + firmware_has_feature(FW_FEATURE_SET_MODE)) { + spin_lock(&kvm_global_user_count_lock); + BUG_ON(kvm_global_user_count == 0); + if (--kvm_global_user_count == 0) + pSeries_enable_reloc_on_exc(); + spin_unlock(&kvm_global_user_count_lock); + } + kvm->arch.relon_disabled = false; +} + int kvmppc_vcpu_run(struct kvm_run *kvm_run, struct kvm_vcpu *vcpu) { int ret; @@ -1234,6 +1275,9 @@ int kvmppc_vcpu_run(struct kvm_run *kvm_run, struct kvm_vcpu *vcpu) goto out; } + if (!vcpu->kvm->arch.relon_disabled) + disable_relon_interrupts(vcpu->kvm); + /* Save FPU state in stack */ if (current->thread.regs->msr & MSR_FP) giveup_fpu(current); @@ -1400,9 +1444,6 @@ void kvmppc_core_flush_memslot(struct kvm *kvm, struct kvm_memory_slot *memslot) { } -static unsigned int kvm_global_user_count = 0; -static DEFINE_SPINLOCK(kvm_global_user_count_lock); - int kvmppc_core_init_vm(struct kvm *kvm) { #ifdef CONFIG_PPC64 @@ -1411,28 +1452,18 @@ int kvmppc_core_init_vm(struct kvm *kvm) #endif mutex_init(&kvm->arch.hpt_mutex); - if (firmware_has_feature(FW_FEATURE_SET_MODE)) { - spin_lock(&kvm_global_user_count_lock); - if (++kvm_global_user_count == 1) - pSeries_disable_reloc_on_exc(); - spin_unlock(&kvm_global_user_count_lock); - } + /* + * If we don't have relocation-on interrupts at all, + * then we can consider them to be already disabled. + */ + kvm->arch.relon_disabled = !firmware_has_feature(FW_FEATURE_SET_MODE); + return 0; } void kvmppc_core_destroy_vm(struct kvm *kvm) { -#ifdef CONFIG_PPC64 - WARN_ON(!list_empty(&kvm->arch.spapr_tce_tables)); -#endif - - if (firmware_has_feature(FW_FEATURE_SET_MODE)) { - spin_lock(&kvm_global_user_count_lock); - BUG_ON(kvm_global_user_count == 0); - if (--kvm_global_user_count == 0) - pSeries_enable_reloc_on_exc(); - spin_unlock(&kvm_global_user_count_lock); - } + enable_relon_interrupts(kvm); } static int kvmppc_book3s_init(void)
When we are running a PR KVM guest on POWER8, we have to disable the new POWER8 feature of taking interrupts with relocation on, that is, of taking interrupts without disabling the MMU, because the SLB does not contain the normal kernel SLB entries while in the guest. Currently we disable relocation-on interrupts when a PR guest is created, and leave it disabled until there are no more PR guests in existence. This defers the disabling of relocation-on interrupts until the first time a PR KVM guest vcpu is run. The reason is that in future we will support both PR and HV guests in the same kernel, and this will avoid disabling relocation-on interrupts unnecessarily for guests which turn out to be HV guests, as we will not know at VM creation time whether it will be a PR or a HV guest. Signed-off-by: Paul Mackerras <paulus@samba.org> --- arch/powerpc/include/asm/kvm_host.h | 1 + arch/powerpc/kvm/book3s_pr.c | 71 ++++++++++++++++++++++++++----------- 2 files changed, 52 insertions(+), 20 deletions(-)