Re: [Xen-devel] [RFC PATCH 9/9] xen: Add use_iommu flag to createdomain domctl

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 24.03.17 at 12:19, <olekstysh@xxxxxxxxx> wrote:
> Hi Jan
> 
> On Thu, Mar 23, 2017 at 7:05 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>>> On 23.03.17 at 17:36, <olekstysh@xxxxxxxxx> wrote:
>>> On Wed, Mar 22, 2017 at 5:56 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>>>>> On 15.03.17 at 21:05, <olekstysh@xxxxxxxxx> wrote:
>>>>> --- a/xen/include/public/domctl.h
>>>>> +++ b/xen/include/public/domctl.h
>>>>> @@ -66,6 +66,9 @@ struct xen_domctl_createdomain {
>>>>>   /* Is this a xenstore domain? */
>>>>>  #define _XEN_DOMCTL_CDF_xs_domain     5
>>>>>  #define XEN_DOMCTL_CDF_xs_domain      (1U<<_XEN_DOMCTL_CDF_xs_domain)
>>>>> + /* Should IOMMU page tables be populated at the domain creation time? */
>>>>> +#define _XEN_DOMCTL_CDF_use_iommu     6
>>>>> +#define XEN_DOMCTL_CDF_use_iommu      (1U<<_XEN_DOMCTL_CDF_use_iommu)
>>>>>      uint32_t flags;
>>>>
>>>> The need for this to be done via domain creation flag (rather than
>>>> as a separate, later step) needs to be well explained. Generally
>>>> what to add here should only be things which can't be done later
>>>> in a reasonable way.
>>>
>>> Well, the non-shared IOMMU should populate its page table by the time
>>> the P2M code will have started update mappings. Theoretically, it
>>> might happen right after p2m_init has been completed,
>>> that is, even during createdomain domctl execution. For example, I see
>>> that domain_vgic_init() inserts mapping to P2M table, because of
>>> map_mmio_regions() is being called during VGIC initialization.
>>> Not sure that GIC mmio ranges must be present in the IOMMU page table,
>>> but anyway, it might be the per-domain initialization of other IPs,
>>> co-processors that mapping mustn't be skipped because of IOMMU is not
>>> ready.
>>> So, as both iommu_domain_init() and p2m_init() are called from
>>> arch_domain_create(), i.e. during createdomain domctl execution, we
>>> have to know exactly should we use IOMMU for this domain to pass
>>> proper value to iommu_domain_init().
>>
>> Well, no, iommu_domain_init() is not supposed to do any table
>> setup, so it shouldn't need to know.
> 
> So, does this mean that you disagree to what this patch does as well
> as the preceding patch
> [RFC PATCH 6/9] iommu: Pass additional use_iommu argument to
> iommu_domain_init().

Yes.

> As we need this use_iommu flag on ARM only, the possible solution
> might be to hide it in struct xen_arch_domainconfig for ARM.
> In such case we would always call iommu_domain_init() with use_iommu
> forced to false on x86.

If that won't involve a domain creation flag addition anymore,
I'd be fine and leave this to the ARM maintainers.

>>> If don't take into account everything I wrote above, yes, it is
>>> possible to introduce new domctl for this purpose that will be called
>>> later, but before any operations with guest_physmap.
>>
>> Exactly. Any other things needing syncing, but being done during
>> domain creation may then need syncing over. One might question
>> whether some of those things then actually are being done too
>> early (and quite possibly have been done that way just for simplicity).
> 
> Sorry, I'm afraid I don't entirely understand you here.

You had mentioned a couple of things where you think you need
to know ahead of the time whether IOMMU use will actually be
needed. In turn I question whether these things can't be done
later, i.e. whether they're being done in their current ordering
just for convenience of the original code authors.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel