[for-4.7 v3 0/2] xen: XENMEM_add_to_physmap_batch: Mark 'foreign_id" as reserved for dev_mmio

[for-4.7 v3 0/2] xen: XENMEM_add_to_physmap_batch: Mark 'foreign_id" as reserved for dev_mmio

[Julien Grall]

Hello all,

This series is based on the thread [1]. To allow future extension, the new
space dev_mmio should have all unused fields marked as reserved.

This series is candidate for Xen 4.7, the first patch ensure a clean ABI
for the new space which will allow future extension.

See in each patch for all the changes.

Regards,

[1] http://lists.xenproject.org/archives/html/xen-devel/2016-05/msg02328.html

Julien Grall (2):
  xen: XENMEM_add_to_physmap_batch: Mark 'foreign_id' as reserved for
    dev_mmio
  xen/arm: Document the behavior of XENMAPSPACE_dev_mmio

 xen/arch/arm/mm.c           |  9 +++++++--
 xen/arch/x86/mm.c           |  4 ++--
 xen/common/compat/memory.c  |  7 +++++++
 xen/common/memory.c         | 12 +++++++++---
 xen/include/public/memory.h | 16 ++++++++++++++--
 xen/include/xen/mm.h        |  3 ++-
 6 files changed, 41 insertions(+), 10 deletions(-)

-- 
1.9.1


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

[for-4.7 v3 1/2] xen: XENMEM_add_to_physmap_batch: Mark 'foreign_id' as reserved for dev_mmio

[Julien Grall]

The field 'foreign_id' is not used when the space is dev_mmio. As the
space is not yet part of the stable ABI, the field is marked as reserved
for future use.

The value should always be 0, other values will return -EOPNOTSUPP.

Signed-off-by: Julien Grall <julien.grall@arm.com>

---
    Changes in v3:
        - s/add_to_physmap_batch_extra/xen_add_to_physmap_batch_extra/
        - Add a comment in compat

    Changes in v2:
        - Return -EOPNOTSUPP rather than -ENOSYS
        - Introduce a union in the structure xenmem_add_to_physmap_batch
