diff mbox series

[PULL,09/12] kvm: clear dirty bitmaps from all overlapping memslots

Message ID 20190930131955.101131-10-borntraeger@de.ibm.com
State New
Headers show
Series [PULL,01/12] MAINTAINERS: Update S390 PCI Maintainer | expand

Commit Message

Christian Borntraeger Sept. 30, 2019, 1:19 p.m. UTC
From: Paolo Bonzini <pbonzini@redhat.com>

Currently MemoryRegionSection has 1:1 mapping to KVMSlot.
However next patch will allow splitting MemoryRegionSection into
several KVMSlot-s, make sure that kvm_physical_log_slot_clear()
is able to handle such 1:N mapping.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Message-Id: <20190924144751.24149-3-imammedo@redhat.com>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 accel/kvm/kvm-all.c | 36 ++++++++++++++++++++++--------------
 1 file changed, 22 insertions(+), 14 deletions(-)

Comments

Kevin Wolf Oct. 2, 2019, 4:01 p.m. UTC | #1
Am 30.09.2019 um 15:19 hat Christian Borntraeger geschrieben:
> From: Paolo Bonzini <pbonzini@redhat.com>
> 
> Currently MemoryRegionSection has 1:1 mapping to KVMSlot.
> However next patch will allow splitting MemoryRegionSection into
> several KVMSlot-s, make sure that kvm_physical_log_slot_clear()
> is able to handle such 1:N mapping.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> Reviewed-by: Peter Xu <peterx@redhat.com>
> Message-Id: <20190924144751.24149-3-imammedo@redhat.com>
> Acked-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>

This broke the build for me on F29:

  CC      x86_64-softmmu/accel/kvm/kvm-all.o
/tmp/qemu/accel/kvm/kvm-all.c: In function 'kvm_log_clear':
/tmp/qemu/accel/kvm/kvm-all.c:1086:8: error: 'ret' may be used uninitialized in this function [-Werror=maybe-uninitialized]
     if (r < 0) {
        ^
cc1: all warnings being treated as errors

Kevin
Christian Borntraeger Oct. 2, 2019, 4:13 p.m. UTC | #2
On 02.10.19 18:01, Kevin Wolf wrote:
> Am 30.09.2019 um 15:19 hat Christian Borntraeger geschrieben:
>> From: Paolo Bonzini <pbonzini@redhat.com>
>>
>> Currently MemoryRegionSection has 1:1 mapping to KVMSlot.
>> However next patch will allow splitting MemoryRegionSection into
>> several KVMSlot-s, make sure that kvm_physical_log_slot_clear()
>> is able to handle such 1:N mapping.
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
>> Reviewed-by: Peter Xu <peterx@redhat.com>
>> Message-Id: <20190924144751.24149-3-imammedo@redhat.com>
>> Acked-by: Paolo Bonzini <pbonzini@redhat.com>
>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> 
> This broke the build for me on F29:
> 
>   CC      x86_64-softmmu/accel/kvm/kvm-all.o
> /tmp/qemu/accel/kvm/kvm-all.c: In function 'kvm_log_clear':
> /tmp/qemu/accel/kvm/kvm-all.c:1086:8: error: 'ret' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>      if (r < 0) {
>         ^
> cc1: all warnings being treated as errors
> 

Does 

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index aabe097c410f..e2605da22e7e 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -712,7 +712,7 @@ static int kvm_physical_log_clear(KVMMemoryListener *kml,
     KVMState *s = kvm_state;
     uint64_t start, size, offset, count;
     KVMSlot *mem;
-    int ret, i;
+    int ret = 0, i;
 
     if (!s->manual_dirty_log_protect) {
         /* No need to do explicit clear */


fix the message?
Paolo Bonzini Oct. 2, 2019, 4:18 p.m. UTC | #3
On 02/10/19 18:13, Christian Borntraeger wrote:
> Does 
> 
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index aabe097c410f..e2605da22e7e 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -712,7 +712,7 @@ static int kvm_physical_log_clear(KVMMemoryListener *kml,
>      KVMState *s = kvm_state;
>      uint64_t start, size, offset, count;
>      KVMSlot *mem;
> -    int ret, i;
> +    int ret = 0, i;
>  
>      if (!s->manual_dirty_log_protect) {
>          /* No need to do explicit clear */
> 
> 
> fix the message?
> 

Yes, I'm sending shortly a pull request.

Paolo
diff mbox series

Patch

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index a85ec09486dd..ff9b95c0d103 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -589,8 +589,8 @@  static int kvm_log_clear_one_slot(KVMSlot *mem, int as_id, uint64_t start,
      * satisfy the KVM interface requirement.  Firstly, do the start
      * page alignment on 64 host pages
      */
-    bmap_start = (start - mem->start_addr) & KVM_CLEAR_LOG_MASK;
-    start_delta = start - mem->start_addr - bmap_start;
+    bmap_start = start & KVM_CLEAR_LOG_MASK;
+    start_delta = start - bmap_start;
     bmap_start /= psize;
 
     /*
@@ -694,8 +694,8 @@  static int kvm_physical_log_clear(KVMMemoryListener *kml,
                                   MemoryRegionSection *section)
 {
     KVMState *s = kvm_state;
-    uint64_t start, size;
-    KVMSlot *mem = NULL;
+    uint64_t start, size, offset, count;
+    KVMSlot *mem;
     int ret, i;
 
     if (!s->manual_dirty_log_protect) {
@@ -713,22 +713,30 @@  static int kvm_physical_log_clear(KVMMemoryListener *kml,
 
     kvm_slots_lock(kml);
 
-    /* Find any possible slot that covers the section */
     for (i = 0; i < s->nr_slots; i++) {
         mem = &kml->slots[i];
-        if (mem->start_addr <= start &&
-            start + size <= mem->start_addr + mem->memory_size) {
+        /* Discard slots that are empty or do not overlap the section */
+        if (!mem->memory_size ||
+            mem->start_addr > start + size - 1 ||
+            start > mem->start_addr + mem->memory_size - 1) {
+            continue;
+        }
+
+        if (start >= mem->start_addr) {
+            /* The slot starts before section or is aligned to it.  */
+            offset = start - mem->start_addr;
+            count = MIN(mem->memory_size - offset, size);
+        } else {
+            /* The slot starts after section.  */
+            offset = 0;
+            count = MIN(mem->memory_size, size - (mem->start_addr - start));
+        }
+        ret = kvm_log_clear_one_slot(mem, kml->as_id, offset, count);
+        if (ret < 0) {
             break;
         }
     }
 
-    /*
-     * We should always find one memslot until this point, otherwise
-     * there could be something wrong from the upper layer
-     */
-    assert(mem && i != s->nr_slots);
-    ret = kvm_log_clear_one_slot(mem, kml->as_id, start, size);
-
     kvm_slots_unlock(kml);
 
     return ret;