Re: [Xen-ia64-devel] [PATCH 14/14] memmap: allow huge size efi memory map of real machine

[Alex Williamson <alex.williamson@xxxxxx>]

On Thu, 2007-05-17 at 19:43 +0900, Isaku Yamahata wrote:
> 14 / 14 

Hi Isaku,

   I'm having some troubles with this one.  It works fine on my main
test system, but on another system, that also does not exceed the
available 64 entry MDT, it fails to boot xen.  I found this bug:

diff -r 23fde14d35b5 xen/arch/ia64/xen/dom_fw_utils.c
--- a/xen/arch/ia64/xen/dom_fw_utils.c  Mon May 21 14:30:09 2007 -0600
+++ b/xen/arch/ia64/xen/dom_fw_utils.c  Tue May 22 16:36:53 2007 -0600
@@ -280,17 +280,17 @@ dom_fw_setup(domain_t *d, unsigned long 
         fw_tables_size = sizeof(*fw_tables) +
             fw_tables->num_mds * sizeof(fw_tables->efi_memmap[0]);
 
+        /* copy fw_tables into domain pseudo physical address space */
+        for (gpaddr = FW_TABLES_BASE_PADDR;
+             gpaddr < fw_tables->fw_end_paddr;
+             gpaddr += PAGE_SIZE)
+            assign_new_domain_page_if_dom0(d, gpaddr);
+
         /* clear domain builder internal use member */
         fw_tables->fw_tables_size = 0;
         fw_tables->fw_end_paddr = 0;
         fw_tables->fw_tables_end_paddr = 0;
         fw_tables->num_mds = 0;
-
-        /* copy fw_tables into domain pseudo physical address space */
-        for (gpaddr = FW_TABLES_BASE_PADDR;
-             gpaddr < fw_tables->fw_end_paddr;
-             gpaddr += PAGE_SIZE)
-            assign_new_domain_page_if_dom0(d, gpaddr);
 
         dom_fw_copy_to(d, FW_TABLES_BASE_PADDR, fw_tables, fw_tables_size);
         xfree(fw_tables);

Even with that fix, my system hangs at the xfree(fw_tables) call near
the end of this chunk.  If I skip that xfree(), we hang at the next call
to xfree.  Looks like we corrupted memory somewhere, but I haven't found
it yet.

   Also, this looks a little strange to me:

+        /* estimate necessary efi memmap size and allocate memory for it */
+        fw_tables_size = sizeof(*fw_tables) +
+            NUM_MEM_DESCS * sizeof(fw_tables->efi_memmap[0]) +
+            ia64_boot_param->efi_memdesc_size;

Shouldn't this be something like:

fw_tables_size = sizeof(*fw_tables) +
 ((ia64_boot_param->efi_memmap_size / ia64_boot_param->efi_memdesc_size)
  + NUM_MEM_DESCS) * sizeof(fw_tables->efi_memmap[0]);

In this case NUM_MEM_DESCS should be very small since it's only adding
the descriptors we add for xen and the rest is dynamically sized based
on the number of entries in the MDT provided by firmware.  Thanks,

        Alex

-- 
Alex Williamson                             HP Open Source & Linux Org.


_______________________________________________
Xen-ia64-devel mailing list
Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ia64-devel