Messages in this thread | | | Date | Fri, 26 May 2023 12:28:15 +0800 | Subject | Re: [PATCH v3 10/10] iommu/vt-d: Disallow nesting on domains with read-only mappings | From | Baolu Lu <> |
| |
On 5/24/23 3:44 PM, Tian, Kevin wrote: >> From: Liu, Yi L <yi.l.liu@intel.com> >> Sent: Thursday, May 11, 2023 10:51 PM >> >> From: Lu Baolu <baolu.lu@linux.intel.com> >> >> 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.
Fair enough.
>> >> + 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.
Sure.
Best regards, baolu
| |