2023-01-10 14:44:58

by Suthikulpanit, Suravee

[permalink] [raw]
Subject: [PATCH 0/4] iommu/amd: Force SNP-enabled VFIO domain to 4K page size

To support VFIO pass-through device with SNP-enabled guest, IOMMU needs to
setup IOMMU page table with matching page size to the RMP. In order for
the IOMMU driver to setup page table appropriately, it needs to determine:

1. If an IOMMU domain is a VFIO domain (PATCH 1)
2. If an IOMMU domain belongs to an SNP-enabled guest (PATCH 2,3)
3. Appropriate page size the IOMMU domain (PATCH 4)

Please note that patch 2/4 is a preparatory patch for an upcoming series
to support SNP, which implements the call-back for the struct
amd_iommu_svm_ops.is_snp_guest().

Best Regards,
Suravee

Suravee Suthikulpanit (4):
iommu/amd: Introduce Protection-domain flag VFIO
iommu/amd: Introduce structure amd_iommu_svm_ops.is_snp_guest()
iommu: Introduce IOMMU call-back for processing struct KVM assigned to
VFIO
iommu/amd: Force SNP-enabled VFIO domain to 4K page size

drivers/iommu/amd/amd_iommu_types.h | 3 ++
drivers/iommu/amd/iommu.c | 45 +++++++++++++++++++++++++++--
drivers/iommu/iommu.c | 10 +++++++
drivers/vfio/vfio_main.c | 1 +
include/linux/amd-iommu.h | 6 ++++
include/linux/iommu.h | 4 +++
6 files changed, 67 insertions(+), 2 deletions(-)

--
2.32.0


2023-01-10 14:48:44

by Suthikulpanit, Suravee

[permalink] [raw]
Subject: [PATCH 4/4] iommu/amd: Force SNP-enabled VFIO domain to 4K page size

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 <[email protected]>
Signed-off-by: Vasant Hegde <[email protected]>
Signed-off-by: Suravee Suthikulpanit <[email protected]>
---
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;
}

+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);
+}
+
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,
}
};

--
2.32.0

2023-01-10 14:48:58

by Suthikulpanit, Suravee

[permalink] [raw]
Subject: [PATCH 2/4] iommu/amd: Introduce structure amd_iommu_svm_ops.is_snp_guest()

The structure can be used to provide call-back functions, which
are called by the AMD IOMMU driver when it needs to handle certain
operations.

SVM driver can define handler for each operation in the struct
amd_iommu_svm_ops and register the structure using the
amd_iommu_register_svm_ops() function.

Initially, it contains a function pointer is_snp_guest(), which
is used to check whether SNP is active for a KVM guest.

Signed-off-by: Suravee Suthikulpanit <[email protected]>
---
drivers/iommu/amd/iommu.c | 10 ++++++++++
include/linux/amd-iommu.h | 6 ++++++
2 files changed, 16 insertions(+)

diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 681ab1fdb7d5..a03723930f70 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -754,6 +754,16 @@ static void iommu_poll_ppr_log(struct amd_iommu *iommu)
}
}

+static const struct amd_iommu_svm_ops *amd_iommu_svm_ops;
+
+int amd_iommu_register_svm_ops(const struct amd_iommu_svm_ops *ops)
+{
+ amd_iommu_svm_ops = ops;
+
+ return 0;
+}
+EXPORT_SYMBOL(amd_iommu_register_svm_ops);
+
#ifdef CONFIG_IRQ_REMAP
static int (*iommu_ga_log_notifier)(u32);

diff --git a/include/linux/amd-iommu.h b/include/linux/amd-iommu.h
index 953e6f12fa1c..d4837af75550 100644
--- a/include/linux/amd-iommu.h
+++ b/include/linux/amd-iommu.h
@@ -11,6 +11,7 @@
#include <linux/types.h>

struct amd_iommu;
+struct kvm;

/*
* This is mainly used to communicate information back-and-forth
@@ -26,6 +27,10 @@ struct amd_iommu_pi_data {
void *ir_data;
};

+struct amd_iommu_svm_ops {
+ bool (*is_snp_guest)(struct kvm *kvm);
+};
+
#ifdef CONFIG_AMD_IOMMU

struct task_struct;
@@ -205,6 +210,7 @@ int amd_iommu_pc_set_reg(struct amd_iommu *iommu, u8 bank, u8 cntr, u8 fxn,
int amd_iommu_pc_get_reg(struct amd_iommu *iommu, u8 bank, u8 cntr, u8 fxn,
u64 *value);
struct amd_iommu *get_amd_iommu(unsigned int idx);
+int amd_iommu_register_svm_ops(const struct amd_iommu_svm_ops *ops);

#ifdef CONFIG_AMD_MEM_ENCRYPT
int amd_iommu_snp_enable(void);
--
2.32.0

2023-01-16 13:52:14

by Eric van Tassell

[permalink] [raw]
Subject: Re: [PATCH 0/4] iommu/amd: Force SNP-enabled VFIO domain to 4K page size

what commit should this series apply onto?


-evt

On 1/10/23 08:31, Suravee Suthikulpanit wrote:
> To support VFIO pass-through device with SNP-enabled guest, IOMMU needs to
> setup IOMMU page table with matching page size to the RMP. In order for
> the IOMMU driver to setup page table appropriately, it needs to determine:
>
> 1. If an IOMMU domain is a VFIO domain (PATCH 1)
> 2. If an IOMMU domain belongs to an SNP-enabled guest (PATCH 2,3)
> 3. Appropriate page size the IOMMU domain (PATCH 4)
>
> Please note that patch 2/4 is a preparatory patch for an upcoming series
> to support SNP, which implements the call-back for the struct
> amd_iommu_svm_ops.is_snp_guest().
>
> Best Regards,
> Suravee
>
> Suravee Suthikulpanit (4):
> iommu/amd: Introduce Protection-domain flag VFIO
> iommu/amd: Introduce structure amd_iommu_svm_ops.is_snp_guest()
> iommu: Introduce IOMMU call-back for processing struct KVM assigned to
> VFIO
> iommu/amd: Force SNP-enabled VFIO domain to 4K page size
>
> drivers/iommu/amd/amd_iommu_types.h | 3 ++
> drivers/iommu/amd/iommu.c | 45 +++++++++++++++++++++++++++--
> drivers/iommu/iommu.c | 10 +++++++
> drivers/vfio/vfio_main.c | 1 +
> include/linux/amd-iommu.h | 6 ++++
> include/linux/iommu.h | 4 +++
> 6 files changed, 67 insertions(+), 2 deletions(-)
>

2023-01-17 13:31:33

by Eric van Tassell

[permalink] [raw]
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 <[email protected]>
> Signed-off-by: Vasant Hegde <[email protected]>
> Signed-off-by: Suravee Suthikulpanit <[email protected]>
> ---
> 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,
> }
> };
>