Re: [RFC PATCH] KVM: PPC: Book3S HV: Remove support for running HPT guest on RPT host without mixed mode support
Nicholas Piggin writes:
> This reverts much of commit c01015091a770 ("KVM: PPC: Book3S HV: Run HPT
> guests on POWER9 radix hosts"), which was required to run HPT guests on
> RPT hosts on early POWER9 CPUs without support for "mixed mode", which
> meant the host could not run with MMU on while guests were running.
Would be worth mentioning which CPU versions.
Looking at the code it seems like it's P9N < 2.2 and P9C < 1.1.
> This code has some corner case bugs, e.g., when the guest hits a machine
> check or HMI the primary locks up waiting for secondaries to switch LPCR
> to host, which they never do. This could all be fixed in software, but
> most CPUs in production have mixed mode support, and those that don't
> are believed to be all in installations that don't use this capability.
> So simplify things and remove support.
Key detail being, AFAICS, you retain enough code to detect that we're in
that configuration and cleanly return an error, rather than crashing or
anything horrible.
Otherwise looks good to me.
cheers
> diff --git a/arch/powerpc/include/asm/kvm_book3s_asm.h
> b/arch/powerpc/include/asm/kvm_book3s_asm.h
> index 078f4648ea27..b6d31bff5209 100644
> --- a/arch/powerpc/include/asm/kvm_book3s_asm.h
> +++ b/arch/powerpc/include/asm/kvm_book3s_asm.h
> @@ -74,16 +74,6 @@ struct kvm_split_mode {
> u8 do_nap;
> u8 napped[MAX_SMT_THREADS];
> struct kvmppc_vcore *vc[MAX_SUBCORES];
> - /* Bits for changing lpcr on P9 */
> - unsigned long lpcr_req;
> - unsigned long lpidr_req;
> - unsigned long host_lpcr;
> - u32 do_set;
> - u32 do_restore;
> - union {
> - u32 allphases;
> - u8 phase[4];
> - } lpcr_sync;
> };
>
> /*
> @@ -110,7 +100,6 @@ struct kvmppc_host_state {
> u8 hwthread_state;
> u8 host_ipi;
> u8 ptid;/* thread number within subcore when split */
> - u8 tid; /* thread number within whole core */
> u8 fake_suspend;
> struct kvm_vcpu *kvm_vcpu;
> struct kvmppc_vcore *kvm_vcore;
> diff --git a/arch/powerpc/kernel/asm-offsets.c
> b/arch/powerpc/kernel/asm-offsets.c
> index c2722ff36e98..21496ea09bf1 100644
> --- a/arch/powerpc/kernel/asm-offsets.c
> +++ b/arch/powerpc/kernel/asm-offsets.c
> @@ -690,7 +690,6 @@ int main(void)
> HSTATE_FIELD(HSTATE_SAVED_XIRR, saved_xirr);
> HSTATE_FIELD(HSTATE_HOST_IPI, host_ipi);
> HSTATE_FIELD(HSTATE_PTID, ptid);
> - HSTATE_FIELD(HSTATE_TID, tid);
> HSTATE_FIELD(HSTATE_FAKE_SUSPEND, fake_suspend);
> HSTATE_FIELD(HSTATE_MMCR0, host_mmcr[0]);
> HSTATE_FIELD(HSTATE_MMCR1, host_mmcr[1]);
> @@ -720,8 +719,6 @@ int main(void)
> OFFSET(KVM_SPLIT_LDBAR, kvm_split_mode, ldbar);
> OFFSET(KVM_SPLIT_DO_NAP, kvm_split_mode, do_nap);
> OFFSET(KVM_SPLIT_NAPPED, kvm_split_mode, napped);
> - OFFSET(KVM_SPLIT_DO_SET, kvm_split_mode, do_set);
> - OFFSET(KVM_SPLIT_DO_RESTORE, kvm_split_mode, do_restore);
> #endif /* CONFIG_KVM_BOOK3S_HV_POSSIBLE */
>
> #ifdef CONFIG_PPC_BOOK3S_64
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index c94f9595133d..86b78f8e3dde 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -134,7 +134,7 @@ static inline bool nesting_enabled(struct kvm *kvm)
> }
>
> /* If set, the threads on each CPU core have to be in the same MMU mode */
> -static bool no_mixing_hpt_and_radix;
> +static bool no_mixing_hpt_and_radix __read_mostly;
>
> static int kvmppc_hv_setup_htab_rma(struct kvm_vcpu *vcpu);
>
> @@ -2855,11 +2855,6 @@ static bool can_dynamic_split(struct kvmppc_vcore *vc,
> struct core_info *cip)
> if (one_vm_per_core && vc->kvm != cip->vc[0]->kvm)
> return false;
>
> - /* Some POWER9 chips require all threads to be in the same MMU mode */
> - if (no_mixing_hpt_and_radix &&
> - kvm_is_radix(vc->kvm) != kvm_is_radix(cip->vc[0]->kvm))
> - return false;
> -
> if (n_threads < cip->max_subcore_threads)
> n_threads = cip->max_subcore_threads;
> if (!subcore_config_ok(cip->n_subcores + 1, n_threads))
> @@ -2898,6 +2893,9 @@ static void prepare_threads(struct kvmppc_vcore *vc)
> for_each_runnable_thread(i, vcpu, vc) {
> if (signal_pending(vcpu->arch.run_task))
> vcpu->arch.ret = -EINTR;
> + else if (no_mixing_hpt_and_radix &&
> + kvm_is_radix(vc->kvm) != radix_enabled())
> + vcpu->arch.ret = -EINVAL;
> else if (vcpu->arch.vpa.update_pending ||
>vcpu->arch.slb_shadow.update_pending ||
>vcpu->arch.dtl.update_pending)
> @@ -3103,7 +3101,6 @@ static noinline void kvmppc_run_core(struct
> kvmppc_vcore *vc)
> int controlled_threads;
> int trap;
> bool is_power8;
> -
[RFC PATCH] KVM: PPC: Book3S HV: Remove support for running HPT guest on RPT host without mixed mode support
This reverts much of commit c01015091a770 ("KVM: PPC: Book3S HV: Run HPT
guests on POWER9 radix hosts"), which was required to run HPT guests on
RPT hosts on early POWER9 CPUs without support for "mixed mode", which
meant the host could not run with MMU on while guests were running.
This code has some corner case bugs, e.g., when the guest hits a machine
check or HMI the primary locks up waiting for secondaries to switch LPCR
to host, which they never do. This could all be fixed in software, but
most CPUs in production have mixed mode support, and those that don't
are believed to be all in installations that don't use this capability.
So simplify things and remove support.
Signed-off-by: Nicholas Piggin
---
arch/powerpc/include/asm/kvm_book3s_asm.h | 11 ---
arch/powerpc/kernel/asm-offsets.c | 3 -
arch/powerpc/kvm/book3s_hv.c | 56 +++--
arch/powerpc/kvm/book3s_hv_builtin.c | 99 +--
arch/powerpc/kvm/book3s_hv_rmhandlers.S | 80 +-
5 files changed, 32 insertions(+), 217 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm_book3s_asm.h
b/arch/powerpc/include/asm/kvm_book3s_asm.h
index 078f4648ea27..b6d31bff5209 100644
--- a/arch/powerpc/include/asm/kvm_book3s_asm.h
+++ b/arch/powerpc/include/asm/kvm_book3s_asm.h
@@ -74,16 +74,6 @@ struct kvm_split_mode {
u8 do_nap;
u8 napped[MAX_SMT_THREADS];
struct kvmppc_vcore *vc[MAX_SUBCORES];
- /* Bits for changing lpcr on P9 */
- unsigned long lpcr_req;
- unsigned long lpidr_req;
- unsigned long host_lpcr;
- u32 do_set;
- u32 do_restore;
- union {
- u32 allphases;
- u8 phase[4];
- } lpcr_sync;
};
/*
@@ -110,7 +100,6 @@ struct kvmppc_host_state {
u8 hwthread_state;
u8 host_ipi;
u8 ptid;/* thread number within subcore when split */
- u8 tid; /* thread number within whole core */
u8 fake_suspend;
struct kvm_vcpu *kvm_vcpu;
struct kvmppc_vcore *kvm_vcore;
diff --git a/arch/powerpc/kernel/asm-offsets.c
b/arch/powerpc/kernel/asm-offsets.c
index c2722ff36e98..21496ea09bf1 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -690,7 +690,6 @@ int main(void)
HSTATE_FIELD(HSTATE_SAVED_XIRR, saved_xirr);
HSTATE_FIELD(HSTATE_HOST_IPI, host_ipi);
HSTATE_FIELD(HSTATE_PTID, ptid);
- HSTATE_FIELD(HSTATE_TID, tid);
HSTATE_FIELD(HSTATE_FAKE_SUSPEND, fake_suspend);
HSTATE_FIELD(HSTATE_MMCR0, host_mmcr[0]);
HSTATE_FIELD(HSTATE_MMCR1, host_mmcr[1]);
@@ -720,8 +719,6 @@ int main(void)
OFFSET(KVM_SPLIT_LDBAR, kvm_split_mode, ldbar);
OFFSET(KVM_SPLIT_DO_NAP, kvm_split_mode, do_nap);
OFFSET(KVM_SPLIT_NAPPED, kvm_split_mode, napped);
- OFFSET(KVM_SPLIT_DO_SET, kvm_split_mode, do_set);
- OFFSET(KVM_SPLIT_DO_RESTORE, kvm_split_mode, do_restore);
#endif /* CONFIG_KVM_BOOK3S_HV_POSSIBLE */
#ifdef CONFIG_PPC_BOOK3S_64
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index c94f9595133d..86b78f8e3dde 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -134,7 +134,7 @@ static inline bool nesting_enabled(struct kvm *kvm)
}
/* If set, the threads on each CPU core have to be in the same MMU mode */
-static bool no_mixing_hpt_and_radix;
+static bool no_mixing_hpt_and_radix __read_mostly;
static int kvmppc_hv_setup_htab_rma(struct kvm_vcpu *vcpu);
@@ -2855,11 +2855,6 @@ static bool can_dynamic_split(struct kvmppc_vcore *vc,
struct core_info *cip)
if (one_vm_per_core && vc->kvm != cip->vc[0]->kvm)
return false;
- /* Some POWER9 chips require all threads to be in the same MMU mode */
- if (no_mixing_hpt_and_radix &&
- kvm_is_radix(vc->kvm) != kvm_is_radix(cip->vc[0]->kvm))
- return false;
-
if (n_threads < cip->max_subcore_threads)
n_threads = cip->max_subcore_threads;
if (!subcore_config_ok(cip->n_subcores + 1, n_threads))
@@ -2898,6 +2893,9 @@ static void prepare_threads(struct kvmppc_vcore *vc)
for_each_runnable_thread(i, vcpu, vc) {
if (signal_pending(vcpu->arch.run_task))
vcpu->arch.ret = -EINTR;
+ else if (no_mixing_hpt_and_radix &&
+kvm_is_radix(vc->kvm) != radix_enabled())
+ vcpu->arch.ret = -EINVAL;
else if (vcpu->arch.vpa.update_pending ||
vcpu->arch.slb_shadow.update_pending ||
vcpu->arch.dtl.update_pending)
@@ -3103,7 +3101,6 @@ static noinline void kvmppc_run_core(struct kvmppc_vcore
*vc)
int controlled_threads;
int trap;
bool is_power8;
- bool hpt_on_radix;
/*
*
