[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v3 2/5] xen: extend XEN_DOMCTL_memory_mapping to handle memory policy



Hi,

On 6/19/19 12:20 AM, Stefano Stabellini wrote:
Reuse the existing padding field to pass memory policy information. On
Arm, the caller can specify whether the memory should be mapped as
Device-nGnRE (Device Memory on Armv7) at stage-2, which is the default
and the only possibility today, or cacheable memory write-back. The
resulting memory attributes will be a combination of stage-2 and stage-1
memory attributes: it will actually be the strongest between the 2
stages attributes.

On x86, the only option is uncachable. The current behavior becomes the
default (numerically '0'). Also explicitely set the memory_policy field
to 0 in libxc.

On ARM, map Device-nGnRE as p2m_mmio_direct_dev (as it is already done

s/ARM/Arm/

today) and WB cacheable memory as p2m_mmio_direct_c.

On x86, return error if the memory policy requested is not
MEMORY_POLICY_X86_UC_MINUS.

Signed-off-by: Stefano Stabellini <stefanos@xxxxxxxxxx>
CC: JBeulich@xxxxxxxx
CC: andrew.cooper3@xxxxxxxxxx

---

Andrew suggested to remove MEMORY_POLICY_X86_UC_MINUS completely.
If that's the consensus I am happy to respin the series removing code.


Changes in v3:
- error handling in default label of the switch
- set memory_policy to 0 in libxc
- improve commit message
- improve comments
- s/Device-nGRE/Device-nGnRE/g
- add in-code comment
- s/MEMORY_POLICY_X86_UC/MEMORY_POLICY_X86_UC_MINUS/g
- #ifdef hypercall defines according to arch

Changes in v2:
- rebase
- use p2m_mmio_direct_c
- use EOPNOTSUPP
- rename cache_policy to memory policy
- rename MEMORY_POLICY_DEVMEM to MEMORY_POLICY_ARM_DEV_nGRE
- rename MEMORY_POLICY_MEMORY to MEMORY_POLICY_ARM_MEM_WB
- add MEMORY_POLICY_X86_UC
- add MEMORY_POLICY_DEFAULT and use it
---
  tools/libxc/xc_domain.c     |  1 +
  xen/common/domctl.c         | 24 ++++++++++++++++++++++--
  xen/include/public/domctl.h | 23 ++++++++++++++++++++++-
  3 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
index 05d771f2ce..8531298563 100644
--- a/tools/libxc/xc_domain.c
+++ b/tools/libxc/xc_domain.c
@@ -2070,6 +2070,7 @@ int xc_domain_memory_mapping(
      domctl.cmd = XEN_DOMCTL_memory_mapping;
      domctl.domain = domid;
      domctl.u.memory_mapping.add_mapping = add_mapping;
+    domctl.u.memory_mapping.memory_policy = 0;
      max_batch_sz = nr_mfns;
      do
      {
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index c6fd88d285..f21f6957b0 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -928,6 +928,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) 
u_domctl)
          unsigned long mfn_end = mfn + nr_mfns - 1;
          int add = op->u.memory_mapping.add_mapping;
          p2m_type_t p2mt;
+        uint32_t memory_policy = op->u.memory_mapping.memory_policy;
ret = -EINVAL;
          if ( mfn_end < mfn || /* wrap? */
@@ -958,9 +959,28 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) 
u_domctl)
          if ( add )
          {
              printk(XENLOG_G_DEBUG
-                   "memory_map:add: dom%d gfn=%lx mfn=%lx nr=%lx\n",
-                   d->domain_id, gfn, mfn, nr_mfns);
+                   "memory_map:add: dom%d gfn=%lx mfn=%lx nr=%lx cache=%u\n",
+                   d->domain_id, gfn, mfn, nr_mfns, memory_policy);
+ switch ( memory_policy )
+            {
+#ifdef CONFIG_ARM
+                case MEMORY_POLICY_ARM_MEM_WB:
+                    p2mt = p2m_mmio_direct_c;
+                    break;
+                case MEMORY_POLICY_ARM_DEV_nGnRE:
+                    p2mt = p2m_mmio_direct_dev;
+                    break;
+#endif
+#ifdef CONFIG_X86
+                case MEMORY_POLICY_X86_UC_MINUS:
+                    p2mt = p2m_mmio_direct;
+                    break;
+#endif
+                default:

AFAICT, ret will be zero in this path and therefore the caller may think the mapping succeeded. I think we want to set 'ret' to -EINVAL.

+                    domctl_lock_release();
+                    goto domctl_out_unlock_domonly;
+            }
              ret = map_mmio_regions(d, _gfn(gfn), nr_mfns, _mfn(mfn), p2mt);
              if ( ret < 0 )
                  printk(XENLOG_G_WARNING
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index 19486d5e32..e51caada35 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -571,12 +571,33 @@ struct xen_domctl_bind_pt_irq {
  */
  #define DPCI_ADD_MAPPING         1
  #define DPCI_REMOVE_MAPPING      0
+/*
+ * Default memory policy. Corresponds to:
+ * Arm: MEMORY_POLICY_ARM_DEV_nGnRE
+ * x86: MEMORY_POLICY_X86_UC_MINUS
+ */
+#define MEMORY_POLICY_DEFAULT         0
+#if defined(__i386__) || defined(__x86_64__)
+/* x86 only. Memory type UNCACHABLE */
+# define MEMORY_POLICY_X86_UC_MINUS    0
+#elif defined(__arm__) || defined (__aarch64__)
+/* Arm only. Outer Shareable, Device-nGnRE memory (Device Memory on Armv7) */
+# define MEMORY_POLICY_ARM_DEV_nGnRE      0
+/* Arm only. Outer Shareable, Outer/Inner Write-Back Cacheable memory */
+# define MEMORY_POLICY_ARM_MEM_WB         1
+/*
+ * On ARM, MEMORY_POLICY selects the stage-2 memory attributes, but note

s/ARM/Arm/

+ * that the resulting memory attributes will be a combination of stage-2
+ * and stage-1 memory attributes: it will be the strongest between the 2
+ * stages attributes.
+ */
+#endif
  struct xen_domctl_memory_mapping {
      uint64_aligned_t first_gfn; /* first page (hvm guest phys page) in range 
*/
      uint64_aligned_t first_mfn; /* first page (machine page) in range */
      uint64_aligned_t nr_mfns;   /* number of pages in range (>0) */
      uint32_t add_mapping;       /* add or remove mapping */
-    uint32_t padding;           /* padding for 64-bit aligned structure */
+    uint32_t memory_policy;      /* cacheability of the memory mapping */
  };

Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.