Re: [PATCH v3 16/19] iommu/amd: Add domain_alloc_user based domain allocation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Joao,

On 9/23/2023 8:25 AM, Joao Martins wrote:
Add the domain_alloc_user op implementation. To that end, refactor
amd_iommu_domain_alloc() to receive a dev pointer and flags, while
renaming it to .. such that it becomes a common function shared with
domain_alloc_user() implementation. The sole difference with
domain_alloc_user() is that we initialize also other fields that
iommu_domain_alloc() does. It lets it return the iommu domain
correctly initialized in one function.

This is in preparation to add dirty enforcement on AMD implementation
of domain_alloc_user.

Signed-off-by: Joao Martins <joao.m.martins@xxxxxxxxxx>
---
  drivers/iommu/amd/iommu.c | 46 ++++++++++++++++++++++++++++++++++++---
  1 file changed, 43 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 95bd7c25ba6f..af36c627022f 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -37,6 +37,7 @@
  #include <asm/iommu.h>
  #include <asm/gart.h>
  #include <asm/dma.h>
+#include <uapi/linux/iommufd.h>
#include "amd_iommu.h"
  #include "../dma-iommu.h"
@@ -2155,7 +2156,10 @@ static inline u64 dma_max_address(void)
  	return ((1ULL << PM_LEVEL_SHIFT(amd_iommu_gpt_level)) - 1);
  }
-static struct iommu_domain *amd_iommu_domain_alloc(unsigned type)
+static struct iommu_domain *do_iommu_domain_alloc(unsigned int type,
+						  struct amd_iommu *iommu,
+						  struct device *dev,
+						  u32 flags)

Instead of passing in the struct amd_iommu here, what if we just derive it in the do_iommu_domain_alloc() as needed? This way, we don't need to ... (see below)

  {
  	struct protection_domain *domain;
@@ -2164,19 +2168,54 @@ static struct iommu_domain *amd_iommu_domain_alloc(unsigned type)
  	 * default to use IOMMU_DOMAIN_DMA[_FQ].
  	 */
  	if (amd_iommu_snp_en && (type == IOMMU_DOMAIN_IDENTITY))
-		return NULL;
+		return ERR_PTR(-EINVAL);
domain = protection_domain_alloc(type);
  	if (!domain)
-		return NULL;
+		return ERR_PTR(-ENOMEM);
domain->domain.geometry.aperture_start = 0;
  	domain->domain.geometry.aperture_end   = dma_max_address();
  	domain->domain.geometry.force_aperture = true;
+ if (dev) {
+		domain->domain.type = type;
+		domain->domain.pgsize_bitmap =
+			iommu->iommu.ops->pgsize_bitmap;
+		domain->domain.ops =
+			iommu->iommu.ops->default_domain_ops;
+	}
+
  	return &domain->domain;
  }
+static struct iommu_domain *amd_iommu_domain_alloc(unsigned type)
+{
+	struct iommu_domain *domain;
+
+	domain = do_iommu_domain_alloc(type, NULL, NULL, 0);

... pass iommu = NULL here unnecessarily.

+	if (IS_ERR(domain))
+		return NULL;
+
+	return domain;
+}
+
+static struct iommu_domain *amd_iommu_domain_alloc_user(struct device *dev,
+							u32 flags)
+{
+	unsigned int type = IOMMU_DOMAIN_UNMANAGED;
+	struct amd_iommu *iommu;
+
+	iommu = rlookup_amd_iommu(dev);
+	if (!iommu)
+		return ERR_PTR(-ENODEV);

We should not need to derive this here.

Other than this part.

Reviewed-by: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>

Thanks,
Suravee



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux