Re: [Xen-devel] [PATCH v4 10/12] IOMMU: introduce IOMMU_MIXED config option

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 02.10.18 at 13:00, <julien.grall@xxxxxxx> wrote:
> On 02/10/2018 11:42, Jan Beulich wrote:
>>>>> On 02.10.18 at 12:38, <julien.grall@xxxxxxx> wrote:
>>> On 02/10/2018 11:18, Jan Beulich wrote:
>>>> ARM is intended to gain support for heterogeneous IOMMUs on a single
>>>> system. This not only disallows boot time replacement of respective
>>>> indirect calls (handling of which is the main goal of the introduction
>>>> here), but more generally disallows calls using the iommu_ops() return
>>>> value directly - all such calls need to have means (commonly a domain
>>>> pointer) to know the targeted IOMMU.
>>>>
>>>> Disallow all hooks lacking such context for the time being, which in
>>>> effect is some dead code elimination for ARM. Once extended suitably,
>>>> individual of these hooks can be moved out of their guards again in the
>>>> future.
>>>
>>> While in theory it is possible to have platform with hetereneous IOMMUs.
>>>    I don't see such such support coming in Xen for the foreseeable
>>> future. Note that even Linux does not support such case.
>>>
>>> This patch is going to make more complicate to unshare page-tables as
>>> now we would need to care of mixed case. So I would rather not set
>>> IOMMU_MIXED on Arm until we have a use case for it.
>> 
>> Interesting. I essence this is the exact opposite of what you've
>> told me when I inquired about indirect call patching of the IOMMU
>> code. The sole purpose of this new option is to have a key off of
>> which I can tell whether to use patchable indirect calls or plain
>> ones.
> 
> I don't think I am saying the opposite. I opened the door for mixed use 
> case. It does not mean, I want to see half of the helpers dropped on Arm 
> because you want them to be patchable. There are other way to do it.

I drop no helpers (or really hooks), I merely hide ones which can't
(currently) be used in mixed-IOMMU environments. And the
mixed-IOMMU case was what you've used to request that indirect
calls here not be patched for ARM.

>> I'm also not following how this change would complicate anything
>> for you. There's effectively no change for ARM, except for some
>> dead code not getting built anymore.
> 
> Well, with your patch free_page_table, resume & co are under 
> !IOMMU_MIXED. So they are unusable on Arm until we effectively rework
> the code to handle mixed case. More likely those callback will be 
> necessary on Arm before mixed case. So I don't think this patch as such 
> is what we want for Arm at the moment.
> 
> I would much prefer if you drop IOMMU_MIXED and implement iommu_{v,}call 
> on Arm as a iommu_ops->fun(...) (i.e no alternative for now). So call 
> are still not patchable for Arm, yet we still have access to all IOMMU 
> functions.

Well, that's what I would have done if you hadn't brought up the
mixed-IOMMU case - clearly, without being able to test, I wouldn't
have dared to try and implement patching of indirect calls for ARM.

I can certainly drop that patch, but in the end it means you've
made me do more work than would have been needed to reach
the immediate goal I have.

Jan



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