[PATCH 2/4] vtd: Collapse x86 subdirectory

Teddy Astie posted 4 patches 1 week ago
There is a newer version of this series
[PATCH 2/4] vtd: Collapse x86 subdirectory
Posted by Teddy Astie 1 week ago
As VT-d only exists on x86, it doesn't make sense to have a x86-specific
subdirectory. Moreover, now that vtd.c empty (once we moved the deprecated
iommu_inclusive_mapping parameter), only ats.c remain which could be moved to
the upper directory.

Collapse this directory to make the driver structure clearer.

Signed-off-by: Teddy Astie <teddy.astie@vates.tech>
---
Do we care much about iommu_inclusive_mapping ?

 xen/drivers/passthrough/iommu.c             | 10 ++++++
 xen/drivers/passthrough/vtd/Makefile        |  3 +-
 xen/drivers/passthrough/vtd/{x86 => }/ats.c | 10 +++---
 xen/drivers/passthrough/vtd/x86/Makefile    |  2 --
 xen/drivers/passthrough/vtd/x86/vtd.c       | 38 ---------------------
 5 files changed, 16 insertions(+), 47 deletions(-)
 rename xen/drivers/passthrough/vtd/{x86 => }/ats.c (97%)
 delete mode 100644 xen/drivers/passthrough/vtd/x86/Makefile
 delete mode 100644 xen/drivers/passthrough/vtd/x86/vtd.c

diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index c9425d6971..2e2037502d 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -57,6 +57,16 @@ bool __read_mostly iommu_hwdom_passthrough;
 bool __hwdom_initdata iommu_hwdom_inclusive;
 int8_t __hwdom_initdata iommu_hwdom_reserved = -1;
 
+#ifdef CONFIG_X86
+/*
+ * Deprecated
+ *
+ * iommu_inclusive_mapping: when set, all memory below 4GB is included in dom0
+ * 1:1 iommu mappings except xen and unusable regions.
+ */
+boolean_param("iommu_inclusive_mapping", iommu_hwdom_inclusive);
+#endif
+
 #ifndef iommu_hap_pt_share
 bool __read_mostly iommu_hap_pt_share = true;
 #endif
diff --git a/xen/drivers/passthrough/vtd/Makefile b/xen/drivers/passthrough/vtd/Makefile
index fde7555fac..328a014016 100644
--- a/xen/drivers/passthrough/vtd/Makefile
+++ b/xen/drivers/passthrough/vtd/Makefile
@@ -1,5 +1,4 @@
-obj-$(CONFIG_X86) += x86/
-
+obj-y += ats.o
 obj-y += iommu.o
 obj-y += dmar.o
 obj-y += utils.o
diff --git a/xen/drivers/passthrough/vtd/x86/ats.c b/xen/drivers/passthrough/vtd/ats.c
similarity index 97%
rename from xen/drivers/passthrough/vtd/x86/ats.c
rename to xen/drivers/passthrough/vtd/ats.c
index 61052ef580..d481eff6d7 100644
--- a/xen/drivers/passthrough/vtd/x86/ats.c
+++ b/xen/drivers/passthrough/vtd/ats.c
@@ -22,11 +22,11 @@
 #include <xen/pci.h>
 #include <xen/pci_regs.h>
 #include <asm/msi.h>
-#include "../iommu.h"
-#include "../dmar.h"
-#include "../vtd.h"
-#include "../extern.h"
-#include "../../ats.h"
+#include "iommu.h"
+#include "dmar.h"
+#include "vtd.h"
+#include "extern.h"
+#include "../ats.h"
 
 static LIST_HEAD(ats_dev_drhd_units);
 
diff --git a/xen/drivers/passthrough/vtd/x86/Makefile b/xen/drivers/passthrough/vtd/x86/Makefile
deleted file mode 100644
index fe20a0b019..0000000000
--- a/xen/drivers/passthrough/vtd/x86/Makefile
+++ /dev/null
@@ -1,2 +0,0 @@
-obj-y += ats.o
-obj-y += vtd.o
diff --git a/xen/drivers/passthrough/vtd/x86/vtd.c b/xen/drivers/passthrough/vtd/x86/vtd.c
deleted file mode 100644
index b0798dc6a1..0000000000
--- a/xen/drivers/passthrough/vtd/x86/vtd.c
+++ /dev/null
@@ -1,38 +0,0 @@
-/*
- * Copyright (c) 2008, Intel Corporation.
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms and conditions of the GNU General Public License,
- * version 2, as published by the Free Software Foundation.
- *
- * This program is distributed in the hope it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
- * more details.
- *
- * You should have received a copy of the GNU General Public License along with
- * this program; If not, see <http://www.gnu.org/licenses/>.
- *
- * Copyright (C) Allen Kay <allen.m.kay@intel.com>
- * Copyright (C) Weidong Han <weidong.han@intel.com>
- */
-
-#include <xen/param.h>
-#include <xen/sched.h>
-#include <xen/softirq.h>
-#include <xen/domain_page.h>
-#include <asm/paging.h>
-#include <xen/iommu.h>
-#include <xen/irq.h>
-#include <xen/numa.h>
-#include <asm/fixmap.h>
-#include "../iommu.h"
-#include "../dmar.h"
-#include "../vtd.h"
-#include "../extern.h"
-
-/*
- * iommu_inclusive_mapping: when set, all memory below 4GB is included in dom0
- * 1:1 iommu mappings except xen and unusable regions.
- */
-boolean_param("iommu_inclusive_mapping", iommu_hwdom_inclusive);
-- 
2.51.1



