Re: [Xen-devel] [PATCH v6 04/16] libxl/arm: Estimate the size of ACPI tables

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Thu, Sep 22, 2016 at 08:52:21PM +0800, z00226004 wrote:
> From: Shannon Zhao <shannon.zhao@xxxxxxxxxx>
> 
> Estimate the size of ACPI tables and reserve a memory map space for ACPI
> tables.
> 
> Signed-off-by: Shannon Zhao <shannon.zhao@xxxxxxxxxx>
> ---
>  tools/libxl/libxl_arm_acpi.c | 98 
> ++++++++++++++++++++++++++++++++++++++++++++
>  xen/include/acpi/actbl1.h    |  2 +
>  2 files changed, 100 insertions(+)
> 
> diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c
> index 0851411..1dbd7e5 100644
> --- a/tools/libxl/libxl_arm_acpi.c
> +++ b/tools/libxl/libxl_arm_acpi.c
> @@ -34,12 +34,108 @@ extern const unsigned char dsdt_anycpu_arm[];
>  _hidden
>  extern const int dsdt_anycpu_arm_len;
>  
> +enum {
> +    RSDP,
> +    XSDT,
> +    GTDT,
> +    MADT,
> +    FADT,
> +    DSDT,
> +    NUMS,

NUMS is not a very good name. Maybe MAX_TABLE_NUMS or something?

Unfortunately I'm bad at naming things...

> +};
> +
> +struct acpitable {
> +    uint64_t addr;
> +    size_t size;
> +};
> +
> +static int libxl__estimate_madt_size(libxl__gc *gc,
> +                                     libxl_domain_build_info *info,
> +                                     xc_domain_configuration_t *xc_config)

Please try not to overload the return value for output. Use a dedicated
output parameter. And maybe constify things that you don't intend to
change in the function.

The rest looks reasonable to me.

Wei.

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