---
 xen/arch/arm/mm.c           |  9 +++++++--
 xen/arch/x86/mm.c           |  4 ++--
 xen/common/compat/memory.c  |  7 +++++++
 xen/common/memory.c         | 12 +++++++++---
 xen/include/public/memory.h | 10 +++++++++-
 xen/include/xen/mm.h        |  3 ++-
 6 files changed, 36 insertions(+), 9 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index b46e23e..0aa8092 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1047,7 +1047,7 @@ void share_xen_page_with_privileged_guests(
 int xenmem_add_to_physmap_one(
     struct domain *d,
     unsigned int space,
-    domid_t foreign_domid,
+    union xen_add_to_physmap_batch_extra extra,
     unsigned long idx,
     xen_pfn_t gpfn)
 {
@@ -1103,7 +1103,8 @@ int xenmem_add_to_physmap_one(
     {
         struct domain *od;
         p2m_type_t p2mt;
-        od = rcu_lock_domain_by_any_id(foreign_domid);
+
+        od = rcu_lock_domain_by_any_id(extra.foreign_domid);
         if ( od == NULL )
             return -ESRCH;
 
@@ -1143,6 +1144,10 @@ int xenmem_add_to_physmap_one(
         break;
     }
     case XENMAPSPACE_dev_mmio:
+        /* extra should be 0. Reserved for future use. */
+        if ( extra.res0 )
+            return -EOPNOTSUPP;
+
         rc = map_dev_mmio_region(d, gpfn, 1, idx);
         return rc;
 
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 03a4d5b..8d10a3e 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -4769,7 +4769,7 @@ static int handle_iomem_range(unsigned long s, unsigned long e, void *p)
 int xenmem_add_to_physmap_one(
     struct domain *d,
     unsigned int space,
-    domid_t foreign_domid,
+    union xen_add_to_physmap_batch_extra extra,
     unsigned long idx,
     xen_pfn_t gpfn)
 {
@@ -4830,7 +4830,7 @@ int xenmem_add_to_physmap_one(
             break;
         }
         case XENMAPSPACE_gmfn_foreign:
-            return p2m_add_foreign(d, idx, gpfn, foreign_domid);
+            return p2m_add_foreign(d, idx, gpfn, extra.foreign_domid);
         default:
             break;
     }
diff --git a/xen/common/compat/memory.c b/xen/common/compat/memory.c
index 19a914d..20c7671 100644
--- a/xen/common/compat/memory.c
+++ b/xen/common/compat/memory.c
@@ -253,6 +253,13 @@ int compat_memory_op(unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) compat)
             unsigned int size = cmp.atpb.size;
             xen_ulong_t *idxs = (void *)(nat.atpb + 1);
             xen_pfn_t *gpfns = (void *)(idxs + limit);
+            /*
+             * The union will always be 16-bit width. So it is not
+             * necessary to have the exact field which correspond to the
+             * space.
+             */
+            enum XLAT_add_to_physmap_batch_u u =
+                XLAT_add_to_physmap_batch_u_res0;
 
             if ( copy_from_guest(&cmp.atpb, compat, 1) ||
                  !compat_handle_okay(cmp.atpb.idxs, size) ||
diff --git a/xen/common/memory.c b/xen/common/memory.c
index 644f81a..ccc6436 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -639,9 +639,15 @@ static int xenmem_add_to_physmap(struct domain *d,
 {
     unsigned int done = 0;
     long rc = 0;
+    union xen_add_to_physmap_batch_extra extra;
+
+    if ( xatp->space != XENMAPSPACE_gmfn_foreign )
+        extra.res0 = 0;
+    else
+        extra.foreign_domid = DOMID_INVALID;
 
     if ( xatp->space != XENMAPSPACE_gmfn_range )
-        return xenmem_add_to_physmap_one(d, xatp->space, DOMID_INVALID,
+        return xenmem_add_to_physmap_one(d, xatp->space, extra,
                                          xatp->idx, xatp->gpfn);
 
     if ( xatp->size < start )
@@ -658,7 +664,7 @@ static int xenmem_add_to_physmap(struct domain *d,
 
     while ( xatp->size > done )
     {
-        rc = xenmem_add_to_physmap_one(d, xatp->space, DOMID_INVALID,
+        rc = xenmem_add_to_physmap_one(d, xatp->space, extra,
                                        xatp->idx, xatp->gpfn);
         if ( rc < 0 )
             break;
@@ -719,7 +725,7 @@ static int xenmem_add_to_physmap_batch(struct domain *d,
         }
 
         rc = xenmem_add_to_physmap_one(d, xatpb->space,
-                                       xatpb->foreign_domid,
+                                       xatpb->u,
                                        idx, gpfn);
 
         if ( unlikely(__copy_to_guest_offset(xatpb->errs, 0, &rc, 1)) )
diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
index fe52ee1..5f7e0d2 100644
--- a/xen/include/public/memory.h
+++ b/xen/include/public/memory.h
@@ -259,7 +259,15 @@ struct xen_add_to_physmap_batch {
 
     /* Number of pages to go through */
     uint16_t size;
-    domid_t foreign_domid; /* IFF gmfn_foreign */
+
+#if __XEN_INTERFACE_VERSION__ < 0x00040700
+    domid_t foreign_domid; /* IFF gmfn_foreign. Should be 0 for other spaces. */
+#else
+    union xen_add_to_physmap_batch_extra {
+        domid_t foreign_domid; /* gmfn_foreign */
+        uint16_t res0;  /* All the other spaces. Should be 0 */
+    } u;
+#endif
 
     /* Indexes into space being mapped. */
     XEN_GUEST_HANDLE(xen_ulong_t) idxs;
diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
index d4721fc..3cf646a 100644
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -50,6 +50,7 @@
 #include <xen/list.h>
 #include <xen/spinlock.h>
 #include <xen/typesafe.h>
+#include <public/memory.h>
 
 TYPE_SAFE(unsigned long, mfn);
 #define PRI_mfn          "05lx"
@@ -505,7 +506,7 @@ void scrub_one_page(struct page_info *);
 #endif
 
 int xenmem_add_to_physmap_one(struct domain *d, unsigned int space,
-                              domid_t foreign_domid,
+                              union xen_add_to_physmap_batch_extra extra,
                               unsigned long idx, xen_pfn_t gpfn);
 
 /* Returns 1 on success, 0 on error, negative if the ring
-- 
1.9.1


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

[for-4.7 v3 2/2] xen/arm: Document the behavior of XENMAPSPACE_dev_mmio

[Julien Grall]

Currently, XENMAPSPACE_dev_mmio maps MMIO regions using one of the most
restrictive memory attribute (Device-nGnRE).

Signed-off-by: Julien Grall <julien.grall@arm.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Acked-by: Jan Beulich <jbeulich@suse.com>

---
    Changes in v3:
        - Update comment
        - Add Stefano's Jan's ack

    Changes in v2:
        - s/Device_nGnRE/Device-nGnRE/ to match the spec
        - Update the comment to mention the name for ARMv7.
---
 xen/include/public/memory.h | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
index 5f7e0d2..29ec571 100644
--- a/xen/include/public/memory.h
+++ b/xen/include/public/memory.h
@@ -220,7 +220,11 @@ DEFINE_XEN_GUEST_HANDLE(xen_machphys_mapping_t);
 #define XENMAPSPACE_gmfn_range   3 /* GMFN range, XENMEM_add_to_physmap only. */
 #define XENMAPSPACE_gmfn_foreign 4 /* GMFN from another dom,
                                     * XENMEM_add_to_physmap_batch only. */
-#define XENMAPSPACE_dev_mmio     5 /* device mmio region */
+#define XENMAPSPACE_dev_mmio     5 /* device mmio region
+                                      ARM only; the region is mapped in
+                                      Stage-2 using the memory attribute
+                                      "Device-nGnRE" (previously named
+                                      "Device" on ARMv7) */
 /* ` } */
 
 /*
-- 
1.9.1


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

Re: [for-4.7 v3 1/2] xen: XENMEM_add_to_physmap_batch: Mark 'foreign_id' as reserved for dev_mmio

[Stefano Stabellini]

On Fri, 27 May 2016, Julien Grall wrote:
> The field 'foreign_id' is not used when the space is dev_mmio. As the
> space is not yet part of the stable ABI, the field is marked as reserved
> for future use.
> 
> The value should always be 0, other values will return -EOPNOTSUPP.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Acked-by: Stefano Stabellini <sstabellini@kernel.org>


>     Changes in v3:
>         - s/add_to_physmap_batch_extra/xen_add_to_physmap_batch_extra/
>         - Add a comment in compat
> 
>     Changes in v2:
>         - Return -EOPNOTSUPP rather than -ENOSYS
>         - Introduce a union in the structure xenmem_add_to_physmap_batch
> ---
>  xen/arch/arm/mm.c           |  9 +++++++--
>  xen/arch/x86/mm.c           |  4 ++--
>  xen/common/compat/memory.c  |  7 +++++++
>  xen/common/memory.c         | 12 +++++++++---
>  xen/include/public/memory.h | 10 +++++++++-
>  xen/include/xen/mm.h        |  3 ++-
>  6 files changed, 36 insertions(+), 9 deletions(-)
> 
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index b46e23e..0aa8092 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -1047,7 +1047,7 @@ void share_xen_page_with_privileged_guests(
>  int xenmem_add_to_physmap_one(
>      struct domain *d,
>      unsigned int space,
> -    domid_t foreign_domid,
> +    union xen_add_to_physmap_batch_extra extra,
>      unsigned long idx,
>      xen_pfn_t gpfn)
>  {
> @@ -1103,7 +1103,8 @@ int xenmem_add_to_physmap_one(
>      {
>          struct domain *od;
>          p2m_type_t p2mt;
> -        od = rcu_lock_domain_by_any_id(foreign_domid);
> +
> +        od = rcu_lock_domain_by_any_id(extra.foreign_domid);
>          if ( od == NULL )
>              return -ESRCH;
>  
> @@ -1143,6 +1144,10 @@ int xenmem_add_to_physmap_one(
>          break;
>      }
>      case XENMAPSPACE_dev_mmio:
> +        /* extra should be 0. Reserved for future use. */
> +        if ( extra.res0 )
> +            return -EOPNOTSUPP;
> +
>          rc = map_dev_mmio_region(d, gpfn, 1, idx);
>          return rc;
>  
> diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
> index 03a4d5b..8d10a3e 100644
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -4769,7 +4769,7 @@ static int handle_iomem_range(unsigned long s, unsigned long e, void *p)
>  int xenmem_add_to_physmap_one(
>      struct domain *d,
>      unsigned int space,
> -    domid_t foreign_domid,
> +    union xen_add_to_physmap_batch_extra extra,
>      unsigned long idx,
>      xen_pfn_t gpfn)
>  {
> @@ -4830,7 +4830,7 @@ int xenmem_add_to_physmap_one(
>              break;
>          }
>          case XENMAPSPACE_gmfn_foreign:
> -            return p2m_add_foreign(d, idx, gpfn, foreign_domid);
> +            return p2m_add_foreign(d, idx, gpfn, extra.foreign_domid);
>          default:
>              break;
>      }
> diff --git a/xen/common/compat/memory.c b/xen/common/compat/memory.c
> index 19a914d..20c7671 100644
> --- a/xen/common/compat/memory.c
> +++ b/xen/common/compat/memory.c
> @@ -253,6 +253,13 @@ int compat_memory_op(unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) compat)
>              unsigned int size = cmp.atpb.size;
>              xen_ulong_t *idxs = (void *)(nat.atpb + 1);
>              xen_pfn_t *gpfns = (void *)(idxs + limit);
> +            /*
> +             * The union will always be 16-bit width. So it is not
> +             * necessary to have the exact field which correspond to the
> +             * space.
> +             */
> +            enum XLAT_add_to_physmap_batch_u u =
> +                XLAT_add_to_physmap_batch_u_res0;
>  
>              if ( copy_from_guest(&cmp.atpb, compat, 1) ||
>                   !compat_handle_okay(cmp.atpb.idxs, size) ||
> diff --git a/xen/common/memory.c b/xen/common/memory.c
> index 644f81a..ccc6436 100644
> --- a/xen/common/memory.c
> +++ b/xen/common/memory.c
> @@ -639,9 +639,15 @@ static int xenmem_add_to_physmap(struct domain *d,
>  {
>      unsigned int done = 0;
>      long rc = 0;
> +    union xen_add_to_physmap_batch_extra extra;
> +
> +    if ( xatp->space != XENMAPSPACE_gmfn_foreign )
> +        extra.res0 = 0;
> +    else
> +        extra.foreign_domid = DOMID_INVALID;
>  
>      if ( xatp->space != XENMAPSPACE_gmfn_range )
> -        return xenmem_add_to_physmap_one(d, xatp->space, DOMID_INVALID,
> +        return xenmem_add_to_physmap_one(d, xatp->space, extra,
>                                           xatp->idx, xatp->gpfn);
>  
>      if ( xatp->size < start )
> @@ -658,7 +664,7 @@ static int xenmem_add_to_physmap(struct domain *d,
>  
>      while ( xatp->size > done )
>      {
> -        rc = xenmem_add_to_physmap_one(d, xatp->space, DOMID_INVALID,
> +        rc = xenmem_add_to_physmap_one(d, xatp->space, extra,
>                                         xatp->idx, xatp->gpfn);
>          if ( rc < 0 )
>              break;
> @@ -719,7 +725,7 @@ static int xenmem_add_to_physmap_batch(struct domain *d,
>          }
>  
>          rc = xenmem_add_to_physmap_one(d, xatpb->space,
> -                                       xatpb->foreign_domid,
> +                                       xatpb->u,
>                                         idx, gpfn);
>  
>          if ( unlikely(__copy_to_guest_offset(xatpb->errs, 0, &rc, 1)) )
> diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
> index fe52ee1..5f7e0d2 100644
> --- a/xen/include/public/memory.h
> +++ b/xen/include/public/memory.h
> @@ -259,7 +259,15 @@ struct xen_add_to_physmap_batch {
>  
>      /* Number of pages to go through */
>      uint16_t size;
> -    domid_t foreign_domid; /* IFF gmfn_foreign */
> +
> +#if __XEN_INTERFACE_VERSION__ < 0x00040700
> +    domid_t foreign_domid; /* IFF gmfn_foreign. Should be 0 for other spaces. */
> +#else
> +    union xen_add_to_physmap_batch_extra {
> +        domid_t foreign_domid; /* gmfn_foreign */
> +        uint16_t res0;  /* All the other spaces. Should be 0 */
> +    } u;
> +#endif
>  
>      /* Indexes into space being mapped. */
>      XEN_GUEST_HANDLE(xen_ulong_t) idxs;
> diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
> index d4721fc..3cf646a 100644
> --- a/xen/include/xen/mm.h
> +++ b/xen/include/xen/mm.h
> @@ -50,6 +50,7 @@
>  #include <xen/list.h>
>  #include <xen/spinlock.h>
>  #include <xen/typesafe.h>
> +#include <public/memory.h>
>  
>  TYPE_SAFE(unsigned long, mfn);
>  #define PRI_mfn          "05lx"
> @@ -505,7 +506,7 @@ void scrub_one_page(struct page_info *);
>  #endif
>  
>  int xenmem_add_to_physmap_one(struct domain *d, unsigned int space,
> -                              domid_t foreign_domid,
> +                              union xen_add_to_physmap_batch_extra extra,
>                                unsigned long idx, xen_pfn_t gpfn);
>  
>  /* Returns 1 on success, 0 on error, negative if the ring
> -- 
> 1.9.1
> 

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

Re: [for-4.7 v3 1/2] xen: XENMEM_add_to_physmap_batch: Mark 'foreign_id' as reserved for dev_mmio

[Jan Beulich]

>>> On 27.05.16 at 18:06, <julien.grall@arm.com> wrote:
> The field 'foreign_id' is not used when the space is dev_mmio. As the
> space is not yet part of the stable ABI, the field is marked as reserved
> for future use.
> 
> The value should always be 0, other values will return -EOPNOTSUPP.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Acked-by: Jan Beulich <jbeulich@suse.com>


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

Re: [for-4.7 v3 0/2] xen: XENMEM_add_to_physmap_batch: Mark 'foreign_id" as reserved for dev_mmio

[Wei Liu]

On Fri, May 27, 2016 at 05:06:19PM +0100, Julien Grall wrote:
> Hello all,
> 
> This series is based on the thread [1]. To allow future extension, the new
> space dev_mmio should have all unused fields marked as reserved.
> 
> This series is candidate for Xen 4.7, the first patch ensure a clean ABI
> for the new space which will allow future extension.
> 
> See in each patch for all the changes.
> 
> Regards,
> 
> [1] http://lists.xenproject.org/archives/html/xen-devel/2016-05/msg02328.html
> 
> Julien Grall (2):
>   xen: XENMEM_add_to_physmap_batch: Mark 'foreign_id' as reserved for
>     dev_mmio
>   xen/arm: Document the behavior of XENMAPSPACE_dev_mmio
> 
>  xen/arch/arm/mm.c           |  9 +++++++--
>  xen/arch/x86/mm.c           |  4 ++--
>  xen/common/compat/memory.c  |  7 +++++++
>  xen/common/memory.c         | 12 +++++++++---
>  xen/include/public/memory.h | 16 ++++++++++++++--
>  xen/include/xen/mm.h        |  3 ++-
>  6 files changed, 41 insertions(+), 10 deletions(-)
> 
> -- 
> 1.9.1
> 

Release-acked-by: Wei Liu <wei.liu2@citrix.com>

Thanks everyone.

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