[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH V5 3/3] xen/arm: Updates for extended regions support
On 07.10.21 04:50, Stefano Stabellini wrote: Hi Stefano On Wed, 6 Oct 2021, Oleksandr Tyshchenko wrote:From: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx> This is a follow-up of "b6fe410 xen/arm: Add handling of extended regions for Dom0" Add various in-code comments, update Xen hypervisor device tree bindings text, change the log level for some prints and clarify format specifier, reuse dt_for_each_range() to avoid open-coding in find_memory_holes(). Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>Thanks for the patch, it looks like you addressed all Julien's comments well. I believe so) A couple of minor issues below.--- New patch --- docs/misc/arm/device-tree/guest.txt | 12 ++-- xen/arch/arm/domain_build.c | 108 ++++++++++++++++++++++-------------- 2 files changed, 73 insertions(+), 47 deletions(-) diff --git a/docs/misc/arm/device-tree/guest.txt b/docs/misc/arm/device-tree/guest.txt index 418f1e9..c115751 100644 --- a/docs/misc/arm/device-tree/guest.txt +++ b/docs/misc/arm/device-tree/guest.txt @@ -7,10 +7,14 @@ the following properties: compatible = "xen,xen-<version>", "xen,xen"; where <version> is the version of the Xen ABI of the platform.-- reg: specifies the base physical address and size of a region in- memory where the grant table should be mapped to, using an - HYPERVISOR_memory_op hypercall. The memory region is large enough to map - the whole grant table (it is larger or equal to gnttab_max_grant_frames()). +- reg: specifies the base physical address and size of the regions in memory + where the special resources should be mapped to, using an HYPERVISOR_memory_op + hypercall. + Region 0 is reserved for mapping grant table, it must be always present. + The memory region is large enough to map the whole grant table (it is larger + or equal to gnttab_max_grant_frames()). + Regions 1...N are extended regions (unused address space) for mapping foreign + GFNs and grants, they might be absent if there is nothing to expose. This property is unnecessary when booting Dom0 using ACPI.- interrupts: the interrupt used by Xen to inject event notifications.diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index c5afbe2..d9f40d4 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -898,7 +898,10 @@ static int __init add_ext_regions(unsigned long s, unsigned long e, void *data) if ( ext_regions->nr_banks >= ARRAY_SIZE(ext_regions->bank) ) return 0;- /* Both start and size of the extended region should be 2MB aligned */+ /* + * Both start and size of the extended region should be 2MB aligned to + * potentially allow superpage mapping. + */ start = (s + SZ_2M - 1) & ~(SZ_2M - 1); if ( start > e ) return 0; @@ -909,6 +912,12 @@ static int __init add_ext_regions(unsigned long s, unsigned long e, void *data) */ e += 1; size = (e - start) & ~(SZ_2M - 1); + + /* + * Reasonable size. Not too little to pick up small ranges which are + * not quite useful itself but increase bookkeeping and not too much^ remove itself ^ large ok + * to skip a large proportion of unused address space. + */ if ( size < MB(64) ) return 0;@@ -919,6 +928,14 @@ static int __init add_ext_regions(unsigned long s, unsigned long e, void *data)return 0; }+/*+ * Find unused regions of Host address space which can be exposed to Dom0 + * as extended regions for the special memory mappings. In order to calculate + * regions we exclude every assigned to Dom0 region from the Host RAM:^ region assigned ^ remove ok + * - domain RAM + * - reserved-memory + * - grant table space + */ static int __init find_unallocated_memory(const struct kernel_info *kinfo, struct meminfo *ext_regions) { @@ -942,7 +959,7 @@ static int __init find_unallocated_memory(const struct kernel_info *kinfo, res = rangeset_add_range(unalloc_mem, start, end - 1); if ( res ) { - printk(XENLOG_ERR "Failed to add: %#"PRIx64"->%#"PRIx64"\n", + printk(XENLOG_ERR "Failed to add: %#"PRIpaddr"->%#"PRIpaddr"\n", start, end); goto out; } @@ -956,7 +973,7 @@ static int __init find_unallocated_memory(const struct kernel_info *kinfo, res = rangeset_remove_range(unalloc_mem, start, end - 1); if ( res ) { - printk(XENLOG_ERR "Failed to remove: %#"PRIx64"->%#"PRIx64"\n", + printk(XENLOG_ERR "Failed to remove: %#"PRIpaddr"->%#"PRIpaddr"\n", start, end); goto out; } @@ -971,7 +988,7 @@ static int __init find_unallocated_memory(const struct kernel_info *kinfo, res = rangeset_remove_range(unalloc_mem, start, end - 1); if ( res ) { - printk(XENLOG_ERR "Failed to remove: %#"PRIx64"->%#"PRIx64"\n", + printk(XENLOG_ERR "Failed to remove: %#"PRIpaddr"->%#"PRIpaddr"\n", start, end); goto out; } @@ -983,7 +1000,7 @@ static int __init find_unallocated_memory(const struct kernel_info *kinfo, res = rangeset_remove_range(unalloc_mem, start, end - 1); if ( res ) { - printk(XENLOG_ERR "Failed to remove: %#"PRIx64"->%#"PRIx64"\n", + printk(XENLOG_ERR "Failed to remove: %#"PRIpaddr"->%#"PRIpaddr"\n", start, end); goto out; } @@ -1003,6 +1020,35 @@ out: return res; }+static int __init handle_pci_range(const struct dt_device_node *dev,+ u64 addr, u64 len, void *data) +{ + struct rangeset *mem_holes = data; + paddr_t start, end; + int res; + + start = addr & PAGE_MASK; + end = PAGE_ALIGN(addr + len); + res = rangeset_remove_range(mem_holes, start, end - 1); + if ( res ) + { + printk(XENLOG_ERR "Failed to remove: %#"PRIpaddr"->%#"PRIpaddr"\n", + start, end); + return res; + } + + return 0; +} + +/* + * 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: + * - MMIO + * - Host RAM + * - PCI bar^ PCI aperture ok + */ static int __init find_memory_holes(const struct kernel_info *kinfo, struct meminfo *ext_regions) { @@ -1024,7 +1070,7 @@ static int __init find_memory_holes(const struct kernel_info *kinfo, res = rangeset_add_range(mem_holes, start, end); if ( res ) { - printk(XENLOG_ERR "Failed to add: %#"PRIx64"->%#"PRIx64"\n", + printk(XENLOG_ERR "Failed to add: %#"PRIpaddr"->%#"PRIpaddr"\n", start, end); goto out; } @@ -1055,49 +1101,25 @@ static int __init find_memory_holes(const struct kernel_info *kinfo, res = rangeset_remove_range(mem_holes, start, end - 1); if ( res ) { - printk(XENLOG_ERR "Failed to remove: %#"PRIx64"->%#"PRIx64"\n", + printk(XENLOG_ERR "Failed to remove: %#"PRIpaddr"->%#"PRIpaddr"\n", start, end); goto out; } }- if ( dt_device_type_is_equal(np, "pci" ) )+ if ( dt_device_type_is_equal(np, "pci") ) { - unsigned int range_size, nr_ranges; - int na, ns, pna; - const __be32 *ranges; - u32 len; - /* - * Looking for non-empty ranges property which in this context - * describes the PCI host bridge aperture. + * The ranges property in this context describes the PCI host + * bridge aperture. It shall be absent if no addresses are mapped + * through the bridge. */ - ranges = dt_get_property(np, "ranges", &len); - if ( !ranges || !len ) + if ( !dt_get_property(np, "ranges", NULL) ) continue;- pna = dt_n_addr_cells(np);- na = dt_child_n_addr_cells(np); - ns = dt_child_n_size_cells(np); - range_size = pna + na + ns; - nr_ranges = len / sizeof(__be32) / range_size; - - for ( i = 0; i < nr_ranges; i++, ranges += range_size ) - { - /* Skip the child address and get the parent (CPU) address */ - addr = dt_read_number(ranges + na, pna); - size = dt_read_number(ranges + na + pna, ns); - - start = addr & PAGE_MASK; - end = PAGE_ALIGN(addr + size); - res = rangeset_remove_range(mem_holes, start, end - 1); - if ( res ) - { - printk(XENLOG_ERR "Failed to remove: %#"PRIx64"->%#"PRIx64"\n", - start, end); - goto out; - } - } + res = dt_for_each_range(np, &handle_pci_range, mem_holes); + if ( res ) + goto out; } }@@ -1152,12 +1174,12 @@ static int __init make_hypervisor_node(struct domain *d, if ( !opt_ext_regions ){ - printk(XENLOG_DEBUG "The extended regions support is disabled\n"); + printk(XENLOG_INFO "The extended regions support is disabled\n"); nr_ext_regions = 0; } else if ( is_32bit_domain(d) ) { - printk(XENLOG_DEBUG "The extended regions are only supported for 64-bit guest currently\n"); + printk(XENLOG_WARNING "The extended regions are only supported for 64-bit guest currently\n"); nr_ext_regions = 0; } else @@ -1193,8 +1215,8 @@ static int __init make_hypervisor_node(struct domain *d, u64 start = ext_regions->bank[i].start; u64 size = ext_regions->bank[i].size;- dt_dprintk("Extended region %d: %#"PRIx64"->%#"PRIx64"\n",- i, start, start + size); + printk("Extended region %d: %#"PRIx64"->%#"PRIx64"\n", + i, start, start + size);Also should be PRIpaddr I thought I needed to change specifier only for variables of type "paddr_t", but here "u64". -- Regards, Oleksandr Tyshchenko
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |