Message ID | 20220214031956.3726764-3-penny.zheng@arm.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | direct-map memory map | expand |
(+ Jan) Hi Penny, I am CCing Jan to give him a chance to... On 14/02/2022 03:19, Penny Zheng wrote: > diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h > index cfb0b47f13..24eb4cc7d3 100644 > --- a/xen/include/xen/domain.h > +++ b/xen/include/xen/domain.h > @@ -31,6 +31,10 @@ void arch_get_domain_info(const struct domain *d, > /* CDF_* constant. Internal flags for domain creation. */ > /* Is this a privileged domain? */ > #define CDF_privileged (1U << 0) > +#ifdef CONFIG_ARM > +/* Should domain memory be directly mapped? */ > +#define CDF_directmap (1U << 1) > +#endif ... comment on this approach. I would be happy to switch to an ASSERT() if that's preferred. Please note that as you modify x86 code (even it is a couple of lines) you should technically CC the x86 maintainers. Similarly the changes in include/xen/domain.h should have the REST CCed. We have a script that will find the proper correct CC for each patch (see scripts/add_maintainers.pl). The workflow is written down in the script itself. Cheers,
On 15.02.2022 21:26, Julien Grall wrote: > (+ Jan) > > Hi Penny, > > I am CCing Jan to give him a chance to... Thanks, but ... > On 14/02/2022 03:19, Penny Zheng wrote: >> diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h >> index cfb0b47f13..24eb4cc7d3 100644 >> --- a/xen/include/xen/domain.h >> +++ b/xen/include/xen/domain.h >> @@ -31,6 +31,10 @@ void arch_get_domain_info(const struct domain *d, >> /* CDF_* constant. Internal flags for domain creation. */ >> /* Is this a privileged domain? */ >> #define CDF_privileged (1U << 0) >> +#ifdef CONFIG_ARM >> +/* Should domain memory be directly mapped? */ >> +#define CDF_directmap (1U << 1) >> +#endif > > ... comment on this approach. I would be happy to switch to an ASSERT() > if that's preferred. ... I think I did signal agreement with this approach beforehand. It leaves the option to use the same bit for something x86-specific down the road. Jan
Hi Jan, On 16/02/2022 09:34, Jan Beulich wrote: > On 15.02.2022 21:26, Julien Grall wrote: >> (+ Jan) >> >> Hi Penny, >> >> I am CCing Jan to give him a chance to... > > Thanks, but ... > >> On 14/02/2022 03:19, Penny Zheng wrote: >>> diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h >>> index cfb0b47f13..24eb4cc7d3 100644 >>> --- a/xen/include/xen/domain.h >>> +++ b/xen/include/xen/domain.h >>> @@ -31,6 +31,10 @@ void arch_get_domain_info(const struct domain *d, >>> /* CDF_* constant. Internal flags for domain creation. */ >>> /* Is this a privileged domain? */ >>> #define CDF_privileged (1U << 0) >>> +#ifdef CONFIG_ARM >>> +/* Should domain memory be directly mapped? */ >>> +#define CDF_directmap (1U << 1) >>> +#endif >> >> ... comment on this approach. I would be happy to switch to an ASSERT() >> if that's preferred. > > ... I think I did signal agreement with this approach beforehand. You raised a concern and it wasn't 100% obvious whether you would still be happy if we merged it as-is. So I preferred to ask rather than merging it and getting an angry e-mail/message afterwards :). Anyway, this series is now fully acked. So I will commit the series in a bit. Cheers,
Hi, On 17/02/2022 11:01, Julien Grall wrote: > On 16/02/2022 09:34, Jan Beulich wrote: >> On 15.02.2022 21:26, Julien Grall wrote: >>> (+ Jan) >>> >>> Hi Penny, >>> >>> I am CCing Jan to give him a chance to... >> >> Thanks, but ... >> >>> On 14/02/2022 03:19, Penny Zheng wrote: >>>> diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h >>>> index cfb0b47f13..24eb4cc7d3 100644 >>>> --- a/xen/include/xen/domain.h >>>> +++ b/xen/include/xen/domain.h >>>> @@ -31,6 +31,10 @@ void arch_get_domain_info(const struct domain *d, >>>> /* CDF_* constant. Internal flags for domain creation. */ >>>> /* Is this a privileged domain? */ >>>> #define CDF_privileged (1U << 0) >>>> +#ifdef CONFIG_ARM >>>> +/* Should domain memory be directly mapped? */ >>>> +#define CDF_directmap (1U << 1) >>>> +#endif >>> >>> ... comment on this approach. I would be happy to switch to an ASSERT() >>> if that's preferred. >> >> ... I think I did signal agreement with this approach beforehand. > > You raised a concern and it wasn't 100% obvious whether you would still > be happy if we merged it as-is. > > So I preferred to ask rather than merging it and getting an angry > e-mail/message afterwards :). > > Anyway, this series is now fully acked. So I will commit the series in a > bit. Well, I forgot to explicitely Acked this patch. So: Acked-by: Julien Grall <jgrall@amazon.com> This series is now fully committed. Thank you for your contribution! Cheers,
diff --git a/docs/misc/arm/device-tree/booting.txt b/docs/misc/arm/device-tree/booting.txt index 71895663a4..a94125394e 100644 --- a/docs/misc/arm/device-tree/booting.txt +++ b/docs/misc/arm/device-tree/booting.txt @@ -182,6 +182,12 @@ with the following properties: Both #address-cells and #size-cells need to be specified because both sub-nodes (described shortly) have reg properties. +- direct-map + + Only available when statically allocated memory is used for the domain. + An empty property to request the memory of the domain to be + direct-map (guest physical address == physical address). + Under the "xen,domain" compatible node, one or more sub-nodes are present for the DomU kernel and ramdisk. diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c index 92a6c509e5..8110c1df86 100644 --- a/xen/arch/arm/domain.c +++ b/xen/arch/arm/domain.c @@ -692,7 +692,8 @@ int arch_sanitise_domain_config(struct xen_domctl_createdomain *config) } int arch_domain_create(struct domain *d, - struct xen_domctl_createdomain *config) + struct xen_domctl_createdomain *config, + unsigned int flags) { int rc, count = 0; @@ -708,6 +709,8 @@ int arch_domain_create(struct domain *d, ioreq_domain_init(d); #endif + d->arch.directmap = flags & CDF_directmap; + /* p2m_init relies on some value initialized by the IOMMU subsystem */ if ( (rc = iommu_domain_init(d, config->iommu_opts)) != 0 ) goto fail; diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index 0fab8604de..6467e8ee32 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -3029,10 +3029,20 @@ void __init create_domUs(void) .max_maptrack_frames = -1, .grant_opts = XEN_DOMCTL_GRANT_version(opt_gnttab_max_version), }; + unsigned int flags = 0U; if ( !dt_device_is_compatible(node, "xen,domain") ) continue; + if ( dt_property_read_bool(node, "direct-map") ) + { + if ( !IS_ENABLED(CONFIG_STATIC_MEMORY) || !dt_find_property(node, "xen,static-mem", NULL) ) + panic("direct-map is not valid for domain %s without static allocation.\n", + dt_node_name(node)); + + flags |= CDF_directmap; + } + if ( !dt_property_read_u32(node, "cpus", &d_cfg.max_vcpus) ) panic("Missing property 'cpus' for domain %s\n", dt_node_name(node)); @@ -3058,7 +3068,7 @@ void __init create_domUs(void) * very important to use the pre-increment operator to call * domain_create() with a domid > 0. (domid == 0 is reserved for Dom0) */ - d = domain_create(++max_init_domid, &d_cfg, 0); + d = domain_create(++max_init_domid, &d_cfg, flags); if ( IS_ERR(d) ) panic("Error creating domain %s\n", dt_node_name(node)); @@ -3160,7 +3170,7 @@ void __init create_dom0(void) if ( iommu_enabled ) dom0_cfg.flags |= XEN_DOMCTL_CDF_iommu; - dom0 = domain_create(0, &dom0_cfg, CDF_privileged); + dom0 = domain_create(0, &dom0_cfg, CDF_privileged | CDF_directmap); if ( IS_ERR(dom0) || (alloc_dom0_vcpu0(dom0) == NULL) ) panic("Error creating domain 0\n"); diff --git a/xen/arch/arm/include/asm/domain.h b/xen/arch/arm/include/asm/domain.h index 9b3647587a..aabe942cde 100644 --- a/xen/arch/arm/include/asm/domain.h +++ b/xen/arch/arm/include/asm/domain.h @@ -29,8 +29,7 @@ enum domain_type { #define is_64bit_domain(d) (0) #endif -/* The hardware domain has always its memory direct mapped. */ -#define is_domain_direct_mapped(d) is_hardware_domain(d) +#define is_domain_direct_mapped(d) (d)->arch.directmap struct vtimer { struct vcpu *v; @@ -89,6 +88,8 @@ struct arch_domain #ifdef CONFIG_TEE void *tee; #endif + + bool directmap; } __cacheline_aligned; struct arch_vcpu diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c index ef1812dc14..9835f90ea0 100644 --- a/xen/arch/x86/domain.c +++ b/xen/arch/x86/domain.c @@ -722,7 +722,8 @@ static bool emulation_flags_ok(const struct domain *d, uint32_t emflags) } int arch_domain_create(struct domain *d, - struct xen_domctl_createdomain *config) + struct xen_domctl_createdomain *config, + unsigned int flags) { bool paging_initialised = false; uint32_t emflags; diff --git a/xen/common/domain.c b/xen/common/domain.c index a79103e04a..3742322d22 100644 --- a/xen/common/domain.c +++ b/xen/common/domain.c @@ -659,7 +659,7 @@ struct domain *domain_create(domid_t domid, radix_tree_init(&d->pirq_tree); } - if ( (err = arch_domain_create(d, config)) != 0 ) + if ( (err = arch_domain_create(d, config, flags)) != 0 ) goto fail; init_status |= INIT_arch; diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h index cfb0b47f13..24eb4cc7d3 100644 --- a/xen/include/xen/domain.h +++ b/xen/include/xen/domain.h @@ -31,6 +31,10 @@ void arch_get_domain_info(const struct domain *d, /* CDF_* constant. Internal flags for domain creation. */ /* Is this a privileged domain? */ #define CDF_privileged (1U << 0) +#ifdef CONFIG_ARM +/* Should domain memory be directly mapped? */ +#define CDF_directmap (1U << 1) +#endif /* * Arch-specifics. @@ -65,7 +69,8 @@ int map_vcpu_info(struct vcpu *v, unsigned long gfn, unsigned offset); void unmap_vcpu_info(struct vcpu *v); int arch_domain_create(struct domain *d, - struct xen_domctl_createdomain *config); + struct xen_domctl_createdomain *config, + unsigned int flags); void arch_domain_destroy(struct domain *d);