On 25/02/2019 10:59, Jan Beulich wrote: >>>> On 25.02.19 at 11:02, <paul.durr...@citrix.com> wrote: >>> From: Andrew Cooper [mailto:andrew.coop...@citrix.com] >>> Sent: 22 February 2019 19:13 >>> >>> --- a/xen/drivers/passthrough/vtd/qinval.c >>> +++ b/xen/drivers/passthrough/vtd/qinval.c >>> @@ -84,7 +84,7 @@ static int __must_check >>> queue_invalidate_context_sync(struct vtd_iommu *iommu, >>> >>> spin_lock_irqsave(&iommu->register_lock, flags); >>> index = qinval_next_index(iommu); >>> - entry_base = iommu_qi_ctrl(iommu)->qinval_maddr + >>> + entry_base = iommu->qinval_maddr + >>> ((index >> QINVAL_ENTRY_ORDER) << PAGE_SHIFT); >> ^ This calculation looks worthy of a macro or an inline. It is repeated a >> lot. > And indeed the other day I was surprised that there is > GET_IREMAP_ENTRY(), but no qinval equivalent.
I won't be making any changes like that to this patch. There is an orthogonal piece of work to vmap these structures and replace all of this logic with a straight array lookup by index. Unfortunately we don't have memremap() in Xen yet and I don't have time right now to sort that. GET_IREMAP_ENTRY() is a particularly bad example of a helper, as results in several necessary pieces of calculation in certain cases, and hides a use of map_domain_page() which results in the logic which uses it looking asymmetric. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel