Excerpts from Fabiano Rosas's message of January 25, 2022 8:49 am:
> Nicholas Piggin <npig...@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 <npig...@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'?
In kvmppc_core_init_vm_pr(), so while there is a PR guest running in the
system.
> And host as in "nested host"
> surely. Unless I completely misunderstood the patch (likely).
Yes the PR KVM host. I didn't want to say nested because it runs in
supervisor mode so is basically no difference whether under a HV or
not so I'm not sure if that's the right term for PR KVM or could
confused with nested HV.
I will see if I can make it a bit clearer.
> 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"?
That's not a bad idea, also if you run a PR guest under it you lose
AIL in the host which also slows down interrupts and system calls.
Thanks,
Nick
>
>> /* 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);
>
