[Xen-devel] [PATCH] iommu: avoid triggering ASSERT_UNREACHABLE() on ARM...

* [Xen-devel] [PATCH] iommu: avoid triggering ASSERT_UNREACHABLE() on ARM...
@ 2019-09-26 10:03 Paul Durrant
  2019-09-26 11:24 ` Julien Grall
  2019-09-26 12:12 ` Jan Beulich
  0 siblings, 2 replies; 6+ messages in thread
From: Paul Durrant @ 2019-09-26 10:03 UTC (permalink / raw)
  To: xen-devel
  Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
	George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan, Oleksandr,
	Julien Grall, Paul Durrant, Jan Beulich

...when the IOMMU is not enabled.

80ff3d338dc9 "iommu: tidy up iommu_use_hap_pt() and need_iommu_pt_sync()
macros" introduced CONFIG_IOMMU_FORCE_PT_SHARE, which causes the global
'iommu_hap_pt_share' option to be replaced with a #define-d value of true.
In this configuration, calling clear_iommu_hap_pt_share() will result
trigger the aforementioned ASSERT.

CONFIG_IOMMU_FORCE_PT_SHARE is always selected for ARM builds and,
because clear_iommu_hap_pt_share() is called by the common iommu_setup()
function if the IOMMU is not enabled, it is no longer safe to disable the
IOMMU on ARM systems.

However, running with the IOMMU disabled is a valid configuration for ARM
systems, so this patch rectifies the problem by removing the call to
clear_iommu_hap_pt_share() from common code. As a side effect of this,
however, it becomes possible on x86 systems for iommu_enabled to be false
but iommu_hap_pt_share to be true. Thus the code in sysctl.c
needs to be changed to make sure that XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share
is not erroneously advertised when the IOMMU has been disabled.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Reported-by: Oleksandr <olekstysh@gmail.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wl@xen.org>
---
 xen/common/sysctl.c             | 6 ++++--
 xen/drivers/passthrough/iommu.c | 1 -
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
index e8763c7fdf..f88a285e7f 100644
--- a/xen/common/sysctl.c
+++ b/xen/common/sysctl.c
@@ -268,9 +268,11 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
         pi->max_mfn = get_upper_mfn_bound();
         arch_do_physinfo(pi);
         if ( iommu_enabled )
+        {
             pi->capabilities |= XEN_SYSCTL_PHYSCAP_directio;
-        if ( iommu_hap_pt_share )
-            pi->capabilities |= XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share;
+            if ( iommu_hap_pt_share )
+                pi->capabilities |= XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share;
+        }
 
         if ( copy_to_guest(u_sysctl, op, 1) )
             ret = -EFAULT;
diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index e8fddc2dc7..c33ca70ec9 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -456,7 +456,6 @@ int __init iommu_setup(void)
     if ( !iommu_enabled )
     {
         iommu_intremap = 0;
-        clear_iommu_hap_pt_share();
     }
 
     if ( (force_iommu && !iommu_enabled) ||
-- 
2.20.1.2.gb21ebb671


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 6+ messages in thread