Re: [Xen-devel] [PATCH v3 14/25] xen/arm: introduce construct_domU

[Julien Grall <julien.grall@xxxxxxx>]

Hi,

Title: You don't really introduce "construct_domU". You implement it. So 
a better title would be "Implement construct_domU".
On 01/08/18 00:27, 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.
>
> Signed-off-by: Stefano Stabellini <stefanos@xxxxxxxxxx>
>
> ---
> 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 | 36 +++++++++++++++++++++++++++++++++++-
>   1 file changed, 35 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 8f7ac54..101cca2 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -2208,7 +2208,41 @@ static int __init __construct_domain(struct domain *d, 
> struct kernel_info *kinfo
>   
>   static int __init construct_domU(struct domain *d, struct dt_device_node *node)
>   {
> -    return 0;
> +    struct kernel_info kinfo = {};
> +    int rc;
> +    u32 mem;
> +
> +    printk("*** LOADING DOMU ***\n");
> +
> +    rc = dt_property_read_u32(node, "cpus", &d->max_vcpus);
> +    if ( !rc )
> +        return -EINVAL;
For a user PoV this will be difficult to differentiate between the 
-EINVAL here and the only below.
It is probably useful to log some error message. Similarly you might 
want to add some print telling the information about the domain created.
> +
> +    rc = dt_property_read_u32(node, "memory", &mem);
I don't think 32-bit integer is going to be enough to cover all the 
case. Nowadays Arm supports up to 52-bits Address-Space.
So it would be best to cover all the case and use 64-bit here. That 
would also mean updating the documentation.
> +    if ( !rc ) > +        return -EINVAL;
> +    kinfo.unassigned_mem = (paddr_t)mem << 10;
kinfo.unassigned_mem = KB(mem);


> +
> +    d->vcpu = xzalloc_array(struct vcpu *, d->max_vcpus);
> +    if ( !d->vcpu )
> +        return -ENOMEM;;
> +    if ( alloc_vcpu(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)
Cheers,

--
Julien Grall

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