[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 09/14] xen: Introduce HAS_M2P config and use to protect mfn_to_gmfn call
>>> On 07.05.19 at 17:14, <julien.grall@xxxxxxx> wrote: > 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. Was it you or someone else to suggest it be restricted to non-translated guests in the first place? I'd prefer this over the #ifdef-ary you add. > - 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. There's no use of mfn_to_gmfn() in either of the wrappers, so why mention this to-be-removed one? > --- 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)); What is the intended behavior on 32-bit Arm here? Do you really mean to return a value with 32 bits of ones (instead of 64 bits of them) in this 64-bit field? > --- a/xen/drivers/passthrough/iommu.c > +++ b/xen/drivers/passthrough/iommu.c > @@ -188,9 +188,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 ) > { > @@ -217,6 +218,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 = -EOPNOTSUPP; > +#endif > > if ( rc ) > printk(XENLOG_WARNING "d%d: IOMMU mapping failed: %d\n", Would you mind extending the scope of the #ifdef beyond this printk()? It seems pretty pointless to me to unconditionally emit a log message here. > --- a/xen/include/public/domctl.h > +++ b/xen/include/public/domctl.h > @@ -118,6 +118,10 @@ struct xen_domctl_getdomaininfo { > uint64_aligned_t outstanding_pages; > uint64_aligned_t shr_pages; > uint64_aligned_t paged_pages; > + /* > + * GFN of shared_info struct. Some architectures (e.g Arm) may not > + * provide a valid GFN. > + */ Along the lines of what I've said above, I think you want to spell out here what the value is going to be in this case. > --- a/xen/include/xen/domain.h > +++ b/xen/include/xen/domain.h > @@ -118,4 +118,12 @@ struct vnuma_info { > > void vnuma_destroy(struct vnuma_info *vnuma); > > +#ifdef CONFIG_HAS_M2P > +#define domain_shared_info_gfn(d) ({ \ > + const struct domain *d_ = (d); \ > + \ > + mfn_to_gfn(d_, _mfn(__virt_to_mfn(d_->shared_info))); \ > +}) > +#endif And an inline function doesn't work here? Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |