| Message ID | 20230309082207.612346-4-yi.l.liu@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,1/5] iommufd: Add nesting related data structures for Intel VT-d | expand |
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
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 --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 */