Messages in this thread | | | Date | Fri, 26 May 2023 12:24:52 +0800 | Subject | Re: [PATCH v3 06/10] iommu/vt-d: Set the nested domain to a device | From | Baolu Lu <> |
| |
On 5/24/23 3:22 PM, Tian, Kevin wrote: >> From: Liu, Yi L <yi.l.liu@intel.com> >> Sent: Thursday, May 11, 2023 10:51 PM >> >> + >> +static int intel_nested_attach_dev(struct iommu_domain *domain, >> + struct device *dev) >> +{ >> + struct device_domain_info *info = dev_iommu_priv_get(dev); >> + struct dmar_domain *dmar_domain = to_dmar_domain(domain); >> + struct intel_iommu *iommu = info->iommu; >> + unsigned long flags; >> + int ret = 0; >> + >> + if (info->domain) >> + device_block_translation(dev); >> + >> + /* Is s2_domain compatible with this IOMMU? */ >> + ret = prepare_domain_attach_device(&dmar_domain->s2_domain- >>> domain, dev); >> + if (ret) { >> + dev_err_ratelimited(dev, "s2 domain is not compatible\n"); >> + return ret; >> + } > > this also includes logic to trim higher page levels: > > /* > * Knock out extra levels of page tables if necessary > */ > while (iommu->agaw < dmar_domain->agaw) { > struct dma_pte *pte; > > pte = dmar_domain->pgd; > if (dma_pte_present(pte)) { > dmar_domain->pgd = phys_to_virt(dma_pte_addr(pte)); > free_pgtable_page(pte); > } > dmar_domain->agaw--; > } > > What's the background of doing such truncation instead of simply > failing the request?
This code existed a long time ago. I'm not sure if it's still reasonable so far.
> In any means it's probably fine before the domain includes any mapping > but really unreasonable to apply it to an existing s2 when it's used as > a parent.
But for the new nested translation, it is obviously unreasonable.
Let me revisit it.
Best regards, baolu
| |