x2apic_bsp_setup() gets called ahead of iommu_setup(), and since x2APIC
mode (physical vs clustered) depends on iommu_intremap, that variable
needs to be set to off as soon as we know we can't / won't enable
interrupt remapping, i.e. in particular when parsing of the respective
ACPI tables failed. Move the turning off of iommu_intremap from AMD
specific code into acpi_iommu_init(), accompanying it by clearing of
iommu_enable.
Take the opportunity and also fully skip ACPI table parsing logic on
VT-d when both "iommu=off" and "iommu=no-intremap" are in effect anyway,
like was already the case for AMD.
The tag below only references the commit uncovering a pre-existing
anomaly.
Fixes: d8bd82327b0f ("AMD/IOMMU: obtain IVHD type to use earlier")
Reported-by: Andrew Cooper <andrew.coop...@citrix.com>
Signed-off-by: Jan Beulich <jbeul...@suse.com>
---
While the change here deals with apic_x2apic_probe() as called from
x2apic_bsp_setup(), the check_x2apic_preenabled() path looks to be
similarly affected. That call occurs before acpi_boot_init(), which is
what calls acpi_iommu_init(). The ordering in setup.c is in part
relatively fragile, which is why for the moment I'm still hesitant to
move the generic_apic_probe() call down. Plus I don't have easy access
to a suitable system to test this case. Thoughts?
---
v2: Treat iommu_enable and iommu_intremap as separate options.
--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
@@ -183,9 +183,6 @@ int __init acpi_ivrs_init(void)
{
int rc;
- if ( !iommu_enable && !iommu_intremap )
- return 0;
-
rc = amd_iommu_get_supported_ivhd_type();
if ( rc < 0 )
return rc;
@@ -193,10 +190,7 @@ int __init acpi_ivrs_init(void)
ivhd_type = rc;
if ( (amd_iommu_detect_acpi() !=0) || (iommu_found() == 0) )
- {
- iommu_intremap = iommu_intremap_off;
return -ENODEV;
- }
iommu_init_ops = &_iommu_init_ops;
--- a/xen/drivers/passthrough/vtd/dmar.c
+++ b/xen/drivers/passthrough/vtd/dmar.c
@@ -777,11 +777,7 @@ static int __init acpi_parse_dmar(struct
dmar = (struct acpi_table_dmar *)table;
dmar_flags = dmar->flags;
- if ( !iommu_enable && !iommu_intremap )
- {
- ret = -EINVAL;
- goto out;
- }
+ ASSERT(iommu_enable || iommu_intremap);
if ( !dmar->width )
{
--- a/xen/drivers/passthrough/x86/iommu.c
+++ b/xen/drivers/passthrough/x86/iommu.c
@@ -41,6 +41,24 @@ enum iommu_intremap __read_mostly iommu_
bool __read_mostly iommu_intpost;
#endif
+void __init acpi_iommu_init(void)
+{
+ int ret;
+
+ if ( !iommu_enable && !iommu_intremap )
+ return;
+
+ ret = acpi_dmar_init();
+ if ( ret == -ENODEV )
+ ret = acpi_ivrs_init();
+
+ if ( ret )
+ {
+ iommu_enable = false;
+ iommu_intremap = iommu_intremap_off;
+ }
+}
+
int __init iommu_hardware_setup(void)
{
struct IO_APIC_route_entry **ioapic_entries = NULL;
--- a/xen/include/asm-x86/acpi.h
+++ b/xen/include/asm-x86/acpi.h
@@ -141,16 +141,10 @@ extern u32 x86_acpiid_to_apicid[];
extern u32 pmtmr_ioport;
extern unsigned int pmtmr_width;
+void acpi_iommu_init(void);
int acpi_dmar_init(void);
int acpi_ivrs_init(void);
-static inline int acpi_iommu_init(void)
-{
- int ret = acpi_dmar_init();
-
- return ret == -ENODEV ? acpi_ivrs_init() : ret;
-}
-
void acpi_mmcfg_init(void);
/* Incremented whenever we transition through S3. Value is 1 during boot. */
