Re: [PATCH 6/6] arch/*/: remove CONFIG_VIRT_TO_BUS

2022-06-07 Thread Michael Ellerman
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

2021-10-01 Thread Michael Ellerman
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()

2021-09-16 Thread Michael Ellerman
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()

2021-09-14 Thread Michael Ellerman
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()

2021-08-17 Thread Michael Ellerman
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()

2021-08-02 Thread Michael Ellerman
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

2020-10-29 Thread Michael Ellerman
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

2020-10-29 Thread Michael Ellerman
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

2020-10-28 Thread Michael Ellerman
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

2020-09-17 Thread Michael Ellerman
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

2020-08-19 Thread Michael Ellerman
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()

2020-08-02 Thread Michael Ellerman
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()

2020-07-30 Thread Michael Ellerman
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

2020-06-29 Thread Michael Ellerman
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

2020-04-19 Thread Michael Ellerman
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

2019-12-17 Thread Michael Ellerman
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

2019-12-08 Thread Michael Ellerman
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

2019-11-14 Thread Michael Ellerman
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

2019-11-14 Thread Michael Ellerman
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

2019-09-19 Thread Michael Ellerman



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

2019-09-01 Thread Michael Ellerman
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

2019-08-26 Thread Michael Ellerman
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

2019-07-22 Thread Michael Ellerman
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

2019-07-22 Thread Michael Ellerman
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

2019-02-22 Thread Michael Ellerman
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

2019-01-08 Thread Michael Ellerman
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

2018-12-16 Thread Michael Ellerman
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

2018-12-14 Thread Michael Ellerman
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

2018-12-09 Thread Michael Ellerman
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

2018-12-07 Thread Michael Ellerman
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

2018-11-28 Thread Michael Ellerman
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

2018-11-26 Thread Michael Ellerman
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

2018-09-25 Thread Michael Ellerman
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

2018-09-24 Thread Michael Ellerman
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

2018-08-03 Thread Michael Ellerman
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

2018-07-31 Thread Michael Ellerman
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

2018-07-31 Thread Michael Ellerman
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

2018-04-17 Thread Michael Ellerman
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

2018-02-19 Thread Michael Ellerman
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

2018-02-13 Thread Michael Ellerman
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

2018-02-12 Thread Michael Ellerman
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_

2018-01-02 Thread Michael Ellerman
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_

2018-01-02 Thread Michael Ellerman
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

2017-08-25 Thread Michael Ellerman
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

2017-08-23 Thread Michael Ellerman
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

2017-08-23 Thread Michael Ellerman
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

2017-06-14 Thread Michael Ellerman
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

2016-02-08 Thread Michael Ellerman
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()

2015-10-13 Thread Michael Ellerman
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

2014-10-17 Thread Michael Ellerman
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

2014-09-15 Thread Michael Ellerman
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()

2014-07-08 Thread Michael Ellerman
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