Re: [PATCH v3 34/41] KVM: PPC: Book3S HV: Remove support for dependent threads mode on P9

["Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>]

Nicholas Piggin <npiggin@gmail.com> writes:

> Radix guest support will be removed from the P7/8 path, so disallow
> dependent threads mode on P9.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  arch/powerpc/include/asm/kvm_host.h |  1 -
>  arch/powerpc/kvm/book3s_hv.c        | 27 +++++----------------------
>  2 files changed, 5 insertions(+), 23 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
> index 05fb00d37609..dd017dfa4e65 100644
> --- a/arch/powerpc/include/asm/kvm_host.h
> +++ b/arch/powerpc/include/asm/kvm_host.h
> @@ -304,7 +304,6 @@ struct kvm_arch {
>  	u8 fwnmi_enabled;
>  	u8 secure_guest;
>  	u8 svm_enabled;
> -	bool threads_indep;
>  	bool nested_enable;
>  	bool dawr1_enabled;
>  	pgd_t *pgtable;
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index cb428e2f7140..928ed8180d9d 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -103,13 +103,9 @@ static int target_smt_mode;
>  module_param(target_smt_mode, int, 0644);
>  MODULE_PARM_DESC(target_smt_mode, "Target threads per core (0 = max)");
>  
> -static bool indep_threads_mode = true;
> -module_param(indep_threads_mode, bool, S_IRUGO | S_IWUSR);
> -MODULE_PARM_DESC(indep_threads_mode, "Independent-threads mode (only on POWER9)");
> -
>  static bool one_vm_per_core;
>  module_param(one_vm_per_core, bool, S_IRUGO | S_IWUSR);
> -MODULE_PARM_DESC(one_vm_per_core, "Only run vCPUs from the same VM on a core (requires indep_threads_mode=N)");
> +MODULE_PARM_DESC(one_vm_per_core, "Only run vCPUs from the same VM on a core (requires POWER8 or older)");

Isn't this also a security feature, where there was an ask to make sure
threads/vCPU from other VM won't run on this core? In that context isn't
this applicable also for P9?


>  
>  #ifdef CONFIG_KVM_XICS
>  static const struct kernel_param_ops module_param_ops = {
> @@ -2227,7 +2223,7 @@ static int kvmppc_set_one_reg_hv(struct kvm_vcpu *vcpu, u64 id,
>   */
>  static int threads_per_vcore(struct kvm *kvm)
>  {
> -	if (kvm->arch.threads_indep)
> +	if (cpu_has_feature(CPU_FTR_ARCH_300))
>  		return 1;
>  	return threads_per_subcore;
>  }
> @@ -4319,7 +4315,7 @@ static int kvmppc_vcpu_run_hv(struct kvm_vcpu *vcpu)
>  	vcpu->arch.state = KVMPPC_VCPU_BUSY_IN_HOST;
>  
>  	do {
> -		if (kvm->arch.threads_indep && kvm_is_radix(kvm))
> +		if (kvm_is_radix(kvm))
>  			r = kvmhv_run_single_vcpu(vcpu, ~(u64)0,
>  						  vcpu->arch.vcore->lpcr);
>  		else
> @@ -4934,21 +4930,8 @@ static int kvmppc_core_init_vm_hv(struct kvm *kvm)
>  	/*
>  	 * Track that we now have a HV mode VM active. This blocks secondary
>  	 * CPU threads from coming online.
> -	 * On POWER9, we only need to do this if the "indep_threads_mode"
> -	 * module parameter has been set to N.
>  	 */
> -	if (cpu_has_feature(CPU_FTR_ARCH_300)) {
> -		if (!indep_threads_mode && !cpu_has_feature(CPU_FTR_HVMODE)) {
> -			pr_warn("KVM: Ignoring indep_threads_mode=N in nested hypervisor\n");
> -			kvm->arch.threads_indep = true;
> -		} else if (!indep_threads_mode && cpu_has_feature(CPU_FTR_P9_RADIX_PREFETCH_BUG)) {
> -			pr_warn("KVM: Ignoring indep_threads_mode=N on pre-DD2.2 POWER9\n");
> -			kvm->arch.threads_indep = true;
> -		} else {
> -			kvm->arch.threads_indep = indep_threads_mode;
> -		}
> -	}
> -	if (!kvm->arch.threads_indep)
> +	if (!cpu_has_feature(CPU_FTR_ARCH_300))
>  		kvm_hv_vm_activated();
>  
>  	/*
> @@ -4989,7 +4972,7 @@ static void kvmppc_core_destroy_vm_hv(struct kvm *kvm)
>  {
>  	debugfs_remove_recursive(kvm->arch.debugfs_dir);
>  
> -	if (!kvm->arch.threads_indep)
> +	if (!cpu_has_feature(CPU_FTR_ARCH_300))
>  		kvm_hv_vm_deactivated();
>  
>  	kvmppc_free_vcores(kvm);
> -- 
> 2.23.0