On 7/5/25 10:27, Koichiro Den wrote:
> Implement ARM DEN 0057A PV time support for domains created via the
> toolstack, utilizing the newly introduced XENMAPSPACE_pv_time.
>
> Signed-off-by: Koichiro Den <d...@valinux.co.jp>
> ---
> tools/libs/light/libxl_arm.c | 185 ++++++++++++++++++++++++++++-------
> xen/arch/arm/mm.c | 14 +++
> xen/include/public/memory.h | 1 +
> 3 files changed, 167 insertions(+), 33 deletions(-)
>
> diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
> index 4a19a8d22bdf..33251520c07a 100644
> --- a/tools/libs/light/libxl_arm.c
> +++ b/tools/libs/light/libxl_arm.c
> @@ -684,6 +684,40 @@ static int make_memory_nodes(libxl__gc *gc, void *fdt,
> return 0;
> }
>
> +static int make_resv_memory_node(libxl__gc *gc, void *fdt,
> + const struct xc_dom_image *dom)
> +{
> + int res;
> +
> + if (strcmp(dom->guest_type, "xen-3.0-aarch64"))
> + /*
> + * The stolen time shared memory region for ARM DEN 0057A is
> currently
> + * the only user of /reserved-memory node when a domain is created
> via
> + * the toolstack, and it requires both the hypervisor and the domain
> to
> + * be AArch64.
> + */
> + return 0;
> +
> + res = fdt_begin_node(fdt, "reserved-memory");
> + if (res) return res;
> +
> + res = fdt_property_cell(fdt, "#address-cells", GUEST_ROOT_ADDRESS_CELLS);
> + if (res) return res;
> +
> + res = fdt_property_cell(fdt, "#size-cells", GUEST_ROOT_SIZE_CELLS);
> + if (res) return res;
> +
> + /* reg 0 is a placeholder for PV time region */
> + res = fdt_property_reg_placeholder(gc, fdt, GUEST_ROOT_ADDRESS_CELLS,
> + GUEST_ROOT_SIZE_CELLS, 1);
> + if (res) return res;
> +
> + res = fdt_end_node(fdt);
> + if (res) return res;
> +
> + return 0;
> +}
> +
> static int make_gicv2_node(libxl__gc *gc, void *fdt,
> uint64_t gicd_base, uint64_t gicd_size,
> uint64_t gicc_base, uint64_t gicc_size)
> @@ -1352,6 +1386,7 @@ next_resize:
> FDT( make_psci_node(gc, fdt) );
>
> FDT( make_memory_nodes(gc, fdt, dom) );
> + FDT( make_resv_memory_node(gc, fdt, dom) );
>
> switch (info->arch_arm.gic_version) {
> case LIBXL_GIC_VERSION_V2:
> @@ -1519,6 +1554,9 @@ static void finalise_one_node(libxl__gc *gc, void *fdt,
> const char *uname,
>
> #define EXT_REGION_MIN_SIZE xen_mk_ullong(0x0004000000) /* 64MB */
>
> +/* As per ARM DEN 0057A, stolen time memory regions are 64-byte aligned */
> +#define PV_REGIONS_PER_PAGE (XC_PAGE_SIZE / 64)
> +
> static int compare_iomem(const void *a, const void *b)
> {
> const libxl_iomem_range *x = a, *y = b;
> @@ -1530,24 +1568,92 @@ static int compare_iomem(const void *a, const void *b)
> return 0;
> }
>
> -static int finalize_hypervisor_node(libxl__gc *gc,
> - libxl_domain_build_info *b_info,
> - struct xc_dom_image *dom)
> +static int get_pv_region(libxl_domain_build_info *b_info,
> + struct xc_dom_image *dom,
> + uint64_t *start, uint64_t end,
> + uint64_t *region_base, uint64_t *region_size)
> +{
> + unsigned int npages = DIV_ROUNDUP(b_info->max_vcpus,
> PV_REGIONS_PER_PAGE);
> + unsigned int len = npages * XC_PAGE_SIZE;
> + uint32_t domid = dom->guest_domid;
> + xc_interface *xch = dom->xch;
> + unsigned long idx = 0;
> + uint64_t size;
> + int rc;
> +
> + if (*start >= end)
> + return -1;
> + size = end - *start;
> + if (size < len)
> + return -1;
> +
> + for (; npages; npages--, idx++) {
> + rc = xc_domain_add_to_physmap(xch, domid, XENMAPSPACE_pv_time, idx,
> + (*start >> XC_PAGE_SHIFT) + idx);
> + if (rc)
> + return rc;
> + }
> +
> + region_base[0] = *start;
> + region_size[0] = len;
> + *start += len;
> + return 0;
> +}
> +
> +static void get_ext_region(uint64_t start, uint64_t end, uint64_t
> *region_base,
> + uint64_t *region_size, unsigned int *nr_regions)
> +{
> + uint64_t size;
> +
> + start = ALIGN_UP_TO_2MB(start);
> + if (start >= end)
> + return;
> +
> + size = end - start;
> + if (size < EXT_REGION_MIN_SIZE)
> + return;
> +
> + region_base[*nr_regions] = start;
> + region_size[*nr_regions] = size;
> + (*nr_regions)++;
> +}
> +
> +static int finalize_extra_regions(libxl__gc *gc,
> + libxl_domain_build_info *b_info,
> + struct xc_dom_image *dom)
> {
> void *fdt = dom->devicetree_blob;
> - uint64_t region_base[MAX_NR_EXT_REGIONS],
> region_size[MAX_NR_EXT_REGIONS];
> - uint32_t regs[(GUEST_ROOT_ADDRESS_CELLS + GUEST_ROOT_SIZE_CELLS) *
> +
> + /* For extended regions */
> + uint64_t ext_region_base[MAX_NR_EXT_REGIONS],
> ext_region_size[MAX_NR_EXT_REGIONS];
> + uint32_t ext_regs[(GUEST_ROOT_ADDRESS_CELLS + GUEST_ROOT_SIZE_CELLS) *
> (MAX_NR_EXT_REGIONS + 1)];
> - be32 *cells = ®s[0];
> + be32 *ext_cells = &ext_regs[0];
> + int hyp_offset;
> +
> + /* For pv regions */
> + uint64_t pv_region_base[1], pv_region_size[1];
> + uint32_t pv_regs[GUEST_ROOT_ADDRESS_CELLS + GUEST_ROOT_SIZE_CELLS];
> + be32 *pv_cells = &pv_regs[0];
> + int resv_offset;
> +
> const uint64_t bankbase[] = GUEST_RAM_BANK_BASES;
> const uint64_t banksize[] = GUEST_RAM_BANK_SIZES;
> unsigned int i, j, len, nr_regions = 0;
> + bool pv_region_pending = true;
> libxl_dominfo info;
> - int offset, rc;
> + int rc;
>
> - offset = fdt_path_offset(fdt, "/hypervisor");
> - if (offset < 0)
> - return offset;
> + resv_offset = fdt_path_offset(fdt, "/reserved-memory");
> + if (!strcmp(dom->guest_type, "xen-3.0-aarch64")) {
> + if (resv_offset < 0)
> + return resv_offset;
> + } else
> + pv_region_pending = false;
> +
> + hyp_offset = fdt_path_offset(fdt, "/hypervisor");
> + if (hyp_offset < 0)
> + return hyp_offset;
>
> rc = libxl_domain_info(CTX, &info, dom->guest_domid);
> if (rc)
> @@ -1572,8 +1678,7 @@ static int finalize_hypervisor_node(libxl__gc *gc,
> } unallocated;
> uint64_t unallocated_size = 0;
>
> - unallocated.start = bankbase[i] +
> - ALIGN_UP_TO_2MB((uint64_t)dom->rambank_size[i] << XC_PAGE_SHIFT);
> + unallocated.start = bankbase[i] + ((uint64_t)dom->rambank_size[i] <<
> XC_PAGE_SHIFT);
>
> unallocated.end = ~0ULL >> (64 - info.gpaddr_bits);
> unallocated.end = min(unallocated.end, bankbase[i] + banksize[i] -
> 1);
> @@ -1581,7 +1686,7 @@ static int finalize_hypervisor_node(libxl__gc *gc,
> if (unallocated.end >= unallocated.start)
> unallocated_size = unallocated.end - unallocated.start + 1;
>
> - if (unallocated_size < EXT_REGION_MIN_SIZE)
> + if (unallocated_size <= 0)
> continue;
>
> /* Exclude iomem */
> @@ -1605,14 +1710,14 @@ static int finalize_hypervisor_node(libxl__gc *gc,
> if (unallocated.start > unallocated.end)
> break;
> } else {
> - uint64_t size = iomem.start - unallocated.start;
> -
> - if (size >= EXT_REGION_MIN_SIZE) {
> - region_base[nr_regions] = unallocated.start;
> - region_size[nr_regions] = size;
> - nr_regions++;
> + if (pv_region_pending) {
> + rc = get_pv_region(b_info, dom, &unallocated.start,
> iomem.start,
> + pv_region_base, pv_region_size);
> + if (!rc)
> + pv_region_pending = false;
> }
> -
> + get_ext_region(unallocated.start, iomem.start,
> ext_region_base,
> + ext_region_size, &nr_regions);
> unallocated.start = iomem.end + 1;
>
> if (unallocated.start > unallocated.end)
> @@ -1624,38 +1729,52 @@ static int finalize_hypervisor_node(libxl__gc *gc,
> if (unallocated.end >= unallocated.start
> && nr_regions < MAX_NR_EXT_REGIONS)
> {
> - uint64_t size = unallocated.end - unallocated.start + 1;
> -
> - if (size >= EXT_REGION_MIN_SIZE) {
> - region_base[nr_regions] = unallocated.start;
> - region_size[nr_regions] = size;
> - nr_regions++;
> + if (pv_region_pending) {
> + rc = get_pv_region(b_info, dom, &unallocated.start,
> unallocated.end,
> + pv_region_base, pv_region_size);
> + if (!rc)
> + pv_region_pending = false;
> }
> + get_ext_region(unallocated.start, unallocated.end,
> ext_region_base,
> + ext_region_size, &nr_regions);
> }
> }
>
> + if (!strcmp(dom->guest_type, "xen-3.0-aarch64")) {
> + if (pv_region_pending) {
> + LOG(ERROR, "The PV time region cannot be allocated, not enough
> space");
> + return ERROR_FAIL;
> + }
> + set_range(&pv_cells, GUEST_ROOT_ADDRESS_CELLS, GUEST_ROOT_SIZE_CELLS,
> + pv_region_base[0], pv_region_size[0]);
> + len = sizeof(pv_regs[0]) * (GUEST_ROOT_ADDRESS_CELLS +
> GUEST_ROOT_SIZE_CELLS);
> + rc = fdt_setprop(fdt, resv_offset, "reg", pv_regs, len);
I can appreciate the effort that it took to get this working. However,
given the relatively small size of PV time region, is there a reason
that you chose to allocate it from unallocated guest RAM rather than
reserving a new region/space in xen/include/public/arch-arm.h? Doing so
may also simplify the dom0less case.
E.g.:
diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
index e2412a17474e..681e3d4778ba 100644
--- a/xen/include/public/arch-arm.h
+++ b/xen/include/public/arch-arm.h
@@ -466,6 +466,13 @@ typedef uint64_t xen_callback_t;
#define GUEST_VPCI_MEM_ADDR xen_mk_ullong(0x23000000)
#define GUEST_VPCI_MEM_SIZE xen_mk_ullong(0x10000000)
+/* Current supported guest VCPUs */
+#define GUEST_MAX_VCPUS 128
+
+/* PV time region */
+#define GUEST_PVTIME_BASE xen_mk_ullong(0x37000000)
+#define GUEST_PVTIME_SIZE xen_mk_ullong(GUEST_MAX_VCPUS * 64)
+
/*
* 16MB == 4096 pages reserved for guest to use as a region to map its
* grant table in.
@@ -501,9 +508,6 @@ 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 }
-/* Current supported guest VCPUs */
-#define GUEST_MAX_VCPUS 128
-
/* Interrupts */
#define GUEST_TIMER_VIRT_PPI 27
