save requires a valid arch.pfn_to_mfn_frame_list_list MFN. However, there is no guarantee that this is up to date, since a previous restore is considered complete as soon as the domain is unpaused: if not paused: dominfo.unpause() dominfo.completeRestore(handler.store_mfn, handler.console_mfn) It seems that Linux is being lucky here, in that rebuilding the MFNs is the first thing it does after suspend(). On Solaris, it occurs somewhat later in the resume process due to constraints on locking within our MMU code. This doesn''t seem specific to migration either, a save just after a restore has completed can hit this race as far as I can see. I''m short on ideas that don''t involve a new interface (like the domain writing back a xenstore value when it''s done resuming). Suggestions? regards john _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
> save requires a valid arch.pfn_to_mfn_frame_list_list MFN. However, > there is no guarantee that this is up to date, since a previous > restore is considered complete as soon as the domain is unpaused: > > if not paused: > dominfo.unpause() > > dominfo.completeRestore(handler.store_mfn,handler.console_mfn)> > It seems that Linux is being lucky here, in that rebuilding the MFNsis> the first thing it does after suspend(). On Solaris, it occurssomewhat> later in the resume process due to constraints on locking within ourMMU> code. > > This doesn''t seem specific to migration either, a save just after a > restore has completed can hit this race as far as I can see. I''m short > on ideas that don''t involve a new interface (like the domain writing > back a xenstore value when it''s done resuming). Suggestions?Just ignore any new suspend request until you''ve got the frame list list rebuilt. In the Linux code this is implicit as xenbus and even interrupts aren''t re-enabled until after this. Ian _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Tue, Jan 23, 2007 at 10:15:29PM -0000, Ian Pratt wrote:> Just ignore any new suspend request until you''ve got the frame list list > rebuilt.see xc_linux_save.c: 667 live_p2m_frame_list_list 668 xc_map_foreign_range(xc_handle, dom, PAGE_SIZE, PROT_READ, 669 live_shinfo->arch.pfn_to_mfn_frame_list_list); ... 1087 if (suspend_and_state(suspend, xc_handle, io_fd, dom, &info, 1088 &ctxt)) { 1089 ERROR("Domain appears not to have suspended"); 1090 goto out; 1091 } We use the value long before the domain is told to suspend. regards john _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
> > Just ignore any new suspend request until you''ve got the frame listlist> > rebuilt. > > see xc_linux_save.c: > > 667 live_p2m_frame_list_list > 668 xc_map_foreign_range(xc_handle, dom, PAGE_SIZE,PROT_READ,> 669 live_shinfo->arch.pfn_to_mfn_frame_list_list); > > ... > > 1087 if (suspend_and_state(suspend, xc_handle, io_fd,dom,> &info, > 1088 &ctxt)) { > 1089 ERROR("Domain appears not to havesuspended");> 1090 goto out; > 1091 } > > We use the value long before the domain is told to suspend.Ah, I see what you''re getting at. We should make xc_linux_save cope more gracefully with arch.pfn_to_mfn_frame_list_list being NULL, have xc_linux_save zero the field in shared_info before writing it out (its an MFN and not valid anyhow), and then move the field''s re-initialisation a little later in the post_suspend function to close the race. Best, Ian _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Tue, Jan 23, 2007 at 11:38:29PM -0000, Ian Pratt wrote:> We should make xc_linux_save cope more gracefully with > arch.pfn_to_mfn_frame_list_list being NULL, have xc_linux_save zero the > field in shared_info before writing it out (its an MFN and not valid > anyhow), and then move the field''s re-initialisation a little later in > the post_suspend function to close the race.I''ve done some testing with the below, and it seems to do the trick. regards john # HG changeset patch # User john.levon@sun.com # Date 1169600022 28800 # Node ID 3121e0cb7be68fa46c59f61422f05adc03ec5a5e # 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. Signed-off-by: John Levon <john.levon@sun.com> 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 = 3000; + 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@lists.xensource.com http://lists.xensource.com/xen-devel
> > We should make xc_linux_save cope more gracefully with > > arch.pfn_to_mfn_frame_list_list being NULL, have xc_linux_save zerothe> > field in shared_info before writing it out (its an MFN and not valid > > anyhow), and then move the field''s re-initialisation a little laterin> > the post_suspend function to close the race. > > I''ve done some testing with the below, and it seems to do the trick.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. Thanks, Ian> regards > john > > > # HG changeset patch > # User john.levon@sun.com > # Date 1169600022 28800 > # Node ID 3121e0cb7be68fa46c59f61422f05adc03ec5a5e > # 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 useit.> > Signed-off-by: John Levon <john.levon@sun.com> > > 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 nothave> +** finished resuming from a previous restore operation, so we wait awhile> 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 = 3000; > + 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 mustbe> @@ -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@lists.xensource.com http://lists.xensource.com/xen-devel
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@sun.com # 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@sun.com> 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@lists.xensource.com http://lists.xensource.com/xen-devel
>Sure. Patch below is for 3.0.4 but should essentially apply to >xen-unstable too.Applied to -unstable and 3.0.4 thanks, S. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Hi, I Came across this thread trying to understand live migration better. I did some experiments and atleast on linux, I could do away with the waiting for the arch.pfn_to_mfn_frame_list_list to get initialized after a restore. I believe it will work with other OSes as well. I request your comments regarding this. Please see if the below patch (against xen-unstable) can be used. What I did was, simple-minded though: Instead of the guest building p2m frame-list-list and the frame-list _after_ a restore, we build it from dom0 _during_ restore. - In save, canonicalise the pfn_to_mfn_frame_list_list mfn along with the entries in that page and store them in state file. (Note that we are anyway saving canonicalised p2m_frame_list pages) - In restore, uncanonicalise pfn_to_mfn_frame_list_list pfn, and the entries of the page and copy it to appropriate place in guest memory. Also copy p2m_frame_list entries to appropriate place in guest memory. However, what I do not know is the exact reason why this initialization of pfn_to_mfn_frame_list_list and pfn_to_mfn_frame_list is being done by the guest domain after unpausing currently. Could you please clarify this? Thanks and regards Srikanth Patch for inline viewing: # HG changeset patch # User srao@srikanth # Node ID 6443c0465741c05ffef24f5ca7c7fd3907089d04 # Parent 8bc64a3a505413e4d3bd7def06539ef088f2ca0c Avoid waiting for pfn-to-mfn-frame-list-list etc., to get initialized when doing a save after restore. Signed-off-by: Srikanth S. M. <srikanth_sm@symantec.com> diff -r 8bc64a3a5054 -r 6443c0465741 linux-2.6-xen-sparse/drivers/xen/core/machine_reboot.c --- a/linux-2.6-xen-sparse/drivers/xen/core/machine_reboot.c Mon Feb 5 16:40:19 2007 +++ b/linux-2.6-xen-sparse/drivers/xen/core/machine_reboot.c Tue Feb 6 14:42:34 2007 @@ -87,10 +87,7 @@ static void post_suspend(int suspend_cancelled) { - int i, j, k, fpp; extern unsigned long max_pfn; - extern unsigned long *pfn_to_mfn_frame_list_list; - extern unsigned long *pfn_to_mfn_frame_list[]; if (suspend_cancelled) { xen_start_info->store_mfn @@ -105,20 +102,7 @@ memset(empty_zero_page, 0, PAGE_SIZE); - fpp = PAGE_SIZE/sizeof(unsigned long); - for (i = 0, j = 0, k = -1; i < max_pfn; i += fpp, j++) { - if ((j % fpp) == 0) { - k++; - pfn_to_mfn_frame_list_list[k] - virt_to_mfn(pfn_to_mfn_frame_list[k]); - j = 0; - } - pfn_to_mfn_frame_list[k][j] - 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 -r 8bc64a3a5054 -r 6443c0465741 tools/libxc/xc_linux_restore.c --- a/tools/libxc/xc_linux_restore.c Mon Feb 5 16:40:19 2007 +++ b/tools/libxc/xc_linux_restore.c Tue Feb 6 14:42:34 2007 @@ -172,6 +172,9 @@ /* A copy of the pfn-to-mfn table frame list. */ xen_pfn_t *p2m_frame_list = NULL; + /* A temporary mapping of pfn-to-mfn table frame list */ + xen_pfn_t *live_p2m_fl = NULL; + /* A temporary mapping of the guest''s start_info page. */ start_info_t *start_info; @@ -816,11 +819,41 @@ for ( i = 0; i < MAX_VIRT_CPUS; i++ ) shared_info->vcpu_info[i].evtchn_pending_sel = 0; - /* Copy saved contents of shared-info page. No checking needed. */ + /* + * Uncanonicalise fll pfn. Copy saved contents of shared-info page. + * No checking needed. + */ + shared_info->arch.pfn_to_mfn_frame_list_list + p2m[shared_info->arch.pfn_to_mfn_frame_list_list]; page = xc_map_foreign_range( xc_handle, dom, PAGE_SIZE, PROT_WRITE, shared_info_frame); memcpy(page, shared_info, PAGE_SIZE); munmap(page, PAGE_SIZE); + + /* Update fll page. Uncanonicalise entries in fll */ + page = xc_map_foreign_range(xc_handle, dom, PAGE_SIZE, PROT_WRITE, + shared_info->arch.pfn_to_mfn_frame_list_list); + if (page == NULL) { + ERROR("Cant map fll page"); + goto out; + } + if (!read_exact(io_fd, page, PAGE_SIZE)) { + ERROR("Cant read fll page"); + goto out; + } + for (i = 0; i < P2M_FLL_ENTRIES; i++) { + xen_pfn_t *p = page; + p[i] = p2m[p[i]]; + } + + /* Map pfn-to-mfn frame list pages */ + live_p2m_fl = xc_map_foreign_batch(xc_handle, dom, PROT_WRITE, + page, P2M_FLL_ENTRIES); + munmap(page, PAGE_SIZE); + if (live_p2m_fl == NULL) { + ERROR("Cant map p2m frame-list pages"); + goto out; + } /* Uncanonicalise the pfn-to-mfn table frame-number list. */ for (i = 0; i < P2M_FL_ENTRIES; i++) { @@ -832,6 +865,13 @@ p2m_frame_list[i] = p2m[pfn]; } + + /* + * Copy the p2m_frame_list (local copy) + * to the live-p2m-frame-list + */ + memcpy(live_p2m_fl, p2m_frame_list, P2M_FL_ENTRIES * sizeof (xen_pfn_t)); + munmap(live_p2m_fl, P2M_FLL_ENTRIES * PAGE_SIZE); /* Copy the P2M we''ve constructed to the ''live'' P2M */ if (!(live_p2m = xc_map_foreign_batch(xc_handle, dom, PROT_WRITE, diff -r 8bc64a3a5054 -r 6443c0465741 tools/libxc/xc_linux_save.c --- a/tools/libxc/xc_linux_save.c Mon Feb 5 16:40:19 2007 +++ b/tools/libxc/xc_linux_save.c Tue Feb 6 14:42:34 2007 @@ -426,34 +426,6 @@ } /* -** 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 ** converted into a ''canonical'' form where references to actual mfns ** are replaced with references to the corresponding pfns. @@ -616,7 +588,6 @@ } - int xc_linux_save(int xc_handle, int io_fd, uint32_t dom, uint32_t max_iters, uint32_t max_factor, uint32_t flags, int (*suspend)(int)) { @@ -714,12 +685,14 @@ } max_pfn = live_shinfo->arch.max_pfn; - - 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_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 xc_map_foreign_batch(xc_handle, dom, PROT_READ, @@ -1225,14 +1198,32 @@ 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(). + * Canonicalise fll mfn */ memcpy(page, live_shinfo, PAGE_SIZE); - ((shared_info_t *)page)->arch.pfn_to_mfn_frame_list_list = 0; + if (!translate_mfn_to_pfn( + &((shared_info_t *)page)->arch.pfn_to_mfn_frame_list_list)) { + ERROR("Cant canonicalise fll mfn"); + goto out; + } if (!write_exact(io_fd, &ctxt, sizeof(ctxt)) || !write_exact(io_fd, page, PAGE_SIZE)) { ERROR("Error when writing to state file (1) (errno %d)", errno); + goto out; + } + + /* canonicalise fll entries and send fll page */ + memcpy(page, live_p2m_frame_list_list, PAGE_SIZE); + for (i = 0; i < P2M_FLL_ENTRIES; i++) { + if (!translate_mfn_to_pfn(&((xen_pfn_t *)page)[i])) { + ERROR("Cant canonicalise fll entry %d", i); + goto out; + } + } + + if (!write_exact(io_fd, page, PAGE_SIZE)) { + ERROR("Cant write fll page to state file"); goto out; }> > 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_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Tue, Feb 06, 2007 at 08:34:21PM +0530, Srikanth SM wrote:> What I did was, simple-minded though: Instead of the guest building > p2m frame-list-list and the frame-list _after_ a restore, we build it from > dom0 _during_ restore.You broke compatibility... I have no idea why it was the guest''s responsibility, but it''s here to stay now. regards, john _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel