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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c310e42d-d8a8-4ca0-f308-e5bb4e978002@amd.com>
Date: Wed, 22 Jan 2025 09:22:52 -0600
From: Tom Lendacky <thomas.lendacky@....com>
To: Ashish Kalra <Ashish.Kalra@....com>, seanjc@...gle.com,
 pbonzini@...hat.com, tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
 dave.hansen@...ux.intel.com, x86@...nel.org, hpa@...or.com,
 john.allen@....com, herbert@...dor.apana.org.au, davem@...emloft.net,
 joro@...tes.org, suravee.suthikulpanit@....com, will@...nel.org,
 robin.murphy@....com
Cc: michael.roth@....com, dionnaglaze@...gle.com, vasant.hegde@....com,
 kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-crypto@...r.kernel.org, linux-coco@...ts.linux.dev,
 iommu@...ts.linux.dev
Subject: Re: [PATCH 1/4] iommu/amd: Check SNP support before enabling IOMMU

On 1/21/25 19:00, Ashish Kalra wrote:
> From: Vasant Hegde <vasant.hegde@....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@....com>
> Signed-off-by: Vasant Hegde <vasant.hegde@....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;