Re: [PATCH v6 05/22] arm/acpi: Prepare MADT table for Dom0

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

Hi Shannon,

On 17/03/2016 09:40, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
>
> Copy main MADT table contents and distributor subtable from physical
> ACPI MADT table. Make other subtables through the callback of
> gic_hw_ops.
>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

With the 2 changes mentioned below:

Acked-by: Julien Grall <julien.grall@arm.com>

> ---
>   xen/arch/arm/domain_build.c | 50 +++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 50 insertions(+)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index d9b7213..ed257e0 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -1357,6 +1357,52 @@ static int prepare_dtb(struct domain *d, struct kernel_info *kinfo)
>   }
>
>   #ifdef CONFIG_ACPI
> +static int acpi_create_madt(struct domain *d, struct membank tbl_add[])
> +{
> +    struct acpi_table_header *table = NULL;
> +    struct acpi_table_madt *madt = NULL;
> +    struct acpi_subtable_header *header;
> +    struct acpi_madt_generic_distributor *gicd;
> +    u32 table_size = sizeof(struct acpi_table_madt);
> +    u32 offset = acpi_get_table_offset(tbl_add, TBL_MADT);
> +    acpi_status status;
> +    u8 *base_ptr, checksum;
> +
> +    status = acpi_get_table(ACPI_SIG_MADT, 0, &table);
> +
> +    if ( ACPI_FAILURE(status) )
> +    {
> +        const char *msg = acpi_format_exception(status);
> +
> +        printk("Failed to get MADT table, %s\n", msg);
> +        return -EINVAL;
> +    }
> +
> +    base_ptr = d->arch.efi_acpi_table + offset;
> +    ACPI_MEMCPY(base_ptr, table, table_size);
> +
> +    /* Add Generic Distributor */
> +    header = acpi_table_get_entry_madt(ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR, 0);
> +    if ( !header )
> +        panic("Can't get GICD entry");

Please avoid panic when it's possible to return an error.

> +    gicd = container_of(header, struct acpi_madt_generic_distributor, header);
> +    ACPI_MEMCPY(base_ptr + table_size, gicd,
> +                sizeof(struct acpi_madt_generic_distributor));
> +    table_size += sizeof(struct acpi_madt_generic_distributor);
> +
> +    /* Add other subtables*/

NIT: Missing space before */

> +    table_size += gic_make_hwdom_madt(d, offset + table_size);
> +
> +    madt = (struct acpi_table_madt *)base_ptr;
> +    madt->header.length = table_size;
> +    checksum = acpi_tb_checksum(ACPI_CAST_PTR(u8, madt), table_size);
> +    madt->header.checksum -= checksum;
> +
> +    tbl_add[TBL_MADT].start = d->arch.efi_acpi_gpa + offset;
> +    tbl_add[TBL_MADT].size = table_size;
> +
> +    return 0;
> +}
>
>   static int acpi_create_fadt(struct domain *d, struct membank tbl_add[])
>   {

[...]

Regards,

-- 
Julien Grall

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