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

Paolo Bonzini
2013-11-25T10:41:19Z

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