On 6/12/25 05:06, Orzel, Michal wrote:
> On 11/06/2025 19:51, Stewart Hildebrand wrote:
>> Similarly to fba1b0974dd8, when a device is passed through to a
>> direct-map dom0less domU, the xen,reg ranges may overlap with the
>> extended regions. Remove xen,reg from direct-map domU extended regions.
>>
>> Take the opportunity to update the comment ahead of find_memory_holes().
>>
>> Signed-off-by: Stewart Hildebrand <stewart.hildebr...@amd.com>
>> ---
>> v4->v5:
>> * remove extranous -1 and +1
>> * create local helper function to count ranges
>>
>> v3->v4:
>> * conditionally allocate xen_reg
>> * use rangeset_report_ranges()
>> * make find_unallocated_memory() cope with NULL entry
>>
>> v2->v3:
>> * new patch
>> ---
>> xen/arch/arm/domain_build.c | 80 +++++++++++++++++++++++++--
>> xen/common/device-tree/domain-build.c | 5 ++
>> 2 files changed, 80 insertions(+), 5 deletions(-)
>>
>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>> index 590f38e52053..cef6c85e22ec 100644
>> --- a/xen/arch/arm/domain_build.c
>> +++ b/xen/arch/arm/domain_build.c
>> @@ -792,15 +792,17 @@ static int __init handle_pci_range(const struct
>> dt_device_node *dev,
>> }
>>
>> /*
>> - * Find the holes in the Host DT which can be exposed to Dom0 as extended
>> - * regions for the special memory mappings. In order to calculate regions
>> - * we exclude every addressable memory region described by "reg" and
>> "ranges"
>> - * properties from the maximum possible addressable physical memory range:
>> + * Find the holes in the Host DT which can be exposed to Dom0 or a
>> direct-map
> If you took occasion to improve this comment, then I would replace Dom0 with
> hwdom.
Will do.
>> + * domU as extended regions for the special memory mappings. In order to
>> + * calculate regions we exclude every addressable memory region described by
>> + * "reg" and "ranges" properties from the maximum possible addressable
>> physical
>> + * memory range:
>> * - MMIO
>> * - Host RAM
>> * - PCI aperture
>> * - Static shared memory regions, which are described by special property
>> * "xen,shared-mem"
>> + * - xen,reg mappings
>> */
>> static int __init find_memory_holes(const struct kernel_info *kinfo,
>> struct membanks *ext_regions)
>> @@ -882,6 +884,13 @@ static int __init find_memory_holes(const struct
>> kernel_info *kinfo,
>> }
>> }
>>
>> + if ( kinfo->xen_reg_assigned )
>> + {
>> + res = rangeset_subtract(mem_holes, kinfo->xen_reg_assigned);
>> + if ( res )
>> + goto out;
>> + }
>> +
>> start = 0;
>> end = (1ULL << p2m_ipa_bits) - 1;
>> res = rangeset_report_ranges(mem_holes, PFN_DOWN(start), PFN_DOWN(end),
>> @@ -962,11 +971,51 @@ static int __init find_domU_holes(const struct
>> kernel_info *kinfo,
>> return res;
>> }
>>
>> +static int __init count(unsigned long s, unsigned long e, void *data)
>> +{
>> + unsigned int *cnt = data;
>> +
>> + (*cnt)++;
>> +
>> + return 0;
>> +}
>> +
>> +static unsigned int __init count_ranges(struct rangeset *r)
>> +{
>> + unsigned int cnt = 0;
>> +
>> + rangeset_report_ranges(r, 0, PFN_DOWN((1ULL << p2m_ipa_bits) - 1),
>> count,
> I don't think it's ok with MISRA C to not check the return code, even though
> in
> this case it's always 0. Either we should check or have (void).
I'll add (void).
>> + &cnt);
>> +
>> + return cnt;
>> +}
>> +
>> +static int __init rangeset_to_membank(unsigned long s_gfn, unsigned long
>> e_gfn,
> Here you use s_gfn, e_gfn but for count() you used s,e. Even if unused, I
> would
> prefer consistency.
I'll make it consistent.
>> + void *data)
>> +{
>> + struct membanks *membank = data;
>> + paddr_t s = pfn_to_paddr(s_gfn);
>> + paddr_t e = pfn_to_paddr(e_gfn + 1);
>> +
>> + if ( membank->nr_banks >= membank->max_banks )
>> + return 0;
>> +
>> + membank->bank[membank->nr_banks].start = s;
>> + membank->bank[membank->nr_banks].size = e - s;
>> + membank->nr_banks++;
>> +
>> + return 0;
>> +}
>> +
>> static int __init find_host_extended_regions(const struct kernel_info
>> *kinfo,
>> struct membanks *ext_regions)
>> {
>> int res;
>> struct membanks *gnttab = membanks_xzalloc(1, MEMORY);
>> + struct membanks *xen_reg =
>> + kinfo->xen_reg_assigned
>> + ? membanks_xzalloc(count_ranges(kinfo->xen_reg_assigned), MEMORY)
>> + : NULL;
>>
>> /*
>> * Exclude the following regions:
>> @@ -974,6 +1023,7 @@ static int __init find_host_extended_regions(const
>> struct kernel_info *kinfo,
>> * 2) Remove reserved memory
>> * 3) Grant table assigned to domain
>> * 4) Remove static shared memory (when the feature is enabled)
>> + * 5) Remove xen,reg
>> */
>> const struct membanks *mem_banks[] = {
>> kernel_info_get_mem_const(kinfo),
>> @@ -982,12 +1032,29 @@ static int __init find_host_extended_regions(const
>> struct kernel_info *kinfo,
>> #ifdef CONFIG_STATIC_SHM
>> bootinfo_get_shmem(),
>> #endif
>> + xen_reg,
>> };
>>
>> dt_dprintk("Find unallocated memory for extended regions\n");
>>
>> if ( !gnttab )
>> - return -ENOMEM;
>> + {
>> + res = -ENOMEM;
>> + goto out;
>> + }
>> +
>> + if ( kinfo->xen_reg_assigned )
>> + {
>> + if ( !xen_reg )
>> + {
>> + res = -ENOMEM;
>> + goto out;
>> + }
>> +
>> + rangeset_report_ranges(kinfo->xen_reg_assigned, 0,
>> + PFN_DOWN((1ULL << p2m_ipa_bits) - 1),
>> + rangeset_to_membank, xen_reg);
>> + }
>>
>> gnttab->nr_banks = 1;
>> gnttab->bank[0].start = kinfo->gnttab_start;
>> @@ -995,6 +1062,9 @@ static int __init find_host_extended_regions(const
>> struct kernel_info *kinfo,
>>
>> res = find_unallocated_memory(kinfo, mem_banks, ARRAY_SIZE(mem_banks),
>> ext_regions, add_ext_regions);
>> +
>> + out:
>> + xfree(xen_reg);
>> xfree(gnttab);
>>
>> return res;
>> diff --git a/xen/common/device-tree/domain-build.c
>> b/xen/common/device-tree/domain-build.c
>> index 6b8b8d7cacb6..99ea81198a76 100644
>> --- a/xen/common/device-tree/domain-build.c
>> +++ b/xen/common/device-tree/domain-build.c
>> @@ -193,6 +193,10 @@ int __init find_unallocated_memory(const struct
>> kernel_info *kinfo,
>>
>> /* Remove all regions listed in mem_banks */
>> for ( i = 0; i < nr_mem_banks; i++ )
>> + {
>> + if ( !mem_banks[i] )
>> + continue;
>> +
>> for ( j = 0; j < mem_banks[i]->nr_banks; j++ )
>> {
>> start = mem_banks[i]->bank[j].start;
>> @@ -212,6 +216,7 @@ int __init find_unallocated_memory(const struct
>> kernel_info *kinfo,
>> goto out;
>> }
>> }
>> + }
>>
>> start = 0;
>> end = (1ULL << p2m_ipa_bits) - 1;
>
> Other than that, LGTM:
> Reviewed-by: Michal Orzel <michal.or...@amd.com>
Thanks!
