From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: [PATCH] hypercall/mem: Introduce XENMEM_machphys_compat_mfn_list Date: Fri, 18 Apr 2014 09:44:31 -0400 Message-ID: <20140418134431.GG4429@phenom.dumpdata.com> References: <1397750005-6963-1-git-send-email-andrew.cooper3@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <1397750005-6963-1-git-send-email-andrew.cooper3@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Andrew Cooper Cc: Keir Fraser , Ian Campbell , Tim Deegan , Ian Jackson , Xen-devel , Jan Beulich List-Id: xen-devel@lists.xenproject.org On Thu, Apr 17, 2014 at 04:53:25PM +0100, Andrew Cooper 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. first > > 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 ^^^^ - manner > 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. Ouch1 > > This patch introduces XENMEM_machphys_compat_mfn_list which permits a 64bit > toolstack to access the compat m2p mfn list, for the purpose of correctly > identifying Xen entries in a 32bit guest. > > It is worth noting for completeness that 64bit PV guests don't have any of > these games to play. The Xen mappings are present at a known location in all > L4 tables, so can be safely shot by 32 and 64bit toolstacks without looking at > where the mapping points to. > > Signed-off-by: Andrew Cooper > CC: Keir Fraser > CC: Jan Beulich > CC: Tim Deegan > CC: Ian Campbell > CC: Ian Jackson > > --- > > I am happy for this to live as part of my "migration v2" series, but is > presented here for individual review. > > Changes in v2: > * Don't alias other local scope variables in subarch_memory_op > --- > xen/arch/x86/x86_64/compat/mm.c | 1 + > xen/arch/x86/x86_64/mm.c | 30 +++++++++++++++++++++++++++++- > xen/include/public/memory.h | 10 ++++++++++ > 3 files changed, 40 insertions(+), 1 deletion(-) > > diff --git a/xen/arch/x86/x86_64/compat/mm.c b/xen/arch/x86/x86_64/compat/mm.c > index 0a8408b..6d3bc31 100644 > --- a/xen/arch/x86/x86_64/compat/mm.c > +++ b/xen/arch/x86/x86_64/compat/mm.c > @@ -146,6 +146,7 @@ int compat_arch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg) > } > > case XENMEM_machphys_mfn_list: > + case XENMEM_machphys_compat_mfn_list: > { > unsigned long limit; > compat_pfn_t last_mfn; > diff --git a/xen/arch/x86/x86_64/mm.c b/xen/arch/x86/x86_64/mm.c > index 71ae519..ff96997 100644 > --- a/xen/arch/x86/x86_64/mm.c > +++ b/xen/arch/x86/x86_64/mm.c > @@ -953,7 +953,7 @@ long subarch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg) > struct xen_machphys_mfn_list xmml; > l3_pgentry_t l3e; > l2_pgentry_t l2e; > - unsigned long v; > + unsigned long v, limit; > xen_pfn_t mfn, last_mfn; > unsigned int i; > long rc = 0; > @@ -1000,6 +1000,34 @@ long subarch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg) > > break; > > + case XENMEM_machphys_compat_mfn_list: > + if ( copy_from_guest(&xmml, arg, 1) ) > + return -EFAULT; > + > + limit = (unsigned long)(compat_machine_to_phys_mapping + max_page); > + if ( limit > RDWR_COMPAT_MPT_VIRT_END ) > + limit = RDWR_COMPAT_MPT_VIRT_END; > + for ( i = 0, v = RDWR_COMPAT_MPT_VIRT_START, last_mfn = 0; > + (i != xmml.max_extents) && (v < limit); > + i++, v += 1 << L2_PAGETABLE_SHIFT ) > + { > + l2e = compat_idle_pg_table_l2[l2_table_offset(v)]; > + if ( l2e_get_flags(l2e) & _PAGE_PRESENT ) > + mfn = l2e_get_pfn(l2e); > + else > + mfn = last_mfn; > + ASSERT(mfn); > + if ( copy_to_guest_offset(xmml.extent_start, i, &mfn, 1) ) > + return -EFAULT; > + last_mfn = mfn; > + } > + > + xmml.nr_extents = i; > + if ( __copy_to_guest(arg, &xmml, 1) ) > + rc = -EFAULT; > + > + break; > + > case XENMEM_get_sharing_freed_pages: > return mem_sharing_get_nr_saved_mfns(); > > diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h > index f19ac14..be49beb 100644 > --- a/xen/include/public/memory.h > +++ b/xen/include/public/memory.h > @@ -465,6 +465,16 @@ DEFINE_XEN_GUEST_HANDLE(xen_mem_sharing_op_t); > * The zero value is appropiate. > */ > > +/* > + * For a compat toolstack domain, this is indentical to ^^^^^^^^^ - identical > + * XENMEM_machphys_mfn_list. > + * > + * For a non compat toolstack domain, this functions similarly to > + * XENMEM_machphys_mfn_list, but returns the mfns making up the compatibility > + * m2p table. > + */ > +#define XENMEM_machphys_compat_mfn_list 25 > + > #endif /* defined(__XEN__) || defined(__XEN_TOOLS__) */ > > #endif /* __XEN_PUBLIC_MEMORY_H__ */ > -- > 1.7.10.4 > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel