[PATCH 0/4] Various IOMMU related simplifications

[PATCH 0/4] Various IOMMU related simplifications

[Teddy Astie]

Various simplifications in the VT-d and IO-APIC code (mostly related to legacy ia64 code).

No functional change intended.

Teddy Astie (4):
  vtd: Move (un)map_vtd_domain_page to extern.h
  vtd: Collapse x86 subdirectory
  vtd: Remove IO_xAPIC_route_entry macro
  x86/ioapic: Don't open-code 32-bits rte reads

 xen/arch/x86/include/asm/io_apic.h          |  1 +
 xen/arch/x86/io_apic.c                      | 29 +++++--------
 xen/drivers/passthrough/iommu.c             | 10 +++++
 xen/drivers/passthrough/vtd/Makefile        |  3 +-
 xen/drivers/passthrough/vtd/{x86 => }/ats.c | 10 ++---
 xen/drivers/passthrough/vtd/extern.h        | 13 +++++-
 xen/drivers/passthrough/vtd/intremap.c      | 19 ++++----
 xen/drivers/passthrough/vtd/vtd.h           |  3 --
 xen/drivers/passthrough/vtd/x86/Makefile    |  2 -
 xen/drivers/passthrough/vtd/x86/vtd.c       | 48 ---------------------
 10 files changed, 47 insertions(+), 91 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

-- 
2.51.1



--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

[PATCH 2/4] vtd: Collapse x86 subdirectory

[Teddy Astie]

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

[PATCH 3/4] vtd: Remove IO_xAPIC_route_entry macro

[Teddy Astie]

This macro was introduced to abstract between IO-APIC and IO-SAPIC (ia64),
now that ia64 isn't supported anymore, this macro now only refers to IO-APIC.

Signed-off-by: Teddy Astie <teddy.astie@vates.tech>
---
 xen/drivers/passthrough/vtd/intremap.c | 10 +++++-----
 xen/drivers/passthrough/vtd/vtd.h      |  3 ---
 2 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/xen/drivers/passthrough/vtd/intremap.c b/xen/drivers/passthrough/vtd/intremap.c
