* Re: [PATCH V2 0/3] mm/debug: Add more arch page table helper tests
From: Anshuman Khandual @ 2020-04-09 1:06 UTC (permalink / raw)
To: Gerald Schaefer
Cc: linux-doc, Heiko Carstens, linux-mm, Paul Mackerras,
H. Peter Anvin, linux-riscv, Will Deacon, linux-arch, linux-s390,
Jonathan Corbet, x86, Mike Rapoport, Christian Borntraeger,
Ingo Molnar, Catalin Marinas, linux-snps-arc, Vasily Gorbik,
Borislav Petkov, Paul Walmsley, Kirill A . Shutemov,
Thomas Gleixner, linux-arm-kernel, Vineet Gupta, linux-kernel,
Palmer Dabbelt, Andrew Morton, linuxppc-dev
In-Reply-To: <20200408141500.75b2e1a7@thinkpad>
On 04/08/2020 05:45 PM, Gerald Schaefer wrote:
> On Wed, 8 Apr 2020 12:41:51 +0530
> Anshuman Khandual <anshuman.khandual@arm.com> wrote:
>
> [...]
>>>
>>>>
>>>> Some thing like this instead.
>>>>
>>>> pte_t pte = READ_ONCE(*ptep);
>>>> pte = pte_mkhuge(__pte((pte_val(pte) | RANDOM_ORVALUE) & PMD_MASK));
>>>>
>>>> We cannot use mk_pte_phys() as it is defined only on some platforms
>>>> without any generic fallback for others.
>>>
>>> Oh, didn't know that, sorry. What about using mk_pte() instead, at least
>>> it would result in a present pte:
>>>
>>> pte = pte_mkhuge(mk_pte(phys_to_page(RANDOM_ORVALUE & PMD_MASK), prot));
>>
>> Lets use mk_pte() here but can we do this instead
>>
>> paddr = (__pfn_to_phys(pfn) | RANDOM_ORVALUE) & PMD_MASK;
>> pte = pte_mkhuge(mk_pte(phys_to_page(paddr), prot));
>>
>
> Sure, that will also work.
>
> BTW, this RANDOM_ORVALUE is not really very random, the way it is
> defined. For s390 we already changed it to mask out some arch bits,
> but I guess there are other archs and bits that would always be
> set with this "not so random" value, and I wonder if/how that would
> affect all the tests using this value, see also below.
RANDOM_ORVALUE is a constant which was added in order to make sure
that the page table entries should have some non-zero value before
getting called with pxx_clear() and followed by a pxx_none() check.
This is currently used only in pxx_clear_tests() tests. Hence there
is no impact for the existing tests.
>
>>>
>>> And if you also want to do some with the existing value, which seems
>>> to be an empty pte, then maybe just check if writing and reading that
>>> value with set_huge_pte_at() / huge_ptep_get() returns the same,
>>> i.e. initially w/o RANDOM_ORVALUE.
>>>
>>> So, in combination, like this (BTW, why is the barrier() needed, it
>>> is not used for the other set_huge_pte_at() calls later?):
>>
>> Ahh missed, will add them. Earlier we faced problem without it after
>> set_pte_at() for a test on powerpc (64) platform. Hence just added it
>> here to be extra careful.
>>
>>>
>>> @@ -733,24 +733,28 @@ static void __init hugetlb_advanced_test
>>> struct page *page = pfn_to_page(pfn);
>>> pte_t pte = READ_ONCE(*ptep);
>>>
>>> - pte = __pte(pte_val(pte) | RANDOM_ORVALUE);
>>> + set_huge_pte_at(mm, vaddr, ptep, pte);
>>> + WARN_ON(!pte_same(pte, huge_ptep_get(ptep)));
>>> +
>>> + pte = pte_mkhuge(mk_pte(phys_to_page(RANDOM_ORVALUE & PMD_MASK), prot));
>>> set_huge_pte_at(mm, vaddr, ptep, pte);
>>> barrier();
>>> WARN_ON(!pte_same(pte, huge_ptep_get(ptep)));
>>>
>>> This would actually add a new test "write empty pte with
>>> set_huge_pte_at(), then verify with huge_ptep_get()", which happens
>>> to trigger a warning on s390 :-)
>>
>> On arm64 as well which checks for pte_present() in set_huge_pte_at().
>> But PTE present check is not really present in each set_huge_pte_at()
>> implementation especially without __HAVE_ARCH_HUGE_SET_HUGE_PTE_AT.
>> Hence wondering if we should add this new test here which will keep
>> giving warnings on s390 and arm64 (at the least).
>
> Hmm, interesting. I forgot about huge swap / migration, which is not
> (and probably cannot be) supported on s390. The pte_present() check
> on arm64 seems to check for such huge swap / migration entries,
> according to the comment.
>
> The new test "write empty pte with set_huge_pte_at(), then verify
> with huge_ptep_get()" would then probably trigger the
> WARN_ON(!pte_present(pte)) in arm64 code. So I guess "writing empty
> ptes with set_huge_pte_at()" is not really a valid use case in practice,
> or else you would have seen this warning before. In that case, it
> might not be a good idea to add this test.
Got it.
>
> I also do wonder now, why the original test with
> "pte = __pte(pte_val(pte) | RANDOM_ORVALUE);"
> did not also trigger that warning on arm64. On s390 this test failed
> exactly because the constructed pte was not present (initially empty,
> or'ing RANDOM_ORVALUE does not make it present for s390). I guess this
> just worked by chance on arm64, because the bits from RANDOM_ORVALUE
> also happened to mark the pte present for arm64.
That is correct. RANDOM_ORVALUE has got PTE_PROT_NONE bit set that makes
the PTE test for pte_present().
On arm64 platform,
#define pte_present(pte) (!!(pte_val(pte) & (PTE_VALID | PTE_PROT_NONE)))
>
> This brings us back to the question above, regarding the "randomness"
> of RANDOM_ORVALUE. Not really sure what the intention behind that was,
> but maybe it would make sense to restrict this RANDOM_ORVALUE to
> non-arch-specific bits, i.e. only bits that would be part of the
> address value within a page table entry? Or was it intentionally
> chosen to also mess with other bits?
As mentioned before, RANDOM_ORVALUE just helped make a given page table
entry contain non-zero values before getting cleared. AFAICS we should
not need RANDOM_ORVALUE for HugeTLB test here. I believe the following
'paddr' construct will just be fine instead.
paddr = __pfn_to_phys(pfn) & PMD_MASK;
pte = pte_mkhuge(mk_pte(phys_to_page(paddr), prot));
>
> Regards,
> Gerald
>
>
^ permalink raw reply
* Re: [PATCH 17/28] mm: remove the prot argument from vm_map_ram
From: Gao Xiang @ 2020-04-09 0:39 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-hyperv, David Airlie, dri-devel, linux-mm, K. Y. Srinivasan,
Sumit Semwal, linux-arch, linux-s390, Wei Liu, Stephen Hemminger,
x86, Peter Zijlstra, Laura Abbott, Nitin Gupta, Daniel Vetter,
Haiyang Zhang, linaro-mm-sig, bpf, linux-arm-kernel, Robin Murphy,
linux-kernel, Minchan Kim, iommu, Sakari Ailus, Andrew Morton,
linuxppc-dev
In-Reply-To: <20200408115926.1467567-18-hch@lst.de>
On Wed, Apr 08, 2020 at 01:59:15PM +0200, Christoph Hellwig wrote:
> This is always GFP_KERNEL - for long term mappings with other properties
> vmap should be used.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> drivers/gpu/drm/i915/gem/selftests/mock_dmabuf.c | 2 +-
> drivers/media/common/videobuf2/videobuf2-dma-sg.c | 3 +--
> drivers/media/common/videobuf2/videobuf2-vmalloc.c | 3 +--
> fs/erofs/decompressor.c | 2 +-
For EROFS part,
Acked-by: Gao Xiang <xiang@kernel.org>
Thanks,
Gao Xiang
^ permalink raw reply
* [PATCHv2 1/2] powerpc/pseries: group lmb operation and memblock's
From: Pingfan Liu @ 2020-04-09 1:56 UTC (permalink / raw)
To: linuxppc-dev
Cc: Libor Pechacek, Pingfan Liu, kexec, Paul Mackerras, Leonardo Bras,
Nathan Fontenot, Hari Bathini
This patch prepares for the incoming patch which swaps the order of KOBJ_
uevent and dt's updating.
It has no functional effect, just groups lmb operation and memblock's in
order to insert dt updating operation easily, and makes it easier to
review.
Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Hari Bathini <hbathini@linux.ibm.com>
Cc: Leonardo Bras <leonardo@linux.ibm.com>
Cc: Libor Pechacek <lpechacek@suse.cz>
Cc: Nathan Fontenot <nfont@linux.vnet.ibm.com>
To: linuxppc-dev@lists.ozlabs.org
Cc: kexec@lists.infradead.org
---
arch/powerpc/platforms/pseries/hotplug-memory.c | 26 ++++++++++++++++---------
1 file changed, 17 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
index b2cde17..4bd9004 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -377,7 +377,8 @@ static int dlpar_add_lmb(struct drmem_lmb *);
static int dlpar_remove_lmb(struct drmem_lmb *lmb)
{
unsigned long block_sz;
- int rc;
+ phys_addr_t base_addr;
+ int rc, nid;
if (!lmb_is_removable(lmb))
return -EINVAL;
@@ -386,17 +387,19 @@ static int dlpar_remove_lmb(struct drmem_lmb *lmb)
if (rc)
return rc;
+ base_addr = lmb->base_addr;
+ nid = lmb->nid;
block_sz = pseries_memory_block_size();
- __remove_memory(lmb->nid, lmb->base_addr, block_sz);
-
- /* Update memory regions for memory remove */
- memblock_remove(lmb->base_addr, block_sz);
-
invalidate_lmb_associativity_index(lmb);
lmb_clear_nid(lmb);
lmb->flags &= ~DRCONF_MEM_ASSIGNED;
+ __remove_memory(nid, base_addr, block_sz);
+
+ /* Update memory regions for memory remove */
+ memblock_remove(base_addr, block_sz);
+
return 0;
}
@@ -663,6 +666,8 @@ static int dlpar_add_lmb(struct drmem_lmb *lmb)
}
lmb_set_nid(lmb);
+ lmb->flags |= DRCONF_MEM_ASSIGNED;
+
block_sz = memory_block_size_bytes();
/* Add the memory */
@@ -674,11 +679,14 @@ static int dlpar_add_lmb(struct drmem_lmb *lmb)
rc = dlpar_online_lmb(lmb);
if (rc) {
- __remove_memory(lmb->nid, lmb->base_addr, block_sz);
+ int nid = lmb->nid;
+ phys_addr_t base_addr = lmb->base_addr;
+
invalidate_lmb_associativity_index(lmb);
lmb_clear_nid(lmb);
- } else {
- lmb->flags |= DRCONF_MEM_ASSIGNED;
+ lmb->flags &= ~DRCONF_MEM_ASSIGNED;
+
+ __remove_memory(nid, base_addr, block_sz);
}
return rc;
--
2.7.5
^ permalink raw reply related
* [PATCHv2 2/2] powerpc/pseries: update device tree before ejecting hotplug uevents
From: Pingfan Liu @ 2020-04-09 1:56 UTC (permalink / raw)
To: linuxppc-dev
Cc: Libor Pechacek, Pingfan Liu, kexec, Paul Mackerras, Leonardo Bras,
Nathan Fontenot, Hari Bathini
In-Reply-To: <1586397411-24259-1-git-send-email-kernelfans@gmail.com>
A bug is observed on pseries by taking the following steps on rhel:
-1. drmgr -c mem -r -q 5
-2. echo c > /proc/sysrq-trigger
And then, the failure looks like:
kdump: saving to /sysroot//var/crash/127.0.0.1-2020-01-16-02:06:14/
kdump: saving vmcore-dmesg.txt
kdump: saving vmcore-dmesg.txt complete
kdump: saving vmcore
Checking for memory holes : [ 0.0 %] / Checking for memory holes : [100.0 %] | Excluding unnecessary pages : [100.0 %] \ Copying data : [ 0.3 %] - eta: 38s[ 44.337636] hash-mmu: mm: Hashing failure ! EA=0x7fffba400000 access=0x8000000000000004 current=makedumpfile
[ 44.337663] hash-mmu: trap=0x300 vsid=0x13a109c ssize=1 base psize=2 psize 2 pte=0xc000000050000504
[ 44.337677] hash-mmu: mm: Hashing failure ! EA=0x7fffba400000 access=0x8000000000000004 current=makedumpfile
[ 44.337692] hash-mmu: trap=0x300 vsid=0x13a109c ssize=1 base psize=2 psize 2 pte=0xc000000050000504
[ 44.337708] makedumpfile[469]: unhandled signal 7 at 00007fffba400000 nip 00007fffbbc4d7fc lr 000000011356ca3c code 2
[ 44.338548] Core dump to |/bin/false pipe failed
/lib/kdump-lib-initramfs.sh: line 98: 469 Bus error $CORE_COLLECTOR /proc/vmcore $_mp/$KDUMP_PATH/$HOST_IP-$DATEDIR/vmcore-incomplete
kdump: saving vmcore failed
* Root cause *
After analyzing, it turns out that in the current implementation,
when hot-removing lmb, the KOBJ_REMOVE event ejects before the dt updating as
the code __remove_memory() comes before drmem_update_dt().
So in kdump kernel, when read_from_oldmem() resorts to
pSeries_lpar_hpte_insert() to install hpte, but fails with -2 due to
non-exist pfn. And finally, low_hash_fault() raise SIGBUS to process, as it
can be observed "Bus error"
From a viewpoint of listener and publisher, the publisher notifies the
listener before data is ready. This introduces a problem where udev
launches kexec-tools (due to KOBJ_REMOVE) and loads a stale dt before
updating. And in capture kernel, makedumpfile will access the memory based
on the stale dt info, and hit a SIGBUS error due to an un-existed lmb.
* Fix *
In order to fix this issue, update dt before __remove_memory(), and
accordingly the same rule in hot-add path.
This will introduce extra dt updating payload for each involved lmb when hotplug.
But it should be fine since drmem_update_dt() is memory based operation and
hotplug is not a hot path.
Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Hari Bathini <hbathini@linux.ibm.com>
Cc: Leonardo Bras <leonardo@linux.ibm.com>
Cc: Libor Pechacek <lpechacek@suse.cz>
Cc: Nathan Fontenot <nfont@linux.vnet.ibm.com>
To: linuxppc-dev@lists.ozlabs.org
Cc: kexec@lists.infradead.org
---
v1 -> v2: improve commit, and more detail about the SIGBUG failure path
arch/powerpc/platforms/pseries/hotplug-memory.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
index 4bd9004..72cd4a5 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -394,6 +394,9 @@ static int dlpar_remove_lmb(struct drmem_lmb *lmb)
invalidate_lmb_associativity_index(lmb);
lmb_clear_nid(lmb);
lmb->flags &= ~DRCONF_MEM_ASSIGNED;
+ rtas_hp_event = true;
+ drmem_update_dt();
+ rtas_hp_event = false;
__remove_memory(nid, base_addr, block_sz);
@@ -667,6 +670,9 @@ static int dlpar_add_lmb(struct drmem_lmb *lmb)
lmb_set_nid(lmb);
lmb->flags |= DRCONF_MEM_ASSIGNED;
+ rtas_hp_event = true;
+ drmem_update_dt();
+ rtas_hp_event = false;
block_sz = memory_block_size_bytes();
@@ -685,6 +691,9 @@ static int dlpar_add_lmb(struct drmem_lmb *lmb)
invalidate_lmb_associativity_index(lmb);
lmb_clear_nid(lmb);
lmb->flags &= ~DRCONF_MEM_ASSIGNED;
+ rtas_hp_event = true;
+ drmem_update_dt();
+ rtas_hp_event = false;
__remove_memory(nid, base_addr, block_sz);
}
@@ -941,12 +950,6 @@ int dlpar_memory(struct pseries_hp_errorlog *hp_elog)
break;
}
- if (!rc) {
- rtas_hp_event = true;
- rc = drmem_update_dt();
- rtas_hp_event = false;
- }
-
unlock_device_hotplug();
return rc;
}
--
2.7.5
^ permalink raw reply related
* Re: [PATCH v1 1/2] powerpc/pseries/hotplug-memory: stop checking is_mem_section_removable()
From: piliu @ 2020-04-09 2:59 UTC (permalink / raw)
To: Baoquan He, David Hildenbrand
Cc: Michal Hocko, linux-kernel, Wei Yang, linux-mm, Paul Mackerras,
Nathan Fontenot, Andrew Morton, linuxppc-dev, Oscar Salvador
In-Reply-To: <20200408024630.GQ2402@MiWiFi-R3L-srv>
On 04/08/2020 10:46 AM, Baoquan He wrote:
> Add Pingfan to CC since he usually handles ppc related bugs for RHEL.
>
> On 04/07/20 at 03:54pm, David Hildenbrand wrote:
>> In commit 53cdc1cb29e8 ("drivers/base/memory.c: indicate all memory
>> blocks as removable"), the user space interface to compute whether a memory
>> block can be offlined (exposed via
>> /sys/devices/system/memory/memoryX/removable) has effectively been
>> deprecated. We want to remove the leftovers of the kernel implementation.
>
> Pingfan, can you have a look at this change on PPC? Please feel free to
> give comments if any concern, or offer ack if it's OK to you.
>
>>
>> When offlining a memory block (mm/memory_hotplug.c:__offline_pages()),
>> we'll start by:
>> 1. Testing if it contains any holes, and reject if so
>> 2. Testing if pages belong to different zones, and reject if so
>> 3. Isolating the page range, checking if it contains any unmovable pages
>>
>> Using is_mem_section_removable() before trying to offline is not only racy,
>> it can easily result in false positives/negatives. Let's stop manually
>> checking is_mem_section_removable(), and let device_offline() handle it
>> completely instead. We can remove the racy is_mem_section_removable()
>> implementation next.
>>
>> We now take more locks (e.g., memory hotplug lock when offlining and the
>> zone lock when isolating), but maybe we should optimize that
>> implementation instead if this ever becomes a real problem (after all,
>> memory unplug is already an expensive operation). We started using
>> is_mem_section_removable() in commit 51925fb3c5c9 ("powerpc/pseries:
>> Implement memory hotplug remove in the kernel"), with the initial
>> hotremove support of lmbs.
>>
>> Cc: Nathan Fontenot <nfont@linux.vnet.ibm.com>
>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> Cc: Paul Mackerras <paulus@samba.org>
>> Cc: Michal Hocko <mhocko@suse.com>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: Oscar Salvador <osalvador@suse.de>
>> Cc: Baoquan He <bhe@redhat.com>
>> Cc: Wei Yang <richard.weiyang@gmail.com>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>> .../platforms/pseries/hotplug-memory.c | 26 +++----------------
>> 1 file changed, 3 insertions(+), 23 deletions(-)
>>
>> diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
>> index b2cde1732301..5ace2f9a277e 100644
>> --- a/arch/powerpc/platforms/pseries/hotplug-memory.c
>> +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
>> @@ -337,39 +337,19 @@ static int pseries_remove_mem_node(struct device_node *np)
>>
>> static bool lmb_is_removable(struct drmem_lmb *lmb)
>> {
>> - int i, scns_per_block;
>> - bool rc = true;
>> - unsigned long pfn, block_sz;
>> - u64 phys_addr;
>> -
>> if (!(lmb->flags & DRCONF_MEM_ASSIGNED))
>> return false;
>>
>> - block_sz = memory_block_size_bytes();
>> - scns_per_block = block_sz / MIN_MEMORY_BLOCK_SIZE;
>> - phys_addr = lmb->base_addr;
>> -
>> #ifdef CONFIG_FA_DUMP
>> /*
>> * Don't hot-remove memory that falls in fadump boot memory area
>> * and memory that is reserved for capturing old kernel memory.
>> */
>> - if (is_fadump_memory_area(phys_addr, block_sz))
>> + if (is_fadump_memory_area(lmb->base_addr, memory_block_size_bytes()))
>> return false;
>> #endif
>> -
>> - for (i = 0; i < scns_per_block; i++) {
>> - pfn = PFN_DOWN(phys_addr);
>> - if (!pfn_in_present_section(pfn)) {
>> - phys_addr += MIN_MEMORY_BLOCK_SIZE;
>> - continue;
>> - }
>> -
>> - rc = rc && is_mem_section_removable(pfn, PAGES_PER_SECTION);
>> - phys_addr += MIN_MEMORY_BLOCK_SIZE;
>> - }
>> -
>> - return rc;
>> + /* device_offline() will determine if we can actually remove this lmb */
>> + return true;
So I think here swaps the check and do sequence. At least it breaks
dlpar_memory_remove_by_count(). It is doable to remove
is_mem_section_removable(), but here should be more effort to re-arrange
the code.
Thanks,
Pingfan
>> }
>>
>> static int dlpar_add_lmb(struct drmem_lmb *);
>> --
>> 2.25.1
>>
^ permalink raw reply
* Re: Linux-next POWER9 NULL pointer NIP since 1st Apr.
From: Qian Cai @ 2020-04-09 3:40 UTC (permalink / raw)
To: Steven Rostedt, Michael Ellerman; +Cc: linuxppc-dev, LKML, Nicholas Piggin
In-Reply-To: <20200407093054.3eb23e45@gandalf.local.home>
> On Apr 7, 2020, at 9:30 AM, Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Tue, 7 Apr 2020 09:01:10 -0400
> Qian Cai <cai@lca.pw> wrote:
>
>> + Steven
>>
>>> On Apr 7, 2020, at 8:42 AM, Michael Ellerman <mpe@ellerman.id.au> wrote:
>>>
>>> Qian Cai <cai@lca.pw> writes:
>>>> Ever since 1st Apr, linux-next starts to trigger a NULL pointer NIP on POWER9 below using
>>>> this config,
>>>>
>>>> https://raw.githubusercontent.com/cailca/linux-mm/master/powerpc.config
>>>>
>>>> It takes a while to reproduce, so before I bury myself into bisecting and just send a head-up
>>>> to see if anyone spots anything obvious.
>>>>
>>>> [ 206.744625][T13224] LTP: starting fallocate04
>>>> [ 207.601583][T27684] /dev/zero: Can't open blockdev
>>>> [ 208.674301][T27684] EXT4-fs (loop0): mounting ext3 file system using the ext4 subsystem
>>>> [ 208.680347][T27684] BUG: Unable to handle kernel instruction fetch (NULL pointer?)
>>>> [ 208.680383][T27684] Faulting instruction address: 0x00000000
>>>> [ 208.680406][T27684] Oops: Kernel access of bad area, sig: 11 [#1]
>>>> [ 208.680439][T27684] LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=256 DEBUG_PAGEALLOC NUMA PowerNV
>>>> [ 208.680474][T27684] Modules linked in: ext4 crc16 mbcache jbd2 loop kvm_hv kvm ip_tables x_tables xfs sd_mod bnx2x ahci libahci mdio tg3 libata libphy firmware_class dm_mirror dm_region_hash dm_log dm_mod
>>>> [ 208.680576][T27684] CPU: 117 PID: 27684 Comm: fallocate04 Tainted: G W 5.6.0-next-20200401+ #288
>>>> [ 208.680614][T27684] NIP: 0000000000000000 LR: c0080000102c0048 CTR: 0000000000000000
>>>> [ 208.680657][T27684] REGS: c000200361def420 TRAP: 0400 Tainted: G W (5.6.0-next-20200401+)
>>>> [ 208.680700][T27684] MSR: 900000004280b033 <SF,HV,VEC,VSX,EE,FP,ME,IR,DR,RI,LE> CR: 42022228 XER: 20040000
>>>> [ 208.680760][T27684] CFAR: c00800001032c494 IRQMASK: 0
>>>> [ 208.680760][T27684] GPR00: c0000000005ac3f8 c000200361def6b0 c00000000165c200 c00020107dae0bd0
>>>> [ 208.680760][T27684] GPR04: 0000000000000000 0000000000000400 0000000000000000 0000000000000000
>>>> [ 208.680760][T27684] GPR08: c000200361def6e8 c0080000102c0040 000000007fffffff c000000001614e80
>>>> [ 208.680760][T27684] GPR12: 0000000000000000 c000201fff671280 0000000000000000 0000000000000002
>>>> [ 208.680760][T27684] GPR16: 0000000000000002 0000000000040001 c00020030f5a1000 c00020030f5a1548
>>>> [ 208.680760][T27684] GPR20: c0000000015fbad8 c00000000168c654 c000200361def818 c0000000005b4c10
>>>> [ 208.680760][T27684] GPR24: 0000000000000000 c0080000103365b8 c00020107dae0bd0 0000000000000400
>>>> [ 208.680760][T27684] GPR28: c00000000168c3a8 0000000000000000 0000000000000000 0000000000000000
>>>> [ 208.681014][T27684] NIP [0000000000000000] 0x0
>>>> [ 208.681065][T27684] LR [c0080000102c0048] ext4_iomap_end+0x8/0x30 [ext4]
>>>
>>> That LR looks like it's pointing to the return from _mcount in
>>> ext4_iomap_end(), which means we have probably crashed in ftrace
>>> somewhere.
>>>
>>> Did you have tracing enabled when you ran the test? Or does it do
>>> tracing itself?
>>
>> Yes, it run ftrace at first before running LTP to trigger it,
>>
>> https://github.com/cailca/linux-mm/blob/master/test.sh
>>
>> echo function > /sys/kernel/debug/tracing/current_tracer
>> echo nop > /sys/kernel/debug/tracing/current_tracer
>>
>> There is another crash with even non-NULL NIP, but then symbol behaves weird.
>>
>> # ./scripts/faddr2line vmlinux sysctl_net_busy_read+0x0/0x4
>> skipping sysctl_net_busy_read address at 0xc0000000016804ac due to non-function symbol of type 'D'
>>
>> [ 148.110969][T13115] LTP: starting chown04_16
>> [ 148.255048][T13380] kernel tried to execute exec-protected page (c0000000016804ac) - exploit attempt? (uid: 0)
>> [ 148.255099][T13380] BUG: Unable to handle kernel instruction fetch
>> [ 148.255122][T13380] Faulting instruction address: 0xc0000000016804ac
>> [ 148.255136][T13380] Oops: Kernel access of bad area, sig: 11 [#1]
>> [ 148.255157][T13380] LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=256 DEBUG_PAGEALLOC NUMA PowerNV
>> [ 148.255171][T13380] Modules linked in: loop kvm_hv kvm xfs sd_mod bnx2x mdio ahci tg3 libahci libphy libata firmware_class dm_mirror dm_region_hash dm_log dm_mod
>> [ 148.255213][T13380] CPU: 45 PID: 13380 Comm: chown04_16 Tainted: G W 5.6.0+ #7
>> [ 148.255236][T13380] NIP: c0000000016804ac LR: c00800000fa60408 CTR: c0000000016804ac
>> [ 148.255250][T13380] REGS: c0000010a6fafa00 TRAP: 0400 Tainted: G W (5.6.0+)
>> [ 148.255281][T13380] MSR: 9000000010009033 <SF,HV,EE,ME,IR,DR,RI,LE> CR: 84000248 XER: 20040000
>> [ 148.255310][T13380] CFAR: c00800000fa66534 IRQMASK: 0
>> [ 148.255310][T13380] GPR00: c000000000973268 c0000010a6fafc90 c000000001648200 0000000000000000
>> [ 148.255310][T13380] GPR04: c000000d8a22dc00 c0000010a6fafd30 00000000b5e98331 ffffffff00012c9f
>> [ 148.255310][T13380] GPR08: c000000d8a22dc00 0000000000000000 0000000000000000 c00000000163c520
>> [ 148.255310][T13380] GPR12: c0000000016804ac c000001ffffdad80 0000000000000000 0000000000000000
>> [ 148.255310][T13380] GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>> [ 148.255310][T13380] GPR20: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>> [ 148.255310][T13380] GPR24: 00007fff8f5e2e48 0000000000000000 c00800000fa6a488 c0000010a6fafd30
>> [ 148.255310][T13380] GPR28: 0000000000000000 000000007fffffff c00800000fa60400 c000000efd0c6780
>> [ 148.255494][T13380] NIP [c0000000016804ac] sysctl_net_busy_read+0x0/0x4
>> [ 148.255516][T13380] LR [c00800000fa60408] find_free_cb+0x8/0x30 [loop]
>> [ 148.255528][T13380] Call Trace:
>> [ 148.255538][T13380] [c0000010a6fafc90] [c0000000009732c0] idr_for_each+0xf0/0x170 (unreliable)
>> [ 148.255572][T13380] [c0000010a6fafd10] [c00800000fa626c4] loop_lookup.part.1+0x4c/0xb0 [loop]
>> [ 148.255597][T13380] [c0000010a6fafd50] [c00800000fa634d8] loop_control_ioctl+0x120/0x1d0 [loop]
>> [ 148.255623][T13380] [c0000010a6fafdb0] [c0000000004ddc08] ksys_ioctl+0xd8/0x130
>> [ 148.255636][T13380] [c0000010a6fafe00] [c0000000004ddc88] sys_ioctl+0x28/0x40
>> [ 148.255669][T13380] [c0000010a6fafe20] [c00000000000b378] system_call+0x5c/0x68
>> [ 148.255699][T13380] Instruction dump:
>> [ 148.255718][T13380] XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX
>> [ 148.255744][T13380] XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX
>> [ 148.255772][T13380] ---[ end trace a5894a74208c22ec ]---
>> [ 148.576663][T13380]
>> [ 149.576765][T13380] Kernel panic - not syncing: Fatal exception
>>
>> The bisect so far indicated the bad ones always have this,
>>
>> aa1a8ce53332 Merge tag 'trace-v5.7' of git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace
>>
>> I’ll go to bisect some more but it is going to take a while.
>>
>> $ git log --oneline 4c205c84e249..8e99cf91b99b
>> 8e99cf91b99b tracing: Do not allocate buffer in trace_find_next_entry() in atomic
>> 2ab2a0924b99 tracing: Add documentation on set_ftrace_notrace_pid and set_event_notrace_pid
>> ebed9628f5c2 selftests/ftrace: Add test to test new set_event_notrace_pid file
>> ed8839e072b8 selftests/ftrace: Add test to test new set_ftrace_notrace_pid file
>> 276836260301 tracing: Create set_event_notrace_pid to not trace tasks
>
>> b3b1e6ededa4 ftrace: Create set_ftrace_notrace_pid to not trace tasks
>> 717e3f5ebc82 ftrace: Make function trace pid filtering a bit more exact
>
> If it is affecting function tracing, it is probably one of the above two
> commits.
I tested reverting those 3 commits but it does NOT help.
276836260301 tracing: Create set_event_notrace_pid to not trace tasks
b3b1e6ededa4 ftrace: Create set_ftrace_notrace_pid to not trace tasks
717e3f5ebc82 ftrace: Make function trace pid filtering a bit more exact
So, it is either one of those is bad.
9c94b39560c3 Merge tag 'ext4_for_linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4
aa1a8ce53332 Merge tag 'trace-v5.7' of git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace
At this point, I suspect this one in ext4_for_linus,
ac58e4fb03f9 ext4: move ext4 bmap to use iomap infrastructure
given the first a few days, it was crashed due to this new ext4_bmap -> iomap_bmap
callsite from the above commit,
LR: ext4_iomap_end
The next few days, even though it starts to crash at,
LR: find_free_cb
by the LTP chown04_16 which is still somewhat ext4 related.
# /opt/ltp/testcases/bin/chown04_16
chown04_16 0 TINFO : Found free device 0 '/dev/loop0'
chown04_16 0 TINFO : Formatting /dev/loop0 with ext2 opts='' extra opts=''
mke2fs 1.45.4 (23-Sep-2019)
chown04_16 1 TBROK : safe_macros.c:764: chown04.c:175: mount(/dev/loop0, mntpoint, ext2, 1, (nil)) failed: errno=ENODEV(19): No such device
chown04_16 2 TBROK : safe_macros.c:764: Remaining cases broken
Is it possible that iomap somewhat corrupt the NIP?
In the next a few hours, I should be able to tell if the trace merge commit is clear.
>
> -- Steve
>
>
>> 6a13a0d7b4d1 ftrace/kprobe: Show the maxactive number on kprobe_events
>> 8a815e6b8b88 tracing: Have the document reflect that the trace file keeps tracing enabled
>> c9b7a4a72ff6 ring-buffer/tracing: Have iterator acknowledge dropped events
>> 06e0a548bad0 tracing: Do not disable tracing when reading the trace file
>> 1039221cc278 ring-buffer: Do not disable recording when there is an iterator
>> 07b8b10ec94f ring-buffer: Make resize disable per cpu buffer instead of total buffer
>> 153368ce1bd0 ring-buffer: Optimize rb_iter_head_event()
>> ff84c50cfb4b ring-buffer: Do not die if rb_iter_peek() fails more than thrice
>> 785888c544e0 ring-buffer: Have rb_iter_head_event() handle concurrent writer
>> 28e3fc56a471 ring-buffer: Add page_stamp to iterator for synchronization
>> bc1a72afdc4a ring-buffer: Rename ring_buffer_read() to read_buffer_iter_advance()
>> ead6ecfddea5 ring-buffer: Have ring_buffer_empty() not depend on tracing stopped
>> ff895103a84a tracing: Save off entry when peeking at next entry
>> 8c77f0ba4156 selftest/ftrace: Fix function trigger test to handle trace not disabling the tracer
>> bf2cbe044da2 tracing: Use address-of operator on section symbols
>> bbd9d05618a6 gpu/trace: add a gpu total memory usage tracepoint
>> 89b74cac7834 tools/bootconfig: Show line and column in parse error
>> 306b69dce926 bootconfig: Support O=<builddir> option
>> 5412e0b763e0 tracing: Remove unused TRACE_BUFFER bits
>> b396bfdebffc tracing: Have hwlat ts be first instance and record count of instances
>>
>>
>>>
>>> cheers
>>>
>>>> [ 208.681091][T27684] Call Trace:
>>>> [ 208.681129][T27684] [c000200361def6b0] [c0000000005ac3bc] iomap_apply+0x20c/0x920 (unreliable) iomap_apply at fs/iomap/apply.c:80 (discriminator 4)
>>>> [ 208.681173][T27684] [c000200361def7f0] [c0000000005b4adc] iomap_bmap+0xfc/0x160 iomap_bmap at fs/iomap/fiemap.c:142
>>>> [ 208.681228][T27684] [c000200361def850] [c0080000102c2c1c] ext4_bmap+0xa4/0x180 [ext4] ext4_bmap at fs/ext4/inode.c:3213
>>>> [ 208.681260][T27684] [c000200361def890] [c0000000004f71fc] bmap+0x4c/0x80
>>>> [ 208.681281][T27684] [c000200361def8c0] [c00800000fdb0acc] jbd2_journal_init_inode+0x44/0x1a0 [jbd2] jbd2_journal_init_inode at fs/jbd2/journal.c:1255
>>>> [ 208.681326][T27684] [c000200361def960] [c00800001031c808] ext4_load_journal+0x440/0x860 [ext4]
>>>> [ 208.681371][T27684] [c000200361defa30] [c008000010322a14] ext4_fill_super+0x342c/0x3ab0 [ext4]
>>>> [ 208.681414][T27684] [c000200361defba0] [c0000000004cb0bc] mount_bdev+0x25c/0x290
>>>> [ 208.681478][T27684] [c000200361defc40] [c008000010310250] ext4_mount+0x28/0x50 [ext4]
>>>> [ 208.681520][T27684] [c000200361defc60] [c00000000053242c] legacy_get_tree+0x4c/0xb0
>>>> [ 208.681556][T27684] [c000200361defc90] [c0000000004c864c] vfs_get_tree+0x4c/0x130
>>>> [ 208.681593][T27684] [c000200361defd00] [c00000000050a1c8] do_mount+0xa18/0xc50
>>>> [ 208.681641][T27684] [c000200361defdd0] [c00000000050a9a8] sys_mount+0x158/0x180
>>>> [ 208.681679][T27684] [c000200361defe20] [c00000000000b3f8] system_call+0x5c/0x68
>>>> [ 208.681726][T27684] Instruction dump:
>>>> [ 208.681747][T27684] XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX
>>>> [ 208.681797][T27684] XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX
>>>> [ 208.681839][T27684] ---[ end trace 4e9e2bab7f1d4048 ]---
>>>> [ 208.802259][T27684]
>>>> [ 209.802373][T27684] Kernel panic - not syncing: Fatal exception
>>>>
>>>> [ 215.281666][T16896] LTP: starting chown04_16
>>>> [ 215.424203][T18297] BUG: Unable to handle kernel instruction fetch (NULL pointer?)
>>>> [ 215.424289][T18297] Faulting instruction address: 0x00000000
>>>> [ 215.424313][T18297] Oops: Kernel access of bad area, sig: 11 [#1]
>>>> [ 215.424341][T18297] LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=256 DEBUG_PAGEALLOC NUMA PowerNV
>>>> [ 215.424383][T18297] Modules linked in: loop kvm_hv kvm ip_tables x_tables xfs sd_mod bnx2x mdio tg3 ahci libahci libphy libata firmware_class dm_mirror dm_region_hash dm_log dm_mod
>>>> [ 215.424459][T18297] CPU: 85 PID: 18297 Comm: chown04_16 Tainted: G W 5.6.0-next-20200405+ #3
>>>> [ 215.424489][T18297] NIP: 0000000000000000 LR: c00800000fbc0408 CTR: 0000000000000000
>>>> [ 215.424530][T18297] REGS: c000200b8606f990 TRAP: 0400 Tainted: G W (5.6.0-next-20200405+)
>>>> [ 215.424570][T18297] MSR: 9000000040009033 <SF,HV,EE,ME,IR,DR,RI,LE> CR: 84000248 XER: 20040000
>>>> [ 215.424619][T18297] CFAR: c00800000fbc64f4 IRQMASK: 0
>>>> [ 215.424619][T18297] GPR00: c0000000006c2238 c000200b8606fc20 c00000000165ce00 0000000000000000
>>>> [ 215.424619][T18297] GPR04: c000201a58106400 c000200b8606fcc0 000000005f037e7d ffffffff00013bfb
>>>> [ 215.424619][T18297] GPR08: c000201a58106400 0000000000000000 0000000000000000 c000000001652ee0
>>>> [ 215.424619][T18297] GPR12: 0000000000000000 c000201fff69a600 0000000000000000 0000000000000000
>>>> [ 215.424619][T18297] GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>>>> [ 215.424619][T18297] GPR20: 0000000000000000 0000000000000000 0000000000000000 0000000000000007
>>>> [ 215.424619][T18297] GPR24: 0000000000000000 0000000000000000 c00800000fbc8688 c000200b8606fcc0
>>>> [ 215.424619][T18297] GPR28: 0000000000000000 000000007fffffff c00800000fbc0400 c00020068b8c0e70
>>>> [ 215.424914][T18297] NIP [0000000000000000] 0x0
>>>> [ 215.424953][T18297] LR [c00800000fbc0408] find_free_cb+0x8/0x30 [loop]
>>>> find_free_cb at drivers/block/loop.c:2129
>>>> [ 215.424997][T18297] Call Trace:
>>>> [ 215.425036][T18297] [c000200b8606fc20] [c0000000006c2290] idr_for_each+0xf0/0x170 (unreliable)
>>>> [ 215.425073][T18297] [c000200b8606fca0] [c00800000fbc2744] loop_lookup.part.2+0x4c/0xb0 [loop]
>>>> loop_lookup at drivers/block/loop.c:2144
>>>> [ 215.425105][T18297] [c000200b8606fce0] [c00800000fbc3558] loop_control_ioctl+0x120/0x1d0 [loop]
>>>> [ 215.425149][T18297] [c000200b8606fd40] [c0000000004eb688] ksys_ioctl+0xd8/0x130
>>>> [ 215.425190][T18297] [c000200b8606fd90] [c0000000004eb708] sys_ioctl+0x28/0x40
>>>> [ 215.425233][T18297] [c000200b8606fdb0] [c00000000003cc30] system_call_exception+0x110/0x1e0
>>>> [ 215.425274][T18297] [c000200b8606fe20] [c00000000000c9f0] system_call_common+0xf0/0x278
>>>> [ 215.425314][T18297] Instruction dump:
>>>> [ 215.425338][T18297] XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX
>>>> [ 215.425374][T18297] XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX
>>>> [ 215.425422][T18297] ---[ end trace ebed248fad431966 ]---
>>>> [ 215.642114][T18297]
>>>> [ 216.642220][T18297] Kernel panic - not syncing: Fatal exception
^ permalink raw reply
* Re: [RFC PATCH v0 0/5] powerpc/mm/radix: Memory unplug fixes
From: Bharata B Rao @ 2020-04-09 4:31 UTC (permalink / raw)
To: linuxppc-dev; +Cc: leonardo, aneesh.kumar, npiggin
In-Reply-To: <20200406034925.22586-1-bharata@linux.ibm.com>
On Mon, Apr 06, 2020 at 09:19:20AM +0530, Bharata B Rao wrote:
> Memory unplug has a few bugs which I had attempted to fix ealier
> at https://lists.ozlabs.org/pipermail/linuxppc-dev/2019-July/194087.html
>
> Now with Leonardo's patch for PAPR changes that add a separate flag bit
> to LMB flags for explicitly identifying hot-removable memory
> (https://lore.kernel.org/linuxppc-dev/f55a7b65a43cc9dc7b22385cf9960f8b11d5ce2e.camel@linux.ibm.com/T/#t),
> a few other issues around memory unplug on radix can be fixed. This
> series is a combination of those fixes.
>
> This series works on top of above mentioned Leonardo's patch.
>
> Bharata B Rao (5):
> powerpc/pseries/hotplug-memory: Set DRCONF_MEM_HOTREMOVABLE for
> hot-plugged mem
> powerpc/mm/radix: Create separate mappings for hot-plugged memory
> powerpc/mm/radix: Fix PTE/PMD fragment count for early page table
> mappings
> powerpc/mm/radix: Free PUD table when freeing pagetable
> powerpc/mm/radix: Remove split_kernel_mapping()
3/5 in this series fixes long-standing bug and multiple versions of it
has been posted outside of this series earlier.
4/5 fixes a memory leak.
I included the above tow in this series because with the patches to
explicitly mark the hotplugged memory (1/5 and 2/5), reproducing the bug
fixed by 3/5 becomes easier.
Hence 3/5 and 4/5 can be considered as standalone fixes too.
Regards,
Bharata.
^ permalink raw reply
* Re: [PATCH v5 05/21] powerpc: Use a function for getting the instruction op code
From: Jordan Niethe @ 2020-04-09 4:48 UTC (permalink / raw)
To: Segher Boessenkool
Cc: Alistair Popple, Daniel Axtens, linuxppc-dev, Nicholas Piggin,
Balamuruhan S
In-Reply-To: <20200408182109.GH26902@gate.crashing.org>
On Thu, Apr 9, 2020 at 4:21 AM Segher Boessenkool
<segher@kernel.crashing.org> wrote:
>
> Hi!
>
> On Mon, Apr 06, 2020 at 06:09:20PM +1000, Jordan Niethe wrote:
> > +static inline int ppc_inst_opcode(u32 x)
> > +{
> > + return x >> 26;
> > +}
>
> Maybe you should have "primary opcode" in this function name?
Thanks, that is a good idea.
>
>
> Segher
^ permalink raw reply
* Re: [PATCH v5 13/21] powerpc/xmon: Use a function for reading instructions
From: Balamuruhan S @ 2020-04-09 5:04 UTC (permalink / raw)
To: Jordan Niethe
Cc: Alistair Popple, linuxppc-dev, Nicholas Piggin, Daniel Axtens
In-Reply-To: <CACzsE9r8hAKGwN1reVo00UfO5UiONo2GpUdMsNgqgAJa=LDbSg@mail.gmail.com>
On Wed, 2020-04-08 at 12:18 +1000, Jordan Niethe wrote:
> On Tue, Apr 7, 2020 at 9:31 PM Balamuruhan S <bala24@linux.ibm.com> wrote:
> > On Mon, 2020-04-06 at 18:09 +1000, Jordan Niethe wrote:
> > > Currently in xmon, mread() is used for reading instructions. In
> > > preparation for prefixed instructions, create and use a new function,
> > > mread_instr(), especially for reading instructions.
> > >
> > > Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> > > ---
> > > v5: New to series, seperated from "Add prefixed instructions to
> > > instruction data type"
> > > ---
> > > arch/powerpc/xmon/xmon.c | 24 ++++++++++++++++++++----
> > > 1 file changed, 20 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> > > index 5e3949322a6c..6f4cf01a58c1 100644
> > > --- a/arch/powerpc/xmon/xmon.c
> > > +++ b/arch/powerpc/xmon/xmon.c
> > > @@ -125,6 +125,7 @@ extern unsigned int bpt_table[NBPTS * BPT_WORDS];
> > > static int cmds(struct pt_regs *);
> > > static int mread(unsigned long, void *, int);
> > > static int mwrite(unsigned long, void *, int);
> > > +static int mread_instr(unsigned long, struct ppc_inst *);
> > > static int handle_fault(struct pt_regs *);
> > > static void byterev(unsigned char *, int);
> > > static void memex(void);
> > > @@ -899,7 +900,7 @@ static void insert_bpts(void)
> > > for (i = 0; i < NBPTS; ++i, ++bp) {
> > > if ((bp->enabled & (BP_TRAP|BP_CIABR)) == 0)
> > > continue;
> > > - if (mread(bp->address, &instr, 4) != 4) {
> > > + if (!mread_instr(bp->address, &instr)) {
> >
> > Are these checks made based on whether `ppc_inst_len()` returns bool from
> > mread_instr() ?
> No, it was meant to be the length itself returned with a length of 0
> indicating an error. I will need to fix that.
I doubt it would return 0, whether we read instruction or not ppc_inst_len()
would always return sizeof(struct ppc_inst).
can we do something like,
static int
mread_instr(unsigned long adrs, struct ppc_inst *instr)
{
int size = 0;
if (setjmp(bus_error_jmp) == 0) {
catch_memory_errors = 1;
sync();
*instr = ppc_inst_read((struct ppc_inst *)adrs);
sync();
/* wait a little while to see if we get a machine check */
__delay(200);
size = ppc_inst_len(instr);
}
catch_memory_errors = 0;
return size;
}
-- Bala
> > -- Bala
> >
> >
> > > printf("Couldn't read instruction at %lx, "
> > > "disabling breakpoint there\n", bp-
> > > >address);
> > > bp->enabled = 0;
> > > @@ -949,7 +950,7 @@ static void remove_bpts(void)
> > > for (i = 0; i < NBPTS; ++i, ++bp) {
> > > if ((bp->enabled & (BP_TRAP|BP_CIABR)) != BP_TRAP)
> > > continue;
> > > - if (mread(bp->address, &instr, 4) == 4
> > > + if (mread_instr(bp->address, &instr)
> > > && ppc_inst_equal(instr, ppc_inst(bpinstr))
> > > && patch_instruction(
> > > (struct ppc_inst *)bp->address, ppc_inst_read(bp-
> > > > instr)) != 0)
> > > @@ -1165,7 +1166,7 @@ static int do_step(struct pt_regs *regs)
> > > force_enable_xmon();
> > > /* check we are in 64-bit kernel mode, translation enabled */
> > > if ((regs->msr & (MSR_64BIT|MSR_PR|MSR_IR)) == (MSR_64BIT|MSR_IR))
> > > {
> > > - if (mread(regs->nip, &instr, 4) == 4) {
> > > + if (mread_instr(regs->nip, &instr)) {
> > > stepped = emulate_step(regs, instr);
> > > if (stepped < 0) {
> > > printf("Couldn't single-step %s
> > > instruction\n",
> > > @@ -1332,7 +1333,7 @@ static long check_bp_loc(unsigned long addr)
> > > printf("Breakpoints may only be placed at kernel
> > > addresses\n");
> > > return 0;
> > > }
> > > - if (!mread(addr, &instr, sizeof(instr))) {
> > > + if (!mread_instr(addr, &instr)) {
> > > printf("Can't read instruction at address %lx\n", addr);
> > > return 0;
> > > }
> > > @@ -2125,6 +2126,21 @@ mwrite(unsigned long adrs, void *buf, int size)
> > > return n;
> > > }
> > >
> > > +static int
> > > +mread_instr(unsigned long adrs, struct ppc_inst *instr)
> > > +{
> > > + if (setjmp(bus_error_jmp) == 0) {
> > > + catch_memory_errors = 1;
> > > + sync();
> > > + *instr = ppc_inst_read((struct ppc_inst *)adrs);
> > > + sync();
> > > + /* wait a little while to see if we get a machine check */
> > > + __delay(200);
> > > + }
> > > + catch_memory_errors = 0;
> > > + return ppc_inst_len(*instr);
> > > +}
> > > +
> > > static int fault_type;
> > > static int fault_except;
> > > static char *fault_chars[] = { "--", "**", "##" };
^ permalink raw reply
* Re: [PATCH v5 13/21] powerpc/xmon: Use a function for reading instructions
From: Jordan Niethe @ 2020-04-09 5:14 UTC (permalink / raw)
To: Balamuruhan S
Cc: Alistair Popple, linuxppc-dev, Nicholas Piggin, Daniel Axtens
In-Reply-To: <1e58ff6f13a8fe730c57a73ac0d7b0548277d17c.camel@linux.ibm.com>
On Thu, Apr 9, 2020 at 3:04 PM Balamuruhan S <bala24@linux.ibm.com> wrote:
>
> On Wed, 2020-04-08 at 12:18 +1000, Jordan Niethe wrote:
> > On Tue, Apr 7, 2020 at 9:31 PM Balamuruhan S <bala24@linux.ibm.com> wrote:
> > > On Mon, 2020-04-06 at 18:09 +1000, Jordan Niethe wrote:
> > > > Currently in xmon, mread() is used for reading instructions. In
> > > > preparation for prefixed instructions, create and use a new function,
> > > > mread_instr(), especially for reading instructions.
> > > >
> > > > Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> > > > ---
> > > > v5: New to series, seperated from "Add prefixed instructions to
> > > > instruction data type"
> > > > ---
> > > > arch/powerpc/xmon/xmon.c | 24 ++++++++++++++++++++----
> > > > 1 file changed, 20 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> > > > index 5e3949322a6c..6f4cf01a58c1 100644
> > > > --- a/arch/powerpc/xmon/xmon.c
> > > > +++ b/arch/powerpc/xmon/xmon.c
> > > > @@ -125,6 +125,7 @@ extern unsigned int bpt_table[NBPTS * BPT_WORDS];
> > > > static int cmds(struct pt_regs *);
> > > > static int mread(unsigned long, void *, int);
> > > > static int mwrite(unsigned long, void *, int);
> > > > +static int mread_instr(unsigned long, struct ppc_inst *);
> > > > static int handle_fault(struct pt_regs *);
> > > > static void byterev(unsigned char *, int);
> > > > static void memex(void);
> > > > @@ -899,7 +900,7 @@ static void insert_bpts(void)
> > > > for (i = 0; i < NBPTS; ++i, ++bp) {
> > > > if ((bp->enabled & (BP_TRAP|BP_CIABR)) == 0)
> > > > continue;
> > > > - if (mread(bp->address, &instr, 4) != 4) {
> > > > + if (!mread_instr(bp->address, &instr)) {
> > >
> > > Are these checks made based on whether `ppc_inst_len()` returns bool from
> > > mread_instr() ?
> > No, it was meant to be the length itself returned with a length of 0
> > indicating an error. I will need to fix that.
>
>
> I doubt it would return 0, whether we read instruction or not ppc_inst_len()
> would always return sizeof(struct ppc_inst).
Yes, sorry I meant I would have to change the function so that it
would return 0.
>
> can we do something like,
>
> static int
> mread_instr(unsigned long adrs, struct ppc_inst *instr)
> {
> int size = 0;
> if (setjmp(bus_error_jmp) == 0) {
> catch_memory_errors = 1;
> sync();
> *instr = ppc_inst_read((struct ppc_inst *)adrs);
> sync();
> /* wait a little while to see if we get a machine check */
> __delay(200);
> size = ppc_inst_len(instr);
> }
> catch_memory_errors = 0;
> return size;
> }
Yeah that looks right.
>
> -- Bala
> > > -- Bala
> > >
> > >
> > > > printf("Couldn't read instruction at %lx, "
> > > > "disabling breakpoint there\n", bp-
> > > > >address);
> > > > bp->enabled = 0;
> > > > @@ -949,7 +950,7 @@ static void remove_bpts(void)
> > > > for (i = 0; i < NBPTS; ++i, ++bp) {
> > > > if ((bp->enabled & (BP_TRAP|BP_CIABR)) != BP_TRAP)
> > > > continue;
> > > > - if (mread(bp->address, &instr, 4) == 4
> > > > + if (mread_instr(bp->address, &instr)
> > > > && ppc_inst_equal(instr, ppc_inst(bpinstr))
> > > > && patch_instruction(
> > > > (struct ppc_inst *)bp->address, ppc_inst_read(bp-
> > > > > instr)) != 0)
> > > > @@ -1165,7 +1166,7 @@ static int do_step(struct pt_regs *regs)
> > > > force_enable_xmon();
> > > > /* check we are in 64-bit kernel mode, translation enabled */
> > > > if ((regs->msr & (MSR_64BIT|MSR_PR|MSR_IR)) == (MSR_64BIT|MSR_IR))
> > > > {
> > > > - if (mread(regs->nip, &instr, 4) == 4) {
> > > > + if (mread_instr(regs->nip, &instr)) {
> > > > stepped = emulate_step(regs, instr);
> > > > if (stepped < 0) {
> > > > printf("Couldn't single-step %s
> > > > instruction\n",
> > > > @@ -1332,7 +1333,7 @@ static long check_bp_loc(unsigned long addr)
> > > > printf("Breakpoints may only be placed at kernel
> > > > addresses\n");
> > > > return 0;
> > > > }
> > > > - if (!mread(addr, &instr, sizeof(instr))) {
> > > > + if (!mread_instr(addr, &instr)) {
> > > > printf("Can't read instruction at address %lx\n", addr);
> > > > return 0;
> > > > }
> > > > @@ -2125,6 +2126,21 @@ mwrite(unsigned long adrs, void *buf, int size)
> > > > return n;
> > > > }
> > > >
> > > > +static int
> > > > +mread_instr(unsigned long adrs, struct ppc_inst *instr)
> > > > +{
> > > > + if (setjmp(bus_error_jmp) == 0) {
> > > > + catch_memory_errors = 1;
> > > > + sync();
> > > > + *instr = ppc_inst_read((struct ppc_inst *)adrs);
> > > > + sync();
> > > > + /* wait a little while to see if we get a machine check */
> > > > + __delay(200);
> > > > + }
> > > > + catch_memory_errors = 0;
> > > > + return ppc_inst_len(*instr);
> > > > +}
> > > > +
> > > > static int fault_type;
> > > > static int fault_except;
> > > > static char *fault_chars[] = { "--", "**", "##" };
>
^ permalink raw reply
* Re: [PATCH 31/35] powerpc: docs: cxl.rst: mark two section titles as such
From: Mauro Carvalho Chehab @ 2020-04-09 5:53 UTC (permalink / raw)
To: Andrew Donnellan
Cc: Jonathan Corbet, Linux Doc Mailing List, linux-kernel,
Paul Mackerras, Frederic Barrat, linuxppc-dev
In-Reply-To: <fc649189-91cc-bb73-8d07-34054629a2b3@linux.ibm.com>
Em Thu, 9 Apr 2020 10:37:52 +1000
Andrew Donnellan <ajd@linux.ibm.com> escreveu:
> On 9/4/20 1:46 am, Mauro Carvalho Chehab wrote:
> > The User API chapter contains two sub-chapters. Mark them as
> > such.
> >
> > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
>
> Thanks.
>
> Though the other subsections in this file use ----- rather than ^^^^^,
> what's the difference?
ReST syntax allows the usage of several different markup symbols for
titles. It dynamically attributes the first one it finds as level 1,
the second one as level 2 and so on.
As we added the "^^^^" markup before "-----", after this patch, it now has:
=======
level 1
=======
level 2
=======
level 3
^^^^^^^
level 4
-------
>
> Acked-by: Andrew Donnellan <ajd@linux.ibm.com>
>
> > ---
> > Documentation/powerpc/cxl.rst | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/Documentation/powerpc/cxl.rst b/Documentation/powerpc/cxl.rst
> > index 920546d81326..d2d77057610e 100644
> > --- a/Documentation/powerpc/cxl.rst
> > +++ b/Documentation/powerpc/cxl.rst
> > @@ -133,6 +133,7 @@ User API
> > ========
> >
> > 1. AFU character devices
> > +^^^^^^^^^^^^^^^^^^^^^^^^
> >
> > For AFUs operating in AFU directed mode, two character device
> > files will be created. /dev/cxl/afu0.0m will correspond to a
> > @@ -395,6 +396,7 @@ read
> >
> >
> > 2. Card character device (powerVM guest only)
> > +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >
> > In a powerVM guest, an extra character device is created for the
> > card. The device is only used to write (flash) a new image on the
> >
>
Thanks,
Mauro
^ permalink raw reply
* Re: [PATCH v5 02/21] powerpc/xmon: Move out-of-line instructions to text section
From: Christophe Leroy @ 2020-04-09 6:11 UTC (permalink / raw)
To: Jordan Niethe, linuxppc-dev; +Cc: bala24, alistair, dja, npiggin
In-Reply-To: <20200406080936.7180-3-jniethe5@gmail.com>
Le 06/04/2020 à 10:09, Jordan Niethe a écrit :
> To execute an instruction out of line after a breakpoint, the NIP is set
> to the address of struct bpt::instr. Here a copy of the instruction that
> was replaced with a breakpoint is kept, along with a trap so normal flow
> can be resumed after XOLing. The struct bpt's are located within the
> data section. This is problematic as the data section may be marked as
> no execute.
>
> Instead of each struct bpt holding the instructions to be XOL'd, make a
> new array, bpt_table[], with enough space to hold instructions for the
> number of supported breakpoints. Place this array in the text section.
> Make struct bpt::instr a pointer to the instructions in bpt_table[]
> associated with that breakpoint. This association is a simple mapping:
> bpts[n] -> bpt_table[n * words per breakpoint]. Currently we only need
> the copied instruction followed by a trap, so 2 words per breakpoint.
>
> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> ---
> v4: New to series
> v5: - Do not use __section(), use a .space directive in .S file
I was going to comment to use __section() instead of creating a
dedicated .S file.
Why did you change that in v5 ?
> - Simplify in_breakpoint_table() calculation
> - Define BPT_SIZE
> ---
> arch/powerpc/xmon/Makefile | 2 +-
> arch/powerpc/xmon/xmon.c | 23 +++++++++++++----------
> arch/powerpc/xmon/xmon_bpts.S | 8 ++++++++
> arch/powerpc/xmon/xmon_bpts.h | 8 ++++++++
> 4 files changed, 30 insertions(+), 11 deletions(-)
> create mode 100644 arch/powerpc/xmon/xmon_bpts.S
> create mode 100644 arch/powerpc/xmon/xmon_bpts.h
>
> diff --git a/arch/powerpc/xmon/Makefile b/arch/powerpc/xmon/Makefile
> index c3842dbeb1b7..515a13ea6f28 100644
> --- a/arch/powerpc/xmon/Makefile
> +++ b/arch/powerpc/xmon/Makefile
> @@ -21,7 +21,7 @@ endif
>
> ccflags-$(CONFIG_PPC64) := $(NO_MINIMAL_TOC)
>
> -obj-y += xmon.o nonstdio.o spr_access.o
> +obj-y += xmon.o nonstdio.o spr_access.o xmon_bpts.o
>
> ifdef CONFIG_XMON_DISASSEMBLY
> obj-y += ppc-dis.o ppc-opc.o
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index 02e3bd62cab4..049375206510 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -62,6 +62,7 @@
>
> #include "nonstdio.h"
> #include "dis-asm.h"
> +#include "xmon_bpts.h"
>
> #ifdef CONFIG_SMP
> static cpumask_t cpus_in_xmon = CPU_MASK_NONE;
> @@ -97,7 +98,7 @@ static long *xmon_fault_jmp[NR_CPUS];
> /* Breakpoint stuff */
> struct bpt {
> unsigned long address;
> - unsigned int instr[2];
> + unsigned int *instr;
> atomic_t ref_count;
> int enabled;
> unsigned long pad;
> @@ -108,7 +109,6 @@ struct bpt {
> #define BP_TRAP 2
> #define BP_DABR 4
>
> -#define NBPTS 256
> static struct bpt bpts[NBPTS];
> static struct bpt dabr;
> static struct bpt *iabr;
> @@ -116,6 +116,10 @@ static unsigned bpinstr = 0x7fe00008; /* trap */
>
> #define BP_NUM(bp) ((bp) - bpts + 1)
>
> +#define BPT_SIZE (sizeof(unsigned int) * 2)
> +#define BPT_WORDS (BPT_SIZE / sizeof(unsigned int))
Wouldn't it make more sense to do the following ? :
#define BPT_WORDS 2
#define BPT_SIZE (BPT_WORDS * sizeof(unsigned int))
> +extern unsigned int bpt_table[NBPTS * BPT_WORDS];
Should go in xmon_bpts.h if we keep the definition in xmon_bpts.S
> +
> /* Prototypes */
> static int cmds(struct pt_regs *);
> static int mread(unsigned long, void *, int);
> @@ -853,15 +857,13 @@ static struct bpt *in_breakpoint_table(unsigned long nip, unsigned long *offp)
> {
> unsigned long off;
>
> - off = nip - (unsigned long) bpts;
> - if (off >= sizeof(bpts))
> + off = nip - (unsigned long) bpt_table;
> + if (off >= sizeof(bpt_table))
> return NULL;
> - off %= sizeof(struct bpt);
> - if (off != offsetof(struct bpt, instr[0])
> - && off != offsetof(struct bpt, instr[1]))
> + *offp = off % BPT_SIZE;
Can we use logical operation instead ?
*offp = off & (BPT_SIZE - 1);
> + if (*offp != 0 && *offp != 4)
Could be:
if (off & 3)
> return NULL;
> - *offp = off - offsetof(struct bpt, instr[0]);
> - return (struct bpt *) (nip - off);
> + return bpts + (off / BPT_SIZE);
> }
>
> static struct bpt *new_breakpoint(unsigned long a)
> @@ -876,7 +878,8 @@ static struct bpt *new_breakpoint(unsigned long a)
> for (bp = bpts; bp < &bpts[NBPTS]; ++bp) {
> if (!bp->enabled && atomic_read(&bp->ref_count) == 0) {
> bp->address = a;
> - patch_instruction(&bp->instr[1], bpinstr);
> + bp->instr = bpt_table + ((bp - bpts) * BPT_WORDS);
> + patch_instruction(bp->instr + 1, bpinstr);
> return bp;
> }
> }
> diff --git a/arch/powerpc/xmon/xmon_bpts.S b/arch/powerpc/xmon/xmon_bpts.S
> new file mode 100644
> index 000000000000..ebb2dbc70ca8
> --- /dev/null
> +++ b/arch/powerpc/xmon/xmon_bpts.S
> @@ -0,0 +1,8 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#include <asm/ppc_asm.h>
> +#include <asm/asm-compat.h>
> +#include "xmon_bpts.h"
> +
> +.global bpt_table
> +bpt_table:
> + .space NBPTS * 8
Should use BPT_SIZE instead of raw coding 8.
> diff --git a/arch/powerpc/xmon/xmon_bpts.h b/arch/powerpc/xmon/xmon_bpts.h
> new file mode 100644
> index 000000000000..840e70be7945
> --- /dev/null
> +++ b/arch/powerpc/xmon/xmon_bpts.h
> @@ -0,0 +1,8 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef XMON_BPTS_H
> +#define XMON_BPTS_H
> +
> +#define NBPTS 256
> +
> +#endif /* XMON_BPTS_H */
> +
>
I think it would be better to split this patch in two patches:
1/ First patch to move breakpoints out of struct bpt into a bpt_table.
2/ Second patch to move bpt_table into .text section.
Christophe
^ permalink raw reply
* [PATCH] powernv/pci: Print an error when device enable is blocked
From: Oliver O'Halloran @ 2020-04-09 6:13 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Oliver O'Halloran
If the platform decides to block enabling the device nothing is printed
currently. This can lead to some confusion since the dmesg output will
usually print an error with no context e.g.
e1000e: probe of 0022:01:00.0 failed with error -22
This shouldn't be spammy since pci_enable_device() already prints a
messages when it succeeds.
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
arch/powerpc/platforms/powernv/pci-ioda.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index cda0933..17fdf46 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -3296,8 +3296,10 @@ static bool pnv_pci_enable_device_hook(struct pci_dev *dev)
return true;
pdn = pci_get_pdn(dev);
- if (!pdn || pdn->pe_number == IODA_INVALID_PE)
+ if (!pdn || pdn->pe_number == IODA_INVALID_PE) {
+ pci_err("pci_enable_device() blocked, no PE assigned.\n");
return false;
+ }
return true;
}
--
2.9.5
^ permalink raw reply related
* Re: [PATCH] powernv/pci: Print an error when device enable is blocked
From: Oliver O'Halloran @ 2020-04-09 6:20 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <20200409061337.9187-1-oohall@gmail.com>
On Thu, Apr 9, 2020 at 4:13 PM Oliver O'Halloran <oohall@gmail.com> wrote:
>
> If the platform decides to block enabling the device nothing is printed
> currently. This can lead to some confusion since the dmesg output will
> usually print an error with no context e.g.
>
> e1000e: probe of 0022:01:00.0 failed with error -22
>
> This shouldn't be spammy since pci_enable_device() already prints a
> messages when it succeeds.
>
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
> ---
> arch/powerpc/platforms/powernv/pci-ioda.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
> index cda0933..17fdf46 100644
> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
> @@ -3296,8 +3296,10 @@ static bool pnv_pci_enable_device_hook(struct pci_dev *dev)
> return true;
>
> pdn = pci_get_pdn(dev);
> - if (!pdn || pdn->pe_number == IODA_INVALID_PE)
> + if (!pdn || pdn->pe_number == IODA_INVALID_PE) {
> + pci_err("pci_enable_device() blocked, no PE assigned.\n");
Maybe I should start compiling my code before I sent it out. Maybe.
> return false;
> + }
>
> return true;
> }
> --
> 2.9.5
>
^ permalink raw reply
* Re: usb: gadget: fsl_udc_core: Checking for a failed platform_get_irq() call in fsl_udc_probe()
From: Markus Elfring @ 2020-04-09 6:28 UTC (permalink / raw)
To: Li Yang, linux-usb, linuxppc-dev
Cc: Felipe Balbi, Tang Bin, kernel-janitors, LKML, Greg Kroah-Hartman
In-Reply-To: <CADRPPNRe=YxwjCOYbEjKg4LCOx2suK5WxZp17NJhTm76szdU0w@mail.gmail.com>
>> Would you like to reconsider the shown condition check?
>
> Thanks for the finding. This is truly a software issue that need to
> be fixed.
I was unsure if I noticed another programming mistake.
> Would you submit a patch for it
Do other contributors know the affected software module better than me?
> or you want us to fix it?
I would find it nice if another developer will convert the bug report
into corresponding improvements.
Regards,
Markus
^ permalink raw reply
* Re: [PATCH v5 00/21] Initial Prefixed Instruction support
From: Christophe Leroy @ 2020-04-09 6:39 UTC (permalink / raw)
To: Jordan Niethe, linuxppc-dev; +Cc: bala24, alistair, dja, npiggin
In-Reply-To: <20200406080936.7180-1-jniethe5@gmail.com>
On 04/06/2020 08:09 AM, Jordan Niethe wrote:
> A future revision of the ISA will introduce prefixed instructions. A
> prefixed instruction is composed of a 4-byte prefix followed by a
> 4-byte suffix.
>
> All prefixes have the major opcode 1. A prefix will never be a valid
> word instruction. A suffix may be an existing word instruction or a
> new instruction.
>
> This series enables prefixed instructions and extends the instruction
> emulation to support them. Then the places where prefixed instructions
> might need to be emulated are updated.
>
> v5 is based on feedback from Nick Piggins, Michael Ellerman, Balamuruhan
> Suriyakumar and Alistair Popple.
> The major changes:
> - The ppc instruction type is now a struct
> - Series now just based on next
> - ppc_inst_masked() dropped
> - Space for xmon breakpoints allocated in an assembly file
> - "Add prefixed instructions to instruction data type" patch seperated in
> to smaller patches
> - Calling convention for create_branch() is changed
> - Some places which had not been updated to use the data type are now updated
Build fails. I have not investigated why:
CC arch/powerpc/kernel/process.o
In file included from ./arch/powerpc/include/asm/code-patching.h:14:0,
from arch/powerpc/kernel/process.c:60:
./arch/powerpc/include/asm/inst.h:69:38: error: unknown type name ‘ppc_inst’
static inline bool ppc_inst_prefixed(ppc_inst x)
^
./arch/powerpc/include/asm/inst.h:79:19: error: redefinition of
‘ppc_inst_val’
static inline u32 ppc_inst_val(struct ppc_inst x)
^
./arch/powerpc/include/asm/inst.h:21:19: note: previous definition of
‘ppc_inst_val’ was here
static inline u32 ppc_inst_val(struct ppc_inst x)
^
./arch/powerpc/include/asm/inst.h: In function ‘ppc_inst_len’:
./arch/powerpc/include/asm/inst.h:103:10: error: implicit declaration of
function ‘ppc_inst_prefixed’ [-Werror=implicit-function-declaration]
return (ppc_inst_prefixed(x)) ? 8 : 4;
^
Christophe
>
> v4 is based on feedback from Nick Piggins, Christophe Leroy and Daniel Axtens.
> The major changes:
> - Move xmon breakpoints from data section to text section
> - Introduce a data type for instructions on powerpc
>
> v3 is based on feedback from Christophe Leroy. The major changes:
> - Completely replacing store_inst() with patch_instruction() in
> xmon
> - Improve implementation of mread_instr() to not use mread().
> - Base the series on top of
> https://patchwork.ozlabs.org/patch/1232619/ as this will effect
> kprobes.
> - Some renaming and simplification of conditionals.
>
> v2 incorporates feedback from Daniel Axtens and and Balamuruhan
> S. The major changes are:
> - Squashing together all commits about SRR1 bits
> - Squashing all commits for supporting prefixed load stores
> - Changing abbreviated references to sufx/prfx -> suffix/prefix
> - Introducing macros for returning the length of an instruction
> - Removing sign extension flag from pstd/pld in sstep.c
> - Dropping patch "powerpc/fault: Use analyse_instr() to check for
> store with updates to sp" from the series, it did not really fit
> with prefixed enablement in the first place and as reported by Greg
> Kurz did not work correctly.
>
>
> Alistair Popple (1):
> powerpc: Enable Prefixed Instructions
>
> Jordan Niethe (20):
> powerpc/xmon: Remove store_inst() for patch_instruction()
> powerpc/xmon: Move out-of-line instructions to text section
> powerpc: Change calling convention for create_branch() et. al.
> powerpc: Use a macro for creating instructions from u32s
> powerpc: Use a function for getting the instruction op code
> powerpc: Use an accessor for instructions
> powerpc: Use a function for byte swapping instructions
> powerpc: Introduce functions for instruction equality
> powerpc: Use a datatype for instructions
> powerpc: Use a function for reading instructions
> powerpc: Define and use __get_user_instr{,inatomic}()
> powerpc: Introduce a function for reporting instruction length
> powerpc/xmon: Use a function for reading instructions
> powerpc/xmon: Move insertion of breakpoint for xol'ing
> powerpc: Make test_translate_branch() independent of instruction
> length
> powerpc: Define new SRR1 bits for a future ISA version
> powerpc64: Add prefixed instructions to instruction data type
> powerpc: Support prefixed instructions in alignment handler
> powerpc sstep: Add support for prefixed load/stores
> powerpc sstep: Add support for prefixed fixed-point arithmetic
>
> arch/powerpc/include/asm/code-patching.h | 37 +-
> arch/powerpc/include/asm/inst.h | 106 ++++++
> arch/powerpc/include/asm/kprobes.h | 2 +-
> arch/powerpc/include/asm/reg.h | 7 +-
> arch/powerpc/include/asm/sstep.h | 15 +-
> arch/powerpc/include/asm/uaccess.h | 28 ++
> arch/powerpc/include/asm/uprobes.h | 7 +-
> arch/powerpc/kernel/align.c | 13 +-
> arch/powerpc/kernel/epapr_paravirt.c | 5 +-
> arch/powerpc/kernel/hw_breakpoint.c | 5 +-
> arch/powerpc/kernel/jump_label.c | 5 +-
> arch/powerpc/kernel/kgdb.c | 9 +-
> arch/powerpc/kernel/kprobes.c | 24 +-
> arch/powerpc/kernel/mce_power.c | 5 +-
> arch/powerpc/kernel/module_64.c | 3 +-
> arch/powerpc/kernel/optprobes.c | 91 +++--
> arch/powerpc/kernel/optprobes_head.S | 3 +
> arch/powerpc/kernel/security.c | 9 +-
> arch/powerpc/kernel/setup_32.c | 4 +-
> arch/powerpc/kernel/trace/ftrace.c | 190 ++++++----
> arch/powerpc/kernel/traps.c | 20 +-
> arch/powerpc/kernel/uprobes.c | 3 +-
> arch/powerpc/kernel/vecemu.c | 20 +-
> arch/powerpc/kvm/book3s_hv_nested.c | 2 +-
> arch/powerpc/kvm/book3s_hv_rm_mmu.c | 2 +-
> arch/powerpc/kvm/emulate_loadstore.c | 2 +-
> arch/powerpc/lib/code-patching.c | 289 +++++++-------
> arch/powerpc/lib/feature-fixups.c | 69 ++--
> arch/powerpc/lib/sstep.c | 455 ++++++++++++++++-------
> arch/powerpc/lib/test_emulate_step.c | 56 +--
> arch/powerpc/perf/core-book3s.c | 4 +-
> arch/powerpc/xmon/Makefile | 2 +-
> arch/powerpc/xmon/xmon.c | 94 +++--
> arch/powerpc/xmon/xmon_bpts.S | 10 +
> arch/powerpc/xmon/xmon_bpts.h | 8 +
> 35 files changed, 1042 insertions(+), 562 deletions(-)
> create mode 100644 arch/powerpc/include/asm/inst.h
> create mode 100644 arch/powerpc/xmon/xmon_bpts.S
> create mode 100644 arch/powerpc/xmon/xmon_bpts.h
>
^ permalink raw reply
* Re: [PATCH] powerpc/powernv: Add a print indicating when an IODA PE is released
From: Sam Bobroff @ 2020-04-09 7:02 UTC (permalink / raw)
To: Oliver O'Halloran; +Cc: linuxppc-dev
In-Reply-To: <20200408112213.5549-1-oohall@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 902 bytes --]
On Wed, Apr 08, 2020 at 09:22:13PM +1000, Oliver O'Halloran wrote:
> Quite useful to know in some cases.
>
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
Agreed.
Reviewed-by: Sam Bobroff <sbobroff@linux.ibm.com>
> ---
> arch/powerpc/platforms/powernv/pci-ioda.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
> index 3d81c01..82e5098 100644
> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
> @@ -3475,6 +3475,8 @@ static void pnv_ioda_release_pe(struct pnv_ioda_pe *pe)
> struct pnv_phb *phb = pe->phb;
> struct pnv_ioda_pe *slave, *tmp;
>
> + pe_info(pe, "Releasing PE\n");
> +
> mutex_lock(&phb->ioda.pe_list_mutex);
> list_del(&pe->list);
> mutex_unlock(&phb->ioda.pe_list_mutex);
> --
> 2.9.5
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: usb: gadget: fsl_udc_core: Checking for a failed platform_get_irq() call in fsl_udc_probe()
From: Tang Bin @ 2020-04-09 6:51 UTC (permalink / raw)
To: markus.elfring@web.de, Li Yang
Cc: balbi@kernel.org, tangbin@cmss.chinamobile.com,
gregkh@linuxfoundation.org, kernel-janitors@vger.kernel.org,
linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org
[-- Attachment #1: Type: text/plain, Size: 334 bytes --]
On Thu,Apr 9,2020 08:28:28 Markus Elfring <Markus.Elfring@web.de> wrote:
> I was unsure if I noticed another programming mistake.
> Do other contributors know the affected software module better than me?
I discovered this problem fews days ago, and doing experiments on the hardware to test my idea.
Thanks
Tang Bin
[-- Attachment #2: Type: text/html, Size: 3817 bytes --]
^ permalink raw reply
* Re: [PATCH v1 1/2] powerpc/pseries/hotplug-memory: stop checking is_mem_section_removable()
From: Michael Ellerman @ 2020-04-09 7:26 UTC (permalink / raw)
To: David Hildenbrand, linux-kernel
Cc: Michal Hocko, Baoquan He, David Hildenbrand, Wei Yang, linux-mm,
Paul Mackerras, Nathan Fontenot, Andrew Morton, linuxppc-dev,
Oscar Salvador
In-Reply-To: <20200407135416.24093-2-david@redhat.com>
David Hildenbrand <david@redhat.com> writes:
> In commit 53cdc1cb29e8 ("drivers/base/memory.c: indicate all memory
> blocks as removable"), the user space interface to compute whether a memory
> block can be offlined (exposed via
> /sys/devices/system/memory/memoryX/removable) has effectively been
> deprecated. We want to remove the leftovers of the kernel implementation.
>
> When offlining a memory block (mm/memory_hotplug.c:__offline_pages()),
> we'll start by:
> 1. Testing if it contains any holes, and reject if so
> 2. Testing if pages belong to different zones, and reject if so
> 3. Isolating the page range, checking if it contains any unmovable pages
>
> Using is_mem_section_removable() before trying to offline is not only racy,
> it can easily result in false positives/negatives. Let's stop manually
> checking is_mem_section_removable(), and let device_offline() handle it
> completely instead. We can remove the racy is_mem_section_removable()
> implementation next.
>
> We now take more locks (e.g., memory hotplug lock when offlining and the
> zone lock when isolating), but maybe we should optimize that
> implementation instead if this ever becomes a real problem (after all,
> memory unplug is already an expensive operation). We started using
> is_mem_section_removable() in commit 51925fb3c5c9 ("powerpc/pseries:
> Implement memory hotplug remove in the kernel"), with the initial
> hotremove support of lmbs.
It's also not very pretty in dmesg.
Before:
pseries-hotplug-mem: Attempting to hot-add 10 LMB(s)
pseries-hotplug-mem: Memory hot-add failed, removing any added LMBs
dlpar: Could not handle DLPAR request "memory add count 10"
After:
pseries-hotplug-mem: Attempting to hot-remove 10 LMB(s)
page:c00c000001ca8200 refcount:1 mapcount:0 mapping:0000000018c4a547 index:0xc00000072a080180
flags: 0x7ffff000000200(slab)
raw: 007ffff000000200 c00c000001cffd48 c000000781c51410 c000000793327580
raw: c00000072a080180 0000000001550001 00000001ffffffff 0000000000000000
page dumped because: unmovable page
page:c00c000001cc4a80 refcount:1 mapcount:0 mapping:0000000018c4a547 index:0xc00000079b110080
flags: 0x7ffff000000000()
raw: 007ffff000000000 5deadbeef0000100 5deadbeef0000122 0000000000000000
raw: c00000079b110080 0000000000000000 00000001ffffffff 0000000000000000
page dumped because: unmovable page
page:c00c000001d08200 refcount:1 mapcount:0 mapping:0000000018c4a547 index:0xc00000074208ff00
flags: 0x7ffff000000200(slab)
raw: 007ffff000000200 c000000781c5f150 c00c000001d37f88 c000000798a24880
raw: c00000074208ff00 0000000001550002 00000001ffffffff 0000000000000000
page dumped because: unmovable page
page:c00c000001d40140 refcount:1 mapcount:0 mapping:0000000018c4a547 index:0xc000000750059c00
flags: 0x7ffff000000200(slab)
raw: 007ffff000000200 c00c000001dfcfc8 c00c000001d3c288 c0000007851c2d00
raw: c000000750059c00 0000000001000003 00000001ffffffff 0000000000000000
page dumped because: unmovable page
page:c00c000001d9c000 refcount:1 mapcount:0 mapping:0000000018c4a547 index:0xc000000002370080
flags: 0x7ffff000000000()
raw: 007ffff000000000 5deadbeef0000100 5deadbeef0000122 0000000000000000
raw: c000000002370080 0000000000000000 00000001ffffffff 0000000000000000
page dumped because: unmovable page
page:c00c000001dc0200 refcount:1 mapcount:0 mapping:0000000018c4a547 index:0xc000000002370080
flags: 0x7ffff000000000()
raw: 007ffff000000000 5deadbeef0000100 5deadbeef0000122 0000000000000000
raw: c000000002370080 0000000000000000 00000001ffffffff 0000000000000000
page dumped because: unmovable page
page:c00c000001e00000 refcount:1 mapcount:0 mapping:0000000018c4a547 index:0x0
flags: 0x7ffff000000200(slab)
raw: 007ffff000000200 5deadbeef0000100 5deadbeef0000122 c0000007a801f500
raw: 0000000000000000 0000000008000800 00000001ffffffff 0000000000000000
page dumped because: unmovable page
page:c00c000001e40440 refcount:1 mapcount:0 mapping:0000000018c4a547 index:0x0
flags: 0x7ffff000000200(slab)
raw: 007ffff000000200 5deadbeef0000100 5deadbeef0000122 c0000007a801e380
raw: 0000000000000000 0000000000400040 00000001ffffffff 0000000000000000
page dumped because: unmovable page
page:c00c000001e80000 refcount:1 mapcount:0 mapping:0000000018c4a547 index:0xc0000007a0000640
flags: 0x7ffff000000200(slab)
raw: 007ffff000000200 c00c000001e5af48 c00c000001e80408 c000000f42d00a00
raw: c0000007a0000640 00000000066600a2 00000001ffffffff 0000000000000000
page dumped because: unmovable page
page:c00c000003c89d40 refcount:2 mapcount:1 mapping:0000000018c4a547 index:0x10a41
anon flags: 0x17ffff000080024(uptodate|active|swapbacked)
raw: 017ffff000080024 5deadbeef0000100 5deadbeef0000122 c0000007986b03c9
raw: 0000000000010a41 0000000000000000 0000000200000000 c00000000340b000
page dumped because: unmovable page
page->mem_cgroup:c00000000340b000
page:c00c000003cc0000 refcount:1 mapcount:0 mapping:0000000018c4a547 index:0xc000000f3000fd38
flags: 0x17ffff000000200(slab)
raw: 017ffff000000200 c000000f3c911890 c000000f3c911890 c00000079fffd980
raw: c000000f3000fd38 0000000000700003 00000001ffffffff 0000000000000000
page dumped because: unmovable page
page:c00c000003d00000 refcount:1 mapcount:0 mapping:0000000018c4a547 index:0xc0000007a2ec0000
flags: 0x17ffff000000000()
raw: 017ffff000000000 5deadbeef0000100 5deadbeef0000122 0000000000000000
raw: c0000007a2ec0000 0000000000000000 00000001ffffffff 0000000000000000
page dumped because: unmovable page
page:c00c000003e2c000 refcount:1 mapcount:0 mapping:0000000018c4a547 index:0xc000000f8b008400
flags: 0x27ffff000000200(slab)
raw: 027ffff000000200 c000000f8e000190 c000000f8e000190 c0000007a801e380
raw: c000000f8b008400 0000000000400038 00000001ffffffff 0000000000000000
page dumped because: unmovable page
page:c00c000003fec000 refcount:1 mapcount:0 mapping:0000000018c4a547 index:0x0
flags: 0x37ffff000000000()
raw: 037ffff000000000 5deadbeef0000100 5deadbeef0000122 0000000000000000
raw: 0000000000000000 0000000000000000 00000001ffffffff 0000000000000000
page dumped because: unmovable page
pseries-hotplug-mem: Memory hot-remove failed, adding LMB's back
dlpar: Could not handle DLPAR request "memory remove count 10"
It looks like set_migratetype_isolate() and start_isolate_page_range()
can be told not to report those warnings, but we're just calling
device_offline() which doesn't let us specify that.
cheers
^ permalink raw reply
* Re: [PATCH v5 02/21] powerpc/xmon: Move out-of-line instructions to text section
From: Jordan Niethe @ 2020-04-09 7:26 UTC (permalink / raw)
To: Christophe Leroy
Cc: Alistair Popple, Nicholas Piggin, Balamuruhan S, linuxppc-dev,
Daniel Axtens
In-Reply-To: <a6adbfc6-a715-99aa-25ac-7a36b3804b82@c-s.fr>
On Thu, Apr 9, 2020 at 4:11 PM Christophe Leroy <christophe.leroy@c-s.fr> wrote:
>
>
>
> Le 06/04/2020 à 10:09, Jordan Niethe a écrit :
> > To execute an instruction out of line after a breakpoint, the NIP is set
> > to the address of struct bpt::instr. Here a copy of the instruction that
> > was replaced with a breakpoint is kept, along with a trap so normal flow
> > can be resumed after XOLing. The struct bpt's are located within the
> > data section. This is problematic as the data section may be marked as
> > no execute.
> >
> > Instead of each struct bpt holding the instructions to be XOL'd, make a
> > new array, bpt_table[], with enough space to hold instructions for the
> > number of supported breakpoints. Place this array in the text section.
> > Make struct bpt::instr a pointer to the instructions in bpt_table[]
> > associated with that breakpoint. This association is a simple mapping:
> > bpts[n] -> bpt_table[n * words per breakpoint]. Currently we only need
> > the copied instruction followed by a trap, so 2 words per breakpoint.
> >
> > Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> > ---
> > v4: New to series
> > v5: - Do not use __section(), use a .space directive in .S file
>
> I was going to comment to use __section() instead of creating a
> dedicated .S file.
>
> Why did you change that in v5 ?
I noticed with some toolchains I was getting this message:
Warning: setting incorrect section attributes for .text.xmon_bpts
I was talking with mpe about it and he said that the usual way for
doing things like this was with .space in a .S file so I changed to
that way.
>
> > - Simplify in_breakpoint_table() calculation
> > - Define BPT_SIZE
> > ---
> > arch/powerpc/xmon/Makefile | 2 +-
> > arch/powerpc/xmon/xmon.c | 23 +++++++++++++----------
> > arch/powerpc/xmon/xmon_bpts.S | 8 ++++++++
> > arch/powerpc/xmon/xmon_bpts.h | 8 ++++++++
> > 4 files changed, 30 insertions(+), 11 deletions(-)
> > create mode 100644 arch/powerpc/xmon/xmon_bpts.S
> > create mode 100644 arch/powerpc/xmon/xmon_bpts.h
> >
> > diff --git a/arch/powerpc/xmon/Makefile b/arch/powerpc/xmon/Makefile
> > index c3842dbeb1b7..515a13ea6f28 100644
> > --- a/arch/powerpc/xmon/Makefile
> > +++ b/arch/powerpc/xmon/Makefile
> > @@ -21,7 +21,7 @@ endif
> >
> > ccflags-$(CONFIG_PPC64) := $(NO_MINIMAL_TOC)
> >
> > -obj-y += xmon.o nonstdio.o spr_access.o
> > +obj-y += xmon.o nonstdio.o spr_access.o xmon_bpts.o
> >
> > ifdef CONFIG_XMON_DISASSEMBLY
> > obj-y += ppc-dis.o ppc-opc.o
> > diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> > index 02e3bd62cab4..049375206510 100644
> > --- a/arch/powerpc/xmon/xmon.c
> > +++ b/arch/powerpc/xmon/xmon.c
> > @@ -62,6 +62,7 @@
> >
> > #include "nonstdio.h"
> > #include "dis-asm.h"
> > +#include "xmon_bpts.h"
> >
> > #ifdef CONFIG_SMP
> > static cpumask_t cpus_in_xmon = CPU_MASK_NONE;
> > @@ -97,7 +98,7 @@ static long *xmon_fault_jmp[NR_CPUS];
> > /* Breakpoint stuff */
> > struct bpt {
> > unsigned long address;
> > - unsigned int instr[2];
> > + unsigned int *instr;
> > atomic_t ref_count;
> > int enabled;
> > unsigned long pad;
> > @@ -108,7 +109,6 @@ struct bpt {
> > #define BP_TRAP 2
> > #define BP_DABR 4
> >
> > -#define NBPTS 256
> > static struct bpt bpts[NBPTS];
> > static struct bpt dabr;
> > static struct bpt *iabr;
> > @@ -116,6 +116,10 @@ static unsigned bpinstr = 0x7fe00008; /* trap */
> >
> > #define BP_NUM(bp) ((bp) - bpts + 1)
> >
> > +#define BPT_SIZE (sizeof(unsigned int) * 2)
> > +#define BPT_WORDS (BPT_SIZE / sizeof(unsigned int))
>
> Wouldn't it make more sense to do the following ? :
>
> #define BPT_WORDS 2
> #define BPT_SIZE (BPT_WORDS * sizeof(unsigned int))
I defined it like that so when we changed unsigned int -> struct
ppc_inst it would be the correct size whether or not the struct
included a suffix.
Otherwise it would be more straightforward to do it like that.
>
> > +extern unsigned int bpt_table[NBPTS * BPT_WORDS];
>
> Should go in xmon_bpts.h if we keep the definition in xmon_bpts.S
Right.
>
> > +
> > /* Prototypes */
> > static int cmds(struct pt_regs *);
> > static int mread(unsigned long, void *, int);
> > @@ -853,15 +857,13 @@ static struct bpt *in_breakpoint_table(unsigned long nip, unsigned long *offp)
> > {
> > unsigned long off;
> >
> > - off = nip - (unsigned long) bpts;
> > - if (off >= sizeof(bpts))
> > + off = nip - (unsigned long) bpt_table;
> > + if (off >= sizeof(bpt_table))
> > return NULL;
> > - off %= sizeof(struct bpt);
> > - if (off != offsetof(struct bpt, instr[0])
> > - && off != offsetof(struct bpt, instr[1]))
> > + *offp = off % BPT_SIZE;
>
> Can we use logical operation instead ?
Sure, I was just adapting how it was already but that would probably be clearer.
>
> *offp = off & (BPT_SIZE - 1);
>
> > + if (*offp != 0 && *offp != 4)
>
> Could be:
> if (off & 3)
>
> > return NULL;
> > - *offp = off - offsetof(struct bpt, instr[0]);
> > - return (struct bpt *) (nip - off);
> > + return bpts + (off / BPT_SIZE);
> > }
> >
> > static struct bpt *new_breakpoint(unsigned long a)
> > @@ -876,7 +878,8 @@ static struct bpt *new_breakpoint(unsigned long a)
> > for (bp = bpts; bp < &bpts[NBPTS]; ++bp) {
> > if (!bp->enabled && atomic_read(&bp->ref_count) == 0) {
> > bp->address = a;
> > - patch_instruction(&bp->instr[1], bpinstr);
> > + bp->instr = bpt_table + ((bp - bpts) * BPT_WORDS);
> > + patch_instruction(bp->instr + 1, bpinstr);
> > return bp;
> > }
> > }
> > diff --git a/arch/powerpc/xmon/xmon_bpts.S b/arch/powerpc/xmon/xmon_bpts.S
> > new file mode 100644
> > index 000000000000..ebb2dbc70ca8
> > --- /dev/null
> > +++ b/arch/powerpc/xmon/xmon_bpts.S
> > @@ -0,0 +1,8 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#include <asm/ppc_asm.h>
> > +#include <asm/asm-compat.h>
> > +#include "xmon_bpts.h"
> > +
> > +.global bpt_table
> > +bpt_table:
> > + .space NBPTS * 8
>
> Should use BPT_SIZE instead of raw coding 8.
Right.
>
> > diff --git a/arch/powerpc/xmon/xmon_bpts.h b/arch/powerpc/xmon/xmon_bpts.h
> > new file mode 100644
> > index 000000000000..840e70be7945
> > --- /dev/null
> > +++ b/arch/powerpc/xmon/xmon_bpts.h
> > @@ -0,0 +1,8 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#ifndef XMON_BPTS_H
> > +#define XMON_BPTS_H
> > +
> > +#define NBPTS 256
> > +
> > +#endif /* XMON_BPTS_H */
> > +
> >
>
> I think it would be better to split this patch in two patches:
> 1/ First patch to move breakpoints out of struct bpt into a bpt_table.
> 2/ Second patch to move bpt_table into .text section.
Bala suggested so too, I will separate them next time.
>
> Christophe
^ permalink raw reply
* Re: [PATCH v1 1/2] powerpc/pseries/hotplug-memory: stop checking is_mem_section_removable()
From: David Hildenbrand @ 2020-04-09 7:26 UTC (permalink / raw)
To: piliu, Baoquan He
Cc: Michal Hocko, linux-kernel, Wei Yang, linux-mm, Paul Mackerras,
Nathan Fontenot, Andrew Morton, linuxppc-dev, Oscar Salvador
In-Reply-To: <16187f69-0e5b-c9c2-a31b-8658425758aa@redhat.com>
On 09.04.20 04:59, piliu wrote:
>
>
> On 04/08/2020 10:46 AM, Baoquan He wrote:
>> Add Pingfan to CC since he usually handles ppc related bugs for RHEL.
>>
>> On 04/07/20 at 03:54pm, David Hildenbrand wrote:
>>> In commit 53cdc1cb29e8 ("drivers/base/memory.c: indicate all memory
>>> blocks as removable"), the user space interface to compute whether a memory
>>> block can be offlined (exposed via
>>> /sys/devices/system/memory/memoryX/removable) has effectively been
>>> deprecated. We want to remove the leftovers of the kernel implementation.
>>
>> Pingfan, can you have a look at this change on PPC? Please feel free to
>> give comments if any concern, or offer ack if it's OK to you.
>>
>>>
>>> When offlining a memory block (mm/memory_hotplug.c:__offline_pages()),
>>> we'll start by:
>>> 1. Testing if it contains any holes, and reject if so
>>> 2. Testing if pages belong to different zones, and reject if so
>>> 3. Isolating the page range, checking if it contains any unmovable pages
>>>
>>> Using is_mem_section_removable() before trying to offline is not only racy,
>>> it can easily result in false positives/negatives. Let's stop manually
>>> checking is_mem_section_removable(), and let device_offline() handle it
>>> completely instead. We can remove the racy is_mem_section_removable()
>>> implementation next.
>>>
>>> We now take more locks (e.g., memory hotplug lock when offlining and the
>>> zone lock when isolating), but maybe we should optimize that
>>> implementation instead if this ever becomes a real problem (after all,
>>> memory unplug is already an expensive operation). We started using
>>> is_mem_section_removable() in commit 51925fb3c5c9 ("powerpc/pseries:
>>> Implement memory hotplug remove in the kernel"), with the initial
>>> hotremove support of lmbs.
>>>
>>> Cc: Nathan Fontenot <nfont@linux.vnet.ibm.com>
>>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>>> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>>> Cc: Paul Mackerras <paulus@samba.org>
>>> Cc: Michal Hocko <mhocko@suse.com>
>>> Cc: Andrew Morton <akpm@linux-foundation.org>
>>> Cc: Oscar Salvador <osalvador@suse.de>
>>> Cc: Baoquan He <bhe@redhat.com>
>>> Cc: Wei Yang <richard.weiyang@gmail.com>
>>> Signed-off-by: David Hildenbrand <david@redhat.com>
>>> ---
>>> .../platforms/pseries/hotplug-memory.c | 26 +++----------------
>>> 1 file changed, 3 insertions(+), 23 deletions(-)
>>>
>>> diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
>>> index b2cde1732301..5ace2f9a277e 100644
>>> --- a/arch/powerpc/platforms/pseries/hotplug-memory.c
>>> +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
>>> @@ -337,39 +337,19 @@ static int pseries_remove_mem_node(struct device_node *np)
>>>
>>> static bool lmb_is_removable(struct drmem_lmb *lmb)
>>> {
>>> - int i, scns_per_block;
>>> - bool rc = true;
>>> - unsigned long pfn, block_sz;
>>> - u64 phys_addr;
>>> -
>>> if (!(lmb->flags & DRCONF_MEM_ASSIGNED))
>>> return false;
>>>
>>> - block_sz = memory_block_size_bytes();
>>> - scns_per_block = block_sz / MIN_MEMORY_BLOCK_SIZE;
>>> - phys_addr = lmb->base_addr;
>>> -
>>> #ifdef CONFIG_FA_DUMP
>>> /*
>>> * Don't hot-remove memory that falls in fadump boot memory area
>>> * and memory that is reserved for capturing old kernel memory.
>>> */
>>> - if (is_fadump_memory_area(phys_addr, block_sz))
>>> + if (is_fadump_memory_area(lmb->base_addr, memory_block_size_bytes()))
>>> return false;
>>> #endif
>>> -
>>> - for (i = 0; i < scns_per_block; i++) {
>>> - pfn = PFN_DOWN(phys_addr);
>>> - if (!pfn_in_present_section(pfn)) {
>>> - phys_addr += MIN_MEMORY_BLOCK_SIZE;
>>> - continue;
>>> - }
>>> -
>>> - rc = rc && is_mem_section_removable(pfn, PAGES_PER_SECTION);
>>> - phys_addr += MIN_MEMORY_BLOCK_SIZE;
>>> - }
>>> -
>>> - return rc;
>>> + /* device_offline() will determine if we can actually remove this lmb */
>>> + return true;
> So I think here swaps the check and do sequence. At least it breaks
> dlpar_memory_remove_by_count(). It is doable to remove
> is_mem_section_removable(), but here should be more effort to re-arrange
> the code.
>
Thanks Pingfan,
1. "swaps the check and do sequence":
Partially. Any caller of dlpar_remove_lmb() already has to deal with
false positives. device_offline() can easily fail after
dlpar_remove_lmb() == true. It's inherently racy.
2. "breaks dlpar_memory_remove_by_count()"
Can you elaborate why it "breaks" it? It will simply try to
offline+remove lmbs, detect that it wasn't able to offline+remove as
much as it wanted (which could happen before as well easily), and re-add
the already offlined+removed ones.
3. "more effort to re-arrange the code"
What would be your suggestion?
We would rip out that racy check if we can remove as much memory as
requested in dlpar_memory_remove_by_count() and simply always try to
remove + recover.
--
Thanks,
David / dhildenb
^ permalink raw reply
* Re: [PATCH v5 00/21] Initial Prefixed Instruction support
From: Jordan Niethe @ 2020-04-09 7:28 UTC (permalink / raw)
To: Christophe Leroy
Cc: Alistair Popple, Nicholas Piggin, Balamuruhan S, linuxppc-dev,
Daniel Axtens
In-Reply-To: <b30787a2-bcb9-de17-759a-9fcb30ac6644@c-s.fr>
On Thu, Apr 9, 2020 at 4:39 PM Christophe Leroy <christophe.leroy@c-s.fr> wrote:
>
>
>
> On 04/06/2020 08:09 AM, Jordan Niethe wrote:
> > A future revision of the ISA will introduce prefixed instructions. A
> > prefixed instruction is composed of a 4-byte prefix followed by a
> > 4-byte suffix.
> >
> > All prefixes have the major opcode 1. A prefix will never be a valid
> > word instruction. A suffix may be an existing word instruction or a
> > new instruction.
> >
> > This series enables prefixed instructions and extends the instruction
> > emulation to support them. Then the places where prefixed instructions
> > might need to be emulated are updated.
> >
> > v5 is based on feedback from Nick Piggins, Michael Ellerman, Balamuruhan
> > Suriyakumar and Alistair Popple.
> > The major changes:
> > - The ppc instruction type is now a struct
> > - Series now just based on next
> > - ppc_inst_masked() dropped
> > - Space for xmon breakpoints allocated in an assembly file
> > - "Add prefixed instructions to instruction data type" patch seperated in
> > to smaller patches
> > - Calling convention for create_branch() is changed
> > - Some places which had not been updated to use the data type are now updated
>
> Build fails. I have not investigated why:
Thanks, I will check it out.
>
> CC arch/powerpc/kernel/process.o
> In file included from ./arch/powerpc/include/asm/code-patching.h:14:0,
> from arch/powerpc/kernel/process.c:60:
> ./arch/powerpc/include/asm/inst.h:69:38: error: unknown type name ‘ppc_inst’
> static inline bool ppc_inst_prefixed(ppc_inst x)
> ^
> ./arch/powerpc/include/asm/inst.h:79:19: error: redefinition of
> ‘ppc_inst_val’
> static inline u32 ppc_inst_val(struct ppc_inst x)
> ^
> ./arch/powerpc/include/asm/inst.h:21:19: note: previous definition of
> ‘ppc_inst_val’ was here
> static inline u32 ppc_inst_val(struct ppc_inst x)
> ^
> ./arch/powerpc/include/asm/inst.h: In function ‘ppc_inst_len’:
> ./arch/powerpc/include/asm/inst.h:103:10: error: implicit declaration of
> function ‘ppc_inst_prefixed’ [-Werror=implicit-function-declaration]
> return (ppc_inst_prefixed(x)) ? 8 : 4;
> ^
>
> Christophe
>
> >
> > v4 is based on feedback from Nick Piggins, Christophe Leroy and Daniel Axtens.
> > The major changes:
> > - Move xmon breakpoints from data section to text section
> > - Introduce a data type for instructions on powerpc
> >
> > v3 is based on feedback from Christophe Leroy. The major changes:
> > - Completely replacing store_inst() with patch_instruction() in
> > xmon
> > - Improve implementation of mread_instr() to not use mread().
> > - Base the series on top of
> > https://patchwork.ozlabs.org/patch/1232619/ as this will effect
> > kprobes.
> > - Some renaming and simplification of conditionals.
> >
> > v2 incorporates feedback from Daniel Axtens and and Balamuruhan
> > S. The major changes are:
> > - Squashing together all commits about SRR1 bits
> > - Squashing all commits for supporting prefixed load stores
> > - Changing abbreviated references to sufx/prfx -> suffix/prefix
> > - Introducing macros for returning the length of an instruction
> > - Removing sign extension flag from pstd/pld in sstep.c
> > - Dropping patch "powerpc/fault: Use analyse_instr() to check for
> > store with updates to sp" from the series, it did not really fit
> > with prefixed enablement in the first place and as reported by Greg
> > Kurz did not work correctly.
> >
> >
> > Alistair Popple (1):
> > powerpc: Enable Prefixed Instructions
> >
> > Jordan Niethe (20):
> > powerpc/xmon: Remove store_inst() for patch_instruction()
> > powerpc/xmon: Move out-of-line instructions to text section
> > powerpc: Change calling convention for create_branch() et. al.
> > powerpc: Use a macro for creating instructions from u32s
> > powerpc: Use a function for getting the instruction op code
> > powerpc: Use an accessor for instructions
> > powerpc: Use a function for byte swapping instructions
> > powerpc: Introduce functions for instruction equality
> > powerpc: Use a datatype for instructions
> > powerpc: Use a function for reading instructions
> > powerpc: Define and use __get_user_instr{,inatomic}()
> > powerpc: Introduce a function for reporting instruction length
> > powerpc/xmon: Use a function for reading instructions
> > powerpc/xmon: Move insertion of breakpoint for xol'ing
> > powerpc: Make test_translate_branch() independent of instruction
> > length
> > powerpc: Define new SRR1 bits for a future ISA version
> > powerpc64: Add prefixed instructions to instruction data type
> > powerpc: Support prefixed instructions in alignment handler
> > powerpc sstep: Add support for prefixed load/stores
> > powerpc sstep: Add support for prefixed fixed-point arithmetic
> >
> > arch/powerpc/include/asm/code-patching.h | 37 +-
> > arch/powerpc/include/asm/inst.h | 106 ++++++
> > arch/powerpc/include/asm/kprobes.h | 2 +-
> > arch/powerpc/include/asm/reg.h | 7 +-
> > arch/powerpc/include/asm/sstep.h | 15 +-
> > arch/powerpc/include/asm/uaccess.h | 28 ++
> > arch/powerpc/include/asm/uprobes.h | 7 +-
> > arch/powerpc/kernel/align.c | 13 +-
> > arch/powerpc/kernel/epapr_paravirt.c | 5 +-
> > arch/powerpc/kernel/hw_breakpoint.c | 5 +-
> > arch/powerpc/kernel/jump_label.c | 5 +-
> > arch/powerpc/kernel/kgdb.c | 9 +-
> > arch/powerpc/kernel/kprobes.c | 24 +-
> > arch/powerpc/kernel/mce_power.c | 5 +-
> > arch/powerpc/kernel/module_64.c | 3 +-
> > arch/powerpc/kernel/optprobes.c | 91 +++--
> > arch/powerpc/kernel/optprobes_head.S | 3 +
> > arch/powerpc/kernel/security.c | 9 +-
> > arch/powerpc/kernel/setup_32.c | 4 +-
> > arch/powerpc/kernel/trace/ftrace.c | 190 ++++++----
> > arch/powerpc/kernel/traps.c | 20 +-
> > arch/powerpc/kernel/uprobes.c | 3 +-
> > arch/powerpc/kernel/vecemu.c | 20 +-
> > arch/powerpc/kvm/book3s_hv_nested.c | 2 +-
> > arch/powerpc/kvm/book3s_hv_rm_mmu.c | 2 +-
> > arch/powerpc/kvm/emulate_loadstore.c | 2 +-
> > arch/powerpc/lib/code-patching.c | 289 +++++++-------
> > arch/powerpc/lib/feature-fixups.c | 69 ++--
> > arch/powerpc/lib/sstep.c | 455 ++++++++++++++++-------
> > arch/powerpc/lib/test_emulate_step.c | 56 +--
> > arch/powerpc/perf/core-book3s.c | 4 +-
> > arch/powerpc/xmon/Makefile | 2 +-
> > arch/powerpc/xmon/xmon.c | 94 +++--
> > arch/powerpc/xmon/xmon_bpts.S | 10 +
> > arch/powerpc/xmon/xmon_bpts.h | 8 +
> > 35 files changed, 1042 insertions(+), 562 deletions(-)
> > create mode 100644 arch/powerpc/include/asm/inst.h
> > create mode 100644 arch/powerpc/xmon/xmon_bpts.S
> > create mode 100644 arch/powerpc/xmon/xmon_bpts.h
> >
^ permalink raw reply
* Re: [PATCH v1 1/2] powerpc/pseries/hotplug-memory: stop checking is_mem_section_removable()
From: David Hildenbrand @ 2020-04-09 7:32 UTC (permalink / raw)
To: Michael Ellerman, linux-kernel
Cc: Michal Hocko, Baoquan He, Wei Yang, linux-mm, Paul Mackerras,
Nathan Fontenot, Andrew Morton, linuxppc-dev, Oscar Salvador
In-Reply-To: <87sghdjf1y.fsf@mpe.ellerman.id.au>
> It's also not very pretty in dmesg.
>
> Before:
>
> pseries-hotplug-mem: Attempting to hot-add 10 LMB(s)
> pseries-hotplug-mem: Memory hot-add failed, removing any added LMBs
> dlpar: Could not handle DLPAR request "memory add count 10"
>
Thanks for running it through the mill.
Here you test "hotadd", below you test "hot-remove". But yeah, there is
a change in the amount of dmesg.
> After:
>
> pseries-hotplug-mem: Attempting to hot-remove 10 LMB(s)
> page:c00c000001ca8200 refcount:1 mapcount:0 mapping:0000000018c4a547 index:0xc00000072a080180
> flags: 0x7ffff000000200(slab)
> raw: 007ffff000000200 c00c000001cffd48 c000000781c51410 c000000793327580
> raw: c00000072a080180 0000000001550001 00000001ffffffff 0000000000000000
> page dumped because: unmovable page
> page:c00c000001cc4a80 refcount:1 mapcount:0 mapping:0000000018c4a547 index:0xc00000079b110080
> flags: 0x7ffff000000000()
> raw: 007ffff000000000 5deadbeef0000100 5deadbeef0000122 0000000000000000
> raw: c00000079b110080 0000000000000000 00000001ffffffff 0000000000000000
> page dumped because: unmovable page
> page:c00c000001d08200 refcount:1 mapcount:0 mapping:0000000018c4a547 index:0xc00000074208ff00
> flags: 0x7ffff000000200(slab)
> raw: 007ffff000000200 c000000781c5f150 c00c000001d37f88 c000000798a24880
> raw: c00000074208ff00 0000000001550002 00000001ffffffff 0000000000000000
> page dumped because: unmovable page
> page:c00c000001d40140 refcount:1 mapcount:0 mapping:0000000018c4a547 index:0xc000000750059c00
> flags: 0x7ffff000000200(slab)
> raw: 007ffff000000200 c00c000001dfcfc8 c00c000001d3c288 c0000007851c2d00
> raw: c000000750059c00 0000000001000003 00000001ffffffff 0000000000000000
> page dumped because: unmovable page
> page:c00c000001d9c000 refcount:1 mapcount:0 mapping:0000000018c4a547 index:0xc000000002370080
> flags: 0x7ffff000000000()
> raw: 007ffff000000000 5deadbeef0000100 5deadbeef0000122 0000000000000000
> raw: c000000002370080 0000000000000000 00000001ffffffff 0000000000000000
> page dumped because: unmovable page
> page:c00c000001dc0200 refcount:1 mapcount:0 mapping:0000000018c4a547 index:0xc000000002370080
> flags: 0x7ffff000000000()
> raw: 007ffff000000000 5deadbeef0000100 5deadbeef0000122 0000000000000000
> raw: c000000002370080 0000000000000000 00000001ffffffff 0000000000000000
> page dumped because: unmovable page
> page:c00c000001e00000 refcount:1 mapcount:0 mapping:0000000018c4a547 index:0x0
> flags: 0x7ffff000000200(slab)
> raw: 007ffff000000200 5deadbeef0000100 5deadbeef0000122 c0000007a801f500
> raw: 0000000000000000 0000000008000800 00000001ffffffff 0000000000000000
> page dumped because: unmovable page
> page:c00c000001e40440 refcount:1 mapcount:0 mapping:0000000018c4a547 index:0x0
> flags: 0x7ffff000000200(slab)
> raw: 007ffff000000200 5deadbeef0000100 5deadbeef0000122 c0000007a801e380
> raw: 0000000000000000 0000000000400040 00000001ffffffff 0000000000000000
> page dumped because: unmovable page
> page:c00c000001e80000 refcount:1 mapcount:0 mapping:0000000018c4a547 index:0xc0000007a0000640
> flags: 0x7ffff000000200(slab)
> raw: 007ffff000000200 c00c000001e5af48 c00c000001e80408 c000000f42d00a00
> raw: c0000007a0000640 00000000066600a2 00000001ffffffff 0000000000000000
> page dumped because: unmovable page
> page:c00c000003c89d40 refcount:2 mapcount:1 mapping:0000000018c4a547 index:0x10a41
> anon flags: 0x17ffff000080024(uptodate|active|swapbacked)
> raw: 017ffff000080024 5deadbeef0000100 5deadbeef0000122 c0000007986b03c9
> raw: 0000000000010a41 0000000000000000 0000000200000000 c00000000340b000
> page dumped because: unmovable page
> page->mem_cgroup:c00000000340b000
> page:c00c000003cc0000 refcount:1 mapcount:0 mapping:0000000018c4a547 index:0xc000000f3000fd38
> flags: 0x17ffff000000200(slab)
> raw: 017ffff000000200 c000000f3c911890 c000000f3c911890 c00000079fffd980
> raw: c000000f3000fd38 0000000000700003 00000001ffffffff 0000000000000000
> page dumped because: unmovable page
> page:c00c000003d00000 refcount:1 mapcount:0 mapping:0000000018c4a547 index:0xc0000007a2ec0000
> flags: 0x17ffff000000000()
> raw: 017ffff000000000 5deadbeef0000100 5deadbeef0000122 0000000000000000
> raw: c0000007a2ec0000 0000000000000000 00000001ffffffff 0000000000000000
> page dumped because: unmovable page
> page:c00c000003e2c000 refcount:1 mapcount:0 mapping:0000000018c4a547 index:0xc000000f8b008400
> flags: 0x27ffff000000200(slab)
> raw: 027ffff000000200 c000000f8e000190 c000000f8e000190 c0000007a801e380
> raw: c000000f8b008400 0000000000400038 00000001ffffffff 0000000000000000
> page dumped because: unmovable page
> page:c00c000003fec000 refcount:1 mapcount:0 mapping:0000000018c4a547 index:0x0
> flags: 0x37ffff000000000()
> raw: 037ffff000000000 5deadbeef0000100 5deadbeef0000122 0000000000000000
> raw: 0000000000000000 0000000000000000 00000001ffffffff 0000000000000000
> page dumped because: unmovable page
> pseries-hotplug-mem: Memory hot-remove failed, adding LMB's back
> dlpar: Could not handle DLPAR request "memory remove count 10"
>
>
>
> It looks like set_migratetype_isolate() and start_isolate_page_range()
> can be told not to report those warnings, but we're just calling
> device_offline() which doesn't let us specify that.
Yeah, but these messages can easily pop up (to a more limited degree)
with the current code as well, as checking for movable pages without
isolating the page range gives you no guarantees that no unmovable data
will end up on the lmb until you offline it. It's simply racy.
I discussed this output with Michal when we changed the
/sys/devices/system/memory/memoryX/removable behavior, and he had the
opinion that dmesg (debug) output should not really be an issue.
We could make this output
a) configurable at runtime and let powerpc code disable it while calling
device_offline(). So user space attempts will still trigger the messages
b) configurable at compile time
--
Thanks,
David / dhildenb
^ permalink raw reply
* Re: [PATCH 03/35] docs: fix broken references to text files
From: Marc Zyngier @ 2020-04-09 7:45 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: kvm, Linux Doc Mailing List, Peter Zijlstra, Akira Yokosawa,
dri-devel, linux-unionfs, linux-mm, Harry Wei, Alex Shi,
Will Deacon, kvmarm, linux-arch, Jason Gunthorpe, Jonathan Corbet,
linux-rdma, kvm-ppc, David Airlie, Doug Ledford, Alan Stern,
linux-arm-kernel, Federico Vaga, Jade Alglave, Daniel Lustig,
Julien Thierry, Mike Leach, Andrea Parri, Daniel Vetter,
Paul E. McKenney, Suzuki K Poulose, Boqun Feng, Maarten Lankhorst,
Nicholas Piggin, Maxime Ripard, Luc Maranget, OGAWA Hirofumi,
David Howells, Mathieu Poirier, Miklos Szeredi, linux-kernel,
Alexander Shishkin, James Morse, Thomas Zimmermann, linux-fsdevel,
Paolo Bonzini, Andrew Morton, linuxppc-dev
In-Reply-To: <2e12bf13355bd06748fed5d135fd4de3d94ad5fd.1586359676.git.mchehab+huawei@kernel.org>
On Wed, 8 Apr 2020 17:45:55 +0200
Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote:
> Several references got broken due to txt to ReST conversion.
>
> Several of them can be automatically fixed with:
>
> scripts/documentation-file-ref-check --fix
>
> Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org> # hwtracing/coresight/Kconfig
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> ---
> Documentation/virt/kvm/arm/pvtime.rst | 2 +-
> virt/kvm/arm/vgic/vgic-mmio-v3.c | 2 +-
> virt/kvm/arm/vgic/vgic.h | 4 ++--
Acked-by: Marc Zyngier <maz@kernel.org> # kvm/arm64
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* Re: [PATCH] papr/scm: Add bad memory ranges to nvdimm bad ranges
From: Santosh Sivaraj @ 2020-04-09 7:40 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Ganesh Goudar, Oliver, Mahesh Salgaonkar, Aneesh Kumar K.V
In-Reply-To: <20200401074741.1562361-1-santosh@fossix.org>
[-- Attachment #1: Type: text/plain, Size: 5603 bytes --]
On Wed, Apr 1, 2020 at 1:18 PM Santosh Sivaraj <santosh@fossix.org> wrote:
> Subscribe to the MCE notification and add the physical address which
> generated a memory error to nvdimm bad range.
>
> Signed-off-by: Santosh Sivaraj <santosh@fossix.org>
> ---
>
Any comments on this?
Thanks,
Santosh
> This patch depends on "powerpc/mce: Add MCE notification chain" [1].
>
> Unlike the previous series[2], the patch adds badblock registration only
> for
> pseries scm driver. Handling badblocks for baremetal (powernv) PMEM will
> be done
> later and if possible get the badblock handling as a common code.
>
> [1]
> https://lore.kernel.org/linuxppc-dev/20200330071219.12284-1-ganeshgr@linux.ibm.com/
> [2]
> https://lore.kernel.org/linuxppc-dev/20190820023030.18232-1-santosh@fossix.org/
>
> arch/powerpc/platforms/pseries/papr_scm.c | 96 ++++++++++++++++++++++-
> 1 file changed, 95 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/platforms/pseries/papr_scm.c
> b/arch/powerpc/platforms/pseries/papr_scm.c
> index 0b4467e378e5..5012cbf4606e 100644
> --- a/arch/powerpc/platforms/pseries/papr_scm.c
> +++ b/arch/powerpc/platforms/pseries/papr_scm.c
> @@ -12,6 +12,8 @@
> #include <linux/libnvdimm.h>
> #include <linux/platform_device.h>
> #include <linux/delay.h>
> +#include <linux/nd.h>
> +#include <asm/mce.h>
>
> #include <asm/plpar_wrappers.h>
>
> @@ -39,8 +41,12 @@ struct papr_scm_priv {
> struct resource res;
> struct nd_region *region;
> struct nd_interleave_set nd_set;
> + struct list_head region_list;
> };
>
> +LIST_HEAD(papr_nd_regions);
> +DEFINE_MUTEX(papr_ndr_lock);
> +
> static int drc_pmem_bind(struct papr_scm_priv *p)
> {
> unsigned long ret[PLPAR_HCALL_BUFSIZE];
> @@ -372,6 +378,10 @@ static int papr_scm_nvdimm_init(struct papr_scm_priv
> *p)
> dev_info(dev, "Region registered with target node %d and
> online node %d",
> target_nid, online_nid);
>
> + mutex_lock(&papr_ndr_lock);
> + list_add_tail(&p->region_list, &papr_nd_regions);
> + mutex_unlock(&papr_ndr_lock);
> +
> return 0;
>
> err: nvdimm_bus_unregister(p->bus);
> @@ -379,6 +389,68 @@ err: nvdimm_bus_unregister(p->bus);
> return -ENXIO;
> }
>
> +void papr_scm_add_badblock(struct nd_region *region, struct nvdimm_bus
> *bus,
> + u64 phys_addr)
> +{
> + u64 aligned_addr = ALIGN_DOWN(phys_addr, L1_CACHE_BYTES);
> +
> + if (nvdimm_bus_add_badrange(bus, aligned_addr, L1_CACHE_BYTES)) {
> + pr_err("Bad block registration for 0x%llx failed\n",
> phys_addr);
> + return;
> + }
> +
> + pr_debug("Add memory range (0x%llx - 0x%llx) as bad range\n",
> + aligned_addr, aligned_addr + L1_CACHE_BYTES);
> +
> + nvdimm_region_notify(region, NVDIMM_REVALIDATE_POISON);
> +}
> +
> +static int handle_mce_ue(struct notifier_block *nb, unsigned long val,
> + void *data)
> +{
> + struct machine_check_event *evt = data;
> + struct papr_scm_priv *p;
> + u64 phys_addr;
> + bool found = false;
> +
> + if (evt->error_type != MCE_ERROR_TYPE_UE)
> + return NOTIFY_DONE;
> +
> + if (list_empty(&papr_nd_regions))
> + return NOTIFY_DONE;
> +
> + phys_addr = evt->u.ue_error.physical_address +
> + (evt->u.ue_error.effective_address & ~PAGE_MASK);
> +
> + if (!evt->u.ue_error.physical_address_provided ||
> + !is_zone_device_page(pfn_to_page(phys_addr >> PAGE_SHIFT)))
> + return NOTIFY_DONE;
> +
> + /* mce notifier is called from a process context, so mutex is safe
> */
> + mutex_lock(&papr_ndr_lock);
> + list_for_each_entry(p, &papr_nd_regions, region_list) {
> + struct resource res = p->res;
> +
> + if (phys_addr >= res.start && phys_addr <= res.end) {
> + found = true;
> + break;
> + }
> + }
> +
> + mutex_unlock(&papr_ndr_lock);
> +
> + if (!found)
> + return NOTIFY_DONE;
> +
> + papr_scm_add_badblock(p->region, p->bus, phys_addr);
> +
> + return NOTIFY_OK;
> +}
> +
> +static struct notifier_block mce_ue_nb = {
> + .notifier_call = handle_mce_ue
> +};
> +
> static int papr_scm_probe(struct platform_device *pdev)
> {
> struct device_node *dn = pdev->dev.of_node;
> @@ -476,6 +548,10 @@ static int papr_scm_remove(struct platform_device
> *pdev)
> {
> struct papr_scm_priv *p = platform_get_drvdata(pdev);
>
> + mutex_lock(&papr_ndr_lock);
> + list_del(&(p->region_list));
> + mutex_unlock(&papr_ndr_lock);
> +
> nvdimm_bus_unregister(p->bus);
> drc_pmem_unbind(p);
> kfree(p->bus_desc.provider_name);
> @@ -498,7 +574,25 @@ static struct platform_driver papr_scm_driver = {
> },
> };
>
> -module_platform_driver(papr_scm_driver);
> +static int __init papr_scm_init(void)
> +{
> + int ret;
> +
> + ret = platform_driver_register(&papr_scm_driver);
> + if (!ret)
> + mce_register_notifier(&mce_ue_nb);
> +
> + return ret;
> +}
> +module_init(papr_scm_init);
> +
> +static void __exit papr_scm_exit(void)
> +{
> + mce_unregister_notifier(&mce_ue_nb);
> + platform_driver_unregister(&papr_scm_driver);
> +}
> +module_exit(papr_scm_exit);
> +
> MODULE_DEVICE_TABLE(of, papr_scm_match);
> MODULE_LICENSE("GPL");
> MODULE_AUTHOR("IBM Corporation");
> --
> 2.25.1
>
>
[-- Attachment #2: Type: text/html, Size: 7541 bytes --]
^ 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