>
>>>> --- a/xen/include/xen/iommu.h
>>>> +++ b/xen/include/xen/iommu.h
>>>> @@ -110,6 +110,8 @@ extern int8_t iommu_hwdom_reserved;
>>>>
>>>> extern unsigned int iommu_dev_iotlb_timeout;
>>>>
>>>> +#ifdef CONFIG_HAS_PASSTHROUGH
>>>> +
>>>> int iommu_setup(void);
>>>> int iommu_hardware_setup(void);
>>>>
>>>> @@ -122,6 +124,24 @@ int arch_iommu_domain_init(struct domain *d);
>>>> void arch_iommu_check_autotranslated_hwdom(struct domain *d);
>>>> void arch_iommu_hwdom_init(struct domain *d);
>>>>
>>>> +#else
>>>> +
>>>> +static inline int iommu_setup(void)
>>>> +{
>>>> + return -ENODEV;
>>>> +}
>>>> +
>>>> +static inline int iommu_domain_init(struct domain *d, unsigned int opts)
>>>> +{
>>>> + return 0;
>>>
>>> Shouldn't this fail when is_iommu_enabled(d) is true? (The use of the
>>> predicate here as well as in the real function is slightly strange, but
>>> that's the way it is.)
>>
>> Right, probably you know better this code than me, I started from the
>> assumption
>> that when !HAS_PASSTHROUGH, 'iommu_enabled' is false.
>>
>> is_iommu_enabled(d) checks if the domain structure ‘options’ field has
>> XEN_DOMCTL_CDF_iommu, this flag is set on domain creation when
>> ‘iommu_enabled'
>> is true on arm and x86.
>>
>> So when !HAS_PASSTHROUGH can we assume is_iommu_enabled(d) give false?
>> Or shall we return for example the value of is_iommu_enabled(d)?
>
> Since HAS_PASSTHROUGH being selected conditionally a (pretty) new, I
> fear that assumptions shouldn't be made. It's possible the stub could
> remain as is, yet even then - if only for documentation purposes - I'd
> suggest to have some ASSERT() there. In the end it all depends on how
> XEN_DOMCTL_CDF_iommu is handled when !HAS_PASSTHROUGH.
I’ve tried to add an ASSERT(!is_iommu_enabled(d)); but it’s not building, I’m
starting to think there
is some reason why I can’t do that but I didn’t figure out why, I’ve added the
inclusion for xen/sched.h,
but it still says implicit declaration of function ‘is_iommu_enabled’…
But I could assert for !iommu_enabled: I checked into common/domain.c,
sanitise_domain_config,
if a domain is called with XEN_DOMCTL_CDF_iommu set, the function would fail if
!iommu_enabled,
so I would say that the stub returns the expected value (0) since for sure
iommu_enabled is false and
there cannot be a domain with that flag set that has the iommu_enabled=true
under !HAS_PASSTHROUGH.
But would it be ok to add this assert (ASSERT(!iommu_enabled);) even if we know
that iommu_enabled
is false, since !HAS_PASSTHROUGH ?
Please let me know your thoughts on this.
Cheers,
Luca
