* Re: [PATCH 8/8] kconfig: remove CONFIG_MCA leftovers
From: Christoph Hellwig @ 2018-10-17 8:17 UTC (permalink / raw)
To: Masahiro Yamada
Cc: linux-arch, linux-scsi, Linux Kbuild mailing list, linux-pci,
Linux Kernel Mailing List, Dominik Brodowski, Alex Bounine,
linuxppc-dev, Christoph Hellwig
In-Reply-To: <CAK7LNAQwyWUq_ohVMe9QLLbyHHQdjkrkM9DPd3pgYLz8UhG6rA@mail.gmail.com>
On Tue, Oct 16, 2018 at 02:20:23PM +0900, Masahiro Yamada wrote:
> On Sun, Oct 14, 2018 at 12:11 AM Christoph Hellwig <hch@lst.de> wrote:
> >
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > ---
>
>
> Can you use "powerpc:" or something
> for the subject line?
>
> I'd like to see "kconfig:" only for patches
> that touch the scripts/kconfig/ directory.
Sorry, I missed this for v2. Will fix it up for the next version
or let you fix it up if there isn't one.
^ permalink raw reply
* Re: move bus (PCI, PCMCIA, EISA, rapdio) config to drivers/ v2
From: Geert Uytterhoeven @ 2018-10-17 8:30 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Linux-Arch, scsi, linux-kbuild, linux-pci,
Linux Kernel Mailing List, Dominik Brodowski, Masahiro Yamada,
Alex Bounine, linuxppc-dev
In-Reply-To: <20181017080201.10866-1-hch@lst.de>
Hi Christoph,
On Wed, Oct 17, 2018 at 10:03 AM Christoph Hellwig <hch@lst.de> wrote:
> currently every architecture that wants to provide on of the common
> periphal busses needs to add some boilerplate code and include the
> right Kconfig files. This series instead just selects the presence
> (when needed) and then handles everything in the bus-specific
> Kconfig file under drivers/.
>
> Changes since v1:
> - rename all HAS_* Kconfig symbols to HAVE_*
> - drop the CONFIG_PCI_QSPAN option entirely
> - drop duplicate select from powerpc
> - restore missing selection of PCI_MSI for riscv
> - update x86 and riscv defconfigs to include PCI
> - actually inclue drivers/eisa/Kconfig
> - adjust some captilizations
Thanks for the update!
Please use "git format-patch -v<N> --cover" to prepare patch series
for sending with git-send-email.
"-v<N>" to prefix all patches with version number <N>,
"--cover" to have a "[PATCH 0/<M>]" prefix in the cover letter.
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* help
From: Lorenzo Chelini @ 2018-10-17 8:33 UTC (permalink / raw)
To: linuxppc-dev
Hi All,
I am a PhD at IBM Zurich. I am playing around with the new POWER9 servers.
I am interested in plotting a roofline model for a given application, but I
need
to measure the traffic to and from the memory.
Ideally, what I would like to measure is the traffic at the memory
controller level.
Do you know if this is possible using perf? If yes, which performance
counters I should
query to have an estimate on the memory traffic?
Thanks for your time.
Looking forward to hearing from you.
Best regards,
Lorenzo Chelini
^ permalink raw reply
* Re: help
From: Madhavan Srinivasan @ 2018-10-17 9:28 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <OF2E1784F7.5A472205-ONC1258329.002EF388-C1258329.002F029F@ch.ibm.com>
On Wednesday 17 October 2018 02:03 PM, Lorenzo Chelini wrote:
> Hi All,
>
> I am a PhD at IBM Zurich. I am playing around with the new POWER9 servers.
> I am interested in plotting a roofline model for a given application, but I
> need
> to measure the traffic to and from the memory.
> Ideally, what I would like to measure is the traffic at the memory
> controller level.
Yes, you can get that data using Nest IMC counters. And perf
has support to expose these counters. Here is a usage example
$ perf stat -e nest_mcs01_imc/PM_MCS01_128B_RD_DISP_PORT01/ -I 1000
--per-socket
Above command gets you all the Read traffic via specific memory
controller/port.
Option "-I" reads the counter every sec and "--per-socket" presents the
data socket wise.
"perf list nest_mcs" command will list all supportted events for memory
controller.
Maddy
>
> Do you know if this is possible using perf? If yes, which performance
> counters I should
> query to have an estimate on the memory traffic?
>
> Thanks for your time.
> Looking forward to hearing from you.
> Best regards,
> Lorenzo Chelini
>
^ permalink raw reply
* Re: Crash on FSL Book3E due to pte_pgprot()? (was Re: [PATCH v3 12/24] powerpc/mm: use pte helpers in generic code)
From: Aneesh Kumar K.V @ 2018-10-17 9:39 UTC (permalink / raw)
To: Christophe Leroy, Michael Ellerman, Bartlomiej Zolnierkiewicz,
Benjamin Herrenschmidt, Dominik Brodowski, Geoff Levand,
Jens Axboe, Kumar Gala, Li Yang, Nicholas Piggin, Paul Mackerras,
Scott Wood, aneesh.kumar
Cc: linux-fbdev, linux-kernel, dri-devel, linux-block, linuxppc-dev,
linux-arm-kernel
In-Reply-To: <236d72cd-e6d7-61e5-2c80-e4311e41b4f6@c-s.fr>
Christophe Leroy <christophe.leroy@c-s.fr> writes:
> On 10/17/2018 12:59 AM, Michael Ellerman wrote:
>> Christophe Leroy <christophe.leroy@c-s.fr> writes:
>>
>>> Get rid of platform specific _PAGE_XXXX in powerpc common code and
>>> use helpers instead.
>>>
>>> mm/dump_linuxpagetables.c will be handled separately
>>>
>>> Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>>> ---
>>> arch/powerpc/include/asm/book3s/32/pgtable.h | 9 +++------
>>> arch/powerpc/include/asm/nohash/32/pgtable.h | 12 ++++++++----
>>> arch/powerpc/include/asm/nohash/pgtable.h | 3 +--
>>> arch/powerpc/mm/pgtable.c | 21 +++++++--------------
>>> arch/powerpc/mm/pgtable_32.c | 15 ++++++++-------
>>> arch/powerpc/mm/pgtable_64.c | 14 +++++++-------
>>> arch/powerpc/xmon/xmon.c | 12 +++++++-----
>>> 7 files changed, 41 insertions(+), 45 deletions(-)
>>
>> So turns out this patch *also* breaks my p5020ds :)
>>
>> Even with patch 4 merged, see next.
>>
>> It's the same crash:
>>
>> pcieport 2000:00:00.0: AER enabled with IRQ 480
>> Unable to handle kernel paging request for data at address 0x8000080080080000
>> Faulting instruction address: 0xc0000000000192cc
>> Oops: Kernel access of bad area, sig: 11 [#1]
>> BE SMP NR_CPUS=24 CoreNet Generic
>> Modules linked in:
>> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.19.0-rc3-gcc7x-g98c847323b3a #1
>> NIP: c0000000000192cc LR: c0000000005d0f9c CTR: 0000000000100000
>> REGS: c0000000f31bb400 TRAP: 0300 Not tainted (4.19.0-rc3-gcc7x-g98c847323b3a)
>> MSR: 0000000080029000 <CE,EE,ME> CR: 24000224 XER: 00000000
>> DEAR: 8000080080080000 ESR: 0000000000800000 IRQMASK: 0
>> GPR00: c0000000005d0f84 c0000000f31bb688 c00000000117dc00 8000080080080000
>> GPR04: 0000000000000000 0000000000400000 00000ffbff241010 c0000000f31b8000
>> GPR08: 0000000000000000 0000000000100000 0000000000000000 c0000000012d4710
>> GPR12: 0000000084000422 c0000000012ff000 c000000000002774 0000000000000000
>> GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>> GPR20: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>> GPR24: 0000000000000000 0000000000000000 8000080080080000 c0000000ffff89a8
>> GPR28: c0000000f3576400 c0000000f3576410 0000000000400000 c0000000012ecc98
>> NIP [c0000000000192cc] ._memset_io+0x6c/0x9c
>> LR [c0000000005d0f9c] .fsl_qman_probe+0x198/0x928
>> Call Trace:
>> [c0000000f31bb688] [c0000000005d0f84] .fsl_qman_probe+0x180/0x928 (unreliable)
>> [c0000000f31bb728] [c0000000006432ec] .platform_drv_probe+0x60/0xb4
>> [c0000000f31bb7a8] [c00000000064083c] .really_probe+0x294/0x35c
>> [c0000000f31bb848] [c000000000640d2c] .__driver_attach+0x148/0x14c
>> [c0000000f31bb8d8] [c00000000063d7dc] .bus_for_each_dev+0xb0/0x118
>> [c0000000f31bb988] [c00000000063ff28] .driver_attach+0x34/0x4c
>> [c0000000f31bba08] [c00000000063f648] .bus_add_driver+0x174/0x2bc
>> [c0000000f31bbaa8] [c0000000006418bc] .driver_register+0x90/0x180
>> [c0000000f31bbb28] [c000000000643270] .__platform_driver_register+0x60/0x7c
>> [c0000000f31bbba8] [c000000000ee2a70] .fsl_qman_driver_init+0x24/0x38
>> [c0000000f31bbc18] [c0000000000023fc] .do_one_initcall+0x64/0x2b8
>> [c0000000f31bbcf8] [c000000000e9f480] .kernel_init_freeable+0x3a8/0x494
>> [c0000000f31bbda8] [c000000000002798] .kernel_init+0x24/0x148
>> [c0000000f31bbe28] [c0000000000009e8] .ret_from_kernel_thread+0x58/0x70
>> Instruction dump:
>> 4e800020 2ba50003 40dd003c 3925fffc 5488402e 7929f082 7d082378 39290001
>> 550a801e 7d2903a6 7d4a4378 794a0020 <91430000> 38630004 4200fff8 70a50003
>>
>>
>> Comparing a working vs broken kernel, it seems to boil down to the fact
>> that we're filtering out more PTE bits now that we use pte_pgprot() in
>> ioremap_prot().
>>
>> With the old code we get:
>> ioremap_prot: addr 0xff800000 flags 0x241215
>> ioremap_prot: addr 0xff800000 flags 0x241215
>> map_kernel_page: ea 0x8000080080080000 pa 0xff800000 pte 0xff800241215
>>
>>
>> And now we get:
>> ioremap_prot: addr 0xff800000 flags 0x241215 pte 0x241215
>> ioremap_prot: addr 0xff800000 pte 0x241215
>> ioremap_prot: addr 0xff800000 prot 0x241014
>> map_kernel_page: ea 0x8000080080080000 pa 0xff800000 pte 0xff800241014
>>
>> So we're losing 0x201, which for nohash book3e is:
>>
>> #define _PAGE_PRESENT 0x000001 /* software: pte contains a translation */
>> #define _PAGE_PSIZE_4K 0x000200
>>
>>
>> I haven't worked out if it's one or both of those that matter.
>
> At least missing _PAGE_PRESENT is an issue I believe.
>>
>> The question is what's the right way to fix it? Should pte_pgprot() not
>> be filtering those bits out on book3e?
>
> I think we should not use pte_pggrot() for that then. What about the
> below fix ?
>
> Christophe
>
> From: Christophe Leroy <christophe.leroy@c-s.fr>
> Date: Wed, 17 Oct 2018 05:56:25 +0000
> Subject: [PATCH] powerpc/mm: don't use pte_pgprot() in ioremap_prot()
>
> pte_pgprot() filters out some required flags like _PAGE_PRESENT.
>
> This patch replaces pte_pgprot() by __pgprot(pte_val())
> in ioremap_prot()
>
> Fixes: 26973fa5ac0e ("powerpc/mm: use pte helpers in generic code")
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
> arch/powerpc/mm/pgtable_32.c | 3 ++-
> arch/powerpc/mm/pgtable_64.c | 4 ++--
> 2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/mm/pgtable_32.c b/arch/powerpc/mm/pgtable_32.c
> index 5877f5aa8f5d..a606e2f4937b 100644
> --- a/arch/powerpc/mm/pgtable_32.c
> +++ b/arch/powerpc/mm/pgtable_32.c
> @@ -122,7 +122,8 @@ ioremap_prot(phys_addr_t addr, unsigned long size,
> unsigned long flags)
> pte = pte_exprotect(pte);
> pte = pte_mkprivileged(pte);
>
> - return __ioremap_caller(addr, size, pte_pgprot(pte),
> __builtin_return_address(0));
> + return __ioremap_caller(addr, size, __pgprot(pte_val(pte)),
> + __builtin_return_address(0));
That means we pass the pfn bits also to __ioremap_caller right? How about
From b4d5e0f24f8482375b2dd86afaced26ebf716600 Mon Sep 17 00:00:00 2001
From: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>
Date: Wed, 17 Oct 2018 14:07:50 +0530
Subject: [PATCH] powerpc/mm: Make pte_pgprot return all pte bits
Other archs do the same and instead of adding required pte bits (which got
masked out) in __ioremap_at, make sure we filter only pfn bits out.
Fixes: 26973fa5ac0e ("powerpc/mm: use pte helpers in generic code")
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
arch/powerpc/include/asm/book3s/32/pgtable.h | 6 ------
arch/powerpc/include/asm/book3s/64/pgtable.h | 8 --------
arch/powerpc/include/asm/nohash/32/pte-40x.h | 5 -----
arch/powerpc/include/asm/nohash/32/pte-44x.h | 5 -----
arch/powerpc/include/asm/nohash/32/pte-8xx.h | 5 -----
arch/powerpc/include/asm/nohash/32/pte-fsl-booke.h | 5 -----
arch/powerpc/include/asm/nohash/pgtable.h | 1 -
arch/powerpc/include/asm/nohash/pte-book3e.h | 5 -----
arch/powerpc/include/asm/pgtable.h | 10 ++++++++++
9 files changed, 10 insertions(+), 40 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/32/pgtable.h b/arch/powerpc/include/asm/book3s/32/pgtable.h
index 0fbd4c642b51..e61dd3ae5bc0 100644
--- a/arch/powerpc/include/asm/book3s/32/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/32/pgtable.h
@@ -48,11 +48,6 @@ static inline bool pte_user(pte_t pte)
#define _PAGE_CHG_MASK (PTE_RPN_MASK | _PAGE_HASHPTE | _PAGE_DIRTY | \
_PAGE_ACCESSED | _PAGE_SPECIAL)
-/* Mask of bits returned by pte_pgprot() */
-#define PAGE_PROT_BITS (_PAGE_GUARDED | _PAGE_COHERENT | _PAGE_NO_CACHE | \
- _PAGE_WRITETHRU | _PAGE_USER | _PAGE_ACCESSED | \
- _PAGE_RW | _PAGE_DIRTY)
-
/*
* We define 2 sets of base prot bits, one for basic pages (ie,
* cacheable kernel and user pages) and one for non cacheable
@@ -396,7 +391,6 @@ static inline int pte_young(pte_t pte) { return !!(pte_val(pte) & _PAGE_ACCESSE
static inline int pte_special(pte_t pte) { return !!(pte_val(pte) & _PAGE_SPECIAL); }
static inline int pte_none(pte_t pte) { return (pte_val(pte) & ~_PTE_NONE_MASK) == 0; }
static inline bool pte_exec(pte_t pte) { return true; }
-static inline pgprot_t pte_pgprot(pte_t pte) { return __pgprot(pte_val(pte) & PAGE_PROT_BITS); }
static inline int pte_present(pte_t pte)
{
diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
index c34a161dc651..cb5dd4078d42 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -128,13 +128,6 @@
#define H_PTE_PKEY (H_PTE_PKEY_BIT0 | H_PTE_PKEY_BIT1 | H_PTE_PKEY_BIT2 | \
H_PTE_PKEY_BIT3 | H_PTE_PKEY_BIT4)
-/*
- * Mask of bits returned by pte_pgprot()
- */
-#define PAGE_PROT_BITS (_PAGE_SAO | _PAGE_NON_IDEMPOTENT | _PAGE_TOLERANT | \
- H_PAGE_4K_PFN | _PAGE_PRIVILEGED | _PAGE_ACCESSED | \
- _PAGE_READ | _PAGE_WRITE | _PAGE_DIRTY | _PAGE_EXEC | \
- _PAGE_SOFT_DIRTY | H_PTE_PKEY)
/*
* We define 2 sets of base prot bits, one for basic pages (ie,
* cacheable kernel and user pages) and one for non cacheable
@@ -496,7 +489,6 @@ static inline bool pte_exec(pte_t pte)
return !!(pte_raw(pte) & cpu_to_be64(_PAGE_EXEC));
}
-static inline pgprot_t pte_pgprot(pte_t pte) { return __pgprot(pte_val(pte) & PAGE_PROT_BITS); }
#ifdef CONFIG_HAVE_ARCH_SOFT_DIRTY
static inline bool pte_soft_dirty(pte_t pte)
diff --git a/arch/powerpc/include/asm/nohash/32/pte-40x.h b/arch/powerpc/include/asm/nohash/32/pte-40x.h
index 7a8b3c94592f..661f4599f2fc 100644
--- a/arch/powerpc/include/asm/nohash/32/pte-40x.h
+++ b/arch/powerpc/include/asm/nohash/32/pte-40x.h
@@ -73,11 +73,6 @@
/* Until my rework is finished, 40x still needs atomic PTE updates */
#define PTE_ATOMIC_UPDATES 1
-/* Mask of bits returned by pte_pgprot() */
-#define PAGE_PROT_BITS (_PAGE_GUARDED | _PAGE_NO_CACHE | \
- _PAGE_WRITETHRU | _PAGE_USER | _PAGE_ACCESSED | \
- _PAGE_RW | _PAGE_HWWRITE | _PAGE_DIRTY | _PAGE_EXEC)
-
#define _PAGE_BASE_NC (_PAGE_PRESENT | _PAGE_ACCESSED)
#define _PAGE_BASE (_PAGE_BASE_NC)
diff --git a/arch/powerpc/include/asm/nohash/32/pte-44x.h b/arch/powerpc/include/asm/nohash/32/pte-44x.h
index 8d6b268a986f..78bc304f750e 100644
--- a/arch/powerpc/include/asm/nohash/32/pte-44x.h
+++ b/arch/powerpc/include/asm/nohash/32/pte-44x.h
@@ -93,11 +93,6 @@
#define _PAGE_KERNEL_RW (_PAGE_DIRTY | _PAGE_RW)
#define _PAGE_KERNEL_RWX (_PAGE_DIRTY | _PAGE_RW | _PAGE_EXEC)
-/* Mask of bits returned by pte_pgprot() */
-#define PAGE_PROT_BITS (_PAGE_GUARDED | _PAGE_COHERENT | _PAGE_NO_CACHE | \
- _PAGE_WRITETHRU | _PAGE_USER | _PAGE_ACCESSED | \
- _PAGE_RW | _PAGE_DIRTY | _PAGE_EXEC)
-
/* TODO: Add large page lowmem mapping support */
#define _PMD_PRESENT 0
#define _PMD_PRESENT_MASK (PAGE_MASK)
diff --git a/arch/powerpc/include/asm/nohash/32/pte-8xx.h b/arch/powerpc/include/asm/nohash/32/pte-8xx.h
index 1c57efac089d..6bfe041ef59d 100644
--- a/arch/powerpc/include/asm/nohash/32/pte-8xx.h
+++ b/arch/powerpc/include/asm/nohash/32/pte-8xx.h
@@ -55,11 +55,6 @@
#define _PAGE_KERNEL_RW (_PAGE_SH | _PAGE_DIRTY)
#define _PAGE_KERNEL_RWX (_PAGE_SH | _PAGE_DIRTY | _PAGE_EXEC)
-/* Mask of bits returned by pte_pgprot() */
-#define PAGE_PROT_BITS (_PAGE_GUARDED | _PAGE_NO_CACHE | \
- _PAGE_ACCESSED | _PAGE_RO | _PAGE_NA | \
- _PAGE_SH | _PAGE_DIRTY | _PAGE_EXEC)
-
#define _PMD_PRESENT 0x0001
#define _PMD_PRESENT_MASK _PMD_PRESENT
#define _PMD_BAD 0x0fd0
diff --git a/arch/powerpc/include/asm/nohash/32/pte-fsl-booke.h b/arch/powerpc/include/asm/nohash/32/pte-fsl-booke.h
index 1ecf60fe0909..0fc1bd42bb3e 100644
--- a/arch/powerpc/include/asm/nohash/32/pte-fsl-booke.h
+++ b/arch/powerpc/include/asm/nohash/32/pte-fsl-booke.h
@@ -39,11 +39,6 @@
/* No page size encoding in the linux PTE */
#define _PAGE_PSIZE 0
-/* Mask of bits returned by pte_pgprot() */
-#define PAGE_PROT_BITS (_PAGE_GUARDED | _PAGE_COHERENT | _PAGE_NO_CACHE | \
- _PAGE_WRITETHRU | _PAGE_USER | _PAGE_ACCESSED | \
- _PAGE_RW | _PAGE_DIRTY | _PAGE_EXEC)
-
#define _PMD_PRESENT 0
#define _PMD_PRESENT_MASK (PAGE_MASK)
#define _PMD_BAD (~PAGE_MASK)
diff --git a/arch/powerpc/include/asm/nohash/pgtable.h b/arch/powerpc/include/asm/nohash/pgtable.h
index 04e9f0922ad4..70ff23974b59 100644
--- a/arch/powerpc/include/asm/nohash/pgtable.h
+++ b/arch/powerpc/include/asm/nohash/pgtable.h
@@ -52,7 +52,6 @@ static inline int pte_none(pte_t pte) { return (pte_val(pte) & ~_PTE_NONE_MASK)
static inline bool pte_hashpte(pte_t pte) { return false; }
static inline bool pte_ci(pte_t pte) { return pte_val(pte) & _PAGE_NO_CACHE; }
static inline bool pte_exec(pte_t pte) { return pte_val(pte) & _PAGE_EXEC; }
-static inline pgprot_t pte_pgprot(pte_t pte) { return __pgprot(pte_val(pte) & PAGE_PROT_BITS); }
#ifdef CONFIG_NUMA_BALANCING
/*
diff --git a/arch/powerpc/include/asm/nohash/pte-book3e.h b/arch/powerpc/include/asm/nohash/pte-book3e.h
index 58eef8cb569d..f95ab6eaf441 100644
--- a/arch/powerpc/include/asm/nohash/pte-book3e.h
+++ b/arch/powerpc/include/asm/nohash/pte-book3e.h
@@ -82,11 +82,6 @@
#define _PTE_NONE_MASK 0
#endif
-/* Mask of bits returned by pte_pgprot() */
-#define PAGE_PROT_BITS (_PAGE_GUARDED | _PAGE_COHERENT | _PAGE_NO_CACHE | \
- _PAGE_WRITETHRU | _PAGE_USER | _PAGE_ACCESSED | \
- _PAGE_PRIVILEGED | _PAGE_RW | _PAGE_DIRTY | _PAGE_EXEC)
-
/*
* We define 2 sets of base prot bits, one for basic pages (ie,
* cacheable kernel and user pages) and one for non cacheable
diff --git a/arch/powerpc/include/asm/pgtable.h b/arch/powerpc/include/asm/pgtable.h
index fb4b85bba110..9679b7519a35 100644
--- a/arch/powerpc/include/asm/pgtable.h
+++ b/arch/powerpc/include/asm/pgtable.h
@@ -46,6 +46,16 @@ struct mm_struct;
/* Keep these as a macros to avoid include dependency mess */
#define pte_page(x) pfn_to_page(pte_pfn(x))
#define mk_pte(page, pgprot) pfn_pte(page_to_pfn(page), (pgprot))
+/*
+ * Select all bits except the pfn
+ */
+static inline pgprot_t pte_pgprot(pte_t pte)
+{
+ unsigned long pte_flags;
+
+ pte_flags = pte_val(pte) & ~PTE_RPN_MASK;
+ return __pgprot(pte_flags);
+}
/*
* ZERO_PAGE is a global shared page that is always zero: used
--
2.17.2
^ permalink raw reply related
* Re: Crash on FSL Book3E due to pte_pgprot()? (was Re: [PATCH v3 12/24] powerpc/mm: use pte helpers in generic code)
From: Christophe LEROY @ 2018-10-17 9:55 UTC (permalink / raw)
To: Aneesh Kumar K.V, Michael Ellerman, Bartlomiej Zolnierkiewicz,
Benjamin Herrenschmidt, Dominik Brodowski, Geoff Levand,
Jens Axboe, Kumar Gala, Li Yang, Nicholas Piggin, Paul Mackerras,
Scott Wood, aneesh.kumar
Cc: linux-fbdev, linux-kernel, dri-devel, linux-block, linuxppc-dev,
linux-arm-kernel
In-Reply-To: <87ftx4x67q.fsf@linux.ibm.com>
Le 17/10/2018 à 11:39, Aneesh Kumar K.V a écrit :
> Christophe Leroy <christophe.leroy@c-s.fr> writes:
>
>> On 10/17/2018 12:59 AM, Michael Ellerman wrote:
>>> Christophe Leroy <christophe.leroy@c-s.fr> writes:
>>>
>>>> Get rid of platform specific _PAGE_XXXX in powerpc common code and
>>>> use helpers instead.
>>>>
>>>> mm/dump_linuxpagetables.c will be handled separately
>>>>
>>>> Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>>>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>>>> ---
>>>> arch/powerpc/include/asm/book3s/32/pgtable.h | 9 +++------
>>>> arch/powerpc/include/asm/nohash/32/pgtable.h | 12 ++++++++----
>>>> arch/powerpc/include/asm/nohash/pgtable.h | 3 +--
>>>> arch/powerpc/mm/pgtable.c | 21 +++++++--------------
>>>> arch/powerpc/mm/pgtable_32.c | 15 ++++++++-------
>>>> arch/powerpc/mm/pgtable_64.c | 14 +++++++-------
>>>> arch/powerpc/xmon/xmon.c | 12 +++++++-----
>>>> 7 files changed, 41 insertions(+), 45 deletions(-)
>>>
>>> So turns out this patch *also* breaks my p5020ds :)
>>>
>>> Even with patch 4 merged, see next.
>>>
>>> It's the same crash:
>>>
>>> pcieport 2000:00:00.0: AER enabled with IRQ 480
>>> Unable to handle kernel paging request for data at address 0x8000080080080000
>>> Faulting instruction address: 0xc0000000000192cc
>>> Oops: Kernel access of bad area, sig: 11 [#1]
>>> BE SMP NR_CPUS=24 CoreNet Generic
>>> Modules linked in:
>>> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.19.0-rc3-gcc7x-g98c847323b3a #1
>>> NIP: c0000000000192cc LR: c0000000005d0f9c CTR: 0000000000100000
>>> REGS: c0000000f31bb400 TRAP: 0300 Not tainted (4.19.0-rc3-gcc7x-g98c847323b3a)
>>> MSR: 0000000080029000 <CE,EE,ME> CR: 24000224 XER: 00000000
>>> DEAR: 8000080080080000 ESR: 0000000000800000 IRQMASK: 0
>>> GPR00: c0000000005d0f84 c0000000f31bb688 c00000000117dc00 8000080080080000
>>> GPR04: 0000000000000000 0000000000400000 00000ffbff241010 c0000000f31b8000
>>> GPR08: 0000000000000000 0000000000100000 0000000000000000 c0000000012d4710
>>> GPR12: 0000000084000422 c0000000012ff000 c000000000002774 0000000000000000
>>> GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>>> GPR20: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>>> GPR24: 0000000000000000 0000000000000000 8000080080080000 c0000000ffff89a8
>>> GPR28: c0000000f3576400 c0000000f3576410 0000000000400000 c0000000012ecc98
>>> NIP [c0000000000192cc] ._memset_io+0x6c/0x9c
>>> LR [c0000000005d0f9c] .fsl_qman_probe+0x198/0x928
>>> Call Trace:
>>> [c0000000f31bb688] [c0000000005d0f84] .fsl_qman_probe+0x180/0x928 (unreliable)
>>> [c0000000f31bb728] [c0000000006432ec] .platform_drv_probe+0x60/0xb4
>>> [c0000000f31bb7a8] [c00000000064083c] .really_probe+0x294/0x35c
>>> [c0000000f31bb848] [c000000000640d2c] .__driver_attach+0x148/0x14c
>>> [c0000000f31bb8d8] [c00000000063d7dc] .bus_for_each_dev+0xb0/0x118
>>> [c0000000f31bb988] [c00000000063ff28] .driver_attach+0x34/0x4c
>>> [c0000000f31bba08] [c00000000063f648] .bus_add_driver+0x174/0x2bc
>>> [c0000000f31bbaa8] [c0000000006418bc] .driver_register+0x90/0x180
>>> [c0000000f31bbb28] [c000000000643270] .__platform_driver_register+0x60/0x7c
>>> [c0000000f31bbba8] [c000000000ee2a70] .fsl_qman_driver_init+0x24/0x38
>>> [c0000000f31bbc18] [c0000000000023fc] .do_one_initcall+0x64/0x2b8
>>> [c0000000f31bbcf8] [c000000000e9f480] .kernel_init_freeable+0x3a8/0x494
>>> [c0000000f31bbda8] [c000000000002798] .kernel_init+0x24/0x148
>>> [c0000000f31bbe28] [c0000000000009e8] .ret_from_kernel_thread+0x58/0x70
>>> Instruction dump:
>>> 4e800020 2ba50003 40dd003c 3925fffc 5488402e 7929f082 7d082378 39290001
>>> 550a801e 7d2903a6 7d4a4378 794a0020 <91430000> 38630004 4200fff8 70a50003
>>>
>>>
>>> Comparing a working vs broken kernel, it seems to boil down to the fact
>>> that we're filtering out more PTE bits now that we use pte_pgprot() in
>>> ioremap_prot().
>>>
>>> With the old code we get:
>>> ioremap_prot: addr 0xff800000 flags 0x241215
>>> ioremap_prot: addr 0xff800000 flags 0x241215
>>> map_kernel_page: ea 0x8000080080080000 pa 0xff800000 pte 0xff800241215
>>>
>>>
>>> And now we get:
>>> ioremap_prot: addr 0xff800000 flags 0x241215 pte 0x241215
>>> ioremap_prot: addr 0xff800000 pte 0x241215
>>> ioremap_prot: addr 0xff800000 prot 0x241014
>>> map_kernel_page: ea 0x8000080080080000 pa 0xff800000 pte 0xff800241014
>>>
>>> So we're losing 0x201, which for nohash book3e is:
>>>
>>> #define _PAGE_PRESENT 0x000001 /* software: pte contains a translation */
>>> #define _PAGE_PSIZE_4K 0x000200
>>>
>>>
>>> I haven't worked out if it's one or both of those that matter.
>>
>> At least missing _PAGE_PRESENT is an issue I believe.
>>>
>>> The question is what's the right way to fix it? Should pte_pgprot() not
>>> be filtering those bits out on book3e?
>>
>> I think we should not use pte_pggrot() for that then. What about the
>> below fix ?
>>
>> Christophe
>>
>> From: Christophe Leroy <christophe.leroy@c-s.fr>
>> Date: Wed, 17 Oct 2018 05:56:25 +0000
>> Subject: [PATCH] powerpc/mm: don't use pte_pgprot() in ioremap_prot()
>>
>> pte_pgprot() filters out some required flags like _PAGE_PRESENT.
>>
>> This patch replaces pte_pgprot() by __pgprot(pte_val())
>> in ioremap_prot()
>>
>> Fixes: 26973fa5ac0e ("powerpc/mm: use pte helpers in generic code")
>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>> ---
>> arch/powerpc/mm/pgtable_32.c | 3 ++-
>> arch/powerpc/mm/pgtable_64.c | 4 ++--
>> 2 files changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/powerpc/mm/pgtable_32.c b/arch/powerpc/mm/pgtable_32.c
>> index 5877f5aa8f5d..a606e2f4937b 100644
>> --- a/arch/powerpc/mm/pgtable_32.c
>> +++ b/arch/powerpc/mm/pgtable_32.c
>> @@ -122,7 +122,8 @@ ioremap_prot(phys_addr_t addr, unsigned long size,
>> unsigned long flags)
>> pte = pte_exprotect(pte);
>> pte = pte_mkprivileged(pte);
>>
>> - return __ioremap_caller(addr, size, pte_pgprot(pte),
>> __builtin_return_address(0));
>> + return __ioremap_caller(addr, size, __pgprot(pte_val(pte)),
>> + __builtin_return_address(0));
>
>
> That means we pass the pfn bits also to __ioremap_caller right? How about
>
> From b4d5e0f24f8482375b2dd86afaced26ebf716600 Mon Sep 17 00:00:00 2001
> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>
> Date: Wed, 17 Oct 2018 14:07:50 +0530
> Subject: [PATCH] powerpc/mm: Make pte_pgprot return all pte bits
>
> Other archs do the same and instead of adding required pte bits (which got
> masked out) in __ioremap_at, make sure we filter only pfn bits out.
>
> Fixes: 26973fa5ac0e ("powerpc/mm: use pte helpers in generic code")
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Looks good for me.
Reviewed-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
> arch/powerpc/include/asm/book3s/32/pgtable.h | 6 ------
> arch/powerpc/include/asm/book3s/64/pgtable.h | 8 --------
> arch/powerpc/include/asm/nohash/32/pte-40x.h | 5 -----
> arch/powerpc/include/asm/nohash/32/pte-44x.h | 5 -----
> arch/powerpc/include/asm/nohash/32/pte-8xx.h | 5 -----
> arch/powerpc/include/asm/nohash/32/pte-fsl-booke.h | 5 -----
> arch/powerpc/include/asm/nohash/pgtable.h | 1 -
> arch/powerpc/include/asm/nohash/pte-book3e.h | 5 -----
> arch/powerpc/include/asm/pgtable.h | 10 ++++++++++
> 9 files changed, 10 insertions(+), 40 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/book3s/32/pgtable.h b/arch/powerpc/include/asm/book3s/32/pgtable.h
> index 0fbd4c642b51..e61dd3ae5bc0 100644
> --- a/arch/powerpc/include/asm/book3s/32/pgtable.h
> +++ b/arch/powerpc/include/asm/book3s/32/pgtable.h
> @@ -48,11 +48,6 @@ static inline bool pte_user(pte_t pte)
> #define _PAGE_CHG_MASK (PTE_RPN_MASK | _PAGE_HASHPTE | _PAGE_DIRTY | \
> _PAGE_ACCESSED | _PAGE_SPECIAL)
>
> -/* Mask of bits returned by pte_pgprot() */
> -#define PAGE_PROT_BITS (_PAGE_GUARDED | _PAGE_COHERENT | _PAGE_NO_CACHE | \
> - _PAGE_WRITETHRU | _PAGE_USER | _PAGE_ACCESSED | \
> - _PAGE_RW | _PAGE_DIRTY)
> -
> /*
> * We define 2 sets of base prot bits, one for basic pages (ie,
> * cacheable kernel and user pages) and one for non cacheable
> @@ -396,7 +391,6 @@ static inline int pte_young(pte_t pte) { return !!(pte_val(pte) & _PAGE_ACCESSE
> static inline int pte_special(pte_t pte) { return !!(pte_val(pte) & _PAGE_SPECIAL); }
> static inline int pte_none(pte_t pte) { return (pte_val(pte) & ~_PTE_NONE_MASK) == 0; }
> static inline bool pte_exec(pte_t pte) { return true; }
> -static inline pgprot_t pte_pgprot(pte_t pte) { return __pgprot(pte_val(pte) & PAGE_PROT_BITS); }
>
> static inline int pte_present(pte_t pte)
> {
> diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
> index c34a161dc651..cb5dd4078d42 100644
> --- a/arch/powerpc/include/asm/book3s/64/pgtable.h
> +++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
> @@ -128,13 +128,6 @@
>
> #define H_PTE_PKEY (H_PTE_PKEY_BIT0 | H_PTE_PKEY_BIT1 | H_PTE_PKEY_BIT2 | \
> H_PTE_PKEY_BIT3 | H_PTE_PKEY_BIT4)
> -/*
> - * Mask of bits returned by pte_pgprot()
> - */
> -#define PAGE_PROT_BITS (_PAGE_SAO | _PAGE_NON_IDEMPOTENT | _PAGE_TOLERANT | \
> - H_PAGE_4K_PFN | _PAGE_PRIVILEGED | _PAGE_ACCESSED | \
> - _PAGE_READ | _PAGE_WRITE | _PAGE_DIRTY | _PAGE_EXEC | \
> - _PAGE_SOFT_DIRTY | H_PTE_PKEY)
> /*
> * We define 2 sets of base prot bits, one for basic pages (ie,
> * cacheable kernel and user pages) and one for non cacheable
> @@ -496,7 +489,6 @@ static inline bool pte_exec(pte_t pte)
> return !!(pte_raw(pte) & cpu_to_be64(_PAGE_EXEC));
> }
>
> -static inline pgprot_t pte_pgprot(pte_t pte) { return __pgprot(pte_val(pte) & PAGE_PROT_BITS); }
>
> #ifdef CONFIG_HAVE_ARCH_SOFT_DIRTY
> static inline bool pte_soft_dirty(pte_t pte)
> diff --git a/arch/powerpc/include/asm/nohash/32/pte-40x.h b/arch/powerpc/include/asm/nohash/32/pte-40x.h
> index 7a8b3c94592f..661f4599f2fc 100644
> --- a/arch/powerpc/include/asm/nohash/32/pte-40x.h
> +++ b/arch/powerpc/include/asm/nohash/32/pte-40x.h
> @@ -73,11 +73,6 @@
> /* Until my rework is finished, 40x still needs atomic PTE updates */
> #define PTE_ATOMIC_UPDATES 1
>
> -/* Mask of bits returned by pte_pgprot() */
> -#define PAGE_PROT_BITS (_PAGE_GUARDED | _PAGE_NO_CACHE | \
> - _PAGE_WRITETHRU | _PAGE_USER | _PAGE_ACCESSED | \
> - _PAGE_RW | _PAGE_HWWRITE | _PAGE_DIRTY | _PAGE_EXEC)
> -
> #define _PAGE_BASE_NC (_PAGE_PRESENT | _PAGE_ACCESSED)
> #define _PAGE_BASE (_PAGE_BASE_NC)
>
> diff --git a/arch/powerpc/include/asm/nohash/32/pte-44x.h b/arch/powerpc/include/asm/nohash/32/pte-44x.h
> index 8d6b268a986f..78bc304f750e 100644
> --- a/arch/powerpc/include/asm/nohash/32/pte-44x.h
> +++ b/arch/powerpc/include/asm/nohash/32/pte-44x.h
> @@ -93,11 +93,6 @@
> #define _PAGE_KERNEL_RW (_PAGE_DIRTY | _PAGE_RW)
> #define _PAGE_KERNEL_RWX (_PAGE_DIRTY | _PAGE_RW | _PAGE_EXEC)
>
> -/* Mask of bits returned by pte_pgprot() */
> -#define PAGE_PROT_BITS (_PAGE_GUARDED | _PAGE_COHERENT | _PAGE_NO_CACHE | \
> - _PAGE_WRITETHRU | _PAGE_USER | _PAGE_ACCESSED | \
> - _PAGE_RW | _PAGE_DIRTY | _PAGE_EXEC)
> -
> /* TODO: Add large page lowmem mapping support */
> #define _PMD_PRESENT 0
> #define _PMD_PRESENT_MASK (PAGE_MASK)
> diff --git a/arch/powerpc/include/asm/nohash/32/pte-8xx.h b/arch/powerpc/include/asm/nohash/32/pte-8xx.h
> index 1c57efac089d..6bfe041ef59d 100644
> --- a/arch/powerpc/include/asm/nohash/32/pte-8xx.h
> +++ b/arch/powerpc/include/asm/nohash/32/pte-8xx.h
> @@ -55,11 +55,6 @@
> #define _PAGE_KERNEL_RW (_PAGE_SH | _PAGE_DIRTY)
> #define _PAGE_KERNEL_RWX (_PAGE_SH | _PAGE_DIRTY | _PAGE_EXEC)
>
> -/* Mask of bits returned by pte_pgprot() */
> -#define PAGE_PROT_BITS (_PAGE_GUARDED | _PAGE_NO_CACHE | \
> - _PAGE_ACCESSED | _PAGE_RO | _PAGE_NA | \
> - _PAGE_SH | _PAGE_DIRTY | _PAGE_EXEC)
> -
> #define _PMD_PRESENT 0x0001
> #define _PMD_PRESENT_MASK _PMD_PRESENT
> #define _PMD_BAD 0x0fd0
> diff --git a/arch/powerpc/include/asm/nohash/32/pte-fsl-booke.h b/arch/powerpc/include/asm/nohash/32/pte-fsl-booke.h
> index 1ecf60fe0909..0fc1bd42bb3e 100644
> --- a/arch/powerpc/include/asm/nohash/32/pte-fsl-booke.h
> +++ b/arch/powerpc/include/asm/nohash/32/pte-fsl-booke.h
> @@ -39,11 +39,6 @@
> /* No page size encoding in the linux PTE */
> #define _PAGE_PSIZE 0
>
> -/* Mask of bits returned by pte_pgprot() */
> -#define PAGE_PROT_BITS (_PAGE_GUARDED | _PAGE_COHERENT | _PAGE_NO_CACHE | \
> - _PAGE_WRITETHRU | _PAGE_USER | _PAGE_ACCESSED | \
> - _PAGE_RW | _PAGE_DIRTY | _PAGE_EXEC)
> -
> #define _PMD_PRESENT 0
> #define _PMD_PRESENT_MASK (PAGE_MASK)
> #define _PMD_BAD (~PAGE_MASK)
> diff --git a/arch/powerpc/include/asm/nohash/pgtable.h b/arch/powerpc/include/asm/nohash/pgtable.h
> index 04e9f0922ad4..70ff23974b59 100644
> --- a/arch/powerpc/include/asm/nohash/pgtable.h
> +++ b/arch/powerpc/include/asm/nohash/pgtable.h
> @@ -52,7 +52,6 @@ static inline int pte_none(pte_t pte) { return (pte_val(pte) & ~_PTE_NONE_MASK)
> static inline bool pte_hashpte(pte_t pte) { return false; }
> static inline bool pte_ci(pte_t pte) { return pte_val(pte) & _PAGE_NO_CACHE; }
> static inline bool pte_exec(pte_t pte) { return pte_val(pte) & _PAGE_EXEC; }
> -static inline pgprot_t pte_pgprot(pte_t pte) { return __pgprot(pte_val(pte) & PAGE_PROT_BITS); }
>
> #ifdef CONFIG_NUMA_BALANCING
> /*
> diff --git a/arch/powerpc/include/asm/nohash/pte-book3e.h b/arch/powerpc/include/asm/nohash/pte-book3e.h
> index 58eef8cb569d..f95ab6eaf441 100644
> --- a/arch/powerpc/include/asm/nohash/pte-book3e.h
> +++ b/arch/powerpc/include/asm/nohash/pte-book3e.h
> @@ -82,11 +82,6 @@
> #define _PTE_NONE_MASK 0
> #endif
>
> -/* Mask of bits returned by pte_pgprot() */
> -#define PAGE_PROT_BITS (_PAGE_GUARDED | _PAGE_COHERENT | _PAGE_NO_CACHE | \
> - _PAGE_WRITETHRU | _PAGE_USER | _PAGE_ACCESSED | \
> - _PAGE_PRIVILEGED | _PAGE_RW | _PAGE_DIRTY | _PAGE_EXEC)
> -
> /*
> * We define 2 sets of base prot bits, one for basic pages (ie,
> * cacheable kernel and user pages) and one for non cacheable
> diff --git a/arch/powerpc/include/asm/pgtable.h b/arch/powerpc/include/asm/pgtable.h
> index fb4b85bba110..9679b7519a35 100644
> --- a/arch/powerpc/include/asm/pgtable.h
> +++ b/arch/powerpc/include/asm/pgtable.h
> @@ -46,6 +46,16 @@ struct mm_struct;
> /* Keep these as a macros to avoid include dependency mess */
> #define pte_page(x) pfn_to_page(pte_pfn(x))
> #define mk_pte(page, pgprot) pfn_pte(page_to_pfn(page), (pgprot))
> +/*
> + * Select all bits except the pfn
> + */
> +static inline pgprot_t pte_pgprot(pte_t pte)
> +{
> + unsigned long pte_flags;
> +
> + pte_flags = pte_val(pte) & ~PTE_RPN_MASK;
> + return __pgprot(pte_flags);
> +}
>
> /*
> * ZERO_PAGE is a global shared page that is always zero: used
>
^ permalink raw reply
* Re: [PATCH 2/4] mm: speed up mremap by 500x on large regions (v2)
From: Vlastimil Babka @ 2018-10-17 7:38 UTC (permalink / raw)
To: Joel Fernandes
Cc: linux-mips, Rich Felker, linux-ia64, linux-sh, Peter Zijlstra,
Catalin Marinas, Dave Hansen, Will Deacon, mhocko, linux-mm,
lokeshgidra, sparclinux, linux-riscv, kvmarm, Jonas Bonn,
linux-s390, dancol, Yoshinori Sato, Max Filippov, linux-hexagon,
Helge Deller, maintainer:X86 ARCHITECTURE 32-BIT AND 64-BIT,
hughd, James E.J. Bottomley, kasan-dev, elfring,
Christoph Hellwig, Ingo Molnar, Geert Uytterhoeven,
Andrey Ryabinin, linux-snps-arc, kernel-team, Sam Creasey,
linux-xtensa, Jeff Dike, linux-alpha, linux-um,
Stefan Kristiansson, Julia Lawall, linux-m68k, Borislav Petkov,
Andy Lutomirski, Ley Foon Tan, kirill, Stafford Horne,
Guan Xuetao, Chris Zankel, Tony Luck, linux-parisc, pantin,
linux-kernel, Fenghua Yu, minchan, Thomas Gleixner,
Richard Weinberger, anton.ivanov, nios2-dev, akpm, linuxppc-dev,
David S. Miller
In-Reply-To: <20181016194313.GA247930@joelaf.mtv.corp.google.com>
On 10/16/18 9:43 PM, Joel Fernandes wrote:
> On Tue, Oct 16, 2018 at 01:29:52PM +0200, Vlastimil Babka wrote:
>> On 10/16/18 12:33 AM, Joel Fernandes wrote:
>>> On Mon, Oct 15, 2018 at 02:42:09AM -0700, Christoph Hellwig wrote:
>>>> On Fri, Oct 12, 2018 at 06:31:58PM -0700, Joel Fernandes (Google) wrote:
>>>>> Android needs to mremap large regions of memory during memory management
>>>>> related operations.
>>>>
>>>> Just curious: why?
>>>
>>> In Android we have a requirement of moving a large (up to a GB now, but may
>>> grow bigger in future) memory range from one location to another.
>>
>> I think Christoph's "why?" was about the requirement, not why it hurts
>> applications. I admit I'm now also curious :)
>
> This issue was discovered when we wanted to be able to move the physical
> pages of a memory range to another location quickly so that, after the
> application threads are resumed, UFFDIO_REGISTER_MODE_MISSING userfaultfd
> faults can be received on the original memory range. The actual operations
> performed on the memory range are beyond the scope of this discussion. The
> user threads continue to refer to the old address which will now fault. The
> reason we want retain the old memory range and receives faults there is to
> avoid the need to fix the addresses all over the address space of the threads
> after we finish with performing operations on them in the fault handlers, so
> we mremap it and receive faults at the old addresses.
>
> Does that answer your question?
Yes, interesting, thanks!
Vlastimil
> thanks,
>
> - Joel
>
^ permalink raw reply
* Re: [PATCH 1/5] powerpc/64s: Kernel Hypervisor Restricted Access Prevention
From: kbuild test robot @ 2018-10-17 10:07 UTC (permalink / raw)
To: Russell Currey; +Cc: mikey, linuxppc-dev, kbuild-all
In-Reply-To: <20181017064422.26119-1-ruscur@russell.cc>
[-- Attachment #1: Type: text/plain, Size: 6120 bytes --]
Hi Russell,
I love your patch! Yet something to improve:
[auto build test ERROR on powerpc/next]
[also build test ERROR on next-20181016]
[cannot apply to v4.19-rc8]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Russell-Currey/powerpc-64s-Kernel-Hypervisor-Restricted-Access-Prevention/20181017-153543
base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-storcenter_defconfig (attached as .config)
compiler: powerpc-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=powerpc
All errors (new ones prefixed by >>):
In file included from include/linux/uaccess.h:14:0,
from net/core/datagram.c:40:
arch/powerpc/include/asm/uaccess.h: In function 'unlock_user_access':
>> arch/powerpc/include/asm/uaccess.h:69:6: error: implicit declaration of function 'mmu_has_feature'; did you mean 'firmware_has_feature'? [-Werror=implicit-function-declaration]
if (mmu_has_feature(MMU_FTR_RADIX_KHRAP)) {
^~~~~~~~~~~~~~~
firmware_has_feature
>> arch/powerpc/include/asm/uaccess.h:69:22: error: 'MMU_FTR_RADIX_KHRAP' undeclared (first use in this function); did you mean 'CPU_FTR_CAN_NAP'?
if (mmu_has_feature(MMU_FTR_RADIX_KHRAP)) {
^~~~~~~~~~~~~~~~~~~
CPU_FTR_CAN_NAP
arch/powerpc/include/asm/uaccess.h:69:22: note: each undeclared identifier is reported only once for each function it appears in
arch/powerpc/include/asm/uaccess.h: In function 'lock_user_access':
arch/powerpc/include/asm/uaccess.h:83:22: error: 'MMU_FTR_RADIX_KHRAP' undeclared (first use in this function); did you mean 'CPU_FTR_CAN_NAP'?
if (mmu_has_feature(MMU_FTR_RADIX_KHRAP)) {
^~~~~~~~~~~~~~~~~~~
CPU_FTR_CAN_NAP
In file included from include/linux/mm_types.h:18:0,
from include/linux/mm.h:17,
from net/core/datagram.c:41:
arch/powerpc/include/asm/mmu.h: At top level:
>> arch/powerpc/include/asm/mmu.h:209:20: error: conflicting types for 'mmu_has_feature'
static inline bool mmu_has_feature(unsigned long feature)
^~~~~~~~~~~~~~~
In file included from include/linux/uaccess.h:14:0,
from net/core/datagram.c:40:
arch/powerpc/include/asm/uaccess.h:69:6: note: previous implicit declaration of 'mmu_has_feature' was here
if (mmu_has_feature(MMU_FTR_RADIX_KHRAP)) {
^~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
--
In file included from include/linux/uaccess.h:14:0,
from include/linux/crypto.h:26,
from include/crypto/skcipher.h:16,
from include/crypto/chacha20.h:9,
from lib/chacha20.c:17:
arch/powerpc/include/asm/uaccess.h: In function 'unlock_user_access':
>> arch/powerpc/include/asm/uaccess.h:69:6: error: implicit declaration of function 'mmu_has_feature'; did you mean 'firmware_has_feature'? [-Werror=implicit-function-declaration]
if (mmu_has_feature(MMU_FTR_RADIX_KHRAP)) {
^~~~~~~~~~~~~~~
firmware_has_feature
>> arch/powerpc/include/asm/uaccess.h:69:22: error: 'MMU_FTR_RADIX_KHRAP' undeclared (first use in this function); did you mean 'CPU_FTR_CAN_NAP'?
if (mmu_has_feature(MMU_FTR_RADIX_KHRAP)) {
^~~~~~~~~~~~~~~~~~~
CPU_FTR_CAN_NAP
arch/powerpc/include/asm/uaccess.h:69:22: note: each undeclared identifier is reported only once for each function it appears in
arch/powerpc/include/asm/uaccess.h: In function 'lock_user_access':
arch/powerpc/include/asm/uaccess.h:83:22: error: 'MMU_FTR_RADIX_KHRAP' undeclared (first use in this function); did you mean 'CPU_FTR_CAN_NAP'?
if (mmu_has_feature(MMU_FTR_RADIX_KHRAP)) {
^~~~~~~~~~~~~~~~~~~
CPU_FTR_CAN_NAP
cc1: some warnings being treated as errors
--
In file included from include/linux/uaccess.h:14:0,
from arch/powerpc/kernel/module.c:25:
arch/powerpc/include/asm/uaccess.h: In function 'unlock_user_access':
>> arch/powerpc/include/asm/uaccess.h:69:6: error: implicit declaration of function 'mmu_has_feature'; did you mean 'firmware_has_feature'? [-Werror=implicit-function-declaration]
if (mmu_has_feature(MMU_FTR_RADIX_KHRAP)) {
^~~~~~~~~~~~~~~
firmware_has_feature
>> arch/powerpc/include/asm/uaccess.h:69:22: error: 'MMU_FTR_RADIX_KHRAP' undeclared (first use in this function); did you mean 'CPU_FTR_CAN_NAP'?
if (mmu_has_feature(MMU_FTR_RADIX_KHRAP)) {
^~~~~~~~~~~~~~~~~~~
CPU_FTR_CAN_NAP
arch/powerpc/include/asm/uaccess.h:69:22: note: each undeclared identifier is reported only once for each function it appears in
arch/powerpc/include/asm/uaccess.h: In function 'lock_user_access':
arch/powerpc/include/asm/uaccess.h:83:22: error: 'MMU_FTR_RADIX_KHRAP' undeclared (first use in this function); did you mean 'CPU_FTR_CAN_NAP'?
if (mmu_has_feature(MMU_FTR_RADIX_KHRAP)) {
^~~~~~~~~~~~~~~~~~~
CPU_FTR_CAN_NAP
cc1: all warnings being treated as errors
vim +69 arch/powerpc/include/asm/uaccess.h
64
65 static inline unsigned long unlock_user_access(void)
66 {
67 unsigned long amr;
68
> 69 if (mmu_has_feature(MMU_FTR_RADIX_KHRAP)) {
70 amr = mfspr(SPRN_AMR);
71
72 isync();
73 mtspr(SPRN_AMR, 0);
74 isync();
75 return amr;
76 }
77
78 return 0;
79 }
80
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 15031 bytes --]
^ permalink raw reply
* Re: Crash on FSL Book3E due to pte_pgprot()? (was Re: [PATCH v3 12/24] powerpc/mm: use pte helpers in generic code)
From: Michael Ellerman @ 2018-10-17 10:32 UTC (permalink / raw)
To: Christophe Leroy, Bartlomiej Zolnierkiewicz,
Benjamin Herrenschmidt, Dominik Brodowski, Geoff Levand,
Jens Axboe, Kumar Gala, Li Yang, Nicholas Piggin, Paul Mackerras,
Scott Wood, aneesh.kumar
Cc: linux-fbdev, linux-kernel, dri-devel, linux-block, linuxppc-dev,
linux-arm-kernel
In-Reply-To: <236d72cd-e6d7-61e5-2c80-e4311e41b4f6@c-s.fr>
Christophe Leroy <christophe.leroy@c-s.fr> writes:
> On 10/17/2018 12:59 AM, Michael Ellerman wrote:
...
>> The question is what's the right way to fix it? Should pte_pgprot() not
>> be filtering those bits out on book3e?
>
> I think we should not use pte_pggrot() for that then. What about the
> below fix ?
Thanks, that almost works.
pte_mkprivileged() also needs to not strip _PAGE_BAP_SR.
But there's also a use of pte_pgprot() in mm/memory.c, and I think that
is also broken now that we don't add PAGE_KERNEL back in.
Aneesh is going to do a patch to make pte_pgprot() only mask the PFN
which is what other arches do.
cheers
> From: Christophe Leroy <christophe.leroy@c-s.fr>
> Date: Wed, 17 Oct 2018 05:56:25 +0000
> Subject: [PATCH] powerpc/mm: don't use pte_pgprot() in ioremap_prot()
>
> pte_pgprot() filters out some required flags like _PAGE_PRESENT.
>
> This patch replaces pte_pgprot() by __pgprot(pte_val())
> in ioremap_prot()
>
> Fixes: 26973fa5ac0e ("powerpc/mm: use pte helpers in generic code")
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
> arch/powerpc/mm/pgtable_32.c | 3 ++-
> arch/powerpc/mm/pgtable_64.c | 4 ++--
> 2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/mm/pgtable_32.c b/arch/powerpc/mm/pgtable_32.c
> index 5877f5aa8f5d..a606e2f4937b 100644
> --- a/arch/powerpc/mm/pgtable_32.c
> +++ b/arch/powerpc/mm/pgtable_32.c
> @@ -122,7 +122,8 @@ ioremap_prot(phys_addr_t addr, unsigned long size,
> unsigned long flags)
> pte = pte_exprotect(pte);
> pte = pte_mkprivileged(pte);
>
> - return __ioremap_caller(addr, size, pte_pgprot(pte),
> __builtin_return_address(0));
> + return __ioremap_caller(addr, size, __pgprot(pte_val(pte)),
> + __builtin_return_address(0));
> }
> EXPORT_SYMBOL(ioremap_prot);
>
> diff --git a/arch/powerpc/mm/pgtable_64.c b/arch/powerpc/mm/pgtable_64.c
> index fb1375c07e8c..836bf436cabb 100644
> --- a/arch/powerpc/mm/pgtable_64.c
> +++ b/arch/powerpc/mm/pgtable_64.c
> @@ -245,8 +245,8 @@ void __iomem * ioremap_prot(phys_addr_t addr,
> unsigned long size,
> pte = pte_mkprivileged(pte);
>
> if (ppc_md.ioremap)
> - return ppc_md.ioremap(addr, size, pte_pgprot(pte), caller);
> - return __ioremap_caller(addr, size, pte_pgprot(pte), caller);
> + return ppc_md.ioremap(addr, size, __pgprot(pte_val(pte)), caller);
> + return __ioremap_caller(addr, size, __pgprot(pte_val(pte)), caller);
> }
>
>
> --
> 2.13.3
^ permalink raw reply
* Re: Crash on FSL Book3E due to pte_pgprot()? (was Re: [PATCH v3 12/24] powerpc/mm: use pte helpers in generic code)
From: Christophe Leroy @ 2018-10-17 11:12 UTC (permalink / raw)
To: Michael Ellerman, Bartlomiej Zolnierkiewicz,
Benjamin Herrenschmidt, Dominik Brodowski, Geoff Levand,
Jens Axboe, Kumar Gala, Li Yang, Nicholas Piggin, Paul Mackerras,
Scott Wood, aneesh.kumar
Cc: linux-fbdev, linux-kernel, dri-devel, linux-block, linuxppc-dev,
linux-arm-kernel
In-Reply-To: <87efcoeudx.fsf@concordia.ellerman.id.au>
On 10/17/2018 10:32 AM, Michael Ellerman wrote:
> Christophe Leroy <christophe.leroy@c-s.fr> writes:
>> On 10/17/2018 12:59 AM, Michael Ellerman wrote:
> ...
>>> The question is what's the right way to fix it? Should pte_pgprot() not
>>> be filtering those bits out on book3e?
>>
>> I think we should not use pte_pggrot() for that then. What about the
>> below fix ?
>
> Thanks, that almost works.
>
> pte_mkprivileged() also needs to not strip _PAGE_BAP_SR.
Oops, I missed it allthough I knew it. Patch below.
From: Christophe Leroy <christophe.leroy@c-s.fr>
Date: Wed, 17 Oct 2018 10:46:24 +0000
Subject: [PATCH] powerpc/book3e: redefine pte_mkprivileged() and
pte_mkuser()
To: Benjamin Herrenschmidt <benh@kernel.crashing.org>, Paul Mackerras
<paulus@samba.org>, Michael Ellerman <mpe@ellerman.id.au>
Cc: linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org
Book3e defines both _PAGE_USER and _PAGE_PRIVILEGED, so the nohash
default pte_mkprivileged() and pte_mkuser() are not usable.
This patch redefines them for book3e.
Fixes: a0da4bc166f2 ("powerpc/mm: Allow platforms to redefine some helpers")
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/include/asm/nohash/pte-book3e.h | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/arch/powerpc/include/asm/nohash/pte-book3e.h
b/arch/powerpc/include/asm/nohash/pte-book3e.h
index 58eef8cb569d..fb4297dff3e2 100644
--- a/arch/powerpc/include/asm/nohash/pte-book3e.h
+++ b/arch/powerpc/include/asm/nohash/pte-book3e.h
@@ -109,5 +109,19 @@
#define PAGE_READONLY __pgprot(_PAGE_BASE | _PAGE_USER)
#define PAGE_READONLY_X __pgprot(_PAGE_BASE | _PAGE_USER | _PAGE_EXEC)
+static inline pte_t pte_mkprivileged(pte_t pte)
+{
+ return __pte((pte_val(pte) & ~_PAGE_USER) | _PAGE_PRIVILEGED);
+}
+
+#define pte_mkprivileged pte_mkprivileged
+
+static inline pte_t pte_mkuser(pte_t pte)
+{
+ return __pte((pte_val(pte) & ~_PAGE_PRIVILEGED) | _PAGE_USER);
+}
+
+#define pte_mkuser pte_mkuser
+
#endif /* __KERNEL__ */
#endif /* _ASM_POWERPC_NOHASH_PTE_BOOK3E_H */
--
2.13.3
>
>
> But there's also a use of pte_pgprot() in mm/memory.c, and I think that
> is also broken now that we don't add PAGE_KERNEL back in.
>
> Aneesh is going to do a patch to make pte_pgprot() only mask the PFN
> which is what other arches do.
Yes I saw it, that's ok for me.
Christophe
>
> cheers
>
>> From: Christophe Leroy <christophe.leroy@c-s.fr>
>> Date: Wed, 17 Oct 2018 05:56:25 +0000
>> Subject: [PATCH] powerpc/mm: don't use pte_pgprot() in ioremap_prot()
>>
>> pte_pgprot() filters out some required flags like _PAGE_PRESENT.
>>
>> This patch replaces pte_pgprot() by __pgprot(pte_val())
>> in ioremap_prot()
>>
>> Fixes: 26973fa5ac0e ("powerpc/mm: use pte helpers in generic code")
>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>> ---
>> arch/powerpc/mm/pgtable_32.c | 3 ++-
>> arch/powerpc/mm/pgtable_64.c | 4 ++--
>> 2 files changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/powerpc/mm/pgtable_32.c b/arch/powerpc/mm/pgtable_32.c
>> index 5877f5aa8f5d..a606e2f4937b 100644
>> --- a/arch/powerpc/mm/pgtable_32.c
>> +++ b/arch/powerpc/mm/pgtable_32.c
>> @@ -122,7 +122,8 @@ ioremap_prot(phys_addr_t addr, unsigned long size,
>> unsigned long flags)
>> pte = pte_exprotect(pte);
>> pte = pte_mkprivileged(pte);
>>
>> - return __ioremap_caller(addr, size, pte_pgprot(pte),
>> __builtin_return_address(0));
>> + return __ioremap_caller(addr, size, __pgprot(pte_val(pte)),
>> + __builtin_return_address(0));
>> }
>> EXPORT_SYMBOL(ioremap_prot);
>>
>> diff --git a/arch/powerpc/mm/pgtable_64.c b/arch/powerpc/mm/pgtable_64.c
>> index fb1375c07e8c..836bf436cabb 100644
>> --- a/arch/powerpc/mm/pgtable_64.c
>> +++ b/arch/powerpc/mm/pgtable_64.c
>> @@ -245,8 +245,8 @@ void __iomem * ioremap_prot(phys_addr_t addr,
>> unsigned long size,
>> pte = pte_mkprivileged(pte);
>>
>> if (ppc_md.ioremap)
>> - return ppc_md.ioremap(addr, size, pte_pgprot(pte), caller);
>> - return __ioremap_caller(addr, size, pte_pgprot(pte), caller);
>> + return ppc_md.ioremap(addr, size, __pgprot(pte_val(pte)), caller);
>> + return __ioremap_caller(addr, size, __pgprot(pte_val(pte)), caller);
>> }
>>
>>
>> --
>> 2.13.3
^ permalink raw reply related
* Re: [PATCH 1/5] powerpc/64s: Kernel Hypervisor Restricted Access Prevention
From: Michael Ellerman @ 2018-10-17 11:30 UTC (permalink / raw)
To: Russell Currey, linuxppc-dev; +Cc: mikey
In-Reply-To: <20181017064422.26119-1-ruscur@russell.cc>
Russell Currey <ruscur@russell.cc> writes:
> diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
> index 7b1693adff2a..090f72cbb02d 100644
> --- a/arch/powerpc/kernel/entry_64.S
> +++ b/arch/powerpc/kernel/entry_64.S
> @@ -286,6 +286,9 @@ BEGIN_FTR_SECTION
> HMT_MEDIUM_LOW
> END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
>
> + /* headed back to userspace, so unlock the AMR */
> + UNLOCK_AMR(r2)
> +
This one needs an ifdef, or preferable an empty version in a header for
non-book3s 64, otherwise we get:
arch/powerpc/kernel/entry_64.S: Assembler messages:
arch/powerpc/kernel/entry_64.S:290: Error: unrecognized opcode: `unlock_amr(%r2)'
scripts/Makefile.build:405: recipe for target 'arch/powerpc/kernel/entry_64.o' failed
That's a corenet64-ish defconfig.
cheers
^ permalink raw reply
* Re: [PATCH v5 21/22] powerpc/nohash32: allow setting GUARDED attribute in the PMD directly
From: Christophe LEROY @ 2018-10-17 11:51 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
aneesh.kumar
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <0dc7d683dfd155c85b9472bd2e24e7bea5907ca2.1537892499.git.christophe.leroy@c-s.fr>
Le 25/09/2018 à 18:51, Christophe Leroy a écrit :
> On the 8xx, the GUARDED attribute of the pages is managed in the
> L1 entry, therefore to avoid having to copy it into L1 entry
> at each TLB miss, we have to set it in the PMD
>
> In order to allow this, this patch splits the VM alloc space in two
> parts, one for VM alloc and non Guarded IO, and one for Guarded IO.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
I'm not too happy with this part, I think I'll drop it for now and
rework it in a future serie to something more generic using mm slices.
Christophe
> ---
> arch/powerpc/include/asm/book3s/32/pgalloc.h | 2 +-
> arch/powerpc/include/asm/book3s/32/pgtable.h | 2 ++
> arch/powerpc/include/asm/nohash/32/pgalloc.h | 19 ++++++++--
> arch/powerpc/include/asm/nohash/32/pgtable.h | 19 ++++++++--
> arch/powerpc/mm/dump_linuxpagetables.c | 21 +++++++++--
> arch/powerpc/mm/mem.c | 7 ++++
> arch/powerpc/mm/pgtable_32.c | 52 +++++++++++++++++++++++++---
> arch/powerpc/platforms/Kconfig.cputype | 2 ++
> 8 files changed, 112 insertions(+), 12 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/book3s/32/pgalloc.h b/arch/powerpc/include/asm/book3s/32/pgalloc.h
> index 711a8b84e3ee..9097cfd4ce43 100644
> --- a/arch/powerpc/include/asm/book3s/32/pgalloc.h
> +++ b/arch/powerpc/include/asm/book3s/32/pgalloc.h
> @@ -139,7 +139,7 @@ static inline void __pte_free_tlb(struct mmu_gather *tlb, pgtable_t table,
> pgtable_free_tlb(tlb, page_address(table), 0);
> }
>
> -static inline pte_t *early_pte_alloc_kernel(pmd_t *pmdp, unsigned long va)
> +static inline pte_t *early_pte_alloc_kernel(pmd_t *pmdp, unsigned long va, bool is_g)
> {
> if (!pmd_present(*pmdp)) {
> pte_t *ptep = __va(memblock_alloc(PAGE_SIZE, PAGE_SIZE));
> diff --git a/arch/powerpc/include/asm/book3s/32/pgtable.h b/arch/powerpc/include/asm/book3s/32/pgtable.h
> index 7a8a590f6b4c..28001d5eaa89 100644
> --- a/arch/powerpc/include/asm/book3s/32/pgtable.h
> +++ b/arch/powerpc/include/asm/book3s/32/pgtable.h
> @@ -156,6 +156,8 @@ static inline bool pte_user(pte_t pte)
> #define IOREMAP_TOP KVIRT_TOP
> #endif
>
> +#define IOREMAP_BASE VMALLOC_START
> +
> /*
> * Just any arbitrary offset to the start of the vmalloc VM area: the
> * current 16MB value just means that there will be a 64MB "hole" after the
> diff --git a/arch/powerpc/include/asm/nohash/32/pgalloc.h b/arch/powerpc/include/asm/nohash/32/pgalloc.h
> index 77c09bef3122..bfb26c385dac 100644
> --- a/arch/powerpc/include/asm/nohash/32/pgalloc.h
> +++ b/arch/powerpc/include/asm/nohash/32/pgalloc.h
> @@ -60,6 +60,14 @@ static inline void pmd_populate_kernel(struct mm_struct *mm, pmd_t *pmdp,
> *pmdp = __pmd(__pa(pte) | _PMD_PRESENT);
> }
>
> +#ifdef CONFIG_PPC_PMD_GUARDED
> +static inline void pmd_populate_kernel_g(struct mm_struct *mm, pmd_t *pmdp,
> + pte_t *pte)
> +{
> + *pmdp = __pmd(__pa(pte) | _PMD_PRESENT | _PMD_GUARDED);
> +}
> +#endif
> +
> static inline void pmd_populate(struct mm_struct *mm, pmd_t *pmdp,
> pgtable_t pte_page)
> {
> @@ -84,6 +92,10 @@ static inline void pmd_populate(struct mm_struct *mm, pmd_t *pmdp,
> #define pmd_pgtable(pmd) ((pgtable_t)pmd_page_vaddr(pmd))
> #endif
>
> +#ifndef CONFIG_PPC_PMD_GUARDED
> +#define pmd_populate_kernel_g pmd_populate_kernel
> +#endif
> +
> static inline pte_t *pte_alloc_one_kernel(struct mm_struct *mm,
> unsigned long address)
> {
> @@ -151,7 +163,7 @@ static inline void __pte_free_tlb(struct mmu_gather *tlb, pgtable_t table,
> pgtable_free_tlb(tlb, table, 0);
> }
>
> -static inline pte_t *early_pte_alloc_kernel(pmd_t *pmdp, unsigned long va)
> +static inline pte_t *early_pte_alloc_kernel(pmd_t *pmdp, unsigned long va, bool is_g)
> {
> if (!pmd_present(*pmdp)) {
> pte_t *ptep = __va(memblock_alloc(PTE_FRAG_SIZE, PTE_FRAG_SIZE));
> @@ -164,7 +176,10 @@ static inline pte_t *early_pte_alloc_kernel(pmd_t *pmdp, unsigned long va)
> else
> memset(ptep, 0, PTE_FRAG_SIZE);
>
> - pmd_populate_kernel(&init_mm, pmdp, ptep);
> + if (is_g)
> + pmd_populate_kernel_g(&init_mm, pmdp, ptep);
> + else
> + pmd_populate_kernel(&init_mm, pmdp, ptep);
> }
> return pte_offset_kernel(pmdp, va);
> }
> diff --git a/arch/powerpc/include/asm/nohash/32/pgtable.h b/arch/powerpc/include/asm/nohash/32/pgtable.h
> index dc82c10383d5..fccc5620a988 100644
> --- a/arch/powerpc/include/asm/nohash/32/pgtable.h
> +++ b/arch/powerpc/include/asm/nohash/32/pgtable.h
> @@ -84,9 +84,14 @@ extern int icache_44x_need_flush;
> * virtual space that goes below PKMAP and FIXMAP
> */
> #ifdef CONFIG_HIGHMEM
> -#define KVIRT_TOP PKMAP_BASE
> +#define _KVIRT_TOP PKMAP_BASE
> #else
> -#define KVIRT_TOP (0xfe000000UL) /* for now, could be FIXMAP_BASE ? */
> +#define _KVIRT_TOP (0xfe000000UL) /* for now, could be FIXMAP_BASE ? */
> +#endif
> +#ifdef CONFIG_PPC_PMD_GUARDED
> +#define KVIRT_TOP _ALIGN_DOWN(_KVIRT_TOP, PGDIR_SIZE)
> +#else
> +#define KVIRT_TOP _KVIRT_TOP
> #endif
>
> /*
> @@ -99,6 +104,12 @@ extern int icache_44x_need_flush;
> #else
> #define IOREMAP_TOP KVIRT_TOP
> #endif
> +#ifdef CONFIG_PPC_PMD_GUARDED
> +#define IOREMAP_BASE _ALIGN_UP(VMALLOC_START + (IOREMAP_TOP - VMALLOC_START) / 2, \
> + PGDIR_SIZE)
> +#else
> +#define IOREMAP_BASE VMALLOC_START
> +#endif
>
> /*
> * Just any arbitrary offset to the start of the vmalloc VM area: the
> @@ -123,7 +134,11 @@ extern int icache_44x_need_flush;
> #else
> #define VMALLOC_START ((((long)high_memory + VMALLOC_OFFSET) & ~(VMALLOC_OFFSET-1)))
> #endif
> +#ifdef CONFIG_PPC_PMD_GUARDED
> +#define VMALLOC_END IOREMAP_BASE
> +#else
> #define VMALLOC_END ioremap_bot
> +#endif
>
> /*
> * Bits in a linux-style PTE. These match the bits in the
> diff --git a/arch/powerpc/mm/dump_linuxpagetables.c b/arch/powerpc/mm/dump_linuxpagetables.c
> index e60aa6d7456d..105d0118f735 100644
> --- a/arch/powerpc/mm/dump_linuxpagetables.c
> +++ b/arch/powerpc/mm/dump_linuxpagetables.c
> @@ -76,9 +76,9 @@ struct addr_marker {
>
> static struct addr_marker address_markers[] = {
> { 0, "Start of kernel VM" },
> +#ifdef CONFIG_PPC64
> { 0, "vmalloc() Area" },
> { 0, "vmalloc() End" },
> -#ifdef CONFIG_PPC64
> { 0, "isa I/O start" },
> { 0, "isa I/O end" },
> { 0, "phb I/O start" },
> @@ -87,8 +87,19 @@ static struct addr_marker address_markers[] = {
> { 0, "I/O remap end" },
> { 0, "vmemmap start" },
> #else
> +#ifdef CONFIG_PPC_PMD_GUARDED
> + { 0, "vmalloc() Area" },
> + { 0, "vmalloc() End" },
> + { 0, "Early I/O remap start" },
> + { 0, "Early I/O remap end" },
> + { 0, "I/O remap start" },
> + { 0, "I/O remap end" },
> +#else
> { 0, "Early I/O remap start" },
> { 0, "Early I/O remap end" },
> + { 0, "vmalloc() I/O remap start" },
> + { 0, "vmalloc() I/O remap end" },
> +#endif
> #ifdef CONFIG_NOT_COHERENT_CACHE
> { 0, "Consistent mem start" },
> { 0, "Consistent mem end" },
> @@ -286,9 +297,9 @@ static void populate_markers(void)
> int i = 0;
>
> address_markers[i++].start_address = PAGE_OFFSET;
> +#ifdef CONFIG_PPC64
> address_markers[i++].start_address = VMALLOC_START;
> address_markers[i++].start_address = VMALLOC_END;
> -#ifdef CONFIG_PPC64
> address_markers[i++].start_address = ISA_IO_BASE;
> address_markers[i++].start_address = ISA_IO_END;
> address_markers[i++].start_address = PHB_IO_BASE;
> @@ -301,6 +312,12 @@ static void populate_markers(void)
> address_markers[i++].start_address = VMEMMAP_BASE;
> #endif
> #else /* !CONFIG_PPC64 */
> +#ifdef CONFIG_PPC_PMD_GUARDED
> + address_markers[i++].start_address = VMALLOC_START;
> + address_markers[i++].start_address = VMALLOC_END;
> +#endif
> + address_markers[i++].start_address = IOREMAP_BASE;
> + address_markers[i++].start_address = ioremap_bot;
> address_markers[i++].start_address = ioremap_bot;
> address_markers[i++].start_address = IOREMAP_TOP;
> #ifdef CONFIG_NOT_COHERENT_CACHE
> diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
> index dd949d6649a2..1a4a20a95eaf 100644
> --- a/arch/powerpc/mm/mem.c
> +++ b/arch/powerpc/mm/mem.c
> @@ -388,8 +388,15 @@ void __init mem_init(void)
> #endif /* CONFIG_NOT_COHERENT_CACHE */
> pr_info(" * 0x%08lx..0x%08lx : early ioremap\n",
> ioremap_bot, IOREMAP_TOP);
> +#ifdef CONFIG_PPC_PMD_GUARDED
> + pr_info(" * 0x%08lx..0x%08lx : ioremap\n",
> + IOREMAP_BASE, ioremap_bot);
> + pr_info(" * 0x%08lx..0x%08lx : vmalloc\n",
> + VMALLOC_START, VMALLOC_END);
> +#else
> pr_info(" * 0x%08lx..0x%08lx : vmalloc & ioremap\n",
> VMALLOC_START, VMALLOC_END);
> +#endif
> #endif /* CONFIG_PPC32 */
> }
>
> diff --git a/arch/powerpc/mm/pgtable_32.c b/arch/powerpc/mm/pgtable_32.c
> index 745f2f30d4e9..ce78c06db660 100644
> --- a/arch/powerpc/mm/pgtable_32.c
> +++ b/arch/powerpc/mm/pgtable_32.c
> @@ -151,7 +151,14 @@ __ioremap_caller(phys_addr_t addr, unsigned long size, pgprot_t prot, void *call
>
> if (slab_is_available()) {
> struct vm_struct *area;
> - area = get_vm_area_caller(size, VM_IOREMAP, caller);
> + bool is_g = pgprot_val(prot) & _PAGE_GUARDED;
> +
> + if (IS_ENABLED(CONFIG_PPC_PMD_GUARDED) && is_g)
> + area = __get_vm_area_caller(size, VM_IOREMAP, IOREMAP_BASE,
> + ioremap_bot, caller);
> + else
> + area = get_vm_area_caller(size, VM_IOREMAP, caller);
> +
> if (area == 0)
> return NULL;
> area->phys_addr = p;
> @@ -192,19 +199,54 @@ void iounmap(volatile void __iomem *addr)
> }
> EXPORT_SYMBOL(iounmap);
>
> +#ifdef CONFIG_PPC_PMD_GUARDED
> +static int __pte_alloc_kernel_g(pmd_t *pmd, unsigned long address)
> +{
> + pte_t *new = pte_alloc_one_kernel(&init_mm, address);
> + if (!new)
> + return -ENOMEM;
> +
> + smp_wmb(); /* See comment in __pte_alloc */
> +
> + spin_lock(&init_mm.page_table_lock);
> + if (likely(pmd_none(*pmd))) { /* Has another populated it ? */
> + pmd_populate_kernel_g(&init_mm, pmd, new);
> + new = NULL;
> + }
> + spin_unlock(&init_mm.page_table_lock);
> + if (new)
> + pte_free_kernel(&init_mm, new);
> + return 0;
> +}
> +
> +static pte_t *pte_alloc_kernel_g(pmd_t *pmd, unsigned long address)
> +{
> + if (unlikely(pmd_none(*(pmd))) && __pte_alloc_kernel_g(pmd, address))
> + return NULL;
> + return pte_offset_kernel(pmd, address);
> +}
> +#else
> +#define pte_alloc_kernel_g(pmd, address) pte_alloc_kernel(pmd, address)
> +#endif
> +
> __ref int map_kernel_page(unsigned long va, phys_addr_t pa, pgprot_t prot)
> {
> pmd_t *pd;
> pte_t *pg;
> int err = -ENOMEM;
> + bool is_g = pgprot_val(prot) & _PAGE_GUARDED;
>
> /* Use upper 10 bits of VA to index the first level map */
> pd = pmd_offset(pud_offset(pgd_offset_k(va), va), va);
> /* Use middle 10 bits of VA to index the second-level map */
> - if (slab_is_available())
> - pg = pte_alloc_kernel(pd, va);
> - else
> - pg = early_pte_alloc_kernel(pd, va);
> + if (slab_is_available()) {
> + if (is_g)
> + pg = pte_alloc_kernel_g(pd, va);
> + else
> + pg = pte_alloc_kernel(pd, va);
> + } else {
> + pg = early_pte_alloc_kernel(pd, va, is_g);
> + }
> if (pg != 0) {
> err = 0;
> /* The PTE should never be already set nor present in the
> diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype
> index 6c6a7c72cae4..d0984546fbec 100644
> --- a/arch/powerpc/platforms/Kconfig.cputype
> +++ b/arch/powerpc/platforms/Kconfig.cputype
> @@ -355,6 +355,8 @@ config ARCH_ENABLE_HUGEPAGE_MIGRATION
> def_bool y
> depends on PPC_BOOK3S_64 && HUGETLB_PAGE && MIGRATION
>
> +config PPC_PMD_GUARDED
> + bool
>
> config PPC_MMU_NOHASH
> def_bool y
>
^ permalink raw reply
* Re: Crash on FSL Book3E due to pte_pgprot()? (was Re: [PATCH v3 12/24] powerpc/mm: use pte helpers in generic code)
From: Aneesh Kumar K.V @ 2018-10-17 11:53 UTC (permalink / raw)
To: Christophe Leroy, Michael Ellerman, Bartlomiej Zolnierkiewicz,
Benjamin Herrenschmidt, Dominik Brodowski, Geoff Levand,
Jens Axboe, Kumar Gala, Li Yang, Nicholas Piggin, Paul Mackerras,
Scott Wood, aneesh.kumar
Cc: linux-fbdev, linux-kernel, dri-devel, linux-block, linuxppc-dev,
linux-arm-kernel
In-Reply-To: <6ec9cf4b-abd2-8244-4780-4564a7ffb1ad@c-s.fr>
On 10/17/18 4:42 PM, Christophe Leroy wrote:
>
>
> On 10/17/2018 10:32 AM, Michael Ellerman wrote:
>> Christophe Leroy <christophe.leroy@c-s.fr> writes:
>>> On 10/17/2018 12:59 AM, Michael Ellerman wrote:
>> ...
>>>> The question is what's the right way to fix it? Should pte_pgprot() not
>>>> be filtering those bits out on book3e?
>>>
>>> I think we should not use pte_pggrot() for that then. What about the
>>> below fix ?
>>
>> Thanks, that almost works.
>>
>> pte_mkprivileged() also needs to not strip _PAGE_BAP_SR.
>
> Oops, I missed it allthough I knew it. Patch below.
>
> From: Christophe Leroy <christophe.leroy@c-s.fr>
> Date: Wed, 17 Oct 2018 10:46:24 +0000
> Subject: [PATCH] powerpc/book3e: redefine pte_mkprivileged() and
> pte_mkuser()
> To: Benjamin Herrenschmidt <benh@kernel.crashing.org>, Paul Mackerras
> <paulus@samba.org>, Michael Ellerman <mpe@ellerman.id.au>
> Cc: linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org
>
> Book3e defines both _PAGE_USER and _PAGE_PRIVILEGED, so the nohash
> default pte_mkprivileged() and pte_mkuser() are not usable.
>
> This patch redefines them for book3e.
>
> Fixes: a0da4bc166f2 ("powerpc/mm: Allow platforms to redefine some
> helpers")
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
> arch/powerpc/include/asm/nohash/pte-book3e.h | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/nohash/pte-book3e.h
> b/arch/powerpc/include/asm/nohash/pte-book3e.h
> index 58eef8cb569d..fb4297dff3e2 100644
> --- a/arch/powerpc/include/asm/nohash/pte-book3e.h
> +++ b/arch/powerpc/include/asm/nohash/pte-book3e.h
> @@ -109,5 +109,19 @@
> #define PAGE_READONLY __pgprot(_PAGE_BASE | _PAGE_USER)
> #define PAGE_READONLY_X __pgprot(_PAGE_BASE | _PAGE_USER | _PAGE_EXEC)
>
> +static inline pte_t pte_mkprivileged(pte_t pte)
> +{
> + return __pte((pte_val(pte) & ~_PAGE_USER) | _PAGE_PRIVILEGED);
> +}
> +
> +#define pte_mkprivileged pte_mkprivileged
> +
> +static inline pte_t pte_mkuser(pte_t pte)
> +{
> + return __pte((pte_val(pte) & ~_PAGE_PRIVILEGED) | _PAGE_USER);
> +}
> +
> +#define pte_mkuser pte_mkuser
> +
I was build testing a similar patch. We would need to put #ifndef
__ASSEMBLY__ around it.
-aneesh
^ permalink raw reply
* Re: [PATCH 1/5] powerpc/64s: Kernel Hypervisor Restricted Access Prevention
From: Nicholas Piggin @ 2018-10-17 12:59 UTC (permalink / raw)
To: Russell Currey; +Cc: mikey, linuxppc-dev
In-Reply-To: <20181017064422.26119-1-ruscur@russell.cc>
On Wed, 17 Oct 2018 17:44:19 +1100
Russell Currey <ruscur@russell.cc> wrote:
> Kernel Hypervisor Restricted Access Prevention (KHRAP) utilises a feature
> of the Radix MMU which disallows read and write access to userspace
> addresses. By utilising this, the kernel is prevented from accessing
> user data from outside of trusted paths that perform proper safety checks,
> such as copy_{to/from}_user() and friends.
>
> Userspace access is disabled from early boot and is only enabled when:
>
> - exiting the kernel and entering userspace
> - performing an operation like copy_{to/from}_user()
> - context switching to a process that has access enabled
>
> and similarly, access is disabled again when exiting userspace and entering
> the kernel.
>
> This feature has a slight performance impact which I roughly measured to be
> 4% slower (performing 1GB of 1 byte read()/write() syscalls), and is gated
> behind the CONFIG_PPC_RADIX_KHRAP option for performance-critical builds.
>
> This feature can be tested by using the lkdtm driver (CONFIG_LKDTM=y) and
> performing the following:
>
> echo ACCESS_USERSPACE > [debugfs]/provoke-crash/DIRECT
>
> if enabled, this should send SIGSEGV to the thread.
>
> Signed-off-by: Russell Currey <ruscur@russell.cc>
> ---
> More detailed benchmarks soon, there's more optimisations here as well.
Nice, this turned out to be a lot neater than I feared! Good stuff.
> @@ -240,6 +240,22 @@ BEGIN_FTR_SECTION_NESTED(941) \
> mtspr SPRN_PPR,ra; \
> END_FTR_SECTION_NESTED(CPU_FTR_HAS_PPR,CPU_FTR_HAS_PPR,941)
>
> +#define LOCK_AMR(reg) \
> +BEGIN_MMU_FTR_SECTION_NESTED(69) \
> + LOAD_REG_IMMEDIATE(reg,AMR_LOCKED); \
> + isync; \
> + mtspr SPRN_AMR,reg; \
> + isync; \
> +END_MMU_FTR_SECTION_NESTED(MMU_FTR_RADIX_KHRAP,MMU_FTR_RADIX_KHRAP,69)
> +
> +#define UNLOCK_AMR(reg) \
> +BEGIN_MMU_FTR_SECTION_NESTED(420) \
> + li reg,0; \
> + isync; \
> + mtspr SPRN_AMR,reg; \
> + isync; \
> +END_MMU_FTR_SECTION_NESTED(MMU_FTR_RADIX_KHRAP,MMU_FTR_RADIX_KHRAP,420)
I wonder if you can skip the first isync on the way in and the second
isync on the way out because the interrupt and return should be context
synchronizing. Might not make a difference though.
What do you think about making the name match the C code a bit more.
Like AMR_LOCK_USER_ACCESS()?
Thanks,
Nick
^ permalink raw reply
* [PATCH] powerpc/book3e: redefine pte_mkprivileged() and pte_mkuser()
From: Christophe Leroy @ 2018-10-17 13:03 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
aneesh.kumar
Cc: linuxppc-dev, linux-kernel
Book3e defines both _PAGE_USER and _PAGE_PRIVILEGED, so the nohash
default pte_mkprivileged() and pte_mkuser() are not usable.
This patch redefines them for book3e.
In theorie, only pte_mkprivileged() needs to be redefined because
_PAGE_USER includes _PAGE_PRIVILEGED, but it is less confusing
to redefine both.
Fixes: a0da4bc166f2 ("powerpc/mm: Allow platforms to redefine some helpers")
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/include/asm/nohash/pte-book3e.h | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/arch/powerpc/include/asm/nohash/pte-book3e.h b/arch/powerpc/include/asm/nohash/pte-book3e.h
index 58eef8cb569d..fa1451e15b4e 100644
--- a/arch/powerpc/include/asm/nohash/pte-book3e.h
+++ b/arch/powerpc/include/asm/nohash/pte-book3e.h
@@ -109,5 +109,21 @@
#define PAGE_READONLY __pgprot(_PAGE_BASE | _PAGE_USER)
#define PAGE_READONLY_X __pgprot(_PAGE_BASE | _PAGE_USER | _PAGE_EXEC)
+#ifndef __ASSEMBLY__
+static inline pte_t pte_mkprivileged(pte_t pte)
+{
+ return __pte((pte_val(pte) & ~_PAGE_USER) | _PAGE_PRIVILEGED);
+}
+
+#define pte_mkprivileged pte_mkprivileged
+
+static inline pte_t pte_mkuser(pte_t pte)
+{
+ return __pte((pte_val(pte) & ~_PAGE_PRIVILEGED) | _PAGE_USER);
+}
+
+#define pte_mkuser pte_mkuser
+#endif /* __ASSEMBLY__ */
+
#endif /* __KERNEL__ */
#endif /* _ASM_POWERPC_NOHASH_PTE_BOOK3E_H */
--
2.13.3
^ permalink raw reply related
* Re: [PATCH] powerpc/book3e: redefine pte_mkprivileged() and pte_mkuser()
From: Aneesh Kumar K.V @ 2018-10-17 13:23 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <19885260cc71182f8c03840204e2553ef42acb67.1539781367.git.christophe.leroy@c-s.fr>
On 10/17/18 6:33 PM, Christophe Leroy wrote:
> Book3e defines both _PAGE_USER and _PAGE_PRIVILEGED, so the nohash
> default pte_mkprivileged() and pte_mkuser() are not usable.
>
> This patch redefines them for book3e.
>
> In theorie, only pte_mkprivileged() needs to be redefined because
> _PAGE_USER includes _PAGE_PRIVILEGED, but it is less confusing
> to redefine both.
>
> Fixes: a0da4bc166f2 ("powerpc/mm: Allow platforms to redefine some helpers")
Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
> arch/powerpc/include/asm/nohash/pte-book3e.h | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/nohash/pte-book3e.h b/arch/powerpc/include/asm/nohash/pte-book3e.h
> index 58eef8cb569d..fa1451e15b4e 100644
> --- a/arch/powerpc/include/asm/nohash/pte-book3e.h
> +++ b/arch/powerpc/include/asm/nohash/pte-book3e.h
> @@ -109,5 +109,21 @@
> #define PAGE_READONLY __pgprot(_PAGE_BASE | _PAGE_USER)
> #define PAGE_READONLY_X __pgprot(_PAGE_BASE | _PAGE_USER | _PAGE_EXEC)
>
> +#ifndef __ASSEMBLY__
> +static inline pte_t pte_mkprivileged(pte_t pte)
> +{
> + return __pte((pte_val(pte) & ~_PAGE_USER) | _PAGE_PRIVILEGED);
> +}
> +
> +#define pte_mkprivileged pte_mkprivileged
> +
> +static inline pte_t pte_mkuser(pte_t pte)
> +{
> + return __pte((pte_val(pte) & ~_PAGE_PRIVILEGED) | _PAGE_USER);
> +}
> +
> +#define pte_mkuser pte_mkuser
> +#endif /* __ASSEMBLY__ */
> +
> #endif /* __KERNEL__ */
> #endif /* _ASM_POWERPC_NOHASH_PTE_BOOK3E_H */
>
^ permalink raw reply
* Re: [PATCH v06 3/5] migration/memory: Add hotplug READD_MULTIPLE
From: Michael Bringmann @ 2018-10-17 15:12 UTC (permalink / raw)
To: Michael Ellerman, Tyrel Datwyler, linuxppc-dev
Cc: Nathan Fontenot, Juliet Kim, Thomas Falcon
In-Reply-To: <871s8pl7p4.fsf@concordia.ellerman.id.au>
On 10/16/2018 07:48 PM, Michael Ellerman wrote:
> Michael Bringmann <mwb@linux.vnet.ibm.com> writes:
>> On 10/16/2018 02:57 PM, Tyrel Datwyler wrote:
>>> On 10/15/2018 05:39 PM, Michael Ellerman wrote:
>>>> Michael Bringmann <mwb@linux.vnet.ibm.com> writes:
>>>>> diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
>>>>> index 2b796da..9c76345 100644
>>>>> --- a/arch/powerpc/platforms/pseries/hotplug-memory.c
>>>>> +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
>>>>> @@ -541,6 +549,23 @@ static int dlpar_memory_readd_by_index(u32 drc_index)
>>>>> return rc;
>>>>> }
>>>>>
>>>>> +static int dlpar_memory_readd_multiple(void)
>>>>> +{
>>>>> + struct drmem_lmb *lmb;
>>>>> + int rc;
>>>>> +
>>>>> + pr_info("Attempting to update multiple LMBs\n");
>>>>> +
>>>>> + for_each_drmem_lmb(lmb) {
>>>>> + if (drmem_lmb_update(lmb)) {
>>>>> + rc = dlpar_memory_readd_helper(lmb);
>>>>> + drmem_remove_lmb_update(lmb);
>>>>> + }
>>>>> + }
>>>>> +
>>>>> + return rc;
>>>>> +}
>>>>
>>>> This leaves rc potentially uninitialised.
>>>>
>>>> What should the result be in that case, -EINVAL ?
>>>
>>> On another note if there are multiple LMBs to update the value of rc only reflects the final dlpar_memory_readd_helper() call.
>>
>> Correct. But that is what happens when we compress common code
>> between two disparate uses i.e. updating memory association after
>> a migration event with no reporting mechanism other than the console
>> log, vs re-adding a single LMB by index for the purposes of DLPAR / drmgr.
>>
>> I could discard the return value from dlpar_memory_readd_helper entirely
>> in this function and just return 0, but in my experience, once errors start
>> to occur in memory dlpar ops, they tend to keep on occurring, so I was
>> returning the last one. We could also make the code smart enough to
>> capture and return the first/last non-zero return code. I didn't believe
>> that the frequency of errors for this operation warranted the overhead.
>
> The actual error value is probably not very relevant.
>
> But dropping errors entirely is almost always a bad idea.
>
> So I think you should at least return an error if any error occurred,
> that way at least an error will be returned up to the caller(s).
>
> Something like:
>
> int rc;
>
> rc = 0;
> for_each_drmem_lmb(lmb) {
> if (drmem_lmb_update(lmb)) {
> rc |= dlpar_memory_readd_helper(lmb);
> drmem_remove_lmb_update(lmb);
> }
> }
>
> if (rc)
> return -EIO;
Okay.
>
> cheers
>
Thanks.
--
Michael W. Bringmann
Linux Technology Center
IBM Corporation
Tie-Line 363-5196
External: (512) 286-5196
Cell: (512) 466-0650
mwb@linux.vnet.ibm.com
^ permalink raw reply
* Re: MPC8321 boot failure
From: Christophe LEROY @ 2018-10-17 16:36 UTC (permalink / raw)
To: David Gounaris, linuxppc-dev@lists.ozlabs.org
Cc: gregkh@linuxfoundation.org, Mathias Thore
In-Reply-To: <CY1PR10MB020424967E404E0B1BBE1E82EFFF0@CY1PR10MB0204.namprd10.prod.outlook.com>
Hi,
Yes I discovered the same issue today on MPC8321E, I plan to look at it
more closely tomorrow morning (Paris Time).
I think we are missing commit 8183d99f4a22c2abbc543847a588df3666ef0c0c ,
I didn't realise it when we applied the serie to 4.14,
patch_instruction() is called too early without that patch.
If you have opportunity to test now, you are welcome, otherwise I'll
test it tomorrow.
Christophe
Le 17/10/2018 à 17:18, David Gounaris a écrit :
> Hello, I got into troubles when I upgraded to Linux kernel 4.14.76 on
> boards with MPC8321.
>
>
> The symptom that I see is that the boot process gets cyclic, and no
> printouts are seen from the Linux kernel. It seems like it resets.
>
>
> When I revert the following commits it works again.
>
> af1a8101794dfea897290e057f61086dabfe6c91, powerpc/lib: fix book3s/32
> boot failure due to code patching
> 609fbeddb24c4035d24fc32d82dc08b30ae3dfc0, powerpc: Avoid code patching
> freed init sections
>
> Any ideas of how to continue?
>
> BR / David Gounaris
>
>
>
^ permalink raw reply
* [PATCH] powerpc: Add missing include <asm/stacktrace.h>
From: Mathieu Malaterre @ 2018-10-17 19:25 UTC (permalink / raw)
To: Michael Ellerman
Cc: Mathieu Malaterre, Paul Mackerras, linuxppc-dev, linux-kernel
In commit 88b0fe175735 ("powerpc: Add show_user_instructions()") the
function show_user_instructions was added.
This commit adds an include of header file <asm/stacktrace.h> to provide
the missing function prototype. Silence the following gcc warning
(treated as error with W=1):
arch/powerpc/kernel/process.c:1302:6: error: no previous prototype for ‘show_user_instructions’ [-Werror=missing-prototypes]
Signed-off-by: Mathieu Malaterre <malat@debian.org>
---
arch/powerpc/kernel/process.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index bb6ac471a784..1c64491e9702 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -65,6 +65,7 @@
#include <asm/livepatch.h>
#include <asm/cpu_has_feature.h>
#include <asm/asm-prototypes.h>
+#include <asm/stacktrace.h>
#include <linux/kprobes.h>
#include <linux/kdebug.h>
--
2.11.0
^ permalink raw reply related
* [PATCH v07 0/5] powerpc/migration: Affinity fix for memory
From: Michael Bringmann @ 2018-10-17 19:37 UTC (permalink / raw)
To: linuxppc-dev, mwb
Cc: Juliet Kim, Thomas Falcon, Tyrel Datwyler, Nathan Fontenot
The migration of LPARs across Power systems affects many attributes
including that of the associativity of memory blocks. The patches
in this set execute when a system is coming up fresh upon a migration
target. They are intended to,
* Recognize changes to the associativity of memory recorded in
internal data structures when compared to the latest copies in
the device tree (e.g. ibm,dynamic-memory, ibm,dynamic-memory-v2).
* Recognize changes to the associativity mapping (e.g. ibm,
associativity-lookup-arrays), locate all assigned memory blocks
corresponding to each changed row, and readd all such blocks.
* Generate calls to other code layers to reset the data structures
related to associativity of memory.
* Re-register the 'changed' entities into the target system.
Re-registration of memory blocks mostly entails acting as if they
have been newly hot-added into the target system.
This code builds upon features introduced in a previous patch set
that updates CPUs for affinity changes that may occur during LPM.
Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
Michael Bringmann (5):
powerpc/drmem: Export 'dynamic-memory' loader
powerpc/drmem: Add internal_flags feature
migration/memory: Add hotplug flags READD_MULTIPLE
migration/memory: Evaluate LMB assoc changes
migration/memory: Support 'ibm,dynamic-memory-v2'
---
Changes in v07:
-- Provide more useful return value from dlpar_memory_readd_multiple
Changes in v06:
-- Rebase to powerpc next branch to account for recent code changes.
-- Fix prototype problem when CONFIG_MEMORY_HOTPLUG not defined.
Changes in v05:
-- Add walk_drmem_lmbs_pairs to replace macro for_each_pair_lmb
-- Use walk_drmem_lmbs_pairs and callback instead of local loop
-- Provide dlpar_memory_readd_helper routine to compress some common code
-- Move common structure from numa.c + hotplug-memory.c to header file.
-- Clarify some comments.
Changes in v04:
-- Move dlpar_memory_readd_multiple() to patch with new ACTION
constant.
-- Move init of 'lmb->internal_flags' in init_drmem_v2_lmbs to
patch with other references to flag.
-- Correct spacing in one of the patches
Changes in v03:
-- Change operation to tag changed LMBs in DRMEM array instead of
queuing a potentially huge number of structures.
-- Added another hotplug queue event for CPU/memory operations
-- Added internal_flags feature to DRMEM
-- Improve the patch description language for the patch set.
-- Revise patch set to queue worker for memory association
updates directly to pseries worker queue.
^ permalink raw reply
* [PATCH v07 1/5] powerpc/drmem: Export 'dynamic-memory' loader
From: Michael Bringmann @ 2018-10-17 19:38 UTC (permalink / raw)
To: linuxppc-dev, mwb
Cc: Juliet Kim, Thomas Falcon, Tyrel Datwyler, Nathan Fontenot
In-Reply-To: <20181017193745.4844.11915.stgit@ltcalpine2-lp9.aus.stglabs.ibm.com>
powerpc/drmem: Export many of the functions of DRMEM to parse
"ibm,dynamic-memory" and "ibm,dynamic-memory-v2" during hotplug
operations and for Post Migration events.
Also modify the DRMEM initialization code to allow it to,
* Be called after system initialization
* Provide a separate user copy of the LMB array that is produces
* Free the user copy upon request
In addition, a couple of changes were made to make the creation
of additional copies of the LMB array more useful including,
* Add iterator function to work through a pair of drmem_info arrays
with a callback function to apply specific tests.
* Modify DRMEM code to replace usages of dt_root_addr_cells, and
dt_mem_next_cell, as these are only available at first boot.
Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
---
Changes in v05:
-- Add walk_drmem_lmbs_pairs to replace macro for_each_pair_lmb
---
arch/powerpc/include/asm/drmem.h | 13 +++++
arch/powerpc/mm/drmem.c | 96 ++++++++++++++++++++++++++++++--------
2 files changed, 89 insertions(+), 20 deletions(-)
diff --git a/arch/powerpc/include/asm/drmem.h b/arch/powerpc/include/asm/drmem.h
index 7c1d8e7..cfe8598 100644
--- a/arch/powerpc/include/asm/drmem.h
+++ b/arch/powerpc/include/asm/drmem.h
@@ -35,6 +35,11 @@ struct drmem_lmb_info {
&drmem_info->lmbs[0], \
&drmem_info->lmbs[drmem_info->n_lmbs - 1])
+#define for_each_dinfo_lmb(dinfo, lmb) \
+ for_each_drmem_lmb_in_range((lmb), \
+ &dinfo->lmbs[0], \
+ &dinfo->lmbs[dinfo->n_lmbs - 1])
+
/*
* The of_drconf_cell_v1 struct defines the layout of the LMB data
* specified in the ibm,dynamic-memory device tree property.
@@ -94,6 +99,14 @@ void __init walk_drmem_lmbs(struct device_node *dn,
void (*func)(struct drmem_lmb *, const __be32 **));
int drmem_update_dt(void);
+struct drmem_lmb_info *drmem_lmbs_init(struct property *prop);
+void drmem_lmbs_free(struct drmem_lmb_info *dinfo);
+int walk_drmem_lmbs_pairs(struct drmem_lmb_info *dinfo_oth,
+ int (*func)(struct drmem_lmb *cnt,
+ struct drmem_lmb *oth,
+ void *data),
+ void *data);
+
#ifdef CONFIG_PPC_PSERIES
void __init walk_drmem_lmbs_early(unsigned long node,
void (*func)(struct drmem_lmb *, const __be32 **));
diff --git a/arch/powerpc/mm/drmem.c b/arch/powerpc/mm/drmem.c
index 3f18036..ded9dbf 100644
--- a/arch/powerpc/mm/drmem.c
+++ b/arch/powerpc/mm/drmem.c
@@ -20,6 +20,7 @@
static struct drmem_lmb_info __drmem_info;
struct drmem_lmb_info *drmem_info = &__drmem_info;
+static int n_root_addr_cells;
u64 drmem_lmb_memory_max(void)
{
@@ -193,12 +194,13 @@ int drmem_update_dt(void)
return rc;
}
-static void __init read_drconf_v1_cell(struct drmem_lmb *lmb,
+static void read_drconf_v1_cell(struct drmem_lmb *lmb,
const __be32 **prop)
{
const __be32 *p = *prop;
- lmb->base_addr = dt_mem_next_cell(dt_root_addr_cells, &p);
+ lmb->base_addr = of_read_number(p, n_root_addr_cells);
+ p += n_root_addr_cells;
lmb->drc_index = of_read_number(p++, 1);
p++; /* skip reserved field */
@@ -209,7 +211,7 @@ static void __init read_drconf_v1_cell(struct drmem_lmb *lmb,
*prop = p;
}
-static void __init __walk_drmem_v1_lmbs(const __be32 *prop, const __be32 *usm,
+static void __walk_drmem_v1_lmbs(const __be32 *prop, const __be32 *usm,
void (*func)(struct drmem_lmb *, const __be32 **))
{
struct drmem_lmb lmb;
@@ -225,13 +227,14 @@ static void __init __walk_drmem_v1_lmbs(const __be32 *prop, const __be32 *usm,
}
}
-static void __init read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
+static void read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
const __be32 **prop)
{
const __be32 *p = *prop;
dr_cell->seq_lmbs = of_read_number(p++, 1);
- dr_cell->base_addr = dt_mem_next_cell(dt_root_addr_cells, &p);
+ dr_cell->base_addr = of_read_number(p, n_root_addr_cells);
+ p += n_root_addr_cells;
dr_cell->drc_index = of_read_number(p++, 1);
dr_cell->aa_index = of_read_number(p++, 1);
dr_cell->flags = of_read_number(p++, 1);
@@ -239,7 +242,7 @@ static void __init read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
*prop = p;
}
-static void __init __walk_drmem_v2_lmbs(const __be32 *prop, const __be32 *usm,
+static void __walk_drmem_v2_lmbs(const __be32 *prop, const __be32 *usm,
void (*func)(struct drmem_lmb *, const __be32 **))
{
struct of_drconf_cell_v2 dr_cell;
@@ -275,6 +278,9 @@ void __init walk_drmem_lmbs_early(unsigned long node,
const __be32 *prop, *usm;
int len;
+ if (n_root_addr_cells == 0)
+ n_root_addr_cells = dt_root_addr_cells;
+
prop = of_get_flat_dt_prop(node, "ibm,lmb-size", &len);
if (!prop || len < dt_root_size_cells * sizeof(__be32))
return;
@@ -353,24 +359,47 @@ void __init walk_drmem_lmbs(struct device_node *dn,
}
}
-static void __init init_drmem_v1_lmbs(const __be32 *prop)
+int walk_drmem_lmbs_pairs(struct drmem_lmb_info *dinfo_oth,
+ int (*func)(struct drmem_lmb *cnt,
+ struct drmem_lmb *oth,
+ void *data),
+ void *data)
+{
+ struct drmem_lmb *lmb1, *lmb2;
+ int rc = 0;
+
+ for ((lmb1) = (&drmem_info->lmbs[0]), (lmb2) = (&dinfo_oth->lmbs[0]);
+ ((lmb1) <= (&drmem_info->lmbs[drmem_info->n_lmbs - 1])) &&
+ ((lmb2) <= (&dinfo_oth->lmbs[dinfo_oth->n_lmbs - 1]));
+ (lmb1)++, (lmb2)++)
+ {
+ rc = func(lmb1, lmb2, data);
+ if (rc <= 0)
+ break;
+ }
+ return rc;
+}
+
+static void init_drmem_v1_lmbs(const __be32 *prop,
+ struct drmem_lmb_info *dinfo)
{
struct drmem_lmb *lmb;
- drmem_info->n_lmbs = of_read_number(prop++, 1);
- if (drmem_info->n_lmbs == 0)
+ dinfo->n_lmbs = of_read_number(prop++, 1);
+ if (dinfo->n_lmbs == 0)
return;
- drmem_info->lmbs = kcalloc(drmem_info->n_lmbs, sizeof(*lmb),
+ dinfo->lmbs = kcalloc(dinfo->n_lmbs, sizeof(*lmb),
GFP_KERNEL);
- if (!drmem_info->lmbs)
+ if (!dinfo->lmbs)
return;
- for_each_drmem_lmb(lmb)
+ for_each_dinfo_lmb(dinfo, lmb)
read_drconf_v1_cell(lmb, &prop);
}
-static void __init init_drmem_v2_lmbs(const __be32 *prop)
+static void init_drmem_v2_lmbs(const __be32 *prop,
+ struct drmem_lmb_info *dinfo)
{
struct drmem_lmb *lmb;
struct of_drconf_cell_v2 dr_cell;
@@ -386,12 +415,12 @@ static void __init init_drmem_v2_lmbs(const __be32 *prop)
p = prop;
for (i = 0; i < lmb_sets; i++) {
read_drconf_v2_cell(&dr_cell, &p);
- drmem_info->n_lmbs += dr_cell.seq_lmbs;
+ dinfo->n_lmbs += dr_cell.seq_lmbs;
}
- drmem_info->lmbs = kcalloc(drmem_info->n_lmbs, sizeof(*lmb),
+ dinfo->lmbs = kcalloc(dinfo->n_lmbs, sizeof(*lmb),
GFP_KERNEL);
- if (!drmem_info->lmbs)
+ if (!dinfo->lmbs)
return;
/* second pass, read in the LMB information */
@@ -402,10 +431,10 @@ static void __init init_drmem_v2_lmbs(const __be32 *prop)
read_drconf_v2_cell(&dr_cell, &p);
for (j = 0; j < dr_cell.seq_lmbs; j++) {
- lmb = &drmem_info->lmbs[lmb_index++];
+ lmb = &dinfo->lmbs[lmb_index++];
lmb->base_addr = dr_cell.base_addr;
- dr_cell.base_addr += drmem_info->lmb_size;
+ dr_cell.base_addr += dinfo->lmb_size;
lmb->drc_index = dr_cell.drc_index;
dr_cell.drc_index++;
@@ -416,11 +445,38 @@ static void __init init_drmem_v2_lmbs(const __be32 *prop)
}
}
+void drmem_lmbs_free(struct drmem_lmb_info *dinfo)
+{
+ if (dinfo) {
+ kfree(dinfo->lmbs);
+ kfree(dinfo);
+ }
+}
+
+struct drmem_lmb_info *drmem_lmbs_init(struct property *prop)
+{
+ struct drmem_lmb_info *dinfo;
+
+ dinfo = kzalloc(sizeof(*dinfo), GFP_KERNEL);
+ if (!dinfo)
+ return NULL;
+
+ if (!strcmp("ibm,dynamic-memory", prop->name))
+ init_drmem_v1_lmbs(prop->value, dinfo);
+ else if (!strcmp("ibm,dynamic-memory-v2", prop->name))
+ init_drmem_v2_lmbs(prop->value, dinfo);
+
+ return dinfo;
+}
+
static int __init drmem_init(void)
{
struct device_node *dn;
const __be32 *prop;
+ if (n_root_addr_cells == 0)
+ n_root_addr_cells = dt_root_addr_cells;
+
dn = of_find_node_by_path("/ibm,dynamic-reconfiguration-memory");
if (!dn) {
pr_info("No dynamic reconfiguration memory found\n");
@@ -434,11 +490,11 @@ static int __init drmem_init(void)
prop = of_get_property(dn, "ibm,dynamic-memory", NULL);
if (prop) {
- init_drmem_v1_lmbs(prop);
+ init_drmem_v1_lmbs(prop, drmem_info);
} else {
prop = of_get_property(dn, "ibm,dynamic-memory-v2", NULL);
if (prop)
- init_drmem_v2_lmbs(prop);
+ init_drmem_v2_lmbs(prop, drmem_info);
}
of_node_put(dn);
^ permalink raw reply related
* [PATCH v07 2/5] powerpc/drmem: Add internal_flags feature
From: Michael Bringmann @ 2018-10-17 19:38 UTC (permalink / raw)
To: linuxppc-dev, mwb
Cc: Juliet Kim, Thomas Falcon, Tyrel Datwyler, Nathan Fontenot
In-Reply-To: <20181017193745.4844.11915.stgit@ltcalpine2-lp9.aus.stglabs.ibm.com>
powerpc/drmem: Add internal_flags field to each LMB to allow
marking of kernel software-specific operations that need not
be exported to other users. For instance, if information about
selected LMBs needs to be maintained for subsequent passes
through the system, it can be encoded into the LMB array itself
without requiring the allocation and maintainance of additional
data structures.
Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
---
Changes in v04:
-- Add another initialization of 'lmb->internal_flags' to
init_drmem_v2_lmbs.
---
arch/powerpc/include/asm/drmem.h | 18 ++++++++++++++++++
arch/powerpc/mm/drmem.c | 3 +++
2 files changed, 21 insertions(+)
diff --git a/arch/powerpc/include/asm/drmem.h b/arch/powerpc/include/asm/drmem.h
index cfe8598..dbb3e6c 100644
--- a/arch/powerpc/include/asm/drmem.h
+++ b/arch/powerpc/include/asm/drmem.h
@@ -17,6 +17,7 @@ struct drmem_lmb {
u32 drc_index;
u32 aa_index;
u32 flags;
+ u32 internal_flags;
};
struct drmem_lmb_info {
@@ -94,6 +95,23 @@ static inline bool drmem_lmb_reserved(struct drmem_lmb *lmb)
return lmb->flags & DRMEM_LMB_RESERVED;
}
+#define DRMEM_LMBINT_UPDATE 0x00000001
+
+static inline void drmem_mark_lmb_update(struct drmem_lmb *lmb)
+{
+ lmb->internal_flags |= DRMEM_LMBINT_UPDATE;
+}
+
+static inline void drmem_remove_lmb_update(struct drmem_lmb *lmb)
+{
+ lmb->internal_flags &= ~DRMEM_LMBINT_UPDATE;
+}
+
+static inline bool drmem_lmb_update(struct drmem_lmb *lmb)
+{
+ return lmb->internal_flags & DRMEM_LMBINT_UPDATE;
+}
+
u64 drmem_lmb_memory_max(void);
void __init walk_drmem_lmbs(struct device_node *dn,
void (*func)(struct drmem_lmb *, const __be32 **));
diff --git a/arch/powerpc/mm/drmem.c b/arch/powerpc/mm/drmem.c
index ded9dbf..f199fe5 100644
--- a/arch/powerpc/mm/drmem.c
+++ b/arch/powerpc/mm/drmem.c
@@ -207,6 +207,7 @@ static void read_drconf_v1_cell(struct drmem_lmb *lmb,
lmb->aa_index = of_read_number(p++, 1);
lmb->flags = of_read_number(p++, 1);
+ lmb->internal_flags = 0;
*prop = p;
}
@@ -265,6 +266,7 @@ static void __walk_drmem_v2_lmbs(const __be32 *prop, const __be32 *usm,
lmb.aa_index = dr_cell.aa_index;
lmb.flags = dr_cell.flags;
+ lmb.internal_flags = 0;
func(&lmb, &usm);
}
@@ -441,6 +443,7 @@ static void init_drmem_v2_lmbs(const __be32 *prop,
lmb->aa_index = dr_cell.aa_index;
lmb->flags = dr_cell.flags;
+ lmb->internal_flags = 0;
}
}
}
^ permalink raw reply related
* [PATCH v07 3/5] migration/memory: Add hotplug READD_MULTIPLE
From: Michael Bringmann @ 2018-10-17 19:38 UTC (permalink / raw)
To: linuxppc-dev, mwb
Cc: Juliet Kim, Thomas Falcon, Tyrel Datwyler, Nathan Fontenot
In-Reply-To: <20181017193745.4844.11915.stgit@ltcalpine2-lp9.aus.stglabs.ibm.com>
migration/memory: This patch adds a new pseries hotplug action
for CPU and memory operations, PSERIES_HP_ELOG_ACTION_READD_MULTIPLE.
This is a variant of the READD operation which performs the action
upon multiple instances of the resource at one time. The operation
is to be triggered by device-tree analysis of updates by RTAS events
analyzed by 'migation_store' during post-migration processing. It
will be used for memory updates, initially.
Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
---
Changes in v07:
-- Provide more useful return value from dlpar_memory_readd_multiple
Changes in v05:
-- Provide dlpar_memory_readd_helper routine to compress some common code
Changes in v04:
-- Move init of 'lmb->internal_flags' in init_drmem_v2_lmbs to
previous patch.
-- Pull in implementation of dlpar_memory_readd_multiple() to go
with operation flag.
---
arch/powerpc/include/asm/rtas.h | 1
arch/powerpc/platforms/pseries/hotplug-memory.c | 47 ++++++++++++++++++++---
2 files changed, 42 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
index 0183e95..cc00451 100644
--- a/arch/powerpc/include/asm/rtas.h
+++ b/arch/powerpc/include/asm/rtas.h
@@ -333,6 +333,7 @@ struct pseries_hp_errorlog {
#define PSERIES_HP_ELOG_ACTION_ADD 1
#define PSERIES_HP_ELOG_ACTION_REMOVE 2
#define PSERIES_HP_ELOG_ACTION_READD 3
+#define PSERIES_HP_ELOG_ACTION_READD_MULTIPLE 4
#define PSERIES_HP_ELOG_ID_DRC_NAME 1
#define PSERIES_HP_ELOG_ID_DRC_INDEX 2
diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
index 2b796da..c44c6a6 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -507,6 +507,19 @@ static int dlpar_memory_remove_by_index(u32 drc_index)
return rc;
}
+static int dlpar_memory_readd_helper(struct drmem_lmb *lmb)
+{
+ int rc;
+
+ rc = dlpar_remove_lmb(lmb);
+ if (!rc) {
+ rc = dlpar_add_lmb(lmb);
+ if (rc)
+ dlpar_release_drc(lmb->drc_index);
+ }
+ return rc;
+}
+
static int dlpar_memory_readd_by_index(u32 drc_index)
{
struct drmem_lmb *lmb;
@@ -519,12 +532,7 @@ static int dlpar_memory_readd_by_index(u32 drc_index)
for_each_drmem_lmb(lmb) {
if (lmb->drc_index == drc_index) {
lmb_found = 1;
- rc = dlpar_remove_lmb(lmb);
- if (!rc) {
- rc = dlpar_add_lmb(lmb);
- if (rc)
- dlpar_release_drc(lmb->drc_index);
- }
+ rc = dlpar_memory_readd_helper(lmb);
break;
}
}
@@ -541,6 +549,26 @@ static int dlpar_memory_readd_by_index(u32 drc_index)
return rc;
}
+static int dlpar_memory_readd_multiple(void)
+{
+ struct drmem_lmb *lmb;
+ int rc = 0;
+
+ pr_info("Attempting to update multiple LMBs\n");
+
+ for_each_drmem_lmb(lmb) {
+ if (drmem_lmb_update(lmb)) {
+ rc |= dlpar_memory_readd_helper(lmb);
+ drmem_remove_lmb_update(lmb);
+ }
+ }
+
+ if (rc)
+ return -EIO;
+
+ return rc;
+}
+
static int dlpar_memory_remove_by_ic(u32 lmbs_to_remove, u32 drc_index)
{
struct drmem_lmb *lmb, *start_lmb, *end_lmb;
@@ -641,6 +669,10 @@ static int dlpar_memory_readd_by_index(u32 drc_index)
{
return -EOPNOTSUPP;
}
+static int dlpar_memory_readd_multiple(void)
+{
+ return -EOPNOTSUPP;
+}
static int dlpar_memory_remove_by_ic(u32 lmbs_to_remove, u32 drc_index)
{
@@ -918,6 +950,9 @@ int dlpar_memory(struct pseries_hp_errorlog *hp_elog)
drc_index = hp_elog->_drc_u.drc_index;
rc = dlpar_memory_readd_by_index(drc_index);
break;
+ case PSERIES_HP_ELOG_ACTION_READD_MULTIPLE:
+ rc = dlpar_memory_readd_multiple();
+ break;
default:
pr_err("Invalid action (%d) specified\n", hp_elog->action);
rc = -EINVAL;
^ permalink raw reply related
* [PATCH v07 4/5] migration/memory: Evaluate LMB assoc changes
From: Michael Bringmann @ 2018-10-17 19:38 UTC (permalink / raw)
To: linuxppc-dev, mwb
Cc: Juliet Kim, Thomas Falcon, Tyrel Datwyler, Nathan Fontenot
In-Reply-To: <20181017193745.4844.11915.stgit@ltcalpine2-lp9.aus.stglabs.ibm.com>
migration/memory: This patch adds code that recognizes changes to
the associativity of memory blocks described by the device-tree
properties in order to drive equivalent 'hotplug' operations to
update local and general kernel data structures to reflect those
changes. These differences may include:
* Evaluate 'ibm,dynamic-memory' properties when processing the
updated device-tree properties of the system during Post Migration
events (migration_store). The new functionality looks for changes
to the aa_index values for each drc_index/LMB to identify any memory
blocks that should be readded.
* In an LPAR migration scenario, the "ibm,associativity-lookup-arrays"
property may change. In the event that a row of the array differs,
locate all assigned memory blocks with that 'aa_index' and 're-add'
them to the system memory block data structures. In the process of
the 're-add', the system routines will update the corresponding entry
for the memory in the LMB structures and any other relevant kernel
data structures.
A number of previous extensions made to the DRMEM code for scanning
device-tree properties and creating LMB arrays are used here to
ensure that the resulting code is simpler and more usable:
* Use new paired list iterator for the DRMEM LMB info arrays to find
differences in old and new versions of properties.
* Use new iterator for copies of the DRMEM info arrays to evaluate
completely new structures.
* Combine common code for parsing and evaluating memory description
properties based on the DRMEM LMB array model to greatly simplify
extension from the older property 'ibm,dynamic-memory' to the new
property model of 'ibm,dynamic-memory-v2'.
For support, add a new pseries hotplug action for DLPAR operations,
PSERIES_HP_ELOG_ACTION_READD_MULTIPLE. It is a variant of the READD
operation which performs the action upon multiple instances of the
resource at one time. The operation is to be triggered by device-tree
analysis of updates by RTAS events analyzed by 'migation_store' during
post-migration processing. It will be used for memory updates,
initially.
Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
---
Changes in v06:
-- Rebase to powerpc next branch to account for recent code changes.
-- Fix prototype problem when CONFIG_MEMORY_HOTPLUG not defined.
Changes in v05:
-- Move common structure from numa.c + hotplug-memory.c to header file.
-- Clarify some comments.
-- Use walk_drmem_lmbs_pairs and callback instead of local loop
Changes in v04:
-- Move dlpar_memory_readd_multiple() function definition and use
into previous patch along with action constant definition.
-- Correct spacing in patch
Changes in v03:
-- Modify the code that parses the memory affinity attributes to
mark relevant DRMEM LMB array entries using the internal_flags
mechanism instead of generate unique hotplug actions for each
memory block to be readded. The change is intended to both
simplify the code, and to require fewer resources on systems
with huge amounts of memory.
-- Save up notice about any all LMB entries until the end of the
'migration_store' operation at which point a single action is
queued to scan the entire DRMEM array.
-- Add READD_MULTIPLE function for memory that scans the DRMEM
array to identify multiple entries that were marked previously.
The corresponding memory blocks are to be readded to the system
to update relevant data structures outside of the powerpc-
specific code.
-- Change dlpar_memory_pmt_changes_action to directly queue worker
to pseries work queue.
---
arch/powerpc/include/asm/topology.h | 7 +
arch/powerpc/mm/numa.c | 6 -
arch/powerpc/platforms/pseries/hotplug-memory.c | 207 +++++++++++++++++++----
arch/powerpc/platforms/pseries/mobility.c | 3
arch/powerpc/platforms/pseries/pseries.h | 8 +
5 files changed, 186 insertions(+), 45 deletions(-)
diff --git a/arch/powerpc/include/asm/topology.h b/arch/powerpc/include/asm/topology.h
index a4a718d..fbe03df 100644
--- a/arch/powerpc/include/asm/topology.h
+++ b/arch/powerpc/include/asm/topology.h
@@ -135,5 +135,12 @@ static inline void shared_proc_topology_init(void) {}
#endif
#endif
+
+struct assoc_arrays {
+ u32 n_arrays;
+ u32 array_sz;
+ const __be32 *arrays;
+};
+
#endif /* __KERNEL__ */
#endif /* _ASM_POWERPC_TOPOLOGY_H */
diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index 693ae1c..f1e7287 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -368,12 +368,6 @@ static unsigned long read_n_cells(int n, const __be32 **buf)
return result;
}
-struct assoc_arrays {
- u32 n_arrays;
- u32 array_sz;
- const __be32 *arrays;
-};
-
/*
* Retrieve and validate the list of associativity arrays for drconf
* memory from the ibm,associativity-lookup-arrays property of the
diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
index c44c6a6..6856010 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -537,8 +537,11 @@ static int dlpar_memory_readd_by_index(u32 drc_index)
}
}
- if (!lmb_found)
- rc = -EINVAL;
+ if (!lmb_found) {
+ pr_info("Failed to update memory for drc index %lx\n",
+ (unsigned long) drc_index);
+ return -EINVAL;
+ }
if (rc)
pr_info("Failed to update memory at %llx\n",
@@ -1001,55 +1004,173 @@ static int pseries_add_mem_node(struct device_node *np)
return (ret < 0) ? -EINVAL : 0;
}
-static int pseries_update_drconf_memory(struct of_reconfig_data *pr)
+static int pmt_changes = 0;
+
+void dlpar_memory_pmt_changes_set(void)
+{
+ pmt_changes = 1;
+}
+
+void dlpar_memory_pmt_changes_clear(void)
{
- struct of_drconf_cell_v1 *new_drmem, *old_drmem;
+ pmt_changes = 0;
+}
+
+int dlpar_memory_pmt_changes(void)
+{
+ return pmt_changes;
+}
+
+void dlpar_memory_pmt_changes_action(void)
+{
+ if (dlpar_memory_pmt_changes()) {
+ struct pseries_hp_errorlog hp_errlog;
+
+ hp_errlog.resource = PSERIES_HP_ELOG_RESOURCE_MEM;
+ hp_errlog.action = PSERIES_HP_ELOG_ACTION_READD_MULTIPLE;
+ hp_errlog.id_type = 0;
+
+ queue_hotplug_event(&hp_errlog);
+
+ dlpar_memory_pmt_changes_clear();
+ }
+}
+
+struct dlpar_memory_lmb_chk_affinity_data {
unsigned long memblock_size;
- u32 entries;
- __be32 *p;
- int i, rc = -EINVAL;
+};
+
+int dlpar_memory_lmb_chk_affinity(struct drmem_lmb *old_lmb,
+ struct drmem_lmb *new_lmb,
+ void *data)
+{
+ struct dlpar_memory_lmb_chk_affinity_data *ldata = data;
+ int rc = 1;
+
+ if (new_lmb->drc_index != old_lmb->drc_index)
+ return rc;
+
+ if ((old_lmb->flags & DRCONF_MEM_ASSIGNED) &&
+ (!(new_lmb->flags & DRCONF_MEM_ASSIGNED))) {
+ rc = pseries_remove_memblock(
+ old_lmb->base_addr,
+ ldata->memblock_size);
+ } else if ((!(old_lmb->flags & DRCONF_MEM_ASSIGNED)) &&
+ (new_lmb->flags & DRCONF_MEM_ASSIGNED)) {
+ rc = memblock_add(old_lmb->base_addr,
+ ldata->memblock_size);
+ rc = (rc < 0) ? -EINVAL : 0;
+ } else if ((old_lmb->aa_index != new_lmb->aa_index) &&
+ (new_lmb->flags & DRCONF_MEM_ASSIGNED)) {
+ drmem_mark_lmb_update(old_lmb);
+ dlpar_memory_pmt_changes_set();
+ }
+
+ return rc;
+}
+
+static int pseries_update_drconf_memory(struct drmem_lmb_info *new_dinfo)
+{
+ struct dlpar_memory_lmb_chk_affinity_data data;
+ int rc = 0;
if (rtas_hp_event)
return 0;
- memblock_size = pseries_memory_block_size();
- if (!memblock_size)
+ data.memblock_size = pseries_memory_block_size();
+ if (!data.memblock_size)
return -EINVAL;
+ /* Arrays should have the same size and DRC indexes */
+ rc = walk_drmem_lmbs_pairs(new_dinfo,
+ dlpar_memory_lmb_chk_affinity, &data);
+ return rc;
+}
+
+static void pseries_update_ala_memory_aai(int aa_index)
+{
+ struct drmem_lmb *lmb;
+
+ /* Readd all LMBs which were previously using the
+ * specified aa_index value.
+ */
+ for_each_drmem_lmb(lmb) {
+ if ((lmb->aa_index == aa_index) &&
+ (lmb->flags & DRCONF_MEM_ASSIGNED)) {
+ drmem_mark_lmb_update(lmb);
+ dlpar_memory_pmt_changes_set();
+ }
+ }
+}
+
+static int pseries_update_ala_memory(struct of_reconfig_data *pr)
+{
+ struct assoc_arrays new_ala, old_ala;
+ __be32 *p;
+ int i, lim;
+
+ if (rtas_hp_event)
+ return 0;
+
+ /*
+ * The layout of the ibm,associativity-lookup-arrays
+ * property is a number N indicating the number of
+ * associativity arrays, followed by a number M
+ * indicating the size of each associativity array,
+ * followed by a list of N associativity arrays.
+ */
+
p = (__be32 *) pr->old_prop->value;
if (!p)
return -EINVAL;
+ old_ala.n_arrays = of_read_number(p++, 1);
+ old_ala.array_sz = of_read_number(p++, 1);
+ old_ala.arrays = p;
- /* The first int of the property is the number of lmb's described
- * by the property. This is followed by an array of of_drconf_cell
- * entries. Get the number of entries and skip to the array of
- * of_drconf_cell's.
- */
- entries = be32_to_cpu(*p++);
- old_drmem = (struct of_drconf_cell_v1 *)p;
-
- p = (__be32 *)pr->prop->value;
- p++;
- new_drmem = (struct of_drconf_cell_v1 *)p;
-
- for (i = 0; i < entries; i++) {
- if ((be32_to_cpu(old_drmem[i].flags) & DRCONF_MEM_ASSIGNED) &&
- (!(be32_to_cpu(new_drmem[i].flags) & DRCONF_MEM_ASSIGNED))) {
- rc = pseries_remove_memblock(
- be64_to_cpu(old_drmem[i].base_addr),
- memblock_size);
- break;
- } else if ((!(be32_to_cpu(old_drmem[i].flags) &
- DRCONF_MEM_ASSIGNED)) &&
- (be32_to_cpu(new_drmem[i].flags) &
- DRCONF_MEM_ASSIGNED)) {
- rc = memblock_add(be64_to_cpu(old_drmem[i].base_addr),
- memblock_size);
- rc = (rc < 0) ? -EINVAL : 0;
- break;
+ p = (__be32 *) pr->prop->value;
+ if (!p)
+ return -EINVAL;
+ new_ala.n_arrays = of_read_number(p++, 1);
+ new_ala.array_sz = of_read_number(p++, 1);
+ new_ala.arrays = p;
+
+ lim = (new_ala.n_arrays > old_ala.n_arrays) ? old_ala.n_arrays :
+ new_ala.n_arrays;
+
+ if (old_ala.array_sz == new_ala.array_sz) {
+
+ /* Iterate comparing rows of the old and new
+ * ibm,associativity-lookup-arrays looking for
+ * changes. If a row has changed, then mark all
+ * memory blocks using that index for readd.
+ */
+ for (i = 0; i < lim; i++) {
+ int index = (i * new_ala.array_sz);
+
+ if (!memcmp(&old_ala.arrays[index],
+ &new_ala.arrays[index],
+ new_ala.array_sz))
+ continue;
+
+ pseries_update_ala_memory_aai(i);
}
+
+ /* Reset any entries representing the extra rows.
+ * There shouldn't be any, but just in case ...
+ */
+ for (i = lim; i < new_ala.n_arrays; i++)
+ pseries_update_ala_memory_aai(i);
+
+ } else {
+ /* Update all entries representing these rows;
+ * as all rows have different sizes, none can
+ * have equivalent values.
+ */
+ for (i = 0; i < lim; i++)
+ pseries_update_ala_memory_aai(i);
}
- return rc;
+
+ return 0;
}
static int pseries_memory_notifier(struct notifier_block *nb,
@@ -1066,8 +1187,16 @@ static int pseries_memory_notifier(struct notifier_block *nb,
err = pseries_remove_mem_node(rd->dn);
break;
case OF_RECONFIG_UPDATE_PROPERTY:
- if (!strcmp(rd->prop->name, "ibm,dynamic-memory"))
- err = pseries_update_drconf_memory(rd);
+ if (!strcmp(rd->prop->name, "ibm,dynamic-memory")) {
+ struct drmem_lmb_info *dinfo =
+ drmem_lmbs_init(rd->prop);
+ if (!dinfo)
+ return -EINVAL;
+ err = pseries_update_drconf_memory(dinfo);
+ drmem_lmbs_free(dinfo);
+ } else if (!strcmp(rd->prop->name,
+ "ibm,associativity-lookup-arrays"))
+ err = pseries_update_ala_memory(rd);
break;
}
return notifier_from_errno(err);
diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
index 7da222d..2557a42 100644
--- a/arch/powerpc/platforms/pseries/mobility.c
+++ b/arch/powerpc/platforms/pseries/mobility.c
@@ -380,6 +380,9 @@ static ssize_t migration_store(struct class *class,
post_mobility_fixup();
+ /* Apply any necessary changes identified during fixup */
+ dlpar_memory_pmt_changes_action();
+
start_topology_update();
return count;
diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h
index 72c0b89..0c33f79 100644
--- a/arch/powerpc/platforms/pseries/pseries.h
+++ b/arch/powerpc/platforms/pseries/pseries.h
@@ -63,13 +63,21 @@ extern struct device_node *dlpar_configure_connector(__be32,
void queue_hotplug_event(struct pseries_hp_errorlog *hp_errlog);
int handle_dlpar_errorlog(struct pseries_hp_errorlog *hp_errlog);
+void dlpar_memory_pmt_changes_set(void);
+void dlpar_memory_pmt_changes_clear(void);
+int dlpar_memory_pmt_changes(void);
+
#ifdef CONFIG_MEMORY_HOTPLUG
int dlpar_memory(struct pseries_hp_errorlog *hp_elog);
+void dlpar_memory_pmt_changes_action(void);
#else
static inline int dlpar_memory(struct pseries_hp_errorlog *hp_elog)
{
return -EOPNOTSUPP;
}
+static inline void dlpar_memory_pmt_changes_action(void)
+{
+}
#endif
#ifdef CONFIG_HOTPLUG_CPU
^ permalink raw reply related
* [PATCH v07 5/5] migration/memory: Support 'ibm,dynamic-memory-v2'
From: Michael Bringmann @ 2018-10-17 19:38 UTC (permalink / raw)
To: linuxppc-dev, mwb
Cc: Juliet Kim, Thomas Falcon, Tyrel Datwyler, Nathan Fontenot
In-Reply-To: <20181017193745.4844.11915.stgit@ltcalpine2-lp9.aus.stglabs.ibm.com>
migration/memory: This patch adds recognition for changes to the
associativity of memory blocks described by 'ibm,dynamic-memory-v2'.
If the associativity of an LMB has changed, it should be readded to
the system in order to update local and general kernel data structures.
This patch builds upon previous enhancements that scan the device-tree
"ibm,dynamic-memory" properties using the base LMB array, and a copy
derived from the updated properties.
Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
---
arch/powerpc/platforms/pseries/hotplug-memory.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
index 6856010..03c5e49 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -1187,7 +1187,8 @@ static int pseries_memory_notifier(struct notifier_block *nb,
err = pseries_remove_mem_node(rd->dn);
break;
case OF_RECONFIG_UPDATE_PROPERTY:
- if (!strcmp(rd->prop->name, "ibm,dynamic-memory")) {
+ if (!strcmp(rd->prop->name, "ibm,dynamic-memory") ||
+ !strcmp(rd->prop->name, "ibm,dynamic-memory-v2")) {
struct drmem_lmb_info *dinfo =
drmem_lmbs_init(rd->prop);
if (!dinfo)
^ 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