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

[Nicholas Piggin <npiggin@gmail.com>]

Excerpts from Aneesh Kumar K.V's message of March 18, 2021 1:11 am:
> 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?

I'm not sure about an ask, but it is a possible security feature that 
would be relevant to all SMT CPUs running KVM guests.

It doesn't make much sense to plumb P9 support all through the P8 path 
just for that though, in my opinion? Is it tested? Who uses it? It's
lacking features of the P9 path.

It would be better added to KVM/QEMU in general (or until that is 
available, disable SMT, or use CPU pinning and isolcpus to prevent host 
code running on secondaries, and isolating VMs from one another, etc).

I think it's quite possible to rendezvous threads in kernel, move them
onto the threads of a core, and then have them all running in KVM code 
before they enter the guest, without disabling SMT in the host.

You could do it with kernel threads on the secondaries even, but I 
wouldn't like to have to plumb the vcore concept entirely through 
everywhere so I would actually prefer to see QEMU grow an understanding 
of it so it would know it has to call the ioctl on every guest SMT 
thread.

Thanks,
Nick