All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jan Beulich <jbeulich@suse.com>
To: "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>
Cc: "Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Wei Liu" <wl@xen.org>, "Roger Pau Monné" <roger.pau@citrix.com>,
	"Paul Durrant" <paul@xen.org>
Subject: [PATCH] x86/IOMMU: mark IOMMU / intremap not in use when ACPI tables are missing
Date: Wed, 20 Oct 2021 12:36:37 +0200	[thread overview]
Message-ID: <684e4a04-3ef3-46be-3302-149d5ce30177@suse.com> (raw)

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 the
IOMMU, i.e. in particular when
- parsing of the respective ACPI tables failed,
- "iommu=off" is in effect, but not "iommu=no-intremap".
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 skip ACPI table parsing altogether when
"iommu=off" is in effect anyway.

Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
I've deliberately not added a Fixes: tag here, as I'm of the opinion
that d8bd82327b0f ("AMD/IOMMU: obtain IVHD type to use earlier") only
uncovered a pre-existing anomaly. This particularly applies to the
"iommu=off" aspect. (This now allows me to remove an item from my TODO
list: I was meaning to figure out why one of my systems wouldn't come
up properly with "iommu=off", and I had never thought of combining this
with "no-intremap".)

Arguably "iommu=off" should turn off subordinate features in common
IOMMU code, but doing so in parse_iommu_param() would be wrong (as
there might be multiple "iommu=" to parse). This could be placed in
iommu_supports_x2apic(), but see the next item.

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?

--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
@@ -193,10 +193,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/x86/iommu.c
+++ b/xen/drivers/passthrough/x86/iommu.c
@@ -41,6 +41,23 @@ enum iommu_intremap __read_mostly iommu_
 bool __read_mostly iommu_intpost;
 #endif
 
+void __init acpi_iommu_init(void)
+{
+    if ( iommu_enable )
+    {
+        int ret = acpi_dmar_init();
+
+        if ( ret == -ENODEV )
+            ret = acpi_ivrs_init();
+
+        if ( ret )
+            iommu_enable = false;
+    }
+
+    if ( !iommu_enable )
+        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. */



             reply	other threads:[~2021-10-20 10:37 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-20 10:36 Jan Beulich [this message]
2021-10-20 20:01 ` [PATCH] x86/IOMMU: mark IOMMU / intremap not in use when ACPI tables are missing Andrew Cooper
2021-10-21  7:21   ` Jan Beulich
2021-10-21  7:37     ` Jan Beulich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=684e4a04-3ef3-46be-3302-149d5ce30177@suse.com \
    --to=jbeulich@suse.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=paul@xen.org \
    --cc=roger.pau@citrix.com \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.