[PATCH v2 0/2] x86/dom0: be less restrictive with the Interrupt Address Range

[PATCH v2 0/2] x86/dom0: be less restrictive with the Interrupt Address Range

[Roger Pau Monne]

Hello,

First patch is a preparatory change to reduce the changes required in
patch 2.  I would have wanted those to go in 4.20 to fix the issues on
Lenovo Thinkpads, but it's too late now.

Thanks, Roger.

Roger Pau Monne (2):
  x86/iommu: account for IOMEM caps when populating dom0 IOMMU
    page-tables
  x86/dom0: be less restrictive with the Interrupt Address Range

 xen/arch/x86/dom0_build.c           | 23 ++++++++---
 xen/arch/x86/hvm/io.c               | 16 --------
 xen/arch/x86/include/asm/hvm/io.h   |  3 --
 xen/drivers/passthrough/x86/iommu.c | 61 +++++++++++++----------------
 4 files changed, 45 insertions(+), 58 deletions(-)

-- 
2.46.0

[PATCH v2 1/2] x86/iommu: account for IOMEM caps when populating dom0 IOMMU page-tables

[Roger Pau Monne]

The current code in arch_iommu_hwdom_init() kind of open-codes the same
MMIO permission ranges that are added to the hardware domain ->iomem_caps.
Avoid this duplication and use ->iomem_caps in arch_iommu_hwdom_init() to
filter which memory regions should be added to the dom0 IOMMU page-tables.

This should result in no change in the memory regions that end up identity
mapped in the domain IOMMU page tables.

Note the IO-APIC and MCFG page(s) must be set as not accessible for a PVH
dom0, otherwise the internal Xen emulation for those ranges won't work.
This requires an adjustment in dom0_setup_permissions().

Also the special casing of E820_UNUSABLE regions no longer needs to be done
in arch_iommu_hwdom_init(), as those regions are already blocked in
->iomem_caps and thus would be removed from the rangeset as part of
->iomem_caps processing in arch_iommu_hwdom_init().

Suggested-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Changes since v1:
 - New in this version.
---
 xen/arch/x86/dom0_build.c           | 19 ++++++++-
 xen/arch/x86/hvm/io.c               | 16 --------
 xen/arch/x86/include/asm/hvm/io.h   |  3 --
 xen/drivers/passthrough/x86/iommu.c | 61 +++++++++++++----------------
 4 files changed, 45 insertions(+), 54 deletions(-)

diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
index e8f5bf5447bc..22da9ba5a362 100644
--- a/xen/arch/x86/dom0_build.c
+++ b/xen/arch/x86/dom0_build.c
@@ -552,7 +552,8 @@ int __init dom0_setup_permissions(struct domain *d)
     for ( i = 0; i < nr_ioapics; i++ )
     {
         mfn = paddr_to_pfn(mp_ioapics[i].mpc_apicaddr);
-        if ( !rangeset_contains_singleton(mmio_ro_ranges, mfn) )
+        if ( is_hvm_domain(d) ||
+             !rangeset_contains_singleton(mmio_ro_ranges, mfn) )
             rc |= iomem_deny_access(d, mfn, mfn);
     }
     /* MSI range. */
@@ -593,6 +594,22 @@ int __init dom0_setup_permissions(struct domain *d)
             rc |= rangeset_add_singleton(mmio_ro_ranges, mfn);
     }
 
+    /* For PVH dom0 prevent access to MCFG, it's emulated by Xen. */
+    if ( is_hvm_domain(d) )
+    {
+        for ( i = 0; i < pci_mmcfg_config_num; i++ )
+        {
+            const unsigned long s =
+                PFN_DOWN(pci_mmcfg_config[i].address) +
+                PCI_BDF(pci_mmcfg_config[i].start_bus_number, 0, 0);
+            const unsigned long e =
+                PFN_DOWN(pci_mmcfg_config[i].address) +
+                PCI_BDF(pci_mmcfg_config[i].end_bus_number, ~0, ~0);
+
+            rc |= iomem_deny_access(d, s, e);
+        }
+    }
+
     return rc;
 }
 
diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c
index db726b38177b..174c3285a636 100644
--- a/xen/arch/x86/hvm/io.c
+++ b/xen/arch/x86/hvm/io.c
@@ -363,22 +363,6 @@ static const struct hvm_mmcfg *vpci_mmcfg_find(const struct domain *d,
     return NULL;
 }
 
-int __hwdom_init vpci_subtract_mmcfg(const struct domain *d, struct rangeset *r)
-{
-    const struct hvm_mmcfg *mmcfg;
-
-    list_for_each_entry ( mmcfg, &d->arch.hvm.mmcfg_regions, next )
-    {
-        int rc = rangeset_remove_range(r, PFN_DOWN(mmcfg->addr),
-                                       PFN_DOWN(mmcfg->addr + mmcfg->size - 1));
-
-        if ( rc )
-            return rc;
-    }
-
-    return 0;
-}
-
 static unsigned int vpci_mmcfg_decode_addr(const struct hvm_mmcfg *mmcfg,
                                            paddr_t addr, pci_sbdf_t *sbdf)
 {
diff --git a/xen/arch/x86/include/asm/hvm/io.h b/xen/arch/x86/include/asm/hvm/io.h
index f2b8431facb0..0881e9e9f72d 100644
--- a/xen/arch/x86/include/asm/hvm/io.h
+++ b/xen/arch/x86/include/asm/hvm/io.h
@@ -132,9 +132,6 @@ int register_vpci_mmcfg_handler(struct domain *d, paddr_t addr,
 /* Destroy tracked MMCFG areas. */
 void destroy_vpci_mmcfg(struct domain *d);
 
-/* Remove MMCFG regions from a given rangeset. */
-int vpci_subtract_mmcfg(const struct domain *d, struct rangeset *r);
-
 #endif /* __ASM_X86_HVM_IO_H__ */
 
 
diff --git a/xen/drivers/passthrough/x86/iommu.c b/xen/drivers/passthrough/x86/iommu.c
index 8b1e0596b84a..3b386eb19d13 100644
--- a/xen/drivers/passthrough/x86/iommu.c
+++ b/xen/drivers/passthrough/x86/iommu.c
@@ -320,6 +320,26 @@ static int __hwdom_init cf_check map_subtract(unsigned long s, unsigned long e,
     return rangeset_remove_range(map, s, e);
 }
 
+struct handle_iomemcap {
+    struct rangeset *r;
+    unsigned long last;
+};
+static int __hwdom_init cf_check map_subtract_iomemcap(unsigned long s,
+                                                       unsigned long e,
+                                                       void *data)
+{
+    struct handle_iomemcap *h = data;
+    int rc = 0;
+
+    if ( h->last != s )
+        rc = rangeset_remove_range(h->r, h->last, s - 1);
+
+    /* Be careful with overflows. */
+    h->last = max(e + 1, e);
+
+    return rc;
+}
+
 struct map_data {
     struct domain *d;
     unsigned int flush_flags;
@@ -400,6 +420,7 @@ void __hwdom_init arch_iommu_hwdom_init(struct domain *d)
     unsigned int i;
     struct rangeset *map;
     struct map_data map_data = { .d = d };
+    struct handle_iomemcap iomem = {};
     int rc;
 
     BUG_ON(!is_hardware_domain(d));
@@ -442,14 +463,6 @@ void __hwdom_init arch_iommu_hwdom_init(struct domain *d)
 
         switch ( entry.type )
         {
-        case E820_UNUSABLE:
-            /* Only relevant for inclusive mode, otherwise this is a no-op. */
-            rc = rangeset_remove_range(map, PFN_DOWN(entry.addr),
-                                       PFN_DOWN(entry.addr + entry.size - 1));
-            if ( rc )
-                panic("IOMMU failed to remove unusable memory: %d\n", rc);
-            continue;
-
         case E820_RESERVED:
             if ( !iommu_hwdom_inclusive && !iommu_hwdom_reserved )
                 continue;
@@ -475,22 +488,13 @@ void __hwdom_init arch_iommu_hwdom_init(struct domain *d)
     if ( rc )
         panic("IOMMU failed to remove Xen ranges: %d\n", rc);
 
-    /* Remove any overlap with the Interrupt Address Range. */
-    rc = rangeset_remove_range(map, 0xfee00, 0xfeeff);
+    iomem.r = map;
+    rc = rangeset_report_ranges(d->iomem_caps, 0, ~0UL, map_subtract_iomemcap,
+                                &iomem);
+    if ( !rc && iomem.last < ~0UL )
+        rc = rangeset_remove_range(map, iomem.last, ~0UL);
     if ( rc )
-        panic("IOMMU failed to remove Interrupt Address Range: %d\n", rc);
-
-    /* If emulating IO-APIC(s) make sure the base address is unmapped. */
-    if ( has_vioapic(d) )
-    {
-        for ( i = 0; i < d->arch.hvm.nr_vioapics; i++ )
-        {
-            rc = rangeset_remove_singleton(map,
-                PFN_DOWN(domain_vioapic(d, i)->base_address));
-            if ( rc )
-                panic("IOMMU failed to remove IO-APIC: %d\n", rc);
-        }
-    }
+        panic("IOMMU failed to remove forbidden regions: %d\n", rc);
 
     if ( is_pv_domain(d) )
     {
@@ -506,17 +510,6 @@ void __hwdom_init arch_iommu_hwdom_init(struct domain *d)
             panic("IOMMU failed to remove read-only regions: %d\n", rc);
     }
 
-    if ( has_vpci(d) )
-    {
-        /*
-         * TODO: runtime added MMCFG regions are not checked to make sure they
-         * don't overlap with already mapped regions, thus preventing trapping.
-         */
-        rc = vpci_subtract_mmcfg(d, map);
-        if ( rc )
-            panic("IOMMU unable to remove MMCFG areas: %d\n", rc);
-    }
-
     /* Remove any regions past the last address addressable by the domain. */
     rc = rangeset_remove_range(map, PFN_DOWN(1UL << domain_max_paddr_bits(d)),
                                ~0UL);
-- 
2.46.0

Re: [PATCH v2 1/2] x86/iommu: account for IOMEM caps when populating dom0 IOMMU page-tables

[Jan Beulich]

On 17.02.2025 15:16, Roger Pau Monne wrote:
> The current code in arch_iommu_hwdom_init() kind of open-codes the same
> MMIO permission ranges that are added to the hardware domain ->iomem_caps.
> Avoid this duplication and use ->iomem_caps in arch_iommu_hwdom_init() to
> filter which memory regions should be added to the dom0 IOMMU page-tables.
> 
> This should result in no change in the memory regions that end up identity
> mapped in the domain IOMMU page tables.
> 
> Note the IO-APIC and MCFG page(s) must be set as not accessible for a PVH
> dom0, otherwise the internal Xen emulation for those ranges won't work.
> This requires an adjustment in dom0_setup_permissions().
> 
> Also the special casing of E820_UNUSABLE regions no longer needs to be done
> in arch_iommu_hwdom_init(), as those regions are already blocked in
> ->iomem_caps and thus would be removed from the rangeset as part of
> ->iomem_caps processing in arch_iommu_hwdom_init().

Only almost: ->iomem_caps has them removed only for addresses 1Mb and up.

> --- a/xen/arch/x86/dom0_build.c
> +++ b/xen/arch/x86/dom0_build.c
> @@ -552,7 +552,8 @@ int __init dom0_setup_permissions(struct domain *d)
>      for ( i = 0; i < nr_ioapics; i++ )
>      {
>          mfn = paddr_to_pfn(mp_ioapics[i].mpc_apicaddr);
> -        if ( !rangeset_contains_singleton(mmio_ro_ranges, mfn) )
> +        if ( is_hvm_domain(d) ||
> +             !rangeset_contains_singleton(mmio_ro_ranges, mfn) )
>              rc |= iomem_deny_access(d, mfn, mfn);
>      }

The original code used has_vioapic() and had a comment. At least one of#
the two would better be transplanted here, I think.

> @@ -593,6 +594,22 @@ int __init dom0_setup_permissions(struct domain *d)
>              rc |= rangeset_add_singleton(mmio_ro_ranges, mfn);
>      }
>  
> +    /* For PVH dom0 prevent access to MCFG, it's emulated by Xen. */
> +    if ( is_hvm_domain(d) )
> +    {
> +        for ( i = 0; i < pci_mmcfg_config_num; i++ )
> +        {
> +            const unsigned long s =
> +                PFN_DOWN(pci_mmcfg_config[i].address) +
> +                PCI_BDF(pci_mmcfg_config[i].start_bus_number, 0, 0);
> +            const unsigned long e =
> +                PFN_DOWN(pci_mmcfg_config[i].address) +
> +                PCI_BDF(pci_mmcfg_config[i].end_bus_number, ~0, ~0);
> +
> +            rc |= iomem_deny_access(d, s, e);
> +        }
> +    }

Similarly here, the original code used has_vpci() and also had a comment.
Is there actually a strong reason to replace the original MCFG code?

> --- a/xen/drivers/passthrough/x86/iommu.c
> +++ b/xen/drivers/passthrough/x86/iommu.c
> @@ -320,6 +320,26 @@ static int __hwdom_init cf_check map_subtract(unsigned long s, unsigned long e,
>      return rangeset_remove_range(map, s, e);
>  }
>  
> +struct handle_iomemcap {
> +    struct rangeset *r;
> +    unsigned long last;
> +};
> +static int __hwdom_init cf_check map_subtract_iomemcap(unsigned long s,
> +                                                       unsigned long e,
> +                                                       void *data)
> +{
> +    struct handle_iomemcap *h = data;
> +    int rc = 0;
> +
> +    if ( h->last != s )
> +        rc = rangeset_remove_range(h->r, h->last, s - 1);
> +
> +    /* Be careful with overflows. */
> +    h->last = max(e + 1, e);

What overflow could occur here? Addresses are limited to 52 bits; frame
numbers are limited to 40 bits. And ->iomem_caps starts out with a range
ending at the last address permitted by paddr_bits.

> @@ -475,22 +488,13 @@ void __hwdom_init arch_iommu_hwdom_init(struct domain *d)
>      if ( rc )
>          panic("IOMMU failed to remove Xen ranges: %d\n", rc);
>  
> -    /* Remove any overlap with the Interrupt Address Range. */
> -    rc = rangeset_remove_range(map, 0xfee00, 0xfeeff);
> +    iomem.r = map;
> +    rc = rangeset_report_ranges(d->iomem_caps, 0, ~0UL, map_subtract_iomemcap,
> +                                &iomem);
> +    if ( !rc && iomem.last < ~0UL )
> +        rc = rangeset_remove_range(map, iomem.last, ~0UL);

Similarly here I'm wondering about the upper bound of ~0UL. Is this just
to be on the safe side? And/or just because it's simpler than calculating
the actual upper bound? No ranges above the system physical address width
should ever be entered into the rangeset. Kind of as an implication
iomem.last also is guaranteed to be below ~0UL when making it here.

Jan

Re: [PATCH v2 1/2] x86/iommu: account for IOMEM caps when populating dom0 IOMMU page-tables

[Roger Pau Monné]

On Tue, Feb 18, 2025 at 04:32:33PM +0100, Jan Beulich wrote:
> On 17.02.2025 15:16, Roger Pau Monne wrote:
> > The current code in arch_iommu_hwdom_init() kind of open-codes the same
> > MMIO permission ranges that are added to the hardware domain ->iomem_caps.
> > Avoid this duplication and use ->iomem_caps in arch_iommu_hwdom_init() to
> > filter which memory regions should be added to the dom0 IOMMU page-tables.
> > 
> > This should result in no change in the memory regions that end up identity
> > mapped in the domain IOMMU page tables.
> > 
> > Note the IO-APIC and MCFG page(s) must be set as not accessible for a PVH
> > dom0, otherwise the internal Xen emulation for those ranges won't work.
> > This requires an adjustment in dom0_setup_permissions().
> > 
> > Also the special casing of E820_UNUSABLE regions no longer needs to be done
> > in arch_iommu_hwdom_init(), as those regions are already blocked in
> > ->iomem_caps and thus would be removed from the rangeset as part of
> > ->iomem_caps processing in arch_iommu_hwdom_init().
> 
> Only almost: ->iomem_caps has them removed only for addresses 1Mb and up.

Right, but we would like to have parity between IOMMU and CPU
page-tables, and hence should also allow IOMMU to map unusable regions
below 1Mb if the CPU is also allowed to map them.  I can tweak the
commit message to note this is done on purpose.

> > --- a/xen/arch/x86/dom0_build.c
> > +++ b/xen/arch/x86/dom0_build.c
> > @@ -552,7 +552,8 @@ int __init dom0_setup_permissions(struct domain *d)
> >      for ( i = 0; i < nr_ioapics; i++ )
> >      {
> >          mfn = paddr_to_pfn(mp_ioapics[i].mpc_apicaddr);
> > -        if ( !rangeset_contains_singleton(mmio_ro_ranges, mfn) )
> > +        if ( is_hvm_domain(d) ||
> > +             !rangeset_contains_singleton(mmio_ro_ranges, mfn) )
> >              rc |= iomem_deny_access(d, mfn, mfn);
> >      }
> 
> The original code used has_vioapic() and had a comment. At least one of#
> the two would better be transplanted here, I think.

I can indeed switch to has_vioapic() and keep the comment.

> > @@ -593,6 +594,22 @@ int __init dom0_setup_permissions(struct domain *d)
> >              rc |= rangeset_add_singleton(mmio_ro_ranges, mfn);
> >      }
> >  
> > +    /* For PVH dom0 prevent access to MCFG, it's emulated by Xen. */
> > +    if ( is_hvm_domain(d) )
> > +    {
> > +        for ( i = 0; i < pci_mmcfg_config_num; i++ )
> > +        {
> > +            const unsigned long s =
> > +                PFN_DOWN(pci_mmcfg_config[i].address) +
> > +                PCI_BDF(pci_mmcfg_config[i].start_bus_number, 0, 0);
> > +            const unsigned long e =
> > +                PFN_DOWN(pci_mmcfg_config[i].address) +
> > +                PCI_BDF(pci_mmcfg_config[i].end_bus_number, ~0, ~0);
> > +
> > +            rc |= iomem_deny_access(d, s, e);
> > +        }
> > +    }
> 
> Similarly here, the original code used has_vpci() and also had a comment.
> Is there actually a strong reason to replace the original MCFG code?

Hm, I see, I could tweak vpci_subtract_mmcfg() to remove the regions
from ->iomem_cap instead of open-coding here.

> > --- a/xen/drivers/passthrough/x86/iommu.c
> > +++ b/xen/drivers/passthrough/x86/iommu.c
> > @@ -320,6 +320,26 @@ static int __hwdom_init cf_check map_subtract(unsigned long s, unsigned long e,
> >      return rangeset_remove_range(map, s, e);
> >  }
> >  
> > +struct handle_iomemcap {
> > +    struct rangeset *r;
> > +    unsigned long last;
> > +};
> > +static int __hwdom_init cf_check map_subtract_iomemcap(unsigned long s,
> > +                                                       unsigned long e,
> > +                                                       void *data)
> > +{
> > +    struct handle_iomemcap *h = data;
> > +    int rc = 0;
> > +
> > +    if ( h->last != s )
> > +        rc = rangeset_remove_range(h->r, h->last, s - 1);
> > +
> > +    /* Be careful with overflows. */
> > +    h->last = max(e + 1, e);
> 
> What overflow could occur here? Addresses are limited to 52 bits; frame
> numbers are limited to 40 bits. And ->iomem_caps starts out with a range
> ending at the last address permitted by paddr_bits.

I was misremembering ->iomem_caps being initialized as [0, ~0UL] for
dom0, but that's not the case.  I can indeed drop the max() here.

> > @@ -475,22 +488,13 @@ void __hwdom_init arch_iommu_hwdom_init(struct domain *d)
> >      if ( rc )
> >          panic("IOMMU failed to remove Xen ranges: %d\n", rc);
> >  
> > -    /* Remove any overlap with the Interrupt Address Range. */
> > -    rc = rangeset_remove_range(map, 0xfee00, 0xfeeff);
> > +    iomem.r = map;
> > +    rc = rangeset_report_ranges(d->iomem_caps, 0, ~0UL, map_subtract_iomemcap,
> > +                                &iomem);
> > +    if ( !rc && iomem.last < ~0UL )
> > +        rc = rangeset_remove_range(map, iomem.last, ~0UL);
> 
> Similarly here I'm wondering about the upper bound of ~0UL. Is this just
> to be on the safe side? And/or just because it's simpler than calculating
> the actual upper bound?

I could use the actual upper bound, as we already use it a bit below
in:

    /* Remove any regions past the last address addressable by the domain. */
    rc = rangeset_remove_range(map, PFN_DOWN(1UL << domain_max_paddr_bits(d)),
                               ~0UL);

However using ~0UL is also correct for the purposes here.

> No ranges above the system physical address width
> should ever be entered into the rangeset. Kind of as an implication
> iomem.last also is guaranteed to be below ~0UL when making it here.

I could add an assert:

ASSERT(iomem.last < PFN_DOWN(1UL << domain_max_paddr_bits(d)));

But then I would need to adjust dom0_setup_permissions() to also use
domain_max_paddr_bits() instead of using paddr_bits for the
->iomem_caps initial max address.  That should likely be a separate
patch on it's own.

Thanks, Roger.

[PATCH v2 2/2] x86/dom0: be less restrictive with the Interrupt Address Range

[Roger Pau Monne]

Xen currently prevents dom0 from creating CPU or IOMMU page-table mappings
into the interrupt address range [0xfee00000, 0xfeefffff].  This range has
two different purposes.  For accesses from the CPU is contains the default
position of local APIC page at 0xfee00000.  For accesses from devices
it's the MSI address range, so the address field in the MSI entries
(usually) point to an address on that range to trigger an interrupt.

There are reports of Lenovo Thinkpad devices placing what seems to be the
UCSI shared mailbox at address 0xfeec2000 in the interrupt address range.
Attempting to use that device with a Linux PV dom0 leads to an error when
Linux kernel maps 0xfeec2000:

RIP: e030:xen_mc_flush+0x1e8/0x2b0
 xen_leave_lazy_mmu+0x15/0x60
 vmap_range_noflush+0x408/0x6f0
 __ioremap_caller+0x20d/0x350
 acpi_os_map_iomem+0x1a3/0x1c0
 acpi_ex_system_memory_space_handler+0x229/0x3f0
 acpi_ev_address_space_dispatch+0x17e/0x4c0
 acpi_ex_access_region+0x28a/0x510
 acpi_ex_field_datum_io+0x95/0x5c0
 acpi_ex_extract_from_field+0x36b/0x4e0
 acpi_ex_read_data_from_field+0xcb/0x430
 acpi_ex_resolve_node_to_value+0x2e0/0x530
 acpi_ex_resolve_to_value+0x1e7/0x550
 acpi_ds_evaluate_name_path+0x107/0x170
 acpi_ds_exec_end_op+0x392/0x860
 acpi_ps_parse_loop+0x268/0xa30
 acpi_ps_parse_aml+0x221/0x5e0
 acpi_ps_execute_method+0x171/0x3e0
 acpi_ns_evaluate+0x174/0x5d0
 acpi_evaluate_object+0x167/0x440
 acpi_evaluate_dsm+0xb6/0x130
 ucsi_acpi_dsm+0x53/0x80
 ucsi_acpi_read+0x2e/0x60
 ucsi_register+0x24/0xa0
 ucsi_acpi_probe+0x162/0x1e3
 platform_probe+0x48/0x90
 really_probe+0xde/0x340
 __driver_probe_device+0x78/0x110
 driver_probe_device+0x1f/0x90
 __driver_attach+0xd2/0x1c0
 bus_for_each_dev+0x77/0xc0
 bus_add_driver+0x112/0x1f0
 driver_register+0x72/0xd0
 do_one_initcall+0x48/0x300
 do_init_module+0x60/0x220
 __do_sys_init_module+0x17f/0x1b0
 do_syscall_64+0x82/0x170

Remove the restrictions to create mappings the interrupt address range for
dom0.  Note that the restriction to map the local APIC page is enforced
separately, and that continues to be present.

Note that even if the interrupt address range entries are populated in the
IOMMU page-tables no device access will reach those pages.  Device accesses
to the Interrupt Address Range will always be converted into Interrupt
Messages and are not subject to DMA remapping.

There's also the following restriction noted in Intel VT-d:

> Software must not program paging-structure entries to remap any address to
> the interrupt address range. Untranslated requests and translation requests
> that result in an address in the interrupt range will be blocked with
> condition code LGN.4 or SGN.8. Translated requests with an address in the
> interrupt address range are treated as Unsupported Request (UR).

Similarly for AMD-Vi:

> Accesses to the interrupt address range (Table 3) are defined to go through
> the interrupt remapping portion of the IOMMU and not through address
> translation processing. Therefore, when a transaction is being processed as
> an interrupt remapping operation, the transaction attribute of
> pretranslated or untranslated is ignored.
>
> Software Note: The IOMMU should
> not be configured such that an address translation results in a special
> address such as the interrupt address range.

However those restrictions don't apply to the identity mappings possibly
created for dom0, since the interrupt address range is never subject to DMA
remapping, and hence there's no output address after translation that
belongs to the interrupt address range.

Reported-by: Jürgen Groß <jgross@suse.com>
Link: https://lore.kernel.org/xen-devel/baade0a7-e204-4743-bda1-282df74e5f89@suse.com/
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Changes since v1:
 - No longer needs to modify arch_iommu_hwdom_init().
---
 xen/arch/x86/dom0_build.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
index 22da9ba5a362..2fe6b449149e 100644
--- a/xen/arch/x86/dom0_build.c
+++ b/xen/arch/x86/dom0_build.c
@@ -556,10 +556,6 @@ int __init dom0_setup_permissions(struct domain *d)
              !rangeset_contains_singleton(mmio_ro_ranges, mfn) )
             rc |= iomem_deny_access(d, mfn, mfn);
     }
-    /* MSI range. */
-    rc |= iomem_deny_access(d, paddr_to_pfn(MSI_ADDR_BASE_LO),
-                            paddr_to_pfn(MSI_ADDR_BASE_LO +
-                                         MSI_ADDR_DEST_ID_MASK));
     /* HyperTransport range. */
     if ( boot_cpu_data.x86_vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON) )
     {
-- 
2.46.0

Re: [PATCH v2 2/2] x86/dom0: be less restrictive with the Interrupt Address Range

[Jan Beulich]

On 17.02.2025 15:16, Roger Pau Monne wrote:
> Xen currently prevents dom0 from creating CPU or IOMMU page-table mappings
> into the interrupt address range [0xfee00000, 0xfeefffff].  This range has
> two different purposes.  For accesses from the CPU is contains the default
> position of local APIC page at 0xfee00000.  For accesses from devices
> it's the MSI address range, so the address field in the MSI entries
> (usually) point to an address on that range to trigger an interrupt.
> 
> There are reports of Lenovo Thinkpad devices placing what seems to be the
> UCSI shared mailbox at address 0xfeec2000 in the interrupt address range.
> Attempting to use that device with a Linux PV dom0 leads to an error when
> Linux kernel maps 0xfeec2000:
> 
> RIP: e030:xen_mc_flush+0x1e8/0x2b0
>  xen_leave_lazy_mmu+0x15/0x60
>  vmap_range_noflush+0x408/0x6f0
>  __ioremap_caller+0x20d/0x350
>  acpi_os_map_iomem+0x1a3/0x1c0
>  acpi_ex_system_memory_space_handler+0x229/0x3f0
>  acpi_ev_address_space_dispatch+0x17e/0x4c0
>  acpi_ex_access_region+0x28a/0x510
>  acpi_ex_field_datum_io+0x95/0x5c0
>  acpi_ex_extract_from_field+0x36b/0x4e0
>  acpi_ex_read_data_from_field+0xcb/0x430
>  acpi_ex_resolve_node_to_value+0x2e0/0x530
>  acpi_ex_resolve_to_value+0x1e7/0x550
>  acpi_ds_evaluate_name_path+0x107/0x170
>  acpi_ds_exec_end_op+0x392/0x860
>  acpi_ps_parse_loop+0x268/0xa30
>  acpi_ps_parse_aml+0x221/0x5e0
>  acpi_ps_execute_method+0x171/0x3e0
>  acpi_ns_evaluate+0x174/0x5d0
>  acpi_evaluate_object+0x167/0x440
>  acpi_evaluate_dsm+0xb6/0x130
>  ucsi_acpi_dsm+0x53/0x80
>  ucsi_acpi_read+0x2e/0x60
>  ucsi_register+0x24/0xa0
>  ucsi_acpi_probe+0x162/0x1e3
>  platform_probe+0x48/0x90
>  really_probe+0xde/0x340
>  __driver_probe_device+0x78/0x110
>  driver_probe_device+0x1f/0x90
>  __driver_attach+0xd2/0x1c0
>  bus_for_each_dev+0x77/0xc0
>  bus_add_driver+0x112/0x1f0
>  driver_register+0x72/0xd0
>  do_one_initcall+0x48/0x300
>  do_init_module+0x60/0x220
>  __do_sys_init_module+0x17f/0x1b0
>  do_syscall_64+0x82/0x170
> 
> Remove the restrictions to create mappings the interrupt address range for
> dom0.  Note that the restriction to map the local APIC page is enforced
> separately, and that continues to be present.
> 
> Note that even if the interrupt address range entries are populated in the
> IOMMU page-tables no device access will reach those pages.  Device accesses
> to the Interrupt Address Range will always be converted into Interrupt
> Messages and are not subject to DMA remapping.
> 
> There's also the following restriction noted in Intel VT-d:
> 
>> Software must not program paging-structure entries to remap any address to
>> the interrupt address range. Untranslated requests and translation requests
>> that result in an address in the interrupt range will be blocked with
>> condition code LGN.4 or SGN.8. Translated requests with an address in the
>> interrupt address range are treated as Unsupported Request (UR).
> 
> Similarly for AMD-Vi:
> 
>> Accesses to the interrupt address range (Table 3) are defined to go through
>> the interrupt remapping portion of the IOMMU and not through address
>> translation processing. Therefore, when a transaction is being processed as
>> an interrupt remapping operation, the transaction attribute of
>> pretranslated or untranslated is ignored.
>>
>> Software Note: The IOMMU should
>> not be configured such that an address translation results in a special
>> address such as the interrupt address range.
> 
> However those restrictions don't apply to the identity mappings possibly
> created for dom0, since the interrupt address range is never subject to DMA
> remapping, and hence there's no output address after translation that
> belongs to the interrupt address range.
> 
> Reported-by: Jürgen Groß <jgross@suse.com>
> Link: https://lore.kernel.org/xen-devel/baade0a7-e204-4743-bda1-282df74e5f89@suse.com/
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>