Re: [PATCH v5 16/19] libxc: arm: allow passing a device tree blob to the guest

[Julien Grall <julien.grall@linaro.org>]

On 11/13/2013 06:11 PM, Ian Campbell wrote:
> Placement of the blob in guest RAM is simplistic but will do for now.
>
> This operation is only supported on ARM.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
> ---
> v4: Return EINVAL on non-ARM platforms (which do not support device-tree)
>      Slightly less rubbish DTB placement which isn't perfect but at least now
>      doesn't fail with <128MB of RAM.
> ---
>   tools/libxc/xc_dom.h      |    8 +++++++
>   tools/libxc/xc_dom_arm.c  |   22 ++++++++++++++++++-
>   tools/libxc/xc_dom_core.c |   53 +++++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 82 insertions(+), 1 deletion(-)
>

[..]

> diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c
> index 9f3fdd3..4c59177 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 = SCTLR_GUEST_INIT;
>
> @@ -191,6 +192,25 @@ int arch_setup_meminit(struct xc_dom_image *dom)
>               0, 0, &dom->p2m_host[i]);
>       }
>
> +    if ( dom->devicetree_blob )
> +    {
> +        const uint64_t rambase = dom->rambase_pfn << XC_PAGE_SHIFT;
> +        const uint64_t ramend = rambase + ( dom->total_pages << XC_PAGE_SHIFT );
> +        const uint64_t dtbsize = ( dom->devicetree_size + 3 ) & ~0x3;
> +
> +        /* Place at 128MB if there is sufficient RAM */
> +        if (ramend >= rambase + 128*1024*1024 + dtbsize )

Minor coding style error : if ( ... )

Also, I'm wondering if (128 << 20) will be more clear?

[..]

-- 
Julien Grall