From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: Re: [PATCH]: Allow tools to map arbitrarily large machphys_mfn_list on 32bit dom0 Date: Mon, 14 Mar 2011 16:03:38 +0000 Message-ID: <1300118618.17339.2194.camel@zakaz.uk.xensource.com> References: <1300098009.17339.2110.camel@zakaz.uk.xensource.com> <1300115112.17229.78.camel@qabil.uk.xensource.com> <1300115469.17339.2188.camel@zakaz.uk.xensource.com> <1300115967.17229.82.camel@qabil.uk.xensource.com> <4D7E489302000078000365B5@vpn.id2.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <4D7E489302000078000365B5@vpn.id2.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Jan Beulich Cc: Xen Devel , Tim Deegan , Keir Fraser , "Keir@citrix.com" , Fraser , Gianni Tedesco List-Id: xen-devel@lists.xenproject.org On Mon, 2011-03-14 at 15:55 +0000, Jan Beulich wrote: > >>> On 14.03.11 at 16:19, Gianni Tedesco wrote: > > > > This permits suspend/resume to work with 32bit dom0/tools. AFAICT the > > limit to MACH2PHYS_COMPAT_NR_ENTRIES is redundant since that refers to a > > limit in 32bit guest compat mappings under 64bit hypervisors, not > > userspace where there may be gigabytes of useful virtual space available > > for this. > > > > Suggested-by: Ian Campbell > > Signed-off-by: Gianni Tedesco > > > > diff -r 8b5cbccbc654 xen/arch/x86/x86_64/compat/mm.c > > --- a/xen/arch/x86/x86_64/compat/mm.c Mon Mar 14 14:59:27 2011 +0000 > > +++ b/xen/arch/x86/x86_64/compat/mm.c Mon Mar 14 15:17:59 2011 +0000 > > @@ -161,9 +161,7 @@ int compat_arch_memory_op(int op, XEN_GU > > if ( copy_from_guest(&xmml, arg, 1) ) > > return -EFAULT; > > > > - limit = (unsigned long)(compat_machine_to_phys_mapping + > > - min_t(unsigned long, max_page, > > - MACH2PHYS_COMPAT_NR_ENTRIES(current->domain))); > > + limit = (unsigned long)(compat_machine_to_phys_mapping + max_page); > > While doing this shouldn't hurt (except slightly for performance of > the hypercall), I don't see why it's useful: For slots past > MACH2PHYS_COMPAT_NR_ENTRIES(current->domain) you > wouldn't read non-null page table entries anyway (up to > RDWR_COMPAT_MPT_VIRT_END), so I don't see why the tools > couldn't equally well do with what we have currently (after all > they get told how many slots were filled). In order to be able to migrate any guest the tools in domain 0 need to see the entire of host M2P, not just the subset which the kernel sees mapped into its hypervisor hole (which is what MACH2PHYS_COMPAT_NR_ENTRIES represents). The hypercall reads from the global compat M2P mapping, not the guest kernel mapping of it, so it should read valid entries all the way up to RDWR_COMPAT_MPT_VIRT_END, AFAICT. Ian. > > Jan > > > 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; > >