x86: Fix clip_to_limit().

There are issues in updating the e820 map in the middle of a loop that
iterates over it. For example, after memmove(&e820.map[i],
&e820.map[i+1], ...), the original e820.map[i+1] become current
e820.map[i] but the next loop count is i+1, so the original
e820.map[i+1] will be skipped.

Fix and clarify the code by making a double loop.

Original bug discovery and fix by Xiao Guangrong <ericxiao.gr@gmail.com>

Signed-off-by: Keir Fraser <keir.fraser@citrix.com>

1 files changed, 24 insertions, 11 deletions

diff --git a/xen/arch/x86/e820.c b/xen/arch/x86/e820.c
index eac051fad8..fb76b5c51a 100644
--- a/xen/arch/x86/e820.c
+++ b/xen/arch/x86/e820.c
@@ -367,19 +367,32 @@ static void __init clip_to_limit(uint64_t limit, char *warnmsg)
     char _warnmsg[160];
     uint64_t old_limit = 0;
 
-    for ( i = 0; i < e820.nr_map; i++ )
+    for ( ; ; )
     {
-        if ( (e820.map[i].type != E820_RAM) ||
-             ((e820.map[i].addr + e820.map[i].size) <= limit) )
-            continue;
-        old_limit = e820.map[i].addr + e820.map[i].size;
+        /* Find a RAM region needing clipping. */
+        for ( i = 0; i < e820.nr_map; i++ )
+            if ( (e820.map[i].type == E820_RAM) &&
+                 ((e820.map[i].addr + e820.map[i].size) > limit) )
+                break;
+
+        /* If none found, we are done. */
+        if ( i == e820.nr_map )
+            break;        
+
+        old_limit = max_t(
+            uint64_t, old_limit, e820.map[i].addr + e820.map[i].size);
+
+        /* We try to convert clipped RAM areas to E820_UNUSABLE. */
         if ( e820_change_range_type(&e820, max(e820.map[i].addr, limit),
-                                    old_limit, E820_RAM, E820_UNUSABLE) )
-        {
-            /* Start again now e820 map must have changed. */
-            i = 0;
-        }
-        else if ( e820.map[i].addr < limit )
+                                    e820.map[i].addr + e820.map[i].size,
+                                    E820_RAM, E820_UNUSABLE) )
+            continue;
+
+        /*
+         * If the type change fails (e.g., not space in table) then we clip or 
+         * delete the region as appropriate.
+         */
+        if ( e820.map[i].addr < limit )
         {
             e820.map[i].size = limit - e820.map[i].addr;
         }