* [RFC PATCH 1/7] mm: sparse: set/clear subsection bitmap when pages are onlined/offlined.
From: Zi Yan @ 2021-05-06 15:26 UTC (permalink / raw)
To: David Hildenbrand, Oscar Salvador
Cc: Michal Hocko, linux-ia64, Wei Yang, Anshuman Khandual,
Rafael J . Wysocki, x86, Dan Williams, linux-kernel, linux-mm,
Andy Lutomirski, Thomas Gleixner, Zi Yan, linuxppc-dev,
Andrew Morton, Mike Rapoport
In-Reply-To: <20210506152623.178731-1-zi.yan@sent.com>
From: Zi Yan <ziy@nvidia.com>
subsection bitmap was set/cleared when a section is added/removed, but
pfn_to_online_page() uses subsection bitmap to check if the page is
online, which is not accurate. It was working when a whole section is
added/removed during memory hotplug and hotremove. When the following
patches enable memory hotplug and hotremove for subsections,
subsection bitmap needs to be changed during page online/offline time,
otherwise, pfn_to_online_page() will not give right answers. Move the
subsection bitmap manipulation code from section_activate() to
online_mem_sections() and section_deactivate() to
offline_mem_sections(), respectively.
Signed-off-by: Zi Yan <ziy@nvidia.com>
---
mm/sparse.c | 36 +++++++++++++++++++++++++++++++++---
1 file changed, 33 insertions(+), 3 deletions(-)
diff --git a/mm/sparse.c b/mm/sparse.c
index b2ada9dc00cb..7637208b8874 100644
--- a/mm/sparse.c
+++ b/mm/sparse.c
@@ -606,6 +606,7 @@ void __init sparse_init(void)
#ifdef CONFIG_MEMORY_HOTPLUG
+static int fill_subsection_map(unsigned long pfn, unsigned long nr_pages);
/* Mark all memory sections within the pfn range as online */
void online_mem_sections(unsigned long start_pfn, unsigned long end_pfn)
{
@@ -621,9 +622,12 @@ void online_mem_sections(unsigned long start_pfn, unsigned long end_pfn)
ms = __nr_to_section(section_nr);
ms->section_mem_map |= SECTION_IS_ONLINE;
+ fill_subsection_map(pfn, min(end_pfn, pfn + PAGES_PER_SECTION) - pfn);
}
}
+static int clear_subsection_map(unsigned long pfn, unsigned long nr_pages);
+static bool is_subsection_map_empty(struct mem_section *ms);
/* Mark all memory sections within the pfn range as offline */
void offline_mem_sections(unsigned long start_pfn, unsigned long end_pfn)
{
@@ -641,7 +645,13 @@ void offline_mem_sections(unsigned long start_pfn, unsigned long end_pfn)
continue;
ms = __nr_to_section(section_nr);
- ms->section_mem_map &= ~SECTION_IS_ONLINE;
+
+ if (end_pfn < pfn + PAGES_PER_SECTION) {
+ clear_subsection_map(pfn, end_pfn - pfn);
+ if (is_subsection_map_empty(ms))
+ ms->section_mem_map &= ~SECTION_IS_ONLINE;
+ } else
+ ms->section_mem_map &= ~SECTION_IS_ONLINE;
}
}
@@ -668,6 +678,17 @@ static void free_map_bootmem(struct page *memmap)
vmemmap_free(start, end, NULL);
}
+static int subsection_map_intersects(struct mem_section *ms, unsigned long pfn,
+ unsigned long nr_pages)
+{
+ DECLARE_BITMAP(map, SUBSECTIONS_PER_SECTION) = { 0 };
+ unsigned long *subsection_map = &ms->usage->subsection_map[0];
+
+ subsection_mask_set(map, pfn, nr_pages);
+
+ return bitmap_intersects(map, subsection_map, SUBSECTIONS_PER_SECTION);
+}
+
static int clear_subsection_map(unsigned long pfn, unsigned long nr_pages)
{
DECLARE_BITMAP(map, SUBSECTIONS_PER_SECTION) = { 0 };
@@ -760,6 +781,12 @@ static void free_map_bootmem(struct page *memmap)
}
}
+static int subsection_map_intersects(struct mem_section *ms, unsigned long pfn,
+ unsigned long nr_pages)
+{
+ return 0;
+}
+
static int clear_subsection_map(unsigned long pfn, unsigned long nr_pages)
{
return 0;
@@ -800,7 +827,10 @@ static void section_deactivate(unsigned long pfn, unsigned long nr_pages,
struct page *memmap = NULL;
bool empty;
- if (clear_subsection_map(pfn, nr_pages))
+ if (WARN((IS_ENABLED(CONFIG_SPARSEMEM_VMEMMAP) && !ms->usage) ||
+ subsection_map_intersects(ms, pfn, nr_pages),
+ "section already deactivated (%#lx + %ld)\n",
+ pfn, nr_pages))
return;
empty = is_subsection_map_empty(ms);
@@ -855,7 +885,7 @@ static struct page * __meminit section_activate(int nid, unsigned long pfn,
ms->usage = usage;
}
- rc = fill_subsection_map(pfn, nr_pages);
+ rc = !nr_pages || subsection_map_intersects(ms, pfn, nr_pages);
if (rc) {
if (usage)
ms->usage = NULL;
--
2.30.2
^ permalink raw reply related
* Re: [RFC PATCH 0/7] Memory hotplug/hotremove at subsection size
From: David Hildenbrand @ 2021-05-06 15:31 UTC (permalink / raw)
To: Zi Yan, Oscar Salvador
Cc: Michal Hocko, linux-ia64, Wei Yang, Anshuman Khandual,
Rafael J . Wysocki, x86, Dan Williams, linux-kernel, linux-mm,
Andy Lutomirski, Thomas Gleixner, linuxppc-dev, Andrew Morton,
Mike Rapoport
In-Reply-To: <20210506152623.178731-1-zi.yan@sent.com>
On 06.05.21 17:26, Zi Yan wrote:
> From: Zi Yan <ziy@nvidia.com>
>
> Hi all,
>
> This patchset tries to remove the restriction on memory hotplug/hotremove
> granularity, which is always greater or equal to memory section size[1].
> With the patchset, kernel is able to online/offline memory at a size independent
> of memory section size, as small as 2MB (the subsection size).
... which doesn't make any sense as we can only online/offline whole
memory block devices.
>
> The motivation is to increase MAX_ORDER of the buddy allocator and pageblock
> size without increasing memory hotplug/hotremove granularity at the same time,
Gah, no. Please no. No.
> so that the kernel can allocator 1GB pages using buddy allocator and utilizes
> existing pageblock based anti-fragmentation, paving the road for 1GB THP
> support[2].
Not like this, please no.
>
> The patchset utilizes the existing subsection support[3] and changes the
> section size alignment checks to subsection size alignment checks. There are
> also changes to pageblock code to support partial pageblocks, when pageblock
> size is increased along with MAX_ORDER. Increasing pageblock size can enable
> kernel to utilize existing anti-fragmentation mechanism for gigantic page
> allocations.
Please not like this.
>
> The last patch increases SECTION_SIZE_BITS to demonstrate the use of memory
> hotplug/hotremove subsection, but is not intended to be merged as is. It is
> there in case one wants to try this out and will be removed during the final
> submission.
>
> Feel free to give suggestions and comments. I am looking forward to your
> feedback.
Please not like this.
--
Thanks,
David / dhildenb
^ permalink raw reply
* [PATCH] powerpc/interrupts: Fix kuep_unlock() call
From: Christophe Leroy @ 2021-05-06 14:49 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
Same as kuap_user_restore(), kuep_unlock() has to be called when
really returning to user, that is in interrupt_exit_user_prepare(),
not in interrupt_exit_prepare().
Fixes: b5efec00b671 ("powerpc/32s: Move KUEP locking/unlocking in C")
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/interrupt.h | 2 --
arch/powerpc/kernel/interrupt.c | 1 +
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/interrupt.h b/arch/powerpc/include/asm/interrupt.h
index 44cde2e129b8..c77e8f57ff06 100644
--- a/arch/powerpc/include/asm/interrupt.h
+++ b/arch/powerpc/include/asm/interrupt.h
@@ -153,8 +153,6 @@ static inline void interrupt_enter_prepare(struct pt_regs *regs, struct interrup
*/
static inline void interrupt_exit_prepare(struct pt_regs *regs, struct interrupt_state *state)
{
- if (user_mode(regs))
- kuep_unlock();
}
static inline void interrupt_async_enter_prepare(struct pt_regs *regs, struct interrupt_state *state)
diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
index 30a596182baa..e0938ba298f2 100644
--- a/arch/powerpc/kernel/interrupt.c
+++ b/arch/powerpc/kernel/interrupt.c
@@ -424,6 +424,7 @@ notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs, unsigned
/* Restore user access locks last */
kuap_user_restore(regs);
+ kuep_unlock();
return ret;
}
--
2.25.0
^ permalink raw reply related
* Re: [PATCH] KVM: PPC: Book3S HV: Fix conversion to gfn-based MMU notifier callbacks
From: Michael Ellerman @ 2021-05-06 13:43 UTC (permalink / raw)
To: Paolo Bonzini, Nicholas Piggin, kvm-ppc
Cc: Sean Christopherson, linuxppc-dev, Aneesh Kumar K . V, kvm,
Bharata B Rao
In-Reply-To: <9e0a256b-fb5a-4468-ed21-68d524d6ea56@redhat.com>
Paolo Bonzini <pbonzini@redhat.com> writes:
> On 05/05/21 14:15, Nicholas Piggin wrote:
>> Commit b1c5356e873c ("KVM: PPC: Convert to the gfn-based MMU notifier
>> callbacks") causes unmap_gfn_range and age_gfn callbacks to only work
>> on the first gfn in the range. It also makes the aging callbacks call
>> into both radix and hash aging functions for radix guests. Fix this.
>>
>> Add warnings for the single-gfn calls that have been converted to range
>> callbacks, in case they ever receieve ranges greater than 1.
>>
>> Fixes: b1c5356e873c ("KVM: PPC: Convert to the gfn-based MMU notifier callbacks")
>> Reported-by: Bharata B Rao <bharata@linux.ibm.com>
>> Tested-by: Bharata B Rao <bharata@linux.ibm.com>
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>
> Sorry for the breakage. I queued this patch.
Thanks. Are you planning to send it to Linus before rc1?
If not I can pick it up as I already have some things in my next and am
intending to send a pull request anyway.
cheers
^ permalink raw reply
* [PATCH] powerpc/32s: Remove asm/book3s/32/hash.h
From: Christophe Leroy @ 2021-05-06 13:32 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
Move the PAGE bits into pgtable.h to be more similar to book3s/64.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/book3s/32/hash.h | 45 --------------------
arch/powerpc/include/asm/book3s/32/pgtable.h | 38 ++++++++++++++++-
2 files changed, 37 insertions(+), 46 deletions(-)
delete mode 100644 arch/powerpc/include/asm/book3s/32/hash.h
diff --git a/arch/powerpc/include/asm/book3s/32/hash.h b/arch/powerpc/include/asm/book3s/32/hash.h
deleted file mode 100644
index 2a0a467d2985..000000000000
--- a/arch/powerpc/include/asm/book3s/32/hash.h
+++ /dev/null
@@ -1,45 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef _ASM_POWERPC_BOOK3S_32_HASH_H
-#define _ASM_POWERPC_BOOK3S_32_HASH_H
-#ifdef __KERNEL__
-
-/*
- * The "classic" 32-bit implementation of the PowerPC MMU uses a hash
- * table containing PTEs, together with a set of 16 segment registers,
- * to define the virtual to physical address mapping.
- *
- * We use the hash table as an extended TLB, i.e. a cache of currently
- * active mappings. We maintain a two-level page table tree, much
- * like that used by the i386, for the sake of the Linux memory
- * management code. Low-level assembler code in hash_low_32.S
- * (procedure hash_page) is responsible for extracting ptes from the
- * tree and putting them into the hash table when necessary, and
- * updating the accessed and modified bits in the page table tree.
- */
-
-#define _PAGE_PRESENT 0x001 /* software: pte contains a translation */
-#define _PAGE_HASHPTE 0x002 /* hash_page has made an HPTE for this pte */
-#define _PAGE_USER 0x004 /* usermode access allowed */
-#define _PAGE_GUARDED 0x008 /* G: prohibit speculative access */
-#define _PAGE_COHERENT 0x010 /* M: enforce memory coherence (SMP systems) */
-#define _PAGE_NO_CACHE 0x020 /* I: cache inhibit */
-#define _PAGE_WRITETHRU 0x040 /* W: cache write-through */
-#define _PAGE_DIRTY 0x080 /* C: page changed */
-#define _PAGE_ACCESSED 0x100 /* R: page referenced */
-#define _PAGE_EXEC 0x200 /* software: exec allowed */
-#define _PAGE_RW 0x400 /* software: user write access allowed */
-#define _PAGE_SPECIAL 0x800 /* software: Special page */
-
-#ifdef CONFIG_PTE_64BIT
-/* We never clear the high word of the pte */
-#define _PTE_NONE_MASK (0xffffffff00000000ULL | _PAGE_HASHPTE)
-#else
-#define _PTE_NONE_MASK _PAGE_HASHPTE
-#endif
-
-#define _PMD_PRESENT 0
-#define _PMD_PRESENT_MASK (PAGE_MASK)
-#define _PMD_BAD (~PAGE_MASK)
-
-#endif /* __KERNEL__ */
-#endif /* _ASM_POWERPC_BOOK3S_32_HASH_H */
diff --git a/arch/powerpc/include/asm/book3s/32/pgtable.h b/arch/powerpc/include/asm/book3s/32/pgtable.h
index 83c65845a1a9..609c80f67194 100644
--- a/arch/powerpc/include/asm/book3s/32/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/32/pgtable.h
@@ -4,7 +4,43 @@
#include <asm-generic/pgtable-nopmd.h>
-#include <asm/book3s/32/hash.h>
+/*
+ * The "classic" 32-bit implementation of the PowerPC MMU uses a hash
+ * table containing PTEs, together with a set of 16 segment registers,
+ * to define the virtual to physical address mapping.
+ *
+ * We use the hash table as an extended TLB, i.e. a cache of currently
+ * active mappings. We maintain a two-level page table tree, much
+ * like that used by the i386, for the sake of the Linux memory
+ * management code. Low-level assembler code in hash_low_32.S
+ * (procedure hash_page) is responsible for extracting ptes from the
+ * tree and putting them into the hash table when necessary, and
+ * updating the accessed and modified bits in the page table tree.
+ */
+
+#define _PAGE_PRESENT 0x001 /* software: pte contains a translation */
+#define _PAGE_HASHPTE 0x002 /* hash_page has made an HPTE for this pte */
+#define _PAGE_USER 0x004 /* usermode access allowed */
+#define _PAGE_GUARDED 0x008 /* G: prohibit speculative access */
+#define _PAGE_COHERENT 0x010 /* M: enforce memory coherence (SMP systems) */
+#define _PAGE_NO_CACHE 0x020 /* I: cache inhibit */
+#define _PAGE_WRITETHRU 0x040 /* W: cache write-through */
+#define _PAGE_DIRTY 0x080 /* C: page changed */
+#define _PAGE_ACCESSED 0x100 /* R: page referenced */
+#define _PAGE_EXEC 0x200 /* software: exec allowed */
+#define _PAGE_RW 0x400 /* software: user write access allowed */
+#define _PAGE_SPECIAL 0x800 /* software: Special page */
+
+#ifdef CONFIG_PTE_64BIT
+/* We never clear the high word of the pte */
+#define _PTE_NONE_MASK (0xffffffff00000000ULL | _PAGE_HASHPTE)
+#else
+#define _PTE_NONE_MASK _PAGE_HASHPTE
+#endif
+
+#define _PMD_PRESENT 0
+#define _PMD_PRESENT_MASK (PAGE_MASK)
+#define _PMD_BAD (~PAGE_MASK)
/* And here we include common definitions */
--
2.25.0
^ permalink raw reply related
* [PATCH] powerpc: Only pad struct pt_regs when needed
From: Christophe Leroy @ 2021-05-06 13:30 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
If neither KUAP nor PPC64 is selected, there is nothing in the second
union of struct pt_regs, so the alignment padding is waste of memory.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/ptrace.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
index 9c9ab2746168..4fd3a3bd5272 100644
--- a/arch/powerpc/include/asm/ptrace.h
+++ b/arch/powerpc/include/asm/ptrace.h
@@ -47,7 +47,7 @@ struct pt_regs
unsigned long result;
};
};
-
+#if defined(CONFIG_PPC64) || defined(CONFIG_PPC_KUAP)
union {
struct {
#ifdef CONFIG_PPC64
@@ -67,6 +67,7 @@ struct pt_regs
};
unsigned long __pad[4]; /* Maintain 16 byte interrupt stack alignment */
};
+#endif
};
#endif
--
2.25.0
^ permalink raw reply related
* [PATCH] powerpc/32s: Speed up likely path of kuap_update_sr()
From: Christophe Leroy @ 2021-05-06 13:27 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In most cases, kuap_update_sr() will update a single segment
register.
We know that first update will always be done, if there is no
segment register to update at all, kuap_update_sr() is not
called.
Avoid recurring calculations and tests in that case.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/book3s/32/kup.h | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/32/kup.h b/arch/powerpc/include/asm/book3s/32/kup.h
index 1670dfe9d4f1..c1f7c2e625a6 100644
--- a/arch/powerpc/include/asm/book3s/32/kup.h
+++ b/arch/powerpc/include/asm/book3s/32/kup.h
@@ -15,11 +15,13 @@ static inline void kuap_update_sr(u32 sr, u32 addr, u32 end)
{
addr &= 0xf0000000; /* align addr to start of segment */
barrier(); /* make sure thread.kuap is updated before playing with SRs */
- while (addr < end) {
+ for (;;) {
mtsr(sr, addr);
+ addr += 0x10000000; /* address of next segment */
+ if (addr >= end)
+ break;
sr += 0x111; /* next VSID */
sr &= 0xf0ffffff; /* clear VSID overflow */
- addr += 0x10000000; /* address of next segment */
}
isync(); /* Context sync required after mtsr() */
}
--
2.25.0
^ permalink raw reply related
* [PATCH] powerpc/syscall: Calling kuap_save_and_lock() is wrong
From: Christophe Leroy @ 2021-05-06 11:56 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
kuap_save_and_lock() is only for interrupts inside kernel.
system call are only from user, calling kuap_save_and_lock()
is wrong.
Fixes: c16728835eec ("powerpc/32: Manage KUAP in C")
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/kernel/interrupt.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
index e4559f8914eb..30a596182baa 100644
--- a/arch/powerpc/kernel/interrupt.c
+++ b/arch/powerpc/kernel/interrupt.c
@@ -34,9 +34,6 @@ notrace long system_call_exception(long r3, long r4, long r5,
syscall_fn f;
kuep_lock();
-#ifdef CONFIG_PPC32
- kuap_save_and_lock(regs);
-#endif
regs->orig_gpr3 = r3;
--
2.25.0
^ permalink raw reply related
* Re: [RESEND PATCH v4 10/11] powerpc: Protect patching_mm with a lock
From: Peter Zijlstra @ 2021-05-06 10:51 UTC (permalink / raw)
To: Christopher M. Riedl; +Cc: tglx, x86, linuxppc-dev, linux-hardening, keescook
In-Reply-To: <20210506043452.9674-11-cmr@linux.ibm.com>
On Wed, May 05, 2021 at 11:34:51PM -0500, Christopher M. Riedl wrote:
> Powerpc allows for multiple CPUs to patch concurrently. When patching
> with STRICT_KERNEL_RWX a single patching_mm is allocated for use by all
> CPUs for the few times that patching occurs. Use a spinlock to protect
> the patching_mm from concurrent use.
>
> Modify patch_instruction() to acquire the lock, perform the patch op,
> and then release the lock.
>
> Also introduce {lock,unlock}_patching() along with
> patch_instruction_unlocked() to avoid per-iteration lock overhead when
> patch_instruction() is called in a loop. A follow-up patch converts some
> uses of patch_instruction() to use patch_instruction_unlocked() instead.
x86 uses text_mutex for all this, why not do the same?
^ permalink raw reply
* [PATCH] powerpc/32s: Remove m8260_gorom()
From: Christophe Leroy @ 2021-05-06 9:10 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
Last user of m8260_gorom() was removed by
Commit 917f0af9e5a9 ("powerpc: Remove arch/ppc and include/asm-ppc")
removed last user of m8260_gorom().
In fact m8260_gorom() was ported to arch/powerpc/ but the
platform using it died with arch/ppc/
Remove it.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/kernel/head_book3s_32.S | 36 ----------------------------
1 file changed, 36 deletions(-)
diff --git a/arch/powerpc/kernel/head_book3s_32.S b/arch/powerpc/kernel/head_book3s_32.S
index c1d2f0d1d6b2..74296708b35e 100644
--- a/arch/powerpc/kernel/head_book3s_32.S
+++ b/arch/powerpc/kernel/head_book3s_32.S
@@ -1204,42 +1204,6 @@ setup_usbgecko_bat:
blr
#endif
-#ifdef CONFIG_8260
-/* Jump into the system reset for the rom.
- * We first disable the MMU, and then jump to the ROM reset address.
- *
- * r3 is the board info structure, r4 is the location for starting.
- * I use this for building a small kernel that can load other kernels,
- * rather than trying to write or rely on a rom monitor that can tftp load.
- */
- .globl m8260_gorom
-m8260_gorom:
- mfmsr r0
- rlwinm r0,r0,0,17,15 /* clear MSR_EE in r0 */
- sync
- mtmsr r0
- sync
- mfspr r11, SPRN_HID0
- lis r10, 0
- ori r10,r10,HID0_ICE|HID0_DCE
- andc r11, r11, r10
- mtspr SPRN_HID0, r11
- isync
- li r5, MSR_ME|MSR_RI
- lis r6,2f@h
- addis r6,r6,-KERNELBASE@h
- ori r6,r6,2f@l
- mtspr SPRN_SRR0,r6
- mtspr SPRN_SRR1,r5
- isync
- sync
- rfi
-2:
- mtlr r4
- blr
-#endif
-
-
/*
* We put a few things here that have to be page-aligned.
* This stuff goes at the beginning of the data segment,
--
2.25.0
^ permalink raw reply related
* Re: [FSL P50x0] Xorg always restarts again and again after the the PowerPC updates 5.13-1
From: Christophe Leroy @ 2021-05-06 8:09 UTC (permalink / raw)
To: Christian Zigotzky
Cc: Darren Stevens, R.T.Dickinson, mad skateman, linuxppc-dev,
Christian Zigotzky
In-Reply-To: <B67668EF-EDC8-45F9-A340-5B453065166C@xenosoft.de>
Hi,
Le 06/05/2021 à 09:56, Christian Zigotzky a écrit :
> Hi Christophe,
>
> Ok, so let's summarise from my side.
>
> The issue is in the PowerPC updates 5.13-1. I reverted these and after that the issue is gone.
> We know that only BookE machines are affected. Book3S machines are working with the PowerPC updates.
> I think it’s not directly an Xorg issue. It’s more a symptom that Xorg restarts again and again. In my point of view the changes for BookE machines in the PowerPC updates are responsible for this issue.
> Bisecting costs a lot of time and I don’t have time for my main work anymore.
> Bisecting is good but sometime you have to check your code yourself. We know all facts and now it’s time to check the code because of BookE compatibility.
>
> @All
> You can test it with QEMU as well. I provide some virtual machines and kernels for testing. Guys, it is really important that you test your changes before you release them.
>
So, summary from my side:
You popped up telling that commit 887f3ceb51cd was the reason of your problem. As I am the one who
released that commit, I took a look, and identified that 525642624783 should have fixed it.
You are working with a 64 bits kernel. My domain is 32 bits kernels.
I have no problem at all with corenet64_smp_defconfig booting QEMU with any of the commits you pointed.
On my side QEMU doesn't work at all with the configuration you provided, I don't get any output at
all on the screen.
So how can we progress ?
I know bisecting is not always easy, and for sure you must have spend a lot of time with all those
skipped steps. But it provided us good information anyway and I'm sure we could progress quickly if
you can do the few tests I suggested in my last email:
- Can you check that 887f3ceb51cd with cherry-picked 525642624783 has Xorg working ?
- Can you bisect between 887f3ceb51cd[good] and 56bec2f9d4d0[bad] to identify first bad commit that
stops after loading the dtb and uImage ?
- Once that first bad commit is identified, can you check whether the preceeding commit with
cherry-picked 525642624783 has Xorg working or not ?
Thanks
Christophe
^ permalink raw reply
* Re: [PATCH 3/3] i2c: mpc: implement erratum A-004447 workaround
From: Andy Shevchenko @ 2021-05-06 8:03 UTC (permalink / raw)
To: Chris Packham
Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
wsa@kernel.org, robh+dt@kernel.org, linux-i2c@vger.kernel.org,
andriy.shevchenko@linux.intel.com, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20210506011015.17347-4-chris.packham@alliedtelesis.co.nz>
[-- Attachment #1: Type: text/plain, Size: 4798 bytes --]
On Thursday, May 6, 2021, Chris Packham <chris.packham@alliedtelesis.co.nz>
wrote:
> The P2040/P2041 has an erratum where the normal i2c recovery mechanism
> does not work. Implement the alternative recovery mechanism documented
> in the P2040 Chip Errata Rev Q.
>
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
> drivers/i2c/busses/i2c-mpc.c | 88 +++++++++++++++++++++++++++++++++++-
> 1 file changed, 86 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
> index 30d9e89a3db2..052e37718771 100644
> --- a/drivers/i2c/busses/i2c-mpc.c
> +++ b/drivers/i2c/busses/i2c-mpc.c
> @@ -45,6 +45,7 @@
> #define CCR_MTX 0x10
> #define CCR_TXAK 0x08
> #define CCR_RSTA 0x04
> +#define CCR_RSVD 0x02
>
> #define CSR_MCF 0x80
> #define CSR_MAAS 0x40
> @@ -97,7 +98,7 @@ struct mpc_i2c {
> u32 block;
> int rc;
> int expect_rxack;
> -
> + bool has_errata_A004447;
> };
>
> struct mpc_i2c_divider {
> @@ -136,6 +137,83 @@ static void mpc_i2c_fixup(struct mpc_i2c *i2c)
> }
> }
>
> +static int i2c_mpc_wait_sr(struct mpc_i2c *i2c, int mask)
> +{
> + unsigned long timeout = jiffies + usecs_to_jiffies(100);
> + int ret = 0;
> +
> + while ((readb(i2c->base + MPC_I2C_SR) & mask) == 0) {
> + if (time_after(jiffies, timeout)) {
> + ret = -ETIMEDOUT;
> + break;
> + }
> + cond_resched();
> + }
> +
> + return ret;
> +}
readb_poll_timeout()
> +
> +/*
> + * Workaround for Erratum A004447. From the P2040CE Rev Q
> + *
> + * 1. Set up the frequency divider and sampling rate.
> + * 2. I2CCR - a0h
> + * 3. Poll for I2CSR[MBB] to get set.
> + * 4. If I2CSR[MAL] is set (an indication that SDA is stuck low), then
> go to
> + * step 5. If MAL is not set, then go to step 13.
> + * 5. I2CCR - 00h
> + * 6. I2CCR - 22h
> + * 7. I2CCR - a2h
> + * 8. Poll for I2CSR[MBB] to get set.
> + * 9. Issue read to I2CDR.
> + * 10. Poll for I2CSR[MIF] to be set.
> + * 11. I2CCR - 82h
> + * 12. Workaround complete. Skip the next steps.
> + * 13. Issue read to I2CDR.
> + * 14. Poll for I2CSR[MIF] to be set.
> + * 15. I2CCR - 80h
> + */
> +static void mpc_i2c_fixup_A004447(struct mpc_i2c *i2c)
> +{
> + int ret;
> + u32 val;
> +
> + writeccr(i2c, CCR_MEN | CCR_MSTA);
> + ret = i2c_mpc_wait_sr(i2c, CSR_MBB);
> + if (ret) {
> + dev_err(i2c->dev, "timeout waiting for CSR_MBB\n");
> + return;
> + }
> +
> + val = readb(i2c->base + MPC_I2C_SR);
> +
> + if (val & CSR_MAL) {
> + writeccr(i2c, 0x00);
> + writeccr(i2c, CCR_MSTA | CCR_RSVD);
> + writeccr(i2c, CCR_MEN | CCR_MSTA | CCR_RSVD);
> + ret = i2c_mpc_wait_sr(i2c, CSR_MBB);
> + if (ret) {
> + dev_err(i2c->dev, "timeout waiting for CSR_MBB\n");
> + return;
> + }
> + val = readb(i2c->base + MPC_I2C_DR);
> + ret = i2c_mpc_wait_sr(i2c, CSR_MIF);
> + if (ret) {
> + dev_err(i2c->dev, "timeout waiting for CSR_MIF\n");
> + return;
> + }
> + writeccr(i2c, CCR_MEN | CCR_RSVD);
> + } else {
> + val = readb(i2c->base + MPC_I2C_DR);
> + ret = i2c_mpc_wait_sr(i2c, CSR_MIF);
> + if (ret) {
> + dev_err(i2c->dev, "timeout waiting for CSR_MIF\n");
> + return;
> + }
> + writeccr(i2c, CCR_MEN);
> + }
> +}
> +
> #if defined(CONFIG_PPC_MPC52xx) || defined(CONFIG_PPC_MPC512x)
> static const struct mpc_i2c_divider mpc_i2c_dividers_52xx[] = {
> {20, 0x20}, {22, 0x21}, {24, 0x22}, {26, 0x23},
> @@ -670,7 +748,10 @@ static int fsl_i2c_bus_recovery(struct i2c_adapter
> *adap)
> {
> struct mpc_i2c *i2c = i2c_get_adapdata(adap);
>
> - mpc_i2c_fixup(i2c);
> + if (i2c->has_errata_A004447)
> + mpc_i2c_fixup_A004447(i2c);
> + else
> + mpc_i2c_fixup(i2c);
>
> return 0;
> }
> @@ -767,6 +848,9 @@ static int fsl_i2c_probe(struct platform_device *op)
> }
> dev_info(i2c->dev, "timeout %u us\n", mpc_ops.timeout * 1000000 /
> HZ);
>
> + if (of_property_read_bool(op->dev.of_node,
> "fsl,i2c-erratum-a004447"))
> + i2c->has_errata_A004447 = true;
> +
> i2c->adap = mpc_ops;
> scnprintf(i2c->adap.name, sizeof(i2c->adap.name),
> "MPC adapter (%s)", of_node_full_name(op->dev.of_node));
> --
> 2.31.1
>
>
--
With Best Regards,
Andy Shevchenko
[-- Attachment #2: Type: text/html, Size: 6322 bytes --]
^ permalink raw reply
* Re: [FSL P50x0] Xorg always restarts again and again after the the PowerPC updates 5.13-1
From: Christian Zigotzky @ 2021-05-06 7:56 UTC (permalink / raw)
To: Christophe Leroy
Cc: Darren Stevens, R.T.Dickinson, mad skateman, linuxppc-dev,
Christian Zigotzky
In-Reply-To: <b5a2b9b7-6f1e-8dcb-8466-053b8ff8241d@csgroup.eu>
Hi Christophe,
Ok, so let's summarise from my side.
The issue is in the PowerPC updates 5.13-1. I reverted these and after that the issue is gone.
We know that only BookE machines are affected. Book3S machines are working with the PowerPC updates.
I think it’s not directly an Xorg issue. It’s more a symptom that Xorg restarts again and again. In my point of view the changes for BookE machines in the PowerPC updates are responsible for this issue.
Bisecting costs a lot of time and I don’t have time for my main work anymore.
Bisecting is good but sometime you have to check your code yourself. We know all facts and now it’s time to check the code because of BookE compatibility.
@All
You can test it with QEMU as well. I provide some virtual machines and kernels for testing. Guys, it is really important that you test your changes before you release them.
Thanks,
Christian
> On 6. May 2021, at 08:13, Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
>
>
>
>> Le 05/05/2021 à 14:43, Christian Zigotzky a écrit :
>>> On 04 May 2021 at 05:17pm, Christophe Leroy wrote:
>>> Le 04/05/2021 à 16:59, Christian Zigotzky a écrit :
>>>> On 04 May 2021 at 04:41pm Christophe Leroy wrote:
>>>>> Le 04/05/2021 à 13:02, Christian Zigotzky a écrit :
>>>>>> On 04 May 2021 at 12:07pm, Christian Zigotzky wrote:
>>>>>>> On 04 May 2021 at 11:49am, Christophe Leroy wrote:
>>>>>>>
>>>>>>> I am bisecting currently.
>>>>>>>
>>>>>>> $ git bisect start -- arch/powerpc
>>>>>>> $ git bisect good 887f3ceb51cd3~
>>>>>>> $ git bisect bad c70a4be130de333ea079c59da41cc959712bb01c
>>>>>> OK, there is another issue after the second bisecting step. The boot stops after loading the dtb and uImage file. I can't solve 2 issues with bisecting at the same time.
>>>>>
>>>>> In that case, you can use 'git bisect skip' to skip the one that is not booting at all.
>>>> In my point of view 'git bisect skip' isn't a good idea because I will not find out if the skipped commit is good or bad and maybe the first bad commit.
>>>
>>> The second problem may be completely unrelated to the first one so it could work.
>>>
>>> In any case, if 'git bisect' finds out that the bad commit is in the middle of a skipped area, it will tell you. So I think it is worth it.
>>>
>>> The second solution could be to first focus on that 'boot stops after loading problem' and try to find out which commit introduces the bug, then which one fixes it. But it may not be necessary.
>>>
>>> Other solution, as you were thinking that the conversion of 'booke' to C interrupt entry/exit, you can also try around that: See if d738ee8 has the problem and 2e2a441 doesn't have the problem.
>>>
>>> If so, you can bisect between those two commits (There are 8 commits inbetween).
>> Hi Christophe,
>> I am bisecting with skipping the boot issue currently. Unfortunately it seems there is another bug. I had to skip two times because the kernel didn't compile.
>
>> Should I continue bisecting?
>> You can find the other steps (21 and higher) here: https://forum.hyperion-entertainment.com/viewtopic.php?p=53103#p53103
>
> Ok, so let's summarise:
>
> 887f3ceb51cd = Xorg doesn't work
> 887f3ceb51cd is the "first bad commit" identified by your first "bisect"
> 887f3ceb51cd~ = 627b72bee84d works ok
> c70a4be130de = Xorg doesn't work
>
> Can you check that 887f3ceb51cd with cherry-picked 525642624783 has Xorg working ?
>
> Can you provide 'git bisect log' ?
>
> It seems there is some mismatch between the commit and the description. For instance, you say fd6db2892eba and 14b3c9d24a7a don't build. I see no reason for that, I agree there is that build failure but with dc6231821a14, 0c2472de23ae, 3db8aa10de9a and 097157e16cf8. That is fixed by ceff77efa4f8. Note that that build failure should not occur if you have CONFIG_CONTEXT_TRACKING, but it is not our problem for the time being.
>
> Anyway, what I learn from your details is:
>
> 56bec2f9d4d0 is the first one you tested which stops after loading the dtb and uImage.
>
> Can you bisect between 887f3ceb51cd[good] and 56bec2f9d4d0[bad] to identify first bad commit that stops after loading the dtb and uImage ?
>
> Once that first bad commit is identified, can you check whether the preceeding commit with cherry-picked 525642624783 has Xorg working or not ?
>
>
> ceff77efa4f8 is the last one you tested which stops after loading the dtb and uImage.
> 49c1d07fd04f is bad (Xorg not working)
>
> Can you bisect between ceff77efa4f8[good] and 49c1d07fd04f[bad] to identify first commit that doesn't stop after loading the dtb and uImage ? (Here you have to make a mental exercice:
> the ones that stop after loading dtb/uImage are the 'good' and the ones booting OK are the 'bad')
>
> Once you have found out what breaks booting and what makes it work again, then we should be able to progress on the investigation.
>
> Thanks
> Christophe
^ permalink raw reply
* Re: [PATCH v7 3/6] KVM: PPC: Book3S HV: Add support for H_RPT_INVALIDATE
From: Bharata B Rao @ 2021-05-06 6:31 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: farosas, aneesh.kumar, kvm-ppc, linuxppc-dev, david
In-Reply-To: <1620279244.mpmwjm8qjk.astroid@bobo.none>
On Thu, May 06, 2021 at 03:45:21PM +1000, Nicholas Piggin wrote:
> Excerpts from Bharata B Rao's message of May 6, 2021 1:46 am:
> >
> > +static long kvmppc_h_rpt_invalidate(struct kvm_vcpu *vcpu,
> > + unsigned long id, unsigned long target,
> > + unsigned long type, unsigned long pg_sizes,
> > + unsigned long start, unsigned long end)
> > +{
> > + unsigned long psize;
> > + struct mmu_psize_def *def;
> > +
> > + if (!kvm_is_radix(vcpu->kvm))
> > + return H_UNSUPPORTED;
> > +
> > + if (end < start)
> > + return H_P5;
> > +
> > + /*
> > + * Partition-scoped invalidation for nested guests.
> > + * Not yet supported
> > + */
> > + if (type & H_RPTI_TYPE_NESTED)
> > + return H_P3;
> > +
> > + /*
> > + * Process-scoped invalidation for L1 guests.
> > + */
> > + for (psize = 0; psize < MMU_PAGE_COUNT; psize++) {
> > + def = &mmu_psize_defs[psize];
> > + if (!(pg_sizes & def->h_rpt_pgsize))
> > + continue;
>
> Not that it really matters but why did you go this approach rather than
> use a bitmask iteration over h_rpt_pgsize?
If you are asking why I am not just looping over the hcall argument
@pg_sizes bitmask then, I was doing that in my earlier version. But
David suggested that it would be good to have page size encodings
of H_RPT_INVALIDATE within mmu_pgsize_defs[]. Based on this, I am
populating mmu_pgsize_defs[] during radix page size initialization
and using that here to check for those page sizes that have been set
in @pg_sizes.
>
> I would actually prefer to put this loop into the TLB invalidation code
> itself.
Yes, I could easily move it there.
>
> The reason is that not all flush types are based on page size. You only
> need to do IS=1/2/3 flushes once and it takes out all page sizes.
I see. So we have to do explicit flushing for different page sizes
only if we are doing range based invalidation (IS=0). For rest of
the cases (IS=1/2/3), that's not necessary.
>
> You don't need to do all these optimisations right now, but it would
> be good to make them possible to implement.
Sure.
> > +void do_h_rpt_invalidate_prt(unsigned long pid, unsigned long lpid,
> > + unsigned long type, unsigned long page_size,
> > + unsigned long psize, unsigned long start,
> > + unsigned long end)
> > +{
> > + /*
> > + * A H_RPTI_TYPE_ALL request implies RIC=3, hence
> > + * do a single IS=1 based flush.
> > + */
> > + if ((type & H_RPTI_TYPE_ALL) == H_RPTI_TYPE_ALL) {
> > + _tlbie_pid_lpid(pid, lpid, RIC_FLUSH_ALL);
> > + return;
> > + }
> > +
> > + if (type & H_RPTI_TYPE_PWC)
> > + _tlbie_pid_lpid(pid, lpid, RIC_FLUSH_PWC);
> > +
> > + if (start == 0 && end == -1) /* PID */
> > + _tlbie_pid_lpid(pid, lpid, RIC_FLUSH_TLB);
> > + else /* EA */
> > + _tlbie_va_range_lpid(start, end, pid, lpid, page_size,
> > + psize, false);
>
> At least one thing that is probably needed is to use the
> single_page_flush_ceiling to flip the va range flush over to a pid
> flush, so the guest can't cause problems in the hypervisor with an
> enormous range.
Yes, makes sense. I shall do this and the above as later optimizations.
Regards,
Bharata.
^ permalink raw reply
* Re: [PATCH v7 4/6] KVM: PPC: Book3S HV: Nested support in H_RPT_INVALIDATE
From: Nicholas Piggin @ 2021-05-06 6:15 UTC (permalink / raw)
To: Bharata B Rao, kvm-ppc, linuxppc-dev; +Cc: aneesh.kumar, farosas, david
In-Reply-To: <20210505154642.178702-5-bharata@linux.ibm.com>
Excerpts from Bharata B Rao's message of May 6, 2021 1:46 am:
> Enable support for process-scoped invalidations from nested
> guests and partition-scoped invalidations for nested guests.
>
> Process-scoped invalidations for any level of nested guests
> are handled by implementing H_RPT_INVALIDATE handler in the
> nested guest exit path in L0.
>
> Partition-scoped invalidation requests are forwarded to the
> right nested guest, handled there and passed down to L0
> for eventual handling.
>
> Signed-off-by: Bharata B Rao <bharata@linux.ibm.com>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> [Nested guest partition-scoped invalidation changes]
> ---
> .../include/asm/book3s/64/tlbflush-radix.h | 4 +
> arch/powerpc/include/asm/kvm_book3s.h | 3 +
> arch/powerpc/kvm/book3s_hv.c | 66 ++++++++++-
> arch/powerpc/kvm/book3s_hv_nested.c | 104 ++++++++++++++++++
> arch/powerpc/mm/book3s64/radix_tlb.c | 4 -
> 5 files changed, 174 insertions(+), 7 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/book3s/64/tlbflush-radix.h b/arch/powerpc/include/asm/book3s/64/tlbflush-radix.h
> index 8b33601cdb9d..a46fd37ad552 100644
> --- a/arch/powerpc/include/asm/book3s/64/tlbflush-radix.h
> +++ b/arch/powerpc/include/asm/book3s/64/tlbflush-radix.h
> @@ -4,6 +4,10 @@
>
> #include <asm/hvcall.h>
>
> +#define RIC_FLUSH_TLB 0
> +#define RIC_FLUSH_PWC 1
> +#define RIC_FLUSH_ALL 2
> +
> struct vm_area_struct;
> struct mm_struct;
> struct mmu_gather;
> diff --git a/arch/powerpc/include/asm/kvm_book3s.h b/arch/powerpc/include/asm/kvm_book3s.h
> index a6e9a5585e61..fdf54741c58c 100644
> --- a/arch/powerpc/include/asm/kvm_book3s.h
> +++ b/arch/powerpc/include/asm/kvm_book3s.h
> @@ -307,6 +307,9 @@ void kvmhv_set_ptbl_entry(unsigned int lpid, u64 dw0, u64 dw1);
> void kvmhv_release_all_nested(struct kvm *kvm);
> long kvmhv_enter_nested_guest(struct kvm_vcpu *vcpu);
> long kvmhv_do_nested_tlbie(struct kvm_vcpu *vcpu);
> +long do_h_rpt_invalidate_pat(struct kvm_vcpu *vcpu, unsigned long lpid,
> + unsigned long type, unsigned long pg_sizes,
> + unsigned long start, unsigned long end);
> int kvmhv_run_single_vcpu(struct kvm_vcpu *vcpu,
> u64 time_limit, unsigned long lpcr);
> void kvmhv_save_hv_regs(struct kvm_vcpu *vcpu, struct hv_guest_state *hr);
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index bcf34246bbe9..a2e7fbec796a 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -925,6 +925,41 @@ static int kvmppc_get_yield_count(struct kvm_vcpu *vcpu)
> return yield_count;
> }
>
> +/*
> + * H_RPT_INVALIDATE hcall handler for nested guests.
> + *
> + * Handles only nested process-scoped invalidation requests in L0.
> + */
> +static int kvmppc_nested_h_rpt_invalidate(struct kvm_vcpu *vcpu)
> +{
> + unsigned long type = kvmppc_get_gpr(vcpu, 6);
> + unsigned long pid, pg_sizes, start, end, psize;
> + struct mmu_psize_def *def;
> +
> + /*
> + * The partition-scoped invalidations aren't handled here in L0.
> + */
> + if (type & H_RPTI_TYPE_NESTED)
> + return RESUME_HOST;
> +
> + pid = kvmppc_get_gpr(vcpu, 4);
> + pg_sizes = kvmppc_get_gpr(vcpu, 7);
> + start = kvmppc_get_gpr(vcpu, 8);
> + end = kvmppc_get_gpr(vcpu, 9);
> +
> + for (psize = 0; psize < MMU_PAGE_COUNT; psize++) {
> + def = &mmu_psize_defs[psize];
> + if (pg_sizes & def->h_rpt_pgsize)
> + do_h_rpt_invalidate_prt(pid,
> + vcpu->arch.nested->shadow_lpid,
> + type, (1UL << def->shift),
> + psize, start, end);
> + }
> +
> + kvmppc_set_gpr(vcpu, 3, H_SUCCESS);
> + return RESUME_GUEST;
> +}
> +
> static long kvmppc_h_rpt_invalidate(struct kvm_vcpu *vcpu,
> unsigned long id, unsigned long target,
> unsigned long type, unsigned long pg_sizes,
> @@ -941,10 +976,18 @@ static long kvmppc_h_rpt_invalidate(struct kvm_vcpu *vcpu,
>
> /*
> * Partition-scoped invalidation for nested guests.
> - * Not yet supported
> */
> - if (type & H_RPTI_TYPE_NESTED)
> - return H_P3;
> + if (type & H_RPTI_TYPE_NESTED) {
> + if (!nesting_enabled(vcpu->kvm))
> + return H_FUNCTION;
> +
> + /* Support only cores as target */
> + if (target != H_RPTI_TARGET_CMMU)
> + return H_P2;
> +
> + return do_h_rpt_invalidate_pat(vcpu, id, type, pg_sizes,
> + start, end);
> + }
>
> /*
> * Process-scoped invalidation for L1 guests.
> @@ -1639,6 +1682,23 @@ static int kvmppc_handle_nested_exit(struct kvm_vcpu *vcpu)
> if (!xics_on_xive())
> kvmppc_xics_rm_complete(vcpu, 0);
> break;
> + case BOOK3S_INTERRUPT_SYSCALL:
> + {
> + unsigned long req = kvmppc_get_gpr(vcpu, 3);
> +
> + /*
> + * The H_RPT_INVALIDATE hcalls issued by nested
> + * guests for process-scoped invalidations when
> + * GTSE=0, are handled here in L0.
> + */
> + if (req == H_RPT_INVALIDATE) {
> + r = kvmppc_nested_h_rpt_invalidate(vcpu);
> + break;
> + }
> +
> + r = RESUME_HOST;
> + break;
> + }
> default:
> r = RESUME_HOST;
> break;
> diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c
> index 60724f674421..91f10290130d 100644
> --- a/arch/powerpc/kvm/book3s_hv_nested.c
> +++ b/arch/powerpc/kvm/book3s_hv_nested.c
> @@ -1214,6 +1214,110 @@ long kvmhv_do_nested_tlbie(struct kvm_vcpu *vcpu)
> return H_SUCCESS;
> }
>
> +static long do_tlb_invalidate_nested_tlb(struct kvm_vcpu *vcpu,
> + unsigned long lpid,
> + unsigned long page_size,
> + unsigned long ap,
> + unsigned long start,
> + unsigned long end)
> +{
> + unsigned long addr = start;
> + int ret;
> +
> + do {
> + ret = kvmhv_emulate_tlbie_tlb_addr(vcpu, lpid, ap,
> + get_epn(addr));
> + if (ret)
> + return ret;
> + addr += page_size;
> + } while (addr < end);
> +
> + return ret;
> +}
Similar comments for single page thresholds, and multiple page sizes.
> +
> +static long do_tlb_invalidate_nested_all(struct kvm_vcpu *vcpu,
> + unsigned long lpid, unsigned long ric)
> +{
> + struct kvm *kvm = vcpu->kvm;
> + struct kvm_nested_guest *gp;
> +
> + gp = kvmhv_get_nested(kvm, lpid, false);
> + if (gp) {
> + kvmhv_emulate_tlbie_lpid(vcpu, gp, ric);
> + kvmhv_put_nested(gp);
> + }
> + return H_SUCCESS;
> +}
> +
> +/*
> + * Performs partition-scoped invalidations for nested guests
> + * as part of H_RPT_INVALIDATE hcall.
> + */
> +long do_h_rpt_invalidate_pat(struct kvm_vcpu *vcpu, unsigned long lpid,
> + unsigned long type, unsigned long pg_sizes,
> + unsigned long start, unsigned long end)
> +{
> + struct kvm_nested_guest *gp;
> + long ret;
> + unsigned long psize, ap;
> +
> + /*
> + * If L2 lpid isn't valid, we need to return H_PARAMETER.
> + *
> + * However, nested KVM issues a L2 lpid flush call when creating
> + * partition table entries for L2. This happens even before the
> + * corresponding shadow lpid is created in HV which happens in
> + * H_ENTER_NESTED call. Since we can't differentiate this case from
> + * the invalid case, we ignore such flush requests and return success.
> + */
> + gp = kvmhv_find_nested(vcpu->kvm, lpid);
> + if (!gp)
> + return H_SUCCESS;
> +
> + /*
> + * A flush all request can be handled by a full lpid flush only.
> + */
> + if ((type & H_RPTI_TYPE_NESTED_ALL) == H_RPTI_TYPE_NESTED_ALL)
> + return do_tlb_invalidate_nested_all(vcpu, lpid, RIC_FLUSH_ALL);
> +
> +#if 0
> + /*
> + * We don't need to handle a PWC flush like process table here,
> + * because intermediate partition scoped table in nested guest doesn't
> + * really have PWC. Only level we have PWC is in L0 and for nested
> + * invalidate at L0 we always do kvm_flush_lpid() which does
> + * radix__flush_all_lpid(). For range invalidate at any level, we
> + * are not removing the higher level page tables and hence there is
> + * no PWC invalidate needed.
> + */
> + if (type & H_RPTI_TYPE_PWC) {
> + ret = do_tlb_invalidate_nested_all(vcpu, lpid, RIC_FLUSH_PWC);
> + if (ret)
> + return H_P4;
> + }
> +#endif
I think removing this #if 0 and the unnecessary code is fine, just a bit
more explanation in the comment would help. And "doesn't really" implies
it sort of might a little bit, I think what you want is "really doesn't"
:)
As I understand it, the L0 does not cache any intermediate levels of the
nested guest's partition scope at all. Only the nested HV's pte entries
are copied into the shadow page table, so we only care if the PTEs are
changed, and the PWCs that the processor creates for the shadow page
table are managed by the kvmppc_unmap_pte() etc functions... I think?
Thanks,
Nick
> +
> + if (start == 0 && end == -1)
> + return do_tlb_invalidate_nested_all(vcpu, lpid, RIC_FLUSH_TLB);
> +
> + if (type & H_RPTI_TYPE_TLB) {
> + struct mmu_psize_def *def;
> +
> + for (psize = 0; psize < MMU_PAGE_COUNT; psize++) {
> + def = &mmu_psize_defs[psize];
> + if (!(pg_sizes & def->h_rpt_pgsize))
> + continue;
> +
> + ret = do_tlb_invalidate_nested_tlb(vcpu, lpid,
> + (1UL << def->shift),
> + ap, start, end);
> + if (ret)
> + return H_P4;
> + }
> + }
> + return H_SUCCESS;
> +}
> +
> /* Used to convert a nested guest real address to a L1 guest real address */
> static int kvmhv_translate_addr_nested(struct kvm_vcpu *vcpu,
> struct kvm_nested_guest *gp,
> diff --git a/arch/powerpc/mm/book3s64/radix_tlb.c b/arch/powerpc/mm/book3s64/radix_tlb.c
> index 65aad9ce3557..613198c41006 100644
> --- a/arch/powerpc/mm/book3s64/radix_tlb.c
> +++ b/arch/powerpc/mm/book3s64/radix_tlb.c
> @@ -20,10 +20,6 @@
>
> #include "internal.h"
>
> -#define RIC_FLUSH_TLB 0
> -#define RIC_FLUSH_PWC 1
> -#define RIC_FLUSH_ALL 2
> -
> /*
> * tlbiel instruction for radix, set invalidation
> * i.e., r=1 and is=01 or is=10 or is=11
> --
> 2.26.2
>
>
^ permalink raw reply
* Re: [FSL P50x0] Xorg always restarts again and again after the the PowerPC updates 5.13-1
From: Christophe Leroy @ 2021-05-06 6:12 UTC (permalink / raw)
To: Christian Zigotzky, Michael Ellerman
Cc: Darren Stevens, linuxppc-dev, mad skateman, R.T.Dickinson,
Christian Zigotzky
In-Reply-To: <9f150fb5-3034-f256-04f9-9c802e40f855@xenosoft.de>
Le 05/05/2021 à 14:43, Christian Zigotzky a écrit :
> On 04 May 2021 at 05:17pm, Christophe Leroy wrote:
>> Le 04/05/2021 à 16:59, Christian Zigotzky a écrit :
>>> On 04 May 2021 at 04:41pm Christophe Leroy wrote:
>>>> Le 04/05/2021 à 13:02, Christian Zigotzky a écrit :
>>>>> On 04 May 2021 at 12:07pm, Christian Zigotzky wrote:
>>>>>> On 04 May 2021 at 11:49am, Christophe Leroy wrote:
>>>>>>
>>>>>> I am bisecting currently.
>>>>>>
>>>>>> $ git bisect start -- arch/powerpc
>>>>>> $ git bisect good 887f3ceb51cd3~
>>>>>> $ git bisect bad c70a4be130de333ea079c59da41cc959712bb01c
>>>>> OK, there is another issue after the second bisecting step. The boot stops after loading the
>>>>> dtb and uImage file. I can't solve 2 issues with bisecting at the same time.
>>>>
>>>> In that case, you can use 'git bisect skip' to skip the one that is not booting at all.
>>> In my point of view 'git bisect skip' isn't a good idea because I will not find out if the
>>> skipped commit is good or bad and maybe the first bad commit.
>>
>> The second problem may be completely unrelated to the first one so it could work.
>>
>> In any case, if 'git bisect' finds out that the bad commit is in the middle of a skipped area, it
>> will tell you. So I think it is worth it.
>>
>> The second solution could be to first focus on that 'boot stops after loading problem' and try to
>> find out which commit introduces the bug, then which one fixes it. But it may not be necessary.
>>
>> Other solution, as you were thinking that the conversion of 'booke' to C interrupt entry/exit, you
>> can also try around that: See if d738ee8 has the problem and 2e2a441 doesn't have the problem.
>>
>> If so, you can bisect between those two commits (There are 8 commits inbetween).
> Hi Christophe,
>
> I am bisecting with skipping the boot issue currently. Unfortunately it seems there is another bug.
> I had to skip two times because the kernel didn't compile.
>
>
> Should I continue bisecting?
>
> You can find the other steps (21 and higher) here:
> https://forum.hyperion-entertainment.com/viewtopic.php?p=53103#p53103
>
Ok, so let's summarise:
887f3ceb51cd = Xorg doesn't work
887f3ceb51cd is the "first bad commit" identified by your first "bisect"
887f3ceb51cd~ = 627b72bee84d works ok
c70a4be130de = Xorg doesn't work
Can you check that 887f3ceb51cd with cherry-picked 525642624783 has Xorg working ?
Can you provide 'git bisect log' ?
It seems there is some mismatch between the commit and the description. For instance, you say
fd6db2892eba and 14b3c9d24a7a don't build. I see no reason for that, I agree there is that build
failure but with dc6231821a14, 0c2472de23ae, 3db8aa10de9a and 097157e16cf8. That is fixed by
ceff77efa4f8. Note that that build failure should not occur if you have CONFIG_CONTEXT_TRACKING, but
it is not our problem for the time being.
Anyway, what I learn from your details is:
56bec2f9d4d0 is the first one you tested which stops after loading the dtb and uImage.
Can you bisect between 887f3ceb51cd[good] and 56bec2f9d4d0[bad] to identify first bad commit that
stops after loading the dtb and uImage ?
Once that first bad commit is identified, can you check whether the preceeding commit with
cherry-picked 525642624783 has Xorg working or not ?
ceff77efa4f8 is the last one you tested which stops after loading the dtb and uImage.
49c1d07fd04f is bad (Xorg not working)
Can you bisect between ceff77efa4f8[good] and 49c1d07fd04f[bad] to identify first commit that
doesn't stop after loading the dtb and uImage ? (Here you have to make a mental exercice:
the ones that stop after loading dtb/uImage are the 'good' and the ones booting OK are the 'bad')
Once you have found out what breaks booting and what makes it work again, then we should be able to
progress on the investigation.
Thanks
Christophe
^ permalink raw reply
* Re: [PATCH v7 3/6] KVM: PPC: Book3S HV: Add support for H_RPT_INVALIDATE
From: Nicholas Piggin @ 2021-05-06 5:45 UTC (permalink / raw)
To: Bharata B Rao, kvm-ppc, linuxppc-dev; +Cc: aneesh.kumar, farosas, david
In-Reply-To: <20210505154642.178702-4-bharata@linux.ibm.com>
Excerpts from Bharata B Rao's message of May 6, 2021 1:46 am:
> H_RPT_INVALIDATE does two types of TLB invalidations:
>
> 1. Process-scoped invalidations for guests when LPCR[GTSE]=0.
> This is currently not used in KVM as GTSE is not usually
> disabled in KVM.
> 2. Partition-scoped invalidations that an L1 hypervisor does on
> behalf of an L2 guest. This is currently handled
> by H_TLB_INVALIDATE hcall and this new replaces the old that.
>
> This commit enables process-scoped invalidations for L1 guests.
> Support for process-scoped and partition-scoped invalidations
> from/for nested guests will be added separately.
>
> Process scoped tlbie invalidations from L1 and nested guests
> need RS register for TLBIE instruction to contain both PID and
> LPID. This patch introduces primitives that execute tlbie
> instruction with both PID and LPID set in prepartion for
> H_RPT_INVALIDATE hcall.
>
> A description of H_RPT_INVALIDATE follows:
>
> int64 /* H_Success: Return code on successful completion */
> /* H_Busy - repeat the call with the same */
> /* H_Parameter, H_P2, H_P3, H_P4, H_P5 : Invalid
> parameters */
> hcall(const uint64 H_RPT_INVALIDATE, /* Invalidate RPT
> translation
> lookaside information */
> uint64 id, /* PID/LPID to invalidate */
> uint64 target, /* Invalidation target */
> uint64 type, /* Type of lookaside information */
> uint64 pg_sizes, /* Page sizes */
> uint64 start, /* Start of Effective Address (EA)
> range (inclusive) */
> uint64 end) /* End of EA range (exclusive) */
>
> Invalidation targets (target)
> -----------------------------
> Core MMU 0x01 /* All virtual processors in the
> partition */
> Core local MMU 0x02 /* Current virtual processor */
> Nest MMU 0x04 /* All nest/accelerator agents
> in use by the partition */
>
> A combination of the above can be specified,
> except core and core local.
>
> Type of translation to invalidate (type)
> ---------------------------------------
> NESTED 0x0001 /* invalidate nested guest partition-scope */
> TLB 0x0002 /* Invalidate TLB */
> PWC 0x0004 /* Invalidate Page Walk Cache */
> PRT 0x0008 /* Invalidate caching of Process Table
> Entries if NESTED is clear */
> PAT 0x0008 /* Invalidate caching of Partition Table
> Entries if NESTED is set */
>
> A combination of the above can be specified.
>
> Page size mask (pages)
> ----------------------
> 4K 0x01
> 64K 0x02
> 2M 0x04
> 1G 0x08
> All sizes (-1UL)
>
> A combination of the above can be specified.
> All page sizes can be selected with -1.
>
> Semantics: Invalidate radix tree lookaside information
> matching the parameters given.
> * Return H_P2, H_P3 or H_P4 if target, type, or pageSizes parameters
> are different from the defined values.
> * Return H_PARAMETER if NESTED is set and pid is not a valid nested
> LPID allocated to this partition
> * Return H_P5 if (start, end) doesn't form a valid range. Start and
> end should be a valid Quadrant address and end > start.
> * Return H_NotSupported if the partition is not in running in radix
> translation mode.
> * May invalidate more translation information than requested.
> * If start = 0 and end = -1, set the range to cover all valid
> addresses. Else start and end should be aligned to 4kB (lower 11
> bits clear).
> * If NESTED is clear, then invalidate process scoped lookaside
> information. Else pid specifies a nested LPID, and the invalidation
> is performed on nested guest partition table and nested guest
> partition scope real addresses.
> * If pid = 0 and NESTED is clear, then valid addresses are quadrant 3
> and quadrant 0 spaces, Else valid addresses are quadrant 0.
> * Pages which are fully covered by the range are to be invalidated.
> Those which are partially covered are considered outside
> invalidation range, which allows a caller to optimally invalidate
> ranges that may contain mixed page sizes.
> * Return H_SUCCESS on success.
>
> Signed-off-by: Bharata B Rao <bharata@linux.ibm.com>
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> arch/powerpc/include/asm/mmu_context.h | 11 ++
> arch/powerpc/kvm/book3s_hv.c | 46 ++++++++
> arch/powerpc/mm/book3s64/radix_tlb.c | 148 +++++++++++++++++++++++++
> 3 files changed, 205 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
> index 4bc45d3ed8b0..128760eb598e 100644
> --- a/arch/powerpc/include/asm/mmu_context.h
> +++ b/arch/powerpc/include/asm/mmu_context.h
> @@ -124,8 +124,19 @@ static inline bool need_extra_context(struct mm_struct *mm, unsigned long ea)
>
> #if defined(CONFIG_KVM_BOOK3S_HV_POSSIBLE) && defined(CONFIG_PPC_RADIX_MMU)
> extern void radix_kvm_prefetch_workaround(struct mm_struct *mm);
> +void do_h_rpt_invalidate_prt(unsigned long pid, unsigned long lpid,
> + unsigned long type, unsigned long page_size,
> + unsigned long psize, unsigned long start,
> + unsigned long end);
> #else
> static inline void radix_kvm_prefetch_workaround(struct mm_struct *mm) { }
> +static inline void do_h_rpt_invalidate_prt(unsigned long pid,
> + unsigned long lpid,
> + unsigned long type,
> + unsigned long page_size,
> + unsigned long psize,
> + unsigned long start,
> + unsigned long end) { }
> #endif
>
> extern void switch_cop(struct mm_struct *next);
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 28a80d240b76..bcf34246bbe9 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -76,6 +76,7 @@
> #include <asm/kvm_book3s_uvmem.h>
> #include <asm/ultravisor.h>
> #include <asm/dtl.h>
> +#include <asm/plpar_wrappers.h>
>
> #include "book3s.h"
>
> @@ -924,6 +925,42 @@ static int kvmppc_get_yield_count(struct kvm_vcpu *vcpu)
> return yield_count;
> }
>
> +static long kvmppc_h_rpt_invalidate(struct kvm_vcpu *vcpu,
> + unsigned long id, unsigned long target,
> + unsigned long type, unsigned long pg_sizes,
> + unsigned long start, unsigned long end)
> +{
> + unsigned long psize;
> + struct mmu_psize_def *def;
> +
> + if (!kvm_is_radix(vcpu->kvm))
> + return H_UNSUPPORTED;
> +
> + if (end < start)
> + return H_P5;
> +
> + /*
> + * Partition-scoped invalidation for nested guests.
> + * Not yet supported
> + */
> + if (type & H_RPTI_TYPE_NESTED)
> + return H_P3;
> +
> + /*
> + * Process-scoped invalidation for L1 guests.
> + */
> + for (psize = 0; psize < MMU_PAGE_COUNT; psize++) {
> + def = &mmu_psize_defs[psize];
> + if (!(pg_sizes & def->h_rpt_pgsize))
> + continue;
Not that it really matters but why did you go this approach rather than
use a bitmask iteration over h_rpt_pgsize?
I would actually prefer to put this loop into the TLB invalidation code
itself.
The reason is that not all flush types are based on page size. You only
need to do IS=1/2/3 flushes once and it takes out all page sizes.
You don't need to do all these optimisations right now, but it would
be good to make them possible to implement.
> +
> + do_h_rpt_invalidate_prt(id, vcpu->kvm->arch.lpid,
> + type, (1UL << def->shift),
> + psize, start, end);
> + }
> + return H_SUCCESS;
> +}
> +
> int kvmppc_pseries_do_hcall(struct kvm_vcpu *vcpu)
> {
> unsigned long req = kvmppc_get_gpr(vcpu, 3);
> @@ -1132,6 +1169,14 @@ int kvmppc_pseries_do_hcall(struct kvm_vcpu *vcpu)
> */
> ret = kvmppc_h_svm_init_abort(vcpu->kvm);
> break;
> + case H_RPT_INVALIDATE:
> + ret = kvmppc_h_rpt_invalidate(vcpu, kvmppc_get_gpr(vcpu, 4),
> + kvmppc_get_gpr(vcpu, 5),
> + kvmppc_get_gpr(vcpu, 6),
> + kvmppc_get_gpr(vcpu, 7),
> + kvmppc_get_gpr(vcpu, 8),
> + kvmppc_get_gpr(vcpu, 9));
> + break;
>
> default:
> return RESUME_HOST;
> @@ -1178,6 +1223,7 @@ static int kvmppc_hcall_impl_hv(unsigned long cmd)
> case H_XIRR_X:
> #endif
> case H_PAGE_INIT:
> + case H_RPT_INVALIDATE:
> return 1;
> }
>
> diff --git a/arch/powerpc/mm/book3s64/radix_tlb.c b/arch/powerpc/mm/book3s64/radix_tlb.c
> index 409e61210789..65aad9ce3557 100644
> --- a/arch/powerpc/mm/book3s64/radix_tlb.c
> +++ b/arch/powerpc/mm/book3s64/radix_tlb.c
> @@ -130,6 +130,21 @@ static __always_inline void __tlbie_pid(unsigned long pid, unsigned long ric)
> trace_tlbie(0, 0, rb, rs, ric, prs, r);
> }
>
> +static __always_inline void __tlbie_pid_lpid(unsigned long pid,
> + unsigned long lpid,
> + unsigned long ric)
> +{
> + unsigned long rb, rs, prs, r;
> +
> + rb = PPC_BIT(53); /* IS = 1 */
> + rs = (pid << PPC_BITLSHIFT(31)) | (lpid & ~(PPC_BITMASK(0, 31)));
> + prs = 1; /* process scoped */
> + r = 1; /* radix format */
> +
> + asm volatile(PPC_TLBIE_5(%0, %4, %3, %2, %1)
> + : : "r"(rb), "i"(r), "i"(prs), "i"(ric), "r"(rs) : "memory");
> + trace_tlbie(0, 0, rb, rs, ric, prs, r);
> +}
> static __always_inline void __tlbie_lpid(unsigned long lpid, unsigned long ric)
> {
> unsigned long rb,rs,prs,r;
> @@ -190,6 +205,23 @@ static __always_inline void __tlbie_va(unsigned long va, unsigned long pid,
> trace_tlbie(0, 0, rb, rs, ric, prs, r);
> }
>
> +static __always_inline void __tlbie_va_lpid(unsigned long va, unsigned long pid,
> + unsigned long lpid,
> + unsigned long ap, unsigned long ric)
> +{
> + unsigned long rb, rs, prs, r;
> +
> + rb = va & ~(PPC_BITMASK(52, 63));
> + rb |= ap << PPC_BITLSHIFT(58);
> + rs = (pid << PPC_BITLSHIFT(31)) | (lpid & ~(PPC_BITMASK(0, 31)));
> + prs = 1; /* process scoped */
> + r = 1; /* radix format */
> +
> + asm volatile(PPC_TLBIE_5(%0, %4, %3, %2, %1)
> + : : "r"(rb), "i"(r), "i"(prs), "i"(ric), "r"(rs) : "memory");
> + trace_tlbie(0, 0, rb, rs, ric, prs, r);
> +}
> +
> static __always_inline void __tlbie_lpid_va(unsigned long va, unsigned long lpid,
> unsigned long ap, unsigned long ric)
> {
> @@ -235,6 +267,22 @@ static inline void fixup_tlbie_va_range(unsigned long va, unsigned long pid,
> }
> }
>
> +static inline void fixup_tlbie_va_range_lpid(unsigned long va,
> + unsigned long pid,
> + unsigned long lpid,
> + unsigned long ap)
> +{
> + if (cpu_has_feature(CPU_FTR_P9_TLBIE_ERAT_BUG)) {
> + asm volatile("ptesync" : : : "memory");
> + __tlbie_pid_lpid(0, lpid, RIC_FLUSH_TLB);
> + }
> +
> + if (cpu_has_feature(CPU_FTR_P9_TLBIE_STQ_BUG)) {
> + asm volatile("ptesync" : : : "memory");
> + __tlbie_va_lpid(va, pid, lpid, ap, RIC_FLUSH_TLB);
> + }
> +}
> +
> static inline void fixup_tlbie_pid(unsigned long pid)
> {
> /*
> @@ -254,6 +302,25 @@ static inline void fixup_tlbie_pid(unsigned long pid)
> }
> }
>
> +static inline void fixup_tlbie_pid_lpid(unsigned long pid, unsigned long lpid)
> +{
> + /*
> + * We can use any address for the invalidation, pick one which is
> + * probably unused as an optimisation.
> + */
> + unsigned long va = ((1UL << 52) - 1);
> +
> + if (cpu_has_feature(CPU_FTR_P9_TLBIE_ERAT_BUG)) {
> + asm volatile("ptesync" : : : "memory");
> + __tlbie_pid_lpid(0, lpid, RIC_FLUSH_TLB);
> + }
> +
> + if (cpu_has_feature(CPU_FTR_P9_TLBIE_STQ_BUG)) {
> + asm volatile("ptesync" : : : "memory");
> + __tlbie_va_lpid(va, pid, lpid, mmu_get_ap(MMU_PAGE_64K),
> + RIC_FLUSH_TLB);
> + }
> +}
>
> static inline void fixup_tlbie_lpid_va(unsigned long va, unsigned long lpid,
> unsigned long ap)
> @@ -344,6 +411,31 @@ static inline void _tlbie_pid(unsigned long pid, unsigned long ric)
> asm volatile("eieio; tlbsync; ptesync": : :"memory");
> }
>
> +static inline void _tlbie_pid_lpid(unsigned long pid, unsigned long lpid,
> + unsigned long ric)
> +{
> + asm volatile("ptesync" : : : "memory");
> +
> + /*
> + * Workaround the fact that the "ric" argument to __tlbie_pid
> + * must be a compile-time contraint to match the "i" constraint
> + * in the asm statement.
> + */
> + switch (ric) {
> + case RIC_FLUSH_TLB:
> + __tlbie_pid_lpid(pid, lpid, RIC_FLUSH_TLB);
> + fixup_tlbie_pid_lpid(pid, lpid);
> + break;
> + case RIC_FLUSH_PWC:
> + __tlbie_pid_lpid(pid, lpid, RIC_FLUSH_PWC);
> + break;
> + case RIC_FLUSH_ALL:
> + default:
> + __tlbie_pid_lpid(pid, lpid, RIC_FLUSH_ALL);
> + fixup_tlbie_pid_lpid(pid, lpid);
> + }
> + asm volatile("eieio; tlbsync; ptesync" : : : "memory");
> +}
> struct tlbiel_pid {
> unsigned long pid;
> unsigned long ric;
> @@ -469,6 +561,20 @@ static inline void __tlbie_va_range(unsigned long start, unsigned long end,
> fixup_tlbie_va_range(addr - page_size, pid, ap);
> }
>
> +static inline void __tlbie_va_range_lpid(unsigned long start, unsigned long end,
> + unsigned long pid, unsigned long lpid,
> + unsigned long page_size,
> + unsigned long psize)
> +{
> + unsigned long addr;
> + unsigned long ap = mmu_get_ap(psize);
> +
> + for (addr = start; addr < end; addr += page_size)
> + __tlbie_va_lpid(addr, pid, lpid, ap, RIC_FLUSH_TLB);
> +
> + fixup_tlbie_va_range_lpid(addr - page_size, pid, lpid, ap);
> +}
> +
> static __always_inline void _tlbie_va(unsigned long va, unsigned long pid,
> unsigned long psize, unsigned long ric)
> {
> @@ -549,6 +655,18 @@ static inline void _tlbie_va_range(unsigned long start, unsigned long end,
> asm volatile("eieio; tlbsync; ptesync": : :"memory");
> }
>
> +static inline void _tlbie_va_range_lpid(unsigned long start, unsigned long end,
> + unsigned long pid, unsigned long lpid,
> + unsigned long page_size,
> + unsigned long psize, bool also_pwc)
> +{
> + asm volatile("ptesync" : : : "memory");
> + if (also_pwc)
> + __tlbie_pid_lpid(pid, lpid, RIC_FLUSH_PWC);
> + __tlbie_va_range_lpid(start, end, pid, lpid, page_size, psize);
> + asm volatile("eieio; tlbsync; ptesync" : : : "memory");
> +}
> +
> static inline void _tlbiel_va_range_multicast(struct mm_struct *mm,
> unsigned long start, unsigned long end,
> unsigned long pid, unsigned long page_size,
> @@ -1381,4 +1499,34 @@ extern void radix_kvm_prefetch_workaround(struct mm_struct *mm)
> }
> }
> EXPORT_SYMBOL_GPL(radix_kvm_prefetch_workaround);
> +
> +/*
> + * Performs process-scoped invalidations for a given LPID
> + * as part of H_RPT_INVALIDATE hcall.
> + */
> +void do_h_rpt_invalidate_prt(unsigned long pid, unsigned long lpid,
> + unsigned long type, unsigned long page_size,
> + unsigned long psize, unsigned long start,
> + unsigned long end)
> +{
> + /*
> + * A H_RPTI_TYPE_ALL request implies RIC=3, hence
> + * do a single IS=1 based flush.
> + */
> + if ((type & H_RPTI_TYPE_ALL) == H_RPTI_TYPE_ALL) {
> + _tlbie_pid_lpid(pid, lpid, RIC_FLUSH_ALL);
> + return;
> + }
> +
> + if (type & H_RPTI_TYPE_PWC)
> + _tlbie_pid_lpid(pid, lpid, RIC_FLUSH_PWC);
> +
> + if (start == 0 && end == -1) /* PID */
> + _tlbie_pid_lpid(pid, lpid, RIC_FLUSH_TLB);
> + else /* EA */
> + _tlbie_va_range_lpid(start, end, pid, lpid, page_size,
> + psize, false);
At least one thing that is probably needed is to use the
single_page_flush_ceiling to flip the va range flush over to a pid
flush, so the guest can't cause problems in the hypervisor with an
enormous range.
Thanks,
Nick
> +}
> +EXPORT_SYMBOL_GPL(do_h_rpt_invalidate_prt);
> +
> #endif /* CONFIG_KVM_BOOK3S_HV_POSSIBLE */
> --
> 2.26.2
>
>
^ permalink raw reply
* Re: [PATCH] powerpc/papr_scm: Make 'perf_stats' invisible if perf-stats unavailable
From: Aneesh Kumar K.V @ 2021-05-06 5:02 UTC (permalink / raw)
To: Vaibhav Jain, linuxppc-dev, linux-nvdimm; +Cc: Vaibhav Jain
In-Reply-To: <20210505191708.51939-1-vaibhav@linux.ibm.com>
Vaibhav Jain <vaibhav@linux.ibm.com> writes:
> In case performance stats for an nvdimm are not available, reading the
> 'perf_stats' sysfs file returns an -ENOENT error. A better approach is
> to make the 'perf_stats' file entirely invisible to indicate that
> performance stats for an nvdimm are unavailable.
>
> So this patch updates 'papr_nd_attribute_group' to add a 'is_visible'
> callback implemented as newly introduced 'papr_nd_attribute_visible()'
> that returns an appropriate mode in case performance stats aren't
> supported in a given nvdimm.
>
> Also the initialization of 'papr_scm_priv.stat_buffer_len' is moved
> from papr_scm_nvdimm_init() to papr_scm_probe() so that it value is
> available when 'papr_nd_attribute_visible()' is called during nvdimm
> initialization.
>
> Fixes: 2d02bf835e57('powerpc/papr_scm: Fetch nvdimm performance stats from PHYP')
> Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
> ---
> arch/powerpc/platforms/pseries/papr_scm.c | 37 ++++++++++++++++-------
> 1 file changed, 26 insertions(+), 11 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
> index 12f1513f0fca..90f0af8fefe8 100644
> --- a/arch/powerpc/platforms/pseries/papr_scm.c
> +++ b/arch/powerpc/platforms/pseries/papr_scm.c
> @@ -907,6 +907,20 @@ static ssize_t flags_show(struct device *dev,
> }
> DEVICE_ATTR_RO(flags);
>
> +umode_t papr_nd_attribute_visible(struct kobject *kobj, struct attribute *attr,
> + int n)
> +{
> + struct device *dev = container_of(kobj, typeof(*dev), kobj);
> + struct nvdimm *nvdimm = to_nvdimm(dev);
> + struct papr_scm_priv *p = nvdimm_provider_data(nvdimm);
> +
> + /* For if perf-stats not available remove perf_stats sysfs */
> + if (attr == &dev_attr_perf_stats.attr && p->stat_buffer_len == 0)
> + return 0;
> +
> + return attr->mode;
> +}
> +
> /* papr_scm specific dimm attributes */
> static struct attribute *papr_nd_attributes[] = {
> &dev_attr_flags.attr,
> @@ -916,6 +930,7 @@ static struct attribute *papr_nd_attributes[] = {
>
> static struct attribute_group papr_nd_attribute_group = {
> .name = "papr",
> + .is_visible = papr_nd_attribute_visible,
> .attrs = papr_nd_attributes,
> };
>
> @@ -931,7 +946,6 @@ static int papr_scm_nvdimm_init(struct papr_scm_priv *p)
> struct nd_region_desc ndr_desc;
> unsigned long dimm_flags;
> int target_nid, online_nid;
> - ssize_t stat_size;
>
> p->bus_desc.ndctl = papr_scm_ndctl;
> p->bus_desc.module = THIS_MODULE;
> @@ -1016,16 +1030,6 @@ static int papr_scm_nvdimm_init(struct papr_scm_priv *p)
> list_add_tail(&p->region_list, &papr_nd_regions);
> mutex_unlock(&papr_ndr_lock);
>
> - /* Try retriving the stat buffer and see if its supported */
> - stat_size = drc_pmem_query_stats(p, NULL, 0);
> - if (stat_size > 0) {
> - p->stat_buffer_len = stat_size;
> - dev_dbg(&p->pdev->dev, "Max perf-stat size %lu-bytes\n",
> - p->stat_buffer_len);
> - } else {
> - dev_info(&p->pdev->dev, "Dimm performance stats unavailable\n");
> - }
> -
> return 0;
>
> err: nvdimm_bus_unregister(p->bus);
> @@ -1102,6 +1106,7 @@ static int papr_scm_probe(struct platform_device *pdev)
> u64 blocks, block_size;
> struct papr_scm_priv *p;
> const char *uuid_str;
> + ssize_t stat_size;
> u64 uuid[2];
> int rc;
>
> @@ -1179,6 +1184,16 @@ static int papr_scm_probe(struct platform_device *pdev)
> p->res.name = pdev->name;
> p->res.flags = IORESOURCE_MEM;
>
> + /* Try retriving the stat buffer and see if its supported */
> + stat_size = drc_pmem_query_stats(p, NULL, 0);
> + if (stat_size > 0) {
> + p->stat_buffer_len = stat_size;
> + dev_dbg(&p->pdev->dev, "Max perf-stat size %lu-bytes\n",
> + p->stat_buffer_len);
> + } else {
> + dev_info(&p->pdev->dev, "Dimm performance stats unavailable\n");
> + }
With this patch https://lore.kernel.org/linuxppc-dev/20210505191606.51666-1-vaibhav@linux.ibm.com
We are adding details of whyy performance stat query hcall failed. Do we
need to print again here? Are we being more verbose here?
-aneesh
^ permalink raw reply
* Re: [PATCH v2] powerpc/papr_scm: Reduce error severity if nvdimm stats inaccessible
From: Ira Weiny @ 2021-05-06 4:58 UTC (permalink / raw)
To: Vaibhav Jain
Cc: Santosh Sivaraj, linux-nvdimm, Aneesh Kumar K . V, Dan Williams,
linuxppc-dev
In-Reply-To: <20210505191606.51666-1-vaibhav@linux.ibm.com>
On Thu, May 06, 2021 at 12:46:06AM +0530, Vaibhav Jain wrote:
> Currently drc_pmem_qeury_stats() generates a dev_err in case
> "Enable Performance Information Collection" feature is disabled from
> HMC or performance stats are not available for an nvdimm. The error is
> of the form below:
>
> papr_scm ibm,persistent-memory:ibm,pmemory@44104001: Failed to query
> performance stats, Err:-10
>
> This error message confuses users as it implies a possible problem
> with the nvdimm even though its due to a disabled/unavailable
> feature. We fix this by explicitly handling the H_AUTHORITY and
> H_UNSUPPORTED errors from the H_SCM_PERFORMANCE_STATS hcall.
>
> In case of H_AUTHORITY error an info message is logged instead of an
> error, saying that "Permission denied while accessing performance
> stats". Also '-EACCES' error is return instead of -EPERM.
I thought you clarified before that this was a permission issue. So why change
the error to EACCES?
>
> In case of H_UNSUPPORTED error we return a -EPERM error back from
> drc_pmem_query_stats() indicating that performance stats-query
> operation is not supported on this nvdimm.
EPERM seems wrong here too... ENOTSUP?
Ira
^ permalink raw reply
* Re: [PATCH] KVM: PPC: Book3S HV: Fix conversion to gfn-based MMU notifier callbacks
From: Nicholas Piggin @ 2021-05-06 4:57 UTC (permalink / raw)
To: Sean Christopherson
Cc: kvm, Aneesh Kumar K . V, kvm-ppc, Bharata B Rao, Paolo Bonzini,
linuxppc-dev
In-Reply-To: <YJK/KDCV5CvTNhoo@google.com>
Excerpts from Sean Christopherson's message of May 6, 2021 1:52 am:
> On Wed, May 05, 2021, Nicholas Piggin wrote:
>> Commit b1c5356e873c ("KVM: PPC: Convert to the gfn-based MMU notifier
>> callbacks") causes unmap_gfn_range and age_gfn callbacks to only work
>> on the first gfn in the range. It also makes the aging callbacks call
>> into both radix and hash aging functions for radix guests. Fix this.
>
> Ugh, the rest of kvm_handle_hva_range() was so similar to the x86 code that I
> glossed right over the for-loop. My apologies :-/
No problem, we should have noticed it here in testing earlier too.
>
>> Add warnings for the single-gfn calls that have been converted to range
>> callbacks, in case they ever receieve ranges greater than 1.
>>
>> Fixes: b1c5356e873c ("KVM: PPC: Convert to the gfn-based MMU notifier callbacks")
>> Reported-by: Bharata B Rao <bharata@linux.ibm.com>
>> Tested-by: Bharata B Rao <bharata@linux.ibm.com>
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> ---
>> The e500 change in that commit also looks suspicious, why is it okay
>> to remove kvm_flush_remote_tlbs() there? Also is the the change from
>> returning false to true intended?
>
> The common code interprets a return of "true" as "do kvm_flush_remote_tlbs()".
> There is technically a functional change, as the deferring the flush to common
> code will batch flushes if the invalidation spans multiple memslots. But the
> mmu_lock is held the entire time, so batching is a good thing unless e500 has
> wildly different MMU semantics.
Ah okay that explains it. That sounds good, but I don't know the e500
KVM code or have a way to test it myself.
Thanks,
Nick
^ permalink raw reply
* [PATCH v2 1/2] powerpc/64s: Fix crashes when toggling stf barrier
From: Michael Ellerman @ 2021-05-06 4:49 UTC (permalink / raw)
To: linuxppc-dev; +Cc: nathanl, anton, npiggin, dja
The STF (store-to-load forwarding) barrier mitigation can be
enabled/disabled at runtime via a debugfs file (stf_barrier), which
causes the kernel to patch itself to enable/disable the relevant
mitigations.
However depending on which mitigation we're using, it may not be safe to
do that patching while other CPUs are active. For example the following
crash:
User access of kernel address (c00000003fff5af0) - exploit attempt? (uid: 0)
segfault (11) at c00000003fff5af0 nip 7fff8ad12198 lr 7fff8ad121f8 code 1
code: 40820128 e93c00d0 e9290058 7c292840 40810058 38600000 4bfd9a81 e8410018
code: 2c030006 41810154 3860ffb6 e9210098 <e94d8ff0> 7d295279 39400000 40820a3c
Shows that we returned to userspace without restoring the user r13
value, due to executing the partially patched STF exit code.
Fix it by doing the patching under stop machine. The CPUs that aren't
doing the patching will be spinning in the core of the stop machine
logic. That is currently sufficient for our purposes, because none of
the patching we do is to that code or anywhere in the vicinity.
Fixes: a048a07d7f45 ("powerpc/64s: Add support for a store forwarding barrier at kernel entry/exit")
Cc: stable@vger.kernel.org # v4.17+
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/lib/feature-fixups.c | 19 +++++++++++++++++--
1 file changed, 17 insertions(+), 2 deletions(-)
v2: Fix the bugs.
Pass a pointer to types, rather than wedging into the void *.
Use stop_machine().
diff --git a/arch/powerpc/lib/feature-fixups.c b/arch/powerpc/lib/feature-fixups.c
index 1fd31b4b0e13..10083add8b33 100644
--- a/arch/powerpc/lib/feature-fixups.c
+++ b/arch/powerpc/lib/feature-fixups.c
@@ -14,6 +14,7 @@
#include <linux/string.h>
#include <linux/init.h>
#include <linux/sched/mm.h>
+#include <linux/stop_machine.h>
#include <asm/cputable.h>
#include <asm/code-patching.h>
#include <asm/page.h>
@@ -227,11 +228,25 @@ static void do_stf_exit_barrier_fixups(enum stf_barrier_type types)
: "unknown");
}
+static int __do_stf_barrier_fixups(void *data)
+{
+ enum stf_barrier_type *types = data;
+
+ do_stf_entry_barrier_fixups(*types);
+ do_stf_exit_barrier_fixups(*types);
+
+ return 0;
+}
void do_stf_barrier_fixups(enum stf_barrier_type types)
{
- do_stf_entry_barrier_fixups(types);
- do_stf_exit_barrier_fixups(types);
+ /*
+ * The call to the fallback entry flush, and the fallback/sync-ori exit
+ * flush can not be safely patched in/out while other CPUs are executing
+ * them. So call __do_stf_barrier_fixups() on one CPU while all other CPUs
+ * spin in the stop machine core with interrupts hard disabled.
+ */
+ stop_machine(__do_stf_barrier_fixups, &types, NULL);
}
void do_uaccess_flush_fixups(enum l1d_flush_type types)
--
2.25.1
^ permalink raw reply related
* [PATCH v2 2/2] powerpc/64s: Fix crashes when toggling entry flush barrier
From: Michael Ellerman @ 2021-05-06 4:49 UTC (permalink / raw)
To: linuxppc-dev; +Cc: nathanl, anton, npiggin, dja
In-Reply-To: <20210506044959.1298123-1-mpe@ellerman.id.au>
The entry flush mitigation can be enabled/disabled at runtime via a
debugfs file (entry_flush), which causes the kernel to patch itself to
enable/disable the relevant mitigations.
However depending on which mitigation we're using, it may not be safe to
do that patching while other CPUs are active. For example the following
crash:
sleeper[15639]: segfault (11) at c000000000004c20 nip c000000000004c20 lr c000000000004c20
Shows that we returned to userspace with a corrupted LR that points into
the kernel, due to executing the partially patched call to the fallback
entry flush (ie. we missed the LR restore).
Fix it by doing the patching under stop machine. The CPUs that aren't
doing the patching will be spinning in the core of the stop machine
logic. That is currently sufficient for our purposes, because none of
the patching we do is to that code or anywhere in the vicinity.
Fixes: f79643787e0a ("powerpc/64s: flush L1D on kernel entry")
Cc: stable@vger.kernel.org # v5.10+
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/lib/feature-fixups.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
v2: Fix the bugs.
Pass a pointer to types, rather than wedging into the void *.
Use stop_machine().
diff --git a/arch/powerpc/lib/feature-fixups.c b/arch/powerpc/lib/feature-fixups.c
index 10083add8b33..0aefa6a4a259 100644
--- a/arch/powerpc/lib/feature-fixups.c
+++ b/arch/powerpc/lib/feature-fixups.c
@@ -299,8 +299,9 @@ void do_uaccess_flush_fixups(enum l1d_flush_type types)
: "unknown");
}
-void do_entry_flush_fixups(enum l1d_flush_type types)
+static int __do_entry_flush_fixups(void *data)
{
+ enum l1d_flush_type types = *(enum l1d_flush_type *)data;
unsigned int instrs[3], *dest;
long *start, *end;
int i;
@@ -369,6 +370,19 @@ void do_entry_flush_fixups(enum l1d_flush_type types)
: "ori type" :
(types & L1D_FLUSH_MTTRIG) ? "mttrig type"
: "unknown");
+
+ return 0;
+}
+
+void do_entry_flush_fixups(enum l1d_flush_type types)
+{
+ /*
+ * The call to the fallback flush can not be safely patched in/out while
+ * other CPUs are executing it. So call __do_entry_flush_fixups() on one
+ * CPU while all other CPUs spin in the stop machine core with interrupts
+ * hard disabled.
+ */
+ stop_machine(__do_entry_flush_fixups, &types, NULL);
}
void do_rfi_flush_fixups(enum l1d_flush_type types)
--
2.25.1
^ permalink raw reply related
* [RESEND PATCH v4 00/11] Use per-CPU temporary mappings for patching
From: Christopher M. Riedl @ 2021-05-06 4:34 UTC (permalink / raw)
To: linuxppc-dev; +Cc: tglx, x86, linux-hardening, keescook
When compiled with CONFIG_STRICT_KERNEL_RWX, the kernel must create
temporary mappings when patching itself. These mappings temporarily
override the strict RWX text protections to permit a write. Currently,
powerpc allocates a per-CPU VM area for patching. Patching occurs as
follows:
1. Map page in per-CPU VM area w/ PAGE_KERNEL protection
2. Patch text
3. Remove the temporary mapping
While the VM area is per-CPU, the mapping is actually inserted into the
kernel page tables. Presumably, this could allow another CPU to access
the normally write-protected text - either malicously or accidentally -
via this same mapping if the address of the VM area is known. Ideally,
the mapping should be kept local to the CPU doing the patching [0].
x86 introduced "temporary mm" structs which allow the creation of
mappings local to a particular CPU [1]. This series intends to bring the
notion of a temporary mm to powerpc and harden powerpc by using such a
mapping for patching a kernel with strict RWX permissions.
The first four patches implement an LKDTM test "proof-of-concept" which
exploits the potential vulnerability (ie. the temporary mapping during
patching is exposed in the kernel page tables and accessible by other
CPUs) using a simple brute-force approach. This test is implemented for
both powerpc and x86_64. The test passes on powerpc with this new
series, fails on upstream powerpc, passes on upstream x86_64, and fails
on an older (ancient) x86_64 tree without the x86_64 temporary mm
patches. The remaining patches add support for and use a temporary mm
for code patching on powerpc.
Tested boot, ftrace, and repeated LKDTM "hijack":
- QEMU+KVM (host: POWER9 Blackbird): Radix MMU w/ KUAP
- QEMU+KVM (host: POWER9 Blackbird): Hash MMU w/o KUAP
- QEMU+KVM (host: POWER9 Blackbird): Hash MMU w/ KUAP
Tested repeated LKDTM "hijack":
- QEMU+KVM (host: AMD desktop): x86_64 upstream
- QEMU+KVM (host: AMD desktop): x86_64 w/o percpu temp mm to
verify the LKDTM "hijack" fails
Tested boot and ftrace:
- QEMU+TCG: ppc44x (bamboo)
- QEMU+TCG: g5 (mac99)
I also tested with various extra config options enabled as suggested in
section 12) in Documentation/process/submit-checklist.rst.
[ Apologies about the resend - some of the patches were dropped by
the CC'd linux-hardening list due to tls problems between mailbox.org
and vger.kernel.org. I re-signed my patches w/ my IBM email; no other
changes. ]
v4: * It's time to revisit this series again since @jpn and @mpe fixed
our known STRICT_*_RWX bugs on powerpc/64s.
* Rebase on linuxppc/next:
commit ee1bc694fbaec ("powerpc/kvm: Fix build error when PPC_MEM_KEYS/PPC_PSERIES=n")
* Completely rework how map_patch() works on book3s64 Hash MMU
* Split the LKDTM x86_64 and powerpc bits into separate patches
* Annotate commit messages with changes from v3 instead of
listing them here completely out-of context...
v3: * Rebase on linuxppc/next: commit 9123e3a74ec7 ("Linux 5.9-rc1")
* Move temporary mm implementation into code-patching.c where it
belongs
* Implement LKDTM hijacker test on x86_64 (on IBM time oof) Do
* not use address zero for the patching address in the
temporary mm (thanks @dja for pointing this out!)
* Wrap the LKDTM test w/ CONFIG_SMP as suggested by Christophe
Leroy
* Comments to clarify PTE pre-allocation and patching addr
selection
v2: * Rebase on linuxppc/next:
commit 105fb38124a4 ("powerpc/8xx: Modify ptep_get()")
* Always dirty pte when mapping patch
* Use `ppc_inst_len` instead of `sizeof` on instructions
* Declare LKDTM patching addr accessor in header where it belongs
v1: * Rebase on linuxppc/next (4336b9337824)
* Save and restore second hw watchpoint
* Use new ppc_inst_* functions for patching check and in LKDTM test
rfc-v2: * Many fixes and improvements mostly based on extensive feedback
and testing by Christophe Leroy (thanks!).
* Make patching_mm and patching_addr static and move
'__ro_after_init' to after the variable name (more common in
other parts of the kernel)
* Use 'asm/debug.h' header instead of 'asm/hw_breakpoint.h' to
fix PPC64e compile
* Add comment explaining why we use BUG_ON() during the init
call to setup for patching later
* Move ptep into patch_mapping to avoid walking page tables a
second time when unmapping the temporary mapping
* Use KUAP under non-radix, also manually dirty the PTE for patch
mapping on non-BOOK3S_64 platforms
* Properly return any error from __patch_instruction
* Do not use 'memcmp' where a simple comparison is appropriate
* Simplify expression for patch address by removing pointer maths
* Add LKDTM test
[0]: https://github.com/linuxppc/issues/issues/224
[1]: https://lore.kernel.org/kernel-hardening/20190426232303.28381-1-nadav.amit@gmail.com/
Christopher M. Riedl (11):
powerpc: Add LKDTM accessor for patching addr
lkdtm/powerpc: Add test to hijack a patch mapping
x86_64: Add LKDTM accessor for patching addr
lkdtm/x86_64: Add test to hijack a patch mapping
powerpc/64s: Add ability to skip SLB preload
powerpc: Introduce temporary mm
powerpc/64s: Make slb_allocate_user() non-static
powerpc: Initialize and use a temporary mm for patching
lkdtm/powerpc: Fix code patching hijack test
powerpc: Protect patching_mm with a lock
powerpc: Use patch_instruction_unlocked() in loops
arch/powerpc/include/asm/book3s/64/mmu-hash.h | 1 +
arch/powerpc/include/asm/book3s/64/mmu.h | 3 +
arch/powerpc/include/asm/code-patching.h | 8 +
arch/powerpc/include/asm/debug.h | 1 +
arch/powerpc/include/asm/mmu_context.h | 13 +
arch/powerpc/kernel/epapr_paravirt.c | 9 +-
arch/powerpc/kernel/optprobes.c | 22 +-
arch/powerpc/kernel/process.c | 5 +
arch/powerpc/lib/code-patching.c | 348 +++++++++++++-----
arch/powerpc/lib/feature-fixups.c | 114 ++++--
arch/powerpc/mm/book3s64/mmu_context.c | 2 +
arch/powerpc/mm/book3s64/slb.c | 60 +--
arch/powerpc/xmon/xmon.c | 22 +-
arch/x86/include/asm/text-patching.h | 4 +
arch/x86/kernel/alternative.c | 7 +
drivers/misc/lkdtm/core.c | 1 +
drivers/misc/lkdtm/lkdtm.h | 1 +
drivers/misc/lkdtm/perms.c | 149 ++++++++
18 files changed, 608 insertions(+), 162 deletions(-)
--
2.26.1
^ permalink raw reply
* [RESEND PATCH v4 11/11] powerpc: Use patch_instruction_unlocked() in loops
From: Christopher M. Riedl @ 2021-05-06 4:34 UTC (permalink / raw)
To: linuxppc-dev; +Cc: tglx, x86, linux-hardening, keescook
In-Reply-To: <20210506043452.9674-1-cmr@linux.ibm.com>
Now that patching requires a lock to prevent concurrent access to
patching_mm, every call to patch_instruction() acquires and releases a
spinlock. There are several places where patch_instruction() is called
in a loop. Convert these to acquire the lock once before the loop, call
patch_instruction_unlocked() in the loop body, and then release the lock
again after the loop terminates - as in:
for (i = 0; i < n; ++i)
patch_instruction(...); <-- lock/unlock every iteration
changes to:
flags = lock_patching(); <-- lock once
for (i = 0; i < n; ++i)
patch_instruction_unlocked(...);
unlock_patching(flags); <-- unlock once
Signed-off-by: Christopher M. Riedl <cmr@linux.ibm.com>
---
v4: * New to series.
---
arch/powerpc/kernel/epapr_paravirt.c | 9 ++-
arch/powerpc/kernel/optprobes.c | 22 ++++--
arch/powerpc/lib/feature-fixups.c | 114 +++++++++++++++++++--------
arch/powerpc/xmon/xmon.c | 22 ++++--
4 files changed, 120 insertions(+), 47 deletions(-)
diff --git a/arch/powerpc/kernel/epapr_paravirt.c b/arch/powerpc/kernel/epapr_paravirt.c
index 2ed14d4a47f59..b639e71cf9dec 100644
--- a/arch/powerpc/kernel/epapr_paravirt.c
+++ b/arch/powerpc/kernel/epapr_paravirt.c
@@ -28,6 +28,7 @@ static int __init early_init_dt_scan_epapr(unsigned long node,
const u32 *insts;
int len;
int i;
+ unsigned long flags;
insts = of_get_flat_dt_prop(node, "hcall-instructions", &len);
if (!insts)
@@ -36,14 +37,18 @@ static int __init early_init_dt_scan_epapr(unsigned long node,
if (len % 4 || len > (4 * 4))
return -1;
+ flags = lock_patching();
+
for (i = 0; i < (len / 4); i++) {
struct ppc_inst inst = ppc_inst(be32_to_cpu(insts[i]));
- patch_instruction((struct ppc_inst *)(epapr_hypercall_start + i), inst);
+ patch_instruction_unlocked((struct ppc_inst *)(epapr_hypercall_start + i), inst);
#if !defined(CONFIG_64BIT) || defined(CONFIG_PPC_BOOK3E_64)
- patch_instruction((struct ppc_inst *)(epapr_ev_idle_start + i), inst);
+ patch_instruction_unlocked((struct ppc_inst *)(epapr_ev_idle_start + i), inst);
#endif
}
+ unlock_patching(flags);
+
#if !defined(CONFIG_64BIT) || defined(CONFIG_PPC_BOOK3E_64)
if (of_get_flat_dt_prop(node, "has-idle", NULL))
epapr_has_idle = true;
diff --git a/arch/powerpc/kernel/optprobes.c b/arch/powerpc/kernel/optprobes.c
index cdf87086fa33a..deaeb6e8d1a00 100644
--- a/arch/powerpc/kernel/optprobes.c
+++ b/arch/powerpc/kernel/optprobes.c
@@ -200,7 +200,7 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p)
struct ppc_inst branch_op_callback, branch_emulate_step, temp;
kprobe_opcode_t *op_callback_addr, *emulate_step_addr, *buff;
long b_offset;
- unsigned long nip, size;
+ unsigned long nip, size, flags;
int rc, i;
kprobe_ppc_optinsn_slots.insn_size = MAX_OPTINSN_SIZE;
@@ -237,13 +237,20 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p)
/* We can optimize this via patch_instruction_window later */
size = (TMPL_END_IDX * sizeof(kprobe_opcode_t)) / sizeof(int);
pr_devel("Copying template to %p, size %lu\n", buff, size);
+
+ flags = lock_patching();
+
for (i = 0; i < size; i++) {
- rc = patch_instruction((struct ppc_inst *)(buff + i),
- ppc_inst(*(optprobe_template_entry + i)));
- if (rc < 0)
+ rc = patch_instruction_unlocked((struct ppc_inst *)(buff + i),
+ ppc_inst(*(optprobe_template_entry + i)));
+ if (rc < 0) {
+ unlock_patching(flags);
goto error;
+ }
}
+ unlock_patching(flags);
+
/*
* Fixup the template with instructions to:
* 1. load the address of the actual probepoint
@@ -322,6 +329,9 @@ void arch_optimize_kprobes(struct list_head *oplist)
struct ppc_inst instr;
struct optimized_kprobe *op;
struct optimized_kprobe *tmp;
+ unsigned long flags;
+
+ flags = lock_patching();
list_for_each_entry_safe(op, tmp, oplist, list) {
/*
@@ -333,9 +343,11 @@ void arch_optimize_kprobes(struct list_head *oplist)
create_branch(&instr,
(struct ppc_inst *)op->kp.addr,
(unsigned long)op->optinsn.insn, 0);
- patch_instruction((struct ppc_inst *)op->kp.addr, instr);
+ patch_instruction_unlocked((struct ppc_inst *)op->kp.addr, instr);
list_del_init(&op->list);
}
+
+ unlock_patching(flags);
}
void arch_unoptimize_kprobe(struct optimized_kprobe *op)
diff --git a/arch/powerpc/lib/feature-fixups.c b/arch/powerpc/lib/feature-fixups.c
index 1fd31b4b0e139..2c3d413c9d9b3 100644
--- a/arch/powerpc/lib/feature-fixups.c
+++ b/arch/powerpc/lib/feature-fixups.c
@@ -123,6 +123,7 @@ static void do_stf_entry_barrier_fixups(enum stf_barrier_type types)
unsigned int instrs[3], *dest;
long *start, *end;
int i;
+ unsigned long flags;
start = PTRRELOC(&__start___stf_entry_barrier_fixup);
end = PTRRELOC(&__stop___stf_entry_barrier_fixup);
@@ -144,24 +145,29 @@ static void do_stf_entry_barrier_fixups(enum stf_barrier_type types)
instrs[i++] = 0x63ff0000; /* ori 31,31,0 speculation barrier */
}
+ flags = lock_patching();
+
for (i = 0; start < end; start++, i++) {
dest = (void *)start + *start;
pr_devel("patching dest %lx\n", (unsigned long)dest);
- patch_instruction((struct ppc_inst *)dest, ppc_inst(instrs[0]));
+ patch_instruction_unlocked((struct ppc_inst *)dest, ppc_inst(instrs[0]));
if (types & STF_BARRIER_FALLBACK)
- patch_branch((struct ppc_inst *)(dest + 1),
- (unsigned long)&stf_barrier_fallback,
- BRANCH_SET_LINK);
+ patch_branch_unlocked((struct ppc_inst *)(dest + 1),
+ (unsigned long)&stf_barrier_fallback,
+ BRANCH_SET_LINK);
else
- patch_instruction((struct ppc_inst *)(dest + 1),
- ppc_inst(instrs[1]));
+ patch_instruction_unlocked((struct ppc_inst *)(dest + 1),
+ ppc_inst(instrs[1]));
- patch_instruction((struct ppc_inst *)(dest + 2), ppc_inst(instrs[2]));
+ patch_instruction_unlocked((struct ppc_inst *)(dest + 2),
+ ppc_inst(instrs[2]));
}
+ unlock_patching(flags);
+
printk(KERN_DEBUG "stf-barrier: patched %d entry locations (%s barrier)\n", i,
(types == STF_BARRIER_NONE) ? "no" :
(types == STF_BARRIER_FALLBACK) ? "fallback" :
@@ -175,6 +181,7 @@ static void do_stf_exit_barrier_fixups(enum stf_barrier_type types)
unsigned int instrs[6], *dest;
long *start, *end;
int i;
+ unsigned long flags;
start = PTRRELOC(&__start___stf_exit_barrier_fixup);
end = PTRRELOC(&__stop___stf_exit_barrier_fixup);
@@ -207,18 +214,23 @@ static void do_stf_exit_barrier_fixups(enum stf_barrier_type types)
instrs[i++] = 0x7e0006ac; /* eieio + bit 6 hint */
}
+ flags = lock_patching();
+
for (i = 0; start < end; start++, i++) {
dest = (void *)start + *start;
pr_devel("patching dest %lx\n", (unsigned long)dest);
- patch_instruction((struct ppc_inst *)dest, ppc_inst(instrs[0]));
- patch_instruction((struct ppc_inst *)(dest + 1), ppc_inst(instrs[1]));
- patch_instruction((struct ppc_inst *)(dest + 2), ppc_inst(instrs[2]));
- patch_instruction((struct ppc_inst *)(dest + 3), ppc_inst(instrs[3]));
- patch_instruction((struct ppc_inst *)(dest + 4), ppc_inst(instrs[4]));
- patch_instruction((struct ppc_inst *)(dest + 5), ppc_inst(instrs[5]));
+ patch_instruction_unlocked((struct ppc_inst *)dest, ppc_inst(instrs[0]));
+ patch_instruction_unlocked((struct ppc_inst *)(dest + 1), ppc_inst(instrs[1]));
+ patch_instruction_unlocked((struct ppc_inst *)(dest + 2), ppc_inst(instrs[2]));
+ patch_instruction_unlocked((struct ppc_inst *)(dest + 3), ppc_inst(instrs[3]));
+ patch_instruction_unlocked((struct ppc_inst *)(dest + 4), ppc_inst(instrs[4]));
+ patch_instruction_unlocked((struct ppc_inst *)(dest + 5), ppc_inst(instrs[5]));
}
+
+ unlock_patching(flags);
+
printk(KERN_DEBUG "stf-barrier: patched %d exit locations (%s barrier)\n", i,
(types == STF_BARRIER_NONE) ? "no" :
(types == STF_BARRIER_FALLBACK) ? "fallback" :
@@ -239,6 +251,7 @@ void do_uaccess_flush_fixups(enum l1d_flush_type types)
unsigned int instrs[4], *dest;
long *start, *end;
int i;
+ unsigned long flags;
start = PTRRELOC(&__start___uaccess_flush_fixup);
end = PTRRELOC(&__stop___uaccess_flush_fixup);
@@ -262,18 +275,22 @@ void do_uaccess_flush_fixups(enum l1d_flush_type types)
if (types & L1D_FLUSH_MTTRIG)
instrs[i++] = 0x7c12dba6; /* mtspr TRIG2,r0 (SPR #882) */
+ flags = lock_patching();
+
for (i = 0; start < end; start++, i++) {
dest = (void *)start + *start;
pr_devel("patching dest %lx\n", (unsigned long)dest);
- patch_instruction((struct ppc_inst *)dest, ppc_inst(instrs[0]));
+ patch_instruction_unlocked((struct ppc_inst *)dest, ppc_inst(instrs[0]));
- patch_instruction((struct ppc_inst *)(dest + 1), ppc_inst(instrs[1]));
- patch_instruction((struct ppc_inst *)(dest + 2), ppc_inst(instrs[2]));
- patch_instruction((struct ppc_inst *)(dest + 3), ppc_inst(instrs[3]));
+ patch_instruction_unlocked((struct ppc_inst *)(dest + 1), ppc_inst(instrs[1]));
+ patch_instruction_unlocked((struct ppc_inst *)(dest + 2), ppc_inst(instrs[2]));
+ patch_instruction_unlocked((struct ppc_inst *)(dest + 3), ppc_inst(instrs[3]));
}
+ unlock_patching(flags);
+
printk(KERN_DEBUG "uaccess-flush: patched %d locations (%s flush)\n", i,
(types == L1D_FLUSH_NONE) ? "no" :
(types == L1D_FLUSH_FALLBACK) ? "fallback displacement" :
@@ -289,6 +306,7 @@ void do_entry_flush_fixups(enum l1d_flush_type types)
unsigned int instrs[3], *dest;
long *start, *end;
int i;
+ unsigned long flags;
instrs[0] = 0x60000000; /* nop */
instrs[1] = 0x60000000; /* nop */
@@ -309,6 +327,8 @@ void do_entry_flush_fixups(enum l1d_flush_type types)
if (types & L1D_FLUSH_MTTRIG)
instrs[i++] = 0x7c12dba6; /* mtspr TRIG2,r0 (SPR #882) */
+ flags = lock_patching();
+
start = PTRRELOC(&__start___entry_flush_fixup);
end = PTRRELOC(&__stop___entry_flush_fixup);
for (i = 0; start < end; start++, i++) {
@@ -316,15 +336,17 @@ void do_entry_flush_fixups(enum l1d_flush_type types)
pr_devel("patching dest %lx\n", (unsigned long)dest);
- patch_instruction((struct ppc_inst *)dest, ppc_inst(instrs[0]));
+ patch_instruction_unlocked((struct ppc_inst *)dest, ppc_inst(instrs[0]));
if (types == L1D_FLUSH_FALLBACK)
- patch_branch((struct ppc_inst *)(dest + 1), (unsigned long)&entry_flush_fallback,
- BRANCH_SET_LINK);
+ patch_branch_unlocked((struct ppc_inst *)(dest + 1),
+ (unsigned long)&entry_flush_fallback,
+ BRANCH_SET_LINK);
else
- patch_instruction((struct ppc_inst *)(dest + 1), ppc_inst(instrs[1]));
+ patch_instruction_unlocked((struct ppc_inst *)(dest + 1),
+ ppc_inst(instrs[1]));
- patch_instruction((struct ppc_inst *)(dest + 2), ppc_inst(instrs[2]));
+ patch_instruction_unlocked((struct ppc_inst *)(dest + 2), ppc_inst(instrs[2]));
}
start = PTRRELOC(&__start___scv_entry_flush_fixup);
@@ -334,17 +356,20 @@ void do_entry_flush_fixups(enum l1d_flush_type types)
pr_devel("patching dest %lx\n", (unsigned long)dest);
- patch_instruction((struct ppc_inst *)dest, ppc_inst(instrs[0]));
+ patch_instruction_unlocked((struct ppc_inst *)dest, ppc_inst(instrs[0]));
if (types == L1D_FLUSH_FALLBACK)
- patch_branch((struct ppc_inst *)(dest + 1), (unsigned long)&scv_entry_flush_fallback,
- BRANCH_SET_LINK);
+ patch_branch_unlocked((struct ppc_inst *)(dest + 1),
+ (unsigned long)&scv_entry_flush_fallback,
+ BRANCH_SET_LINK);
else
- patch_instruction((struct ppc_inst *)(dest + 1), ppc_inst(instrs[1]));
+ patch_instruction_unlocked((struct ppc_inst *)(dest + 1),
+ ppc_inst(instrs[1]));
- patch_instruction((struct ppc_inst *)(dest + 2), ppc_inst(instrs[2]));
+ patch_instruction_unlocked((struct ppc_inst *)(dest + 2), ppc_inst(instrs[2]));
}
+ unlock_patching(flags);
printk(KERN_DEBUG "entry-flush: patched %d locations (%s flush)\n", i,
(types == L1D_FLUSH_NONE) ? "no" :
@@ -361,6 +386,7 @@ void do_rfi_flush_fixups(enum l1d_flush_type types)
unsigned int instrs[3], *dest;
long *start, *end;
int i;
+ unsigned long flags;
start = PTRRELOC(&__start___rfi_flush_fixup);
end = PTRRELOC(&__stop___rfi_flush_fixup);
@@ -382,16 +408,20 @@ void do_rfi_flush_fixups(enum l1d_flush_type types)
if (types & L1D_FLUSH_MTTRIG)
instrs[i++] = 0x7c12dba6; /* mtspr TRIG2,r0 (SPR #882) */
+ flags = lock_patching();
+
for (i = 0; start < end; start++, i++) {
dest = (void *)start + *start;
pr_devel("patching dest %lx\n", (unsigned long)dest);
- patch_instruction((struct ppc_inst *)dest, ppc_inst(instrs[0]));
- patch_instruction((struct ppc_inst *)(dest + 1), ppc_inst(instrs[1]));
- patch_instruction((struct ppc_inst *)(dest + 2), ppc_inst(instrs[2]));
+ patch_instruction_unlocked((struct ppc_inst *)dest, ppc_inst(instrs[0]));
+ patch_instruction_unlocked((struct ppc_inst *)(dest + 1), ppc_inst(instrs[1]));
+ patch_instruction_unlocked((struct ppc_inst *)(dest + 2), ppc_inst(instrs[2]));
}
+ unlock_patching(flags);
+
printk(KERN_DEBUG "rfi-flush: patched %d locations (%s flush)\n", i,
(types == L1D_FLUSH_NONE) ? "no" :
(types == L1D_FLUSH_FALLBACK) ? "fallback displacement" :
@@ -407,6 +437,7 @@ void do_barrier_nospec_fixups_range(bool enable, void *fixup_start, void *fixup_
unsigned int instr, *dest;
long *start, *end;
int i;
+ unsigned long flags;
start = fixup_start;
end = fixup_end;
@@ -418,13 +449,17 @@ void do_barrier_nospec_fixups_range(bool enable, void *fixup_start, void *fixup_
instr = 0x63ff0000; /* ori 31,31,0 speculation barrier */
}
+ flags = lock_patching();
+
for (i = 0; start < end; start++, i++) {
dest = (void *)start + *start;
pr_devel("patching dest %lx\n", (unsigned long)dest);
- patch_instruction((struct ppc_inst *)dest, ppc_inst(instr));
+ patch_instruction_unlocked((struct ppc_inst *)dest, ppc_inst(instr));
}
+ unlock_patching(flags);
+
printk(KERN_DEBUG "barrier-nospec: patched %d locations\n", i);
}
@@ -448,6 +483,7 @@ void do_barrier_nospec_fixups_range(bool enable, void *fixup_start, void *fixup_
unsigned int instr[2], *dest;
long *start, *end;
int i;
+ unsigned long flags;
start = fixup_start;
end = fixup_end;
@@ -461,27 +497,37 @@ void do_barrier_nospec_fixups_range(bool enable, void *fixup_start, void *fixup_
instr[1] = PPC_INST_SYNC;
}
+ flags = lock_patching();
+
for (i = 0; start < end; start++, i++) {
dest = (void *)start + *start;
pr_devel("patching dest %lx\n", (unsigned long)dest);
- patch_instruction((struct ppc_inst *)dest, ppc_inst(instr[0]));
- patch_instruction((struct ppc_inst *)(dest + 1), ppc_inst(instr[1]));
+ patch_instruction_unlocked((struct ppc_inst *)dest, ppc_inst(instr[0]));
+ patch_instruction_unlocked((struct ppc_inst *)(dest + 1), ppc_inst(instr[1]));
}
+ unlock_patching(flags);
+
printk(KERN_DEBUG "barrier-nospec: patched %d locations\n", i);
}
static void patch_btb_flush_section(long *curr)
{
unsigned int *start, *end;
+ unsigned long flags;
start = (void *)curr + *curr;
end = (void *)curr + *(curr + 1);
+
+ flags = lock_patching();
+
for (; start < end; start++) {
pr_devel("patching dest %lx\n", (unsigned long)start);
- patch_instruction((struct ppc_inst *)start, ppc_inst(PPC_INST_NOP));
+ patch_instruction_unlocked((struct ppc_inst *)start, ppc_inst(PPC_INST_NOP));
}
+
+ unlock_patching(flags);
}
void do_btb_flush_fixups(void)
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index ff2b92bfeedcc..e8a00041c04bf 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -905,6 +905,9 @@ static void insert_bpts(void)
int i;
struct ppc_inst instr, instr2;
struct bpt *bp, *bp2;
+ unsigned long flags;
+
+ flags = lock_patching();
bp = bpts;
for (i = 0; i < NBPTS; ++i, ++bp) {
@@ -945,19 +948,21 @@ static void insert_bpts(void)
continue;
}
- patch_instruction(bp->instr, instr);
- patch_instruction(ppc_inst_next(bp->instr, &instr),
- ppc_inst(bpinstr));
+ patch_instruction_unlocked(bp->instr, instr);
+ patch_instruction_unlocked(ppc_inst_next(bp->instr, &instr),
+ ppc_inst(bpinstr));
if (bp->enabled & BP_CIABR)
continue;
- if (patch_instruction((struct ppc_inst *)bp->address,
- ppc_inst(bpinstr)) != 0) {
+ if (patch_instruction_unlocked((struct ppc_inst *)bp->address,
+ ppc_inst(bpinstr)) != 0) {
printf("Couldn't write instruction at %lx, "
"disabling breakpoint there\n", bp->address);
bp->enabled &= ~BP_TRAP;
continue;
}
}
+
+ unlock_patching(flags);
}
static void insert_cpu_bpts(void)
@@ -984,6 +989,9 @@ static void remove_bpts(void)
int i;
struct bpt *bp;
struct ppc_inst instr;
+ unsigned long flags;
+
+ flags = lock_patching();
bp = bpts;
for (i = 0; i < NBPTS; ++i, ++bp) {
@@ -991,11 +999,13 @@ static void remove_bpts(void)
continue;
if (mread_instr(bp->address, &instr)
&& ppc_inst_equal(instr, ppc_inst(bpinstr))
- && patch_instruction(
+ && patch_instruction_unlocked(
(struct ppc_inst *)bp->address, ppc_inst_read(bp->instr)) != 0)
printf("Couldn't remove breakpoint at %lx\n",
bp->address);
}
+
+ unlock_patching(flags);
}
static void remove_cpu_bpts(void)
--
2.26.1
^ permalink raw reply related
* [RESEND PATCH v4 06/11] powerpc: Introduce temporary mm
From: Christopher M. Riedl @ 2021-05-06 4:34 UTC (permalink / raw)
To: linuxppc-dev; +Cc: tglx, x86, linux-hardening, keescook
In-Reply-To: <20210506043452.9674-1-cmr@linux.ibm.com>
x86 supports the notion of a temporary mm which restricts access to
temporary PTEs to a single CPU. A temporary mm is useful for situations
where a CPU needs to perform sensitive operations (such as patching a
STRICT_KERNEL_RWX kernel) requiring temporary mappings without exposing
said mappings to other CPUs. A side benefit is that other CPU TLBs do
not need to be flushed when the temporary mm is torn down.
Mappings in the temporary mm can be set in the userspace portion of the
address-space.
Interrupts must be disabled while the temporary mm is in use. HW
breakpoints, which may have been set by userspace as watchpoints on
addresses now within the temporary mm, are saved and disabled when
loading the temporary mm. The HW breakpoints are restored when unloading
the temporary mm. All HW breakpoints are indiscriminately disabled while
the temporary mm is in use.
With the Book3s64 Hash MMU the SLB is preloaded with entries from the
current thread_info struct during switch_slb(). This could cause a
Machine Check (MCE) due to an SLB Multihit when creating arbitrary
userspace mappings in the temporary mm later. Disable SLB preload from
the thread_info struct for any temporary mm to avoid this.
Based on x86 implementation:
commit cefa929c034e
("x86/mm: Introduce temporary mm structs")
Signed-off-by: Christopher M. Riedl <cmr@linux.ibm.com>
---
v4: * Pass the prev mm instead of NULL to switch_mm_irqs_off() when
using/unusing the temp mm as suggested by Jann Horn to keep
the context.active counter in-sync on mm/nohash.
* Disable SLB preload in the temporary mm when initializing the
temp_mm struct.
* Include asm/debug.h header to fix build issue with
ppc44x_defconfig.
---
arch/powerpc/include/asm/debug.h | 1 +
arch/powerpc/kernel/process.c | 5 +++
arch/powerpc/lib/code-patching.c | 67 ++++++++++++++++++++++++++++++++
3 files changed, 73 insertions(+)
diff --git a/arch/powerpc/include/asm/debug.h b/arch/powerpc/include/asm/debug.h
index 86a14736c76c3..dfd82635ea8b3 100644
--- a/arch/powerpc/include/asm/debug.h
+++ b/arch/powerpc/include/asm/debug.h
@@ -46,6 +46,7 @@ static inline int debugger_fault_handler(struct pt_regs *regs) { return 0; }
#endif
void __set_breakpoint(int nr, struct arch_hw_breakpoint *brk);
+void __get_breakpoint(int nr, struct arch_hw_breakpoint *brk);
bool ppc_breakpoint_available(void);
#ifdef CONFIG_PPC_ADV_DEBUG_REGS
extern void do_send_trap(struct pt_regs *regs, unsigned long address,
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 89e34aa273e21..8e94cabaea3c3 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -864,6 +864,11 @@ static inline int set_breakpoint_8xx(struct arch_hw_breakpoint *brk)
return 0;
}
+void __get_breakpoint(int nr, struct arch_hw_breakpoint *brk)
+{
+ memcpy(brk, this_cpu_ptr(¤t_brk[nr]), sizeof(*brk));
+}
+
void __set_breakpoint(int nr, struct arch_hw_breakpoint *brk)
{
memcpy(this_cpu_ptr(¤t_brk[nr]), brk, sizeof(*brk));
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index 2b1b3e9043ade..cbdfba8a39360 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -17,6 +17,8 @@
#include <asm/code-patching.h>
#include <asm/setup.h>
#include <asm/inst.h>
+#include <asm/mmu_context.h>
+#include <asm/debug.h>
static int __patch_instruction(struct ppc_inst *exec_addr, struct ppc_inst instr,
struct ppc_inst *patch_addr)
@@ -46,6 +48,71 @@ int raw_patch_instruction(struct ppc_inst *addr, struct ppc_inst instr)
}
#ifdef CONFIG_STRICT_KERNEL_RWX
+
+struct temp_mm {
+ struct mm_struct *temp;
+ struct mm_struct *prev;
+ struct arch_hw_breakpoint brk[HBP_NUM_MAX];
+};
+
+static inline void init_temp_mm(struct temp_mm *temp_mm, struct mm_struct *mm)
+{
+ /* Do not preload SLB entries from the thread_info struct */
+ if (IS_ENABLED(CONFIG_PPC_BOOK3S_64) && !radix_enabled())
+ skip_slb_preload_mm(mm);
+
+ temp_mm->temp = mm;
+ temp_mm->prev = NULL;
+ memset(&temp_mm->brk, 0, sizeof(temp_mm->brk));
+}
+
+static inline void use_temporary_mm(struct temp_mm *temp_mm)
+{
+ lockdep_assert_irqs_disabled();
+
+ temp_mm->prev = current->active_mm;
+ switch_mm_irqs_off(temp_mm->prev, temp_mm->temp, current);
+
+ WARN_ON(!mm_is_thread_local(temp_mm->temp));
+
+ if (ppc_breakpoint_available()) {
+ struct arch_hw_breakpoint null_brk = {0};
+ int i = 0;
+
+ for (; i < nr_wp_slots(); ++i) {
+ __get_breakpoint(i, &temp_mm->brk[i]);
+ if (temp_mm->brk[i].type != 0)
+ __set_breakpoint(i, &null_brk);
+ }
+ }
+}
+
+static inline void unuse_temporary_mm(struct temp_mm *temp_mm)
+{
+ lockdep_assert_irqs_disabled();
+
+ switch_mm_irqs_off(temp_mm->temp, temp_mm->prev, current);
+
+ /*
+ * On book3s64 the active_cpus counter increments in
+ * switch_mm_irqs_off(). With the Hash MMU this counter affects if TLB
+ * flushes are local. We have to manually decrement that counter here
+ * along with removing our current CPU from the mm's cpumask so that in
+ * the future a different CPU can reuse the temporary mm and still rely
+ * on local TLB flushes.
+ */
+ dec_mm_active_cpus(temp_mm->temp);
+ cpumask_clear_cpu(smp_processor_id(), mm_cpumask(temp_mm->temp));
+
+ if (ppc_breakpoint_available()) {
+ int i = 0;
+
+ for (; i < nr_wp_slots(); ++i)
+ if (temp_mm->brk[i].type != 0)
+ __set_breakpoint(i, &temp_mm->brk[i]);
+ }
+}
+
static DEFINE_PER_CPU(struct vm_struct *, text_poke_area);
#if IS_BUILTIN(CONFIG_LKDTM)
--
2.26.1
^ permalink raw reply related
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