Re: [Xen-devel] [PATCH v4 04/24] arm/acpi: Estimate memory required for acpi/efi tables

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 28.02.16 at 12:19, <zhaoshenglong@xxxxxxxxxx> wrote:
> --- a/xen/common/efi/boot.c
> +++ b/xen/common/efi/boot.c
> @@ -13,6 +13,7 @@
>  #include <xen/multiboot.h>
>  #include <xen/pci_regs.h>
>  #include <xen/pfn.h>
> +#include <asm/acpi.h>
>  #if EFI_PAGE_SIZE != PAGE_SIZE
>  # error Cannot use xen/pfn.h here!
>  #endif
> @@ -1171,6 +1172,25 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE 
> *SystemTable)
>      for( ; ; ); /* not reached */
>  }
>  
> +#if defined (CONFIG_ACPI) && defined (CONFIG_ARM)

Are both parts really necessary? It would seem to me that the
latter should suffice.

> +/* Constant to indicate "Xen" in unicode u16 format */
> +static const u16 XEN_EFI_FW_VENDOR[] ={0x0058,0x0065,0x006E,0x0000};

This should be a wide string literal, with the variable type being
CHAR16. Also variable names shouldn't be all upper case.

> +int __init estimate_efi_size(int mem_nr_banks)
> +{
> +    int size = 0;
> +    int est_size = sizeof(EFI_SYSTEM_TABLE);
> +    int ect_size = sizeof(EFI_CONFIGURATION_TABLE);
> +    int emd_size = sizeof(EFI_MEMORY_DESCRIPTOR);
> +    int fw_vendor_size = sizeof(XEN_EFI_FW_VENDOR);
> +
> +    size += ROUNDUP((est_size + ect_size + fw_vendor_size), 8);
> +    size += ROUNDUP(emd_size * (mem_nr_banks + acpi_mem.nr_banks + 1), 8);
> +
> +    return size;
> +}

It would seem to me that none of the variables involved really
holds a signed quantity. This should be reflected by the types
chosen - likely you need s/int/size_t/ for the entire function,
except for the function parameter, which looks like it wants to
be unsigned int.

Also the initializer of "size" could be easily got rid of, and there's
a pair of pointless parentheses in the first ROUNDUP().

As to the configuration table - the sizeof() covers a single table
entry only afaict - is that really intended?

Jan


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