I plan to commit this patch, unless someone objects
On Thu, 8 May 2025, Stewart Hildebrand wrote:
> When a device is passed through to a xl domU, the iomem ranges may
> overlap with the extended regions. Remove iomem from extended regions.
>
> Signed-off-by: Stewart Hildebrand <stewart.hildebr...@amd.com>
> ---
> Not sure if we need a Fixes: tag, but if we do:
> Fixes: 57f87857dc2d ("libxl/arm: Add handling of extended regions for DomU")
>
> v1->v2:
> * no change
> ---
> tools/libs/light/libxl_arm.c | 118 +++++++++++++++++++++++++++++------
> 1 file changed, 99 insertions(+), 19 deletions(-)
>
> diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
> index 75c811053c7c..8ae16a1726fc 100644
> --- a/tools/libs/light/libxl_arm.c
> +++ b/tools/libs/light/libxl_arm.c
> @@ -798,6 +798,8 @@ static int make_timer_node(libxl__gc *gc, void *fdt,
> return 0;
> }
>
> +#define MAX_NR_EXT_REGIONS 256
> +
> static int make_hypervisor_node(libxl__gc *gc, void *fdt,
> const libxl_version_info *vers)
> {
> @@ -821,7 +823,7 @@ static int make_hypervisor_node(libxl__gc *gc, void *fdt,
> */
> res = fdt_property_reg_placeholder(gc, fdt, GUEST_ROOT_ADDRESS_CELLS,
> GUEST_ROOT_SIZE_CELLS,
> - GUEST_RAM_BANKS + 1);
> + MAX_NR_EXT_REGIONS + 1);
> if (res) return res;
>
> /*
> @@ -1517,17 +1519,29 @@ static void finalise_one_node(libxl__gc *gc, void
> *fdt, const char *uname,
>
> #define EXT_REGION_MIN_SIZE xen_mk_ullong(0x0004000000) /* 64MB */
>
> -static int finalize_hypervisor_node(libxl__gc *gc, struct xc_dom_image *dom)
> +static int compare_iomem(const void *a, const void *b)
> +{
> + const libxl_iomem_range *x = a, *y = b;
> +
> + if (x->gfn < y->gfn)
> + return -1;
> + if (x->gfn > y->gfn)
> + return 1;
> + return 0;
> +}
> +
> +static int finalize_hypervisor_node(libxl__gc *gc,
> + libxl_domain_build_info *b_info,
> + 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];
> + 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) *
> - (GUEST_RAM_BANKS + 1)];
> + (MAX_NR_EXT_REGIONS + 1)];
> be32 *cells = ®s[0];
> const uint64_t bankbase[] = GUEST_RAM_BANK_BASES;
> const uint64_t banksize[] = GUEST_RAM_BANK_SIZES;
> - unsigned int i, len, nr_regions = 0;
> + unsigned int i, j, len, nr_regions = 0;
> libxl_dominfo info;
> int offset, rc;
>
> @@ -1542,20 +1556,90 @@ static int finalize_hypervisor_node(libxl__gc *gc,
> struct xc_dom_image *dom)
> if (info.gpaddr_bits > 64)
> return ERROR_INVAL;
>
> + qsort(b_info->iomem, b_info->num_iomem, sizeof(libxl_iomem_range),
> + compare_iomem);
> +
> /*
> * 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] +
> + for (i = 0; i < GUEST_RAM_BANKS && nr_regions < MAX_NR_EXT_REGIONS; i++)
> {
> + struct {
> + uint64_t start;
> + uint64_t end; /* inclusive */
> + } unallocated;
> + uint64_t size = 0;
> +
> + unallocated.start = 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;
> + unallocated.end = ~0ULL >> (64 - info.gpaddr_bits);
> + unallocated.end = min(unallocated.end, bankbase[i] + banksize[i] -
> 1);
> +
> + if (unallocated.end > unallocated.start)
> + size = unallocated.end - unallocated.start + 1;
> +
> + if (size < EXT_REGION_MIN_SIZE)
> + continue;
> +
> + /* Exclude iomem */
> + for (j = 0; j < b_info->num_iomem && nr_regions < MAX_NR_EXT_REGIONS;
> + j++) {
> + struct {
> + uint64_t start;
> + uint64_t end; /* inclusive */
> + } iomem;
> +
> + iomem.start = b_info->iomem[j].gfn << XC_PAGE_SHIFT;
> + iomem.end = ((b_info->iomem[j].gfn + b_info->iomem[j].number)
> + << XC_PAGE_SHIFT) - 1;
> +
> + if (iomem.end >= unallocated.start
> + && iomem.start <= unallocated.end) {
> +
> + if (iomem.start <= unallocated.start) {
> + unallocated.start = iomem.end + 1;
> +
> + if (iomem.end >= unallocated.end)
> + /* Complete overlap, discard unallocated region */
> + break;
> +
> + /* Beginning overlap */
> + continue;
> + }
> +
> + if (iomem.start > unallocated.start) {
> + assert(unallocated.end > unallocated.start);
> + size = iomem.start - unallocated.start;
> +
> + if (size >= EXT_REGION_MIN_SIZE) {
> + region_base[nr_regions] = unallocated.start;
> + region_size[nr_regions] = size;
> + nr_regions++;
> + }
> +
> + unallocated.start = iomem.end + 1;
> +
> + if (iomem.end >= unallocated.end)
> + /* End overlap, discard remaining unallocated region
> */
> + break;
> + }
> + }
> + }
> +
> + if (unallocated.end > unallocated.start
> + && nr_regions < MAX_NR_EXT_REGIONS)
> + {
> + 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++;
> + }
> + }
> }
>
> /*
> @@ -1565,16 +1649,12 @@ static int finalize_hypervisor_node(libxl__gc *gc,
> struct xc_dom_image *dom)
> 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;
> -
> + for (i = 0; i < nr_regions; i++) {
> LOG(DEBUG, "Extended region %u: %#"PRIx64"->%#"PRIx64"",
> - nr_regions, region_base[i], region_base[i] + region_size[i]);
> + i, 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)
> @@ -1626,7 +1706,7 @@ int
> libxl__arch_domain_finalise_hw_description(libxl__gc *gc,
>
> }
>
> - res = finalize_hypervisor_node(gc, dom);
> + res = finalize_hypervisor_node(gc, &d_config->b_info, dom);
> if (res)
> return res;
>
> --
> 2.49.0
>
>
