| Message ID | 20190930131955.101131-10-borntraeger@de.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [PULL,01/12] MAINTAINERS: Update S390 PCI Maintainer | expand |
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
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?
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 --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;