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

[Paolo Bonzini <pbonzini@redhat.com>]

Il 25/11/2013 11:16, Thomas Huth ha scritto:
> 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>

Thanks, applied to uq/master with your suggested change.

Paolo