Re: [Qemu-devel] [PATCH] kvm: Query KVM for available memory slots

[Thomas Huth <thuth@linux.vnet.ibm.com>]

On Fri, 22 Nov 2013 12:12:44 -0700
Alex Williamson <alex.williamson@redhat.com> wrote:

> KVM reports the number of available memory slots (KVM_CAP_NR_MEMSLOTS)
> using the extension interface.  Both x86 and s390 implement this, ARM
> and powerpc do not yet enable it.  Convert the static slots array to
> be dynamically allocated, supporting more slots when available.
> Default to 32 when KVM_CAP_NR_MEMSLOTS is not implemented.  The
> motivation for this change is to support more assigned devices, where
> memory mapped PCI MMIO BARs typically take one slot each.
> 
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---
>  kvm-all.c |   30 +++++++++++++++++++++---------
>  1 file changed, 21 insertions(+), 9 deletions(-)
> 
> diff --git a/kvm-all.c b/kvm-all.c
> index 4478969..63c4e9b 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -72,7 +72,8 @@ typedef struct kvm_dirty_log KVMDirtyLog;
> 
>  struct KVMState
>  {
> -    KVMSlot slots[32];
> +    KVMSlot *slots;
> +    int nr_slots;
>      int fd;
>      int vmfd;
>      int coalesced_mmio;
> @@ -125,7 +126,7 @@ static KVMSlot *kvm_alloc_slot(KVMState *s)
>  {
>      int i;
> 
> -    for (i = 0; i < ARRAY_SIZE(s->slots); i++) {
> +    for (i = 0; i < s->nr_slots; i++) {
>          if (s->slots[i].memory_size == 0) {
>              return &s->slots[i];
>          }
> @@ -141,7 +142,7 @@ static KVMSlot *kvm_lookup_matching_slot(KVMState *s,
>  {
>      int i;
> 
> -    for (i = 0; i < ARRAY_SIZE(s->slots); i++) {
> +    for (i = 0; i < s->nr_slots; i++) {
>          KVMSlot *mem = &s->slots[i];
> 
>          if (start_addr == mem->start_addr &&
> @@ -163,7 +164,7 @@ static KVMSlot *kvm_lookup_overlapping_slot(KVMState *s,
>      KVMSlot *found = NULL;
>      int i;
> 
> -    for (i = 0; i < ARRAY_SIZE(s->slots); i++) {
> +    for (i = 0; i < s->nr_slots; i++) {
>          KVMSlot *mem = &s->slots[i];
> 
>          if (mem->memory_size == 0 ||
> @@ -185,7 +186,7 @@ int kvm_physical_memory_addr_from_host(KVMState *s, void *ram,
>  {
>      int i;
> 
> -    for (i = 0; i < ARRAY_SIZE(s->slots); i++) {
> +    for (i = 0; i < s->nr_slots; i++) {
>          KVMSlot *mem = &s->slots[i];
> 
>          if (ram >= mem->ram && ram < mem->ram + mem->memory_size) {
> @@ -357,7 +358,7 @@ static int kvm_set_migration_log(int enable)
> 
>      s->migration_log = enable;
> 
> -    for (i = 0; i < ARRAY_SIZE(s->slots); i++) {
> +    for (i = 0; i < s->nr_slots; i++) {
>          mem = &s->slots[i];
> 
>          if (!mem->memory_size) {
> @@ -1383,9 +1384,6 @@ int kvm_init(void)
>  #ifdef KVM_CAP_SET_GUEST_DEBUG
>      QTAILQ_INIT(&s->kvm_sw_breakpoints);
>  #endif
> -    for (i = 0; i < ARRAY_SIZE(s->slots); i++) {
> -        s->slots[i].slot = i;
> -    }
>      s->vmfd = -1;
>      s->fd = qemu_open("/dev/kvm", O_RDWR);
>      if (s->fd == -1) {
> @@ -1409,6 +1407,19 @@ int kvm_init(void)
>          goto err;
>      }
> 
> +    s->nr_slots = kvm_check_extension(s, KVM_CAP_NR_MEMSLOTS);
> +
> +    /* If unspecified, use the previous default value */

I'd remove the "previous" in the comment here, because if you look at
this code in a couple of years, nobody will remember what "previous" is
refering to here.

> +    if (!s->nr_slots) {
> +        s->nr_slots = 32;
> +    }
> +
> +    s->slots = g_malloc0(s->nr_slots * sizeof(KVMSlot));
> +
> +    for (i = 0; i < s->nr_slots; i++) {
> +        s->slots[i].slot = i;
> +    }
> +
>      /* check the vcpu limits */
>      soft_vcpus_limit = kvm_recommended_vcpus(s);
>      hard_vcpus_limit = kvm_max_vcpus(s);
> @@ -1527,6 +1538,7 @@ err:
>      if (s->fd != -1) {
>          close(s->fd);
>      }
> +    g_free(s->slots);
>      g_free(s);
> 
>      return ret;

Reviewed-by: Thomas Huth <thuth@linux.vnet.ibm.com>