[PATCH v3 0/4] xen/arm: Add support for XSM

[PATCH v3 0/4] xen/arm: Add support for XSM

[Julien Grall]

Hello all,

This patch series aims to add support for XSM on ARM.
I've tried to boot Xen on my board with the policy provided in the repository
(see tools/flask/policy). It boots with few warnings because some hypercall
are not describe in the policy.


Major changes in v3:
    - Patch #1-#11 from v2 already upstreamed
    - Minor change in "Add support for device tree"

Major changes in v2:
    - Fix various typo in commit message
    - Fix compilation on ARM64
    - Add xsm_core_init (patch #14) to avoid code duplication

A working tree can be found here:
    git://xenbits.xen.org/people/julieng/xen-unstable.git branch xsm-v3

Sincerely yours,

Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>

Julien Grall (4):
  xen/xsm: Don't use multiboot by default to initialize XSM
  xen/xsm: Add xsm_core_init function
  xen/xsm: Add support for device tree
  xen/arm: Add support for XSM

 docs/misc/arm/device-tree/booting.txt |    1 +
 xen/arch/arm/setup.c                  |    4 +--
 xen/arch/arm/traps.c                  |    1 +
 xen/arch/x86/setup.c                  |    2 +-
 xen/common/device_tree.c              |    2 ++
 xen/include/asm-x86/config.h          |    2 ++
 xen/include/xen/device_tree.h         |    3 +-
 xen/include/xsm/xsm.h                 |   35 ++++++++++++++-----
 xen/xsm/xsm_core.c                    |   59 ++++++++++++++++++++++++++-------
 xen/xsm/xsm_policy.c                  |   47 ++++++++++++++++++++++++--
 10 files changed, 129 insertions(+), 27 deletions(-)

-- 
1.7.10.4

[PATCH v3 1/4] xen/xsm: Don't use multiboot by default to initialize XSM

[Julien Grall]

Multiboot protocol is only used by x86. When XSM support for ARM will be
introduced, another way will be used to retrieve the policy blob.

Introduce CONFIG_MULTIBOOT to compile XSM specific multiboot code and
rename xsm_init into xsm_multiboot_init to avoid confusion.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Keir Fraser <keir@xen.org>
---
 xen/arch/x86/setup.c         |    2 +-
 xen/include/asm-x86/config.h |    2 ++
 xen/include/xsm/xsm.h        |   23 +++++++++++++++--------
 xen/xsm/xsm_core.c           |    9 ++++++---
 xen/xsm/xsm_policy.c         |   10 +++++++---
 5 files changed, 31 insertions(+), 15 deletions(-)

diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 4dbf2b7..e9c2c51 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1251,7 +1251,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
     bitmap_fill(module_map, mbi->mods_count);
     __clear_bit(0, module_map); /* Dom0 kernel is always first */
 
-    xsm_init(module_map, mbi, bootstrap_map);
+    xsm_multiboot_init(module_map, mbi, bootstrap_map);
 
     microcode_grab_module(module_map, mbi, bootstrap_map);
 
diff --git a/xen/include/asm-x86/config.h b/xen/include/asm-x86/config.h
index 99d0e2c..24f95a3 100644
--- a/xen/include/asm-x86/config.h
+++ b/xen/include/asm-x86/config.h
@@ -52,6 +52,8 @@
 #define CONFIG_KEXEC 1
 #define CONFIG_WATCHDOG 1
 
+#define CONFIG_MULTIBOOT 1
+
 #define HZ 100
 
 #define OPT_CONSOLE_STR "vga"
diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index 15acb3b..4863e41 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -643,11 +643,15 @@ static inline int xsm_map_gmfn_foreign (xsm_default_t def, struct domain *d, str
 
 #endif /* XSM_NO_WRAPPERS */
 
-extern int xsm_init(unsigned long *module_map, const multiboot_info_t *mbi,
-                    void *(*bootstrap_map)(const module_t *));
-extern int xsm_policy_init(unsigned long *module_map,
-                           const multiboot_info_t *mbi,
-                           void *(*bootstrap_map)(const module_t *));
+#ifdef CONFIG_MULTIBOOT
+extern int xsm_multiboot_init(unsigned long *module_map,
+                              const multiboot_info_t *mbi,
+                              void *(*bootstrap_map)(const module_t *));
+extern int xsm_multiboot_policy_init(unsigned long *module_map,
+                                     const multiboot_info_t *mbi,
+                                     void *(*bootstrap_map)(const module_t *));
+#endif
+
 extern int register_xsm(struct xsm_operations *ops);
 extern int unregister_xsm(struct xsm_operations *ops);
 
@@ -658,12 +662,15 @@ extern void xsm_fixup_ops(struct xsm_operations *ops);
 
 #include <xsm/dummy.h>
 
-static inline int xsm_init (unsigned long *module_map,
-                            const multiboot_info_t *mbi,
-                            void *(*bootstrap_map)(const module_t *))
+#ifdef CONFIG_MULTIBOOT
+static inline int xsm_multiboot_init (unsigned long *module_map,
+                                      const multiboot_info_t *mbi,
+                                      void *(*bootstrap_map)(const module_t *))
 {
     return 0;
 }
+#endif
+
 #endif /* XSM_ENABLE */
 
 #endif /* __XSM_H */
diff --git a/xen/xsm/xsm_core.c b/xen/xsm/xsm_core.c
index b16c071..11a9ca7 100644
--- a/xen/xsm/xsm_core.c
+++ b/xen/xsm/xsm_core.c
@@ -43,8 +43,10 @@ static void __init do_xsm_initcalls(void)
     }
 }
 
-int __init xsm_init(unsigned long *module_map, const multiboot_info_t *mbi,
-                    void *(*bootstrap_map)(const module_t *))
+#ifdef CONFIG_MULTIBOOT
+int __init xsm_multiboot_init(unsigned long *module_map,
+                              const multiboot_info_t *mbi,
+                              void *(*bootstrap_map)(const module_t *))
 {
     int ret = 0;
 
@@ -52,7 +54,7 @@ int __init xsm_init(unsigned long *module_map, const multiboot_info_t *mbi,
 
     if ( XSM_MAGIC )
     {
-        ret = xsm_policy_init(module_map, mbi, bootstrap_map);
+        ret = xsm_multiboot_policy_init(module_map, mbi, bootstrap_map);
         if ( ret )
         {
             bootstrap_map(NULL);
@@ -75,6 +77,7 @@ int __init xsm_init(unsigned long *module_map, const multiboot_info_t *mbi,
 
     return 0;
 }
+#endif
 
 int register_xsm(struct xsm_operations *ops)
 {
diff --git a/xen/xsm/xsm_policy.c b/xen/xsm/xsm_policy.c
index 0186f30..3d5f66a 100644
--- a/xen/xsm/xsm_policy.c
+++ b/xen/xsm/xsm_policy.c
@@ -19,15 +19,18 @@
  */
 
 #include <xsm/xsm.h>
+#ifdef CONFIG_MULTIBOOT
 #include <xen/multiboot.h>
+#endif
 #include <xen/bitops.h>
 
 char *__initdata policy_buffer = NULL;
 u32 __initdata policy_size = 0;
 
-int __init xsm_policy_init(unsigned long *module_map,
-                           const multiboot_info_t *mbi,
-                           void *(*bootstrap_map)(const module_t *))
+#ifdef CONFIG_MULTIBOOT
+int __init xsm_multiboot_policy_init(unsigned long *module_map,
+                                     const multiboot_info_t *mbi,
+                                     void *(*bootstrap_map)(const module_t *))
 {
     int i;
     module_t *mod = (module_t *)__va(mbi->mods_addr);
@@ -65,3 +68,4 @@ int __init xsm_policy_init(unsigned long *module_map,
 
     return rc;
 }
+#endif
-- 
1.7.10.4

[PATCH v3 2/4] xen/xsm: Add xsm_core_init function

[Julien Grall]

This function contains non-specific architecture code (mostly the tail of
xsm_multiboot_init). It will be used later to avoid code duplication.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>

---
    Changes in v2:
        - Patch added
---
 xen/xsm/xsm_core.c |   26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/xen/xsm/xsm_core.c b/xen/xsm/xsm_core.c
index 11a9ca7..24b0ff4 100644
--- a/xen/xsm/xsm_core.c
+++ b/xen/xsm/xsm_core.c
@@ -43,6 +43,21 @@ static void __init do_xsm_initcalls(void)
     }
 }
 
+static int __init xsm_core_init(void)
+{
+    if ( verify(&dummy_xsm_ops) )
+    {
+        printk("%s could not verify "
+               "dummy_xsm_ops structure.\n", __FUNCTION__);
+        return -EIO;
+    }
+
+    xsm_ops = &dummy_xsm_ops;
+    do_xsm_initcalls();
+
+    return 0;
+}
+
 #ifdef CONFIG_MULTIBOOT
 int __init xsm_multiboot_init(unsigned long *module_map,
                               const multiboot_info_t *mbi,
@@ -63,16 +78,7 @@ int __init xsm_multiboot_init(unsigned long *module_map,
         }
     }
 
-    if ( verify(&dummy_xsm_ops) )
-    {
-        bootstrap_map(NULL);
-        printk("%s could not verify "
-               "dummy_xsm_ops structure.\n", __FUNCTION__);
-        return -EIO;
-    }
-
-    xsm_ops = &dummy_xsm_ops;
-    do_xsm_initcalls();
+    ret = xsm_core_init();
     bootstrap_map(NULL);
 
     return 0;
-- 
1.7.10.4

[PATCH v3 3/4] xen/xsm: Add support for device tree

[Julien Grall]

This patch adds a new module "xen,xsm-blob" to allow the user to load the XSM
policy when Xen is booting.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>

---
    Changes in v3:
        - xfree is able to handle NULL correctly
    Changes in v2:
        - Use xsm_core_init function
---
 docs/misc/arm/device-tree/booting.txt |    1 +
 xen/common/device_tree.c              |    2 ++
 xen/include/xen/device_tree.h         |    3 ++-
 xen/include/xsm/xsm.h                 |   12 +++++++++++
 xen/xsm/xsm_core.c                    |   26 +++++++++++++++++++++++
 xen/xsm/xsm_policy.c                  |   37 +++++++++++++++++++++++++++++++++
 6 files changed, 80 insertions(+), 1 deletion(-)

diff --git a/docs/misc/arm/device-tree/booting.txt b/docs/misc/arm/device-tree/booting.txt
index 07fde27..85988fb 100644
--- a/docs/misc/arm/device-tree/booting.txt
+++ b/docs/misc/arm/device-tree/booting.txt
@@ -16,6 +16,7 @@ Each node contains the following properties:
 
 	- "linux-zimage" -- the dom0 kernel
 	- "linux-initrd" -- the dom0 ramdisk
+	- "xsm-blob"	 -- XSM policy blob
 
 - reg
 
diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
index 55716a8..91146fb 100644
--- a/xen/common/device_tree.c
+++ b/xen/common/device_tree.c
@@ -354,6 +354,8 @@ static void __init process_multiboot_node(const void *fdt, int node,
         nr = MOD_KERNEL;
     else if ( fdt_node_check_compatible(fdt, node, "xen,linux-initrd") == 0)
         nr = MOD_INITRD;
+    else if ( fdt_node_check_compatible(fdt, node, "xen,xsm-blob") == 0 )
+        nr = MOD_XSM;
     else
         early_panic("%s not a known xen multiboot type\n", name);
 
diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
index 9a8c3de..76faf11 100644
--- a/xen/include/xen/device_tree.h
+++ b/xen/include/xen/device_tree.h
@@ -24,7 +24,8 @@
 #define MOD_FDT    1
 #define MOD_KERNEL 2
 #define MOD_INITRD 3
-#define NR_MODULES 4
+#define MOD_XSM    4
+#define NR_MODULES 5
 
 #define MOD_DISCARD_FIRST MOD_FDT
 
diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index 4863e41..2cd3a3b 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -652,6 +652,11 @@ extern int xsm_multiboot_policy_init(unsigned long *module_map,
                                      void *(*bootstrap_map)(const module_t *));
 #endif
 
+#ifdef HAS_DEVICE_TREE
+extern int xsm_dt_init(void);
+extern int xsm_dt_policy_init(void);
+#endif
+
 extern int register_xsm(struct xsm_operations *ops);
 extern int unregister_xsm(struct xsm_operations *ops);
 
@@ -671,6 +676,13 @@ static inline int xsm_multiboot_init (unsigned long *module_map,
 }
 #endif
 
+#ifdef HAS_DEVICE_TREE
+static inline int xsm_dt_init(void)
+{
+    return 0;
+}
+#endif
+
 #endif /* XSM_ENABLE */
 
 #endif /* __XSM_H */
diff --git a/xen/xsm/xsm_core.c b/xen/xsm/xsm_core.c
index 24b0ff4..0ac6d03 100644
--- a/xen/xsm/xsm_core.c
+++ b/xen/xsm/xsm_core.c
@@ -85,6 +85,32 @@ int __init xsm_multiboot_init(unsigned long *module_map,
 }
 #endif
 
+#ifdef HAS_DEVICE_TREE
+int __init xsm_dt_init(void)
+{
+    int ret = 0;
+
+    printk("XSM Framework v" XSM_FRAMEWORK_VERSION " initialized\n");
+
+    if ( XSM_MAGIC )
+    {
+        ret = xsm_dt_policy_init();
+        if ( ret )
+        {
+            printk("%s: Error initializing policy (rc = %d).\n",
+                   __FUNCTION__, ret);
+            return -EINVAL;
+        }
+    }
+
+    ret = xsm_core_init();
+
+    xfree(policy_buffer);
+
+    return ret;
+}
+#endif
+
 int register_xsm(struct xsm_operations *ops)
 {
     if ( verify(ops) )
diff --git a/xen/xsm/xsm_policy.c b/xen/xsm/xsm_policy.c
index 3d5f66a..a0dee09 100644
--- a/xen/xsm/xsm_policy.c
+++ b/xen/xsm/xsm_policy.c
@@ -23,6 +23,10 @@
 #include <xen/multiboot.h>
 #endif
 #include <xen/bitops.h>
+#ifdef HAS_DEVICE_TREE
+# include <asm/setup.h>
+# include <xen/device_tree.h>
+#endif
 
 char *__initdata policy_buffer = NULL;
 u32 __initdata policy_size = 0;
@@ -69,3 +73,36 @@ int __init xsm_multiboot_policy_init(unsigned long *module_map,
     return rc;
 }
 #endif
+
+#ifdef HAS_DEVICE_TREE
+int __init xsm_dt_policy_init(void)
+{
+    paddr_t paddr = early_info.modules.module[MOD_XSM].start;
+    paddr_t len = early_info.modules.module[MOD_XSM].size;
+    xsm_magic_t magic;
+
+    if ( !len )
+        return 0;
+
+    copy_from_paddr(&magic, paddr, sizeof(magic));
+
+    if ( magic != XSM_MAGIC )
+    {
+        printk(XENLOG_ERR "xsm: Invalid magic for XSM blob got 0x%x "
+               "expected 0x%x\n", magic, XSM_MAGIC);
+        return -EINVAL;
+    }
+
+    printk("xsm: Policy len = 0x%"PRIpaddr" start at 0x%"PRIpaddr"\n",
+           len, paddr);
+
+    policy_buffer = xmalloc_bytes(len);
+    if ( !policy_buffer )
+        return -ENOMEM;
+
+    copy_from_paddr(policy_buffer, paddr, len);
+    policy_size = len;
+
+    return 0;
+}
+#endif
-- 
1.7.10.4

[PATCH v3 4/4] xen/arm: Add support for XSM

[Julien Grall]

Signed-off-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/arch/arm/setup.c |    4 ++--
 xen/arch/arm/traps.c |    1 +
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 215ec24..dbe3831 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -44,6 +44,7 @@
 #include <asm/cpufeature.h>
 #include <asm/platform.h>
 #include <asm/procinfo.h>
+#include <xsm/xsm.h>
 
 struct cpuinfo_arm __read_mostly boot_cpu_data;
 
@@ -719,8 +720,7 @@ void __init start_xen(unsigned long boot_phys_offset,
 
     init_IRQ();
 
-    gic_route_ppis();
-    gic_route_spis();
+    xsm_dt_init();
 
     init_maintenance_interrupt();
     init_timer_interrupt();
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index ec43e65..a7edc4e 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1006,6 +1006,7 @@ static arm_hypercall_t arm_hypercall_table[] = {
     HYPERCALL(sched_op, 2),
     HYPERCALL(console_io, 3),
     HYPERCALL(xen_version, 2),
+    HYPERCALL(xsm_op, 1),
     HYPERCALL(event_channel_op, 2),
     HYPERCALL(physdev_op, 2),
     HYPERCALL(sysctl, 2),
-- 
1.7.10.4

Re: [PATCH v3 4/4] xen/arm: Add support for XSM

[Julien Grall]

On 03/21/2014 01:05 PM, Julien Grall wrote:
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
> ---
>  xen/arch/arm/setup.c |    4 ++--
>  xen/arch/arm/traps.c |    1 +
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 215ec24..dbe3831 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -44,6 +44,7 @@
>  #include <asm/cpufeature.h>
>  #include <asm/platform.h>
>  #include <asm/procinfo.h>
> +#include <xsm/xsm.h>
>  
>  struct cpuinfo_arm __read_mostly boot_cpu_data;
>  
> @@ -719,8 +720,7 @@ void __init start_xen(unsigned long boot_phys_offset,
>  
>      init_IRQ();
>  
> -    gic_route_ppis();
> -    gic_route_spis();
> +    xsm_dt_init();

Hmmm I badly rebased this patch. It should be

commit b6948551a38564ea8d250ed9f3bac24a73d2fc3c
Author: Julien Grall <julien.grall@linaro.org>
Date:   Wed Mar 12 10:41:19 2014 +0000

    xen/arm: Add support for XSM
    
    Signed-off-by: Julien Grall <julien.grall@linaro.org>
    Acked-by: Ian Campbell <ian.campbell@citrix.com>

diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 215ec24..154b707 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -44,6 +44,7 @@
 #include <asm/cpufeature.h>
 #include <asm/platform.h>
 #include <asm/procinfo.h>
+#include <xsm/xsm.h>
 
 struct cpuinfo_arm __read_mostly boot_cpu_data;
 
@@ -721,6 +722,7 @@ void __init start_xen(unsigned long boot_phys_offset,
 
     gic_route_ppis();
     gic_route_spis();
+    xsm_dt_init();
 
     init_maintenance_interrupt();
     init_timer_interrupt();
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index ec43e65..a7edc4e 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1006,6 +1006,7 @@ static arm_hypercall_t arm_hypercall_table[] = {
     HYPERCALL(sched_op, 2),
     HYPERCALL(console_io, 3),
     HYPERCALL(xen_version, 2),
+    HYPERCALL(xsm_op, 1),
     HYPERCALL(event_channel_op, 2),
     HYPERCALL(physdev_op, 2),
     HYPERCALL(sysctl, 2),

-- 
Julien Grall

Re: [PATCH v3 2/4] xen/xsm: Add xsm_core_init function

[Jan Beulich]

>>> On 21.03.14 at 14:05, Julien Grall <julien.grall@linaro.org> wrote:
> --- a/xen/xsm/xsm_core.c
> +++ b/xen/xsm/xsm_core.c
> @@ -43,6 +43,21 @@ static void __init do_xsm_initcalls(void)
>      }
>  }
>  
> +static int __init xsm_core_init(void)
> +{
> +    if ( verify(&dummy_xsm_ops) )
> +    {
> +        printk("%s could not verify "
> +               "dummy_xsm_ops structure.\n", __FUNCTION__);
> +        return -EIO;
> +    }
> +
> +    xsm_ops = &dummy_xsm_ops;
> +    do_xsm_initcalls();
> +
> +    return 0;
> +}
> +
>  #ifdef CONFIG_MULTIBOOT
>  int __init xsm_multiboot_init(unsigned long *module_map,
>                                const multiboot_info_t *mbi,
> @@ -63,16 +78,7 @@ int __init xsm_multiboot_init(unsigned long *module_map,
>          }
>      }
>  
> -    if ( verify(&dummy_xsm_ops) )
> -    {
> -        bootstrap_map(NULL);

So where did that call go?

Jan

> -        printk("%s could not verify "
> -               "dummy_xsm_ops structure.\n", __FUNCTION__);
> -        return -EIO;
> -    }
> -
> -    xsm_ops = &dummy_xsm_ops;
> -    do_xsm_initcalls();
> +    ret = xsm_core_init();
>      bootstrap_map(NULL);
>  
>      return 0;

Re: [PATCH v3 2/4] xen/xsm: Add xsm_core_init function

[Julien Grall]

Hi Jan,

On 03/21/2014 01:39 PM, Jan Beulich wrote:
>>>> On 21.03.14 at 14:05, Julien Grall <julien.grall@linaro.org> wrote:
>> --- a/xen/xsm/xsm_core.c
>> +++ b/xen/xsm/xsm_core.c
>> @@ -43,6 +43,21 @@ static void __init do_xsm_initcalls(void)
>>      }
>>  }
>>  
>> +static int __init xsm_core_init(void)
>> +{
>> +    if ( verify(&dummy_xsm_ops) )
>> +    {
>> +        printk("%s could not verify "
>> +               "dummy_xsm_ops structure.\n", __FUNCTION__);
>> +        return -EIO;
>> +    }
>> +
>> +    xsm_ops = &dummy_xsm_ops;
>> +    do_xsm_initcalls();
>> +
>> +    return 0;
>> +}
>> +
>>  #ifdef CONFIG_MULTIBOOT
>>  int __init xsm_multiboot_init(unsigned long *module_map,
>>                                const multiboot_info_t *mbi,
>> @@ -63,16 +78,7 @@ int __init xsm_multiboot_init(unsigned long *module_map,
>>          }
>>      }
>>  
>> -    if ( verify(&dummy_xsm_ops) )
>> -    {
>> -        bootstrap_map(NULL);
> 
> So where did that call go?

After xsm_core_init (see below).

>> -        printk("%s could not verify "
>> -               "dummy_xsm_ops structure.\n", __FUNCTION__);
>> -        return -EIO;
>> -    }
>> -
>> -    xsm_ops = &dummy_xsm_ops;
>> -    do_xsm_initcalls();
>> +    ret = xsm_core_init();
>>      bootstrap_map(NULL);

Just here.

Regards,

-- 
Julien Grall

Re: [PATCH v3 3/4] xen/xsm: Add support for device tree

[Ian Campbell]

On Fri, 2014-03-21 at 13:05 +0000, Julien Grall wrote:
> This patch adds a new module "xen,xsm-blob" to allow the user to load the XSM
> policy when Xen is booting.

Why "xsm-blob" and not something descriptive like "xsm-policy"?

Ian.

Re: [PATCH v3 2/4] xen/xsm: Add xsm_core_init function

[Daniel De Graaf]

On 03/21/2014 09:05 AM, Julien Grall wrote:
> This function contains non-specific architecture code (mostly the tail of
> xsm_multiboot_init). It will be used later to avoid code duplication.
>
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>

Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>

Re: [PATCH v3 3/4] xen/xsm: Add support for device tree

[Julien Grall]

Hi Ian,

On 03/21/2014 02:12 PM, Ian Campbell wrote:
> On Fri, 2014-03-21 at 13:05 +0000, Julien Grall wrote:
>> This patch adds a new module "xen,xsm-blob" to allow the user to load the XSM
>> policy when Xen is booting.
> 
> Why "xsm-blob" and not something descriptive like "xsm-policy"?

Actually I didn't though about xsm-policy. Do you mind if I resend only
this patch?

I'm also wondering if I should #ifdef XSM_ENABLE this new part in
common/device_tree.c

Regards,

-- 
Julien Grall

Re: [PATCH v3 3/4] xen/xsm: Add support for device tree

[Julien Grall]

On 03/21/2014 02:20 PM, Julien Grall wrote:
> Hi Ian,
> 
> On 03/21/2014 02:12 PM, Ian Campbell wrote:
>> On Fri, 2014-03-21 at 13:05 +0000, Julien Grall wrote:
>>> This patch adds a new module "xen,xsm-blob" to allow the user to load the XSM
>>> policy when Xen is booting.
>>
>> Why "xsm-blob" and not something descriptive like "xsm-policy"?
> 
> Actually I didn't though about xsm-policy. Do you mind if I resend only
> this patch?

I'm actually there is another rebase issue on the patch #4. I will
resend the whole series.

-- 
Julien Grall

Re: [PATCH v3 2/4] xen/xsm: Add xsm_core_init function

[Jan Beulich]

>>> On 21.03.14 at 15:02, Julien Grall <julien.grall@linaro.org> wrote:
> Hi Jan,
> 
> On 03/21/2014 01:39 PM, Jan Beulich wrote:
>>>>> On 21.03.14 at 14:05, Julien Grall <julien.grall@linaro.org> wrote:
>>> --- a/xen/xsm/xsm_core.c
>>> +++ b/xen/xsm/xsm_core.c
>>> @@ -43,6 +43,21 @@ static void __init do_xsm_initcalls(void)
>>>      }
>>>  }
>>>  
>>> +static int __init xsm_core_init(void)
>>> +{
>>> +    if ( verify(&dummy_xsm_ops) )
>>> +    {
>>> +        printk("%s could not verify "
>>> +               "dummy_xsm_ops structure.\n", __FUNCTION__);
>>> +        return -EIO;
>>> +    }
>>> +
>>> +    xsm_ops = &dummy_xsm_ops;
>>> +    do_xsm_initcalls();
>>> +
>>> +    return 0;
>>> +}
>>> +
>>>  #ifdef CONFIG_MULTIBOOT
>>>  int __init xsm_multiboot_init(unsigned long *module_map,
>>>                                const multiboot_info_t *mbi,
>>> @@ -63,16 +78,7 @@ int __init xsm_multiboot_init(unsigned long *module_map,
>>>          }
>>>      }
>>>  
>>> -    if ( verify(&dummy_xsm_ops) )
>>> -    {
>>> -        bootstrap_map(NULL);
>> 
>> So where did that call go?
> 
> After xsm_core_init (see below).
> 
>>> -        printk("%s could not verify "
>>> -               "dummy_xsm_ops structure.\n", __FUNCTION__);
>>> -        return -EIO;
>>> -    }
>>> -
>>> -    xsm_ops = &dummy_xsm_ops;
>>> -    do_xsm_initcalls();
>>> +    ret = xsm_core_init();
>>>      bootstrap_map(NULL);
> 
> Just here.

Oh, right, you're just folding both paths as far as this function is
concerned. That's fine then of course.

Jan

Re: [PATCH v3 3/4] xen/xsm: Add support for device tree

[Ian Campbell]

On Fri, 2014-03-21 at 14:21 +0000, Julien Grall wrote:
> On 03/21/2014 02:20 PM, Julien Grall wrote:
> > Hi Ian,
> > 
> > On 03/21/2014 02:12 PM, Ian Campbell wrote:
> >> On Fri, 2014-03-21 at 13:05 +0000, Julien Grall wrote:
> >>> This patch adds a new module "xen,xsm-blob" to allow the user to load the XSM
> >>> policy when Xen is booting.
> >>
> >> Why "xsm-blob" and not something descriptive like "xsm-policy"?
> > 
> > Actually I didn't though about xsm-policy. Do you mind if I resend only
> > this patch?
> 
> I'm actually there is another rebase issue on the patch #4. I will
> resend the whole series.

OK.

> I'm also wondering if I should #ifdef XSM_ENABLE this new part in
> common/device_tree.c

Might as well, given you are resending. On the other hand then you maybe
might feel you wanted to ifdef the MOD_XSM and early_device_tree struct
stuff, which is probably getting carried away.

So you choose ;-)

Ian.