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

Reply via email to