On 17.02.2025 11:27, Luca Fancellu wrote:
> When Xen is built without HAS_PASSTHROUGH, there are some parts
> in arm and x86 where iommu_* functions are called in the codebase,
> but their implementation is under xen/drivers/passthrough that is
> not built.
Why the mention of x86, where HAS_PASSTHROUGH is always selected?
> So provide some stub for these functions in order to build Xen
> when !HAS_PASSTHROUGH, which is the case for example on systems
> with MPU support.
Is this fixing a build issue when MPU=y? If so, ...
> For gnttab_need_iommu_mapping() in the Arm part, modify the macro
> to use IS_ENABLED for the HAS_PASSTHROUGH Kconfig.
>
> Signed-off-by: Luca Fancellu <luca.fance...@arm.com>
... is there a Fixes: tag missing?
> --- 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.)
> @@ -381,17 +423,19 @@ struct domain_iommu {
> #define iommu_set_feature(d, f) set_bit(f, dom_iommu(d)->features)
> #define iommu_clear_feature(d, f) clear_bit(f, dom_iommu(d)->features)
>
> +/* Does the IOMMU pagetable need to be kept synchronized with the P2M */
This comment belongs to just ...
> +#ifdef CONFIG_HAS_PASSTHROUGH
> /* Are we using the domain P2M table as its IOMMU pagetable? */
> #define iommu_use_hap_pt(d) (IS_ENABLED(CONFIG_HVM) && \
> dom_iommu(d)->hap_pt_share)
>
> -/* Does the IOMMU pagetable need to be kept synchronized with the P2M */
> -#ifdef CONFIG_HAS_PASSTHROUGH
> #define need_iommu_pt_sync(d) (dom_iommu(d)->need_sync)
... this, not also iommu_use_hap_pt(). There's a connection between the
two, but that is unrelated to what the comment says. I'm also not clear
about the need for ...
> int iommu_do_domctl(struct xen_domctl *domctl, struct domain *d,
> XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl);
> #else
> +#define iommu_use_hap_pt(d) ({ (void)(d); false; })
> +
> #define need_iommu_pt_sync(d) ({ (void)(d); false; })
... this change, i.e. the need for a stub. Afaics there's nothing in the
description to help understanding this need.
Jan
