Re: [Xen-devel] [PATCH v2 16/17] libxc/xc_dom_arm: Copy ACPI tables to guest space

[Julien Grall <julien.grall@xxxxxxx>]

Hi Shannon,

On 23/06/2016 04:17, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@xxxxxxxxxx>
>
> Copy all the ACPI tables to guest space so that UEFI or guest could
> access them.
>
> Signed-off-by: Shannon Zhao <shannon.zhao@xxxxxxxxxx>
> ---
>  tools/libxc/xc_dom_arm.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
>
> diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c
> index 64a8b67..6a0a5b7 100644
> --- a/tools/libxc/xc_dom_arm.c
> +++ b/tools/libxc/xc_dom_arm.c
> @@ -63,6 +63,47 @@ static int setup_pgtables_arm(struct xc_dom_image *dom)
>
>  /* ------------------------------------------------------------------------ */
>
> +static int xc_dom_copy_acpi(struct xc_dom_image *dom)
> +{
> +    int rc, i;
> +    uint32_t pages_num = ROUNDUP(dom->acpitable_size, XC_PAGE_SHIFT) >>
> +                         XC_PAGE_SHIFT;
> +    const xen_pfn_t base = GUEST_ACPI_BASE >> XC_PAGE_SHIFT;
> +    xen_pfn_t *p2m;
> +    void *acpi_pages;
> +
> +    p2m = malloc(pages_num * sizeof(*p2m));
> +    for (i = 0; i < pages_num; i++)
> +        p2m[i] = base + i;
> +
> +    rc = xc_domain_populate_physmap_exact(dom->xch, dom->guest_domid,
> +                                          pages_num, 0, 0, p2m);
Hmmmm... it looks like this is working because libxl is setting the 
maximum size of the domain with some slack (1MB). However, I guess the 
slack was for something else. Wei, Stefano, Ian, can you confirm?
> +    if ( rc )
> +    {
> +        DOMPRINTF("%s: xc_domain_populate_physmap_exact failed with %d",
> +                  __FUNCTION__, rc);
> +        goto out;
> +    }
> +
> +    acpi_pages = xc_map_foreign_range(dom->xch, dom->guest_domid,
> +                                      PAGE_SIZE * pages_num,
> +                                      PROT_READ | PROT_WRITE, base);
> +    if ( !acpi_pages )
> +    {
> +        DOMPRINTF("%s Can't map acpi_pages", __FUNCTION__);
> +        rc = -1;
> +        goto out;
> +    }
> +
> +    memcpy(acpi_pages, dom->acpitable_blob, dom->acpitable_size);
> +
> +out:
> +    munmap(acpi_pages, pages_num * PAGE_SIZE);
> +    free(p2m);
> +
> +    return rc;
> +}
> +
>  static int alloc_magic_pages(struct xc_dom_image *dom)
>  {
        >      int rc, i;
> @@ -100,6 +141,16 @@ static int alloc_magic_pages(struct xc_dom_image *dom)
>      xc_hvm_param_set(dom->xch, dom->guest_domid, HVM_PARAM_STORE_EVTCHN,
>              dom->xenstore_evtchn);
>
> +    if ( dom->acpitable_blob && dom->acpitable_size > 0 )
> +    {
> +        rc = xc_dom_copy_acpi(dom);
> +        if ( rc != 0 )
> +        {
> +            DOMPRINTF("Unable to copy ACPI tables");
> +            return rc;
> +        }
> +    }
alloc_magic_pages looks the wrong place with this function. Any reason 
to not have a generic ACPI blob loading in xc_dom_core.c as we do for 
devicetree?
Regards,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel