Re: [PATCH 6/6] arch/*/: remove CONFIG_VIRT_TO_BUS
Arnd Bergmann writes: > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig > index be68c1f02b79..48e1aa0536b6 100644 > --- a/arch/powerpc/Kconfig > +++ b/arch/powerpc/Kconfig > @@ -277,7 +277,6 @@ config PPC > select SYSCTL_EXCEPTION_TRACE > select THREAD_INFO_IN_TASK > select TRACE_IRQFLAGS_SUPPORT > - select VIRT_TO_BUS if !PPC64 > # > # Please keep this list sorted alphabetically. > # > diff --git a/arch/powerpc/include/asm/io.h b/arch/powerpc/include/asm/io.h > index c5a5f7c9b231..73fcd5cdb662 100644 > --- a/arch/powerpc/include/asm/io.h > +++ b/arch/powerpc/include/asm/io.h > @@ -985,8 +985,6 @@ static inline void * bus_to_virt(unsigned long address) > } > #define bus_to_virt bus_to_virt > > -#define page_to_bus(page)(page_to_phys(page) + PCI_DRAM_OFFSET) > - > #endif /* CONFIG_PPC32 */ Seems that's not used by any drivers, so fine to remove. Acked-by: Michael Ellerman (powerpc) cheers ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH kernel] powerpc/iommu: Report the correct most efficient DMA mask for PCI devices
On Thu, 30 Sep 2021 13:44:54 +1000, Alexey Kardashevskiy wrote: > According to dma-api.rst, the dma_get_required_mask() helper should return > "the mask that the platform requires to operate efficiently". Which in > the case of PPC64 means the bypass mask and not a mask from an IOMMU table > which is shorter and slower to use due to map/unmap operations (especially > expensive on "pseries"). > > However the existing implementation ignores the possibility of bypassing > and returns the IOMMU table mask on the pseries platform which makes some > drivers (mpt3sas is one example) choose 32bit DMA even though bypass is > supported. The powernv platform sort of handles it by having a bigger > default window with a mask >=40 but it only works as drivers choose > 63/64bit if the required mask is >32 which is rather pointless. > > [...] Applied to powerpc/fixes. [1/1] powerpc/iommu: Report the correct most efficient DMA mask for PCI devices https://git.kernel.org/powerpc/c/23c216b335d1fbd716076e8263b54a714ea3cf0e cheers ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 4/8] powerpc/pseries/svm: Add a powerpc version of cc_platform_has()
Christoph Hellwig writes: > On Wed, Sep 15, 2021 at 07:18:34PM +0200, Christophe Leroy wrote: >> Could you please provide more explicit explanation why inlining such an >> helper is considered as bad practice and messy ? > > Because now we get architectures to all subly differ. Look at the mess > for ioremap and the ioremap* variant. > > The only good reason to allow for inlines if if they are used in a hot > path. Which cc_platform_has is not, especially not on powerpc. Yes I agree, it's not a hot path so it doesn't really matter, which is why I Acked it. I think it is possible to do both, share the declaration across arches but also give arches flexibility to use an inline if they prefer, see patch below. I'm not suggesting we actually do that for this series now, but I think it would solve the problem if we ever needed to in future. cheers diff --git a/arch/powerpc/platforms/pseries/cc_platform.c b/arch/powerpc/include/asm/cc_platform.h similarity index 74% rename from arch/powerpc/platforms/pseries/cc_platform.c rename to arch/powerpc/include/asm/cc_platform.h index e8021af83a19..6285c3c385a6 100644 --- a/arch/powerpc/platforms/pseries/cc_platform.c +++ b/arch/powerpc/include/asm/cc_platform.h @@ -7,13 +7,10 @@ * Author: Tom Lendacky */ -#include #include - -#include #include -bool cc_platform_has(enum cc_attr attr) +static inline bool arch_cc_platform_has(enum cc_attr attr) { switch (attr) { case CC_ATTR_MEM_ENCRYPT: @@ -23,4 +20,3 @@ bool cc_platform_has(enum cc_attr attr) return false; } } -EXPORT_SYMBOL_GPL(cc_platform_has); diff --git a/arch/x86/include/asm/cc_platform.h b/arch/x86/include/asm/cc_platform.h new file mode 100644 index ..0a4220697043 --- /dev/null +++ b/arch/x86/include/asm/cc_platform.h @@ -0,0 +1,7 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _ASM_X86_CC_PLATFORM_H_ +#define _ASM_X86_CC_PLATFORM_H_ + +bool arch_cc_platform_has(enum cc_attr attr); + +#endif // _ASM_X86_CC_PLATFORM_H_ diff --git a/arch/x86/kernel/cc_platform.c b/arch/x86/kernel/cc_platform.c index 3c9bacd3c3f3..77e8c3465979 100644 --- a/arch/x86/kernel/cc_platform.c +++ b/arch/x86/kernel/cc_platform.c @@ -11,11 +11,11 @@ #include #include -bool cc_platform_has(enum cc_attr attr) +bool arch_cc_platform_has(enum cc_attr attr) { if (sme_me_mask) return amd_cc_platform_has(attr); return false; } -EXPORT_SYMBOL_GPL(cc_platform_has); +EXPORT_SYMBOL_GPL(arch_cc_platform_has); diff --git a/include/linux/cc_platform.h b/include/linux/cc_platform.h index 253f3ea66cd8..f3306647c5d9 100644 --- a/include/linux/cc_platform.h +++ b/include/linux/cc_platform.h @@ -65,6 +65,8 @@ enum cc_attr { #ifdef CONFIG_ARCH_HAS_CC_PLATFORM +#include + /** * cc_platform_has() - Checks if the specified cc_attr attribute is active * @attr: Confidential computing attribute to check @@ -77,7 +79,10 @@ enum cc_attr { * * TRUE - Specified Confidential Computing attribute is active * * FALSE - Specified Confidential Computing attribute is not active */ -bool cc_platform_has(enum cc_attr attr); +static inline bool cc_platform_has(enum cc_attr attr) +{ + return arch_cc_platform_has(attr); +} #else /* !CONFIG_ARCH_HAS_CC_PLATFORM */ ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 4/8] powerpc/pseries/svm: Add a powerpc version of cc_platform_has()
Borislav Petkov writes: > On Wed, Sep 08, 2021 at 05:58:35PM -0500, Tom Lendacky wrote: >> Introduce a powerpc version of the cc_platform_has() function. This will >> be used to replace the powerpc mem_encrypt_active() implementation, so >> the implementation will initially only support the CC_ATTR_MEM_ENCRYPT >> attribute. >> >> Cc: Michael Ellerman >> Cc: Benjamin Herrenschmidt >> Cc: Paul Mackerras >> Signed-off-by: Tom Lendacky >> --- >> arch/powerpc/platforms/pseries/Kconfig | 1 + >> arch/powerpc/platforms/pseries/Makefile | 2 ++ >> arch/powerpc/platforms/pseries/cc_platform.c | 26 >> 3 files changed, 29 insertions(+) >> create mode 100644 arch/powerpc/platforms/pseries/cc_platform.c > > Michael, > > can I get an ACK for the ppc bits to carry them through the tip tree > pls? Yeah. I don't love it, a new C file and an out-of-line call to then call back to a static inline that for most configuration will return false ... but whatever :) Acked-by: Michael Ellerman (powerpc) > Btw, on a related note, cross-compiling this throws the following error here: > > $ make > CROSS_COMPILE=/home/share/src/crosstool/gcc-9.4.0-nolibc/powerpc64-linux/bin/powerpc64-linux- > V=1 ARCH=powerpc > > ... > > /home/share/src/crosstool/gcc-9.4.0-nolibc/powerpc64-linux/bin/powerpc64-linux-gcc > -Wp,-MD,arch/powerpc/boot/.crt0.o.d -D__ASSEMBLY__ -Wall -Wundef > -Wstrict-prototypes -Wno-trigraphs -fno-strict-aliasing -O2 -msoft-float > -mno-altivec -mno-vsx -pipe -fomit-frame-pointer -fno-builtin -fPIC -nostdinc > -include ./include/linux/compiler_attributes.h -I./arch/powerpc/include > -I./arch/powerpc/include/generated -I./include -I./arch/powerpc/include/uapi > -I./arch/powerpc/include/generated/uapi -I./include/uapi > -I./include/generated/uapi -include ./include/linux/compiler-version.h > -include ./include/linux/kconfig.h -m32 -isystem > /home/share/src/crosstool/gcc-9.4.0-nolibc/powerpc64-linux/bin/../lib/gcc/powerpc64-linux/9.4.0/include > -mbig-endian -nostdinc -c -o arch/powerpc/boot/crt0.o > arch/powerpc/boot/crt0.S > In file included from : > ././include/linux/compiler_attributes.h:62:5: warning: "__has_attribute" is > not defined, evaluates to 0 [-Wundef] >62 | #if __has_attribute(__assume_aligned__) > | ^~~ > ././include/linux/compiler_attributes.h:62:20: error: missing binary operator > before token "(" >62 | #if __has_attribute(__assume_aligned__) > |^ > ././include/linux/compiler_attributes.h:88:5: warning: "__has_attribute" is > not defined, evaluates to 0 [-Wundef] >88 | #if __has_attribute(__copy__) > | ^~~ > ... > > Known issue? Yeah, fixed in mainline today, thanks for trying to cross compile :) cheers ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 04/12] powerpc/pseries/svm: Add a powerpc version of prot_guest_has()
Tom Lendacky writes: > Introduce a powerpc version of the prot_guest_has() function. This will > be used to replace the powerpc mem_encrypt_active() implementation, so > the implementation will initially only support the PATTR_MEM_ENCRYPT > attribute. > > Cc: Michael Ellerman > Cc: Benjamin Herrenschmidt > Cc: Paul Mackerras > Signed-off-by: Tom Lendacky > --- > arch/powerpc/include/asm/protected_guest.h | 30 ++ > arch/powerpc/platforms/pseries/Kconfig | 1 + > 2 files changed, 31 insertions(+) > create mode 100644 arch/powerpc/include/asm/protected_guest.h > > diff --git a/arch/powerpc/include/asm/protected_guest.h > b/arch/powerpc/include/asm/protected_guest.h > new file mode 100644 > index ..ce55c2c7e534 > --- /dev/null > +++ b/arch/powerpc/include/asm/protected_guest.h > @@ -0,0 +1,30 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* > + * Protected Guest (and Host) Capability checks > + * > + * Copyright (C) 2021 Advanced Micro Devices, Inc. > + * > + * Author: Tom Lendacky > + */ > + > +#ifndef _POWERPC_PROTECTED_GUEST_H > +#define _POWERPC_PROTECTED_GUEST_H Minor nit, we would usually use _ASM_POWERPC_PROTECTED_GUEST_H Otherwise looks OK to me. Acked-by: Michael Ellerman cheers ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] powerpc/svm: Don't issue ultracalls if !mem_encrypt_active()
Will Deacon writes: > Commit ad6c00283163 ("swiotlb: Free tbl memory in swiotlb_exit()") > introduced a set_memory_encrypted() call to swiotlb_exit() so that the > buffer pages are returned to an encrypted state prior to being freed. > > Sachin reports that this leads to the following crash on a Power server: > > [0.010799] software IO TLB: tearing down default memory pool > [0.010805] [ cut here ] > [0.010808] kernel BUG at arch/powerpc/kernel/interrupt.c:98! > > Nick spotted that this is because set_memory_encrypted() is issuing an > ultracall which doesn't exist for the processor, and should therefore > be gated by mem_encrypt_active() to mirror the x86 implementation. > > Cc: Konrad Rzeszutek Wilk > Cc: Claire Chang > Cc: Christoph Hellwig > Cc: Robin Murphy > Fixes: ad6c00283163 ("swiotlb: Free tbl memory in swiotlb_exit()") > Suggested-by: Nicholas Piggin > Reported-by: Sachin Sant > Tested-by: Sachin Sant > Tested-by: Nathan Chancellor > Link: > https://lore.kernel.org/r/1905cd70-7656-42ae-99e2-a31fc3812...@linux.vnet.ibm.com/ > Signed-off-by: Will Deacon > --- > arch/powerpc/platforms/pseries/svm.c | 6 ++ > 1 file changed, 6 insertions(+) Thanks. Acked-by: Michael Ellerman I assume Konrad will take this via the swiotlb tree? cheers ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH kernel v4 2/2] powerpc/dma: Fallback to dma_ops when persistent memory present
Alexey Kardashevskiy writes: > So far we have been using huge DMA windows to map all the RAM available. > The RAM is normally mapped to the VM address space contiguously, and > there is always a reasonable upper limit for possible future hot plugged > RAM which makes it easy to map all RAM via IOMMU. > > Now there is persistent memory ("ibm,pmemory" in the FDT) which (unlike > normal RAM) can map anywhere in the VM space beyond the maximum RAM size > and since it can be used for DMA, it requires extending the huge window > up to MAX_PHYSMEM_BITS which requires hypervisor support for: > 1. huge TCE tables; > 2. multilevel TCE tables; > 3. huge IOMMU pages. > > Certain hypervisors cannot do either so the only option left is > restricting the huge DMA window to include only RAM and fallback to > the default DMA window for persistent memory. > > This defines arch_dma_map_direct/etc to allow generic DMA code perform > additional checks on whether direct DMA is still possible. > > This checks if the system has persistent memory. If it does not, > the DMA bypass mode is selected, i.e. > * dev->bus_dma_limit = 0 > * dev->dma_ops_bypass = true <- this avoid calling dma_ops for mapping. > > If there is such memory, this creates identity mapping only for RAM and > sets the dev->bus_dma_limit to let the generic code decide whether to > call into the direct DMA or the indirect DMA ops. > > This should not change the existing behaviour when no persistent memory > as dev->dma_ops_bypass is expected to be set. > > Signed-off-by: Alexey Kardashevskiy Acked-by: Michael Ellerman cheers ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH kernel v3 2/2] powerpc/dma: Fallback to dma_ops when persistent memory present
Alexey Kardashevskiy writes: > On 29/10/2020 11:40, Michael Ellerman wrote: >> Alexey Kardashevskiy writes: >>> @@ -1126,7 +1129,7 @@ static u64 enable_ddw(struct pci_dev *dev, struct >>> device_node *pdn) >>> >>> mutex_lock(&direct_window_init_mutex); >>> >>> - dma_addr = find_existing_ddw(pdn); >>> + dma_addr = find_existing_ddw(pdn, &len); >> >> I don't see len used anywhere? >> >>> if (dma_addr != 0) >>> goto out_unlock; >>> >>> @@ -1212,14 +1215,26 @@ static u64 enable_ddw(struct pci_dev *dev, struct >>> device_node *pdn) >>> } >>> /* verify the window * number of ptes will map the partition */ >>> /* check largest block * page size > max memory hotplug addr */ >>> - max_addr = ddw_memory_hotplug_max(); >>> - if (query.largest_available_block < (max_addr >> page_shift)) { >>> - dev_dbg(&dev->dev, "can't map partition max 0x%llx with %llu " >>> - "%llu-sized pages\n", max_addr, >>> query.largest_available_block, >>> - 1ULL << page_shift); >>> + /* >>> +* The "ibm,pmemory" can appear anywhere in the address space. >>> +* Assuming it is still backed by page structs, try MAX_PHYSMEM_BITS >>> +* for the upper limit and fallback to max RAM otherwise but this >>> +* disables device::dma_ops_bypass. >>> +*/ >>> + len = max_ram_len; >> >> Here you override whatever find_existing_ddw() wrote to len? > > Not always, there is a bunch of gotos before this line to the end of the > function and one (which returns the existing window) is legit. Thanks, Ah yep I see it. Gotos considered confusing ;) cheers ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH kernel v3 2/2] powerpc/dma: Fallback to dma_ops when persistent memory present
Alexey Kardashevskiy writes: > diff --git a/arch/powerpc/platforms/pseries/iommu.c > b/arch/powerpc/platforms/pseries/iommu.c > index e4198700ed1a..91112e748491 100644 > --- a/arch/powerpc/platforms/pseries/iommu.c > +++ b/arch/powerpc/platforms/pseries/iommu.c > @@ -,11 +1112,13 @@ static void reset_dma_window(struct pci_dev *dev, > struct device_node *par_dn) > */ > static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn) > { > - int len, ret; > + int len = 0, ret; > + bool pmem_present = of_find_node_by_type(NULL, "ibm,pmemory") != NULL; That leaks a reference on the returned node. dn = of_find_node_by_type(NULL, "ibm,pmemory"); pmem_present = dn != NULL; of_node_put(dn); > @@ -1126,7 +1129,7 @@ static u64 enable_ddw(struct pci_dev *dev, struct > device_node *pdn) > > mutex_lock(&direct_window_init_mutex); > > - dma_addr = find_existing_ddw(pdn); > + dma_addr = find_existing_ddw(pdn, &len); I don't see len used anywhere? > if (dma_addr != 0) > goto out_unlock; > > @@ -1212,14 +1215,26 @@ static u64 enable_ddw(struct pci_dev *dev, struct > device_node *pdn) > } > /* verify the window * number of ptes will map the partition */ > /* check largest block * page size > max memory hotplug addr */ > - max_addr = ddw_memory_hotplug_max(); > - if (query.largest_available_block < (max_addr >> page_shift)) { > - dev_dbg(&dev->dev, "can't map partition max 0x%llx with %llu " > - "%llu-sized pages\n", max_addr, > query.largest_available_block, > - 1ULL << page_shift); > + /* > + * The "ibm,pmemory" can appear anywhere in the address space. > + * Assuming it is still backed by page structs, try MAX_PHYSMEM_BITS > + * for the upper limit and fallback to max RAM otherwise but this > + * disables device::dma_ops_bypass. > + */ > + len = max_ram_len; Here you override whatever find_existing_ddw() wrote to len? > + if (pmem_present) { > + if (query.largest_available_block >= > + (1ULL << (MAX_PHYSMEM_BITS - page_shift))) > + len = MAX_PHYSMEM_BITS - page_shift; > + else > + dev_info(&dev->dev, "Skipping ibm,pmemory"); > + } > + > + if (query.largest_available_block < (1ULL << (len - page_shift))) { > + dev_dbg(&dev->dev, "can't map partition max 0x%llx with %llu > %llu-sized pages\n", > + 1ULL << len, query.largest_available_block, 1ULL << > page_shift); > goto out_failed; > } > - len = order_base_2(max_addr); > win64 = kzalloc(sizeof(struct property), GFP_KERNEL); > if (!win64) { > dev_info(&dev->dev, cheers ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3] powerpc/pseries/svm: Allocate SWIOTLB buffer anywhere in memory
On Tue, 18 Aug 2020 19:11:26 -0300, Thiago Jung Bauermann wrote: > POWER secure guests (i.e., guests which use the Protection Execution > Facility) need to use SWIOTLB to be able to do I/O with the hypervisor, but > they don't need the SWIOTLB memory to be in low addresses since the > hypervisor doesn't have any addressing limitation. > > This solves a SWIOTLB initialization problem we are seeing in secure guests > with 128 GB of RAM: they are configured with 4 GB of crashkernel reserved > memory, which leaves no space for SWIOTLB in low addresses. > > [...] Applied to powerpc/next. [1/1] powerpc/pseries/svm: Allocate SWIOTLB buffer anywhere in memory https://git.kernel.org/powerpc/c/eae9eec476d13fad9af6da1f44a054ee02b7b161 cheers ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3] powerpc/pseries/svm: Allocate SWIOTLB buffer anywhere in memory
Christoph Hellwig writes: > On Tue, Aug 18, 2020 at 07:11:26PM -0300, Thiago Jung Bauermann wrote: >> POWER secure guests (i.e., guests which use the Protection Execution >> Facility) need to use SWIOTLB to be able to do I/O with the hypervisor, but >> they don't need the SWIOTLB memory to be in low addresses since the >> hypervisor doesn't have any addressing limitation. >> >> This solves a SWIOTLB initialization problem we are seeing in secure guests >> with 128 GB of RAM: they are configured with 4 GB of crashkernel reserved >> memory, which leaves no space for SWIOTLB in low addresses. >> >> To do this, we use mostly the same code as swiotlb_init(), but allocate the >> buffer using memblock_alloc() instead of memblock_alloc_low(). >> >> Signed-off-by: Thiago Jung Bauermann > > Looks fine to me (except for the pointlessly long comment lines, but I've > been told that's the powerpc way). They're 80 columns AFAICS? cheers ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 06/15] powerpc: fadamp: simplify fadump_reserve_crash_area()
Mike Rapoport writes: > On Thu, Jul 30, 2020 at 10:15:13PM +1000, Michael Ellerman wrote: >> Mike Rapoport writes: >> > From: Mike Rapoport >> > >> > fadump_reserve_crash_area() reserves memory from a specified base address >> > till the end of the RAM. >> > >> > Replace iteration through the memblock.memory with a single call to >> > memblock_reserve() with appropriate that will take care of proper memory >> ^ >> parameters? >> > reservation. >> > >> > Signed-off-by: Mike Rapoport >> > --- >> > arch/powerpc/kernel/fadump.c | 20 +--- >> > 1 file changed, 1 insertion(+), 19 deletions(-) >> >> I think this looks OK to me, but I don't have a setup to test it easily. >> I've added Hari to Cc who might be able to. >> >> But I'll give you an ack in the hope that it works :) > > Actually, I did some digging in the git log and the traversal was added > there on purpose by the commit b71a693d3db3 ("powerpc/fadump: exclude > memory holes while reserving memory in second kernel") > Presuming this is still reqruired I'm going to drop this patch and will > simply replace for_each_memblock() with for_each_mem_range() in v2. Thanks. cheers ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 06/15] powerpc: fadamp: simplify fadump_reserve_crash_area()
Mike Rapoport writes: > From: Mike Rapoport > > fadump_reserve_crash_area() reserves memory from a specified base address > till the end of the RAM. > > Replace iteration through the memblock.memory with a single call to > memblock_reserve() with appropriate that will take care of proper memory ^ parameters? > reservation. > > Signed-off-by: Mike Rapoport > --- > arch/powerpc/kernel/fadump.c | 20 +--- > 1 file changed, 1 insertion(+), 19 deletions(-) I think this looks OK to me, but I don't have a setup to test it easily. I've added Hari to Cc who might be able to. But I'll give you an ack in the hope that it works :) Acked-by: Michael Ellerman > diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c > index 78ab9a6ee6ac..2446a61e3c25 100644 > --- a/arch/powerpc/kernel/fadump.c > +++ b/arch/powerpc/kernel/fadump.c > @@ -1658,25 +1658,7 @@ int __init fadump_reserve_mem(void) > /* Preserve everything above the base address */ > static void __init fadump_reserve_crash_area(u64 base) > { > - struct memblock_region *reg; > - u64 mstart, msize; > - > - for_each_memblock(memory, reg) { > - mstart = reg->base; > - msize = reg->size; > - > - if ((mstart + msize) < base) > - continue; > - > - if (mstart < base) { > - msize -= (base - mstart); > - mstart = base; > - } > - > - pr_info("Reserving %lluMB of memory at %#016llx for preserving > crash data", > - (msize >> 20), mstart); > - memblock_reserve(mstart, msize); > - } > + memblock_reserve(base, memblock_end_of_DRAM() - base); > } > > unsigned long __init arch_reserved_kernel_pages(void) > -- > 2.26.2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 13/13] powerpc/dma: Remove dev->archdata.iommu_domain
Joerg Roedel writes: > From: Joerg Roedel > > There are no users left, so remove the pointer and save some memory. > > Signed-off-by: Joerg Roedel > --- > arch/powerpc/include/asm/device.h | 3 --- > 1 file changed, 3 deletions(-) It's a little hard to confirm there are no users left just with grep, but I think you've got them all, and the compiler should tell us if you've missed any. Acked-by: Michael Ellerman (powerpc) cheers > diff --git a/arch/powerpc/include/asm/device.h > b/arch/powerpc/include/asm/device.h > index 266542769e4b..1bc595213338 100644 > --- a/arch/powerpc/include/asm/device.h > +++ b/arch/powerpc/include/asm/device.h > @@ -34,9 +34,6 @@ struct dev_archdata { > struct iommu_table *iommu_table_base; > #endif > > -#ifdef CONFIG_IOMMU_API > - void*iommu_domain; > -#endif > #ifdef CONFIG_PPC64 > struct pci_dn *pci_data; > #endif > -- > 2.27.0 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu: spapr_tce: Disable compile testing to fix build on book3s_32 config
Christophe Leroy writes: > On 04/14/2020 02:26 PM, Krzysztof Kozlowski wrote: >> Although SPAPR_TCE_IOMMU itself can be compile tested on certain PowerPC >> configurations, its presence makes arch/powerpc/kvm/Makefile to select >> modules which do not build in such configuration. >> >> The arch/powerpc/kvm/ modules use kvm_arch.spapr_tce_tables which exists >> only with CONFIG_PPC_BOOK3S_64. However these modules are selected when >> COMPILE_TEST and SPAPR_TCE_IOMMU are chosen leading to build failures: >> >> In file included from >> arch/powerpc/include/asm/book3s/64/mmu-hash.h:20:0, >> from arch/powerpc/kvm/book3s_64_vio_hv.c:22: >> arch/powerpc/include/asm/book3s/64/pgtable.h:17:0: error: "_PAGE_EXEC" >> redefined [-Werror] >> #define _PAGE_EXEC 0x1 /* execute permission */ >> >> In file included from arch/powerpc/include/asm/book3s/32/pgtable.h:8:0, >> from arch/powerpc/include/asm/book3s/pgtable.h:8, >> from arch/powerpc/include/asm/pgtable.h:18, >> from include/linux/mm.h:95, >> from arch/powerpc/include/asm/io.h:29, >> from include/linux/io.h:13, >> from include/linux/irq.h:20, >> from arch/powerpc/include/asm/hardirq.h:6, >> from include/linux/hardirq.h:9, >> from include/linux/kvm_host.h:7, >> from arch/powerpc/kvm/book3s_64_vio_hv.c:12: >> arch/powerpc/include/asm/book3s/32/hash.h:29:0: note: this is the >> location of the previous definition >> #define _PAGE_EXEC 0x200 /* software: exec allowed */ >> >> Reported-by: Geert Uytterhoeven >> Fixes: e93a1695d7fb ("iommu: Enable compile testing for some of drivers") >> Signed-off-by: Krzysztof Kozlowski >> --- >> drivers/iommu/Kconfig | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig >> index 58b4a4dbfc78..3532b1ead19d 100644 >> --- a/drivers/iommu/Kconfig >> +++ b/drivers/iommu/Kconfig >> @@ -362,7 +362,7 @@ config IPMMU_VMSA >> >> config SPAPR_TCE_IOMMU >> bool "sPAPR TCE IOMMU Support" >> -depends on PPC_POWERNV || PPC_PSERIES || (PPC && COMPILE_TEST) >> +depends on PPC_POWERNV || PPC_PSERIES >> select IOMMU_API >> help >>Enables bits of IOMMU API required by VFIO. The iommu_ops >> > > Should it be fixed the other way round, something like: That doesn't actually fix this specific issue, the code will build but then not link: ld: arch/powerpc/../../virt/kvm/vfio.o: in function `.kvm_spapr_tce_release_vfio_group': vfio.c:(.text.kvm_spapr_tce_release_vfio_group+0xb0): undefined reference to `.kvm_spapr_tce_release_iommu_group' ld: arch/powerpc/../../virt/kvm/vfio.o: in function `.kvm_vfio_set_group': vfio.c:(.text.kvm_vfio_set_group+0x7f4): undefined reference to `.kvm_spapr_tce_attach_iommu_group' ld: arch/powerpc/kvm/powerpc.o: in function `.kvm_arch_vm_ioctl': (.text.kvm_arch_vm_ioctl+0x1a4): undefined reference to `.kvm_vm_ioctl_create_spapr_tce' ld: (.text.kvm_arch_vm_ioctl+0x230): undefined reference to `.kvm_vm_ioctl_create_spapr_tce' make[1]: *** [/home/michael/linux/Makefile:1106: vmlinux] Error 1 > diff --git a/arch/powerpc/kvm/Makefile b/arch/powerpc/kvm/Makefile > index 2bfeaa13befb..906707d15810 100644 > --- a/arch/powerpc/kvm/Makefile > +++ b/arch/powerpc/kvm/Makefile > @@ -135,4 +135,4 @@ obj-$(CONFIG_KVM_BOOK3S_32) += kvm.o > obj-$(CONFIG_KVM_BOOK3S_64_PR) += kvm-pr.o > obj-$(CONFIG_KVM_BOOK3S_64_HV) += kvm-hv.o > > -obj-y += $(kvm-book3s_64-builtin-objs-y) > +obj-$(CONFIG_KVM_BOOK3S_64) += $(kvm-book3s_64-builtin-objs-y) But this is probably still a good thing to do, as it would have made the error messages clearer in this case I think. cheers ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] powerpc: ensure that swiotlb buffer is allocated from low memory
On Wed, 2019-12-04 at 12:35:24 UTC, Mike Rapoport wrote: > From: Mike Rapoport > > Some powerpc platforms (e.g. 85xx) limit DMA-able memory way below 4G. If a > system has more physical memory than this limit, the swiotlb buffer is not > addressable because it is allocated from memblock using top-down mode. > > Force memblock to bottom-up mode before calling swiotlb_init() to ensure > that the swiotlb buffer is DMA-able. > > Link: > https://lkml.kernel.org/r/f1ebb706-73df-430e-9020-c214ec8ed...@xenosoft.de > Reported-by: Christian Zigotzky > Signed-off-by: Mike Rapoport Applied to powerpc fixes, thanks. https://git.kernel.org/powerpc/c/8fabc623238e68b3ac63c0dd1657bf86c1fa33af cheers ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] powerpc: ensure that swiotlb buffer is allocated from low memory
Mike Rapoport writes: > From: Mike Rapoport > > Some powerpc platforms (e.g. 85xx) limit DMA-able memory way below 4G. If a > system has more physical memory than this limit, the swiotlb buffer is not > addressable because it is allocated from memblock using top-down mode. > > Force memblock to bottom-up mode before calling swiotlb_init() to ensure > that the swiotlb buffer is DMA-able. > > Link: > https://lkml.kernel.org/r/f1ebb706-73df-430e-9020-c214ec8ed...@xenosoft.de This wasn't bisected, but I thought it was a regression. Do we know what commit caused it? Was it 25078dc1f74b ("powerpc: use mm zones more sensibly") ? Or was that a red herring? cheers > Reported-by: Christian Zigotzky > Signed-off-by: Mike Rapoport > Cc: Benjamin Herrenschmidt > Cc: Christoph Hellwig > Cc: Darren Stevens > Cc: mad skateman > Cc: Michael Ellerman > Cc: Nicolas Saenz Julienne > Cc: Paul Mackerras > Cc: Robin Murphy > Cc: Rob Herring > --- > arch/powerpc/mm/mem.c | 8 > 1 file changed, 8 insertions(+) > > diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c > index be941d382c8d..14c2c53e3f9e 100644 > --- a/arch/powerpc/mm/mem.c > +++ b/arch/powerpc/mm/mem.c > @@ -260,6 +260,14 @@ void __init mem_init(void) > BUILD_BUG_ON(MMU_PAGE_COUNT > 16); > > #ifdef CONFIG_SWIOTLB > + /* > + * Some platforms (e.g. 85xx) limit DMA-able memory way below > + * 4G. We force memblock to bottom-up mode to ensure that the > + * memory allocated in swiotlb_init() is DMA-able. > + * As it's the last memblock allocation, no need to reset it > + * back to to-down. > + */ > + memblock_set_bottom_up(true); > swiotlb_init(0); > #endif > > -- > 2.24.0 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 3/3] powerpc: remove support for NULL dev in __phys_to_dma / __dma_to_phys
Christoph Hellwig writes: > Support for calling the DMA API functions without a valid device pointer > was removed a while ago, so remove the stale support for that from the > powerpc __phys_to_dma / __dma_to_phys helpers. > > Signed-off-by: Christoph Hellwig > --- > arch/powerpc/include/asm/dma-direct.h | 4 > 1 file changed, 4 deletions(-) Acked-by: Michael Ellerman cheers > diff --git a/arch/powerpc/include/asm/dma-direct.h > b/arch/powerpc/include/asm/dma-direct.h > index e29e8a236b8d..abc154d784b0 100644 > --- a/arch/powerpc/include/asm/dma-direct.h > +++ b/arch/powerpc/include/asm/dma-direct.h > @@ -4,15 +4,11 @@ > > static inline dma_addr_t __phys_to_dma(struct device *dev, phys_addr_t paddr) > { > - if (!dev) > - return paddr + PCI_DRAM_OFFSET; > return paddr + dev->archdata.dma_offset; > } > > static inline phys_addr_t __dma_to_phys(struct device *dev, dma_addr_t daddr) > { > - if (!dev) > - return daddr - PCI_DRAM_OFFSET; > return daddr - dev->archdata.dma_offset; > } > #endif /* ASM_POWERPC_DMA_DIRECT_H */ > -- > 2.20.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/3] dma-direct: unify the dma_capable definitions
Christoph Hellwig writes: > Currently each architectures that wants to override dma_to_phys and > phys_to_dma also has to provide dma_capable. But there isn't really > any good reason for that. powerpc and mips just have copies of the > generic one minus the latests fix, and the arm one was the inspiration > for said fix, but misses the bus_dma_mask handling. > Make all architectures use the generic version instead. > > Signed-off-by: Christoph Hellwig > --- > arch/arm/include/asm/dma-direct.h | 19 --- > arch/mips/include/asm/dma-direct.h| 8 > arch/powerpc/include/asm/dma-direct.h | 9 - Looks OK to me. Acked-by: Michael Ellerman (powerpc) cheers > diff --git a/arch/arm/include/asm/dma-direct.h > b/arch/arm/include/asm/dma-direct.h > index b67e5fc1fe43..7c3001a6a775 100644 > --- a/arch/arm/include/asm/dma-direct.h > +++ b/arch/arm/include/asm/dma-direct.h > @@ -14,23 +14,4 @@ static inline phys_addr_t __dma_to_phys(struct device > *dev, dma_addr_t dev_addr) > return __pfn_to_phys(dma_to_pfn(dev, dev_addr)) + offset; > } > > -static inline bool dma_capable(struct device *dev, dma_addr_t addr, size_t > size) > -{ > - u64 limit, mask; > - > - if (!dev->dma_mask) > - return 0; > - > - mask = *dev->dma_mask; > - > - limit = (mask + 1) & ~mask; > - if (limit && size > limit) > - return 0; > - > - if ((addr | (addr + size - 1)) & ~mask) > - return 0; > - > - return 1; > -} > - > #endif /* ASM_ARM_DMA_DIRECT_H */ > diff --git a/arch/mips/include/asm/dma-direct.h > b/arch/mips/include/asm/dma-direct.h > index b5c240806e1b..14e352651ce9 100644 > --- a/arch/mips/include/asm/dma-direct.h > +++ b/arch/mips/include/asm/dma-direct.h > @@ -2,14 +2,6 @@ > #ifndef _MIPS_DMA_DIRECT_H > #define _MIPS_DMA_DIRECT_H 1 > > -static inline bool dma_capable(struct device *dev, dma_addr_t addr, size_t > size) > -{ > - if (!dev->dma_mask) > - return false; > - > - return addr + size - 1 <= *dev->dma_mask; > -} > - > dma_addr_t __phys_to_dma(struct device *dev, phys_addr_t paddr); > phys_addr_t __dma_to_phys(struct device *dev, dma_addr_t daddr); > > diff --git a/arch/powerpc/include/asm/dma-direct.h > b/arch/powerpc/include/asm/dma-direct.h > index a2912b47102c..e29e8a236b8d 100644 > --- a/arch/powerpc/include/asm/dma-direct.h > +++ b/arch/powerpc/include/asm/dma-direct.h > @@ -2,15 +2,6 @@ > #ifndef ASM_POWERPC_DMA_DIRECT_H > #define ASM_POWERPC_DMA_DIRECT_H 1 > > -static inline bool dma_capable(struct device *dev, dma_addr_t addr, size_t > size) > -{ > - if (!dev->dma_mask) > - return false; > - > - return addr + size - 1 <= > - min_not_zero(*dev->dma_mask, dev->bus_dma_mask); > -} > - > static inline dma_addr_t __phys_to_dma(struct device *dev, phys_addr_t paddr) > { > if (!dev) > diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h > index 6db863c3eb93..991f8aa2676e 100644 > --- a/include/linux/dma-direct.h > +++ b/include/linux/dma-direct.h > @@ -24,6 +24,7 @@ static inline phys_addr_t __dma_to_phys(struct device *dev, > dma_addr_t dev_addr) > > return paddr + ((phys_addr_t)dev->dma_pfn_offset << PAGE_SHIFT); > } > +#endif /* !CONFIG_ARCH_HAS_PHYS_TO_DMA */ > > static inline bool dma_capable(struct device *dev, dma_addr_t addr, size_t > size) > { > @@ -38,7 +39,6 @@ static inline bool dma_capable(struct device *dev, > dma_addr_t addr, size_t size) > > return end <= min_not_zero(*dev->dma_mask, dev->bus_dma_mask); > } > -#endif /* !CONFIG_ARCH_HAS_PHYS_TO_DMA */ > > #ifdef CONFIG_ARCH_HAS_FORCE_DMA_UNENCRYPTED > bool force_dma_unencrypted(struct device *dev); > -- > 2.20.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [GIT PULL] dma-mapping updates for 5.4
On 20 September 2019 6:33:50 am AEST, Linus Torvalds wrote: >On Wed, Sep 18, 2019 at 8:27 AM Christoph Hellwig >wrote: >> >> please pull the dma-mapping updates for 5.4. > >Pulled. > >> In addition to the usual Kconfig conflics where you just want to keep >> both edits there are a few more interesting merge issues this time: >> >> - most importanly powerpc and microblaze add new callers of >>dma_atomic_pool_init, while this tree marks the function static >>and calls it from a common postcore_initcall(). The trivial >>functions added in powerpc and microblaze adding the calls >>need to be removed for the code to compile. This will not show up >>as a merge conflict and needs to be dealt with manually! > >So I haven't gotten the powerpc or microblaze pull requests yet, so >I'm not able to fix that part up yet. > >Intead, I'm cc'ing Michael Ellerman and Michal Simek to ask them to >remind me when they _do_ send those pull requests, since otherwise I >may well forget and miss it. Without an actual data conflict, and >since this won't show up in my build tests either, it would be very >easy for me to forget. > >Micha[e]l, can you both please make sure to remind me? Yeah I was aware of it, and will make sure to remind you in my pull request. cheers -- Sent from my Android phone with K-9 Mail. Please excuse my brevity.
Re: [PATCH v4 1/6] x86, s390: Move ARCH_HAS_MEM_ENCRYPT definition to arch/Kconfig
On Tue, 2019-08-06 at 04:49:14 UTC, Thiago Jung Bauermann wrote: > powerpc is also going to use this feature, so put it in a generic location. > > Signed-off-by: Thiago Jung Bauermann > Reviewed-by: Thomas Gleixner > Reviewed-by: Christoph Hellwig Series applied to powerpc topic/mem-encrypt, thanks. https://git.kernel.org/powerpc/c/0c9c1d56397518eb823d458b00b06bcccd956794 cheers ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 09/11] dma-direct: turn ARCH_ZONE_DMA_BITS into a variable
Nicolas Saenz Julienne writes: > diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h > index 0d52f57fca04..73668a21ae78 100644 > --- a/arch/powerpc/include/asm/page.h > +++ b/arch/powerpc/include/asm/page.h > @@ -319,13 +319,4 @@ struct vm_area_struct; > #endif /* __ASSEMBLY__ */ > #include > > -/* > - * Allow 30-bit DMA for very limited Broadcom wifi chips on many powerbooks. This comment got lost. > diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c > index 9191a66b3bc5..2a69f87585df 100644 > --- a/arch/powerpc/mm/mem.c > +++ b/arch/powerpc/mm/mem.c > @@ -237,9 +238,14 @@ void __init paging_init(void) > printk(KERN_DEBUG "Memory hole size: %ldMB\n", > (long int)((top_of_ram - total_ram) >> 20)); > > + if (IS_ENABLED(CONFIG_PPC32)) Can you please propagate it here? > + zone_dma_bits = 30; > + else > + zone_dma_bits = 31; > + cheers
Re: [PATCH] powerpc/dma: Fix invalid DMA mmap behavior
Shawn Anastasio writes: > On 7/22/19 7:16 AM, Michael Ellerman wrote: >> Arnd Bergmann writes: >>> On Thu, Jul 18, 2019 at 11:52 AM Christoph Hellwig wrote: >>>> On Thu, Jul 18, 2019 at 10:49:34AM +0200, Christoph Hellwig wrote: >>>>> On Thu, Jul 18, 2019 at 01:45:16PM +1000, Oliver O'Halloran wrote: >>>>>>> Other than m68k, mips, and arm64, everybody else that doesn't have >>>>>>> ARCH_NO_COHERENT_DMA_MMAP set uses this default implementation, so >>>>>>> I assume this behavior is acceptable on those architectures. >>>>>> >>>>>> It might be acceptable, but there's no reason to use pgport_noncached >>>>>> if the platform supports cache-coherent DMA. >>>>>> >>>>>> Christoph (+cc) made the change so maybe he saw something we're missing. >>>>> >>>>> I always found the forcing of noncached access even for coherent >>>>> devices a little odd, but this was inherited from the previous >>>>> implementation, which surprised me a bit as the different attributes >>>>> are usually problematic even on x86. Let me dig into the history a >>>>> bit more, but I suspect the righ fix is to default to cached mappings >>>>> for coherent devices. >>>> >>>> Ok, some history: >>>> >>>> The generic dma mmap implementation, which we are effectively still >>>> using today was added by: >>>> >>>> commit 64ccc9c033c6089b2d426dad3c56477ab066c999 >>>> Author: Marek Szyprowski >>>> Date: Thu Jun 14 13:03:04 2012 +0200 >>>> >>>> common: dma-mapping: add support for generic dma_mmap_* calls >>>> >>>> and unconditionally uses pgprot_noncached in dma_common_mmap, which is >>>> then used as the fallback by dma_mmap_attrs if no ->mmap method is >>>> present. At that point we already had the powerpc implementation >>>> that only uses pgprot_noncached for non-coherent mappings, and >>>> the arm one, which uses pgprot_writecombine if DMA_ATTR_WRITE_COMBINE >>>> is set and otherwise pgprot_dmacoherent, which seems to be uncached. >>>> Arm did support coherent platforms at that time, but they might have >>>> been an afterthought and not handled properly. >>> >>> Cache-coherent devices are still very rare on 32-bit ARM. >>> >>> Among the callers of dma_mmap_coherent(), almost all are in platform >>> specific device drivers that only ever run on noncoherent ARM SoCs, >>> which explains why nobody would have noticed problems. >>> >>> There is also a difference in behavior between ARM and PowerPC >>> when dealing with mismatched cacheability attributes: If the same >>> page is mapped as both cached and uncached to, this may >>> cause silent undefined behavior on ARM, while PowerPC should >>> enter a checkstop as soon as it notices. >> >> On newer Power CPUs it's actually more like the ARM behaviour. >> >> I don't know for sure that it will *never* checkstop but there are at >> least cases where it won't. There's some (not much) detail in the >> Power8/9 user manuals. > > The issue was discovered due to sporadic checkstops on P9, so it > seems like it will happen at least sometimes. Yeah true. I wasn't sure if that checkstop was actually caused by a cached/uncached mismatch or something else, but looks like it was, from the hostboot issue (https://github.com/open-power/hostboot/issues/180): 12.47015| Signature Description: pu.ex:k0:n0:s0:p00:c0 (L2FIR[16]) Cache line inhibited hit cacheable space So I'm not really sure how to square that with the documentation in the user manual: If a caching-inhibited load instruction hits in the L1 data cache, the load data is serviced from the L1 data cache and no request is sent to the NCU. If a caching-inhibited store instruction hits in the L1 data cache, the store data is written to the L1 data cache and sent to the NCU. Note that the L1 data cache and L2 cache are no longer coherent. I guess I'm either misinterpreting that section or there's some *other* documentation somewhere I haven't found that says that it will also checkstop. cheers ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] powerpc/dma: Fix invalid DMA mmap behavior
Arnd Bergmann writes: > On Thu, Jul 18, 2019 at 11:52 AM Christoph Hellwig wrote: >> On Thu, Jul 18, 2019 at 10:49:34AM +0200, Christoph Hellwig wrote: >> > On Thu, Jul 18, 2019 at 01:45:16PM +1000, Oliver O'Halloran wrote: >> > > > Other than m68k, mips, and arm64, everybody else that doesn't have >> > > > ARCH_NO_COHERENT_DMA_MMAP set uses this default implementation, so >> > > > I assume this behavior is acceptable on those architectures. >> > > >> > > It might be acceptable, but there's no reason to use pgport_noncached >> > > if the platform supports cache-coherent DMA. >> > > >> > > Christoph (+cc) made the change so maybe he saw something we're missing. >> > >> > I always found the forcing of noncached access even for coherent >> > devices a little odd, but this was inherited from the previous >> > implementation, which surprised me a bit as the different attributes >> > are usually problematic even on x86. Let me dig into the history a >> > bit more, but I suspect the righ fix is to default to cached mappings >> > for coherent devices. >> >> Ok, some history: >> >> The generic dma mmap implementation, which we are effectively still >> using today was added by: >> >> commit 64ccc9c033c6089b2d426dad3c56477ab066c999 >> Author: Marek Szyprowski >> Date: Thu Jun 14 13:03:04 2012 +0200 >> >> common: dma-mapping: add support for generic dma_mmap_* calls >> >> and unconditionally uses pgprot_noncached in dma_common_mmap, which is >> then used as the fallback by dma_mmap_attrs if no ->mmap method is >> present. At that point we already had the powerpc implementation >> that only uses pgprot_noncached for non-coherent mappings, and >> the arm one, which uses pgprot_writecombine if DMA_ATTR_WRITE_COMBINE >> is set and otherwise pgprot_dmacoherent, which seems to be uncached. >> Arm did support coherent platforms at that time, but they might have >> been an afterthought and not handled properly. > > Cache-coherent devices are still very rare on 32-bit ARM. > > Among the callers of dma_mmap_coherent(), almost all are in platform > specific device drivers that only ever run on noncoherent ARM SoCs, > which explains why nobody would have noticed problems. > > There is also a difference in behavior between ARM and PowerPC > when dealing with mismatched cacheability attributes: If the same > page is mapped as both cached and uncached to, this may > cause silent undefined behavior on ARM, while PowerPC should > enter a checkstop as soon as it notices. On newer Power CPUs it's actually more like the ARM behaviour. I don't know for sure that it will *never* checkstop but there are at least cases where it won't. There's some (not much) detail in the Power8/9 user manuals. cheers ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [01/32] net: pasemi: set a 64-bit DMA mask on the DMA device
On Wed, 2019-02-13 at 07:01:02 UTC, Christoph Hellwig wrote: > The pasemi driver never set a DMA mask, and given that the powerpc > DMA mapping routines never check it this worked ok so far. But the > generic dma-direct code which I plan to switch on for powerpc checks > the DMA mask and fails unsupported mapping requests, so we need to > make sure the proper 64-bit mask is set. > > Reported-by: Christian Zigotzky > Signed-off-by: Christoph Hellwig > Tested-by: Christian Zigotzky Series applied to powerpc next, thanks. https://git.kernel.org/powerpc/c/74ebe3e733b791f37415b3a1b917ee50 cheers ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3] crypto: talitos - fix ablkcipher for CONFIG_VMAP_STACK
Christophe Leroy writes: > Le 04/01/2019 à 16:24, Horia Geanta a écrit : >> On 1/4/2019 5:17 PM, Horia Geanta wrote: >>> On 12/21/2018 10:07 AM, Christophe Leroy wrote: >>> [snip] IV cannot be on stack when CONFIG_VMAP_STACK is selected because the stack cannot be DMA mapped anymore. This looks better, thanks. >>> This patch copies the IV into the extended descriptor when iv is not a valid linear address. >>> Though I am not sure the checks in place are enough. >>> Fixes: 4de9d0b547b9 ("crypto: talitos - Add ablkcipher algorithms") Cc: sta...@vger.kernel.org Signed-off-by: Christophe Leroy --- v3: Using struct edesc buffer. v2: Using per-request context. >>> [snip] + if (ivsize && !virt_addr_valid(iv)) + alloc_len += ivsize; >>> [snip] + if (ivsize && !virt_addr_valid(iv)) >>> A more precise condition would be (!is_vmalloc_addr || is_vmalloc_addr(iv)) >>> >> Sorry for the typo, I meant: >> (!virt_addr_valid(iv) || is_vmalloc_addr(iv)) > > As far as I know, virt_addr_valid() means the address is in the linear > memory space. So it cannot be a vmalloc_addr if it is a linear space > addr, can it ? That matches my understanding. This is one of those historical macros that has no documentation and its behaviour is only defined in terms of other things, so it's kind of hard to track down. In commit e41e53cd4fe3 ("powerpc/mm: Fix virt_addr_valid() etc. on 64-bit hash") https://git.kernel.org/torvalds/c/e41e53cd4fe3 I said: virt_addr_valid() is supposed to tell you if it's OK to call virt_to_page() on an address. What this means in practice is that it should only return true for addresses in the linear mapping which are backed by a valid PFN. Each arch can define virt_to_page(), so presumably you *could* set things up such that virt_to_page() worked on vmalloc addresses. Originally virt_to_page() would basically just mask and right shift the address and then use that as an array index to get the page. Things are more complicated now that we have SPARSEMEM etc. but it still only works for linear mapping addresses. Hopefully someone who knows mm stuff can chime in and tell us if we're missing anything :) cheers ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: use generic DMA mapping code in powerpc V4
Christoph Hellwig writes: > FYI, given that we are one week before the expected 4.20 release > date and I haven't found the bug plaging Christians setups I think > we need to defer most of this to the next merge window. OK, sorry I couldn't help. I tried powering up my pasemi board last week but it just gives me a couple of status leds and nothing else, the fan never spins up. > I'd still like to get a few bits in earlier, which I will send out > separately now. OK. cheers ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 12/34] powerpc/cell: move dma direct window setup out of dma_configure
Christoph Hellwig writes: > On Sun, Dec 09, 2018 at 09:23:39PM +1100, Michael Ellerman wrote: >> Christoph Hellwig writes: >> >> > Configure the dma settings at device setup time, and stop playing games >> > with get_pci_dma_ops. This prepares for using the common dma_configure >> > code later on. >> > >> > Signed-off-by: Christoph Hellwig >> > --- >> > arch/powerpc/platforms/cell/iommu.c | 20 +++- >> > 1 file changed, 11 insertions(+), 9 deletions(-) >> >> This one's crashing, haven't dug into why yet: > > Can you provide a gdb assembly of the exact crash site? This looks > like for some odd reason the DT structures aren't fully setup by the > time we are probing the device, which seems odd. It's dev->of_node which is NULL. Because we were passed a platform_device which doesn't have an of_node. It's the cbe-mic device created in cell_publish_devices(). I can fix that by simply checking for a NULL node, then the system boots but then I have no network devices, due to: tg3 :00:01.0: enabling device (0140 -> 0142) tg3 :00:01.0: DMA engine test failed, aborting tg3: probe of :00:01.0 failed with error -12 tg3 :00:01.1: enabling device (0140 -> 0142) tg3 :00:01.1: DMA engine test failed, aborting tg3: probe of :00:01.1 failed with error -12 I think the problem is that we don't want to set iommu_bypass_supported unless cell_iommu_fixed_mapping_init() succeeds. Yep. This makes it work for me on cell on top of your v5. cheers diff --git a/arch/powerpc/platforms/cell/iommu.c b/arch/powerpc/platforms/cell/iommu.c index 348a815779c1..8329fda17cc8 100644 --- a/arch/powerpc/platforms/cell/iommu.c +++ b/arch/powerpc/platforms/cell/iommu.c @@ -813,6 +813,10 @@ static u64 cell_iommu_get_fixed_address(struct device *dev) int i, len, best, naddr, nsize, pna, range_size; np = of_node_get(dev->of_node); + if (!np) + /* We can be called for platform devices that have no of_node */ + goto out; + while (1) { naddr = of_n_addr_cells(np); nsize = of_n_size_cells(np); @@ -1065,8 +1069,11 @@ static int __init cell_iommu_init(void) /* Setup various callbacks */ cell_pci_controller_ops.dma_dev_setup = cell_pci_dma_dev_setup; - if (!iommu_fixed_disabled && cell_iommu_fixed_mapping_init() == 0) + if (!iommu_fixed_disabled && cell_iommu_fixed_mapping_init() == 0) { + cell_pci_controller_ops.iommu_bypass_supported = + cell_pci_iommu_bypass_supported; goto done; + } /* Create an iommu for each /axon node. */ for_each_node_by_name(np, "axon") { @@ -1085,10 +1092,6 @@ static int __init cell_iommu_init(void) } done: /* Setup default PCI iommu ops */ - if (!iommu_fixed_disabled) { - cell_pci_controller_ops.iommu_bypass_supported = - cell_pci_iommu_bypass_supported; - } set_pci_dma_ops(&dma_iommu_ops); cell_iommu_enabled = true; bail: ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 12/34] powerpc/cell: move dma direct window setup out of dma_configure
Christoph Hellwig writes: > Configure the dma settings at device setup time, and stop playing games > with get_pci_dma_ops. This prepares for using the common dma_configure > code later on. > > Signed-off-by: Christoph Hellwig > --- > arch/powerpc/platforms/cell/iommu.c | 20 +++- > 1 file changed, 11 insertions(+), 9 deletions(-) This one's crashing, haven't dug into why yet: [1.347085] Unable to handle kernel paging request for data at address 0x0040 [1.391505] Faulting instruction address: 0xc06b6e6c cpu 0x0: Vector: 380 (Data SLB Access) at [c007fc9032d0] pc: c06b6e6c: .of_n_addr_cells+0x34/0xc0 lr: c0070b30: .cell_iommu_get_fixed_address+0x58/0x2b0 sp: c007fc903560 msr: 90009032 dar: 40 current = 0xc007fc8d paca= 0xc0f6 irqmask: 0x03 irq_happened: 0x01 pid = 1, comm = swapper/0 Linux version 4.20.0-rc2-gcc7x-g1e32f48 (kerkins@p82) (gcc version 7.4.1 20181208 (Custom eb377405ab2d1900)) #1 SMP Sun Dec 9 12:16:48 AEDT 2018 enter ? for help [c007fc9035f0] c0070b30 .cell_iommu_get_fixed_address+0x58/0x2b0 [c007fc9036c0] c00711ac .cell_dma_dev_setup.part.1+0x24/0x118 [c007fc903740] c0071374 .cell_of_bus_notify+0x6c/0xbc [c007fc9037c0] c00e7ef0 .notifier_call_chain+0x90/0xf8 [c007fc903860] c00e8c2c .blocking_notifier_call_chain+0x84/0xb8 [c007fc9038f0] c0597544 .device_add+0x584/0x7b8 [c007fc9039c0] c05a0308 .platform_device_add+0x148/0x2f0 [c007fc903a60] c05a1508 .platform_device_register_full+0x148/0x168 [c007fc903ae0] c0a9a8a0 .__machine_initcall_cell_cell_publish_devices+0x1bc/0x210 [c007fc903be0] c000eca4 .do_one_initcall+0x64/0x2d8 [c007fc903cc0] c0a844ec .kernel_init_freeable+0x3dc/0x4e4 [c007fc903da0] c000f06c .kernel_init+0x24/0x150 [c007fc903e20] c000a9c0 .ret_from_kernel_thread+0x58/0x78 cheers > diff --git a/arch/powerpc/platforms/cell/iommu.c > b/arch/powerpc/platforms/cell/iommu.c > index 12352a58072a..cce5bf9515e5 100644 > --- a/arch/powerpc/platforms/cell/iommu.c > +++ b/arch/powerpc/platforms/cell/iommu.c > @@ -657,14 +657,21 @@ static const struct dma_map_ops dma_iommu_fixed_ops = { > .mapping_error = dma_iommu_mapping_error, > }; > > +static u64 cell_iommu_get_fixed_address(struct device *dev); > + > static void cell_dma_dev_setup(struct device *dev) > { > - if (get_pci_dma_ops() == &dma_iommu_ops) > + if (get_pci_dma_ops() == &dma_iommu_ops) { > + u64 addr = cell_iommu_get_fixed_address(dev); > + > + if (addr != OF_BAD_ADDR) > + set_dma_offset(dev, addr + dma_iommu_fixed_base); > set_iommu_table_base(dev, cell_get_iommu_table(dev)); > - else if (get_pci_dma_ops() == &dma_nommu_ops) > + } else if (get_pci_dma_ops() == &dma_nommu_ops) { > set_dma_offset(dev, cell_dma_nommu_offset); > - else > + } else { > BUG(); > + } > } > > static void cell_pci_dma_dev_setup(struct pci_dev *dev) > @@ -950,19 +957,14 @@ static int dma_suported_and_switch(struct device *dev, > u64 dma_mask) > { > if (dma_mask == DMA_BIT_MASK(64) && > cell_iommu_get_fixed_address(dev) != OF_BAD_ADDR) { > - u64 addr = cell_iommu_get_fixed_address(dev) + > - dma_iommu_fixed_base; > dev_dbg(dev, "iommu: 64-bit OK, using fixed ops\n"); > - dev_dbg(dev, "iommu: fixed addr = %llx\n", addr); > set_dma_ops(dev, &dma_iommu_fixed_ops); > - set_dma_offset(dev, addr); > return 1; > } > > if (dma_iommu_dma_supported(dev, dma_mask)) { > dev_dbg(dev, "iommu: not 64-bit, using default ops\n"); > - set_dma_ops(dev, get_pci_dma_ops()); > - cell_dma_dev_setup(dev); > + set_dma_ops(dev, &dma_iommu_ops); > return 1; > } > > -- > 2.19.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 01/34] powerpc: use mm zones more sensibly
Christoph Hellwig writes: > Ben / Michael, > > can we get this one queued up for 4.21 to prepare for the DMA work later > on? I was hoping the PASEMI / NXP regressions could be solved before merging. My p5020ds is booting fine with this series, so I'm not sure why it's causing problems on Christian's machine. The last time I turned on my PASEMI board it tripped some breakers, so I need to investigate that before I can help test that. I'll see how things look on Monday and either merge the commits you identified or the whole series depending on if there's any more info from Christian. cheers > On Wed, Nov 14, 2018 at 09:22:41AM +0100, Christoph Hellwig wrote: >> Powerpc has somewhat odd usage where ZONE_DMA is used for all memory on >> common 64-bit configfs, and ZONE_DMA32 is used for 31-bit schemes. >> >> Move to a scheme closer to what other architectures use (and I dare to >> say the intent of the system): >> >> - ZONE_DMA: optionally for memory < 31-bit (64-bit embedded only) >> - ZONE_NORMAL: everything addressable by the kernel >> - ZONE_HIGHMEM: memory > 32-bit for 32-bit kernels >> >> Also provide information on how ZONE_DMA is used by defining >> ARCH_ZONE_DMA_BITS. >> >> Contains various fixes from Benjamin Herrenschmidt. >> >> Signed-off-by: Christoph Hellwig >> --- >> arch/powerpc/Kconfig | 8 +--- >> arch/powerpc/include/asm/page.h | 2 + >> arch/powerpc/include/asm/pgtable.h| 1 - >> arch/powerpc/kernel/dma-swiotlb.c | 6 +-- >> arch/powerpc/kernel/dma.c | 7 +-- >> arch/powerpc/mm/mem.c | 47 +++ >> arch/powerpc/platforms/85xx/corenet_generic.c | 10 >> arch/powerpc/platforms/85xx/qemu_e500.c | 9 >> include/linux/mmzone.h| 2 +- >> 9 files changed, 25 insertions(+), 67 deletions(-) >> >> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig >> index 8be31261aec8..c3613bc1 100644 >> --- a/arch/powerpc/Kconfig >> +++ b/arch/powerpc/Kconfig >> @@ -374,9 +374,9 @@ config PPC_ADV_DEBUG_DAC_RANGE >> depends on PPC_ADV_DEBUG_REGS && 44x >> default y >> >> -config ZONE_DMA32 >> +config ZONE_DMA >> bool >> -default y if PPC64 >> +default y if PPC_BOOK3E_64 >> >> config PGTABLE_LEVELS >> int >> @@ -869,10 +869,6 @@ config ISA >>have an IBM RS/6000 or pSeries machine, say Y. If you have an >>embedded board, consult your board documentation. >> >> -config ZONE_DMA >> -bool >> -default y >> - >> config GENERIC_ISA_DMA >> bool >> depends on ISA_DMA_API >> diff --git a/arch/powerpc/include/asm/page.h >> b/arch/powerpc/include/asm/page.h >> index f6a1265face2..fc8c9ac0c6be 100644 >> --- a/arch/powerpc/include/asm/page.h >> +++ b/arch/powerpc/include/asm/page.h >> @@ -354,4 +354,6 @@ typedef struct page *pgtable_t; >> #endif /* __ASSEMBLY__ */ >> #include >> >> +#define ARCH_ZONE_DMA_BITS 31 >> + >> #endif /* _ASM_POWERPC_PAGE_H */ >> diff --git a/arch/powerpc/include/asm/pgtable.h >> b/arch/powerpc/include/asm/pgtable.h >> index 9679b7519a35..8af32ce93c7f 100644 >> --- a/arch/powerpc/include/asm/pgtable.h >> +++ b/arch/powerpc/include/asm/pgtable.h >> @@ -66,7 +66,6 @@ extern unsigned long empty_zero_page[]; >> >> extern pgd_t swapper_pg_dir[]; >> >> -void limit_zone_pfn(enum zone_type zone, unsigned long max_pfn); >> int dma_pfn_limit_to_zone(u64 pfn_limit); >> extern void paging_init(void); >> >> diff --git a/arch/powerpc/kernel/dma-swiotlb.c >> b/arch/powerpc/kernel/dma-swiotlb.c >> index 5fc335f4d9cd..678811abccfc 100644 >> --- a/arch/powerpc/kernel/dma-swiotlb.c >> +++ b/arch/powerpc/kernel/dma-swiotlb.c >> @@ -108,12 +108,8 @@ int __init swiotlb_setup_bus_notifier(void) >> >> void __init swiotlb_detect_4g(void) >> { >> -if ((memblock_end_of_DRAM() - 1) > 0x) { >> +if ((memblock_end_of_DRAM() - 1) > 0x) >> ppc_swiotlb_enable = 1; >> -#ifdef CONFIG_ZONE_DMA32 >> -limit_zone_pfn(ZONE_DMA32, (1ULL << 32) >> PAGE_SHIFT); >> -#endif >> -} >> } >> >> static int __init check_swiotlb_enabled(void) >> diff --git a/arch/powerpc/kernel/dma.c b/arch/powerpc/kernel/dma.c >> index dbfc7056d7df..6551685a4ed0 100644 >> --- a/arch/powerpc/kernel/dma.c >> +++ b/arch/powerpc/kernel/dma.c >> @@ -50,7 +50,7 @@ static int dma_nommu_dma_supported(struct device *dev, u64 >> mask) >> return 1; >> >> #ifdef CONFIG_FSL_SOC >> -/* Freescale gets another chance via ZONE_DMA/ZONE_DMA32, however >> +/* Freescale gets another chance via ZONE_DMA, however >> * that will have to be refined if/when they support iommus >> */ >> return 1; >> @@ -94,13 +94,10 @@ void *__dma_nommu_alloc_coherent(struct device *dev, >> size_t size, >> } >> >> switch (zone) { >> +#ifdef CONFIG_ZONE_DMA >> case ZONE_DMA: >> flag |= GFP_DMA; >>
Re: use generic DMA mapping code in powerpc V4
Christoph Hellwig writes: > Any comments? I'd like to at least get the ball moving on the easy > bits. Nothing specific yet. I'm a bit worried it might break one of the many old obscure platforms we have that aren't well tested. There's not much we can do about that, but I'll just try and test it on everything I can find. Is the plan that you take these via the dma-mapping tree or that they go via powerpc? cheers > On Wed, Nov 14, 2018 at 09:22:40AM +0100, Christoph Hellwig wrote: >> Hi all, >> >> this series switches the powerpc port to use the generic swiotlb and >> noncoherent dma ops, and to use more generic code for the coherent >> direct mapping, as well as removing a lot of dead code. >> >> As this series is very large and depends on the dma-mapping tree I've >> also published a git tree: >> >> git://git.infradead.org/users/hch/misc.git powerpc-dma.4 >> >> Gitweb: >> >> >> http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/powerpc-dma.4 >> >> Changes since v3: >> - rebase on the powerpc fixes tree >> - add a new patch to actually make the baseline amigaone config >>configure without warnings >> - only use ZONE_DMA for 64-bit embedded CPUs, on pseries an IOMMU is >>always present >> - fix compile in mem.c for one configuration >> - drop the full npu removal for now, will be resent separately >> - a few git bisection fixes >> >> The changes since v1 are to big to list and v2 was not posted in public. >> >> ___ >> iommu mailing list >> iommu@lists.linux-foundation.org >> https://lists.linuxfoundation.org/mailman/listinfo/iommu > ---end quoted text--- ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH V2] mm: Replace all open encodings for NUMA_NO_NODE
Anshuman Khandual writes: > At present there are multiple places where invalid node number is encoded > as -1. Even though implicitly understood it is always better to have macros > in there. Replace these open encodings for an invalid node number with the > global macro NUMA_NO_NODE. This helps remove NUMA related assumptions like > 'invalid node' from various places redirecting them to a common definition. > > Signed-off-by: Anshuman Khandual > --- > Changes in V2: > > - Added inclusion of 'numa.h' header at various places per Andrew > - Updated 'dev_to_node' to use NUMA_NO_NODE instead per Vinod > > Changes in V1: (https://lkml.org/lkml/2018/11/23/485) > > - Dropped OCFS2 changes per Joseph > - Dropped media/video drivers changes per Hans > > RFC - https://patchwork.kernel.org/patch/10678035/ > > Build tested this with multiple cross compiler options like alpha, sparc, > arm64, x86, powerpc, powerpc64le etc with their default config which might > not have compiled tested all driver related changes. I will appreciate > folks giving this a test in their respective build environment. > > All these places for replacement were found by running the following grep > patterns on the entire kernel code. Please let me know if this might have > missed some instances. This might also have replaced some false positives. > I will appreciate suggestions, inputs and review. > > 1. git grep "nid == -1" > 2. git grep "node == -1" > 3. git grep "nid = -1" > 4. git grep "node = -1" > > arch/alpha/include/asm/topology.h | 3 ++- > arch/ia64/kernel/numa.c | 2 +- > arch/ia64/mm/discontig.c | 6 +++--- > arch/ia64/sn/kernel/io_common.c | 3 ++- > arch/powerpc/include/asm/pci-bridge.h | 3 ++- > arch/powerpc/kernel/paca.c| 3 ++- > arch/powerpc/kernel/pci-common.c | 3 ++- > arch/powerpc/mm/numa.c| 14 +++--- > arch/powerpc/platforms/powernv/memtrace.c | 5 +++-- These powerpc changes all look fine. Acked-by: Michael Ellerman cheers ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 4/4] dma-mapping: clear dev->dma_ops in arch_teardown_dma_ops
Christoph Hellwig writes: > Looking at the code I think this commit is simply broken for > architectures not using arch_setup_dma_ops, but instead setting up > the dma ops through arch specific magic. I arch_setup_dma_ops() called from of_dma_configure(), but pci_dma_configure() doesn't call that for this device: static int pci_dma_configure(struct device *dev) { ... if (IS_ENABLED(CONFIG_OF) && bridge->parent && bridge->parent->of_node) { ret = of_dma_configure(dev, bridge->parent->of_node, true); bridge->parent is NULL. So I don't think arch_setup_dma_ops() would help here? > I'll revert the patch. Thanks. cheers ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 4/4] dma-mapping: clear dev->dma_ops in arch_teardown_dma_ops
Guenter Roeck writes: > Hi, > > On Mon, Aug 27, 2018 at 10:47:11AM +0200, Christoph Hellwig wrote: >> There is no reason to leave the per-device dma_ops around when >> deconfiguring a device, so move this code from arm64 into the >> common code. >> Signed-off-by: Christoph Hellwig >> Reviewed-by: Robin Murphy > > This patch causes various ppc images to crash in -next due to NULL > DMA ops in dma_alloc_attrs(). I finally remembered where you autobuilder is :) https://kerneltests.org/builders/qemu-ppc-next/builds/970/steps/qemubuildcommand_1/logs/stdio Looks like mac99 at least is failing. Just reproduced it on my setup. > Looking into the code, the macio driver tries to instantiate on > the :f0:0d.0 PCI address (the driver maps to all Apple PCI IDs) > and fails. This results in a call to arch_teardown_dma_ops(). Later, > the same device pointer is used to instantiate ohci-pci, which > crashes in probe because the dma_ops pointer has been cleared. > > I don't claim to fully understand the code, but to me it looks like > the pci device is allocated and waiting for a driver to attach to. > See arch/powerpc/kernel/pci_of_scan.c:of_create_pci_dev(). > If attaching a driver (here: macio) fails, the same device pointer > is then reused for the next matching driver until a match is found > and the device is successfully instantiated. Of course this fails > quite badly if the device pointer has been scrubbed after the first > failure. > > I don't know if this is generic PCI behavior or ppc specific. > I am copying pci and ppc maintainers for additional input. I would guess this is some PPC special sauce O_o Will have a look. cheers ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: powerpc: do not redefined NEED_DMA_MAP_STATE
On Mon, 2018-07-30 at 07:37:21 UTC, Christoph Hellwig wrote: > kernel/dma/Kconfig already defines NEED_DMA_MAP_STATE, just select it > from PPC64 and NOT_COHERENT_CACHE instead. > > Signed-off-by: Christoph Hellwig Applied to powerpc next, thanks. https://git.kernel.org/powerpc/c/06832fc004815b4b43628d21fc8171 cheers ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] powerpc: do not redefined NEED_DMA_MAP_STATE
Christoph Hellwig writes: > kernel/dma/Kconfig already defines NEED_DMA_MAP_STATE, just select it > from PPC64 and NOT_COHERENT_CACHE instead. > > Signed-off-by: Christoph Hellwig > --- > arch/powerpc/Kconfig | 3 --- > arch/powerpc/platforms/Kconfig.cputype | 2 ++ > 2 files changed, 2 insertions(+), 3 deletions(-) Thanks. I did this instead: commit 870771ae76010c5e42ee8e0278f5823e46e96e3f (HEAD -> next-test) Author: Christoph Hellwig AuthorDate: Mon Jul 30 09:37:21 2018 +0200 Commit: Michael Ellerman CommitDate: Tue Jul 31 20:43:57 2018 +1000 powerpc: Do not redefine NEED_DMA_MAP_STATE kernel/dma/Kconfig already defines NEED_DMA_MAP_STATE, just select it from CONFIG_PPC using the same condition as an if guard. Signed-off-by: Christoph Hellwig [mpe: Move it under PPC] Signed-off-by: Michael Ellerman diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index 5eb4d969afbf..ee38fce075ee 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -226,6 +226,7 @@ config PPC select IRQ_DOMAIN select IRQ_FORCED_THREADING select MODULES_USE_ELF_RELA + select NEED_DMA_MAP_STATE if PPC64 || NOT_COHERENT_CACHE select NEED_SG_DMA_LENGTH select NO_BOOTMEM select OF @@ -885,9 +886,6 @@ config ZONE_DMA bool default y -config NEED_DMA_MAP_STATE - def_bool (PPC64 || NOT_COHERENT_CACHE) - config GENERIC_ISA_DMA bool depends on ISA_DMA_API cheers ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] PCI: call dma_debug_add_bus for pci_bus_type in common code
Christoph Hellwig writes: > There is nothing arch specific about PCI or dma-debug, so move this > call to common code just after registering the bus type. > > Signed-off-by: Christoph Hellwig > --- > arch/powerpc/kernel/dma.c | 3 --- > arch/sh/drivers/pci/pci.c | 2 -- > arch/x86/kernel/pci-dma.c | 3 --- > drivers/pci/pci-driver.c | 2 +- > 4 files changed, 1 insertion(+), 9 deletions(-) > > diff --git a/arch/powerpc/kernel/dma.c b/arch/powerpc/kernel/dma.c > index 155170d70324..dbfc7056d7df 100644 > --- a/arch/powerpc/kernel/dma.c > +++ b/arch/powerpc/kernel/dma.c > @@ -357,9 +357,6 @@ EXPORT_SYMBOL_GPL(dma_get_required_mask); > > static int __init dma_init(void) > { > -#ifdef CONFIG_PCI > - dma_debug_add_bus(&pci_bus_type); > -#endif > #ifdef CONFIG_IBMVIO > dma_debug_add_bus(&vio_bus_type); > #endif Acked-by: Michael Ellerman (powerpc) cheers ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 01/12] iommu-common: move to arch/sparc
Anshuman Khandual writes: > On 04/16/2018 07:28 PM, David Miller wrote: >> From: Anshuman Khandual >> Date: Mon, 16 Apr 2018 14:26:07 +0530 >> >>> On 04/15/2018 08:29 PM, Christoph Hellwig wrote: This code is only used by sparc, and all new iommu drivers should use the drivers/iommu/ framework. Also remove the unused exports. Signed-off-by: Christoph Hellwig >>> >>> Right, these functions are used only from SPARC architecture. Simple >>> git grep confirms it as well. Hence it makes sense to move them into >>> arch code instead. >> >> Well, we put these into a common location and used type friendly for >> powerpc because we hoped powerpc would convert over to using this >> common piece of code as well. >> >> But nobody did the powerpc work. Sorry. >> If you look at the powerpc iommu support, it's the same code basically >> for entry allocation. > > I understand. But there are some differences in iommu_table structure, > how both regular and large IOMMU pools are being initialized etc. So > if the movement of code into SPARC help cleaning up these generic config > options in general, I guess we should do that. But I will leave it upto > others who have more experience in this area. > > +mpe This is the first I've heard of it, I guess it's probably somewhere on Ben's append-only TODO list. Some of the code does look very similar, but not 100%. So someone would need to do some work to reconcile the two and test the result. TBH I doubt we're going to get around to it any time soon. Unless we have a volunteer? cheers ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] fix double ;;s in code
Daniel Vetter writes: > On Sun, Feb 18, 2018 at 11:00:56AM +0100, Christophe LEROY wrote: >> Le 17/02/2018 à 22:19, Pavel Machek a écrit : >> > >> > Fix double ;;'s in code. >> > >> > Signed-off-by: Pavel Machek >> >> A summary of the files modified on top of the patch would help understand >> the impact. >> >> A maybe there should be one patch by area, eg one for each arch specific >> modif and one for drivers/ and one for block/ ? > > Yeah, pls split this into one patch per area, with a suitable patch > subject prefix. Look at git log of each file to get a feeling for what's > the standard in each area. This part is actually pretty annoying. I hacked up a script (below) which seems to do a reasonable job in most cases. For this patch it gives: $ for f in $(git ls-files -m); do ./guess-prefix.py $f; done ARC: ARC: ARM: arm64: ptrace: KVM: PPC: powerpc/powernv: x86/efi: block/sed-opal: clocksource: mips-gic: clocksource/drivers/sun5i: drm/amd/display: drm/amd/powerplay: drm/msm/mdp5: drm: iommu/vt-d: md: soc: imx: gpc: I think those are correct except for: - "drm:" for "drivers/gpu/drm/scheduler" which has only a single commit. - "md:" for "drivers/md/raid1.c" which is tricked by inconsistent usage of "md: raid1:" and "md/raid1:". But that seems like a reasonable hit rate. Another approach would be to have a file that defines for each subsystem what the preferred style is, but that is likely to be a PITA to maintain. cheers #!/usr/bin/python3 import sys import re from subprocess import check_output from collections import Counter if len(sys.argv) != 2: print('Usage: %s ' % sys.argv[0], file=sys.stderr) sys.exit(1) fname = sys.argv[1] cmd = ['git', 'log', '--format=%s', '-n', '100', fname] output = check_output(cmd).decode('utf-8') ignore = ['License', 'Merge'] # Ordered list of patterns patterns = [ # Common style "foo/bar/baz: Fix the foo" re.compile('^([\w\-_]+: )+'), # Less common "foo bar baz: Fix the foo" re.compile('^([\w\-_]+:? )+: ') ] words = [] for line in output.splitlines(): prefix = line.split()[0] for patt in patterns: match = patt.search(line) if match: prefix = match.group(0) break if prefix in ignore: continue words.append(prefix) # Warn if we didn't find many examples if len(words) < 5: print("Warning: only found %d previous commits to guess from for" % len(words), fname, file=sys.stderr) counts = Counter(words) print(counts.most_common(1)[0][0]) ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] headers: untangle kmemleak.h from mm.h
Randy Dunlap writes: > On 02/12/2018 04:28 AM, Michael Ellerman wrote: >> Randy Dunlap writes: >> >>> From: Randy Dunlap >>> >>> Currently #includes for no obvious >>> reason. It looks like it's only a convenience, so remove kmemleak.h >>> from slab.h and add to any users of kmemleak_* >>> that don't already #include it. >>> Also remove from source files that do not use it. >>> >>> This is tested on i386 allmodconfig and x86_64 allmodconfig. It >>> would be good to run it through the 0day bot for other $ARCHes. >>> I have neither the horsepower nor the storage space for the other >>> $ARCHes. >>> >>> [slab.h is the second most used header file after module.h; kernel.h >>> is right there with slab.h. There could be some minor error in the >>> counting due to some #includes having comments after them and I >>> didn't combine all of those.] >>> >>> This is Lingchi patch #1 (death by a thousand cuts, applied to kernel >>> header files). >>> >>> Signed-off-by: Randy Dunlap >> >> I threw it at a random selection of configs and so far the only failures >> I'm seeing are: >> >> lib/test_firmware.c:134:2: error: implicit declaration of function 'vfree' >> [-Werror=implicit-function-declaration] >> >> lib/test_firmware.c:620:25: error: implicit declaration of function >> 'vzalloc' [-Werror=implicit-function-declaration] >> lib/test_firmware.c:620:2: error: implicit declaration of function >> 'vzalloc' [-Werror=implicit-function-declaration] >> security/integrity/digsig.c:146:2: error: implicit declaration of function >> 'vfree' [-Werror=implicit-function-declaration] > > Both of those source files need to #include . Yep, I added those and rebuilt. I don't see any more failures that look related to your patch. http://kisskb.ellerman.id.au/kisskb/head/13399/ I haven't gone through the defconfigs I have enabled for a while, so it's possible I have some missing but it's still a reasonable cross section. cheers ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] headers: untangle kmemleak.h from mm.h
Randy Dunlap writes: > From: Randy Dunlap > > Currently #includes for no obvious > reason. It looks like it's only a convenience, so remove kmemleak.h > from slab.h and add to any users of kmemleak_* > that don't already #include it. > Also remove from source files that do not use it. > > This is tested on i386 allmodconfig and x86_64 allmodconfig. It > would be good to run it through the 0day bot for other $ARCHes. > I have neither the horsepower nor the storage space for the other > $ARCHes. > > [slab.h is the second most used header file after module.h; kernel.h > is right there with slab.h. There could be some minor error in the > counting due to some #includes having comments after them and I > didn't combine all of those.] > > This is Lingchi patch #1 (death by a thousand cuts, applied to kernel > header files). > > Signed-off-by: Randy Dunlap I threw it at a random selection of configs and so far the only failures I'm seeing are: lib/test_firmware.c:134:2: error: implicit declaration of function 'vfree' [-Werror=implicit-function-declaration] lib/test_firmware.c:620:25: error: implicit declaration of function 'vzalloc' [-Werror=implicit-function-declaration] lib/test_firmware.c:620:2: error: implicit declaration of function 'vzalloc' [-Werror=implicit-function-declaration] security/integrity/digsig.c:146:2: error: implicit declaration of function 'vfree' [-Werror=implicit-function-declaration] Full results trickling in here, not all the failures there are caused by this patch, ie. some configs are broken in mainline: http://kisskb.ellerman.id.au/kisskb/head/13396/ cheers ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 16/67] powerpc: rename dma_direct_ to dma_nommu_
Geert Uytterhoeven writes: > On Tue, Jan 2, 2018 at 10:45 AM, Michael Ellerman wrote: >> Christoph Hellwig writes: >> >>> We want to use the dma_direct_ namespace for a generic implementation, >>> so rename powerpc to the second best choice: dma_nommu_. >> >> I'm not a fan of "nommu". Some of the users of direct ops *are* using an >> IOMMU, they're just setting up a 1:1 mapping once at init time, rather >> than mapping dynamically. >> >> Though I don't have a good idea for a better name, maybe "1to1", >> "linear", "premapped" ? > > "identity"? I think that would be wrong, but thanks for trying to help :) The address on the device side is sometimes (often?) offset from the CPU address. So eg. the device can DMA to RAM address 0x0 using address 0x800. Identity would imply 0 == 0 etc. I think "bijective" is the correct term, but that's probably a bit esoteric. cheers ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 16/67] powerpc: rename dma_direct_ to dma_nommu_
Christoph Hellwig writes: > We want to use the dma_direct_ namespace for a generic implementation, > so rename powerpc to the second best choice: dma_nommu_. I'm not a fan of "nommu". Some of the users of direct ops *are* using an IOMMU, they're just setting up a 1:1 mapping once at init time, rather than mapping dynamically. Though I don't have a good idea for a better name, maybe "1to1", "linear", "premapped" ? cheers ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 4/4] iommu/pamu: Add support for generic iommu-device
Joerg Roedel writes: > Hi Michael, > > On Thu, Aug 24, 2017 at 12:04:13PM +1000, Michael Ellerman wrote: >> Joerg Roedel writes: >> > Can you please try the attached patch? >> >> Thanks, that works. > > Great, thanks for testing it. I'll queue it on-top of the original > patch-set. Thanks. cheers ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 4/4] iommu/pamu: Add support for generic iommu-device
Joerg Roedel writes: > Hello Michael, > > On Wed, Aug 23, 2017 at 10:17:39PM +1000, Michael Ellerman wrote: > >> [0.358192] Call Trace: >> [0.360624] [c000f7173680] [c000f7173710] 0xc000f7173710 >> (unreliable) >> [0.368088] [c000f7173710] [c02abf7c] >> .kernfs_find_and_get_ns+0x4c/0x7c >> [0.375729] [c000f71737a0] [c02b13c8] >> .sysfs_add_link_to_group+0x44/0x9c >> [0.383456] [c000f7173840] [c0591660] >> .iommu_device_link+0x70/0x144 >> [0.390744] [c000f71738e0] [c05942dc] >> .fsl_pamu_add_device+0x4c/0x80 >> [0.398121] [c000f7173960] [c058ce8c] >> .add_iommu_group+0x5c/0x9c >> [0.405153] [c000f71739e0] [c059d6b8] >> .bus_for_each_dev+0x98/0xfc >> [0.412269] [c000f7173a80] [c058f1a0] >> .bus_set_iommu+0xd8/0x11c >> [0.419218] [c000f7173b20] [c0d86998] >> .pamu_domain_init+0xb0/0xe0 >> [0.426331] [c000f7173ba0] [c0d86864] >> .fsl_pamu_init+0xec/0x170 >> [0.433276] [c000f7173c30] [c0001934] >> .do_one_initcall+0x68/0x1b8 >> [0.440395] [c000f7173d00] [c0d440e4] >> .kernel_init_freeable+0x24c/0x324 >> [0.448031] [c000f7173db0] [c0001b90] .kernel_init+0x20/0x140 >> [0.454801] [c000f7173e30] [c9bc] >> .ret_from_kernel_thread+0x58/0x9c >> [0.462438] Instruction dump: >> [0.465390] 7c0802a6 fb81ffe0 f8010010 fba1ffe8 fbc1fff0 fbe1fff8 >> f821ff71 7c7e1b78 >> [0.473114] 7cbd2b78 7c9c2378 6000 6000 311d >> ebfe0048 552906b4 >> [0.481017] ---[ end trace 5d11d3e6c29380bd ]--- > > Thanks for the report, this looks like an initialization ordering > problem. Yes I suspect so. > Can you please try the attached patch? Thanks, that works. It boots happily, much later in boot I see these messages: [2.085616] iommu: Adding device ffe301000.jr to group 9 [2.091101] iommu: Adding device ffe302000.jr to group 10 [2.096633] iommu: Adding device ffe303000.jr to group 11 [2.102161] iommu: Adding device ffe304000.jr to group 12 In /sys I see: root@p5020ds:~# ls -l /sys/class/iommu/iommu0 lrwxrwxrwx 1 root root 0 Aug 24 12:01 /sys/class/iommu/iommu0 -> ../../devices/virtual/iommu/iommu0 And: root@p5020ds:~# ls -l /sys/devices/virtual/iommu/iommu0/devices/ total 0 lrwxrwxrwx 1 root root 0 Aug 24 12:02 :00:00.0 -> ../../../../platform/ffe20.pcie/pci:00/:00:00.0 lrwxrwxrwx 1 root root 0 Aug 24 12:02 2000:00:00.0 -> ../../../../platform/ffe202000.pcie/pci2000:00/2000:00:00.0 lrwxrwxrwx 1 root root 0 Aug 24 12:02 2000:01:00.0 -> ../../../../platform/ffe202000.pcie/pci2000:00/2000:00:00.0/2000:01:00.0 lrwxrwxrwx 1 root root 0 Aug 24 12:02 2000:01:00.1 -> ../../../../platform/ffe202000.pcie/pci2000:00/2000:00:00.0/2000:01:00.1 lrwxrwxrwx 1 root root 0 Aug 24 12:02 ffe100300.dma -> ../../../../platform/ffe00.soc/ffe100300.dma lrwxrwxrwx 1 root root 0 Aug 24 12:02 ffe101300.dma -> ../../../../platform/ffe00.soc/ffe101300.dma lrwxrwxrwx 1 root root 0 Aug 24 12:02 ffe114000.sdhc -> ../../../../platform/ffe00.soc/ffe114000.sdhc lrwxrwxrwx 1 root root 0 Aug 24 12:02 ffe20.pcie -> ../../../../platform/ffe20.pcie lrwxrwxrwx 1 root root 0 Aug 24 12:02 ffe202000.pcie -> ../../../../platform/ffe202000.pcie lrwxrwxrwx 1 root root 0 Aug 24 12:02 ffe21.usb -> ../../../../platform/ffe00.soc/ffe21.usb lrwxrwxrwx 1 root root 0 Aug 24 12:02 ffe211000.usb -> ../../../../platform/ffe00.soc/ffe211000.usb lrwxrwxrwx 1 root root 0 Aug 24 12:02 ffe22.sata -> ../../../../platform/ffe00.soc/ffe22.sata lrwxrwxrwx 1 root root 0 Aug 24 12:02 ffe221000.sata -> ../../../../platform/ffe00.soc/ffe221000.sata lrwxrwxrwx 1 root root 0 Aug 24 12:02 ffe301000.jr -> ../../../../platform/ffe00.soc/ffe30.crypto/ffe301000.jr lrwxrwxrwx 1 root root 0 Aug 24 12:02 ffe302000.jr -> ../../../../platform/ffe00.soc/ffe30.crypto/ffe302000.jr lrwxrwxrwx 1 root root 0 Aug 24 12:02 ffe303000.jr -> ../../../../platform/ffe00.soc/ffe30.crypto/ffe303000.jr lrwxrwxrwx 1 root root 0 Aug 24 12:02 ffe304000.jr -> ../../../../platform/ffe00.soc/ffe30.crypto/ffe304000.jr Which seems like it's working? cheers ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 4/4] iommu/pamu: Add support for generic iommu-device
Joerg Roedel writes: > From: Joerg Roedel > > This patch adds a global iommu-handle to the pamu driver and > initializes it at probe time. Also link devices added to the > iommu to this handle. > > Signed-off-by: Joerg Roedel > --- > drivers/iommu/fsl_pamu.c| 17 + > drivers/iommu/fsl_pamu.h| 3 +++ > drivers/iommu/fsl_pamu_domain.c | 5 - > drivers/iommu/fsl_pamu_domain.h | 2 ++ > 4 files changed, 26 insertions(+), 1 deletion(-) This seems to be causing my p5020ds to blow up with: [0.096728] Machine: fsl,P5020DS [0.096729] SoC family: QorIQ [0.096730] SoC ID: svr:0x82280020, Revision: 2.0 [0.098143] Found FSL PCI host bridge at 0x000ffe20. Firmware bus number: 0->0 [0.098145] PCI host bridge /pcie@ffe20 ranges: [0.098149] MEM 0x000c..0x000c1fff -> 0xe000 [0.098151] IO 0x000ff800..0x000ff800 -> 0x [0.098161] /pcie@ffe20: PCICSRBAR @ 0xdf00 [0.098162] setup_pci_atmu: end of DRAM 1 [0.098167] /pcie@ffe20: Setup 64-bit PCI DMA window [0.098168] /pcie@ffe20: DMA window size is 0xdf00 [0.098326] Found FSL PCI host bridge at 0x000ffe202000. Firmware bus number: 0->1 [0.098327] PCI host bridge /pcie@ffe202000 ranges: [0.098330] MEM 0x000c4000..0x000c5fff -> 0xe000 [0.098332] IO 0x000ff802..0x000ff802 -> 0x [0.098340] /pcie@ffe202000: PCICSRBAR @ 0xdf00 [0.098340] setup_pci_atmu: end of DRAM 1 [0.098345] /pcie@ffe202000: Setup 64-bit PCI DMA window [0.098346] /pcie@ffe202000: DMA window size is 0xdf00 [0.204289] iommu: Adding device ffe100300.dma to group 0 [0.209606] Unable to handle kernel paging request for data at address 0x0068 [0.217059] Faulting instruction address: 0xc02abe18 [0.222703] Oops: Kernel access of bad area, sig: 11 [#1] [0.228078] SMP NR_CPUS=24 [0.228080] CoreNet Generic [0.233634] Modules linked in: [0.236674] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.13.0-rc5-gcc-6.3.1-4-g68a17f0be6fe #41 [0.245613] task: c000f7168000 task.stack: c000f717 [0.251515] NIP: c02abe18 LR: c02abf7c CTR: c0025dc8 [0.258545] REGS: c000f7173400 TRAP: 0300 Not tainted (4.13.0-rc5-gcc-6.3.1-4-g68a17f0be6fe) [0.267919] MSR: 80029000 [0.267924] CR: 28adb242 XER: 2000 [0.276165] DEAR: 0068 ESR: SOFTE: 1 [0.276165] GPR00: c02abf7c c000f7173680 c0fbf600 [0.276165] GPR04: c0c6dc88 c000f72e0190 [0.276165] GPR08: c000f7168000 0038 [0.276165] GPR12: 22adb444 c0003fff5000 c0001b70 [0.276165] GPR16: [0.276165] GPR20: [0.276165] GPR24: c0ef4f58 c0d3b408 c0c6dc88 [0.276165] GPR28: c0c6dc88 c0e3b658 [0.346218] NIP [c02abe18] .kernfs_find_ns+0x30/0x148 [0.351943] LR [c02abf7c] .kernfs_find_and_get_ns+0x4c/0x7c [0.358192] Call Trace: [0.360624] [c000f7173680] [c000f7173710] 0xc000f7173710 (unreliable) [0.368088] [c000f7173710] [c02abf7c] .kernfs_find_and_get_ns+0x4c/0x7c [0.375729] [c000f71737a0] [c02b13c8] .sysfs_add_link_to_group+0x44/0x9c [0.383456] [c000f7173840] [c0591660] .iommu_device_link+0x70/0x144 [0.390744] [c000f71738e0] [c05942dc] .fsl_pamu_add_device+0x4c/0x80 [0.398121] [c000f7173960] [c058ce8c] .add_iommu_group+0x5c/0x9c [0.405153] [c000f71739e0] [c059d6b8] .bus_for_each_dev+0x98/0xfc [0.412269] [c000f7173a80] [c058f1a0] .bus_set_iommu+0xd8/0x11c [0.419218] [c000f7173b20] [c0d86998] .pamu_domain_init+0xb0/0xe0 [0.426331] [c000f7173ba0] [c0d86864] .fsl_pamu_init+0xec/0x170 [0.433276] [c000f7173c30] [c0001934] .do_one_initcall+0x68/0x1b8 [0.440395] [c000f7173d00] [c0d440e4] .kernel_init_freeable+0x24c/0x324 [0.448031] [c000f7173db0] [c0001b90] .kernel_init+0x20/0x140 [0.454801] [c000f7173e30] [c9bc] .ret_from_kernel_thread+0x58/0x9c [0.462438] Instruction dump: [0.465390] 7c0802a6 fb81ffe0 f8010010 fba1ffe8 fbc1fff0 fbe1fff8 f821ff71 7c7e1b78 [0.473114] 7cbd2b78 7c9c2378 6000 6000 311d ebfe0048 552906b4 [0.481017] ---[ end trace 5d11d3e6c29380bd ]--- # first bad commit: [68a17f0be6feb8de1f5e26b93f49791031374c4
Re: [PATCH 21/44] powerpc: implement ->mapping_error
Christoph Hellwig writes: > DMA_ERROR_CODE is going to go away, so don't rely on it. Instead > define a ->mapping_error method for all IOMMU based dma operation > instances. The direct ops don't ever return an error and don't > need a ->mapping_error method. > > Signed-off-by: Christoph Hellwig > --- > arch/powerpc/include/asm/dma-mapping.h | 4 > arch/powerpc/include/asm/iommu.h | 4 > arch/powerpc/kernel/dma-iommu.c| 6 ++ > arch/powerpc/kernel/iommu.c| 28 ++-- > arch/powerpc/platforms/cell/iommu.c| 1 + > arch/powerpc/platforms/pseries/vio.c | 3 ++- > 6 files changed, 27 insertions(+), 19 deletions(-) I also see: arch/powerpc/kernel/dma.c:const struct dma_map_ops dma_direct_ops = { Which you mentioned can't fail. arch/powerpc/platforms/pseries/ibmebus.c:static const struct dma_map_ops ibmebus_dma_ops = { Which can't fail. And: arch/powerpc/platforms/powernv/npu-dma.c:static const struct dma_map_ops dma_npu_ops = { arch/powerpc/platforms/ps3/system-bus.c:static const struct dma_map_ops ps3_sb_dma_ops = { arch/powerpc/platforms/ps3/system-bus.c:static const struct dma_map_ops ps3_ioc0_dma_ops = { All of which look like they definitely can fail, but return 0 on error and don't implement ->mapping_error. So I guess I'm acking this and adding a TODO to fix up the NPU code at least, the ps3 code is probably better left alone these days. Acked-by: Michael Ellerman cheers ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] powerpc: Fix incorrect PPC32 PAMU dependency
On Thu, 2016-02-04 at 20:16 -0600, Andy Fleming wrote: > The Freescale PAMU can also be enabled on 64-bit power > chips. Commit 477ab7a19cec8409e4e2dd10e7348e4cac3c06e5 > (iommu: Make more drivers depend on COMPILE_TEST) > added this false dependency. Fixed it by allowing PPC64, too. > > Signed-off-by: Andy Fleming > --- > drivers/iommu/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig > index a1e75cb..63ec7ae 100644 > --- a/drivers/iommu/Kconfig > +++ b/drivers/iommu/Kconfig > @@ -57,7 +57,7 @@ config IOMMU_DMA > > config FSL_PAMU > bool "Freescale IOMMU support" > - depends on PPC32 > + depends on PPC32 || PPC64 That's == PPC. > depends on PPC_E500MC || COMPILE_TEST But then you have that ^ Multiple depends are joined with &&, so you get: depends on PPC && (PPC_E500MC || COMPILE_TEST) PPC_E500MC depends (indirectly) on PPC, so I think it would be clearer as: depends on PPC_E500MC || (COMPILE_TEST && PPC) Which I /think/ matches the original intention? cheers ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [v7,58/60] PCI: Introduce resource_disabled()
s_mem_pref_64(res->flags)) > @@ -905,7 +905,7 @@ static int pnv_pci_vf_resource_shift(struct pci_dev *dev, > int offset) >*/ > for (i = 0; i < PCI_SRIOV_NUM_BARS; i++) { > res = &dev->resource[i + PCI_IOV_RESOURCES]; > - if (!res->flags || !res->parent) > + if (resource_disabled(res) || !res->parent) > continue; > > if (!pnv_pci_is_mem_pref_64(res->flags)) > @@ -1188,7 +1188,7 @@ static int pnv_pci_vf_assign_m64(struct pci_dev *pdev, > u16 num_vfs) > > for (i = 0; i < PCI_SRIOV_NUM_BARS; i++) { > res = &pdev->resource[i + PCI_IOV_RESOURCES]; > - if (!res->flags || !res->parent) > + if (resource_disabled(res) || !res->parent) > continue; > > if (!pnv_pci_is_mem_pref_64(res->flags)) > @@ -2757,7 +2757,7 @@ static void pnv_pci_ioda_fixup_iov_resources(struct > pci_dev *pdev) > > for (i = 0; i < PCI_SRIOV_NUM_BARS; i++) { > res = &pdev->resource[i + PCI_IOV_RESOURCES]; > - if (!res->flags || res->parent) > + if (resource_disabled(res) || res->parent) > continue; > if (!pnv_pci_is_mem_pref_64(res->flags)) { > dev_warn(&pdev->dev, " non M64 VF BAR%d: %pR\n", > @@ -2779,7 +2779,7 @@ static void pnv_pci_ioda_fixup_iov_resources(struct > pci_dev *pdev) > > for (i = 0; i < PCI_SRIOV_NUM_BARS; i++) { > res = &pdev->resource[i + PCI_IOV_RESOURCES]; > - if (!res->flags || res->parent) > + if (resource_disabled(res) || res->parent) > continue; > if (!pnv_pci_is_mem_pref_64(res->flags)) { > dev_warn(&pdev->dev, "Skipping expanding VF BAR%d: > %pR\n", > @@ -2820,7 +2820,7 @@ static void pnv_ioda_setup_pe_seg(struct pci_controller > *hose, > BUG_ON(!(pe->flags & (PNV_IODA_PE_BUS | PNV_IODA_PE_BUS_ALL))); > > pci_bus_for_each_resource(pe->pbus, res, i) { > - if (!res || !res->flags || > + if (!res || resource_disabled(res) || > res->start > res->end) > continue; Those conversions all look correct to me. Acked-by: Michael Ellerman (powerpc) cheers ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 21/27] Powerpc/MSI: Use MSI chip framework to configure MSI/MSI-X irq
On Wed, 2014-10-15 at 11:07 +0800, Yijing Wang wrote: > Use MSI chip framework instead of arch MSI functions to configure > MSI/MSI-X irq. So we can manage MSI/MSI-X irq in a unified framework. > > Signed-off-by: Yijing Wang > --- > Hi Michael, >I dropped the Acked-by , because this version has a > lot changes compared to last. So, I guess you may want to check it again. OK thanks. Still looks OK and boots on one of my test systems that uses MSI. Acked-by: Michael Ellerman (for powerpc) cheers ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v1 15/21] Powerpc/MSI: Use MSI chip framework to configure MSI/MSI-X irq
On Fri, 2014-09-05 at 18:10 +0800, Yijing Wang wrote: > Use MSI chip framework instead of arch MSI functions to configure > MSI/MSI-X irq. So we can manage MSI/MSI-X irq in a unified framework. > > Signed-off-by: Yijing Wang This looks fine and seems to boot OK. Acked-by: Michael Ellerman (powerpc) cheers ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/3] PCI/MSI: Add pci_enable_msi_partial()
On Wed, 2014-07-02 at 14:22 -0600, Bjorn Helgaas wrote: > On Tue, Jun 10, 2014 at 03:10:30PM +0200, Alexander Gordeev wrote: > > There are PCI devices that require a particular value written > > to the Multiple Message Enable (MME) register while aligned on > > power of 2 boundary value of actually used MSI vectors 'nvec' > > is a lesser of that MME value: > > > > roundup_pow_of_two(nvec) < 'Multiple Message Enable' > > > > However the existing pci_enable_msi_block() interface is not > > able to configure such devices, since the value written to the > > MME register is calculated from the number of requested MSIs > > 'nvec': > > > > 'Multiple Message Enable' = roundup_pow_of_two(nvec) > > For MSI, software learns how many vectors a device requests by reading > the Multiple Message Capable (MMC) field. This field is encoded, so a > device can only request 1, 2, 4, 8, etc., vectors. It's impossible > for a device to request 3 vectors; it would have to round up that up > to a power of two and request 4 vectors. > > Software writes similarly encoded values to MME to tell the device how > many vectors have been allocated for its use. For example, it's > impossible to tell the device that it can use 3 vectors; the OS has to > round that up and tell the device it can use 4 vectors. > > So if I understand correctly, the point of this series is to take > advantage of device-specific knowledge, e.g., the device requests 4 > vectors via MMC, but we "know" the device is only capable of using 3. > Moreover, we tell the device via MME that 4 vectors are available, but > we've only actually set up 3 of them. > > This makes me uneasy because we're lying to the device, and the device > is perfectly within spec to use all 4 of those vectors. If anything > changes the number of vectors the device uses (new device revision, > firmware upgrade, etc.), this is liable to break. It also adds more complexity into the already complex MSI API, across all architectures, all so a single Intel chipset can save a couple of MSIs. That seems like the wrong trade off to me. cheers ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu