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@rong.moe>
---
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

 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

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@rong.moe>
> ---
> 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?

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.

-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
>

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@rong.moe>
> > ---
> > 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
> > 

On Thu, Aug 20, 2026 at 01:38:35AM +0800, Rong Zhang wrote:
> 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
> > >        ...
> > > 

../..

> > > ---
> > > 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.

Agreed on patch 1 - the state machine is the right place for those checks and
I have no objection to it. My concern is only patch 2 dropping the
"amd_iommu_detected" check.

> 
> 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.

Commenting out amd_iommu_detect() is right way to emulate this, and the trace 
confirms the entry path. But the boot options are what make it
come out clean: amd_iommu=off / iommu=off set amd_iommu_disabled / no_iommu,
so IOMMU_START_STATE bails on its first if - that's the 0 => 9., which
is clearly visible above.

But,
Xen's own iommu= sits on the hypervisor command line and goes to Xen's parser,
not to Linux's iommu_setup(), and there no one sets no_iommu or amd_iommu_disabled.

The wider point is that amd_iommu_detect() never running is the platform mean
Linux does not own this IOMMU - Xen opts out of pci_iommu_alloc() and installs
xen_swiotlb_dma_ops instead. 'amd_iommu_detected' was the only thing making the
irq-remapping entry path honour that, so without it (i.e. patch 2 removes it) 
early_amd_iommu_init() proceeds on hardware the hypervisor owns.

-Ankit

> 
> 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
> > >

Hi Ankit,

On Thu, 2026-08-20 at 05:48 +0000, Ankit Soni wrote:
> On Thu, Aug 20, 2026 at 01:38:35AM +0800, Rong Zhang wrote:
> > 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
> > > >        ...
> > > > 
> 
> ../..
> 
> > > > ---
> > > > 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.
> 
> Agreed on patch 1 - the state machine is the right place for those checks and
> I have no objection to it. My concern is only patch 2 dropping the
> "amd_iommu_detected" check.
> 
> > 
> > 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.
> 
> Commenting out amd_iommu_detect() is right way to emulate this, and the trace 
> confirms the entry path. But the boot options are what make it
> come out clean: amd_iommu=off / iommu=off set amd_iommu_disabled / no_iommu,
> so IOMMU_START_STATE bails on its first if - that's the 0 => 9., which
> is clearly visible above.
> 
> But,
> Xen's own iommu= sits on the hypervisor command line and goes to Xen's parser,
> not to Linux's iommu_setup(), and there no one sets no_iommu or amd_iommu_disabled.
> 
> The wider point is that amd_iommu_detect() never running is the platform mean
> Linux does not own this IOMMU - Xen opts out of pci_iommu_alloc() and installs
> xen_swiotlb_dma_ops instead. 'amd_iommu_detected' was the only thing making the
> irq-remapping entry path honour that, so without it (i.e. patch 2 removes it) 
> early_amd_iommu_init() proceeds on hardware the hypervisor owns.

Ah, indeed! Skipping calling amd_iommu_detect() acts as a kill switch to
gate IOMMU initialization anyway, and `amd_iommu_detected' is used by in
this case.

That said, I believe relying on the flag to gate IOMMU initialization in
Xen PV is too late that detect_ivrs() has been called.

The whole purpose of the series is to prevent calling detect_ivrs() at
all when unnecessary. Although detect_ivrs() is effectively a no-op due
to Xen PV always requesting ACS, I still prefer completely skipping it in
Xen PV to prevent unnecessarily getting the IVRS table and accessing the
PCI config space.

After rethinking, I propose disallowing implicit START_STATE =>
IVRS_DETECTED transition. IOW, the IVRS_DETECTED state can only be
requested explicitly by amd_iommu_detect(). This should also help prevent
potential bugs if more housekeeping work is added to amd_iommu_detect()
in the future. In this manner, we should be able to safely remove
`amd_iommu_detected'.

This will make parts of PATCH 1 unnecessary, but I prefer leaving PATCH 1
as is it makes the intention clearer.

I will include the proposed change in v2.

Thanks,
Rong

> 
> -Ankit
> 
> > 
> > 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
> > > >