> From: Liu, Yi L <yi.l.liu@xxxxxxxxx> > Sent: Thursday, May 11, 2023 10:51 PM > > 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. > > Disallow the nested translation when there are read-only pages in the > corresponding second-stage mappings. And, no read-only pages are allowed > to be configured in the second-stage table of a nested translation. > For the latter, an alternative is to disallow read-only mappings in > any stage-2 domain as long as it's ever been used as a parent. In this > way, we can simply replace the user counter with a flag. > > In concept if the user understands this errata and does expect to > enable nested translation it should never install any RO mapping > in stage-2 in the entire VM life cycle." IMHO the alternative is reasonable and simpler. If the user decides to enable nesting it should keep the nesting-friendly configuration static since whether nesting is enabled on a device is according to viommu configuration (i.e. whether the guest attaches the device to identity domain or non-identity domain) and it's not good to break the nesting setup just because the host inadvertently adds a RO mapping to s2 in the middle between guest is detached/put back to identity domain and then re-attach to an unmanaged domain. > > + if (!(prot & DMA_PTE_WRITE) && !domain->read_only_mapped) { > + spin_lock_irqsave(&domain->lock, flags); > + if (domain->nested_users > 0) { > + spin_unlock_irqrestore(&domain->lock, flags); > + return -EINVAL; > + } > + this is worth a one-off warning. Same in the other path.