diff mbox series

[v3] iommu/vtd: fix address translation for leaf entries

Message ID 20230525080859.29859-1-roger.pau@citrix.com (mailing list archive)
State New, archived
Headers show
Series [v3] iommu/vtd: fix address translation for leaf entries | expand

Commit Message

Roger Pau Monné May 25, 2023, 8:08 a.m. UTC
Fix two issues related to leaf address lookups in VT-d:

* When translating an address that falls inside of a superpage in the
  IOMMU page tables the fetching of the PTE value wasn't masking of the
  contiguous related data, which caused the returned data to be
  corrupt as it would contain bits that the caller would interpret as
  part of the address.

* When the requested leaf address wasn't mapped by a superpage the
  returned value wouldn't have any of the low 12 bits set, thus missing
  the permission bits expected by the caller.

Take the opportunity to also adjust the function comment to note that
when returning the full PTE the bits above PADDR_BITS are removed.

Fixes: c71e55501a61 ('VT-d: have callers specify the target level for page table walks')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Changes since v2:
 - Use PADDR_MASK.
 - Also fix the non-superpage case.

Changes since v1:
 - Return all the PTE bits except for the contiguous count ones.
---
 xen/drivers/passthrough/vtd/iommu.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Comments

Jan Beulich May 25, 2023, 8:56 a.m. UTC | #1
On 25.05.2023 10:08, Roger Pau Monne wrote:
> Fix two issues related to leaf address lookups in VT-d:
> 
> * When translating an address that falls inside of a superpage in the
>   IOMMU page tables the fetching of the PTE value wasn't masking of the
>   contiguous related data, which caused the returned data to be
>   corrupt as it would contain bits that the caller would interpret as
>   part of the address.
> 
> * When the requested leaf address wasn't mapped by a superpage the
>   returned value wouldn't have any of the low 12 bits set, thus missing
>   the permission bits expected by the caller.
> 
> Take the opportunity to also adjust the function comment to note that
> when returning the full PTE the bits above PADDR_BITS are removed.
> 
> Fixes: c71e55501a61 ('VT-d: have callers specify the target level for page table walks')
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
Tian, Kevin June 16, 2023, 1:09 a.m. UTC | #2
> From: Roger Pau Monne <roger.pau@citrix.com>
> Sent: Thursday, May 25, 2023 4:09 PM
> 
> Fix two issues related to leaf address lookups in VT-d:
> 
> * When translating an address that falls inside of a superpage in the
>   IOMMU page tables the fetching of the PTE value wasn't masking of the
>   contiguous related data, which caused the returned data to be
>   corrupt as it would contain bits that the caller would interpret as
>   part of the address.
> 
> * When the requested leaf address wasn't mapped by a superpage the
>   returned value wouldn't have any of the low 12 bits set, thus missing
>   the permission bits expected by the caller.
> 
> Take the opportunity to also adjust the function comment to note that
> when returning the full PTE the bits above PADDR_BITS are removed.
> 
> Fixes: c71e55501a61 ('VT-d: have callers specify the target level for page table
> walks')
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Reviewed-by: Kevin Tian <kevin.tian@intel.com>
diff mbox series

Patch

diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index 130a159cde07..0e3062c820f9 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -310,7 +310,7 @@  static u64 bus_to_context_maddr(struct vtd_iommu *iommu, u8 bus)
  *   failure,
  * - for target > 0 the physical address of the page table holding the leaf
  *   PTE for the requested address,
- * - for target == 0 the full PTE.
+ * - for target == 0 the full PTE contents below PADDR_BITS limit.
  */
 static uint64_t addr_to_dma_page_maddr(struct domain *domain, daddr_t addr,
                                        unsigned int target,
@@ -368,7 +368,7 @@  static uint64_t addr_to_dma_page_maddr(struct domain *domain, daddr_t addr,
                  * with the address adjusted to account for the residual of
                  * the walk.
                  */
-                pte_maddr = pte->val +
+                pte_maddr = (pte->val & PADDR_MASK) +
                     (addr & ((1UL << level_to_offset_bits(level)) - 1) &
                      PAGE_MASK);
                 if ( !target )
@@ -413,7 +413,11 @@  static uint64_t addr_to_dma_page_maddr(struct domain *domain, daddr_t addr,
         }
 
         if ( --level == target )
+        {
+            if ( !target )
+                pte_maddr = pte->val & PADDR_MASK;
             break;
+        }
 
         unmap_vtd_domain_page(parent);
         parent = map_vtd_domain_page(pte_maddr);