diff mbox

[v3,2/3] altp2m: Merge p2m_set_altp2m_mem_access and p2m_set_mem_access

Message ID 1454707342-14479-2-git-send-email-tlengyel@novetta.com (mailing list archive)
State New, archived
Headers show

Commit Message

Tamas K Lengyel Feb. 5, 2016, 9:22 p.m. UTC
The altp2m subsystem in its current form duplicates much of the existing
code present in p2m for setting mem_access permissions. In this patch we
consolidate the two versions but keep the separate MEMOP and HVMOP interfaces.

Signed-off-by: Tamas K Lengyel <tlengyel@novetta.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Razvan Cojocaru <rcojocaru@bitdefender.com>
Cc: Stefano Stabellini <stefano.stabellini@citrix.com>
Cc: Keir Fraser <keir@xen.org>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <george.dunlap@eu.citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
v3: Keep the external-facing interfaces and tools as they are
    Pass gfn_t and make p2m_set_altp2m_mem_access inline
    Remove goto and just return rc directly
v2: Don't deprecate the HVMOP hypercall for setting mem_access
    Use unsigned int instead of unsigned long
---
 xen/arch/arm/p2m.c           |   9 +--
 xen/arch/x86/hvm/hvm.c       |   6 +-
 xen/arch/x86/mm/p2m.c        | 168 +++++++++++++++++++------------------------
 xen/common/mem_access.c      |   2 +-
 xen/include/asm-x86/p2m.h    |   4 --
 xen/include/xen/p2m-common.h |   3 +-
 6 files changed, 86 insertions(+), 106 deletions(-)

Comments

Razvan Cojocaru Feb. 6, 2016, 7:09 a.m. UTC | #1
On 02/05/2016 11:22 PM, Tamas K Lengyel wrote:
> The altp2m subsystem in its current form duplicates much of the existing
> code present in p2m for setting mem_access permissions. In this patch we
> consolidate the two versions but keep the separate MEMOP and HVMOP interfaces.
> 
> Signed-off-by: Tamas K Lengyel <tlengyel@novetta.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Razvan Cojocaru <rcojocaru@bitdefender.com>
> Cc: Stefano Stabellini <stefano.stabellini@citrix.com>
> Cc: Keir Fraser <keir@xen.org>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: George Dunlap <george.dunlap@eu.citrix.com>
> Acked-by: Wei Liu <wei.liu2@citrix.com>
> ---
> v3: Keep the external-facing interfaces and tools as they are
>     Pass gfn_t and make p2m_set_altp2m_mem_access inline
>     Remove goto and just return rc directly
> v2: Don't deprecate the HVMOP hypercall for setting mem_access
>     Use unsigned int instead of unsigned long
> ---
>  xen/arch/arm/p2m.c           |   9 +--
>  xen/arch/x86/hvm/hvm.c       |   6 +-
>  xen/arch/x86/mm/p2m.c        | 168 +++++++++++++++++++------------------------
>  xen/common/mem_access.c      |   2 +-
>  xen/include/asm-x86/p2m.h    |   4 --
>  xen/include/xen/p2m-common.h |   3 +-
>  6 files changed, 86 insertions(+), 106 deletions(-)

Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com>


Thanks,
Razvan
Ian Campbell Feb. 8, 2016, 3:17 p.m. UTC | #2
On Fri, 2016-02-05 at 14:22 -0700, Tamas K Lengyel wrote:
> The altp2m subsystem in its current form duplicates much of the existing
> code present in p2m for setting mem_access permissions. In this patch we
> consolidate the two versions but keep the separate MEMOP and HVMOP
> interfaces.
> 
> Signed-off-by: Tamas K Lengyel <tlengyel@novetta.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Razvan Cojocaru <rcojocaru@bitdefender.com>
> Cc: Stefano Stabellini <stefano.stabellini@citrix.com>
> Cc: Keir Fraser <keir@xen.org>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: George Dunlap <george.dunlap@eu.citrix.com>
> Acked-by: Wei Liu <wei.liu2@citrix.com>

For the ARM side:

    Acked-by: Ian Campbell <    ian.campbell@citrix.com    >

Ian.
George Dunlap Feb. 9, 2016, 3:05 p.m. UTC | #3
On 05/02/16 21:22, Tamas K Lengyel wrote:
> The altp2m subsystem in its current form duplicates much of the existing
> code present in p2m for setting mem_access permissions. In this patch we
> consolidate the two versions but keep the separate MEMOP and HVMOP interfaces.
> 
> Signed-off-by: Tamas K Lengyel <tlengyel@novetta.com>

Reviewed-by: George Dunlap <george.dunlap@citrix.com>

Sorry for the delay!

> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Razvan Cojocaru <rcojocaru@bitdefender.com>
> Cc: Stefano Stabellini <stefano.stabellini@citrix.com>
> Cc: Keir Fraser <keir@xen.org>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: George Dunlap <george.dunlap@eu.citrix.com>
> Acked-by: Wei Liu <wei.liu2@citrix.com>
> ---
> v3: Keep the external-facing interfaces and tools as they are
>     Pass gfn_t and make p2m_set_altp2m_mem_access inline
>     Remove goto and just return rc directly
> v2: Don't deprecate the HVMOP hypercall for setting mem_access
>     Use unsigned int instead of unsigned long
> ---
>  xen/arch/arm/p2m.c           |   9 +--
>  xen/arch/x86/hvm/hvm.c       |   6 +-
>  xen/arch/x86/mm/p2m.c        | 168 +++++++++++++++++++------------------------
>  xen/common/mem_access.c      |   2 +-
>  xen/include/asm-x86/p2m.h    |   4 --
>  xen/include/xen/p2m-common.h |   3 +-
>  6 files changed, 86 insertions(+), 106 deletions(-)
> 
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index 2190908..8568087 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -1709,13 +1709,13 @@ bool_t p2m_mem_access_check(paddr_t gpa, vaddr_t gla, const struct npfec npfec)
>      if ( npfec.write_access && xma == XENMEM_access_rx2rw )
>      {
>          rc = p2m_set_mem_access(v->domain, _gfn(paddr_to_pfn(gpa)), 1,
> -                                0, ~0, XENMEM_access_rw);
> +                                0, ~0, XENMEM_access_rw, 0);
>          return false;
>      }
>      else if ( xma == XENMEM_access_n2rwx )
>      {
>          rc = p2m_set_mem_access(v->domain, _gfn(paddr_to_pfn(gpa)), 1,
> -                                0, ~0, XENMEM_access_rwx);
> +                                0, ~0, XENMEM_access_rwx, 0);
>      }
>  
>      /* Otherwise, check if there is a vm_event monitor subscriber */
> @@ -1737,7 +1737,7 @@ bool_t p2m_mem_access_check(paddr_t gpa, vaddr_t gla, const struct npfec npfec)
>                  /* A listener is not required, so clear the access
>                   * restrictions. */
>                  rc = p2m_set_mem_access(v->domain, _gfn(paddr_to_pfn(gpa)), 1,
> -                                        0, ~0, XENMEM_access_rwx);
> +                                        0, ~0, XENMEM_access_rwx, 0);
>              }
>          }
>  
> @@ -1788,7 +1788,8 @@ bool_t p2m_mem_access_check(paddr_t gpa, vaddr_t gla, const struct npfec npfec)
>   * If gfn == INVALID_GFN, sets the default access type.
>   */
>  long p2m_set_mem_access(struct domain *d, gfn_t gfn, uint32_t nr,
> -                        uint32_t start, uint32_t mask, xenmem_access_t access)
> +                        uint32_t start, uint32_t mask, xenmem_access_t access,
> +                        unsigned int altp2m_idx)
>  {
>      struct p2m_domain *p2m = p2m_get_hostp2m(d);
>      p2m_access_t a;
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index 674feea..37305fb 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -6398,9 +6398,9 @@ static int do_altp2m_op(
>          if ( a.u.set_mem_access.pad )
>              rc = -EINVAL;
>          else
> -            rc = p2m_set_altp2m_mem_access(d, a.u.set_mem_access.view,
> -                    _gfn(a.u.set_mem_access.gfn),
> -                    a.u.set_mem_access.hvmmem_access);
> +            rc = p2m_set_mem_access(d, _gfn(a.u.set_mem_access.gfn), 1, 0, 0,
> +                                    a.u.set_mem_access.hvmmem_access,
> +                                    a.u.set_mem_access.view);
>          break;
>  
>      case HVMOP_altp2m_change_gfn:
> diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
> index a45ee35..226490a 100644
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -1777,14 +1777,56 @@ bool_t p2m_mem_access_check(paddr_t gpa, unsigned long gla,
>      return (p2ma == p2m_access_n2rwx);
>  }
>  
> +static inline
> +int p2m_set_altp2m_mem_access(struct domain *d, struct p2m_domain *hp2m,
> +                              struct p2m_domain *ap2m, p2m_access_t a,
> +                              gfn_t gfn)
> +{
> +    mfn_t mfn;
> +    p2m_type_t t;
> +    p2m_access_t old_a;
> +    unsigned int page_order;
> +    unsigned long gfn_l = gfn_x(gfn);
> +    int rc;
> +
> +    mfn = ap2m->get_entry(ap2m, gfn_l, &t, &old_a, 0, NULL, NULL);
> +
> +    /* Check host p2m if no valid entry in alternate */
> +    if ( !mfn_valid(mfn) )
> +    {
> +        mfn = hp2m->get_entry(hp2m, gfn_l, &t, &old_a,
> +                              P2M_ALLOC | P2M_UNSHARE, &page_order, NULL);
> +
> +        rc = -ESRCH;
> +        if ( !mfn_valid(mfn) || t != p2m_ram_rw )
> +            return rc;
> +
> +        /* If this is a superpage, copy that first */
> +        if ( page_order != PAGE_ORDER_4K )
> +        {
> +            unsigned long mask = ~((1UL << page_order) - 1);
> +            unsigned long gfn2_l = gfn_l & mask;
> +            mfn_t mfn2 = _mfn(mfn_x(mfn) & mask);
> +
> +            rc = ap2m->set_entry(ap2m, gfn2_l, mfn2, page_order, t, old_a, 1);
> +            if ( rc )
> +                return rc;
> +        }
> +    }
> +
> +    return ap2m->set_entry(ap2m, gfn_l, mfn, PAGE_ORDER_4K, t, a,
> +                         (current->domain != d));
> +}
> +
>  /*
>   * Set access type for a region of gfns.
>   * If gfn == INVALID_GFN, sets the default access type.
>   */
>  long p2m_set_mem_access(struct domain *d, gfn_t gfn, uint32_t nr,
> -                        uint32_t start, uint32_t mask, xenmem_access_t access)
> +                        uint32_t start, uint32_t mask, xenmem_access_t access,
> +                        unsigned int altp2m_idx)
>  {
> -    struct p2m_domain *p2m = p2m_get_hostp2m(d);
> +    struct p2m_domain *p2m = p2m_get_hostp2m(d), *ap2m = NULL;
>      p2m_access_t a, _a;
>      p2m_type_t t;
>      mfn_t mfn;
> @@ -1806,6 +1848,16 @@ long p2m_set_mem_access(struct domain *d, gfn_t gfn, uint32_t nr,
>  #undef ACCESS
>      };
>  
> +    /* altp2m view 0 is treated as the hostp2m */
> +    if ( altp2m_idx )
> +    {
> +        if ( altp2m_idx >= MAX_ALTP2M ||
> +             d->arch.altp2m_eptp[altp2m_idx] == INVALID_MFN )
> +            return -EINVAL;
> +
> +        ap2m = d->arch.altp2m_p2m[altp2m_idx];
> +    }
> +
>      switch ( access )
>      {
>      case 0 ... ARRAY_SIZE(memaccess) - 1:
> @@ -1826,12 +1878,25 @@ long p2m_set_mem_access(struct domain *d, gfn_t gfn, uint32_t nr,
>      }
>  
>      p2m_lock(p2m);
> +    if ( ap2m )
> +        p2m_lock(ap2m);
> +
>      for ( gfn_l = gfn_x(gfn) + start; nr > start; ++gfn_l )
>      {
> -        mfn = p2m->get_entry(p2m, gfn_l, &t, &_a, 0, NULL, NULL);
> -        rc = p2m->set_entry(p2m, gfn_l, mfn, PAGE_ORDER_4K, t, a, -1);
> -        if ( rc )
> -            break;
> +        if ( ap2m )
> +        {
> +            rc = p2m_set_altp2m_mem_access(d, p2m, ap2m, a, _gfn(gfn_l));
> +            /* If the corresponding mfn is invalid we will just skip it */
> +            if ( rc && rc != -ESRCH )
> +                break;
> +        }
> +        else
> +        {
> +            mfn = p2m->get_entry(p2m, gfn_l, &t, &_a, 0, NULL, NULL);
> +            rc = p2m->set_entry(p2m, gfn_l, mfn, PAGE_ORDER_4K, t, a, -1);
> +            if ( rc )
> +                break;
> +        }
>  
>          /* Check for continuation if it's not the last iteration. */
>          if ( nr > ++start && !(start & mask) && hypercall_preempt_check() )
> @@ -1840,7 +1905,11 @@ long p2m_set_mem_access(struct domain *d, gfn_t gfn, uint32_t nr,
>              break;
>          }
>      }
> +
> +    if ( ap2m )
> +        p2m_unlock(ap2m);
>      p2m_unlock(p2m);
> +
>      return rc;
>  }
>  
> @@ -2395,93 +2464,6 @@ int p2m_switch_domain_altp2m_by_id(struct domain *d, unsigned int idx)
>      return rc;
>  }
>  
> -int p2m_set_altp2m_mem_access(struct domain *d, unsigned int idx,
> -                              gfn_t gfn, xenmem_access_t access)
> -{
> -    struct p2m_domain *hp2m, *ap2m;
> -    p2m_access_t req_a, old_a;
> -    p2m_type_t t;
> -    mfn_t mfn;
> -    unsigned int page_order;
> -    int rc = -EINVAL;
> -
> -    static const p2m_access_t memaccess[] = {
> -#define ACCESS(ac) [XENMEM_access_##ac] = p2m_access_##ac
> -        ACCESS(n),
> -        ACCESS(r),
> -        ACCESS(w),
> -        ACCESS(rw),
> -        ACCESS(x),
> -        ACCESS(rx),
> -        ACCESS(wx),
> -        ACCESS(rwx),
> -#undef ACCESS
> -    };
> -
> -    if ( idx >= MAX_ALTP2M || d->arch.altp2m_eptp[idx] == INVALID_MFN )
> -        return rc;
> -
> -    ap2m = d->arch.altp2m_p2m[idx];
> -
> -    switch ( access )
> -    {
> -    case 0 ... ARRAY_SIZE(memaccess) - 1:
> -        req_a = memaccess[access];
> -        break;
> -    case XENMEM_access_default:
> -        req_a = ap2m->default_access;
> -        break;
> -    default:
> -        return rc;
> -    }
> -
> -    /* If request to set default access */
> -    if ( gfn_x(gfn) == INVALID_GFN )
> -    {
> -        ap2m->default_access = req_a;
> -        return 0;
> -    }
> -
> -    hp2m = p2m_get_hostp2m(d);
> -
> -    p2m_lock(ap2m);
> -
> -    mfn = ap2m->get_entry(ap2m, gfn_x(gfn), &t, &old_a, 0, NULL, NULL);
> -
> -    /* Check host p2m if no valid entry in alternate */
> -    if ( !mfn_valid(mfn) )
> -    {
> -        mfn = hp2m->get_entry(hp2m, gfn_x(gfn), &t, &old_a,
> -                              P2M_ALLOC | P2M_UNSHARE, &page_order, NULL);
> -
> -        if ( !mfn_valid(mfn) || t != p2m_ram_rw )
> -            goto out;
> -
> -        /* If this is a superpage, copy that first */
> -        if ( page_order != PAGE_ORDER_4K )
> -        {
> -            gfn_t gfn2;
> -            unsigned long mask;
> -            mfn_t mfn2;
> -
> -            mask = ~((1UL << page_order) - 1);
> -            gfn2 = _gfn(gfn_x(gfn) & mask);
> -            mfn2 = _mfn(mfn_x(mfn) & mask);
> -
> -            if ( ap2m->set_entry(ap2m, gfn_x(gfn2), mfn2, page_order, t, old_a, 1) )
> -                goto out;
> -        }
> -    }
> -
> -    if ( !ap2m->set_entry(ap2m, gfn_x(gfn), mfn, PAGE_ORDER_4K, t, req_a,
> -                          (current->domain != d)) )
> -        rc = 0;
> -
> - out:
> -    p2m_unlock(ap2m);
> -    return rc;
> -}
> -
>  int p2m_change_altp2m_gfn(struct domain *d, unsigned int idx,
>                            gfn_t old_gfn, gfn_t new_gfn)
>  {
> diff --git a/xen/common/mem_access.c b/xen/common/mem_access.c
> index 159c036..92ebead 100644
> --- a/xen/common/mem_access.c
> +++ b/xen/common/mem_access.c
> @@ -67,7 +67,7 @@ int mem_access_memop(unsigned long cmd,
>              break;
>  
>          rc = p2m_set_mem_access(d, _gfn(mao.pfn), mao.nr, start_iter,
> -                                MEMOP_CMD_MASK, mao.access);
> +                                MEMOP_CMD_MASK, mao.access, 0);
>          if ( rc > 0 )
>          {
>              ASSERT(!(rc & MEMOP_CMD_MASK));
> diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
> index fa46dd9..c0df1ea 100644
> --- a/xen/include/asm-x86/p2m.h
> +++ b/xen/include/asm-x86/p2m.h
> @@ -808,10 +808,6 @@ int p2m_destroy_altp2m_by_id(struct domain *d, unsigned int idx);
>  /* Switch alternate p2m for entire domain */
>  int p2m_switch_domain_altp2m_by_id(struct domain *d, unsigned int idx);
>  
> -/* Set access type for a gfn */
> -int p2m_set_altp2m_mem_access(struct domain *d, unsigned int idx,
> -                              gfn_t gfn, xenmem_access_t access);
> -
>  /* Change a gfn->mfn mapping */
>  int p2m_change_altp2m_gfn(struct domain *d, unsigned int idx,
>                            gfn_t old_gfn, gfn_t new_gfn);
> diff --git a/xen/include/xen/p2m-common.h b/xen/include/xen/p2m-common.h
> index 47c40c7..8b70459 100644
> --- a/xen/include/xen/p2m-common.h
> +++ b/xen/include/xen/p2m-common.h
> @@ -49,7 +49,8 @@ int unmap_mmio_regions(struct domain *d,
>   * If gfn == INVALID_GFN, sets the default access type.
>   */
>  long p2m_set_mem_access(struct domain *d, gfn_t gfn, uint32_t nr,
> -                        uint32_t start, uint32_t mask, xenmem_access_t access);
> +                        uint32_t start, uint32_t mask, xenmem_access_t access,
> +                        unsigned int altp2m_idx);
>  
>  /*
>   * Get access type for a gfn.
>
diff mbox

Patch

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 2190908..8568087 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1709,13 +1709,13 @@  bool_t p2m_mem_access_check(paddr_t gpa, vaddr_t gla, const struct npfec npfec)
     if ( npfec.write_access && xma == XENMEM_access_rx2rw )
     {
         rc = p2m_set_mem_access(v->domain, _gfn(paddr_to_pfn(gpa)), 1,
-                                0, ~0, XENMEM_access_rw);
+                                0, ~0, XENMEM_access_rw, 0);
         return false;
     }
     else if ( xma == XENMEM_access_n2rwx )
     {
         rc = p2m_set_mem_access(v->domain, _gfn(paddr_to_pfn(gpa)), 1,
-                                0, ~0, XENMEM_access_rwx);
+                                0, ~0, XENMEM_access_rwx, 0);
     }
 
     /* Otherwise, check if there is a vm_event monitor subscriber */
@@ -1737,7 +1737,7 @@  bool_t p2m_mem_access_check(paddr_t gpa, vaddr_t gla, const struct npfec npfec)
                 /* A listener is not required, so clear the access
                  * restrictions. */
                 rc = p2m_set_mem_access(v->domain, _gfn(paddr_to_pfn(gpa)), 1,
-                                        0, ~0, XENMEM_access_rwx);
+                                        0, ~0, XENMEM_access_rwx, 0);
             }
         }
 
@@ -1788,7 +1788,8 @@  bool_t p2m_mem_access_check(paddr_t gpa, vaddr_t gla, const struct npfec npfec)
  * If gfn == INVALID_GFN, sets the default access type.
  */
 long p2m_set_mem_access(struct domain *d, gfn_t gfn, uint32_t nr,
-                        uint32_t start, uint32_t mask, xenmem_access_t access)
+                        uint32_t start, uint32_t mask, xenmem_access_t access,
+                        unsigned int altp2m_idx)
 {
     struct p2m_domain *p2m = p2m_get_hostp2m(d);
     p2m_access_t a;
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 674feea..37305fb 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -6398,9 +6398,9 @@  static int do_altp2m_op(
         if ( a.u.set_mem_access.pad )
             rc = -EINVAL;
         else
-            rc = p2m_set_altp2m_mem_access(d, a.u.set_mem_access.view,
-                    _gfn(a.u.set_mem_access.gfn),
-                    a.u.set_mem_access.hvmmem_access);
+            rc = p2m_set_mem_access(d, _gfn(a.u.set_mem_access.gfn), 1, 0, 0,
+                                    a.u.set_mem_access.hvmmem_access,
+                                    a.u.set_mem_access.view);
         break;
 
     case HVMOP_altp2m_change_gfn:
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index a45ee35..226490a 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -1777,14 +1777,56 @@  bool_t p2m_mem_access_check(paddr_t gpa, unsigned long gla,
     return (p2ma == p2m_access_n2rwx);
 }
 
+static inline
+int p2m_set_altp2m_mem_access(struct domain *d, struct p2m_domain *hp2m,
+                              struct p2m_domain *ap2m, p2m_access_t a,
+                              gfn_t gfn)
+{
+    mfn_t mfn;
+    p2m_type_t t;
+    p2m_access_t old_a;
+    unsigned int page_order;
+    unsigned long gfn_l = gfn_x(gfn);
+    int rc;
+
+    mfn = ap2m->get_entry(ap2m, gfn_l, &t, &old_a, 0, NULL, NULL);
+
+    /* Check host p2m if no valid entry in alternate */
+    if ( !mfn_valid(mfn) )
+    {
+        mfn = hp2m->get_entry(hp2m, gfn_l, &t, &old_a,
+                              P2M_ALLOC | P2M_UNSHARE, &page_order, NULL);
+
+        rc = -ESRCH;
+        if ( !mfn_valid(mfn) || t != p2m_ram_rw )
+            return rc;
+
+        /* If this is a superpage, copy that first */
+        if ( page_order != PAGE_ORDER_4K )
+        {
+            unsigned long mask = ~((1UL << page_order) - 1);
+            unsigned long gfn2_l = gfn_l & mask;
+            mfn_t mfn2 = _mfn(mfn_x(mfn) & mask);
+
+            rc = ap2m->set_entry(ap2m, gfn2_l, mfn2, page_order, t, old_a, 1);
+            if ( rc )
+                return rc;
+        }
+    }
+
+    return ap2m->set_entry(ap2m, gfn_l, mfn, PAGE_ORDER_4K, t, a,
+                         (current->domain != d));
+}
+
 /*
  * Set access type for a region of gfns.
  * If gfn == INVALID_GFN, sets the default access type.
  */
 long p2m_set_mem_access(struct domain *d, gfn_t gfn, uint32_t nr,
-                        uint32_t start, uint32_t mask, xenmem_access_t access)
+                        uint32_t start, uint32_t mask, xenmem_access_t access,
+                        unsigned int altp2m_idx)
 {
-    struct p2m_domain *p2m = p2m_get_hostp2m(d);
+    struct p2m_domain *p2m = p2m_get_hostp2m(d), *ap2m = NULL;
     p2m_access_t a, _a;
     p2m_type_t t;
     mfn_t mfn;
@@ -1806,6 +1848,16 @@  long p2m_set_mem_access(struct domain *d, gfn_t gfn, uint32_t nr,
 #undef ACCESS
     };
 
+    /* altp2m view 0 is treated as the hostp2m */
+    if ( altp2m_idx )
+    {
+        if ( altp2m_idx >= MAX_ALTP2M ||
+             d->arch.altp2m_eptp[altp2m_idx] == INVALID_MFN )
+            return -EINVAL;
+
+        ap2m = d->arch.altp2m_p2m[altp2m_idx];
+    }
+
     switch ( access )
     {
     case 0 ... ARRAY_SIZE(memaccess) - 1:
@@ -1826,12 +1878,25 @@  long p2m_set_mem_access(struct domain *d, gfn_t gfn, uint32_t nr,
     }
 
     p2m_lock(p2m);
+    if ( ap2m )
+        p2m_lock(ap2m);
+
     for ( gfn_l = gfn_x(gfn) + start; nr > start; ++gfn_l )
     {
-        mfn = p2m->get_entry(p2m, gfn_l, &t, &_a, 0, NULL, NULL);
-        rc = p2m->set_entry(p2m, gfn_l, mfn, PAGE_ORDER_4K, t, a, -1);
-        if ( rc )
-            break;
+        if ( ap2m )
+        {
+            rc = p2m_set_altp2m_mem_access(d, p2m, ap2m, a, _gfn(gfn_l));
+            /* If the corresponding mfn is invalid we will just skip it */
+            if ( rc && rc != -ESRCH )
+                break;
+        }
+        else
+        {
+            mfn = p2m->get_entry(p2m, gfn_l, &t, &_a, 0, NULL, NULL);
+            rc = p2m->set_entry(p2m, gfn_l, mfn, PAGE_ORDER_4K, t, a, -1);
+            if ( rc )
+                break;
+        }
 
         /* Check for continuation if it's not the last iteration. */
         if ( nr > ++start && !(start & mask) && hypercall_preempt_check() )
@@ -1840,7 +1905,11 @@  long p2m_set_mem_access(struct domain *d, gfn_t gfn, uint32_t nr,
             break;
         }
     }
+
+    if ( ap2m )
+        p2m_unlock(ap2m);
     p2m_unlock(p2m);
+
     return rc;
 }
 
@@ -2395,93 +2464,6 @@  int p2m_switch_domain_altp2m_by_id(struct domain *d, unsigned int idx)
     return rc;
 }
 
-int p2m_set_altp2m_mem_access(struct domain *d, unsigned int idx,
-                              gfn_t gfn, xenmem_access_t access)
-{
-    struct p2m_domain *hp2m, *ap2m;
-    p2m_access_t req_a, old_a;
-    p2m_type_t t;
-    mfn_t mfn;
-    unsigned int page_order;
-    int rc = -EINVAL;
-
-    static const p2m_access_t memaccess[] = {
-#define ACCESS(ac) [XENMEM_access_##ac] = p2m_access_##ac
-        ACCESS(n),
-        ACCESS(r),
-        ACCESS(w),
-        ACCESS(rw),
-        ACCESS(x),
-        ACCESS(rx),
-        ACCESS(wx),
-        ACCESS(rwx),
-#undef ACCESS
-    };
-
-    if ( idx >= MAX_ALTP2M || d->arch.altp2m_eptp[idx] == INVALID_MFN )
-        return rc;
-
-    ap2m = d->arch.altp2m_p2m[idx];
-
-    switch ( access )
-    {
-    case 0 ... ARRAY_SIZE(memaccess) - 1:
-        req_a = memaccess[access];
-        break;
-    case XENMEM_access_default:
-        req_a = ap2m->default_access;
-        break;
-    default:
-        return rc;
-    }
-
-    /* If request to set default access */
-    if ( gfn_x(gfn) == INVALID_GFN )
-    {
-        ap2m->default_access = req_a;
-        return 0;
-    }
-
-    hp2m = p2m_get_hostp2m(d);
-
-    p2m_lock(ap2m);
-
-    mfn = ap2m->get_entry(ap2m, gfn_x(gfn), &t, &old_a, 0, NULL, NULL);
-
-    /* Check host p2m if no valid entry in alternate */
-    if ( !mfn_valid(mfn) )
-    {
-        mfn = hp2m->get_entry(hp2m, gfn_x(gfn), &t, &old_a,
-                              P2M_ALLOC | P2M_UNSHARE, &page_order, NULL);
-
-        if ( !mfn_valid(mfn) || t != p2m_ram_rw )
-            goto out;
-
-        /* If this is a superpage, copy that first */
-        if ( page_order != PAGE_ORDER_4K )
-        {
-            gfn_t gfn2;
-            unsigned long mask;
-            mfn_t mfn2;
-
-            mask = ~((1UL << page_order) - 1);
-            gfn2 = _gfn(gfn_x(gfn) & mask);
-            mfn2 = _mfn(mfn_x(mfn) & mask);
-
-            if ( ap2m->set_entry(ap2m, gfn_x(gfn2), mfn2, page_order, t, old_a, 1) )
-                goto out;
-        }
-    }
-
-    if ( !ap2m->set_entry(ap2m, gfn_x(gfn), mfn, PAGE_ORDER_4K, t, req_a,
-                          (current->domain != d)) )
-        rc = 0;
-
- out:
-    p2m_unlock(ap2m);
-    return rc;
-}
-
 int p2m_change_altp2m_gfn(struct domain *d, unsigned int idx,
                           gfn_t old_gfn, gfn_t new_gfn)
 {
diff --git a/xen/common/mem_access.c b/xen/common/mem_access.c
index 159c036..92ebead 100644
--- a/xen/common/mem_access.c
+++ b/xen/common/mem_access.c
@@ -67,7 +67,7 @@  int mem_access_memop(unsigned long cmd,
             break;
 
         rc = p2m_set_mem_access(d, _gfn(mao.pfn), mao.nr, start_iter,
-                                MEMOP_CMD_MASK, mao.access);
+                                MEMOP_CMD_MASK, mao.access, 0);
         if ( rc > 0 )
         {
             ASSERT(!(rc & MEMOP_CMD_MASK));
diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
index fa46dd9..c0df1ea 100644
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -808,10 +808,6 @@  int p2m_destroy_altp2m_by_id(struct domain *d, unsigned int idx);
 /* Switch alternate p2m for entire domain */
 int p2m_switch_domain_altp2m_by_id(struct domain *d, unsigned int idx);
 
-/* Set access type for a gfn */
-int p2m_set_altp2m_mem_access(struct domain *d, unsigned int idx,
-                              gfn_t gfn, xenmem_access_t access);
-
 /* Change a gfn->mfn mapping */
 int p2m_change_altp2m_gfn(struct domain *d, unsigned int idx,
                           gfn_t old_gfn, gfn_t new_gfn);
diff --git a/xen/include/xen/p2m-common.h b/xen/include/xen/p2m-common.h
index 47c40c7..8b70459 100644
--- a/xen/include/xen/p2m-common.h
+++ b/xen/include/xen/p2m-common.h
@@ -49,7 +49,8 @@  int unmap_mmio_regions(struct domain *d,
  * If gfn == INVALID_GFN, sets the default access type.
  */
 long p2m_set_mem_access(struct domain *d, gfn_t gfn, uint32_t nr,
-                        uint32_t start, uint32_t mask, xenmem_access_t access);
+                        uint32_t start, uint32_t mask, xenmem_access_t access,
+                        unsigned int altp2m_idx);
 
 /*
  * Get access type for a gfn.