[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v4 06/23] xen/arm: don't add duplicate boot modules, introduce domU flag



Hi Stefano,

On 05/10/2018 19:47, Stefano Stabellini wrote:
Don't add duplicate boot modules (same kind and same start address),
they are freed later, we don't want to introduce double-free errors.

Introduce a domU flag in struct bootmodule and struct bootcmdline. Set
it for kernels and ramdisks of "xen,domain" nodes to avoid getting
confused in kernel_probe, where we try to guess which is the dom0 kernel
and initrd to be compatible with older versions of the multiboot spec.

This comments need at least to be replicated in the structure. So we have an idea how to use it.

Lastly, technically it is not older versions of the specification. Your extension does not deal with Dom0.


boot_module_find_by_kind and boot_cmdline_find_by_kind automatically
check for !domU entries (they are only used for non-domU modules).

Signed-off-by: Stefano Stabellini <stefanos@xxxxxxxxxx>

---
Changes in v4:
- use unsigned int
- better commit message
- introduce domU flag and usage

Changes in v2:
- new patch
---
  xen/arch/arm/bootfdt.c      | 14 +++++++++-----
  xen/arch/arm/setup.c        | 21 +++++++++++++++++----
  xen/include/asm-arm/setup.h |  4 +++-
  3 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
index 273032b..349aa9d 100644
--- a/xen/arch/arm/bootfdt.c
+++ b/xen/arch/arm/bootfdt.c
@@ -164,7 +164,8 @@ static void __init process_memory_node(const void *fdt, int 
node,
  }
static void __init add_boot_cmdline(const void *fdt, int node,
-                                    const char *name, bootmodule_kind kind)
+                                    const char *name, bootmodule_kind kind,
+                                    bool domU)
  {
      struct bootcmdlines *cmds = &bootinfo.cmdlines;
      struct bootcmdline *cmd;
@@ -184,6 +185,7 @@ static void __init add_boot_cmdline(const void *fdt, int 
node,
cmd = &cmds->cmdline[cmds->nr_mods++];
      cmd->kind = kind;
+    cmd->domU = domU;
ASSERT(strlen(name) <= DT_MAX_NAME);
      safe_strcpy(cmd->dt_name, name);
@@ -206,6 +208,7 @@ static void __init process_multiboot_node(const void *fdt, 
int node,
      int len = sizeof("/chosen");
      char path[8]; /* sizeof "/chosen" */
      int parent_node;
+    bool domU;
parent_node = fdt_parent_offset(fdt, node);
      ASSERT(parent_node >= 0);
@@ -263,8 +266,9 @@ static void __init process_multiboot_node(const void *fdt, 
int node,
              kind = BOOTMOD_XSM;
      }
- add_boot_module(kind, start, size);
-    add_boot_cmdline(fdt, node, fdt_get_name(fdt, parent_node, &len), kind);
+    domU = fdt_node_check_compatible(fdt, parent_node, "xen,domain") == 0;
+    add_boot_module(kind, start, size, domU);
+    add_boot_cmdline(fdt, node, fdt_get_name(fdt, parent_node, &len), kind, 
domU);
  }
static void __init process_chosen_node(const void *fdt, int node,
@@ -310,7 +314,7 @@ static void __init process_chosen_node(const void *fdt, int 
node,
printk("Initrd %"PRIpaddr"-%"PRIpaddr"\n", start, end); - add_boot_module(BOOTMOD_RAMDISK, start, end-start);
+    add_boot_module(BOOTMOD_RAMDISK, start, end-start, false);
  }
static int __init early_scan_node(const void *fdt,
@@ -381,7 +385,7 @@ size_t __init boot_fdt_info(const void *fdt, paddr_t paddr)
      if ( ret < 0 )
          panic("No valid device tree\n");
- add_boot_module(BOOTMOD_FDT, paddr, fdt_totalsize(fdt));
+    add_boot_module(BOOTMOD_FDT, paddr, fdt_totalsize(fdt), false);
device_tree_for_each_node((void *)fdt, early_scan_node, NULL);
      early_print_info();
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index bc7dd97..dbab232 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -201,10 +201,12 @@ void __init dt_unreserved_regions(paddr_t s, paddr_t e,
  }
struct bootmodule __init *add_boot_module(bootmodule_kind kind,
-                                          paddr_t start, paddr_t size)
+                                          paddr_t start, paddr_t size,
+                                          bool domU)
  {
      struct bootmodules *mods = &bootinfo.modules;
      struct bootmodule *mod;
This at least need to be replica
+    unsigned int i;
if ( mods->nr_mods == MAX_MODULES )
      {
@@ -212,11 +214,22 @@ struct bootmodule __init *add_boot_module(bootmodule_kind 
kind,
                 boot_module_kind_as_string(kind), start, start + size);
          return NULL;
      }
+    for ( i = 0 ; i < mods->nr_mods ; i++ )
+    {
+        mod = &mods->module[i];
+        if ( mod->kind == kind && mod->start == start )
+        {
+            if ( !domU )
+                mod->domU = false;
+            return mod;
+        }
+    }
mod = &mods->module[mods->nr_mods++];
      mod->kind = kind;
      mod->start = start;
      mod->size = size;
+    mod->domU = domU;
return mod;
  }
@@ -229,7 +242,7 @@ struct bootmodule * __init 
boot_module_find_by_kind(bootmodule_kind kind)
      for (i = 0 ; i < mods->nr_mods ; i++ )
      {
          mod = &mods->module[i];
-        if ( mod->kind == kind )
+        if ( mod->kind == kind && !mod->domU )

From the name of the function is it unclear why we would only return module with !mod->domU. So this needs some clarifications in the code.

              return mod;
      }
      return NULL;
@@ -244,7 +257,7 @@ struct bootcmdline * __init 
boot_cmdline_find_by_kind(bootmodule_kind kind)
      for ( i = 0 ; i < cmds->nr_mods ; i++ )
      {
          cmd = &cmds->cmdline[i];
-        if ( cmd->kind == kind )
+        if ( cmd->kind == kind && !cmd->domU )

Ditto here.

              return cmd;
      }
      return NULL;
@@ -738,7 +751,7 @@ void __init start_xen(unsigned long boot_phys_offset,
      /* Register Xen's load address as a boot module. */
      xen_bootmodule = add_boot_module(BOOTMOD_XEN,
                               (paddr_t)(uintptr_t)(_start + boot_phys_offset),
-                             (paddr_t)(uintptr_t)(_end - _start + 1));
+                             (paddr_t)(uintptr_t)(_end - _start + 1), false);
      BUG_ON(!xen_bootmodule);
xen_paddr = get_xen_paddr();
diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h
index c2ed5cc..711b4a2 100644
--- a/xen/include/asm-arm/setup.h
+++ b/xen/include/asm-arm/setup.h
@@ -33,6 +33,7 @@ struct meminfo {
  #define BOOTMOD_MAX_CMDLINE 1024
  struct bootmodule {
      bootmodule_kind kind;
+    bool domU;

This needs some documentation in the code.

Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.