WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] [PATCH] save/restore race

On Wed, Jan 24, 2007 at 02:19:42AM -0000, Ian Pratt wrote:

> 30 seconds is quite a time to wait... Wouldn't a second be more
> appropriate?
> 
> While you're at it, I'd be grateful if you could move the shared_info
> assignment in linux's post_suspend() and setup_arch() [i386 and x86_64]
> below the list building code.

Sure. Patch below is for 3.0.4 but should essentially apply to
xen-unstable too.

thanks
john

# HG changeset patch
# User john.levon@xxxxxxx
# Date 1169607991 28800
# Node ID b5ac8fda95bc5f9b789df80095d59e23e7f40205
# Parent  a75413c0072fa5b892bd8b6a05c1f1d3435bb093
Close save-after-restore race.

Make xc_linux_save() wait for the frame_list_list MFN to be updated by the
domain before trying to use it. Make Linux set the top-level MFN /after/
updating the other MFN lists.

Signed-off-by: John Levon <john.levon@xxxxxxx>

diff --git a/linux-2.6-xen-sparse/arch/i386/kernel/setup-xen.c 
b/linux-2.6-xen-sparse/arch/i386/kernel/setup-xen.c
--- a/linux-2.6-xen-sparse/arch/i386/kernel/setup-xen.c
+++ b/linux-2.6-xen-sparse/arch/i386/kernel/setup-xen.c
@@ -1777,8 +1777,6 @@ void __init setup_arch(char **cmdline_p)
                 * frames that make up the p2m table. Used by save/restore
                 */
                pfn_to_mfn_frame_list_list = alloc_bootmem_low_pages(PAGE_SIZE);
-               HYPERVISOR_shared_info->arch.pfn_to_mfn_frame_list_list =
-                    virt_to_mfn(pfn_to_mfn_frame_list_list);
 
                fpp = PAGE_SIZE/sizeof(unsigned long);
                for (i=0, j=0, k=-1; i< max_pfn; i+=fpp, j++) {
@@ -1795,6 +1793,8 @@ void __init setup_arch(char **cmdline_p)
                                virt_to_mfn(&phys_to_machine_mapping[i]);
                }
                HYPERVISOR_shared_info->arch.max_pfn = max_pfn;
+               HYPERVISOR_shared_info->arch.pfn_to_mfn_frame_list_list =
+                    virt_to_mfn(pfn_to_mfn_frame_list_list);
        }
 
        /*
diff --git a/linux-2.6-xen-sparse/arch/x86_64/kernel/setup-xen.c 
b/linux-2.6-xen-sparse/arch/x86_64/kernel/setup-xen.c
--- a/linux-2.6-xen-sparse/arch/x86_64/kernel/setup-xen.c
+++ b/linux-2.6-xen-sparse/arch/x86_64/kernel/setup-xen.c
@@ -862,8 +862,6 @@ void __init setup_arch(char **cmdline_p)
                          * save/restore.
                         */
                        pfn_to_mfn_frame_list_list = 
alloc_bootmem_pages(PAGE_SIZE);
-                       HYPERVISOR_shared_info->arch.pfn_to_mfn_frame_list_list 
=
-                               virt_to_mfn(pfn_to_mfn_frame_list_list);
 
                        fpp = PAGE_SIZE/sizeof(unsigned long);
                        for (i=0, j=0, k=-1; i< end_pfn; i+=fpp, j++) {
@@ -880,6 +878,8 @@ void __init setup_arch(char **cmdline_p)
                                        
virt_to_mfn(&phys_to_machine_mapping[i]);
                        }
                        HYPERVISOR_shared_info->arch.max_pfn = end_pfn;
+                       HYPERVISOR_shared_info->arch.pfn_to_mfn_frame_list_list 
=
+                               virt_to_mfn(pfn_to_mfn_frame_list_list);
                }
 
        }
diff --git a/linux-2.6-xen-sparse/drivers/xen/core/machine_reboot.c 
b/linux-2.6-xen-sparse/drivers/xen/core/machine_reboot.c
--- a/linux-2.6-xen-sparse/drivers/xen/core/machine_reboot.c
+++ b/linux-2.6-xen-sparse/drivers/xen/core/machine_reboot.c
@@ -98,9 +98,6 @@ static void post_suspend(void)
 
        memset(empty_zero_page, 0, PAGE_SIZE);
 
-       HYPERVISOR_shared_info->arch.pfn_to_mfn_frame_list_list =
-               virt_to_mfn(pfn_to_mfn_frame_list_list);
-
        fpp = PAGE_SIZE/sizeof(unsigned long);
        for (i = 0, j = 0, k = -1; i < max_pfn; i += fpp, j++) {
                if ((j % fpp) == 0) {
@@ -113,6 +110,8 @@ static void post_suspend(void)
                        virt_to_mfn(&phys_to_machine_mapping[i]);
        }
        HYPERVISOR_shared_info->arch.max_pfn = max_pfn;
+       HYPERVISOR_shared_info->arch.pfn_to_mfn_frame_list_list =
+               virt_to_mfn(pfn_to_mfn_frame_list_list);
 }
 
 #else /* !(defined(__i386__) || defined(__x86_64__)) */
diff --git a/tools/libxc/xc_linux_save.c b/tools/libxc/xc_linux_save.c
--- a/tools/libxc/xc_linux_save.c
+++ b/tools/libxc/xc_linux_save.c
@@ -403,6 +403,33 @@ static int suspend_and_state(int (*suspe
     return -1;
 }
 
+/*
+** Map the top-level page of MFNs from the guest. The guest might not have
+** finished resuming from a previous restore operation, so we wait a while for
+** it to update the MFN to a reasonable value.
+*/
+static void *map_frame_list_list(int xc_handle, uint32_t dom,
+                                 shared_info_t *shinfo)
+{
+    int count = 100;
+    void *p;
+
+    while (count-- && shinfo->arch.pfn_to_mfn_frame_list_list == 0)
+        usleep(10000);
+
+    if (shinfo->arch.pfn_to_mfn_frame_list_list == 0) {
+        ERROR("Timed out waiting for frame list updated.");
+        return NULL;
+    }
+
+    p = xc_map_foreign_range(xc_handle, dom, PAGE_SIZE, PROT_READ,
+                             shinfo->arch.pfn_to_mfn_frame_list_list);
+
+    if (p == NULL)
+        ERROR("Couldn't map p2m_frame_list_list (errno %d)", errno);
+
+    return p;
+}
 
 /*
 ** During transfer (or in the state file), all page-table pages must be
@@ -663,14 +690,11 @@ int xc_linux_save(int xc_handle, int io_
 
     max_pfn = live_shinfo->arch.max_pfn;
 
-    live_p2m_frame_list_list =
-        xc_map_foreign_range(xc_handle, dom, PAGE_SIZE, PROT_READ,
-                             live_shinfo->arch.pfn_to_mfn_frame_list_list);
-
-    if (!live_p2m_frame_list_list) {
-        ERROR("Couldn't map p2m_frame_list_list (errno %d)", errno);
-        goto out;
-    }
+    live_p2m_frame_list_list = map_frame_list_list(xc_handle, dom,
+                                                   live_shinfo);
+
+    if (!live_p2m_frame_list_list)
+        goto out;
 
     live_p2m_frame_list =
         xc_map_foreign_batch(xc_handle, dom, PROT_READ,
@@ -1169,8 +1193,14 @@ int xc_linux_save(int xc_handle, int io_
     ctxt.ctrlreg[3] = 
         xen_pfn_to_cr3(mfn_to_pfn(xen_cr3_to_pfn(ctxt.ctrlreg[3])));
 
+    /*
+     * Reset the MFN to be a known-invalid value. See map_frame_list_list().
+     */
+    memcpy(page, live_shinfo, PAGE_SIZE);
+    ((shared_info_t *)page)->arch.pfn_to_mfn_frame_list_list = 0;
+
     if (!write_exact(io_fd, &ctxt, sizeof(ctxt)) ||
-        !write_exact(io_fd, live_shinfo, PAGE_SIZE)) {
+        !write_exact(io_fd, page, PAGE_SIZE)) {
         ERROR("Error when writing to state file (1) (errno %d)", errno);
         goto out;
     }

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