Re: [PATCH v1 1/2] PCI: ATS: Add function to check ATS page aligned request status.
On 2/8/2019 8:02 PM, sathyanarayanan kuppuswamy wrote: This means that you should probably have some kind of version check here. There is no version field in ATS v1.0 spec. Also, If I follow the history log in PCI spec, I think ATS if first added at v1.2. Please correct me if I am wrong. v1.2 was incorporated into PCIe spec at that time. However, the ATS spec is old and there could be some HW that could claim to be ATS compatible. I know AMD GPUs declare ATS capability. See this ECN https://composter.com.ua/documents/ats_r1.1_26Jan09.pdf You need to validate the version field from ATS capability header to be 1 before reading this register. See Table 5-1: ATS Extended Capability Header ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v1 1/2] PCI: ATS: Add function to check ATS page aligned request status.
On 2/7/19 5:58 PM, Sinan Kaya wrote: On 2/7/2019 5:16 PM, sathyanarayanan kuppuswamy wrote: If I remember this right, aligned request is only supported on ATS v1.1 but not supported on v1.0. Its added in v1.1. This means that you should probably have some kind of version check here. There is no version field in ATS v1.0 spec. Also, If I follow the history log in PCI spec, I think ATS if first added at v1.2. Please correct me if I am wrong. -- Sathyanarayanan Kuppuswamy Linux kernel developer ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [git pull] IOMMU Fixes for Linux v5.0-rc5
The pull request you sent on Fri, 8 Feb 2019 17:49:43 +0100: > git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git > tags/iommu-fixes-v5.0-rc5 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/2e277fa0893936bb4ab8432828f5d422a9ed0a7f Thank you! -- Deet-doot-dot, I am a bot. https://korg.wiki.kernel.org/userdoc/prtracker ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v1 3/7] iommu/vt-d: Use dev_printk() when possible
From: Bjorn Helgaas Use dev_printk() when possible so the IOMMU messages are more consistent with other messages related to the device. Signed-off-by: Bjorn Helgaas --- drivers/iommu/intel-iommu.c | 54 +++ 1 file changed, 24 insertions(+), 30 deletions(-) diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c index 2bd9ac285c0d..81077803880f 100644 --- a/drivers/iommu/intel-iommu.c +++ b/drivers/iommu/intel-iommu.c @@ -19,6 +19,7 @@ */ #define pr_fmt(fmt) "DMAR: " fmt +#define dev_fmt(fmt)pr_fmt(fmt) #include #include @@ -1815,7 +1816,7 @@ static int dmar_init_reserved_ranges(void) IOVA_PFN(r->start), IOVA_PFN(r->end)); if (!iova) { - pr_err("Reserve iova failed\n"); + pci_err(pdev, "Reserve iova for %pR failed\n", r); return -ENODEV; } } @@ -2544,8 +2545,7 @@ static struct dmar_domain *dmar_insert_one_dev_info(struct intel_iommu *iommu, if (dev && dev_is_pci(dev) && sm_supported(iommu)) { ret = intel_pasid_alloc_table(dev); if (ret) { - pr_err("PASID table allocation for %s failed\n", - dev_name(dev)); + dev_err(dev, "PASID table allocation failed\n"); dmar_remove_one_dev_info(domain, dev); return NULL; } @@ -2560,15 +2560,14 @@ static struct dmar_domain *dmar_insert_one_dev_info(struct intel_iommu *iommu, dev, PASID_RID2PASID); spin_unlock(&iommu->lock); if (ret) { - pr_err("Setup RID2PASID for %s failed\n", - dev_name(dev)); + dev_err(dev, "Setup RID2PASID failed\n"); dmar_remove_one_dev_info(domain, dev); return NULL; } } if (dev && domain_context_mapping(domain, dev)) { - pr_err("Domain context map for %s failed\n", dev_name(dev)); + dev_err(dev, "Domain context map failed\n"); dmar_remove_one_dev_info(domain, dev); return NULL; } @@ -2723,13 +2722,12 @@ static int domain_prepare_identity_map(struct device *dev, range which is reserved in E820, so which didn't get set up to start with in si_domain */ if (domain == si_domain && hw_pass_through) { - pr_warn("Ignoring identity map for HW passthrough device %s [0x%Lx - 0x%Lx]\n", - dev_name(dev), start, end); + dev_warn(dev, "Ignoring identity map for HW passthrough [0x%Lx - 0x%Lx]\n", +start, end); return 0; } - pr_info("Setting identity map for device %s [0x%Lx - 0x%Lx]\n", - dev_name(dev), start, end); + dev_info(dev, "Setting identity map [0x%Lx - 0x%Lx]\n", start, end); if (end < start) { WARN(1, "Your BIOS is broken; RMRR ends before it starts!\n" @@ -3016,8 +3014,8 @@ static int __init dev_prepare_static_identity_mapping(struct device *dev, int hw ret = domain_add_dev_info(si_domain, dev); if (!ret) - pr_info("%s identity mapping for device %s\n", - hw ? "Hardware" : "Software", dev_name(dev)); + dev_info(dev, "%s identity mapping\n", +hw ? "Hardware" : "Software"); else if (ret == -ENODEV) /* device not associated with an iommu */ ret = 0; @@ -3550,8 +3548,7 @@ static unsigned long intel_alloc_iova(struct device *dev, iova_pfn = alloc_iova_fast(&domain->iovad, nrpages, IOVA_PFN(dma_mask), true); if (unlikely(!iova_pfn)) { - pr_err("Allocating %ld-page iova for %s failed", - nrpages, dev_name(dev)); + dev_err(dev, "Allocating %ld-page iova failed", nrpages); return 0; } @@ -3599,7 +3596,7 @@ struct dmar_domain *get_valid_domain_for_dev(struct device *dev) out: if (!domain) - pr_err("Allocating domain for %s failed\n", dev_name(dev)); + dev_err(dev, "Allocating domain failed\n"); return domain; @@ -3626,8 +3623,7 @@ static int iommu_no_mapping(struct device *dev) * to non-identity mapping. */ dmar_remove_one_dev_info(si_domain, dev); - pr_info("32bit %s uses non-identity mapping\n", - dev_name(dev)); + dev_info(dev, "32bit DMA use
[PATCH v1 6/7] iommu/vt-d: Remove misleading "domain 0" test from domain_exit()
From: Bjorn Helgaas The "Domain 0 is reserved, so dont process it" comment suggests that a NULL pointer corresponds to domain 0. I don't think that's true, and in any case, every caller supplies a non-NULL domain pointer that has already been dereferenced, so the test is unnecessary. Remove the test for a null "domain" pointer. No functional change intended. This null pointer check was added by 5e98c4b1d6e8 ("Allocation and free functions of virtual machine domain"). Signed-off-by: Bjorn Helgaas --- drivers/iommu/intel-iommu.c |4 1 file changed, 4 deletions(-) diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c index 6e9f277bfd6d..b0860a8c48d4 100644 --- a/drivers/iommu/intel-iommu.c +++ b/drivers/iommu/intel-iommu.c @@ -1903,10 +1903,6 @@ static void domain_exit(struct dmar_domain *domain) { struct page *freelist; - /* Domain 0 is reserved, so dont process it */ - if (!domain) - return; - /* Remove associated devices and clear attached or cached domains */ rcu_read_lock(); domain_remove_dev_info(domain); ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v1 4/7] iommu/vt-d: Remove unnecessary local variable initializations
From: Bjorn Helgaas A local variable initialization is a hint that the variable will be used in an unusual way. If the initialization is unnecessary, that hint becomes a distraction. Remove unnecessary initializations. No functional change intended. Signed-off-by: Bjorn Helgaas --- drivers/iommu/intel-iommu.c | 27 +-- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c index 81077803880f..2acd08c82cdc 100644 --- a/drivers/iommu/intel-iommu.c +++ b/drivers/iommu/intel-iommu.c @@ -865,7 +865,7 @@ static void free_context_table(struct intel_iommu *iommu) static struct dma_pte *pfn_to_dma_pte(struct dmar_domain *domain, unsigned long pfn, int *target_level) { - struct dma_pte *parent, *pte = NULL; + struct dma_pte *parent, *pte; int level = agaw_to_level(domain->agaw); int offset; @@ -922,7 +922,7 @@ static struct dma_pte *dma_pfn_level_pte(struct dmar_domain *domain, unsigned long pfn, int level, int *large_page) { - struct dma_pte *parent, *pte = NULL; + struct dma_pte *parent, *pte; int total = agaw_to_level(domain->agaw); int offset; @@ -954,7 +954,7 @@ static void dma_pte_clear_range(struct dmar_domain *domain, unsigned long start_pfn, unsigned long last_pfn) { - unsigned int large_page = 1; + unsigned int large_page; struct dma_pte *first_pte, *pte; BUG_ON(!domain_pfn_supported(domain, start_pfn)); @@ -1132,7 +1132,7 @@ static struct page *domain_unmap(struct dmar_domain *domain, unsigned long start_pfn, unsigned long last_pfn) { - struct page *freelist = NULL; + struct page *freelist; BUG_ON(!domain_pfn_supported(domain, start_pfn)); BUG_ON(!domain_pfn_supported(domain, last_pfn)); @@ -1763,7 +1763,7 @@ static int domain_attach_iommu(struct dmar_domain *domain, static int domain_detach_iommu(struct dmar_domain *domain, struct intel_iommu *iommu) { - int num, count = INT_MAX; + int num, count; assert_spin_locked(&device_domain_lock); assert_spin_locked(&iommu->lock); @@ -1902,7 +1902,7 @@ static int domain_init(struct dmar_domain *domain, struct intel_iommu *iommu, static void domain_exit(struct dmar_domain *domain) { - struct page *freelist = NULL; + struct page *freelist; /* Domain 0 is reserved, so dont process it */ if (!domain) @@ -2583,7 +2583,7 @@ static int get_last_alias(struct pci_dev *pdev, u16 alias, void *opaque) static struct dmar_domain *find_or_alloc_domain(struct device *dev, int gaw) { - struct device_domain_info *info = NULL; + struct device_domain_info *info; struct dmar_domain *domain = NULL; struct intel_iommu *iommu; u16 dma_alias; @@ -2807,7 +2807,7 @@ static int md_domain_init(struct dmar_domain *domain, int guest_width); static int __init si_domain_init(int hw) { - int nid, ret = 0; + int nid, ret; si_domain = alloc_domain(DOMAIN_FLAG_STATIC_IDENTITY); if (!si_domain) @@ -2931,7 +2931,6 @@ static bool device_is_rmrr_locked(struct device *dev) static int iommu_should_identity_map(struct device *dev, int startup) { - if (dev_is_pci(dev)) { struct pci_dev *pdev = to_pci_dev(dev); @@ -3527,7 +3526,7 @@ static unsigned long intel_alloc_iova(struct device *dev, struct dmar_domain *domain, unsigned long nrpages, uint64_t dma_mask) { - unsigned long iova_pfn = 0; + unsigned long iova_pfn; /* Restrict dma_mask to the width that the iommu can handle */ dma_mask = min_t(uint64_t, DOMAIN_MAX_ADDR(domain->gaw), dma_mask); @@ -4333,7 +4332,7 @@ int dmar_check_one_atsr(struct acpi_dmar_header *hdr, void *arg) static int intel_iommu_add(struct dmar_drhd_unit *dmaru) { - int sp, ret = 0; + int sp, ret; struct intel_iommu *iommu = dmaru->iommu; if (g_iommus[iommu->seq_id]) @@ -4497,7 +4496,7 @@ int dmar_find_matched_atsr_unit(struct pci_dev *dev) int dmar_iommu_notify_scope_dev(struct dmar_pci_notify_info *info) { - int ret = 0; + int ret; struct dmar_rmrr_unit *rmrru; struct dmar_atsr_unit *atsru; struct acpi_dmar_atsr *atsr; @@ -4514,7 +4513,7 @@ int dmar_iommu_notify_scope_dev(struct dmar_pci_notify_info *info) ((void *)rmrr) + rmrr->header.length, rmrr->segment, rmrru->devices, rmrru->devices_cnt); - if(ret < 0) +
[PATCH v1 7/7] iommu/vt-d: Simplify control flow
From: Bjorn Helgaas Simplify control flow by returning immediately when we know the result. No functional change intended. Signed-off-by: Bjorn Helgaas --- drivers/iommu/intel-iommu.c | 31 +-- 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c index b0860a8c48d4..6eaa4ada6e1d 100644 --- a/drivers/iommu/intel-iommu.c +++ b/drivers/iommu/intel-iommu.c @@ -509,12 +509,12 @@ static void set_iommu_domain(struct intel_iommu *iommu, u16 did, void *alloc_pgtable_page(int node) { struct page *page; - void *vaddr = NULL; page = alloc_pages_node(node, GFP_ATOMIC | __GFP_ZERO, 0); - if (page) - vaddr = page_address(page); - return vaddr; + if (!page) + return NULL; + + return page_address(page); } void free_pgtable_page(void *vaddr) @@ -2606,20 +2606,19 @@ static struct dmar_domain *find_or_alloc_domain(struct device *dev, int gaw) /* DMA alias already has a domain, use it */ if (info) - goto out; + return domain; } /* Allocate and initialize new domain for the device */ domain = alloc_domain(0); if (!domain) return NULL; + if (domain_init(domain, iommu, gaw)) { domain_exit(domain); return NULL; } -out: - return domain; } @@ -2665,11 +2664,11 @@ static struct dmar_domain *get_domain_for_dev(struct device *dev, int gaw) domain = find_domain(dev); if (domain) - goto out; + return domain; domain = find_or_alloc_domain(dev, gaw); if (!domain) - goto out; + return NULL; tmp = set_domain_for_dev(dev, domain); if (!tmp || domain != tmp) { @@ -2677,8 +2676,6 @@ static struct dmar_domain *get_domain_for_dev(struct device *dev, int gaw) domain = tmp; } -out: - return domain; } @@ -3558,11 +3555,13 @@ struct dmar_domain *get_valid_domain_for_dev(struct device *dev) domain = find_domain(dev); if (domain) - goto out; + return domain; domain = find_or_alloc_domain(dev, DEFAULT_DOMAIN_ADDRESS_WIDTH); - if (!domain) - goto out; + if (!domain) { + dev_err(dev, "Allocating domain failed\n"); + return NULL; + } /* We have a new domain - setup possible RMRRs for the device */ rcu_read_lock(); @@ -3587,12 +3586,8 @@ struct dmar_domain *get_valid_domain_for_dev(struct device *dev) domain = tmp; } -out: - if (!domain) dev_err(dev, "Allocating domain failed\n"); - - return domain; } ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v1 2/7] iommu/amd: Use dev_printk() when possible
From: Bjorn Helgaas Use dev_printk() when possible so the IOMMU messages are more consistent with other messages related to the device. Signed-off-by: Bjorn Helgaas --- drivers/iommu/amd_iommu.c | 25 +++-- drivers/iommu/amd_iommu_init.c | 20 ++-- 2 files changed, 21 insertions(+), 24 deletions(-) diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c index 87ba23a75b38..fee9c9049d7a 100644 --- a/drivers/iommu/amd_iommu.c +++ b/drivers/iommu/amd_iommu.c @@ -18,6 +18,7 @@ */ #define pr_fmt(fmt) "AMD-Vi: " fmt +#define dev_fmt(fmt)pr_fmt(fmt) #include #include @@ -279,10 +280,10 @@ static u16 get_alias(struct device *dev) return pci_alias; } - pr_info("Using IVRS reported alias %02x:%02x.%d " - "for device %s[%04x:%04x], kernel reported alias " + pci_info(pdev, "Using IVRS reported alias %02x:%02x.%d " + "for device [%04x:%04x], kernel reported alias " "%02x:%02x.%d\n", PCI_BUS_NUM(ivrs_alias), PCI_SLOT(ivrs_alias), - PCI_FUNC(ivrs_alias), dev_name(dev), pdev->vendor, pdev->device, + PCI_FUNC(ivrs_alias), pdev->vendor, pdev->device, PCI_BUS_NUM(pci_alias), PCI_SLOT(pci_alias), PCI_FUNC(pci_alias)); @@ -293,9 +294,8 @@ static u16 get_alias(struct device *dev) if (pci_alias == devid && PCI_BUS_NUM(ivrs_alias) == pdev->bus->number) { pci_add_dma_alias(pdev, ivrs_alias & 0xff); - pr_info("Added PCI DMA alias %02x.%d for %s\n", - PCI_SLOT(ivrs_alias), PCI_FUNC(ivrs_alias), - dev_name(dev)); + pci_info(pdev, "Added PCI DMA alias %02x.%d\n", + PCI_SLOT(ivrs_alias), PCI_FUNC(ivrs_alias)); } return ivrs_alias; @@ -545,7 +545,7 @@ static void amd_iommu_report_page_fault(u16 devid, u16 domain_id, dev_data = get_dev_data(&pdev->dev); if (dev_data && __ratelimit(&dev_data->rs)) { - dev_err(&pdev->dev, "Event logged [IO_PAGE_FAULT domain=0x%04x address=0x%llx flags=0x%04x]\n", + pci_err(pdev, "Event logged [IO_PAGE_FAULT domain=0x%04x address=0x%llx flags=0x%04x]\n", domain_id, address, flags); } else if (printk_ratelimit()) { pr_err("Event logged [IO_PAGE_FAULT device=%02x:%02x.%x domain=0x%04x address=0x%llx flags=0x%04x]\n", @@ -2251,8 +2251,7 @@ static int amd_iommu_add_device(struct device *dev) ret = iommu_init_device(dev); if (ret) { if (ret != -ENOTSUPP) - pr_err("Failed to initialize device %s - trying to proceed anyway\n", - dev_name(dev)); + dev_err(dev, "Failed to initialize - trying to proceed anyway\n"); iommu_ignore_device(dev); dev->dma_ops = NULL; @@ -2605,8 +2604,7 @@ static int map_sg(struct device *dev, struct scatterlist *sglist, return nelems; out_unmap: - pr_err("%s: IOMMU mapping error in map_sg (io-pages: %d)\n", - dev_name(dev), npages); + dev_err(dev, "IOMMU mapping error in map_sg (io-pages: %d)\n", npages); for_each_sg(sglist, s, nelems, i) { int j, pages = iommu_num_pages(sg_phys(s), s->length, PAGE_SIZE); @@ -2800,7 +2798,7 @@ static int init_reserved_iova_ranges(void) IOVA_PFN(r->start), IOVA_PFN(r->end)); if (!val) { - pr_err("Reserve pci-resource range failed\n"); + pci_err(pdev, "Reserve pci-resource range %pR failed\n", r); return -ENOMEM; } } @@ -3170,8 +3168,7 @@ static void amd_iommu_get_resv_regions(struct device *dev, length, prot, IOMMU_RESV_DIRECT); if (!region) { - pr_err("Out of memory allocating dm-regions for %s\n", - dev_name(dev)); + dev_err(dev, "Out of memory allocating dm-regions\n"); return; } list_add_tail(®ion->list, head); diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c index 66123b911ec8..f773792d77fd 100644 --- a/drivers/iommu/amd_iommu_init.c +++ b/drivers/iommu/amd_iommu_init.c @@ -18,6 +18,7 @@ */ #define pr_fmt(fmt) "AMD-Vi: " fmt +#define dev_fmt(fmt)pr_fmt(fmt) #include #include @@ -1457,8 +1458,7 @@ static void amd_iommu_erratum_746_workaround(struct amd_iommu *iommu) pci_write_config_dword(iommu->dev, 0xf0, 0x90 | (1 << 8)); pci_write_confi
[PATCH v1 5/7] iommu/vt-d: Remove unused dmar_remove_one_dev_info() argument
From: Bjorn Helgaas domain_remove_dev_info() takes a struct dmar_domain * argument, but doesn't use it. Remove it. No functional change intended. The last use of this argument was removed by 127c761598f7 ("iommu/vt-d: Pass device_domain_info to __dmar_remove_one_dev_info"). Signed-off-by: Bjorn Helgaas --- drivers/iommu/intel-iommu.c | 20 +--- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c index 2acd08c82cdc..6e9f277bfd6d 100644 --- a/drivers/iommu/intel-iommu.c +++ b/drivers/iommu/intel-iommu.c @@ -343,8 +343,7 @@ static int g_num_of_iommus; static void domain_exit(struct dmar_domain *domain); static void domain_remove_dev_info(struct dmar_domain *domain); -static void dmar_remove_one_dev_info(struct dmar_domain *domain, -struct device *dev); +static void dmar_remove_one_dev_info(struct device *dev); static void __dmar_remove_one_dev_info(struct device_domain_info *info); static void domain_context_clear(struct intel_iommu *iommu, struct device *dev); @@ -2546,7 +2545,7 @@ static struct dmar_domain *dmar_insert_one_dev_info(struct intel_iommu *iommu, ret = intel_pasid_alloc_table(dev); if (ret) { dev_err(dev, "PASID table allocation failed\n"); - dmar_remove_one_dev_info(domain, dev); + dmar_remove_one_dev_info(dev); return NULL; } @@ -2561,14 +2560,14 @@ static struct dmar_domain *dmar_insert_one_dev_info(struct intel_iommu *iommu, spin_unlock(&iommu->lock); if (ret) { dev_err(dev, "Setup RID2PASID failed\n"); - dmar_remove_one_dev_info(domain, dev); + dmar_remove_one_dev_info(dev); return NULL; } } if (dev && domain_context_mapping(domain, dev)) { dev_err(dev, "Domain context map failed\n"); - dmar_remove_one_dev_info(domain, dev); + dmar_remove_one_dev_info(dev); return NULL; } @@ -3621,7 +3620,7 @@ static int iommu_no_mapping(struct device *dev) * 32 bit DMA is removed from si_domain and fall back * to non-identity mapping. */ - dmar_remove_one_dev_info(si_domain, dev); + dmar_remove_one_dev_info(dev); dev_info(dev, "32bit DMA uses non-identity mapping\n"); return 0; } @@ -4567,7 +4566,7 @@ static int device_notifier(struct notifier_block *nb, if (!domain) return 0; - dmar_remove_one_dev_info(domain, dev); + dmar_remove_one_dev_info(dev); if (!domain_type_is_vm_or_si(domain) && list_empty(&domain->devices)) domain_exit(domain); @@ -4980,8 +4979,7 @@ static void __dmar_remove_one_dev_info(struct device_domain_info *info) free_devinfo_mem(info); } -static void dmar_remove_one_dev_info(struct dmar_domain *domain, -struct device *dev) +static void dmar_remove_one_dev_info(struct device *dev) { struct device_domain_info *info; unsigned long flags; @@ -5070,7 +5068,7 @@ static int intel_iommu_attach_device(struct iommu_domain *domain, old_domain = find_domain(dev); if (old_domain) { rcu_read_lock(); - dmar_remove_one_dev_info(old_domain, dev); + dmar_remove_one_dev_info(dev); rcu_read_unlock(); if (!domain_type_is_vm_or_si(old_domain) && @@ -5117,7 +5115,7 @@ static int intel_iommu_attach_device(struct iommu_domain *domain, static void intel_iommu_detach_device(struct iommu_domain *domain, struct device *dev) { - dmar_remove_one_dev_info(to_dmar_domain(domain), dev); + dmar_remove_one_dev_info(dev); } static int intel_iommu_map(struct iommu_domain *domain, ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v1 0/7] iommu: Minor cleanups and dev_printk() usage
I've had these minor iommu cleanups lying around for a while, but the ugly dmesg logs from [1] prompted me to finally post them. Take what you like, ignore the rest, and tell me so I can clear out my queue of old stuff. These fix no actual bugs. [1] https://lore.kernel.org/linux-pci/1547649064-19019-1-git-send-email-liudongdo...@huawei.com --- Bjorn Helgaas (7): iommu: Use dev_printk() when possible iommu/amd: Use dev_printk() when possible iommu/vt-d: Use dev_printk() when possible iommu/vt-d: Remove unnecessary local variable initializations iommu/vt-d: Remove unused dmar_remove_one_dev_info() argument iommu/vt-d: Remove misleading "domain 0" test from domain_exit() iommu/vt-d: Simplify control flow drivers/iommu/amd_iommu.c | 25 +++ drivers/iommu/amd_iommu_init.c | 20 +++--- drivers/iommu/intel-iommu.c| 136 +--- drivers/iommu/iommu.c |8 +- 4 files changed, 84 insertions(+), 105 deletions(-) ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v1 1/7] iommu: Use dev_printk() when possible
From: Bjorn Helgaas Use dev_printk() when possible so the IOMMU messages are more consistent with other messages related to the device. E.g., I think these messages related to surprise hotplug: pciehp :80:10.0:pcie004: Slot(36): Link Down iommu: Removing device :87:00.0 from group 12 pciehp :80:10.0:pcie004: Slot(36): Card present pcieport :80:10.0: Data Link Layer Link Active not set in 1000 msec would be easier to read as these (also requires some PCI changes not included here): pci :80:10.0: Slot(36): Link Down pci :87:00.0: Removing from iommu group 12 pci :80:10.0: Slot(36): Card present pci :80:10.0: Data Link Layer Link Active not set in 1000 msec Signed-off-by: Bjorn Helgaas --- drivers/iommu/iommu.c |8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index 3ed4db334341..54c9d18fe31d 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -668,7 +668,7 @@ int iommu_group_add_device(struct iommu_group *group, struct device *dev) trace_add_device_to_group(group->id, dev); - pr_info("Adding device %s to group %d\n", dev_name(dev), group->id); + dev_info(dev, "Adding to iommu group %d\n", group->id); return 0; @@ -684,7 +684,7 @@ int iommu_group_add_device(struct iommu_group *group, struct device *dev) sysfs_remove_link(&dev->kobj, "iommu_group"); err_free_device: kfree(device); - pr_err("Failed to add device %s to group %d: %d\n", dev_name(dev), group->id, ret); + dev_err(dev, "Failed to add to iommu group %d: %d\n", group->id, ret); return ret; } EXPORT_SYMBOL_GPL(iommu_group_add_device); @@ -701,7 +701,7 @@ void iommu_group_remove_device(struct device *dev) struct iommu_group *group = dev->iommu_group; struct group_device *tmp_device, *device = NULL; - pr_info("Removing device %s from group %d\n", dev_name(dev), group->id); + dev_info(dev, "Removing from iommu group %d\n", group->id); /* Pre-notify listeners that a device is being removed. */ blocking_notifier_call_chain(&group->notifier, @@ -1951,7 +1951,7 @@ int iommu_request_dm_for_dev(struct device *dev) iommu_domain_free(group->default_domain); group->default_domain = dm_domain; - pr_info("Using direct mapping for device %s\n", dev_name(dev)); + dev_info(dev, "Using iommu direct mapping\n"); ret = 0; out: ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH/RFC] driver core: Postpone DMA tear-down until after devres release
Hi Robin, On Fri, Feb 8, 2019 at 6:55 PM Robin Murphy wrote: > On 08/02/2019 16:40, Joerg Roedel wrote: > > On Thu, Feb 07, 2019 at 08:36:53PM +0100, Geert Uytterhoeven wrote: > >> diff --git a/drivers/base/dd.c b/drivers/base/dd.c > >> index 8ac10af17c0043a3..d62487d024559620 100644 > >> --- a/drivers/base/dd.c > >> +++ b/drivers/base/dd.c > >> @@ -968,9 +968,9 @@ static void __device_release_driver(struct device > >> *dev, struct device *parent) > >> drv->remove(dev); > >> > >> device_links_driver_cleanup(dev); > >> -arch_teardown_dma_ops(dev); > >> > >> devres_release_all(dev); > >> +arch_teardown_dma_ops(dev); > >> dev->driver = NULL; > >> dev_set_drvdata(dev, NULL); > >> if (dev->pm_domain && dev->pm_domain->dismiss) > > > > Thanks for the fix! Should it also be tagged for stable and get a Fixes FTR, Greg has added it to driver-core-testing, with a CC to stable. > > tag? I know it only triggers with a fix in v5.0-rc, but still... > > I think so: > > Fixes: 09515ef5ddad ("of/acpi: Configure dma operations at probe time > for platform/amba/pci bus devices") Thanks! It won't backport cleanly due to commit dc3c05504d38849f ("dma-mapping: remove dma_deconfigure") in v4.20, though. > There aren't many drivers using dmam_alloc_*(), let alone which would > also find themselves behind an IOMMU on an Arm system, but it turns out > I actually have another one which can reproduce the BUG() with 5.0-rc. SATA core uses dmam_alloc_*(). > I've tried a 4.12 kernel with a bit of instrumentation[1] and sure > enough the devres-managed buffer is freed with the wrong ops[2] even > then. How it manages not to blow up more catastrophically I have no > idea... I guess at best it just leaks the buffers and IOMMU mappings, > and at worst quietly frees random other pages instead. May depend on the actual ops, and whether CMA is used or not. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH/RFC] driver core: Postpone DMA tear-down until after devres release
On 08/02/2019 16:40, Joerg Roedel wrote: Hi Geert, On Thu, Feb 07, 2019 at 08:36:53PM +0100, Geert Uytterhoeven wrote: diff --git a/drivers/base/dd.c b/drivers/base/dd.c index 8ac10af17c0043a3..d62487d024559620 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -968,9 +968,9 @@ static void __device_release_driver(struct device *dev, struct device *parent) drv->remove(dev); device_links_driver_cleanup(dev); - arch_teardown_dma_ops(dev); devres_release_all(dev); + arch_teardown_dma_ops(dev); dev->driver = NULL; dev_set_drvdata(dev, NULL); if (dev->pm_domain && dev->pm_domain->dismiss) Thanks for the fix! Should it also be tagged for stable and get a Fixes tag? I know it only triggers with a fix in v5.0-rc, but still... I think so: Fixes: 09515ef5ddad ("of/acpi: Configure dma operations at probe time for platform/amba/pci bus devices") There aren't many drivers using dmam_alloc_*(), let alone which would also find themselves behind an IOMMU on an Arm system, but it turns out I actually have another one which can reproduce the BUG() with 5.0-rc. I've tried a 4.12 kernel with a bit of instrumentation[1] and sure enough the devres-managed buffer is freed with the wrong ops[2] even then. How it manages not to blow up more catastrophically I have no idea... I guess at best it just leaks the buffers and IOMMU mappings, and at worst quietly frees random other pages instead. Robin. -- [1] diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h index 4f3eecedca2d..f4dbaa5598e3 100644 --- a/include/linux/dma-mapping.h +++ b/include/linux/dma-mapping.h @@ -491,6 +491,7 @@ static inline void *dma_alloc_attrs(struct device *dev, size_t size, return NULL; cpu_addr = ops->alloc(dev, size, dma_handle, flag, attrs); + dev_info(dev, "alloc %lx %lx\n", (unsigned long)cpu_addr, (unsigned long)ops); debug_dma_alloc_coherent(dev, size, *dma_handle, cpu_addr); return cpu_addr; } @@ -512,6 +513,7 @@ static inline void dma_free_attrs(struct device *dev, size_t size, debug_dma_free_coherent(dev, size, cpu_addr, dma_handle); ops->free(dev, size, cpu_addr, dma_handle, attrs); + dev_info(dev, "free %lx %lx\n", (unsigned long)cpu_addr, (unsigned long)ops); } static inline void *dma_alloc_coherent(struct device *dev, size_t size, - [2] / # echo ':03:00.0' > /sys/bus/pci/drivers/sata_sil24/bind [ 107.417252] sata_sil24 :03:00.0: alloc 0a6f9000 089b8090 [ 107.424397] sata_sil24 :03:00.0: alloc 0a719000 089b8090 [ 107.432216] scsi host0: sata_sil24 [ 107.436134] scsi host1: sata_sil24 [ 107.439853] ata7: SATA max UDMA/100 host m128@0x50084000 port 0x5008 irq 51 [ 107.447228] ata8: SATA max UDMA/100 host m128@0x50084000 port 0x50082000 irq 51 / # echo ':03:00.0' > /sys/bus/pci/drivers/sata_sil24/unbind ... [ 112.048654] sata_sil24 :03:00.0: free 0a719000 089b8120 [ 112.055579] sata_sil24 :03:00.0: free 0a6f9000 089b8120 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[git pull] IOMMU Fixes for Linux v5.0-rc5
Hi Linus, The following changes since commit 9825bd94e3a2baae1f4874767ae3a7d4c049720e: iommu/amd: Fix IOMMU page flush when detach device from a domain (2019-01-24 15:24:49 +0100) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git tags/iommu-fixes-v5.0-rc5 for you to fetch changes up to 8950dcd83ae7d62bdc2a60507949acebd85399f2: iommu/vt-d: Leave scalable mode default off (2019-01-30 17:23:58 +0100) IOMMU Fix for Linux v5.0-rc5: * Intel decided to leave the newly added Scalable Mode Feature default-disabled for now. The patch here accomplishes that. Lu Baolu (1): iommu/vt-d: Leave scalable mode default off Documentation/admin-guide/kernel-parameters.txt | 7 +++ drivers/iommu/intel-iommu.c | 8 2 files changed, 7 insertions(+), 8 deletions(-) Please pull. Thanks, Joerg signature.asc Description: Digital signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH/RFC] driver core: Postpone DMA tear-down until after devres release
Hi Geert, On Thu, Feb 07, 2019 at 08:36:53PM +0100, Geert Uytterhoeven wrote: > diff --git a/drivers/base/dd.c b/drivers/base/dd.c > index 8ac10af17c0043a3..d62487d024559620 100644 > --- a/drivers/base/dd.c > +++ b/drivers/base/dd.c > @@ -968,9 +968,9 @@ static void __device_release_driver(struct device *dev, > struct device *parent) > drv->remove(dev); > > device_links_driver_cleanup(dev); > - arch_teardown_dma_ops(dev); > > devres_release_all(dev); > + arch_teardown_dma_ops(dev); > dev->driver = NULL; > dev_set_drvdata(dev, NULL); > if (dev->pm_domain && dev->pm_domain->dismiss) Thanks for the fix! Should it also be tagged for stable and get a Fixes tag? I know it only triggers with a fix in v5.0-rc, but still... Regards, Joerg ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] remove unused end_pfn
On Fri, Feb 08, 2019 at 02:04:38PM +, Tom Murphy wrote: > This variable is useless. > > --- > drivers/iommu/dma-iommu.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) A similar change is already in the iommu tree[1]. Joerg [1] https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git/commit/?h=core ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH V3 2/3] HYPERV/IOMMU: Add Hyper-V stub IOMMU driver
On Thu, Feb 07, lantianyu1...@gmail.com wrote: > +++ b/drivers/iommu/Kconfig > +config HYPERV_IOMMU > + bool "Hyper-V x2APIC IRQ Handling" > + depends on HYPERV > + select IOMMU_API > + help Consider adding 'default HYPERV' like some other drivers already do it. Olaf signature.asc Description: PGP signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] fix flush_tlb_all typo
--- include/linux/iommu.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/linux/iommu.h b/include/linux/iommu.h index a1d28f42cb77..bb4bf1269e5d 100644 --- a/include/linux/iommu.h +++ b/include/linux/iommu.h @@ -167,7 +167,7 @@ struct iommu_resv_region { * @detach_dev: detach device from an iommu domain * @map: map a physically contiguous memory region to an iommu domain * @unmap: unmap a physically contiguous memory region from an iommu domain - * @flush_tlb_all: Synchronously flush all hardware TLBs for this domain + * @flush_iotlb_all: Synchronously flush all hardware TLBs for this domain * @tlb_range_add: Add a given iova range to the flush queue for this domain * @tlb_sync: Flush all queued ranges from the hardware TLBs and empty flush *queue -- 2.17.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] remove unused end_pfn
This variable is useless. --- drivers/iommu/dma-iommu.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c index 8e04b0603a4a..eff301d5e496 100644 --- a/drivers/iommu/dma-iommu.c +++ b/drivers/iommu/dma-iommu.c @@ -291,7 +291,7 @@ int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base, { struct iommu_dma_cookie *cookie = domain->iova_cookie; struct iova_domain *iovad = &cookie->iovad; - unsigned long order, base_pfn, end_pfn; + unsigned long order, base_pfn; int attr; if (!cookie || cookie->type != IOMMU_DMA_IOVA_COOKIE) @@ -300,7 +300,6 @@ int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base, /* Use the smallest supported page size for IOVA granularity */ order = __ffs(domain->pgsize_bitmap); base_pfn = max_t(unsigned long, 1, base >> order); - end_pfn = (base + size - 1) >> order; /* Check the domain allows at least some access to the device... */ if (domain->geometry.force_aperture) { -- 2.17.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH/RFC] driver core: Postpone DMA tear-down until after devres release
On 07/02/2019 19:36, Geert Uytterhoeven wrote: When unbinding the (IOMMU-enabled) R-Car SATA device on Salvator-XS (R-Car H3 ES2.0), in preparation of rebinding against vfio-platform for device pass-through for virtualization: echo ee30.sata > /sys/bus/platform/drivers/sata_rcar/unbind the kernel crashes with: Unable to handle kernel paging request at virtual address ffbf029c Mem abort info: ESR = 0x9606 Exception class = DABT (current EL), IL = 32 bits SET = 0, FnV = 0 EA = 0, S1PTW = 0 Data abort info: ISV = 0, ISS = 0x0006 CM = 0, WnR = 0 swapper pgtable: 4k pages, 39-bit VAs, pgdp = 7e8c586c [ffbf029c] pgd=00073bfc6003, pud=00073bfc6003, pmd= Internal error: Oops: 9606 [#1] SMP Modules linked in: CPU: 0 PID: 1098 Comm: bash Not tainted 5.0.0-rc5-salvator-x-00452-g37596f884f4318ef #287 Hardware name: Renesas Salvator-X 2nd version board based on r8a7795 ES2.0+ (DT) pstate: 6045 (nZCv daif +PAN -UAO) pc : __free_pages+0x8/0x58 lr : __dma_direct_free_pages+0x50/0x5c sp : ff801268baa0 x29: ff801268baa0 x28: x27: ffc6f9c60bf0 x26: ffc6f9c60bf0 x25: ffc6f9c60810 x24: x23: f000 x22: ff8012145000 x21: 0800 x20: ffbf029fffc8 x19: x18: ffc6f86c42c8 x17: x16: 0070 x15: 0003 x14: x13: ff801103d7f8 x12: 0028 x11: ff807604 x10: 9ad8 x9 : ff80110126d0 x8 : ffc6f7563000 x7 : 6b6b6b6b6b6b6b6b x6 : 0018 x5 : ff8011cf3cc8 x4 : 4000 x3 : 0008 x2 : 0001 x1 : x0 : ffbf029fffc8 Process bash (pid: 1098, stack limit = 0xc38e3e32) Call trace: __free_pages+0x8/0x58 __dma_direct_free_pages+0x50/0x5c arch_dma_free+0x1c/0x98 dma_direct_free+0x14/0x24 dma_free_attrs+0x9c/0xdc dmam_release+0x18/0x20 release_nodes+0x25c/0x28c devres_release_all+0x48/0x4c device_release_driver_internal+0x184/0x1f0 device_release_driver+0x14/0x1c unbind_store+0x70/0xb8 drv_attr_store+0x24/0x34 sysfs_kf_write+0x4c/0x64 kernfs_fop_write+0x154/0x1c4 __vfs_write+0x34/0x164 vfs_write+0xb4/0x16c ksys_write+0x5c/0xbc __arm64_sys_write+0x14/0x1c el0_svc_common+0x98/0x114 el0_svc_handler+0x1c/0x24 el0_svc+0x8/0xc Code: d51b4234 17fa a9bf7bfd 910003fd (b9403404) ---[ end trace 8c564cdd3a1a840f ]--- While I've bisected this to commit e8e683ae9a736407 ("iommu/of: Fix probe-deferral"), and reverting that commit on post-v5.0-rc4 kernels does fix the problem, this turned out to be a red herring. On arm64, arch_teardown_dma_ops() resets dev->dma_ops to NULL. Hence if a driver has used a managed DMA allocation API, the allocated DMA memory will be freed using the direct DMA ops, while it may have been allocated using a custom DMA ops (iommu_dma_ops in this case). Fix this by reversing the order of the calls to devres_release_all() and arch_teardown_dma_ops(). Signed-off-by: Geert Uytterhoeven --- Question: Is this safe on arm32, which calls arm_teardown_iommu_dma_ops() instead of resetting dev->dma_ops? Should be - the principle is the same, and even if it did break that would only be indicative of a separate bug being hidden by this one. This fix looks entirely valid and correct to me: Reviewed-by: Robin Murphy --- Adding some debug code, and comparing before/after commit e8e683ae9a736407: sata_rcar ee30.sata: of_iommu_configure:227: err = -517 -sata_rcar ee30.sata: of_iommu_configure:230: calling iommu_probe_device() -sata_rcar ee30.sata: iommu_probe_device:122: Calling ipmmu_add_device -sata_rcar ee30.sata: ipmmu_add_device:893 -sata_rcar ee30.sata: of_iommu_configure:232: iommu_probe_device() returned -19 -sata_rcar ee30.sata: dma_alloc_attrs:257: size 2048, ops = (null) -sata_rcar ee30.sata: __dma_direct_alloc_pages:104: size 4096 -sata_rcar ee30.sata: __dma_direct_alloc_pages:115: trying dma_alloc_from_contiguous() -sata_rcar ee30.sata: dma_alloc_from_contiguous:202: cma_alloc(1) returned page ffbf00d20e00 -sata_rcar ee30.sata: dma_alloc_attrs:271: allocated using dma_direct_alloc() -sata_rcar ee30.sata: dmam_alloc_attrs:98: size 2048 vaddr ff8011e95000 dma_handle 0x0x7c04 attrs 0x0 -scsi host0: sata_rcar -ata1: SATA max UDMA/133 irq 172 -ata1: SATA link up 1.5 Gbps (SStatus 113 SControl 300) DMA memory used to be allocated from CMA, dma_map_ops = NULL. The SATA driver was probed and initialized. +sata_
Re: use generic DMA mapping code in powerpc V4
OK, I will test it. — Christian Sent from my iPhone > On 8. Feb 2019, at 10:18, Christoph Hellwig wrote: > >> On Fri, Feb 08, 2019 at 10:01:46AM +0100, Christian Zigotzky wrote: >> Hi Christoph, >> >> Your new patch fixes the problems with the P.A. Semi Ethernet! :-) > > Thanks a lot once again for testing! > > Now can you test with this patch and the whole series? > > I've updated the powerpc-dma.6 branch to include this fix. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH/RFC] driver core: Postpone DMA tear-down until after devres release
On Thu, Feb 7, 2019 at 8:42 PM Geert Uytterhoeven wrote: > > When unbinding the (IOMMU-enabled) R-Car SATA device on Salvator-XS > (R-Car H3 ES2.0), in preparation of rebinding against vfio-platform for > device pass-through for virtualization: > > echo ee30.sata > /sys/bus/platform/drivers/sata_rcar/unbind > > the kernel crashes with: > > Unable to handle kernel paging request at virtual address ffbf029c > Mem abort info: > ESR = 0x9606 > Exception class = DABT (current EL), IL = 32 bits > SET = 0, FnV = 0 > EA = 0, S1PTW = 0 > Data abort info: > ISV = 0, ISS = 0x0006 > CM = 0, WnR = 0 > swapper pgtable: 4k pages, 39-bit VAs, pgdp = 7e8c586c > [ffbf029c] pgd=00073bfc6003, pud=00073bfc6003, > pmd= > Internal error: Oops: 9606 [#1] SMP > Modules linked in: > CPU: 0 PID: 1098 Comm: bash Not tainted > 5.0.0-rc5-salvator-x-00452-g37596f884f4318ef #287 > Hardware name: Renesas Salvator-X 2nd version board based on r8a7795 > ES2.0+ (DT) > pstate: 6045 (nZCv daif +PAN -UAO) > pc : __free_pages+0x8/0x58 > lr : __dma_direct_free_pages+0x50/0x5c > sp : ff801268baa0 > x29: ff801268baa0 x28: > x27: ffc6f9c60bf0 x26: ffc6f9c60bf0 > x25: ffc6f9c60810 x24: > x23: f000 x22: ff8012145000 > x21: 0800 x20: ffbf029fffc8 > x19: x18: ffc6f86c42c8 > x17: x16: 0070 > x15: 0003 x14: > x13: ff801103d7f8 x12: 0028 > x11: ff807604 x10: 9ad8 > x9 : ff80110126d0 x8 : ffc6f7563000 > x7 : 6b6b6b6b6b6b6b6b x6 : 0018 > x5 : ff8011cf3cc8 x4 : 4000 > x3 : 0008 x2 : 0001 > x1 : x0 : ffbf029fffc8 > Process bash (pid: 1098, stack limit = 0xc38e3e32) > Call trace: > __free_pages+0x8/0x58 > __dma_direct_free_pages+0x50/0x5c > arch_dma_free+0x1c/0x98 > dma_direct_free+0x14/0x24 > dma_free_attrs+0x9c/0xdc > dmam_release+0x18/0x20 > release_nodes+0x25c/0x28c > devres_release_all+0x48/0x4c > device_release_driver_internal+0x184/0x1f0 > device_release_driver+0x14/0x1c > unbind_store+0x70/0xb8 > drv_attr_store+0x24/0x34 > sysfs_kf_write+0x4c/0x64 > kernfs_fop_write+0x154/0x1c4 > __vfs_write+0x34/0x164 > vfs_write+0xb4/0x16c > ksys_write+0x5c/0xbc > __arm64_sys_write+0x14/0x1c > el0_svc_common+0x98/0x114 > el0_svc_handler+0x1c/0x24 > el0_svc+0x8/0xc > Code: d51b4234 17fa a9bf7bfd 910003fd (b9403404) > ---[ end trace 8c564cdd3a1a840f ]--- > > While I've bisected this to commit e8e683ae9a736407 ("iommu/of: Fix > probe-deferral"), and reverting that commit on post-v5.0-rc4 kernels > does fix the problem, this turned out to be a red herring. > > On arm64, arch_teardown_dma_ops() resets dev->dma_ops to NULL. > Hence if a driver has used a managed DMA allocation API, the allocated > DMA memory will be freed using the direct DMA ops, while it may have > been allocated using a custom DMA ops (iommu_dma_ops in this case). > > Fix this by reversing the order of the calls to devres_release_all() and > arch_teardown_dma_ops(). > > Signed-off-by: Geert Uytterhoeven Reviewed-by: Rafael J. Wysocki > --- > Question: > Is this safe on arm32, which calls arm_teardown_iommu_dma_ops() instead > of resetting dev->dma_ops? > > --- > Adding some debug code, and comparing before/after commit > e8e683ae9a736407: > > sata_rcar ee30.sata: of_iommu_configure:227: err = -517 > -sata_rcar ee30.sata: of_iommu_configure:230: calling > iommu_probe_device() > -sata_rcar ee30.sata: iommu_probe_device:122: Calling ipmmu_add_device > -sata_rcar ee30.sata: ipmmu_add_device:893 > -sata_rcar ee30.sata: of_iommu_configure:232: iommu_probe_device() > returned -19 > -sata_rcar ee30.sata: dma_alloc_attrs:257: size 2048, ops = > (null) > -sata_rcar ee30.sata: __dma_direct_alloc_pages:104: size 4096 > -sata_rcar ee30.sata: __dma_direct_alloc_pages:115: trying > dma_alloc_from_contiguous() > -sata_rcar ee30.sata: dma_alloc_from_contiguous:202: cma_alloc(1) > returned page ffbf00d20e00 > -sata_rcar ee30.sata: dma_alloc_attrs:271: allocated using > dma_direct_alloc() > -sata_rcar ee30.sata: dmam_alloc_attrs:98: size 2048 vaddr > ff8011e95000 dma_handle 0x0x7c04 attrs 0x0 > -scsi host0: sata_rcar > -ata1: SATA max UDMA/133 irq 172 > -ata1: SATA link up 1.5 Gbps (SStatus 113 SControl 300) > > DMA memory used to be allocated from CMA, dma_map_ops = NULL. > The SATA driver was probed and initialized. > > +sata_rcar ee30.sata: of_io
Re: use generic DMA mapping code in powerpc V4
On Fri, Feb 08, 2019 at 10:01:46AM +0100, Christian Zigotzky wrote: > Hi Christoph, > > Your new patch fixes the problems with the P.A. Semi Ethernet! :-) Thanks a lot once again for testing! Now can you test with this patch and the whole series? I've updated the powerpc-dma.6 branch to include this fix. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: use generic DMA mapping code in powerpc V4
Hi Christoph, Your new patch fixes the problems with the P.A. Semi Ethernet! :-) Thanks, Christian On 07 February 2019 at 05:34AM, Christian Zigotzky wrote: Hi Christoph, I also didn’t notice the 32-bit DMA mask in your patch. I have to read your patches and descriptions carefully in the future. I will test your new patch at the weekend. Thanks, Christian Sent from my iPhone On 6. Feb 2019, at 16:16, Christoph Hellwig wrote: On Wed, Feb 06, 2019 at 04:15:05PM +0100, Christoph Hellwig wrote: The last good one was 29e7e2287e196f48fe5d2a6e017617723ea979bf ("dma-direct: we might need GFP_DMA for 32-bit dma masks"), if I remember correctly. powerpc/dma: use the dma_direct mapping routines was the one that you said makes the pasemi ethernet stop working. Can you post the dmesg from the failing runs? But I just noticed I sent you a wrong patch - the pasemi ethernet should set a 64-bit DMA mask, not 32-bit. Updated version below, 32-bit would just keep the previous status quo. commit 6c8f88045dee3597b9ce2ea5371eee37073a Author: Christoph Hellwig Date: Mon Feb 4 13:38:22 2019 +0100 pasemi WIP diff --git a/drivers/net/ethernet/pasemi/pasemi_mac.c b/drivers/net/ethernet/pasemi/pasemi_mac.c index 8a31a02c9f47..2d7d1589490a 100644 --- a/drivers/net/ethernet/pasemi/pasemi_mac.c +++ b/drivers/net/ethernet/pasemi/pasemi_mac.c @@ -1716,6 +1716,7 @@ pasemi_mac_probe(struct pci_dev *pdev, const struct pci_device_id *ent) err = -ENODEV; goto out; } +dma_set_mask(&mac->dma_pdev->dev, DMA_BIT_MASK(64)); mac->iob_pdev = pci_get_device(PCI_VENDOR_ID_PASEMI, 0xa001, NULL); if (!mac->iob_pdev) { ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu