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

[Oleksandr Tyshchenko <olekstysh@xxxxxxxxx>]

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().
Am I right?

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 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.

-- 
Regards,

Oleksandr Tyshchenko

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