Re: [PATCH RFC 3/4] KVM: Define KVM_USER_MEM_SLOTS in arch-neutral include/linux/kvm_host.h

[Vitaly Kuznetsov <vkuznets@redhat.com>]

Sean Christopherson <seanjc@google.com> writes:

> On Fri, Jan 15, 2021, Vitaly Kuznetsov wrote:
>> Memory slots are allocated dynamically when added so the only real
>> limitation in KVM is 'id_to_index' array which is 'short'. Define
>> KVM_USER_MEM_SLOTS to the maximum possible value in the arch-neutral
>> include/linux/kvm_host.h, architectures can still overtide the setting
>> if needed.
>
> Leaving the max number of slots nearly unbounded is probably a bad idea.  If my
> math is not completely wrong, this would let userspace allocate 6mb of kernel
> memory per VM.  Actually, worst case scenario would be 12mb since modifying
> memslots temporarily has two allocations.

Yea I had thought too but on the other hand, if your VMM went rogue and
and is trying to eat all your memory, how is allocating 32k memslots
different from e.g. creating 64 VMs with 512 slots each? We use
GFP_KERNEL_ACCOUNT to allocate memslots (and other per-VM stuff) so
e.g. cgroup limits should work ...

>
> If we remove the arbitrary limit, maybe replace it with a module param with a
> sane default?

This can be a good solution indeed. The only question then is what should
we pick as the default? It seems to me this can be KVM_MAX_VCPUS
dependent, e.g. 4 x KVM_MAX_VCPUS would suffice.

>
>> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>> ---
>>  include/linux/kvm_host.h | 4 ++++
>>  1 file changed, 4 insertions(+)
>> 
>> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
>> index f3b1013fb22c..ab83a20a52ca 100644
>> --- a/include/linux/kvm_host.h
>> +++ b/include/linux/kvm_host.h
>> @@ -425,6 +425,10 @@ struct kvm_irq_routing_table {
>>  #define KVM_PRIVATE_MEM_SLOTS 0
>>  #endif
>>  
>> +#ifndef KVM_USER_MEM_SLOTS
>> +#define KVM_USER_MEM_SLOTS (SHRT_MAX - KVM_PRIVATE_MEM_SLOTS)
>> +#endif
>> +
>>  #ifndef KVM_MEM_SLOTS_NUM
>>  #define KVM_MEM_SLOTS_NUM (KVM_USER_MEM_SLOTS + KVM_PRIVATE_MEM_SLOTS)
>>  #endif
>> -- 
>> 2.29.2
>> 
>

-- 
Vitaly