| Message ID | 20220124102417.3741427-2-npiggin@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | KVM: PPC: Book3S PR: SCV fixes | expand |
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);
Excerpts from Fabiano Rosas's message of January 25, 2022 8:49 am: > 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'? 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); >
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; + } + /* 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);
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(-)