Re: [PATCH 0/2] iommu/amd: Do not request ACS when IOMMU is not going to be initialized
From: Rong Zhang
Date: Wed Aug 19 2026 - 13:44:48 EST
Hi Ankit,
Thanks for your review.
On Wed, 2026-08-19 at 10:32 +0000, Ankit Soni wrote:
> On Tue, Aug 11, 2026 at 04:36:17AM +0800, Rong Zhang wrote:
> > 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.
> >
> > Fix it 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.
> > This doesn't change the IOMMU initialization/failure sequence since
> > commit 9f81ca8d1fd6 ("iommu/amd: Don't call early_amd_iommu_init() when
> > AMD IOMMU is disabled").
> >
> > Signed-off-by: Rong Zhang <i@xxxxxxxx>
> > ---
> > Rong Zhang (2):
> > iommu/amd: Do not request ACS when IOMMU is not going to be initialized
> > iommu/amd: Remove ad-hoc checks that are never true
>
> Hi,
>
> Thanks for the cleanup, and the analysis of the ACS call paths matches what I see.
> Two things worth a look before this goes in.
>
> 1. Patch 2: I don't think the !amd_iommu_detected check in
> early_amd_iommu_init() is unreachable. pci_iommu_alloc() returns early for
> xen_pv_domain() before calling amd_iommu_detect(), but Xen PV still sets
>
> x86_init.irqs.intr_mode_init = x86_64_probe_apic;
>
> so x86_64_probe_apic() -> enable_IR_x2apic() -> irq_remapping_prepare() ->
> amd_iommu_prepare() enters the state machine anyway, and none of the new
> IOMMU_START_STATE conditions are true. That check looks like
> the only thing currently stopping early_amd_iommu_init() from running
> there. Could you double-check that case before dropping it?
This has been resolved by PATCH 1, where the checks have been embedded
into the state machine.
I did some quick tests by commenting out the call to amd_iommu_detect()
in pci_iommu_alloc(), and adding a debug WARN() to iommu_go_to_state().
When I booted a VM with emulated IOMMU (-device amd-iommu) and with
either `amd_iommu=off' or `iommu=off':
[ 0.081948] APIC: Switch to symmetric I/O mode setup
[ 0.082553] ------------[ cut here ]------------
[ 0.083144] init_state: 0 => 9, ret: -22
[ 0.083617] WARNING: drivers/iommu/amd/init.c:3506 at iommu_go_to_state+0xf18/0x1c00, CPU#0: swapper/0/0
[ 0.084809] Modules linked in:
[ 0.085122] CPU: 0 UID: 0 PID: 0 Comm: swapper/0 Not tainted 7.2.0-02579-ge0f12714fa53-dirty #38 PREEMPT(lazy)
[ 0.086124] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.17.0-debian-1.17.0-1 04/01/2014
[ 0.087076] RIP: 0010:iommu_go_to_state+0xf23/0x1c00
[ 0.087629] Code: 1b e7 2c 00 eb 12 c7 05 0f e7 2c 00 06 00 00 00 eb 03 90 0f 0b 45 31 f6 48 8d 3d e8 90 b6 ff 8b 15 fa e6 2c 00 44 89 f1 89 de <67> 48 0f b9 3a 45 85 f6 0f 84 f4 f0 ff ff e8 8a c0 73 fe 48 8b 05
[ 0.089556] RSP: 0000:ffffffff93a03e40 EFLAGS: 00010202
[ 0.090039] RAX: 00000000fffffff9 RBX: 0000000000000000 RCX: 00000000ffffffea
[ 0.090677] RDX: 0000000000000009 RSI: 0000000000000000 RDI: ffffffff93d17440
[ 0.091338] RBP: ffff8fbd47fd5c80 R08: 3fffffffffffdfff R09: ffffffff93b20aa8
[ 0.092025] R10: 0000000000000002 R11: 0000000000000002 R12: 000000000000001b
[ 0.092662] R13: 0000000000000000 R14: 00000000ffffffea R15: 0000000000014770
[ 0.093319] FS: 0000000000000000(0000) GS:ffff8fbdb3634000(0000) knlGS:0000000000000000
[ 0.094055] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 0.094572] CR2: ffff8fbd4728f000 CR3: 0000000006844000 CR4: 00000000000100b0
[ 0.095214] Call Trace:
[ 0.095437] <TASK>
[ 0.095628] amd_iommu_prepare+0x15/0x40
[ 0.096025] irq_remapping_prepare+0x43/0x60
[ 0.096406] enable_IR_x2apic+0x22/0x190
[ 0.096759] x86_64_probe_apic+0xa/0x50
[ 0.097130] apic_intr_mode_init+0x70/0xd0
[ 0.097495] x86_late_time_init+0x28/0x40
[ 0.097858] start_kernel+0x6f9/0x7a0
[ 0.098209] x86_64_start_reservations+0x24/0x30
[ 0.098629] x86_64_start_kernel+0xd1/0xe0
[ 0.099016] common_startup_64+0x13e/0x158
[ 0.099393] </TASK>
[ 0.099595] ---[ end trace 0000000000000000 ]---
[ 0.100266] x2apic enabled
[ 0.100758] APIC: Switched APIC routing to: physical x2apic
...where 0 is IOMMU_START_STATE and 9 is IOMMU_CMDLINE_DISABLED.
Therefore, embedding the checks into the state machine ensures the checks
are done in all initialization paths and makes replicating the same
checks in state transition methods unnecessary.
>
> 2. Patch 1 Fixes: 9f81ca8d1fd6 doesn't look right - it only moved the
> amd_iommu_disabled check ahead of early_amd_iommu_init() and left
> IOMMU_START_STATE and detect_ivrs() alone, so ACS was requested with
> amd_iommu=off both before and after it. The commit that actually put
> pci_request_acs() on the detect path is 1adb7d31b051 ("iommu/amd: Fix
> pci_request_acs() call-place"). I'd drop Fixes: and Cc: stable and post
> this as a cleanup.
Makes sense. Will drop them when resubmitting. I will defer to your
opinion on my explanation to the first question before resubmitting :)
Thanks,
Rong
>
> -Ankit
>
> >
> > drivers/iommu/amd/init.c | 36 ++++++++++++++----------------------
> > 1 file changed, 14 insertions(+), 22 deletions(-)
> > ---
> > base-commit: db2ddb87143519e20a95aa36c60b36107b736a58
> > change-id: d58d0f31-amd-iommu-fix-acs-8f3d4aba8183
> >
> > Thanks,
> > Rong
> >