On Fri, Sep 24, 2021 at 11:52:47AM +0200, Jan Beulich wrote: > ... depending on feature availability (and absence of quirks). > > Also make the page table dumping function aware of superpages. > > Signed-off-by: Jan Beulich <[email protected]>
Just some minor nits. > --- a/xen/drivers/passthrough/vtd/iommu.c > +++ b/xen/drivers/passthrough/vtd/iommu.c > @@ -743,18 +743,37 @@ static int __must_check iommu_flush_iotl > return iommu_flush_iotlb(d, INVALID_DFN, 0, 0); > } > > +static void queue_free_pt(struct domain *d, mfn_t mfn, unsigned int > next_level) Same comment as the AMD side patch, about naming the parameter just level. > @@ -1901,13 +1926,15 @@ static int __must_check intel_iommu_map_ > } > > page = (struct dma_pte *)map_vtd_domain_page(pg_maddr); > - pte = &page[dfn_x(dfn) & LEVEL_MASK]; > + pte = &page[address_level_offset(dfn_to_daddr(dfn), level)]; > old = *pte; > > dma_set_pte_addr(new, mfn_to_maddr(mfn)); > dma_set_pte_prot(new, > ((flags & IOMMUF_readable) ? DMA_PTE_READ : 0) | > ((flags & IOMMUF_writable) ? DMA_PTE_WRITE : 0)); > + if ( IOMMUF_order(flags) ) You seem to use level > 1 in other places to check for whether the entry is intended to be a super-page. Is there any reason to use IOMMUF_order here instead? > @@ -2328,6 +2361,11 @@ static int __init vtd_setup(void) > cap_sps_2mb(iommu->cap) ? ", 2MB" : "", > cap_sps_1gb(iommu->cap) ? ", 1GB" : ""); > > + if ( !cap_sps_2mb(iommu->cap) ) > + large_sizes &= ~PAGE_SIZE_2M; > + if ( !cap_sps_1gb(iommu->cap) ) > + large_sizes &= ~PAGE_SIZE_1G; > + > #ifndef iommu_snoop > if ( iommu_snoop && !ecap_snp_ctl(iommu->ecap) ) > iommu_snoop = false; > @@ -2399,6 +2437,9 @@ static int __init vtd_setup(void) > if ( ret ) > goto error; > > + ASSERT(iommu_ops.page_sizes & PAGE_SIZE_4K); Since you are adding the assert, it might be more complete to check no other page sizes are set, iommu_ops.page_sizes == PAGE_SIZE_4K? Thanks, Roger.
