plat: Increase debug messages of the memregion coalescing function

Add more debugging messages to `ukplat_memregion_list_coalesce` to help
in finding inconsistencies as well as misbehaving memory region maps.

Furthermore, replace returns of error codes with `UK_ASSERT` and, thus,
change `ukplat_memregion_list_coalesce`'s signature to return nothing
as well as its callers' site accordingly.

Signed-off-by: Sergiu Moga <sergiu@unikraft.io>
Reviewed-by: Michalis Pappas <michalis@unikraft.io>
Approved-by: Razvan Deaconescu <razvand@unikraft.io>
GitHub-Closes: #1060

diff --git a/plat/common/include/uk/plat/common/memory.h b/plat/common/include/uk/plat/common/memory.h
index ef2970df265e850833673cb38ba3714a5d5ce04f..b39a9bb5fa779c1e5581db3a45fbb41591c0a841 100644 (file)
--- a/plat/common/include/uk/plat/common/memory.h
+++ b/plat/common/include/uk/plat/common/memory.h
@@ -382,11 +382,8 @@ ukplat_memregion_print_desc(struct ukplat_memregion_desc *mrd __unused) { }
  *
  * @param list
  *   The list whose memory region descriptors to coalesce.
- *
- * @return
- *   0 on success, < 0 otherwise.
  */
-int ukplat_memregion_list_coalesce(struct ukplat_memregion_list *list);
+void ukplat_memregion_list_coalesce(struct ukplat_memregion_list *list);
 
 /**
  * Initializes the platform memory mappings which require an allocator. This

diff --git a/plat/common/memory.c b/plat/common/memory.c
index a46f1344568b1aed2a1c4b464f7fa5922e7baa4b..10eb35bd75ce1371ed5d5db6198f77f3a5f9fc0c 100644 (file)
--- a/plat/common/memory.c
+++ b/plat/common/memory.c
@@ -304,7 +304,7 @@ static void ukplat_memregion_swap_if_unordered(struct ukplat_memregion_list *l,
        }
 }
 
-int ukplat_memregion_list_coalesce(struct ukplat_memregion_list *list)
+void ukplat_memregion_list_coalesce(struct ukplat_memregion_list *list)
 {
        struct ukplat_memregion_desc *m, *ml, *mr;
        __paddr_t ml_opbase, mr_opbase;
@@ -313,6 +313,10 @@ int ukplat_memregion_list_coalesce(struct ukplat_memregion_list *list)
        __u8 del; /* lets us know if a deletion happened */
        __u32 i;
 
+       UK_ASSERT(list);
+
+       uk_pr_debug("Coalescing memory region list.\n");
+
        i = 0;
        m = list->mrds;
        while (i + 1 < list->count) {
@@ -324,13 +328,20 @@ int ukplat_memregion_list_coalesce(struct ukplat_memregion_list *list)
                ml = &m[i];
                mr = &m[i + 1];
 
+               uk_pr_debug("Currently coalescing the following memory "
+                           "regions:\n");
+               ukplat_memregion_print_desc(ml);
+               ukplat_memregion_print_desc(mr);
+
+               UK_ASSERT(ml->pbase <= mr->pbase);
+
                ml_prio = get_mrd_prio(ml);
-               if (unlikely(ml_prio < 0))
-                       return -EINVAL;
+               uk_pr_debug("Priority of left memory region: %d\n", ml_prio);
+               UK_ASSERT(ml_prio >= 0);
 
                mr_prio = get_mrd_prio(mr);
-               if (unlikely(mr_prio < 0))
-                       return -EINVAL;
+               uk_pr_debug("Priority of right memory region: %d\n", mr_prio);
+               UK_ASSERT(mr_prio >= 0);
 
                ukplat_memregion_align_mrd(ml, &ml_opbase, &ml_olen);
                ukplat_memregion_align_mrd(mr, &mr_opbase, &mr_olen);
@@ -338,13 +349,15 @@ int ukplat_memregion_list_coalesce(struct ukplat_memregion_list *list)
                if (RANGE_OVERLAP(ml->pbase, ml->len, mr->pbase, mr->len)) {
                        /* If they are not of the same priority */
                        if (ml_prio != mr_prio) {
+                               uk_pr_debug("mrd's of different priority "
+                                           "overlap!\n");
+
                                /* At least one of the overlapping regions must
                                 * be a free one. Otherwise, something wrong
                                 * happened.
                                 */
-                               if (unlikely(!(mr_prio == MRD_PRIO_FREE ||
-                                              ml_prio == MRD_PRIO_FREE)))
-                                       return -EINVAL;
+                               UK_ASSERT(mr_prio == MRD_PRIO_FREE ||
+                                         ml_prio == MRD_PRIO_FREE);
 
                                overlapping_mrd_fixup(list, ml, mr, ml_prio,
                                                      mr_prio, i, i + 1);
@@ -352,9 +365,11 @@ int ukplat_memregion_list_coalesce(struct ukplat_memregion_list *list)
                                /* Remove dropped regions */
                                del = 1;
                                if (ml->len == 0) {
+                                       uk_pr_debug("Deleting left mrd!\n");
                                        ukplat_memregion_list_delete(list, i);
 
                                } else if (mr->len == 0) {
+                                       uk_pr_debug("Deleting right mrd!\n");
                                        ukplat_memregion_list_delete(list,
                                                                     i + 1);
                                } else {
@@ -362,19 +377,22 @@ int ukplat_memregion_list_coalesce(struct ukplat_memregion_list *list)
                                        del = 0;  /* No deletions */
                                }
 
-                       } else if (ml->flags != mr->flags) {
-                               return -EINVAL;
-
-                       /* If they have the same priority and same flags, merge
-                        * them. If they are contained within each other, drop
-                        * the contained one.
+                       /* If they have the same priority, merge them. If they
+                        * are contained within each other, drop the contained
+                        * one.
                         */
                        } else {
+                               /* We do not allow overlaps of same priority
+                                * and of different flags.
+                                */
+                               UK_ASSERT(ml->flags == mr->flags);
+
                                /* If the left region is contained within the
                                 * right region, drop it
                                 */
                                if (RANGE_CONTAIN(mr->pbase, mr->len,
                                                  ml->pbase, ml->len)) {
+                                       uk_pr_debug("Deleting left mrd!\n");
                                        ukplat_memregion_list_delete(list, i);
                                        del = 1;
 
@@ -385,6 +403,7 @@ int ukplat_memregion_list_coalesce(struct ukplat_memregion_list *list)
                                 */
                                } else if (RANGE_CONTAIN(ml->pbase, ml->len,
                                                         mr->pbase, mr->len)) {
+                                       uk_pr_debug("Deleting right mrd!\n");
                                        ukplat_memregion_list_delete(list,
                                                                     i + 1);
                                        del = 1;
@@ -392,6 +411,8 @@ int ukplat_memregion_list_coalesce(struct ukplat_memregion_list *list)
                                        goto restore_mrds;
                                }
 
+                               uk_pr_debug("Merging two overlapping mrd's.\n");
+
                                /* If they are not contained within each other,
                                 * merge them.
                                 */
@@ -414,10 +435,12 @@ int ukplat_memregion_list_coalesce(struct ukplat_memregion_list *list)
                 */
                } else if (ml->pbase + ml->len == mr->pbase &&
                           ml_prio == mr_prio && ml->flags == mr->flags) {
+                       uk_pr_debug("Merging two contiguous mrd's.\n");
                        ml->len += mr->len;
                        ukplat_memregion_list_delete(list, i + 1);
                        del = 1;
                } else {
+                       uk_pr_debug("No adjustment for these mrd's.\n");
                        i++;
                }
 
@@ -453,8 +476,6 @@ restore_mrds:
         * we exit this function
         */
        m[i].vbase = m[i].pbase;
-
-       return 0;
 }
 
 int ukplat_memregion_count(void)

diff --git a/plat/kvm/efi.c b/plat/kvm/efi.c
index 669f8feb258af40845968e309e14ec46d43b147a..b324c6c751d5e283f74eade9c915f4a2f841a26c 100644 (file)
--- a/plat/kvm/efi.c
+++ b/plat/kvm/efi.c
@@ -335,9 +335,7 @@ static void uk_efi_setup_bootinfo_mrds(struct ukplat_bootinfo *bi)
                        uk_efi_crash("Failed to insert mrd\n");
        }
 
-       rc = ukplat_memregion_list_coalesce(&bi->mrds);
-       if (unlikely(rc))
-               uk_efi_crash("Failed to coalesce memory regions\n");
+       ukplat_memregion_list_coalesce(&bi->mrds);
 
 #if defined(__X86_64__)
        rc = ukplat_memregion_alloc_sipi_vect();

diff --git a/plat/kvm/x86/lxboot.c b/plat/kvm/x86/lxboot.c
index 8f137a37ffd31d95b951bef329c2f31f5049cd3f..9b5696531661367228e62271faa5188ae82c8361 100644 (file)
--- a/plat/kvm/x86/lxboot.c
+++ b/plat/kvm/x86/lxboot.c
@@ -154,9 +154,7 @@ void lxboot_entry(struct lcpu *lcpu, struct lxboot_params *bp)
        lxboot_init_cmdline(bi, bp);
        lxboot_init_initrd(bi, bp);
        lxboot_init_mem(bi, bp);
-       rc = ukplat_memregion_list_coalesce(&bi->mrds);
-       if (unlikely(rc))
-               lxboot_crash(rc, "Could not coalesce memory regions");
+       ukplat_memregion_list_coalesce(&bi->mrds);
 
        memcpy(bi->bootprotocol, "lxboot", sizeof("lxboot"));
 

diff --git a/plat/kvm/x86/multiboot.c b/plat/kvm/x86/multiboot.c
index 8dfcc52514072a8a2bbb9e20f2e0c8779fe9a847..c3ae2ad6d3b4da39e30f67c87843bff2114148da 100644 (file)
--- a/plat/kvm/x86/multiboot.c
+++ b/plat/kvm/x86/multiboot.c
@@ -156,9 +156,7 @@ void multiboot_entry(struct lcpu *lcpu, struct multiboot_info *mi)
                }
        }
 
-       rc = ukplat_memregion_list_coalesce(&bi->mrds);
-       if (unlikely(rc))
-               multiboot_crash("Could not coalesce memory regions", rc);
+       ukplat_memregion_list_coalesce(&bi->mrds);
 
        rc = ukplat_memregion_alloc_sipi_vect();
        if (unlikely(rc))

diff --git a/plat/xen/x86/setup.c b/plat/xen/x86/setup.c
index 5baf1f28ce0718cf73a56855ccfe368ba4963f89..71ed4a8c62c29d5b8d700dd2710ec611c0599f88 100644 (file)
--- a/plat/xen/x86/setup.c
+++ b/plat/xen/x86/setup.c
@@ -192,7 +192,9 @@ static int _init_mem(struct ukplat_bootinfo *const bi)
                        return rc;
        }
 
-       return ukplat_memregion_list_coalesce(&bi->mrds);
+       ukplat_memregion_list_coalesce(&bi->mrds);
+
+       return 0;
 }
 
 static char *cmdline;