linux-kernel - Re: [PATCH 4/4] iommu/amd: Force SNP-enabled VFIO domain to 4K page size

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: <209a6ebd-b83c-d1c7-7e36-c109b09779f3@amd.com>
Date:   Tue, 17 Jan 2023 07:10:50 -0600
From:   Eric van Tassell <evantass@....com>
To:     Suravee Suthikulpanit <suravee.suthikulpanit@....com>,
        linux-kernel@...r.kernel.org, iommu@...ts.linux.dev
Subject: Re: [PATCH 4/4] iommu/amd: Force SNP-enabled VFIO domain to 4K page
 size


On 1/10/23 08:31, Suravee Suthikulpanit wrote:
> SNP only supports 2M and 4K page sizes. Other page sizes requires
> page smashing to supported sizes. For SNP-enabled guests
> with pass-through devices (via VFIO), it also requires RMP and IOMMU
> page sizes to match.
>
> To simplify the support, for SNP-enabled guest, SNP will smash guest pages
> to 4K page size only, and IOMMU driver will setup the IOMMU v1 page table
> for the VFIO domain of the guest to match the 4K page size.
>
> Co-developed-by: Vasant Hegde <vasant.hegde@....com>
> Signed-off-by: Vasant Hegde <vasant.hegde@....com>
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@....com>
> ---
>   drivers/iommu/amd/amd_iommu_types.h |  2 ++
>   drivers/iommu/amd/iommu.c           | 28 ++++++++++++++++++++++++++++
>   2 files changed, 30 insertions(+)
>
> diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
> index ad124959a26a..5249ac18ce6e 100644
> --- a/drivers/iommu/amd/amd_iommu_types.h
> +++ b/drivers/iommu/amd/amd_iommu_types.h
> @@ -279,6 +279,7 @@
>   #define AMD_IOMMU_PGSIZES	((~0xFFFUL) & ~(2ULL << 38))
>   /* 4K, 2MB, 1G page sizes are supported */
>   #define AMD_IOMMU_PGSIZES_V2	(PAGE_SIZE | (1ULL << 21) | (1ULL << 30))
> +#define AMD_IOMMU_PGSIZES_4K	(PAGE_SIZE)
>   
>   /* Bit value definition for dte irq remapping fields*/
>   #define DTE_IRQ_PHYS_ADDR_MASK	(((1ULL << 45)-1) << 6)
> @@ -440,6 +441,7 @@
>   #define PD_IOMMUV2_MASK		(1UL << 3) /* domain has gcr3 table */
>   #define PD_GIOV_MASK		(1UL << 4) /* domain enable GIOV support */
>   #define PD_VFIO_MASK		(1UL << 5) /* domain enable VFIO support */
> +#define PD_SNP_MASK		(1UL << 6) /* domain enable SNP support */
>   
>   extern bool amd_iommu_dump;
>   #define DUMP_printk(format, arg...)				\
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index a03723930f70..9a1b010a7d00 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -2422,6 +2422,33 @@ static bool amd_iommu_enforce_cache_coherency(struct iommu_domain *domain)
>   	return true;97921e769dda1
>   }
>   
> +static void amd_iommu_set_kvm(struct iommu_domain *domain, struct kvm *kvm)
> +{
> +	struct protection_domain *pdom = to_pdomain(domain);
> +
> +	if (!amd_iommu_snp_en || !amd_iommu_svm_ops ||
> +	    !pdom || !(pdom->flags & PD_VFIO_MASK))
> +		return;
> +
> +	/*
> +	 * The parameter kvm can be NULL when calling from kvm_vfio_group_del()
> +	 * and kvm_vfio_destroy().
> +	 */
> +	if (!kvm ||
> +	    !amd_iommu_svm_ops->is_snp_guest ||
> +	    !amd_iommu_svm_ops->is_snp_guest(kvm))
> +		return;
> +
> +	/*
> +	 * VFIO Domain for SNP guest requires IOMMU and RMP page-size to match,
> +	 * which can only support 4K and 2M. Currently, only support 4K
> +	 * IOMMU page-size.
> +	 */
> +	pdom->flags |= PD_SNP_MASK;
> +	pdom->domain.pgsize_bitmap = AMD_IOMMU_PGSIZES_4K;
> +	pr_debug("%s: Force domain %u page size to 4K.\n", __func__, pdom->id);
> +}
> +


In my opinion the name of this function is to generic and doesn't 
describe what it does.

I'd prefer something like amd_iommu_set_4k_pgsz()


>   const struct iommu_ops amd_iommu_ops = {
>   	.capable = amd_iommu_capable,
>   	.domain_alloc = amd_iommu_domain_alloc,
> @@ -2444,6 +2471,7 @@ const struct iommu_ops amd_iommu_ops = {
>   		.iotlb_sync	= amd_iommu_iotlb_sync,
>   		.free		= amd_iommu_domain_free,
>   		.enforce_cache_coherency = amd_iommu_enforce_cache_coherency,
> +		.set_kvm	= amd_iommu_set_kvm,
>   	}
>   };
>