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); >