Re: [PATCH v2 33/41] arm : acpi prepare acpi tables for dom0

[Julien Grall <julien.grall@citrix.com>]

On 17/05/2015 21:04, Parth Dixit wrote:
> Map acpi tables described in uefi table to DOM0 address space
>
> Signed-off-by: Parth Dixit <parth.dixit@linaro.org>
> ---
>   xen/arch/arm/domain_build.c | 59 ++++++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 58 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index f2ca525..90bdd01 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -1225,6 +1225,50 @@ static int handle_node(struct domain *d, struct kernel_info *kinfo,
>       return res;
>   }
>   #ifdef CONFIG_ACPI
> +static int prepare_acpi(struct domain *d, struct kernel_info *kinfo, struct membank tbl_add[])
> +{
> +    unsigned long res;
> +    u64 addr, size;
> +    int i = 0;
> +
> +    addr = acpi_os_get_root_pointer();
> +    if( !addr )
> +        return -ENODEV;
> +
> +    size = sizeof(struct acpi_table_rsdp);
> +
> +    res = map_regions(d,
> +                      paddr_to_pfn(addr & PAGE_MASK),
> +                      DIV_ROUND_UP(size, PAGE_SIZE),
> +                      paddr_to_pfn(addr & PAGE_MASK));
> +    if ( res )
> +    {
> +         printk(XENLOG_ERR "Unable to map 0x%"PRIx64
> +                " - 0x%"PRIx64" in domain \n",
> +                addr & PAGE_MASK, PAGE_ALIGN(addr + size) - 1);
> +         return res;
> +    }
> +
> +    for( i = 0; i < acpi_gbl_root_table_list.count; i++ )

The description of the patch suggest that you will use acpi_mem 
introduced in patch #29 but you are using acpi_glb_root_table_list.

Please either update the commit message or change the loop.

IHMO, the latter would be cleaner.

> +    {
> +        addr = acpi_gbl_root_table_list.tables[i].address;
> +        size = acpi_gbl_root_table_list.tables[i].length;
> +        res = map_regions(d,
> +                          paddr_to_pfn(addr & PAGE_MASK),
> +                          DIV_ROUND_UP(size, PAGE_SIZE),
> +                          paddr_to_pfn(addr & PAGE_MASK));
> +        if ( res )
> +        {
> +             printk(XENLOG_ERR "Unable to map 0x%"PRIx64
> +                    " - 0x%"PRIx64" in domain \n",
> +                    addr & PAGE_MASK, PAGE_ALIGN(addr + size) - 1);
> +             return res;
> +        }
> +    }
> +
> +    return 0;
> +}
> +
>   static int estimate_acpi_size(struct domain *d,struct kernel_info *kinfo, struct membank tbl_add[])
>   {
>       int size = 0;
> @@ -1429,6 +1473,10 @@ static int create_acpi_dtb(struct domain *d, struct kernel_info *kinfo, struct m
>   {
>       return -EINVAL;
>   }
> +static int prepare_acpi(struct domain *d, struct kernel_info *kinfo, struct membank tbl_add[])
> +{
	BUG();

> +    return -EINVAL;
> +}
>   #endif
>   static int prepare_dtb(struct domain *d, struct kernel_info *kinfo)
>   {
> @@ -1647,10 +1695,19 @@ int construct_dom0(struct domain *d)
>        * as the initrd & fdt in RAM, so call it first.
>        */
>       kernel_load(&kinfo);
> +
> +    if ( !acpi_disabled )
> +    {
> +        rc = prepare_acpi(d, &kinfo, tbl_add);
> +        if ( rc < 0 )
> +            return rc;
> +    }
> +
>       /* initrd_load will fix up the fdt, so call it before dtb_load */
>       initrd_load(&kinfo);
>       /* Allocate the event channel IRQ and fix up the device tree */
> -    evtchn_fixup(d, &kinfo);
> +    if( acpi_disabled )
> +        evtchn_fixup(d, &kinfo);

This change doesn't belong to this patch.

>       dtb_load(&kinfo);
>
>       /* Now that we are done restore the original p2m and current. */
>

Regards,

-- 
Julien Grall