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

[Ankit Soni]

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

Reviewed-by: Ankit Soni <Ankit.Soni@xxxxxxx>

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