* [PATCH v3 1/3] iommu/amd: Do not request ACS when IOMMU is not going to be initialized
2026-08-22 22:20 [PATCH v3 0/3] iommu/amd: Do not request ACS when IOMMU is not going to be initialized Rong Zhang
@ 2026-08-22 22:20 ` Rong Zhang
2026-08-28 6:44 ` Ankit Soni
2026-08-22 22:20 ` [PATCH v3 2/3] iommu/amd: Disallow implicit START_STATE => IVRS_DETECTED transition Rong Zhang
` (3 subsequent siblings)
4 siblings, 1 reply; 9+ messages in thread
From: Rong Zhang @ 2026-08-22 22:20 UTC (permalink / raw)
To: Joerg Roedel (AMD),
Suravee Suthikulpanit, Vasant Hegde, Will Deacon, Robin Murphy,
Huang Rui
Cc: Ankit Soni, Sairaj Kodilkar, iommu, linux-kernel, 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.
Note that the condition (iommu_detected && !gart_iommu_aperture) is
dropped as it's dead code since the relevant code being removed. It
meant to detect what commit 6631ee9d0099 ("x86, AMD IOMMU: add
dma_ops initialization function") had introduced, but commit
d7f077697533 ("x86/amd-iommu: Fall back to GART if initialization
fails") has removed it. Later, commit 78013eaadf69 ("x86: remove the
IOMMU table infrastructure") ensured AMD IOMMU being detected before
Intel's and led to iommu_detected == gart_iommu_aperture in any case.
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..0fd0801a873f 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 (!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
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v3 1/3] iommu/amd: Do not request ACS when IOMMU is not going to be initialized
2026-08-22 22:20 ` [PATCH v3 1/3] " Rong Zhang
@ 2026-08-28 6:44 ` Ankit Soni
0 siblings, 0 replies; 9+ messages in thread
From: Ankit Soni @ 2026-08-28 6:44 UTC (permalink / raw)
To: Rong Zhang
Cc: Joerg Roedel (AMD),
Suravee Suthikulpanit, Vasant Hegde, Will Deacon, Robin Murphy,
Huang Rui, Sairaj Kodilkar, iommu, linux-kernel
On Sun, Aug 23, 2026 at 06:20:15AM +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.
>
> 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.
>
> Note that the condition (iommu_detected && !gart_iommu_aperture) is
> dropped as it's dead code since the relevant code being removed. It
> meant to detect what commit 6631ee9d0099 ("x86, AMD IOMMU: add
> dma_ops initialization function") had introduced, but commit
> d7f077697533 ("x86/amd-iommu: Fall back to GART if initialization
> fails") has removed it. Later, commit 78013eaadf69 ("x86: remove the
> IOMMU table infrastructure") ensured AMD IOMMU being detected before
> Intel's and led to iommu_detected == gart_iommu_aperture in any case.
>
> Signed-off-by: Rong Zhang <i@rong.moe>
Reviewed-by: Ankit Soni <Ankit.Soni@amd.com>
> ---
> 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..0fd0801a873f 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 (!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
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3 2/3] iommu/amd: Disallow implicit START_STATE => IVRS_DETECTED transition
2026-08-22 22:20 [PATCH v3 0/3] iommu/amd: Do not request ACS when IOMMU is not going to be initialized Rong Zhang
2026-08-22 22:20 ` [PATCH v3 1/3] " Rong Zhang
@ 2026-08-22 22:20 ` Rong Zhang
2026-08-28 6:47 ` Ankit Soni
2026-08-22 22:20 ` [PATCH v3 3/3] iommu/amd: Remove ad-hoc checks that are never true Rong Zhang
` (2 subsequent siblings)
4 siblings, 1 reply; 9+ messages in thread
From: Rong Zhang @ 2026-08-22 22:20 UTC (permalink / raw)
To: Joerg Roedel (AMD),
Suravee Suthikulpanit, Vasant Hegde, Will Deacon, Robin Murphy,
Huang Rui
Cc: Ankit Soni, Sairaj Kodilkar, iommu, linux-kernel, 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 0fd0801a873f..72b5b11b9d75 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
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v3 2/3] iommu/amd: Disallow implicit START_STATE => IVRS_DETECTED transition
2026-08-22 22:20 ` [PATCH v3 2/3] iommu/amd: Disallow implicit START_STATE => IVRS_DETECTED transition Rong Zhang
@ 2026-08-28 6:47 ` Ankit Soni
0 siblings, 0 replies; 9+ messages in thread
From: Ankit Soni @ 2026-08-28 6:47 UTC (permalink / raw)
To: Rong Zhang
Cc: Joerg Roedel (AMD),
Suravee Suthikulpanit, Vasant Hegde, Will Deacon, Robin Murphy,
Huang Rui, Sairaj Kodilkar, iommu, linux-kernel
On Sun, Aug 23, 2026 at 06:20:16AM +0800, Rong Zhang wrote:
> 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>
Reviewed-by: Ankit Soni <Ankit.Soni@amd.com>
> ---
> 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 0fd0801a873f..72b5b11b9d75 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
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3 3/3] iommu/amd: Remove ad-hoc checks that are never true
2026-08-22 22:20 [PATCH v3 0/3] iommu/amd: Do not request ACS when IOMMU is not going to be initialized Rong Zhang
2026-08-22 22:20 ` [PATCH v3 1/3] " Rong Zhang
2026-08-22 22:20 ` [PATCH v3 2/3] iommu/amd: Disallow implicit START_STATE => IVRS_DETECTED transition Rong Zhang
@ 2026-08-22 22:20 ` Rong Zhang
2026-08-28 15:53 ` Ankit Soni
2026-09-19 11:17 ` [PATCH v3 0/3] iommu/amd: Do not request ACS when IOMMU is not going to be initialized Rong Zhang
2026-09-24 11:21 ` Joerg Roedel (AMD)
4 siblings, 1 reply; 9+ messages in thread
From: Rong Zhang @ 2026-08-22 22:20 UTC (permalink / raw)
To: Joerg Roedel (AMD),
Suravee Suthikulpanit, Vasant Hegde, Will Deacon, Robin Murphy,
Huang Rui
Cc: Ankit Soni, Sairaj Kodilkar, iommu, linux-kernel, 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 72b5b11b9d75..d81adb8efe0e 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
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v3 3/3] iommu/amd: Remove ad-hoc checks that are never true
2026-08-22 22:20 ` [PATCH v3 3/3] iommu/amd: Remove ad-hoc checks that are never true Rong Zhang
@ 2026-08-28 15:53 ` Ankit Soni
0 siblings, 0 replies; 9+ messages in thread
From: Ankit Soni @ 2026-08-28 15:53 UTC (permalink / raw)
To: Rong Zhang
Cc: Joerg Roedel (AMD),
Suravee Suthikulpanit, Vasant Hegde, Will Deacon, Robin Murphy,
Huang Rui, Sairaj Kodilkar, iommu, linux-kernel
On Sun, Aug 23, 2026 at 06:20:17AM +0800, Rong Zhang wrote:
> 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>
Reviewed-by: Ankit Soni <Ankit.Soni@amd.com>
> ---
> 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 72b5b11b9d75..d81adb8efe0e 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
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 0/3] iommu/amd: Do not request ACS when IOMMU is not going to be initialized
2026-08-22 22:20 [PATCH v3 0/3] iommu/amd: Do not request ACS when IOMMU is not going to be initialized Rong Zhang
` (2 preceding siblings ...)
2026-08-22 22:20 ` [PATCH v3 3/3] iommu/amd: Remove ad-hoc checks that are never true Rong Zhang
@ 2026-09-19 11:17 ` Rong Zhang
2026-09-24 11:21 ` Joerg Roedel (AMD)
4 siblings, 0 replies; 9+ messages in thread
From: Rong Zhang @ 2026-09-19 11:17 UTC (permalink / raw)
To: Joerg Roedel (AMD),
Suravee Suthikulpanit, Vasant Hegde, Will Deacon, Robin Murphy,
Huang Rui
Cc: Ankit Soni, Sairaj Kodilkar, iommu, linux-kernel
Hi All,
On Sun, 2026-08-23 at 06:20 +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.
>
> 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>
Gentle ping.
Thanks,
Rong
> ---
> 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/20260821-amd-iommu-fix-acs-v2-0-982472452638@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: 26260251022fbc2f248a3d747a9b2b961b18d2d8
> change-id: d58d0f31-amd-iommu-fix-acs-8f3d4aba8183
>
> Thanks,
> Rong
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v3 0/3] iommu/amd: Do not request ACS when IOMMU is not going to be initialized
2026-08-22 22:20 [PATCH v3 0/3] iommu/amd: Do not request ACS when IOMMU is not going to be initialized Rong Zhang
` (3 preceding siblings ...)
2026-09-19 11:17 ` [PATCH v3 0/3] iommu/amd: Do not request ACS when IOMMU is not going to be initialized Rong Zhang
@ 2026-09-24 11:21 ` Joerg Roedel (AMD)
4 siblings, 0 replies; 9+ messages in thread
From: Joerg Roedel (AMD) @ 2026-09-24 11:21 UTC (permalink / raw)
To: Rong Zhang
Cc: Suravee Suthikulpanit, Vasant Hegde, Will Deacon, Robin Murphy,
Huang Rui, Ankit Soni, Sairaj Kodilkar, iommu, linux-kernel
On Sun, Aug 23, 2026 at 06:20:14AM +0800, Rong Zhang wrote:
> 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
Applied, thanks.
^ permalink raw reply [flat|nested] 9+ messages in thread