From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [V7 PATCH 3/7] pvh dom0: implement XENMEM_add_to_physmap_range for x86 Date: Tue, 17 Dec 2013 13:59:31 +0000 Message-ID: <1387288771.27441.59.camel@kazak.uk.xensource.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 1VsvBR-00081Y-2J for xen-devel@lists.xenproject.org; Tue, 17 Dec 2013 13:59:37 +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, 2013-12-17 at 13:07 +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. Hrm, the ARM code does things this way as well. But you are of course right. We could change the code but we could also tighten the interface requirements, either by explicit specifying that the range is handled in reverse order or by mandating that index/gpfn must not be repeated (whether or not we actively try and detect such cases). Ian.