Re: [Xen-devel] [v3][PATCH 06/16] hvmloader: get guest memory map into memory_map[]

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 11.06.15 at 03:15, <tiejun.chen@xxxxxxxxx> wrote:
> --- a/tools/firmware/hvmloader/hvmloader.c
> +++ b/tools/firmware/hvmloader/hvmloader.c
> @@ -107,6 +107,8 @@ asm (
>      "    .text                       \n"
>      );
>  
> +struct e820map memory_map;

Imo this should live in e820.c.

> @@ -199,6 +201,39 @@ static void apic_setup(void)
>      ioapic_write(0x11, SET_APIC_ID(LAPIC_ID(0)));
>  }
>  
> +void memory_map_setup(void)

And perhaps this one too. Or if not, it should be static.

> +{
> +    unsigned int nr_entries = E820MAX, i;
> +    int rc;
> +    uint64_t alloc_addr = RESERVED_MEMORY_DYNAMIC_START - 1;
> +    uint64_t alloc_size = RESERVED_MEMORY_DYNAMIC_END - alloc_addr;
> +
> +    rc = get_mem_mapping_layout(memory_map.map, &nr_entries);
> +
> +    if ( rc )
> +    {
> +        printf("Failed to get guest memory map.\n");
> +        BUG();
> +    }
> +
> +    BUG_ON(!nr_entries);

Please be consistent: printf()+BUG() or BUG_ON(). Also I think the
two (sanity) checks above could combined into one (and the
printf() should then print both rc and nr_entries).


> @@ -368,6 +379,21 @@ uuid_to_string(char *dest, uint8_t *uuid)
>      *p = '\0';
>  }
>  
> +int get_mem_mapping_layout(struct e820entry entries[], uint32_t *max_entries)

Again no need for a fixed width type here afaict.

Jan


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