diff mbox series

[v2,1/2] xen/passthrough: Provide stub functions when !HAS_PASSTHROUGH

Message ID 20250217102732.2343729-2-luca.fancellu@arm.com (mailing list archive)
State Superseded
Headers show
Series Prerequisite patches for Arm64 MPU build | expand

Commit Message

Luca Fancellu Feb. 17, 2025, 10:27 a.m. UTC
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.

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.

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.fancellu@arm.com>
---
v2 Changes:
 - modify gnttab_need_iommu_mapping to use IS_ENABLED
 - removed macro that didn't allow some of the parameter to be
   evaluated
 - Changed commit message
---
---
 xen/arch/arm/include/asm/grant_table.h |  5 +--
 xen/include/xen/iommu.h                | 48 ++++++++++++++++++++++++--
 2 files changed, 49 insertions(+), 4 deletions(-)

Comments

Jan Beulich Feb. 17, 2025, 10:50 a.m. UTC | #1
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.fancellu@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
Luca Fancellu Feb. 17, 2025, 11:55 a.m. UTC | #2
Hi Jan,

thanks for your review,

> On 17 Feb 2025, at 10:50, Jan Beulich <jbeulich@suse.com> wrote:
> 
> 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?

sure, I’ll remove x86

> 
>> 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.fancellu@arm.com>
> 
> ... is there a Fixes: tag missing?

right, I’ll add a tag, but I don’t expect it to be backported, also the MPU will still
have some changes needed before being able to build, should I put a tag even
if this is the case?

> 
>> --- 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)?

> 
>> @@ -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().

I’ll move that close to need_iommu_pt_sync(d)

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

Ok, so in arch/arm/p2m.c the function p2m_set_way_flush() uses this,
so I provided a stub, do you think I should provide something in the
commit message to explain that or shold I try to find another way in order to
don’t provide this stub?

Cheers,
Luca
Luca Fancellu Feb. 17, 2025, 11:58 a.m. UTC | #3
>>> +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)?

Sorry, just a clarification here, I don’t mean return the value of is_iommu_enabled straight away,
but use this to compute the return value of the stub.

Cheers,
Luca
Jan Beulich Feb. 17, 2025, 12:10 p.m. UTC | #4
On 17.02.2025 12:55, Luca Fancellu wrote:
>> On 17 Feb 2025, at 10:50, Jan Beulich <jbeulich@suse.com> wrote:
>> 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?
> 
> sure, I’ll remove x86
> 
>>
>>> 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.fancellu@arm.com>
>>
>> ... is there a Fixes: tag missing?
> 
> right, I’ll add a tag, but I don’t expect it to be backported, also the MPU will still
> have some changes needed before being able to build, should I put a tag even
> if this is the case?

If you're fixing an issue an earlier commit introduced, it's always a
good idea to add a Fixes: tag. That'll also help reviewers and
observers.

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

>>> @@ -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().
> 
> I’ll move that close to need_iommu_pt_sync(d)
> 
>> 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.
> 
> Ok, so in arch/arm/p2m.c the function p2m_set_way_flush() uses this,
> so I provided a stub, do you think I should provide something in the
> commit message to explain that or shold I try to find another way in order to
> don’t provide this stub?

Finding another way would be preferred, but isn't a requirement. Looking
at p2m_set_way_flush() I for one can't figure how page table arrangements
can matter there. Nor can I see how "flush by VA" fits with MPU (where,
aiui, there's no real notion of VA). So yes, if this can't be done
differently to avoid the need for the stub, something will imo want saying
in the description.

Jan
Luca Fancellu Feb. 17, 2025, 4:14 p.m. UTC | #5
> 
>>>> --- 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
Jan Beulich Feb. 17, 2025, 4:27 p.m. UTC | #6
On 17.02.2025 17:14, Luca Fancellu wrote:
>>
>>>>> --- 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’…

Well, xen/sched.h includes xen/iommu.h. Hence when you make the latter
include xen/sched.h, that'll have a meaningful effect on use sites
of xen/iommu.h; wherever xen/sched.h is used the nested #include will
do nothing due to the include guard.

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

Such an assertion then isn't very useful, imo. Since, as you say,
sanitise_domain_config() properly covers the !HAS_PASSTHROUGH case even
for cases like the MPU one, I think the code is fine then. A brief
comment might be nice ...

Jan
diff mbox series

Patch

diff --git a/xen/arch/arm/include/asm/grant_table.h b/xen/arch/arm/include/asm/grant_table.h
index d3c518a926b9..c5d87b60c4df 100644
--- a/xen/arch/arm/include/asm/grant_table.h
+++ b/xen/arch/arm/include/asm/grant_table.h
@@ -73,8 +73,9 @@  int replace_grant_host_mapping(uint64_t gpaddr, mfn_t frame,
 #define gnttab_status_gfn(d, t, i)                                       \
     page_get_xenheap_gfn(gnttab_status_page(t, i))
 
-#define gnttab_need_iommu_mapping(d)                    \
-    (is_domain_direct_mapped(d) && is_iommu_enabled(d))
+#define gnttab_need_iommu_mapping(d)                                     \
+    (IS_ENABLED(CONFIG_HAS_PASSTHROUGH) && is_domain_direct_mapped(d) && \
+     is_iommu_enabled(d))
 
 #endif /* __ASM_GRANT_TABLE_H__ */
 /*
diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
index b928c67e1995..3e49fef8544e 100644
--- 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;
+}
+
+static inline void iommu_hwdom_init(struct domain *d) {}
+
+static inline void iommu_domain_destroy(struct domain *d) {}
+
+#endif /* HAS_PASSTHROUGH */
+
 /*
  * The following flags are passed to map (applicable ones also to unmap)
  * operations, while some are passed back by lookup operations.
@@ -209,6 +229,8 @@  struct msi_msg;
 #ifdef CONFIG_HAS_DEVICE_TREE
 #include <xen/device_tree.h>
 
+#ifdef CONFIG_HAS_PASSTHROUGH
+
 int iommu_assign_dt_device(struct domain *d, struct dt_device_node *dev);
 int iommu_deassign_dt_device(struct domain *d, struct dt_device_node *dev);
 int iommu_dt_domain_init(struct domain *d);
@@ -238,6 +260,26 @@  int iommu_do_dt_domctl(struct xen_domctl *domctl, struct domain *d,
  */
 int iommu_remove_dt_device(struct dt_device_node *np);
 
+#else
+
+static inline int iommu_assign_dt_device(struct domain *d,
+                                         struct dt_device_node *dev)
+{
+    return -EINVAL;
+}
+
+static inline int iommu_add_dt_device(struct dt_device_node *np)
+{
+    return 1;
+}
+
+static inline int iommu_release_dt_devices(struct domain *d)
+{
+    return 0;
+}
+
+#endif /* HAS_PASSTHROUGH */
+
 #endif /* HAS_DEVICE_TREE */
 
 struct page_info;
@@ -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 */
+#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)
 
 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; })
 
 static inline int iommu_do_domctl(struct xen_domctl *domctl, struct domain *d,