Re: [Xen-devel] [PATCH v4 13/23] xen/arm: implement construct_domU

[Julien Grall <julien.grall@xxxxxxx>]

Hi,

On 05/10/2018 19:47, Stefano Stabellini wrote:
> Similar to construct_dom0, construct_domU creates a barebone DomU guest.
>
> The device tree node passed as argument is compatible "xen,domain", see
> docs/misc/arm/device-tree/booting.txt.
>
> Add const to kernel_probe dt_device_node parameter.
This likely belongs to patch #7 where the parameter was added.

> Signed-off-by: Stefano Stabellini <stefanos@xxxxxxxxxx>
>
> ---
> Changes in v4:
> - constify kernel_probe
> - change title
> - better error messages and printed info
> - 64bit memory
>
> Changes in v3:
> - move setting type before allocate_memory
> - add ifdef around it and a comment
>
> Changes in v2:
> - rename mem to memory
> - make cpus and memory mandatory
> - remove wront comment from commit message
> - cpus and memory are read as integers
> - read the vpl011 option
> ---
>   xen/arch/arm/domain_build.c | 37 ++++++++++++++++++++++++++++++++++---
>   xen/arch/arm/kernel.c       |  3 ++-
>   xen/arch/arm/kernel.h       |  2 +-
>   3 files changed, 37 insertions(+), 5 deletions(-)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 547b624..efb530a 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -369,7 +369,6 @@ static void __init allocate_memory_11(struct domain *d,
>       }
>   }
>   
> -#if 0
Please add a word about this change in the commit message.

>   static bool __init allocate_bank_memory(struct domain *d,
>                                           struct kernel_info *kinfo,
>                                           gfn_t sgfn,
> @@ -450,7 +449,6 @@ fail:
>               (unsigned long)kinfo->unassigned_mem >> 10);
>       BUG();
>   }
> -#endif
>   
>   static int __init write_properties(struct domain *d, struct kernel_info *kinfo,
>                                      const struct dt_device_node *node)
> @@ -2294,7 +2292,40 @@ static int __init __construct_domain(struct domain *d, 
> struct kernel_info *kinfo
>   static int __init construct_domU(struct domain *d,
>                                    const struct dt_device_node *node)
>   {
> -    return -ENOSYS;
> +    struct kernel_info kinfo = {};
> +    int rc;
> +    u64 mem;
> +
> +    rc = dt_property_read_u64(node, "memory", &mem);
> +    if ( !rc )
> +    {
> +        printk("Error building DomU: cannot read \"memory\" property\n");
> +        return -EINVAL;
> +    }
> +    kinfo.unassigned_mem = (paddr_t)mem << 10;
I noticed I forgot to answer to:
"KB() only works for numbers, it is defined as: (_AC(_kb, ULL) << 10)"

unsigned long long is always going to be bigger than paddr_t. Also, we 
already use MB(...) in similar situation. So I am not sure to understand 
your concern here.
> +
> +    printk("*** LOADING DOMU cpus=%u memory=%luKB ***\n", d->max_vcpus, mem);
> +
> +    d->vcpu = xzalloc_array(struct vcpu *, d->max_vcpus);
> +    if ( !d->vcpu )
> +        return -ENOMEM;;
d->vcpu is already allocated by domain_create.

> +    if ( vcpu_create(d, 0, 0) == NULL )
> +        return -ENOMEM;
> +    d->max_pages = ~0U;
> +
> +    kinfo.d = d;
> +
> +    rc = kernel_probe(&kinfo, node);
> +    if ( rc < 0 )
> +        return rc;
> +
> +#ifdef CONFIG_ARM_64
> +    /* type must be set before allocate memory */
> +    d->arch.type = kinfo.type;
> +#endif
> +    allocate_memory(d, &kinfo);
> +
> +    return __construct_domain(d, &kinfo);
>   }
>   
>   void __init create_domUs(void)
> diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
> index e5b8213..2239a07 100644
> --- a/xen/arch/arm/kernel.c
> +++ b/xen/arch/arm/kernel.c
> @@ -421,7 +421,8 @@ static int __init kernel_zimage32_probe(struct kernel_info 
> *info,
>       return 0;
>   }
>   
> -int __init kernel_probe(struct kernel_info *info, struct dt_device_node *domain)
> +int __init kernel_probe(struct kernel_info *info,
> +                        const struct dt_device_node *domain)
>   {
>       struct bootmodule *mod = NULL;
>       struct bootcmdline *cmd = NULL;
> diff --git a/xen/arch/arm/kernel.h b/xen/arch/arm/kernel.h
> index 4a65289..4320f72 100644
> --- a/xen/arch/arm/kernel.h
> +++ b/xen/arch/arm/kernel.h
> @@ -55,7 +55,7 @@ struct kernel_info {
>    *  ->type
>    *  ->load hook, and sets loader specific variables ->zimage
>    */
> -int kernel_probe(struct kernel_info *info, struct dt_device_node *domain);
> +int kernel_probe(struct kernel_info *info, const struct dt_device_node 
> *domain);
>   
>   /*
>    * Loads the kernel into guest RAM.
Cheers,

--
Julien Grall

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