index 7726ee618a..e0314aa469 100644
--- a/xen/drivers/passthrough/vtd/intremap.c
+++ b/xen/drivers/passthrough/vtd/intremap.c
@@ -268,7 +268,7 @@ static unsigned int alloc_remap_entry(struct vtd_iommu *iommu, unsigned int nr)
 }
 
 static int remap_entry_to_ioapic_rte(
-    struct vtd_iommu *iommu, int index, struct IO_xAPIC_route_entry *old_rte)
+    struct vtd_iommu *iommu, int index, struct IO_APIC_route_entry *old_rte)
 {
     struct iremap_entry *iremap_entry = NULL, *iremap_entries;
     unsigned long flags;
@@ -316,8 +316,8 @@ static int remap_entry_to_ioapic_rte(
 }
 
 static int ioapic_rte_to_remap_entry(struct vtd_iommu *iommu,
-    int apic, unsigned int ioapic_pin, struct IO_xAPIC_route_entry *old_rte,
-    struct IO_xAPIC_route_entry new_rte)
+    int apic, unsigned int ioapic_pin, struct IO_APIC_route_entry *old_rte,
+    struct IO_APIC_route_entry new_rte)
 {
     struct iremap_entry *iremap_entry = NULL, *iremap_entries;
     struct iremap_entry new_ire;
@@ -398,7 +398,7 @@ unsigned int cf_check io_apic_read_remap_rte(
 {
     unsigned int ioapic_pin = (reg - 0x10) / 2;
     int index;
-    struct IO_xAPIC_route_entry old_rte = { };
+    struct IO_APIC_route_entry old_rte = { };
     int rte_upper = (reg & 1) ? 1 : 0;
     struct vtd_iommu *iommu = ioapic_to_iommu(IO_APIC_ID(apic));
 
@@ -420,7 +420,7 @@ unsigned int cf_check io_apic_read_remap_rte(
 void cf_check io_apic_write_remap_rte(
     unsigned int apic, unsigned int pin, uint64_t rte)
 {
-    struct IO_xAPIC_route_entry old_rte = {}, new_rte;
+    struct IO_APIC_route_entry old_rte = {}, new_rte;
     struct vtd_iommu *iommu = ioapic_to_iommu(IO_APIC_ID(apic));
     int rc;
 
diff --git a/xen/drivers/passthrough/vtd/vtd.h b/xen/drivers/passthrough/vtd/vtd.h
index b95124517b..f0286b40c3 100644
--- a/xen/drivers/passthrough/vtd/vtd.h
+++ b/xen/drivers/passthrough/vtd/vtd.h
@@ -31,9 +31,6 @@
 #define MAP_ERROR_RECOVERY    (1u << 2)
 #define UNMAP_ME_PHANTOM_FUNC (1u << 3)
 
-/* Allow for both IOAPIC and IOSAPIC. */
-#define IO_xAPIC_route_entry IO_APIC_route_entry
-
 struct IO_APIC_route_remap_entry {
     union {
         u64 val;
-- 
2.51.1



--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

[PATCH 1/4] vtd: Move (un)map_vtd_domain_page to extern.h

[Teddy Astie]

These functions are basically wrappers of map_domain_page; move
them to the shared extern.h and make them inline to help with code
generation.

Signed-off-by: Teddy Astie <teddy.astie@vates.tech>
---
 xen/drivers/passthrough/vtd/extern.h  | 13 +++++++++++--
 xen/drivers/passthrough/vtd/x86/vtd.c | 10 ----------
 2 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/xen/drivers/passthrough/vtd/extern.h b/xen/drivers/passthrough/vtd/extern.h
index c16583c951..a62310b3e7 100644
--- a/xen/drivers/passthrough/vtd/extern.h
+++ b/xen/drivers/passthrough/vtd/extern.h
@@ -21,6 +21,7 @@
 #define DRIVERS__PASSTHROUGH__VTD__EXTERN_H
 
 #include "dmar.h"
+#include <xen/domain_page.h>
 #include <xen/keyhandler.h>
 
 #define VTDPREFIX "[VT-D]"
@@ -78,8 +79,6 @@ int __must_check qinval_device_iotlb_sync(struct vtd_iommu *iommu,
 
 uint64_t alloc_pgtable_maddr(unsigned long npages, nodeid_t node);
 void free_pgtable_maddr(u64 maddr);
-void *map_vtd_domain_page(u64 maddr);
-void unmap_vtd_domain_page(const void *va);
 int domain_context_mapping_one(struct domain *domain, struct vtd_iommu *iommu,
                                uint8_t bus, uint8_t devfn,
                                const struct pci_dev *pdev, domid_t domid,
@@ -114,4 +113,14 @@ void quirk_iommu_caps(struct vtd_iommu *iommu);
 bool platform_supports_intremap(void);
 bool platform_supports_x2apic(void);
 
+static inline void *map_vtd_domain_page(u64 maddr)
+{
+    return map_domain_page(_mfn(paddr_to_pfn(maddr)));
+}
+
+static inline void unmap_vtd_domain_page(const void *va)
+{
+    unmap_domain_page(va);
+}
+
 #endif // DRIVERS__PASSTHROUGH__VTD__EXTERN_H
diff --git a/xen/drivers/passthrough/vtd/x86/vtd.c b/xen/drivers/passthrough/vtd/x86/vtd.c
index 76f12adc23..b0798dc6a1 100644
--- a/xen/drivers/passthrough/vtd/x86/vtd.c
+++ b/xen/drivers/passthrough/vtd/x86/vtd.c
@@ -36,13 +36,3 @@
  * 1:1 iommu mappings except xen and unusable regions.
  */
 boolean_param("iommu_inclusive_mapping", iommu_hwdom_inclusive);
-
-void *map_vtd_domain_page(u64 maddr)
-{
-    return map_domain_page(_mfn(paddr_to_pfn(maddr)));
-}
-
-void unmap_vtd_domain_page(const void *va)
-{
-    unmap_domain_page(va);
-}
-- 
2.51.1



--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

[PATCH 4/4] x86/ioapic: Don't open-code 32-bits rte reads

[Teddy Astie]

There are many places where we use interesting ways of reading 32-bits
components of the RTE. Introduce and use low and high components directly
to the rte structure instead.

Also take the opportunity to simplify "x & 1 ? 1 : 0".

Signed-off-by: Teddy Astie <teddy.astie@vates.tech>
---
 xen/arch/x86/include/asm/io_apic.h     |  1 +
 xen/arch/x86/io_apic.c                 | 29 ++++++++++----------------
 xen/drivers/passthrough/vtd/intremap.c |  9 +++-----
 3 files changed, 15 insertions(+), 24 deletions(-)

diff --git a/xen/arch/x86/include/asm/io_apic.h b/xen/arch/x86/include/asm/io_apic.h
index 4680dce9e1..0e85f2a860 100644
--- a/xen/arch/x86/include/asm/io_apic.h
+++ b/xen/arch/x86/include/asm/io_apic.h
@@ -122,6 +122,7 @@ struct IO_APIC_route_entry {
             } dest;
         };
         uint64_t raw;
+        struct { uint32_t low, high; };
     };
 };
 
diff --git a/xen/arch/x86/io_apic.c b/xen/arch/x86/io_apic.c
index 24447aef6c..9d2edec179 100644
--- a/xen/arch/x86/io_apic.c
+++ b/xen/arch/x86/io_apic.c
@@ -257,28 +257,23 @@ nomem:
     return NULL;
 }
 
-union entry_union {
-    struct { u32 w1, w2; };
-    struct IO_APIC_route_entry entry;
-};
-
 struct IO_APIC_route_entry __ioapic_read_entry(
     unsigned int apic, unsigned int pin, bool raw)
 {
-    union entry_union eu;
+    struct IO_APIC_route_entry entry;
 
     if ( raw || !iommu_intremap )
     {
-        eu.w1 = __io_apic_read(apic, 0x10 + 2 * pin);
-        eu.w2 = __io_apic_read(apic, 0x11 + 2 * pin);
+        entry.low  = __io_apic_read(apic, 0x10 + 2 * pin);
+        entry.high = __io_apic_read(apic, 0x11 + 2 * pin);
     }
     else
     {
-        eu.w1 = iommu_read_apic_from_ire(apic, 0x10 + 2 * pin);
-        eu.w2 = iommu_read_apic_from_ire(apic, 0x11 + 2 * pin);
+        entry.low  = iommu_read_apic_from_ire(apic, 0x10 + 2 * pin);
+        entry.high = iommu_read_apic_from_ire(apic, 0x11 + 2 * pin);
     }
 
-    return eu.entry;
+    return entry;
 }
 
 static struct IO_APIC_route_entry ioapic_read_entry(
@@ -297,12 +292,10 @@ void __ioapic_write_entry(
     unsigned int apic, unsigned int pin, bool raw,
     struct IO_APIC_route_entry e)
 {
-    union entry_union eu = { .entry = e };
-
     if ( raw || !iommu_intremap )
     {
-        __io_apic_write(apic, 0x11 + 2 * pin, eu.w2);
-        __io_apic_write(apic, 0x10 + 2 * pin, eu.w1);
+        __io_apic_write(apic, 0x11 + 2 * pin, e.high);
+        __io_apic_write(apic, 0x10 + 2 * pin, e.low);
         /*
          * Might be called before io_apic_pin_eoi is allocated.  Entry will be
          * initialized to the RTE value once the cache is allocated.
@@ -2235,7 +2228,7 @@ int ioapic_guest_read(unsigned long physbase, unsigned int reg, u32 *pval)
     dprintk(XENLOG_INFO, "IO-APIC: apic=%d, pin=%d, irq=%d\n" \
             XENLOG_INFO "IO-APIC: new_entry=%08x\n"           \
             XENLOG_INFO "IO-APIC: " f "\n",                   \
-            apic, pin, irq, *(u32 *)&rte, ##a )
+            apic, pin, irq, rte.low, ##a )
 
 int ioapic_guest_write(unsigned long physbase, unsigned int reg, u32 val)
 {
@@ -2254,7 +2247,7 @@ int ioapic_guest_write(unsigned long physbase, unsigned int reg, u32 val)
     pin = (reg - 0x10) >> 1;
 
     /* Write first half from guest; second half is target info. */
-    *(u32 *)&rte = val;
+    rte.low = val;
 
     /*
      * What about weird destination types?
@@ -2305,7 +2298,7 @@ int ioapic_guest_write(unsigned long physbase, unsigned int reg, u32 val)
         ret = io_apic_read(apic, 0x10 + 2 * pin);
         spin_unlock_irqrestore(&ioapic_lock, flags);
         rte.vector = desc->arch.vector;
-        if ( *(u32*)&rte != ret )
+        if ( rte.low != ret )
             WARN_BOGUS_WRITE("old_entry=%08x pirq=%d\n" XENLOG_INFO
                              "IO-APIC: Attempt to modify IO-APIC pin for in-use IRQ!",
                              ret, pirq);
diff --git a/xen/drivers/passthrough/vtd/intremap.c b/xen/drivers/passthrough/vtd/intremap.c
index e0314aa469..3cdb892559 100644
--- a/xen/drivers/passthrough/vtd/intremap.c
+++ b/xen/drivers/passthrough/vtd/intremap.c
@@ -399,7 +399,7 @@ unsigned int cf_check io_apic_read_remap_rte(
     unsigned int ioapic_pin = (reg - 0x10) / 2;
     int index;
     struct IO_APIC_route_entry old_rte = { };
-    int rte_upper = (reg & 1) ? 1 : 0;
+    unsigned int rte_upper = reg & 1;
     struct vtd_iommu *iommu = ioapic_to_iommu(IO_APIC_ID(apic));
 
     if ( !iommu->intremap.num ||
@@ -410,11 +410,8 @@ unsigned int cf_check io_apic_read_remap_rte(
 
     if ( remap_entry_to_ioapic_rte(iommu, index, &old_rte) )
         return __io_apic_read(apic, reg);
-
-    if ( rte_upper )
-        return (*(((u32 *)&old_rte) + 1));
-    else
-        return (*(((u32 *)&old_rte) + 0));
+    
+    return rte_upper ? old_rte.high : old_rte.low;
 }
 
 void cf_check io_apic_write_remap_rte(
-- 
2.51.1



--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

Re: [PATCH 1/4] vtd: Move (un)map_vtd_domain_page to extern.h

[Andrew Cooper]

On 22/10/2025 10:51 am, Teddy Astie wrote:
> These functions are basically wrappers of map_domain_page; move

We tend to write map_domain_page() with brackets, to make it clearer
that we're talking about a function.

> diff --git a/xen/drivers/passthrough/vtd/extern.h b/xen/drivers/passthrough/vtd/extern.h
> index c16583c951..a62310b3e7 100644
> --- a/xen/drivers/passthrough/vtd/extern.h
> +++ b/xen/drivers/passthrough/vtd/extern.h
> @@ -114,4 +113,14 @@ void quirk_iommu_caps(struct vtd_iommu *iommu);
>  bool platform_supports_intremap(void);
>  bool platform_supports_x2apic(void);
>  
> +static inline void *map_vtd_domain_page(u64 maddr)
> +{
> +    return map_domain_page(_mfn(paddr_to_pfn(maddr)));

maddr_to_mfn() drops the _mfn() wrapper.  u64 wants to become paddr_t.

Otherwise, Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>.  I can
fix up when queueing.

~Andrew

Re: [PATCH 2/4] vtd: Collapse x86 subdirectory

[Andrew Cooper]

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 3/4] vtd: Remove IO_xAPIC_route_entry macro

[Andrew Cooper]

On 22/10/2025 10:51 am, Teddy Astie wrote:
> This macro was introduced to abstract between IO-APIC and IO-SAPIC (ia64),
> now that ia64 isn't supported anymore, this macro now only refers to IO-APIC.
>
> Signed-off-by: Teddy Astie <teddy.astie@vates.tech>

Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

> diff --git a/xen/drivers/passthrough/vtd/intremap.c b/xen/drivers/passthrough/vtd/intremap.c
> index 7726ee618a..e0314aa469 100644
> --- a/xen/drivers/passthrough/vtd/intremap.c
> +++ b/xen/drivers/passthrough/vtd/intremap.c
> @@ -398,7 +398,7 @@ unsigned int cf_check io_apic_read_remap_rte(
>  {
>      unsigned int ioapic_pin = (reg - 0x10) / 2;
>      int index;
> -    struct IO_xAPIC_route_entry old_rte = { };
> +    struct IO_APIC_route_entry old_rte = { };

This should be tidied up to '= {};' as the line is being edited anyway.

~Andrew

Re: [PATCH 4/4] x86/ioapic: Don't open-code 32-bits rte reads

[Andrew Cooper]

On 22/10/2025 10:51 am, Teddy Astie wrote:
> diff --git a/xen/drivers/passthrough/vtd/intremap.c b/xen/drivers/passthrough/vtd/intremap.c
> index e0314aa469..3cdb892559 100644
> --- a/xen/drivers/passthrough/vtd/intremap.c
> +++ b/xen/drivers/passthrough/vtd/intremap.c
> @@ -399,7 +399,7 @@ unsigned int cf_check io_apic_read_remap_rte(
>      unsigned int ioapic_pin = (reg - 0x10) / 2;
>      int index;
>      struct IO_APIC_route_entry old_rte = { };
> -    int rte_upper = (reg & 1) ? 1 : 0;
> +    unsigned int rte_upper = reg & 1;

bool?

But overall, a very nice cleanup.

~Andrew

Re: [PATCH 1/4] vtd: Move (un)map_vtd_domain_page to extern.h

[Jan Beulich]

On 22.10.2025 11:51, Teddy Astie wrote:
> These functions are basically wrappers of map_domain_page;

Given this, ...

> move
> them to the shared extern.h and make them inline to help with code
> generation.

... rather than moving can't we just drop them?

Jan

Re: [PATCH 2/4] vtd: Collapse x86 subdirectory

[Jan Beulich]

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

[Teddy Astie]

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

Re: [PATCH 1/4] vtd: Move (un)map_vtd_domain_page to extern.h

[Teddy Astie]

Le 22/10/2025 à 12:28, Jan Beulich a écrit :
> On 22.10.2025 11:51, Teddy Astie wrote:
>> These functions are basically wrappers of map_domain_page;
>
> Given this, ...
>
>> move
>> them to the shared extern.h and make them inline to help with code
>> generation.
>
> ... rather than moving can't we just drop them?
>

That would be better, but that's a larger set of changes
(map_vtd_domain_page uses maddr instead of mfn).
Ideally, we would want to change the code to use mfn directly and avoid
converting all around between mfn and maddr when refering to pages.

> Jan



--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech