[PATCH V7 0/2] Add handling of extended regions (safe ranges) on Arm (Was "xen/memory: Introduce a hypercall to provide unallocated space")

[PATCH V7 0/2] Add handling of extended regions (safe ranges) on Arm (Was "xen/memory: Introduce a hypercall to provide unallocated space")

[Oleksandr Tyshchenko]

From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>

You can find an initial discussion at [1]-[7].

The extended region (safe range) is a region of guest physical address space
which is unused and could be safely used to create grant/foreign mappings instead
of wasting real RAM pages from the domain memory for establishing these mappings.

The extended regions are chosen at the domain creation time and advertised
to it via "reg" property under hypervisor node in the guest device-tree
(the indexes for extended regions are 1...N).
No device tree bindings update is needed, guest infers the presense of extended
regions from the number of regions in "reg" property.
New compatible/property will be needed (but only after this patch [8] or alternative
goes in) to indicate that "region 0 is safe to use". Until this patch is merged it is
not safe to use extended regions for the grant table space.

The extended regions are calculated differently for direct mapped Dom0 (with and without
IOMMU) and non-direct mapped DomUs.

Please note the following limitations:
- The extended region feature is only supported for 64-bit domain currently.
- The ACPI case is not covered.

Please note that support for Dom0 was already committed, so these patches are remaining DomU bits.

Xen patch series is also available at [9]. The corresponding Linux patch series is at [10]
for now (last 4 patches).

Tested on Renesas Salvator-X board + H3 ES3.0 SoC (Arm64) with updated virtio-disk backend [11]
running in Dom0 (256MB RAM) and DomD (2GB RAM). In both cases the backend pre-maps DomU memory
which is 3GB. All foreign memory gets mapped into extended regions (so the amount of RAM in
the backend domain is not reduced). No issues were observed.

[1] https://lore.kernel.org/xen-devel/1627489110-25633-1-git-send-email-olekstysh@gmail.com/
[2] https://lore.kernel.org/xen-devel/1631034578-12598-1-git-send-email-olekstysh@gmail.com/
[3] https://lore.kernel.org/xen-devel/1631297924-8658-1-git-send-email-olekstysh@gmail.com/
[4] https://lore.kernel.org/xen-devel/1632437334-12015-1-git-send-email-olekstysh@gmail.com/
[5] https://lore.kernel.org/xen-devel/1632955927-27911-1-git-send-email-olekstysh@gmail.com/
[6] https://lore.kernel.org/xen-devel/1633519346-3686-1-git-send-email-olekstysh@gmail.com/
[7] https://lore.kernel.org/xen-devel/1633974539-7380-1-git-send-email-olekstysh@gmail.com/
[8] https://lore.kernel.org/xen-devel/1632425551-18910-1-git-send-email-olekstysh@gmail.com/
[9] https://github.com/otyshchenko1/xen/commits/map_opt_ml8
[10] https://github.com/otyshchenko1/linux/commits/map_opt_ml4
[11] https://github.com/otyshchenko1/virtio-disk/commits/map_opt_next

Oleksandr Tyshchenko (2):
  xen/arm: Introduce gpaddr_bits field to struct
    xen_domctl_getdomaininfo
  libxl/arm: Add handling of extended regions for DomU

 tools/include/libxl.h            |   8 +++
 tools/include/xenctrl.h          |   1 +
 tools/libs/ctrl/xc_domain.c      |   1 +
 tools/libs/light/libxl_arm.c     | 106 +++++++++++++++++++++++++++++++++++++--
 tools/libs/light/libxl_domain.c  |   1 +
 tools/libs/light/libxl_types.idl |   1 +
 xen/arch/arm/domctl.c            |   2 +
 xen/arch/x86/domctl.c            |   1 +
 xen/common/domctl.c              |   4 +-
 xen/common/sysctl.c              |   2 +-
 xen/include/public/arch-arm.h    |   5 ++
 xen/include/public/domctl.h      |   3 ++
 xen/include/public/sysctl.h      |   2 +-
 13 files changed, 128 insertions(+), 9 deletions(-)

-- 
2.7.4

[PATCH V7 1/2] xen/arm: Introduce gpaddr_bits field to struct xen_domctl_getdomaininfo

[Oleksandr Tyshchenko]

From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>

We need to pass info about maximum supported guest physical
address space size to the toolstack on Arm in order to properly
calculate the base and size of the extended region (safe range)
for the guest. The extended region is unused address space which
could be safely used by domain for foreign/grant mappings on Arm.
The extended region itself will be handled by the subsequent
patch.

Currently the same guest physical address space size is used
for all guests (p2m_ipa_bits variable on Arm, the x86 equivalent
is hap_paddr_bits).

Add an explicit padding after "gpaddr_bits" field and also
(while at it) after "domain" field.

Also make sure that full structure is cleared in all cases by
moving the clearing into getdomaininfo(). Currently it is only
cleared by the sysctl caller (and only once).

Please note, we do not need to bump XEN_DOMCTL_INTERFACE_VERSION
as a bump has already occurred in this release cycle. But we do
need to bump XEN_SYSCTL_INTERFACE_VERSION as the structure is
re-used in a sysctl.

Suggested-by: Julien Grall <jgrall@amazon.com>
Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
Reviewed-by: Ian Jackson <iwj@xenproject.org>
[hypervisor parts]
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
Changes RFC -> V2:
   - update patch subject/description
   - replace arch-specific sub-struct with common gpaddr_bits
     field and update code to reflect that

Changes V2 -> V3:
   - make the field uint8_t and add uint8_t pad[7] after
   - remove leading blanks in libxl.h

Changes V3 -> V4:
   - also print gpaddr_bits from output_physinfo()
   - add Michal's R-b

Changes V4 -> V5:
   - update patch subject and description
   - drop Michal's R-b
   - pass gpaddr_bits via createdomain domctl
     (struct xen_arch_domainconfig)

Changes V5 -> V6:
   - update patch subject and description
   - pass gpaddr_bits via getdomaininfo domctl
     (struct xen_domctl_getdomaininfo)

Changes V6 -> V7:
   - update patch description
   - do not bump XEN_DOMCTL_INTERFACE_VERSION
   - bump XEN_SYSCTL_INTERFACE_VERSION
   - add explicit paddings
   - clear the full structure in getdomaininfo()

Changes V7 -> V7.1:
   - add Jan's R-b
   - drop non-useful change (info->flags |= ...) in getdomaininfo()
---
 tools/include/libxl.h            | 8 ++++++++
 tools/include/xenctrl.h          | 1 +
 tools/libs/ctrl/xc_domain.c      | 1 +
 tools/libs/light/libxl_domain.c  | 1 +
 tools/libs/light/libxl_types.idl | 1 +
 xen/arch/arm/domctl.c            | 2 ++
 xen/arch/x86/domctl.c            | 1 +
 xen/common/domctl.c              | 4 ++--
 xen/common/sysctl.c              | 2 +-
 xen/include/public/domctl.h      | 3 +++
 xen/include/public/sysctl.h      | 2 +-
 11 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/tools/include/libxl.h b/tools/include/libxl.h
index ee73eb0..2e8679d 100644
--- a/tools/include/libxl.h
+++ b/tools/include/libxl.h
@@ -886,6 +886,14 @@ typedef struct libxl__ctx libxl_ctx;
 #define LIBXL_HAVE_DOMINFO_NEVER_STOP 1
 
 /*
+ * LIBXL_HAVE_DOMINFO_GPADDR_BITS
+ *
+ * If this is defined, libxl_dominfo will contain an uint8 field called
+ * gpaddr_bits, containing the guest physical address space size.
+ */
+#define LIBXL_HAVE_DOMINFO_GPADDR_BITS 1
+
+/*
  * LIBXL_HAVE_QXL
  *
  * If defined, then the libxl_vga_interface_type will contain another value:
diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h
index a306399..07b96e6 100644
--- a/tools/include/xenctrl.h
+++ b/tools/include/xenctrl.h
@@ -462,6 +462,7 @@ typedef struct xc_dominfo {
     unsigned int  max_vcpu_id;
     xen_domain_handle_t handle;
     unsigned int  cpupool;
+    uint8_t       gpaddr_bits;
     struct xen_arch_domainconfig arch_config;
 } xc_dominfo_t;
 
diff --git a/tools/libs/ctrl/xc_domain.c b/tools/libs/ctrl/xc_domain.c
index 23322b7..b155d6a 100644
--- a/tools/libs/ctrl/xc_domain.c
+++ b/tools/libs/ctrl/xc_domain.c
@@ -396,6 +396,7 @@ int xc_domain_getinfo(xc_interface *xch,
         info->nr_online_vcpus = domctl.u.getdomaininfo.nr_online_vcpus;
         info->max_vcpu_id = domctl.u.getdomaininfo.max_vcpu_id;
         info->cpupool = domctl.u.getdomaininfo.cpupool;
+        info->gpaddr_bits = domctl.u.getdomaininfo.gpaddr_bits;
         info->arch_config = domctl.u.getdomaininfo.arch_config;
 
         memcpy(info->handle, domctl.u.getdomaininfo.handle,
diff --git a/tools/libs/light/libxl_domain.c b/tools/libs/light/libxl_domain.c
index 51a6127..544a9bf 100644
--- a/tools/libs/light/libxl_domain.c
+++ b/tools/libs/light/libxl_domain.c
@@ -306,6 +306,7 @@ void libxl__xcinfo2xlinfo(libxl_ctx *ctx,
     xlinfo->vcpu_max_id = xcinfo->max_vcpu_id;
     xlinfo->vcpu_online = xcinfo->nr_online_vcpus;
     xlinfo->cpupool = xcinfo->cpupool;
+    xlinfo->gpaddr_bits = xcinfo->gpaddr_bits;
     xlinfo->domain_type = (xcinfo->flags & XEN_DOMINF_hvm_guest) ?
         LIBXL_DOMAIN_TYPE_HVM : LIBXL_DOMAIN_TYPE_PV;
 }
diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl
index b96fb5c..608d55a 100644
--- a/tools/libs/light/libxl_types.idl
+++ b/tools/libs/light/libxl_types.idl
@@ -357,6 +357,7 @@ libxl_dominfo = Struct("dominfo",[
     ("vcpu_max_id", uint32),
     ("vcpu_online", uint32),
     ("cpupool",     uint32),
+    ("gpaddr_bits", uint8),
     ("domain_type", libxl_domain_type),
     ], dir=DIR_OUT)
 
diff --git a/xen/arch/arm/domctl.c b/xen/arch/arm/domctl.c
index b7d27f3..6245af6 100644
--- a/xen/arch/arm/domctl.c
+++ b/xen/arch/arm/domctl.c
@@ -20,6 +20,8 @@ void arch_get_domain_info(const struct domain *d,
 {
     /* All ARM domains use hardware assisted paging. */
     info->flags |= XEN_DOMINF_hap;
+
+    info->gpaddr_bits = p2m_ipa_bits;
 }
 
 static int handle_vuart_init(struct domain *d, 
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 26a76d2..7d102e0 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -151,6 +151,7 @@ void arch_get_domain_info(const struct domain *d,
         info->flags |= XEN_DOMINF_hap;
 
     info->arch_config.emulation_flags = d->arch.emulation_flags;
+    info->gpaddr_bits = hap_paddr_bits;
 }
 
 static int do_vmtrace_op(struct domain *d, struct xen_domctl_vmtrace_op *op,
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 12d6144..271862a 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -69,10 +69,10 @@ void getdomaininfo(struct domain *d, struct xen_domctl_getdomaininfo *info)
     int flags = XEN_DOMINF_blocked;
     struct vcpu_runstate_info runstate;
 
+    memset(info, 0, sizeof(*info));
+
     info->domain = d->domain_id;
     info->max_vcpu_id = XEN_INVALID_MAX_VCPU_ID;
-    info->nr_online_vcpus = 0;
-    info->ssidref = 0;
 
     /*
      * - domain is marked as blocked only if all its vcpus are blocked
diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
index 6e7189b..f2dab72 100644
--- a/xen/common/sysctl.c
+++ b/xen/common/sysctl.c
@@ -76,7 +76,7 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
     case XEN_SYSCTL_getdomaininfolist:
     { 
         struct domain *d;
-        struct xen_domctl_getdomaininfo info = { 0 };
+        struct xen_domctl_getdomaininfo info;
         u32 num_domains = 0;
 
         rcu_read_lock(&domlist_read_lock);
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index a53cbd1..9099dc1 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -108,6 +108,7 @@ struct xen_domctl_createdomain {
 struct xen_domctl_getdomaininfo {
     /* OUT variables. */
     domid_t  domain;              /* Also echoed in domctl.domain */
+    uint16_t pad1;
  /* Domain is scheduled to die. */
 #define _XEN_DOMINF_dying     0
 #define XEN_DOMINF_dying      (1U<<_XEN_DOMINF_dying)
@@ -152,6 +153,8 @@ struct xen_domctl_getdomaininfo {
     uint32_t ssidref;
     xen_domain_handle_t handle;
     uint32_t cpupool;
+    uint8_t gpaddr_bits; /* Guest physical address space size. */
+    uint8_t pad2[7];
     struct xen_arch_domainconfig arch_config;
 };
 typedef struct xen_domctl_getdomaininfo xen_domctl_getdomaininfo_t;
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index fead0e5..3e53681 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -35,7 +35,7 @@
 #include "domctl.h"
 #include "physdev.h"
 
-#define XEN_SYSCTL_INTERFACE_VERSION 0x00000013
+#define XEN_SYSCTL_INTERFACE_VERSION 0x00000014
 
 /*
  * Read console content from Xen buffer ring.
-- 
2.7.4

[PATCH V7 2/2] libxl/arm: Add handling of extended regions for DomU

[Oleksandr Tyshchenko]

From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>

The extended region (safe range) is a region of guest physical
address space which is unused and could be safely used to create
grant/foreign mappings instead of wasting real RAM pages from
the domain memory for establishing these mappings.

The extended regions are chosen at the domain creation time and
advertised to it via "reg" property under hypervisor node in
the guest device-tree. As region 0 is reserved for grant table
space (always present), the indexes for extended regions are 1...N.
If extended regions could not be allocated for some reason,
Xen doesn't fail and behaves as usual, so only inserts region 0.

Please note the following limitations:
- The extended region feature is only supported for 64-bit domain
  currently.
- The ACPI case is not covered.

***

The algorithm to choose extended regions for non-direct mapped
DomU is simpler in comparison with the algorithm for direct mapped
Dom0. We usually have a lot of unused space above 4GB, and might
have some unused space below 4GB (depends on guest memory size).
Try to allocate separate 2MB-aligned extended regions from the first
(below 4GB) and second (above 4GB) RAM banks taking into the account
the maximum supported guest physical address space size and the amount
of memory assigned to the guest. The minimum size of extended region
the same as for Dom0 (64MB).

Please note, we introduce fdt_property_reg_placeholder helper which
purpose is to create N ranges that are zeroed. The interesting fact
is that libfdt already has fdt_property_placeholder(). But this was
introduced only in 2017, so there is a risk that some distros may not
ship the last libfdt version. This is why we implement our own light
variant for now.

Suggested-by: Julien Grall <jgrall@amazon.com>
Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
---
! Stefano, Ian I dropped your A-b/R-b again as the patch has changed
significantly !

Changes RFC -> V2:
   - update patch description
   - drop uneeded "extended-region" DT property
   - clear reg array in finalise_ext_region() and add a TODO

Changes V2 -> V3:
   - update patch description, comments in code
   - only pick up regions with size >= 64MB
   - move the region calculation to make_hypervisor_node() and drop
     finalise_ext_region()
   - extend the list of arguments for make_hypervisor_node()
   - do not show warning for 32-bit domain
   - change the region alignment from 1GB to 2MB
   - move EXT_REGION_SIZE to public/arch-arm.h

Changes V3 -> V4:
   - add R-b, A-b and T-b

Changes V4 -> V5:
   - update patch description and comments in code
   - reflect changes done in previous patch to pass gpaddr_bits
     via createdomain domctl (struct xen_arch_domainconfig)
   - drop R-b, A-b and T-b
   - drop limit for maximum extended region size (128GB)
   - try to also allocate region below 4GB, optimize code
     for calculating extended regions

Change V5 -> V6:
   - reflect changes done in previous patch to pass gpaddr_bits
     via getdomaininfo domctl (struct xen_domctl_getdomaininfo)
   - reduce the number of local variables, rework calculations

Change V6 -> V7:
   - return finalize_*() back and put all logic there with re-using
     fdt_setprop() to update placeholders
   - introduce fdt_property_reg_placeholder() helper
   - rework regions calculation to not rely on the fact that Bank 0
     is always below 4GB
   - drop check for 32-bit domain and assert for invalid gpaddr_bits
   - change a formula to calculate bankend value
   - move EXT_REGION_MIN_SIZE definition from the public header to
     libxl_arm.c
   - do not use asserts for the return values, propagate errors to
     the callers
   - add a comment in public header
---
 tools/libs/light/libxl_arm.c  | 106 ++++++++++++++++++++++++++++++++++++++++--
 xen/include/public/arch-arm.h |   5 ++
 2 files changed, 106 insertions(+), 5 deletions(-)

diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
index e3140a6..a780155 100644
--- a/tools/libs/light/libxl_arm.c
+++ b/tools/libs/light/libxl_arm.c
@@ -269,6 +269,21 @@ static int fdt_property_regs(libxl__gc *gc, void *fdt,
     return fdt_property(fdt, "reg", regs, sizeof(regs));
 }
 
+static int fdt_property_reg_placeholder(libxl__gc *gc, void *fdt,
+                                        unsigned int addr_cells,
+                                        unsigned int size_cells,
+                                        unsigned int num_regs)
+{
+    uint32_t regs[num_regs * (addr_cells + size_cells)];
+    be32 *cells = &regs[0];
+    unsigned int i;
+
+    for (i = 0; i < num_regs; i++)
+        set_range(&cells, addr_cells, size_cells, 0, 0);
+
+    return fdt_property(fdt, "reg", regs, sizeof(regs));
+}
+
 static int make_root_properties(libxl__gc *gc,
                                 const libxl_version_info *vers,
                                 void *fdt)
@@ -615,9 +630,13 @@ static int make_hypervisor_node(libxl__gc *gc, void *fdt,
                               "xen,xen");
     if (res) return res;
 
-    /* reg 0 is grant table space */
-    res = fdt_property_regs(gc, fdt, GUEST_ROOT_ADDRESS_CELLS, GUEST_ROOT_SIZE_CELLS,
-                            1,GUEST_GNTTAB_BASE, GUEST_GNTTAB_SIZE);
+    /*
+     * reg 0 is a placeholder for grant table space, reg 1...N are
+     * the placeholders for extended regions.
+     */
+    res = fdt_property_reg_placeholder(gc, fdt, GUEST_ROOT_ADDRESS_CELLS,
+                                       GUEST_ROOT_SIZE_CELLS,
+                                       GUEST_RAM_BANKS + 1);
     if (res) return res;
 
     /*
@@ -1069,20 +1088,93 @@ static void finalise_one_node(libxl__gc *gc, void *fdt, const char *uname,
     }
 }
 
+#define ALIGN_UP_TO_2MB(x)   (((x) + MB(2) - 1) & (~(MB(2) - 1)))
+
+#define EXT_REGION_MIN_SIZE   xen_mk_ullong(0x0004000000) /* 64MB */
+
+static int finalize_hypervisor_node(libxl__gc *gc, struct xc_dom_image *dom)
+{
+    void *fdt = dom->devicetree_blob;
+    uint64_t region_size[GUEST_RAM_BANKS] = {0}, region_base[GUEST_RAM_BANKS],
+        bankend[GUEST_RAM_BANKS];
+    uint32_t regs[(GUEST_ROOT_ADDRESS_CELLS + GUEST_ROOT_SIZE_CELLS) *
+                  (GUEST_RAM_BANKS + 1)];
+    be32 *cells = &regs[0];
+    const uint64_t bankbase[] = GUEST_RAM_BANK_BASES;
+    const uint64_t banksize[] = GUEST_RAM_BANK_SIZES;
+    unsigned int i, len, nr_regions = 0;
+    libxl_dominfo info;
+    int offset, rc;
+
+    offset = fdt_path_offset(fdt, "/hypervisor");
+    if (offset < 0)
+        return offset;
+
+    rc = libxl_domain_info(CTX, &info, dom->guest_domid);
+    if (rc)
+        return rc;
+
+    if (info.gpaddr_bits > 64)
+        return ERROR_INVAL;
+
+    /*
+     * Try to allocate separate 2MB-aligned extended regions from the first
+     * and second RAM banks taking into the account the maximum supported
+     * guest physical address space size and the amount of memory assigned
+     * to the guest.
+     */
+    for (i = 0; i < GUEST_RAM_BANKS; i++) {
+        region_base[i] = bankbase[i] +
+            ALIGN_UP_TO_2MB((uint64_t)dom->rambank_size[i] << XC_PAGE_SHIFT);
+
+        bankend[i] = ~0ULL >> (64 - info.gpaddr_bits);
+        bankend[i] = min(bankend[i], bankbase[i] + banksize[i] - 1);
+        if (bankend[i] > region_base[i])
+            region_size[i] = bankend[i] - region_base[i] + 1;
+    }
+
+    /*
+     * The region 0 for grant table space must be always present. If we managed
+     * to allocate the extended regions then insert them as regions 1...N.
+     */
+    set_range(&cells, GUEST_ROOT_ADDRESS_CELLS, GUEST_ROOT_SIZE_CELLS,
+              GUEST_GNTTAB_BASE, GUEST_GNTTAB_SIZE);
+
+    for (i = 0; i < GUEST_RAM_BANKS; i++) {
+        if (region_size[i] < EXT_REGION_MIN_SIZE)
+            continue;
+
+        LOG(DEBUG, "Extended region %u: %#"PRIx64"->%#"PRIx64"",
+            nr_regions, region_base[i], region_base[i] + region_size[i]);
+
+        set_range(&cells, GUEST_ROOT_ADDRESS_CELLS, GUEST_ROOT_SIZE_CELLS,
+                  region_base[i], region_size[i]);
+        nr_regions++;
+    }
+
+    if (!nr_regions)
+        LOG(WARN, "The extended regions cannot be allocated, not enough space");
+
+    len = sizeof(regs[0]) * (GUEST_ROOT_ADDRESS_CELLS + GUEST_ROOT_SIZE_CELLS) *
+        (nr_regions + 1);
+
+    return fdt_setprop(fdt, offset, "reg", regs, len);
+}
+
 int libxl__arch_domain_finalise_hw_description(libxl__gc *gc,
                                                uint32_t domid,
                                                libxl_domain_config *d_config,
                                                struct xc_dom_image *dom)
 {
     void *fdt = dom->devicetree_blob;
-    int i;
+    int i, res;
     const uint64_t bankbase[] = GUEST_RAM_BANK_BASES;
 
     const struct xc_dom_seg *ramdisk = dom->modules[0].blob ?
         &dom->modules[0].seg : NULL;
 
     if (ramdisk) {
-        int chosen, res;
+        int chosen;
         uint64_t val;
 
         /* Neither the fdt_path_offset() nor either of the
@@ -1109,6 +1201,10 @@ int libxl__arch_domain_finalise_hw_description(libxl__gc *gc,
 
     }
 
+    res = finalize_hypervisor_node(gc, dom);
+    if (res)
+        return res;
+
     for (i = 0; i < GUEST_RAM_BANKS; i++) {
         const uint64_t size = (uint64_t)dom->rambank_size[i] << XC_PAGE_SHIFT;
 
diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
index d46c61f..96ead3d 100644
--- a/xen/include/public/arch-arm.h
+++ b/xen/include/public/arch-arm.h
@@ -438,6 +438,11 @@ typedef uint64_t xen_callback_t;
 
 #define GUEST_RAM_BANKS   2
 
+/*
+ * The way to find the extended regions (to be exposed to the guest as unused
+ * address space) relies on the fact that the regions reserved for the RAM
+ * below are big enough to also accommodate such regions.
+ */
 #define GUEST_RAM0_BASE   xen_mk_ullong(0x40000000) /* 3GB of low RAM @ 1GB */
 #define GUEST_RAM0_SIZE   xen_mk_ullong(0xc0000000)
 
-- 
2.7.4

Re: [PATCH V7 2/2] libxl/arm: Add handling of extended regions for DomU

[Julien Grall]

Hi Oleksandr,

On 14/10/2021 12:40, Oleksandr Tyshchenko wrote:
> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> 
> The extended region (safe range) is a region of guest physical
> address space which is unused and could be safely used to create
> grant/foreign mappings instead of wasting real RAM pages from
> the domain memory for establishing these mappings.
> 
> The extended regions are chosen at the domain creation time and
> advertised to it via "reg" property under hypervisor node in
> the guest device-tree. As region 0 is reserved for grant table
> space (always present), the indexes for extended regions are 1...N.
> If extended regions could not be allocated for some reason,
> Xen doesn't fail and behaves as usual, so only inserts region 0.
> 
> Please note the following limitations:
> - The extended region feature is only supported for 64-bit domain
>    currently.
> - The ACPI case is not covered.
> 
> ***
> 
> The algorithm to choose extended regions for non-direct mapped
> DomU is simpler in comparison with the algorithm for direct mapped
> Dom0. We usually have a lot of unused space above 4GB, and might
> have some unused space below 4GB (depends on guest memory size).
> Try to allocate separate 2MB-aligned extended regions from the first
> (below 4GB) and second (above 4GB) RAM banks taking into the account
> the maximum supported guest physical address space size and the amount
> of memory assigned to the guest. The minimum size of extended region
> the same as for Dom0 (64MB).
> 
> Please note, we introduce fdt_property_reg_placeholder helper which
> purpose is to create N ranges that are zeroed. The interesting fact
> is that libfdt already has fdt_property_placeholder(). But this was
> introduced only in 2017, so there is a risk that some distros may not
> ship the last libfdt version. This is why we implement our own light
> variant for now.
> 
> Suggested-by: Julien Grall <jgrall@amazon.com>
> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>

Reviewed-by: Julien Grall <jgrall@amazon.com>

Cheers,

-- 
Julien Grall

Re: [PATCH V7 2/2] libxl/arm: Add handling of extended regions for DomU

[Ian Jackson]

Julien Grall writes ("Re: [PATCH V7 2/2] libxl/arm: Add handling of extended regions for DomU"):
> On 14/10/2021 12:40, Oleksandr Tyshchenko wrote:
> > From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
...
> > Suggested-by: Julien Grall <jgrall@amazon.com>
> > Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> 
> Reviewed-by: Julien Grall <jgrall@amazon.com>

Again, on that basis,

Acked-by: Ian Jackson <iwj@xenproject.org>

Thanks all.

Ian.

Re: [PATCH V7 0/2] Add handling of extended regions (safe ranges) on Arm (Was "xen/memory: Introduce a hypercall to provide unallocated space")

[Ian Jackson]

Oleksandr Tyshchenko writes ("[PATCH V7 0/2] Add handling of extended regions (safe ranges) on Arm (Was "xen/memory: Introduce a hypercall to provide unallocated space")"):
> You can find an initial discussion at [1]-[7].
> 
> The extended region (safe range) is a region of guest physical address space
> which is unused and could be safely used to create grant/foreign mappings instead
> of wasting real RAM pages from the domain memory for establishing these mappings.

Thanks.

This patch has all the required acks, but I was aware of an
outstanding concern from Andrew, as set out in his most
recent mail on the subject:
  Subject: Re: [RFC PATCH 1/3] xen: Introduce "gpaddr_bits" field to XEN_SYSCTL_physinfo
  Date: Tue, 7 Sep 2021 18:35:47 +0100
  Message-ID: <973f5344-aa10-3ad6-ff02-ad5f358ad279@citrix.com>

I think it would be within the process to just commit the patch now,
but I thought it best to check as best I could that we weren't missing
anything.  The process is supposed to support our continuing
development and also our quality, so I aim to do those things.

I reviewed that mail and had a conversation with Julien about it on
irc.  My understanding is that Julien and Oleksandr's intent is that
Andrew's concerns have been addressed, although we don't have a
confirmation of that from Andrew.

In particular, I wanted to convince myself that if in fact there was
still a problem, we hadn't made a problem for ourselves with the API
here.

The new hypercalls are in unstable interfaces, so if we need to change
them in a future version (eg to make ARM migration work) that's OK.
Julien tells me that he doesn't believe there to be any impact on the
(x86) migration stream right now.

There is a new libxl stable interface.  But I think it is
inoffensive.  In particular, basically any mechanism to do this would
have that API.  And that doesn't seem to touch on the implementation
issues described by Andrew.

Therefore, I think (i) we have tried to address the issues (ii) any
reminaing problems can be dealt with as followups, without trouble.

So I have just pushed these two.

Thanks,
Ian.

Re: [PATCH V7 0/2] Add handling of extended regions (safe ranges) on Arm (Was "xen/memory: Introduce a hypercall to provide unallocated space")

[Oleksandr]

On 14.10.21 17:28, Ian Jackson wrote:

Hi Ian

> Oleksandr Tyshchenko writes ("[PATCH V7 0/2] Add handling of extended regions (safe ranges) on Arm (Was "xen/memory: Introduce a hypercall to provide unallocated space")"):
>> You can find an initial discussion at [1]-[7].
>>
>> The extended region (safe range) is a region of guest physical address space
>> which is unused and could be safely used to create grant/foreign mappings instead
>> of wasting real RAM pages from the domain memory for establishing these mappings.
> Thanks.
>
> This patch has all the required acks, but I was aware of an
> outstanding concern from Andrew, as set out in his most
> recent mail on the subject:
>    Subject: Re: [RFC PATCH 1/3] xen: Introduce "gpaddr_bits" field to XEN_SYSCTL_physinfo
>    Date: Tue, 7 Sep 2021 18:35:47 +0100
>    Message-ID: <973f5344-aa10-3ad6-ff02-ad5f358ad279@citrix.com>
>
> I think it would be within the process to just commit the patch now,
> but I thought it best to check as best I could that we weren't missing
> anything.  The process is supposed to support our continuing
> development and also our quality, so I aim to do those things.
>
> I reviewed that mail and had a conversation with Julien about it on
> irc.  My understanding is that Julien and Oleksandr's intent is that
> Andrew's concerns have been addressed, although we don't have a
> confirmation of that from Andrew.
>
> In particular, I wanted to convince myself that if in fact there was
> still a problem, we hadn't made a problem for ourselves with the API
> here.
>
> The new hypercalls are in unstable interfaces, so if we need to change
> them in a future version (eg to make ARM migration work) that's OK.
> Julien tells me that he doesn't believe there to be any impact on the
> (x86) migration stream right now.
>
> There is a new libxl stable interface.  But I think it is
> inoffensive.  In particular, basically any mechanism to do this would
> have that API.  And that doesn't seem to touch on the implementation
> issues described by Andrew.
>
> Therefore, I think (i) we have tried to address the issues (ii) any
> reminaing problems can be dealt with as followups, without trouble.

Completely agree with both statements.


>
> So I have just pushed these two.

Thanks!


> Thanks,
> Ian.

-- 
Regards,

Oleksandr Tyshchenko

Re: [PATCH V7 0/2] Add handling of extended regions (safe ranges) on Arm (Was "xen/memory: Introduce a hypercall to provide unallocated space")

[Oleksandr]

Hello all


On 14.10.21 14:40, Oleksandr Tyshchenko wrote:
> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>
> You can find an initial discussion at [1]-[7].
>
> The extended region (safe range) is a region of guest physical address space
> which is unused and could be safely used to create grant/foreign mappings instead
> of wasting real RAM pages from the domain memory for establishing these mappings.
>
> The extended regions are chosen at the domain creation time and advertised
> to it via "reg" property under hypervisor node in the guest device-tree
> (the indexes for extended regions are 1...N).
> No device tree bindings update is needed, guest infers the presense of extended
> regions from the number of regions in "reg" property.
> New compatible/property will be needed (but only after this patch [8] or alternative
> goes in) to indicate that "region 0 is safe to use". Until this patch is merged it is
> not safe to use extended regions for the grant table space.
>
> The extended regions are calculated differently for direct mapped Dom0 (with and without
> IOMMU) and non-direct mapped DomUs.
>
> Please note the following limitations:
> - The extended region feature is only supported for 64-bit domain currently.
> - The ACPI case is not covered.
>
> Please note that support for Dom0 was already committed, so these patches are remaining DomU bits.
>
> Xen patch series is also available at [9]. The corresponding Linux patch series is at [10]
> for now (last 4 patches).

So, all Xen changes are already committed (thanks!) and I will start 
trying to push Linux changes. I believe that with your help in review, 
it will be possible to finish this enabling work.


[snip]

-- 
Regards,

Oleksandr Tyshchenko