Xen project Mailing List

[Xen-devel] [PATCH v11 3/9] iommu: push use of type-safe DFN and MFN into iommu_ops

From: Paul Durrant <paul.durrant@xxxxxxxxxx>

Date: Fri, 21 Sep 2018 11:56:20 +0100

Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Paul Durrant <paul.durrant@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>

Delivery-date: Fri, 21 Sep 2018 10:56:37 +0000

List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

This patch modifies the methods in struct iommu_ops to use type-safe DFN and MFN. This follows on from the prior patch that modified the functions exported in xen/iommu.h. Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx> Reviewed-by: Wei Liu <wei.liu2@xxxxxxxxxx> Reviewed-by: Kevin Tian <kevin.tian@xxxxxxxxx> Reviewed-by: Roger Pau Monne <roger.pau@xxxxxxxxxx> Acked-by: Jan Beulich <jbeulich@xxxxxxxx> --- Cc: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> Cc: George Dunlap <george.dunlap@xxxxxxxxxx> v9: - Re-base. v7: - Re-base and re-name BFN -> DFN. - Added Jan's A-b since re-naming was purely mechanical. v6: - Re-base. v3: - Remove some use of intermediate 'frame' variables. v2: - Addressed comments from Jan. - Extend use of intermediate 'frame' variable to avoid directly encapsulating gfn values as bfns (now dfns) or vice versa. --- xen/drivers/passthrough/amd/iommu_map.c | 46 ++++++++++++++++----------- xen/drivers/passthrough/amd/pci_amd_iommu.c | 2 +- xen/drivers/passthrough/arm/smmu.c | 16 +++++----- xen/drivers/passthrough/iommu.c | 9 +++--- xen/drivers/passthrough/vtd/iommu.c | 26 +++++++-------- xen/drivers/passthrough/x86/iommu.c | 2 +- xen/include/asm-x86/hvm/svm/amd-iommu-proto.h | 8 ++--- xen/include/xen/iommu.h | 13 +++++--- 8 files changed, 67 insertions(+), 55 deletions(-) diff --git a/xen/drivers/passthrough/amd/iommu_map.c b/xen/drivers/passthrough/amd/iommu_map.c index 61ade71850..c89c54fdb6 100644 --- a/xen/drivers/passthrough/amd/iommu_map.c +++ b/xen/drivers/passthrough/amd/iommu_map.c @@ -631,7 +631,7 @@ static int update_paging_mode(struct domain *d, unsigned long dfn) return 0; } -int amd_iommu_map_page(struct domain *d, unsigned long dfn, unsigned long mfn, +int amd_iommu_map_page(struct domain *d, dfn_t dfn, mfn_t mfn, unsigned int flags) { bool_t need_flush = 0; @@ -651,7 +651,8 @@ int amd_iommu_map_page(struct domain *d, unsigned long dfn, unsigned long mfn, if ( rc ) { spin_unlock(&hd->arch.mapping_lock); - AMD_IOMMU_DEBUG("Root table alloc failed, dfn = %lx\n", dfn); + AMD_IOMMU_DEBUG("Root table alloc failed, dfn = %"PRI_dfn"\n", + dfn_x(dfn)); domain_crash(d); return rc; } @@ -660,25 +661,27 @@ int amd_iommu_map_page(struct domain *d, unsigned long dfn, unsigned long mfn, * we might need a deeper page table for wider dfn now */ if ( is_hvm_domain(d) ) { - if ( update_paging_mode(d, dfn) ) + if ( update_paging_mode(d, dfn_x(dfn)) ) { spin_unlock(&hd->arch.mapping_lock); - AMD_IOMMU_DEBUG("Update page mode failed dfn = %lx\n", dfn); + AMD_IOMMU_DEBUG("Update page mode failed dfn = %"PRI_dfn"\n", + dfn_x(dfn)); domain_crash(d); return -EFAULT; } } - if ( iommu_pde_from_dfn(d, dfn, pt_mfn) || (pt_mfn[1] == 0) ) + if ( iommu_pde_from_dfn(d, dfn_x(dfn), pt_mfn) || (pt_mfn[1] == 0) ) { spin_unlock(&hd->arch.mapping_lock); - AMD_IOMMU_DEBUG("Invalid IO pagetable entry dfn = %lx\n", dfn); + AMD_IOMMU_DEBUG("Invalid IO pagetable entry dfn = %"PRI_dfn"\n", + dfn_x(dfn)); domain_crash(d); return -EFAULT; } /* Install 4k mapping first */ - need_flush = set_iommu_pte_present(pt_mfn[1], dfn, mfn, + need_flush = set_iommu_pte_present(pt_mfn[1], dfn_x(dfn), mfn_x(mfn), IOMMU_PAGING_MODE_LEVEL_1, !!(flags & IOMMUF_writable), !!(flags & IOMMUF_readable)); @@ -690,7 +693,7 @@ int amd_iommu_map_page(struct domain *d, unsigned long dfn, unsigned long mfn, /* 4K mapping for PV guests never changes, * no need to flush if we trust non-present bits */ if ( is_hvm_domain(d) ) - amd_iommu_flush_pages(d, dfn, 0); + amd_iommu_flush_pages(d, dfn_x(dfn), 0); for ( merge_level = IOMMU_PAGING_MODE_LEVEL_2; merge_level <= hd->arch.paging_mode; merge_level++ ) @@ -698,15 +701,16 @@ int amd_iommu_map_page(struct domain *d, unsigned long dfn, unsigned long mfn, if ( pt_mfn[merge_level] == 0 ) break; if ( !iommu_update_pde_count(d, pt_mfn[merge_level], - dfn, mfn, merge_level) ) + dfn_x(dfn), mfn_x(mfn), merge_level) ) break; - if ( iommu_merge_pages(d, pt_mfn[merge_level], dfn, + if ( iommu_merge_pages(d, pt_mfn[merge_level], dfn_x(dfn), flags, merge_level) ) { spin_unlock(&hd->arch.mapping_lock); AMD_IOMMU_DEBUG("Merge iommu page failed at level %d, " - "dfn = %lx mfn = %lx\n", merge_level, dfn, mfn); + "dfn = %"PRI_dfn" mfn = %"PRI_mfn"\n", + merge_level, dfn_x(dfn), mfn_x(mfn)); domain_crash(d); return -EFAULT; } @@ -720,7 +724,7 @@ out: return 0; } -int amd_iommu_unmap_page(struct domain *d, unsigned long dfn) +int amd_iommu_unmap_page(struct domain *d, dfn_t dfn) { unsigned long pt_mfn[7]; struct domain_iommu *hd = dom_iommu(d); @@ -742,31 +746,33 @@ int amd_iommu_unmap_page(struct domain *d, unsigned long dfn) * we might need a deeper page table for lager dfn now */ if ( is_hvm_domain(d) ) { - int rc = update_paging_mode(d, dfn); + int rc = update_paging_mode(d, dfn_x(dfn)); if ( rc ) { spin_unlock(&hd->arch.mapping_lock); - AMD_IOMMU_DEBUG("Update page mode failed dfn = %lx\n", dfn); + AMD_IOMMU_DEBUG("Update page mode failed dfn = %"PRI_dfn"\n", + dfn_x(dfn)); if ( rc != -EADDRNOTAVAIL ) domain_crash(d); return rc; } } - if ( iommu_pde_from_dfn(d, dfn, pt_mfn) || (pt_mfn[1] == 0) ) + if ( iommu_pde_from_dfn(d, dfn_x(dfn), pt_mfn) || (pt_mfn[1] == 0) ) { spin_unlock(&hd->arch.mapping_lock); - AMD_IOMMU_DEBUG("Invalid IO pagetable entry dfn = %lx\n", dfn); + AMD_IOMMU_DEBUG("Invalid IO pagetable entry dfn = %"PRI_dfn"\n", + dfn_x(dfn)); domain_crash(d); return -EFAULT; } /* mark PTE as 'page not present' */ - clear_iommu_pte_present(pt_mfn[1], dfn); + clear_iommu_pte_present(pt_mfn[1], dfn_x(dfn)); spin_unlock(&hd->arch.mapping_lock); - amd_iommu_flush_pages(d, dfn, 0); + amd_iommu_flush_pages(d, dfn_x(dfn), 0); return 0; } @@ -787,7 +793,9 @@ int amd_iommu_reserve_domain_unity_map(struct domain *domain, gfn = phys_addr >> PAGE_SHIFT; for ( i = 0; i < npages; i++ ) { - rt = amd_iommu_map_page(domain, gfn +i, gfn +i, flags); + unsigned long frame = gfn + i; + + rt = amd_iommu_map_page(domain, _dfn(frame), _mfn(frame), flags); if ( rt != 0 ) return rt; } diff --git a/xen/drivers/passthrough/amd/pci_amd_iommu.c b/xen/drivers/passthrough/amd/pci_amd_iommu.c index aa9eba02bd..5e99b6988e 100644 --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c @@ -278,7 +278,7 @@ static void __hwdom_init amd_iommu_hwdom_init(struct domain *d) */ if ( mfn_valid(_mfn(pfn)) ) { - int ret = amd_iommu_map_page(d, pfn, pfn, + int ret = amd_iommu_map_page(d, _dfn(pfn), _mfn(pfn), IOMMUF_readable|IOMMUF_writable); if ( !rc ) diff --git a/xen/drivers/passthrough/arm/smmu.c b/xen/drivers/passthrough/arm/smmu.c index 1eda96a72a..53e5823d05 100644 --- a/xen/drivers/passthrough/arm/smmu.c +++ b/xen/drivers/passthrough/arm/smmu.c @@ -2550,8 +2550,7 @@ static int __must_check arm_smmu_iotlb_flush_all(struct domain *d) return 0; } -static int __must_check arm_smmu_iotlb_flush(struct domain *d, - unsigned long dfn, +static int __must_check arm_smmu_iotlb_flush(struct domain *d, dfn_t dfn, unsigned int page_count) { /* ARM SMMU v1 doesn't have flush by VMA and VMID */ @@ -2748,8 +2747,8 @@ static void arm_smmu_iommu_domain_teardown(struct domain *d) xfree(xen_domain); } -static int __must_check arm_smmu_map_page(struct domain *d, unsigned long dfn, - unsigned long mfn, unsigned int flags) +static int __must_check arm_smmu_map_page(struct domain *d, dfn_t dfn, + mfn_t mfn, unsigned int flags) { p2m_type_t t; @@ -2762,7 +2761,7 @@ static int __must_check arm_smmu_map_page(struct domain *d, unsigned long dfn, * function should only be used by gnttab code with gfn == mfn == dfn. */ BUG_ON(!is_domain_direct_mapped(d)); - BUG_ON(mfn != dfn); + BUG_ON(mfn_x(mfn) != dfn_x(dfn)); /* We only support readable and writable flags */ if (!(flags & (IOMMUF_readable | IOMMUF_writable))) @@ -2774,10 +2773,11 @@ static int __must_check arm_smmu_map_page(struct domain *d, unsigned long dfn, * The function guest_physmap_add_entry replaces the current mapping * if there is already one... */ - return guest_physmap_add_entry(d, _gfn(dfn), _mfn(dfn), 0, t); + return guest_physmap_add_entry(d, _gfn(dfn_x(dfn)), _mfn(dfn_x(dfn)), + 0, t); } -static int __must_check arm_smmu_unmap_page(struct domain *d, unsigned long dfn) +static int __must_check arm_smmu_unmap_page(struct domain *d, dfn_t dfn) { /* * This function should only be used by gnttab code when the domain @@ -2786,7 +2786,7 @@ static int __must_check arm_smmu_unmap_page(struct domain *d, unsigned long dfn) if ( !is_domain_direct_mapped(d) ) return -EINVAL; - return guest_physmap_remove_page(d, _gfn(dfn), _mfn(dfn), 0); + return guest_physmap_remove_page(d, _gfn(dfn_x(dfn)), _mfn(dfn_x(dfn)), 0); } static const struct iommu_ops arm_smmu_iommu_ops = { diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c index 53419a8531..c1122562a3 100644 --- a/xen/drivers/passthrough/iommu.c +++ b/xen/drivers/passthrough/iommu.c @@ -224,7 +224,8 @@ void __hwdom_init iommu_hwdom_init(struct domain *d) == PGT_writable_page) ) mapping |= IOMMUF_writable; - ret = hd->platform_ops->map_page(d, dfn, mfn, mapping); + ret = hd->platform_ops->map_page(d, _dfn(dfn), _mfn(mfn), + mapping); if ( !rc ) rc = ret; @@ -294,7 +295,7 @@ int iommu_map_page(struct domain *d, dfn_t dfn, mfn_t mfn, if ( !iommu_enabled || !hd->platform_ops ) return 0; - rc = hd->platform_ops->map_page(d, dfn_x(dfn), mfn_x(mfn), flags); + rc = hd->platform_ops->map_page(d, dfn, mfn, flags); if ( unlikely(rc) ) { if ( !d->is_shutting_down && printk_ratelimit() ) @@ -317,7 +318,7 @@ int iommu_unmap_page(struct domain *d, dfn_t dfn) if ( !iommu_enabled || !hd->platform_ops ) return 0; - rc = hd->platform_ops->unmap_page(d, dfn_x(dfn)); + rc = hd->platform_ops->unmap_page(d, dfn); if ( unlikely(rc) ) { if ( !d->is_shutting_down && printk_ratelimit() ) @@ -357,7 +358,7 @@ int iommu_iotlb_flush(struct domain *d, dfn_t dfn, unsigned int page_count) if ( !iommu_enabled || !hd->platform_ops || !hd->platform_ops->iotlb_flush ) return 0; - rc = hd->platform_ops->iotlb_flush(d, dfn_x(dfn), page_count); + rc = hd->platform_ops->iotlb_flush(d, dfn, page_count); if ( unlikely(rc) ) { if ( !d->is_shutting_down && printk_ratelimit() ) diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c index 507a3f3afa..80264c6cc0 100644 --- a/xen/drivers/passthrough/vtd/iommu.c +++ b/xen/drivers/passthrough/vtd/iommu.c @@ -584,8 +584,7 @@ static int __must_check iommu_flush_all(void) return rc; } -static int __must_check iommu_flush_iotlb(struct domain *d, - unsigned long dfn, +static int __must_check iommu_flush_iotlb(struct domain *d, dfn_t dfn, bool_t dma_old_pte_present, unsigned int page_count) { @@ -612,12 +611,12 @@ static int __must_check iommu_flush_iotlb(struct domain *d, if ( iommu_domid == -1 ) continue; - if ( page_count != 1 || dfn == dfn_x(INVALID_DFN) ) + if ( page_count != 1 || dfn_eq(dfn, INVALID_DFN) ) rc = iommu_flush_iotlb_dsi(iommu, iommu_domid, 0, flush_dev_iotlb); else rc = iommu_flush_iotlb_psi(iommu, iommu_domid, - __dfn_to_daddr(dfn), + dfn_to_daddr(dfn), PAGE_ORDER_4K, !dma_old_pte_present, flush_dev_iotlb); @@ -633,7 +632,7 @@ static int __must_check iommu_flush_iotlb(struct domain *d, } static int __must_check iommu_flush_iotlb_pages(struct domain *d, - unsigned long dfn, + dfn_t dfn, unsigned int page_count) { return iommu_flush_iotlb(d, dfn, 1, page_count); @@ -641,7 +640,7 @@ static int __must_check iommu_flush_iotlb_pages(struct domain *d, static int __must_check iommu_flush_iotlb_all(struct domain *d) { - return iommu_flush_iotlb(d, dfn_x(INVALID_DFN), 0, 0); + return iommu_flush_iotlb(d, INVALID_DFN, 0, 0); } /* clear one page's page table */ @@ -676,7 +675,7 @@ static int __must_check dma_pte_clear_one(struct domain *domain, u64 addr) iommu_flush_cache_entry(pte, sizeof(struct dma_pte)); if ( !this_cpu(iommu_dont_flush_iotlb) ) - rc = iommu_flush_iotlb_pages(domain, addr >> PAGE_SHIFT_4K, 1); + rc = iommu_flush_iotlb_pages(domain, daddr_to_dfn(addr), 1); unmap_vtd_domain_page(page); @@ -1770,8 +1769,7 @@ static void iommu_domain_teardown(struct domain *d) } static int __must_check intel_iommu_map_page(struct domain *d, - unsigned long dfn, - unsigned long mfn, + dfn_t dfn, mfn_t mfn, unsigned int flags) { struct domain_iommu *hd = dom_iommu(d); @@ -1789,16 +1787,16 @@ static int __must_check intel_iommu_map_page(struct domain *d, spin_lock(&hd->arch.mapping_lock); - pg_maddr = addr_to_dma_page_maddr(d, __dfn_to_daddr(dfn), 1); + pg_maddr = addr_to_dma_page_maddr(d, dfn_to_daddr(dfn), 1); if ( pg_maddr == 0 ) { spin_unlock(&hd->arch.mapping_lock); return -ENOMEM; } page = (struct dma_pte *)map_vtd_domain_page(pg_maddr); - pte = page + (dfn & LEVEL_MASK); + pte = page + (dfn_x(dfn) & LEVEL_MASK); old = *pte; - dma_set_pte_addr(new, (paddr_t)mfn << PAGE_SHIFT_4K); + 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)); @@ -1826,13 +1824,13 @@ static int __must_check intel_iommu_map_page(struct domain *d, } static int __must_check intel_iommu_unmap_page(struct domain *d, - unsigned long dfn) + dfn_t dfn) { /* Do nothing if hardware domain and iommu supports pass thru. */ if ( iommu_hwdom_passthrough && is_hardware_domain(d) ) return 0; - return dma_pte_clear_one(d, __dfn_to_daddr(dfn)); + return dma_pte_clear_one(d, dfn_to_daddr(dfn)); } int iommu_pte_flush(struct domain *d, uint64_t dfn, uint64_t *pte, diff --git a/xen/drivers/passthrough/x86/iommu.c b/xen/drivers/passthrough/x86/iommu.c index f9bd1c8bb2..b511f822ad 100644 --- a/xen/drivers/passthrough/x86/iommu.c +++ b/xen/drivers/passthrough/x86/iommu.c @@ -68,7 +68,7 @@ int arch_iommu_populate_page_table(struct domain *d) { ASSERT(!(gfn >> DEFAULT_DOMAIN_ADDRESS_WIDTH)); BUG_ON(SHARED_M2P(gfn)); - rc = hd->platform_ops->map_page(d, gfn, mfn, + rc = hd->platform_ops->map_page(d, _dfn(gfn), _mfn(mfn), IOMMUF_readable | IOMMUF_writable); } diff --git a/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h b/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h index 99bc21c7b3..3083d625bd 100644 --- a/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h +++ b/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h @@ -52,9 +52,9 @@ int amd_iommu_init(void); int amd_iommu_update_ivrs_mapping_acpi(void); /* mapping functions */ -int __must_check amd_iommu_map_page(struct domain *d, unsigned long gfn, - unsigned long mfn, unsigned int flags); -int __must_check amd_iommu_unmap_page(struct domain *d, unsigned long gfn); +int __must_check amd_iommu_map_page(struct domain *d, dfn_t dfn, + mfn_t mfn, unsigned int flags); +int __must_check amd_iommu_unmap_page(struct domain *d, dfn_t dfn); u64 amd_iommu_get_next_table_from_pte(u32 *entry); int __must_check amd_iommu_alloc_root(struct domain_iommu *hd); int amd_iommu_reserve_domain_unity_map(struct domain *domain, @@ -77,7 +77,7 @@ void iommu_dte_set_guest_cr3(u32 *dte, u16 dom_id, u64 gcr3, /* send cmd to iommu */ void amd_iommu_flush_all_pages(struct domain *d); -void amd_iommu_flush_pages(struct domain *d, unsigned long gfn, +void amd_iommu_flush_pages(struct domain *d, unsigned long dfn, unsigned int order); void amd_iommu_flush_iotlb(u8 devfn, const struct pci_dev *pdev, uint64_t gaddr, unsigned int order); diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h index f9d86fc816..7313957c81 100644 --- a/xen/include/xen/iommu.h +++ b/xen/include/xen/iommu.h @@ -48,6 +48,11 @@ static inline dfn_t dfn_add(dfn_t dfn, unsigned long i) return _dfn(dfn_x(dfn) + i); } +static inline bool_t dfn_eq(dfn_t x, dfn_t y) +{ + return dfn_x(x) == dfn_x(y); +} + extern bool_t iommu_enable, iommu_enabled; extern bool_t force_iommu, iommu_verbose; extern bool_t iommu_workaround_bios_bug, iommu_igfx; @@ -174,9 +179,9 @@ struct iommu_ops { #endif /* HAS_PCI */ void (*teardown)(struct domain *d); - int __must_check (*map_page)(struct domain *d, unsigned long dfn, - unsigned long mfn, unsigned int flags); - int __must_check (*unmap_page)(struct domain *d, unsigned long dfn); + int __must_check (*map_page)(struct domain *d, dfn_t dfn, mfn_t mfn, + unsigned int flags); + int __must_check (*unmap_page)(struct domain *d, dfn_t dfn); void (*free_page_table)(struct page_info *); #ifdef CONFIG_X86 void (*update_ire_from_apic)(unsigned int apic, unsigned int reg, unsigned int value); @@ -187,7 +192,7 @@ struct iommu_ops { void (*resume)(void); void (*share_p2m)(struct domain *d); void (*crash_shutdown)(void); - int __must_check (*iotlb_flush)(struct domain *d, unsigned long dfn, + int __must_check (*iotlb_flush)(struct domain *d, dfn_t dfn, unsigned int page_count); int __must_check (*iotlb_flush_all)(struct domain *d); int (*get_reserved_device_memory)(iommu_grdm_t *, void *); -- 2.11.0 _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel

©2013 Xen Project, A Linux Foundation Collaborative Project. All Rights Reserved.
Linux Foundation is a registered trademark of The Linux Foundation.
Xen Project is a trademark of The Linux Foundation.