--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech
Re: [PATCH 2/4] vtd: Collapse x86 subdirectory
Posted by Jan Beulich 1 week ago
On 22.10.2025 11:51, Teddy Astie wrote:
> As VT-d only exists on x86, it doesn't make sense to have a x86-specific
> subdirectory. Moreover, now that vtd.c empty (once we moved the deprecated
> iommu_inclusive_mapping parameter), only ats.c remain which could be moved to
> the upper directory.
> 
> Collapse this directory to make the driver structure clearer.
> 
> Signed-off-by: Teddy Astie <teddy.astie@vates.tech>

On top of what Andrew said, ...

> --- a/xen/drivers/passthrough/iommu.c
> +++ b/xen/drivers/passthrough/iommu.c
> @@ -57,6 +57,16 @@ bool __read_mostly iommu_hwdom_passthrough;
>  bool __hwdom_initdata iommu_hwdom_inclusive;
>  int8_t __hwdom_initdata iommu_hwdom_reserved = -1;
>  
> +#ifdef CONFIG_X86
> +/*
> + * Deprecated
> + *
> + * iommu_inclusive_mapping: when set, all memory below 4GB is included in dom0
> + * 1:1 iommu mappings except xen and unusable regions.
> + */
> +boolean_param("iommu_inclusive_mapping", iommu_hwdom_inclusive);
> +#endif

... such arch-specific #ifdef-ary is precisely what we'd like to avoid where
possible. Plus the way it's done you're now re-introducing the option even
to INTEL_IOMMU=n builds.

If we were to drop the option, besides editing the command line doc, a
ChangeLog entry would also be needed.

> --- a/xen/drivers/passthrough/vtd/Makefile
> +++ b/xen/drivers/passthrough/vtd/Makefile
> @@ -1,5 +1,4 @@
> -obj-$(CONFIG_X86) += x86/
> -
> +obj-y += ats.o

Losing the CONFIG_X86 dependency is likely right here (ATS is arch-independent
after all), but would imo want to come with explicit justification. The earlier
use of CONFIG_X86 was here for doc purposes (to somewhat help possible future
re-use for another architecture).

Jan
Re: [PATCH 2/4] vtd: Collapse x86 subdirectory
Posted by Andrew Cooper 1 week ago
On 22/10/2025 10:51 am, Teddy Astie wrote:
> As VT-d only exists on x86, it doesn't make sense to have a x86-specific
> subdirectory.

VT-d does exist elsewhere.

Xen (via the absence of ia64 support) only implements VT-d on x86.

>  Moreover, now that vtd.c empty (once we moved the deprecated
> iommu_inclusive_mapping parameter), only ats.c remain which could be moved to
> the upper directory.
>
> Collapse this directory to make the driver structure clearer.
>
> Signed-off-by: Teddy Astie <teddy.astie@vates.tech>
> ---
> Do we care much about iommu_inclusive_mapping ?

What do you mean?  The functionality, yes.  The top-level option option
that's been listed as deprecated since Xen 4.12 (7 years now), probably not.

But if you're going to delete it, it should be in a patch of it's own,
including editing xen-command-line.pandoc.

~Andrew

Re: [PATCH 2/4] vtd: Collapse x86 subdirectory
Posted by Teddy Astie 1 week ago
Le 22/10/2025 à 12:13, Andrew Cooper a écrit :
> On 22/10/2025 10:51 am, Teddy Astie wrote:
>> As VT-d only exists on x86, it doesn't make sense to have a x86-specific
>> subdirectory.
> 
> VT-d does exist elsewhere.
> 
> Xen (via the absence of ia64 support) only implements VT-d on x86.
> 
>>   Moreover, now that vtd.c empty (once we moved the deprecated
>> iommu_inclusive_mapping parameter), only ats.c remain which could be moved to
>> the upper directory.
>>
>> Collapse this directory to make the driver structure clearer.
>>
>> Signed-off-by: Teddy Astie <teddy.astie@vates.tech>
>> ---
>> Do we care much about iommu_inclusive_mapping ?
> 
> What do you mean?  The functionality, yes.  The top-level option option
> that's been listed as deprecated since Xen 4.12 (7 years now), probably not.
> 

Yes, I'm speaking about iommu_inclusive_mapping=<boolean> command-line 
option (that lived in vtd code) that is actually deprecated and replaced 
by dom0-iommu=map-inclusive.

> But if you're going to delete it, it should be in a patch of it's own,
> including editing xen-command-line.pandoc.

I can add one (along with a approriate change in changelog).

> 
> ~Andrew



--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech