Re: [PATCH] KVM: PPC: Book3S HV: Context-switch EBB registers properly

[Thomas Huth <thuth@redhat.com>]

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 <paulus@ozlabs.org>
> ---
>  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 <thuth@redhat.com>