On 28/11/18 10:17, Atsushi Nemoto wrote:
> The e1000 driver calls memory_region_add_coalescing but
> kvm_coalesce_mmio_region never been called for those regions.
>
> Update coalesced_range on memory_region_transaction_commit to fix this.
>
> It seems very old bug since memory region API
> was merged on commit 093bc2cd885e ("Hierarchical memory region API").
>
> Signed-off-by: Atsushi Nemoto <[email protected]>
I attach the patch I came up with. What do you think?
Thanks,
Paolo
> memory.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/memory.c b/memory.c
> index d14c6dec1d..f773b0be50 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -1021,6 +1021,8 @@ void memory_region_transaction_begin(void)
> ++memory_region_transaction_depth;
> }
>
> +static void memory_region_update_coalesced_range_as(MemoryRegion *mr,
> AddressSpace *as);
> +
> void memory_region_transaction_commit(void)
> {
> AddressSpace *as;
> @@ -1038,6 +1040,7 @@ void memory_region_transaction_commit(void)
> QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
> address_space_set_flatview(as);
> address_space_update_ioeventfds(as);
> + memory_region_update_coalesced_range_as(NULL, as);
> }
> memory_region_update_pending = false;
> ioeventfd_update_pending = false;
> @@ -2142,7 +2145,7 @@ static void
> memory_region_update_coalesced_range_as(MemoryRegion *mr, AddressSpa
>
> view = address_space_get_flatview(as);
> FOR_EACH_FLAT_RANGE(fr, view) {
> - if (fr->mr == mr) {
> + if (fr->mr == mr || (!mr && !QTAILQ_EMPTY(&fr->mr->coalesced))) {
> section = (MemoryRegionSection) {
> .fv = view,
> .offset_within_address_space = int128_get64(fr->addr.start),
> @@ -2152,7 +2155,7 @@ static void
> memory_region_update_coalesced_range_as(MemoryRegion *mr, AddressSpa
> MEMORY_LISTENER_CALL(as, coalesced_io_del, Reverse, §ion,
> int128_get64(fr->addr.start),
> int128_get64(fr->addr.size));
> - QTAILQ_FOREACH(cmr, &mr->coalesced, link) {
> + QTAILQ_FOREACH(cmr, &fr->mr->coalesced, link) {
> tmp = addrrange_shift(cmr->addr,
> int128_sub(fr->addr.start,
>
> int128_make64(fr->offset_in_region)));
>
diff --git a/memory.c b/memory.c
index d14c6dec1d..5759f74034 100644
--- a/memory.c
+++ b/memory.c
@@ -217,6 +217,7 @@ struct FlatRange {
bool romd_mode;
bool readonly;
bool nonvolatile;
+ int has_coalesced_range;
};
#define FOR_EACH_FLAT_RANGE(var, view) \
@@ -650,6 +651,7 @@ static void render_memory_region(FlatView *view,
fr.romd_mode = mr->romd_mode;
fr.readonly = readonly;
fr.nonvolatile = nonvolatile;
+ fr.has_coalesced_range = 0;
/* Render the region itself into any gaps left by the current view. */
for (i = 0; i < view->nr && int128_nz(remain); ++i) {
@@ -850,6 +852,49 @@ static void address_space_update_ioeventfds(AddressSpace *as)
flatview_unref(view);
}
+static void flat_range_coalesced_io_del(FlatRange *fr, AddressSpace *as)
+{
+ if (!fr->has_coalesced_range) {
+ return;
+ }
+
+ if (--fr->has_coalesced_range > 0) {
+ return;
+ }
+
+ MEMORY_LISTENER_UPDATE_REGION(fr, as, Reverse, coalesced_io_del,
+ int128_get64(fr->addr.start),
+ int128_get64(fr->addr.size));
+}
+
+static void flat_range_coalesced_io_add(FlatRange *fr, AddressSpace *as)
+{
+ MemoryRegion *mr = fr->mr;
+ CoalescedMemoryRange *cmr;
+ AddrRange tmp;
+
+ if (QTAILQ_EMPTY(&mr->coalesced)) {
+ return;
+ }
+
+ if (fr->has_coalesced_range++) {
+ return;
+ }
+
+ QTAILQ_FOREACH(cmr, &mr->coalesced, link) {
+ tmp = addrrange_shift(cmr->addr,
+ int128_sub(fr->addr.start,
+ int128_make64(fr->offset_in_region)));
+ if (!addrrange_intersects(tmp, fr->addr)) {
+ continue;
+ }
+ tmp = addrrange_intersection(tmp, fr->addr);
+ MEMORY_LISTENER_UPDATE_REGION(fr, as, Forward, coalesced_io_add,
+ int128_get64(tmp.start),
+ int128_get64(tmp.size));
+ }
+}
+
static void address_space_update_topology_pass(AddressSpace *as,
const FlatView *old_view,
const FlatView *new_view,
@@ -882,6 +927,7 @@ static void address_space_update_topology_pass(AddressSpace *as,
/* In old but not in new, or in both but attributes changed. */
if (!adding) {
+ flat_range_coalesced_io_del(frold, as);
MEMORY_LISTENER_UPDATE_REGION(frold, as, Reverse, region_del);
}
@@ -889,7 +935,9 @@ static void address_space_update_topology_pass(AddressSpace *as,
} else if (frold && frnew && flatrange_equal(frold, frnew)) {
/* In both and unchanged (except logging may have changed) */
- if (adding) {
+ if (!adding) {
+ flat_range_coalesced_io_del(frold, as);
+ } else {
MEMORY_LISTENER_UPDATE_REGION(frnew, as, Forward, region_nop);
if (frnew->dirty_log_mask & ~frold->dirty_log_mask) {
MEMORY_LISTENER_UPDATE_REGION(frnew, as, Forward, log_start,
@@ -901,6 +949,7 @@ static void address_space_update_topology_pass(AddressSpace *as,
frold->dirty_log_mask,
frnew->dirty_log_mask);
}
+ flat_range_coalesced_io_add(frnew, as);
}
++iold;
@@ -910,6 +959,7 @@ static void address_space_update_topology_pass(AddressSpace *as,
if (adding) {
MEMORY_LISTENER_UPDATE_REGION(frnew, as, Forward, region_add);
+ flat_range_coalesced_io_add(frnew, as);
}
++inew;
@@ -2136,34 +2186,12 @@ static void memory_region_update_coalesced_range_as(MemoryRegion *mr, AddressSpa
{
FlatView *view;
FlatRange *fr;
- CoalescedMemoryRange *cmr;
- AddrRange tmp;
- MemoryRegionSection section;
view = address_space_get_flatview(as);
FOR_EACH_FLAT_RANGE(fr, view) {
if (fr->mr == mr) {
- section = (MemoryRegionSection) {
- .fv = view,
- .offset_within_address_space = int128_get64(fr->addr.start),
- .size = fr->addr.size,
- };
-
- MEMORY_LISTENER_CALL(as, coalesced_io_del, Reverse, §ion,
- int128_get64(fr->addr.start),
- int128_get64(fr->addr.size));
- QTAILQ_FOREACH(cmr, &mr->coalesced, link) {
- tmp = addrrange_shift(cmr->addr,
- int128_sub(fr->addr.start,
- int128_make64(fr->offset_in_region)));
- if (!addrrange_intersects(tmp, fr->addr)) {
- continue;
- }
- tmp = addrrange_intersection(tmp, fr->addr);
- MEMORY_LISTENER_CALL(as, coalesced_io_add, Forward, §ion,
- int128_get64(tmp.start),
- int128_get64(tmp.size));
- }
+ flat_range_coalesced_io_del(fr, as);
+ flat_range_coalesced_io_add(fr, as);
}
}
flatview_unref(view);
