[PATCH 04/12] x86/boot: introduce module release

["Daniel P. Smith" <dpsmith@xxxxxxxxxxxxxxxxxxxx>]

A precarious approach was used to release the pages used to hold a boot module.
The precariousness stemmed from the fact that in the case of PV dom0, the
initrd module pages may be either mapped or explicitly copied into the dom0
address space. So to handle this situation, the PV dom0 construction code will
set the size of the module to zero, relying on discard_initial_images() to skip
any modules with a size of zero.

A function is introduced to release a module when it is no longer needed that
accepts a boolean parameter, free_mem, to indicate if the corresponding pages
can be freed. To track that a module has been released, the boot module flag
`released` is introduced.

The previous release model was a free all at once except those of size zeros,
which would handle any unused modules passed. The new model is one of, free
consumed module after usage is complete, thus unconsumed modules do not have a
consumer to free them. To address this, the discard_uknown_boot_modules() is
introduced and called after the last module identification occurs, initrd, to
free the pages of any boot modules that are identified as not being released.
After domain construction completes, all modules should be freed. A walk of the
boot modules is added after domain construction to validate and notify if a
module is found not to have been released.

Signed-off-by: Daniel P. Smith <dpsmith@xxxxxxxxxxxxxxxxxxxx>
---
Changes since v7:
- This is a new approach as an alternative to the `consumed` flag.
---
 xen/arch/x86/cpu/microcode/core.c   |  4 ++
 xen/arch/x86/hvm/dom0_build.c       |  7 ++--
 xen/arch/x86/include/asm/bootinfo.h |  2 +
 xen/arch/x86/include/asm/setup.h    |  3 +-
 xen/arch/x86/pv/dom0_build.c        | 20 ++--------
 xen/arch/x86/setup.c                | 57 +++++++++++++++++++++++------
 xen/xsm/xsm_core.c                  |  5 +++
 7 files changed, 67 insertions(+), 31 deletions(-)

diff --git a/xen/arch/x86/cpu/microcode/core.c 
b/xen/arch/x86/cpu/microcode/core.c
index b09cf83249f6..349e857f539a 100644
--- a/xen/arch/x86/cpu/microcode/core.c
+++ b/xen/arch/x86/cpu/microcode/core.c
@@ -756,6 +756,10 @@ static int __init cf_check microcode_init_cache(void)
         return rc;
     }
 
+    /* If raw module, we can free it now */
+    if ( !opt_scan )
+        release_boot_module(&bi->mods[early_mod_idx], true);
+
     if ( !patch )
         return -ENOENT;
 
diff --git a/xen/arch/x86/hvm/dom0_build.c b/xen/arch/x86/hvm/dom0_build.c
index 3dd913bdb029..a4ac262db463 100644
--- a/xen/arch/x86/hvm/dom0_build.c
+++ b/xen/arch/x86/hvm/dom0_build.c
@@ -704,6 +704,8 @@ static int __init pvh_load_kernel(struct domain *d, const 
module_t *image,
         return rc;
     }
 
+    release_module(image, true);
+
     /*
      * Find a RAM region big enough (and that doesn't overlap with the loaded
      * kernel) in order to load the initrd and the metadata. Note it could be
@@ -751,10 +753,9 @@ static int __init pvh_load_kernel(struct domain *d, const 
module_t *image,
             last_addr += len;
         }
         last_addr = ROUNDUP(last_addr, PAGE_SIZE);
-    }
 
-    /* Free temporary buffers. */
-    discard_initial_images();
+        release_module(initrd, true);
+    }
 
     if ( cmdline != NULL )
     {
diff --git a/xen/arch/x86/include/asm/bootinfo.h 
b/xen/arch/x86/include/asm/bootinfo.h
index 0fbf36739782..b1549d8c8f93 100644
--- a/xen/arch/x86/include/asm/bootinfo.h
+++ b/xen/arch/x86/include/asm/bootinfo.h
@@ -33,8 +33,10 @@ struct boot_module {
     /*
      * Module State Flags:
      *   relocated: indicates module has been relocated in memory.
+     *   released:  indicates module's pages have been freed.
      */
     bool relocated:1;
+    bool released:1;
 };
 
 /*
diff --git a/xen/arch/x86/include/asm/setup.h b/xen/arch/x86/include/asm/setup.h
index d7ed4f40024c..d68d37a5293b 100644
--- a/xen/arch/x86/include/asm/setup.h
+++ b/xen/arch/x86/include/asm/setup.h
@@ -36,12 +36,13 @@ void setup_io_bitmap(struct domain *d);
 extern struct boot_info xen_boot_info;
 
 unsigned long initial_images_nrpages(nodeid_t node);
-void discard_initial_images(void);
+void release_module(const module_t *m, bool mapped);
 
 struct boot_module;
 void *bootstrap_map_bm(const struct boot_module *bm);
 void *bootstrap_map(const module_t *mod);
 void bootstrap_unmap(void);
+void release_boot_module(struct boot_module *bm, bool mapped);
 
 struct rangeset;
 int remove_xen_ranges(struct rangeset *r);
diff --git a/xen/arch/x86/pv/dom0_build.c b/xen/arch/x86/pv/dom0_build.c
index cc882bee61c3..c1f44502a1ac 100644
--- a/xen/arch/x86/pv/dom0_build.c
+++ b/xen/arch/x86/pv/dom0_build.c
@@ -630,9 +630,7 @@ static int __init dom0_construct(struct domain *d,
                 }
             memcpy(page_to_virt(page), mfn_to_virt(initrd->mod_start),
                    initrd_len);
-            mpt_alloc = (paddr_t)initrd->mod_start << PAGE_SHIFT;
-            init_domheap_pages(mpt_alloc,
-                               mpt_alloc + PAGE_ALIGN(initrd_len));
+            release_module(initrd, true);
             initrd->mod_start = initrd_mfn = mfn_x(page_to_mfn(page));
         }
         else
@@ -640,18 +638,9 @@ static int __init dom0_construct(struct domain *d,
             while ( count-- )
                 if ( assign_pages(mfn_to_page(_mfn(mfn++)), 1, d, 0) )
                     BUG();
+            release_module(initrd, false);
         }
 
-        /*
-         * We have either:
-         * - Mapped the initrd directly into dom0, or
-         * - Copied it and freed the module.
-         *
-         * Either way, tell discard_initial_images() to not free it a second
-         * time.
-         */
-        initrd->mod_end = 0;
-
         iommu_memory_setup(d, "initrd", mfn_to_page(_mfn(initrd_mfn)),
                            PFN_UP(initrd_len), &flush_flags);
     }
@@ -839,7 +828,9 @@ static int __init dom0_construct(struct domain *d,
         printk("Failed to load the kernel binary\n");
         goto out;
     }
+    /* All done with kernel, release the module pages */
     bootstrap_unmap();
+    release_module(image, true);
 
     if ( UNSET_ADDR != parms.virt_hypercall )
     {
@@ -854,9 +845,6 @@ static int __init dom0_construct(struct domain *d,
         init_hypercall_page(d, _p(parms.virt_hypercall));
     }
 
-    /* Free temporary buffers. */
-    discard_initial_images();
-
     /* Set up start info area. */
     si = (start_info_t *)vstartinfo_start;
     clear_page(si);
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index d061ece0541f..e6d2d25fd038 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -341,27 +341,55 @@ unsigned long __init initial_images_nrpages(nodeid_t node)
     return nr;
 }
 
-void __init discard_initial_images(void) /* a.k.a. Free boot modules */
+void __init release_boot_module(struct boot_module *bm, bool free_mem)
+{
+    uint64_t start = pfn_to_paddr(bm->mod->mod_start);
+    uint64_t size  = bm->mod->mod_end;
+
+    if ( bm->released )
+    {
+        printk(XENLOG_WARNING "Attempt second release boot module of type 
%d\n",
+               bm->type);
+        return;
+    }
+
+    if ( free_mem )
+        init_domheap_pages(start, start + PAGE_ALIGN(size));
+
+    bm->released = true;
+}
+
+void __init release_module(const module_t *m, bool free_mem)
 {
     struct boot_info *bi = &xen_boot_info;
     unsigned int i;
 
-    for ( i = 0; i < bi->nr_modules; ++i )
+    for ( i = 0; i < bi->nr_modules; i++ )
     {
-        uint64_t start = pfn_to_paddr(bi->mods[i].mod->mod_start);
-        uint64_t size  = bi->mods[i].mod->mod_end;
+        if ( bi->mods[i].mod == m )
+            release_boot_module(&bi->mods[i], free_mem);
+    }
+}
 
-        /*
-         * Sometimes the initrd is mapped, rather than copied, into dom0.
-         * Size being 0 is how we're instructed to leave the module alone.
-         */
-        if ( size == 0 )
+static void __init discard_unknown_boot_modules(void)
+{
+    struct boot_info *bi = &xen_boot_info;
+    unsigned int i, count = 0;
+
+    for_each_boot_module_by_type(i, bi, BOOTMOD_UNKNOWN)
+    {
+        struct boot_module *bm = &bi->mods[i];
+
+        if ( bm == NULL || bm->released )
             continue;
 
-        init_domheap_pages(start, start + PAGE_ALIGN(size));
+        release_boot_module(bm, true);
+        count++;
     }
 
-    bi->nr_modules = 0;
+    if ( count )
+        printk(XENLOG_DEBUG "Releasing pages for uknown boot module %d\n",
+               count);
 }
 
 static void __init init_idle_domain(void)
@@ -2111,6 +2139,8 @@ void asmlinkage __init noreturn __start_xen(void)
                    initrdidx);
     }
 
+    discard_unknown_boot_modules();
+
     /*
      * We're going to setup domain0 using the module(s) that we stashed safely
      * above our heap. The second module, if present, is an initrd ramdisk.
@@ -2122,6 +2152,11 @@ void asmlinkage __init noreturn __start_xen(void)
     if ( !dom0 )
         panic("Could not set up DOM0 guest OS\n");
 
+    /* Check and warn if any modules did not get released */
+    for ( i = 0; i < bi->nr_modules; i++ )
+        if ( !bi->mods[i].released )
+            printk(XENLOG_ERR "Boot module %d not released, memory leaked", i);
+
     heap_init_late();
 
     init_trace_bufs();
diff --git a/xen/xsm/xsm_core.c b/xen/xsm/xsm_core.c
index f255fb63bf6f..d5875599b63a 100644
--- a/xen/xsm/xsm_core.c
+++ b/xen/xsm/xsm_core.c
@@ -162,6 +162,11 @@ int __init xsm_multiboot_init(struct boot_info *bi)
     ret = xsm_core_init(policy_buffer, policy_size);
     bootstrap_unmap();
 
+    /* If the policy was loaded from a boot module, release it's memory */
+    ret = first_boot_module_index(bi, BOOTMOD_XSM_POLICY);
+    if ( ret >= 0 && ret < bi->nr_modules )
+        release_boot_module(&bi->mods[ret], true);
+
     return 0;
 }
 #endif
-- 
2.30.2