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