| Message ID | 20190218113600.9540-9-julien.grall@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | xen/arm: Properly disable M2P on Arm. | expand |
>>> On 18.02.19 at 12:35, <julien.grall@arm.com> wrote: > --- a/xen/common/domctl.c > +++ b/xen/common/domctl.c > @@ -205,7 +205,7 @@ void getdomaininfo(struct domain *d, struct xen_domctl_getdomaininfo *info) > info->outstanding_pages = d->outstanding_pages; > info->shr_pages = atomic_read(&d->shr_pages); > info->paged_pages = atomic_read(&d->paged_pages); > - info->shared_info_frame = mfn_to_gmfn(d, virt_to_mfn(d->shared_info)); > + info->shared_info_frame = gfn_x(domain_shared_info_gfn(d)); I think this change wants to be accompanied by a warning attached to the field declaration in the public header. But I'd also like to have the tool stack maintainers' view on making this field effectively unusable for Arm. > --- a/xen/common/memory.c > +++ b/xen/common/memory.c > @@ -510,6 +510,7 @@ static bool propagate_node(unsigned int xmf, unsigned int *memflags) > return true; > } > > +#ifdef CONFIG_M2P > static long memory_exchange(XEN_GUEST_HANDLE_PARAM(xen_memory_exchange_t) arg) > { > struct xen_memory_exchange exch; > @@ -802,6 +803,7 @@ static long memory_exchange(XEN_GUEST_HANDLE_PARAM(xen_memory_exchange_t) arg) > rc = -EFAULT; > return rc; > } > +#endif Please can you move the #ifdef inside the function body, add #else to return -EOPNOTSUPP, and ... > @@ -1233,12 +1235,14 @@ long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg) > > break; > > +#ifdef CONFIG_M2P > case XENMEM_exchange: > if ( unlikely(start_extent) ) > return -EINVAL; > > rc = memory_exchange(guest_handle_cast(arg, xen_memory_exchange_t)); > break; > +#endif ... avoid the extra #ifdef-ary here altogether? > --- a/xen/drivers/passthrough/iommu.c > +++ b/xen/drivers/passthrough/iommu.c > @@ -186,9 +186,10 @@ void __hwdom_init iommu_hwdom_init(struct domain *d) > hd->need_sync = iommu_hwdom_strict && !iommu_use_hap_pt(d); > if ( need_iommu_pt_sync(d) ) > { > + int rc = 0; > +#ifdef CONFIG_HAS_M2P > struct page_info *page; > unsigned int i = 0, flush_flags = 0; > - int rc = 0; > > page_list_for_each ( page, &d->page_list ) > { > @@ -215,6 +216,9 @@ void __hwdom_init iommu_hwdom_init(struct domain *d) > /* Use while-break to avoid compiler warning */ > while ( iommu_iotlb_flush_all(d, flush_flags) ) > break; > +#else > + rc = -ENOSYS; -EOPNOTSUPP please. > --- a/xen/include/asm-arm/domain.h > +++ b/xen/include/asm-arm/domain.h > @@ -267,6 +267,11 @@ static inline void free_vcpu_guest_context(struct > vcpu_guest_context *vgc) > > static inline void arch_vcpu_block(struct vcpu *v) {} > > +static inline gfn_t domain_shared_info_gfn(struct domain *d) > +{ > + return INVALID_GFN; > +} > + > #endif /* __ASM_DOMAIN_H__ */ > > /* > diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h > index d1bfc82f57..00d8b09794 100644 > --- a/xen/include/xen/domain.h > +++ b/xen/include/xen/domain.h > @@ -118,4 +118,13 @@ struct vnuma_info { > > void vnuma_destroy(struct vnuma_info *vnuma); > > +#ifdef CONFIG_HAS_M2P > +#define domain_shared_info_gfn(d_) ({ \ > + struct domain *d = (d_); \ const Also the naming needs to be the other way around: The macro parameter should be named d (all instances will get substituted anyway, and hence name collisions are impossible) while the local variable should be named d_, to avoid collisions with names in the outer scopes. > + \ I won't insist, but especially small macros are often an exception to the blank-line-between-declarations-and-statements rule. IOW I'd prefer if you dropped this line, unless others correct my view here. Jan
Hi Jan, On 13/03/2019 15:20, Jan Beulich wrote: >>>> On 18.02.19 at 12:35, <julien.grall@arm.com> wrote: >> --- a/xen/common/domctl.c >> +++ b/xen/common/domctl.c >> @@ -205,7 +205,7 @@ void getdomaininfo(struct domain *d, struct xen_domctl_getdomaininfo *info) >> info->outstanding_pages = d->outstanding_pages; >> info->shr_pages = atomic_read(&d->shr_pages); >> info->paged_pages = atomic_read(&d->paged_pages); >> - info->shared_info_frame = mfn_to_gmfn(d, virt_to_mfn(d->shared_info)); >> + info->shared_info_frame = gfn_x(domain_shared_info_gfn(d)); > > I think this change wants to be accompanied by a warning attached > to the field declaration in the public header. Make sense. > > But I'd also like to have the tool stack maintainers' view on making > this field effectively unusable for Arm. The value in shared_info_frame was plain wrong since the creation of Xen Arm. So this is just making the error more obvious. I don't expect any user of it on Arm. > >> --- a/xen/common/memory.c >> +++ b/xen/common/memory.c >> @@ -510,6 +510,7 @@ static bool propagate_node(unsigned int xmf, unsigned int *memflags) >> return true; >> } >> >> +#ifdef CONFIG_M2P >> static long memory_exchange(XEN_GUEST_HANDLE_PARAM(xen_memory_exchange_t) arg) >> { >> struct xen_memory_exchange exch; >> @@ -802,6 +803,7 @@ static long memory_exchange(XEN_GUEST_HANDLE_PARAM(xen_memory_exchange_t) arg) >> rc = -EFAULT; >> return rc; >> } >> +#endif > > Please can you move the #ifdef inside the function body, add #else > to return -EOPNOTSUPP, and ... Sure. > >> @@ -1233,12 +1235,14 @@ long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg) >> >> break; >> >> +#ifdef CONFIG_M2P >> case XENMEM_exchange: >> if ( unlikely(start_extent) ) >> return -EINVAL; >> >> rc = memory_exchange(guest_handle_cast(arg, xen_memory_exchange_t)); >> break; >> +#endif > > ... avoid the extra #ifdef-ary here altogether? > >> --- a/xen/drivers/passthrough/iommu.c >> +++ b/xen/drivers/passthrough/iommu.c >> @@ -186,9 +186,10 @@ void __hwdom_init iommu_hwdom_init(struct domain *d) >> hd->need_sync = iommu_hwdom_strict && !iommu_use_hap_pt(d); >> if ( need_iommu_pt_sync(d) ) >> { >> + int rc = 0; >> +#ifdef CONFIG_HAS_M2P >> struct page_info *page; >> unsigned int i = 0, flush_flags = 0; >> - int rc = 0; >> >> page_list_for_each ( page, &d->page_list ) >> { >> @@ -215,6 +216,9 @@ void __hwdom_init iommu_hwdom_init(struct domain *d) >> /* Use while-break to avoid compiler warning */ >> while ( iommu_iotlb_flush_all(d, flush_flags) ) >> break; >> +#else >> + rc = -ENOSYS; > > -EOPNOTSUPP please. Sure. > >> --- a/xen/include/asm-arm/domain.h >> +++ b/xen/include/asm-arm/domain.h >> @@ -267,6 +267,11 @@ static inline void free_vcpu_guest_context(struct >> vcpu_guest_context *vgc) >> >> static inline void arch_vcpu_block(struct vcpu *v) {} >> >> +static inline gfn_t domain_shared_info_gfn(struct domain *d) >> +{ >> + return INVALID_GFN; >> +} >> + >> #endif /* __ASM_DOMAIN_H__ */ >> >> /* >> diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h >> index d1bfc82f57..00d8b09794 100644 >> --- a/xen/include/xen/domain.h >> +++ b/xen/include/xen/domain.h >> @@ -118,4 +118,13 @@ struct vnuma_info { >> >> void vnuma_destroy(struct vnuma_info *vnuma); >> >> +#ifdef CONFIG_HAS_M2P >> +#define domain_shared_info_gfn(d_) ({ \ >> + struct domain *d = (d_); \ > > const > > Also the naming needs to be the other way around: The macro > parameter should be named d (all instances will get substituted > anyway, and hence name collisions are impossible) while the > local variable should be named d_, to avoid collisions with names > in the outer scopes. I will do. > >> + \ > > I won't insist, but especially small macros are often an exception > to the blank-line-between-declarations-and-statements rule. IOW > I'd prefer if you dropped this line, unless others correct my view > here. The newline is here for clarity. I am not keen to drop it unless someone else agrees with the removal. Cheers,
>>> On 13.03.19 at 18:30, <julien.grall@arm.com> wrote: > On 13/03/2019 15:20, Jan Beulich wrote: >>>>> On 18.02.19 at 12:35, <julien.grall@arm.com> wrote: >>> --- a/xen/common/domctl.c >>> +++ b/xen/common/domctl.c >>> @@ -205,7 +205,7 @@ void getdomaininfo(struct domain *d, struct > xen_domctl_getdomaininfo *info) >>> info->outstanding_pages = d->outstanding_pages; >>> info->shr_pages = atomic_read(&d->shr_pages); >>> info->paged_pages = atomic_read(&d->paged_pages); >>> - info->shared_info_frame = mfn_to_gmfn(d, virt_to_mfn(d->shared_info)); >>> + info->shared_info_frame = gfn_x(domain_shared_info_gfn(d)); >> >> I think this change wants to be accompanied by a warning attached >> to the field declaration in the public header. > > Make sense. > >> >> But I'd also like to have the tool stack maintainers' view on making >> this field effectively unusable for Arm. > > The value in shared_info_frame was plain wrong since the creation of Xen Arm. So > this is just making the error more obvious. I don't expect any user of it on Arm. Well, my request for tool stack maintainer input wasn't to put under question that the field can't currently be used sensibly on Arm. Instead I'm meaning to know whether it can be sensibly expected for the tool stack to want to use the field uniformly, in which case rather than making it more obviously not work it should be fixed instead. Jan
On Wed, 13 Mar 2019, Jan Beulich wrote: > > @@ -215,6 +216,9 @@ void __hwdom_init iommu_hwdom_init(struct domain *d) > > /* Use while-break to avoid compiler warning */ > > while ( iommu_iotlb_flush_all(d, flush_flags) ) > > break; > > +#else > > + rc = -ENOSYS; > > -EOPNOTSUPP please. The patch is fine by me and I am also fine with Jan's comments. I only want to point out that we haven't been entirely consistent with -ENOSYS and/or -EOPNOTSUPP. We have lots of places that return -ENOSYS. Should we change them to -EOPNOTSUPP going forward?
Hi, @Wei, @Ian: Do you have any input? On 14/03/2019 07:55, Jan Beulich wrote: >>>> On 13.03.19 at 18:30, <julien.grall@arm.com> wrote: >> On 13/03/2019 15:20, Jan Beulich wrote: >>>>>> On 18.02.19 at 12:35, <julien.grall@arm.com> wrote: >>>> --- a/xen/common/domctl.c >>>> +++ b/xen/common/domctl.c >>>> @@ -205,7 +205,7 @@ void getdomaininfo(struct domain *d, struct >> xen_domctl_getdomaininfo *info) >>>> info->outstanding_pages = d->outstanding_pages; >>>> info->shr_pages = atomic_read(&d->shr_pages); >>>> info->paged_pages = atomic_read(&d->paged_pages); >>>> - info->shared_info_frame = mfn_to_gmfn(d, virt_to_mfn(d->shared_info)); >>>> + info->shared_info_frame = gfn_x(domain_shared_info_gfn(d)); >>> >>> I think this change wants to be accompanied by a warning attached >>> to the field declaration in the public header. >> >> Make sense. >> >>> >>> But I'd also like to have the tool stack maintainers' view on making >>> this field effectively unusable for Arm. >> >> The value in shared_info_frame was plain wrong since the creation of Xen Arm. So >> this is just making the error more obvious. I don't expect any user of it on Arm. > > Well, my request for tool stack maintainer input wasn't to put under > question that the field can't currently be used sensibly on Arm. > Instead I'm meaning to know whether it can be sensibly expected > for the tool stack to want to use the field uniformly, in which case > rather than making it more obviously not work it should be fixed > instead. > > Jan > >
On Wed, Apr 17, 2019 at 06:42:47PM +0100, Julien Grall wrote: > Hi, > > @Wei, @Ian: Do you have any input? Sorry I haven't been following this closely, but shouldn't we fix the interface to return gfn instead? And then the mapping of it should interpret the value properly per guest type. I don't see immediately how user space program will need to access this page for autotranslated guests though. Wei. > > On 14/03/2019 07:55, Jan Beulich wrote: > > > > > On 13.03.19 at 18:30, <julien.grall@arm.com> wrote: > > > On 13/03/2019 15:20, Jan Beulich wrote: > > > > > > > On 18.02.19 at 12:35, <julien.grall@arm.com> wrote: > > > > > --- a/xen/common/domctl.c > > > > > +++ b/xen/common/domctl.c > > > > > @@ -205,7 +205,7 @@ void getdomaininfo(struct domain *d, struct > > > xen_domctl_getdomaininfo *info) > > > > > info->outstanding_pages = d->outstanding_pages; > > > > > info->shr_pages = atomic_read(&d->shr_pages); > > > > > info->paged_pages = atomic_read(&d->paged_pages); > > > > > - info->shared_info_frame = mfn_to_gmfn(d, virt_to_mfn(d->shared_info)); > > > > > + info->shared_info_frame = gfn_x(domain_shared_info_gfn(d)); > > > > > > > > I think this change wants to be accompanied by a warning attached > > > > to the field declaration in the public header. > > > > > > Make sense. > > > > > > > > > > > But I'd also like to have the tool stack maintainers' view on making > > > > this field effectively unusable for Arm. > > > > > > The value in shared_info_frame was plain wrong since the creation of Xen Arm. So > > > this is just making the error more obvious. I don't expect any user of it on Arm. > > > > Well, my request for tool stack maintainer input wasn't to put under > > question that the field can't currently be used sensibly on Arm. > > Instead I'm meaning to know whether it can be sensibly expected > > for the tool stack to want to use the field uniformly, in which case > > rather than making it more obviously not work it should be fixed > > instead. > > > > Jan > > > > > > -- > Julien Grall
Hi, On 18/04/2019 12:46, Wei Liu wrote: > On Wed, Apr 17, 2019 at 06:42:47PM +0100, Julien Grall wrote: >> Hi, >> >> @Wei, @Ian: Do you have any input? > > Sorry I haven't been following this closely, but shouldn't we fix the > interface to return gfn instead? And then the mapping of it should > interpret the value properly per guest type. We already return a GFN today. The problem is we only hold an MFN at that time. At the moment, we rely on the M2P to find the corresponding GFN. As we don't have an M2P on Arm, we would have to store the associated GFN. But all this logic is a bit broken. It relies on the toolstack (or the guest) to have mapped the shared info frame in the P2M using the physmap hypervisor with XENMAPSPACE_shared_info beforehand. This is also racy as you may return a GFN that was unmapped/remapped to something else before the toolstack has a chance to map in its address space the page. The correct solution would be to phase out the field shared_info_frame and use XENMEM_acquire_resource instead. However, this requires the associated ioctl to be implemented in Linux. > > I don't see immediately how user space program will need to access this > page for autotranslated guests though. If there are no use, then I am not convinced it is worth trying to make shared_info_frame working on Arm. Cheers,
Hi, @Ian, @Wei: Gentle ping. On 18/04/2019 16:09, Julien Grall wrote: > Hi, > > On 18/04/2019 12:46, Wei Liu wrote: >> On Wed, Apr 17, 2019 at 06:42:47PM +0100, Julien Grall wrote: >>> Hi, >>> >>> @Wei, @Ian: Do you have any input? >> >> Sorry I haven't been following this closely, but shouldn't we fix the >> interface to return gfn instead? And then the mapping of it should >> interpret the value properly per guest type. > > We already return a GFN today. The problem is we only hold an MFN at that time. > > At the moment, we rely on the M2P to find the corresponding GFN. As we don't > have an M2P on Arm, we would have to store the associated GFN. > > But all this logic is a bit broken. It relies on the toolstack (or the guest) to > have mapped the shared info frame in the P2M using the physmap hypervisor with > XENMAPSPACE_shared_info beforehand. This is also racy as you may return a GFN > that was unmapped/remapped to something else before the toolstack has a chance > to map in its address space the page. > > The correct solution would be to phase out the field shared_info_frame and use > XENMEM_acquire_resource instead. However, this requires the associated ioctl to > be implemented in Linux. > >> >> I don't see immediately how user space program will need to access this >> page for autotranslated guests though. > > If there are no use, then I am not convinced it is worth trying to make > shared_info_frame working on Arm. > > Cheers, >
On Thu, Apr 18, 2019 at 04:09:21PM +0100, Julien Grall wrote: > Hi, > > On 18/04/2019 12:46, Wei Liu wrote: > > On Wed, Apr 17, 2019 at 06:42:47PM +0100, Julien Grall wrote: > > > Hi, > > > > > > @Wei, @Ian: Do you have any input? > > > > Sorry I haven't been following this closely, but shouldn't we fix the > > interface to return gfn instead? And then the mapping of it should > > interpret the value properly per guest type. > > We already return a GFN today. The problem is we only hold an MFN at that time. > > At the moment, we rely on the M2P to find the corresponding GFN. As we don't > have an M2P on Arm, we would have to store the associated GFN. > > But all this logic is a bit broken. It relies on the toolstack (or the > guest) to have mapped the shared info frame in the P2M using the physmap > hypervisor with XENMAPSPACE_shared_info beforehand. This is also racy as you > may return a GFN that was unmapped/remapped to something else before the > toolstack has a chance to map in its address space the page. > > The correct solution would be to phase out the field shared_info_frame and > use XENMEM_acquire_resource instead. However, this requires the associated > ioctl to be implemented in Linux. OK. That seems to be a sensible way forward. Wei.
>>> On 16.04.19 at 00:17, <sstabellini@kernel.org> wrote: > On Wed, 13 Mar 2019, Jan Beulich wrote: >> > @@ -215,6 +216,9 @@ void __hwdom_init iommu_hwdom_init(struct domain *d) >> > /* Use while-break to avoid compiler warning */ >> > while ( iommu_iotlb_flush_all(d, flush_flags) ) >> > break; >> > +#else >> > + rc = -ENOSYS; >> >> -EOPNOTSUPP please. > > The patch is fine by me and I am also fine with Jan's comments. > > I only want to point out that we haven't been entirely consistent with > -ENOSYS and/or -EOPNOTSUPP. We have lots of places that return -ENOSYS. > Should we change them to -EOPNOTSUPP going forward? Ideally we would, but iirc a small step I had once made in that direction did face resistance. Hence I've changed to the mode of simply trying to keep new instances of ENOSYS abuse from going in. Jan
diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig index 5c2d1070b6..1892bc3895 100644 --- a/xen/arch/x86/Kconfig +++ b/xen/arch/x86/Kconfig @@ -16,6 +16,7 @@ config X86 select HAS_IOPORTS select HAS_KEXEC select MEM_ACCESS_ALWAYS_ON + select HAS_M2P select HAS_MEM_PAGING select HAS_MEM_SHARING select HAS_NS16550 diff --git a/xen/common/Kconfig b/xen/common/Kconfig index 04384628bb..65c0282e90 100644 --- a/xen/common/Kconfig +++ b/xen/common/Kconfig @@ -52,6 +52,9 @@ config HAS_GDBSX config HAS_IOPORTS bool +config HAS_M2P + bool + config NEEDS_LIBELF bool diff --git a/xen/common/domctl.c b/xen/common/domctl.c index d08b6274e2..ce157e11fe 100644 --- a/xen/common/domctl.c +++ b/xen/common/domctl.c @@ -205,7 +205,7 @@ void getdomaininfo(struct domain *d, struct xen_domctl_getdomaininfo *info) info->outstanding_pages = d->outstanding_pages; info->shr_pages = atomic_read(&d->shr_pages); info->paged_pages = atomic_read(&d->paged_pages); - info->shared_info_frame = mfn_to_gmfn(d, virt_to_mfn(d->shared_info)); + info->shared_info_frame = gfn_x(domain_shared_info_gfn(d)); BUG_ON(SHARED_M2P(info->shared_info_frame)); info->cpupool = d->cpupool ? d->cpupool->cpupool_id : CPUPOOLID_NONE; diff --git a/xen/common/memory.c b/xen/common/memory.c index b6cf09585c..6cbbe4c3c8 100644 --- a/xen/common/memory.c +++ b/xen/common/memory.c @@ -510,6 +510,7 @@ static bool propagate_node(unsigned int xmf, unsigned int *memflags) return true; } +#ifdef CONFIG_M2P static long memory_exchange(XEN_GUEST_HANDLE_PARAM(xen_memory_exchange_t) arg) { struct xen_memory_exchange exch; @@ -802,6 +803,7 @@ static long memory_exchange(XEN_GUEST_HANDLE_PARAM(xen_memory_exchange_t) arg) rc = -EFAULT; return rc; } +#endif int xenmem_add_to_physmap(struct domain *d, struct xen_add_to_physmap *xatp, unsigned int start) @@ -1233,12 +1235,14 @@ long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg) break; +#ifdef CONFIG_M2P case XENMEM_exchange: if ( unlikely(start_extent) ) return -EINVAL; rc = memory_exchange(guest_handle_cast(arg, xen_memory_exchange_t)); break; +#endif case XENMEM_maximum_ram_page: if ( unlikely(start_extent) ) diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c index 5ecaa10bb4..5742cd05b8 100644 --- a/xen/drivers/passthrough/iommu.c +++ b/xen/drivers/passthrough/iommu.c @@ -186,9 +186,10 @@ void __hwdom_init iommu_hwdom_init(struct domain *d) hd->need_sync = iommu_hwdom_strict && !iommu_use_hap_pt(d); if ( need_iommu_pt_sync(d) ) { + int rc = 0; +#ifdef CONFIG_HAS_M2P struct page_info *page; unsigned int i = 0, flush_flags = 0; - int rc = 0; page_list_for_each ( page, &d->page_list ) { @@ -215,6 +216,9 @@ void __hwdom_init iommu_hwdom_init(struct domain *d) /* Use while-break to avoid compiler warning */ while ( iommu_iotlb_flush_all(d, flush_flags) ) break; +#else + rc = -ENOSYS; +#endif if ( rc ) printk(XENLOG_WARNING "d%d: IOMMU mapping failed: %d\n", diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h index 312fec8932..d61b0188da 100644 --- a/xen/include/asm-arm/domain.h +++ b/xen/include/asm-arm/domain.h @@ -267,6 +267,11 @@ static inline void free_vcpu_guest_context(struct vcpu_guest_context *vgc) static inline void arch_vcpu_block(struct vcpu *v) {} +static inline gfn_t domain_shared_info_gfn(struct domain *d) +{ + return INVALID_GFN; +} + #endif /* __ASM_DOMAIN_H__ */ /* diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h index d1bfc82f57..00d8b09794 100644 --- a/xen/include/xen/domain.h +++ b/xen/include/xen/domain.h @@ -118,4 +118,13 @@ struct vnuma_info { void vnuma_destroy(struct vnuma_info *vnuma); +#ifdef CONFIG_HAS_M2P +#define domain_shared_info_gfn(d_) ({ \ + struct domain *d = (d_); \ + \ + _gfn(mfn_to_gfn(d, _mfn(__virt_to_mfn(d->shared_info)))); \ +}) + +#endif + #endif /* __XEN_DOMAIN_H__ */
While Arm never had a M2P, the implementation of mfn_to_gmfn is pretty bogus as we directly return the MFN passed in parameter. Thankfully, the use of mfn_to_gmfn is pretty limited on Arm today. There are only 3 callers: - iommu_hwdom_init: mfn_to_gmfn is used for creating IOMMU page-tables when the P2M is not shared with the IOMMU. No issues so far as Arm does not yet support non-shared P2M case. - memory_exchange: Arm cannot not use it because steal_page is not implemented. - getdomaininfo: Toolstack may map the shared page. It looks like this is mostly used for mapping the P2M of PV guest. Therefore the issue might be minor. Implementing the M2P on Arm is not planned. The M2P would require significant amount of VA address (very tough on 32-bit) that can hardly be justified with the current use of mfn_to_gmfn. - iommu_hwdom_init: mfn_to_gmfn is used because the creating of the IOMMU page-tables is delayed until the first device is assigned. In the embedded case, we will known in most of the times what devices are assigned during the domain creation. So it is possible to take to enable the IOMMU from start. See [1] for the patch. - memory_exchange: This does not work and I haven't seen any request for it so far. - getdomaininfo: The structure on Arm does not seem to contain a lot of useful information on Arm. It is unclear whether we want to allow the toolstack mapping it on Arm. This patch introduces a config option HAS_M2P to tell whether an architecture implements the M2P. - iommu_hwdom_init: For now, we require the M2P support when the IOMMU is not sharing the P2M. - memory_exchange: The hypercall is marked as not supported when the M2P does not exist. - getdomaininfo: A new helper is introduced to wrap the call to mfn_to_gfn/mfn_to_gmfn. For Arm, it returns an invalid GFN so the mapping will fail. [1] https://patchwork.kernel.org/patch/9719913/ Signed-off-by Julien Grall <julien.grall@arm.com> --- Cc: oleksandr_tyshchenko@epam.com Cc: andrii_anisov@epam.com --- xen/arch/x86/Kconfig | 1 + xen/common/Kconfig | 3 +++ xen/common/domctl.c | 2 +- xen/common/memory.c | 4 ++++ xen/drivers/passthrough/iommu.c | 6 +++++- xen/include/asm-arm/domain.h | 5 +++++ xen/include/xen/domain.h | 9 +++++++++ 7 files changed, 28 insertions(+), 2 deletions(-)