[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH] iommu/vtd: fix address translation for superpages
On 10.05.2023 10:27, Roger Pau Monné wrote: > On Tue, May 09, 2023 at 06:06:45PM +0200, Jan Beulich wrote: >> On 09.05.2023 12:41, Roger Pau Monne wrote: >>> When translating an address that falls inside of a superpage in the >>> IOMMU page tables the fetching of the PTE physical address field >>> wasn't using dma_pte_addr(), which caused the returned data to be >>> corrupt as it would contain bits not related to the address field. >> >> I'm afraid I don't understand: >> >>> --- a/xen/drivers/passthrough/vtd/iommu.c >>> +++ b/xen/drivers/passthrough/vtd/iommu.c >>> @@ -359,16 +359,18 @@ static uint64_t addr_to_dma_page_maddr(struct domain >>> *domain, daddr_t addr, >>> >>> if ( !alloc ) >>> { >>> - pte_maddr = 0; >>> if ( !dma_pte_present(*pte) ) >>> + { >>> + pte_maddr = 0; >>> break; >>> + } >>> >>> /* >>> * When the leaf entry was requested, pass back the full >>> PTE, >>> * with the address adjusted to account for the residual of >>> * the walk. >>> */ >>> - pte_maddr = pte->val + >>> + pte_maddr += >>> (addr & ((1UL << level_to_offset_bits(level)) - 1) & >>> PAGE_MASK); >> >> With this change you're now violating what the comment says (plus what >> the comment ahead of the function says). And it says what it says for >> a reason - see intel_iommu_lookup_page(), which I think your change is >> breaking. > > Hm, but the code in intel_iommu_lookup_page() is now wrong as it takes > the bits in DMA_PTE_CONTIG_MASK as part of the physical address when > doing the conversion to mfn? maddr_to_mfn() doesn't perform a any > masking to remove the bits above PADDR_BITS. Oh, right. But that's a missing dma_pte_addr() in intel_iommu_lookup_page() then. (It would likely be better anyway to switch "uint64_t val" to "struct dma_pte pte" there, to make more visible that it's a PTE we're dealing with.) I indeed overlooked this aspect when doing the earlier change. Jan
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |