[PATCH RFC 00/15] xen: arm: 64-bit guest support and domU FDT autogeneration

[PATCH RFC 00/15] xen: arm: 64-bit guest support and domU FDT autogeneration

[Ian Campbell]

This isn't quite ready yet (see TODOs in the commit messages) but I just
took a high priority interrupt so I thought I'd send it out for initial
comments (it's good enough for that I think)

The actual 64-bit guest support is mostly pretty simple but there was
one huge yakk which needed shaving: arm64 Linux does not support
appended dtb so I had to implement autogeneration of guest dtb, which
was quite a bit of code. Still, it needed to be done anyway, so it's
useful...

I've not tried any of this with 32-bit guests yet.

Ian.

[PATCH RFC 01/15] xen: arm: Report aarch64 capability.

[Ian Campbell]

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/arch/arm/setup.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 49f344c..faf9d50 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -730,6 +730,10 @@ void arch_get_xen_caps(xen_capabilities_info_t *info)
 
     (*info)[0] = '\0';
 
+#ifdef CONFIG_ARM_64
+    snprintf(s, sizeof(s), "xen-%d.%d-aarch64 ", major, minor);
+    safe_strcat(*info, s);
+#endif
     snprintf(s, sizeof(s), "xen-%d.%d-armv7l ", major, minor);
     safe_strcat(*info, s);
 }
-- 
1.7.10.4

Re: [PATCH RFC 01/15] xen: arm: Report aarch64 capability.

[Julien Grall]

On 10/07/2013 05:39 PM, Ian Campbell wrote:
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Julien Grall <julien.grall@linaro.org>

> ---
>  xen/arch/arm/setup.c |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 49f344c..faf9d50 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -730,6 +730,10 @@ void arch_get_xen_caps(xen_capabilities_info_t *info)
>  
>      (*info)[0] = '\0';
>  
> +#ifdef CONFIG_ARM_64
> +    snprintf(s, sizeof(s), "xen-%d.%d-aarch64 ", major, minor);
> +    safe_strcat(*info, s);
> +#endif
>      snprintf(s, sizeof(s), "xen-%d.%d-armv7l ", major, minor);
>      safe_strcat(*info, s);
>  }
> 


-- 
Julien Grall

[PATCH RFC 02/15] xen: arm: Add comment regard arm64 zImage v0 vs v1

[Ian Campbell]

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/arch/arm/kernel.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index 6d2c164..8efd24a 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -132,6 +132,7 @@ static int kernel_try_zimage64_prepare(struct kernel_info *info,
         uint64_t text_offset;  /* Image load offset */
         uint64_t res1;
         uint64_t res2;
+        /* zImage V1 only from here */
         uint64_t res3;
         uint64_t res4;
         uint64_t res5;
-- 
1.7.10.4

Re: [PATCH RFC 02/15] xen: arm: Add comment regard arm64 zImage v0 vs v1

[Julien Grall]

On 10/07/2013 05:39 PM, Ian Campbell wrote:
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Julien Grall <julien.grall@linaro.org>

> ---
>  xen/arch/arm/kernel.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
> index 6d2c164..8efd24a 100644
> --- a/xen/arch/arm/kernel.c
> +++ b/xen/arch/arm/kernel.c
> @@ -132,6 +132,7 @@ static int kernel_try_zimage64_prepare(struct kernel_info *info,
>          uint64_t text_offset;  /* Image load offset */
>          uint64_t res1;
>          uint64_t res2;
> +        /* zImage V1 only from here */
>          uint64_t res3;
>          uint64_t res4;
>          uint64_t res5;
> 


-- 
Julien Grall

[PATCH RFC 03/15] xen: arm: allocate dom0 memory separately from preparing the dtb

[Ian Campbell]

Mixing these two together is a pain, it forces us to prepare the dtb before
processing the kernel which means we don't know whether the guest is 32- or
64-bit while we construct its DTB.

Instead split out the memory allocation (including 1:1 workaround handling)
and p2m setup into a seaprate phase and then fill in the memory nodes in the
DTB based on the result while generating the DTB.

This allows us to move kernel parsing before DTB setup.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/arch/arm/domain_build.c |   94 ++++++++++++++++++++++++++++++-------------
 1 file changed, 65 insertions(+), 29 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index fb1fa56..1287934 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -63,11 +63,8 @@ struct vcpu *__init alloc_dom0_vcpu0(void)
     return alloc_vcpu(dom0, 0, 0);
 }
 
-static int set_memory_reg_11(struct domain *d, struct kernel_info *kinfo,
-                             const struct dt_property *pp,
-                             const struct dt_device_node *np, __be32 *new_cell)
+static int allocate_memory_11(struct domain *d, struct kernel_info *kinfo)
 {
-    int reg_size = dt_cells_to_size(dt_n_addr_cells(np) + dt_n_size_cells(np));
     paddr_t start;
     paddr_t size;
     struct page_info *pg;
@@ -91,40 +88,53 @@ static int set_memory_reg_11(struct domain *d, struct kernel_info *kinfo,
     if ( res )
         panic("Unable to add pages in DOM0: %d\n", res);
 
-    dt_set_range(&new_cell, np, start, size);
-
     kinfo->mem.bank[0].start = start;
     kinfo->mem.bank[0].size = size;
     kinfo->mem.nr_banks = 1;
 
-    return reg_size;
+    kinfo->unassigned_mem -= size;
+
+    return 0;
 }
 
-static int set_memory_reg(struct domain *d, struct kernel_info *kinfo,
-                          const struct dt_property *pp,
-                          const struct dt_device_node *np, __be32 *new_cell)
+static int allocate_memory(struct domain *d, struct kernel_info *kinfo)
 {
-    int reg_size = dt_cells_to_size(dt_n_addr_cells(np) + dt_n_size_cells(np));
-    int l = 0;
+
+    const struct dt_device_node *memory;
+    const void *reg;
+    u32 reg_len, reg_size;
+    int l = 0, ret;
     unsigned int bank = 0;
-    u64 start;
-    u64 size;
-    int ret;
 
     if ( platform_has_quirk(PLATFORM_QUIRK_DOM0_MAPPING_11) )
-        return set_memory_reg_11(d, kinfo, pp, np, new_cell);
+        return allocate_memory_11(d, kinfo);
+
+    memory = dt_find_node_by_type(NULL, "memory");
+    if ( memory == NULL )
+    {
+        panic("No host memory node when building dom0!\n");
+    }
+
+    reg_size = dt_cells_to_size(dt_n_addr_cells(memory) + dt_n_size_cells(memory));
 
-    while ( kinfo->unassigned_mem > 0 && l + reg_size <= pp->length
+    reg = dt_get_property(memory, "reg", &reg_len);
+    if ( reg == NULL )
+    {
+        panic("Memory node has now reg property!\n");
+    }
+
+    while ( kinfo->unassigned_mem > 0 && l + reg_size <= reg_len
             && kinfo->mem.nr_banks < NR_MEM_BANKS )
     {
-        ret = dt_device_get_address(np, bank, &start, &size);
+        paddr_t start, size;
+
+        ret = dt_device_get_address(memory, bank, &start, &size);
         if ( ret )
             panic("Unable to retrieve the bank %u for %s\n",
-                  bank, dt_node_full_name(np));
+                  bank, dt_node_full_name(memory));
 
         if ( size > kinfo->unassigned_mem )
             size = kinfo->unassigned_mem;
-        dt_set_range(&new_cell, np, start, size);
 
         printk("Populate P2M %#"PRIx64"->%#"PRIx64"\n", start, start + size);
         if ( p2m_populate_ram(d, start, start + size) < 0 )
@@ -132,12 +142,34 @@ static int set_memory_reg(struct domain *d, struct kernel_info *kinfo,
         kinfo->mem.bank[kinfo->mem.nr_banks].start = start;
         kinfo->mem.bank[kinfo->mem.nr_banks].size = size;
         kinfo->mem.nr_banks++;
+
         kinfo->unassigned_mem -= size;
 
         l += reg_size;
     }
+    return 0;
+}
 
-    return l;
+static int set_memory_reg(struct domain *d, struct kernel_info *kinfo,
+                          const struct dt_device_node *np, __be32 *new_cell)
+{
+    int reg_size = dt_cells_to_size(dt_n_addr_cells(np) + dt_n_size_cells(np));
+    u64 start;
+    u64 size;
+    int i;
+
+    for ( i = 0 ; i < kinfo->mem.nr_banks; i++ )
+    {
+        start = kinfo->mem.bank[i].start;
+        size = kinfo->mem.bank[i].size;
+
+        DPRINT("Memory Bank %d: %#"PRIx64"->%#"PRIx64"\n",
+                i, start, start + size);
+
+        dt_set_range(&new_cell, np, start, size);
+    }
+
+    return kinfo->mem.nr_banks * reg_size;
 }
 
 static int write_properties(struct domain *d, struct kernel_info *kinfo,
@@ -194,7 +226,7 @@ static int write_properties(struct domain *d, struct kernel_info *kinfo,
                 if ( new_data  == NULL )
                     return -FDT_ERR_XEN(ENOMEM);
 
-                prop_len = set_memory_reg(d, kinfo, pp, np,
+                prop_len = set_memory_reg(d, kinfo, np,
                                           (__be32 *)new_data);
                 prop_data = new_data;
             }
@@ -768,8 +800,6 @@ static int prepare_dtb(struct domain *d, struct kernel_info *kinfo)
 
     ASSERT(dt_host && (dt_host->sibling == NULL));
 
-    kinfo->unassigned_mem = dom0_mem;
-
     fdt = device_tree_flattened;
 
     new_size = fdt_totalsize(fdt) + DOM0_FDT_EXTRA_SIZE;
@@ -858,7 +888,9 @@ int construct_dom0(struct domain *d)
 
     d->max_pages = ~0U;
 
-    rc = prepare_dtb(d, &kinfo);
+    kinfo.unassigned_mem = dom0_mem;
+
+    rc = allocate_memory(d, &kinfo);
     if ( rc < 0 )
         return rc;
 
@@ -866,6 +898,14 @@ int construct_dom0(struct domain *d)
     if ( rc < 0 )
         return rc;
 
+#ifdef CONFIG_ARM_64
+    d->arch.type = kinfo.type;
+#endif
+
+    rc = prepare_dtb(d, &kinfo);
+    if ( rc < 0 )
+        return rc;
+
     rc = platform_specific_mapping(d);
     if ( rc < 0 )
         return rc;
@@ -888,10 +928,6 @@ int construct_dom0(struct domain *d)
 
     regs->pc = (register_t)kinfo.entry;
 
-#ifdef CONFIG_ARM_64
-    d->arch.type = kinfo.type;
-#endif
-
     if ( is_pv32_domain(d) )
     {
         regs->cpsr = PSR_GUEST32_INIT;
-- 
1.7.10.4

Re: [PATCH RFC 03/15] xen: arm: allocate dom0 memory separately from preparing the dtb

[Julien Grall]

On 10/07/2013 05:39 PM, Ian Campbell wrote:
> Mixing these two together is a pain, it forces us to prepare the dtb before
> processing the kernel which means we don't know whether the guest is 32- or
> 64-bit while we construct its DTB.
> 
> Instead split out the memory allocation (including 1:1 workaround handling)
> and p2m setup into a seaprate phase and then fill in the memory nodes in the

separate

> DTB based on the result while generating the DTB.
> 
> This allows us to move kernel parsing before DTB setup.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
>  xen/arch/arm/domain_build.c |   94 ++++++++++++++++++++++++++++++-------------
>  1 file changed, 65 insertions(+), 29 deletions(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index fb1fa56..1287934 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -63,11 +63,8 @@ struct vcpu *__init alloc_dom0_vcpu0(void)
>      return alloc_vcpu(dom0, 0, 0);
>  }
>  
> -static int set_memory_reg_11(struct domain *d, struct kernel_info *kinfo,
> -                             const struct dt_property *pp,
> -                             const struct dt_device_node *np, __be32 *new_cell)
> +static int allocate_memory_11(struct domain *d, struct kernel_info *kinfo)

This function always return 0 or panic if an error occurred. Perhaps you
can move the return type to void?

>  {
> -    int reg_size = dt_cells_to_size(dt_n_addr_cells(np) + dt_n_size_cells(np));
>      paddr_t start;
>      paddr_t size;
>      struct page_info *pg;
> @@ -91,40 +88,53 @@ static int set_memory_reg_11(struct domain *d, struct kernel_info *kinfo,
>      if ( res )
>          panic("Unable to add pages in DOM0: %d\n", res);
>  
> -    dt_set_range(&new_cell, np, start, size);
> -
>      kinfo->mem.bank[0].start = start;
>      kinfo->mem.bank[0].size = size;
>      kinfo->mem.nr_banks = 1;
>  
> -    return reg_size;
> +    kinfo->unassigned_mem -= size;
> +
> +    return 0;
>  }
>  
> -static int set_memory_reg(struct domain *d, struct kernel_info *kinfo,
> -                          const struct dt_property *pp,
> -                          const struct dt_device_node *np, __be32 *new_cell)
> +static int allocate_memory(struct domain *d, struct kernel_info *kinfo)

Same here.

>  {
> -    int reg_size = dt_cells_to_size(dt_n_addr_cells(np) + dt_n_size_cells(np));
> -    int l = 0;
> +
> +    const struct dt_device_node *memory;
> +    const void *reg;
> +    u32 reg_len, reg_size;
> +    int l = 0, ret;
>      unsigned int bank = 0;
> -    u64 start;
> -    u64 size;
> -    int ret;
>  
>      if ( platform_has_quirk(PLATFORM_QUIRK_DOM0_MAPPING_11) )
> -        return set_memory_reg_11(d, kinfo, pp, np, new_cell);
> +        return allocate_memory_11(d, kinfo);
> +
> +    memory = dt_find_node_by_type(NULL, "memory");

Can we try to have the same way to retrieve the memory node in each place?

- common/device_tree.c: looking by memory@unit
- arch/arm/domain_build.c:write_properties: looking only the exact node
name "memory"
- here: looking by type "memory"

Furthermore, your are assuming that there is only one memory node in DTS
tree. Perhaps a loop is better here?

-- 
Julien Grall

Re: [PATCH RFC 03/15] xen: arm: allocate dom0 memory separately from preparing the dtb

[Ian Campbell]

On Thu, 2013-10-10 at 15:38 +0100, Julien Grall wrote:

> This function always return 0 or panic if an error occurred. Perhaps you
> can move the return type to void?

Yes, done all the way up the call chain.

> >      if ( platform_has_quirk(PLATFORM_QUIRK_DOM0_MAPPING_11) )
> > -        return set_memory_reg_11(d, kinfo, pp, np, new_cell);
> > +        return allocate_memory_11(d, kinfo);
> > +
> > +    memory = dt_find_node_by_type(NULL, "memory");
> 
> Can we try to have the same way to retrieve the memory node in each place?

Hrm. The first one (device_tree.c) is using the early stuff while the
other two are using the full datastructures.

But for the last two I think the right thing to do is to check for
device_type = "memory".

> - common/device_tree.c: looking by memory@unit
> - arch/arm/domain_build.c:write_properties: looking only the exact node
> name "memory"
> - here: looking by type "memory"
> 
> Furthermore, your are assuming that there is only one memory node in DTS
> tree. Perhaps a loop is better here?

Yes, I'll change that.

Ian.

[PATCH RFC 04/15] xen: arm: add enable-method to cpu nodes for arm64 guests.

[Ian Campbell]

This is required by the Linux arm64 boot protocol.

We use PSCI.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/arch/arm/domain_build.c |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 1287934..10dea5d 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -468,6 +468,13 @@ static int make_cpus_node(const struct domain *d, void *fdt,
         if ( res )
             return res;
 
+        if ( is_pv64_domain(d) )
+        {
+            res = fdt_property_string(fdt, "enable-method", "psci");
+            if ( res )
+                return res;
+        }
+
         res = fdt_end_node(fdt);
         if ( res )
             return res;
-- 
1.7.10.4

Re: [PATCH RFC 04/15] xen: arm: add enable-method to cpu nodes for arm64 guests.

[Julien Grall]

On 10/07/2013 05:39 PM, Ian Campbell wrote:
> This is required by the Linux arm64 boot protocol.
> 
> We use PSCI.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Julien Grall <julien.grall@linaro.org>

> ---
>  xen/arch/arm/domain_build.c |    7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 1287934..10dea5d 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -468,6 +468,13 @@ static int make_cpus_node(const struct domain *d, void *fdt,
>          if ( res )
>              return res;
>  
> +        if ( is_pv64_domain(d) )
> +        {
> +            res = fdt_property_string(fdt, "enable-method", "psci");
> +            if ( res )
> +                return res;
> +        }
> +
>          res = fdt_end_node(fdt);
>          if ( res )
>              return res;
> 


-- 
Julien Grall

[PATCH RFC 05/15] xen: arm: implement XEN_DOMCTL_set_address_size

[Ian Campbell]

This subarch specific to plumb through to arm32 and arm64 versions.

The toolstack uses this to select 32- vs 64-bit guests (or rather it does on
x86 an soon will for arm too).

I noticed that domctl.c and sysctl.c weren't including xen/hypercall.h to get
their arch_do_fooctl prototypes, so fix that.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/arch/arm/arm32/Makefile     |    1 +
 xen/arch/arm/arm32/domctl.c     |   35 +++++++++++++++++++++++
 xen/arch/arm/arm64/Makefile     |    1 +
 xen/arch/arm/arm64/domctl.c     |   59 +++++++++++++++++++++++++++++++++++++++
 xen/arch/arm/domctl.c           |    7 ++++-
 xen/arch/arm/sysctl.c           |    1 +
 xen/include/asm-arm/hypercall.h |    3 ++
 7 files changed, 106 insertions(+), 1 deletion(-)
 create mode 100644 xen/arch/arm/arm32/domctl.c
 create mode 100644 xen/arch/arm/arm64/domctl.c

diff --git a/xen/arch/arm/arm32/Makefile b/xen/arch/arm/arm32/Makefile
index aacdcb9..65ecff4 100644
--- a/xen/arch/arm/arm32/Makefile
+++ b/xen/arch/arm/arm32/Makefile
@@ -7,5 +7,6 @@ obj-y += traps.o
 obj-y += domain.o
 obj-y += vfp.o
 obj-y += smpboot.o
+obj-y += domctl.o
 
 obj-$(EARLY_PRINTK) += debug.o
diff --git a/xen/arch/arm/arm32/domctl.c b/xen/arch/arm/arm32/domctl.c
new file mode 100644
index 0000000..c2ca4d3
--- /dev/null
+++ b/xen/arch/arm/arm32/domctl.c
@@ -0,0 +1,35 @@
+/******************************************************************************
+ * Subarch-specific domctl.c
+ *
+ * Copyright (c) 2013, Citrix Systems
+ */
+
+#include <xen/config.h>
+#include <xen/types.h>
+#include <xen/lib.h>
+#include <xen/errno.h>
+#include <xen/sched.h>
+#include <xen/hypercall.h>
+#include <public/domctl.h>
+
+long subarch_do_domctl(struct xen_domctl *domctl, struct domain *d,
+               XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
+{
+    switch ( domctl->cmd )
+    {
+    case XEN_DOMCTL_set_address_size:
+        return domctl->u.address_size.size == 32 ? 0 : -EINVAL;
+    default:
+        return -ENOSYS;
+    }
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/arm/arm64/Makefile b/xen/arch/arm/arm64/Makefile
index 5d28bad..d2d5875 100644
--- a/xen/arch/arm/arm64/Makefile
+++ b/xen/arch/arm/arm64/Makefile
@@ -6,5 +6,6 @@ obj-y += traps.o
 obj-y += domain.o
 obj-y += vfp.o
 obj-y += smpboot.o
+obj-y += domctl.o
 
 obj-$(EARLY_PRINTK) += debug.o
diff --git a/xen/arch/arm/arm64/domctl.c b/xen/arch/arm/arm64/domctl.c
new file mode 100644
index 0000000..e2b4617
--- /dev/null
+++ b/xen/arch/arm/arm64/domctl.c
@@ -0,0 +1,59 @@
+/******************************************************************************
+ * Subarch-specific domctl.c
+ *
+ * Copyright (c) 2013, Citrix Systems
+ */
+
+#include <xen/config.h>
+#include <xen/types.h>
+#include <xen/lib.h>
+#include <xen/errno.h>
+#include <xen/sched.h>
+#include <xen/hypercall.h>
+#include <public/domctl.h>
+
+static long switch_mode(struct domain *d, enum domain_type type)
+{
+    if ( d == NULL )
+        return -EINVAL;
+    if ( d->tot_pages != 0 )
+        return -EBUSY;
+    if ( d->arch.type == type )
+        return 0;
+
+    d->arch.type = type;
+
+    return 0;
+}
+
+long subarch_do_domctl(struct xen_domctl *domctl, struct domain *d,
+                       XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
+{
+    switch ( domctl->cmd )
+    {
+    case XEN_DOMCTL_set_address_size:
+        switch ( domctl->u.address_size.size )
+        {
+        case 32:
+            return switch_mode(d, DOMAIN_PV32);
+        case 64:
+            return switch_mode(d, DOMAIN_PV64);
+        default:
+            return -EINVAL;
+        }
+        break;
+
+    default:
+        return -ENOSYS;
+    }
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/arm/domctl.c b/xen/arch/arm/domctl.c
index 851ee40..546e86b 100644
--- a/xen/arch/arm/domctl.c
+++ b/xen/arch/arm/domctl.c
@@ -9,12 +9,17 @@
 #include <xen/lib.h>
 #include <xen/errno.h>
 #include <xen/sched.h>
+#include <xen/hypercall.h>
 #include <public/domctl.h>
 
 long arch_do_domctl(struct xen_domctl *domctl, struct domain *d,
                     XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
 {
-    return -ENOSYS;
+    switch ( domctl->cmd )
+    {
+    default:
+        return subarch_do_domctl(domctl, d, u_domctl);
+    }
 }
 
 void arch_get_info_guest(struct vcpu *v, vcpu_guest_context_u c)
diff --git a/xen/arch/arm/sysctl.c b/xen/arch/arm/sysctl.c
index 6388204..98bab6a 100644
--- a/xen/arch/arm/sysctl.c
+++ b/xen/arch/arm/sysctl.c
@@ -10,6 +10,7 @@
 #include <xen/types.h>
 #include <xen/lib.h>
 #include <xen/errno.h>
+#include <xen/hypercall.h>
 #include <public/sysctl.h>
 
 void arch_do_physinfo(xen_sysctl_physinfo_t *pi) { }
diff --git a/xen/include/asm-arm/hypercall.h b/xen/include/asm-arm/hypercall.h
index 3327a96..94a92d4 100644
--- a/xen/include/asm-arm/hypercall.h
+++ b/xen/include/asm-arm/hypercall.h
@@ -6,6 +6,9 @@ int do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg);
 
 long do_arm_vcpu_op(int cmd, int vcpuid, XEN_GUEST_HANDLE_PARAM(void) arg);
 
+long subarch_do_domctl(struct xen_domctl *domctl, struct domain *d,
+                       XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl);
+
 #endif /* __ASM_ARM_HYPERCALL_H__ */
 /*
  * Local variables:
-- 
1.7.10.4

[PATCH RFC 06/15] xen: arm: implement arch_set_info_guest for 64-bit vcpus

[Ian Campbell]

This all seems too easy...

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/arch/arm/domain.c |   66 ++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 52 insertions(+), 14 deletions(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index cb0424d..907d5c0 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -550,7 +550,7 @@ void arch_domain_destroy(struct domain *d)
     free_xenheap_page(d->shared_info);
 }
 
-static int is_guest_psr(uint32_t psr)
+static int is_guest_pv32_psr(uint32_t psr)
 {
     switch (psr & PSR_MODE_MASK)
     {
@@ -569,6 +569,29 @@ static int is_guest_psr(uint32_t psr)
     }
 }
 
+
+#ifdef CONFIG_ARM_64
+static int is_guest_pv64_psr(uint32_t psr)
+{
+    if ( psr & PSR_MODE_BIT )
+        return 0;
+
+    switch (psr & PSR_MODE_MASK)
+    {
+    case PSR_MODE_EL1h:
+    case PSR_MODE_EL1t:
+    case PSR_MODE_EL0t:
+        return 1;
+    case PSR_MODE_EL3h:
+    case PSR_MODE_EL3t:
+    case PSR_MODE_EL2h:
+    case PSR_MODE_EL2t:
+    default:
+        return 0;
+    }
+}
+#endif
+
 /*
  * Initialise VCPU state. The context can be supplied by either the
  * toolstack (XEN_DOMCTL_setvcpucontext) or the guest
@@ -577,22 +600,37 @@ static int is_guest_psr(uint32_t psr)
 int arch_set_info_guest(
     struct vcpu *v, vcpu_guest_context_u c)
 {
+    struct domain *d = v->domain;
     struct vcpu_guest_context *ctxt = c.nat;
     struct vcpu_guest_core_regs *regs = &c.nat->user_regs;
 
-    if ( !is_guest_psr(regs->cpsr) )
-        return -EINVAL;
-
-    if ( regs->spsr_svc && !is_guest_psr(regs->spsr_svc) )
-        return -EINVAL;
-    if ( regs->spsr_abt && !is_guest_psr(regs->spsr_abt) )
-        return -EINVAL;
-    if ( regs->spsr_und && !is_guest_psr(regs->spsr_und) )
-        return -EINVAL;
-    if ( regs->spsr_irq && !is_guest_psr(regs->spsr_irq) )
-        return -EINVAL;
-    if ( regs->spsr_fiq && !is_guest_psr(regs->spsr_fiq) )
-        return -EINVAL;
+
+    if ( is_pv32_domain(d) )
+    {
+        if ( !is_guest_pv32_psr(regs->cpsr) )
+            return -EINVAL;
+
+        if ( regs->spsr_svc && !is_guest_pv32_psr(regs->spsr_svc) )
+            return -EINVAL;
+        if ( regs->spsr_abt && !is_guest_pv32_psr(regs->spsr_abt) )
+            return -EINVAL;
+        if ( regs->spsr_und && !is_guest_pv32_psr(regs->spsr_und) )
+            return -EINVAL;
+        if ( regs->spsr_irq && !is_guest_pv32_psr(regs->spsr_irq) )
+            return -EINVAL;
+        if ( regs->spsr_fiq && !is_guest_pv32_psr(regs->spsr_fiq) )
+            return -EINVAL;
+    }
+#ifdef CONFIG_ARM_64
+    else
+    {
+        if ( !is_guest_pv64_psr(regs->cpsr) )
+            return -EINVAL;
+
+        if ( regs->spsr_el1 && !is_guest_pv64_psr(regs->spsr_el1) )
+            return -EINVAL;
+    }
+#endif
 
     vcpu_regs_user_to_hyp(v, regs);
 
-- 
1.7.10.4

[PATCH RFC 07/15] xenctx: fix typo in arm64 output

[Ian Campbell]

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 tools/xentrace/xenctx.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/xentrace/xenctx.c b/tools/xentrace/xenctx.c
index 1214185..4b774af 100644
--- a/tools/xentrace/xenctx.c
+++ b/tools/xentrace/xenctx.c
@@ -497,7 +497,7 @@ static void print_ctx_64(vcpu_guest_context_t *ctx)
     print_symbol(regs->pc64);
     printf("\n");
 
-    printf("LR:       %016"PRIx64"zn", regs->x30);
+    printf("LR:       %016"PRIx64"\n", regs->x30);
     printf("ELR_EL1:  %016"PRIx64"\n", regs->elr_el1);
 
     printf("CPSR:     %08"PRIx32"\n", regs->cpsr);
-- 
1.7.10.4

Re: [PATCH RFC 07/15] xenctx: fix typo in arm64 output

[Julien Grall]

On 10/07/2013 05:39 PM, Ian Campbell wrote:
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Julien Grall <julien.grall@linaro.org>
> ---
>  tools/xentrace/xenctx.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/xentrace/xenctx.c b/tools/xentrace/xenctx.c
> index 1214185..4b774af 100644
> --- a/tools/xentrace/xenctx.c
> +++ b/tools/xentrace/xenctx.c
> @@ -497,7 +497,7 @@ static void print_ctx_64(vcpu_guest_context_t *ctx)
>      print_symbol(regs->pc64);
>      printf("\n");
>  
> -    printf("LR:       %016"PRIx64"zn", regs->x30);
> +    printf("LR:       %016"PRIx64"\n", regs->x30);
>      printf("ELR_EL1:  %016"PRIx64"\n", regs->elr_el1);
>  
>      printf("CPSR:     %08"PRIx32"\n", regs->cpsr);
> 


-- 
Julien Grall

Re: [PATCH RFC 07/15] xenctx: fix typo in arm64 output

[Ian Campbell]

On Thu, 2013-10-10 at 15:43 +0100, Julien Grall wrote:
> On 10/07/2013 05:39 PM, Ian Campbell wrote:
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Acked-by: Julien Grall <julien.grall@linaro.org>

Thanks. I pushed this one since it isn't really much to do with the rest
of the series.

[PATCH RFC 08/15] tools: check for libfdt when building for ARM

[Ian Campbell]

libxl is going to want this to aid in the creation of guest device tree blobs.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 tools/config.h.in  |    3 +++
 tools/configure    |   52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 tools/configure.ac |    6 ++++++
 3 files changed, 61 insertions(+)

diff --git a/tools/config.h.in b/tools/config.h.in
index b1c9531..015f2a1 100644
--- a/tools/config.h.in
+++ b/tools/config.h.in
@@ -9,6 +9,9 @@
 /* Define to 1 if you have the `crypto' library (-lcrypto). */
 #undef HAVE_LIBCRYPTO
 
+/* Define to 1 if you have the `fdt' library (-lfdt). */
+#undef HAVE_LIBFDT
+
 /* Define to 1 if you have the `yajl' library (-lyajl). */
 #undef HAVE_LIBYAJL
 
diff --git a/tools/configure b/tools/configure
index 1da8652..2503990 100755
--- a/tools/configure
+++ b/tools/configure
@@ -7941,6 +7941,58 @@ fi
 
 
 
+# FDT is needed only on ARM
+case "$host_cpu" in
+arm*|aarch64)
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for fdt_create in -lfdt" >&5
+$as_echo_n "checking for fdt_create in -lfdt... " >&6; }
+if ${ac_cv_lib_fdt_fdt_create+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  ac_check_lib_save_LIBS=$LIBS
+LIBS="-lfdt  $LIBS"
+cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+
+/* Override any GCC internal prototype to avoid an error.
+   Use char because int might match the return type of a GCC
+   builtin and then its argument prototype would still apply.  */
+#ifdef __cplusplus
+extern "C"
+#endif
+char fdt_create ();
+int
+main ()
+{
+return fdt_create ();
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_c_try_link "$LINENO"; then :
+  ac_cv_lib_fdt_fdt_create=yes
+else
+  ac_cv_lib_fdt_fdt_create=no
+fi
+rm -f core conftest.err conftest.$ac_objext \
+    conftest$ac_exeext conftest.$ac_ext
+LIBS=$ac_check_lib_save_LIBS
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_lib_fdt_fdt_create" >&5
+$as_echo "$ac_cv_lib_fdt_fdt_create" >&6; }
+if test "x$ac_cv_lib_fdt_fdt_create" = xyes; then :
+  cat >>confdefs.h <<_ACEOF
+#define HAVE_LIBFDT 1
+_ACEOF
+
+  LIBS="-lfdt $LIBS"
+
+else
+  as_fn_error $? "Could not find libfdt" "$LINENO" 5
+fi
+
+esac
+
 # Checks for header files.
 for ac_header in yajl/yajl_version.h sys/eventfd.h
 do :
diff --git a/tools/configure.ac b/tools/configure.ac
index 4f3c33a..c366efb 100644
--- a/tools/configure.ac
+++ b/tools/configure.ac
@@ -216,6 +216,12 @@ AC_CHECK_LIB([z], [deflateCopy], [], [AC_MSG_ERROR([Could not find zlib])])
 AC_CHECK_LIB([iconv], [libiconv_open], [libiconv="y"], [libiconv="n"])
 AC_SUBST(libiconv)
 
+# FDT is needed only on ARM
+case "$host_cpu" in
+arm*|aarch64)
+AC_CHECK_LIB([fdt], [fdt_create], [], [AC_MSG_ERROR([Could not find libfdt])])
+esac
+
 # Checks for header files.
 AC_CHECK_HEADERS([yajl/yajl_version.h sys/eventfd.h])
 
-- 
1.7.10.4

[PATCH RFC 09/15] libxc: arm: rename various bits of zimage load with 32 suffix

[Ian Campbell]

Making room for a 64 bit implementation.

Also fix a typo and stop refering to it as a bzImage, which is an x86-ism.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 tools/libxc/xc_dom_armzimageloader.c |   44 ++++++++++++++++++++--------------
 1 file changed, 26 insertions(+), 18 deletions(-)

diff --git a/tools/libxc/xc_dom_armzimageloader.c b/tools/libxc/xc_dom_armzimageloader.c
index 54728b8..b693390 100644
--- a/tools/libxc/xc_dom_armzimageloader.c
+++ b/tools/libxc/xc_dom_armzimageloader.c
@@ -36,12 +36,6 @@
  */
 #define GUEST_RAM_BASE 0x80000000
 
-#define ZIMAGE_MAGIC_OFFSET 0x24
-#define ZIMAGE_START_OFFSET 0x28
-#define ZIMAGE_END_OFFSET   0x2c
-
-#define ZIMAGE_MAGIC 0x016f2818
-
 struct minimal_dtb_header {
     uint32_t magic;
     uint32_t total_size;
@@ -50,7 +44,17 @@ struct minimal_dtb_header {
 
 #define DTB_MAGIC 0xd00dfeed
 
-static int xc_dom_probe_zimage_kernel(struct xc_dom_image *dom)
+/* ------------------------------------------------------------ */
+/* 32-bit zImage Support                                        */
+/* ------------------------------------------------------------ */
+
+#define ZIMAGE32_MAGIC_OFFSET 0x24
+#define ZIMAGE32_START_OFFSET 0x28
+#define ZIMAGE32_END_OFFSET   0x2c
+
+#define ZIMAGE32_MAGIC 0x016f2818
+
+static int xc_dom_probe_zimage32_kernel(struct xc_dom_image *dom)
 {
     uint32_t *zimage;
     uint32_t end;
@@ -69,13 +73,13 @@ static int xc_dom_probe_zimage_kernel(struct xc_dom_image *dom)
     }
 
     zimage = (uint32_t *)dom->kernel_blob;
-    if ( zimage[ZIMAGE_MAGIC_OFFSET/4] != ZIMAGE_MAGIC )
+    if ( zimage[ZIMAGE32_MAGIC_OFFSET/4] != ZIMAGE32_MAGIC )
     {
-        xc_dom_printf(dom->xch, "%s: kernel is not a bzImage", __FUNCTION__);
+        xc_dom_printf(dom->xch, "%s: kernel is not an arm32 zImage", __FUNCTION__);
         return -EINVAL;
     }
 
-    end = zimage[ZIMAGE_END_OFFSET/4];
+    end = zimage[ZIMAGE32_END_OFFSET/4];
 
     /*
      * Check for an appended DTB.
@@ -94,7 +98,7 @@ static int xc_dom_probe_zimage_kernel(struct xc_dom_image *dom)
     return 0;
 }
 
-static int xc_dom_parse_zimage_kernel(struct xc_dom_image *dom)
+static int xc_dom_parse_zimage32_kernel(struct xc_dom_image *dom)
 {
     uint32_t *zimage;
     uint32_t start, entry_addr;
@@ -111,7 +115,7 @@ static int xc_dom_parse_zimage_kernel(struct xc_dom_image *dom)
     v_start = rambase + 0x8000;
     v_end = v_start + dom->kernel_size;
 
-    start = zimage[ZIMAGE_START_OFFSET/4];
+    start = zimage[ZIMAGE32_START_OFFSET/4];
 
     if (start == 0)
         entry_addr = v_start;
@@ -134,6 +138,10 @@ static int xc_dom_parse_zimage_kernel(struct xc_dom_image *dom)
     return 0;
 }
 
+/* ------------------------------------------------------------ */
+/* Common zImage Support                                        */
+/* ------------------------------------------------------------ */
+
 static int xc_dom_load_zimage_kernel(struct xc_dom_image *dom)
 {
     void *dst;
@@ -148,7 +156,7 @@ static int xc_dom_load_zimage_kernel(struct xc_dom_image *dom)
         return -1;
     }
 
-    DOMPRINTF("%s: kernel sed %#"PRIx64"-%#"PRIx64,
+    DOMPRINTF("%s: kernel seg %#"PRIx64"-%#"PRIx64,
               __func__, dom->kernel_seg.vstart, dom->kernel_seg.vend);
     DOMPRINTF("%s: copy %zd bytes from blob %p to dst %p",
               __func__, dom->kernel_size, dom->kernel_blob, dst);
@@ -158,16 +166,16 @@ static int xc_dom_load_zimage_kernel(struct xc_dom_image *dom)
     return 0;
 }
 
-static struct xc_dom_loader zimage_loader = {
-    .name = "Linux zImage (ARM)",
-    .probe = xc_dom_probe_zimage_kernel,
-    .parser = xc_dom_parse_zimage_kernel,
+static struct xc_dom_loader zimage32_loader = {
+    .name = "Linux zImage (ARM32)",
+    .probe = xc_dom_probe_zimage32_kernel,
+    .parser = xc_dom_parse_zimage32_kernel,
     .loader = xc_dom_load_zimage_kernel,
 };
 
 static void __init register_loader(void)
 {
-    xc_dom_register_loader(&zimage_loader);
+    xc_dom_register_loader(&zimage32_loader);
 }
 
 /*
-- 
1.7.10.4

Re: [PATCH RFC 09/15] libxc: arm: rename various bits of zimage load with 32 suffix

[Julien Grall]

On 10/07/2013 05:39 PM, Ian Campbell wrote:
> Making room for a 64 bit implementation.
> 
> Also fix a typo and stop refering to it as a bzImage, which is an x86-ism.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Julien Grall <julien.grall@linaro.org>

> ---
>  tools/libxc/xc_dom_armzimageloader.c |   44 ++++++++++++++++++++--------------
>  1 file changed, 26 insertions(+), 18 deletions(-)
> 
> diff --git a/tools/libxc/xc_dom_armzimageloader.c b/tools/libxc/xc_dom_armzimageloader.c
> index 54728b8..b693390 100644
> --- a/tools/libxc/xc_dom_armzimageloader.c
> +++ b/tools/libxc/xc_dom_armzimageloader.c
> @@ -36,12 +36,6 @@
>   */
>  #define GUEST_RAM_BASE 0x80000000
>  
> -#define ZIMAGE_MAGIC_OFFSET 0x24
> -#define ZIMAGE_START_OFFSET 0x28
> -#define ZIMAGE_END_OFFSET   0x2c
> -
> -#define ZIMAGE_MAGIC 0x016f2818
> -
>  struct minimal_dtb_header {
>      uint32_t magic;
>      uint32_t total_size;
> @@ -50,7 +44,17 @@ struct minimal_dtb_header {
>  
>  #define DTB_MAGIC 0xd00dfeed
>  
> -static int xc_dom_probe_zimage_kernel(struct xc_dom_image *dom)
> +/* ------------------------------------------------------------ */
> +/* 32-bit zImage Support                                        */
> +/* ------------------------------------------------------------ */
> +
> +#define ZIMAGE32_MAGIC_OFFSET 0x24
> +#define ZIMAGE32_START_OFFSET 0x28
> +#define ZIMAGE32_END_OFFSET   0x2c
> +
> +#define ZIMAGE32_MAGIC 0x016f2818
> +
> +static int xc_dom_probe_zimage32_kernel(struct xc_dom_image *dom)
>  {
>      uint32_t *zimage;
>      uint32_t end;
> @@ -69,13 +73,13 @@ static int xc_dom_probe_zimage_kernel(struct xc_dom_image *dom)
>      }
>  
>      zimage = (uint32_t *)dom->kernel_blob;
> -    if ( zimage[ZIMAGE_MAGIC_OFFSET/4] != ZIMAGE_MAGIC )
> +    if ( zimage[ZIMAGE32_MAGIC_OFFSET/4] != ZIMAGE32_MAGIC )
>      {
> -        xc_dom_printf(dom->xch, "%s: kernel is not a bzImage", __FUNCTION__);
> +        xc_dom_printf(dom->xch, "%s: kernel is not an arm32 zImage", __FUNCTION__);
>          return -EINVAL;
>      }
>  
> -    end = zimage[ZIMAGE_END_OFFSET/4];
> +    end = zimage[ZIMAGE32_END_OFFSET/4];
>  
>      /*
>       * Check for an appended DTB.
> @@ -94,7 +98,7 @@ static int xc_dom_probe_zimage_kernel(struct xc_dom_image *dom)
>      return 0;
>  }
>  
> -static int xc_dom_parse_zimage_kernel(struct xc_dom_image *dom)
> +static int xc_dom_parse_zimage32_kernel(struct xc_dom_image *dom)
>  {
>      uint32_t *zimage;
>      uint32_t start, entry_addr;
> @@ -111,7 +115,7 @@ static int xc_dom_parse_zimage_kernel(struct xc_dom_image *dom)
>      v_start = rambase + 0x8000;
>      v_end = v_start + dom->kernel_size;
>  
> -    start = zimage[ZIMAGE_START_OFFSET/4];
> +    start = zimage[ZIMAGE32_START_OFFSET/4];
>  
>      if (start == 0)
>          entry_addr = v_start;
> @@ -134,6 +138,10 @@ static int xc_dom_parse_zimage_kernel(struct xc_dom_image *dom)
>      return 0;
>  }
>  
> +/* ------------------------------------------------------------ */
> +/* Common zImage Support                                        */
> +/* ------------------------------------------------------------ */
> +
>  static int xc_dom_load_zimage_kernel(struct xc_dom_image *dom)
>  {
>      void *dst;
> @@ -148,7 +156,7 @@ static int xc_dom_load_zimage_kernel(struct xc_dom_image *dom)
>          return -1;
>      }
>  
> -    DOMPRINTF("%s: kernel sed %#"PRIx64"-%#"PRIx64,
> +    DOMPRINTF("%s: kernel seg %#"PRIx64"-%#"PRIx64,
>                __func__, dom->kernel_seg.vstart, dom->kernel_seg.vend);
>      DOMPRINTF("%s: copy %zd bytes from blob %p to dst %p",
>                __func__, dom->kernel_size, dom->kernel_blob, dst);
> @@ -158,16 +166,16 @@ static int xc_dom_load_zimage_kernel(struct xc_dom_image *dom)
>      return 0;
>  }
>  
> -static struct xc_dom_loader zimage_loader = {
> -    .name = "Linux zImage (ARM)",
> -    .probe = xc_dom_probe_zimage_kernel,
> -    .parser = xc_dom_parse_zimage_kernel,
> +static struct xc_dom_loader zimage32_loader = {
> +    .name = "Linux zImage (ARM32)",
> +    .probe = xc_dom_probe_zimage32_kernel,
> +    .parser = xc_dom_parse_zimage32_kernel,
>      .loader = xc_dom_load_zimage_kernel,
>  };
>  
>  static void __init register_loader(void)
>  {
> -    xc_dom_register_loader(&zimage_loader);
> +    xc_dom_register_loader(&zimage32_loader);
>  }
>  
>  /*
> 


-- 
Julien Grall

[PATCH RFC 10/15] libxc: allow caller to specify guest rambase rather than hardcoding

[Ian Campbell]

It's still hardcoded but it could now be plausibly be made variable.

TODO: the #if is a bit lame.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 tools/libxc/xc_dom.h                 |    1 +
 tools/libxc/xc_dom_armzimageloader.c |   12 +-----------
 tools/libxc/xc_dom_core.c            |    8 ++++++++
 tools/libxl/libxl_dom.c              |   10 ++++++++++
 4 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/tools/libxc/xc_dom.h b/tools/libxc/xc_dom.h
index 86e23ee..ed49aa7 100644
--- a/tools/libxc/xc_dom.h
+++ b/tools/libxc/xc_dom.h
@@ -197,6 +197,7 @@ struct xc_dom_image *xc_dom_allocate(xc_interface *xch,
                                      const char *cmdline, const char *features);
 void xc_dom_release_phys(struct xc_dom_image *dom);
 void xc_dom_release(struct xc_dom_image *dom);
+int xc_dom_rambase_init(struct xc_dom_image *dom, uint64_t rambase);
 int xc_dom_mem_init(struct xc_dom_image *dom, unsigned int mem_mb);
 
 /* Set this larger if you have enormous ramdisks/kernels. Note that
diff --git a/tools/libxc/xc_dom_armzimageloader.c b/tools/libxc/xc_dom_armzimageloader.c
index b693390..4e3f7ae 100644
--- a/tools/libxc/xc_dom_armzimageloader.c
+++ b/tools/libxc/xc_dom_armzimageloader.c
@@ -30,12 +30,6 @@
 
 #include <arpa/inet.h> /* XXX ntohl is not the right function... */
 
-/*
- * Guest virtual RAM starts here. This must be consistent with the DTB
- * appended to the guest kernel.
- */
-#define GUEST_RAM_BASE 0x80000000
-
 struct minimal_dtb_header {
     uint32_t magic;
     uint32_t total_size;
@@ -103,14 +97,12 @@ static int xc_dom_parse_zimage32_kernel(struct xc_dom_image *dom)
     uint32_t *zimage;
     uint32_t start, entry_addr;
     uint64_t v_start, v_end;
-    uint64_t rambase = GUEST_RAM_BASE;
+    uint64_t rambase = dom->rambase_pfn << XC_PAGE_SHIFT;
 
     DOMPRINTF_CALLED(dom->xch);
 
     zimage = (uint32_t *)dom->kernel_blob;
 
-    dom->rambase_pfn = rambase >> XC_PAGE_SHIFT;
-
     /* Do not load kernel at the very first RAM address */
     v_start = rambase + 0x8000;
     v_end = v_start + dom->kernel_size;
@@ -130,8 +122,6 @@ static int xc_dom_parse_zimage32_kernel(struct xc_dom_image *dom)
     dom->parms.virt_base = rambase;
 
     dom->guest_type = "xen-3.0-armv7l";
-    DOMPRINTF("%s: %s: RAM starts at %"PRI_xen_pfn,
-              __FUNCTION__, dom->guest_type, dom->rambase_pfn);
     DOMPRINTF("%s: %s: 0x%" PRIx64 " -> 0x%" PRIx64 "",
               __FUNCTION__, dom->guest_type,
               dom->kernel_seg.vstart, dom->kernel_seg.vend);
diff --git a/tools/libxc/xc_dom_core.c b/tools/libxc/xc_dom_core.c
index 0f367f6..a67f718 100644
--- a/tools/libxc/xc_dom_core.c
+++ b/tools/libxc/xc_dom_core.c
@@ -785,6 +785,14 @@ int xc_dom_parse_image(struct xc_dom_image *dom)
     return -1;
 }
 
+int xc_dom_rambase_init(struct xc_dom_image *dom, uint64_t rambase)
+{
+    dom->rambase_pfn = rambase >> XC_PAGE_SHIFT;
+    DOMPRINTF("%s: RAM starts at %"PRI_xen_pfn,
+              __FUNCTION__, dom->rambase_pfn);
+    return 0;
+}
+
 int xc_dom_mem_init(struct xc_dom_image *dom, unsigned int mem_mb)
 {
     unsigned int page_shift;
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 6e2252a..631cdf5 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -24,6 +24,10 @@
 #include <xen/hvm/hvm_info_table.h>
 #include <xen/hvm/hvm_xs_strings.h>
 
+#if defined(__arm__) || defined(__aarch64__)
+#define GUEST_RAM_BASE 0x80000000
+#endif
+
 libxl_domain_type libxl__domain_type(libxl__gc *gc, uint32_t domid)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
@@ -380,6 +384,12 @@ int libxl__build_pv(libxl__gc *gc, uint32_t domid,
         LOGE(ERROR, "xc_dom_boot_xen_init failed");
         goto out;
     }
+#ifdef GUEST_RAM_BASE
+    if ( (ret = xc_dom_rambase_init(dom, GUEST_RAM_BASE)) != 0 ) {
+        LOGE(ERROR, "xc_dom_rambase failed");
+        goto out;
+    }
+#endif
     if ( (ret = xc_dom_parse_image(dom)) != 0 ) {
         LOGE(ERROR, "xc_dom_parse_image failed");
         goto out;
-- 
1.7.10.4

Re: [PATCH RFC 10/15] libxc: allow caller to specify guest rambase rather than hardcoding

[Julien Grall]

On 10/07/2013 05:39 PM, Ian Campbell wrote:
> It's still hardcoded but it could now be plausibly be made variable.

There is a thread were people asking to change the base address.
For the xl side, how about a parameter in the configuration file?
The default value would be 0.

> 
> TODO: the #if is a bit lame.

-- 
Julien Grall

Re: [PATCH RFC 10/15] libxc: allow caller to specify guest rambase rather than hardcoding

[Ian Campbell]

On Thu, 2013-10-10 at 16:31 +0100, Julien Grall wrote:
> On 10/07/2013 05:39 PM, Ian Campbell wrote:
> > It's still hardcoded but it could now be plausibly be made variable.
> 
> There is a thread were people asking to change the base address.
> For the xl side, how about a parameter in the configuration file?
> The default value would be 0.

For future work, sure.

At the moment I don't want to start shaving the yak of plumbing ARM
specific parameters through libxl to xl.

I've been thinking of making the hardcoded default be 0 in any case.

Ian.

[PATCH RFC 11/15] libxc: allow passing a device tree blob to the guest

[Ian Campbell]

TODO: placement is rather simplistic

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 tools/libxc/xc_dom.h      |    8 ++++++++
 tools/libxc/xc_dom_arm.c  |   15 +++++++++++++-
 tools/libxc/xc_dom_core.c |   48 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 70 insertions(+), 1 deletion(-)

diff --git a/tools/libxc/xc_dom.h b/tools/libxc/xc_dom.h
index ed49aa7..bf56436 100644
--- a/tools/libxc/xc_dom.h
+++ b/tools/libxc/xc_dom.h
@@ -54,9 +54,12 @@ struct xc_dom_image {
     size_t kernel_size;
     void *ramdisk_blob;
     size_t ramdisk_size;
+    void *devicetree_blob;
+    size_t devicetree_size;
 
     size_t max_kernel_size;
     size_t max_ramdisk_size;
+    size_t max_devicetree_size;
 
     /* arguments and parameters */
     char *cmdline;
@@ -217,6 +220,8 @@ int xc_dom_kernel_max_size(struct xc_dom_image *dom, size_t sz);
 int xc_dom_ramdisk_check_size(struct xc_dom_image *dom, size_t sz);
 int xc_dom_ramdisk_max_size(struct xc_dom_image *dom, size_t sz);
 
+int xc_dom_devicetree_max_size(struct xc_dom_image *dom, size_t sz);
+
 size_t xc_dom_check_gzip(xc_interface *xch,
                      void *blob, size_t ziplen);
 int xc_dom_do_gunzip(xc_interface *xch,
@@ -229,6 +234,9 @@ int xc_dom_kernel_mem(struct xc_dom_image *dom, const void *mem,
                       size_t memsize);
 int xc_dom_ramdisk_mem(struct xc_dom_image *dom, const void *mem,
                        size_t memsize);
+int xc_dom_devicetree_file(struct xc_dom_image *dom, const char *filename);
+int xc_dom_devicetree_mem(struct xc_dom_image *dom, const void *mem,
+                          size_t memsize);
 
 int xc_dom_parse_image(struct xc_dom_image *dom);
 struct xc_dom_arch *xc_dom_find_arch_hooks(xc_interface *xch, char *guest_type);
diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c
index df59ffb..ecdaea9 100644
--- a/tools/libxc/xc_dom_arm.c
+++ b/tools/libxc/xc_dom_arm.c
@@ -124,7 +124,8 @@ static int vcpu_arm(struct xc_dom_image *dom, void *ptr)
      * using CONFIG_ARM_APPENDED_DTB. Ensure that r2 does not look
      * like a valid pointer to a set of ATAGS or a DTB.
      */
-    ctxt->user_regs.r2_usr = 0xffffffff;
+    ctxt->user_regs.r2_usr = dom->devicetree_blob ?
+        dom->devicetree_seg.vstart : 0xffffffff;
 
     ctxt->sctlr = /* #define SCTLR_BASE */0x00c50078;
 
@@ -191,6 +192,18 @@ int arch_setup_meminit(struct xc_dom_image *dom)
             0, 0, &dom->p2m_host[i]);
     }
 
+    if ( dom->devicetree_blob )
+    {
+        uint64_t rambase = dom->rambase_pfn << XC_PAGE_SHIFT;
+        /* XXX intelligent placement */
+        dom->devicetree_seg.vstart = rambase + 127*1024*1024;
+        dom->devicetree_seg.vend =
+            dom->devicetree_seg.vstart + dom->devicetree_size;
+        DOMPRINTF("%s: devicetree: 0x%" PRIx64 " -> 0x%" PRIx64 "",
+                  __FUNCTION__,
+                  dom->devicetree_seg.vstart, dom->devicetree_seg.vend);
+    }
+
     return 0;
 }
 
diff --git a/tools/libxc/xc_dom_core.c b/tools/libxc/xc_dom_core.c
index a67f718..6854160 100644
--- a/tools/libxc/xc_dom_core.c
+++ b/tools/libxc/xc_dom_core.c
@@ -671,6 +671,7 @@ struct xc_dom_image *xc_dom_allocate(xc_interface *xch,
 
     dom->max_kernel_size = XC_DOM_DECOMPRESS_MAX;
     dom->max_ramdisk_size = XC_DOM_DECOMPRESS_MAX;
+    dom->max_devicetree_size = XC_DOM_DECOMPRESS_MAX;
 
     if ( cmdline )
         dom->cmdline = xc_dom_strdup(dom, cmdline);
@@ -706,6 +707,13 @@ int xc_dom_ramdisk_max_size(struct xc_dom_image *dom, size_t sz)
     return 0;
 }
 
+int xc_dom_devicetree_max_size(struct xc_dom_image *dom, size_t sz)
+{
+    DOMPRINTF("%s: devicetree_max_size=%zx", __FUNCTION__, sz);
+    dom->max_devicetree_size = sz;
+    return 0;
+}
+
 int xc_dom_kernel_file(struct xc_dom_image *dom, const char *filename)
 {
     DOMPRINTF("%s: filename=\"%s\"", __FUNCTION__, filename);
@@ -729,6 +737,18 @@ int xc_dom_ramdisk_file(struct xc_dom_image *dom, const char *filename)
     return 0;
 }
 
+int xc_dom_devicetree_file(struct xc_dom_image *dom, const char *filename)
+{
+    DOMPRINTF("%s: filename=\"%s\"", __FUNCTION__, filename);
+    dom->devicetree_blob =
+        xc_dom_malloc_filemap(dom, filename, &dom->devicetree_size,
+                              dom->max_devicetree_size);
+
+    if ( dom->devicetree_blob == NULL )
+        return -1;
+    return 0;
+}
+
 int xc_dom_kernel_mem(struct xc_dom_image *dom, const void *mem, size_t memsize)
 {
     DOMPRINTF_CALLED(dom->xch);
@@ -747,6 +767,15 @@ int xc_dom_ramdisk_mem(struct xc_dom_image *dom, const void *mem,
     return 0;
 }
 
+int xc_dom_devicetree_mem(struct xc_dom_image *dom, const void *mem,
+                          size_t memsize)
+{
+    DOMPRINTF_CALLED(dom->xch);
+    dom->devicetree_blob = (void *)mem;
+    dom->devicetree_size = memsize;
+    return 0;
+}
+
 int xc_dom_parse_image(struct xc_dom_image *dom)
 {
     int i;
@@ -916,6 +945,25 @@ int xc_dom_build_image(struct xc_dom_image *dom)
             memcpy(ramdiskmap, dom->ramdisk_blob, dom->ramdisk_size);
     }
 
+    /* load devicetree */
+    if ( dom->devicetree_blob )
+    {
+        void *devicetreemap;
+
+        if ( xc_dom_alloc_segment(dom, &dom->devicetree_seg, "devicetree",
+                                  dom->devicetree_seg.vstart,
+                                  dom->devicetree_size) != 0 )
+            goto err;
+        devicetreemap = xc_dom_seg_to_ptr(dom, &dom->devicetree_seg);
+        if ( devicetreemap == NULL )
+        {
+            DOMPRINTF("%s: xc_dom_seg_to_ptr(dom, &dom->devicetree_seg) => NULL",
+                      __FUNCTION__);
+            goto err;
+        }
+        memcpy(devicetreemap, dom->devicetree_blob, dom->devicetree_size);
+    }
+
     /* allocate other pages */
     if ( dom->arch_hooks->alloc_magic_pages(dom) != 0 )
         goto err;
-- 
1.7.10.4

[PATCH RFC 12/15] libxc: support for arm64 Image format

[Ian Campbell]

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 tools/libxc/xc_dom_armzimageloader.c |   88 ++++++++++++++++++++++++++++++++++
 1 file changed, 88 insertions(+)

diff --git a/tools/libxc/xc_dom_armzimageloader.c b/tools/libxc/xc_dom_armzimageloader.c
index 4e3f7ae..609d3a0 100644
--- a/tools/libxc/xc_dom_armzimageloader.c
+++ b/tools/libxc/xc_dom_armzimageloader.c
@@ -129,6 +129,86 @@ static int xc_dom_parse_zimage32_kernel(struct xc_dom_image *dom)
 }
 
 /* ------------------------------------------------------------ */
+/* 64-bit zImage Support                                        */
+/* ------------------------------------------------------------ */
+
+#define ZIMAGE64_MAGIC_V0 0x14000008
+#define ZIMAGE64_MAGIC_V1 0x644d5241 /* "ARM\x64" */
+
+/* linux/Documentation/arm64/booting.txt */
+struct zimage64_hdr {
+    uint32_t magic0;
+    uint32_t res0;
+    uint64_t text_offset;  /* Image load offset */
+    uint64_t res1;
+    uint64_t res2;
+    /* zImage V1 only from here */
+    uint64_t res3;
+    uint64_t res4;
+    uint64_t res5;
+    uint32_t magic1;
+    uint32_t res6;
+};
+static int xc_dom_probe_zimage64_kernel(struct xc_dom_image *dom)
+{
+    struct zimage64_hdr *zimage;
+
+    if ( dom->kernel_blob == NULL )
+    {
+        xc_dom_panic(dom->xch, XC_INTERNAL_ERROR,
+                     "%s: no kernel image loaded", __FUNCTION__);
+        return -EINVAL;
+    }
+
+    if ( dom->kernel_size < sizeof(*zimage) )
+    {
+        xc_dom_printf(dom->xch, "%s: kernel image too small", __FUNCTION__);
+        return -EINVAL;
+    }
+
+    zimage =  dom->kernel_blob;
+    if ( zimage->magic0 != ZIMAGE64_MAGIC_V0 &&
+         zimage->magic1 != ZIMAGE64_MAGIC_V1 )
+    {
+        xc_dom_printf(dom->xch, "%s: kernel is not an arm64 Image", __FUNCTION__);
+        return -EINVAL;
+    }
+
+    return 0;
+}
+
+static int xc_dom_parse_zimage64_kernel(struct xc_dom_image *dom)
+{
+    struct zimage64_hdr *zimage;
+    uint32_t entry_addr;
+    uint64_t v_start, v_end;
+    uint64_t rambase = dom->rambase_pfn << XC_PAGE_SHIFT;
+
+    DOMPRINTF_CALLED(dom->xch);
+
+    zimage = dom->kernel_blob;
+
+    v_start = rambase + zimage->text_offset;
+    v_end = v_start + dom->kernel_size;
+
+    entry_addr = v_start;
+
+    /* find kernel segment */
+    dom->kernel_seg.vstart = v_start;
+    dom->kernel_seg.vend   = v_start + dom->kernel_size;
+
+    dom->parms.virt_entry = entry_addr;
+    dom->parms.virt_base = rambase;
+
+    dom->guest_type = "xen-3.0-aarch64";
+    DOMPRINTF("%s: %s: 0x%" PRIx64 " -> 0x%" PRIx64 "",
+              __FUNCTION__, dom->guest_type,
+              dom->kernel_seg.vstart, dom->kernel_seg.vend);
+
+    return 0;
+}
+
+/* ------------------------------------------------------------ */
 /* Common zImage Support                                        */
 /* ------------------------------------------------------------ */
 
@@ -163,9 +243,17 @@ static struct xc_dom_loader zimage32_loader = {
     .loader = xc_dom_load_zimage_kernel,
 };
 
+static struct xc_dom_loader zimage64_loader = {
+    .name = "Linux zImage (ARM64)",
+    .probe = xc_dom_probe_zimage64_kernel,
+    .parser = xc_dom_parse_zimage64_kernel,
+    .loader = xc_dom_load_zimage_kernel,
+};
+
 static void __init register_loader(void)
 {
     xc_dom_register_loader(&zimage32_loader);
+    xc_dom_register_loader(&zimage64_loader);
 }
 
 /*
-- 
1.7.10.4

Re: [PATCH RFC 12/15] libxc: support for arm64 Image format

[Julien Grall]

I think all the code in this patch should in an ifdef __aarch64__.

On 10/07/2013 05:39 PM, Ian Campbell wrote:
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
>  tools/libxc/xc_dom_armzimageloader.c |   88 ++++++++++++++++++++++++++++++++++
>  1 file changed, 88 insertions(+)
> 
> diff --git a/tools/libxc/xc_dom_armzimageloader.c b/tools/libxc/xc_dom_armzimageloader.c
> index 4e3f7ae..609d3a0 100644
> --- a/tools/libxc/xc_dom_armzimageloader.c
> +++ b/tools/libxc/xc_dom_armzimageloader.c
> @@ -129,6 +129,86 @@ static int xc_dom_parse_zimage32_kernel(struct xc_dom_image *dom)
>  }
>  
>  /* ------------------------------------------------------------ */
> +/* 64-bit zImage Support                                        */
> +/* ------------------------------------------------------------ */
> +
> +#define ZIMAGE64_MAGIC_V0 0x14000008
> +#define ZIMAGE64_MAGIC_V1 0x644d5241 /* "ARM\x64" */
> +
> +/* linux/Documentation/arm64/booting.txt */
> +struct zimage64_hdr {
> +    uint32_t magic0;
> +    uint32_t res0;
> +    uint64_t text_offset;  /* Image load offset */
> +    uint64_t res1;
> +    uint64_t res2;
> +    /* zImage V1 only from here */
> +    uint64_t res3;
> +    uint64_t res4;
> +    uint64_t res5;
> +    uint32_t magic1;
> +    uint32_t res6;
> +};
> +static int xc_dom_probe_zimage64_kernel(struct xc_dom_image *dom)
> +{
> +    struct zimage64_hdr *zimage;
> +
> +    if ( dom->kernel_blob == NULL )
> +    {
> +        xc_dom_panic(dom->xch, XC_INTERNAL_ERROR,
> +                     "%s: no kernel image loaded", __FUNCTION__);
> +        return -EINVAL;
> +    }
> +
> +    if ( dom->kernel_size < sizeof(*zimage) )
> +    {
> +        xc_dom_printf(dom->xch, "%s: kernel image too small", __FUNCTION__);
> +        return -EINVAL;
> +    }
> +
> +    zimage =  dom->kernel_blob;
> +    if ( zimage->magic0 != ZIMAGE64_MAGIC_V0 &&
> +         zimage->magic1 != ZIMAGE64_MAGIC_V1 )
> +    {
> +        xc_dom_printf(dom->xch, "%s: kernel is not an arm64 Image", __FUNCTION__);
> +        return -EINVAL;
> +    }
> +
> +    return 0;
> +}
> +
> +static int xc_dom_parse_zimage64_kernel(struct xc_dom_image *dom)
> +{
> +    struct zimage64_hdr *zimage;
> +    uint32_t entry_addr;
> +    uint64_t v_start, v_end;
> +    uint64_t rambase = dom->rambase_pfn << XC_PAGE_SHIFT;
> +
> +    DOMPRINTF_CALLED(dom->xch);
> +
> +    zimage = dom->kernel_blob;
> +
> +    v_start = rambase + zimage->text_offset;
> +    v_end = v_start + dom->kernel_size;
> +
> +    entry_addr = v_start;
> +
> +    /* find kernel segment */
> +    dom->kernel_seg.vstart = v_start;
> +    dom->kernel_seg.vend   = v_start + dom->kernel_size;
> +
> +    dom->parms.virt_entry = entry_addr;
> +    dom->parms.virt_base = rambase;
> +
> +    dom->guest_type = "xen-3.0-aarch64";
> +    DOMPRINTF("%s: %s: 0x%" PRIx64 " -> 0x%" PRIx64 "",
> +              __FUNCTION__, dom->guest_type,
> +              dom->kernel_seg.vstart, dom->kernel_seg.vend);
> +
> +    return 0;
> +}
> +
> +/* ------------------------------------------------------------ */
>  /* Common zImage Support                                        */
>  /* ------------------------------------------------------------ */
>  
> @@ -163,9 +243,17 @@ static struct xc_dom_loader zimage32_loader = {
>      .loader = xc_dom_load_zimage_kernel,
>  };
>  
> +static struct xc_dom_loader zimage64_loader = {
> +    .name = "Linux zImage (ARM64)",
> +    .probe = xc_dom_probe_zimage64_kernel,
> +    .parser = xc_dom_parse_zimage64_kernel,
> +    .loader = xc_dom_load_zimage_kernel,
> +};
> +
>  static void __init register_loader(void)
>  {
>      xc_dom_register_loader(&zimage32_loader);
> +    xc_dom_register_loader(&zimage64_loader);
>  }
>  
>  /*
> 


-- 
Julien Grall

Re: [PATCH RFC 12/15] libxc: support for arm64 Image format

[Ian Campbell]

On Thu, 2013-10-10 at 16:43 +0100, Julien Grall wrote:
> I think all the code in this patch should in an ifdef __aarch64__.

Oops, yes. I did say I hadn't tried it on 32 bit ;-)

> 
> On 10/07/2013 05:39 PM, Ian Campbell wrote:
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > ---
> >  tools/libxc/xc_dom_armzimageloader.c |   88 ++++++++++++++++++++++++++++++++++
> >  1 file changed, 88 insertions(+)
> > 
> > diff --git a/tools/libxc/xc_dom_armzimageloader.c b/tools/libxc/xc_dom_armzimageloader.c
> > index 4e3f7ae..609d3a0 100644
> > --- a/tools/libxc/xc_dom_armzimageloader.c
> > +++ b/tools/libxc/xc_dom_armzimageloader.c
> > @@ -129,6 +129,86 @@ static int xc_dom_parse_zimage32_kernel(struct xc_dom_image *dom)
> >  }
> >  
> >  /* ------------------------------------------------------------ */
> > +/* 64-bit zImage Support                                        */
> > +/* ------------------------------------------------------------ */
> > +
> > +#define ZIMAGE64_MAGIC_V0 0x14000008
> > +#define ZIMAGE64_MAGIC_V1 0x644d5241 /* "ARM\x64" */
> > +
> > +/* linux/Documentation/arm64/booting.txt */
> > +struct zimage64_hdr {
> > +    uint32_t magic0;
> > +    uint32_t res0;
> > +    uint64_t text_offset;  /* Image load offset */
> > +    uint64_t res1;
> > +    uint64_t res2;
> > +    /* zImage V1 only from here */
> > +    uint64_t res3;
> > +    uint64_t res4;
> > +    uint64_t res5;
> > +    uint32_t magic1;
> > +    uint32_t res6;
> > +};
> > +static int xc_dom_probe_zimage64_kernel(struct xc_dom_image *dom)
> > +{
> > +    struct zimage64_hdr *zimage;
> > +
> > +    if ( dom->kernel_blob == NULL )
> > +    {
> > +        xc_dom_panic(dom->xch, XC_INTERNAL_ERROR,
> > +                     "%s: no kernel image loaded", __FUNCTION__);
> > +        return -EINVAL;
> > +    }
> > +
> > +    if ( dom->kernel_size < sizeof(*zimage) )
> > +    {
> > +        xc_dom_printf(dom->xch, "%s: kernel image too small", __FUNCTION__);
> > +        return -EINVAL;
> > +    }
> > +
> > +    zimage =  dom->kernel_blob;
> > +    if ( zimage->magic0 != ZIMAGE64_MAGIC_V0 &&
> > +         zimage->magic1 != ZIMAGE64_MAGIC_V1 )
> > +    {
> > +        xc_dom_printf(dom->xch, "%s: kernel is not an arm64 Image", __FUNCTION__);
> > +        return -EINVAL;
> > +    }
> > +
> > +    return 0;
> > +}
> > +
> > +static int xc_dom_parse_zimage64_kernel(struct xc_dom_image *dom)
> > +{
> > +    struct zimage64_hdr *zimage;
> > +    uint32_t entry_addr;
> > +    uint64_t v_start, v_end;
> > +    uint64_t rambase = dom->rambase_pfn << XC_PAGE_SHIFT;
> > +
> > +    DOMPRINTF_CALLED(dom->xch);
> > +
> > +    zimage = dom->kernel_blob;
> > +
> > +    v_start = rambase + zimage->text_offset;
> > +    v_end = v_start + dom->kernel_size;
> > +
> > +    entry_addr = v_start;
> > +
> > +    /* find kernel segment */
> > +    dom->kernel_seg.vstart = v_start;
> > +    dom->kernel_seg.vend   = v_start + dom->kernel_size;
> > +
> > +    dom->parms.virt_entry = entry_addr;
> > +    dom->parms.virt_base = rambase;
> > +
> > +    dom->guest_type = "xen-3.0-aarch64";
> > +    DOMPRINTF("%s: %s: 0x%" PRIx64 " -> 0x%" PRIx64 "",
> > +              __FUNCTION__, dom->guest_type,
> > +              dom->kernel_seg.vstart, dom->kernel_seg.vend);
> > +
> > +    return 0;
> > +}
> > +
> > +/* ------------------------------------------------------------ */
> >  /* Common zImage Support                                        */
> >  /* ------------------------------------------------------------ */
> >  
> > @@ -163,9 +243,17 @@ static struct xc_dom_loader zimage32_loader = {
> >      .loader = xc_dom_load_zimage_kernel,
> >  };
> >  
> > +static struct xc_dom_loader zimage64_loader = {
> > +    .name = "Linux zImage (ARM64)",
> > +    .probe = xc_dom_probe_zimage64_kernel,
> > +    .parser = xc_dom_parse_zimage64_kernel,
> > +    .loader = xc_dom_load_zimage_kernel,
> > +};
> > +
> >  static void __init register_loader(void)
> >  {
> >      xc_dom_register_loader(&zimage32_loader);
> > +    xc_dom_register_loader(&zimage64_loader);
> >  }
> >  
> >  /*
> > 
> 
> 

Re: [PATCH RFC 12/15] libxc: support for arm64 Image format

[Ian Campbell]

On Thu, 2013-10-10 at 16:56 +0100, Ian Campbell wrote:
> On Thu, 2013-10-10 at 16:43 +0100, Julien Grall wrote:
> > I think all the code in this patch should in an ifdef __aarch64__.
> 
> Oops, yes. I did say I hadn't tried it on 32 bit ;-)

Upon second thought -- it is reasonable to want to build a 64-bit guest
while running a 32-bit dom0 on a 64-bit Xen.

I think this should work, although I've not tried it.

Ian.

Re: [PATCH RFC 12/15] libxc: support for arm64 Image format

[Julien Grall]

On 10/21/2013 10:46 AM, Ian Campbell wrote:
> On Thu, 2013-10-10 at 16:56 +0100, Ian Campbell wrote:
>> On Thu, 2013-10-10 at 16:43 +0100, Julien Grall wrote:
>>> I think all the code in this patch should in an ifdef __aarch64__.
>>
>> Oops, yes. I did say I hadn't tried it on 32 bit ;-)
>
> Upon second thought -- it is reasonable to want to build a 64-bit guest
> while running a 32-bit dom0 on a 64-bit Xen.

Right, I forget that it's possible to boot 32 kernel, so 32 bit 
userspace, on ARM64.

> I think this should work, although I've not tried it.

There is a bunch of #ifdef __aarch64__ in the public interface. To 
support dom0 32 bit, we will need to remove theses #ifdef.

-- 
Julien Grall

[PATCH RFC 13/15] libxc: arm64 vcpu initialisation

[Ian Campbell]

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 tools/libxc/xc_dom_arm.c |   90 ++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 88 insertions(+), 2 deletions(-)

diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c
index ecdaea9..a6d2259 100644
--- a/tools/libxc/xc_dom_arm.c
+++ b/tools/libxc/xc_dom_arm.c
@@ -105,7 +105,7 @@ static int shared_info_arm(struct xc_dom_image *dom, void *ptr)
 
 /* ------------------------------------------------------------------------ */
 
-static int vcpu_arm(struct xc_dom_image *dom, void *ptr)
+static int vcpu_arm32(struct xc_dom_image *dom, void *ptr)
 {
     vcpu_guest_context_t *ctxt = ptr;
 
@@ -143,6 +143,41 @@ static int vcpu_arm(struct xc_dom_image *dom, void *ptr)
     return 0;
 }
 
+static int vcpu_arm64(struct xc_dom_image *dom, void *ptr)
+{
+    vcpu_guest_context_t *ctxt = ptr;
+
+    DOMPRINTF_CALLED(dom->xch);
+    /* clear everything */
+    memset(ctxt, 0, sizeof(*ctxt));
+
+    ctxt->user_regs.pc64 = dom->parms.virt_entry;
+
+    /* Linux boot protocol. See linux.Documentation/arm/Booting. */
+    ctxt->user_regs.x0 = dom->devicetree_blob ?
+        dom->devicetree_seg.vstart : 0xffffffff;
+    ctxt->user_regs.x1 = 0;
+    ctxt->user_regs.x2 = 0;
+    ctxt->user_regs.x3 = 0;
+
+    DOMPRINTF("DTB %"PRIx64, ctxt->user_regs.x0);
+
+    ctxt->sctlr = /* #define SCTLR_BASE */0x00c50078;
+
+    ctxt->ttbr0 = 0;
+    ctxt->ttbr1 = 0;
+    ctxt->ttbcr = 0; /* Defined Reset Value */
+
+    ctxt->user_regs.cpsr = PSR_ABT_MASK|PSR_FIQ_MASK|PSR_IRQ_MASK|PSR_MODE_EL1h;
+
+    ctxt->flags = VGCF_online;
+
+    DOMPRINTF("Initial state CPSR %#"PRIx32" PC %#"PRIx64,
+           ctxt->user_regs.cpsr, ctxt->user_regs.pc64);
+
+    return 0;
+}
+
 /* ------------------------------------------------------------------------ */
 
 static struct xc_dom_arch xc_dom_32 = {
@@ -155,12 +190,59 @@ static struct xc_dom_arch xc_dom_32 = {
     .setup_pgtables = setup_pgtables_arm,
     .start_info = start_info_arm,
     .shared_info = shared_info_arm,
-    .vcpu = vcpu_arm,
+    .vcpu = vcpu_arm32,
+};
+
+static struct xc_dom_arch xc_dom_64 = {
+    .guest_type = "xen-3.0-aarch64",
+    .native_protocol = XEN_IO_PROTO_ABI_ARM,
+    .page_shift = PAGE_SHIFT_ARM,
+    .sizeof_pfn = 8,
+    .alloc_magic_pages = alloc_magic_pages,
+    .count_pgtables = count_pgtables_arm,
+    .setup_pgtables = setup_pgtables_arm,
+    .start_info = start_info_arm,
+    .shared_info = shared_info_arm,
+    .vcpu = vcpu_arm64,
 };
 
 static void __init register_arch_hooks(void)
 {
     xc_dom_register_arch_hooks(&xc_dom_32);
+    xc_dom_register_arch_hooks(&xc_dom_64);
+}
+
+static int set_mode(xc_interface *xch, domid_t domid, char *guest_type)
+{
+    static const struct {
+        char           *guest;
+        uint32_t        size;
+    } types[] = {
+        { "xen-3.0-aarch64", 64 },
+        { "xen-3.0-armv7l",  32 },
+    };
+    DECLARE_DOMCTL;
+    int i,rc;
+
+    domctl.domain = domid;
+    domctl.cmd    = XEN_DOMCTL_set_address_size;
+    for ( i = 0; i < sizeof(types)/sizeof(types[0]); i++ )
+        if ( !strcmp(types[i].guest, guest_type) )
+            domctl.u.address_size.size = types[i].size;
+    if ( domctl.u.address_size.size == 0 )
+    {
+        xc_dom_printf(xch, "%s: warning: unknown guest type %s",
+                      __FUNCTION__, guest_type);
+        return -EINVAL;
+    }
+
+    xc_dom_printf(xch, "%s: guest %s, address size %" PRId32 "", __FUNCTION__,
+                  guest_type, domctl.u.address_size.size);
+    rc = do_domctl(xch, &domctl);
+    if ( rc != 0 )
+        xc_dom_printf(xch, "%s: warning: failed (rc=%d)",
+                      __FUNCTION__, rc);
+    return rc;
 }
 
 int arch_setup_meminit(struct xc_dom_image *dom)
@@ -168,6 +250,10 @@ int arch_setup_meminit(struct xc_dom_image *dom)
     int rc;
     xen_pfn_t pfn, allocsz, i;
 
+    rc = set_mode(dom->xch, dom->guest_domid, dom->guest_type);
+    if ( rc )
+        return rc;
+
     dom->shadow_enabled = 1;
 
     dom->p2m_host = xc_dom_malloc(dom, sizeof(xen_pfn_t) * dom->total_pages);
-- 
1.7.10.4

Re: [PATCH RFC 13/15] libxc: arm64 vcpu initialisation

[Julien Grall]

On 10/07/2013 05:39 PM, Ian Campbell wrote:
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
>  tools/libxc/xc_dom_arm.c |   90 ++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 88 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c
> index ecdaea9..a6d2259 100644
> --- a/tools/libxc/xc_dom_arm.c
> +++ b/tools/libxc/xc_dom_arm.c
> @@ -105,7 +105,7 @@ static int shared_info_arm(struct xc_dom_image *dom, void *ptr)
>  
>  /* ------------------------------------------------------------------------ */
>  
> -static int vcpu_arm(struct xc_dom_image *dom, void *ptr)
> +static int vcpu_arm32(struct xc_dom_image *dom, void *ptr)
>  {
>      vcpu_guest_context_t *ctxt = ptr;
>  
> @@ -143,6 +143,41 @@ static int vcpu_arm(struct xc_dom_image *dom, void *ptr)
>      return 0;
>  }
>  
> +static int vcpu_arm64(struct xc_dom_image *dom, void *ptr)
> +{
> +    vcpu_guest_context_t *ctxt = ptr;
> +
> +    DOMPRINTF_CALLED(dom->xch);
> +    /* clear everything */
> +    memset(ctxt, 0, sizeof(*ctxt));
> +
> +    ctxt->user_regs.pc64 = dom->parms.virt_entry;
> +
> +    /* Linux boot protocol. See linux.Documentation/arm/Booting. */
> +    ctxt->user_regs.x0 = dom->devicetree_blob ?
> +        dom->devicetree_seg.vstart : 0xffffffff;
> +    ctxt->user_regs.x1 = 0;
> +    ctxt->user_regs.x2 = 0;
> +    ctxt->user_regs.x3 = 0;
> +
> +    DOMPRINTF("DTB %"PRIx64, ctxt->user_regs.x0);
> +
> +    ctxt->sctlr = /* #define SCTLR_BASE */0x00c50078;
> +
> +    ctxt->ttbr0 = 0;
> +    ctxt->ttbr1 = 0;
> +    ctxt->ttbcr = 0; /* Defined Reset Value */
> +
> +    ctxt->user_regs.cpsr = PSR_ABT_MASK|PSR_FIQ_MASK|PSR_IRQ_MASK|PSR_MODE_EL1h;

PSR_MODE_EL1h is only defined on arm64. I think you will need to prefix
all the code by #ifdef __aarch64__

BTW, can you use PSR_GUEST64_INIT here? So if we modified it in Xen, the
behaviour will also change here.

-- 
Julien Grall

Re: [PATCH RFC 13/15] libxc: arm64 vcpu initialisation

[Ian Campbell]

On Thu, 2013-10-10 at 16:54 +0100, Julien Grall wrote:
> On 10/07/2013 05:39 PM, Ian Campbell wrote:
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > ---
> >  tools/libxc/xc_dom_arm.c |   90 ++++++++++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 88 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c
> > index ecdaea9..a6d2259 100644
> > --- a/tools/libxc/xc_dom_arm.c
> > +++ b/tools/libxc/xc_dom_arm.c
> > @@ -105,7 +105,7 @@ static int shared_info_arm(struct xc_dom_image *dom, void *ptr)
> >  
> >  /* ------------------------------------------------------------------------ */
> >  
> > -static int vcpu_arm(struct xc_dom_image *dom, void *ptr)
> > +static int vcpu_arm32(struct xc_dom_image *dom, void *ptr)
> >  {
> >      vcpu_guest_context_t *ctxt = ptr;
> >  
> > @@ -143,6 +143,41 @@ static int vcpu_arm(struct xc_dom_image *dom, void *ptr)
> >      return 0;
> >  }
> >  
> > +static int vcpu_arm64(struct xc_dom_image *dom, void *ptr)
> > +{
> > +    vcpu_guest_context_t *ctxt = ptr;
> > +
> > +    DOMPRINTF_CALLED(dom->xch);
> > +    /* clear everything */
> > +    memset(ctxt, 0, sizeof(*ctxt));
> > +
> > +    ctxt->user_regs.pc64 = dom->parms.virt_entry;
> > +
> > +    /* Linux boot protocol. See linux.Documentation/arm/Booting. */
> > +    ctxt->user_regs.x0 = dom->devicetree_blob ?
> > +        dom->devicetree_seg.vstart : 0xffffffff;
> > +    ctxt->user_regs.x1 = 0;
> > +    ctxt->user_regs.x2 = 0;
> > +    ctxt->user_regs.x3 = 0;
> > +
> > +    DOMPRINTF("DTB %"PRIx64, ctxt->user_regs.x0);
> > +
> > +    ctxt->sctlr = /* #define SCTLR_BASE */0x00c50078;
> > +
> > +    ctxt->ttbr0 = 0;
> > +    ctxt->ttbr1 = 0;
> > +    ctxt->ttbcr = 0; /* Defined Reset Value */
> > +
> > +    ctxt->user_regs.cpsr = PSR_ABT_MASK|PSR_FIQ_MASK|PSR_IRQ_MASK|PSR_MODE_EL1h;
> 
> PSR_MODE_EL1h is only defined on arm64. I think you will need to prefix
> all the code by #ifdef __aarch64__

Correct.

> BTW, can you use PSR_GUEST64_INIT here? So if we modified it in Xen, the
> behaviour will also change here.

It's a bit annoying, but PSR_GUEST*_INIT are no in the public interfaces
and libxc can't include anything else. I don't want to expose this to
guests incase it becomes ABI...

Oh, maybe I can just us #if on __XEN_TOOLS__ and/or __XEN__ as
appropaite.

I'll have a play with that.

Re: [PATCH RFC 13/15] libxc: arm64 vcpu initialisation

[Julien Grall]

On 10/10/2013 04:59 PM, Ian Campbell wrote:
>> BTW, can you use PSR_GUEST64_INIT here? So if we modified it in Xen, the
>> behaviour will also change here.
> 
> It's a bit annoying, but PSR_GUEST*_INIT are no in the public interfaces
> and libxc can't include anything else. I don't want to expose this to
> guests incase it becomes ABI...

Until the commit bcac10f "xen: arm: support building a 64-bit dom0
domain", PSR_GUEST*_INIT was in the public interface. I'm not sure why
you have removed from the interface.

> Oh, maybe I can just us #if on __XEN_TOOLS__ and/or __XEN__ as
> appropaite.

Guest doesn't need that, but tools should have the same PSR as Linux.
Why boot VCPU initialization can't be done in Xen?

-- 
Julien Grall

Re: [PATCH RFC 13/15] libxc: arm64 vcpu initialisation

[Ian Campbell]

On Thu, 2013-10-10 at 17:04 +0100, Julien Grall wrote:
> On 10/10/2013 04:59 PM, Ian Campbell wrote:
> >> BTW, can you use PSR_GUEST64_INIT here? So if we modified it in Xen, the
> >> behaviour will also change here.
> > 
> > It's a bit annoying, but PSR_GUEST*_INIT are no in the public interfaces
> > and libxc can't include anything else. I don't want to expose this to
> > guests incase it becomes ABI...
> 
> Until the commit bcac10f "xen: arm: support building a 64-bit dom0
> domain", PSR_GUEST*_INIT was in the public interface. I'm not sure why
> you have removed from the interface.

It's an implementation detail which the guest shouldn't rely upon.

> > Oh, maybe I can just us #if on __XEN_TOOLS__ and/or __XEN__ as
> > appropaite.
> 
> Guest doesn't need that, but tools should have the same PSR as Linux.

"the same PSR as Linux" -- did you mean Xen?

> Why boot VCPU initialization can't be done in Xen?

The domain builder is the one which knows what is going on, what the
layout is etc, it's appropriate for it to also control the initial state
of the vcpus.

Ian.

Re: [PATCH RFC 13/15] libxc: arm64 vcpu initialisation

[Julien Grall]

On 10/10/2013 05:09 PM, Ian Campbell wrote:
> On Thu, 2013-10-10 at 17:04 +0100, Julien Grall wrote:
>> Guest doesn't need that, but tools should have the same PSR as Linux.
>
> "the same PSR as Linux" -- did you mean Xen?

Yes

-- 
Julien Grall

[PATCH RFC 14/15] libxl: build a device tree for ARM guests

[Ian Campbell]

Uses xc_dom_devicetree_mem which was just added. The call to this needs to be
carefully sequenced to be after xc_dom_parse_image (so we can tell which kind
of guest we are building, although we don't use this yet) and before
xc_dom_mem_init which tries to decide where to place the FDT in guest RAM.

Removes libxl_noarch which would only have been used by IA64 after this
change. Remove IA64 as part of this patch.

There is no attempt to expose this as a configuration setting for the user.

Includes a debug hook to dump the dtb to a file for inspection.

TODO:
- Hardcoded armv8 bits need abstracting. Perhaps e.g. read CPU compatiblity
  node from sysfs?
- Try it with armv7
- Debug hook should be #ifdef DEBUG?
- Various values (e.g. GIC base addresses, evtchn PPI) should be passed
  down to the hypervisor by some mechanism I've not decided on yet. Perhaps
  domctl but maybe better would be via the HVM save format to more easily
  support migration of the settings?
- TODOs and //comments in the code

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 tools/libxl/Makefile       |    6 +-
 tools/libxl/libxl_arch.h   |    4 +
 tools/libxl/libxl_arm.c    |  523 ++++++++++++++++++++++++++++++++++++++++++++
 tools/libxl/libxl_dom.c    |    4 +
 tools/libxl/libxl_noarch.c |    8 -
 tools/libxl/libxl_x86.c    |    7 +
 6 files changed, 542 insertions(+), 10 deletions(-)
 create mode 100644 tools/libxl/libxl_arm.c
 delete mode 100644 tools/libxl/libxl_noarch.c

diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
index cf214bb..d8495bb 100644
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -28,9 +28,12 @@ CFLAGS_LIBXL += $(CFLAGS_libxenstore)
 CFLAGS_LIBXL += $(CFLAGS_libblktapctl) 
 CFLAGS_LIBXL += -Wshadow
 
+LIBXL_LIBS-$(CONFIG_ARM) += -lfdt
+
 CFLAGS += $(PTHREAD_CFLAGS)
 LDFLAGS += $(PTHREAD_LDFLAGS)
 LIBXL_LIBS += $(PTHREAD_LIBS)
+LIBXL_LIBS += $(LIBXL_LIBS-y)
 
 LIBXLU_LIBS =
 
@@ -41,8 +44,7 @@ else
 LIBXL_OBJS-y += libxl_noblktap2.o
 endif
 LIBXL_OBJS-$(CONFIG_X86) += libxl_cpuid.o libxl_x86.o
-LIBXL_OBJS-$(CONFIG_IA64) += libxl_nocpuid.o libxl_noarch.o
-LIBXL_OBJS-$(CONFIG_ARM) += libxl_nocpuid.o libxl_noarch.o
+LIBXL_OBJS-$(CONFIG_ARM) += libxl_nocpuid.o libxl_arm.o
 
 ifeq ($(CONFIG_NetBSD),y)
 LIBXL_OBJS-y += libxl_netbsd.o
diff --git a/tools/libxl/libxl_arch.h b/tools/libxl/libxl_arch.h
index abe6685..3de2d76 100644
--- a/tools/libxl/libxl_arch.h
+++ b/tools/libxl/libxl_arch.h
@@ -19,4 +19,8 @@
 int libxl__arch_domain_create(libxl__gc *gc, libxl_domain_config *d_config,
                uint32_t domid);
 
+/* */
+int libxl__arch_domain_configure(libxl__gc *gc,
+                                 libxl_domain_build_info *info,
+                                 struct xc_dom_image *dom);
 #endif
diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
new file mode 100644
index 0000000..913741d
--- /dev/null
+++ b/tools/libxl/libxl_arm.c
@@ -0,0 +1,523 @@
+#include "libxl_internal.h"
+#include "libxl_arch.h"
+
+#include <xc_dom.h>
+#include <libfdt.h>
+#include <assert.h>
+
+int libxl__arch_domain_create(libxl__gc *gc, libxl_domain_config *d_config,
+                              uint32_t domid)
+{
+    return 0;
+}
+
+enum {
+    PHANDLE_NONE = 0,
+    PHANDLE_GIC,
+};
+
+typedef uint32_t __be32;
+typedef __be32 gic_interrupt_t[3];
+
+#define ROOT_ADDRESS_CELLS 2
+#define ROOT_SIZE_CELLS 2
+
+static void set_cell(__be32 **cellp, int size, uint64_t val)
+{
+    int cells = size;
+
+    while ( size-- )
+    {
+        (*cellp)[size] = cpu_to_fdt32(val);
+        val >>= 32;
+    }
+
+    (*cellp) += cells;
+}
+
+static void set_interrupt_ppi(gic_interrupt_t interrupt, unsigned int irq,
+                              unsigned int cpumask, unsigned int level)
+{
+    __be32 *cells = interrupt;
+
+    /* See linux Documentation/devictree/bindings/arm/gic.txt */
+    set_cell(&cells, 1, 1); /* is a PPI */
+    set_cell(&cells, 1, irq - 16); /* PPIs start at 16 */
+    set_cell(&cells, 1, (cpumask << 8) | level);
+}
+
+static void set_range(__be32 **cellp,
+                      int address_cells, int size_cells,
+                      uint64_t address, uint64_t size)
+{
+    set_cell(cellp, address_cells, address);
+    set_cell(cellp, size_cells, size);
+}
+
+static int fdt_property_compat(libxl__gc *gc, void *fdt, unsigned nr_compat, ...)
+{
+    const char *compats[nr_compat];
+    int i;
+    size_t sz;
+    va_list ap;
+    char *compat, *p;
+
+    va_start(ap, nr_compat);
+    sz = 0;
+    for (i = 0; i < nr_compat; i++) {
+        const char *c = va_arg(ap, const char *);
+        compats[i] = c;
+        sz += strlen(compats[i]) + 1;
+    }
+    va_end(ap);
+
+    p = compat = libxl__zalloc(gc, sz);
+    for (i = 0; i < nr_compat; i++) {
+        strcpy(p, compats[i]);
+        p += strlen(compats[i]) + 1;
+    }
+
+    fdt_property(fdt, "compatible", compat, sz);
+
+    return 0;
+}
+
+static int fdt_property_interrupts(libxl__gc *gc, void *fdt,
+                                   gic_interrupt_t *intr,
+                                   unsigned num_irq)
+{
+    int res;
+
+    res = fdt_property(fdt, "interrupts", intr, sizeof (intr[0]) * num_irq);
+    if ( res )
+        return res;
+
+    res = fdt_property_cell(fdt, "interrupt-parent", PHANDLE_GIC);
+
+    return res;
+}
+
+static int fdt_property_regs(libxl__gc *gc, void *fdt,
+                             unsigned addr_cells,
+                             unsigned size_cells,
+                             unsigned num_regs, ...)
+{
+    uint32_t regs[num_regs*(addr_cells+size_cells)];
+    __be32 *cells = &regs[0];
+    int i;
+    va_list ap;
+    uint64_t base, size;
+
+    va_start(ap, num_regs);
+    for (i = 0 ; i < num_regs; i++) {
+        base = addr_cells ? va_arg(ap, uint64_t) : 0;
+        size = size_cells ? va_arg(ap, uint64_t) : 0;
+        set_range(&cells, addr_cells, size_cells, base, size);
+    }
+    va_end(ap);
+
+    return fdt_property(fdt, "reg", regs, sizeof(regs));
+}
+
+static int make_root_properties(libxl__gc *gc,
+                                const libxl_version_info *vers,
+                                void *fdt)
+{
+    int res;
+
+    res = fdt_property_string(fdt, "model", GCSPRINTF("XENVM-%d.%d",
+                                                      vers->xen_version_major,
+                                                      vers->xen_version_minor));
+    if ( res )
+        return res;
+
+    res = fdt_property_compat(gc, fdt, 2,
+                              GCSPRINTF("xen,xenvm-%d.%d",
+                                        vers->xen_version_major,
+                                        vers->xen_version_minor),
+                              "xen,xenvm");
+    if ( res )
+        return res;
+
+    res = fdt_property_cell(fdt, "interrupt-parent", PHANDLE_GIC);
+    if ( res )
+        return res;
+
+    res = fdt_property_cell(fdt, "#address-cells", ROOT_ADDRESS_CELLS);
+    if ( res )
+        return res;
+
+    res = fdt_property_cell(fdt, "#size-cells", ROOT_SIZE_CELLS);
+    if ( res )
+        return res;
+
+    return 0;
+}
+
+static int make_chosen_node(libxl__gc *gc, void *fdt,
+                            const libxl_domain_build_info *info)
+{
+    int res;
+
+    /* See linux Documentation/devicetree/... */
+    res = fdt_begin_node(fdt, "chosen");
+    if ( res )
+        return res;
+
+    res = fdt_property_string(fdt, "bootargs", info->u.pv.cmdline);
+    if ( res )
+        return res;
+
+    res = fdt_end_node(fdt);
+
+    return res;
+}
+
+static int make_cpus_node(libxl__gc *gc, void *fdt, int nr_cpus)
+{
+    int res, i;
+
+    res = fdt_begin_node(fdt, "cpus");
+    if ( res )
+        return res;
+
+    res = fdt_property_cell(fdt, "#address-cells", 1);
+    if ( res )
+        return res;
+
+    res = fdt_property_cell(fdt, "#size-cells", 0);
+    if ( res )
+        return res;
+
+    for (i = 0; i < nr_cpus; i++) {
+        char name[strlen("cpu@") + 2 + 1];
+
+        if ( snprintf(name, sizeof(name)-1, "cpu@%d", i) < 0 )
+        {
+            perror("snprintf");
+            exit(1);
+        }
+
+        res = fdt_begin_node(fdt, name);
+        if ( res )
+            return res;
+
+        res = fdt_property_string(fdt, "device_type", "cpu");
+        if ( res )
+            return res;
+
+        res = fdt_property_compat(gc, fdt, 1, "arm,armv8");
+        if ( res )
+            return res;
+
+        res = fdt_property_string(fdt, "enable-method", "psci");
+        if ( res )
+            return res;
+
+        res = fdt_property_regs(gc, fdt, 1, 0, 1, (uint64_t)i);
+        if ( res )
+            return res;
+
+        res = fdt_end_node(fdt);
+        if ( res )
+            return res;
+
+    }
+
+    res = fdt_end_node(fdt);
+
+    return res;
+}
+
+static int make_psci_node(libxl__gc *gc, void *fdt)
+{
+    int res;
+
+    res = fdt_begin_node(fdt, "psci");
+    if ( res )
+        return res;
+
+    res = fdt_property_compat(gc, fdt, 1, "arm,psci");
+    if ( res )
+        return res;
+
+    res = fdt_property_string(fdt, "method", "hvc");
+    if ( res )
+        return res;
+
+    res = fdt_property_cell(fdt, "cpu_off", 0x1);
+    if ( res )
+        return res;
+
+    res = fdt_property_cell(fdt, "cpu_on", 0x2);
+    if ( res )
+        return res;
+
+    res = fdt_end_node(fdt);
+
+    return res;
+}
+
+static int make_memory_node(libxl__gc *gc, void *fdt,
+                            unsigned long long base,
+                            unsigned long long size)
+{
+    int res;
+    char name[strlen("memory@") + 8 + 1];
+
+    snprintf(name, sizeof(name), "memory@%08llx", base);
+
+    res = fdt_begin_node(fdt, name);
+    if ( res )
+        return res;
+
+    res = fdt_property_string(fdt, "device_type", "memory");
+    if ( res )
+        return res;
+
+    res = fdt_property_regs(gc, fdt, ROOT_ADDRESS_CELLS, ROOT_SIZE_CELLS,
+                            1, (uint64_t)base, (uint64_t)size);
+    if ( res )
+        return res;
+
+    res = fdt_end_node(fdt);
+
+    return res;
+}
+
+static int make_intc_node(libxl__gc *gc, void *fdt,
+                          unsigned long long gicd_base,
+                          unsigned long long gicd_size,
+                          unsigned long long gicc_base,
+                          unsigned long long gicc_size)
+{
+    int res;
+    char name[strlen("interrupt-controller@") + 8 + 1];
+
+    snprintf(name, sizeof(name), "interrupt-controller@%08llx", gicd_base);
+
+    res = fdt_begin_node(fdt, name);
+    if ( res )
+        return res;
+
+    res = fdt_property_compat(gc, fdt, 2,
+                              "arm,cortex-a15-gic",
+                              "arm,cortex-a9-gic");
+    if ( res )
+        return res;
+
+
+    res = fdt_property_cell(fdt, "#interrupt-cells", 3);
+    if ( res )
+        return res;
+
+    res = fdt_property_cell(fdt, "#address-cells", 0);
+    if ( res )
+        return res;
+
+    res = fdt_property(fdt, "interrupt-controller", NULL, 0);
+    if ( res )
+        return res;
+
+    res = fdt_property_regs(gc, fdt, ROOT_ADDRESS_CELLS, ROOT_SIZE_CELLS,
+                            2,
+                            (uint64_t)gicd_base, (uint64_t)gicd_size,
+                            (uint64_t)gicc_base, (uint64_t)gicc_size);
+    if ( res )
+        return res;
+
+    res = fdt_property_cell(fdt, "linux,phandle", PHANDLE_GIC);
+    if ( res )
+        return res;
+
+    res = fdt_property_cell(fdt, "phandle", PHANDLE_GIC);
+    if ( res )
+        return res;
+
+    res = fdt_end_node(fdt);
+
+    return res;
+}
+
+static int make_timer_node(libxl__gc *gc, void *fdt)
+{
+    int res;
+    gic_interrupt_t ints[4];
+
+    res = fdt_begin_node(fdt, "timer");
+    if ( res )
+        return res;
+
+    res = fdt_property_compat(gc, fdt, 1, "arm,armv8-timer");
+    if ( res )
+        return res;
+
+    set_interrupt_ppi(ints[0], 29, 0xf, 0x8);
+    set_interrupt_ppi(ints[1], 30, 0xf, 0x8);
+    set_interrupt_ppi(ints[2], 27, 0xf, 0x8);
+    set_interrupt_ppi(ints[3], 26, 0xf, 0x8);
+
+    res = fdt_property_interrupts(gc, fdt, ints, 4);
+    if ( res )
+        return res;
+
+    res = fdt_end_node(fdt);
+
+    return res;
+}
+
+static int make_hypervisor_node(libxl__gc *gc, void *fdt,
+                                const libxl_version_info *vers)
+{
+    int res;
+    gic_interrupt_t intr;
+
+    /* See linux Documentation/devicetree/bindings/arm/xen.txt */
+    res = fdt_begin_node(fdt, "hypervisor");
+    if ( res )
+        return res;
+
+    res = fdt_property_compat(gc, fdt, 2,
+                              GCSPRINTF("xen,xen-%d.%d",
+                                        vers->xen_version_major,
+                                        vers->xen_version_minor),
+                              "xen,xen");
+    if ( res )
+        return res;
+
+    //DPRINT("  Grant table range: 0xb0000000-0x20000\n");
+    /* reg 0 is grant table space */
+    res = fdt_property_regs(gc, fdt, ROOT_ADDRESS_CELLS, ROOT_SIZE_CELLS,
+                            1,
+                            (uint64_t)0xb0000000,
+                            (uint64_t)0x20000);
+    if ( res )
+        return res;
+
+    /*
+     * interrupts is evtchn upcall:
+     *  - Active-low level-sensitive
+     *  - All cpus
+     *
+     * TODO: Handle correctly the cpumask
+     */
+    //DPRINT("  Event channel interrupt to %u\n", VGIC_IRQ_EVTCHN_CALLBACK);
+    set_interrupt_ppi(intr, 31, 0xf, 0x8);
+
+    res = fdt_property_interrupts(gc, fdt, &intr, 1);
+    if ( res )
+        return res;
+
+    res = fdt_end_node(fdt);
+
+    return res;
+}
+
+#define FDT_ERROR(what) do { \
+    LOG(ERROR, "FDT: %s failed: %d = %s", what, res, fdt_strerror(res)); \
+    rc = ERROR_FAIL; \
+    goto out; \
+} while(0)
+
+int libxl__arch_domain_configure(libxl__gc *gc,
+                                 libxl_domain_build_info *info,
+                                 struct xc_dom_image *dom)
+{
+    void *fdt;
+    int rc, res, fd;
+
+    const libxl_version_info *vers;
+
+    assert(info->type == LIBXL_DOMAIN_TYPE_PV);
+
+    vers = libxl_get_version_info(CTX);
+    if (vers == NULL) return ERROR_FAIL;
+
+    LOG(INFO, "constructing DTB for Xen version %d.%d guest",
+        vers->xen_version_major, vers->xen_version_minor);
+
+    LOG(INFO, "ram base = %#"PRIx64,
+        (uint64_t)dom->rambase_pfn << XC_PAGE_SHIFT);
+    LOG(INFO, "cpus = %d", info->max_vcpus);
+
+    fdt = libxl__zalloc(gc, 4096);
+
+    res = fdt_create(fdt, 4096);
+    if (res) FDT_ERROR("fdt_create");
+
+    fdt_finish_reservemap(fdt);
+
+    res = fdt_begin_node(fdt, "");
+    if (res) FDT_ERROR("dfdt_begin_node");
+
+    res = make_root_properties(gc, vers, fdt);
+    if (res) FDT_ERROR("make_root_properties");
+
+    res = make_chosen_node(gc, fdt, info);
+    if (res) FDT_ERROR("make_chosen_node");
+
+    res = make_cpus_node(gc, fdt, 1);
+    if (res) FDT_ERROR("make_cpus_node");
+
+    res = make_psci_node(gc, fdt);
+    if (res) FDT_ERROR("make_psci_node");
+
+    res = make_memory_node(gc, fdt, 0x80000000UL, 0x8000000UL);
+    if (res) FDT_ERROR("make_memory_node");
+
+    res = make_intc_node(gc, fdt,
+                         0x2c001000ULL, 0x1000ULL,
+                         0x2c002000ULL, 0x100ULL);
+    if (res) FDT_ERROR("make_intc_node");
+
+    res = make_timer_node(gc, fdt);
+    if (res) FDT_ERROR("make_timer_node");
+
+    res = make_hypervisor_node(gc, fdt, vers);
+    if (res) FDT_ERROR("make_hypervisor_node");
+
+    res = fdt_end_node(fdt);
+    if (res) FDT_ERROR("fdt_end_node");
+
+    res = fdt_finish(fdt);
+    if (res) FDT_ERROR("fdt_finish");
+
+    LOG(DEBUG, "fdt total size %d", fdt_totalsize(fdt));
+
+    if ( (res = xc_dom_devicetree_mem(dom, fdt, fdt_totalsize(fdt))) != 0 ) {
+        LOGE(ERROR, "xc_dom_devicetree_file failed");
+        rc = ERROR_FAIL;
+        goto out;
+    }
+
+#ifndef NDEBUG
+    if ( getenv("LIBXL_DEBUG_DUMP_DTB") ) {
+        const char *dtb = getenv("LIBXL_DEBUG_DUMP_DTB");
+
+        fd = open(dtb, O_CREAT|O_TRUNC|O_WRONLY, 0666);
+        if ( fd < 0 ) {
+            LOGE(DEBUG, "cannot open %s for LIBXL_DEBUG_DUMP_DTB", dtb);
+            goto no_debug_dump;
+        }
+
+        rc = libxl_write_exactly(CTX, fd, fdt, fdt_totalsize(fdt),
+                            dtb, "dtb");
+        if ( rc < 0 ) {
+            LOG(DEBUG, "unable to write DTB debug dump output %d", rc);
+            goto no_debug_dump;
+        }
+
+        res = close(fd);
+        if ( res < 0 ) {
+            LOGE(DEBUG, "unable to close DTB debug dump output");
+            goto no_debug_dump;
+        }
+    }
+no_debug_dump:
+#endif
+
+    rc = 0;
+
+out:
+    return 0;
+}
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 631cdf5..78ba8f1 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -394,6 +394,10 @@ int libxl__build_pv(libxl__gc *gc, uint32_t domid,
         LOGE(ERROR, "xc_dom_parse_image failed");
         goto out;
     }
+    if ( (ret = libxl__arch_domain_configure(gc, info, dom)) != 0 ) {
+        LOGE(ERROR, "libxl__arch_domain_configure failed");
+        goto out;
+    }
     if ( (ret = xc_dom_mem_init(dom, info->target_memkb / 1024)) != 0 ) {
         LOGE(ERROR, "xc_dom_mem_init failed");
         goto out;
diff --git a/tools/libxl/libxl_noarch.c b/tools/libxl/libxl_noarch.c
deleted file mode 100644
index 7893535..0000000
--- a/tools/libxl/libxl_noarch.c
+++ /dev/null
@@ -1,8 +0,0 @@
-#include "libxl_internal.h"
-#include "libxl_arch.h"
-
-int libxl__arch_domain_create(libxl__gc *gc, libxl_domain_config *d_config,
-                              uint32_t domid)
-{
-    return 0;
-}
diff --git a/tools/libxl/libxl_x86.c b/tools/libxl/libxl_x86.c
index a78c91d..dd13c45 100644
--- a/tools/libxl/libxl_x86.c
+++ b/tools/libxl/libxl_x86.c
@@ -308,3 +308,10 @@ int libxl__arch_domain_create(libxl__gc *gc, libxl_domain_config *d_config,
 
     return ret;
 }
+
+int libxl__arch_domain_configure(libxl__gc *gc,
+                                 libxl_domain_build_info *info,
+                                 struct xc_dom_image *dom)
+{
+    return 0;
+}
-- 
1.7.10.4

Re: [PATCH RFC 14/15] libxl: build a device tree for ARM guests

[Julien Grall]

On 10/07/2013 05:40 PM, Ian Campbell wrote:
> Uses xc_dom_devicetree_mem which was just added. The call to this needs to be
> carefully sequenced to be after xc_dom_parse_image (so we can tell which kind
> of guest we are building, although we don't use this yet) and before
> xc_dom_mem_init which tries to decide where to place the FDT in guest RAM.
>
> Removes libxl_noarch which would only have been used by IA64 after this
> change. Remove IA64 as part of this patch.
>
> There is no attempt to expose this as a configuration setting for the user.
>
> Includes a debug hook to dump the dtb to a file for inspection.
>
> TODO:
> - Hardcoded armv8 bits need abstracting. Perhaps e.g. read CPU compatiblity
>    node from sysfs?

I'm wondering if it's usefull to have the property compatible. From 
Linux documentation:
   So under /cpus, you are supposed to create a node for every CPU on
   the machine. There is no specific restriction on the name of the
   CPU, though it's common to call it <architecture>,<core>. For
   example, Apple uses PowerPC,G5 while IBM uses PowerPC,970FX.
   However, the Generic Names convention suggests that it would be
   better to simply use 'cpu' for each cpu node and use the compatible
   property to identify the specific cpu core.

Linux doesn't seems to retrieve the compatible node and Freebsd also.
The only way to read this property is from /proc/device-tree. But this 
directory exists only if CONFIG_PROC_DEVICETREE=y.

> - Try it with armv7

The previous point will be the same for the timer and the GIC.

> - Debug hook should be #ifdef DEBUG?
> - Various values (e.g. GIC base addresses, evtchn PPI) should be passed
>    down to the hypervisor by some mechanism I've not decided on yet. Perhaps
>    domctl but maybe better would be via the HVM save format to more easily
>    support migration of the settings?
> - TODOs and //comments in the code
   - Add initrd support?

>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

[..]

> diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
> new file mode 100644
> index 0000000..913741d
> --- /dev/null
> +++ b/tools/libxl/libxl_arm.c

[..]

> +static int fdt_property_interrupts(libxl__gc *gc, void *fdt,
> +                                   gic_interrupt_t *intr,
> +                                   unsigned num_irq)
> +{
> +    int res;
> +
> +    res = fdt_property(fdt, "interrupts", intr, sizeof (intr[0]) * num_irq);
> +    if ( res )
> +        return res;
> +
> +    res = fdt_property_cell(fdt, "interrupt-parent", PHANDLE_GIC);
> +
> +    return res;
> +}

You don't need this function if the interrupt-parent properties is set 
in the root node. Which is the case below in make_root_properties.

[..]

> +static int make_cpus_node(libxl__gc *gc, void *fdt, int nr_cpus)
> +{
> +    int res, i;
> +
> +    res = fdt_begin_node(fdt, "cpus");
> +    if ( res )
> +        return res;
> +
> +    res = fdt_property_cell(fdt, "#address-cells", 1);
> +    if ( res )
> +        return res;
> +
> +    res = fdt_property_cell(fdt, "#size-cells", 0);
> +    if ( res )
> +        return res;
> +
> +    for (i = 0; i < nr_cpus; i++) {
> +        char name[strlen("cpu@") + 2 + 1];
> +
> +        if ( snprintf(name, sizeof(name)-1, "cpu@%d", i) < 0 )
> +        {
> +            perror("snprintf");
> +            exit(1);
> +        }
> +
> +        res = fdt_begin_node(fdt, name);
> +        if ( res )
> +            return res;
> +
> +        res = fdt_property_string(fdt, "device_type", "cpu");
> +        if ( res )
> +            return res;
> +
> +        res = fdt_property_compat(gc, fdt, 1, "arm,armv8");
> +        if ( res )
> +            return res;
> +
> +        res = fdt_property_string(fdt, "enable-method", "psci");

#ifdef CONFIG_ARM64 and if it's a 64 bit domain?

> +        if ( res )
> +            return res;
> +
> +        res = fdt_property_regs(gc, fdt, 1, 0, 1, (uint64_t)i);
> +        if ( res )
> +            return res;
> +
> +        res = fdt_end_node(fdt);
> +        if ( res )
> +            return res;
> +
> +    }
> +
> +    res = fdt_end_node(fdt);
> +
> +    return res;
> +}
> +
> +static int make_psci_node(libxl__gc *gc, void *fdt)
> +{
> +    int res;
> +
> +    res = fdt_begin_node(fdt, "psci");
> +    if ( res )
> +        return res;
> +
> +    res = fdt_property_compat(gc, fdt, 1, "arm,psci");
> +    if ( res )
> +        return res;
> +
> +    res = fdt_property_string(fdt, "method", "hvc");
> +    if ( res )
> +        return res;
> +
> +    res = fdt_property_cell(fdt, "cpu_off", 0x1);
> +    if ( res )
> +        return res;
> +
> +    res = fdt_property_cell(fdt, "cpu_on", 0x2);
> +    if ( res )
> +        return res;

Can we export include/asm-arm/psci.h and reuse the value here?

> +
> +    res = fdt_end_node(fdt);
> +
> +    return res;
> +}

[..]

> +static int make_hypervisor_node(libxl__gc *gc, void *fdt,
> +                                const libxl_version_info *vers)
> +{
> +    int res;
> +    gic_interrupt_t intr;
> +
> +    /* See linux Documentation/devicetree/bindings/arm/xen.txt */
> +    res = fdt_begin_node(fdt, "hypervisor");
> +    if ( res )
> +        return res;
> +
> +    res = fdt_property_compat(gc, fdt, 2,
> +                              GCSPRINTF("xen,xen-%d.%d",
> +                                        vers->xen_version_major,
> +                                        vers->xen_version_minor),
> +                              "xen,xen");
> +    if ( res )
> +        return res;
> +
> +    //DPRINT("  Grant table range: 0xb0000000-0x20000\n");
> +    /* reg 0 is grant table space */
> +    res = fdt_property_regs(gc, fdt, ROOT_ADDRESS_CELLS, ROOT_SIZE_CELLS,
> +                            1,
> +                            (uint64_t)0xb0000000,

I still don't know where this value comes from... if it's a random 
value, can we autogenerate it?

-- 
Julien Grall

Re: [PATCH RFC 14/15] libxl: build a device tree for ARM guests

[Ian Campbell]

On Tue, 2013-10-15 at 00:11 +0100, Julien Grall wrote:
> On 10/07/2013 05:40 PM, Ian Campbell wrote:
> > Uses xc_dom_devicetree_mem which was just added. The call to this needs to be
> > carefully sequenced to be after xc_dom_parse_image (so we can tell which kind
> > of guest we are building, although we don't use this yet) and before
> > xc_dom_mem_init which tries to decide where to place the FDT in guest RAM.
> >
> > Removes libxl_noarch which would only have been used by IA64 after this
> > change. Remove IA64 as part of this patch.
> >
> > There is no attempt to expose this as a configuration setting for the user.
> >
> > Includes a debug hook to dump the dtb to a file for inspection.
> >
> > TODO:
> > - Hardcoded armv8 bits need abstracting. Perhaps e.g. read CPU compatiblity
> >    node from sysfs?
> 
> I'm wondering if it's usefull to have the property compatible. From 
> Linux documentation:
>    So under /cpus, you are supposed to create a node for every CPU on
>    the machine. There is no specific restriction on the name of the
>    CPU, though it's common to call it <architecture>,<core>. For
>    example, Apple uses PowerPC,G5 while IBM uses PowerPC,970FX.
>    However, the Generic Names convention suggests that it would be
>    better to simply use 'cpu' for each cpu node and use the compatible
>    property to identify the specific cpu core.

Hrm this seems to imply that either you should name the node
<architecture>,<core> *or* you should call it cpu and have a
compatibility property <architecture>,<core>.

> Linux doesn't seems to retrieve the compatible node and Freebsd also.
> The only way to read this property is from /proc/device-tree. But this 
> directory exists only if CONFIG_PROC_DEVICETREE=y.

Yes.

I suppose we could try /proc/device-tree and fall back to /proc/cpuinfo
(which should at least get us v7 vs v8) and finally fallback to a
hardcoded value based on the architecture the tools are compiled for
(which is lame, but better than nothing?).

Or maybe we add a hypercall to get the underlying CPU compatibility node
direct from Xen. It's the sort of thing I guess "xl info" ought to be
able to display I suppose.

> > - Try it with armv7
> 
> The previous point will be the same for the timer and the GIC.

Yes.

> > +static int fdt_property_interrupts(libxl__gc *gc, void *fdt,
> > +                                   gic_interrupt_t *intr,
> > +                                   unsigned num_irq)
> > +{
> > +    int res;
> > +
> > +    res = fdt_property(fdt, "interrupts", intr, sizeof (intr[0]) * num_irq);
> > +    if ( res )
> > +        return res;
> > +
> > +    res = fdt_property_cell(fdt, "interrupt-parent", PHANDLE_GIC);
> > +
> > +    return res;
> > +}
> 
> You don't need this function if the interrupt-parent properties is set 
> in the root node. Which is the case below in make_root_properties.

OK. The reason we have this for dom0 is that we can't control whether
the host DTB has interrupt-parent or not?

> > +
> > +        res = fdt_property_string(fdt, "enable-method", "psci");
> 
> #ifdef CONFIG_ARM64 and if it's a 64 bit domain?

Yes.

> > +    res = fdt_property_cell(fdt, "cpu_on", 0x2);
> > +    if ( res )
> > +        return res;
> 
> Can we export include/asm-arm/psci.h and reuse the value here?

I suppose we ought to, since Xen is the one implementing the actual
functionality. I notice that even Xen itself isn't using those #defines
(nothing is AFAICT!)

> > +    //DPRINT("  Grant table range: 0xb0000000-0x20000\n");
> > +    /* reg 0 is grant table space */
> > +    res = fdt_property_regs(gc, fdt, ROOT_ADDRESS_CELLS, ROOT_SIZE_CELLS,
> > +                            1,
> > +                            (uint64_t)0xb0000000,
> 
> I still don't know where this value comes from... if it's a random 
> value, can we autogenerate it?

It's an arbitrary value which we picked when we defined our guest
virtual platform. It's "random" in the same way as the address of the
UART picked by an SoC designer is.

It should be a #define for sure though.

Hardcoding this for dom0 seems like a mistake though, we need a way to
find a hole in the paddr space...

Ian.

Re: [PATCH RFC 14/15] libxl: build a device tree for ARM guests

[Julien Grall]

On 10/15/2013 11:00 AM, Ian Campbell wrote:
> On Tue, 2013-10-15 at 00:11 +0100, Julien Grall wrote:
>> On 10/07/2013 05:40 PM, Ian Campbell wrote:
>>> Uses xc_dom_devicetree_mem which was just added. The call to this needs to be
>>> carefully sequenced to be after xc_dom_parse_image (so we can tell which kind
>>> of guest we are building, although we don't use this yet) and before
>>> xc_dom_mem_init which tries to decide where to place the FDT in guest RAM.
>>>
>>> Removes libxl_noarch which would only have been used by IA64 after this
>>> change. Remove IA64 as part of this patch.
>>>
>>> There is no attempt to expose this as a configuration setting for the user.
>>>
>>> Includes a debug hook to dump the dtb to a file for inspection.
>>>
>>> TODO:
>>> - Hardcoded armv8 bits need abstracting. Perhaps e.g. read CPU compatiblity
>>>    node from sysfs?
>>
>> I'm wondering if it's usefull to have the property compatible. From 
>> Linux documentation:
>>    So under /cpus, you are supposed to create a node for every CPU on
>>    the machine. There is no specific restriction on the name of the
>>    CPU, though it's common to call it <architecture>,<core>. For
>>    example, Apple uses PowerPC,G5 while IBM uses PowerPC,970FX.
>>    However, the Generic Names convention suggests that it would be
>>    better to simply use 'cpu' for each cpu node and use the compatible
>>    property to identify the specific cpu core.
> 
> Hrm this seems to imply that either you should name the node
> <architecture>,<core> *or* you should call it cpu and have a
> compatibility property <architecture>,<core>.
> 
>> Linux doesn't seems to retrieve the compatible node and Freebsd also.
>> The only way to read this property is from /proc/device-tree. But this 
>> directory exists only if CONFIG_PROC_DEVICETREE=y.
> 
> Yes.
> 
> I suppose we could try /proc/device-tree and fall back to /proc/cpuinfo
> (which should at least get us v7 vs v8) and finally fallback to a
> hardcoded value based on the architecture the tools are compiled for
> (which is lame, but better than nothing?).

Reading the value from /proc/device-tree is a bit complicate. You need
to parse each directory to find a CPU.

>>> - Try it with armv7
>>
>> The previous point will be the same for the timer and the GIC.
> 
> Yes.
> 
>>> +static int fdt_property_interrupts(libxl__gc *gc, void *fdt,
>>> +                                   gic_interrupt_t *intr,
>>> +                                   unsigned num_irq)
>>> +{
>>> +    int res;
>>> +
>>> +    res = fdt_property(fdt, "interrupts", intr, sizeof (intr[0]) * num_irq);
>>> +    if ( res )
>>> +        return res;
>>> +
>>> +    res = fdt_property_cell(fdt, "interrupt-parent", PHANDLE_GIC);
>>> +
>>> +    return res;
>>> +}
>>
>> You don't need this function if the interrupt-parent properties is set 
>> in the root node. Which is the case below in make_root_properties.
> 
> OK. The reason we have this for dom0 is that we can't control whether
> the host DTB has interrupt-parent or not?

Right. Some device tree have a simple-bus where all nodes live and also
the interrupt-property

/ {

  smb {
    compatible = "simple,bus"
    interrupt-parent = &gic
    ...
  }
}

During dom0 DT creation, Xen will add the node in / which may not have
the right property.

>>> +    res = fdt_property_cell(fdt, "cpu_on", 0x2);
>>> +    if ( res )
>>> +        return res;
>>
>> Can we export include/asm-arm/psci.h and reuse the value here?
> 
> I suppose we ought to, since Xen is the one implementing the actual
> functionality. I notice that even Xen itself isn't using those #defines
> (nothing is AFAICT!)

It's used in traps.c via in the macro PSCI and in domain_build.c when
the PSCI node is creating.

>>> +    //DPRINT("  Grant table range: 0xb0000000-0x20000\n");
>>> +    /* reg 0 is grant table space */
>>> +    res = fdt_property_regs(gc, fdt, ROOT_ADDRESS_CELLS, ROOT_SIZE_CELLS,
>>> +                            1,
>>> +                            (uint64_t)0xb0000000,
>>
>> I still don't know where this value comes from... if it's a random 
>> value, can we autogenerate it?
> 
> It's an arbitrary value which we picked when we defined our guest
> virtual platform. It's "random" in the same way as the address of the
> UART picked by an SoC designer is.
> 
> It should be a #define for sure though.

If you choose to hardcode this address, can you add a TODO? So we won't
forget later.

-- 
Julien Grall

Re: [PATCH RFC 14/15] libxl: build a device tree for ARM guests

[Ian Campbell]

On Tue, 2013-10-15 at 14:23 +0100, Julien Grall wrote:
> >>> +    res = fdt_property_cell(fdt, "cpu_on", 0x2);
> >>> +    if ( res )
> >>> +        return res;
> >>
> >> Can we export include/asm-arm/psci.h and reuse the value here?
> > 
> > I suppose we ought to, since Xen is the one implementing the actual
> > functionality. I notice that even Xen itself isn't using those #defines
> > (nothing is AFAICT!)
> 
> It's used in traps.c via in the macro PSCI and in domain_build.c when
> the PSCI node is creating.

Ah, macros ;-) Also I was grepping for __PSCI_cpu_suspend which isn't
used...

> 
> >>> +    //DPRINT("  Grant table range: 0xb0000000-0x20000\n");
> >>> +    /* reg 0 is grant table space */
> >>> +    res = fdt_property_regs(gc, fdt, ROOT_ADDRESS_CELLS, ROOT_SIZE_CELLS,
> >>> +                            1,
> >>> +                            (uint64_t)0xb0000000,
> >>
> >> I still don't know where this value comes from... if it's a random 
> >> value, can we autogenerate it?
> > 
> > It's an arbitrary value which we picked when we defined our guest
> > virtual platform. It's "random" in the same way as the address of the
> > UART picked by an SoC designer is.
> > 
> > It should be a #define for sure though.
> 
> If you choose to hardcode this address, can you add a TODO? So we won't
> forget later.

Unless you have a use case for making this value changeable there is
nothing wrong with us picking a number and using it, I don't think there
is any TODO here.

Ian.

Re: [PATCH RFC 14/15] libxl: build a device tree for ARM guests

[Julien Grall]

On 10/15/2013 02:33 PM, Ian Campbell wrote:
> 
> Unless you have a use case for making this value changeable there is
> nothing wrong with us picking a number and using it, I don't think there
> is any TODO here.

What about a large memory RAM for the domain? With this solution the
only space available is between 0x80000000 (base ram address) and
0xb0000000 (base grant table). So we are limiting the domain to 768MB of
RAM.

I think this limitation should be at least documented somewhere.

-- 
Julien Grall

Re: [PATCH RFC 14/15] libxl: build a device tree for ARM guests

[Ian Campbell]

On Tue, 2013-10-15 at 14:49 +0100, Julien Grall wrote:
> On 10/15/2013 02:33 PM, Ian Campbell wrote:
> > 
> > Unless you have a use case for making this value changeable there is
> > nothing wrong with us picking a number and using it, I don't think there
> > is any TODO here.
> 
> What about a large memory RAM for the domain? With this solution the
> only space available is between 0x80000000 (base ram address) and
> 0xb0000000 (base grant table). So we are limiting the domain to 768MB of
> RAM.

We can either move the grant table to a different hardcoded address to
avoid any clash or use multiple memory banks.

> I think this limitation should be at least documented somewhere.

In general the virtual platform could do with being written down more
clearly..

Re: [PATCH RFC 14/15] libxl: build a device tree for ARM guests

[Julien Grall]

On 10/07/2013 05:40 PM, Ian Campbell wrote:
> Uses xc_dom_devicetree_mem which was just added. The call to this needs to be
> carefully sequenced to be after xc_dom_parse_image (so we can tell which kind
> of guest we are building, although we don't use this yet) and before
> xc_dom_mem_init which tries to decide where to place the FDT in guest RAM.
> 
> Removes libxl_noarch which would only have been used by IA64 after this
> change. Remove IA64 as part of this patch.
> 
> There is no attempt to expose this as a configuration setting for the user.
> 
> Includes a debug hook to dump the dtb to a file for inspection.
> 
> TODO:
> - Hardcoded armv8 bits need abstracting. Perhaps e.g. read CPU compatiblity
>   node from sysfs?
> - Try it with armv7
> - Debug hook should be #ifdef DEBUG?
> - Various values (e.g. GIC base addresses, evtchn PPI) should be passed
>   down to the hypervisor by some mechanism I've not decided on yet. Perhaps
>   domctl but maybe better would be via the HVM save format to more easily
>   support migration of the settings?
> - TODOs and //comments in the code
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
[..]

> +int libxl__arch_domain_configure(libxl__gc *gc,
> +                                 libxl_domain_build_info *info,
> +                                 struct xc_dom_image *dom)
> +{
[..]

> +    res = make_memory_node(gc, fdt, 0x80000000UL, 0x8000000UL);

dom->rambase_pfn << PAGE_SHIFT.
and the size should not be hardcoded.

-- 
Julien Grall

[PATCH RFC 15/15] libxl: remove spurious newline from LOG() message

[Ian Campbell]

The macro already includes this.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 tools/libxl/libxl_dom.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 78ba8f1..25bb182 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -342,7 +342,7 @@ int libxl__build_pv(libxl__gc *gc, uint32_t domid,
         return ERROR_FAIL;
     }
 
-    LOG(DEBUG, "pv kernel mapped %d path %s\n", state->pv_kernel.mapped, state->pv_kernel.path);
+    LOG(DEBUG, "pv kernel mapped %d path %s", state->pv_kernel.mapped, state->pv_kernel.path);
     if (state->pv_kernel.mapped) {
         ret = xc_dom_kernel_mem(dom,
                                 state->pv_kernel.data,
-- 
1.7.10.4