Re: [PATCH] iommu: fix min_not_zero() type mismatch warning
I think we agree that the parameter should be a u64 instead, and either Nicolas or Robin (sorry, fading memory) promised to send me a patch for that. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RESEND PATCH v9 2/4] uacce: add uacce driver
On 2019/12/11 上午8:09, kbuild test robot wrote: Hi Zhangfei, Thank you for the patch! Yet something to improve: [auto build test ERROR on cryptodev/master] [also build test ERROR on crypto/master char-misc/char-misc-testing v5.5-rc1 next-20191210] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982] url: https://github.com/0day-ci/linux/commits/Zhangfei-Gao/Add-uacce-module-for-Accelerator/20191210-160210 base: https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git master config: sparc64-allmodconfig (attached as .config) compiler: sparc64-linux-gcc (GCC) 7.5.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree GCC_VERSION=7.5.0 make.cross ARCH=sparc64 If you fix the issue, kindly add following tag Reported-by: kbuild test robot All errors (new ones prefixed by >>): drivers/misc/uacce/uacce.c:112:15: error: variable 'uacce_sva_ops' has initializer but incomplete type static struct iommu_sva_ops uacce_sva_ops = { ^ drivers/misc/uacce/uacce.c:113:3: error: 'struct iommu_sva_ops' has no member named 'mm_exit' .mm_exit = uacce_sva_exit, ^~~ drivers/misc/uacce/uacce.c:113:13: warning: excess elements in struct initializer .mm_exit = uacce_sva_exit, ^~ drivers/misc/uacce/uacce.c:113:13: note: (near initialization for 'uacce_sva_ops') drivers/misc/uacce/uacce.c: In function 'uacce_mm_get': drivers/misc/uacce/uacce.c:144:12: error: implicit declaration of function 'iommu_sva_bind_device'; did you mean 'bus_find_device'? [-Werror=implicit-function-declaration] handle = iommu_sva_bind_device(uacce->parent, mm, uacce_mm); ^ bus_find_device drivers/misc/uacce/uacce.c:144:10: warning: assignment makes pointer from integer without a cast [-Wint-conversion] handle = iommu_sva_bind_device(uacce->parent, mm, uacce_mm); ^ drivers/misc/uacce/uacce.c:148:9: error: implicit declaration of function 'iommu_sva_set_ops'; did you mean 'iommu_setup_dma_ops'? [-Werror=implicit-function-declaration] ret = iommu_sva_set_ops(handle, &uacce_sva_ops); ^ iommu_setup_dma_ops drivers/misc/uacce/uacce.c:152:21: error: implicit declaration of function 'iommu_sva_get_pasid' [-Werror=implicit-function-declaration] uacce_mm->pasid = iommu_sva_get_pasid(handle); ^~~ drivers/misc/uacce/uacce.c:153:26: error: 'IOMMU_PASID_INVALID' undeclared (first use in this function); did you mean 'HV_MSIVALID_INVALID'? if (uacce_mm->pasid == IOMMU_PASID_INVALID) ^~~ HV_MSIVALID_INVALID drivers/misc/uacce/uacce.c:153:26: note: each undeclared identifier is reported only once for each function it appears in drivers/misc/uacce/uacce.c:168:3: error: implicit declaration of function 'iommu_sva_unbind_device'; did you mean 'bus_find_device'? [-Werror=implicit-function-declaration] iommu_sva_unbind_device(handle); ^~~ bus_find_device drivers/misc/uacce/uacce.c: At top level: drivers/misc/uacce/uacce.c:274:21: error: variable 'uacce_vm_ops' has initializer but incomplete type static const struct vm_operations_struct uacce_vm_ops = { ^~~~ drivers/misc/uacce/uacce.c:275:3: error: 'const struct vm_operations_struct' has no member named 'close' .close = uacce_vma_close, ^ drivers/misc/uacce/uacce.c:275:11: warning: excess elements in struct initializer .close = uacce_vma_close, ^~~ drivers/misc/uacce/uacce.c:275:11: note: (near initialization for 'uacce_vm_ops') drivers/misc/uacce/uacce.c: In function 'uacce_fops_mmap': drivers/misc/uacce/uacce.c:295:19: error: 'VM_DONTCOPY' undeclared (first use in this function) vma->vm_flags |= VM_DONTCOPY | VM_DONTEXPAND | VM_WIPEONFORK; ^~~ drivers/misc/uacce/uacce.c:295:33: error: 'VM_DONTEXPAND' undeclared (first use in this function); did you mean 'VM_DONTCOPY'? vma->vm_flags |= VM_DONTCOPY | VM_DONTEXPAND | VM_WIPEONFORK;
[PATCH v2] arm64: dts: Add m4u and smi-larbs nodes for mt8183
Add nodes for M4U, smi-common, and smi-larbs. Signed-off-by: Yong Wu --- change notes: v2: Rebase on v5.5-rc1 and power_domain nodes[1]. [1] https://lore.kernel.org/patchwork/patch/1164746/ v1: https://lore.kernel.org/patchwork/patch/1054099/ --- arch/arm64/boot/dts/mediatek/mt8183.dtsi | 85 1 file changed, 85 insertions(+) diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi index 91217e4f..0f8f78e 100644 --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi @@ -8,6 +8,7 @@ #include #include #include +#include #include #include "mt8183-pinfunc.h" @@ -335,6 +336,15 @@ clock-names = "clk13m"; }; + iommu: iommu@10205000 { + compatible = "mediatek,mt8183-m4u"; + reg = <0 0x10205000 0 0x1000>; + interrupts = ; + mediatek,larbs = <&larb0 &larb1 &larb2 &larb3 + &larb4 &larb5 &larb6>; + #iommu-cells = <1>; + }; + auxadc: auxadc@11001000 { compatible = "mediatek,mt8183-auxadc", "mediatek,mt8173-auxadc"; @@ -651,9 +661,25 @@ #clock-cells = <1>; }; + larb0: larb@14017000 { + compatible = "mediatek,mt8183-smi-larb"; + reg = <0 0x14017000 0 0x1000>; + mediatek,smi = <&smi_common>; + clocks = <&mmsys CLK_MM_SMI_LARB0>, +<&mmsys CLK_MM_SMI_LARB0>; + power-domains = <&scpsys MT8183_POWER_DOMAIN_DISP>; + clock-names = "apb", "smi"; + }; + smi_common: smi@14019000 { compatible = "mediatek,mt8183-smi-common", "syscon"; reg = <0 0x14019000 0 0x1000>; + clocks = <&mmsys CLK_MM_SMI_COMMON>, +<&mmsys CLK_MM_SMI_COMMON>, +<&mmsys CLK_MM_GALS_COMM0>, +<&mmsys CLK_MM_GALS_COMM1>; + clock-names = "apb", "smi", "gals0", "gals1"; + power-domains = <&scpsys MT8183_POWER_DOMAIN_DISP>; }; imgsys: syscon@1502 { @@ -662,18 +688,57 @@ #clock-cells = <1>; }; + larb5: larb@15021000 { + compatible = "mediatek,mt8183-smi-larb"; + reg = <0 0x15021000 0 0x1000>; + mediatek,smi = <&smi_common>; + clocks = <&imgsys CLK_IMG_LARB5>, <&imgsys CLK_IMG_LARB5>, +<&mmsys CLK_MM_GALS_IMG2MM>; + clock-names = "apb", "smi", "gals"; + power-domains = <&scpsys MT8183_POWER_DOMAIN_ISP>; + }; + + larb2: larb@1502f000 { + compatible = "mediatek,mt8183-smi-larb"; + reg = <0 0x1502f000 0 0x1000>; + mediatek,smi = <&smi_common>; + clocks = <&imgsys CLK_IMG_LARB2>, <&imgsys CLK_IMG_LARB2>, +<&mmsys CLK_MM_GALS_IPU2MM>; + clock-names = "apb", "smi", "gals"; + power-domains = <&scpsys MT8183_POWER_DOMAIN_ISP>; + }; + vdecsys: syscon@1600 { compatible = "mediatek,mt8183-vdecsys", "syscon"; reg = <0 0x1600 0 0x1000>; #clock-cells = <1>; }; + larb1: larb@1601 { + compatible = "mediatek,mt8183-smi-larb"; + reg = <0 0x1601 0 0x1000>; + mediatek,smi = <&smi_common>; + clocks = <&vdecsys CLK_VDEC_VDEC>, <&vdecsys CLK_VDEC_LARB1>; + clock-names = "apb", "smi"; + power-domains = <&scpsys MT8183_POWER_DOMAIN_VDEC>; + }; + vencsys: syscon@1700 { compatible = "mediatek,mt8183-vencsys", "syscon"; reg = <0 0x1700 0 0x1000>; #clock-cells = <1>; }; + larb4: larb@1701 { + compatible = "mediatek,mt8183-smi-larb"; + reg = <0 0x1701 0 0x1000>; + mediatek,smi = <&smi_common>; + clocks = <&vencsys CLK_VENC_LARB>, +<&vencsys CLK_VENC_LARB>; + clock-names = "apb", "smi"; + power-dom
[PATCH v3 0/6] Use 1st-level for IOVA translation
Intel VT-d in scalable mode supports two types of page tables for DMA translation: the first level page table and the second level page table. The first level page table uses the same format as the CPU page table, while the second level page table keeps compatible with previous formats. The software is able to choose any one of them for DMA remapping according to the use case. This patchset aims to move IOVA (I/O Virtual Address) translation to 1st-level page table in scalable mode. This will simplify vIOMMU (IOMMU simulated by VM hypervisor) design by using the two-stage translation, a.k.a. nested mode translation. As Intel VT-d architecture offers caching mode, guest IOVA (GIOVA) support is currently implemented in a shadow page manner. The device simulation software, like QEMU, has to figure out GIOVA->GPA mappings and write them to a shadowed page table, which will be used by the physical IOMMU. Each time when mappings are created or destroyed in vIOMMU, the simulation software has to intervene. Hence, the changes on GIOVA->GPA could be shadowed to host. .---. | vIOMMU | |---| .. | |IOTLB flush trap |QEMU| .---. (map/unmap) || |GIOVA->GPA |>|.. | '---' || GIOVA->HPA | | | | |'' | '---' || || '' | < | v VFIO/IOMMU API .---. | pIOMMU | |---| | | .---. |GIOVA->HPA | '---' | | '---' In VT-d 3.0, scalable mode is introduced, which offers two-level translation page tables and nested translation mode. Regards to GIOVA support, it can be simplified by 1) moving the GIOVA support over 1st-level page table to store GIOVA->GPA mapping in vIOMMU, 2) binding vIOMMU 1st level page table to the pIOMMU, 3) using pIOMMU second level for GPA->HPA translation, and 4) enable nested (a.k.a. dual-stage) translation in host. Compared with current shadow GIOVA support, the new approach makes the vIOMMU design simpler and more efficient as we only need to flush the pIOMMU IOTLB and possible device-IOTLB when an IOVA mapping in vIOMMU is torn down. .---. | vIOMMU | |---| .---. | |IOTLB flush trap | QEMU| .---.(unmap) |---| |GIOVA->GPA |>| | '---' '---' | | | '---' | <-- | VFIO/IOMMU | cache invalidation and | guest gpd bind interfaces v .---. | pIOMMU | |---| .---. |GIOVA->GPA |<---First level '---' | GPA->HPA |<---Scond level '---' '---' This patch applies the first level page table for IOVA translation unless the DOMAIN_ATTR_NESTING domain attribution has been set. Setting of this attribution means the second level will be used to map gPA (guest physical address) to hPA (host physical address), and the mappings between gVA (guest virtual address) and gPA will be maintained by the guest with the page table address binding to host's first level. Based-on-idea-by: Ashok Raj Based-on-idea-by: Kevin Tian Based-on-idea-by: Liu Yi L Based-on-idea-by: Jacob Pan Based-on-idea-by: Sanjay Kumar Based-on-idea-by: Lu Baolu Change log: v2->v3: - The previous version was posted here https://lkml.org/lkml/2019/11/27/1831 - Accept Jacob's suggestion on merging two page tables. v1->v2 - The first series was posted here https://lkml.org/lkml/2019/9/23/297 - Use per domain page table ops to handle different page tables. - Use first level for DMA remapping by default on both bare metal and vm guest. - Code refine according to code review comments for v1. Lu Baolu (6): iommu/vt-d: Identify domains using first level page table iommu/vt-d: Add set domain DOMAIN_ATTR_NESTING attr iommu/vt-d: Add PASID_FLAG_FL5LP for first-level pasid setup iommu/vt-d: Setup pasid entries for iova over first level iommu/vt-d: Flush PASID-based iotlb for iova over first level iommu/vt-d: Use iova over first level drivers/iommu/dmar.c| 41 drivers/iommu/intel-iommu.c | 185 drivers/iommu/intel-pasid.c | 7 +- drivers/iommu/intel-pasid.h | 6 ++ drivers/iommu/intel-svm.c | 8 +- inc
[PATCH v3 5/6] iommu/vt-d: Flush PASID-based iotlb for iova over first level
When software has changed first-level tables, it should invalidate the affected IOTLB and the paging-structure-caches using the PASID- based-IOTLB Invalidate Descriptor defined in spec 6.5.2.4. Signed-off-by: Lu Baolu --- drivers/iommu/dmar.c| 41 ++ drivers/iommu/intel-iommu.c | 44 - include/linux/intel-iommu.h | 2 ++ 3 files changed, 72 insertions(+), 15 deletions(-) diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c index 3acfa6a25fa2..fb30d5053664 100644 --- a/drivers/iommu/dmar.c +++ b/drivers/iommu/dmar.c @@ -1371,6 +1371,47 @@ void qi_flush_dev_iotlb(struct intel_iommu *iommu, u16 sid, u16 pfsid, qi_submit_sync(&desc, iommu); } +/* PASID-based IOTLB invalidation */ +void qi_flush_piotlb(struct intel_iommu *iommu, u16 did, u32 pasid, u64 addr, +unsigned long npages, bool ih) +{ + struct qi_desc desc = {.qw2 = 0, .qw3 = 0}; + + /* +* npages == -1 means a PASID-selective invalidation, otherwise, +* a positive value for Page-selective-within-PASID invalidation. +* 0 is not a valid input. +*/ + if (WARN_ON(!npages)) { + pr_err("Invalid input npages = %ld\n", npages); + return; + } + + if (npages == -1) { + desc.qw0 = QI_EIOTLB_PASID(pasid) | + QI_EIOTLB_DID(did) | + QI_EIOTLB_GRAN(QI_GRAN_NONG_PASID) | + QI_EIOTLB_TYPE; + desc.qw1 = 0; + } else { + int mask = ilog2(__roundup_pow_of_two(npages)); + unsigned long align = (1ULL << (VTD_PAGE_SHIFT + mask)); + + if (WARN_ON_ONCE(!ALIGN(addr, align))) + addr &= ~(align - 1); + + desc.qw0 = QI_EIOTLB_PASID(pasid) | + QI_EIOTLB_DID(did) | + QI_EIOTLB_GRAN(QI_GRAN_PSI_PASID) | + QI_EIOTLB_TYPE; + desc.qw1 = QI_EIOTLB_ADDR(addr) | + QI_EIOTLB_IH(ih) | + QI_EIOTLB_AM(mask); + } + + qi_submit_sync(&desc, iommu); +} + /* * Disable Queued Invalidation interface. */ diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c index 83a7abf0c4f0..e47f5fe37b59 100644 --- a/drivers/iommu/intel-iommu.c +++ b/drivers/iommu/intel-iommu.c @@ -1520,18 +1520,24 @@ static void iommu_flush_iotlb_psi(struct intel_iommu *iommu, if (ih) ih = 1 << 6; - /* -* Fallback to domain selective flush if no PSI support or the size is -* too big. -* PSI requires page size to be 2 ^ x, and the base address is naturally -* aligned to the size -*/ - if (!cap_pgsel_inv(iommu->cap) || mask > cap_max_amask_val(iommu->cap)) - iommu->flush.flush_iotlb(iommu, did, 0, 0, - DMA_TLB_DSI_FLUSH); - else - iommu->flush.flush_iotlb(iommu, did, addr | ih, mask, - DMA_TLB_PSI_FLUSH); + + if (domain_use_first_level(domain)) { + qi_flush_piotlb(iommu, did, domain->default_pasid, + addr, pages, ih); + } else { + /* +* Fallback to domain selective flush if no PSI support or +* the size is too big. PSI requires page size to be 2 ^ x, +* and the base address is naturally aligned to the size. +*/ + if (!cap_pgsel_inv(iommu->cap) || + mask > cap_max_amask_val(iommu->cap)) + iommu->flush.flush_iotlb(iommu, did, 0, 0, + DMA_TLB_DSI_FLUSH); + else + iommu->flush.flush_iotlb(iommu, did, addr | ih, mask, + DMA_TLB_PSI_FLUSH); + } /* * In caching mode, changes of pages from non-present to present require @@ -1546,8 +1552,11 @@ static inline void __mapping_notify_one(struct intel_iommu *iommu, struct dmar_domain *domain, unsigned long pfn, unsigned int pages) { - /* It's a non-present to present mapping. Only flush if caching mode */ - if (cap_caching_mode(iommu->cap)) + /* +* It's a non-present to present mapping. Only flush if caching mode +* and second level. +*/ + if (cap_caching_mode(iommu->cap) && !domain_use_first_level(domain)) iommu_flush_iotlb_psi(iommu, domain, pfn, pages, 0, 1); else iommu_flush_write_buffer(iommu); @@ -1564,7 +1573,12 @@ static void iommu_flush_iova(struct iova_domain *iovad)
[PATCH v3 2/6] iommu/vt-d: Add set domain DOMAIN_ATTR_NESTING attr
This adds the Intel VT-d specific callback of setting DOMAIN_ATTR_NESTING domain attribution. It is necessary to let the VT-d driver know that the domain represents a virtual machine which requires the IOMMU hardware to support nested translation mode. Return success if the IOMMU hardware suports nested mode, otherwise failure. Signed-off-by: Yi Sun Signed-off-by: Lu Baolu --- drivers/iommu/intel-iommu.c | 56 + 1 file changed, 56 insertions(+) diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c index c93fe716e6b0..2b5a47584baf 100644 --- a/drivers/iommu/intel-iommu.c +++ b/drivers/iommu/intel-iommu.c @@ -315,6 +315,12 @@ static int hw_pass_through = 1; */ #define DOMAIN_FLAG_USE_FIRST_LEVELBIT(2) +/* + * Domain represents a virtual machine which demands iommu nested + * translation mode support. + */ +#define DOMAIN_FLAG_NESTING_MODE BIT(3) + #define for_each_domain_iommu(idx, domain) \ for (idx = 0; idx < g_num_of_iommus; idx++) \ if (domain->iommu_refcnt[idx]) @@ -5635,6 +5641,24 @@ static inline bool iommu_pasid_support(void) return ret; } +static inline bool nested_mode_support(void) +{ + struct dmar_drhd_unit *drhd; + struct intel_iommu *iommu; + bool ret = true; + + rcu_read_lock(); + for_each_active_iommu(iommu, drhd) { + if (!sm_supported(iommu) || !ecap_nest(iommu->ecap)) { + ret = false; + break; + } + } + rcu_read_unlock(); + + return ret; +} + static bool intel_iommu_capable(enum iommu_cap cap) { if (cap == IOMMU_CAP_CACHE_COHERENCY) @@ -6013,10 +6037,42 @@ static bool intel_iommu_is_attach_deferred(struct iommu_domain *domain, return dev->archdata.iommu == DEFER_DEVICE_DOMAIN_INFO; } +static int +intel_iommu_domain_set_attr(struct iommu_domain *domain, + enum iommu_attr attr, void *data) +{ + struct dmar_domain *dmar_domain = to_dmar_domain(domain); + unsigned long flags; + int ret = 0; + + if (domain->type != IOMMU_DOMAIN_UNMANAGED) + return -EINVAL; + + switch (attr) { + case DOMAIN_ATTR_NESTING: + spin_lock_irqsave(&device_domain_lock, flags); + if (nested_mode_support() && + list_empty(&dmar_domain->devices)) { + dmar_domain->flags |= DOMAIN_FLAG_NESTING_MODE; + dmar_domain->flags &= ~DOMAIN_FLAG_USE_FIRST_LEVEL; + } else { + ret = -ENODEV; + } + spin_unlock_irqrestore(&device_domain_lock, flags); + break; + default: + ret = -EINVAL; + break; + } + + return ret; +} + const struct iommu_ops intel_iommu_ops = { .capable= intel_iommu_capable, .domain_alloc = intel_iommu_domain_alloc, .domain_free= intel_iommu_domain_free, + .domain_set_attr= intel_iommu_domain_set_attr, .attach_dev = intel_iommu_attach_device, .detach_dev = intel_iommu_detach_device, .aux_attach_dev = intel_iommu_aux_attach_device, -- 2.17.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v3 6/6] iommu/vt-d: Use iova over first level
After we make all map/unmap paths support first level page table. Let's turn it on if hardware supports scalable mode. Signed-off-by: Lu Baolu --- drivers/iommu/intel-iommu.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c index e47f5fe37b59..9228f121a040 100644 --- a/drivers/iommu/intel-iommu.c +++ b/drivers/iommu/intel-iommu.c @@ -1749,15 +1749,13 @@ static void free_dmar_iommu(struct intel_iommu *iommu) /* * Check and return whether first level is used by default for - * DMA translation. Currently, we make it off by setting - * first_level_support = 0, and will change it to -1 after all - * map/unmap paths support first level page table. + * DMA translation. */ static bool first_level_by_default(void) { struct dmar_drhd_unit *drhd; struct intel_iommu *iommu; - static int first_level_support = 0; + static int first_level_support = -1; if (likely(first_level_support != -1)) return first_level_support; -- 2.17.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v3 1/6] iommu/vt-d: Identify domains using first level page table
This checks whether a domain should use the first level page table for map/unmap and marks it in the domain structure. Signed-off-by: Lu Baolu --- drivers/iommu/intel-iommu.c | 39 + 1 file changed, 39 insertions(+) diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c index 82b9687e82d3..c93fe716e6b0 100644 --- a/drivers/iommu/intel-iommu.c +++ b/drivers/iommu/intel-iommu.c @@ -307,6 +307,14 @@ static int hw_pass_through = 1; */ #define DOMAIN_FLAG_LOSE_CHILDREN BIT(1) +/* + * When VT-d works in the scalable mode, it allows DMA translation to + * happen through either first level or second level page table. This + * bit marks that the DMA translation for the domain goes through the + * first level page table, otherwise, it goes through the second level. + */ +#define DOMAIN_FLAG_USE_FIRST_LEVELBIT(2) + #define for_each_domain_iommu(idx, domain) \ for (idx = 0; idx < g_num_of_iommus; idx++) \ if (domain->iommu_refcnt[idx]) @@ -1714,6 +1722,35 @@ static void free_dmar_iommu(struct intel_iommu *iommu) #endif } +/* + * Check and return whether first level is used by default for + * DMA translation. Currently, we make it off by setting + * first_level_support = 0, and will change it to -1 after all + * map/unmap paths support first level page table. + */ +static bool first_level_by_default(void) +{ + struct dmar_drhd_unit *drhd; + struct intel_iommu *iommu; + static int first_level_support = 0; + + if (likely(first_level_support != -1)) + return first_level_support; + + first_level_support = 1; + + rcu_read_lock(); + for_each_active_iommu(iommu, drhd) { + if (!sm_supported(iommu) || !ecap_flts(iommu->ecap)) { + first_level_support = 0; + break; + } + } + rcu_read_unlock(); + + return first_level_support; +} + static struct dmar_domain *alloc_domain(int flags) { struct dmar_domain *domain; @@ -1725,6 +1762,8 @@ static struct dmar_domain *alloc_domain(int flags) memset(domain, 0, sizeof(*domain)); domain->nid = NUMA_NO_NODE; domain->flags = flags; + if (first_level_by_default()) + domain->flags |= DOMAIN_FLAG_USE_FIRST_LEVEL; domain->has_iotlb_device = false; INIT_LIST_HEAD(&domain->devices); -- 2.17.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v3 4/6] iommu/vt-d: Setup pasid entries for iova over first level
Intel VT-d in scalable mode supports two types of page tables for IOVA translation: first level and second level. The IOMMU driver can choose one from both for IOVA translation according to the use case. This sets up the pasid entry if a domain is selected to use the first-level page table for iova translation. Signed-off-by: Lu Baolu --- drivers/iommu/intel-iommu.c | 48 +++-- include/linux/intel-iommu.h | 10 2 files changed, 52 insertions(+), 6 deletions(-) diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c index 2b5a47584baf..83a7abf0c4f0 100644 --- a/drivers/iommu/intel-iommu.c +++ b/drivers/iommu/intel-iommu.c @@ -571,6 +571,11 @@ static inline int domain_type_is_si(struct dmar_domain *domain) return domain->flags & DOMAIN_FLAG_STATIC_IDENTITY; } +static inline bool domain_use_first_level(struct dmar_domain *domain) +{ + return domain->flags & DOMAIN_FLAG_USE_FIRST_LEVEL; +} + static inline int domain_pfn_supported(struct dmar_domain *domain, unsigned long pfn) { @@ -2288,6 +2293,8 @@ static int __domain_mapping(struct dmar_domain *domain, unsigned long iov_pfn, return -EINVAL; prot &= DMA_PTE_READ | DMA_PTE_WRITE | DMA_PTE_SNP; + if (domain_use_first_level(domain)) + prot |= DMA_FL_PTE_PRESENT; if (!sg) { sg_res = nr_pages; @@ -2515,6 +2522,36 @@ dmar_search_domain_by_dev_info(int segment, int bus, int devfn) return NULL; } +static int domain_setup_first_level(struct intel_iommu *iommu, + struct dmar_domain *domain, + struct device *dev, + int pasid) +{ + int flags = PASID_FLAG_SUPERVISOR_MODE; + struct dma_pte *pgd = domain->pgd; + int agaw, level; + + /* +* Skip top levels of page tables for iommu which has +* less agaw than default. Unnecessary for PT mode. +*/ + for (agaw = domain->agaw; agaw > iommu->agaw; agaw--) { + pgd = phys_to_virt(dma_pte_addr(pgd)); + if (!dma_pte_present(pgd)) + return -ENOMEM; + } + + level = agaw_to_level(agaw); + if (level != 4 && level != 5) + return -EINVAL; + + flags |= (level == 5) ? PASID_FLAG_FL5LP : 0; + + return intel_pasid_setup_first_level(iommu, dev, (pgd_t *)pgd, pasid, +domain->iommu_did[iommu->seq_id], +flags); +} + static struct dmar_domain *dmar_insert_one_dev_info(struct intel_iommu *iommu, int bus, int devfn, struct device *dev, @@ -2614,6 +2651,9 @@ static struct dmar_domain *dmar_insert_one_dev_info(struct intel_iommu *iommu, if (hw_pass_through && domain_type_is_si(domain)) ret = intel_pasid_setup_pass_through(iommu, domain, dev, PASID_RID2PASID); + else if (domain_use_first_level(domain)) + ret = domain_setup_first_level(iommu, domain, dev, + PASID_RID2PASID); else ret = intel_pasid_setup_second_level(iommu, domain, dev, PASID_RID2PASID); @@ -5369,8 +5409,12 @@ static int aux_domain_add_dev(struct dmar_domain *domain, goto attach_failed; /* Setup the PASID entry for mediated devices: */ - ret = intel_pasid_setup_second_level(iommu, domain, dev, -domain->default_pasid); + if (domain_use_first_level(domain)) + ret = domain_setup_first_level(iommu, domain, dev, + domain->default_pasid); + else + ret = intel_pasid_setup_second_level(iommu, domain, dev, +domain->default_pasid); if (ret) goto table_failed; spin_unlock(&iommu->lock); diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h index aaece25c055f..66b525bad434 100644 --- a/include/linux/intel-iommu.h +++ b/include/linux/intel-iommu.h @@ -34,10 +34,12 @@ #define VTD_STRIDE_SHIFT(9) #define VTD_STRIDE_MASK (((u64)-1) << VTD_STRIDE_SHIFT) -#define DMA_PTE_READ (1) -#define DMA_PTE_WRITE (2) -#define DMA_PTE_LARGE_PAGE (1 << 7) -#define DMA_PTE_SNP (1 << 11) +#define DMA_PTE_READ (1) +#define DMA_PTE_WRITE (2) +#define DMA_PTE_LARGE_PAGE (1 << 7) +#define DMA_PTE_SNP(1 << 11) + +#define DMA_FL_PTE_PRESENT (1) #define CONTEXT_TT_MULTI_LEVEL 0 #define CONTEXT_TT_DEV_IOTLB 1 -- 2.17.1 ___
[PATCH v3 3/6] iommu/vt-d: Add PASID_FLAG_FL5LP for first-level pasid setup
Current intel_pasid_setup_first_level() use 5-level paging for first level translation if CPUs use 5-level paging mode too. This makes sense for SVA usages since the page table is shared between CPUs and IOMMUs. But it makes no sense if we only want to use first level for IOVA translation. Add PASID_FLAG_FL5LP bit in the flags which indicates whether the 5-level paging mode should be used. Signed-off-by: Lu Baolu --- drivers/iommu/intel-pasid.c | 7 ++- drivers/iommu/intel-pasid.h | 6 ++ drivers/iommu/intel-svm.c | 8 ++-- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/drivers/iommu/intel-pasid.c b/drivers/iommu/intel-pasid.c index 3cb569e76642..22b30f10b396 100644 --- a/drivers/iommu/intel-pasid.c +++ b/drivers/iommu/intel-pasid.c @@ -477,18 +477,15 @@ int intel_pasid_setup_first_level(struct intel_iommu *iommu, pasid_set_sre(pte); } -#ifdef CONFIG_X86 - /* Both CPU and IOMMU paging mode need to match */ - if (cpu_feature_enabled(X86_FEATURE_LA57)) { + if (flags & PASID_FLAG_FL5LP) { if (cap_5lp_support(iommu->cap)) { pasid_set_flpm(pte, 1); } else { - pr_err("VT-d has no 5-level paging support for CPU\n"); + pr_err("No 5-level paging support for first-level\n"); pasid_clear_entry(pte); return -EINVAL; } } -#endif /* CONFIG_X86 */ pasid_set_domain_id(pte, did); pasid_set_address_width(pte, iommu->agaw); diff --git a/drivers/iommu/intel-pasid.h b/drivers/iommu/intel-pasid.h index fc8cd8f17de1..92de6df24ccb 100644 --- a/drivers/iommu/intel-pasid.h +++ b/drivers/iommu/intel-pasid.h @@ -37,6 +37,12 @@ */ #define PASID_FLAG_SUPERVISOR_MODE BIT(0) +/* + * The PASID_FLAG_FL5LP flag Indicates using 5-level paging for first- + * level translation, otherwise, 4-level paging will be used. + */ +#define PASID_FLAG_FL5LP BIT(1) + struct pasid_dir_entry { u64 val; }; diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c index 04023033b79f..d7f2a5358900 100644 --- a/drivers/iommu/intel-svm.c +++ b/drivers/iommu/intel-svm.c @@ -364,7 +364,9 @@ int intel_svm_bind_mm(struct device *dev, int *pasid, int flags, struct svm_dev_ ret = intel_pasid_setup_first_level(iommu, dev, mm ? mm->pgd : init_mm.pgd, svm->pasid, FLPT_DEFAULT_DID, - mm ? 0 : PASID_FLAG_SUPERVISOR_MODE); + (mm ? 0 : PASID_FLAG_SUPERVISOR_MODE) | + (cpu_feature_enabled(X86_FEATURE_LA57) ? +PASID_FLAG_FL5LP : 0)); spin_unlock(&iommu->lock); if (ret) { if (mm) @@ -385,7 +387,9 @@ int intel_svm_bind_mm(struct device *dev, int *pasid, int flags, struct svm_dev_ ret = intel_pasid_setup_first_level(iommu, dev, mm ? mm->pgd : init_mm.pgd, svm->pasid, FLPT_DEFAULT_DID, - mm ? 0 : PASID_FLAG_SUPERVISOR_MODE); + (mm ? 0 : PASID_FLAG_SUPERVISOR_MODE) | + (cpu_feature_enabled(X86_FEATURE_LA57) ? + PASID_FLAG_FL5LP : 0)); spin_unlock(&iommu->lock); if (ret) { kfree(sdev); -- 2.17.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 5/8] iommu/vt-d: Add first level page table interfaces
Hi Jacob, On 12/3/19 7:27 AM, Jacob Pan wrote: On Thu, 28 Nov 2019 10:25:47 +0800 Lu Baolu wrote: This adds functions to manipulate first level page tables which could be used by a scalale mode capable IOMMU unit. FL and SL page tables are very similar, and I presume we are not using all the flag bits in FL paging structures for DMA mapping. Are there enough relevant differences to warrant a new set of helper functions for FL? Or we can merge into one. I evaluated your suggestion these days. It turned out that your suggestion make code simpler and easier for maintainence. Thank you for the comment and I will send out a new version for review soon. Best regards, baolu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 1/1] iommu/vt-d: trace: Extend map_sg trace event
Current map_sg stores trace message in a coarse manner. This extends it so that more detailed messages could be traced. The map_sg trace message looks like: map_sg: dev=:00:17.0 [1/9] dev_addr=0xf8f9 phys_addr=0x158051000 size=4096 map_sg: dev=:00:17.0 [2/9] dev_addr=0xf8f91000 phys_addr=0x15a858000 size=4096 map_sg: dev=:00:17.0 [3/9] dev_addr=0xf8f92000 phys_addr=0x15aa13000 size=4096 map_sg: dev=:00:17.0 [4/9] dev_addr=0xf8f93000 phys_addr=0x1570f1000 size=8192 map_sg: dev=:00:17.0 [5/9] dev_addr=0xf8f95000 phys_addr=0x15c6d size=4096 map_sg: dev=:00:17.0 [6/9] dev_addr=0xf8f96000 phys_addr=0x157194000 size=4096 map_sg: dev=:00:17.0 [7/9] dev_addr=0xf8f97000 phys_addr=0x169552000 size=4096 map_sg: dev=:00:17.0 [8/9] dev_addr=0xf8f98000 phys_addr=0x169dde000 size=4096 map_sg: dev=:00:17.0 [9/9] dev_addr=0xf8f99000 phys_addr=0x148351000 size=4096 Signed-off-by: Lu Baolu --- drivers/iommu/intel-iommu.c| 7 +++-- include/trace/events/intel_iommu.h | 48 ++ 2 files changed, 47 insertions(+), 8 deletions(-) diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c index 72c10e082257..0726705d2d89 100644 --- a/drivers/iommu/intel-iommu.c +++ b/drivers/iommu/intel-iommu.c @@ -3776,8 +3776,8 @@ static int intel_map_sg(struct device *dev, struct scatterlist *sglist, int nele return 0; } - trace_map_sg(dev, iova_pfn << PAGE_SHIFT, -sg_phys(sglist), size << VTD_PAGE_SHIFT); + for_each_sg(sglist, sg, nelems, i) + trace_map_sg(dev, i + 1, nelems, sg); return nelems; } @@ -3989,6 +3989,9 @@ bounce_map_sg(struct device *dev, struct scatterlist *sglist, int nelems, sg_dma_len(sg) = sg->length; } + for_each_sg(sglist, sg, nelems, i) + trace_bounce_map_sg(dev, i + 1, nelems, sg); + return nelems; out_unmap: diff --git a/include/trace/events/intel_iommu.h b/include/trace/events/intel_iommu.h index 54e61d456cdf..112bd06487bf 100644 --- a/include/trace/events/intel_iommu.h +++ b/include/trace/events/intel_iommu.h @@ -49,12 +49,6 @@ DEFINE_EVENT(dma_map, map_single, TP_ARGS(dev, dev_addr, phys_addr, size) ); -DEFINE_EVENT(dma_map, map_sg, - TP_PROTO(struct device *dev, dma_addr_t dev_addr, phys_addr_t phys_addr, -size_t size), - TP_ARGS(dev, dev_addr, phys_addr, size) -); - DEFINE_EVENT(dma_map, bounce_map_single, TP_PROTO(struct device *dev, dma_addr_t dev_addr, phys_addr_t phys_addr, size_t size), @@ -99,6 +93,48 @@ DEFINE_EVENT(dma_unmap, bounce_unmap_single, TP_ARGS(dev, dev_addr, size) ); +DECLARE_EVENT_CLASS(dma_map_sg, + TP_PROTO(struct device *dev, int index, int total, +struct scatterlist *sg), + + TP_ARGS(dev, index, total, sg), + + TP_STRUCT__entry( + __string(dev_name, dev_name(dev)) + __field(dma_addr_t, dev_addr) + __field(phys_addr_t, phys_addr) + __field(size_t, size) + __field(int, index) + __field(int, total) + ), + + TP_fast_assign( + __assign_str(dev_name, dev_name(dev)); + __entry->dev_addr = sg->dma_address; + __entry->phys_addr = sg_phys(sg); + __entry->size = sg->dma_length; + __entry->index = index; + __entry->total = total; + ), + + TP_printk("dev=%s [%d/%d] dev_addr=0x%llx phys_addr=0x%llx size=%zu", + __get_str(dev_name), __entry->index, __entry->total, + (unsigned long long)__entry->dev_addr, + (unsigned long long)__entry->phys_addr, + __entry->size) +); + +DEFINE_EVENT(dma_map_sg, map_sg, + TP_PROTO(struct device *dev, int index, int total, +struct scatterlist *sg), + TP_ARGS(dev, index, total, sg) +); + +DEFINE_EVENT(dma_map_sg, bounce_map_sg, + TP_PROTO(struct device *dev, int index, int total, +struct scatterlist *sg), + TP_ARGS(dev, index, total, sg) +); #endif /* _TRACE_INTEL_IOMMU_H */ /* This part must be outside protection */ -- 2.17.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 1/1] iommu/vt-d: Fix dmar pte read access not set error
If the default DMA domain of a group doesn't fit a device, it will still sit in the group but use a private identity domain. When map/unmap/iova_to_phys come through iommu API, the driver should still serve them, otherwise, other devices in the same group will be impacted. Since identity domain has been mapped with the whole available memory space and RMRRs, we don't need to worry about the impact on it. Link: https://www.spinics.net/lists/iommu/msg40416.html Cc: Jerry Snitselaar Reported-by: Jerry Snitselaar Fixes: 942067f1b6b97 ("iommu/vt-d: Identify default domains replaced with private") Cc: sta...@vger.kernel.org # v5.3+ Signed-off-by: Lu Baolu --- drivers/iommu/intel-iommu.c | 8 1 file changed, 8 deletions(-) diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c index 0c8d81f56a30..b73bebea9148 100644 --- a/drivers/iommu/intel-iommu.c +++ b/drivers/iommu/intel-iommu.c @@ -5478,9 +5478,6 @@ static int intel_iommu_map(struct iommu_domain *domain, int prot = 0; int ret; - if (dmar_domain->flags & DOMAIN_FLAG_LOSE_CHILDREN) - return -EINVAL; - if (iommu_prot & IOMMU_READ) prot |= DMA_PTE_READ; if (iommu_prot & IOMMU_WRITE) @@ -5523,8 +5520,6 @@ static size_t intel_iommu_unmap(struct iommu_domain *domain, /* Cope with horrid API which requires us to unmap more than the size argument if it happens to be a large-page mapping. */ BUG_ON(!pfn_to_dma_pte(dmar_domain, iova >> VTD_PAGE_SHIFT, &level)); - if (dmar_domain->flags & DOMAIN_FLAG_LOSE_CHILDREN) - return 0; if (size < VTD_PAGE_SIZE << level_to_offset_bits(level)) size = VTD_PAGE_SIZE << level_to_offset_bits(level); @@ -5556,9 +5551,6 @@ static phys_addr_t intel_iommu_iova_to_phys(struct iommu_domain *domain, int level = 0; u64 phys = 0; - if (dmar_domain->flags & DOMAIN_FLAG_LOSE_CHILDREN) - return 0; - pte = pfn_to_dma_pte(dmar_domain, iova >> VTD_PAGE_SHIFT, &level); if (pte) phys = dma_pte_addr(pte); -- 2.17.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu: set group default domain before creating direct mappings
Hi, On 12/11/19 2:56 AM, Jerry Snitselaar wrote: iommu_group_create_direct_mappings uses group->default_domain, but right after it is called, request_default_domain_for_dev calls iommu_domain_free for the default domain, and sets the group default domain to a different domain. Move the iommu_group_create_direct_mappings call to after the group default domain is set, so the direct mappings get associated with that domain. Cc: Joerg Roedel Cc: Lu Baolu This fix looks good to me. Reviewed-by: Lu Baolu Best regards, baolu Cc: iommu@lists.linux-foundation.org Cc: sta...@vger.kernel.org Fixes: 7423e01741dd ("iommu: Add API to request DMA domain for device") Signed-off-by: Jerry Snitselaar --- drivers/iommu/iommu.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index db7bfd4f2d20..fa908179b80b 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -2282,13 +2282,13 @@ request_default_domain_for_dev(struct device *dev, unsigned long type) goto out; } - iommu_group_create_direct_mappings(group, dev); - /* Make the domain the default for this group */ if (group->default_domain) iommu_domain_free(group->default_domain); group->default_domain = domain; + iommu_group_create_direct_mappings(group, dev); + dev_info(dev, "Using iommu %s mapping\n", type == IOMMU_DOMAIN_DMA ? "dma" : "direct"); ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RESEND PATCH v9 2/4] uacce: add uacce driver
Hi Zhangfei, Thank you for the patch! Yet something to improve: [auto build test ERROR on cryptodev/master] [also build test ERROR on crypto/master char-misc/char-misc-testing v5.5-rc1 next-20191210] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982] url: https://github.com/0day-ci/linux/commits/Zhangfei-Gao/Add-uacce-module-for-Accelerator/20191210-160210 base: https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git master config: sparc64-allmodconfig (attached as .config) compiler: sparc64-linux-gcc (GCC) 7.5.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree GCC_VERSION=7.5.0 make.cross ARCH=sparc64 If you fix the issue, kindly add following tag Reported-by: kbuild test robot All errors (new ones prefixed by >>): drivers/misc/uacce/uacce.c:112:15: error: variable 'uacce_sva_ops' has initializer but incomplete type static struct iommu_sva_ops uacce_sva_ops = { ^ drivers/misc/uacce/uacce.c:113:3: error: 'struct iommu_sva_ops' has no member named 'mm_exit' .mm_exit = uacce_sva_exit, ^~~ drivers/misc/uacce/uacce.c:113:13: warning: excess elements in struct initializer .mm_exit = uacce_sva_exit, ^~ drivers/misc/uacce/uacce.c:113:13: note: (near initialization for 'uacce_sva_ops') drivers/misc/uacce/uacce.c: In function 'uacce_mm_get': drivers/misc/uacce/uacce.c:144:12: error: implicit declaration of function 'iommu_sva_bind_device'; did you mean 'bus_find_device'? [-Werror=implicit-function-declaration] handle = iommu_sva_bind_device(uacce->parent, mm, uacce_mm); ^ bus_find_device drivers/misc/uacce/uacce.c:144:10: warning: assignment makes pointer from integer without a cast [-Wint-conversion] handle = iommu_sva_bind_device(uacce->parent, mm, uacce_mm); ^ drivers/misc/uacce/uacce.c:148:9: error: implicit declaration of function 'iommu_sva_set_ops'; did you mean 'iommu_setup_dma_ops'? [-Werror=implicit-function-declaration] ret = iommu_sva_set_ops(handle, &uacce_sva_ops); ^ iommu_setup_dma_ops drivers/misc/uacce/uacce.c:152:21: error: implicit declaration of function 'iommu_sva_get_pasid' [-Werror=implicit-function-declaration] uacce_mm->pasid = iommu_sva_get_pasid(handle); ^~~ >> drivers/misc/uacce/uacce.c:153:26: error: 'IOMMU_PASID_INVALID' undeclared >> (first use in this function); did you mean 'HV_MSIVALID_INVALID'? if (uacce_mm->pasid == IOMMU_PASID_INVALID) ^~~ HV_MSIVALID_INVALID drivers/misc/uacce/uacce.c:153:26: note: each undeclared identifier is reported only once for each function it appears in drivers/misc/uacce/uacce.c:168:3: error: implicit declaration of function 'iommu_sva_unbind_device'; did you mean 'bus_find_device'? [-Werror=implicit-function-declaration] iommu_sva_unbind_device(handle); ^~~ bus_find_device drivers/misc/uacce/uacce.c: At top level: >> drivers/misc/uacce/uacce.c:274:21: error: variable 'uacce_vm_ops' has >> initializer but incomplete type static const struct vm_operations_struct uacce_vm_ops = { ^~~~ >> drivers/misc/uacce/uacce.c:275:3: error: 'const struct vm_operations_struct' >> has no member named 'close' .close = uacce_vma_close, ^ drivers/misc/uacce/uacce.c:275:11: warning: excess elements in struct initializer .close = uacce_vma_close, ^~~ drivers/misc/uacce/uacce.c:275:11: note: (near initialization for 'uacce_vm_ops') drivers/misc/uacce/uacce.c: In function 'uacce_fops_mmap': >> drivers/misc/uacce/uacce.c:295:19: error: 'VM_DONTCOPY' undeclared (first >> use in this function) vma->vm_flags |= VM_DONTCOPY | VM_DONTEXPAND | VM_WIPEONFORK; ^~~ >> drivers/misc/uacce/uacce.c:295:33: error: 'VM_DONTEXPAND' undeclared (first >> use in this function); did you mean 'VM_DONTCOPY'? vma->vm_flags |= VM_DONTCOPY | VM_DONTEXPAND | VM_WIPEONFORK; ^
Re: [PATCH v6 2/3] PCI: Add parameter nr_devfns to pci_add_dma_alias
[+cc Joerg] On Tue, Dec 03, 2019 at 03:43:53PM +, James Sewart wrote: > pci_add_dma_alias can now be used to create a dma alias for a range of > devfns. > > Reviewed-by: Logan Gunthorpe > Signed-off-by: James Sewart > --- > drivers/pci/pci.c| 22 +- > drivers/pci/quirks.c | 14 +++--- > include/linux/pci.h | 2 +- > 3 files changed, 25 insertions(+), 13 deletions(-) Heads up Joerg: I also updated drivers/iommu/amd_iommu.c (this is the one reported by the kbuild test robot) and removed the printk there that prints the same thing as the one in pci_add_dma_alias(), and I updated a PCI quirk that was merged after this patch was posted. > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index d3c83248f3ce..dbb01aceafda 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -5857,7 +5857,8 @@ int pci_set_vga_state(struct pci_dev *dev, bool decode, > /** > * pci_add_dma_alias - Add a DMA devfn alias for a device > * @dev: the PCI device for which alias is added > - * @devfn: alias slot and function > + * @devfn_from: alias slot and function > + * @nr_devfns: Number of subsequent devfns to alias > * > * This helper encodes an 8-bit devfn as a bit number in dma_alias_mask > * which is used to program permissible bus-devfn source addresses for DMA > @@ -5873,8 +5874,13 @@ int pci_set_vga_state(struct pci_dev *dev, bool decode, > * cannot be left as a userspace activity). DMA aliases should therefore > * be configured via quirks, such as the PCI fixup header quirk. > */ > -void pci_add_dma_alias(struct pci_dev *dev, u8 devfn) > +void pci_add_dma_alias(struct pci_dev *dev, u8 devfn_from, unsigned > nr_devfns) > { > + int devfn_to; > + > + nr_devfns = min(nr_devfns, (unsigned)MAX_NR_DEVFNS); > + devfn_to = devfn_from + nr_devfns - 1; I made this look like: + devfn_to = min(devfn_from + nr_devfns - 1, + (unsigned) MAX_NR_DEVFNS - 1); so devfn_from=0xf0, nr_devfns=0x20 doesn't cause devfn_to to wrap around. I did keep Logan's reviewed-by, so let me know if I broke something. > if (!dev->dma_alias_mask) > dev->dma_alias_mask = bitmap_zalloc(MAX_NR_DEVFNS, GFP_KERNEL); > if (!dev->dma_alias_mask) { > @@ -5882,9 +5888,15 @@ void pci_add_dma_alias(struct pci_dev *dev, u8 devfn) > return; > } > > - set_bit(devfn, dev->dma_alias_mask); > - pci_info(dev, "Enabling fixed DMA alias to %02x.%d\n", > - PCI_SLOT(devfn), PCI_FUNC(devfn)); > + bitmap_set(dev->dma_alias_mask, devfn_from, nr_devfns); > + > + if (nr_devfns == 1) > + pci_info(dev, "Enabling fixed DMA alias to %02x.%d\n", > + PCI_SLOT(devfn_from), PCI_FUNC(devfn_from)); > + else if(nr_devfns > 1) > + pci_info(dev, "Enabling fixed DMA alias for devfn range from > %02x.%d to %02x.%d\n", > + PCI_SLOT(devfn_from), PCI_FUNC(devfn_from), > + PCI_SLOT(devfn_to), PCI_FUNC(devfn_to)); > } ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v6 1/3] PCI: Fix off by one in dma_alias_mask allocation size
[+cc Joerg] On Tue, Dec 03, 2019 at 03:43:22PM +, James Sewart wrote: > The number of possible devfns is 256, add def and correct uses. > > Reviewed-by: Logan Gunthorpe > Signed-off-by: James Sewart I applied these three patches to pci/virtualization for v5.6, thanks! I moved the MAX_NR_DEVFNS from include/linux/pci.h to drivers/pci/pci.h since nobody outside drivers/pci needs it. > --- > drivers/pci/pci.c| 2 +- > drivers/pci/search.c | 2 +- > include/linux/pci.h | 2 ++ > 3 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index a97e2571a527..d3c83248f3ce 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -5876,7 +5876,7 @@ int pci_set_vga_state(struct pci_dev *dev, bool decode, > void pci_add_dma_alias(struct pci_dev *dev, u8 devfn) > { > if (!dev->dma_alias_mask) > - dev->dma_alias_mask = bitmap_zalloc(U8_MAX, GFP_KERNEL); > + dev->dma_alias_mask = bitmap_zalloc(MAX_NR_DEVFNS, GFP_KERNEL); > if (!dev->dma_alias_mask) { > pci_warn(dev, "Unable to allocate DMA alias mask\n"); > return; > diff --git a/drivers/pci/search.c b/drivers/pci/search.c > index bade14002fd8..9e4dfae47252 100644 > --- a/drivers/pci/search.c > +++ b/drivers/pci/search.c > @@ -43,7 +43,7 @@ int pci_for_each_dma_alias(struct pci_dev *pdev, > if (unlikely(pdev->dma_alias_mask)) { > u8 devfn; > > - for_each_set_bit(devfn, pdev->dma_alias_mask, U8_MAX) { > + for_each_set_bit(devfn, pdev->dma_alias_mask, MAX_NR_DEVFNS) { > ret = fn(pdev, PCI_DEVID(pdev->bus->number, devfn), >data); > if (ret) > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 1a6cf19eac2d..6481da29d667 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -57,6 +57,8 @@ > #define PCI_DEVID(bus, devfn)u16)(bus)) << 8) | (devfn)) > /* return bus from PCI devid = ((u16)bus_number) << 8) | devfn */ > #define PCI_BUS_NUM(x) (((x) >> 8) & 0xff) > +/* Number of possible devfns. devfns can be from 0.0 to 1f.7 inclusive */ > +#define MAX_NR_DEVFNS 256 > > /* pci_slot represents a physical slot */ > struct pci_slot { > -- > 2.24.0 > > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: dmar pte read access not set error messages on hp dl388 gen8 systems
On Tue Dec 10 19, Lu Baolu wrote: Hi, On 12/10/19 1:18 PM, Jerry Snitselaar wrote: On Mon Dec 09 19, Jerry Snitselaar wrote: [snip] A call to iommu_map is failing. [ 36.686881] pci :01:00.2: iommu_group_add_device: calling iommu_group_create_direct_mappings [ 36.689843] pci :01:00.2: iommu_group_create_direct_mappings: iterating through mappings [ 36.692757] pci :01:00.2: iommu_group_create_direct_mappings: calling apply_resv_region [ 36.695526] pci :01:00.2: e_direct_mappings: entry type is direct [ 37.198053] iommu: iommu_map: ops->map failed iova 0xbddde000 pa 0xbddde000 pgsize 0x1000 [ 37.201357] pci :01:00.2: iommu_group_create_direct_mappings: iommu_map failed [ 37.203973] pci :01:00.2: iommu_group_create_direct_mappings: leaving func [ 37.206385] pci :01:00.2: iommu_group_add_device: calling __iommu_attach_device [ 37.208950] pci :01:00.2: Adding to iommu group 25 [ 37.210660] pci :01:00.2: DMAR: domain->type is dma It bails at the dmar_domain->flags & DOMAIN_FLAG_LOSE_CHILDREN check at the beginning of intel_iommu_map. I will verify, but it looks like that is getting set when intel_iommu_add_device is called for 01:00.1. request_default_domain_for_dev for 01:00.1 will return -EBUSY because iommu_group_device_count(group) != 1. Okay, I will send you a fix patch later. Thanks! Best regards, baolu One issue I see is: [ 38.869182] uhci_hcd :01:00.4: UHCI Host Controller [ 39.371173] uhci_hcd :01:00.4: new USB bus registered, assigned bus number 3 [ 39.373708] uhci_hcd :01:00.4: detected 8 ports [ 39.375333] uhci_hcd :01:00.4: port count misdetected? forcing to 2 ports [ 39.377820] uhci_hcd :01:00.4: irq 16, io base 0x3c00 [ 39.379921] uhci_hcd :01:00.4: DMAR: 32bit DMA uses non-identity mapping [ 39.382269] uhci_hcd :01:00.4: unable to allocate consistent memory for frame list [ 39.384920] uhci_hcd :01:00.4: startup error -16 [ 39.386619] uhci_hcd :01:00.4: USB bus 3 deregistered [ 39.388640] uhci_hcd :01:00.4: init :01:00.4 fail, -16 [ 39.390616] uhci_hcd: probe of :01:00.4 failed with error -16 I'm not sure if this is related to the flag and what is allowed now by the api. I need to go look at the code to see what it is doing. I'll try debugging it tonight. Regards, Jerry Also fails for 01:00.4: [ 37.212448] pci :01:00.4: iommu_group_add_device: calling iommu_group_create_direct_mappings [ 37.215382] pci :01:00.4: iommu_group_create_direct_mappings: iterating through mappings [ 37.218170] pci :01:00.4: iommu_group_create_direct_mappings: calling apply_resv_region [ 37.220933] pci :01:00.4: iommu_group_create_direct_mappings: entry type is direct-relaxable [ 37.223932] iommu: iommu_map: ops->map failed iova 0xbddde000 pa 0xbddde000 pgsize 0x1000 [ 37.226857] pci :01:00.4: iommu_group_create_direct_mappings: iommu_map failed [ 37.229300] pci :01:00.4: iommu_group_create_direct_mappings: leaving func [ 37.231648] pci :01:00.4: iommu_group_add_device: calling __iommu_attach_device [ 37.234194] pci :01:00.4: Adding to iommu group 25 [ 37.236192] pci :01:00.4: DMAR: domain->type is dma [ 37.237958] pci :01:00.4: DMAR: device default domain type is identity. requesting identity domain [ 37.241061] pci :01:00.4: don't change mappings of existing d37.489870] pci :01:00.4: DMAR: Device uses a private identity domain. There is an RMRR for 0xbddde000-0xefff: [63Ah 1594 2] Subtable Type : 0001 [Reserved Memory Region] [63Ch 1596 2] Length : 0036 [63Eh 1598 2] Reserved : [640h 1600 2] PCI Segment Number : [642h 1602 8] Base Address : BDDDE000 [64Ah 1610 8] End Address (limit) : BDDDEFFF [652h 1618 1] Device Scope Type : 01 [PCI Endpoint Device] [653h 1619 1] Entry Length : 0A [654h 1620 2] Reserved : [656h 1622 1] Enumeration ID : 00 [657h 1623 1] PCI Bus Number : 00 [658h 1624 2] PCI Path : 1C,07 [65Ah 1626 2] PCI Path : 00,00 [65Ch 1628 1] Device Scope Type : 01 [PCI Endpoint Device] [65Dh 1629 1] Entry Length : 0A [65Eh 1630 2] Reserved : [660h 1632 1] Enumeration ID : 00 [661h 1633 1] PCI Bus Number : 00 [662h 1634 2] PCI Path : 1C,07 [664h 1636 2] PCI Path : 00,02 [666h 1638 1] Device Scope Type : 01 [PCI Endpoint Device] [667h 1639 1] Entry Length : 0A [668h 1640 2] Reserved : [66Ah 1642 1] Enumeration ID : 00 [66Bh 1643 1] PCI Bus Number : 00 [66Ch
Re: [PATCH v3 12/13] PCI/ATS: Add PASID stubs
On Mon, Dec 09, 2019 at 07:05:13PM +0100, Jean-Philippe Brucker wrote: > The SMMUv3 driver, which may be built without CONFIG_PCI, will soon gain > PASID support. Partially revert commit c6e9aefbf9db ("PCI/ATS: Remove > unused PRI and PASID stubs") to re-introduce the PASID stubs, and avoid > adding more #ifdefs to the SMMU driver. > > Signed-off-by: Jean-Philippe Brucker Acked-by: Bjorn Helgaas > --- > include/linux/pci-ats.h | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/include/linux/pci-ats.h b/include/linux/pci-ats.h > index 5d62e78946a3..d08f0869f121 100644 > --- a/include/linux/pci-ats.h > +++ b/include/linux/pci-ats.h > @@ -33,6 +33,9 @@ void pci_disable_pasid(struct pci_dev *pdev); > int pci_pasid_features(struct pci_dev *pdev); > int pci_max_pasids(struct pci_dev *pdev); > #else /* CONFIG_PCI_PASID */ > +static inline int pci_enable_pasid(struct pci_dev *pdev, int features) > +{ return -EINVAL; } > +static inline void pci_disable_pasid(struct pci_dev *pdev) { } > static inline int pci_pasid_features(struct pci_dev *pdev) > { return -EINVAL; } > static inline int pci_max_pasids(struct pci_dev *pdev) > -- > 2.24.0 > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 0/8] VT-d Native Shared virtual memory cleanup and fixes
On Tue, 10 Dec 2019 09:14:07 +0800 Lu Baolu wrote: > Hi Jacob, > > This has been queued for internal test. I will forward it to Joerg if > everything goes well (probably around rc4). > Thanks for the confirmation. I will send out further patches based on this series. > Best regards, > -baolu > > On 12/10/19 1:14 AM, Jacob Pan wrote: > > Hi Joerg and Baolu, > > > > Any more comments on this series? I rebased it on v5.5-rc1 without > > changes. > > > > > > Thanks, > > > > Jacob > > > > On Mon, 2 Dec 2019 11:58:21 -0800 > > Jacob Pan wrote: > > > >> Mostly extracted from nested SVA/SVM series based on review > >> comments of v7. https://lkml.org/lkml/2019/10/24/852 > >> > >> This series also adds a few important fixes for native use of SVA. > >> Nested SVA new code will be submitted separately as a smaller set. > >> Based on the core branch of IOMMU tree staged for v5.5, where > >> common APIs for vSVA were applied. > >> git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git core > >> > >> Changelog: > >> v5 - Regrouped patch 6 and 8, added comments suggested by > >> Joe Perches v4 - Commit message fix > >> > >> V3 > >>- Squashed 1/10 & 2/10 > >>- Deleted "8/10 Fix PASID cache flush" from this series > >>- Addressed reviews from Eric Auger and Baolu > >> V2 > >>- Coding style fixes based on Baolu's input, no functional > >> change > >>- Added Acked-by tags. > >> > >> Thanks, > >> > >> Jacob > >> > >> > >> *** BLURB HERE *** > >> > >> Jacob Pan (8): > >>iommu/vt-d: Fix CPU and IOMMU SVM feature matching checks > >>iommu/vt-d: Match CPU and IOMMU paging mode > >>iommu/vt-d: Reject SVM bind for failed capability check > >>iommu/vt-d: Avoid duplicated code for PASID setup > >>iommu/vt-d: Fix off-by-one in PASID allocation > >>iommu/vt-d: Replace Intel specific PASID allocator with IOASID > >>iommu/vt-d: Avoid sending invalid page response > >>iommu/vt-d: Misc macro clean up for SVM > >> > >> drivers/iommu/Kconfig | 1 + > >> drivers/iommu/intel-iommu.c | 23 +++ > >> drivers/iommu/intel-pasid.c | 96 -- > >> drivers/iommu/intel-svm.c | 163 > >> +--- > >> include/linux/intel-iommu.h | 5 +- 5 files changed, 135 > >> insertions(+), 153 deletions(-) > >> > > > > [Jacob Pan] > > [Jacob Pan] ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu: fix min_not_zero() type mismatch warning
min()/max() require the arguments to be of the same type. When dma_addr_t is not compatible with __u64, this causes a warning: In file included from include/linux/list.h:9, from include/linux/kobject.h:19, from include/linux/of.h:17, from include/linux/irqdomain.h:35, from include/linux/acpi.h:13, from include/linux/acpi_iort.h:10, from drivers/iommu/dma-iommu.c:11: drivers/iommu/dma-iommu.c: In function 'iommu_dma_alloc_iova': include/linux/kernel.h:844:29: error: comparison of distinct pointer types lacks a cast [-Werror] (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1))) ^~ include/linux/kernel.h:858:4: note: in expansion of macro '__typecheck' (__typecheck(x, y) && __no_side_effects(x, y)) ^~~ include/linux/kernel.h:868:24: note: in expansion of macro '__safe_cmp' __builtin_choose_expr(__safe_cmp(x, y), \ ^~ include/linux/kernel.h:877:19: note: in expansion of macro '__careful_cmp' #define min(x, y) __careful_cmp(x, y, <) ^ include/linux/kernel.h:910:39: note: in expansion of macro 'min' __x == 0 ? __y : ((__y == 0) ? __x : min(__x, __y)); }) ^~~ drivers/iommu/dma-iommu.c:424:14: note: in expansion of macro 'min_not_zero' dma_limit = min_not_zero(dma_limit, dev->bus_dma_limit); ^~~~ Add an explicit cast to work around it, as there is no min_not_zero_t() equivalent of min_t(). Fixes: a7ba70f1787f ("dma-mapping: treat dev->bus_dma_mask as a DMA limit") Signed-off-by: Arnd Bergmann --- drivers/iommu/dma-iommu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c index 0cc702a70a96..6fa32231dc9f 100644 --- a/drivers/iommu/dma-iommu.c +++ b/drivers/iommu/dma-iommu.c @@ -421,7 +421,7 @@ static dma_addr_t iommu_dma_alloc_iova(struct iommu_domain *domain, if (iova_len < (1 << (IOVA_RANGE_CACHE_MAX_SIZE - 1))) iova_len = roundup_pow_of_two(iova_len); - dma_limit = min_not_zero(dma_limit, dev->bus_dma_limit); + dma_limit = min_not_zero((u64)dma_limit, dev->bus_dma_limit); if (domain->geometry.force_aperture) dma_limit = min(dma_limit, domain->geometry.aperture_end); -- 2.20.0 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu: set group default domain before creating direct mappings
iommu_group_create_direct_mappings uses group->default_domain, but right after it is called, request_default_domain_for_dev calls iommu_domain_free for the default domain, and sets the group default domain to a different domain. Move the iommu_group_create_direct_mappings call to after the group default domain is set, so the direct mappings get associated with that domain. Cc: Joerg Roedel Cc: Lu Baolu Cc: iommu@lists.linux-foundation.org Cc: sta...@vger.kernel.org Fixes: 7423e01741dd ("iommu: Add API to request DMA domain for device") Signed-off-by: Jerry Snitselaar --- drivers/iommu/iommu.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index db7bfd4f2d20..fa908179b80b 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -2282,13 +2282,13 @@ request_default_domain_for_dev(struct device *dev, unsigned long type) goto out; } - iommu_group_create_direct_mappings(group, dev); - /* Make the domain the default for this group */ if (group->default_domain) iommu_domain_free(group->default_domain); group->default_domain = domain; + iommu_group_create_direct_mappings(group, dev); + dev_info(dev, "Using iommu %s mapping\n", type == IOMMU_DOMAIN_DMA ? "dma" : "direct"); -- 2.24.0 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [Patch v3 0/3] iommu: reduce spinlock contention on fast path
On 06/12/2019 21:38, Cong Wang wrote: This patchset contains three small optimizations for the global spinlock contention in IOVA cache. Our memcache perf test shows this reduced its p999 latency down by 45% on AMD when IOMMU is enabled. Cong Wang (3): iommu: avoid unnecessary magazine allocations iommu: optimize iova_magazine_free_pfns() iommu: avoid taking iova_rbtree_lock twice --- drivers/iommu/iova.c | 75 ++-- 1 file changed, 45 insertions(+), 30 deletions(-) I retested, and got a ~1.1% gain in throughput for my storage test - results here if interested https://pastebin.com/dSQxYpN8 Thanks, John ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RESEND PATCH v9 2/4] uacce: add uacce driver
Hi Zhangfei, Thank you for the patch! Yet something to improve: [auto build test ERROR on cryptodev/master] [also build test ERROR on crypto/master char-misc/char-misc-testing v5.5-rc1 next-20191209] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982] url: https://github.com/0day-ci/linux/commits/Zhangfei-Gao/Add-uacce-module-for-Accelerator/20191210-160210 base: https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git master config: s390-allmodconfig (attached as .config) compiler: s390-linux-gcc (GCC) 7.5.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree GCC_VERSION=7.5.0 make.cross ARCH=s390 If you fix the issue, kindly add following tag Reported-by: kbuild test robot All error/warnings (new ones prefixed by >>): >> drivers/misc/uacce/uacce.c:112:15: error: variable 'uacce_sva_ops' has >> initializer but incomplete type static struct iommu_sva_ops uacce_sva_ops = { ^ >> drivers/misc/uacce/uacce.c:113:3: error: 'struct iommu_sva_ops' has no >> member named 'mm_exit' .mm_exit = uacce_sva_exit, ^~~ >> drivers/misc/uacce/uacce.c:113:13: warning: excess elements in struct >> initializer .mm_exit = uacce_sva_exit, ^~ drivers/misc/uacce/uacce.c:113:13: note: (near initialization for 'uacce_sva_ops') drivers/misc/uacce/uacce.c: In function 'uacce_mm_get': >> drivers/misc/uacce/uacce.c:144:12: error: implicit declaration of function >> 'iommu_sva_bind_device'; did you mean 'bus_find_device'? >> [-Werror=implicit-function-declaration] handle = iommu_sva_bind_device(uacce->parent, mm, uacce_mm); ^ bus_find_device >> drivers/misc/uacce/uacce.c:144:10: warning: assignment makes pointer from >> integer without a cast [-Wint-conversion] handle = iommu_sva_bind_device(uacce->parent, mm, uacce_mm); ^ >> drivers/misc/uacce/uacce.c:148:9: error: implicit declaration of function >> 'iommu_sva_set_ops'; did you mean 'iommu_setup_dma_ops'? >> [-Werror=implicit-function-declaration] ret = iommu_sva_set_ops(handle, &uacce_sva_ops); ^ iommu_setup_dma_ops >> drivers/misc/uacce/uacce.c:152:21: error: implicit declaration of function >> 'iommu_sva_get_pasid' [-Werror=implicit-function-declaration] uacce_mm->pasid = iommu_sva_get_pasid(handle); ^~~ >> drivers/misc/uacce/uacce.c:153:26: error: 'IOMMU_PASID_INVALID' undeclared >> (first use in this function); did you mean '_PAGE_INVALID'? if (uacce_mm->pasid == IOMMU_PASID_INVALID) ^~~ _PAGE_INVALID drivers/misc/uacce/uacce.c:153:26: note: each undeclared identifier is reported only once for each function it appears in >> drivers/misc/uacce/uacce.c:168:3: error: implicit declaration of function >> 'iommu_sva_unbind_device'; did you mean 'bus_find_device'? >> [-Werror=implicit-function-declaration] iommu_sva_unbind_device(handle); ^~~ bus_find_device drivers/misc/uacce/uacce.c: In function 'uacce_alloc': >> drivers/misc/uacce/uacce.c:502:9: error: implicit declaration of function >> 'iommu_dev_enable_feature'; did you mean 'module_enable_ro'? >> [-Werror=implicit-function-declaration] ret = iommu_dev_enable_feature(parent, IOMMU_DEV_FEAT_SVA); ^~~~ module_enable_ro >> drivers/misc/uacce/uacce.c:502:42: error: 'IOMMU_DEV_FEAT_SVA' undeclared >> (first use in this function); did you mean 'NOMMU_VMFLAGS'? ret = iommu_dev_enable_feature(parent, IOMMU_DEV_FEAT_SVA); ^~ NOMMU_VMFLAGS >> drivers/misc/uacce/uacce.c:530:3: error: implicit declaration of function >> 'iommu_dev_disable_feature'; did you mean 'module_disable_ro'? >> [-Werror=implicit-function-declaration] iommu_dev_disable_feature(uacce->parent, IOMMU_DEV_FEAT_SVA); ^ module_disable_ro drivers/misc/uacce/uacce.c: In function 'uacce_remove
Re: [PATCH] iommu/dma: Use a better type for dma_limit
On 10/12/2019 9:28 am, Stephan Gerhold wrote: On Mon, Dec 09, 2019 at 01:08:32PM +, Robin Murphy wrote: It makes little sense for dma_limit to be a dma_addr_t when we only use it to pass u64 arguments, and combine it with another u64 along the way. Just make it u64, and head off any possible size mismatches. Signed-off-by: Robin Murphy --- drivers/iommu/dma-iommu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c index 0cc702a70a96..4acc4199a740 100644 --- a/drivers/iommu/dma-iommu.c +++ b/drivers/iommu/dma-iommu.c @@ -399,7 +399,7 @@ static int dma_info_to_prot(enum dma_data_direction dir, bool coherent, } static dma_addr_t iommu_dma_alloc_iova(struct iommu_domain *domain, - size_t size, dma_addr_t dma_limit, struct device *dev) + size_t size, u64 dma_limit, struct device *dev) { struct iommu_dma_cookie *cookie = domain->iova_cookie; struct iova_domain *iovad = &cookie->iovad; Just wondering, maybe you should do the same for: static dma_addr_t __iommu_dma_map(struct device *dev, phys_addr_t phys, size_t size, int prot, dma_addr_t dma_mask) since that passes on the dma_mask as dma_addr_t instead of u64? Also FYI, this fixes the warning on arm32 reported by Nathan [1], but introduces another similar warning on the min() macro below: Oh bums, I guess it's time to do a proper type audit and some more exhaustive build-testing then - no point in just moving warnings around... Thanks for the heads-up! Robin. In file included from ../include/linux/list.h:9, from ../include/linux/kobject.h:19, from ../include/linux/of.h:17, from ../include/linux/irqdomain.h:35, from ../include/linux/acpi.h:13, from ../include/linux/acpi_iort.h:10, from ../drivers/iommu/dma-iommu.c:11: ../drivers/iommu/dma-iommu.c: In function 'iommu_dma_alloc_iova': ../include/linux/kernel.h:844:29: warning: comparison of distinct pointer types lacks a cast 844 | (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1))) | ^~ ../include/linux/kernel.h:858:4: note: in expansion of macro '__typecheck' 858 | (__typecheck(x, y) && __no_side_effects(x, y)) |^~~ ../include/linux/kernel.h:868:24: note: in expansion of macro '__safe_cmp' 868 | __builtin_choose_expr(__safe_cmp(x, y), \ |^~ ../include/linux/kernel.h:877:19: note: in expansion of macro '__careful_cmp' 877 | #define min(x, y) __careful_cmp(x, y, <) | ^ ../drivers/iommu/dma-iommu.c:427:15: note: in expansion of macro 'min' 427 | dma_limit = min(dma_limit, domain->geometry.aperture_end); | ^~~ min_t(u64, dma_limit, domain->geometry.aperture_end) seems to fix it, but not sure if that is correct. Thanks, Stephan [1]: https://lore.kernel.org/linux-iommu/20191123165108.GA15306@ubuntu-x2-xlarge-x86/ ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/dma: Use a better type for dma_limit
On Mon, Dec 09, 2019 at 01:08:32PM +, Robin Murphy wrote: > It makes little sense for dma_limit to be a dma_addr_t when we only use > it to pass u64 arguments, and combine it with another u64 along the way. > Just make it u64, and head off any possible size mismatches. > > Signed-off-by: Robin Murphy > --- > drivers/iommu/dma-iommu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c > index 0cc702a70a96..4acc4199a740 100644 > --- a/drivers/iommu/dma-iommu.c > +++ b/drivers/iommu/dma-iommu.c > @@ -399,7 +399,7 @@ static int dma_info_to_prot(enum dma_data_direction dir, > bool coherent, > } > > static dma_addr_t iommu_dma_alloc_iova(struct iommu_domain *domain, > - size_t size, dma_addr_t dma_limit, struct device *dev) > + size_t size, u64 dma_limit, struct device *dev) > { > struct iommu_dma_cookie *cookie = domain->iova_cookie; > struct iova_domain *iovad = &cookie->iovad; Just wondering, maybe you should do the same for: static dma_addr_t __iommu_dma_map(struct device *dev, phys_addr_t phys, size_t size, int prot, dma_addr_t dma_mask) since that passes on the dma_mask as dma_addr_t instead of u64? Also FYI, this fixes the warning on arm32 reported by Nathan [1], but introduces another similar warning on the min() macro below: In file included from ../include/linux/list.h:9, from ../include/linux/kobject.h:19, from ../include/linux/of.h:17, from ../include/linux/irqdomain.h:35, from ../include/linux/acpi.h:13, from ../include/linux/acpi_iort.h:10, from ../drivers/iommu/dma-iommu.c:11: ../drivers/iommu/dma-iommu.c: In function 'iommu_dma_alloc_iova': ../include/linux/kernel.h:844:29: warning: comparison of distinct pointer types lacks a cast 844 | (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1))) | ^~ ../include/linux/kernel.h:858:4: note: in expansion of macro '__typecheck' 858 | (__typecheck(x, y) && __no_side_effects(x, y)) |^~~ ../include/linux/kernel.h:868:24: note: in expansion of macro '__safe_cmp' 868 | __builtin_choose_expr(__safe_cmp(x, y), \ |^~ ../include/linux/kernel.h:877:19: note: in expansion of macro '__careful_cmp' 877 | #define min(x, y) __careful_cmp(x, y, <) | ^ ../drivers/iommu/dma-iommu.c:427:15: note: in expansion of macro 'min' 427 | dma_limit = min(dma_limit, domain->geometry.aperture_end); | ^~~ min_t(u64, dma_limit, domain->geometry.aperture_end) seems to fix it, but not sure if that is correct. Thanks, Stephan [1]: https://lore.kernel.org/linux-iommu/20191123165108.GA15306@ubuntu-x2-xlarge-x86/ ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu