> From: Baolu Lu <baolu.lu@xxxxxxxxxxxxxxx> > Sent: Saturday, October 21, 2023 11:24 AM > > On 10/20/23 5:32 PM, Yi Liu wrote: > > From: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx> > > > > When remapping hardware is configured by system software in scalable mode > > as Nested (PGTT=011b) and with PWSNP field Set in the PASID-table-entry, > > it may Set Accessed bit and Dirty bit (and Extended Access bit if enabled) > > in first-stage page-table entries even when second-stage mappings indicate > > that corresponding first-stage page-table is Read-Only. > > > > As the result, contents of pages designated by VMM as Read-Only can be > > modified by IOMMU via PML5E (PML4E for 4-level tables) access as part of > > address translation process due to DMAs issued by Guest. > > > > This disallows read-only mappings in the domain that is supposed to be used > > as nested parent. Reference from Sapphire Rapids Specification Update [1], > > errata details, SPR17. Userspace should know this limitation by checking > > the IOMMU_HW_INFO_VTD_ERRATA_772415_SPR17 flag reported in the > IOMMU_GET_HW_INFO > > ioctl. > > > > [1] https://www.intel.com/content/www/us/en/content-details/772415/content- > details.html > > > > Reviewed-by: Kevin Tian <kevin.tian@xxxxxxxxx> > > Signed-off-by: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx> > > Signed-off-by: Yi Liu <yi.l.liu@xxxxxxxxx> > > --- > > drivers/iommu/intel/iommu.c | 9 +++++++++ > > drivers/iommu/intel/iommu.h | 1 + > > include/uapi/linux/iommufd.h | 12 +++++++++++- > > 3 files changed, 21 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c > > index c7704e7efd4a..a0341a069fbf 100644 > > --- a/drivers/iommu/intel/iommu.c > > +++ b/drivers/iommu/intel/iommu.c > > @@ -2193,6 +2193,11 @@ __domain_mapping(struct dmar_domain *domain, > unsigned long iov_pfn, > > if ((prot & (DMA_PTE_READ|DMA_PTE_WRITE)) == 0) > > return -EINVAL; > > > > + if (!(prot & DMA_PTE_WRITE) && domain->is_nested_parent) { > > + pr_err_ratelimited("Read-only mapping is disallowed on the domain > which serves as the parent in a nested configuration, due to HW errata > (ERRATA_772415_SPR17)\n"); > > + return -EINVAL; > > + } > > + > > attr = prot & (DMA_PTE_READ | DMA_PTE_WRITE | DMA_PTE_SNP); > > attr |= DMA_FL_PTE_PRESENT; > > if (domain->use_first_level) { > > @@ -4101,6 +4106,9 @@ intel_iommu_domain_alloc_user(struct device *dev, u32 > flags, > > domain = iommu_domain_alloc(dev->bus); > > if (!domain) > > return ERR_PTR(-ENOMEM); > > + container_of(domain, > > + struct dmar_domain, > > + domain)->is_nested_parent = request_nest_parent; > > How about > to_dmar_domain(domain)->is_nested_parent = ...; > ? Yes. > > I would also prefer to introduce is_nested_parent_domain to the user > domain allocation patch (patch 7/8). This field should be checked when > allocating a nested user domain. A ctually, no need. This should be a common check, so iommufd core already has the check. So the parent should be a nest parent domain, otherwise already returned in iommufd. + if (!parent->nest_parent) + return ERR_PTR(-EINVAL); https://lore.kernel.org/linux-iommu/20231020091946.12173-8-yi.l.liu@xxxxxxxxx/ > diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c > index 8f81a5c9fcc0..d3f6bc1f6590 100644 > --- a/drivers/iommu/intel/iommu.c > +++ b/drivers/iommu/intel/iommu.c > @@ -4121,6 +4121,8 @@ intel_iommu_domain_alloc_user(struct device *dev, > u32 flags, > return ERR_PTR(-EINVAL); > if (request_nest_parent) > return ERR_PTR(-EINVAL); > + if (!to_dmar_domain(parent)->is_nested_parent) > + return ERR_PTR(-EINVAL); > > return intel_nested_domain_alloc(parent, user_data); > } > > Best regards, > baolu
