diff mbox series

[v2,3/5] iommu/vt-d: Extend dmar_domain to support nested domain

Message ID 20230309082207.612346-4-yi.l.liu@intel.com
State New
Headers
Series Add Intel VT-d nested translation |

Commit Message

Yi Liu March 9, 2023, 8:22 a.m. UTC 
  From: Lu Baolu <baolu.lu@linux.intel.com>

The nested domain fields are exclusive to those that used for a DMA
remapping domain. Use union to avoid memory waste.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Signed-off-by: Yi Liu <yi.l.liu@intel.com>
---
 drivers/iommu/intel/iommu.h | 35 +++++++++++++++++++++++++++++------
 1 file changed, 29 insertions(+), 6 deletions(-)

Comments

Jason Gunthorpe March 20, 2023, 1:54 p.m. UTC | #1 
On Thu, Mar 09, 2023 at 12:22:05AM -0800, Yi Liu wrote:
> From: Lu Baolu <baolu.lu@linux.intel.com>
> 
> The nested domain fields are exclusive to those that used for a DMA
> remapping domain. Use union to avoid memory waste.
> 
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
> ---
>  drivers/iommu/intel/iommu.h | 35 +++++++++++++++++++++++++++++------
>  1 file changed, 29 insertions(+), 6 deletions(-)

Using unions like this often devolves into a mess.

You'd be better to have more structures

struct intel_iommu_domain {
   struct iommu_domain domain;
   [general fields about attachment]
};

struct intel_iopte_domain {
    struct intel_iommu_domain domain;
    [stuff describing the io page table data, pgd, format, etc]
};

strut intel_s1_domain {
     struct intel_iommu_domain domain;
     struct dmar_domain *s2_domain;
     /* user page table pointer (in GPA) */
     unsigned long s1_pgtbl;
     /* page table attributes */
     struct iommu_hwpt_intel_vtd s1_cfg;
};
static_assert(offset_of(struct intel_s1_domain, domain.domain) ==
              offset_of(struct intel_iommu_domain, domain));

The per-domain ops allow to make this work sensibly

Jason
Baolu Lu March 21, 2023, 1:58 a.m. UTC | #2 
On 3/20/23 9:54 PM, Jason Gunthorpe wrote:
> On Thu, Mar 09, 2023 at 12:22:05AM -0800, Yi Liu wrote:
>> From: Lu Baolu <baolu.lu@linux.intel.com>
>>
>> The nested domain fields are exclusive to those that used for a DMA
>> remapping domain. Use union to avoid memory waste.
>>
>> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
>> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
>> ---
>>   drivers/iommu/intel/iommu.h | 35 +++++++++++++++++++++++++++++------
>>   1 file changed, 29 insertions(+), 6 deletions(-)
> 
> Using unions like this often devolves into a mess.
> 
> You'd be better to have more structures
> 
> struct intel_iommu_domain {
>     struct iommu_domain domain;
>     [general fields about attachment]
> };
> 
> struct intel_iopte_domain {
>      struct intel_iommu_domain domain;
>      [stuff describing the io page table data, pgd, format, etc]
> };
> 
> strut intel_s1_domain {
>       struct intel_iommu_domain domain;
>       struct dmar_domain *s2_domain;
>       /* user page table pointer (in GPA) */
>       unsigned long s1_pgtbl;
>       /* page table attributes */
>       struct iommu_hwpt_intel_vtd s1_cfg;
> };
> static_assert(offset_of(struct intel_s1_domain, domain.domain) ==
>                offset_of(struct intel_iommu_domain, domain));
> 
> The per-domain ops allow to make this work sensibly

Yes. This will make the data structures clearer.

However, this will lead to significant code changes. I think it would be
more appropriate to put it in a separate refactoring series later.

Best regards,
baolu
diff mbox series

Patch

diff --git a/drivers/iommu/intel/iommu.h b/drivers/iommu/intel/iommu.h
index 9871de2170eb..9446e17dd202 100644
--- a/drivers/iommu/intel/iommu.h
+++ b/drivers/iommu/intel/iommu.h
@@ -599,15 +599,38 @@  struct dmar_domain {
 	spinlock_t lock;		/* Protect device tracking lists */
 	struct list_head devices;	/* all devices' list */
 
-	struct dma_pte	*pgd;		/* virtual address */
-	int		gaw;		/* max guest address width */
-
-	/* adjusted guest address width, 0 is level 2 30-bit */
-	int		agaw;
 	int		iommu_superpage;/* Level of superpages supported:
 					   0 == 4KiB (no superpages), 1 == 2MiB,
 					   2 == 1GiB, 3 == 512GiB, 4 == 1TiB */
-	u64		max_addr;	/* maximum mapped address */
+	union {
+		/* DMA remapping domain */
+		struct {
+			/* virtual address */
+			struct dma_pte	*pgd;
+			/* max guest address width */
+			int		gaw;
+			/*
+			 * adjusted guest address width:
+			 *   0: level 2 30-bit
+			 *   1: level 3 39-bit
+			 *   2: level 4 48-bit
+			 *   3: level 5 57-bit
+			 */
+			int		agaw;
+			/* maximum mapped address */
+			u64		max_addr;
+		};
+
+		/* Nested user domain */
+		struct {
+			/* 2-level page table the user domain nested */
+			struct dmar_domain *s2_domain;
+			/* user page table pointer (in GPA) */
+			unsigned long s1_pgtbl;
+			/* page table attributes */
+			struct iommu_hwpt_intel_vtd s1_cfg;
+		};
+	};
 
 	struct iommu_domain domain;	/* generic domain data structure for
 					   iommu core */