kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RFC 0/4] KVM: x86: Drastically raise KVM_USER_MEM_SLOTS limit
@ 2021-01-15 13:18 Vitaly Kuznetsov
  2021-01-15 13:18 ` [PATCH RFC 1/4] KVM: x86: Drop redundant KVM_MEM_SLOTS_NUM definition Vitaly Kuznetsov
                   ` (5 more replies)
  0 siblings, 6 replies; 19+ messages in thread
From: Vitaly Kuznetsov @ 2021-01-15 13:18 UTC (permalink / raw)
  To: kvm, Paolo Bonzini; +Cc: Sean Christopherson, Wanpeng Li, Jim Mattson

TL;DR: any particular reason why KVM_USER_MEM_SLOTS is so low?

Longer version:

Current KVM_USER_MEM_SLOTS limit (509) can be a limiting factor for some
configurations. In particular, when QEMU tries to start a Windows guest
with Hyper-V SynIC enabled and e.g. 256 vCPUs the limit is hit as SynIC
requires two pages per vCPU and the guest is free to pick any GFN for
each of them, this fragments memslots as QEMU wants to have a separate
memslot for each of these pages (which are supposed to act as 'overlay'
pages).

Memory slots are allocated dynamically in KVM when added so the only real
limitation is 'id_to_index' array which is 'short'. We don't have any
KVM_MEM_SLOTS_NUM/KVM_USER_MEM_SLOTS-sized statically defined arrays.

We could've just raised the limit to e.g. '1021' (we have 3 private
memslots on x86) and this should be enough for now as KVM_MAX_VCPUS is
'288' but AFAIK there are plans to raise this limit as well.

Vitaly Kuznetsov (4):
  KVM: x86: Drop redundant KVM_MEM_SLOTS_NUM definition
  KVM: mips: Drop KVM_PRIVATE_MEM_SLOTS definition
  KVM: Define KVM_USER_MEM_SLOTS in arch-neutral
    include/linux/kvm_host.h
  KVM: x86: Stop limiting KVM_USER_MEM_SLOTS

 arch/mips/include/asm/kvm_host.h | 2 --
 arch/x86/include/asm/kvm_host.h  | 3 +--
 include/linux/kvm_host.h         | 4 ++++
 3 files changed, 5 insertions(+), 4 deletions(-)

-- 
2.29.2


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH RFC 1/4] KVM: x86: Drop redundant KVM_MEM_SLOTS_NUM definition
  2021-01-15 13:18 [PATCH RFC 0/4] KVM: x86: Drastically raise KVM_USER_MEM_SLOTS limit Vitaly Kuznetsov
@ 2021-01-15 13:18 ` Vitaly Kuznetsov
  2021-01-15 16:47   ` Sean Christopherson
  2021-01-15 13:18 ` [PATCH RFC 2/4] KVM: mips: Drop KVM_PRIVATE_MEM_SLOTS definition Vitaly Kuznetsov
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 19+ messages in thread
From: Vitaly Kuznetsov @ 2021-01-15 13:18 UTC (permalink / raw)
  To: kvm, Paolo Bonzini; +Cc: Sean Christopherson, Wanpeng Li, Jim Mattson

KVM_MEM_SLOTS_NUM is already defined in include/linux/kvm_host.h the
exact same way.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 arch/x86/include/asm/kvm_host.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index c27cbe3baccc..1bcf67d76753 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -43,7 +43,6 @@
 #define KVM_USER_MEM_SLOTS 509
 /* memory slots that are not exposed to userspace */
 #define KVM_PRIVATE_MEM_SLOTS 3
-#define KVM_MEM_SLOTS_NUM (KVM_USER_MEM_SLOTS + KVM_PRIVATE_MEM_SLOTS)
 
 #define KVM_HALT_POLL_NS_DEFAULT 200000
 
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH RFC 2/4] KVM: mips: Drop KVM_PRIVATE_MEM_SLOTS definition
  2021-01-15 13:18 [PATCH RFC 0/4] KVM: x86: Drastically raise KVM_USER_MEM_SLOTS limit Vitaly Kuznetsov
  2021-01-15 13:18 ` [PATCH RFC 1/4] KVM: x86: Drop redundant KVM_MEM_SLOTS_NUM definition Vitaly Kuznetsov
@ 2021-01-15 13:18 ` Vitaly Kuznetsov
  2021-01-15 13:18 ` [PATCH RFC 3/4] KVM: Define KVM_USER_MEM_SLOTS in arch-neutral include/linux/kvm_host.h Vitaly Kuznetsov
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 19+ messages in thread
From: Vitaly Kuznetsov @ 2021-01-15 13:18 UTC (permalink / raw)
  To: kvm, Paolo Bonzini; +Cc: Sean Christopherson, Wanpeng Li, Jim Mattson

KVM_PRIVATE_MEM_SLOTS is set to '0' in include/linux/kvm_host.h already.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 arch/mips/include/asm/kvm_host.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/mips/include/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h
index 24f3d0f9996b..603841ce7f42 100644
--- a/arch/mips/include/asm/kvm_host.h
+++ b/arch/mips/include/asm/kvm_host.h
@@ -84,8 +84,6 @@
 
 #define KVM_MAX_VCPUS		16
 #define KVM_USER_MEM_SLOTS	16
-/* memory slots that does not exposed to userspace */
-#define KVM_PRIVATE_MEM_SLOTS	0
 
 #define KVM_HALT_POLL_NS_DEFAULT 500000
 
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH RFC 3/4] KVM: Define KVM_USER_MEM_SLOTS in arch-neutral include/linux/kvm_host.h
  2021-01-15 13:18 [PATCH RFC 0/4] KVM: x86: Drastically raise KVM_USER_MEM_SLOTS limit Vitaly Kuznetsov
  2021-01-15 13:18 ` [PATCH RFC 1/4] KVM: x86: Drop redundant KVM_MEM_SLOTS_NUM definition Vitaly Kuznetsov
  2021-01-15 13:18 ` [PATCH RFC 2/4] KVM: mips: Drop KVM_PRIVATE_MEM_SLOTS definition Vitaly Kuznetsov
@ 2021-01-15 13:18 ` Vitaly Kuznetsov
  2021-01-15 17:05   ` Sean Christopherson
  2021-01-15 13:18 ` [PATCH RFC 4/4] KVM: x86: Stop limiting KVM_USER_MEM_SLOTS Vitaly Kuznetsov
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 19+ messages in thread
From: Vitaly Kuznetsov @ 2021-01-15 13:18 UTC (permalink / raw)
  To: kvm, Paolo Bonzini; +Cc: Sean Christopherson, Wanpeng Li, Jim Mattson

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.

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


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH RFC 4/4] KVM: x86: Stop limiting KVM_USER_MEM_SLOTS
  2021-01-15 13:18 [PATCH RFC 0/4] KVM: x86: Drastically raise KVM_USER_MEM_SLOTS limit Vitaly Kuznetsov
                   ` (2 preceding siblings ...)
  2021-01-15 13:18 ` [PATCH RFC 3/4] KVM: Define KVM_USER_MEM_SLOTS in arch-neutral include/linux/kvm_host.h Vitaly Kuznetsov
@ 2021-01-15 13:18 ` Vitaly Kuznetsov
  2021-01-15 16:03 ` [PATCH RFC 0/4] KVM: x86: Drastically raise KVM_USER_MEM_SLOTS limit Sean Christopherson
  2021-01-15 18:47 ` Maciej S. Szmigiero
  5 siblings, 0 replies; 19+ messages in thread
From: Vitaly Kuznetsov @ 2021-01-15 13:18 UTC (permalink / raw)
  To: kvm, Paolo Bonzini; +Cc: Sean Christopherson, Wanpeng Li, Jim Mattson

Current KVM_USER_MEM_SLOTS limit (509) can be a limiting factor for some
configurations. In particular, when QEMU tries to start a Windows guest
with Hyper-V SynIC enabled and e.g. 256 vCPUs the limit is hit as SynIC
requires two pages per vCPU and the guest is free to pick any GFN for
each of them, this fragments memslots as QEMU wants to have a separate
memslot for each of these pages (which are supposed to act as 'overlay'
pages).

Memory slots are allocated dynamically in KVM when added so the only real
limitation is 'id_to_index' array which is 'short'. We don't have any
KVM_MEM_SLOTS_NUM/KVM_USER_MEM_SLOTS-sized statically defined arrays.

Let's drop the limitation.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 arch/x86/include/asm/kvm_host.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 1bcf67d76753..546b839de797 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -40,7 +40,7 @@
 #define KVM_MAX_VCPUS 288
 #define KVM_SOFT_MAX_VCPUS 240
 #define KVM_MAX_VCPU_ID 1023
-#define KVM_USER_MEM_SLOTS 509
+
 /* memory slots that are not exposed to userspace */
 #define KVM_PRIVATE_MEM_SLOTS 3
 
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH RFC 0/4] KVM: x86: Drastically raise KVM_USER_MEM_SLOTS limit
  2021-01-15 13:18 [PATCH RFC 0/4] KVM: x86: Drastically raise KVM_USER_MEM_SLOTS limit Vitaly Kuznetsov
                   ` (3 preceding siblings ...)
  2021-01-15 13:18 ` [PATCH RFC 4/4] KVM: x86: Stop limiting KVM_USER_MEM_SLOTS Vitaly Kuznetsov
@ 2021-01-15 16:03 ` Sean Christopherson
  2021-01-15 16:23   ` Vitaly Kuznetsov
  2021-01-15 18:47 ` Maciej S. Szmigiero
  5 siblings, 1 reply; 19+ messages in thread
From: Sean Christopherson @ 2021-01-15 16:03 UTC (permalink / raw)
  To: Vitaly Kuznetsov; +Cc: kvm, Paolo Bonzini, Wanpeng Li, Jim Mattson

On Fri, Jan 15, 2021, Vitaly Kuznetsov wrote:
> TL;DR: any particular reason why KVM_USER_MEM_SLOTS is so low?

Because memslots were allocated statically up until fairly recently (v5.7), and
IIRC consumed ~92kb.  Doubling that for every VM would be quite painful. 

> Longer version:
> 
> Current KVM_USER_MEM_SLOTS limit (509) can be a limiting factor for some
> configurations. In particular, when QEMU tries to start a Windows guest
> with Hyper-V SynIC enabled and e.g. 256 vCPUs the limit is hit as SynIC
> requires two pages per vCPU and the guest is free to pick any GFN for
> each of them, this fragments memslots as QEMU wants to have a separate
> memslot for each of these pages (which are supposed to act as 'overlay'
> pages).

What exactly does QEMU do on the backend?  I poked around the code a bit, but
didn't see anything relevant.

> Memory slots are allocated dynamically in KVM when added so the only real
> limitation is 'id_to_index' array which is 'short'. We don't have any
> KVM_MEM_SLOTS_NUM/KVM_USER_MEM_SLOTS-sized statically defined arrays.
> 
> We could've just raised the limit to e.g. '1021' (we have 3 private
> memslots on x86) and this should be enough for now as KVM_MAX_VCPUS is
> '288' but AFAIK there are plans to raise this limit as well.
> 
> Vitaly Kuznetsov (4):
>   KVM: x86: Drop redundant KVM_MEM_SLOTS_NUM definition
>   KVM: mips: Drop KVM_PRIVATE_MEM_SLOTS definition
>   KVM: Define KVM_USER_MEM_SLOTS in arch-neutral
>     include/linux/kvm_host.h
>   KVM: x86: Stop limiting KVM_USER_MEM_SLOTS
> 
>  arch/mips/include/asm/kvm_host.h | 2 --
>  arch/x86/include/asm/kvm_host.h  | 3 +--
>  include/linux/kvm_host.h         | 4 ++++
>  3 files changed, 5 insertions(+), 4 deletions(-)
> 
> -- 
> 2.29.2
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH RFC 0/4] KVM: x86: Drastically raise KVM_USER_MEM_SLOTS limit
  2021-01-15 16:03 ` [PATCH RFC 0/4] KVM: x86: Drastically raise KVM_USER_MEM_SLOTS limit Sean Christopherson
@ 2021-01-15 16:23   ` Vitaly Kuznetsov
  2021-01-15 16:45     ` Sean Christopherson
  0 siblings, 1 reply; 19+ messages in thread
From: Vitaly Kuznetsov @ 2021-01-15 16:23 UTC (permalink / raw)
  To: Sean Christopherson; +Cc: kvm, Paolo Bonzini, Wanpeng Li, Jim Mattson

Sean Christopherson <seanjc@google.com> writes:

> On Fri, Jan 15, 2021, Vitaly Kuznetsov wrote:
>> TL;DR: any particular reason why KVM_USER_MEM_SLOTS is so low?
>
> Because memslots were allocated statically up until fairly recently (v5.7), and
> IIRC consumed ~92kb.  Doubling that for every VM would be quite painful. 
>

I should've added 'now' to the question). So the main reason is gone,
thanks for the confirmation!

>> Longer version:
>> 
>> Current KVM_USER_MEM_SLOTS limit (509) can be a limiting factor for some
>> configurations. In particular, when QEMU tries to start a Windows guest
>> with Hyper-V SynIC enabled and e.g. 256 vCPUs the limit is hit as SynIC
>> requires two pages per vCPU and the guest is free to pick any GFN for
>> each of them, this fragments memslots as QEMU wants to have a separate
>> memslot for each of these pages (which are supposed to act as 'overlay'
>> pages).
>
> What exactly does QEMU do on the backend?  I poked around the code a bit, but
> didn't see anything relevant.
>

In QEMU's terms it registers memory sub-regions for these two pages (see
synic_update() in hw/hyperv/hyperv.c). Memory for these page-sized
sub-regions is allocated separately so in KVM terms they become
page-sized slots and previously continuous 'system memory' slot breaks
into several slots.

-- 
Vitaly


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH RFC 0/4] KVM: x86: Drastically raise KVM_USER_MEM_SLOTS limit
  2021-01-15 16:23   ` Vitaly Kuznetsov
@ 2021-01-15 16:45     ` Sean Christopherson
  0 siblings, 0 replies; 19+ messages in thread
From: Sean Christopherson @ 2021-01-15 16:45 UTC (permalink / raw)
  To: Vitaly Kuznetsov; +Cc: kvm, Paolo Bonzini, Wanpeng Li, Jim Mattson

On Fri, Jan 15, 2021, Vitaly Kuznetsov wrote:
> Sean Christopherson <seanjc@google.com> writes:
> 
> > On Fri, Jan 15, 2021, Vitaly Kuznetsov wrote:
> >> Longer version:
> >> 
> >> Current KVM_USER_MEM_SLOTS limit (509) can be a limiting factor for some
> >> configurations. In particular, when QEMU tries to start a Windows guest
> >> with Hyper-V SynIC enabled and e.g. 256 vCPUs the limit is hit as SynIC
> >> requires two pages per vCPU and the guest is free to pick any GFN for
> >> each of them, this fragments memslots as QEMU wants to have a separate
> >> memslot for each of these pages (which are supposed to act as 'overlay'
> >> pages).
> >
> > What exactly does QEMU do on the backend?  I poked around the code a bit, but
> > didn't see anything relevant.
> >
> 
> In QEMU's terms it registers memory sub-regions for these two pages (see
> synic_update() in hw/hyperv/hyperv.c). Memory for these page-sized
> sub-regions is allocated separately so in KVM terms they become
> page-sized slots and previously continuous 'system memory' slot breaks
> into several slots.

Doh, I had a super stale version checked out (2.9.50), no wonder I couldn't find
anything.

Isn't the memslot approach inherently flawed in that the SynIC is per-vCPU, but
memslots are per-VM?  E.g. if vCPU1 accesses vCPU0's SynIC GPA, I would expect
that to access real memory, not the overlay.  Or is there more QEMU magic going
on that I'm missing?

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH RFC 1/4] KVM: x86: Drop redundant KVM_MEM_SLOTS_NUM definition
  2021-01-15 13:18 ` [PATCH RFC 1/4] KVM: x86: Drop redundant KVM_MEM_SLOTS_NUM definition Vitaly Kuznetsov
@ 2021-01-15 16:47   ` Sean Christopherson
  0 siblings, 0 replies; 19+ messages in thread
From: Sean Christopherson @ 2021-01-15 16:47 UTC (permalink / raw)
  To: Vitaly Kuznetsov; +Cc: kvm, Paolo Bonzini, Wanpeng Li, Jim Mattson

On Fri, Jan 15, 2021, Vitaly Kuznetsov wrote:
> KVM_MEM_SLOTS_NUM is already defined in include/linux/kvm_host.h the
> exact same way.

I'm pretty sure you can remove the "#ifndef KVM_MEM_SLOTS_NUM" in
linux/kvm_host.h, looks like x86 is the only arch that was defining
KVM_MEM_SLOTS_NUM.

> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
>  arch/x86/include/asm/kvm_host.h | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index c27cbe3baccc..1bcf67d76753 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -43,7 +43,6 @@
>  #define KVM_USER_MEM_SLOTS 509
>  /* memory slots that are not exposed to userspace */
>  #define KVM_PRIVATE_MEM_SLOTS 3
> -#define KVM_MEM_SLOTS_NUM (KVM_USER_MEM_SLOTS + KVM_PRIVATE_MEM_SLOTS)
>  
>  #define KVM_HALT_POLL_NS_DEFAULT 200000
>  
> -- 
> 2.29.2
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH RFC 3/4] KVM: Define KVM_USER_MEM_SLOTS in arch-neutral include/linux/kvm_host.h
  2021-01-15 13:18 ` [PATCH RFC 3/4] KVM: Define KVM_USER_MEM_SLOTS in arch-neutral include/linux/kvm_host.h Vitaly Kuznetsov
@ 2021-01-15 17:05   ` Sean Christopherson
  2021-01-18  9:52     ` Vitaly Kuznetsov
  0 siblings, 1 reply; 19+ messages in thread
From: Sean Christopherson @ 2021-01-15 17:05 UTC (permalink / raw)
  To: Vitaly Kuznetsov; +Cc: kvm, Paolo Bonzini, Wanpeng Li, Jim Mattson

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.

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

> 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
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH RFC 0/4] KVM: x86: Drastically raise KVM_USER_MEM_SLOTS limit
  2021-01-15 13:18 [PATCH RFC 0/4] KVM: x86: Drastically raise KVM_USER_MEM_SLOTS limit Vitaly Kuznetsov
                   ` (4 preceding siblings ...)
  2021-01-15 16:03 ` [PATCH RFC 0/4] KVM: x86: Drastically raise KVM_USER_MEM_SLOTS limit Sean Christopherson
@ 2021-01-15 18:47 ` Maciej S. Szmigiero
  2021-01-27 17:55   ` Vitaly Kuznetsov
  5 siblings, 1 reply; 19+ messages in thread
From: Maciej S. Szmigiero @ 2021-01-15 18:47 UTC (permalink / raw)
  To: Vitaly Kuznetsov
  Cc: Sean Christopherson, Wanpeng Li, Jim Mattson, kvm, Paolo Bonzini

On 15.01.2021 14:18, Vitaly Kuznetsov wrote:
> TL;DR: any particular reason why KVM_USER_MEM_SLOTS is so low?
> 
> Longer version:
> 
> Current KVM_USER_MEM_SLOTS limit (509) can be a limiting factor for some
> configurations. In particular, when QEMU tries to start a Windows guest
> with Hyper-V SynIC enabled and e.g. 256 vCPUs the limit is hit as SynIC
> requires two pages per vCPU and the guest is free to pick any GFN for
> each of them, this fragments memslots as QEMU wants to have a separate
> memslot for each of these pages (which are supposed to act as 'overlay'
> pages).
> 
> Memory slots are allocated dynamically in KVM when added so the only real
> limitation is 'id_to_index' array which is 'short'. We don't have any
> KVM_MEM_SLOTS_NUM/KVM_USER_MEM_SLOTS-sized statically defined arrays.
> 
> We could've just raised the limit to e.g. '1021' (we have 3 private
> memslots on x86) and this should be enough for now as KVM_MAX_VCPUS is
> '288' but AFAIK there are plans to raise this limit as well.
> 

I have a patch series that reworks the whole memslot thing, bringing
performance improvements across the board.
Will post it in few days, together with a new mini benchmark set.

Maciej

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH RFC 3/4] KVM: Define KVM_USER_MEM_SLOTS in arch-neutral include/linux/kvm_host.h
  2021-01-15 17:05   ` Sean Christopherson
@ 2021-01-18  9:52     ` Vitaly Kuznetsov
  2021-01-19 17:20       ` Sean Christopherson
  0 siblings, 1 reply; 19+ messages in thread
From: Vitaly Kuznetsov @ 2021-01-18  9:52 UTC (permalink / raw)
  To: Sean Christopherson; +Cc: kvm, Paolo Bonzini, Wanpeng Li, Jim Mattson

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


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH RFC 3/4] KVM: Define KVM_USER_MEM_SLOTS in arch-neutral include/linux/kvm_host.h
  2021-01-18  9:52     ` Vitaly Kuznetsov
@ 2021-01-19 17:20       ` Sean Christopherson
  2021-01-20 11:34         ` Igor Mammedov
  0 siblings, 1 reply; 19+ messages in thread
From: Sean Christopherson @ 2021-01-19 17:20 UTC (permalink / raw)
  To: Vitaly Kuznetsov; +Cc: kvm, Paolo Bonzini, Wanpeng Li, Jim Mattson

On Mon, Jan 18, 2021, Vitaly Kuznetsov wrote:
> 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 ...

I see it as an easy way to mitigate the damage.  E.g. if a containers use case
is spinning up hundreds of VMs and something goes awry in the config, it would
be the difference between consuming tens of MBs and hundreds of MBs.  Cgroup
limits should also be in play, but defense in depth and all that. 

> > 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.

Hrm, I don't love tying it to KVM_MAX_VPUCS.  Other than SynIC, are there any
other features/modes/configuration that cause the number of memslots to grop
with the number of vCPUs?  But, limiting via a module param does effectively
require using KVM_MAX_VCPUS, otherwise everyone that might run Windows guests
would have to override the default and thereby defeat the purpose of limiting by
default.

Were you planning on adding a capability to check for the new and improved
memslots limit, e.g. to know whether or not KVM might die on a large VM?
If so, requiring the VMM to call an ioctl() to set a higher (or lower?) limit
would be another option.  That wouldn't have the same permission requirements as
a module param, but it would likely be a more effective safeguard in practice,
e.g. use cases with a fixed number of memslots or a well-defined upper bound
could use the capability to limit themselves.

Thoughts?  An ioctl() feels a little over-engineered, but I suspect that adding
a module param that defaults to N*KVM_MAX_VPCUS will be a waste, e.g. no one
will ever touch the param and we'll end up with dead, rarely-tested code.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH RFC 3/4] KVM: Define KVM_USER_MEM_SLOTS in arch-neutral include/linux/kvm_host.h
  2021-01-19 17:20       ` Sean Christopherson
@ 2021-01-20 11:34         ` Igor Mammedov
  2021-01-20 12:02           ` Vitaly Kuznetsov
  0 siblings, 1 reply; 19+ messages in thread
From: Igor Mammedov @ 2021-01-20 11:34 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: Vitaly Kuznetsov, kvm, Paolo Bonzini, Wanpeng Li, Jim Mattson

On Tue, 19 Jan 2021 09:20:42 -0800
Sean Christopherson <seanjc@google.com> wrote:

> On Mon, Jan 18, 2021, Vitaly Kuznetsov wrote:
> > 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 ...  
> 
> I see it as an easy way to mitigate the damage.  E.g. if a containers use case
> is spinning up hundreds of VMs and something goes awry in the config, it would
> be the difference between consuming tens of MBs and hundreds of MBs.  Cgroup
> limits should also be in play, but defense in depth and all that. 
> 
> > > 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.  
> 
> Hrm, I don't love tying it to KVM_MAX_VPUCS.  Other than SynIC, are there any
> other features/modes/configuration that cause the number of memslots to grop

(NV)DIMMs in QEMU also consume slot/device but do not depend on vCPUs number.
Due current slot limit only 256 DIMMs are allowed. But if vCPUs start consuming
extra memslots, they will contend over possible slots.

> with the number of vCPUs?  But, limiting via a module param does effectively
> require using KVM_MAX_VCPUS, otherwise everyone that might run Windows guests
> would have to override the default and thereby defeat the purpose of limiting by
> default.
> 
> Were you planning on adding a capability to check for the new and improved
> memslots limit, e.g. to know whether or not KVM might die on a large VM?
> If so, requiring the VMM to call an ioctl() to set a higher (or lower?) limit
> would be another option.  That wouldn't have the same permission requirements as
> a module param, but it would likely be a more effective safeguard in practice,
> e.g. use cases with a fixed number of memslots or a well-defined upper bound
> could use the capability to limit themselves.
Currently QEMU uses KVM_CAP_NR_MEMSLOTS to get limit, and depending on place the
limit is reached it either fails gracefully (i.e. it checks if free slot is
available before slot allocation) or aborts (in case where it tries to allocate
slot without check).
New ioctl() seems redundant as we already have upper limit check
(unless it would allow go over that limit, which in its turn defeats purpose of
the limit).


> Thoughts?  An ioctl() feels a little over-engineered, but I suspect that adding
> a module param that defaults to N*KVM_MAX_VPCUS will be a waste, e.g. no one
> will ever touch the param and we'll end up with dead, rarely-tested code.
> 


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH RFC 3/4] KVM: Define KVM_USER_MEM_SLOTS in arch-neutral include/linux/kvm_host.h
  2021-01-20 11:34         ` Igor Mammedov
@ 2021-01-20 12:02           ` Vitaly Kuznetsov
  2021-01-20 17:25             ` Sean Christopherson
  0 siblings, 1 reply; 19+ messages in thread
From: Vitaly Kuznetsov @ 2021-01-20 12:02 UTC (permalink / raw)
  To: Igor Mammedov, Sean Christopherson
  Cc: kvm, Paolo Bonzini, Wanpeng Li, Jim Mattson

Igor Mammedov <imammedo@redhat.com> writes:

> On Tue, 19 Jan 2021 09:20:42 -0800
> Sean Christopherson <seanjc@google.com> wrote:
>
>> 
>> Were you planning on adding a capability to check for the new and improved
>> memslots limit, e.g. to know whether or not KVM might die on a large VM?
>> If so, requiring the VMM to call an ioctl() to set a higher (or lower?) limit
>> would be another option.  That wouldn't have the same permission requirements as
>> a module param, but it would likely be a more effective safeguard in practice,
>> e.g. use cases with a fixed number of memslots or a well-defined upper bound
>> could use the capability to limit themselves.
> Currently QEMU uses KVM_CAP_NR_MEMSLOTS to get limit, and depending on place the
> limit is reached it either fails gracefully (i.e. it checks if free slot is
> available before slot allocation) or aborts (in case where it tries to allocate
> slot without check).

FWIW, 'synic problem' causes it to abort.

> New ioctl() seems redundant as we already have upper limit check
> (unless it would allow go over that limit, which in its turn defeats purpose of
> the limit).
>

Right, I didn't plan to add any new CAP as what we already have should
be enough to query the limits. Having an ioctl to set the upper limit
seems complicated: with device and CPU hotplug it may not be easy to
guess what it should be upfront so VMMs will likely just add a call to
raise the limit in memslot modification code so it won't be providing
any new protection.


>> Thoughts?  An ioctl() feels a little over-engineered, but I suspect that adding
>> a module param that defaults to N*KVM_MAX_VPCUS will be a waste, e.g. no one
>> will ever touch the param and we'll end up with dead, rarely-tested code.

Alternatively, we can hard-code KVM_USER_MEM_SLOTS to N*KVM_MAX_VPCUS so
no new parameter is needed but personally, I'd prefer to have it
configurable (in case we decide to set it to something lower than
SHRT_MAX of course) even if it won't be altered very often (which is a
good thing for 'general purpose' usage, right?). First, it will allow
tightening the limit for some very specific deployments (e.g. FaaS/
Firecracker-style) to say '20' which should be enough. Second, we may be
overlooking some configurations where the number of memslots is actually
dependent on the number of vCPUs but nobody complained so far just
because these configutrarions use a farly small number and the ceiling
wasn't hit yet.

One more spare thought. Per-vCPU memslots may come handy if someone
decides to move some of the KVM PV features to userspace. E.g. I can
imagine an attempt to move async_pf out of kernel.

-- 
Vitaly


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH RFC 3/4] KVM: Define KVM_USER_MEM_SLOTS in arch-neutral include/linux/kvm_host.h
  2021-01-20 12:02           ` Vitaly Kuznetsov
@ 2021-01-20 17:25             ` Sean Christopherson
  0 siblings, 0 replies; 19+ messages in thread
From: Sean Christopherson @ 2021-01-20 17:25 UTC (permalink / raw)
  To: Vitaly Kuznetsov
  Cc: Igor Mammedov, kvm, Paolo Bonzini, Wanpeng Li, Jim Mattson

On Wed, Jan 20, 2021, Vitaly Kuznetsov wrote:
> Igor Mammedov <imammedo@redhat.com> writes:
> 
> > On Tue, 19 Jan 2021 09:20:42 -0800
> > Sean Christopherson <seanjc@google.com> wrote:
> >
> >> 
> >> Were you planning on adding a capability to check for the new and improved
> >> memslots limit, e.g. to know whether or not KVM might die on a large VM?
> >> If so, requiring the VMM to call an ioctl() to set a higher (or lower?) limit
> >> would be another option.  That wouldn't have the same permission requirements as
> >> a module param, but it would likely be a more effective safeguard in practice,
> >> e.g. use cases with a fixed number of memslots or a well-defined upper bound
> >> could use the capability to limit themselves.
> > Currently QEMU uses KVM_CAP_NR_MEMSLOTS to get limit, and depending on place the
> > limit is reached it either fails gracefully (i.e. it checks if free slot is
> > available before slot allocation) or aborts (in case where it tries to allocate
> > slot without check).
> 
> FWIW, 'synic problem' causes it to abort.
> 
> > New ioctl() seems redundant as we already have upper limit check
> > (unless it would allow go over that limit, which in its turn defeats purpose of
> > the limit).

Gah, and I even looked for an ioctl().  No idea how I didn't find NR_MEMSLOTS.

> Right, I didn't plan to add any new CAP as what we already have should
> be enough to query the limits. Having an ioctl to set the upper limit
> seems complicated: with device and CPU hotplug it may not be easy to
> guess what it should be upfront so VMMs will likely just add a call to
> raise the limit in memslot modification code so it won't be providing
> any new protection.

True.  Maybe the best approach, if we want to make the limit configurable, would
be to make a lower limit opt-in.  I.e. default=KVM_CAP_NR_MEMSLOTS=SHRT_MAX,
with an ioctl() to set a lower limit.  That would also allow us to defer adding
a new ioctl() until someone actually plans on using it.

> >> Thoughts?  An ioctl() feels a little over-engineered, but I suspect that adding
> >> a module param that defaults to N*KVM_MAX_VPCUS will be a waste, e.g. no one
> >> will ever touch the param and we'll end up with dead, rarely-tested code.
> 
> Alternatively, we can hard-code KVM_USER_MEM_SLOTS to N*KVM_MAX_VPCUS so
> no new parameter is needed but personally, I'd prefer to have it
> configurable (in case we decide to set it to something lower than
> SHRT_MAX of course) even if it won't be altered very often (which is a
> good thing for 'general purpose' usage, right?).
>
> First, it will allow tightening the limit for some very specific deployments
> (e.g. FaaS/ Firecracker-style) to say '20' which should be enough.

A module param likely isn't usable for many such deployments though, as it would
require a completely isolated pool of systems.  That's why an ioctl() is
appealing; the expected number of memslots is a property of the VM, not of the
platform.

> Second, we may be overlooking some configurations where the number of
> memslots is actually dependent on the number of vCPUs but nobody complained
> so far just because these configutrarions use a farly small number and the
> ceiling wasn't hit yet.
> 
> One more spare thought. Per-vCPU memslots may come handy if someone
> decides to move some of the KVM PV features to userspace. E.g. I can
> imagine an attempt to move async_pf out of kernel.

Memslots aren't per-vCPU though, any userspace feature that tries to treat them
as such will be flawed in some way.  Practically speaking, per-vCPU memslots
just aren't feasible, at least not without KVM changes that are likely
unpalatable, e.g. KVM would need to incorporate the existence of a vCPU specific
memslot into the MMU role.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH RFC 0/4] KVM: x86: Drastically raise KVM_USER_MEM_SLOTS limit
  2021-01-15 18:47 ` Maciej S. Szmigiero
@ 2021-01-27 17:55   ` Vitaly Kuznetsov
  2021-01-27 22:26     ` Maciej S. Szmigiero
  0 siblings, 1 reply; 19+ messages in thread
From: Vitaly Kuznetsov @ 2021-01-27 17:55 UTC (permalink / raw)
  To: Maciej S. Szmigiero
  Cc: Sean Christopherson, Wanpeng Li, Jim Mattson, kvm, Paolo Bonzini

"Maciej S. Szmigiero" <maciej.szmigiero@oracle.com> writes:

> On 15.01.2021 14:18, Vitaly Kuznetsov wrote:
>> TL;DR: any particular reason why KVM_USER_MEM_SLOTS is so low?
>> 
>> Longer version:
>> 
>> Current KVM_USER_MEM_SLOTS limit (509) can be a limiting factor for some
>> configurations. In particular, when QEMU tries to start a Windows guest
>> with Hyper-V SynIC enabled and e.g. 256 vCPUs the limit is hit as SynIC
>> requires two pages per vCPU and the guest is free to pick any GFN for
>> each of them, this fragments memslots as QEMU wants to have a separate
>> memslot for each of these pages (which are supposed to act as 'overlay'
>> pages).
>> 
>> Memory slots are allocated dynamically in KVM when added so the only real
>> limitation is 'id_to_index' array which is 'short'. We don't have any
>> KVM_MEM_SLOTS_NUM/KVM_USER_MEM_SLOTS-sized statically defined arrays.
>> 
>> We could've just raised the limit to e.g. '1021' (we have 3 private
>> memslots on x86) and this should be enough for now as KVM_MAX_VCPUS is
>> '288' but AFAIK there are plans to raise this limit as well.
>> 
>
> I have a patch series that reworks the whole memslot thing, bringing
> performance improvements across the board.
> Will post it in few days, together with a new mini benchmark set.

I'm about to send a successor of this series. It will be implmenting
Sean's idea to make the maximum number of memslots a per-VM thing (and
also raise the default). Hope it won't interfere with your work!

-- 
Vitaly


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH RFC 0/4] KVM: x86: Drastically raise KVM_USER_MEM_SLOTS limit
  2021-01-27 17:55   ` Vitaly Kuznetsov
@ 2021-01-27 22:26     ` Maciej S. Szmigiero
  2021-01-28  8:47       ` Vitaly Kuznetsov
  0 siblings, 1 reply; 19+ messages in thread
From: Maciej S. Szmigiero @ 2021-01-27 22:26 UTC (permalink / raw)
  To: Vitaly Kuznetsov
  Cc: Sean Christopherson, Wanpeng Li, Jim Mattson, kvm, Paolo Bonzini

Hi Vitaly,

On 27.01.2021 18:55, Vitaly Kuznetsov wrote:
> "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com> writes:
> 
>> On 15.01.2021 14:18, Vitaly Kuznetsov wrote:
>>> TL;DR: any particular reason why KVM_USER_MEM_SLOTS is so low?
>>>
>>> Longer version:
>>>
>>> Current KVM_USER_MEM_SLOTS limit (509) can be a limiting factor for some
>>> configurations. In particular, when QEMU tries to start a Windows guest
>>> with Hyper-V SynIC enabled and e.g. 256 vCPUs the limit is hit as SynIC
>>> requires two pages per vCPU and the guest is free to pick any GFN for
>>> each of them, this fragments memslots as QEMU wants to have a separate
>>> memslot for each of these pages (which are supposed to act as 'overlay'
>>> pages).
>>>
>>> Memory slots are allocated dynamically in KVM when added so the only real
>>> limitation is 'id_to_index' array which is 'short'. We don't have any
>>> KVM_MEM_SLOTS_NUM/KVM_USER_MEM_SLOTS-sized statically defined arrays.
>>>
>>> We could've just raised the limit to e.g. '1021' (we have 3 private
>>> memslots on x86) and this should be enough for now as KVM_MAX_VCPUS is
>>> '288' but AFAIK there are plans to raise this limit as well.
>>>
>>
>> I have a patch series that reworks the whole memslot thing, bringing
>> performance improvements across the board.
>> Will post it in few days, together with a new mini benchmark set.
> 
> I'm about to send a successor of this series. It will be implmenting
> Sean's idea to make the maximum number of memslots a per-VM thing (and
> also raise the default). Hope it won't interfere with your work!

Thanks for your series and CC'ing me on it.

It looks like there should be no design conflicts, I will merely need to
rebase on top of it.

By the way I had to change a bit the KVM selftest framework memslot
handling for my stuff, too, since otherwise just adding 32k memslots
for a test would take almost forever.

Thanks,
Maciej

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH RFC 0/4] KVM: x86: Drastically raise KVM_USER_MEM_SLOTS limit
  2021-01-27 22:26     ` Maciej S. Szmigiero
@ 2021-01-28  8:47       ` Vitaly Kuznetsov
  0 siblings, 0 replies; 19+ messages in thread
From: Vitaly Kuznetsov @ 2021-01-28  8:47 UTC (permalink / raw)
  To: Maciej S. Szmigiero
  Cc: Sean Christopherson, Wanpeng Li, Jim Mattson, kvm, Paolo Bonzini

"Maciej S. Szmigiero" <maciej.szmigiero@oracle.com> writes:

> Hi Vitaly,
>
> On 27.01.2021 18:55, Vitaly Kuznetsov wrote:
>> "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com> writes:
>> 
>>> On 15.01.2021 14:18, Vitaly Kuznetsov wrote:
>>>> TL;DR: any particular reason why KVM_USER_MEM_SLOTS is so low?
>>>>
>>>> Longer version:
>>>>
>>>> Current KVM_USER_MEM_SLOTS limit (509) can be a limiting factor for some
>>>> configurations. In particular, when QEMU tries to start a Windows guest
>>>> with Hyper-V SynIC enabled and e.g. 256 vCPUs the limit is hit as SynIC
>>>> requires two pages per vCPU and the guest is free to pick any GFN for
>>>> each of them, this fragments memslots as QEMU wants to have a separate
>>>> memslot for each of these pages (which are supposed to act as 'overlay'
>>>> pages).
>>>>
>>>> Memory slots are allocated dynamically in KVM when added so the only real
>>>> limitation is 'id_to_index' array which is 'short'. We don't have any
>>>> KVM_MEM_SLOTS_NUM/KVM_USER_MEM_SLOTS-sized statically defined arrays.
>>>>
>>>> We could've just raised the limit to e.g. '1021' (we have 3 private
>>>> memslots on x86) and this should be enough for now as KVM_MAX_VCPUS is
>>>> '288' but AFAIK there are plans to raise this limit as well.
>>>>
>>>
>>> I have a patch series that reworks the whole memslot thing, bringing
>>> performance improvements across the board.
>>> Will post it in few days, together with a new mini benchmark set.
>> 
>> I'm about to send a successor of this series. It will be implmenting
>> Sean's idea to make the maximum number of memslots a per-VM thing (and
>> also raise the default). Hope it won't interfere with your work!
>
> Thanks for your series and CC'ing me on it.
>
> It looks like there should be no design conflicts, I will merely need to
> rebase on top of it.
>
> By the way I had to change a bit the KVM selftest framework memslot
> handling for my stuff, too, since otherwise just adding 32k memslots
> for a test would take almost forever.
>

Yea, that's why PATCH5 of the new series increases the default timeout
from 45 seconds to 120.

-- 
Vitaly


^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2021-01-28  8:50 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-15 13:18 [PATCH RFC 0/4] KVM: x86: Drastically raise KVM_USER_MEM_SLOTS limit Vitaly Kuznetsov
2021-01-15 13:18 ` [PATCH RFC 1/4] KVM: x86: Drop redundant KVM_MEM_SLOTS_NUM definition Vitaly Kuznetsov
2021-01-15 16:47   ` Sean Christopherson
2021-01-15 13:18 ` [PATCH RFC 2/4] KVM: mips: Drop KVM_PRIVATE_MEM_SLOTS definition Vitaly Kuznetsov
2021-01-15 13:18 ` [PATCH RFC 3/4] KVM: Define KVM_USER_MEM_SLOTS in arch-neutral include/linux/kvm_host.h Vitaly Kuznetsov
2021-01-15 17:05   ` Sean Christopherson
2021-01-18  9:52     ` Vitaly Kuznetsov
2021-01-19 17:20       ` Sean Christopherson
2021-01-20 11:34         ` Igor Mammedov
2021-01-20 12:02           ` Vitaly Kuznetsov
2021-01-20 17:25             ` Sean Christopherson
2021-01-15 13:18 ` [PATCH RFC 4/4] KVM: x86: Stop limiting KVM_USER_MEM_SLOTS Vitaly Kuznetsov
2021-01-15 16:03 ` [PATCH RFC 0/4] KVM: x86: Drastically raise KVM_USER_MEM_SLOTS limit Sean Christopherson
2021-01-15 16:23   ` Vitaly Kuznetsov
2021-01-15 16:45     ` Sean Christopherson
2021-01-15 18:47 ` Maciej S. Szmigiero
2021-01-27 17:55   ` Vitaly Kuznetsov
2021-01-27 22:26     ` Maciej S. Szmigiero
2021-01-28  8:47       ` Vitaly Kuznetsov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).