Re: [Xen-devel] [PATCH 3/6] x86/vtd: Drop struct qi_ctrl

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 25.02.19 at 12:07, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 25/02/2019 10:59, Jan Beulich wrote:
>>>>> On 25.02.19 at 11:02, <Paul.Durrant@xxxxxxxxxx> wrote:
>>>> From: Andrew Cooper [mailto:andrew.cooper3@xxxxxxxxxx]
>>>> 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.

Oh, I'm sorry if this came over in an ambiguous way: I didn't
mean to suggest to directly mirror that strange macro. I've
just used its existence to support Paul's request.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel