diff mbox series

[V4,3/3] libxl/arm: Add handling of extended regions for DomU

Message ID 1632955927-27911-4-git-send-email-olekstysh@gmail.com (mailing list archive)
State Superseded
Headers show
Series Add handling of extended regions (safe ranges) on Arm (Was "xen/memory: Introduce a hypercall to provide unallocated space") | expand

Commit Message

Oleksandr Tyshchenko Sept. 29, 2021, 10:52 p.m. UTC 
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. As we have a lot of unused space above 4GB, provide single
2MB-aligned region from the second RAM bank taking into the account
the maximum supported guest address space size and the amount of
memory assigned to the guest. The maximum size of the region is 128GB.
The minimum size is 64MB.

Suggested-by: Julien Grall <jgrall@amazon.com>
Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Acked-by: Ian Jackson <iwj@xenproject.org>
Reviewed-by: Michal Orzel <michal.orzel@arm.com>
Tested-by: Michal Orzel <michal.orzel@arm.com>
---
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
---
 tools/libs/light/libxl_arm.c  | 70 +++++++++++++++++++++++++++++++++++++++----
 xen/include/public/arch-arm.h |  3 ++
 2 files changed, 68 insertions(+), 5 deletions(-)

Comments

Oleksandr Tyshchenko Oct. 5, 2021, 7:42 p.m. UTC | #1 
On 30.09.21 01:52, Oleksandr Tyshchenko wrote:

Hi Stefano, Julien.

> 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. As we have a lot of unused space above 4GB, provide single
> 2MB-aligned region from the second RAM bank taking into the account
> the maximum supported guest address space size and the amount of
> memory assigned to the guest. The maximum size of the region is 128GB.
> The minimum size is 64MB.
>
> Suggested-by: Julien Grall <jgrall@amazon.com>
> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
> Acked-by: Ian Jackson <iwj@xenproject.org>
> Reviewed-by: Michal Orzel <michal.orzel@arm.com>
> Tested-by: Michal Orzel <michal.orzel@arm.com>

I though a bit more on this and decided to make a patch more functional 
by trying to also allocate extended region below 4GB, I think we could 
do with it.
Actually if guest memory size is less than GUEST_RAM0_SIZE, we are able 
to provide unused space. I have tested with with various guest memory 
sizes and it worked fine. Also I decided to drop limit for maximum 
extended region size (128GB), we don't apply this limit in Dom0 and I 
don't see why we need it here, moreover the calculation became more 
obvious. I will drop all acks and send updated version. Are there any 
objections?
Stefano Stabellini Oct. 5, 2021, 9:36 p.m. UTC | #2 
On Tue, 5 Oct 2021, Oleksandr 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. As we have a lot of unused space above 4GB, provide single
> > 2MB-aligned region from the second RAM bank taking into the account
> > the maximum supported guest address space size and the amount of
> > memory assigned to the guest. The maximum size of the region is 128GB.
> > The minimum size is 64MB.
> > 
> > Suggested-by: Julien Grall <jgrall@amazon.com>
> > Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> > Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
> > Acked-by: Ian Jackson <iwj@xenproject.org>
> > Reviewed-by: Michal Orzel <michal.orzel@arm.com>
> > Tested-by: Michal Orzel <michal.orzel@arm.com>
> 
> I though a bit more on this and decided to make a patch more functional by
> trying to also allocate extended region below 4GB, I think we could do with
> it.
> Actually if guest memory size is less than GUEST_RAM0_SIZE, we are able to
> provide unused space. I have tested with with various guest memory sizes and
> it worked fine. Also I decided to drop limit for maximum extended region size
> (128GB), we don't apply this limit in Dom0 and I don't see why we need it
> here, moreover the calculation became more obvious. I will drop all acks and
> send updated version. Are there any objections?

I am OK with it; it looks like you made good improvements. One caveat is
that I volunteer to review again no problem, but we'll need a new ack
from Ian Jackson to commit.
Oleksandr Tyshchenko Oct. 6, 2021, 10:11 a.m. UTC | #3 
On 06.10.21 00:36, Stefano Stabellini wrote:

Hi Stefano

> On Tue, 5 Oct 2021, Oleksandr 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. As we have a lot of unused space above 4GB, provide single
>>> 2MB-aligned region from the second RAM bank taking into the account
>>> the maximum supported guest address space size and the amount of
>>> memory assigned to the guest. The maximum size of the region is 128GB.
>>> The minimum size is 64MB.
>>>
>>> Suggested-by: Julien Grall <jgrall@amazon.com>
>>> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>>> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
>>> Acked-by: Ian Jackson <iwj@xenproject.org>
>>> Reviewed-by: Michal Orzel <michal.orzel@arm.com>
>>> Tested-by: Michal Orzel <michal.orzel@arm.com>
>> I though a bit more on this and decided to make a patch more functional by
>> trying to also allocate extended region below 4GB, I think we could do with
>> it.
>> Actually if guest memory size is less than GUEST_RAM0_SIZE, we are able to
>> provide unused space. I have tested with with various guest memory sizes and
>> it worked fine. Also I decided to drop limit for maximum extended region size
>> (128GB), we don't apply this limit in Dom0 and I don't see why we need it
>> here, moreover the calculation became more obvious. I will drop all acks and
>> send updated version. Are there any objections?
> I am OK with it; it looks like you made good improvements. One caveat is
> that I volunteer to review again no problem,

Great, thank you.


> but we'll need a new ack
> from Ian Jackson to commit.

Yes, I know that.
diff mbox series

Patch

diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
index e3140a6..a67b68e 100644
--- a/tools/libs/light/libxl_arm.c
+++ b/tools/libs/light/libxl_arm.c
@@ -598,9 +598,17 @@  static int make_timer_node(libxl__gc *gc, void *fdt,
     return 0;
 }
 
+#define ALIGN_UP_TO_2MB(x)   (((x) + MB(2) - 1) & (~(MB(2) - 1)))
+
 static int make_hypervisor_node(libxl__gc *gc, void *fdt,
-                                const libxl_version_info *vers)
+                                const libxl_version_info *vers,
+                                const libxl_domain_build_info *b_info,
+                                const struct xc_dom_image *dom)
 {
+    uint64_t region_size = 0, region_base, ramsize, bank1size,
+        bank1end_align, bank1end_max;
+    uint8_t gpaddr_bits;
+    libxl_physinfo physinfo;
     int res;
     gic_interrupt intr;
 
@@ -615,9 +623,61 @@  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);
+    if (strcmp(dom->guest_type, "xen-3.0-aarch64")) {
+        LOG(DEBUG, "The extended regions are only supported for 64-bit guest currently");
+        goto out;
+    }
+
+    res = libxl_get_physinfo(CTX, &physinfo);
+    assert(!res);
+
+    gpaddr_bits = physinfo.gpaddr_bits;
+    assert(gpaddr_bits >= 32 && gpaddr_bits <= 48);
+
+    /*
+     * Try to allocate single 2MB-aligned extended region from the second RAM
+     * bank (above 4GB) taking into the account the maximum supported guest
+     * address space size and the amount of memory assigned to the guest.
+     * As the guest memory layout is not populated yet we cannot rely on
+     * dom->rambank_size[1], so calculate the actual size of the second bank
+     * using "max_memkb" value.
+     */
+    bank1end_max = min(1ULL << gpaddr_bits, GUEST_RAM1_BASE + GUEST_RAM1_SIZE);
+    ramsize = b_info->max_memkb * 1024;
+    if (ramsize <= GUEST_RAM0_SIZE)
+        bank1size = 0;
+    else
+        bank1size = ramsize - GUEST_RAM0_SIZE;
+    bank1end_align = GUEST_RAM1_BASE + ALIGN_UP_TO_2MB(bank1size);
+
+    if (bank1end_max <= bank1end_align) {
+        LOG(WARN, "The extended region cannot be allocated, not enough space");
+        goto out;
+    }
+
+    if (bank1end_max - bank1end_align > GUEST_EXT_REGION_MAX_SIZE) {
+        region_base = bank1end_max - GUEST_EXT_REGION_MAX_SIZE;
+        region_size = GUEST_EXT_REGION_MAX_SIZE;
+    } else {
+        region_base = bank1end_align;
+        region_size = bank1end_max - bank1end_align;
+    }
+
+out:
+    /*
+     * The region 0 for grant table space must be always present. If we managed
+     * to allocate the extended region then insert it as region 1.
+     */
+    if (region_size >= GUEST_EXT_REGION_MIN_SIZE) {
+        LOG(DEBUG, "Extended region: %#"PRIx64"->%#"PRIx64"\n",
+            region_base, region_base + region_size);
+
+        res = fdt_property_regs(gc, fdt, GUEST_ROOT_ADDRESS_CELLS, GUEST_ROOT_SIZE_CELLS,
+                                2, GUEST_GNTTAB_BASE, GUEST_GNTTAB_SIZE,
+                                region_base, region_size);
+    } else
+        res = fdt_property_regs(gc, fdt, GUEST_ROOT_ADDRESS_CELLS, GUEST_ROOT_SIZE_CELLS,
+                                1, GUEST_GNTTAB_BASE, GUEST_GNTTAB_SIZE);
     if (res) return res;
 
     /*
@@ -963,7 +1023,7 @@  next_resize:
         }
 
         FDT( make_timer_node(gc, fdt, ainfo, state->clock_frequency) );
-        FDT( make_hypervisor_node(gc, fdt, vers) );
+        FDT( make_hypervisor_node(gc, fdt, vers, info, dom) );
 
         if (info->arch_arm.vuart == LIBXL_VUART_TYPE_SBSA_UART)
             FDT( make_vpl011_uart_node(gc, fdt, ainfo, dom) );
diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
index 6b5a5f8..df59933 100644
--- a/xen/include/public/arch-arm.h
+++ b/xen/include/public/arch-arm.h
@@ -449,6 +449,9 @@  typedef uint64_t xen_callback_t;
 #define GUEST_RAM_BANK_BASES   { GUEST_RAM0_BASE, GUEST_RAM1_BASE }
 #define GUEST_RAM_BANK_SIZES   { GUEST_RAM0_SIZE, GUEST_RAM1_SIZE }
 
+#define GUEST_EXT_REGION_MAX_SIZE   xen_mk_ullong(0x2000000000) /* 128GB */
+#define GUEST_EXT_REGION_MIN_SIZE   xen_mk_ullong(0x0004000000) /* 64MB */
+
 /* Current supported guest VCPUs */
 #define GUEST_MAX_VCPUS 128