From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Huth Subject: Re: [PATCH] KVM: PPC: Book3S HV: Context-switch EBB registers properly Date: Thu, 8 Jun 2017 07:41:20 +0200 Message-ID: <976a2b8d-786b-2310-f169-d66bb9194d79@redhat.com> References: <20170606070908.GB20869@fergus.ozlabs.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: David Gibson To: Paul Mackerras , kvm@vger.kernel.org, kvm-ppc@vger.kernel.org Return-path: Received: from mx1.redhat.com ([209.132.183.28]:46984 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750725AbdFHFlY (ORCPT ); Thu, 8 Jun 2017 01:41:24 -0400 In-Reply-To: <20170606070908.GB20869@fergus.ozlabs.ibm.com> Content-Language: en-US Sender: kvm-owner@vger.kernel.org List-ID: On 06.06.2017 09:09, Paul Mackerras wrote: > This adds code to save the values of three SPRs (special-purpose > registers) used by userspace to control event-based branches (EBBs), > which are essentially interrupts that get delivered directly to > userspace. These registers are loaded up with guest values when > entering the guest, and their values are saved when exiting the > guest, but we were not saving the host values and restoring them > before going back to userspace. > > On POWER8 this would only affect userspace programs which explicitly > request the use of EBBs and also use the KVM_RUN ioctl, since the > only source of EBBs on POWER8 is the PMU, and there is an explicit > enable bit in the PMU registers (and those PMU registers do get > properly context-switched between host and guest). On POWER9 there > is provision for externally-generated EBBs, and these are not subject > to the control in the PMU registers. > > Since these registers only affect userspace, we can save them when > we first come in from userspace and restore them before returning to > userspace, rather than saving/restoring the host values on every > guest entry/exit. Similarly, we don't need to worry about their > values on offline secondary threads since they execute in the context > of the idle task, which never executes in userspace. > > Fixes: b005255e12a3 ("KVM: PPC: Book3S HV: Context-switch new POWER8 SPRs", 2014-01-08) > Cc: stable@vger.kernel.org # v3.14+ > Signed-off-by: Paul Mackerras > --- > arch/powerpc/kvm/book3s_hv.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c > index 1344da7..66740cb 100644 > --- a/arch/powerpc/kvm/book3s_hv.c > +++ b/arch/powerpc/kvm/book3s_hv.c > @@ -3102,6 +3102,7 @@ static int kvmppc_vcpu_run_hv(struct kvm_run *run, struct kvm_vcpu *vcpu) > { > int r; > int srcu_idx; > + unsigned long ebb_regs[3] = {}; /* shut up GCC */ > > if (!vcpu->arch.sane) { > run->exit_reason = KVM_EXIT_INTERNAL_ERROR; > @@ -3129,6 +3130,13 @@ static int kvmppc_vcpu_run_hv(struct kvm_run *run, struct kvm_vcpu *vcpu) > > flush_all_to_thread(current); > > + /* Save userspace EBB register values */ > + if (cpu_has_feature(CPU_FTR_ARCH_207S)) { > + ebb_regs[0] = mfspr(SPRN_EBBHR); > + ebb_regs[1] = mfspr(SPRN_EBBRR); > + ebb_regs[2] = mfspr(SPRN_BESCR); > + } > + > vcpu->arch.wqp = &vcpu->arch.vcore->wq; > vcpu->arch.pgdir = current->mm->pgd; > vcpu->arch.state = KVMPPC_VCPU_BUSY_IN_HOST; > @@ -3155,6 +3163,13 @@ static int kvmppc_vcpu_run_hv(struct kvm_run *run, struct kvm_vcpu *vcpu) > } > } while (is_kvmppc_resume_guest(r)); > > + /* Restore userspace EBB register values */ > + if (cpu_has_feature(CPU_FTR_ARCH_207S)) { > + mtspr(SPRN_EBBHR, ebb_regs[0]); > + mtspr(SPRN_EBBRR, ebb_regs[1]); > + mtspr(SPRN_BESCR, ebb_regs[2]); > + } > + > out: > vcpu->arch.state = KVMPPC_VCPU_NOTREADY; > atomic_dec(&vcpu->kvm->arch.vcpus_running); Reviewed-by: Thomas Huth From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Huth Date: Thu, 08 Jun 2017 05:41:20 +0000 Subject: Re: [PATCH] KVM: PPC: Book3S HV: Context-switch EBB registers properly Message-Id: <976a2b8d-786b-2310-f169-d66bb9194d79@redhat.com> List-Id: References: <20170606070908.GB20869@fergus.ozlabs.ibm.com> In-Reply-To: <20170606070908.GB20869@fergus.ozlabs.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Paul Mackerras , kvm@vger.kernel.org, kvm-ppc@vger.kernel.org Cc: David Gibson On 06.06.2017 09:09, Paul Mackerras wrote: > This adds code to save the values of three SPRs (special-purpose > registers) used by userspace to control event-based branches (EBBs), > which are essentially interrupts that get delivered directly to > userspace. These registers are loaded up with guest values when > entering the guest, and their values are saved when exiting the > guest, but we were not saving the host values and restoring them > before going back to userspace. > > On POWER8 this would only affect userspace programs which explicitly > request the use of EBBs and also use the KVM_RUN ioctl, since the > only source of EBBs on POWER8 is the PMU, and there is an explicit > enable bit in the PMU registers (and those PMU registers do get > properly context-switched between host and guest). On POWER9 there > is provision for externally-generated EBBs, and these are not subject > to the control in the PMU registers. > > Since these registers only affect userspace, we can save them when > we first come in from userspace and restore them before returning to > userspace, rather than saving/restoring the host values on every > guest entry/exit. Similarly, we don't need to worry about their > values on offline secondary threads since they execute in the context > of the idle task, which never executes in userspace. > > Fixes: b005255e12a3 ("KVM: PPC: Book3S HV: Context-switch new POWER8 SPRs", 2014-01-08) > Cc: stable@vger.kernel.org # v3.14+ > Signed-off-by: Paul Mackerras > --- > arch/powerpc/kvm/book3s_hv.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c > index 1344da7..66740cb 100644 > --- a/arch/powerpc/kvm/book3s_hv.c > +++ b/arch/powerpc/kvm/book3s_hv.c > @@ -3102,6 +3102,7 @@ static int kvmppc_vcpu_run_hv(struct kvm_run *run, struct kvm_vcpu *vcpu) > { > int r; > int srcu_idx; > + unsigned long ebb_regs[3] = {}; /* shut up GCC */ > > if (!vcpu->arch.sane) { > run->exit_reason = KVM_EXIT_INTERNAL_ERROR; > @@ -3129,6 +3130,13 @@ static int kvmppc_vcpu_run_hv(struct kvm_run *run, struct kvm_vcpu *vcpu) > > flush_all_to_thread(current); > > + /* Save userspace EBB register values */ > + if (cpu_has_feature(CPU_FTR_ARCH_207S)) { > + ebb_regs[0] = mfspr(SPRN_EBBHR); > + ebb_regs[1] = mfspr(SPRN_EBBRR); > + ebb_regs[2] = mfspr(SPRN_BESCR); > + } > + > vcpu->arch.wqp = &vcpu->arch.vcore->wq; > vcpu->arch.pgdir = current->mm->pgd; > vcpu->arch.state = KVMPPC_VCPU_BUSY_IN_HOST; > @@ -3155,6 +3163,13 @@ static int kvmppc_vcpu_run_hv(struct kvm_run *run, struct kvm_vcpu *vcpu) > } > } while (is_kvmppc_resume_guest(r)); > > + /* Restore userspace EBB register values */ > + if (cpu_has_feature(CPU_FTR_ARCH_207S)) { > + mtspr(SPRN_EBBHR, ebb_regs[0]); > + mtspr(SPRN_EBBRR, ebb_regs[1]); > + mtspr(SPRN_BESCR, ebb_regs[2]); > + } > + > out: > vcpu->arch.state = KVMPPC_VCPU_NOTREADY; > atomic_dec(&vcpu->kvm->arch.vcpus_running); Reviewed-by: Thomas Huth