[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



On 2015/6/17 18:22, Jan Beulich wrote:
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.

Okay.


@@ -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.

I'd like to move this into e820.c as well.


+{
+    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).

What about this?

if ( rc || !nr_entries ) { printf("Get guest memory maps[%d] failed. (%d)\n", nr_entries, rc); BUG();
    }



@@ -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)

s/uint32_t/unsigned int/


Again no need for a fixed width type here afaict.


Thanks
Tiejun

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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.