From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mukesh Rathor Subject: Re: [V7 PATCH 3/7] pvh dom0: implement XENMEM_add_to_physmap_range for x86 Date: Tue, 17 Dec 2013 15:57:53 -0800 Message-ID: <20131217155753.12dcda15@mantra.us.oracle.com> References: <1387247911-28846-1-git-send-email-mukesh.rathor@oracle.com> <1387247911-28846-4-git-send-email-mukesh.rathor@oracle.com> <52B05A9A020000780010E22C@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1Vt4XX-0008Mj-Rv for xen-devel@lists.xenproject.org; Tue, 17 Dec 2013 23:59:04 +0000 In-Reply-To: <52B05A9A020000780010E22C@nat28.tlf.novell.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: Jan Beulich Cc: george.dunlap@eu.citrix.com, xen-devel , keir.xen@gmail.com, tim@xen.org List-Id: xen-devel@lists.xenproject.org On Tue, 17 Dec 2013 13:07:22 +0000 "Jan Beulich" wrote: > >>> On 17.12.13 at 03:38, Mukesh Rathor > >>> wrote: > > --- a/xen/arch/x86/mm.c > > +++ b/xen/arch/x86/mm.c > > @@ -4675,6 +4675,39 @@ static int xenmem_add_to_physmap(struct > > domain *d, return xenmem_add_to_physmap_once(d, xatp); > > } > > > > +static int xenmem_add_to_physmap_range(struct domain *d, > > + struct > > xen_add_to_physmap_range *xatpr) +{ > > + /* Process entries in reverse order to allow continuations */ > > + while ( xatpr->size > 0 ) > > + { > > + int rc; > > + xen_ulong_t idx; > > + xen_pfn_t gpfn; > > + struct xen_add_to_physmap xatp; > > + > > + if ( copy_from_guest_offset(&idx, xatpr->idxs, > > xatpr->size-1, 1) || > > + copy_from_guest_offset(&gpfn, xatpr->gpfns, > > xatpr->size-1, 1) ) > > + return -EFAULT; > > + > > + xatp.space = xatpr->space; > > + xatp.idx = idx; > > + xatp.gpfn = gpfn; > > + rc = xenmem_add_to_physmap_once(d, &xatp); > > + > > + if ( copy_to_guest_offset(xatpr->errs, xatpr->size-1, &rc, > > 1) ) > > + return -EFAULT; > > + > > + xatpr->size--; > > + > > + /* Check for continuation if it's not the last interation > > */ > > + if ( xatpr->size > 0 && hypercall_preempt_check() ) > > + return -EAGAIN; > > + } > > + > > + return 0; > > +} > > Now that I started looking into creating the compat wrapper for this, > I realize that processing this request backwards is wrong: The effect > of the entire hypercall can end up being different between forward > and reverse processing, if an index or gpfn happens to be twice in > the set. While that's perhaps not the usual thing to do, you never > know how a creative user of the interface may find it useful to do so. IIRC, I had gotten that from the arm guys.... it seems plausible to say the mappings are unpredictable in case of repeated idx/gpfn, but I'll see what they are doing to fix, or just change it myself here. thanks for noticing, Mukesh