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

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

[Rong Zhang]

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 <i@rong.moe>
---
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/20260811-amd-iommu-fix-acs-v1-0-d64e2172408d@rong.moe

---
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: 91ec2035134982b98fab0609a9fd8480e8217dc1
change-id: d58d0f31-amd-iommu-fix-acs-8f3d4aba8183

Thanks,
Rong

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

[Rong Zhang]

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.

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

Signed-off-by: Rong Zhang <i@rong.moe>
---
 drivers/iommu/amd/init.c | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index 40726dfef273..a720796cca3b 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -3470,6 +3470,8 @@ static void amd_iommu_apply_erratum_snp(void)
 #endif
 }
 
+static bool amd_iommu_sme_check(void);
+
 /****************************************************************************
  *
  * AMD IOMMU Initialization State Machine
@@ -3482,7 +3484,13 @@ static int __init state_next(void)
 
 	switch (init_state) {
 	case IOMMU_START_STATE:
-		if (!detect_ivrs()) {
+		if (no_iommu || amd_iommu_disabled) {
+			init_state	= IOMMU_CMDLINE_DISABLED;
+			ret		= -EINVAL;
+		} else if ((iommu_detected && !gart_iommu_aperture) || !amd_iommu_sme_check()) {
+			init_state	= IOMMU_INIT_ERROR;
+			ret		= -EINVAL;
+		} else if (!detect_ivrs()) {
 			init_state	= IOMMU_NOT_FOUND;
 			ret		= -ENODEV;
 		} else {
@@ -3490,13 +3498,8 @@ static int __init state_next(void)
 		}
 		break;
 	case IOMMU_IVRS_DETECTED:
-		if (amd_iommu_disabled) {
-			init_state = IOMMU_CMDLINE_DISABLED;
-			ret = -EINVAL;
-		} else {
-			ret = early_amd_iommu_init();
-			init_state = ret ? IOMMU_INIT_ERROR : IOMMU_ACPI_FINISHED;
-		}
+		ret = early_amd_iommu_init();
+		init_state = ret ? IOMMU_INIT_ERROR : IOMMU_ACPI_FINISHED;
 		break;
 	case IOMMU_ACPI_FINISHED:
 		early_enable_iommus();
@@ -3695,12 +3698,6 @@ void __init amd_iommu_detect(void)
 {
 	int ret;
 
-	if (no_iommu || (iommu_detected && !gart_iommu_aperture))
-		goto disable_snp;
-
-	if (!amd_iommu_sme_check())
-		goto disable_snp;
-
 	ret = iommu_go_to_state(IOMMU_IVRS_DETECTED);
 	if (ret)
 		goto disable_snp;

-- 
2.55.0

[PATCH v2 2/3] iommu/amd: Disallow implicit START_STATE => IVRS_DETECTED transition

[Rong Zhang]

Xen PV intentionally skips calling amd_iommu_detect() in order to gate
IOMMU initialization, as the Xen hypervisor has taken over the hardware.
However, due to amd_iommu_prepare() requesting ACPI_FINISHED while
setting up APIC, the state machine is still progressed, only to be
stopped midway due to the `!amd_iommu_detected' check in
early_amd_iommu_init(). This is worthless and fragile. It unnecessarily
calls detect_ivrs(), which leads to extra overhead, i.e., getting the
IVRS table and accessing the PCI config space.

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

Signed-off-by: Rong Zhang <i@rong.moe>
---
 drivers/iommu/amd/init.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index a720796cca3b..0e63ad653284 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -3571,6 +3571,19 @@ static int __init iommu_go_to_state(enum iommu_init_state state)
 {
 	int ret = -EINVAL;
 
+	/*
+	 * Some essential housekeeping work is done by amd_iommu_detect().
+	 * Skipping calling it implies that the platform (e.g., Xen hypervisor)
+	 * has taken over the hardware. Progressing the state machine in this
+	 * case is worthless and fragile.
+	 *
+	 * There are several paths requesting later states, so disallow implicit
+	 * START_STATE => IVRS_DETECTED transition to prevent these paths from
+	 * accidentally progressing the state machine.
+	 */
+	if (init_state == IOMMU_START_STATE && state != IOMMU_IVRS_DETECTED)
+		goto out;
+
 	while (init_state != state) {
 		if (init_state == IOMMU_NOT_FOUND         ||
 		    init_state == IOMMU_INIT_ERROR        ||
@@ -3579,6 +3592,7 @@ static int __init iommu_go_to_state(enum iommu_init_state state)
 		ret = state_next();
 	}
 
+out:
 	/*
 	 * SNP platform initilazation requires IOMMUs to be fully configured.
 	 * If the SNP support on IOMMUs has NOT been checked, simply mark SNP

-- 
2.55.0

[PATCH v2 3/3] iommu/amd: Remove ad-hoc checks that are never true

[Rong Zhang]

The state transitions of the AMD IOMMU Initialization State Machine
imply that some ad-hoc checks will never be true. In detail, they are
only reachable if the previous state transitions have succeeded, but the
conditions are only true if the previous state transitions have failed.

Therefore, remove the redundant ad-hoc checks.

Signed-off-by: Rong Zhang <i@rong.moe>
---
 drivers/iommu/amd/init.c | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index 0e63ad653284..9ecefe12aabf 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -163,7 +163,6 @@ int amd_iommu_gpt_level = PAGE_MODE_4_LEVEL;
 int amd_iommu_guest_ir = AMD_IOMMU_GUEST_IR_VAPIC;
 static int amd_iommu_xt_mode = IRQ_REMAP_XAPIC_MODE;
 
-static bool amd_iommu_detected;
 static bool amd_iommu_disabled __initdata;
 static bool amd_iommu_force_enable __initdata;
 static bool amd_iommu_irtcachedis;
@@ -3222,9 +3221,6 @@ static int __init early_amd_iommu_init(void)
 	acpi_status status;
 	u8 efr_hats, max_vasize;
 
-	if (!amd_iommu_detected)
-		return -ENODEV;
-
 	status = acpi_get_table("IVRS", 0, &ivrs_base);
 	if (status == AE_NOT_FOUND)
 		return -ENODEV;
@@ -3303,7 +3299,7 @@ static int __init early_amd_iommu_init(void)
 	}
 
 	/* Disable any previously enabled IOMMUs */
-	if (!is_kdump_kernel() || amd_iommu_disabled)
+	if (!is_kdump_kernel())
 		disable_iommus();
 
 	if (amd_iommu_irq_remap)
@@ -3398,12 +3394,6 @@ static __init void iommu_snp_enable(void)
 	if (!cc_platform_has(CC_ATTR_HOST_SEV_SNP))
 		return;
 
-	/* SNP support required IOMMU to be ON */
-	if (no_iommu) {
-		pr_warn("SNP: IOMMU disabled, SNP cannot be supported.\n");
-		goto disable_snp;
-	}
-
 	amd_iommu_snp_mode0_sup = check_feature2(FEATURE_SNP_PAGE_MODE0_SUP);
 	/*
 	 * If SNP page mode 0 is not enabled, then SNP support requires that IOMMU
@@ -3716,7 +3706,6 @@ void __init amd_iommu_detect(void)
 	if (ret)
 		goto disable_snp;
 
-	amd_iommu_detected = true;
 	iommu_detected = 1;
 	x86_init.iommu.iommu_init = amd_iommu_init;
 	return;

-- 
2.55.0

Re: [PATCH v2 1/3] iommu/amd: Do not request ACS when IOMMU is not going to be initialized

[Sairaj Kodilkar]

On 8/20/2026 11:55 PM, 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.
> 
> Clean 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.
> 
> Signed-off-by: Rong Zhang <i@rong.moe>
> ---
>  drivers/iommu/amd/init.c | 25 +++++++++++--------------
>  1 file changed, 11 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
> index 40726dfef273..a720796cca3b 100644
> --- a/drivers/iommu/amd/init.c
> +++ b/drivers/iommu/amd/init.c
> @@ -3470,6 +3470,8 @@ static void amd_iommu_apply_erratum_snp(void)
>  #endif
>  }
>  
> +static bool amd_iommu_sme_check(void);
> +
>  /****************************************************************************
>   *
>   * AMD IOMMU Initialization State Machine
> @@ -3482,7 +3484,13 @@ static int __init state_next(void)
>  
>  	switch (init_state) {
>  	case IOMMU_START_STATE:
> -		if (!detect_ivrs()) {
> +		if (no_iommu || amd_iommu_disabled) {
> +			init_state	= IOMMU_CMDLINE_DISABLED;
> +			ret		= -EINVAL;
> +		} else if ((iommu_detected && !gart_iommu_aperture) || !amd_iommu_sme_check()) {

Looks like you can drop the condition
(iommu_detected && !gart_iommu_aperture)

It was added by the commit 6631ee9d00, which set iommu_detected = 1 and
gart_iommu_aperture = 0 in iommu driver. But it is later removed and no
longer present in the lastest iommu code.

Latest code can have following two conditions

1. When CONFIG_GART_IOMMU=y
x86/kernel/aperture_64.c sets both iommu_detected and
gart_iommu_aperture to 1

2. When CONFIG_GART_IOMMU=n
amd_iommu_detect is called only when both iommu_detected and
gart_iommu_aperture are zero.

Thanks
Sairaj.



> +			init_state	= IOMMU_INIT_ERROR;
> +			ret		= -EINVAL;
> +		} else if (!detect_ivrs()) {
>  			init_state	= IOMMU_NOT_FOUND;
>  			ret		= -ENODEV;
>  		} else {
> @@ -3490,13 +3498,8 @@ static int __init state_next(void)
>  		}
>  		break;
>  	case IOMMU_IVRS_DETECTED:
> -		if (amd_iommu_disabled) {
> -			init_state = IOMMU_CMDLINE_DISABLED;
> -			ret = -EINVAL;
> -		} else {
> -			ret = early_amd_iommu_init();
> -			init_state = ret ? IOMMU_INIT_ERROR : IOMMU_ACPI_FINISHED;
> -		}
> +		ret = early_amd_iommu_init();
> +		init_state = ret ? IOMMU_INIT_ERROR : IOMMU_ACPI_FINISHED;
>  		break;
>  	case IOMMU_ACPI_FINISHED:
>  		early_enable_iommus();
> @@ -3695,12 +3698,6 @@ void __init amd_iommu_detect(void)
>  {
>  	int ret;
>  
> -	if (no_iommu || (iommu_detected && !gart_iommu_aperture))
> -		goto disable_snp;
> -
> -	if (!amd_iommu_sme_check())
> -		goto disable_snp;
> -
>  	ret = iommu_go_to_state(IOMMU_IVRS_DETECTED);
>  	if (ret)
>  		goto disable_snp;
> 

Re: [PATCH v2 1/3] iommu/amd: Do not request ACS when IOMMU is not going to be initialized

[Rong Zhang]

Hi Sairaj,

On Fri, 2026-08-21 at 16:12 +0530, Sairaj Kodilkar wrote:
> On 8/20/2026 11:55 PM, 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.
> > 
> > Clean 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.
> > 
> > Signed-off-by: Rong Zhang <i@rong.moe>
> > ---
> >  drivers/iommu/amd/init.c | 25 +++++++++++--------------
> >  1 file changed, 11 insertions(+), 14 deletions(-)
> > 
> > diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
> > index 40726dfef273..a720796cca3b 100644
> > --- a/drivers/iommu/amd/init.c
> > +++ b/drivers/iommu/amd/init.c
> > @@ -3470,6 +3470,8 @@ static void amd_iommu_apply_erratum_snp(void)
> >  #endif
> >  }
> >  
> > +static bool amd_iommu_sme_check(void);
> > +
> >  /****************************************************************************
> >   *
> >   * AMD IOMMU Initialization State Machine
> > @@ -3482,7 +3484,13 @@ static int __init state_next(void)
> >  
> >  	switch (init_state) {
> >  	case IOMMU_START_STATE:
> > -		if (!detect_ivrs()) {
> > +		if (no_iommu || amd_iommu_disabled) {
> > +			init_state	= IOMMU_CMDLINE_DISABLED;
> > +			ret		= -EINVAL;
> > +		} else if ((iommu_detected && !gart_iommu_aperture) || !amd_iommu_sme_check()) {
> 
> Looks like you can drop the condition
> (iommu_detected && !gart_iommu_aperture)
> 
> It was added by the commit 6631ee9d00, which set iommu_detected = 1 and
> gart_iommu_aperture = 0 in iommu driver. But it is later removed and no
> longer present in the lastest iommu code.
> 
> Latest code can have following two conditions
> 
> 1. When CONFIG_GART_IOMMU=y
> x86/kernel/aperture_64.c sets both iommu_detected and
> gart_iommu_aperture to 1
> 
> 2. When CONFIG_GART_IOMMU=n
> amd_iommu_detect is called only when both iommu_detected and
> gart_iommu_aperture are zero.

Makes sense. Will clean it up in v3.

Thanks,
Rong

> 
> Thanks
> Sairaj.
> 
> 
> 
> > +			init_state	= IOMMU_INIT_ERROR;
> > +			ret		= -EINVAL;
> > +		} else if (!detect_ivrs()) {
> >  			init_state	= IOMMU_NOT_FOUND;
> >  			ret		= -ENODEV;
> >  		} else {
> > @@ -3490,13 +3498,8 @@ static int __init state_next(void)
> >  		}
> >  		break;
> >  	case IOMMU_IVRS_DETECTED:
> > -		if (amd_iommu_disabled) {
> > -			init_state = IOMMU_CMDLINE_DISABLED;
> > -			ret = -EINVAL;
> > -		} else {
> > -			ret = early_amd_iommu_init();
> > -			init_state = ret ? IOMMU_INIT_ERROR : IOMMU_ACPI_FINISHED;
> > -		}
> > +		ret = early_amd_iommu_init();
> > +		init_state = ret ? IOMMU_INIT_ERROR : IOMMU_ACPI_FINISHED;
> >  		break;
> >  	case IOMMU_ACPI_FINISHED:
> >  		early_enable_iommus();
> > @@ -3695,12 +3698,6 @@ void __init amd_iommu_detect(void)
> >  {
> >  	int ret;
> >  
> > -	if (no_iommu || (iommu_detected && !gart_iommu_aperture))
> > -		goto disable_snp;
> > -
> > -	if (!amd_iommu_sme_check())
> > -		goto disable_snp;
> > -
> >  	ret = iommu_go_to_state(IOMMU_IVRS_DETECTED);
> >  	if (ret)
> >  		goto disable_snp;
> >