diff mbox

[v4,12/12] x86/hvm/ioreq: add a new mappable resource type...

Message ID 20170905113716.3960-13-paul.durrant@citrix.com (mailing list archive)
State New, archived
Headers show

Commit Message

Paul Durrant Sept. 5, 2017, 11:37 a.m. UTC
... XENMEM_resource_ioreq_server

This patch adds support for a new resource type that can be mapped using
the XENMEM_acquire_resource memory op.

If an emulator makes use of this resource type then, instead of mapping
gfns, the IOREQ server will allocate pages from the heap. These pages
will never be present in the P2M of the guest at any point and so are
not vulnerable to any direct attack by the guest. They are only ever
accessible by Xen and any domain that has mapping privilege over the
guest (which may or may not be limited to the domain running the emulator).

NOTE: Use of the new resource type is not compatible with use of
      XEN_DMOP_get_ioreq_server_info unless the XEN_DMOP_no_gfns flag is
      set.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Acked-by: George Dunlap <George.Dunlap@eu.citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/x86/hvm/ioreq.c        | 123 +++++++++++++++++++++++++++++++++++++++-
 xen/arch/x86/mm.c               |  27 +++++++++
 xen/include/asm-x86/hvm/ioreq.h |   2 +
 xen/include/public/hvm/dm_op.h  |   4 ++
 xen/include/public/memory.h     |   3 +
 5 files changed, 158 insertions(+), 1 deletion(-)

Comments

Wei Liu Sept. 7, 2017, 2:51 p.m. UTC | #1
On Tue, Sep 05, 2017 at 12:37:16PM +0100, Paul Durrant wrote:
>  
> +mfn_t hvm_get_ioreq_server_frame(struct domain *d, ioservid_t id,
> +                                 unsigned int idx)
> +{
> +    struct hvm_ioreq_server *s;
> +    mfn_t mfn = INVALID_MFN;
> +
> +    spin_lock_recursive(&d->arch.hvm_domain.ioreq_server.lock);
> +
> +    s = d->arch.hvm_domain.ioreq_server.server[id];
> +

Check id < MAX_NR_IOREQ_SERVERS before getting s?

> +    if ( id >= MAX_NR_IOREQ_SERVERS || !s || IS_DEFAULT(s) )
> +        goto out;
> +
> +    if ( hvm_ioreq_server_alloc_pages(s) )
> +        goto out;
> +
> +    if ( idx == 0 )
> +        mfn = _mfn(page_to_mfn(s->bufioreq.page));
> +    else if ( idx == 1 )
> +        mfn = _mfn(page_to_mfn(s->ioreq.page));

Does the caller care about the order? If so this should be documented?

The rest looks sensible but I haven't reviewed in detail.

> +
> + out:
> +    spin_unlock_recursive(&d->arch.hvm_domain.ioreq_server.lock);
> +
> +    return mfn;
> +}
> +
Roger Pau Monne Sept. 7, 2017, 2:56 p.m. UTC | #2
On Tue, Sep 05, 2017 at 12:37:16PM +0100, Paul Durrant wrote:
> ... XENMEM_resource_ioreq_server
> 
> This patch adds support for a new resource type that can be mapped using
> the XENMEM_acquire_resource memory op.
> 
> If an emulator makes use of this resource type then, instead of mapping
> gfns, the IOREQ server will allocate pages from the heap. These pages
> will never be present in the P2M of the guest at any point and so are
> not vulnerable to any direct attack by the guest. They are only ever
> accessible by Xen and any domain that has mapping privilege over the
> guest (which may or may not be limited to the domain running the emulator).
> 
> NOTE: Use of the new resource type is not compatible with use of
>       XEN_DMOP_get_ioreq_server_info unless the XEN_DMOP_no_gfns flag is
>       set.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> Acked-by: George Dunlap <George.Dunlap@eu.citrix.com>
> ---
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Tim Deegan <tim@xen.org>
> Cc: Wei Liu <wei.liu2@citrix.com>
> ---
>  xen/arch/x86/hvm/ioreq.c        | 123 +++++++++++++++++++++++++++++++++++++++-
>  xen/arch/x86/mm.c               |  27 +++++++++
>  xen/include/asm-x86/hvm/ioreq.h |   2 +
>  xen/include/public/hvm/dm_op.h  |   4 ++
>  xen/include/public/memory.h     |   3 +
>  5 files changed, 158 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/x86/hvm/ioreq.c b/xen/arch/x86/hvm/ioreq.c
> index 158bfbba32..f4c06a7a2a 100644
> --- a/xen/arch/x86/hvm/ioreq.c
> +++ b/xen/arch/x86/hvm/ioreq.c
> @@ -250,6 +250,15 @@ static int hvm_map_ioreq_gfn(struct hvm_ioreq_server *s, bool buf)
>      struct hvm_ioreq_page *iorp = buf ? &s->bufioreq : &s->ioreq;
>      int rc;
>  
> +    if ( iorp->page )
> +    {
> +        /* Make sure the page has not been allocated */
> +        if ( gfn_eq(iorp->gfn, INVALID_GFN) )
> +            return -EPERM;
> +
> +        return 0;
> +    }
> +
>      if ( d->is_dying )
>          return -EINVAL;
>  
> @@ -272,6 +281,57 @@ static int hvm_map_ioreq_gfn(struct hvm_ioreq_server *s, bool buf)
>      return rc;
>  }
>  
> +static int hvm_alloc_ioreq_mfn(struct hvm_ioreq_server *s, bool buf)
> +{
> +    struct domain *currd = current->domain;
> +    struct hvm_ioreq_page *iorp = buf ? &s->bufioreq : &s->ioreq;
> +
> +    if ( iorp->page )
> +    {
> +        /* Make sure the page has not been mapped */

Maybe it's just me being slightly lost, but here you imply that
iorp->gfn being != INVALID_GFN means the page is mapped, while above
you mention allocated for the same check.

Is it because gfn has different usages depending on the context?

> +        if ( !gfn_eq(iorp->gfn, INVALID_GFN) )
> +            return -EPERM;
[...]
> +mfn_t hvm_get_ioreq_server_frame(struct domain *d, ioservid_t id,
> +                                 unsigned int idx)
> +{
> +    struct hvm_ioreq_server *s;
> +    mfn_t mfn = INVALID_MFN;
> +
> +    spin_lock_recursive(&d->arch.hvm_domain.ioreq_server.lock);
> +
> +    s = d->arch.hvm_domain.ioreq_server.server[id];

Can't you use GET_IOREQ_SERVER here?

Thanks, Roger.
Wei Liu Sept. 7, 2017, 2:59 p.m. UTC | #3
On Thu, Sep 07, 2017 at 03:56:16PM +0100, Roger Pau Monné wrote:
> On Tue, Sep 05, 2017 at 12:37:16PM +0100, Paul Durrant wrote:
> > ... XENMEM_resource_ioreq_server
> > 
> > This patch adds support for a new resource type that can be mapped using
> > the XENMEM_acquire_resource memory op.
> > 
> > If an emulator makes use of this resource type then, instead of mapping
> > gfns, the IOREQ server will allocate pages from the heap. These pages
> > will never be present in the P2M of the guest at any point and so are
> > not vulnerable to any direct attack by the guest. They are only ever
> > accessible by Xen and any domain that has mapping privilege over the
> > guest (which may or may not be limited to the domain running the emulator).
> > 
> > NOTE: Use of the new resource type is not compatible with use of
> >       XEN_DMOP_get_ioreq_server_info unless the XEN_DMOP_no_gfns flag is
> >       set.
> > 
> > Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> > Acked-by: George Dunlap <George.Dunlap@eu.citrix.com>
> > ---
> > Cc: Jan Beulich <jbeulich@suse.com>
> > Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> > Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > Cc: Stefano Stabellini <sstabellini@kernel.org>
> > Cc: Tim Deegan <tim@xen.org>
> > Cc: Wei Liu <wei.liu2@citrix.com>
> > ---
> >  xen/arch/x86/hvm/ioreq.c        | 123 +++++++++++++++++++++++++++++++++++++++-
> >  xen/arch/x86/mm.c               |  27 +++++++++
> >  xen/include/asm-x86/hvm/ioreq.h |   2 +
> >  xen/include/public/hvm/dm_op.h  |   4 ++
> >  xen/include/public/memory.h     |   3 +
> >  5 files changed, 158 insertions(+), 1 deletion(-)
> > 
> > diff --git a/xen/arch/x86/hvm/ioreq.c b/xen/arch/x86/hvm/ioreq.c
> > index 158bfbba32..f4c06a7a2a 100644
> > --- a/xen/arch/x86/hvm/ioreq.c
> > +++ b/xen/arch/x86/hvm/ioreq.c
> > @@ -250,6 +250,15 @@ static int hvm_map_ioreq_gfn(struct hvm_ioreq_server *s, bool buf)
> >      struct hvm_ioreq_page *iorp = buf ? &s->bufioreq : &s->ioreq;
> >      int rc;
> >  
> > +    if ( iorp->page )
> > +    {
> > +        /* Make sure the page has not been allocated */
> > +        if ( gfn_eq(iorp->gfn, INVALID_GFN) )
> > +            return -EPERM;
> > +
> > +        return 0;
> > +    }
> > +
> >      if ( d->is_dying )
> >          return -EINVAL;
> >  
> > @@ -272,6 +281,57 @@ static int hvm_map_ioreq_gfn(struct hvm_ioreq_server *s, bool buf)
> >      return rc;
> >  }
> >  
> > +static int hvm_alloc_ioreq_mfn(struct hvm_ioreq_server *s, bool buf)
> > +{
> > +    struct domain *currd = current->domain;
> > +    struct hvm_ioreq_page *iorp = buf ? &s->bufioreq : &s->ioreq;
> > +
> > +    if ( iorp->page )
> > +    {
> > +        /* Make sure the page has not been mapped */
> 
> Maybe it's just me being slightly lost, but here you imply that
> iorp->gfn being != INVALID_GFN means the page is mapped, while above
> you mention allocated for the same check.
> 
> Is it because gfn has different usages depending on the context?
> 

Haha, I'm not the only one who got confused while reading the patch. ;-)

That's because the "page" can be obtained via two different methods.

"Allocated" means getting it from xen heap, "mapped" means mapping guest
page afaict.
Roger Pau Monne Sept. 7, 2017, 3:23 p.m. UTC | #4
On Thu, Sep 07, 2017 at 03:59:05PM +0100, Wei Liu wrote:
> On Thu, Sep 07, 2017 at 03:56:16PM +0100, Roger Pau Monné wrote:
> > On Tue, Sep 05, 2017 at 12:37:16PM +0100, Paul Durrant wrote:
[...]
> > > +        /* Make sure the page has not been allocated */
> > > +        if ( gfn_eq(iorp->gfn, INVALID_GFN) )
> > > +            return -EPERM;
> > > +
> > > +        return 0;
> > > +    }
> > > +
> > >      if ( d->is_dying )
> > >          return -EINVAL;
> > >  
> > > @@ -272,6 +281,57 @@ static int hvm_map_ioreq_gfn(struct hvm_ioreq_server *s, bool buf)
> > >      return rc;
> > >  }
> > >  
> > > +static int hvm_alloc_ioreq_mfn(struct hvm_ioreq_server *s, bool buf)
> > > +{
> > > +    struct domain *currd = current->domain;
> > > +    struct hvm_ioreq_page *iorp = buf ? &s->bufioreq : &s->ioreq;
> > > +
> > > +    if ( iorp->page )
> > > +    {
> > > +        /* Make sure the page has not been mapped */
> > 
> > Maybe it's just me being slightly lost, but here you imply that
> > iorp->gfn being != INVALID_GFN means the page is mapped, while above
> > you mention allocated for the same check.
> > 
> > Is it because gfn has different usages depending on the context?
> > 
> 
> Haha, I'm not the only one who got confused while reading the patch. ;-)
> 
> That's because the "page" can be obtained via two different methods.
> 
> "Allocated" means getting it from xen heap, "mapped" means mapping guest
> page afaict.

Oh, thanks. I think the comment should be slightly expanded then to
give more context, or else someone else is likely to trip over this
again IMHO.

Roger.
Paul Durrant Sept. 8, 2017, 8:28 a.m. UTC | #5
> -----Original Message-----
> From: Wei Liu [mailto:wei.liu2@citrix.com]
> Sent: 07 September 2017 15:51
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: xen-devel@lists.xenproject.org; Jan Beulich <jbeulich@suse.com>;
> Andrew Cooper <Andrew.Cooper3@citrix.com>; Ian Jackson
> <Ian.Jackson@citrix.com>; Konrad Rzeszutek Wilk
> <konrad.wilk@oracle.com>; Stefano Stabellini <sstabellini@kernel.org>; Tim
> (Xen.org) <tim@xen.org>; Wei Liu <wei.liu2@citrix.com>
> Subject: Re: [PATCH v4 12/12] x86/hvm/ioreq: add a new mappable resource
> type...
> 
> On Tue, Sep 05, 2017 at 12:37:16PM +0100, Paul Durrant wrote:
> >
> > +mfn_t hvm_get_ioreq_server_frame(struct domain *d, ioservid_t id,
> > +                                 unsigned int idx)
> > +{
> > +    struct hvm_ioreq_server *s;
> > +    mfn_t mfn = INVALID_MFN;
> > +
> > +    spin_lock_recursive(&d->arch.hvm_domain.ioreq_server.lock);
> > +
> > +    s = d->arch.hvm_domain.ioreq_server.server[id];
> > +
> 
> Check id < MAX_NR_IOREQ_SERVERS before getting s?

Oh, I should be using my new macro here. Good spot.

> 
> > +    if ( id >= MAX_NR_IOREQ_SERVERS || !s || IS_DEFAULT(s) )
> > +        goto out;
> > +
> > +    if ( hvm_ioreq_server_alloc_pages(s) )
> > +        goto out;
> > +
> > +    if ( idx == 0 )
> > +        mfn = _mfn(page_to_mfn(s->bufioreq.page));
> > +    else if ( idx == 1 )
> > +        mfn = _mfn(page_to_mfn(s->ioreq.page));
> 
> Does the caller care about the order? If so this should be documented?
> 

True. I should document that 0 is the buffered frame and 1+ are synchronous frames (of which there is only one at the moment but this will need to change if we support more vcpus).

> The rest looks sensible but I haven't reviewed in detail.
> 

Ok.

  Paul

> > +
> > + out:
> > +    spin_unlock_recursive(&d->arch.hvm_domain.ioreq_server.lock);
> > +
> > +    return mfn;
> > +}
> > +
Paul Durrant Sept. 8, 2017, 8:29 a.m. UTC | #6
> -----Original Message-----
> From: Roger Pau Monne
> Sent: 07 September 2017 16:24
> To: Wei Liu <wei.liu2@citrix.com>
> Cc: Paul Durrant <Paul.Durrant@citrix.com>; xen-devel@lists.xenproject.org;
> Stefano Stabellini <sstabellini@kernel.org>; Andrew Cooper
> <Andrew.Cooper3@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>; Tim
> (Xen.org) <tim@xen.org>; Jan Beulich <jbeulich@suse.com>
> Subject: Re: [Xen-devel] [PATCH v4 12/12] x86/hvm/ioreq: add a new
> mappable resource type...
> 
> On Thu, Sep 07, 2017 at 03:59:05PM +0100, Wei Liu wrote:
> > On Thu, Sep 07, 2017 at 03:56:16PM +0100, Roger Pau Monné wrote:
> > > On Tue, Sep 05, 2017 at 12:37:16PM +0100, Paul Durrant wrote:
> [...]
> > > > +        /* Make sure the page has not been allocated */
> > > > +        if ( gfn_eq(iorp->gfn, INVALID_GFN) )
> > > > +            return -EPERM;
> > > > +
> > > > +        return 0;
> > > > +    }
> > > > +
> > > >      if ( d->is_dying )
> > > >          return -EINVAL;
> > > >
> > > > @@ -272,6 +281,57 @@ static int hvm_map_ioreq_gfn(struct
> hvm_ioreq_server *s, bool buf)
> > > >      return rc;
> > > >  }
> > > >
> > > > +static int hvm_alloc_ioreq_mfn(struct hvm_ioreq_server *s, bool buf)
> > > > +{
> > > > +    struct domain *currd = current->domain;
> > > > +    struct hvm_ioreq_page *iorp = buf ? &s->bufioreq : &s->ioreq;
> > > > +
> > > > +    if ( iorp->page )
> > > > +    {
> > > > +        /* Make sure the page has not been mapped */
> > >
> > > Maybe it's just me being slightly lost, but here you imply that
> > > iorp->gfn being != INVALID_GFN means the page is mapped, while above
> > > you mention allocated for the same check.
> > >
> > > Is it because gfn has different usages depending on the context?
> > >
> >
> > Haha, I'm not the only one who got confused while reading the patch. ;-)
> >
> > That's because the "page" can be obtained via two different methods.
> >
> > "Allocated" means getting it from xen heap, "mapped" means mapping
> guest
> > page afaict.
> 
> Oh, thanks. I think the comment should be slightly expanded then to
> give more context, or else someone else is likely to trip over this
> again IMHO.

Yes, Wei's explanation is correct. I will expand the comments in both places to explain they are mutually exclusive methods of getting hold of ioreq server pages.

  Paul

> 
> Roger.
diff mbox

Patch

diff --git a/xen/arch/x86/hvm/ioreq.c b/xen/arch/x86/hvm/ioreq.c
index 158bfbba32..f4c06a7a2a 100644
--- a/xen/arch/x86/hvm/ioreq.c
+++ b/xen/arch/x86/hvm/ioreq.c
@@ -250,6 +250,15 @@  static int hvm_map_ioreq_gfn(struct hvm_ioreq_server *s, bool buf)
     struct hvm_ioreq_page *iorp = buf ? &s->bufioreq : &s->ioreq;
     int rc;
 
+    if ( iorp->page )
+    {
+        /* Make sure the page has not been allocated */
+        if ( gfn_eq(iorp->gfn, INVALID_GFN) )
+            return -EPERM;
+
+        return 0;
+    }
+
     if ( d->is_dying )
         return -EINVAL;
 
@@ -272,6 +281,57 @@  static int hvm_map_ioreq_gfn(struct hvm_ioreq_server *s, bool buf)
     return rc;
 }
 
+static int hvm_alloc_ioreq_mfn(struct hvm_ioreq_server *s, bool buf)
+{
+    struct domain *currd = current->domain;
+    struct hvm_ioreq_page *iorp = buf ? &s->bufioreq : &s->ioreq;
+
+    if ( iorp->page )
+    {
+        /* Make sure the page has not been mapped */
+        if ( !gfn_eq(iorp->gfn, INVALID_GFN) )
+            return -EPERM;
+
+        return 0;
+    }
+
+    /*
+     * Allocated IOREQ server pages are assigned to the emulating
+     * domain, not the target domain. This is because the emulator is
+     * likely to be destroyed after the target domain has been torn
+     * down, and we must use MEMF_no_refcount otherwise page allocation
+     * could fail if the emulating domain has already reached its
+     * maximum allocation.
+     */
+    iorp->page = alloc_domheap_page(currd, MEMF_no_refcount);
+    if ( !iorp->page )
+        return -ENOMEM;
+
+    iorp->va = __map_domain_page_global(iorp->page);
+    if ( !iorp->va )
+    {
+        iorp->page = NULL;
+        return -ENOMEM;
+    }
+
+    clear_page(iorp->va);
+    return 0;
+}
+
+static void hvm_free_ioreq_mfn(struct hvm_ioreq_server *s, bool buf)
+{
+    struct hvm_ioreq_page *iorp = buf ? &s->bufioreq : &s->ioreq;
+
+    if ( !iorp->page )
+        return;
+
+    unmap_domain_page_global(iorp->va);
+    iorp->va = NULL;
+
+    put_page(iorp->page);
+    iorp->page = NULL;
+}
+
 bool is_ioreq_server_page(struct domain *d, const struct page_info *page)
 {
     const struct hvm_ioreq_server *s;
@@ -478,6 +538,27 @@  static void hvm_ioreq_server_unmap_pages(struct hvm_ioreq_server *s)
     hvm_unmap_ioreq_gfn(s, false);
 }
 
+static int hvm_ioreq_server_alloc_pages(struct hvm_ioreq_server *s)
+{
+    int rc = -ENOMEM;
+
+    rc = hvm_alloc_ioreq_mfn(s, false);
+
+    if ( !rc && (s->bufioreq_handling != HVM_IOREQSRV_BUFIOREQ_OFF) )
+        rc = hvm_alloc_ioreq_mfn(s, true);
+
+    if ( rc )
+        hvm_free_ioreq_mfn(s, false);
+
+    return rc;
+}
+
+static void hvm_ioreq_server_free_pages(struct hvm_ioreq_server *s)
+{
+    hvm_free_ioreq_mfn(s, true);
+    hvm_free_ioreq_mfn(s, false);
+}
+
 static void hvm_ioreq_server_free_rangesets(struct hvm_ioreq_server *s)
 {
     unsigned int i;
@@ -604,7 +685,18 @@  static int hvm_ioreq_server_init(struct hvm_ioreq_server *s,
 
  fail_add:
     hvm_ioreq_server_remove_all_vcpus(s);
+
+    /*
+     * NOTE: It is safe to call both hvm_ioreq_server_unmap_pages() and
+     *       hvm_ioreq_server_free_pages() in that order.
+     *       This is because the former will do nothing if the pages
+     *       are not mapped, leaving the page to be freed by the latter.
+     *       However if the pages are mapped then the former will set
+     *       the page_info pointer to NULL, meaning the latter will do
+     *       nothing.
+     */
     hvm_ioreq_server_unmap_pages(s);
+    hvm_ioreq_server_free_pages(s);
 
     return rc;
 }
@@ -614,6 +706,7 @@  static void hvm_ioreq_server_deinit(struct hvm_ioreq_server *s)
     ASSERT(!s->enabled);
     hvm_ioreq_server_remove_all_vcpus(s);
     hvm_ioreq_server_unmap_pages(s);
+    hvm_ioreq_server_free_pages(s);
     hvm_ioreq_server_free_rangesets(s);
 }
 
@@ -745,7 +838,8 @@  int hvm_get_ioreq_server_info(struct domain *d, ioservid_t id,
             goto out;
     }
 
-    *ioreq_gfn = gfn_x(s->ioreq.gfn);
+    if ( ioreq_gfn )
+        *ioreq_gfn = gfn_x(s->ioreq.gfn);
 
     if ( HANDLE_BUFIOREQ(s) )
     {
@@ -763,6 +857,33 @@  int hvm_get_ioreq_server_info(struct domain *d, ioservid_t id,
     return rc;
 }
 
+mfn_t hvm_get_ioreq_server_frame(struct domain *d, ioservid_t id,
+                                 unsigned int idx)
+{
+    struct hvm_ioreq_server *s;
+    mfn_t mfn = INVALID_MFN;
+
+    spin_lock_recursive(&d->arch.hvm_domain.ioreq_server.lock);
+
+    s = d->arch.hvm_domain.ioreq_server.server[id];
+
+    if ( id >= MAX_NR_IOREQ_SERVERS || !s || IS_DEFAULT(s) )
+        goto out;
+
+    if ( hvm_ioreq_server_alloc_pages(s) )
+        goto out;
+
+    if ( idx == 0 )
+        mfn = _mfn(page_to_mfn(s->bufioreq.page));
+    else if ( idx == 1 )
+        mfn = _mfn(page_to_mfn(s->ioreq.page));
+
+ out:
+    spin_unlock_recursive(&d->arch.hvm_domain.ioreq_server.lock);
+
+    return mfn;
+}
+
 int hvm_map_io_range_to_ioreq_server(struct domain *d, ioservid_t id,
                                      uint32_t type, uint64_t start,
                                      uint64_t end)
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index b125b85bf9..6c95c58675 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -122,6 +122,7 @@ 
 #include <asm/fixmap.h>
 #include <asm/io_apic.h>
 #include <asm/pci.h>
+#include <asm/hvm/ioreq.h>
 
 #include <asm/hvm/grant_table.h>
 #include <asm/pv/grant_table.h>
@@ -4738,6 +4739,27 @@  static int xenmem_acquire_grant_table(struct domain *d,
     return 0;
 }
 
+static int xenmem_acquire_ioreq_server(struct domain *d,
+                                       unsigned int id,
+                                       unsigned long frame,
+                                       unsigned long nr_frames,
+                                       unsigned long mfn_list[])
+{
+    unsigned int i;
+
+    for ( i = 0; i < nr_frames; i++ )
+    {
+        mfn_t mfn = hvm_get_ioreq_server_frame(d, id, frame + i);
+
+        if ( mfn_eq(mfn, INVALID_MFN) )
+            return -EINVAL;
+
+        mfn_list[i] = mfn_x(mfn);
+    }
+
+    return 0;
+}
+
 static int xenmem_acquire_resource(xen_mem_acquire_resource_t *xmar)
 {
     struct domain *d, *currd = current->domain;
@@ -4772,6 +4794,11 @@  static int xenmem_acquire_resource(xen_mem_acquire_resource_t *xmar)
                                         mfn_list);
         break;
 
+    case XENMEM_resource_ioreq_server:
+        rc = xenmem_acquire_ioreq_server(d, xmar->id, xmar->frame,
+                                         xmar->nr_frames, mfn_list);
+        break;
+
     default:
         rc = -EOPNOTSUPP;
         break;
diff --git a/xen/include/asm-x86/hvm/ioreq.h b/xen/include/asm-x86/hvm/ioreq.h
index 1829fcf43e..032aeb6fa9 100644
--- a/xen/include/asm-x86/hvm/ioreq.h
+++ b/xen/include/asm-x86/hvm/ioreq.h
@@ -31,6 +31,8 @@  int hvm_get_ioreq_server_info(struct domain *d, ioservid_t id,
                               unsigned long *ioreq_gfn,
                               unsigned long *bufioreq_gfn,
                               evtchn_port_t *bufioreq_port);
+mfn_t hvm_get_ioreq_server_frame(struct domain *d, ioservid_t id,
+                                 unsigned int idx);
 int hvm_map_io_range_to_ioreq_server(struct domain *d, ioservid_t id,
                                      uint32_t type, uint64_t start,
                                      uint64_t end);
diff --git a/xen/include/public/hvm/dm_op.h b/xen/include/public/hvm/dm_op.h
index 9677bd74e7..59b6006910 100644
--- a/xen/include/public/hvm/dm_op.h
+++ b/xen/include/public/hvm/dm_op.h
@@ -90,6 +90,10 @@  struct xen_dm_op_create_ioreq_server {
  * the frame numbers passed back in gfns <ioreq_gfn> and <bufioreq_gfn>
  * respectively. (If the IOREQ Server is not handling buffered emulation
  * only <ioreq_gfn> will be valid).
+ *
+ * NOTE: To access the synchronous ioreq structures and buffered ioreq
+ *       ring, it is preferable to use the XENMEM_acquire_resource memory
+ *       op specifying resource type XENMEM_resource_ioreq_server.
  */
 #define XEN_DMOP_get_ioreq_server_info 2
 
diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
index 9bf58e7384..716941dc0c 100644
--- a/xen/include/public/memory.h
+++ b/xen/include/public/memory.h
@@ -664,10 +664,13 @@  struct xen_mem_acquire_resource {
     uint16_t type;
 
 #define XENMEM_resource_grant_table 0
+#define XENMEM_resource_ioreq_server 1
 
     /*
      * IN - a type-specific resource identifier, which must be zero
      *      unless stated otherwise.
+     *
+     * type == XENMEM_resource_ioreq_server -> id == ioreq server id
      */
     uint32_t id;
     /* IN - number of (4K) frames of the resource to be mapped */