[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v4 05/11] xen/arm: introduce direct-map for domUs
On Mon, 20 Dec 2021, Penny Zheng wrote: > Cases where domU needs direct-map memory map: > * IOMMU not present in the system. > * IOMMU disabled if it doesn't cover a specific device and all the guests > are trusted. Thinking a mixed scenario, where a few devices with IOMMU and > a few without, then guest DMA security still could not be totally guaranteed. > So users may want to disable the IOMMU, to at least gain some performance > improvement from IOMMU disabled. > * IOMMU disabled as a workaround when it doesn't have enough bandwidth. > To be specific, in a few extreme situation, when multiple devices do DMA > concurrently, these requests may exceed IOMMU's transmission capacity. > * IOMMU disabled when it adds too much latency on DMA. For example, > TLB may be missing in some IOMMU hardware, which may bring latency in DMA > progress, so users may want to disable it in some realtime scenario. > * Guest OS relies on the host memory layout > > This commit introduces a new helper assign_static_memory_11 to allocate > static memory as guest RAM for direct-map domain. > > Signed-off-by: Penny Zheng <penny.zheng@xxxxxxx> The patch looks good. There are a couple of minor code style issus below that it would be good to fix, at least the function parameters alignment. With that: Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx> > --- > v2 changes: > - split the common codes into two helpers: parse_static_mem_prop and > acquire_static_memory_bank to deduce complexity. > - introduce a new helper allocate_static_memory_11 for allocating static > memory for direct-map guests > - remove redundant use "bool direct_map", to be replaced by > d_cfg.flags & XEN_DOMCTL_CDF_directmap > - remove panic action since it is fine to assign a non-DMA capable device when > IOMMU and direct-map both off > --- > v3 changes: > - doc refinement > - drop the pointless gbank > - add check of the size of nr_banks shall not exceed NR_MEM_BANKS > - add ASSERT_UNREACHABLE to catch any misuse > - add another check of validating flag XEN_DOMCTL_CDF_INTERNAL_directmap only > when CONFIG_STATIC_MEMORY is set. > --- > v4 changes: > - comment refinement > - rename function allocate_static_memory_11() to assign_static_memory_11() > to make clear there is actually no allocation done. Instead we are only > mapping pre-defined host regions to pre-defined guest regions. > - remove tot_size to directly substract psize from kinfo->unassigned_mem > - check kinfo->unassigned_mem doesn't underflow or overflow > - remove nested if/else > - refine "panic" info > --- > xen/arch/arm/domain_build.c | 97 +++++++++++++++++++++++++++++++++++-- > 1 file changed, 94 insertions(+), 3 deletions(-) > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > index 9206ec908d..d74a3eb908 100644 > --- a/xen/arch/arm/domain_build.c > +++ b/xen/arch/arm/domain_build.c > @@ -494,8 +494,17 @@ static bool __init append_static_memory_to_bank(struct > domain *d, > { > int res; > unsigned int nr_pages = PFN_DOWN(size); > - /* Infer next GFN. */ > - gfn_t sgfn = gaddr_to_gfn(bank->start + bank->size); > + gfn_t sgfn; > + > + /* > + * For direct-mapped domain, the GFN match the MFN. > + * Otherwise, this is inferred on what has already been allocated > + * in the bank. > + */ > + if ( !is_domain_direct_mapped(d) ) > + sgfn = gaddr_to_gfn(bank->start + bank->size); > + else > + sgfn = gaddr_to_gfn(mfn_to_maddr(smfn)); > > res = guest_physmap_add_pages(d, sgfn, smfn, nr_pages); > if ( res ) > @@ -668,12 +677,92 @@ static void __init allocate_static_memory(struct domain > *d, > fail: > panic("Failed to allocate requested static memory for domain %pd.", d); > } > + > +/* > + * Allocate static memory as RAM for one specific domain d. > + * The static memory will be directly mapped in the guest(Guest Physical > + * Address == Physical Address). > + */ > +static void __init assign_static_memory_11(struct domain *d, > + struct kernel_info *kinfo, > + const struct dt_device_node > *node) Please align the parameters > +{ > + u32 addr_cells, size_cells, reg_cells; > + unsigned int nr_banks, bank = 0; > + const __be32 *cell; > + paddr_t pbase, psize; > + mfn_t smfn; > + int length; > + > + if ( parse_static_mem_prop(node, &addr_cells, &size_cells, &length, > &cell) ) > + { > + printk(XENLOG_ERR > + "%pd: failed to parse \"xen,static-mem\" property.\n", d); > + goto fail; > + } > + reg_cells = addr_cells + size_cells; > + nr_banks = length / (reg_cells * sizeof (u32)); no space after sizeof > + > + if ( nr_banks > NR_MEM_BANKS ) > + { > + printk(XENLOG_ERR > + "%pd: exceed max number of supported guest memory banks.\n", > d); > + goto fail; > + } > + > + for ( ; bank < nr_banks; bank++ ) > + { > + smfn = acquire_static_memory_bank(d, &cell, addr_cells, size_cells, > + &pbase, &psize); > + if ( mfn_eq(smfn, INVALID_MFN) ) > + goto fail; > + > + printk(XENLOG_INFO "%pd: STATIC BANK[%u] > %#"PRIpaddr"-%#"PRIpaddr"\n", > + d, bank, pbase, pbase + psize); > + > + /* One guest memory bank is matched with one physical memory bank. */ > + kinfo->mem.bank[bank].start = pbase; > + if ( !append_static_memory_to_bank(d, &kinfo->mem.bank[bank], > + smfn, psize) ) > + goto fail; > + > + kinfo->unassigned_mem -= psize; > + } > + > + kinfo->mem.nr_banks = nr_banks; > + > + /* > + * The property 'memory' should match the amount of memory given to > + * the guest. > + * Currently, it is only possible to either acquire static memory or > + * let Xen allocate. *Mixing* is not supported. > + */ > + if ( kinfo->unassigned_mem != 0 ) > + { > + printk(XENLOG_ERR > + "Size of \"memory\" property doesn't match up with the sum-up > of \"xen,static-mem\". Unsupported configuration.\n"); This line would benefit from being broken down, but I am also OK if we leave it as is > + goto fail; > + } > + > + return; > + > + fail: > + panic("Failed to assign requested static memory for direct-map domain > %pd.", > + d); > +} > #else > static void __init allocate_static_memory(struct domain *d, > struct kernel_info *kinfo, > const struct dt_device_node *node) > { > } > + > +static void __init assign_static_memory_11(struct domain *d, > + struct kernel_info *kinfo, > + const struct dt_device_node > *node) > +{ > + ASSERT_UNREACHABLE(); > +} > #endif > > /* > @@ -3023,8 +3112,10 @@ static int __init construct_domU(struct domain *d, > #endif > if ( !dt_find_property(node, "xen,static-mem", NULL) ) > allocate_memory(d, &kinfo); > - else > + else if ( !is_domain_direct_mapped(d) ) > allocate_static_memory(d, &kinfo, node); > + else > + assign_static_memory_11(d, &kinfo, node); > > rc = prepare_dtb_domU(d, &kinfo); > if ( rc < 0 ) > -- > 2.25.1 >
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |