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

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 <[email protected]>
> ---
>  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;
>