[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 3/8] x86/iommu: iommu_igfx, iommu_qinval and iommu_snoop are VT-d specific




On 1/12/23 17:53, Jan Beulich wrote:
On 12.01.2023 16:43, Xenia Ragiadakou wrote:
On 1/12/23 13:49, Xenia Ragiadakou wrote:
On 1/12/23 13:31, Jan Beulich wrote:
On 04.01.2023 09:44, Xenia Ragiadakou wrote:
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -74,9 +74,13 @@ extern enum __packed iommu_intremap {
      iommu_intremap_restricted,
      iommu_intremap_full,
   } iommu_intremap;
-extern bool iommu_igfx, iommu_qinval, iommu_snoop;
   #else
   # define iommu_intremap false
+#endif
+
+#ifdef CONFIG_INTEL_IOMMU
+extern bool iommu_igfx, iommu_qinval, iommu_snoop;
+#else
   # define iommu_snoop false
   #endif

Do these declarations really need touching? In patch 2 you didn't move
amd_iommu_perdev_intremap's either.

Ok, I will revert this change (as I did in v2 of patch 2) since it is
not needed.

Actually, my patch was altering the current behavior by defining
iommu_snoop as false when !INTEL_IOMMU.

IIUC, there is no control over snoop behavior when using the AMD iommu.
Hence, iommu_snoop should evaluate to true for AMD iommu.
However, when using the INTEL iommu the user can disable it via the
"iommu" param, right?

That's the intended behavior, yes, but right now we allow the option
to also affect behavior on AMD - perhaps wrongly so, as there's one
use outside of VT-x and VT-d code. But of course the option is
documented to be there for VT-d only, so one can view it as user
error if it's used on a non-VT-d system.

If that's the case then iommu_snoop needs to be moved from vtd/iommu.c
to x86/iommu.c and iommu_snoop assignment via iommu param needs to be
guarded by CONFIG_INTEL_IOMMU.

Or #define to true when !INTEL_IOMMU and keep the variable where it
is.

Given the current implementation, if defined to true, it will be true even when !iommu_enabled.

--
Xenia



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.