Re: [PATCH 1/2] KVM: PPC: Book3S PR: Disable SCV when running AIL is disabled

[Fabiano Rosas <farosas@linux.ibm.com>]

Nicholas Piggin <npiggin@gmail.com> writes:

> PR KVM does not support running with AIL enabled, and SCV does is not
> supported with AIL disabled.
>
> Fix this by ensuring the SCV facility is disabled with FSCR while a
> CPU can be running with AIL=0. PowerNV host supports disabling AIL on a
> per-CPU basis, so SCV just needs to be disabled when a vCPU is run.
>
> The pSeries machine can only switch AIL on a system-wide basis, so it
> must disable SCV support at boot if the configuration can potentially
> run a PR KVM guest.
>
> SCV is not emulated for the PR guest at the moment, this just fixes the
> host crashes.
>
> Alternatives considered and rejected:
> - SCV support can not be disabled by PR KVM after boot, because it is
>   advertised to userspace with HWCAP.
> - AIL can not be disabled on a per-CPU basis. At least when running on
>   pseries it is a per-LPAR setting.
> - Support for real-mode SCV vectors will not be added because they are
>   at 0x17000 so making such a large fixed head space causes immediate
>   value limits to be exceeded, requiring a lot rework and more code.
> - Disabling SCV for any PR KVM possible kernel will cause a slowdown
>   when not using PR KVM.
> - A boot time option to disable SCV to use PR KVM is user-hostile.
> - System call instruction emulation for SCV facility unavailable
>   instructions is too complex and old emulation code was subtly broken
>   and removed.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  arch/powerpc/kernel/exceptions-64s.S |  4 ++++
>  arch/powerpc/kernel/setup_64.c       | 15 +++++++++++++++
>  arch/powerpc/kvm/book3s_pr.c         | 20 ++++++++++++++------
>  3 files changed, 33 insertions(+), 6 deletions(-)
>
> diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
> index 55caeee37c08..b66dd6f775a4 100644
> --- a/arch/powerpc/kernel/exceptions-64s.S
> +++ b/arch/powerpc/kernel/exceptions-64s.S
> @@ -809,6 +809,10 @@ __start_interrupts:
>   * - MSR_EE|MSR_RI is clear (no reentrant exceptions)
>   * - Standard kernel environment is set up (stack, paca, etc)
>   *
> + * KVM:
> + * These interrupts do not elevate HV 0->1, so HV is not involved. PR KVM
> + * ensures that FSCR[SCV] is disabled whenever it has to force AIL off.
> + *
>   * Call convention:
>   *
>   * syscall register convention is in Documentation/powerpc/syscall64-abi.rst
> diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
> index be8577ac9397..ac52c69a3811 100644
> --- a/arch/powerpc/kernel/setup_64.c
> +++ b/arch/powerpc/kernel/setup_64.c
> @@ -197,6 +197,21 @@ static void __init configure_exceptions(void)
>
>  	/* Under a PAPR hypervisor, we need hypercalls */
>  	if (firmware_has_feature(FW_FEATURE_SET_MODE)) {
> +		/*
> +		 * PR KVM does not support AIL mode interrupts in the host, and
> +		 * SCV system call interrupt vectors are only implemented for
> +		 * AIL mode. Under pseries, AIL mode can only be enabled and
> +		 * disabled system-wide so when PR KVM is loaded, all CPUs in
> +		 * the host are set to AIL=0 mode. SCV can not be disabled
> +		 * dynamically because the feature is advertised to host
> +		 * userspace, so SCV support must not be enabled if PR KVM can
> +		 * possibly be run.
> +		 */
> +		if (IS_ENABLED(CONFIG_KVM_BOOK3S_PR_POSSIBLE) && !radix_enabled()) {
> +			init_task.thread.fscr &= ~FSCR_SCV;
> +			cur_cpu_spec->cpu_user_features2 &= ~PPC_FEATURE2_SCV;
> +		}
> +

"Under pseries, AIL mode can only be enabled and disabled system-wide so
 when PR KVM is loaded, all CPUs in the host are set to AIL=0 mode."

Loaded as in 'modprobe kvm_pr'? And host as in "nested host"
surely. Unless I completely misunderstood the patch (likely).

Is there a way to make this less unexpected to users? Maybe a few words
in the Kconfig entry for PR_POSSIBLE saying "if you enable this and run
a Hash MMU guest, you lose SCV"?

>  		/* Enable AIL if possible */
>  		if (!pseries_enable_reloc_on_exc()) {
>  			init_task.thread.fscr &= ~FSCR_SCV;
> diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
> index 34a801c3604a..4d1c84b94b77 100644
> --- a/arch/powerpc/kvm/book3s_pr.c
> +++ b/arch/powerpc/kvm/book3s_pr.c
> @@ -140,9 +140,12 @@ static void kvmppc_core_vcpu_load_pr(struct kvm_vcpu *vcpu, int cpu)
>  #endif
>
>  	/* Disable AIL if supported */
> -	if (cpu_has_feature(CPU_FTR_HVMODE) &&
> -	    cpu_has_feature(CPU_FTR_ARCH_207S))
> -		mtspr(SPRN_LPCR, mfspr(SPRN_LPCR) & ~LPCR_AIL);
> +	if (cpu_has_feature(CPU_FTR_HVMODE)) {
> +		if (cpu_has_feature(CPU_FTR_ARCH_207S))
> +			mtspr(SPRN_LPCR, mfspr(SPRN_LPCR) & ~LPCR_AIL);
> +		if (cpu_has_feature(CPU_FTR_ARCH_300) && (current->thread.fscr & FSCR_SCV))
> +			mtspr(SPRN_FSCR, mfspr(SPRN_FSCR) & ~FSCR_SCV);
> +	}
>
>  	vcpu->cpu = smp_processor_id();
>  #ifdef CONFIG_PPC_BOOK3S_32
> @@ -175,9 +178,12 @@ static void kvmppc_core_vcpu_put_pr(struct kvm_vcpu *vcpu)
>  	kvmppc_save_tm_pr(vcpu);
>
>  	/* Enable AIL if supported */
> -	if (cpu_has_feature(CPU_FTR_HVMODE) &&
> -	    cpu_has_feature(CPU_FTR_ARCH_207S))
> -		mtspr(SPRN_LPCR, mfspr(SPRN_LPCR) | LPCR_AIL_3);
> +	if (cpu_has_feature(CPU_FTR_HVMODE)) {
> +		if (cpu_has_feature(CPU_FTR_ARCH_207S))
> +			mtspr(SPRN_LPCR, mfspr(SPRN_LPCR) | LPCR_AIL_3);
> +		if (cpu_has_feature(CPU_FTR_ARCH_300) && (current->thread.fscr & FSCR_SCV))
> +			mtspr(SPRN_FSCR, mfspr(SPRN_FSCR) | FSCR_SCV);
> +	}
>
>  	vcpu->cpu = -1;
>  }
> @@ -1037,6 +1043,8 @@ static int kvmppc_handle_fac(struct kvm_vcpu *vcpu, ulong fac)
>
>  void kvmppc_set_fscr(struct kvm_vcpu *vcpu, u64 fscr)
>  {
> +	if (fscr & FSCR_SCV)
> +		fscr &= ~FSCR_SCV; /* SCV must not be enabled */
>  	if ((vcpu->arch.fscr & FSCR_TAR) && !(fscr & FSCR_TAR)) {
>  		/* TAR got dropped, drop it in shadow too */
>  		kvmppc_giveup_fac(vcpu, FSCR_TAR_LG);