Re: [PATCH 1/4] iommu/amd: Check SNP support before enabling IOMMU

Tom Lendacky
2025-01-22T15:23:01Z

On 1/21/25 19:00, Ashish Kalra wrote:
> From: Vasant Hegde <vasant.hegde@amd.com>
>
> iommu_snp_enable() checks for IOMMU feature support and page table
> compatibility. Ideally this check should be done before enabling
> IOMMUs. Currently its done after enabling IOMMUs. Also its causes

Why should it be done before enabling the IOMMUs? In other words, at
some more detail here.

> issue if kvm_amd is builtin.
>
> Hence move SNP enable check before enabling IOMMUs.
>
> Fixes: 04d65a9dbb33 ("iommu/amd: Don't rely on external callers to enable IOMMU SNP support")
> Cc: Ashish Kalra <ashish.kalra@amd.com>
> Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>

Ashish, as the submitter, this requires your Signed-off-by:.

> ---
> drivers/iommu/amd/init.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
> index c5cd92edada0..419a0bc8eeea 100644
> --- a/drivers/iommu/amd/init.c
> +++ b/drivers/iommu/amd/init.c
> @@ -3256,13 +3256,14 @@ static int __init state_next(void)
> }
> break;
> case IOMMU_ACPI_FINISHED:
> + /* SNP enable has to be called after early_amd_iommu_init() */

This comment doesn't really explain anything, so I think it should
either be improved or just remove it.

Thanks,
Tom

> + iommu_snp_enable();
> early_enable_iommus();
> x86_platform.iommu_shutdown = disable_iommus;
> init_state = IOMMU_ENABLED;
> break;
> case IOMMU_ENABLED:
> register_syscore_ops(&amd_iommu_syscore_ops);
> - iommu_snp_enable();
> ret = amd_iommu_init_pci();
> init_state = ret ? IOMMU_INIT_ERROR : IOMMU_PCI_INIT;
> break;