[v6,07/10] use is_iommu_enabled() where appropriate...

Commit Message

Paul Durrant Aug. 16, 2019, 5:19 p.m. UTC

...rather than testing the global iommu_enabled flag and ops pointer.

Now that there is a per-domain flag indicating whether the domain is
permitted to use the IOMMU (which determines whether the ops pointer will
be set), many tests of the global iommu_enabled flag and ops pointer can
be translated into tests of the per-domain flag. Some of the other tests of
purely the global iommu_enabled flag can also be translated into tests of
the per-domain flag.

NOTE: The comment in iommu_share_p2m_table() is also fixed; need_iommu()
      disappeared some time ago. Also, whilst the style of the 'if' in
      flask_iommu_resource_use_perm() is fixed, I have not translated any
      instances of u32 into uint32_t to keep consistency. IMO such a
      translation would be better done globally for the source module in
      a separate patch.
      The change in the initialization of the 'hd' variable in iommu_map()
      and iommu_unmap() is done to keep the PV shim build happy. Without
      this change it will fail to compile with errors of the form:

iommu.c:361:32: error: unused variable ‘hd’ [-Werror=unused-variable]
     const struct domain_iommu *hd = dom_iommu(d);
                                     ^~

Signed-off-by: Paul Durrant <[e-mail]>
---
Cc: Stefano Stabellini <[e-mail]>
Cc: Julien Grall <[e-mail]>
Cc: Volodymyr Babchuk <[e-mail]>
Cc: Jan Beulich <[e-mail]>
Cc: Andrew Cooper <[e-mail]>
Cc: Wei Liu <[e-mail]>
Cc: "Roger Pau Monné" <[e-mail]>
Cc: Jun Nakajima <[e-mail]>
Cc: Kevin Tian <[e-mail]>
Cc: George Dunlap <[e-mail]>
Cc: Suravee Suthikulpanit <[e-mail]>
Cc: Brian Woods <[e-mail]>
Cc: Daniel De Graaf <[e-mail]>

Previously part of series https://lists.xenproject.org/archives/html/xen-devel/2019-07/msg02267.html

v5:
 - Fix logic in ARM p2m_init()
 - Make iommu_do_domctl() return -EOPNOTSUPP rather than -ENOSYS if the
   IOMMU is not enabled
 - Fix test in pci_enable_acs()
 - Fix test in flask_iommu_resource_use_perm()
---
 xen/arch/arm/p2m.c                        |  2 +-
 xen/arch/x86/dom0_build.c                 |  2 +-
 xen/arch/x86/domctl.c                     |  4 +--
 xen/arch/x86/hvm/hvm.c                    |  6 ++--
 xen/arch/x86/hvm/vioapic.c                |  2 +-
 xen/arch/x86/hvm/vmx/vmcs.c               |  2 +-
 xen/arch/x86/hvm/vmx/vmx.c                |  2 +-
 xen/arch/x86/mm/p2m-ept.c                 |  4 +--
 xen/drivers/passthrough/amd/iommu_guest.c |  2 +-
 xen/drivers/passthrough/device_tree.c     |  4 +--
 xen/drivers/passthrough/io.c              |  8 ++---
 xen/drivers/passthrough/iommu.c           | 39 ++++++++++++-----------
 xen/drivers/passthrough/pci.c             | 16 +++++-----
 xen/drivers/passthrough/vtd/iommu.c       |  2 +-
 xen/drivers/passthrough/vtd/x86/hvm.c     |  2 +-
 xen/drivers/passthrough/x86/iommu.c       |  2 +-
 xen/xsm/flask/hooks.c                     | 18 +++++------
 17 files changed, 59 insertions(+), 58 deletions(-)

Comments

Daniel De Graaf Aug. 19, 2019, 8:55 p.m. UTC | #1

On 8/16/19 1:19 PM, Paul Durrant wrote:
> ...rather than testing the global iommu_enabled flag and ops pointer.
> 
> Now that there is a per-domain flag indicating whether the domain is
> permitted to use the IOMMU (which determines whether the ops pointer will
> be set), many tests of the global iommu_enabled flag and ops pointer can
> be translated into tests of the per-domain flag. Some of the other tests of
> purely the global iommu_enabled flag can also be translated into tests of
> the per-domain flag.
> 
> NOTE: The comment in iommu_share_p2m_table() is also fixed; need_iommu()
>        disappeared some time ago. Also, whilst the style of the 'if' in
>        flask_iommu_resource_use_perm() is fixed, I have not translated any
>        instances of u32 into uint32_t to keep consistency. IMO such a
>        translation would be better done globally for the source module in
>        a separate patch.
>        The change in the initialization of the 'hd' variable in iommu_map()
>        and iommu_unmap() is done to keep the PV shim build happy. Without
>        this change it will fail to compile with errors of the form:
> 
> iommu.c:361:32: error: unused variable ‘hd’ [-Werror=unused-variable]
>       const struct domain_iommu *hd = dom_iommu(d);
>                                       ^~
> 
> Signed-off-by: Paul Durrant <[e-mail]>

Acked-by: Daniel De Graaf <[e-mail]>

Tian, Kevin Aug. 23, 2019, 3:04 a.m. UTC | #2

> From: Paul Durrant [mailto:[e-mail]]
> Sent: Saturday, August 17, 2019 1:20 AM
> 
> ...rather than testing the global iommu_enabled flag and ops pointer.
> 
> Now that there is a per-domain flag indicating whether the domain is
> permitted to use the IOMMU (which determines whether the ops pointer will
> be set), many tests of the global iommu_enabled flag and ops pointer can
> be translated into tests of the per-domain flag. Some of the other tests of
> purely the global iommu_enabled flag can also be translated into tests of
> the per-domain flag.
> 
> NOTE: The comment in iommu_share_p2m_table() is also fixed;
> need_iommu()
>       disappeared some time ago. Also, whilst the style of the 'if' in
>       flask_iommu_resource_use_perm() is fixed, I have not translated any
>       instances of u32 into uint32_t to keep consistency. IMO such a
>       translation would be better done globally for the source module in
>       a separate patch.
>       The change in the initialization of the 'hd' variable in iommu_map()
>       and iommu_unmap() is done to keep the PV shim build happy. Without
>       this change it will fail to compile with errors of the form:
> 
> iommu.c:361:32: error: unused variable ‘hd’ [-Werror=unused-variable]
>      const struct domain_iommu *hd = dom_iommu(d);
>                                      ^~
> 
> Signed-off-by: Paul Durrant <[e-mail]>

Reviewed-by: Kevin Tian <[e-mail]>

Roger Pau Monné Aug. 23, 2019, 10:55 a.m. UTC | #3

On Fri, Aug 16, 2019 at 06:19:58PM +0100, Paul Durrant wrote:
> ...rather than testing the global iommu_enabled flag and ops pointer.
> 
> Now that there is a per-domain flag indicating whether the domain is
> permitted to use the IOMMU (which determines whether the ops pointer will
> be set), many tests of the global iommu_enabled flag and ops pointer can
> be translated into tests of the per-domain flag. Some of the other tests of
> purely the global iommu_enabled flag can also be translated into tests of
> the per-domain flag.
> 
> NOTE: The comment in iommu_share_p2m_table() is also fixed; need_iommu()
>       disappeared some time ago. Also, whilst the style of the 'if' in
>       flask_iommu_resource_use_perm() is fixed, I have not translated any
>       instances of u32 into uint32_t to keep consistency. IMO such a
>       translation would be better done globally for the source module in
>       a separate patch.

I've usually changed those instances as the line gets modified anyway,
but I'm not going to ask everyone to do this if they don't feel like
it.

The problem with doing wholesale changes of u32 -> uint32_t is that
you introduce a lot of noise when trying to use git blame afterwards
for example. Doing it when touching the line for legitimate changes
avoids the noise.

>       The change in the initialization of the 'hd' variable in iommu_map()
>       and iommu_unmap() is done to keep the PV shim build happy. Without
>       this change it will fail to compile with errors of the form:
> 
> iommu.c:361:32: error: unused variable ‘hd’ [-Werror=unused-variable]
>      const struct domain_iommu *hd = dom_iommu(d);
>                                      ^~
> 
> Signed-off-by: Paul Durrant <[e-mail]>

Reviewed-by: Roger Pau Monné <[e-mail]>

I haven't looked however if there are further cases where
iommu_enabled should be changed into is_iommu_enabled.

> @@ -556,8 +560,8 @@ int iommu_do_domctl(
>  {
>      int ret = -ENODEV;
>  
> -    if ( !iommu_enabled )
> -        return -ENOSYS;
> +    if ( !is_iommu_enabled(d) )
> +        return -EOPNOTSUPP;

I would use ENOENT here, but I don't have a strong opinion. The
hypercall is supported, it's just that there's no iommu for this
domain.

Thanks, Roger.

Paul Durrant Aug. 29, 2019, 9:17 a.m. UTC | #4

> -----Original Message-----
> From: Roger Pau Monne <[e-mail]>
> Sent: 23 August 2019 11:56
> To: Paul Durrant <[e-mail]>
> Cc: [e-mail]; Stefano Stabellini <[e-mail]>; Julien Grall
> <[e-mail]>; Volodymyr Babchuk <[e-mail]>; Jan Beulich
> <[e-mail]>; Andrew Cooper <[e-mail]>; Wei Liu <[e-mail]>; Jun Nakajima
> <[e-mail]>; Kevin Tian <[e-mail]>; George Dunlap <[e-mail]>;
> Suravee Suthikulpanit <[e-mail]>; Brian Woods <[e-mail]>; Daniel De
> Graaf <[e-mail]>
> Subject: Re: [PATCH v6 07/10] use is_iommu_enabled() where appropriate...
> 
> On Fri, Aug 16, 2019 at 06:19:58PM +0100, Paul Durrant wrote:
> > ...rather than testing the global iommu_enabled flag and ops pointer.
> >
> > Now that there is a per-domain flag indicating whether the domain is
> > permitted to use the IOMMU (which determines whether the ops pointer will
> > be set), many tests of the global iommu_enabled flag and ops pointer can
> > be translated into tests of the per-domain flag. Some of the other tests of
> > purely the global iommu_enabled flag can also be translated into tests of
> > the per-domain flag.
> >
> > NOTE: The comment in iommu_share_p2m_table() is also fixed; need_iommu()
> >       disappeared some time ago. Also, whilst the style of the 'if' in
> >       flask_iommu_resource_use_perm() is fixed, I have not translated any
> >       instances of u32 into uint32_t to keep consistency. IMO such a
> >       translation would be better done globally for the source module in
> >       a separate patch.
> 
> I've usually changed those instances as the line gets modified anyway,
> but I'm not going to ask everyone to do this if they don't feel like
> it.
> 
> The problem with doing wholesale changes of u32 -> uint32_t is that
> you introduce a lot of noise when trying to use git blame afterwards
> for example. Doing it when touching the line for legitimate changes
> avoids the noise.
> 
> >       The change in the initialization of the 'hd' variable in iommu_map()
> >       and iommu_unmap() is done to keep the PV shim build happy. Without
> >       this change it will fail to compile with errors of the form:
> >
> > iommu.c:361:32: error: unused variable ‘hd’ [-Werror=unused-variable]
> >      const struct domain_iommu *hd = dom_iommu(d);
> >                                      ^~
> >
> > Signed-off-by: Paul Durrant <[e-mail]>
> 
> Reviewed-by: Roger Pau Monné <[e-mail]>
> 

Thanks.

> I haven't looked however if there are further cases where
> iommu_enabled should be changed into is_iommu_enabled.
> 

Jan had clearly checked on a previous review iteration because he spotted a couple of places I had missed. I'm reasonably confident I've found them all now.

  Paul

> > @@ -556,8 +560,8 @@ int iommu_do_domctl(
> >  {
> >      int ret = -ENODEV;
> >
> > -    if ( !iommu_enabled )
> > -        return -ENOSYS;
> > +    if ( !is_iommu_enabled(d) )
> > +        return -EOPNOTSUPP;
> 
> I would use ENOENT here, but I don't have a strong opinion. The
> hypercall is supported, it's just that there's no iommu for this
> domain.
> 
> Thanks, Roger.

Jan Beulich Aug. 29, 2019, 1:29 p.m. UTC | #5

On 16.08.2019 19:19, Paul Durrant wrote:
> ...rather than testing the global iommu_enabled flag and ops pointer.
> 
> Now that there is a per-domain flag indicating whether the domain is
> permitted to use the IOMMU (which determines whether the ops pointer will
> be set), many tests of the global iommu_enabled flag and ops pointer can
> be translated into tests of the per-domain flag. Some of the other tests of
> purely the global iommu_enabled flag can also be translated into tests of
> the per-domain flag.
> 
> NOTE: The comment in iommu_share_p2m_table() is also fixed; need_iommu()
>       disappeared some time ago. Also, whilst the style of the 'if' in
>       flask_iommu_resource_use_perm() is fixed, I have not translated any
>       instances of u32 into uint32_t to keep consistency. IMO such a
>       translation would be better done globally for the source module in
>       a separate patch.
>       The change in the initialization of the 'hd' variable in iommu_map()
>       and iommu_unmap() is done to keep the PV shim build happy. Without
>       this change it will fail to compile with errors of the form:
> 
> iommu.c:361:32: error: unused variable ‘hd’ [-Werror=unused-variable]
>      const struct domain_iommu *hd = dom_iommu(d);
>                                      ^~

Why would that be? Afaict there's no short-circuiting of
is_iommu_enabled() (albeit there could be, as we don't mean the shim
to have/use an IOMMU). Oh - I guess I see it: Instead of this change
what I think we want is for x86's iommu_call() to evaluate its 1st
argument. Otherwise we're liable to run into the same issue elsewhere
again.

> --- a/xen/xsm/flask/hooks.c
> +++ b/xen/xsm/flask/hooks.c
> @@ -886,7 +886,7 @@ static int flask_map_domain_msi (struct domain *d, int irq, const void *data,
>  #endif
>  }
>  
> -static u32 flask_iommu_resource_use_perm(void)
> +static u32 flask_iommu_resource_use_perm(struct domain *d)

const?

With these adjustments
Reviewed-by: Jan Beulich <[e-mail]>

Jan

Patch

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index e28ea1c85a..7f1442932a 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1531,7 +1531,7 @@  int p2m_init(struct domain *d)
      * shared with the CPU, Xen has to make sure that the PT changes have
      * reached the memory
      */
-    p2m->clean_pte = iommu_enabled &&
+    p2m->clean_pte = is_iommu_enabled(d) &&
         !iommu_has_feature(d, IOMMU_FEAT_COHERENT_WALK);
 
     rc = p2m_alloc_table(d);
diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
index c69570920c..d381784edd 100644
--- a/xen/arch/x86/dom0_build.c
+++ b/xen/arch/x86/dom0_build.c
@@ -356,7 +356,7 @@  unsigned long __init dom0_compute_nr_pages(
         avail -= d->max_vcpus - 1;
 
     /* Reserve memory for iommu_dom0_init() (rough estimate). */
-    if ( iommu_enabled )
+    if ( is_iommu_enabled(d) )
     {
         unsigned int s;
 
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 2d45e5b8a8..be4b206068 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -715,7 +715,7 @@  long arch_do_domctl(
             break;
 
         ret = -ESRCH;
-        if ( iommu_enabled )
+        if ( is_iommu_enabled(d) )
         {
             pcidevs_lock();
             ret = pt_irq_create_bind(d, bind);
@@ -744,7 +744,7 @@  long arch_do_domctl(
         if ( ret )
             break;
 
-        if ( iommu_enabled )
+        if ( is_iommu_enabled(d) )
         {
             pcidevs_lock();
             ret = pt_irq_destroy_bind(d, bind);
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 029eea3b85..172c860acc 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -465,7 +465,7 @@  void hvm_migrate_timers(struct vcpu *v)
 
 void hvm_migrate_pirq(struct hvm_pirq_dpci *pirq_dpci, const struct vcpu *v)
 {
-    ASSERT(iommu_enabled &&
+    ASSERT(is_iommu_enabled(v->domain) &&
            (is_hardware_domain(v->domain) || hvm_domain_irq(v->domain)->dpci));
 
     if ( (pirq_dpci->flags & HVM_IRQ_DPCI_MACH_MSI) &&
@@ -496,7 +496,7 @@  void hvm_migrate_pirqs(struct vcpu *v)
 {
     struct domain *d = v->domain;
 
-    if ( !iommu_enabled || !hvm_domain_irq(d)->dpci )
+    if ( !is_iommu_enabled(d) || !hvm_domain_irq(d)->dpci )
        return;
 
     spin_lock(&d->event_lock);
@@ -2264,7 +2264,7 @@  int hvm_set_cr0(unsigned long value, bool may_defer)
     }
 
     if ( ((value ^ old_value) & X86_CR0_CD) &&
-         iommu_enabled && hvm_funcs.handle_cd &&
+         is_iommu_enabled(d) && hvm_funcs.handle_cd &&
          (!rangeset_is_empty(d->iomem_caps) ||
           !rangeset_is_empty(d->arch.ioport_caps) ||
           has_arch_pdevs(d)) )
diff --git a/xen/arch/x86/hvm/vioapic.c b/xen/arch/x86/hvm/vioapic.c
index 9c25f72b4d..9aeef32a14 100644
--- a/xen/arch/x86/hvm/vioapic.c
+++ b/xen/arch/x86/hvm/vioapic.c
@@ -536,7 +536,7 @@  void vioapic_update_EOI(struct domain *d, u8 vector)
 
             ent->fields.remote_irr = 0;
 
-            if ( iommu_enabled )
+            if ( is_iommu_enabled(d) )
             {
                 spin_unlock(&d->arch.hvm.irq_lock);
                 hvm_dpci_eoi(d, vioapic->base_gsi + pin, ent);
diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
index 16f14abe8f..ed27e8def7 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -1087,7 +1087,7 @@  static int construct_vmcs(struct vcpu *v)
         vmx_clear_msr_intercept(v, MSR_IA32_SYSENTER_CS, VMX_MSR_RW);
         vmx_clear_msr_intercept(v, MSR_IA32_SYSENTER_ESP, VMX_MSR_RW);
         vmx_clear_msr_intercept(v, MSR_IA32_SYSENTER_EIP, VMX_MSR_RW);
-        if ( paging_mode_hap(d) && (!iommu_enabled || iommu_snoop) )
+        if ( paging_mode_hap(d) && (!is_iommu_enabled(d) || iommu_snoop) )
             vmx_clear_msr_intercept(v, MSR_IA32_CR_PAT, VMX_MSR_RW);
         if ( (vmexit_ctl & VM_EXIT_CLEAR_BNDCFGS) &&
              (vmentry_ctl & VM_ENTRY_LOAD_BNDCFGS) )
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 0060310d74..3b3d5b6250 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -1210,7 +1210,7 @@  static void vmx_handle_cd(struct vcpu *v, unsigned long value)
         {
             v->arch.hvm.cache_mode = NORMAL_CACHE_MODE;
             vmx_set_guest_pat(v, *pat);
-            if ( !iommu_enabled || iommu_snoop )
+            if ( !is_iommu_enabled(v->domain) || iommu_snoop )
                 vmx_clear_msr_intercept(v, MSR_IA32_CR_PAT, VMX_MSR_RW);
             hvm_asid_flush_vcpu(v); /* no need to flush cache */
         }
diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c
index 6b8468c793..93d031cc6c 100644
--- a/xen/arch/x86/mm/p2m-ept.c
+++ b/xen/arch/x86/mm/p2m-ept.c
@@ -260,7 +260,7 @@  static bool_t ept_split_super_page(struct p2m_domain *p2m,
         *epte = *ept_entry;
         epte->sp = (level > 1);
         epte->mfn += i * trunk;
-        epte->snp = (iommu_enabled && iommu_snoop);
+        epte->snp = is_iommu_enabled(p2m->domain) && iommu_snoop;
         epte->suppress_ve = 1;
 
         ept_p2m_type_to_flags(p2m, epte, epte->sa_p2mt, epte->access);
@@ -766,7 +766,7 @@  ept_set_entry(struct p2m_domain *p2m, gfn_t gfn_, mfn_t mfn,
         new_entry.sp = !!i;
         new_entry.sa_p2mt = p2mt;
         new_entry.access = p2ma;
-        new_entry.snp = (iommu_enabled && iommu_snoop);
+        new_entry.snp = is_iommu_enabled(d) && iommu_snoop;
 
         /* the caller should take care of the previous page */
         new_entry.mfn = mfn_x(mfn);
diff --git a/xen/drivers/passthrough/amd/iommu_guest.c b/xen/drivers/passthrough/amd/iommu_guest.c
index 7f2dd662af..1f2bcfbe15 100644
--- a/xen/drivers/passthrough/amd/iommu_guest.c
+++ b/xen/drivers/passthrough/amd/iommu_guest.c
@@ -821,7 +821,7 @@  int guest_iommu_init(struct domain* d)
     struct guest_iommu *iommu;
     struct domain_iommu *hd = dom_iommu(d);
 
-    if ( !is_hvm_domain(d) || !iommu_enabled || !iommuv2_enabled ||
+    if ( !is_hvm_domain(d) || !is_iommu_enabled(d) || !iommuv2_enabled ||
          !has_viommu(d) )
         return 0;
 
diff --git a/xen/drivers/passthrough/device_tree.c b/xen/drivers/passthrough/device_tree.c
index d32b172664..12f2c4c3f2 100644
--- a/xen/drivers/passthrough/device_tree.c
+++ b/xen/drivers/passthrough/device_tree.c
@@ -29,7 +29,7 @@  int iommu_assign_dt_device(struct domain *d, struct dt_device_node *dev)
     int rc = -EBUSY;
     struct domain_iommu *hd = dom_iommu(d);
 
-    if ( !iommu_enabled || !hd->platform_ops )
+    if ( !is_iommu_enabled(d) )
         return -EINVAL;
 
     if ( !dt_device_is_protected(dev) )
@@ -71,7 +71,7 @@  int iommu_deassign_dt_device(struct domain *d, struct dt_device_node *dev)
     const struct domain_iommu *hd = dom_iommu(d);
     int rc;
 
-    if ( !iommu_enabled || !hd->platform_ops )
+    if ( !is_iommu_enabled(d) )
         return -EINVAL;
 
     if ( !dt_device_is_protected(dev) )
diff --git a/xen/drivers/passthrough/io.c b/xen/drivers/passthrough/io.c
index 4290c7c710..b292e79382 100644
--- a/xen/drivers/passthrough/io.c
+++ b/xen/drivers/passthrough/io.c
@@ -434,7 +434,7 @@  int pt_irq_create_bind(
             if ( vcpu )
                 pirq_dpci->gmsi.posted = true;
         }
-        if ( vcpu && iommu_enabled )
+        if ( vcpu && is_iommu_enabled(d) )
             hvm_migrate_pirq(pirq_dpci, vcpu);
 
         /* Use interrupt posting if it is supported. */
@@ -817,7 +817,7 @@  int hvm_do_IRQ_dpci(struct domain *d, struct pirq *pirq)
 
     ASSERT(is_hvm_domain(d));
 
-    if ( !iommu_enabled || (!is_hardware_domain(d) && !dpci) ||
+    if ( !is_iommu_enabled(d) || (!is_hardware_domain(d) && !dpci) ||
          !pirq_dpci || !(pirq_dpci->flags & HVM_IRQ_DPCI_MAPPED) )
         return 0;
 
@@ -869,7 +869,7 @@  static int _hvm_dpci_msi_eoi(struct domain *d,
 
 void hvm_dpci_msi_eoi(struct domain *d, int vector)
 {
-    if ( !iommu_enabled ||
+    if ( !is_iommu_enabled(d) ||
          (!hvm_domain_irq(d)->dpci && !is_hardware_domain(d)) )
        return;
 
@@ -1001,7 +1001,7 @@  void hvm_dpci_eoi(struct domain *d, unsigned int guest_gsi,
     const struct hvm_irq_dpci *hvm_irq_dpci;
     const struct hvm_girq_dpci_mapping *girq;
 
-    if ( !iommu_enabled )
+    if ( !is_iommu_enabled(d) )
         return;
 
     if ( is_hardware_domain(d) )
diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index e61d3d1368..27c75e0786 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -179,7 +179,7 @@  void __hwdom_init iommu_hwdom_init(struct domain *d)
 
     check_hwdom_reqs(d);
 
-    if ( !iommu_enabled )
+    if ( !is_iommu_enabled(d) )
         return;
 
     register_keyhandler('o', &iommu_dump_p2m_table, "dump iommu p2m table", 0);
@@ -284,7 +284,7 @@  int iommu_construct(struct domain *d)
 
 void iommu_domain_destroy(struct domain *d)
 {
-    if ( !iommu_enabled || !dom_iommu(d)->platform_ops )
+    if ( !is_iommu_enabled(d) )
         return;
 
     iommu_teardown(d);
@@ -296,16 +296,18 @@  int iommu_map(struct domain *d, dfn_t dfn, mfn_t mfn,
               unsigned int page_order, unsigned int flags,
               unsigned int *flush_flags)
 {
-    const struct domain_iommu *hd = dom_iommu(d);
+    const struct domain_iommu *hd;
     unsigned long i;
     int rc = 0;
 
-    if ( !iommu_enabled || !hd->platform_ops )
+    if ( !is_iommu_enabled(d) )
         return 0;
 
     ASSERT(IS_ALIGNED(dfn_x(dfn), (1ul << page_order)));
     ASSERT(IS_ALIGNED(mfn_x(mfn), (1ul << page_order)));
 
+    hd = dom_iommu(d);
+
     for ( i = 0; i < (1ul << page_order); i++ )
     {
         rc = iommu_call(hd->platform_ops, map_page, d, dfn_add(dfn, i),
@@ -356,15 +358,17 @@  int iommu_legacy_map(struct domain *d, dfn_t dfn, mfn_t mfn,
 int iommu_unmap(struct domain *d, dfn_t dfn, unsigned int page_order,
                 unsigned int *flush_flags)
 {
-    const struct domain_iommu *hd = dom_iommu(d);
+    const struct domain_iommu *hd;
     unsigned long i;
     int rc = 0;
 
-    if ( !iommu_enabled || !hd->platform_ops )
+    if ( !is_iommu_enabled(d) )
         return 0;
 
     ASSERT(IS_ALIGNED(dfn_x(dfn), (1ul << page_order)));
 
+    hd = dom_iommu(d);
+
     for ( i = 0; i < (1ul << page_order); i++ )
     {
         int err = iommu_call(hd->platform_ops, unmap_page, d, dfn_add(dfn, i),
@@ -413,7 +417,7 @@  int iommu_lookup_page(struct domain *d, dfn_t dfn, mfn_t *mfn,
 {
     const struct domain_iommu *hd = dom_iommu(d);
 
-    if ( !iommu_enabled || !hd->platform_ops || !hd->platform_ops->lookup_page )
+    if ( !is_iommu_enabled(d) || !hd->platform_ops->lookup_page )
         return -EOPNOTSUPP;
 
     return iommu_call(hd->platform_ops, lookup_page, d, dfn, mfn, flags);
@@ -442,8 +446,8 @@  int iommu_iotlb_flush(struct domain *d, dfn_t dfn, unsigned int page_count,
     const struct domain_iommu *hd = dom_iommu(d);
     int rc;
 
-    if ( !iommu_enabled || !hd->platform_ops ||
-         !hd->platform_ops->iotlb_flush || !page_count || !flush_flags )
+    if ( !is_iommu_enabled(d) || !hd->platform_ops->iotlb_flush ||
+         !page_count || !flush_flags )
         return 0;
 
     if ( dfn_eq(dfn, INVALID_DFN) )
@@ -470,8 +474,8 @@  int iommu_iotlb_flush_all(struct domain *d, unsigned int flush_flags)
     const struct domain_iommu *hd = dom_iommu(d);
     int rc;
 
-    if ( !iommu_enabled || !hd->platform_ops ||
-         !hd->platform_ops->iotlb_flush_all || !flush_flags )
+    if ( !is_iommu_enabled(d) || !hd->platform_ops->iotlb_flush_all ||
+         !flush_flags )
         return 0;
 
     /*
@@ -556,8 +560,8 @@  int iommu_do_domctl(
 {
     int ret = -ENODEV;
 
-    if ( !iommu_enabled )
-        return -ENOSYS;
+    if ( !is_iommu_enabled(d) )
+        return -EOPNOTSUPP;
 
 #ifdef CONFIG_HAS_PCI
     ret = iommu_do_pci_domctl(domctl, d, u_domctl);
@@ -576,9 +580,9 @@  void iommu_share_p2m_table(struct domain* d)
     ASSERT(hap_enabled(d));
     /*
      * iommu_use_hap_pt(d) cannot be used here because during domain
-     * construction need_iommu(d) will always return false here.
+     * construction has_iommu_pt(d) will always return false here.
      */
-    if ( iommu_enabled && iommu_hap_pt_share )
+    if ( is_iommu_enabled(d) && iommu_hap_pt_share )
         iommu_get_ops()->share_p2m(d);
 }
 
@@ -608,10 +612,7 @@  int iommu_get_reserved_device_memory(iommu_grdm_t *func, void *ctxt)
 
 bool_t iommu_has_feature(struct domain *d, enum iommu_feature feature)
 {
-    if ( !iommu_enabled )
-        return 0;
-
-    return test_bit(feature, dom_iommu(d)->features);
+    return is_iommu_enabled(d) && test_bit(feature, dom_iommu(d)->features);
 }
 
 static void iommu_dump_p2m_table(unsigned char key)
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index 89536cc067..dee47fe08b 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -595,7 +595,7 @@  static void pci_enable_acs(struct pci_dev *pdev)
     u16 cap, ctrl, seg = pdev->seg;
     u8 bus = pdev->bus;
 
-    if ( !iommu_enabled )
+    if ( !is_iommu_enabled(pdev->domain) )
         return;
 
     pos = pci_find_ext_capability(seg, bus, pdev->devfn, PCI_EXT_CAP_ID_ACS);
@@ -864,7 +864,7 @@  static int pci_clean_dpci_irqs(struct domain *d)
 {
     struct hvm_irq_dpci *hvm_irq_dpci = NULL;
 
-    if ( !iommu_enabled )
+    if ( !is_iommu_enabled(d) )
         return 0;
 
     if ( !is_hvm_domain(d) )
@@ -897,7 +897,7 @@  static int deassign_device(struct domain *d, uint16_t seg, uint8_t bus,
     struct pci_dev *pdev;
     int ret = 0;
 
-    if ( !iommu_enabled || !hd->platform_ops )
+    if ( !is_iommu_enabled(d) )
         return -EINVAL;
 
     ASSERT(pcidevs_locked());
@@ -1383,7 +1383,7 @@  static int iommu_add_device(struct pci_dev *pdev)
     ASSERT(pcidevs_locked());
 
     hd = dom_iommu(pdev->domain);
-    if ( !iommu_enabled || !hd->platform_ops )
+    if ( !is_iommu_enabled(pdev->domain) )
         return 0;
 
     rc = hd->platform_ops->add_device(pdev->devfn, pci_to_dev(pdev));
@@ -1412,7 +1412,7 @@  static int iommu_enable_device(struct pci_dev *pdev)
     ASSERT(pcidevs_locked());
 
     hd = dom_iommu(pdev->domain);
-    if ( !iommu_enabled || !hd->platform_ops ||
+    if ( !is_iommu_enabled(pdev->domain) ||
          !hd->platform_ops->enable_device )
         return 0;
 
@@ -1428,7 +1428,7 @@  static int iommu_remove_device(struct pci_dev *pdev)
         return -EINVAL;
 
     hd = dom_iommu(pdev->domain);
-    if ( !iommu_enabled || !hd->platform_ops )
+    if ( !is_iommu_enabled(pdev->domain) )
         return 0;
 
     for ( devfn = pdev->devfn ; pdev->phantom_stride; )
@@ -1471,7 +1471,7 @@  static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn, u32 flag)
     struct pci_dev *pdev;
     int rc = 0;
 
-    if ( !iommu_enabled || !hd->platform_ops )
+    if ( !is_iommu_enabled(d) )
         return 0;
 
     /* Prevent device assign if mem paging or mem sharing have been 
@@ -1537,7 +1537,7 @@  static int iommu_get_device_group(
     int i = 0;
     const struct iommu_ops *ops = hd->platform_ops;
 
-    if ( !iommu_enabled || !ops || !ops->get_device_group_id )
+    if ( !is_iommu_enabled(d) || !ops->get_device_group_id )
         return 0;
 
     group_id = ops->get_device_group_id(seg, bus, devfn);
diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index 5d72270c5b..01f0bc4689 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -1757,7 +1757,7 @@  static void iommu_domain_teardown(struct domain *d)
         xfree(mrmrr);
     }
 
-    ASSERT(iommu_enabled);
+    ASSERT(is_iommu_enabled(d));
 
     /*
      * We can't use iommu_use_hap_pt here because either IOMMU state
diff --git a/xen/drivers/passthrough/vtd/x86/hvm.c b/xen/drivers/passthrough/vtd/x86/hvm.c
index 6675dca027..f77b35815c 100644
--- a/xen/drivers/passthrough/vtd/x86/hvm.c
+++ b/xen/drivers/passthrough/vtd/x86/hvm.c
@@ -51,7 +51,7 @@  void hvm_dpci_isairq_eoi(struct domain *d, unsigned int isairq)
     struct hvm_irq_dpci *dpci = NULL;
 
     ASSERT(isairq < NR_ISAIRQS);
-    if ( !iommu_enabled )
+    if ( !is_iommu_enabled(d) )
         return;
 
     spin_lock(&d->event_lock);
diff --git a/xen/drivers/passthrough/x86/iommu.c b/xen/drivers/passthrough/x86/iommu.c
index fd05075bb5..9879558c17 100644
--- a/xen/drivers/passthrough/x86/iommu.c
+++ b/xen/drivers/passthrough/x86/iommu.c
@@ -178,7 +178,7 @@  int arch_iommu_populate_page_table(struct domain *d)
 
 void __hwdom_init arch_iommu_check_autotranslated_hwdom(struct domain *d)
 {
-    if ( !iommu_enabled )
+    if ( !is_iommu_enabled(d) )
         panic("Presently, iommu must be enabled for PVH hardware domain\n");
 }
 
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index 791c1f66af..5a0f2e723e 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -886,7 +886,7 @@  static int flask_map_domain_msi (struct domain *d, int irq, const void *data,
 #endif
 }
 
-static u32 flask_iommu_resource_use_perm(void)
+static u32 flask_iommu_resource_use_perm(struct domain *d)
 {
     /* Obtain the permission level required for allowing a domain
      * to use an assigned device.
@@ -899,7 +899,7 @@  static u32 flask_iommu_resource_use_perm(void)
      */
     u32 perm = RESOURCE__USE_NOIOMMU;
 
-    if (iommu_enabled)
+    if ( is_iommu_enabled(d) )
         perm = ( iommu_intremap ? RESOURCE__USE_IOMMU :
                                   RESOURCE__USE_IOMMU_NOINTREMAP );
     return perm;
@@ -910,7 +910,7 @@  static int flask_map_domain_irq (struct domain *d, int irq, const void *data)
     u32 sid, dsid;
     int rc = -EPERM;
     struct avc_audit_data ad;
-    u32 dperm = flask_iommu_resource_use_perm();
+    u32 dperm = flask_iommu_resource_use_perm(d);
 
     if ( irq >= nr_static_irqs && data ) {
         rc = flask_map_domain_msi(d, irq, data, &sid, &ad);
@@ -976,7 +976,7 @@  static int flask_bind_pt_irq (struct domain *d, struct xen_domctl_bind_pt_irq *b
     int rc = -EPERM;
     int irq;
     struct avc_audit_data ad;
-    u32 dperm = flask_iommu_resource_use_perm();
+    u32 dperm = flask_iommu_resource_use_perm(d);
 
     rc = current_has_perm(d, SECCLASS_RESOURCE, RESOURCE__ADD);
     if ( rc )
@@ -1049,7 +1049,7 @@  static int flask_iomem_permission(struct domain *d, uint64_t start, uint64_t end
 
     data.ssid = domain_sid(current->domain);
     data.dsid = domain_sid(d);
-    data.use_perm = flask_iommu_resource_use_perm();
+    data.use_perm = flask_iommu_resource_use_perm(d);
 
     return security_iterate_iomem_sids(start, end, _iomem_has_perm, &data);
 }
@@ -1074,7 +1074,7 @@  static int flask_pci_config_permission(struct domain *d, uint32_t machine_bdf, u
     if ( access && (end >= 0x10 && start < 0x28) )
         perm = RESOURCE__SETUP;
     else
-        perm = flask_iommu_resource_use_perm();
+        perm = flask_iommu_resource_use_perm(d);
 
     AVC_AUDIT_DATA_INIT(&ad, DEV);
     ad.device = (unsigned long) machine_bdf;
@@ -1299,7 +1299,7 @@  static int flask_assign_device(struct domain *d, uint32_t machine_bdf)
     u32 dsid, rsid;
     int rc = -EPERM;
     struct avc_audit_data ad;
-    u32 dperm = flask_iommu_resource_use_perm();
+    u32 dperm = flask_iommu_resource_use_perm(d);
 
     if ( !d )
         return flask_test_assign_device(machine_bdf);
@@ -1358,7 +1358,7 @@  static int flask_assign_dtdevice(struct domain *d, const char *dtpath)
     u32 dsid, rsid;
     int rc = -EPERM;
     struct avc_audit_data ad;
-    u32 dperm = flask_iommu_resource_use_perm();
+    u32 dperm = flask_iommu_resource_use_perm(d);
 
     if ( !d )
         return flask_test_assign_dtdevice(dtpath);
@@ -1543,7 +1543,7 @@  static int flask_ioport_permission(struct domain *d, uint32_t start, uint32_t en
 
     data.ssid = domain_sid(current->domain);
     data.dsid = domain_sid(d);
-    data.use_perm = flask_iommu_resource_use_perm();
+    data.use_perm = flask_iommu_resource_use_perm(d);
 
     return security_iterate_ioport_sids(start, end, _ioport_has_perm, &data);
 }