[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



On Fri, Mar 24, 2017 at 1:38 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>> 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.

OK.

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

Now I got it, but I don't have the right answer at the moment.

-- 
Regards,

Oleksandr Tyshchenko

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.