[PATCH v3 0/3] iommu/amd: Do not request ACS when IOMMU is not going to be initialized

The AMD IOMMU Initialization State Machine has the following state
transition diagram (only the very first states are showed, and the
`IOMMU_' prefix is omitted):

   START_STATE
        |
        v
[0] detect_ivrs() --> NOT_FOUND
        | ok
        v
  IVRS_DETECTED
        |
        v
[1] amd_iommu_disabled? (amd_iommu=off) --> IOMMU_CMDLINE_DISABLED
        | no
        v
[2] early_amd_iommu_init()
        |
        +-- [3] amd_iommu_detected? (!iommu=off && ...) -+
        | yes                                            |
        +-- ... --> IOMMU_INIT_ERROR <-------------------+
        | ok
        v
IOMMU_ACPI_FINISHED
        |
        v
       ...

[0] always calls pci_request_acs() as long as there's a valid IVRS table
and no Stoney Ridge graphics. This is not optimal as ACS is not required
in an [amd_]iommu=off boot.

In a normal boot, ACS is requested due to amd_iommu_detect() requesting
IVRS_DETECTED.

    pci_request_acs+0x9/0x18
    iommu_go_to_state+0x106/0x1a20
    amd_iommu_detect+0x1c/0x50
    pci_iommu_alloc+0x26/0x40
    mm_core_init+0xa/0x120
    start_kernel+0x527/0x7a0
    x86_64_start_reservations+0x24/0x30
    x86_64_start_kernel+0xd1/0xe0
    common_startup_64+0x13e/0x158

This is intended to ensure ACS is requested before the PCI core
initialization, or else a !CONFIG_IRQ_REMAP, nointremap or intremap=off
boot would be broken.

However, in an amd_iommu=off boot, the state machine still requests ACS
at the exact same time, as amd_iommu_detect() has nothing to do with
amd_iommu_disabled.

Even worse, in an iommu=off boot, though amd_iommu_detect() bails out
early, ACS is still requested due to amd_iommu_prepare() requesting
IOMMU_ACPI_FINISHED, which is called by irq_remapping_prepare() thanks
to CONFIG_X86_LOCAL_APIC (always set on X86_64) and CONFIG_IRQ_REMAP
(enabled by defconfig), unless nointremap or intremap=off is also passed
to cmdline.

    pci_request_acs+0x9/0x18
    iommu_go_to_state+0x106/0x1a20
    amd_iommu_prepare+0x15/0x40
    irq_remapping_prepare+0x43/0x60
    enable_IR_x2apic+0x22/0x190
    x86_64_probe_apic+0xa/0x50
    apic_intr_mode_init+0x70/0xd0
    x86_late_time_init+0x28/0x40
    start_kernel+0x6f9/0x7a0
    ...

In both cases, [2] is still gated due to the [1] or [3] check, so that
IOMMU can be disabled per cmdline.

Technically, it makes no sense to detect IVRS at all in an
[amd_]iommu=off boot or if IOMMU is not supported due to platform
settings. This is probably why amd_iommu_detect() bails out before
requesting IVRS_DETECTED. Apparently only bailing out there is not
sufficient, and the bailing-out paths should really have been parts of
the state machine.

PATCH 1 cleans up the initialization routines by moving the bailing-out
paths and [1] to the right place in the state machine (i.e., before
[0]), and always requesting IVRS_DETECTED in amd_iommu_detect() to
initialize the state machine early and properly.

PATCH 2 disallows implicit START_STATE => IVRS_DETECTED transition, so
that amd_iommu_prepare() and other paths no longer progress the state
machine accidentally. This should also help prevent potential bugs if
more housekeeping work is added to amd_iommu_detect() in the future.

PATCH 3 removes all redundant ad-hoc checks, as they are now covered by
the state machine itself.

Signed-off-by: Rong Zhang <[email protected]>
---
Changes in v3:
- Drop the dead condition (iommu_detected && !gart_iommu_aperture) as
  now we always have iommu_detected == gart_iommu_aperture (thanks
  Sairaj Kodilkar)
- Link to v2: https://patch.msgid.link/[email protected]

Changes in v2:
- New patch in the series
  - PATCH 2 ("iommu/amd: Disallow implicit START_STATE => IVRS_DETECTED
    transition")
  - Prevent PATCH 3 from accidentally allowing AMD IOMMU to probe in Xen
    PV (thanks Ankit Soni)
- Drop Fixes: and Cc: stable (ditto)
- Rebase on top of the lastest changes
- Link to v1: https://patch.msgid.link/[email protected]

---
Rong Zhang (3):
      iommu/amd: Do not request ACS when IOMMU is not going to be initialized
      iommu/amd: Disallow implicit START_STATE => IVRS_DETECTED transition
      iommu/amd: Remove ad-hoc checks that are never true

 drivers/iommu/amd/init.c | 52 ++++++++++++++++++++++++------------------------
 1 file changed, 26 insertions(+), 26 deletions(-)
---
base-commit: 26260251022fbc2f248a3d747a9b2b961b18d2d8
change-id: d58d0f31-amd-iommu-fix-acs-8f3d4aba8183

Thanks,
Rong