Re: [Xen-devel] [PATCH 14/14] arm: implement foreign mapping using XENMEM_add_to_physmap_range

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2012-10-05 at 14:36 +0100, Stefano Stabellini wrote:
> On Thu, 4 Oct 2012, Ian Campbell wrote:
> > This interface is prefered for foreign mappings.
> 
> So now we have XENMEM_add_to_physmap_range but we only have
> XENMEM_remove_from_physmap. Would it be worth to introduce a
> XENMEM_remove_from_physmap_range too?

Worth considering but the need isn't so urgent since you wouldn't need
to coexist with XENMAPSPACE_gmfn_range's size parameter and the
foreign_domid one isn't needed either.

I wonder if XENMEM_add_to_physmap_range ought to reject
XENMAPSPACE_gmfn_range, just for the sake of sanity.

> 
> > Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> > ---
> >  arch/arm/xen/enlighten.c       |   14 +++++++++-----
> >  include/xen/interface/memory.h |   18 ++++++++++++++++++
> >  2 files changed, 27 insertions(+), 5 deletions(-)
> > 
> > diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> > index 9956af5..a9946aa 100644
> > --- a/arch/arm/xen/enlighten.c
> > +++ b/arch/arm/xen/enlighten.c
> > @@ -51,15 +51,19 @@ static int map_foreign_page(unsigned long lpfn, 
> > unsigned long fgmfn,
> >                         unsigned int domid)
> >  {
> >     int rc;
> > -   struct xen_add_to_physmap xatp = {
> > +   struct xen_add_to_physmap_range xatp = {
> >             .domid = DOMID_SELF,
> > -           .u.foreign_domid = domid,
> > +           .foreign_domid = domid,
> > +           .size = 1,
> >             .space = XENMAPSPACE_gmfn_foreign,
> > -           .idx = fgmfn,
> > -           .gpfn = lpfn,
> >     };
> > +   unsigned long idx = fgmfn;
> > +   xen_pfn_t gpfn = lpfn;
> >  
> > -   rc = HYPERVISOR_memory_op(XENMEM_add_to_physmap, &xatp);
> > +   set_xen_guest_handle(xatp.idxs, &idx);
> > +   set_xen_guest_handle(xatp.gpfns, &gpfn);
> > +
> > +   rc = HYPERVISOR_memory_op(XENMEM_add_to_physmap_range, &xatp);
> >     if (rc) {
> >             pr_warn("Failed to map pfn to mfn rc:%d pfn:%lx mfn:%lx\n",
> >                     rc, lpfn, fgmfn);
> 
> Wouldn't it make sense to call XENMEM_add_to_physmap_range only once for
> the whole range, rather than once per page?
> 
> 
> > diff --git a/include/xen/interface/memory.h b/include/xen/interface/memory.h
> > index d38bdc1..e5675bc 100644
> > --- a/include/xen/interface/memory.h
> > +++ b/include/xen/interface/memory.h
> > @@ -188,6 +188,24 @@ struct xen_add_to_physmap {
> >  };
> >  DEFINE_GUEST_HANDLE_STRUCT(xen_add_to_physmap);
> >  
> > +#define XENMEM_add_to_physmap_range 23
> > +struct xen_add_to_physmap_range {
> > +    /* Which domain to change the mapping for. */
> > +    domid_t domid;
> > +    uint16_t space; /* => enum phys_map_space */
> > +
> > +    /* Number of pages to go through */
> > +    uint16_t size;
> > +    domid_t foreign_domid; /* IFF gmfn_foreign */
> > +
> > +    /* Indexes into space being mapped. */
> > +    GUEST_HANDLE(ulong) idxs;
> > +
> > +    /* GPFN in domid where the source mapping page should appear. */
> > +    GUEST_HANDLE(xen_pfn_t) gpfns;
> > +};
> > +DEFINE_GUEST_HANDLE_STRUCT(xen_add_to_physmap_range);
> > +
> >  /*
> >   * Translates a list of domain-specific GPFNs into MFNs. Returns a -ve 
> > error
> >   * code on failure. This call only works for auto-translated guests.
> > -- 
> > 1.7.2.5
> > 



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel