[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] hypercall/mem: Introduce XENMEM_machphys_compat_mfn_list
On 17/04/14 12:42, Jan Beulich wrote: >>>> On 17.04.14 at 13:10, <andrew.cooper3@xxxxxxxxxx> wrote: >> To correctly migrate a PV guest, the toolstack must remove Xen mappings from >> the guest pagetables. For 32bit PV guests, the pagetables cannot be walked >> from the top so upon encountering an L2 table, the toolstack must decide >> whether it contains Xen mappings or not, to avoid corrupting L2s without Xen >> mappings. >> >> The migration code performs this search efficiently by knowing that the Xen >> mappings will start at a known L2e and point to a known mfn, which will be >> the >> fist mfn in the m2p table. >> >> Unfortunately there are two m2p tables in use; a regular and a compatibility >> one. The toolstack looks for the first mfn of its own m2p table in the >> guest >> pagetables. This only works if the toolstack is the same bitness as the >> 32bit >> domain being migrated, and leaves a problem for 64bit toolstacks which will >> never be able to find its regular m2p in a compat guest. >> >> It appears that this bug for 64bit toolstacks was discovered, but hacked >> around in an unsafe manor. The code currently shoots any invalid L2es and >> doesn't report a failure for L2 tables in a 32 bit guest, even after the >> guest >> is paused. This means that non Xen entries which should fail the migration >> don't, and the guest will resume on the far side with unexpectedly fewer >> present pagetable entries. > So since that hack isn't being removed here, do you have plans to do > so? I am rewriting migration basically from scratch, which will involve removing/replacing almost all of xc_domain_{save,restore}.c Specifically fixing this bug here, just to delete the fix in the near future seems pointless. I suppose the one piece of information I missed was that I am happy to keep this as part of my migration series, but would like it to be reviewed slightly independently. > Knowing where that is would help judging on the need for this > patch (and I don't want to waste time hunting for it knowing that you > already spotted it)... The bug is in canonicalize_pagetable() in xc_domain_save.c The search for affected L2 tables at line 445 will never succeed for a 64 bit guest, and the decision at line 500 will fail to identify any errors which should be fatal. > >> + case XENMEM_machphys_compat_mfn_list: >> + { >> + unsigned long limit, last_mfn; > You re-use other function scope variables - why not also last_mfn? > Which then raises the question whether limit shouldn't be simply > added there too. > > Jan > I didn't spot last_mfn in the other scope variables. I shall use the outer scope ones ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |