[12/23] memory: separate building the final memory map into two steps

Submitted by Avi Kivity on July 25, 2011, 2:02 p.m.

Details

Message ID 1311602584-23409-13-git-send-email-avi@redhat.com
State New
Headers show

Commit Message

Avi Kivity July 25, 2011, 2:02 p.m.
Instead of adding and deleting regions in one pass, do a delete
pass followed by an add pass.  This fixes the following case:

from:
  0x0000-0x0fff ram  (a1)
  0x1000-0x1fff mmio (a2)
  0x2000-0x2fff ram  (a3)

to:
  0x0000-0x2fff ram  (b1)

The single pass algorithm removed a1, added b2, then removed a2 and a2,
which caused the wrong memory map to be built.  The two pass algorithm
removes a1, a2, and a3, then adds b1.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 memory.c |   38 +++++++++++++++++++++++++++++---------
 1 files changed, 29 insertions(+), 9 deletions(-)

Comments

Anthony Liguori July 25, 2011, 7:12 p.m.
On 07/25/2011 09:02 AM, Avi Kivity wrote:
> Instead of adding and deleting regions in one pass, do a delete
> pass followed by an add pass.  This fixes the following case:
>
> from:
>    0x0000-0x0fff ram  (a1)
>    0x1000-0x1fff mmio (a2)
>    0x2000-0x2fff ram  (a3)
>
> to:
>    0x0000-0x2fff ram  (b1)
>
> The single pass algorithm removed a1, added b2, then removed a2 and a2,

You mean a2 and a3 I suppose?

Regards,

Anthony Liguori

> which caused the wrong memory map to be built.  The two pass algorithm
> removes a1, a2, and a3, then adds b1.
>
> Signed-off-by: Avi Kivity<avi@redhat.com>
> ---
>   memory.c |   38 +++++++++++++++++++++++++++++---------
>   1 files changed, 29 insertions(+), 9 deletions(-)
>
> diff --git a/memory.c b/memory.c
> index a5cde0c..009ad33 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -546,10 +546,11 @@ static void address_space_update_ioeventfds(AddressSpace *as)
>       as->ioeventfd_nb = ioeventfd_nb;
>   }
>
> -static void address_space_update_topology(AddressSpace *as)
> +static void address_space_update_topology_pass(AddressSpace *as,
> +                                               FlatView old_view,
> +                                               FlatView new_view,
> +                                               bool adding)
>   {
> -    FlatView old_view = as->current_map;
> -    FlatView new_view = generate_memory_topology(as->root);
>       unsigned iold, inew;
>       FlatRange *frold, *frnew;
>
> @@ -576,15 +577,20 @@ static void address_space_update_topology(AddressSpace *as)
>                       &&  !flatrange_equal(frold, frnew)))) {
>               /* In old, but (not in new, or in new but attributes changed). */
>
> -            as->ops->range_del(as, frold);
> +            if (!adding) {
> +                as->ops->range_del(as, frold);
> +            }
> +
>               ++iold;
>           } else if (frold&&  frnew&&  flatrange_equal(frold, frnew)) {
>               /* In both (logging may have changed) */
>
> -            if (frold->dirty_log_mask&&  !frnew->dirty_log_mask) {
> -                as->ops->log_stop(as, frnew);
> -            } else if (frnew->dirty_log_mask&&  !frold->dirty_log_mask) {
> -                as->ops->log_start(as, frnew);
> +            if (adding) {
> +                if (frold->dirty_log_mask&&  !frnew->dirty_log_mask) {
> +                    as->ops->log_stop(as, frnew);
> +                } else if (frnew->dirty_log_mask&&  !frold->dirty_log_mask) {
> +                    as->ops->log_start(as, frnew);
> +                }
>               }
>
>               ++iold;
> @@ -592,10 +598,24 @@ static void address_space_update_topology(AddressSpace *as)
>           } else {
>               /* In new */
>
> -            as->ops->range_add(as, frnew);
> +            if (adding) {
> +                as->ops->range_add(as, frnew);
> +            }
> +
>               ++inew;
>           }
>       }
> +}
> +
> +
> +static void address_space_update_topology(AddressSpace *as)
> +{
> +    FlatView old_view = as->current_map;
> +    FlatView new_view = generate_memory_topology(as->root);
> +
> +    address_space_update_topology_pass(as, old_view, new_view, false);
> +    address_space_update_topology_pass(as, old_view, new_view, true);
> +
>       as->current_map = new_view;
>       flatview_destroy(&old_view);
>       address_space_update_ioeventfds(as);
Avi Kivity July 26, 2011, 10:43 a.m.
On 07/25/2011 10:12 PM, Anthony Liguori wrote:
> On 07/25/2011 09:02 AM, Avi Kivity wrote:
>> Instead of adding and deleting regions in one pass, do a delete
>> pass followed by an add pass.  This fixes the following case:
>>
>> from:
>>    0x0000-0x0fff ram  (a1)
>>    0x1000-0x1fff mmio (a2)
>>    0x2000-0x2fff ram  (a3)
>>
>> to:
>>    0x0000-0x2fff ram  (b1)
>>
>> The single pass algorithm removed a1, added b2, then removed a2 and a2,
>
> You mean a2 and a3 I suppose?

Yes; fixed.

Patch hide | download patch | download mbox

diff --git a/memory.c b/memory.c
index a5cde0c..009ad33 100644
--- a/memory.c
+++ b/memory.c
@@ -546,10 +546,11 @@  static void address_space_update_ioeventfds(AddressSpace *as)
     as->ioeventfd_nb = ioeventfd_nb;
 }
 
-static void address_space_update_topology(AddressSpace *as)
+static void address_space_update_topology_pass(AddressSpace *as,
+                                               FlatView old_view,
+                                               FlatView new_view,
+                                               bool adding)
 {
-    FlatView old_view = as->current_map;
-    FlatView new_view = generate_memory_topology(as->root);
     unsigned iold, inew;
     FlatRange *frold, *frnew;
 
@@ -576,15 +577,20 @@  static void address_space_update_topology(AddressSpace *as)
                     && !flatrange_equal(frold, frnew)))) {
             /* In old, but (not in new, or in new but attributes changed). */
 
-            as->ops->range_del(as, frold);
+            if (!adding) {
+                as->ops->range_del(as, frold);
+            }
+
             ++iold;
         } else if (frold && frnew && flatrange_equal(frold, frnew)) {
             /* In both (logging may have changed) */
 
-            if (frold->dirty_log_mask && !frnew->dirty_log_mask) {
-                as->ops->log_stop(as, frnew);
-            } else if (frnew->dirty_log_mask && !frold->dirty_log_mask) {
-                as->ops->log_start(as, frnew);
+            if (adding) {
+                if (frold->dirty_log_mask && !frnew->dirty_log_mask) {
+                    as->ops->log_stop(as, frnew);
+                } else if (frnew->dirty_log_mask && !frold->dirty_log_mask) {
+                    as->ops->log_start(as, frnew);
+                }
             }
 
             ++iold;
@@ -592,10 +598,24 @@  static void address_space_update_topology(AddressSpace *as)
         } else {
             /* In new */
 
-            as->ops->range_add(as, frnew);
+            if (adding) {
+                as->ops->range_add(as, frnew);
+            }
+
             ++inew;
         }
     }
+}
+
+
+static void address_space_update_topology(AddressSpace *as)
+{
+    FlatView old_view = as->current_map;
+    FlatView new_view = generate_memory_topology(as->root);
+
+    address_space_update_topology_pass(as, old_view, new_view, false);
+    address_space_update_topology_pass(as, old_view, new_view, true);
+
     as->current_map = new_view;
     flatview_destroy(&old_view);
     address_space_update_ioeventfds(as);