* Re: [PATCH V3 1/6] powerpc/mm/book3s: Update pmd_present to look at _PAGE_PRESENT bit
From: Christophe LEROY @ 2018-09-21 10:52 UTC (permalink / raw)
To: Aneesh Kumar K.V, npiggin, benh, paulus, mpe; +Cc: linuxppc-dev
In-Reply-To: <e64affb4-858b-1424-e850-07442fd45912@linux.ibm.com>
Le 21/09/2018 à 12:26, Aneesh Kumar K.V a écrit :
> On 9/21/18 11:25 AM, Christophe LEROY wrote:
>>
>>
>> Le 20/09/2018 à 20:09, Aneesh Kumar K.V a écrit :
>
>>> diff --git a/arch/powerpc/mm/pgtable.c b/arch/powerpc/mm/pgtable.c
>>> index d71c7777669c..aee04b209b51 100644
>>> --- a/arch/powerpc/mm/pgtable.c
>>> +++ b/arch/powerpc/mm/pgtable.c
>>> @@ -188,11 +188,10 @@ void set_pte_at(struct mm_struct *mm, unsigned
>>> long addr, pte_t *ptep,
>>> pte_t pte)
>>> {
>>> /*
>>> - * When handling numa faults, we already have the pte marked
>>> - * _PAGE_PRESENT, but we can be sure that it is not in hpte.
>>> - * Hence we can use set_pte_at for them.
>>> + * Make sure hardware valid bit is not set. We don't do
>>> + * tlb flush for this update.
>>> */
>>> - VM_WARN_ON(pte_present(*ptep) && !pte_protnone(*ptep));
>>> + VM_WARN_ON(pte_val(*ptep) & _PAGE_PRESENT);
>>
>> Why not using pte_present() anymore ?
>>
>> Also, you are removing the pte_protnone() check, won't it change the
>> behaviour ?
>>
>> If we can't use pte_present(), can we create a new helper for that
>> (allthough _PAGE_PRESENT exists on all platforms).
>>
>> Christophe
>>
>
> This patch update a page table clear to clear _PAGE_PRESENT and mark it
> invalid via _PAGE_INVALID. The pte_present now looks at both the flag.
> That is we want these transient clear of pte to be considered as present
> pte even if _PAGE_PRESENT is cleared. What we are catching by the debug
> BUG_ON in these function is we are not using them to set a pte where the
> old entry has a hadware valid bit set. This is because we don't do any
> tlb flush with set_pte_at.
>
>
> So the reason for pte_present -> pte_val() & _PAGE_PRESENT is because we
> swtiched the clear to clear _PAGE_PRESENT and set _PAGE_INVALID and
> pte_present now check both.
>
> The reason for the removal of pte_protnone is because we dropped that
> set_pte_at usage from core autonuma code long time back.
>
> Now Considering we are calling this from mm/pgtable.c With your approach
> of not using pte flags directly in core code we could switch this to
> pte_hw_valid(). May be we can do that as an addon patch?
>
Ok, depending on which serie goes first, I'll add it in mine if I have
to rebase.
Christophe
^ permalink raw reply
* Re: powerpc: fix csum_ipv6_magic() on little endian platforms
From: Michael Ellerman @ 2018-09-21 11:59 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
Jianlin Shi, Xin Long
Cc: linuxppc-dev, linux-kernel, stable
In-Reply-To: <3c4cddf185438cc7200908b4f971a8e6ed7472fd.1536558233.git.christophe.leroy@c-s.fr>
On Mon, 2018-09-10 at 06:09:04 UTC, Christophe Leroy wrote:
> On little endian platforms, csum_ipv6_magic() keeps len and proto in
> CPU byte order. This generates a bad results leading to ICMPv6 packets
> from other hosts being dropped by powerpc64le platforms.
>
> In order to fix this, len and proto should be converted to network
> byte order ie bigendian byte order. However checksumming 0x12345678
> and 0x56341278 provide the exact same result so it is enough to
> rotate the sum of len and proto by 1 byte.
>
> PPC32 only support bigendian so the fix is needed for PPC64 only
>
> Fixes: e9c4943a107b ("powerpc: Implement csum_ipv6_magic in assembly")
> Reported-by: Jianlin Shi <jishi@redhat.com>
> Reported-by: Xin Long <lucien.xin@gmail.com>
> Cc: <stable@vger.kernel.org> # 4.18+
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> Tested-by: Xin Long <lucien.xin@gmail.com>
Applied to powerpc fixes, thanks.
https://git.kernel.org/powerpc/c/85682a7e3b9c664995ad477520f917
cheers
^ permalink raw reply
* Re: [kernel] powerpc/powernv/ioda2: Reduce upper limit for DMA window size (retry)
From: Michael Ellerman @ 2018-09-21 11:59 UTC (permalink / raw)
To: Alexey Kardashevskiy, linuxppc-dev; +Cc: Alexey Kardashevskiy
In-Reply-To: <20180911053805.5345-1-aik@ozlabs.ru>
On Tue, 2018-09-11 at 05:38:05 UTC, Alexey Kardashevskiy wrote:
> We use PHB in mode1 which uses bit 59 to select a correct DMA window.
> However there is mode2 which uses bits 59:55 and allows up to 32 DMA
> windows per a PE.
>
> Even though documentation does not clearly specify that, it seems that
> the actual hardware does not support bits 59:55 even in mode1, in other
> words we can create a window as big as 1<<58 but DMA simply won't work.
>
> This reduces the upper limit from 59 to 55 bits to let the userspace know
> about the hardware limits.
>
> Fixes: ce57c6610cc2 "Merge branch 'topic/ppc-kvm' into next"
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Applied to powerpc fixes, thanks.
https://git.kernel.org/powerpc/c/7233b8cab39014620ac9534da11f0f
cheers
^ permalink raw reply
* Re: KVM: PPC: Book3S HV: Fix guest r11 corruption with POWER9 TM workarounds
From: Michael Ellerman @ 2018-09-21 11:59 UTC (permalink / raw)
To: Michael Neuling; +Cc: mikey, linuxppc-dev, sjitindarsingh, kvm-ppc
In-Reply-To: <20180913053347.11389-1-mikey@neuling.org>
On Thu, 2018-09-13 at 05:33:47 UTC, Michael Neuling wrote:
> When we come into the softpatch handler (0x1500), we use r11 to store
> the HSRR0 for later use by the denorm handler.
>
> We also use the softpatch handler for the TM workarounds for
> POWER9. Unfortunately, in kvmppc_interrupt_hv we later store r11 out
> to the vcpu assuming it's still what we got from userspace.
>
> This causes r11 to be corrupted in the VCPU and hence when we restore
> the guest, we get a corrupted r11. We've seen this when running TM
> tests inside guests on P9.
>
> This fixes the problem by only touching r11 in the denorm case.
>
> Fixes: 4bb3c7a020 ("KVM: PPC: Book3S HV: Work around transactional memory bugs in POWER9")
> Cc: <stable@vger.kernel.org> # 4.17+
> Test-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> Reviewed-by: Paul Mackerras <paulus@ozlabs.org>
> Signed-off-by: Michael Neuling <mikey@neuling.org>
Applied to powerpc fixes, thanks.
https://git.kernel.org/powerpc/c/f14040bca89258b8a1c71e2112e430
cheers
^ permalink raw reply
* Re: [v4] powerpc: Avoid code patching freed init sections
From: Michael Ellerman @ 2018-09-21 11:59 UTC (permalink / raw)
To: Michael Neuling
Cc: mikey, Nicholas Piggin, Michal Suchánek, linuxppc-dev,
Haren Myneni
In-Reply-To: <20180914011411.3184-1-mikey@neuling.org>
On Fri, 2018-09-14 at 01:14:11 UTC, Michael Neuling wrote:
> This stops us from doing code patching in init sections after they've
> been freed.
>
> In this chain:
> kvm_guest_init() ->
> kvm_use_magic_page() ->
> fault_in_pages_readable() ->
> __get_user() ->
> __get_user_nocheck() ->
> barrier_nospec();
>
> We have a code patching location at barrier_nospec() and
> kvm_guest_init() is an init function. This whole chain gets inlined,
> so when we free the init section (hence kvm_guest_init()), this code
> goes away and hence should no longer be patched.
>
> We seen this as userspace memory corruption when using a memory
> checker while doing partition migration testing on powervm (this
> starts the code patching post migration via
> /sys/kernel/mobility/migration). In theory, it could also happen when
> using /sys/kernel/debug/powerpc/barrier_nospec.
>
> cc: stable@vger.kernel.org # 4.13+
> Signed-off-by: Michael Neuling <mikey@neuling.org>
> Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> Reviewed-by: Christophe Leroy <christophe.leroy@c-s.fr>
Applied to powerpc fixes, thanks.
https://git.kernel.org/powerpc/c/51c3c62b58b357e8d35e4cc32f7b4e
cheers
^ permalink raw reply
* Re: powerpc/pkeys: Fix reading of ibm, processor-storage-keys property
From: Michael Ellerman @ 2018-09-21 11:59 UTC (permalink / raw)
To: Thiago Jung Bauermann, linuxppc-dev
Cc: Ram Pai, linux-kernel, Thiago Jung Bauermann
In-Reply-To: <20180920043858.28773-1-bauerman@linux.ibm.com>
On Thu, 2018-09-20 at 04:38:58 UTC, Thiago Jung Bauermann wrote:
> scan_pkey_feature() uses of_property_read_u32_array() to read the
> ibm,processor-storage-keys property and calls be32_to_cpu() on the
> value it gets. The problem is that of_property_read_u32_array() already
> returns the value converted to the CPU byte order.
>
> The value of pkeys_total ends up more or less sane because there's a min()
> call in pkey_initialize() which reduces pkeys_total to 32. So in practice
> the kernel ignores the fact that the hypervisor reserved one key for
> itself (the device tree advertises 31 keys in my test VM).
>
> This is wrong, but the effect in practice is that when a process tries to
> allocate the 32nd key, it gets an -EINVAL error instead of -ENOSPC which
> would indicate that there aren't any keys available
>
> Fixes: cf43d3b26452 ("powerpc: Enable pkey subsystem")
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
Applied to powerpc fixes, thanks.
https://git.kernel.org/powerpc/c/c716a25b9b70084e1144f77423f5ae
cheers
^ permalink raw reply
* Re: [PATCH v8 1/3] powerpc: Detect the presence of big-cores via "ibm,thread-groups"
From: Gautham R Shenoy @ 2018-09-21 17:17 UTC (permalink / raw)
To: Michael Neuling
Cc: Gautham R. Shenoy, Aneesh Kumar K.V, Srikar Dronamraju,
Michael Ellerman, Benjamin Herrenschmidt, Vaidyanathan Srinivasan,
Akshay Adiga, Shilpasri G Bhat, Oliver O'Halloran,
Nicholas Piggin, Murilo Opsfelder Araujo, Anton Blanchard,
linuxppc-dev, linux-kernel
In-Reply-To: <d1cf1628ed2641140d3508d355cdc172d6b145e9.camel@neuling.org>
Hello Michael,
On Fri, Sep 21, 2018 at 01:02:45PM +1000, Michael Neuling wrote:
> This doesn't compile for me with:
>
> arch/powerpc/kernel/smp.c: In function ‘smp_prepare_cpus’:
> arch/powerpc/kernel/smp.c:812:23: error: ‘tg.threads_per_group’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> struct thread_groups tg;
> ^
> arch/powerpc/kernel/smp.c:812:23: error: ‘tg.nr_groups’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> cc1: all warnings being treated as errors
> /home/mikey/src/linux-ozlabs/scripts/Makefile.build:305: recipe for target 'arch/powerpc/kernel/smp.o' failed
>
I couldn't get this error with gcc 4.8.5 and 8.1.1 with
pseries_defconfig and powernv_defconfig with CONFIG_PPC_WERROR=y.
Does the following the following delta patch make it work?
-----------------------------X8----------------------------------
>From 6699ce20573dddd0d3d45ea79015751880740e9b Mon Sep 17 00:00:00 2001
From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
Date: Fri, 21 Sep 2018 22:43:05 +0530
Subject: [PATCH] powerpc/smp: Initialize thread_groups local variable
Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
---
arch/powerpc/kernel/smp.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 5cdcf44..356751e 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -809,8 +809,9 @@ static int init_cpu_l1_cache_map(int cpu)
{
struct device_node *dn = of_get_cpu_node(cpu, NULL);
- struct thread_groups tg;
-
+ struct thread_groups tg = {.property = 0,
+ .nr_groups = 0,
+ .threads_per_group = 0};
int first_thread = cpu_first_thread_sibling(cpu);
int i, cpu_group_start = -1, err = 0;
--
1.9.4
^ permalink raw reply related
* Re: [PATCH v8 3/3] powerpc/sysfs: Add topology/smallcore_thread_siblings[_list]
From: Gautham R Shenoy @ 2018-09-21 17:20 UTC (permalink / raw)
To: kbuild test robot
Cc: Gautham R. Shenoy, kbuild-all, Aneesh Kumar K.V,
Srikar Dronamraju, Michael Ellerman, Benjamin Herrenschmidt,
Michael Neuling, Vaidyanathan Srinivasan, Akshay Adiga,
Shilpasri G Bhat, Oliver O'Halloran, Nicholas Piggin,
Murilo Opsfelder Araujo, Anton Blanchard, linuxppc-dev,
linux-kernel
In-Reply-To: <201809211407.677jNLUN%fengguang.wu@intel.com>
On Fri, Sep 21, 2018 at 02:20:15PM +0800, kbuild test robot wrote:
> Hi Gautham,
>
> Thank you for the patch! Yet something to improve:
>
> [auto build test ERROR on powerpc/next]
> [also build test ERROR on v4.19-rc4 next-20180919]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url: https://github.com/0day-ci/linux/commits/Gautham-R-Shenoy/powerpc-Detection-and-scheduler-optimization-for-POWER9-bigcore/20180921-085812
> base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
> config: powerpc-mpc837x_mds_defconfig (attached as .config)
> compiler: powerpc-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
> reproduce:
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> GCC_VERSION=7.2.0 make.cross ARCH=powerpc
>
> All errors (new ones prefixed by >>):
>
> arch/powerpc/kernel/sysfs.c: In function 'smallcore_thread_siblings_show':
> >> arch/powerpc/kernel/sysfs.c:724:45: error: implicit declaration of function 'cpu_smallcore_mask'; did you mean 'cpu_all_mask'? [-Werror=implicit-function-declaration]
> return cpumap_print_to_pagebuf(false, buf, cpu_smallcore_mask(cpu));
> ^~~~~~~~~~~~~~~~~~
No, the smallcore_thread_siblings_show, and the other functions should
only be compiled for CONFIG_SMP.
Will add this. Thanks bot!
> cpu_all_mask
> >> arch/powerpc/kernel/sysfs.c:724:45: error: passing argument 3 of 'cpumap_print_to_pagebuf' makes pointer from integer without a cast [-Werror=int-conversion]
> In file included from include/linux/rcupdate.h:44:0,
> from include/linux/radix-tree.h:28,
> from include/linux/idr.h:15,
> from include/linux/kernfs.h:14,
> from include/linux/sysfs.h:16,
> from include/linux/kobject.h:20,
> from include/linux/device.h:16,
> from arch/powerpc/kernel/sysfs.c:1:
> include/linux/cpumask.h:892:1: note: expected 'const struct cpumask *' but argument is of type 'int'
> cpumap_print_to_pagebuf(bool list, char *buf, const struct cpumask *mask)
> ^~~~~~~~~~~~~~~~~~~~~~~
> arch/powerpc/kernel/sysfs.c: In function 'smallcore_thread_siblings_list_show':
> arch/powerpc/kernel/sysfs.c:734:44: error: passing argument 3 of 'cpumap_print_to_pagebuf' makes pointer from integer without a cast [-Werror=int-conversion]
> return cpumap_print_to_pagebuf(true, buf, cpu_smallcore_mask(cpu));
> ^~~~~~~~~~~~~~~~~~
> In file included from include/linux/rcupdate.h:44:0,
> from include/linux/radix-tree.h:28,
> from include/linux/idr.h:15,
> from include/linux/kernfs.h:14,
> from include/linux/sysfs.h:16,
> from include/linux/kobject.h:20,
> from include/linux/device.h:16,
> from arch/powerpc/kernel/sysfs.c:1:
> include/linux/cpumask.h:892:1: note: expected 'const struct cpumask *' but argument is of type 'int'
> cpumap_print_to_pagebuf(bool list, char *buf, const struct cpumask *mask)
> ^~~~~~~~~~~~~~~~~~~~~~~
> cc1: all warnings being treated as errors
>
> vim +724 arch/powerpc/kernel/sysfs.c
>
> 717
> 718 static ssize_t smallcore_thread_siblings_show(struct device *dev,
> 719 struct device_attribute *attr,
> 720 char *buf)
> 721 {
> 722 int cpu = dev->id;
> 723
> > 724 return cpumap_print_to_pagebuf(false, buf, cpu_smallcore_mask(cpu));
> 725 }
> 726 static DEVICE_ATTR_RO(smallcore_thread_siblings);
> 727
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply
* Re: [PATCH ppc-next] powerpc/fsl-booke: don't load early TLB at once
From: York Sun @ 2018-09-21 17:40 UTC (permalink / raw)
To: Scott Wood, David Lamparter, linuxppc-dev@lists.ozlabs.org; +Cc: Scott Wood
In-Reply-To: <1537489893.7689.38.camel@redhat.com>
On 09/20/2018 05:31 PM, Scott Wood wrote:=0A=
> On Fri, 2018-09-21 at 00:48 +0200, David Lamparter wrote:=0A=
>> This is a *partial* revert of "powerpc/85xx: Load all early TLB entries=
=0A=
>> at once" (d9e1831a420267a7ced708bb259d65b0a3c0344d.)=0A=
>>=0A=
>> My dusty old P4080DS just completely fails to boot (no output at all)=0A=
>> without this revert. I have no clue what's going on here, I just=0A=
>> bisected it down and since it looks like an optimization to me I just=0A=
>> reverted it - and voil=E1, the P4080 boots again.=0A=
> =0A=
> It's not an optimization; it was required to get kdump working, at least=
=0A=
> for certain choices of crash kernel location. I just tried booting a 32-=
=0A=
> bit kernel and did not see this problem -- but I don't have access to a=
=0A=
> p4080ds anymore. I tried with qemu e500mc, and also running a 32-bit=0A=
> kernel on e6500 (needs a tiny change to get past SMP init, since 32-bit=
=0A=
> isn't really supported on e6500, but you do get output even without that)=
.=0A=
> =0A=
> Do you have a JTAG that can be used to find out where it's hanging? If=
=0A=
> not, I can try to get early debug working (needs an early IOMMU mapping).=
=0A=
> =0A=
> York, can you try booting the latest kernel on p4080ds?=0A=
> =0A=
=0A=
Scott,=0A=
=0A=
I haven't tried P4080DS for a long time. What defconfig do you use for=0A=
this board? I tried latest master branch (commit a27fb6d983c7b5) with=0A=
corenet_basic_defconfig, it didn't boot up. Kernel has an exception very=0A=
early (pc ffffae80).=0A=
=0A=
However, before I claimed the board, someone booted Linux 4.14.71 on=0A=
this board. I need to track down where the image came from.=0A=
=0A=
York=0A=
^ permalink raw reply
* Re: [PATCH ppc-next] powerpc/fsl-booke: don't load early TLB at once
From: Scott Wood @ 2018-09-21 17:47 UTC (permalink / raw)
To: York Sun, Scott Wood, David Lamparter,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <DB7PR04MB4298A6AD0F866804A3F4FB6F9A120@DB7PR04MB4298.eurprd04.prod.outlook.com>
On Fri, 2018-09-21 at 17:40 +0000, York Sun wrote:
> On 09/20/2018 05:31 PM, Scott Wood wrote:
> > On Fri, 2018-09-21 at 00:48 +0200, David Lamparter wrote:
> > > This is a *partial* revert of "powerpc/85xx: Load all early TLB entries
> > > at once" (d9e1831a420267a7ced708bb259d65b0a3c0344d.)
> > >
> > > My dusty old P4080DS just completely fails to boot (no output at all)
> > > without this revert. I have no clue what's going on here, I just
> > > bisected it down and since it looks like an optimization to me I just
> > > reverted it - and voilá, the P4080 boots again.
> >
> > It's not an optimization; it was required to get kdump working, at least
> > for certain choices of crash kernel location. I just tried booting a 32-
> > bit kernel and did not see this problem -- but I don't have access to a
> > p4080ds anymore. I tried with qemu e500mc, and also running a 32-bit
> > kernel on e6500 (needs a tiny change to get past SMP init, since 32-bit
> > isn't really supported on e6500, but you do get output even without that).
> >
> > Do you have a JTAG that can be used to find out where it's hanging? If
> > not, I can try to get early debug working (needs an early IOMMU mapping).
> >
> > York, can you try booting the latest kernel on p4080ds?
> >
>
> Scott,
>
> I haven't tried P4080DS for a long time. What defconfig do you use for
> this board? I tried latest master branch (commit a27fb6d983c7b5) with
> corenet_basic_defconfig, it didn't boot up. Kernel has an exception very
> early (pc ffffae80).
>
> However, before I claimed the board, someone booted Linux 4.14.71 on
> this board. I need to track down where the image came from.
Use corenet32_smp_defconfig
corenet_basic_defconfig is just a fragment used by the makefiles in assembling
the final config.
-Scott
^ permalink raw reply
* Re: [PATCH ppc-next] powerpc/fsl-booke: don't load early TLB at once
From: York Sun @ 2018-09-21 18:07 UTC (permalink / raw)
To: Scott Wood, Scott Wood; +Cc: David Lamparter, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <666d304e5aeae639dbae2f5e9f62d7234041d6d2.camel@buserror.net>
On 09/21/2018 10:47 AM, Scott Wood wrote:=0A=
> On Fri, 2018-09-21 at 17:40 +0000, York Sun wrote:=0A=
>> On 09/20/2018 05:31 PM, Scott Wood wrote:=0A=
>>> On Fri, 2018-09-21 at 00:48 +0200, David Lamparter wrote:=0A=
>>>> This is a *partial* revert of "powerpc/85xx: Load all early TLB entrie=
s=0A=
>>>> at once" (d9e1831a420267a7ced708bb259d65b0a3c0344d.)=0A=
>>>>=0A=
>>>> My dusty old P4080DS just completely fails to boot (no output at all)=
=0A=
>>>> without this revert. I have no clue what's going on here, I just=0A=
>>>> bisected it down and since it looks like an optimization to me I just=
=0A=
>>>> reverted it - and voil=E1, the P4080 boots again.=0A=
>>>=0A=
>>> It's not an optimization; it was required to get kdump working, at leas=
t=0A=
>>> for certain choices of crash kernel location. I just tried booting a 3=
2-=0A=
>>> bit kernel and did not see this problem -- but I don't have access to a=
=0A=
>>> p4080ds anymore. I tried with qemu e500mc, and also running a 32-bit=
=0A=
>>> kernel on e6500 (needs a tiny change to get past SMP init, since 32-bit=
=0A=
>>> isn't really supported on e6500, but you do get output even without tha=
t).=0A=
>>>=0A=
>>> Do you have a JTAG that can be used to find out where it's hanging? If=
=0A=
>>> not, I can try to get early debug working (needs an early IOMMU mapping=
).=0A=
>>>=0A=
>>> York, can you try booting the latest kernel on p4080ds?=0A=
>>>=0A=
>>=0A=
>> Scott,=0A=
>>=0A=
>> I haven't tried P4080DS for a long time. What defconfig do you use for=
=0A=
>> this board? I tried latest master branch (commit a27fb6d983c7b5) with=0A=
>> corenet_basic_defconfig, it didn't boot up. Kernel has an exception very=
=0A=
>> early (pc ffffae80).=0A=
>>=0A=
>> However, before I claimed the board, someone booted Linux 4.14.71 on=0A=
>> this board. I need to track down where the image came from.=0A=
> =0A=
> Use corenet32_smp_defconfig=0A=
> =0A=
> corenet_basic_defconfig is just a fragment used by the makefiles in assem=
bling=0A=
> the final config.=0A=
> =0A=
=0A=
Thanks for the instruction. Linux comes up OK with corenet32_smp_defconfig.=
=0A=
=0A=
root@p4080ds:~# uname -a=0A=
Linux p4080ds 4.19.0-rc4-00206-ga27fb6d #1 SMP Fri Sep 21 10:56:36 PDT=0A=
2018 ppc GNU/Linux=0A=
=0A=
York=0A=
=0A=
^ permalink raw reply
* Re: [PATCH net-next 00/22] net: fix return type of ndo_start_xmit function
From: Grygorii Strashko @ 2018-09-20 17:04 UTC (permalink / raw)
To: YueHaibing, davem, dmitry.tarnyagin, wg, mkl, michal.simek,
hsweeten, madalin.bucur, pantelis.antoniou, claudiu.manoil,
leoyang.li, linux, sammy, ralf, nico, steve.glendinning,
f.fainelli, w-kwok2, m-karicheri2, t.sailer, jreuter, kys,
haiyangz, wei.liu2, paul.durrant, arvid.brodin, pshelar
Cc: linux-kernel, netdev, linux-can, linux-arm-kernel, linuxppc-dev,
linux-mips, linux-omap, linux-hams, devel, linux-usb, xen-devel,
dev
In-Reply-To: <20180920123306.14772-1-yuehaibing@huawei.com>
On 09/20/2018 07:32 AM, YueHaibing wrote:
> The method ndo_start_xmit() is defined as returning an 'netdev_tx_t',
> which is a typedef for an enum type, so make sure the implementation in
> this driver has returns 'netdev_tx_t' value, and change the function
> return type to netdev_tx_t.
>
May be I missed smth, but it's acceptable to report standard error codes from
.xmit() callback as per dev_xmit_complete().
--
regards,
-grygorii
^ permalink raw reply
* Re: [PATCH v3 1/3] powerpc/process: fix casting and missing header
From: Murilo Opsfelder Araujo @ 2018-09-21 20:24 UTC (permalink / raw)
To: Christophe Leroy
Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
linux-kernel, linuxppc-dev
In-Reply-To: <113d4a559f6eec448b253bf5f0b4f2a8ffb187af.1536327756.git.christophe.leroy@c-s.fr>
On Fri, Sep 07, 2018 at 01:47:29PM +0000, Christophe Leroy wrote:
> This patch fixes the following warnings. The first ones are leftovers
> from when __get_user() was replaced by probe_kernel_address().
>
> The last one is from when show_user_instructions() was added.
>
> arch/powerpc/kernel/process.c:1287:22: warning: incorrect type in argument 2 (different address spaces)
> arch/powerpc/kernel/process.c:1287:22: expected void const *src
> arch/powerpc/kernel/process.c:1287:22: got unsigned int [noderef] <asn:1>*<noident>
> arch/powerpc/kernel/process.c:1319:21: warning: incorrect type in argument 2 (different address spaces)
> arch/powerpc/kernel/process.c:1319:21: expected void const *src
> arch/powerpc/kernel/process.c:1319:21: got unsigned int [noderef] <asn:1>*<noident>
> arch/powerpc/kernel/process.c:1302:6: warning: symbol 'show_user_instructions' was not declared. Should it be static?
>
> Fixes: 7b051f665c32d ("powerpc: Use probe_kernel_address in show_instructions")
> Fixes: 88b0fe1757359 ("powerpc: Add show_user_instructions()")
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Smoke test passed. Thank you.
Reviewed-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com>
> ---
> v3: new in v3 to fix sparse warnings reported by snowpatch on the serie
>
> arch/powerpc/kernel/process.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index 913c5725cdb2..e108e1ef2b85 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -65,6 +65,7 @@
> #include <asm/livepatch.h>
> #include <asm/cpu_has_feature.h>
> #include <asm/asm-prototypes.h>
> +#include <asm/stacktrace.h>
>
> #include <linux/kprobes.h>
> #include <linux/kdebug.h>
> @@ -1284,7 +1285,7 @@ static void show_instructions(struct pt_regs *regs)
> #endif
>
> if (!__kernel_text_address(pc) ||
> - probe_kernel_address((unsigned int __user *)pc, instr)) {
> + probe_kernel_address((const void *)pc, instr)) {
> pr_cont("XXXXXXXX ");
> } else {
> if (regs->nip == pc)
> @@ -1316,7 +1317,7 @@ void show_user_instructions(struct pt_regs *regs)
> pr_info("%s[%d]: code: ", current->comm, current->pid);
> }
>
> - if (probe_kernel_address((unsigned int __user *)pc, instr)) {
> + if (probe_kernel_address((const void *)pc, instr)) {
> pr_cont("XXXXXXXX ");
> } else {
> if (regs->nip == pc)
> --
> 2.13.3
>
--
Murilo
^ permalink raw reply
* Re: [PATCH 1/4] soc/fsl/qbman: Check if CPU is offline when initializing portals
From: Li Yang @ 2018-09-21 22:15 UTC (permalink / raw)
To: madalin.bucur
Cc: Roy Pledge, claudiu.manoil, Catalin Marinas, Scott Wood,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
linuxppc-dev, lkml
In-Reply-To: <1537456011-10769-2-git-send-email-madalin.bucur@nxp.com>
On Thu, Sep 20, 2018 at 10:09 AM Madalin Bucur <madalin.bucur@nxp.com> wrote:
>
> From: Roy Pledge <roy.pledge@nxp.com>
>
> If the affine portal for a specific CPU is offline at boot time
> affine its interrupt to CPU 0. If the CPU is later brought online
> the hotplug handler will correctly adjust the affinity.
Although this does provide better compatibility with cpu hotplug, we
still have a problem. You are assuming the CPU0 is always online
which is not the case for arm64. How about not setting the affinity
and let the system decide if the dedicated CPU is offline?
>
> Signed-off-by: Roy Pledge <roy.pledge@nxp.com>
> Signed-off-by: Madalin Bucur <madalin.bucur@nxp.com>
> ---
> drivers/soc/fsl/qbman/bman.c | 17 +++++++++++++----
> drivers/soc/fsl/qbman/qman.c | 18 +++++++++++++-----
> 2 files changed, 26 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/soc/fsl/qbman/bman.c b/drivers/soc/fsl/qbman/bman.c
> index f9485cedc648..2e6e682bf16b 100644
> --- a/drivers/soc/fsl/qbman/bman.c
> +++ b/drivers/soc/fsl/qbman/bman.c
> @@ -562,10 +562,19 @@ static int bman_create_portal(struct bman_portal *portal,
> dev_err(c->dev, "request_irq() failed\n");
> goto fail_irq;
> }
> - if (c->cpu != -1 && irq_can_set_affinity(c->irq) &&
> - irq_set_affinity(c->irq, cpumask_of(c->cpu))) {
> - dev_err(c->dev, "irq_set_affinity() failed\n");
> - goto fail_affinity;
> + if (cpu_online(c->cpu) && c->cpu != -1 &&
> + irq_can_set_affinity(c->irq)) {
> + if (irq_set_affinity(c->irq, cpumask_of(c->cpu))) {
> + dev_err(c->dev, "irq_set_affinity() failed %d\n",
> + c->cpu);
> + goto fail_affinity;
> + }
> + } else {
> + /* CPU is offline, direct IRQ to CPU 0 */
> + if (irq_set_affinity(c->irq, cpumask_of(0))) {
> + dev_err(c->dev, "irq_set_affinity() cpu 0 failed\n");
> + goto fail_affinity;
> + }
> }
>
> /* Need RCR to be empty before continuing */
> diff --git a/drivers/soc/fsl/qbman/qman.c b/drivers/soc/fsl/qbman/qman.c
> index ecb22749df0b..7dbcb475a59c 100644
> --- a/drivers/soc/fsl/qbman/qman.c
> +++ b/drivers/soc/fsl/qbman/qman.c
> @@ -935,7 +935,6 @@ static inline int qm_mc_result_timeout(struct qm_portal *portal,
> break;
> udelay(1);
> } while (--timeout);
> -
> return timeout;
> }
>
> @@ -1210,10 +1209,19 @@ static int qman_create_portal(struct qman_portal *portal,
> dev_err(c->dev, "request_irq() failed\n");
> goto fail_irq;
> }
> - if (c->cpu != -1 && irq_can_set_affinity(c->irq) &&
> - irq_set_affinity(c->irq, cpumask_of(c->cpu))) {
> - dev_err(c->dev, "irq_set_affinity() failed\n");
> - goto fail_affinity;
> + if (cpu_online(c->cpu) && c->cpu != -1 &&
> + irq_can_set_affinity(c->irq)) {
> + if (irq_set_affinity(c->irq, cpumask_of(c->cpu))) {
> + dev_err(c->dev, "irq_set_affinity() failed %d\n",
> + c->cpu);
> + goto fail_affinity;
> + }
> + } else {
> + /* CPU is offline, direct IRQ to CPU 0 */
> + if (irq_set_affinity(c->irq, cpumask_of(0))) {
> + dev_err(c->dev, "irq_set_affinity() cpu 0 failed\n");
> + goto fail_affinity;
> + }
> }
>
> /* Need EQCR to be empty before continuing */
> --
> 2.1.0
>
^ permalink raw reply
* Re: [PATCH v3 3/3] powerpc/process: Constify the number of insns printed by show instructions functions.
From: Murilo Opsfelder Araujo @ 2018-09-21 20:25 UTC (permalink / raw)
To: Christophe Leroy
Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
linux-kernel, linuxppc-dev
In-Reply-To: <3fdf75eed98b1fa22da77e675692a86a7a9986a7.1536327756.git.christophe.leroy@c-s.fr>
On Fri, Sep 07, 2018 at 01:47:33PM +0000, Christophe Leroy wrote:
> instructions_to_print var is assigned value 16 and there is no
> way to change it.
>
> This patch replaces it by a constant.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Reviewed-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com>
> ---
> v3: no change
> v2: no change
>
> arch/powerpc/kernel/process.c | 11 +++++------
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index 2a39f7aca846..7d86b4f7949e 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -1261,17 +1261,16 @@ struct task_struct *__switch_to(struct task_struct *prev,
> return last;
> }
>
> -static int instructions_to_print = 16;
> +#define NR_INSN_TO_PRINT 16
>
> static void show_instructions(struct pt_regs *regs)
> {
> int i;
> - unsigned long pc = regs->nip - (instructions_to_print * 3 / 4 *
> - sizeof(int));
> + unsigned long pc = regs->nip - (NR_INSN_TO_PRINT * 3 / 4 * sizeof(int));
>
> printk("Instruction dump:");
>
> - for (i = 0; i < instructions_to_print; i++) {
> + for (i = 0; i < NR_INSN_TO_PRINT; i++) {
> int instr;
>
> if (!(i % 8))
> @@ -1304,11 +1303,11 @@ static void show_instructions(struct pt_regs *regs)
> void show_user_instructions(struct pt_regs *regs)
> {
> unsigned long pc;
> - int n = instructions_to_print;
> + int n = NR_INSN_TO_PRINT;
> struct seq_buf s;
> char buf[96]; /* enough for 8 times 9 + 2 chars */
>
> - pc = regs->nip - (instructions_to_print * 3 / 4 * sizeof(int));
> + pc = regs->nip - (NR_INSN_TO_PRINT * 3 / 4 * sizeof(int));
>
> seq_buf_init(&s, buf, sizeof(buf));
>
> --
> 2.13.3
>
--
Murilo
^ permalink raw reply
* Re: [PATCH v3 2/3] powerpc/process: fix interleaved output in show_user_instructions()
From: Murilo Opsfelder Araujo @ 2018-09-21 20:25 UTC (permalink / raw)
To: Christophe Leroy
Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
linux-kernel, linuxppc-dev
In-Reply-To: <8f4de3aa04f00d67fdb5aaf90f41242197310d4d.1536327756.git.christophe.leroy@c-s.fr>
On Fri, Sep 07, 2018 at 01:47:31PM +0000, Christophe Leroy wrote:
> When two processes crash at the same time, we sometimes encounter
> interleaving in the middle of a line:
>
> [ 4.365317] init[1]: segfault (11) at 0 nip 0 lr 0 code 1
> [ 4.370452] init[1]: code: XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX
> [ 4.372042] init[74]: segfault (11) at 10a74 nip 1000c198 lr 100078c8 code 1 in sh[10000000+14000]
> [ 4.386829] XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX
> [ 4.391542] init[1]: code: XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX
> [ 4.400863] init[74]: code: 90010024 bf61000c 91490a7c 3fa01002 3be00000 7d3e4b78 3bbd0c20 3b600000
> [ 4.409867] init[74]: code: 3b9d0040 7c7fe02e 2f830000 419e0028 <89230000> 2f890000 41be001c 4b7f6e79
>
> This patch fixes it by preparing complete lines in a buffer and
> printing it at once.
>
> Fixes: 88b0fe1757359 ("powerpc: Add show_user_instructions()")
> Cc: Murilo Opsfelder Araujo <muriloo@linux.ibm.com>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Reviewed-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com>
> ---
> v3: no change
> v2: Using seq_buf and reworked the loop to avoid redundant prints.
>
> arch/powerpc/kernel/process.c | 37 +++++++++++++++++++------------------
> 1 file changed, 19 insertions(+), 18 deletions(-)
>
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index e108e1ef2b85..2a39f7aca846 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -43,6 +43,7 @@
> #include <linux/uaccess.h>
> #include <linux/elf-randomize.h>
> #include <linux/pkeys.h>
> +#include <linux/seq_buf.h>
>
> #include <asm/pgtable.h>
> #include <asm/io.h>
> @@ -1303,33 +1304,33 @@ static void show_instructions(struct pt_regs *regs)
> void show_user_instructions(struct pt_regs *regs)
> {
> unsigned long pc;
> - int i;
> + int n = instructions_to_print;
> + struct seq_buf s;
> + char buf[96]; /* enough for 8 times 9 + 2 chars */
>
> pc = regs->nip - (instructions_to_print * 3 / 4 * sizeof(int));
>
> - pr_info("%s[%d]: code: ", current->comm, current->pid);
> + seq_buf_init(&s, buf, sizeof(buf));
>
> - for (i = 0; i < instructions_to_print; i++) {
> - int instr;
> + while (n) {
> + int i;
>
> - if (!(i % 8) && (i > 0)) {
> - pr_cont("\n");
> - pr_info("%s[%d]: code: ", current->comm, current->pid);
> - }
> + seq_buf_clear(&s);
>
> - if (probe_kernel_address((const void *)pc, instr)) {
> - pr_cont("XXXXXXXX ");
> - } else {
> - if (regs->nip == pc)
> - pr_cont("<%08x> ", instr);
> - else
> - pr_cont("%08x ", instr);
> + for (i = 0; i < 8 && n; i++, n--, pc += sizeof(int)) {
> + int instr;
> +
> + if (probe_kernel_address((const void *)pc, instr)) {
> + seq_buf_puts(&s, "XXXXXXXX ");
> + continue;
> + }
> + seq_buf_printf(&s, regs->nip == pc ? "<%08x> " : "%08x ", instr);
> }
>
> - pc += sizeof(int);
> + if (!seq_buf_has_overflowed(&s))
> + pr_info("%s[%d]: code: %s\n", current->comm,
> + current->pid, s.buffer);
> }
> -
> - pr_cont("\n");
> }
>
> struct regbit {
> --
> 2.13.3
>
--
Murilo
^ permalink raw reply
* Re: [PATCH ppc-next] powerpc/fsl-booke: don't load early TLB at once
From: Scott Wood @ 2018-09-22 5:45 UTC (permalink / raw)
To: David Lamparter; +Cc: Scott Wood, linuxppc-dev, York Sun
In-Reply-To: <20180921010019.GL487685@eidolon.nox.tf>
On Fri, Sep 21, 2018 at 03:00:19AM +0200, David Lamparter wrote:
> On Thu, Sep 20, 2018 at 07:31:33PM -0500, Scott Wood wrote:
> > Do you have a JTAG that can be used to find out where it's hanging? If
> > not, I can try to get early debug working (needs an early IOMMU mapping).
s/IOMMU mapping/MMU mapping for MMIO/ :-P
> I only have JTAG tools for ARM chips available; hardware wise I could
> probably solder up an adapter but software wise I have absoutely no clue
> how to fire up a session on anything PPC... I'm a novice openocd user,
> that's it.
>
> > York, can you try booting the latest kernel on p4080ds?
>
> d9e1831a42 has been around for quite some time, 4.4 already has it. I
> was a bit surprised noone has run into this, but then again P4080 is not
> exactly the most recent/interesting hardware.
I don't suppose you're running a relocatable kernel at a non-zero
address, and/or are running in an environment that sets
HID0[EN_L2MMU_MHD] (neither standard U-boot nor Linux sets this bit,
though they probably should)? On 32-bit, we're already running in an AS1
trampoline when loadcam_multi() is called, but loadcam_multi() sets up
its own. This happens to not be catastrophic in standard scenarios, but
it does add a duplicate TLB entry, and we return to AS0 sooner than
expected. I think your patch, plus ifdefs to make the change 32-bit
only, is the appropriate fix.
I also got an earlier udbg for e500 working (and happened to decide to
test it with a relocatable kernel); I'll send that out once I've cleaned
it up (or sooner with the extra TLB dumping included if the above doesn't
explain why you're hitting this bug).
-Scott
^ permalink raw reply
* Re: [PATCH] lib/xz: Put CRC32_POLY_LE in xz_private.h
From: Meelis Roos @ 2018-09-22 9:49 UTC (permalink / raw)
To: Joel Stanley
Cc: Krzysztof Kozlowski, Greg Kroah-Hartman, Herbert Xu,
Michael Ellerman, Christophe LEROY, linuxppc-dev, linux-kernel
In-Reply-To: <20180921025431.28366-1-joel@jms.id.au>
> This fixes a regression introduced by faa16bc404d72a5 ("lib: Use
> existing define with polynomial").
>
> The cleanup added a dependency on include/linux, which broke the PowerPC
> boot wrapper/decompresser when KERNEL_XZ is enabled:
>
> BOOTCC arch/powerpc/boot/decompress.o
> In file included from arch/powerpc/boot/../../../lib/decompress_unxz.c:233,
> from arch/powerpc/boot/decompress.c:42:
> arch/powerpc/boot/../../../lib/xz/xz_crc32.c:18:10: fatal error:
> linux/crc32poly.h: No such file or directory
> #include <linux/crc32poly.h>
> ^~~~~~~~~~~~~~~~~~~
>
> The powerpc decompresser is a hairy corner of the kernel. Even while building
> a 64-bit kernel it needs to build a 32-bit binary and therefore avoid including
> files from include/linux.
>
> This allows users of the xz library to avoid including headers from
> 'include/linux/' while still achieving the cleanup of the magic number.
>
> Fixes: faa16bc404d72a5 ("lib: Use existing define with polynomial")
> Reported-by: Meelis Roos <mroos@linux.ee>
> Reported-by: kbuild test robot <lkp@intel.com>
> Suggested-by: Christophe LEROY <christophe.leroy@c-s.fr>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
Tested-by: Meelis Roos <mroos@linux.ee>
--
Meelis Roos (mroos@linux.ee)
^ permalink raw reply
* Re: [PATCH v8 0/3] powerpc: Detection and scheduler optimization for POWER9 bigcore
From: Gautham R Shenoy @ 2018-09-22 11:03 UTC (permalink / raw)
To: Dave Hansen
Cc: Gautham R. Shenoy, Aneesh Kumar K.V, Srikar Dronamraju,
Michael Ellerman, Benjamin Herrenschmidt, Michael Neuling,
Vaidyanathan Srinivasan, Akshay Adiga, Shilpasri G Bhat,
Oliver O'Halloran, Nicholas Piggin, Murilo Opsfelder Araujo,
Anton Blanchard, linuxppc-dev, linux-kernel
In-Reply-To: <133c7bca-5720-81de-7956-b93870a1bab8@intel.com>
Hi Dave,
On Thu, Sep 20, 2018 at 11:04:54AM -0700, Dave Hansen wrote:
> On 09/20/2018 10:22 AM, Gautham R. Shenoy wrote:
> > -------------------------
> > | L1 Cache |
> > ----------------------------------
> > |L2| | | | |
> > | | 0 | 2 | 4 | 6 |Small Core0
> > |C | | | | |
> > Big |a --------------------------
> > Core |c | | | | |
> > |h | 1 | 3 | 5 | 7 | Small Core1
> > |e | | | | |
> > -----------------------------
> > | L1 Cache |
> > --------------------------
>
> The scheduler already knows about shared caches. Could you elaborate on
> how this is different from the situation today where we have multiple
> cores sharing an L2/L3?
The issue is not so much about the threads in the core sharing L2
cache. But the two group of threads in the core, each of which has its
own L1-cache. This patchset (the second patch in the series) informs
the scheduler of this distinction by defining the SMT sched-domain
have groups correspond to the threads that share L1 cache. With this
the scheduler will treat a pair of threads {1,2} differently from
{1,3} when threads 1 and 3 share the L1 cache, while 1 and 2 don't.
The next sched-domain (CACHE domain) is defined as the group of
threads that share the L2 cache, which happens to be the entire big
core.
Without this patchset, the SMT domain would be defined as the group of
threads that share L2 cache. Thus, the scheduler would treat any two
threads in the big-core in the same way, resulting in run-to-run
variance when the software threads are placed on pair of threads
within the same L1-cache group or on separate ones.
>
> Adding the new sysfs stuff seems like overkill if that's all that you
> are trying to do.
>
The sysfs attributes are to inform the users that we have a big-core
configuration comprising of two small cores, thereby allowing them to
make informed choices should they want to pin the tasks to the CPUs.
--
Thanks and Regards
gautham.
^ permalink raw reply
* Re: [PATCH 4/4] dma-mapping: clear dev->dma_ops in arch_teardown_dma_ops
From: Guenter Roeck @ 2018-09-22 15:01 UTC (permalink / raw)
To: Christoph Hellwig
Cc: iommu, Greg Kroah-Hartman, Robin Murphy, linux-kernel,
linux-arm-kernel, Marek Szyprowski, Bjorn Helgaas, linux-pci,
Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
linuxppc-dev
In-Reply-To: <20180827084711.23407-5-hch@lst.de>
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 <hch@lst.de>
> Reviewed-by: Robin Murphy <robin.murphy@arm.com>
This patch causes various ppc images to crash in -next due to NULL
DMA ops in dma_alloc_attrs().
Looking into the code, the macio driver tries to instantiate on
the 0000: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.
Either case, reverting the patch fixes the problem.
Guenter
---
ohci-pci 0000:f0:0d.0: OHCI PCI host controller
ohci-pci 0000:f0:0d.0: new USB bus registered, assigned bus number 1
------------[ cut here ]------------
kernel BUG at ./include/linux/dma-mapping.h:516!
Oops: Exception in kernel mode, sig: 5 [#1]
BE PREEMPT SMP NR_CPUS=4 NUMA PowerMac
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Tainted: G W
4.19.0-rc4-next-20180921-dirty #1
NIP: c00000000086b824 LR: c00000000086b5dc CTR: c00000000086dd70
REGS: c00000003d30f0b0 TRAP: 0700 Tainted: G W
(4.19.0-rc4-next-20180921-dirty)
MSR: 800000000002b032 <SF,EE,FP,ME,IR,DR,RI> CR: 28008842 XER: 00000000
IRQMASK: 0
GPR00: c00000000086b5dc c00000003d30f330 c000000001199900 c00000003d3ce898
GPR04: c00000000115b798 c00000003d8c3a48 0000000000000000 0000000000000000
GPR08: 0000000000000000 ffffffffffffff00 0000000000000000 0000000000000020
GPR12: 0000000024008442 c000000001299000 c00000000000f720 0000000000000000
GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR20: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR24: 0000000000000000 c0000000010452e8 c000000001045420 0000000000000080
GPR28: 000000000000001c c000000001045408 c00000003d3ce898 c00000003d8c3a30
NIP [c00000000086b824] .ohci_init+0x564/0x570
LR [c00000000086b5dc] .ohci_init+0x31c/0x570
Call Trace:
[c00000003d30f330] [c00000000086b5dc] .ohci_init+0x31c/0x570 (unreliable)
[c00000003d30f3c0] [c00000000086de58] .ohci_pci_reset+0xa8/0xb0
[c00000003d30f440] [c0000000008335ec] .usb_add_hcd+0x35c/0x9b0
[c00000003d30f510] [c00000000084ea90] .usb_hcd_pci_probe+0x320/0x510
[c00000003d30f5c0] [c0000000006c7df8] .local_pci_probe+0x68/0x140
[c00000003d30f660] [c0000000006c92a4] .pci_device_probe+0x144/0x210
[c00000003d30f710] [c00000000074cd48] .really_probe+0x2a8/0x3c0
[c00000003d30f7b0] [c00000000074d100] .driver_probe_device+0x80/0x170
[c00000003d30f840] [c00000000074d33c] .__driver_attach+0x14c/0x150
[c00000003d30f8d0] [c000000000749c6c] .bus_for_each_dev+0xac/0x100
[c00000003d30f970] [c00000000074c334] .driver_attach+0x34/0x50
[c00000003d30f9f0] [c00000000074b9b8] .bus_add_driver+0x178/0x2f0
[c00000003d30fa90] [c00000000074e560] .driver_register+0x90/0x1a0
[c00000003d30fb10] [c0000000006c707c] .__pci_register_driver+0x6c/0x90
[c00000003d30fba0] [c000000000f39f14] .ohci_pci_init+0x90/0xac
[c00000003d30fc10] [c00000000000f380] .do_one_initcall+0x70/0x2d0
[c00000003d30fce0] [c000000000edfca4] .kernel_init_freeable+0x3b8/0x4b0
[c00000003d30fdb0] [c00000000000f744] .kernel_init+0x24/0x160
[c00000003d30fe30] [c00000000000b7a4] .ret_from_kernel_thread+0x58/0x74
---
# bad: [46c163a036b41a29b0ec3c475bf97515d755ff41] Add linux-next specific files for 20180921
# good: [7876320f88802b22d4e2daf7eb027dd14175a0f8] Linux 4.19-rc4
git bisect start 'HEAD' 'v4.19-rc4'
# bad: [03b5533c4d89cc558063a98fa4201a5d2b4eb1f7] Merge remote-tracking branch 'crypto/master'
git bisect bad 03b5533c4d89cc558063a98fa4201a5d2b4eb1f7
# bad: [62c54071a46255d59e26e95528b80bf432796cb4] Merge remote-tracking branch 'v9fs/9p-next'
git bisect bad 62c54071a46255d59e26e95528b80bf432796cb4
# bad: [1eee72bfcf0977daca74e9f902956adbb4f38847] Merge remote-tracking branch 'realtek/for-next'
git bisect bad 1eee72bfcf0977daca74e9f902956adbb4f38847
# good: [30d3220045f49c707bbeec1d35423bd60488c433] Merge remote-tracking branch 'scsi-fixes/fixes'
git bisect good 30d3220045f49c707bbeec1d35423bd60488c433
# bad: [a31a9772a2aa569dc279468da4be555b737e51f8] Merge remote-tracking branch 'at91/at91-next'
git bisect bad a31a9772a2aa569dc279468da4be555b737e51f8
# bad: [3f52a0ad91857e78a5feed28327eafa11c44412c] Merge remote-tracking branch 'arm-soc/for-next'
git bisect bad 3f52a0ad91857e78a5feed28327eafa11c44412c
# good: [92c76bf9685778b2b7e5d6b4c93d74d9ef5d54a7] Merge remote-tracking branch 'leaks/leaks-next'
git bisect good 92c76bf9685778b2b7e5d6b4c93d74d9ef5d54a7
# good: [4e7afff85160ffaa236785591126cf52e11f077c] Merge branch 'fixes' into for-next
git bisect good 4e7afff85160ffaa236785591126cf52e11f077c
# bad: [5748e1b35ba28368515d850e8087929a3a65e055] MIPS: don't select DMA_MAYBE_COHERENT from DMA_PERDEV_COHERENT
git bisect bad 5748e1b35ba28368515d850e8087929a3a65e055
# good: [ccf640f4c9988653ef884672381b03b9be247bec] dma-mapping: remove dma_configure
git bisect good ccf640f4c9988653ef884672381b03b9be247bec
# bad: [46053c73685411915d3de50c5a0045beef32806b] dma-mapping: clear dev->dma_ops in arch_teardown_dma_ops
git bisect bad 46053c73685411915d3de50c5a0045beef32806b
# good: [dc3c05504d38849f77149cb962caeaedd1efa127] dma-mapping: remove dma_deconfigure
git bisect good dc3c05504d38849f77149cb962caeaedd1efa127
# first bad commit: [46053c73685411915d3de50c5a0045beef32806b] dma-mapping: clear dev->dma_ops in arch_teardown_dma_ops
^ permalink raw reply
* Re: [PATCH v1 0/6] mm: online/offline_pages called w.o. mem_hotplug_lock
From: Balbir Singh @ 2018-09-23 2:34 UTC (permalink / raw)
To: David Hildenbrand
Cc: linux-mm, linux-kernel, linux-doc, linuxppc-dev, linux-acpi,
xen-devel, devel, Andrew Morton, Benjamin Herrenschmidt,
Boris Ostrovsky, Dan Williams, Greg Kroah-Hartman, Haiyang Zhang,
Heiko Carstens, John Allen, Jonathan Corbet, Joonsoo Kim,
Juergen Gross, Kate Stewart, K. Y. Srinivasan, Len Brown,
Martin Schwidefsky, Mathieu Malaterre, Michael Ellerman,
Michael Neuling, Michal Hocko, Nathan Fontenot, Oscar Salvador,
Paul Mackerras, Pavel Tatashin, Pavel Tatashin,
Philippe Ombredanne, Rafael J. Wysocki, Rashmica Gupta,
Stephen Hemminger, Thomas Gleixner, Vlastimil Babka,
YASUAKI ISHIMATSU
In-Reply-To: <f3a13f6a-b34c-8561-884a-23fd9aa60331@redhat.com>
On Wed, Sep 19, 2018 at 09:35:07AM +0200, David Hildenbrand wrote:
> Am 19.09.18 um 03:22 schrieb Balbir Singh:
> > On Tue, Sep 18, 2018 at 01:48:16PM +0200, David Hildenbrand wrote:
> >> Reading through the code and studying how mem_hotplug_lock is to be used,
> >> I noticed that there are two places where we can end up calling
> >> device_online()/device_offline() - online_pages()/offline_pages() without
> >> the mem_hotplug_lock. And there are other places where we call
> >> device_online()/device_offline() without the device_hotplug_lock.
> >>
> >> While e.g.
> >> echo "online" > /sys/devices/system/memory/memory9/state
> >> is fine, e.g.
> >> echo 1 > /sys/devices/system/memory/memory9/online
> >> Will not take the mem_hotplug_lock. However the device_lock() and
> >> device_hotplug_lock.
> >>
> >> E.g. via memory_probe_store(), we can end up calling
> >> add_memory()->online_pages() without the device_hotplug_lock. So we can
> >> have concurrent callers in online_pages(). We e.g. touch in online_pages()
> >> basically unprotected zone->present_pages then.
> >>
> >> Looks like there is a longer history to that (see Patch #2 for details),
> >> and fixing it to work the way it was intended is not really possible. We
> >> would e.g. have to take the mem_hotplug_lock in device/base/core.c, which
> >> sounds wrong.
> >>
> >> Summary: We had a lock inversion on mem_hotplug_lock and device_lock().
> >> More details can be found in patch 3 and patch 6.
> >>
> >> I propose the general rules (documentation added in patch 6):
> >>
> >> 1. add_memory/add_memory_resource() must only be called with
> >> device_hotplug_lock.
> >> 2. remove_memory() must only be called with device_hotplug_lock. This is
> >> already documented and holds for all callers.
> >> 3. device_online()/device_offline() must only be called with
> >> device_hotplug_lock. This is already documented and true for now in core
> >> code. Other callers (related to memory hotplug) have to be fixed up.
> >> 4. mem_hotplug_lock is taken inside of add_memory/remove_memory/
> >> online_pages/offline_pages.
> >>
> >> To me, this looks way cleaner than what we have right now (and easier to
> >> verify). And looking at the documentation of remove_memory, using
> >> lock_device_hotplug also for add_memory() feels natural.
> >>
> >
> > That seems reasonable, but also implies that device_online() would hold
> > back add/remove memory, could you please also document what mode
> > read/write the locks need to be held? For example can the device_hotplug_lock
> > be held in read mode while add/remove memory via (mem_hotplug_lock) is held
> > in write mode?
>
> device_hotplug_lock is an ordinary mutex. So no option there.
>
> Only mem_hotplug_lock is a per CPU RW mutex. And as of now it only
> exists to not require get_online_mems()/put_online_mems() to take the
> device_hotplug_lock. Which is perfectly valid, because these users only
> care about memory (not any other devices) not suddenly vanish. And that
> RW lock makes things fast.
>
> Any modifications (online/offline/add/remove) require the
> mem_hotplug_lock in write.
>
> I can add some more details to documentation in patch #6.
>
> "... we should always hold the mem_hotplug_lock (via
> mem_hotplug_begin/mem_hotplug_done) in write mode to serialize memory
> hotplug" ..."
>
> "In addition, mem_hotplug_lock (in contrast to device_hotplug_lock) in
> read mode allows for a quite efficient get_online_mems/put_online_mems
> implementation, so code accessing memory can protect from that memory
> vanishing."
>
> Would that work for you?
Yes, Thanks
Balbir Singh.
^ permalink raw reply
* [PATCH -next] powerpc: Remove duplicated include from pci_32.c
From: YueHaibing @ 2018-09-23 8:12 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, Al Viro,
Paul Burton, Sinan Kaya, Mathieu Malaterre, Andrew Morton,
Stephen Rothwell, Mike Rapoport, Christophe Leroy
Cc: YueHaibing, linuxppc-dev, linux-kernel, kernel-janitors
Remove duplicated include.
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
arch/powerpc/kernel/pci_32.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/powerpc/kernel/pci_32.c b/arch/powerpc/kernel/pci_32.c
index d63be12..274bd14 100644
--- a/arch/powerpc/kernel/pci_32.c
+++ b/arch/powerpc/kernel/pci_32.c
@@ -17,7 +17,6 @@
#include <linux/of.h>
#include <linux/slab.h>
#include <linux/export.h>
-#include <linux/syscalls.h>
#include <asm/processor.h>
#include <asm/io.h>
^ permalink raw reply related
* Re: [PATCH -next] powerpc: Remove duplicated include from pci_32.c
From: Stephen Rothwell @ 2018-09-23 9:05 UTC (permalink / raw)
To: YueHaibing
Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, Al Viro,
Paul Burton, Sinan Kaya, Mathieu Malaterre, Andrew Morton,
Mike Rapoport, Christophe Leroy, linuxppc-dev, linux-kernel,
kernel-janitors
In-Reply-To: <1537690328-77353-1-git-send-email-yuehaibing@huawei.com>
[-- Attachment #1: Type: text/plain, Size: 341 bytes --]
Hi YueHaibing,
On Sun, 23 Sep 2018 08:12:08 +0000 YueHaibing <yuehaibing@huawei.com> wrote:
>
> Remove duplicated include.
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Good catch. It was added by 2 separate commits in v4.18-rc1.
Reviewed-by: Stephen Rothwell <sfr@canb.auug.org.au>
--
Cheers,
Stephen Rothwell
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH 3/3] mm: optimise pte dirty/accessed bit setting by demand based pte insertion
From: Nicholas Piggin @ 2018-09-23 9:23 UTC (permalink / raw)
To: Ley Foon Tan
Cc: Guenter Roeck, linux-mm, linux-arch, linux-kernel, linuxppc-dev,
Andrew Morton, Linus Torvalds, Ley Foon Tan, nios2-dev
In-Reply-To: <1537519325.19048.0.camel@intel.com>
On Fri, 21 Sep 2018 16:42:05 +0800
Ley Foon Tan <ley.foon.tan@intel.com> wrote:
> On Tue, 2018-09-18 at 03:53 +1000, Nicholas Piggin wrote:
> > On Wed, 5 Sep 2018 07:29:51 -0700
> > Guenter Roeck <linux@roeck-us.net> wrote:
> >
> > >
> > > Hi,
> > >
> > > On Tue, Aug 28, 2018 at 09:20:34PM +1000, Nicholas Piggin wrote:
> > > >
> > > > Similarly to the previous patch, this tries to optimise
> > > > dirty/accessed
> > > > bits in ptes to avoid access costs of hardware setting them.
> > > >
> > > This patch results in silent nios2 boot failures, silent meaning
> > > that
> > > the boot stalls.
> > Okay I just got back to looking at this. The reason for the hang is
> > I think a bug in the nios2 TLB code, but maybe other archs have
> > similar
> > issues.
> >
> > In case of a missing / !present Linux pte, nios2 installs a TLB entry
> > with no permissions via its fast TLB exception handler (software TLB
> > fill). Then it relies on that causing a TLB permission exception in a
> > slower handler that calls handle_mm_fault to set the Linux pte and
> > flushes the old TLB. Then the fast exception handler will find the
> > new
> > Linux pte.
> >
> > With this patch, nios2 has a case where handle_mm_fault does not
> > flush
> > the old TLB, which results in the TLB permission exception
> > continually
> > being retried.
> >
> > What happens now is that fault paths like do_read_fault will install
> > a
> > Linux pte with the young bit clear and return. That will cause nios2
> > to
> > fault again but this time go down the bottom of handle_pte_fault and
> > to
> > the access flags update with the young bit set. The young bit is seen
> > to
> > be different, so that causes ptep_set_access_flags to do a TLB flush
> > and
> > that finally allows the fast TLB handler to fire and pick up the new
> > Linux pte.
> >
> > With this patch, the young bit is set in the first handle_mm_fault,
> > so
> > the second handle_mm_fault no longer sees the ptes are different and
> > does not flush the TLB. The spurious fault handler also does not
> > flush
> > them unless FAULT_FLAG_WRITE is set.
> >
> > What nios2 should do is invalidate the TLB in update_mmu_cache. What
> > it
> > *really* should do is install the new TLB entry, I have some patches
> > to
> > make that work in qemu I can submit. But I would like to try getting
> > these dirty/accessed bit optimisation in 4.20, so I will send a
> > simple
> > path to just do the TLB invalidate that could go in Andrew's git
> > tree.
> >
> > Is that agreeable with the nios2 maintainers?
> >
> > Thanks,
> > Nick
> >
> Hi
>
> Do you have patches to test?
I've been working on some, it has taken longer than I expected, I'll
hopefully have something to send out by tomorrow.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH v3 6/9] kbuild: consolidate Devicetree dtb build rules
From: Masahiro Yamada @ 2018-09-23 10:31 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Rob Herring,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux Kernel Mailing List, Frank Rowand, Michal Marek,
Vineet Gupta, Russell King, Catalin Marinas, Yoshinori Sato,
Michal Simek, Ralf Baechle, James Hogan, Benjamin Herrenschmidt,
Paul Mackerras, Michael Ellerman, Chris Zankel, Max Filippov,
linux-kbuild, arcml, Linux ARM,
moderated list:H8/300 ARCHITECTURE, Linux MIPS Mailing List,
nios2-dev, linuxppc-dev, linux-xtensa, Will Deacon, Paul Burton,
Ley Foon Tan
In-Reply-To: <CAMuHMdWEnoh97_jiDWMq=ke4PrhSFbToYnx91CPLBuq3mOGzoQ@mail.gmail.com>
2018-09-13 11:51 GMT-04:00 Geert Uytterhoeven <geert@linux-m68k.org>:
> Hi Yamada-san,
>
> On Wed, Sep 12, 2018 at 3:02 AM Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
>> 2018-09-12 0:40 GMT+09:00 Rob Herring <robh@kernel.org>:
>> > On Mon, Sep 10, 2018 at 10:04 AM Rob Herring <robh@kernel.org> wrote:
>> >> There is nothing arch specific about building dtb files other than their
>> >> location under /arch/*/boot/dts/. Keeping each arch aligned is a pain.
>> >> The dependencies and supported targets are all slightly different.
>> >> Also, a cross-compiler for each arch is needed, but really the host
>> >> compiler preprocessor is perfectly fine for building dtbs. Move the
>> >> build rules to a common location and remove the arch specific ones. This
>> >> is done in a single step to avoid warnings about overriding rules.
>> >>
>> >> The build dependencies had been a mixture of 'scripts' and/or 'prepare'.
>> >> These pull in several dependencies some of which need a target compiler
>> >> (specifically devicetable-offsets.h) and aren't needed to build dtbs.
>> >> All that is really needed is dtc, so adjust the dependencies to only be
>> >> dtc.
>> >>
>> >> This change enables support 'dtbs_install' on some arches which were
>> >> missing the target.
>> >
>> > [...]
>> >
>> >> @@ -1215,6 +1215,33 @@ kselftest-merge:
>> >> $(srctree)/tools/testing/selftests/*/config
>> >> +$(Q)$(MAKE) -f $(srctree)/Makefile olddefconfig
>> >>
>> >> +# ---------------------------------------------------------------------------
>> >> +# Devicetree files
>> >> +
>> >> +ifneq ($(wildcard $(srctree)/arch/$(SRCARCH)/boot/dts/),)
>> >> +dtstree := arch/$(SRCARCH)/boot/dts
>> >> +endif
>> >> +
>> >> +ifdef CONFIG_OF_EARLY_FLATTREE
>> >
>> > This can be true when dtstree is unset. So this line should be this
>> > instead to fix the 0-day reported error:
>> >
>> > ifneq ($(dtstree),)
>> >
>> >> +
>> >> +%.dtb : scripts_dtc
>> >> + $(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@
>> >> +
>> >> +PHONY += dtbs dtbs_install
>> >> +dtbs: scripts_dtc
>> >> + $(Q)$(MAKE) $(build)=$(dtstree)
>> >> +
>> >> +dtbs_install: dtbs
>> >> + $(Q)$(MAKE) $(dtbinst)=$(dtstree)
>> >> +
>> >> +all: dtbs
>> >> +
>> >> +endif
>>
>>
>> Ah, right.
>> Even x86 can enable OF and OF_UNITTEST.
>>
>>
>>
>> Another solution might be,
>> guard it by 'depends on ARCH_SUPPORTS_OF'.
>>
>>
>>
>> This is actually what ACPI does.
>>
>> menuconfig ACPI
>> bool "ACPI (Advanced Configuration and Power Interface) Support"
>> depends on ARCH_SUPPORTS_ACPI
>> ...
>
> ACPI is a real platform feature, as it depends on firmware.
>
> CONFIG_OF can be enabled, and DT overlays can be loaded, on any platform,
> even if it has ACPI ;-)
>
OK, understood.
Thanks!
--
Best Regards
Masahiro Yamada
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox