* Re: [PATCH v2] powerpc/hugetlb: Fix 8M hugepages on 8xx
From: Michael Ellerman @ 2020-02-19 12:39 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
aneesh.kumar
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <778b1a248c4c7ca79640eeff7740044da6a220a0.1581264115.git.christophe.leroy@c-s.fr>
On Sun, 2020-02-09 at 16:02:41 UTC, Christophe Leroy wrote:
> With HW assistance all page tables must be 4k aligned, the 8xx
> drops the last 12 bits during the walk.
>
> Redefine HUGEPD_SHIFT_MASK to mask last 12 bits out.
> HUGEPD_SHIFT_MASK is used to for alignment of page table cache.
>
> Fixes: 22569b881d37 ("powerpc/8xx: Enable 8M hugepage support with HW assistance")
> Cc: stable@vger.kernel.org
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Applied to powerpc fixes, thanks.
https://git.kernel.org/powerpc/c/50a175dd18de7a647e72aca7daf4744e3a5a81e3
cheers
^ permalink raw reply
* Re: [PATCH 1/1] powerpc/eeh: fix deadlock handling dead PHB
From: Michael Ellerman @ 2020-02-19 12:39 UTC (permalink / raw)
To: Sam Bobroff, linuxppc-dev; +Cc: fbarrat
In-Reply-To: <0547e82dbf90ee0729a2979a8cac5c91665c621f.1581051445.git.sbobroff@linux.ibm.com>
On Fri, 2020-02-07 at 04:57:31 UTC, Sam Bobroff wrote:
> Recovering a dead PHB can currently cause a deadlock as the PCI
> rescan/remove lock is taken twice.
>
> This is caused as part of an existing bug in
> eeh_handle_special_event(). The pe is processed while traversing the
> PHBs even though the pe is unrelated to the loop. This causes the pe
> to be, incorrectly, processed more than once.
>
> Untangling this section can move the pe processing out of the loop and
> also outside the locked section, correcting both problems.
>
> Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
Applied to powerpc fixes, thanks.
https://git.kernel.org/powerpc/c/d4f194ed9eb9841a8f978710e4d24296f791a85b
cheers
^ permalink raw reply
* Re: [PATCH] powerpc/hugetlb: Fix 512k hugepages on 8xx with 16k page size
From: Michael Ellerman @ 2020-02-19 12:39 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
aneesh.kumar
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <90ec56a2315be602494619ed0223bba3b0b8d619.1580997007.git.christophe.leroy@c-s.fr>
On Thu, 2020-02-06 at 13:50:28 UTC, Christophe Leroy wrote:
> Commit 55c8fc3f4930 ("powerpc/8xx: reintroduce 16K pages with HW
> assistance") redefined pte_t as a struct of 4 pte_basic_t, because
> in 16K pages mode there are four identical entries in the
> page table. But the size of hugepage tables is calculated based
> of the size of (void *). Therefore, we end up with page tables
> of size 1k instead of 4k for 512k pages.
>
> As 512k hugepage tables are the same size as standard page tables,
> ie 4k, use the standard page tables instead of PGT_CACHE tables.
>
> Fixes: 3fb69c6a1a13 ("powerpc/8xx: Enable 512k hugepage support with HW assistance")
> Cc: stable@vger.kernel.org
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Applied to powerpc fixes, thanks.
https://git.kernel.org/powerpc/c/f2b67ef90b0d5eca0f2255e02cf2f620bc0ddcdb
cheers
^ permalink raw reply
* Re: [PATCH v2 07/13] powerpc: add support for folded p4d page tables
From: Christophe Leroy @ 2020-02-19 12:07 UTC (permalink / raw)
To: Mike Rapoport
Cc: Rich Felker, linux-ia64, Geert Uytterhoeven, linux-sh, linux-mm,
Paul Mackerras, linux-hexagon, Will Deacon, kvmarm, Jonas Bonn,
linux-arch, Brian Cain, Marc Zyngier, Russell King, Ley Foon Tan,
Mike Rapoport, Catalin Marinas, Julien Thierry, uclinux-h8-devel,
Fenghua Yu, Arnd Bergmann, Suzuki K Poulose, kvm-ppc,
Stefan Kristiansson, openrisc, Stafford Horne, Guan Xuetao,
linux-arm-kernel, Tony Luck, Yoshinori Sato, linux-kernel,
James Morse, nios2-dev, Andrew Morton, linuxppc-dev
In-Reply-To: <20200216081843.28670-8-rppt@kernel.org>
Le 16/02/2020 à 09:18, Mike Rapoport a écrit :
> From: Mike Rapoport <rppt@linux.ibm.com>
>
> Implement primitives necessary for the 4th level folding, add walks of p4d
> level where appropriate and replace 5level-fixup.h with pgtable-nop4d.h.
>
> Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
> Tested-by: Christophe Leroy <christophe.leroy@c-s.fr> # 8xx and 83xx
> ---
> arch/powerpc/include/asm/book3s/32/pgtable.h | 1 -
> arch/powerpc/include/asm/book3s/64/hash.h | 4 +-
> arch/powerpc/include/asm/book3s/64/pgalloc.h | 4 +-
> arch/powerpc/include/asm/book3s/64/pgtable.h | 58 ++++++++++--------
> arch/powerpc/include/asm/book3s/64/radix.h | 6 +-
> arch/powerpc/include/asm/nohash/32/pgtable.h | 1 -
> arch/powerpc/include/asm/nohash/64/pgalloc.h | 2 +-
> .../include/asm/nohash/64/pgtable-4k.h | 32 +++++-----
> arch/powerpc/include/asm/nohash/64/pgtable.h | 6 +-
> arch/powerpc/include/asm/pgtable.h | 8 +++
> arch/powerpc/kvm/book3s_64_mmu_radix.c | 59 ++++++++++++++++---
> arch/powerpc/lib/code-patching.c | 7 ++-
> arch/powerpc/mm/book3s32/mmu.c | 2 +-
> arch/powerpc/mm/book3s32/tlb.c | 4 +-
> arch/powerpc/mm/book3s64/hash_pgtable.c | 4 +-
> arch/powerpc/mm/book3s64/radix_pgtable.c | 19 ++++--
> arch/powerpc/mm/book3s64/subpage_prot.c | 6 +-
> arch/powerpc/mm/hugetlbpage.c | 28 +++++----
> arch/powerpc/mm/kasan/kasan_init_32.c | 8 +--
> arch/powerpc/mm/mem.c | 4 +-
> arch/powerpc/mm/nohash/40x.c | 4 +-
> arch/powerpc/mm/nohash/book3e_pgtable.c | 15 +++--
> arch/powerpc/mm/pgtable.c | 25 +++++++-
> arch/powerpc/mm/pgtable_32.c | 28 +++++----
> arch/powerpc/mm/pgtable_64.c | 10 ++--
> arch/powerpc/mm/ptdump/hashpagetable.c | 20 ++++++-
> arch/powerpc/mm/ptdump/ptdump.c | 22 ++++++-
> arch/powerpc/xmon/xmon.c | 17 +++++-
> 28 files changed, 284 insertions(+), 120 deletions(-)
>
> diff --git a/arch/powerpc/mm/ptdump/ptdump.c b/arch/powerpc/mm/ptdump/ptdump.c
> index 206156255247..7bd4b81d5b5d 100644
> --- a/arch/powerpc/mm/ptdump/ptdump.c
> +++ b/arch/powerpc/mm/ptdump/ptdump.c
> @@ -277,9 +277,9 @@ static void walk_pmd(struct pg_state *st, pud_t *pud, unsigned long start)
> }
> }
>
> -static void walk_pud(struct pg_state *st, pgd_t *pgd, unsigned long start)
> +static void walk_pud(struct pg_state *st, p4d_t *p4d, unsigned long start)
> {
> - pud_t *pud = pud_offset(pgd, 0);
> + pud_t *pud = pud_offset(p4d, 0);
> unsigned long addr;
> unsigned int i;
>
> @@ -293,6 +293,22 @@ static void walk_pud(struct pg_state *st, pgd_t *pgd, unsigned long start)
> }
> }
>
> +static void walk_p4d(struct pg_state *st, pgd_t *pgd, unsigned long start)
> +{
> + p4d_t *p4d = p4d_offset(pgd, 0);
> + unsigned long addr;
> + unsigned int i;
> +
> + for (i = 0; i < PTRS_PER_P4D; i++, p4d++) {
> + addr = start + i * P4D_SIZE;
> + if (!p4d_none(*p4d) && !p4d_is_leaf(*p4d))
> + /* p4d exists */
> + walk_pud(st, p4d, addr);
> + else
> + note_page(st, addr, 2, p4d_val(*p4d));
Level 2 is already used by walk_pud().
I think you have to increment the level used in walk_pud() and
walk_pmd() and walk_pte()
> + }
> +}
> +
> static void walk_pagetables(struct pg_state *st)
> {
> unsigned int i;
> @@ -306,7 +322,7 @@ static void walk_pagetables(struct pg_state *st)
> for (i = pgd_index(addr); i < PTRS_PER_PGD; i++, pgd++, addr += PGDIR_SIZE) {
> if (!pgd_none(*pgd) && !pgd_is_leaf(*pgd))
> /* pgd exists */
> - walk_pud(st, pgd, addr);
> + walk_p4d(st, pgd, addr);
> else
> note_page(st, addr, 1, pgd_val(*pgd));
> }
Christophe
^ permalink raw reply
* Re: powerpc Linux scv support and scv system call ABI proposal
From: Nicholas Piggin @ 2020-02-19 11:03 UTC (permalink / raw)
To: Adhemerval Zanella, Libc-alpha Mailing List, linuxppc-dev,
Tulio Magno Quites Machado Filho
In-Reply-To: <874kwe8dm7.fsf@linux.ibm.com>
Tulio Magno Quites Machado Filho's on January 30, 2020 1:51 am:
> Nicholas Piggin <npiggin@gmail.com> writes:
>
>> Adhemerval Zanella's on January 29, 2020 3:26 am:
>>>
>>> We already had to push a similar hack where glibc used to abort transactions
>>> prior syscalls to avoid some side-effects on kernel (commit 56cf2763819d2f).
>>> It was eventually removed from syscall handling by f0458cf4f9ff3d870, where
>>> we only enable TLE if kernel suppors PPC_FEATURE2_HTM_NOSC.
>>>
>>> The transaction syscall abort used to read a variable directly from TCB,
>>> so this could be an option. I would expect that we could optimize it where
>>> if glibc is building against a recent kernel and compiler is building
>>> for a ISA 3.0+ cpu we could remove the 'sc' code.
>>>
>>
>> We would just have to be careful of running on ISA 3.0 CPUs on older
>> kernels which do not support scv.
>
> Can we assume that, if a syscall is available through sc it's also available
> in scv 0?
Was on vacation, thanks for waiting.
Yes, except for the difference in calling convention, we would require
that the syscalls available to `sc` is exactly the same as `scv 0`. This
happens as a natural consequence of the kernel implementation which
re-uses code to select the syscall.
>
> Because if that's true, I believe your suggestion to interpret PPC_FEATURE2_SCV
> as scv 0 support would be helpful to provide this support via IFUNC even
> when glibc is built using --with-cpu=power8, which is the most common scenario
> in ppc64le.
>
> In that scenario, it seems new HWCAP bits for new vectors wouldn't be too
> frequent, which was the only downside of this proposal.
Okay good feedback, thanks.
Thanks,
Nick
^ permalink raw reply
* [PATCH] powerpc/xmon: Lower limits on nidump and ndump
From: Michael Ellerman @ 2020-02-19 11:00 UTC (permalink / raw)
To: linuxppc-dev
In xmon we have two variables that are used by the dump commands.
There's ndump which is the number of bytes to dump using 'd', and
nidump which is the number of instructions to dump using 'di'.
ndump starts as 64 and nidump starts as 16, but both can be set by the
user.
It's fairly common to be pasting addresses into xmon when trying to
debug something, and if you inadvertently double paste an address like
so:
0:mon> di c000000002101f6c c000000002101f6c
The second value is interpreted as the number of instructions to dump.
Luckily it doesn't dump 13 quintrillion instructions, the value is
limited to MAX_DUMP (128K). But as each instruction is dumped on a
single line, that's still a lot of output. If you're on a slow console
that can take multiple minutes to print. If you were "just popping in
and out of xmon quickly before the RCU/hardlockup detector fires" you
are now having a bad day.
Things are not as bad with 'd' because we print 16 bytes per line, so
it's fewer lines. But it's still quite a lot.
So shrink the maximum for 'd' to 64K (one page), which is 4096 lines.
For 'di' add a new limit which is the above / 4 - because instructions
are 4 bytes, meaning again we can dump one page.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/xmon/xmon.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index e8c84d265602..722bf7ed0eda 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -81,8 +81,9 @@ static bool xmon_is_ro = IS_ENABLED(CONFIG_XMON_DEFAULT_RO_MODE);
static unsigned long adrs;
static int size = 1;
-#define MAX_DUMP (128 * 1024)
+#define MAX_DUMP (64 * 1024)
static unsigned long ndump = 64;
+#define MAX_IDUMP (MAX_DUMP >> 2)
static unsigned long nidump = 16;
static unsigned long ncsum = 4096;
static int termch;
@@ -2756,8 +2757,8 @@ dump(void)
scanhex(&nidump);
if (nidump == 0)
nidump = 16;
- else if (nidump > MAX_DUMP)
- nidump = MAX_DUMP;
+ else if (nidump > MAX_IDUMP)
+ nidump = MAX_IDUMP;
adrs += ppc_inst_dump(adrs, nidump, 1);
last_cmd = "di\n";
} else if (c == 'l') {
--
2.21.1
^ permalink raw reply related
* Re: Surprising code generated for vdso_read_begin()
From: Arnd Bergmann @ 2020-02-19 9:52 UTC (permalink / raw)
To: Christophe Leroy
Cc: the arch/x86 maintainers, linux-kernel@vger.kernel.org,
open list:BROADCOM NVRAM DRIVER, Paul Mackerras, Andy Lutomirski,
Thomas Gleixner, Vincenzo Frascino, linuxppc-dev, Linux ARM
In-Reply-To: <305fcee5-2e1b-ea4d-9a2a-a0e8034d40a8@c-s.fr>
On Wed, Feb 19, 2020 at 9:45 AM Christophe Leroy
<christophe.leroy@c-s.fr> wrote:
> Le 16/02/2020 à 19:10, Arnd Bergmann a écrit :
> > On Sat, Jan 11, 2020 at 12:33 PM Segher Boessenkool
> > <segher@kernel.crashing.org> wrote:
> >>
> >> On Fri, Jan 10, 2020 at 07:45:44AM +0100, Christophe Leroy wrote:
> >>> Le 09/01/2020 à 21:07, Segher Boessenkool a écrit :
> >>>> It looks like the compiler did loop peeling. What GCC version is this?
> >>>> Please try current trunk (to become GCC 10), or at least GCC 9?
> >>>
> >>> It is with GCC 5.5
> >>>
> >>> https://mirrors.edge.kernel.org/pub/tools/crosstool/ doesn't have more
> >>> recent than 8.1
> >>
> >> Arnd, can you update the tools? We are at 8.3 and 9.2 now :-) Or is
> >> this hard and/or painful to do?
> >
> > To follow up on this older thread, I have now uploaded 6.5, 7.5, 8.3 and 9.2
> > binaries, as well as a recent 10.0 snapshot.
> >
>
> Thanks Arnd,
>
> I have built the VDSO with 9.2, I get less performant result than with
> 8.2 (same performance as with 5.5).
>
> After a quick look, I see:
> - Irrelevant NOPs to align loops and stuff, allthough -mpcu=860 should
> avoid that.
> - A stack frame is set for saving r31 in __c_kernel_clock_gettime. GCC
> 8.1 don't need that, all VDSO functions are frameless with 8.1
If you think it should be fixed in gcc, maybe try to reproduce it in
https://godbolt.org/ and open a gcc bug against that.
Also, please try the gcc-10 snapshot, which has the highest chance
of getting fixes if it shows the same issue (or worse).
Arnd
^ permalink raw reply
* Re: Surprising code generated for vdso_read_begin()
From: Christophe Leroy @ 2020-02-19 8:45 UTC (permalink / raw)
To: Arnd Bergmann, Segher Boessenkool
Cc: the arch/x86 maintainers, linux-kernel@vger.kernel.org,
open list:BROADCOM NVRAM DRIVER, Paul Mackerras, Andy Lutomirski,
Thomas Gleixner, Vincenzo Frascino, linuxppc-dev, Linux ARM
In-Reply-To: <CAK8P3a11wX1zJ+TAacDTkYsrzvfdVmNrcB6OC23aFvCxF57opQ@mail.gmail.com>
Le 16/02/2020 à 19:10, Arnd Bergmann a écrit :
> On Sat, Jan 11, 2020 at 12:33 PM Segher Boessenkool
> <segher@kernel.crashing.org> wrote:
>>
>> On Fri, Jan 10, 2020 at 07:45:44AM +0100, Christophe Leroy wrote:
>>> Le 09/01/2020 à 21:07, Segher Boessenkool a écrit :
>>>> It looks like the compiler did loop peeling. What GCC version is this?
>>>> Please try current trunk (to become GCC 10), or at least GCC 9?
>>>
>>> It is with GCC 5.5
>>>
>>> https://mirrors.edge.kernel.org/pub/tools/crosstool/ doesn't have more
>>> recent than 8.1
>>
>> Arnd, can you update the tools? We are at 8.3 and 9.2 now :-) Or is
>> this hard and/or painful to do?
>
> To follow up on this older thread, I have now uploaded 6.5, 7.5, 8.3 and 9.2
> binaries, as well as a recent 10.0 snapshot.
>
Thanks Arnd,
I have built the VDSO with 9.2, I get less performant result than with
8.2 (same performance as with 5.5).
After a quick look, I see:
- Irrelevant NOPs to align loops and stuff, allthough -mpcu=860 should
avoid that.
- A stack frame is set for saving r31 in __c_kernel_clock_gettime. GCC
8.1 don't need that, all VDSO functions are frameless with 8.1
Christophe
^ permalink raw reply
* [PATCH] powerpc/kprobes: Blacklist functions running with MMU disabled on PPC32
From: Christophe Leroy @ 2020-02-19 8:10 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
naveen.n.rao
Cc: linuxppc-dev, linux-kernel
kprobe does not handle events happening in real mode, all
functions running with MMU disabled have to be blacklisted.
As already done for PPC64, do it for PPC32.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/include/asm/ppc_asm.h | 10 +++
arch/powerpc/kernel/cpu_setup_6xx.S | 4 +-
arch/powerpc/kernel/entry_32.S | 68 ++++++++------------
arch/powerpc/kernel/fpu.S | 1 +
arch/powerpc/kernel/idle_6xx.S | 2 +-
arch/powerpc/kernel/idle_e500.S | 2 +-
arch/powerpc/kernel/l2cr_6xx.S | 2 +-
arch/powerpc/kernel/misc.S | 2 +
arch/powerpc/kernel/misc_32.S | 4 +-
arch/powerpc/kernel/swsusp_32.S | 6 +-
arch/powerpc/kernel/vector.S | 1 +
arch/powerpc/mm/book3s32/hash_low.S | 38 +++++------
arch/powerpc/mm/mem.c | 2 +
arch/powerpc/platforms/52xx/lite5200_sleep.S | 2 +
arch/powerpc/platforms/82xx/pq2.c | 1 +
arch/powerpc/platforms/83xx/suspend-asm.S | 1 +
arch/powerpc/platforms/powermac/cache.S | 2 +
arch/powerpc/platforms/powermac/sleep.S | 13 ++--
18 files changed, 85 insertions(+), 76 deletions(-)
diff --git a/arch/powerpc/include/asm/ppc_asm.h b/arch/powerpc/include/asm/ppc_asm.h
index 6b03dff61a05..e8f34ba89497 100644
--- a/arch/powerpc/include/asm/ppc_asm.h
+++ b/arch/powerpc/include/asm/ppc_asm.h
@@ -267,8 +267,18 @@ GLUE(.,name):
.pushsection "_kprobe_blacklist","aw"; \
PPC_LONG (entry) ; \
.popsection
+#define _NOKPROBE_ENTRY(entry) \
+ _ASM_NOKPROBE_SYMBOL(entry) \
+ _ENTRY(entry)
+#define _NOKPROBE_GLOBAL(entry) \
+ _ASM_NOKPROBE_SYMBOL(entry) \
+ _GLOBAL(entry)
#else
#define _ASM_NOKPROBE_SYMBOL(entry)
+#define _NOKPROBE_ENTRY(entry) \
+ _ENTRY(entry)
+#define _NOKPROBE_GLOBAL(entry) \
+ _GLOBAL(entry)
#endif
#define FUNC_START(name) _GLOBAL(name)
diff --git a/arch/powerpc/kernel/cpu_setup_6xx.S b/arch/powerpc/kernel/cpu_setup_6xx.S
index f6517f67265a..1cb947268546 100644
--- a/arch/powerpc/kernel/cpu_setup_6xx.S
+++ b/arch/powerpc/kernel/cpu_setup_6xx.S
@@ -276,7 +276,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_NO_DPM)
* in some 750 cpus where using a not yet initialized FPU register after
* power on reset may hang the CPU
*/
-_GLOBAL(__init_fpu_registers)
+_NOKPROBE_GLOBAL(__init_fpu_registers)
mfmsr r10
ori r11,r10,MSR_FP
mtmsr r11
@@ -381,7 +381,7 @@ _GLOBAL(__save_cpu_setup)
* restore CPU state as backed up by the previous
* function. This does not include cache setting
*/
-_GLOBAL(__restore_cpu_setup)
+_NOKPROBE_GLOBAL(__restore_cpu_setup)
/* Some CR fields are volatile, we back it up all */
mfcr r7
diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
index 0713daa651d9..cf9a7640abf0 100644
--- a/arch/powerpc/kernel/entry_32.S
+++ b/arch/powerpc/kernel/entry_32.S
@@ -44,24 +44,21 @@
.align 12
#ifdef CONFIG_BOOKE
- .globl mcheck_transfer_to_handler
-mcheck_transfer_to_handler:
+_NOKPROBE_ENTRY(mcheck_transfer_to_handler)
mfspr r0,SPRN_DSRR0
stw r0,_DSRR0(r11)
mfspr r0,SPRN_DSRR1
stw r0,_DSRR1(r11)
/* fall through */
- .globl debug_transfer_to_handler
-debug_transfer_to_handler:
+_NOKPROBE_ENTRY(debug_transfer_to_handler)
mfspr r0,SPRN_CSRR0
stw r0,_CSRR0(r11)
mfspr r0,SPRN_CSRR1
stw r0,_CSRR1(r11)
/* fall through */
- .globl crit_transfer_to_handler
-crit_transfer_to_handler:
+_NOKPROBE_ENTRY(crit_transfer_to_handler)
#ifdef CONFIG_PPC_BOOK3E_MMU
mfspr r0,SPRN_MAS0
stw r0,MAS0(r11)
@@ -97,8 +94,7 @@ crit_transfer_to_handler:
#endif
#ifdef CONFIG_40x
- .globl crit_transfer_to_handler
-crit_transfer_to_handler:
+_NOKPROBE_ENTRY(crit_transfer_to_handler)
lwz r0,crit_r10@l(0)
stw r0,GPR10(r11)
lwz r0,crit_r11@l(0)
@@ -124,13 +120,11 @@ crit_transfer_to_handler:
* Note that we rely on the caller having set cr0.eq iff the exception
* occurred in kernel mode (i.e. MSR:PR = 0).
*/
- .globl transfer_to_handler_full
-transfer_to_handler_full:
+_NOKPROBE_ENTRY(transfer_to_handler_full)
SAVE_NVGPRS(r11)
/* fall through */
- .globl transfer_to_handler
-transfer_to_handler:
+_NOKPROBE_ENTRY(transfer_to_handler)
stw r2,GPR2(r11)
stw r12,_NIP(r11)
stw r9,_MSR(r11)
@@ -194,8 +188,7 @@ transfer_to_handler:
bt- 31-TLF_NAPPING,4f
bt- 31-TLF_SLEEPING,7f
#endif /* CONFIG_PPC_BOOK3S_32 || CONFIG_E500 */
- .globl transfer_to_handler_cont
-transfer_to_handler_cont:
+_NOKPROBE_ENTRY(transfer_to_handler_cont)
3:
mflr r9
tovirt_novmstack r2, r2 /* set r2 to current */
@@ -297,6 +290,7 @@ reenable_mmu:
* On kernel stack overflow, load up an initial stack pointer
* and call StackOverflow(regs), which should not return.
*/
+_ASM_NOKPROBE_SYMBOL(stack_ovf)
stack_ovf:
/* sometimes we use a statically-allocated stack, which is OK. */
lis r12,_end@h
@@ -460,6 +454,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_NEED_PAIRED_STWCX)
lwz r7,_NIP(r1)
lwz r2,GPR2(r1)
lwz r1,GPR1(r1)
+syscall_exit_finish:
#if defined(CONFIG_PPC_8xx) && defined(CONFIG_PERF_EVENTS)
mtspr SPRN_NRI, r0
#endif
@@ -467,6 +462,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_NEED_PAIRED_STWCX)
mtspr SPRN_SRR1,r8
SYNC
RFI
+_ASM_NOKPROBE_SYMBOL(syscall_exit_finish)
#ifdef CONFIG_44x
2: li r7,0
iccci r0,r0
@@ -750,8 +746,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_SPE)
addi r1,r1,INT_FRAME_SIZE
blr
- .globl fast_exception_return
-fast_exception_return:
+_NOKPROBE_ENTRY(fast_exception_return)
#if !(defined(CONFIG_4xx) || defined(CONFIG_BOOKE))
andi. r10,r9,MSR_RI /* check for recoverable interrupt */
beq 1f /* if not, we've got problems */
@@ -780,8 +775,8 @@ fast_exception_return:
#if !(defined(CONFIG_4xx) || defined(CONFIG_BOOKE))
/* check if the exception happened in a restartable section */
-1: lis r3,exc_exit_restart_end@ha
- addi r3,r3,exc_exit_restart_end@l
+1: lis r3,.Lexc_exit_restart_end@ha
+ addi r3,r3,.Lexc_exit_restart_end@l
cmplw r12,r3
#if CONFIG_PPC_BOOK3S_601
bge 2b
@@ -983,7 +978,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_NEED_PAIRED_STWCX)
#if !(defined(CONFIG_4xx) || defined(CONFIG_BOOKE))
andi. r10,r9,MSR_RI /* check if this exception occurred */
- beql nonrecoverable /* at a bad place (MSR:RI = 0) */
+ beql .Lnonrecoverable /* at a bad place (MSR:RI = 0) */
lwz r10,_CCR(r1)
lwz r11,_LINK(r1)
@@ -1005,15 +1000,13 @@ END_FTR_SECTION_IFSET(CPU_FTR_NEED_PAIRED_STWCX)
LOAD_REG_IMMEDIATE(r10,MSR_KERNEL & ~MSR_RI)
SYNC
mtmsr r10 /* clear the RI bit */
- .globl exc_exit_restart
-exc_exit_restart:
+_NOKPROBE_ENTRY(exc_exit_restart)
lwz r12,_NIP(r1)
mtspr SPRN_SRR0,r12
mtspr SPRN_SRR1,r9
REST_4GPRS(9, r1)
lwz r1,GPR1(r1)
- .globl exc_exit_restart_end
-exc_exit_restart_end:
+.Lexc_exit_restart_end:
SYNC
RFI
@@ -1033,17 +1026,15 @@ exc_exit_restart_end:
li r10, 0
stw r10, 8(r1)
REST_2GPRS(9, r1)
- .globl exc_exit_restart
+_NOKPROBE_ENTRY(exc_exit_restart)
exc_exit_restart:
lwz r11,_NIP(r1)
lwz r12,_MSR(r1)
-exc_exit_start:
mtspr SPRN_SRR0,r11
mtspr SPRN_SRR1,r12
REST_2GPRS(11, r1)
lwz r1,GPR1(r1)
- .globl exc_exit_restart_end
-exc_exit_restart_end:
+.Lexc_exit_restart_end:
PPC405_ERR77_SYNC
rfi
b . /* prevent prefetch past rfi */
@@ -1146,8 +1137,7 @@ exc_exit_restart_end:
#endif
#ifdef CONFIG_40x
- .globl ret_from_crit_exc
-ret_from_crit_exc:
+_NOKPROBE_ENTRY(ret_from_crit_exc)
mfspr r9,SPRN_SPRG_THREAD
lis r10,saved_ksp_limit@ha;
lwz r10,saved_ksp_limit@l(r10);
@@ -1163,8 +1153,7 @@ ret_from_crit_exc:
#endif /* CONFIG_40x */
#ifdef CONFIG_BOOKE
- .globl ret_from_crit_exc
-ret_from_crit_exc:
+_NOKPROBE_ENTRY(ret_from_crit_exc)
mfspr r9,SPRN_SPRG_THREAD
lwz r10,SAVED_KSP_LIMIT(r1)
stw r10,KSP_LIMIT(r9)
@@ -1172,8 +1161,7 @@ ret_from_crit_exc:
RESTORE_MMU_REGS;
RET_FROM_EXC_LEVEL(SPRN_CSRR0, SPRN_CSRR1, PPC_RFCI)
- .globl ret_from_debug_exc
-ret_from_debug_exc:
+_NOKPROBE_ENTRY(ret_from_debug_exc)
mfspr r9,SPRN_SPRG_THREAD
lwz r10,SAVED_KSP_LIMIT(r1)
stw r10,KSP_LIMIT(r9)
@@ -1182,8 +1170,7 @@ ret_from_debug_exc:
RESTORE_MMU_REGS;
RET_FROM_EXC_LEVEL(SPRN_DSRR0, SPRN_DSRR1, PPC_RFDI)
- .globl ret_from_mcheck_exc
-ret_from_mcheck_exc:
+_NOKPROBE_ENTRY(ret_from_mcheck_exc)
mfspr r9,SPRN_SPRG_THREAD
lwz r10,SAVED_KSP_LIMIT(r1)
stw r10,KSP_LIMIT(r9)
@@ -1277,9 +1264,9 @@ do_user_signal: /* r10 contains MSR_KERNEL here */
* that occurred at a place where taking an exception will lose
* state information, such as the contents of SRR0 and SRR1.
*/
-nonrecoverable:
- lis r10,exc_exit_restart_end@ha
- addi r10,r10,exc_exit_restart_end@l
+.Lnonrecoverable:
+ lis r10,.Lexc_exit_restart_end@ha
+ addi r10,r10,.Lexc_exit_restart_end@l
cmplw r12,r10
#ifdef CONFIG_PPC_BOOK3S_601
bgelr
@@ -1332,7 +1319,7 @@ ee_restarts:
* On CHRP, the Run-Time Abstraction Services (RTAS) have to be
* called with the MMU off.
*/
-_GLOBAL(enter_rtas)
+_NOKPROBE_GLOBAL(enter_rtas)
stwu r1,-INT_FRAME_SIZE(r1)
mflr r0
stw r0,INT_FRAME_SIZE+4(r1)
@@ -1365,8 +1352,7 @@ _GLOBAL(enter_rtas)
mtspr SPRN_SRR1,r9
RFI /* return to caller */
- .globl machine_check_in_rtas
-machine_check_in_rtas:
+_NOKPROBE_ENTRY(machine_check_in_rtas)
twi 31,0,0
/* XXX load up BATs and panic */
diff --git a/arch/powerpc/kernel/fpu.S b/arch/powerpc/kernel/fpu.S
index 3235a8da6af7..1dfccf58fbb1 100644
--- a/arch/powerpc/kernel/fpu.S
+++ b/arch/powerpc/kernel/fpu.S
@@ -119,6 +119,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_VSX)
/* restore registers and return */
/* we haven't used ctr or xer or lr */
blr
+_ASM_NOKPROBE_SYMBOL(load_up_fpu)
/*
* save_fpu(tsk)
diff --git a/arch/powerpc/kernel/idle_6xx.S b/arch/powerpc/kernel/idle_6xx.S
index 0ffdd18b9f26..7fd736cf316f 100644
--- a/arch/powerpc/kernel/idle_6xx.S
+++ b/arch/powerpc/kernel/idle_6xx.S
@@ -149,7 +149,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_ALTIVEC)
* address of current. R11 points to the exception frame (physical
* address). We have to preserve r10.
*/
-_GLOBAL(power_save_ppc32_restore)
+_NOKPROBE_GLOBAL(power_save_ppc32_restore)
lwz r9,_LINK(r11) /* interrupted in ppc6xx_idle: */
stw r9,_NIP(r11) /* make it do a blr */
diff --git a/arch/powerpc/kernel/idle_e500.S b/arch/powerpc/kernel/idle_e500.S
index 308f499e146c..8c9a781b7c11 100644
--- a/arch/powerpc/kernel/idle_e500.S
+++ b/arch/powerpc/kernel/idle_e500.S
@@ -78,7 +78,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_CAN_NAP)
* r11 points to the exception frame (physical address).
* We have to preserve r10.
*/
-_GLOBAL(power_save_ppc32_restore)
+_NOKPROBE_GLOBAL(power_save_ppc32_restore)
lwz r9,_LINK(r11) /* interrupted in e500_idle */
stw r9,_NIP(r11) /* make it do a blr */
diff --git a/arch/powerpc/kernel/l2cr_6xx.S b/arch/powerpc/kernel/l2cr_6xx.S
index 2020d255585f..75e00baf0146 100644
--- a/arch/powerpc/kernel/l2cr_6xx.S
+++ b/arch/powerpc/kernel/l2cr_6xx.S
@@ -443,7 +443,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_ALTIVEC)
*
* clobbers r3
*/
-_GLOBAL(__inval_enable_L1)
+_NOKPROBE_GLOBAL(__inval_enable_L1)
/* Enable and then Flash inval the instruction & data cache */
mfspr r3,SPRN_HID0
ori r3,r3, HID0_ICE|HID0_ICFI|HID0_DCE|HID0_DCI
diff --git a/arch/powerpc/kernel/misc.S b/arch/powerpc/kernel/misc.S
index 974f65f79a8e..dae46463fc81 100644
--- a/arch/powerpc/kernel/misc.S
+++ b/arch/powerpc/kernel/misc.S
@@ -36,6 +36,8 @@ _GLOBAL(add_reloc_offset)
add r3,r3,r5
mtlr r0
blr
+_ASM_NOKPROBE_SYMBOL(reloc_offset)
+_ASM_NOKPROBE_SYMBOL(add_reloc_offset)
.align 3
2: PPC_LONG 1b
diff --git a/arch/powerpc/kernel/misc_32.S b/arch/powerpc/kernel/misc_32.S
index d80212be8698..07a2a303f0d9 100644
--- a/arch/powerpc/kernel/misc_32.S
+++ b/arch/powerpc/kernel/misc_32.S
@@ -233,7 +233,7 @@ _GLOBAL(_nmask_and_or_msr)
/*
* Do an IO access in real mode
*/
-_GLOBAL(real_readb)
+_NOKPROBE_GLOBAL(real_readb)
mfmsr r7
rlwinm r0,r7,0,~MSR_DR
sync
@@ -250,7 +250,7 @@ _GLOBAL(real_readb)
/*
* Do an IO access in real mode
*/
-_GLOBAL(real_writeb)
+_NOKPROBE_GLOBAL(real_writeb)
mfmsr r7
rlwinm r0,r7,0,~MSR_DR
sync
diff --git a/arch/powerpc/kernel/swsusp_32.S b/arch/powerpc/kernel/swsusp_32.S
index cbdf86228eaa..2dfe1afd7fc1 100644
--- a/arch/powerpc/kernel/swsusp_32.S
+++ b/arch/powerpc/kernel/swsusp_32.S
@@ -176,7 +176,7 @@ END_MMU_FTR_SECTION_IFSET(MMU_FTR_USE_HIGH_BATS)
/* Resume code */
-_GLOBAL(swsusp_arch_resume)
+_NOKPROBE_GLOBAL(swsusp_arch_resume)
#ifdef CONFIG_ALTIVEC
/* Stop pending alitvec streams and memory accesses */
@@ -367,7 +367,7 @@ END_MMU_FTR_SECTION_IFSET(MMU_FTR_USE_HIGH_BATS)
/* restore the MSR and turn on the MMU */
lwz r3,SL_MSR(r11)
- bl turn_on_mmu
+ bl .Lturn_on_mmu
tovirt(r11,r11)
/* Restore TB */
@@ -399,7 +399,7 @@ END_MMU_FTR_SECTION_IFSET(MMU_FTR_USE_HIGH_BATS)
/* FIXME:This construct is actually not useful since we don't shut
* down the instruction MMU, we could just flip back MSR-DR on.
*/
-turn_on_mmu:
+.Lturn_on_mmu:
mflr r4
mtsrr0 r4
mtsrr1 r3
diff --git a/arch/powerpc/kernel/vector.S b/arch/powerpc/kernel/vector.S
index 25c14a0981bf..403ab2c32dc8 100644
--- a/arch/powerpc/kernel/vector.S
+++ b/arch/powerpc/kernel/vector.S
@@ -89,6 +89,7 @@ _GLOBAL(load_up_altivec)
REST_32VRS(0,r4,r6)
/* restore registers and return */
blr
+_ASM_NOKPROBE_SYMBOL(load_up_altivec)
/*
* save_altivec(tsk)
diff --git a/arch/powerpc/mm/book3s32/hash_low.S b/arch/powerpc/mm/book3s32/hash_low.S
index c11b0a005196..c281045e15a7 100644
--- a/arch/powerpc/mm/book3s32/hash_low.S
+++ b/arch/powerpc/mm/book3s32/hash_low.S
@@ -51,7 +51,7 @@ mmu_hash_lock:
* Uses r0, r3 - r6, r8, r10, ctr, lr.
*/
.text
-_GLOBAL(hash_page)
+_NOKPROBE_GLOBAL(hash_page)
#ifdef CONFIG_SMP
lis r8, (mmu_hash_lock - ADDR_OFFSET)@h
ori r8, r8, (mmu_hash_lock - ADDR_OFFSET)@l
@@ -93,7 +93,7 @@ _GLOBAL(hash_page)
tovirt(r8, r8)
#endif
#ifdef CONFIG_SMP
- beq- hash_page_out /* return if no mapping */
+ beq- .Lhash_page_out /* return if no mapping */
#else
/* XXX it seems like the 601 will give a machine fault on the
rfi if its alignment is wrong (bottom 4 bits of address are
@@ -121,11 +121,11 @@ _GLOBAL(hash_page)
#if (PTE_FLAGS_OFFSET != 0)
addi r8,r8,PTE_FLAGS_OFFSET
#endif
-retry:
+.Lretry:
lwarx r6,0,r8 /* get linux-style pte, flag word */
andc. r5,r3,r6 /* check access & ~permission */
#ifdef CONFIG_SMP
- bne- hash_page_out /* return if access not permitted */
+ bne- .Lhash_page_out /* return if access not permitted */
#else
bnelr-
#endif
@@ -140,7 +140,7 @@ retry:
#endif /* CONFIG_SMP */
#endif /* CONFIG_PTE_64BIT */
stwcx. r5,0,r8 /* attempt to update PTE */
- bne- retry /* retry if someone got there first */
+ bne- .Lretry /* retry if someone got there first */
mfsrin r3,r4 /* get segment reg for segment */
mfctr r0
@@ -162,7 +162,7 @@ retry:
b fast_exception_return
#ifdef CONFIG_SMP
-hash_page_out:
+.Lhash_page_out:
eieio
lis r8, (mmu_hash_lock - ADDR_OFFSET)@ha
li r0,0
@@ -179,7 +179,7 @@ hash_page_out:
* the accessed bit) have already been done and that there is actually
* a hash table in use (i.e. we're not on a 603).
*/
-_GLOBAL(add_hash_page)
+_NOKPROBE_GLOBAL(add_hash_page)
mflr r0
stw r0,4(r1)
@@ -314,7 +314,7 @@ Hash_msk = (((1 << Hash_bits) - 1) * 64)
#define HASH_LEFT 31-(LG_PTEG_SIZE+Hash_bits-1)
#define HASH_RIGHT 31-LG_PTEG_SIZE
-_GLOBAL(create_hpte)
+_NOKPROBE_GLOBAL(create_hpte)
/* Convert linux-style PTE (r5) to low word of PPC-style PTE (r8) */
rlwinm r8,r5,32-9,30,30 /* _PAGE_RW -> PP msb */
rlwinm r0,r5,32-6,30,30 /* _PAGE_DIRTY -> PP msb */
@@ -366,7 +366,7 @@ END_FTR_SECTION_IFCLR(CPU_FTR_NEED_COHERENT)
1: LDPTEu r6,HPTE_SIZE(r4) /* get next PTE */
CMPPTE 0,r6,r5
bdnzf 2,1b /* loop while ctr != 0 && !cr0.eq */
- beq+ found_slot
+ beq+ .Lfound_slot
patch_site 0f, patch__hash_page_B
/* Search the secondary PTEG for a matching PTE */
@@ -378,7 +378,7 @@ END_FTR_SECTION_IFCLR(CPU_FTR_NEED_COHERENT)
2: LDPTEu r6,HPTE_SIZE(r4)
CMPPTE 0,r6,r5
bdnzf 2,2b
- beq+ found_slot
+ beq+ .Lfound_slot
xori r5,r5,PTE_H /* clear H bit again */
/* Search the primary PTEG for an empty slot */
@@ -387,7 +387,7 @@ END_FTR_SECTION_IFCLR(CPU_FTR_NEED_COHERENT)
1: LDPTEu r6,HPTE_SIZE(r4) /* get next PTE */
TST_V(r6) /* test valid bit */
bdnzf 2,1b /* loop while ctr != 0 && !cr0.eq */
- beq+ found_empty
+ beq+ .Lfound_empty
/* update counter of times that the primary PTEG is full */
lis r4, (primary_pteg_full - ADDR_OFFSET)@ha
@@ -405,7 +405,7 @@ END_FTR_SECTION_IFCLR(CPU_FTR_NEED_COHERENT)
2: LDPTEu r6,HPTE_SIZE(r4)
TST_V(r6)
bdnzf 2,2b
- beq+ found_empty
+ beq+ .Lfound_empty
xori r5,r5,PTE_H /* clear H bit again */
/*
@@ -443,9 +443,9 @@ END_FTR_SECTION_IFCLR(CPU_FTR_NEED_COHERENT)
#ifndef CONFIG_SMP
/* Store PTE in PTEG */
-found_empty:
+.Lfound_empty:
STPTE r5,0(r4)
-found_slot:
+.Lfound_slot:
STPTE r8,HPTE_SIZE/2(r4)
#else /* CONFIG_SMP */
@@ -466,8 +466,8 @@ found_slot:
* We do however have to make sure that the PTE is never in an invalid
* state with the V bit set.
*/
-found_empty:
-found_slot:
+.Lfound_empty:
+.Lfound_slot:
CLR_V(r5,r0) /* clear V (valid) bit in PTE */
STPTE r5,0(r4)
sync
@@ -499,7 +499,7 @@ htab_hash_searches:
*
* We assume that there is a hash table in use (Hash != 0).
*/
-_GLOBAL(flush_hash_pages)
+_NOKPROBE_GLOBAL(flush_hash_pages)
/*
* We disable interrupts here, even on UP, because we want
* the _PAGE_HASHPTE bit to be a reliable indication of
@@ -640,7 +640,7 @@ EXPORT_SYMBOL(flush_hash_pages)
/*
* Flush an entry from the TLB
*/
-_GLOBAL(_tlbie)
+_NOKPROBE_GLOBAL(_tlbie)
#ifdef CONFIG_SMP
lwz r8,TASK_CPU(r2)
oris r8,r8,11
@@ -677,7 +677,7 @@ _GLOBAL(_tlbie)
/*
* Flush the entire TLB. 603/603e only
*/
-_GLOBAL(_tlbia)
+_NOKPROBE_GLOBAL(_tlbia)
#if defined(CONFIG_SMP)
lwz r8,TASK_CPU(r2)
oris r8,r8,10
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index ef7b1119b2e2..7a4083a62efe 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -32,6 +32,7 @@
#include <linux/vmalloc.h>
#include <linux/memremap.h>
#include <linux/dma-direct.h>
+#include <linux/kprobes.h>
#include <asm/pgalloc.h>
#include <asm/prom.h>
@@ -470,6 +471,7 @@ static void flush_dcache_icache_phys(unsigned long physaddr)
: "r" (nb), "r" (msr), "i" (bytes), "r" (msr0)
: "ctr", "memory");
}
+NOKPROBE_SYMBOL(flush_dcache_icache_phys)
#endif // !defined(CONFIG_PPC_8xx) && !defined(CONFIG_PPC64)
/*
diff --git a/arch/powerpc/platforms/52xx/lite5200_sleep.S b/arch/powerpc/platforms/52xx/lite5200_sleep.S
index 3a9969c429b3..8ced276a6b47 100644
--- a/arch/powerpc/platforms/52xx/lite5200_sleep.S
+++ b/arch/powerpc/platforms/52xx/lite5200_sleep.S
@@ -193,6 +193,7 @@ sram_code_end:
/* uboot jumps here on resume */
+_ASM_NOKPROBE_SYMBOL(lite5200_wakeup)
lite5200_wakeup:
bl restore_regs
@@ -335,6 +336,7 @@ save_regs:
LOAD_SR(n+2, addr+2); \
LOAD_SR(n+3, addr+3);
+_ASM_NOKPROBE_SYMBOL(restore_regs)
restore_regs:
lis r4, registers@h
ori r4, r4, registers@l
diff --git a/arch/powerpc/platforms/82xx/pq2.c b/arch/powerpc/platforms/82xx/pq2.c
index 1cdd5ed9d896..06043eb718c5 100644
--- a/arch/powerpc/platforms/82xx/pq2.c
+++ b/arch/powerpc/platforms/82xx/pq2.c
@@ -29,6 +29,7 @@ void __noreturn pq2_restart(char *cmd)
panic("Restart failed\n");
}
+NOKPROBE_SYMBOL(pq2_restart)
#ifdef CONFIG_PCI
static int pq2_pci_exclude_device(struct pci_controller *hose,
diff --git a/arch/powerpc/platforms/83xx/suspend-asm.S b/arch/powerpc/platforms/83xx/suspend-asm.S
index 3acd7470dc5e..a7ba0f41ac34 100644
--- a/arch/powerpc/platforms/83xx/suspend-asm.S
+++ b/arch/powerpc/platforms/83xx/suspend-asm.S
@@ -365,6 +365,7 @@ boot_low:
isync
1: b 1b
+_ASM_NOKPROBE_SYMBOL(mpc83xx_deep_resume)
mpc83xx_deep_resume:
lis r4, 1f@h
ori r4, r4, 1f@l
diff --git a/arch/powerpc/platforms/powermac/cache.S b/arch/powerpc/platforms/powermac/cache.S
index da69e0fcb4f1..e15f4aa22e9a 100644
--- a/arch/powerpc/platforms/powermac/cache.S
+++ b/arch/powerpc/platforms/powermac/cache.S
@@ -35,6 +35,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_L2CR)
b __flush_disable_L1
/* This is the code for G3 and 74[01]0 */
+_ASM_NOKPROBE_SYMBOL(flush_disable_75x)
flush_disable_75x:
mflr r10
@@ -186,6 +187,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_ALTIVEC)
blr
/* This code is for 745x processors */
+_ASM_NOKPROBE_SYMBOL(flush_disable_745x)
flush_disable_745x:
/* Turn off EE and DR in MSR */
mfmsr r11
diff --git a/arch/powerpc/platforms/powermac/sleep.S b/arch/powerpc/platforms/powermac/sleep.S
index bd6085b470b7..2a6de1b345c0 100644
--- a/arch/powerpc/platforms/powermac/sleep.S
+++ b/arch/powerpc/platforms/powermac/sleep.S
@@ -177,8 +177,8 @@ END_MMU_FTR_SECTION_IFSET(MMU_FTR_USE_HIGH_BATS)
* at address 0x84. I've have some trouble with those
* parameters however and I no longer use them.
*/
- lis r5,grackle_wake_up@ha
- addi r5,r5,grackle_wake_up@l
+ lis r5,.Lgrackle_wake_up@ha
+ addi r5,r5,.Lgrackle_wake_up@l
tophys(r5,r5)
stw r5,SL_PC(r1)
lis r4,KERNELBASE@h
@@ -202,6 +202,7 @@ END_MMU_FTR_SECTION_IFSET(MMU_FTR_USE_HIGH_BATS)
stw r5,0(r3)
.globl low_cpu_die
+_ASM_NOKPROBE_SYMBOL(low_cpu_die)
low_cpu_die:
/* Flush & disable all caches */
bl flush_disable_caches
@@ -244,7 +245,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_SPEC7450)
mtmsr r2
isync
b 1b
-
/*
* Here is the resume code.
*/
@@ -255,6 +255,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_SPEC7450)
* r4 has the physical address of SL_PC(sp) (unused)
*/
_GLOBAL(core99_wake_up)
+_ASM_NOKPROBE_SYMBOL(core99_wake_up)
/* Make sure HID0 no longer contains any sleep bit and that data cache
* is disabled
*/
@@ -287,7 +288,7 @@ _GLOBAL(core99_wake_up)
* r1 has the physical address of SL_PC(sp).
*/
-grackle_wake_up:
+.Lgrackle_wake_up:
/* Restore the kernel's segment registers before
* we do any r1 memory access as we are not sure they
@@ -407,7 +408,7 @@ END_MMU_FTR_SECTION_IFSET(MMU_FTR_USE_HIGH_BATS)
/* restore the MSR and turn on the MMU */
lwz r3,SL_MSR(r1)
- bl turn_on_mmu
+ bl .Lturn_on_mmu
/* get back the stack pointer */
tovirt(r1,r1)
@@ -430,7 +431,7 @@ END_MMU_FTR_SECTION_IFSET(MMU_FTR_USE_HIGH_BATS)
mtlr r0
blr
-turn_on_mmu:
+.Lturn_on_mmu:
mflr r4
tovirt(r4,r4)
mtsrr0 r4
--
2.25.0
^ permalink raw reply related
* [PATCH v2] powerpc/kprobes: Remove redundant code
From: Christophe Leroy @ 2020-02-19 8:05 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Naveen N. Rao
Cc: linuxppc-dev, linux-kernel
At the time being we have something like
if (something) {
p = get();
if (p) {
if (something_wrong)
goto out;
...
return;
} else if (a != b) {
if (some_error)
goto out;
...
}
goto out;
}
p = get();
if (!p) {
if (a != b) {
if (some_error)
goto out;
...
}
goto out;
}
This is similar to
p = get();
if (!p) {
if (a != b) {
if (some_error)
goto out;
...
}
goto out;
}
if (something) {
if (something_wrong)
goto out;
...
return;
}
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
v2: Reverse the logic by testing (!p) before kprobe_running() as suggested by Naveen.
---
arch/powerpc/kernel/kprobes.c | 80 ++++++++++++++---------------------
1 file changed, 32 insertions(+), 48 deletions(-)
diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
index 9b340af02c38..84567406b53d 100644
--- a/arch/powerpc/kernel/kprobes.c
+++ b/arch/powerpc/kernel/kprobes.c
@@ -274,54 +274,6 @@ int kprobe_handler(struct pt_regs *regs)
preempt_disable();
kcb = get_kprobe_ctlblk();
- /* Check we're not actually recursing */
- if (kprobe_running()) {
- p = get_kprobe(addr);
- if (p) {
- kprobe_opcode_t insn = *p->ainsn.insn;
- if (kcb->kprobe_status == KPROBE_HIT_SS &&
- is_trap(insn)) {
- /* Turn off 'trace' bits */
- regs->msr &= ~MSR_SINGLESTEP;
- regs->msr |= kcb->kprobe_saved_msr;
- goto no_kprobe;
- }
- /* We have reentered the kprobe_handler(), since
- * another probe was hit while within the handler.
- * We here save the original kprobes variables and
- * just single step on the instruction of the new probe
- * without calling any user handlers.
- */
- save_previous_kprobe(kcb);
- set_current_kprobe(p, regs, kcb);
- kprobes_inc_nmissed_count(p);
- kcb->kprobe_status = KPROBE_REENTER;
- if (p->ainsn.boostable >= 0) {
- ret = try_to_emulate(p, regs);
-
- if (ret > 0) {
- restore_previous_kprobe(kcb);
- preempt_enable_no_resched();
- return 1;
- }
- }
- prepare_singlestep(p, regs);
- return 1;
- } else if (*addr != BREAKPOINT_INSTRUCTION) {
- /* If trap variant, then it belongs not to us */
- kprobe_opcode_t cur_insn = *addr;
-
- if (is_trap(cur_insn))
- goto no_kprobe;
- /* The breakpoint instruction was removed by
- * another cpu right after we hit, no further
- * handling of this interrupt is appropriate
- */
- ret = 1;
- }
- goto no_kprobe;
- }
-
p = get_kprobe(addr);
if (!p) {
if (*addr != BREAKPOINT_INSTRUCTION) {
@@ -346,6 +298,38 @@ int kprobe_handler(struct pt_regs *regs)
goto no_kprobe;
}
+ /* Check we're not actually recursing */
+ if (kprobe_running()) {
+ kprobe_opcode_t insn = *p->ainsn.insn;
+ if (kcb->kprobe_status == KPROBE_HIT_SS && is_trap(insn)) {
+ /* Turn off 'trace' bits */
+ regs->msr &= ~MSR_SINGLESTEP;
+ regs->msr |= kcb->kprobe_saved_msr;
+ goto no_kprobe;
+ }
+ /* We have reentered the kprobe_handler(), since
+ * another probe was hit while within the handler.
+ * We here save the original kprobes variables and
+ * just single step on the instruction of the new probe
+ * without calling any user handlers.
+ */
+ save_previous_kprobe(kcb);
+ set_current_kprobe(p, regs, kcb);
+ kprobes_inc_nmissed_count(p);
+ kcb->kprobe_status = KPROBE_REENTER;
+ if (p->ainsn.boostable >= 0) {
+ ret = try_to_emulate(p, regs);
+
+ if (ret > 0) {
+ restore_previous_kprobe(kcb);
+ preempt_enable_no_resched();
+ return 1;
+ }
+ }
+ prepare_singlestep(p, regs);
+ return 1;
+ }
+
kcb->kprobe_status = KPROBE_HIT_ACTIVE;
set_current_kprobe(p, regs, kcb);
if (p->pre_handler && p->pre_handler(p, regs)) {
--
2.25.0
^ permalink raw reply related
* Re: [PATCH v7 01/12] capabilities: introduce CAP_PERFMON to kernel and user space
From: Alexey Budankov @ 2020-02-19 7:54 UTC (permalink / raw)
To: James Morris
Cc: linux-man, linux-doc@vger.kernel.org, Peter Zijlstra,
joonas.lahtinen@linux.intel.com, Alexei Starovoitov,
Stephane Eranian, Paul Mackerras, Jiri Olsa, Ingo Molnar,
Andi Kleen, Will Deacon, Helge Deller, Igor Lubashev,
oprofile-list, Stephen Smalley, Serge Hallyn,
selinux@vger.kernel.org, intel-gfx@lists.freedesktop.org,
Arnaldo Carvalho de Melo, Thomas Gleixner, linux-arm-kernel,
linux-parisc@vger.kernel.org, linux-kernel,
linux-security-module@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <alpine.LRH.2.21.2002190621180.10165@namei.org>
On 18.02.2020 22:21, James Morris wrote:
> On Mon, 17 Feb 2020, Alexey Budankov wrote:
>
>>
>> Introduce CAP_PERFMON capability designed to secure system performance
>> monitoring and observability operations so that CAP_PERFMON would assist
>> CAP_SYS_ADMIN capability in its governing role for performance
>> monitoring and observability subsystems.
>
>
> Acked-by: James Morris <jamorris@linux.microsoft.com>
Thanks James!
I appreciate your involvement and collaboration
w.r.t to the whole patch set.
Gratefully,
Alexey
^ permalink raw reply
* Re: [PATCH 1/2] powerpc/kprobes: Remove redundant code
From: Christophe Leroy @ 2020-02-19 7:48 UTC (permalink / raw)
To: Naveen N. Rao, Benjamin Herrenschmidt, Michael Ellerman,
Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <1582036611.9hm2t8ijhz.naveen@linux.ibm.com>
Le 18/02/2020 à 15:39, Naveen N. Rao a écrit :
> Christophe Leroy wrote:
>> At the time being we have something like
>>
>> if (something) {
>> p = get();
>> if (p) {
>> if (something_wrong)
>> goto out;
>> ...
>> return;
>> } else if (a != b) {
>> if (some_error)
>> goto out;
>> ...
>> }
>> goto out;
>> }
>> p = get();
>> if (!p) {
>> if (a != b) {
>> if (some_error)
>> goto out;
>> ...
>> }
>> goto out;
>> }
>>
>> This is similar to
>>
>> p = get();
>> if (something) {
>> if (p) {
>> if (something_wrong)
>> goto out;
>> ...
>> return;
>> }
>> }
>> if (!p) {
>> if (a != b) {
>> if (some_error)
>> goto out;
>> ...
>> }
>> goto out;
>> }
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>> ---
>> arch/powerpc/kernel/kprobes.c | 15 +--------------
>> 1 file changed, 1 insertion(+), 14 deletions(-)
>
> Good cleanup, thanks.
>
>>
>> diff --git a/arch/powerpc/kernel/kprobes.c
>> b/arch/powerpc/kernel/kprobes.c
>> index f8b848aa65bd..7a925eb76ec0 100644
>> --- a/arch/powerpc/kernel/kprobes.c
>> +++ b/arch/powerpc/kernel/kprobes.c
>> @@ -276,8 +276,8 @@ int kprobe_handler(struct pt_regs *regs)
>> kcb = get_kprobe_ctlblk();
>>
>> /* Check we're not actually recursing */
>> + p = get_kprobe(addr);
>> if (kprobe_running()) {
>> - p = get_kprobe(addr);
>> if (p) {
>> kprobe_opcode_t insn = *p->ainsn.insn;
>> if (kcb->kprobe_status == KPROBE_HIT_SS &&
>> @@ -308,22 +308,9 @@ int kprobe_handler(struct pt_regs *regs)
>> }
>> prepare_singlestep(p, regs);
>> return 1;
>> - } else if (*addr != BREAKPOINT_INSTRUCTION) {
>> - /* If trap variant, then it belongs not to us */
>> - kprobe_opcode_t cur_insn = *addr;
>> -
>> - if (is_trap(cur_insn))
>> - goto no_kprobe;
>> - /* The breakpoint instruction was removed by
>> - * another cpu right after we hit, no further
>> - * handling of this interrupt is appropriate
>> - */
>> - ret = 1;
>> }
>> - goto no_kprobe;
>
> A minot nit -- removing the above goto makes a slight change to the
> logic. But, see my comments for the next patch.
All legs of the (p) case are have either a return or a goto, so that
goto no_kprobe is limited to the !p case, we have to fall_through now.
Christophe
^ permalink raw reply
* RE: [PATCH v2 24/27] nvdimm/ocxl: Implement Overwrite
From: Alastair D'Silva @ 2020-02-19 5:13 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Keith Busch,
Masahiro Yamada, Paul Mackerras, Mauro Carvalho Chehab, Ira Weiny,
Thomas Gleixner, Rob Herring, Dave Jiang, linux-nvdimm,
Vishal Verma, Krzysztof Kozlowski, Anju T Sudhakar,
Mahesh Salgaonkar, Andrew Donnellan, Arnd Bergmann, Greg Kurz,
Nicholas Piggin, Cédric Le Goater, Dan Williams,
Hari Bathini, linux-mm, Greg Kroah-Hartman, linux-kernel,
Frederic Barrat, Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <20200203151019.0000262f@Huawei.com>
On Mon, 2020-02-03 at 15:10 +0000, Jonathan Cameron wrote:
> On Tue, 3 Dec 2019 14:46:52 +1100
> Alastair D'Silva <alastair@au1.ibm.com> wrote:
>
> > From: Alastair D'Silva <alastair@d-silva.org>
> >
> > The near storage command 'Secure Erase' overwrites all data on the
> > media.
> >
> > This patch hooks it up to the security function 'overwrite'.
> >
> > Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
>
> A few things to tidy up in here.
>
> Thanks,
>
> Jonathan
>
>
> > ---
> > drivers/nvdimm/ocxl/scm.c | 164
> > ++++++++++++++++++++++++++++-
> > drivers/nvdimm/ocxl/scm_internal.c | 1 +
> > drivers/nvdimm/ocxl/scm_internal.h | 17 +++
> > 3 files changed, 180 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/nvdimm/ocxl/scm.c b/drivers/nvdimm/ocxl/scm.c
> > index a81eb5916eb3..8deb7862793c 100644
> > --- a/drivers/nvdimm/ocxl/scm.c
> > +++ b/drivers/nvdimm/ocxl/scm.c
> > @@ -169,6 +169,86 @@ static int scm_reserve_metadata(struct
> > scm_data *scm_data,
> > return 0;
> > }
> >
> > +/**
> > + * scm_overwrite() - Overwrite all data on the card
> > + * @scm_data: The SCM device data
>
> I would mention in here that this exists with the lock held and
> where that is unlocked again.
Ok
>
> > + * Return: 0 on success
> > + */
> > +int scm_overwrite(struct scm_data *scm_data)
> > +{
> > + int rc;
> > +
> > + mutex_lock(&scm_data->ns_command.lock);
> > +
> > + rc = scm_ns_command_request(scm_data, NS_COMMAND_SECURE_ERASE);
> > + if (rc)
>
> Perhaps change that goto label to reflect it is the error path rather
> than a shared exit route.
>
Ok
> > + goto out;
> > +
> > + rc = scm_ns_command_execute(scm_data);
> > + if (rc)
> > + goto out;
> > +
> > + scm_data->overwrite_state = SCM_OVERWRITE_BUSY;
> > +
> > + return 0;
> > +
> > +out:
> > + mutex_unlock(&scm_data->ns_command.lock);
> > + return rc;
> > +}
> > +
> > +/**
> > + * scm_secop_overwrite() - Overwrite all data on the card
> > + * @nvdimm: The nvdimm representation of the SCM device to start
> > the overwrite on
> > + * @key_data: Unused (no security key implementation)
> > + * Return: 0 on success
> > + */
> > +static int scm_secop_overwrite(struct nvdimm *nvdimm,
> > + const struct nvdimm_key_data *key_data)
> > +{
> > + struct scm_data *scm_data = nvdimm_provider_data(nvdimm);
> > +
> > + return scm_overwrite(scm_data);
> > +}
> > +
> > +/**
> > + * scm_secop_query_overwrite() - Get the current overwrite state
> > + * @nvdimm: The nvdimm representation of the SCM device to start
> > the overwrite on
> > + * Return: 0 if successful or idle, -EBUSY if busy, -EFAULT if
> > failed
> > + */
> > +static int scm_secop_query_overwrite(struct nvdimm *nvdimm)
> > +{
> > + struct scm_data *scm_data = nvdimm_provider_data(nvdimm);
> > +
> > + if (scm_data->overwrite_state == SCM_OVERWRITE_BUSY)
> > + return -EBUSY;
> > +
> > + if (scm_data->overwrite_state == SCM_OVERWRITE_FAILED)
> > + return -EFAULT;
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * scm_secop_get_flags() - return the security flags for the SCM
> > device
>
> All params need to documented in kernel-doc comments.
Ok
>
> > + */
> > +static unsigned long scm_secop_get_flags(struct nvdimm *nvdimm,
> > + enum nvdimm_passphrase_type ptype)
> > +{
> > + struct scm_data *scm_data = nvdimm_provider_data(nvdimm);
> > +
> > + if (scm_data->overwrite_state == SCM_OVERWRITE_BUSY)
> > + return BIT(NVDIMM_SECURITY_OVERWRITE);
> > +
> > + return BIT(NVDIMM_SECURITY_DISABLED);
> > +}
> > +
> > +static const struct nvdimm_security_ops sec_ops = {
> > + .get_flags = scm_secop_get_flags,
> > + .overwrite = scm_secop_overwrite,
> > + .query_overwrite = scm_secop_query_overwrite,
> > +};
> > +
> > /**
> > * scm_register_lpc_mem() - Discover persistent memory on a device
> > and register it with the NVDIMM subsystem
> > * @scm_data: The SCM device data
> > @@ -224,10 +304,10 @@ static int scm_register_lpc_mem(struct
> > scm_data *scm_data)
> > set_bit(NDD_ALIASING, &nvdimm_flags);
> >
> > snprintf(serial, sizeof(serial), "%llx", fn_config->serial);
> > - nd_mapping_desc.nvdimm = nvdimm_create(scm_data->nvdimm_bus,
> > scm_data,
> > + nd_mapping_desc.nvdimm = __nvdimm_create(scm_data->nvdimm_bus,
> > scm_data,
> > scm_dimm_attribute_groups,
> > nvdimm_flags, nvdimm_cmd_mask,
> > - 0, NULL);
> > + 0, NULL, serial, &sec_ops);
> > if (!nd_mapping_desc.nvdimm)
> > return -ENOMEM;
> >
> > @@ -1530,6 +1610,83 @@ static void scm_dump_error_log(struct
> > scm_data *scm_data)
> > kfree(buf);
> > }
> >
> > +static void scm_handle_nscra_doorbell(struct scm_data *scm_data)
> > +{
> > + int rc;
> > +
> > + if (scm_data->ns_command.op_code == NS_COMMAND_SECURE_ERASE) {
>
> Feels likely that we are going to end up with quite a few blocks like
> this as
> the driver is extended. Perhaps just start out with a switch
> statement and
> separate functions that it calls?
>
At the moment, this is the only near storage command documented on the
device, and I don't think there will be any more.
> > + u64 success, attempted;
> > +
>
> One is enough here.
>
It's not, there is a comparison between them later.
> > +
> > + rc = scm_ns_response(scm_data);
> > + if (rc < 0) {
> > + scm_data->overwrite_state =
> > SCM_OVERWRITE_FAILED;
>
> If this were a separate function as suggested above, I'd use a goto
> to ensure we
> unlock in all paths.
>
> > + mutex_unlock(&scm_data->ns_command.lock);
> > + return;
> > + }
> > + if (rc != STATUS_SUCCESS)
> > + scm_warn_status(scm_data, "Unexpected status
> > from overwrite", rc);
> > +
> > + rc = ocxl_global_mmio_read64(scm_data->ocxl_afu,
> > + scm_data-
> > >ns_command.response_offset +
> > + NS_RESPONSE_SECURE_ERASE_A
> > CCESSIBLE_SUCCESS,
> > + OCXL_HOST_ENDIAN,
> > &success);
> > + if (rc) {
> > + scm_data->overwrite_state =
> > SCM_OVERWRITE_FAILED;
> > + mutex_unlock(&scm_data->ns_command.lock);
> > + return;
> > + }
> > +
> > + rc = ocxl_global_mmio_read64(scm_data->ocxl_afu,
> > + scm_data-
> > >ns_command.response_offset +
> > + NS_RESPONSE_SECURE_ERASE_A
> > CCESSIBLE_ATTEMPTED,
> > + OCXL_HOST_ENDIAN,
> > &attempted);
> > + if (rc) {
> > + scm_data->overwrite_state =
> > SCM_OVERWRITE_FAILED;
> > + mutex_unlock(&scm_data->ns_command.lock);
> > + return;
> > + }
> > +
> > + scm_data->overwrite_state = SCM_OVERWRITE_SUCCESS;
> > + if (success != attempted)
> > + scm_data->overwrite_state =
> > SCM_OVERWRITE_FAILED;
> > +
> > + dev_info(&scm_data->dev,
> > + "Overwritten %llu/%llu accessible pages",
> > success, attempted);
>
> Do we want to spam the log? Feels like dev_dbg maybe?
This only occurs once per overwrite operation. Each overwrite operation
is expected to take a non-trivial amount of time.
>
> > +
> > + rc = ocxl_global_mmio_read64(scm_data->ocxl_afu,
> > + scm_data-
> > >ns_command.response_offset +
> > + NS_RESPONSE_SECURE_ERASE_D
> > EFECTIVE_SUCCESS,
> > + OCXL_HOST_ENDIAN,
> > &success);
> > + if (rc) {
> > + scm_data->overwrite_state =
> > SCM_OVERWRITE_FAILED;
> > + mutex_unlock(&scm_data->ns_command.lock);
> > + return;
> > + }
> > +
> > + rc = ocxl_global_mmio_read64(scm_data->ocxl_afu,
> > + scm_data-
> > >ns_command.response_offset +
> > + NS_RESPONSE_SECURE_ERASE_D
> > EFECTIVE_ATTEMPTED,
> > + OCXL_HOST_ENDIAN,
> > &attempted);
> > + if (rc) {
> > + scm_data->overwrite_state =
> > SCM_OVERWRITE_FAILED;
> > + mutex_unlock(&scm_data->ns_command.lock);
> > + return;
> > + }
> > +
> > + if (success != attempted)
> > + scm_data->overwrite_state =
> > SCM_OVERWRITE_FAILED;
> > +
> > + dev_info(&scm_data->dev,
> > + "Overwritten %llu/%llu defective pages",
> > success, attempted);
>
> Again, maybe dev_dbg?
>
(see above)
> > +
> > + scm_ns_response_handled(scm_data);
> > +
> > + mutex_unlock(&scm_data->ns_command.lock);
> > + return;
> > + }
> > +}
> > +
> > static irqreturn_t scm_imn0_handler(void *private)
> > {
> > struct scm_data *scm_data = private;
> > @@ -1537,6 +1694,9 @@ static irqreturn_t scm_imn0_handler(void
> > *private)
> >
> > (void)scm_chi(scm_data, &chi);
> >
> > + if (chi & GLOBAL_MMIO_CHI_NSCRA)
> > + scm_handle_nscra_doorbell(scm_data);
> > +
> > if (chi & GLOBAL_MMIO_CHI_ELA) {
> > dev_warn(&scm_data->dev, "Error log is available\n");
> >
> > diff --git a/drivers/nvdimm/ocxl/scm_internal.c
> > b/drivers/nvdimm/ocxl/scm_internal.c
> > index 8fc849610eaa..db919a23c69b 100644
> > --- a/drivers/nvdimm/ocxl/scm_internal.c
> > +++ b/drivers/nvdimm/ocxl/scm_internal.c
> > @@ -173,6 +173,7 @@ int scm_ns_response_handled(const struct
> > scm_data *scm_data)
> > OCXL_LITTLE_ENDIAN,
> > GLOBAL_MMIO_CHI_NSCRA);
> > }
> >
> > +
>
> Stray blank line..
Sneaky things...
>
> > void scm_warn_status(const struct scm_data *scm_data, const char
> > *message,
> > u8 status)
> > {
> > diff --git a/drivers/nvdimm/ocxl/scm_internal.h
> > b/drivers/nvdimm/ocxl/scm_internal.h
> > index af19813a7f75..4a29088612a9 100644
> > --- a/drivers/nvdimm/ocxl/scm_internal.h
> > +++ b/drivers/nvdimm/ocxl/scm_internal.h
> > @@ -70,6 +70,15 @@
> > #define ADMIN_COMMAND_CMD_CAPS 0x08u
> > #define ADMIN_COMMAND_MAX 0x08u
> >
> > +#define NS_COMMAND_SECURE_ERASE 0x20ull
> > +
> > +#define NS_RESPONSE_SECURE_ERASE_ACCESSIBLE_SUCCESS 0x20
> > +#define NS_RESPONSE_SECURE_ERASE_ACCESSIBLE_ATTEMPTED 0x28
> > +#define NS_RESPONSE_SECURE_ERASE_DEFECTIVE_SUCCESS 0x30
> > +#define NS_RESPONSE_SECURE_ERASE_DEFECTIVE_ATTEMPTED 0x38
> > +
>
> Lot of blank lines...
Whoops
>
> > +
> > +
> > #define STATUS_SUCCESS 0x00
> > #define STATUS_MEM_UNAVAILABLE 0x20
> > #define STATUS_BAD_OPCODE 0x50
> > @@ -99,6 +108,13 @@ struct scm_function_0 {
> > struct ocxl_fn *ocxl_fn;
> > };
> >
> > +enum overwrite_state {
> > + SCM_OVERWRITE_IDLE = 0,
> > + SCM_OVERWRITE_BUSY,
> > + SCM_OVERWRITE_SUCCESS,
> > + SCM_OVERWRITE_FAILED
> > +};
> > +
> > struct scm_data {
> > struct device dev;
> > struct pci_dev *pdev;
> > @@ -116,6 +132,7 @@ struct scm_data {
> > void *metadata_addr;
> > struct command_metadata admin_command;
> > struct command_metadata ns_command;
> > + enum overwrite_state overwrite_state;
> > struct resource scm_res;
> > struct nd_region *nd_region;
> > struct eventfd_ctx *ev_ctx;
--
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819
^ permalink raw reply
* RE: [PATCH v2 22/27] nvdimm/ocxl: Implement the heartbeat command
From: Alastair D'Silva @ 2020-02-19 5:02 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Keith Busch,
Masahiro Yamada, Paul Mackerras, Mauro Carvalho Chehab, Ira Weiny,
Thomas Gleixner, Rob Herring, Dave Jiang, linux-nvdimm,
Vishal Verma, Krzysztof Kozlowski, Anju T Sudhakar,
Mahesh Salgaonkar, Andrew Donnellan, Arnd Bergmann, Greg Kurz,
Nicholas Piggin, Cédric Le Goater, Dan Williams,
Hari Bathini, linux-mm, Greg Kroah-Hartman, linux-kernel,
Frederic Barrat, Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <20200203151148.00000ae0@Huawei.com>
On Mon, 2020-02-03 at 15:11 +0000, Jonathan Cameron wrote:
> On Tue, 3 Dec 2019 14:46:50 +1100
> Alastair D'Silva <alastair@au1.ibm.com> wrote:
>
> > From: Alastair D'Silva <alastair@d-silva.org>
> >
> > The heartbeat admin command is a simple admin command that
> > exercises
> > the communication mechanisms within the controller.
> >
> > This patch issues a heartbeat command to the card during init to
> > ensure
> > we can communicate with the card's crontroller.
>
> controller
That's a perfectly cromulent misspelling ;)
>
> > Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> > ---
> > drivers/nvdimm/ocxl/scm.c | 43
> > +++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 43 insertions(+)
> >
> > diff --git a/drivers/nvdimm/ocxl/scm.c b/drivers/nvdimm/ocxl/scm.c
> > index 8a30c887b5ed..e8b34262f397 100644
> > --- a/drivers/nvdimm/ocxl/scm.c
> > +++ b/drivers/nvdimm/ocxl/scm.c
> > @@ -353,6 +353,44 @@ static bool scm_is_usable(const struct
> > scm_data *scm_data)
> > return true;
> > }
> >
> > +/**
> > + * scm_heartbeat() - Issue a heartbeat command to the controller
> > + * @scm_data: a pointer to the SCM device data
> > + * Return: 0 if the controller responded correctly, negative on
> > error
> > + */
> > +static int scm_heartbeat(struct scm_data *scm_data)
> > +{
> > + int rc;
> > +
> > + mutex_lock(&scm_data->admin_command.lock);
> > +
> > + rc = scm_admin_command_request(scm_data,
> > ADMIN_COMMAND_HEARTBEAT);
> > + if (rc)
> > + goto out;
> > +
> > + rc = scm_admin_command_execute(scm_data);
> > + if (rc)
> > + goto out;
> > +
> > + rc = scm_admin_command_complete_timeout(scm_data,
> > ADMIN_COMMAND_HEARTBEAT);
> > + if (rc < 0) {
> > + dev_err(&scm_data->dev, "Heartbeat timeout\n");
> > + goto out;
> > + }
> > +
> > + rc = scm_admin_response(scm_data);
> > + if (rc < 0)
> > + goto out;
> > + if (rc != STATUS_SUCCESS)
> > + scm_warn_status(scm_data, "Unexpected status from
> > heartbeat", rc);
> > +
> > + rc = scm_admin_response_handled(scm_data);
> > +
> > +out:
> > + mutex_unlock(&scm_data->admin_command.lock);
> > + return rc;
> > +}
> > +
> > /**
> > * allocate_scm_minor() - Allocate a minor number to use for an
> > SCM device
> > * @scm_data: The SCM device to associate the minor with
> > @@ -1508,6 +1546,11 @@ static int scm_probe(struct pci_dev *pdev,
> > const struct pci_device_id *ent)
> > goto err;
> > }
> >
> > + if (scm_heartbeat(scm_data)) {
> > + dev_err(&pdev->dev, "SCM Heartbeat failed\n");
> > + goto err;
> > + }
> > +
> > elapsed = 0;
> > timeout = scm_data->readiness_timeout + scm_data-
> > >memory_available_timeout;
> > while (!scm_is_usable(scm_data)) {
--
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819
^ permalink raw reply
* RE: [PATCH v2 13/27] nvdimm/ocxl: Add support for Admin commands
From: Alastair D'Silva @ 2020-02-19 5:00 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Masahiro Yamada,
Paul Mackerras, Mauro Carvalho Chehab, Ira Weiny, Thomas Gleixner,
Rob Herring, Dave Jiang, linux-nvdimm, Vishal Verma,
Krzysztof Kozlowski, Anju T Sudhakar, Mahesh Salgaonkar,
Andrew Donnellan, Arnd Bergmann, Greg Kurz, Nicholas Piggin,
Cédric Le Goater, Dan Williams, Hari Bathini, linux-mm,
Greg Kroah-Hartman, linux-kernel, Frederic Barrat, Andrew Morton,
linuxppc-dev, David S. Miller
In-Reply-To: <20200203141846.00004981@Huawei.com>
On Mon, 2020-02-03 at 14:18 +0000, Jonathan Cameron wrote:
> On Tue, 3 Dec 2019 14:46:41 +1100
> Alastair D'Silva <alastair@au1.ibm.com> wrote:
>
> > From: Alastair D'Silva <alastair@d-silva.org>
> >
> > This patch requests the metadata required to issue admin commands,
> > as well
> > as some helper functions to construct and check the completion of
> > the
> > commands.
> >
> > Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
>
> A few trivial bits inline.
>
> Jonathan
>
> > ---
> > drivers/nvdimm/ocxl/scm.c | 67 +++++++++++++
> > drivers/nvdimm/ocxl/scm_internal.c | 152
> > +++++++++++++++++++++++++++++
> > drivers/nvdimm/ocxl/scm_internal.h | 62 ++++++++++++
> > 3 files changed, 281 insertions(+)
> >
> > diff --git a/drivers/nvdimm/ocxl/scm.c b/drivers/nvdimm/ocxl/scm.c
> > index 8088f65c289e..1e175f3c3cf2 100644
> > --- a/drivers/nvdimm/ocxl/scm.c
> > +++ b/drivers/nvdimm/ocxl/scm.c
> > @@ -267,6 +267,58 @@ static int scm_register_lpc_mem(struct
> > scm_data *scm_data)
> > return 0;
> > }
> >
> > +/**
> > + * scm_extract_command_metadata() - Extract command data from MMIO
> > & save it for further use
> > + * @scm_data: a pointer to the SCM device data
> > + * @offset: The base address of the command data structures
> > (address of CREQO)
> > + * @command_metadata: A pointer to the command metadata to
> > populate
> > + * Return: 0 on success, negative on failure
> > + */
> > +static int scm_extract_command_metadata(struct scm_data *scm_data,
> > u32 offset,
> > + struct command_metadata
> > *command_metadata)
> > +{
> > + int rc;
> > + u64 tmp;
> > +
> > + rc = ocxl_global_mmio_read64(scm_data->ocxl_afu, offset,
> > OCXL_LITTLE_ENDIAN,
> > + &tmp);
> > + if (rc)
> > + return rc;
> > +
> > + command_metadata->request_offset = tmp >> 32;
> > + command_metadata->response_offset = tmp & 0xFFFFFFFF;
> > +
> > + rc = ocxl_global_mmio_read64(scm_data->ocxl_afu, offset + 8,
> > OCXL_LITTLE_ENDIAN,
> > + &tmp);
> > + if (rc)
> > + return rc;
> > +
> > + command_metadata->data_offset = tmp >> 32;
> > + command_metadata->data_size = tmp & 0xFFFFFFFF;
> > +
> > + command_metadata->id = 0;
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * scm_setup_command_metadata() - Set up the command metadata
> > + * @scm_data: a pointer to the SCM device data
> > + */
> > +static int scm_setup_command_metadata(struct scm_data *scm_data)
> > +{
> > + int rc;
> > +
> > + mutex_init(&scm_data->admin_command.lock);
> > +
> > + rc = scm_extract_command_metadata(scm_data,
> > GLOBAL_MMIO_ACMA_CREQO,
> > + &scm_data->admin_command);
> > + if (rc)
> > + return rc;
>
> Unless you are adding to this later in the series.
>
Ignored
> return scm_extract_command_metadata(scm_data,...)
>
> > +
> > + return 0;
> > +}
> > +
> > /**
> > * scm_is_usable() - Is a controller usable?
> > * @scm_data: a pointer to the SCM device data
> > @@ -276,6 +328,8 @@ static bool scm_is_usable(const struct scm_data
> > *scm_data)
> > {
> > u64 chi = 0;
> > int rc = scm_chi(scm_data, &chi);
> > + if (rc)
> > + return false;
> >
> > if (!(chi & GLOBAL_MMIO_CHI_CRDY)) {
> > dev_err(&scm_data->dev, "SCM controller is not
> > ready.\n");
> > @@ -502,6 +556,14 @@ static int scm_probe(struct pci_dev *pdev,
> > const struct pci_device_id *ent)
> > }
> > scm_data->pdev = pdev;
> >
> > + scm_data->timeouts[ADMIN_COMMAND_ERRLOG] = 2000; // ms
> > + scm_data->timeouts[ADMIN_COMMAND_HEARTBEAT] = 100; // ms
> > + scm_data->timeouts[ADMIN_COMMAND_SMART] = 100; // ms
> > + scm_data->timeouts[ADMIN_COMMAND_CONTROLLER_DUMP] = 1000; // ms
> > + scm_data->timeouts[ADMIN_COMMAND_CONTROLLER_STATS] = 100; // ms
> > + scm_data->timeouts[ADMIN_COMMAND_SHUTDOWN] = 1000; // ms
> > + scm_data->timeouts[ADMIN_COMMAND_FW_UPDATE] = 16000; // ms
> > +
> > pci_set_drvdata(pdev, scm_data);
> >
> > scm_data->ocxl_fn = ocxl_function_open(pdev);
> > @@ -543,6 +605,11 @@ static int scm_probe(struct pci_dev *pdev,
> > const struct pci_device_id *ent)
> > goto err;
> > }
> >
> > + if (scm_setup_command_metadata(scm_data)) {
> > + dev_err(&pdev->dev, "Could not read OCXL command
> > matada\n");
> > + goto err;
> > + }
> > +
> > elapsed = 0;
> > timeout = scm_data->readiness_timeout + scm_data-
> > >memory_available_timeout;
> > while (!scm_is_usable(scm_data)) {
> > diff --git a/drivers/nvdimm/ocxl/scm_internal.c
> > b/drivers/nvdimm/ocxl/scm_internal.c
> > index 72d3c0e7d846..7b11b56863fb 100644
> > --- a/drivers/nvdimm/ocxl/scm_internal.c
> > +++ b/drivers/nvdimm/ocxl/scm_internal.c
> > @@ -17,3 +17,155 @@ int scm_chi(const struct scm_data *scm_data,
> > u64 *chi)
> >
> > return 0;
> > }
> > +
> > +static int scm_command_request(const struct scm_data *scm_data,
> > + struct command_metadata *cmd, u8
> > op_code)
> > +{
> > + u64 val = op_code;
> > + int rc;
> > + u8 i;
> > +
> > + cmd->op_code = op_code;
> > + cmd->id++;
> > +
> > + val |= ((u64)cmd->id) << 16;
> > +
> > + rc = ocxl_global_mmio_write64(scm_data->ocxl_afu, cmd-
> > >request_offset,
> > + OCXL_LITTLE_ENDIAN, val);
> > + if (rc)
> > + return rc;
> > +
> > + for (i = 0x08; i <= 0x38; i += 0x08) {
>
> perhaps use sizeof(u64) to explain where the 0x08s come from.
> For the 0x38, might be worth a define.
Ok
>
> > + rc = ocxl_global_mmio_write64(scm_data->ocxl_afu,
> > + cmd->request_offset + i,
> > + OCXL_LITTLE_ENDIAN, 0);
> > + if (rc)
> > + return rc;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +int scm_admin_command_request(struct scm_data *scm_data, u8
> > op_code)
> > +{
> > + u64 val;
> > + int rc = ocxl_global_mmio_read64(scm_data->ocxl_afu,
> > GLOBAL_MMIO_CHI,
> > + OCXL_LITTLE_ENDIAN, &val);
> > + if (rc)
> > + return rc;
> > +
> > + return scm_command_request(scm_data, &scm_data->admin_command,
> > op_code);
> > +}
> > +
> > +static int scm_command_response(const struct scm_data *scm_data,
> > + const struct command_metadata *cmd)
> > +{
> > + u64 val;
> > + u16 id;
> > + u8 status;
> > + int rc = ocxl_global_mmio_read64(scm_data->ocxl_afu,
> > + cmd->response_offset,
> > + OCXL_LITTLE_ENDIAN, &val);
> > + if (rc)
> > + return rc;
> > +
> > + status = val & 0xff;
> > + id = (val >> 16) & 0xffff;
> > +
> > + if (id != cmd->id) {
> > + dev_warn(&scm_data->dev,
> > + "Expected response for command %d, but
> > received response for command %d instead.\n",
> > + cmd->id, id);
> > + }
> > +
> > + return status;
> > +}
> > +
> > +int scm_admin_response(const struct scm_data *scm_data)
> > +{
> > + return scm_command_response(scm_data, &scm_data-
> > >admin_command);
> > +}
> > +
> > +
> > +int scm_admin_command_execute(const struct scm_data *scm_data)
> > +{
> > + return ocxl_global_mmio_set64(scm_data->ocxl_afu,
> > GLOBAL_MMIO_HCI,
> > + OCXL_LITTLE_ENDIAN,
> > GLOBAL_MMIO_HCI_ACRW);
> > +}
> > +
> > +static bool scm_admin_command_complete(const struct scm_data
> > *scm_data)
> > +{
> > + u64 val = 0;
> > +
> > + int rc = scm_chi(scm_data, &val);
> > +
> > + WARN_ON(rc);
> > +
> > + return (val & GLOBAL_MMIO_CHI_ACRA) != 0;
> > +}
> > +
> > +int scm_admin_command_complete_timeout(const struct scm_data
> > *scm_data,
> > + int command)
> > +{
> > + u32 timeout = scm_data->timeouts[command];
> > + // 32 is the next power of 2 greater than the 20ms minimum for
> > msleep
> > +#define TIMEOUT_SLEEP_MILLIS 32
> > + timeout /= TIMEOUT_SLEEP_MILLIS;
> > + if (!timeout)
> > + timeout = SCM_DEFAULT_TIMEOUT / TIMEOUT_SLEEP_MILLIS;
> > +
> > + while (timeout-- > 0) {
> > + if (scm_admin_command_complete(scm_data))
> > + return 0;
> > + msleep(TIMEOUT_SLEEP_MILLIS);
> > + }
> > +
> > + if (scm_admin_command_complete(scm_data))
> > + return 0;
> > +
> > + return -EBUSY;
> > +}
> > +
> > +int scm_admin_response_handled(const struct scm_data *scm_data)
> > +{
> > + return ocxl_global_mmio_set64(scm_data->ocxl_afu,
> > GLOBAL_MMIO_CHIC,
> > + OCXL_LITTLE_ENDIAN,
> > GLOBAL_MMIO_CHI_ACRA);
> > +}
> > +
> > +void scm_warn_status(const struct scm_data *scm_data, const char
> > *message,
> > + u8 status)
> > +{
> > + const char *text = "Unknown";
> > +
> > + switch (status) {
> > + case STATUS_SUCCESS:
> > + text = "Success";
> > + break;
> > +
> > + case STATUS_MEM_UNAVAILABLE:
> > + text = "Persistent memory unavailable";
> > + break;
> > +
> > + case STATUS_BAD_OPCODE:
> > + text = "Bad opcode";
> > + break;
> > +
> > + case STATUS_BAD_REQUEST_PARM:
> > + text = "Bad request parameter";
> > + break;
> > +
> > + case STATUS_BAD_DATA_PARM:
> > + text = "Bad data parameter";
> > + break;
> > +
> > + case STATUS_DEBUG_BLOCKED:
> > + text = "Debug action blocked";
> > + break;
> > +
> > + case STATUS_FAIL:
> > + text = "Failed";
> > + break;
> > + }
> > +
> > + dev_warn(&scm_data->dev, "%s: %s (%x)\n", message, text,
> > status);
> > +}
> > diff --git a/drivers/nvdimm/ocxl/scm_internal.h
> > b/drivers/nvdimm/ocxl/scm_internal.h
> > index 584450f55e30..9bff684cd069 100644
> > --- a/drivers/nvdimm/ocxl/scm_internal.h
> > +++ b/drivers/nvdimm/ocxl/scm_internal.h
> > @@ -6,6 +6,8 @@
> > #include <linux/libnvdimm.h>
> > #include <linux/mm.h>
> >
> > +#define SCM_DEFAULT_TIMEOUT 100
> > +
> > #define GLOBAL_MMIO_CHI 0x000
> > #define GLOBAL_MMIO_CHIC 0x008
> > #define GLOBAL_MMIO_CHIE 0x010
> > @@ -80,6 +82,16 @@
> >
> > #define SCM_LABEL_AREA_SIZE (1UL << PA_SECTION_SHIFT)
> >
> > +struct command_metadata {
> > + u32 request_offset;
> > + u32 response_offset;
> > + u32 data_offset;
> > + u32 data_size;
> > + struct mutex lock;
> > + u16 id;
> > + u8 op_code;
> > +};
> > +
> > struct scm_function_0 {
> > struct pci_dev *pdev;
> > struct ocxl_fn *ocxl_fn;
> > @@ -95,9 +107,11 @@ struct scm_data {
> > struct ocxl_afu *ocxl_afu;
> > struct ocxl_context *ocxl_context;
> > void *metadata_addr;
> > + struct command_metadata admin_command;
> > struct resource scm_res;
> > struct nd_region *nd_region;
> > char fw_version[8+1];
> > + u32 timeouts[ADMIN_COMMAND_MAX+1];
> >
> > u32 max_controller_dump_size;
> > u16 scm_revision; // major/minor
> > @@ -122,3 +136,51 @@ struct scm_data {
> > * Returns 0 on success, negative on error
> > */
> > int scm_chi(const struct scm_data *scm_data, u64 *chi);
> > +
> > +/**
> > + * scm_admin_command_request() - Issue an admin command request
> > + * @scm_data: a pointer to the SCM device data
> > + * @op_code: The op-code for the command
> > + *
> > + * Returns an identifier for the command, or negative on error
> > + */
> > +int scm_admin_command_request(struct scm_data *scm_data, u8
> > op_code);
> > +
> > +/**
> > + * scm_admin_response() - Validate an admin response
> > + * @scm_data: a pointer to the SCM device data
> > + * Returns the status code of the command, or negative on error
> > + */
> > +int scm_admin_response(const struct scm_data *scm_data);
> > +
> > +/**
> > + * scm_admin_command_execute() - Notify the controller to start
> > processing a pending admin command
> > + * @scm_data: a pointer to the SCM device data
> > + * Returns 0 on success, negative on error
> > + */
> > +int scm_admin_command_execute(const struct scm_data *scm_data);
> > +
> > +/**
> > + * scm_admin_command_complete_timeout() - Wait for an admin
> > command to finish executing
> > + * @scm_data: a pointer to the SCM device data
> > + * @command: the admin command to wait for completion (determines
> > the timeout)
> > + * Returns 0 on success, -EBUSY on timeout
> > + */
> > +int scm_admin_command_complete_timeout(const struct scm_data
> > *scm_data,
> > + int command);
> > +
> > +/**
> > + * scm_admin_response_handled() - Notify the controller that the
> > admin response has been handled
> > + * @scm_data: a pointer to the SCM device data
> > + * Returns 0 on success, negative on failure
> > + */
> > +int scm_admin_response_handled(const struct scm_data *scm_data);
> > +
> > +/**
> > + * scm_warn_status() - Emit a kernel warning showing a command
> > status.
> > + * @scm_data: a pointer to the SCM device data
> > + * @message: A message to accompany the warning
> > + * @status: The command status
> > + */
> > +void scm_warn_status(const struct scm_data *scm_data, const char
> > *message,
> > + u8 status);
--
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819
^ permalink raw reply
* RE: [PATCH v2 14/27] nvdimm/ocxl: Add support for near storage commands
From: Alastair D'Silva @ 2020-02-19 4:54 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Masahiro Yamada,
Paul Mackerras, Mauro Carvalho Chehab, Ira Weiny, Thomas Gleixner,
Rob Herring, Dave Jiang, linux-nvdimm, Vishal Verma,
Krzysztof Kozlowski, Anju T Sudhakar, Mahesh Salgaonkar,
Andrew Donnellan, Arnd Bergmann, Greg Kurz, Nicholas Piggin,
Cédric Le Goater, Dan Williams, Hari Bathini, linux-mm,
Greg Kroah-Hartman, linux-kernel, Frederic Barrat, Andrew Morton,
linuxppc-dev, David S. Miller
In-Reply-To: <20200203142254.00007377@Huawei.com>
On Mon, 2020-02-03 at 14:22 +0000, Jonathan Cameron wrote:
> On Tue, 3 Dec 2019 14:46:42 +1100
> Alastair D'Silva <alastair@au1.ibm.com> wrote:
>
> > From: Alastair D'Silva <alastair@d-silva.org>
> >
> > Similar to the previous patch, this adds support for near storage
> > commands.
> >
> > Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> > ---
> > drivers/nvdimm/ocxl/scm.c | 6 +++++
> > drivers/nvdimm/ocxl/scm_internal.c | 41
> > ++++++++++++++++++++++++++++++
> > drivers/nvdimm/ocxl/scm_internal.h | 38
> > +++++++++++++++++++++++++++
> > 3 files changed, 85 insertions(+)
> >
> > diff --git a/drivers/nvdimm/ocxl/scm.c b/drivers/nvdimm/ocxl/scm.c
> > index 1e175f3c3cf2..6c16ca7fabfa 100644
> > --- a/drivers/nvdimm/ocxl/scm.c
> > +++ b/drivers/nvdimm/ocxl/scm.c
> > @@ -310,12 +310,18 @@ static int scm_setup_command_metadata(struct
> > scm_data *scm_data)
> > int rc;
> >
> > mutex_init(&scm_data->admin_command.lock);
> > + mutex_init(&scm_data->ns_command.lock);
> >
> > rc = scm_extract_command_metadata(scm_data,
> > GLOBAL_MMIO_ACMA_CREQO,
> > &scm_data->admin_command);
> > if (rc)
> > return rc;
> >
> > + rc = scm_extract_command_metadata(scm_data,
> > GLOBAL_MMIO_NSCMA_CREQO,
> > + &scm_data->ns_command);
> > + if (rc)
> > + return rc;
> > +
>
> Ah. So much for my comment in previous patch. Ignore that...
>
> > return 0;
> > }
> >
> > diff --git a/drivers/nvdimm/ocxl/scm_internal.c
> > b/drivers/nvdimm/ocxl/scm_internal.c
> > index 7b11b56863fb..c405f1d8afb8 100644
> > --- a/drivers/nvdimm/ocxl/scm_internal.c
> > +++ b/drivers/nvdimm/ocxl/scm_internal.c
> > @@ -132,6 +132,47 @@ int scm_admin_response_handled(const struct
> > scm_data *scm_data)
> > OCXL_LITTLE_ENDIAN,
> > GLOBAL_MMIO_CHI_ACRA);
> > }
> >
> > +int scm_ns_command_request(struct scm_data *scm_data, u8 op_code)
> > +{
> > + u64 val;
> > + int rc = ocxl_global_mmio_read64(scm_data->ocxl_afu,
> > GLOBAL_MMIO_CHI,
> > + OCXL_LITTLE_ENDIAN, &val);
> > + if (rc)
> > + return rc;
> > +
> > + if (!(val & GLOBAL_MMIO_CHI_NSCRA))
> > + return -EBUSY;
> > +
> > + return scm_command_request(scm_data, &scm_data->ns_command,
> > op_code);
> > +}
> > +
> > +int scm_ns_response(const struct scm_data *scm_data)
> > +{
> > + return scm_command_response(scm_data, &scm_data->ns_command);
> > +}
> > +
> > +int scm_ns_command_execute(const struct scm_data *scm_data)
> > +{
> > + return ocxl_global_mmio_set64(scm_data->ocxl_afu,
> > GLOBAL_MMIO_HCI,
> > + OCXL_LITTLE_ENDIAN,
> > GLOBAL_MMIO_HCI_NSCRW);
> > +}
> > +
> > +bool scm_ns_command_complete(const struct scm_data *scm_data)
> > +{
> > + u64 val = 0;
> > + int rc = scm_chi(scm_data, &val);
> > +
> > + WARN_ON(rc);
> > +
> > + return (val & GLOBAL_MMIO_CHI_NSCRA) != 0;
> > +}
> > +
> > +int scm_ns_response_handled(const struct scm_data *scm_data)
> > +{
> > + return ocxl_global_mmio_set64(scm_data->ocxl_afu,
> > GLOBAL_MMIO_CHIC,
> > + OCXL_LITTLE_ENDIAN,
> > GLOBAL_MMIO_CHI_NSCRA);
> > +}
> > +
> > void scm_warn_status(const struct scm_data *scm_data, const char
> > *message,
> > u8 status)
> > {
> > diff --git a/drivers/nvdimm/ocxl/scm_internal.h
> > b/drivers/nvdimm/ocxl/scm_internal.h
> > index 9bff684cd069..9575996a89e7 100644
> > --- a/drivers/nvdimm/ocxl/scm_internal.h
> > +++ b/drivers/nvdimm/ocxl/scm_internal.h
> > @@ -108,6 +108,7 @@ struct scm_data {
> > struct ocxl_context *ocxl_context;
> > void *metadata_addr;
> > struct command_metadata admin_command;
> > + struct command_metadata ns_command;
> > struct resource scm_res;
> > struct nd_region *nd_region;
> > char fw_version[8+1];
> > @@ -176,6 +177,42 @@ int scm_admin_command_complete_timeout(const
> > struct scm_data *scm_data,
> > */
> > int scm_admin_response_handled(const struct scm_data *scm_data);
> >
> > +/**
> > + * scm_ns_command_request() - Issue a near storage command request
> > + * @scm_data: a pointer to the SCM device data
> > + * @op_code: The op-code for the command
> > + * Returns an identifier for the command, or negative on error
> > + */
> > +int scm_ns_command_request(struct scm_data *scm_data, u8 op_code);
> > +
> > +/**
> > + * scm_ns_response() - Validate a near storage response
> > + * @scm_data: a pointer to the SCM device data
> > + * Returns the status code of the command, or negative on error
> > + */
> > +int scm_ns_response(const struct scm_data *scm_data);
> > +
> > +/**
> > + * scm_ns_command_execute() - Notify the controller to start
> > processing a pending near storage command
> > + * @scm_data: a pointer to the SCM device data
> > + * Returns 0 on success, negative on error
> > + */
> > +int scm_ns_command_execute(const struct scm_data *scm_data);
> > +
> > +/**
> > + * scm_ns_command_complete() - Is a near storage command executing
> > + * scm_data: a pointer to the SCM device data
> > + * Returns true if the previous admin command has completed
> > + */
> > +bool scm_ns_command_complete(const struct scm_data *scm_data);
> > +
> > +/**
> > + * scm_ns_response_handled() - Notify the controller that the near
> > storage response has been handled
> > + * scm_data: a pointer to the SCM device data
> > + * Returns 0 on success, negative on failure
> > + */
> > +int scm_ns_response_handled(const struct scm_data *scm_data);
> > +
> > /**
> > * scm_warn_status() - Emit a kernel warning showing a command
> > status.
> > * @scm_data: a pointer to the SCM device data
> > @@ -184,3 +221,4 @@ int scm_admin_response_handled(const struct
> > scm_data *scm_data);
> > */
> > void scm_warn_status(const struct scm_data *scm_data, const char
> > *message,
> > u8 status);
> > +
> Stray blank line!
Ok
>
> Now we are into the real nitpicks. Not enough coffee.
>
> Jonathan
>
--
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819
^ permalink raw reply
* RE: [PATCH v2 12/27] nvdimm/ocxl: Read the capability registers & wait for device ready
From: Alastair D'Silva @ 2020-02-19 4:46 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Keith Busch,
Masahiro Yamada, Paul Mackerras, Mauro Carvalho Chehab, Ira Weiny,
Thomas Gleixner, Rob Herring, Dave Jiang, linux-nvdimm,
Vishal Verma, Krzysztof Kozlowski, Anju T Sudhakar,
Mahesh Salgaonkar, Andrew Donnellan, Arnd Bergmann, Greg Kurz,
Nicholas Piggin, Cédric Le Goater, Dan Williams,
Hari Bathini, linux-mm, Greg Kroah-Hartman, linux-kernel,
Frederic Barrat, Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <20200203132351.00005281@Huawei.com>
On Mon, 2020-02-03 at 13:23 +0000, Jonathan Cameron wrote:
> On Tue, 3 Dec 2019 14:46:40 +1100
> Alastair D'Silva <alastair@au1.ibm.com> wrote:
>
> > From: Alastair D'Silva <alastair@d-silva.org>
> >
> > This patch reads timeouts & firmware version from the controller,
> > and
> > uses those timeouts to wait for the controller to report that it is
> > ready
> > before handing the memory over to libnvdimm.
> >
> > Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> > ---
> > drivers/nvdimm/ocxl/Makefile | 2 +-
> > drivers/nvdimm/ocxl/scm.c | 84
> > ++++++++++++++++++++++++++++++
> > drivers/nvdimm/ocxl/scm_internal.c | 19 +++++++
> > drivers/nvdimm/ocxl/scm_internal.h | 24 +++++++++
> > 4 files changed, 128 insertions(+), 1 deletion(-)
> > create mode 100644 drivers/nvdimm/ocxl/scm_internal.c
> >
> > diff --git a/drivers/nvdimm/ocxl/Makefile
> > b/drivers/nvdimm/ocxl/Makefile
> > index 74a1bd98848e..9b6e31f0eb3e 100644
> > --- a/drivers/nvdimm/ocxl/Makefile
> > +++ b/drivers/nvdimm/ocxl/Makefile
> > @@ -4,4 +4,4 @@ ccflags-$(CONFIG_PPC_WERROR) += -Werror
> >
> > obj-$(CONFIG_OCXL_SCM) += ocxlscm.o
> >
> > -ocxlscm-y := scm.o
> > +ocxlscm-y := scm.o scm_internal.o
> > diff --git a/drivers/nvdimm/ocxl/scm.c b/drivers/nvdimm/ocxl/scm.c
> > index 571058a9e7b8..8088f65c289e 100644
> > --- a/drivers/nvdimm/ocxl/scm.c
> > +++ b/drivers/nvdimm/ocxl/scm.c
> > @@ -7,6 +7,7 @@
> >
> > #include <linux/module.h>
> > #include <misc/ocxl.h>
> > +#include <linux/delay.h>
> > #include <linux/ndctl.h>
> > #include <linux/mm_types.h>
> > #include <linux/memory_hotplug.h>
> > @@ -266,6 +267,30 @@ static int scm_register_lpc_mem(struct
> > scm_data *scm_data)
> > return 0;
> > }
> >
> > +/**
> > + * scm_is_usable() - Is a controller usable?
> > + * @scm_data: a pointer to the SCM device data
> > + * Return: true if the controller is usable
> > + */
> > +static bool scm_is_usable(const struct scm_data *scm_data)
> > +{
> > + u64 chi = 0;
> > + int rc = scm_chi(scm_data, &chi);
> > +
> > + if (!(chi & GLOBAL_MMIO_CHI_CRDY)) {
> > + dev_err(&scm_data->dev, "SCM controller is not
> > ready.\n");
> > + return false;
> > + }
> > +
> > + if (!(chi & GLOBAL_MMIO_CHI_MA)) {
> > + dev_err(&scm_data->dev,
> > + "SCM controller does not have memory
> > available.\n");
> > + return false;
> > + }
> > +
> > + return true;
> > +}
> > +
> > /**
> > * allocate_scm_minor() - Allocate a minor number to use for an
> > SCM device
> > * @scm_data: The SCM device to associate the minor with
> > @@ -380,6 +405,48 @@ static void scm_remove(struct pci_dev *pdev)
> > }
> > }
> >
> > +/**
> > + * read_device_metadata() - Retrieve config information from the
> > AFU and save it for future use
> > + * @scm_data: the SCM metadata
> > + * Return: 0 on success, negative on failure
> > + */
> > +static int read_device_metadata(struct scm_data *scm_data)
> > +{
> > + u64 val;
> > + int rc;
> > +
> > + rc = ocxl_global_mmio_read64(scm_data->ocxl_afu,
> > GLOBAL_MMIO_CCAP0,
> > + OCXL_LITTLE_ENDIAN, &val);
> > + if (rc)
> > + return rc;
> > +
> > + scm_data->scm_revision = val & 0xFFFF;
> > + scm_data->read_latency = (val >> 32) & 0xFF;
> > + scm_data->readiness_timeout = (val >> 48) & 0xff;
> > + scm_data->memory_available_timeout = val >> 52;
>
> This overlaps with the masked region for readiness_timeout. I'll
> guess the maks
> on that should be 0xF.
>
Good catch, you're correct.
> > +
> > + rc = ocxl_global_mmio_read64(scm_data->ocxl_afu,
> > GLOBAL_MMIO_CCAP1,
> > + OCXL_LITTLE_ENDIAN, &val);
> > + if (rc)
> > + return rc;
> > +
> > + scm_data->max_controller_dump_size = val & 0xFFFFFFFF;
> > +
> > + // Extract firmware version text
> > + rc = ocxl_global_mmio_read64(scm_data->ocxl_afu,
> > GLOBAL_MMIO_FWVER,
> > + OCXL_HOST_ENDIAN, (u64 *)scm_data-
> > >fw_version);
> > + if (rc)
> > + return rc;
> > +
> > + scm_data->fw_version[8] = '\0';
> > +
> > + dev_info(&scm_data->dev,
> > + "Firmware version '%s' SCM revision %d:%d\n",
> > scm_data->fw_version,
> > + scm_data->scm_revision >> 4, scm_data->scm_revision &
> > 0x0F);
> > +
> > + return 0;
> > +}
> > +
> > /**
> > * scm_probe_function_0 - Set up function 0 for an OpenCAPI
> > Storage Class Memory device
> > * This is important as it enables templates higher than 0 across
> > all other functions,
> > @@ -420,6 +487,8 @@ static int scm_probe_function_0(struct pci_dev
> > *pdev)
> > static int scm_probe(struct pci_dev *pdev, const struct
> > pci_device_id *ent)
> > {
> > struct scm_data *scm_data = NULL;
> > + int elapsed;
> > + u16 timeout;
> >
> > if (PCI_FUNC(pdev->devfn) == 0)
> > return scm_probe_function_0(pdev);
> > @@ -469,6 +538,21 @@ static int scm_probe(struct pci_dev *pdev,
> > const struct pci_device_id *ent)
> > goto err;
> > }
> >
> > + if (read_device_metadata(scm_data)) {
> > + dev_err(&pdev->dev, "Could not read SCM device
> > metadata\n");
> > + goto err;
> > + }
> > +
> > + elapsed = 0;
> > + timeout = scm_data->readiness_timeout + scm_data-
> > >memory_available_timeout;
> > + while (!scm_is_usable(scm_data)) {
> > + if (elapsed++ > timeout) {
> > + dev_warn(&scm_data->dev, "SCM ready
> > timeout.\n");
> > + goto err;
> > + }
> > +
> > + msleep(1000);
> > + }
> > if (scm_register_lpc_mem(scm_data)) {
> > dev_err(&pdev->dev, "Could not register OCXL SCM memory
> > with libnvdimm\n");
> > goto err;
> > diff --git a/drivers/nvdimm/ocxl/scm_internal.c
> > b/drivers/nvdimm/ocxl/scm_internal.c
> > new file mode 100644
> > index 000000000000..72d3c0e7d846
> > --- /dev/null
> > +++ b/drivers/nvdimm/ocxl/scm_internal.c
> > @@ -0,0 +1,19 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +// Copyright 2019 IBM Corp.
> > +
> > +#include <misc/ocxl.h>
> > +#include <linux/delay.h>
> > +#include "scm_internal.h"
> > +
> > +int scm_chi(const struct scm_data *scm_data, u64 *chi)
> > +{
> > + u64 val;
> > + int rc = ocxl_global_mmio_read64(scm_data->ocxl_afu,
> > GLOBAL_MMIO_CHI,
> > + OCXL_LITTLE_ENDIAN, &val);
> > + if (rc)
> > + return rc;
> > +
> > + *chi = val;
> > +
> > + return 0;
> > +}
> > diff --git a/drivers/nvdimm/ocxl/scm_internal.h
> > b/drivers/nvdimm/ocxl/scm_internal.h
> > index d6ab361f5de9..584450f55e30 100644
> > --- a/drivers/nvdimm/ocxl/scm_internal.h
> > +++ b/drivers/nvdimm/ocxl/scm_internal.h
> > @@ -97,4 +97,28 @@ struct scm_data {
> > void *metadata_addr;
> > struct resource scm_res;
> > struct nd_region *nd_region;
> > + char fw_version[8+1];
> > +
> > + u32 max_controller_dump_size;
> > + u16 scm_revision; // major/minor
> > + u8 readiness_timeout; /* The worst case time (in seconds) that
> > the host shall
> > + * wait for the controller to become
> > operational following a reset (CHI.CRDY).
> > + */
> > + u8 memory_available_timeout; /* The worst case time (in
> > seconds) that the host shall
> > + * wait for memory to become
> > available following a reset (CHI.MA).
> > + */
> > +
> > + u16 read_latency; /* The nominal measure of latency (in
> > nanoseconds)
> > + * associated with an unassisted read of a
> > memory block.
> > + * This represents the capability of the raw
> > media technology without assistance
> > + */
> > };
> > +
> > +/**
> > + * scm_chi() - Get the value of the CHI register
> > + * @scm_data: The SCM metadata
> > + * @chi: returns the CHI value
> > + *
> > + * Returns 0 on success, negative on error
> > + */
> > +int scm_chi(const struct scm_data *scm_data, u64 *chi);
--
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819
^ permalink raw reply
* RE: [PATCH v2 10/27] nvdimm: Add driver for OpenCAPI Storage Class Memory
From: Alastair D'Silva @ 2020-02-19 4:40 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Masahiro Yamada,
Paul Mackerras, Mauro Carvalho Chehab, Ira Weiny, Thomas Gleixner,
Rob Herring, Dave Jiang, linux-nvdimm, Vishal Verma,
Krzysztof Kozlowski, Anju T Sudhakar, Mahesh Salgaonkar,
Andrew Donnellan, Arnd Bergmann, Greg Kurz, Nicholas Piggin,
Cédric Le Goater, Dan Williams, Hari Bathini, linux-mm,
Greg Kroah-Hartman, linux-kernel, Frederic Barrat, Andrew Morton,
linuxppc-dev, David S. Miller
In-Reply-To: <20200203132036.00000040@Huawei.com>
On Mon, 2020-02-03 at 13:20 +0000, Jonathan Cameron wrote:
> On Tue, 3 Dec 2019 14:46:38 +1100
> Alastair D'Silva <alastair@au1.ibm.com> wrote:
>
> > From: Alastair D'Silva <alastair@d-silva.org>
> >
> > This driver exposes LPC memory on OpenCAPI SCM cards
> > as an NVDIMM, allowing the existing nvram infrastructure
> > to be used.
> >
> > Namespace metadata is stored on the media itself, so
> > scm_reserve_metadata() maps 1 section's worth of PMEM storage
> > at the start to hold this. The rest of the PMEM range is registered
> > with libnvdimm as an nvdimm. scm_ndctl_config_read/write/size()
> > provide
> > callbacks to libnvdimm to access the metadata.
> >
> > Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> Hi Alastair,
>
> A few bits and bobs inline.
>
> Thanks,
>
> Jonathan
>
> > ---
> > drivers/nvdimm/Kconfig | 2 +
> > drivers/nvdimm/Makefile | 2 +-
> > drivers/nvdimm/ocxl/Kconfig | 15 +
> > drivers/nvdimm/ocxl/Makefile | 7 +
> > drivers/nvdimm/ocxl/scm.c | 519
> > +++++++++++++++++++++++++++++
> > drivers/nvdimm/ocxl/scm_internal.h | 28 ++
> > 6 files changed, 572 insertions(+), 1 deletion(-)
> > create mode 100644 drivers/nvdimm/ocxl/Kconfig
> > create mode 100644 drivers/nvdimm/ocxl/Makefile
> > create mode 100644 drivers/nvdimm/ocxl/scm.c
> > create mode 100644 drivers/nvdimm/ocxl/scm_internal.h
> >
> > diff --git a/drivers/nvdimm/Kconfig b/drivers/nvdimm/Kconfig
> > index 36af7af6b7cf..d1bab36da61c 100644
> > --- a/drivers/nvdimm/Kconfig
> > +++ b/drivers/nvdimm/Kconfig
> > @@ -130,4 +130,6 @@ config NVDIMM_TEST_BUILD
> > core devm_memremap_pages() implementation and other
> > infrastructure.
> >
> > +source "drivers/nvdimm/ocxl/Kconfig"
> > +
> > endif
> > diff --git a/drivers/nvdimm/Makefile b/drivers/nvdimm/Makefile
> > index 29203f3d3069..e33492128042 100644
> > --- a/drivers/nvdimm/Makefile
> > +++ b/drivers/nvdimm/Makefile
> > @@ -1,5 +1,5 @@
> > # SPDX-License-Identifier: GPL-2.0
> > -obj-$(CONFIG_LIBNVDIMM) += libnvdimm.o
> > +obj-$(CONFIG_LIBNVDIMM) += libnvdimm.o ocxl/
> > obj-$(CONFIG_BLK_DEV_PMEM) += nd_pmem.o
> > obj-$(CONFIG_ND_BTT) += nd_btt.o
> > obj-$(CONFIG_ND_BLK) += nd_blk.o
> > diff --git a/drivers/nvdimm/ocxl/Kconfig
> > b/drivers/nvdimm/ocxl/Kconfig
> > new file mode 100644
> > index 000000000000..24099b300f5e
> > --- /dev/null
> > +++ b/drivers/nvdimm/ocxl/Kconfig
> > @@ -0,0 +1,15 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +if LIBNVDIMM
> > +
> > +config OCXL_SCM
> > + tristate "OpenCAPI Storage Class Memory"
> > + depends on LIBNVDIMM && PPC_POWERNV && PCI && EEH
> > + select ZONE_DEVICE
> > + select OCXL
> > + help
> > + Exposes devices that implement the OpenCAPI Storage Class
> > Memory
> > + specification as persistent memory regions.
> > +
> > + Select N if unsure.
> > +
> > +endif
> > diff --git a/drivers/nvdimm/ocxl/Makefile
> > b/drivers/nvdimm/ocxl/Makefile
> > new file mode 100644
> > index 000000000000..74a1bd98848e
> > --- /dev/null
> > +++ b/drivers/nvdimm/ocxl/Makefile
> > @@ -0,0 +1,7 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +ccflags-$(CONFIG_PPC_WERROR) += -Werror
> > +
> > +obj-$(CONFIG_OCXL_SCM) += ocxlscm.o
> > +
> > +ocxlscm-y := scm.o
> > diff --git a/drivers/nvdimm/ocxl/scm.c b/drivers/nvdimm/ocxl/scm.c
> > new file mode 100644
> > index 000000000000..571058a9e7b8
> > --- /dev/null
> > +++ b/drivers/nvdimm/ocxl/scm.c
> > @@ -0,0 +1,519 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +// Copyright 2019 IBM Corp.
> > +
> > +/*
> > + * A driver for Storage Class Memory, connected via OpenCAPI
> > + */
> > +
> > +#include <linux/module.h>
> > +#include <misc/ocxl.h>
> > +#include <linux/ndctl.h>
> > +#include <linux/mm_types.h>
> > +#include <linux/memory_hotplug.h>
> > +#include "scm_internal.h"
> > +
> > +
> > +static const struct pci_device_id scm_pci_tbl[] = {
> > + { PCI_DEVICE(PCI_VENDOR_ID_IBM, 0x0625), },
> > + { }
> > +};
> > +
> > +MODULE_DEVICE_TABLE(pci, scm_pci_tbl);
> > +
> > +#define SCM_NUM_MINORS 256 // Total to reserve
> > +
> > +static dev_t scm_dev;
> > +static struct class *scm_class;
> > +static struct mutex minors_idr_lock;
> > +static struct idr minors_idr;
> > +
> > +static const struct attribute_group *scm_pmem_attribute_groups[] =
> > {
> > + &nvdimm_bus_attribute_group,
> > + NULL,
> > +};
> > +
> > +static const struct attribute_group
> > *scm_pmem_region_attribute_groups[] = {
> > + &nd_region_attribute_group,
> > + &nd_device_attribute_group,
> > + &nd_mapping_attribute_group,
> > + &nd_numa_attribute_group,
> > + NULL,
> > +};
> > +
> > +/**
> > + * scm_ndctl_config_write() - Handle a ND_CMD_SET_CONFIG_DATA
> > command from ndctl
> > + * @scm_data: the SCM metadata
> > + * @command: the incoming data to write
> > + * Return: 0 on success, negative on failure
> > + */
> > +static int scm_ndctl_config_write(struct scm_data *scm_data,
> > + struct nd_cmd_set_config_hdr
> > *command)
> > +{
> > + if (command->in_offset + command->in_length >
> > SCM_LABEL_AREA_SIZE)
> > + return -EINVAL;
> > +
> > + memcpy_flushcache(scm_data->metadata_addr + command->in_offset,
> > command->in_buf,
> > + command->in_length);
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * scm_ndctl_config_read() - Handle a ND_CMD_GET_CONFIG_DATA
> > command from ndctl
> > + * @scm_data: the SCM metadata
> > + * @command: the read request
> > + * Return: 0 on success, negative on failure
> > + */
> > +static int scm_ndctl_config_read(struct scm_data *scm_data,
> > + struct nd_cmd_get_config_data_hdr
> > *command)
> > +{
> > + if (command->in_offset + command->in_length >
> > SCM_LABEL_AREA_SIZE)
> > + return -EINVAL;
> > +
> > + memcpy_mcsafe(command->out_buf, scm_data->metadata_addr +
> > command->in_offset,
> > + command->in_length);
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * scm_ndctl_config_size() - Handle a ND_CMD_GET_CONFIG_SIZE
> > command from ndctl
> > + * @scm_data: the SCM metadata
> > + * @command: the read request
> > + * Return: 0 on success, negative on failure
> > + */
> > +static int scm_ndctl_config_size(struct nd_cmd_get_config_size
> > *command)
> > +{
> > + command->status = 0;
> > + command->config_size = SCM_LABEL_AREA_SIZE;
> > + command->max_xfer = PAGE_SIZE;
> > +
> > + return 0;
> > +}
> > +
> > +static int scm_ndctl(struct nvdimm_bus_descriptor *nd_desc,
> > + struct nvdimm *nvdimm,
> > + unsigned int cmd, void *buf, unsigned int buf_len,
> > int *cmd_rc)
> > +{
> > + struct scm_data *scm_data = container_of(nd_desc, struct
> > scm_data, bus_desc);
> > +
> > + switch (cmd) {
> > + case ND_CMD_GET_CONFIG_SIZE:
> > + *cmd_rc = scm_ndctl_config_size(buf);
> > + return 0;
> > +
> > + case ND_CMD_GET_CONFIG_DATA:
> > + *cmd_rc = scm_ndctl_config_read(scm_data, buf);
> > + return 0;
> > +
> > + case ND_CMD_SET_CONFIG_DATA:
> > + *cmd_rc = scm_ndctl_config_write(scm_data, buf);
> > + return 0;
> > +
> > + default:
> > + return -ENOTTY;
> > + }
> > +}
> > +
> > +static ssize_t serial_show(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + struct nvdimm *nvdimm = to_nvdimm(dev);
> > + struct scm_data *scm_data = nvdimm_provider_data(nvdimm);
> > + const struct ocxl_fn_config *config =
> > ocxl_function_config(scm_data->ocxl_fn);
> > +
> > + return sprintf(buf, "0x%llx\n", config->serial);
> > +}
> > +static DEVICE_ATTR_RO(serial);
> > +
> > +static struct attribute *scm_dimm_attributes[] = {
> > + &dev_attr_serial.attr,
> > + NULL,
> > +};
> > +
> > +static umode_t scm_dimm_attr_visible(struct kobject *kobj,
> > + struct attribute *a, int n)
> > +{
> > + return a->mode;
> > +}
> > +
> > +static const struct attribute_group scm_dimm_attribute_group = {
> > + .name = "ocxl",
> > + .attrs = scm_dimm_attributes,
> > + .is_visible = scm_dimm_attr_visible,
> > +};
> > +
> > +static const struct attribute_group *scm_dimm_attribute_groups[] =
> > {
> > + &nvdimm_attribute_group,
> > + &nd_device_attribute_group,
> > + &scm_dimm_attribute_group,
> > + NULL,
> > +};
> > +
> > +/**
> > + * scm_reserve_metadata() - Reserve space for nvdimm metadata
> > + * @scm_data: The SCM device data
> > + * @lpc_mem: The resource representing the LPC memory of the SCM
> > device
> > + */
> > +static int scm_reserve_metadata(struct scm_data *scm_data,
> > + struct resource *lpc_mem)
> > +{
> > + scm_data->metadata_addr = devm_memremap(&scm_data->dev,
> > lpc_mem->start,
> > + SCM_LABEL_AREA_SIZE,
> > MEMREMAP_WB);
> > + if (IS_ERR(scm_data->metadata_addr))
> > + return PTR_ERR(scm_data->metadata_addr);
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * scm_register_lpc_mem() - Discover persistent memory on a device
> > and register it with the NVDIMM subsystem
> > + * @scm_data: The SCM device data
> > + * Return: 0 on success
> > + */
> > +static int scm_register_lpc_mem(struct scm_data *scm_data)
> > +{
> > + struct nd_region_desc region_desc;
> > + struct nd_mapping_desc nd_mapping_desc;
> > + struct resource *lpc_mem;
> > + const struct ocxl_afu_config *config;
> > + const struct ocxl_fn_config *fn_config;
> > + int rc;
> > + unsigned long nvdimm_cmd_mask = 0;
> > + unsigned long nvdimm_flags = 0;
> > + int target_node;
> > + char serial[16+1];
> > +
> > + // Set up the reserved metadata area
> > + rc = ocxl_afu_map_lpc_mem(scm_data->ocxl_afu);
> > + if (rc < 0)
> > + return rc;
> > +
> > + lpc_mem = ocxl_afu_lpc_mem(scm_data->ocxl_afu);
> > + if (lpc_mem == NULL || lpc_mem->start == 0)
> > + return -EINVAL;
> > +
> > + config = ocxl_afu_config(scm_data->ocxl_afu);
> > + fn_config = ocxl_function_config(scm_data->ocxl_fn);
> > +
> > + rc = scm_reserve_metadata(scm_data, lpc_mem);
> > + if (rc)
> > + return rc;
> > +
> > + scm_data->bus_desc.attr_groups = scm_pmem_attribute_groups;
> > + scm_data->bus_desc.provider_name = "ocxl-scm";
> > + scm_data->bus_desc.ndctl = scm_ndctl;
> > + scm_data->bus_desc.module = THIS_MODULE;
> > +
> > + scm_data->nvdimm_bus = nvdimm_bus_register(&scm_data->dev,
> > + &scm_data->bus_desc);
>
> odd alignment.
Ok
>
> > + if (!scm_data->nvdimm_bus)
> > + return -EINVAL;
> > +
> > + scm_data->scm_res.start = (u64)lpc_mem->start +
> > SCM_LABEL_AREA_SIZE;
> > + scm_data->scm_res.end = (u64)lpc_mem->start + config-
> > >lpc_mem_size - 1;
> > + scm_data->scm_res.name = "SCM persistent memory";
> > +
> > + set_bit(ND_CMD_GET_CONFIG_SIZE, &nvdimm_cmd_mask);
> > + set_bit(ND_CMD_GET_CONFIG_DATA, &nvdimm_cmd_mask);
> > + set_bit(ND_CMD_SET_CONFIG_DATA, &nvdimm_cmd_mask);
> > +
> > + set_bit(NDD_ALIASING, &nvdimm_flags);
> > +
> > + snprintf(serial, sizeof(serial), "%llx", fn_config->serial);
> > + nd_mapping_desc.nvdimm = nvdimm_create(scm_data->nvdimm_bus,
> > scm_data,
> > + scm_dimm_attribute_groups,
> > + nvdimm_flags, nvdimm_cmd_mask,
> > + 0, NULL);
> > + if (!nd_mapping_desc.nvdimm)
> > + return -ENOMEM;
> > +
> > + if (nvdimm_bus_check_dimm_count(scm_data->nvdimm_bus, 1))
> > + return -EINVAL;
> > +
> > + nd_mapping_desc.start = scm_data->scm_res.start;
> > + nd_mapping_desc.size = resource_size(&scm_data->scm_res);
> > + nd_mapping_desc.position = 0;
> > +
> > + scm_data->nd_set.cookie1 = fn_config->serial + 1; // allow for
> > empty serial
> > + scm_data->nd_set.cookie2 = fn_config->serial + 1;
> > +
> > + target_node = of_node_to_nid(scm_data->pdev->dev.of_node);
> > +
> > + memset(®ion_desc, 0, sizeof(region_desc));
> > + region_desc.res = &scm_data->scm_res;
> > + region_desc.attr_groups = scm_pmem_region_attribute_groups;
> > + region_desc.numa_node = NUMA_NO_NODE;
> > + region_desc.target_node = target_node;
> > + region_desc.num_mappings = 1;
> > + region_desc.mapping = &nd_mapping_desc;
> > + region_desc.nd_set = &scm_data->nd_set;
> > +
> > + set_bit(ND_REGION_PAGEMAP, ®ion_desc.flags);
> > + /*
> > + * NB: libnvdimm copies the data from ndr_desc into it's own
> > + * structures so passing a stack pointer is fine.
> > + */
> > + scm_data->nd_region = nvdimm_pmem_region_create(scm_data-
> > >nvdimm_bus,
> > + ®ion_desc);
> > + if (!scm_data->nd_region)
> > + return -EINVAL;
> > +
> > + dev_info(&scm_data->dev,
> > + "Onlining %lluMB of persistent memory\n",
> > + nd_mapping_desc.size / SZ_1M);
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * allocate_scm_minor() - Allocate a minor number to use for an
> > SCM device
> > + * @scm_data: The SCM device to associate the minor with
> > + * Return: the allocated minor number
> > + */
> > +static int allocate_scm_minor(struct scm_data *scm_data)
> > +{
> > + int minor;
> > +
> > + mutex_lock(&minors_idr_lock);
> > + minor = idr_alloc(&minors_idr, scm_data, 0, SCM_NUM_MINORS,
> > GFP_KERNEL);
> > + mutex_unlock(&minors_idr_lock);
> > + return minor;
> > +}
> > +
> > +static void free_scm_minor(struct scm_data *scm_data)
> > +{
> > + mutex_lock(&minors_idr_lock);
> > + idr_remove(&minors_idr, MINOR(scm_data->dev.devt));
> > + mutex_unlock(&minors_idr_lock);
> > +}
> > +
> > +/**
> > + * free_scm() - Free all members of an SCM struct
> > + * @scm_data: the SCM metadata to clear
> > + */
> > +static void free_scm(struct scm_data *scm_data)
> > +{
> > + int rc;
> > +
> > + if (scm_data->nvdimm_bus)
> > + nvdimm_bus_unregister(scm_data->nvdimm_bus);
> > +
> > + free_scm_minor(scm_data);
> > +
> > + if (scm_data->metadata_addr)
> > + devm_memunmap(&scm_data->dev, scm_data->metadata_addr);
> > +
> > + if (scm_data->ocxl_context) {
> > + rc = ocxl_context_detach(scm_data->ocxl_context);
> > + if (rc == -EBUSY)
> > + dev_warn(&scm_data->dev, "Timeout detaching
> > ocxl context\n");
> > + else
> > + ocxl_context_free(scm_data->ocxl_context);
> > +
> > + }
> > +
> > + if (scm_data->ocxl_afu)
> > + ocxl_afu_put(scm_data->ocxl_afu);
> > +
> > + if (scm_data->ocxl_fn)
> > + ocxl_function_close(scm_data->ocxl_fn);
> > +
> > + kfree(scm_data);
> > +}
> > +
> > +/**
> > + * free_scm_dev - Free an SCM device
> > + * @dev: The device struct
> > + */
> > +static void free_scm_dev(struct device *dev)
> > +{
> > + struct scm_data *scm_data = container_of(dev, struct scm_data,
> > dev);
> > +
> > + free_scm(scm_data);
> > +}
> > +
> > +/**
> > + * scm_register - Register an SCM device with the kernel
> > + * @scm_data: the SCM metadata
> > + * Return: 0 on success, negative on failure
> > + */
> > +static int scm_register(struct scm_data *scm_data)
> > +{
> > + int rc;
> > + int minor = allocate_scm_minor(scm_data);
> > +
> > + if (minor < 0)
> > + return minor;
> > +
> > + scm_data->dev.release = free_scm_dev;
> > + rc = dev_set_name(&scm_data->dev, "ocxl-scm%d", minor);
> > + if (rc < 0)
> > + return rc;
> > +
> > + scm_data->dev.devt = MKDEV(MAJOR(scm_dev), minor);
> > + scm_data->dev.class = scm_class;
> > + scm_data->dev.parent = &scm_data->pdev->dev;
> > +
> > + rc = device_register(&scm_data->dev);
> > + return rc;
> return device_register(&scm_data->dev);
>
> Assuming nothing else is added inbetween in later patches...
> If it is then ignore this one.
>
Ok
> > +}
> > +
> > +/**
> > + * scm_remove() - Free an OpenCAPI Storage Class Memory device
> > + * @pdev: the PCI device information struct
> > + */
> > +static void scm_remove(struct pci_dev *pdev)
> > +{
> > + if (PCI_FUNC(pdev->devfn) == 0) {
> > + struct scm_function_0 *scm_func_0 =
> > pci_get_drvdata(pdev);
> > +
> > + if (scm_func_0) {
> > + ocxl_function_close(scm_func_0->ocxl_fn);
> > + scm_func_0->ocxl_fn = NULL;
> > + }
> > + } else {
> > + struct scm_data *scm_data = pci_get_drvdata(pdev);
> > +
> > + if (scm_data)
> > + device_unregister(&scm_data->dev);
> > + }
> > +}
> > +
> > +/**
> > + * scm_probe_function_0 - Set up function 0 for an OpenCAPI
> > Storage Class Memory device
> Overly long line + not consistent on () after function name.
>
> IIRC either () or not is fine, but should be consistent in a gven
> file.
Ok
>
> > + * This is important as it enables templates higher than 0 across
> > all other functions,
> > + * which in turn enables higher bandwidth accesses
> > + * which in turn enables higher bandwidth accesses
>
> Repeated line.
Ok
>
> > + * @pdev: the PCI device information struct
> > + * Return: 0 on success, negative on failure
> > + */
> > +static int scm_probe_function_0(struct pci_dev *pdev)
> > +{
> > + struct scm_function_0 *scm_func_0 = NULL;
> > + struct ocxl_fn *fn;
> > +
> > + scm_func_0 = kzalloc(sizeof(*scm_func_0), GFP_KERNEL);
> > + if (!scm_func_0)
> > + return -ENOMEM;
> > +
> > + scm_func_0->pdev = pdev;
> > + fn = ocxl_function_open(pdev);
> > + if (IS_ERR(fn)) {
> > + kfree(scm_func_0);
> > + dev_err(&pdev->dev, "failed to open OCXL function\n");
> > + return PTR_ERR(fn);
> > + }
> > + scm_func_0->ocxl_fn = fn;
> > +
> > + pci_set_drvdata(pdev, scm_func_0);
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * scm_probe - Init an OpenCAPI Storage Class Memory device
> > + * @pdev: the PCI device information struct
> > + * @ent: The entry from scm_pci_tbl
> > + * Return: 0 on success, negative on failure
> > + */
> > +static int scm_probe(struct pci_dev *pdev, const struct
> > pci_device_id *ent)
> > +{
> > + struct scm_data *scm_data = NULL;
>
> Always set in paths that use it.
Ok
>
> > +
> > + if (PCI_FUNC(pdev->devfn) == 0)
> > + return scm_probe_function_0(pdev);
> > + else if (PCI_FUNC(pdev->devfn) != 1)
> > + return 0;
> > +
> > + scm_data = kzalloc(sizeof(*scm_data), GFP_KERNEL);
> > + if (!scm_data) {
> > + dev_err(&pdev->dev, "Could not allocate SCM
> > metadata\n");
> > + goto err;
> > + }
> > + scm_data->pdev = pdev;
> > +
> > + pci_set_drvdata(pdev, scm_data);
> > +
> > + scm_data->ocxl_fn = ocxl_function_open(pdev);
> > + if (IS_ERR(scm_data->ocxl_fn)) {
> > + kfree(scm_data);
> > + scm_data = NULL;
>
> Doesn't seem like scm_data is used anywhere in the rror path..
Ok
>
> > + pci_set_drvdata(pdev, NULL);
> > + dev_err(&pdev->dev, "failed to open OCXL function\n");
> > + goto err;
> > + }
> > +
> > + scm_data->ocxl_afu = ocxl_function_fetch_afu(scm_data->ocxl_fn,
> > 0);
> > + if (scm_data->ocxl_afu == NULL) {
> > + dev_err(&pdev->dev, "Could not get OCXL AFU from
> > function\n");
> > + goto err;
>
> The comment below suggests to me that free_scm will only be called if
> we succeed
> in scm_register? If so isn't there more error handling to be done
> until that
> happens?
>
I've moved that comment up - once device_register has been called (even
if the call fails), the free handler should always be called via
device_put.
> > + }
> > +
> > + ocxl_afu_get(scm_data->ocxl_afu);
> > +
> > + if (scm_register(scm_data) < 0) {
> > + dev_err(&pdev->dev, "Could not register SCM device with
> > the kernel\n");
> > + goto err;
> > + }
> > +
> > + // Resources allocated below here are cleaned up in the release
> > handler
> > +
> > + if (ocxl_context_alloc(&scm_data->ocxl_context, scm_data-
> > >ocxl_afu, NULL)) {
> > + dev_err(&pdev->dev, "Could not allocate OCXL
> > context\n");
> > + goto err;
> > + }
> > +
> > + if (ocxl_context_attach(scm_data->ocxl_context, 0, NULL)) {
> > + dev_err(&pdev->dev, "Could not attach ocxl context\n");
> > + goto err;
> > + }
> > +
> > + if (scm_register_lpc_mem(scm_data)) {
> > + dev_err(&pdev->dev, "Could not register OCXL SCM memory
> > with libnvdimm\n");
> > + goto err;
> > + }
> > +
> > + return 0;
> > +
> > +err:
> > + /*
> > + * Further cleanup is done in the release handler via
> > free_scm()
> > + * This allows us to keep the character device live to handle
> > IOCTLs to
> > + * investigate issues if the card has an error
> > + */
> > +
> > + dev_err(&pdev->dev,
> > + "Error detected, will not register storage class
> > memory\n");
> > + return -ENXIO;
>
> Probably better to return more specific errors from the various error
> paths.
> -ENOMEM etc.
>
I've updated the code to propogate errors up from the calls.
>
> > +}
> > +
> > +static struct pci_driver scm_pci_driver = {
> > + .name = "ocxl-scm",
> > + .id_table = scm_pci_tbl,
> > + .probe = scm_probe,
> > + .remove = scm_remove,
> > + .shutdown = scm_remove,
> > +};
> > +
> > +static int __init scm_init(void)
> > +{
> > + int rc = 0;
> > +
> > + rc = pci_register_driver(&scm_pci_driver);
> > + if (rc)
> > + return rc;
> > +
> > + return 0;
> > +}
> > +
> > +static void scm_exit(void)
> > +{
> > + pci_unregister_driver(&scm_pci_driver);
> > +}
> > +
> > +module_init(scm_init);
> > +module_exit(scm_exit);
> > +
> > +MODULE_DESCRIPTION("Storage Class Memory");
> > +MODULE_LICENSE("GPL");
> > diff --git a/drivers/nvdimm/ocxl/scm_internal.h
> > b/drivers/nvdimm/ocxl/scm_internal.h
> > new file mode 100644
> > index 000000000000..6340012e0f8a
> > --- /dev/null
> > +++ b/drivers/nvdimm/ocxl/scm_internal.h
> > @@ -0,0 +1,28 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +// Copyright 2019 IBM Corp.
> > +
> > +#include <linux/pci.h>
> > +#include <misc/ocxl.h>
> > +#include <linux/libnvdimm.h>
> > +#include <linux/mm.h>
> > +
> > +#define SCM_LABEL_AREA_SIZE (1UL << PA_SECTION_SHIFT)
> > +
> > +struct scm_function_0 {
> > + struct pci_dev *pdev;
> > + struct ocxl_fn *ocxl_fn;
> > +};
> > +
> > +struct scm_data {
> > + struct device dev;
> > + struct pci_dev *pdev;
> > + struct ocxl_fn *ocxl_fn;
> > + struct nd_interleave_set nd_set;
> > + struct nvdimm_bus_descriptor bus_desc;
> > + struct nvdimm_bus *nvdimm_bus;
> > + struct ocxl_afu *ocxl_afu;
> > + struct ocxl_context *ocxl_context;
> > + void *metadata_addr;
> > + struct resource scm_res;
> > + struct nd_region *nd_region;
> > +};
--
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819
^ permalink raw reply
* RE: [PATCH v2 08/27] ocxl: Save the device serial number in ocxl_fn
From: Alastair D'Silva @ 2020-02-19 4:03 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Keith Busch,
Masahiro Yamada, Paul Mackerras, Mauro Carvalho Chehab, Ira Weiny,
Thomas Gleixner, Rob Herring, Dave Jiang, linux-nvdimm,
Vishal Verma, Krzysztof Kozlowski, Anju T Sudhakar,
Mahesh Salgaonkar, Andrew Donnellan, Arnd Bergmann, Greg Kurz,
Nicholas Piggin, Cédric Le Goater, Dan Williams,
Hari Bathini, linux-mm, Greg Kroah-Hartman, linux-kernel,
Frederic Barrat, Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <20200203125346.0000503f@Huawei.com>
On Mon, 2020-02-03 at 12:53 +0000, Jonathan Cameron wrote:
> On Tue, 3 Dec 2019 14:46:36 +1100
> Alastair D'Silva <alastair@au1.ibm.com> wrote:
>
> > From: Alastair D'Silva <alastair@d-silva.org>
> >
> > This patch retrieves the serial number of the card and makes it
> > available
> > to consumers of the ocxl driver via the ocxl_fn struct.
> >
> > Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> > Acked-by: Frederic Barrat <fbarrat@linux.ibm.com>
> > Acked-by: Andrew Donnellan <ajd@linux.ibm.com>
> > ---
> > drivers/misc/ocxl/config.c | 46
> > ++++++++++++++++++++++++++++++++++++++
> > include/misc/ocxl.h | 1 +
> > 2 files changed, 47 insertions(+)
> >
> > diff --git a/drivers/misc/ocxl/config.c
> > b/drivers/misc/ocxl/config.c
> > index fb0c3b6f8312..a9203c309365 100644
> > --- a/drivers/misc/ocxl/config.c
> > +++ b/drivers/misc/ocxl/config.c
> > @@ -71,6 +71,51 @@ static int find_dvsec_afu_ctrl(struct pci_dev
> > *dev, u8 afu_idx)
> > return 0;
> > }
> >
> > +/**
>
> Make sure anything you mark as kernel doc with /** is valid
> kernel-doc.
>
Ok
> > + * Find a related PCI device (function 0)
> > + * @device: PCI device to match
> > + *
> > + * Returns a pointer to the related device, or null if not found
> > + */
> > +static struct pci_dev *get_function_0(struct pci_dev *dev)
> > +{
> > + unsigned int devfn = PCI_DEVFN(PCI_SLOT(dev->devfn), 0); //
> > Look for function 0
>
> Not sure the trailing comment adds much.
>
> I'd personally not bother with this wrapper at all and just call
> the pci functions directly where needed.
>
I'm not that familiar with the macros, so its not immediately obvious
to me what it's doing, so my preference is to leave it.
> > +
> > + return pci_get_domain_bus_and_slot(pci_domain_nr(dev->bus),
> > + dev->bus->number, devfn);
> > +}
> > +
> > +static void read_serial(struct pci_dev *dev, struct ocxl_fn_config
> > *fn)
> > +{
> > + u32 low, high;
> > + int pos;
> > +
> > + pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DSN);
> > + if (pos) {
> > + pci_read_config_dword(dev, pos + 0x04, &low);
> > + pci_read_config_dword(dev, pos + 0x08, &high);
> > +
> > + fn->serial = low | ((u64)high) << 32;
> > +
> > + return;
> > + }
> > +
> > + if (PCI_FUNC(dev->devfn) != 0) {
> > + struct pci_dev *related = get_function_0(dev);
> > +
> > + if (!related) {
> > + fn->serial = 0;
> > + return;
> > + }
> > +
> > + read_serial(related, fn);
> > + pci_dev_put(related);
> > + return;
> > + }
> > +
> > + fn->serial = 0;
> > +}
> > +
> > static void read_pasid(struct pci_dev *dev, struct ocxl_fn_config
> > *fn)
> > {
> > u16 val;
> > @@ -208,6 +253,7 @@ int ocxl_config_read_function(struct pci_dev
> > *dev, struct ocxl_fn_config *fn)
> > int rc;
> >
> > read_pasid(dev, fn);
> > + read_serial(dev, fn);
> >
> > rc = read_dvsec_tl(dev, fn);
> > if (rc) {
> > diff --git a/include/misc/ocxl.h b/include/misc/ocxl.h
> > index 6f7c02f0d5e3..9843051c3c5b 100644
> > --- a/include/misc/ocxl.h
> > +++ b/include/misc/ocxl.h
> > @@ -46,6 +46,7 @@ struct ocxl_fn_config {
> > int dvsec_afu_info_pos; /* offset of the AFU information DVSEC
> > */
> > s8 max_pasid_log;
> > s8 max_afu_index;
> > + u64 serial;
> > };
> >
> > enum ocxl_endian {
--
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819
^ permalink raw reply
* RE: [PATCH v2 07/27] ocxl: Add functions to map/unmap LPC memory
From: Alastair D'Silva @ 2020-02-19 2:39 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Keith Busch,
Masahiro Yamada, Paul Mackerras, Mauro Carvalho Chehab, Ira Weiny,
Thomas Gleixner, Rob Herring, Dave Jiang, linux-nvdimm,
Vishal Verma, Krzysztof Kozlowski, Anju T Sudhakar,
Mahesh Salgaonkar, Andrew Donnellan, Arnd Bergmann, Greg Kurz,
Nicholas Piggin, Cédric Le Goater, Dan Williams,
Hari Bathini, linux-mm, Greg Kroah-Hartman, linux-kernel,
Frederic Barrat, Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <20200203124942.00003b68@Huawei.com>
On Mon, 2020-02-03 at 12:49 +0000, Jonathan Cameron wrote:
> On Tue, 3 Dec 2019 14:46:35 +1100
> Alastair D'Silva <alastair@au1.ibm.com> wrote:
>
> > From: Alastair D'Silva <alastair@d-silva.org>
> >
> > Add functions to map/unmap LPC memory
> >
> > Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> > ---
> > drivers/misc/ocxl/config.c | 4 +++
> > drivers/misc/ocxl/core.c | 50
> > +++++++++++++++++++++++++++++++
> > drivers/misc/ocxl/ocxl_internal.h | 3 ++
> > include/misc/ocxl.h | 18 +++++++++++
> > 4 files changed, 75 insertions(+)
> >
> > diff --git a/drivers/misc/ocxl/config.c
> > b/drivers/misc/ocxl/config.c
> > index c8e19bfb5ef9..fb0c3b6f8312 100644
> > --- a/drivers/misc/ocxl/config.c
> > +++ b/drivers/misc/ocxl/config.c
> > @@ -568,6 +568,10 @@ static int read_afu_lpc_memory_info(struct
> > pci_dev *dev,
> > afu->special_purpose_mem_size =
> > total_mem_size - lpc_mem_size;
> > }
> > +
> > + dev_info(&dev->dev, "Probed LPC memory of %#llx bytes and
> > special purpose memory of %#llx bytes\n",
> > + afu->lpc_mem_size, afu->special_purpose_mem_size);
> > +
>
> If we are being fussy, this block has nothing todo with the rest of
> the patch
> so we should be seeing it here.
Agreed
>
> > return 0;
> > }
> >
> > diff --git a/drivers/misc/ocxl/core.c b/drivers/misc/ocxl/core.c
> > index 2531c6cf19a0..98611faea219 100644
> > --- a/drivers/misc/ocxl/core.c
> > +++ b/drivers/misc/ocxl/core.c
> > @@ -210,6 +210,55 @@ static void unmap_mmio_areas(struct ocxl_afu
> > *afu)
> > release_fn_bar(afu->fn, afu->config.global_mmio_bar);
> > }
> >
> > +int ocxl_afu_map_lpc_mem(struct ocxl_afu *afu)
> > +{
> > + struct pci_dev *dev = to_pci_dev(afu->fn->dev.parent);
> > +
> > + if ((afu->config.lpc_mem_size + afu-
> > >config.special_purpose_mem_size) == 0)
> > + return 0;
> > +
> > + afu->lpc_base_addr = ocxl_link_lpc_map(afu->fn->link, dev);
> > + if (afu->lpc_base_addr == 0)
> > + return -EINVAL;
> > +
> > + if (afu->config.lpc_mem_size) {
>
> I was happy with the explicit check on 0 above, but we should be
> consistent. Either
> we make use of 0 == false, or we don't and explicitly check vs 0.
>
> Hence
>
> if (afu->config.pc_mem_size != 0) {
>
> here or
>
> if (!(afu->config.pc_mem_size + afu-
> >config.special_purpose_mem_size))
> return 0;
>
> above.
This feels a bit niggly, but sure, changed to a '> 0' check.
>
> > + afu->lpc_res.start = afu->lpc_base_addr + afu-
> > >config.lpc_mem_offset;
> > + afu->lpc_res.end = afu->lpc_res.start + afu-
> > >config.lpc_mem_size - 1;
> > + }
> > +
> > + if (afu->config.special_purpose_mem_size) {
> > + afu->special_purpose_res.start = afu->lpc_base_addr +
> > + afu-
> > >config.special_purpose_mem_offset;
> > + afu->special_purpose_res.end = afu-
> > >special_purpose_res.start +
> > + afu-
> > >config.special_purpose_mem_size - 1;
> > + }
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(ocxl_afu_map_lpc_mem);
> > +
> > +struct resource *ocxl_afu_lpc_mem(struct ocxl_afu *afu)
> > +{
> > + return &afu->lpc_res;
> > +}
> > +EXPORT_SYMBOL_GPL(ocxl_afu_lpc_mem);
> > +
> > +static void unmap_lpc_mem(struct ocxl_afu *afu)
> > +{
> > + struct pci_dev *dev = to_pci_dev(afu->fn->dev.parent);
> > +
> > + if (afu->lpc_res.start || afu->special_purpose_res.start) {
> > + void *link = afu->fn->link;
> > +
> > + ocxl_link_lpc_release(link, dev);
> > +
> > + afu->lpc_res.start = 0;
> > + afu->lpc_res.end = 0;
> > + afu->special_purpose_res.start = 0;
> > + afu->special_purpose_res.end = 0;
> > + }
> > +}
> > +
> > static int configure_afu(struct ocxl_afu *afu, u8 afu_idx, struct
> > pci_dev *dev)
> > {
> > int rc;
> > @@ -251,6 +300,7 @@ static int configure_afu(struct ocxl_afu *afu,
> > u8 afu_idx, struct pci_dev *dev)
> >
> > static void deconfigure_afu(struct ocxl_afu *afu)
> > {
> > + unmap_lpc_mem(afu);
>
> Hmm. This breaks the existing balance between configure_afu and
> deconfigure_afu.
>
> Given comments below on why we don't do map_lpc_mem in the afu bring
> up
> (as it's a shared operation) it seems to me that we should be doing
> this
> outside of the afu deconfigure. Perhaps ocxl_function_close is
> appropriate?
> I don't know this infrastructure well enough to be sure.
>
> If it does need to be here, then a comment to give more info on
> why would be great!
>
Sure, I've added a comment in unmap_lpc_mem explaining that lpc_release
only releases the memory on the link when the last consumer calls
release.
It's in deconfigure_afu as the LPC memory is registered and reported
per-AFU (even though it has to be allocated all at once across the
link).
> > unmap_mmio_areas(afu);
> > reclaim_afu_pasid(afu);
> > reclaim_afu_actag(afu);
> > diff --git a/drivers/misc/ocxl/ocxl_internal.h
> > b/drivers/misc/ocxl/ocxl_internal.h
> > index 20b417e00949..9f4b47900e62 100644
> > --- a/drivers/misc/ocxl/ocxl_internal.h
> > +++ b/drivers/misc/ocxl/ocxl_internal.h
> > @@ -52,6 +52,9 @@ struct ocxl_afu {
> > void __iomem *global_mmio_ptr;
> > u64 pp_mmio_start;
> > void *private;
> > + u64 lpc_base_addr; /* Covers both LPC & special purpose memory
> > */
> > + struct resource lpc_res;
> > + struct resource special_purpose_res;
> > };
> >
> > enum ocxl_context_status {
> > diff --git a/include/misc/ocxl.h b/include/misc/ocxl.h
> > index 06dd5839e438..6f7c02f0d5e3 100644
> > --- a/include/misc/ocxl.h
> > +++ b/include/misc/ocxl.h
> > @@ -212,6 +212,24 @@ int ocxl_irq_set_handler(struct ocxl_context
> > *ctx, int irq_id,
> >
> > // AFU Metadata
> >
> > +/**
> > + * Map the LPC system & special purpose memory for an AFU
> > + *
> > + * Do not call this during device discovery, as there may me
> > multiple
> > + * devices on a link, and the memory is mapped for the whole link,
> > not
> > + * just one device. It should only be called after all devices
> > have
> > + * registered their memory on the link.
> > + *
> > + * afu: The AFU that has the LPC memory to map
> Run kernel-doc over these files and fix all the errors + warnings.
>
Ok.
> @afu: ..
>
> and missing function name etc.
>
>
> > + */
> > +extern int ocxl_afu_map_lpc_mem(struct ocxl_afu *afu);
> > +
> > +/**
> > + * Get the physical address range of LPC memory for an AFU
> > + * afu: The AFU associated with the LPC memory
> > + */
> > +extern struct resource *ocxl_afu_lpc_mem(struct ocxl_afu *afu);
> > +
> > /**
> > * Get a pointer to the config for an AFU
> > *
--
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819
^ permalink raw reply
* Re: [PATCH v2] powerpc/Makefile: Mark phony targets as PHONY
From: Masahiro Yamada @ 2020-02-19 2:18 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev, Linux Kbuild mailing list
In-Reply-To: <20200219000434.15872-1-mpe@ellerman.id.au>
On Wed, Feb 19, 2020 at 9:04 AM Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> Some of our phony targets are not marked as such. This can lead to
> confusing errors, eg:
>
> $ make clean
> $ touch install
> $ make install
> make: 'install' is up to date.
> $
>
> Fix it by adding them to the PHONY variable which is marked phony in
> the top-level Makefile, or in scripts/Makefile.build for the boot
> Makefile.
>
> Suggested-by: Masahiro Yamada <masahiroy@kernel.org>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
Reviewed-by: Masahiro Yamada <masahiroy@kernel.org>
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* Re: [PATCH v2 3/3] ASoC: fsl_easrc: Add EASRC ASoC CPU DAI and platform drivers
From: Nicolin Chen @ 2020-02-19 1:09 UTC (permalink / raw)
To: Shengjiu Wang
Cc: mark.rutland, devicetree, alsa-devel, timur, Xiubo.Lee,
linuxppc-dev, tiwai, lgirdwood, robh+dt, perex, broonie, festevam,
linux-kernel
In-Reply-To: <cea88cec3e32759a49d44c41cb615a839f689937.1582007379.git.shengjiu.wang@nxp.com>
On Tue, Feb 18, 2020 at 02:39:37PM +0800, Shengjiu Wang wrote:
> EASRC (Enhanced Asynchronous Sample Rate Converter) is a new IP module
> found on i.MX8MN. It is different with old ASRC module.
>
> The primary features for the EASRC are as follows:
> - 4 Contexts - groups of channels with an independent time base
> - Fully independent and concurrent context control
> - Simultaneous processing of up to 32 audio channels
> - Programmable filter charachteristics for each context
> - 32, 24, 20, and 16-bit fixed point audio sample support
> - 32-bit floating point audio sample support
> - 8kHz to 384kHz sample rate
> - 1/16 to 8x sample rate conversion ratio
>
> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> ---
> sound/soc/fsl/Kconfig | 10 +
> sound/soc/fsl/Makefile | 2 +
> sound/soc/fsl/fsl_asrc_common.h | 1 +
> sound/soc/fsl/fsl_easrc.c | 2265 +++++++++++++++++++++++++++++++
> sound/soc/fsl/fsl_easrc.h | 668 +++++++++
> sound/soc/fsl/fsl_easrc_dma.c | 440 ++++++
I see a 90% similarity between fsl_asrc_dma and fsl_easrc_dma files.
Would it be possible reuse the existing code? Could share structures
from my point of view, just like it reuses "enum asrc_pair_index", I
know differentiating "pair" and "context" is a big point here though.
A possible quick solution for that, off the top of my head, could be:
1) in fsl_asrc_common.h
struct fsl_asrc {
....
};
struct fsl_asrc_pair {
....
};
2) in fsl_easrc.h
/* Renaming shared structures */
#define fsl_easrc fsl_asrc
#define fsl_easrc_context fsl_asrc_pair
May be a good idea to see if others have some opinion too.
> diff --git a/sound/soc/fsl/fsl_easrc.c b/sound/soc/fsl/fsl_easrc.c
> new file mode 100644
> index 000000000000..6fe2953317f2
> --- /dev/null
> +++ b/sound/soc/fsl/fsl_easrc.c
> +
> +/* set_rs_ratio
> + *
> + * According to the resample taps, calculate the resample ratio
> + */
> +static int set_rs_ratio(struct fsl_easrc_context *ctx)
"fsl_easrc_" prefix? Would be nice to have a formula in the comments.
> +/* resets the pointer of the coeff memory pointers */
> +static int fsl_coeff_mem_ptr_reset(struct fsl_easrc *easrc,
> + unsigned int ctx_id, int mem_type)
> +{
> + /* To reset the write pointer back to zero, the register field
> + * ASRC_CTX_CTRL_EXT1x[PF_COEFF_MEM_RST] can be toggled from
> + * 0x0 to 0x1 to 0x0.
> + */
Please use the style:
/*
* xxx
*/
> +static int fsl_easrc_resampler_config(struct fsl_easrc *easrc)
> +{
> + for (i = 0; i < hdr->interp_scen; i++) {
> + if ((interp[i].num_taps - 1) ==
> + bits_taps_to_val(easrc->rs_num_taps)) {
Could do below to save some indentations from the rest of the routine:
+ if ((interp[i].num_taps - 1) !=
+ bits_taps_to_val(easrc->rs_num_taps))
+ continue;
> + arr = interp[i].coeff;
> + selected_interp = &interp[i];
> + dev_dbg(dev, "Selected interp_filter: %u taps - %u phases\n",
> + selected_interp->num_taps,
> + selected_interp->num_phases);
> + break;
> +/*****************************************************************************
> + * Scale filter coefficients (64 bits float)
> + * For input float32 normalized range (1.0,-1.0) -> output int[16,24,32]:
> + * scale it by multiplying filter coefficients by 2^31
> + * For input int[16, 24, 32] -> output float32
> + * scale it by multiplying filter coefficients by 2^-15, 2^-23, 2^-31
> + * input:
> + * easrc: Structure pointer of fsl_easrc
> + * infilter : Pointer to non-scaled input filter
> + * shift: The multiply factor
> + * output:
> + * outfilter: scaled filter
> + *****************************************************************************/
> +static int NormalizedFilterForFloat32InIntOut(struct fsl_easrc *easrc,
> + u64 *infilter,
> + u64 *outfilter,
> + int shift)
Coding style looks very different, at comments and function naming.
> +{
> + struct device *dev = &easrc->pdev->dev;
> + u64 coef = *infilter;
> + s64 exp = (coef & 0x7ff0000000000000ll) >> 52;
> + u64 outcoef;
> +
> + /*
> + * If exponent is zero (value == 0), or 7ff (value == NaNs)
> + * dont touch the content
> + */
> + if (((coef & 0x7ff0000000000000ll) == 0) ||
> + ((coef & 0x7ff0000000000000ll) == ((u64)0x7ff << 52))) {
> + *outfilter = coef;
> + } else {
> + if ((shift > 0 && (shift + exp) >= 2047) ||
> + (shift < 0 && (exp + shift) <= 0)) {
> + dev_err(dev, "coef error\n");
> + return -EINVAL;
> + }
> +
> + /* coefficient * 2^shift ==> coefficient_exp + shift */
> + exp += shift;
> + outcoef = (u64)(coef & 0x800FFFFFFFFFFFFFll) +
> + ((u64)exp << 52);
> + *outfilter = outcoef;
> + }
> +
> + return 0;
> +}
> +
> +static int write_pf_coeff_mem(struct fsl_easrc *easrc, int ctx_id,
> + u64 *arr, int n_taps, int shift)
Function naming.
> +static int fsl_easrc_prefilter_config(struct fsl_easrc *easrc,
> + unsigned int ctx_id)
> +{
> + struct fsl_easrc_context *ctx;
> + struct asrc_firmware_hdr *hdr;
> + struct prefil_params *prefil, *selected_prefil = NULL;
> + struct device *dev;
> + u32 inrate, outrate, offset = 0;
> + int ret, i;
> +
> + /* to modify prefilter coeficients, the user must perform
> + * a write in ASRC_PRE_COEFF_FIFO[COEFF_DATA] while the
> + * RUN_EN for that context is set to 0
> + */
> + if (!easrc)
> + return -ENODEV;
Hmm..I don't see the relationship between the comments and the code.
> + if (ctx->out_params.sample_rate >= ctx->in_params.sample_rate) {
> + if (ctx->out_params.sample_rate == ctx->in_params.sample_rate)
> + regmap_update_bits(easrc->regmap,
> + REG_EASRC_CCE1(ctx_id),
> + EASRC_CCE1_RS_BYPASS_MASK,
> + EASRC_CCE1_RS_BYPASS);
> +
> + if (ctx->in_params.sample_format == SNDRV_PCM_FORMAT_FLOAT_LE &&
> + ctx->out_params.sample_format != SNDRV_PCM_FORMAT_FLOAT_LE) {
> + ctx->st1_num_taps = 1;
> + ctx->st1_coeff = &easrc->const_coeff;
> + ctx->st1_num_exp = 1;
> + ctx->st2_num_taps = 0;
> + ctx->st1_addexp = 31;
> + } else if (ctx->in_params.sample_format != SNDRV_PCM_FORMAT_FLOAT_LE &&
> + ctx->out_params.sample_format == SNDRV_PCM_FORMAT_FLOAT_LE) {
> + ctx->st1_num_taps = 1;
> + ctx->st1_coeff = &easrc->const_coeff;
> + ctx->st1_num_exp = 1;
> + ctx->st2_num_taps = 0;
> + ctx->st1_addexp -= ctx->in_params.fmt.addexp;
> + } else {
> + ctx->st1_num_taps = 1;
> + ctx->st1_coeff = &easrc->const_coeff;
> + ctx->st1_num_exp = 1;
> + ctx->st2_num_taps = 0;
The first four lines of each path are completely duplicated. Probably
only needs to diff st1_addexp?
> + } else {
> + inrate = ctx->in_params.norm_rate;
> + outrate = ctx->out_params.norm_rate;
> +
> + hdr = easrc->firmware_hdr;
> + prefil = easrc->prefil;
> +
> + for (i = 0; i < hdr->prefil_scen; i++) {
> + if (inrate == prefil[i].insr && outrate == prefil[i].outsr) {
Could do below to save indentations:
+ if (inrate != prefil[i].insr ||
+ outrate != prefil[i].outsr)
+ continue;
> + if (!selected_prefil) {
> + dev_err(dev, "Conversion from in ratio %u(%u) to out ratio %u(%u) is not supported\n",
> + ctx->in_params.sample_rate,
> + inrate,
> + ctx->out_params.sample_rate, outrate);
Could fit into single lines:
+ ctx->in_params.sample_rate, inrate,
+ ctx->out_params.sample_rate, outrate);
> +static int fsl_easrc_config_one_slot(struct fsl_easrc_context *ctx,
> + struct fsl_easrc_slot *slot,
> + unsigned int slot_idx,
> + unsigned int reg0,
> + unsigned int reg1,
> + unsigned int reg2,
> + unsigned int reg3,
> + unsigned int *req_channels,
> + unsigned int *start_channel,
> + unsigned int *avail_channel)
There could be some simplification for the parameters here:
1) slot_idx could be a part of struct fsl_easrc_slot?
2) reg0->reg3 could be a part of struct too, or use some macro
calculating from slot_idx?
> +{
> + struct fsl_easrc *easrc = ctx->easrc;
> + int st1_chanxexp, st1_mem_alloc = 0, st2_mem_alloc = 0;
> + unsigned int addr;
> +
> + if (*req_channels <= *avail_channel) {
> + slot->num_channel = *req_channels;
> + slot->min_channel = *start_channel;
> + slot->max_channel = *start_channel + *req_channels - 1;
> + slot->ctx_index = ctx->index;
> + slot->busy = true;
> + *start_channel += *req_channels;
> + *req_channels = 0;
> + } else {
> + slot->num_channel = *avail_channel;
> + slot->min_channel = *start_channel;
> + slot->max_channel = *start_channel + *avail_channel - 1;
> + slot->ctx_index = ctx->index;
> + slot->busy = true;
> + *start_channel += *avail_channel;
> + *req_channels -= *avail_channel;
> + }
Could merge duplicated parts:
+ if (*req_channels <= *avail_channel) {
+ slot->num_channel = *req_channels;
+ *req_channels = 0;
+ } else {
+ slot->num_channel = *req_channels;
+ *req_channels -= *avail_channel;
+ };
+
+ slot->min_channel = *start_channel;
+ slot->max_channel = *start_channel + slot->num_channel - 1;
+ slot->ctx_index = ctx->index;
+ slot->busy = true;
+ *start_channel += slot->num_channel;
> + if (ctx->st1_num_taps > 0) {
> + if (ctx->st2_num_taps > 0)
> + st1_mem_alloc =
> + (ctx->st1_num_taps - 1) * slot->num_channel *
> + ctx->st1_num_exp + slot->num_channel;
> + else
> + st1_mem_alloc = ctx->st1_num_taps * slot->num_channel;
> +
> + slot->pf_mem_used = st1_mem_alloc;
> + regmap_update_bits(easrc->regmap, reg2,
> + EASRC_DPCS0R2_ST1_MA_MASK,
> + EASRC_DPCS0R2_ST1_MA(st1_mem_alloc));
> +
> + if (slot_idx == 1)
> + addr = 0x1800 - st1_mem_alloc;
Where is this 0x1800 coming from?
> +int fsl_easrc_start_context(struct fsl_easrc_context *ctx)
> +{
> + EASRC_CC_FWMDE_MASK,
> + EASRC_CC_FWMDE);
> + EASRC_COC_FWMDE_MASK,
> + EASRC_COC_FWMDE);
> + EASRC_CC_EN_MASK,
> + EASRC_CC_EN);
They could fit into single lines.
> +static const struct regmap_config fsl_easrc_regmap_config = {
> + .readable_reg = fsl_easrc_readable_reg,
> + .volatile_reg = fsl_easrc_volatile_reg,
> + .writeable_reg = fsl_easrc_writeable_reg,
Can we use regmap_range and regmap_access_table?
> +void easrc_dump_firmware(struct fsl_easrc *easrc)
fsl_easrc_dump_firmware?
> +{
> + dev_dbg(dev, "Firmware v%u dump:\n", firm->firmware_version);
> + pr_debug("Num prefitler scenarios: %u\n", firm->prefil_scen);
> + pr_debug("Num interpolation scenarios: %u\n", firm->interp_scen);
> + pr_debug("\nInterpolation scenarios:\n");
dev_dbg vs. pr_debug?
> +int easrc_get_firmware(struct fsl_easrc *easrc)
fsl_easrc_get_firmware?
> +static int fsl_easrc_probe(struct platform_device *pdev)
> +{
> + /*Set default value*/
White space in the comments
> + ret = of_property_read_u32(np, "fsl,asrc-rate",
> + &easrc->easrc_rate);
Could fit into one line.
> + ret = of_property_read_u32(np, "fsl,asrc-width",
> + &width);
Ditto
> +static int fsl_easrc_runtime_resume(struct device *dev)
> +{
> +}
> +#endif /*CONFIG_PM*/
White space in the comments
> diff --git a/sound/soc/fsl/fsl_easrc.h b/sound/soc/fsl/fsl_easrc.h
> new file mode 100644
> index 000000000000..205f6ef3e1e3
> --- /dev/null
> +++ b/sound/soc/fsl/fsl_easrc.h
> @@ -0,0 +1,668 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2019 NXP
> + */
> +
> +#ifndef _FSL_EASRC_H
> +#define _FSL_EASRC_H
> +
> +#include <sound/asound.h>
> +#include <linux/miscdevice.h>
For miscdevice...
> +/**
> + * fsl_easrc: EASRC private data
> + *
> + * @pdev: platform device pointer
> + * @regmap: regmap handler
> + * @dma_params_rx: DMA parameters for receive channel
> + * @dma_params_tx: DMA parameters for transmit channel
> + * @ctx: context pointer
> + * @slot: slot setting
> + * @mem_clk: clock source to access register
> + * @firmware_hdr: the header of firmware
> + * @interp: pointer to interpolation filter coeff
> + * @prefil: pointer to prefilter coeff
> + * @fw: firmware of coeff table
> + * @fw_name: firmware name
> + * @paddr: physical address to the base address of registers
> + * @rs_num_taps: resample filter taps, 32, 64, or 128
> + * @bps_i2c958: bits per sample of iec958
> + * @chn_avail: available channels, maximum 32
> + * @lock: spin lock for resource protection
> + * @easrc_rate: default sample rate for ASoC Back-Ends
> + * @easrc_format: default sample format for ASoC Back-Ends
> + */
> +
> +struct fsl_easrc {
> + struct platform_device *pdev;
> + struct regmap *regmap;
> + struct miscdevice easrc_miscdev;
This is not being described in the comments area nor used in the
driver. Should probably stay downstream, or be added later when
the downstream feature gets upstream.
^ permalink raw reply
* Re: [PATCH v3] powerpc/kprobes: Ignore traps that happened in real mode
From: Masami Hiramatsu @ 2020-02-19 1:01 UTC (permalink / raw)
To: Christophe Leroy
Cc: linux-kernel, Anil S Keshavamurthy, Paul Mackerras, stable,
Naveen N. Rao, linuxppc-dev, David S. Miller, Larry Finger
In-Reply-To: <424331e2006e7291a1bfe40e7f3fa58825f565e1.1582054578.git.christophe.leroy@c-s.fr>
On Tue, 18 Feb 2020 19:38:27 +0000 (UTC)
Christophe Leroy <christophe.leroy@c-s.fr> wrote:
> When a program check exception happens while MMU translation is
> disabled, following Oops happens in kprobe_handler() in the following
> code:
>
> } else if (*addr != BREAKPOINT_INSTRUCTION) {
>
> [ 33.098554] BUG: Unable to handle kernel data access on read at 0x0000e268
> [ 33.105091] Faulting instruction address: 0xc000ec34
> [ 33.110010] Oops: Kernel access of bad area, sig: 11 [#1]
> [ 33.115348] BE PAGE_SIZE=16K PREEMPT CMPC885
> [ 33.119540] Modules linked in:
> [ 33.122591] CPU: 0 PID: 429 Comm: cat Not tainted 5.6.0-rc1-s3k-dev-00824-g84195dc6c58a #3267
> [ 33.131005] NIP: c000ec34 LR: c000ecd8 CTR: c019cab8
> [ 33.136002] REGS: ca4d3b58 TRAP: 0300 Not tainted (5.6.0-rc1-s3k-dev-00824-g84195dc6c58a)
> [ 33.144324] MSR: 00001032 <ME,IR,DR,RI> CR: 2a4d3c52 XER: 00000000
> [ 33.150699] DAR: 0000e268 DSISR: c0000000
> [ 33.150699] GPR00: c000b09c ca4d3c10 c66d0620 00000000 ca4d3c60 00000000 00009032 00000000
> [ 33.150699] GPR08: 00020000 00000000 c087de44 c000afe0 c66d0ad0 100d3dd6 fffffff3 00000000
> [ 33.150699] GPR16: 00000000 00000041 00000000 ca4d3d70 00000000 00000000 0000416d 00000000
> [ 33.150699] GPR24: 00000004 c53b6128 00000000 0000e268 00000000 c07c0000 c07bb6fc ca4d3c60
> [ 33.188015] NIP [c000ec34] kprobe_handler+0x128/0x290
> [ 33.192989] LR [c000ecd8] kprobe_handler+0x1cc/0x290
> [ 33.197854] Call Trace:
> [ 33.200340] [ca4d3c30] [c000b09c] program_check_exception+0xbc/0x6fc
> [ 33.206590] [ca4d3c50] [c000e43c] ret_from_except_full+0x0/0x4
> [ 33.212392] --- interrupt: 700 at 0xe268
> [ 33.270401] Instruction dump:
> [ 33.273335] 913e0008 81220000 38600001 3929ffff 91220000 80010024 bb410008 7c0803a6
> [ 33.280992] 38210020 4e800020 38600000 4e800020 <813b0000> 6d2a7fe0 2f8a0008 419e0154
> [ 33.288841] ---[ end trace 5b9152d4cdadd06d ]---
>
> kprobe is not prepared to handle events in real mode and functions
> running in real mode should have been blacklisted, so kprobe_handler()
> can safely bail out telling 'this trap is not mine' for any trap that
> happened while in real-mode.
>
> If the trap happened with MSR_IR or MSR_DR cleared, return 0 immediately.
>
Looks good to me.
Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>
Thank you!
> Reported-by: Larry Finger <Larry.Finger@lwfinger.net>
> Fixes: 6cc89bad60a6 ("powerpc/kprobes: Invoke handlers directly")
> Cc: stable@vger.kernel.org
> Cc: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>
> ---
> v3: Also bail out if MSR_DR is cleared.
>
> Resending v2 with a more appropriate name
>
> v2: bailing out instead of converting real-time address to virtual and continuing.
>
> The bug might have existed even before that commit from Naveen.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
> arch/powerpc/kernel/kprobes.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
> index 2d27ec4feee4..9b340af02c38 100644
> --- a/arch/powerpc/kernel/kprobes.c
> +++ b/arch/powerpc/kernel/kprobes.c
> @@ -264,6 +264,9 @@ int kprobe_handler(struct pt_regs *regs)
> if (user_mode(regs))
> return 0;
>
> + if (!(regs->msr & MSR_IR) || !(regs->msr & MSR_DR))
> + return 0;
> +
> /*
> * We don't want to be preempted for the entire
> * duration of kprobe processing
> --
> 2.25.0
>
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply
* [PATCH v2] powerpc/Makefile: Mark phony targets as PHONY
From: Michael Ellerman @ 2020-02-19 0:04 UTC (permalink / raw)
To: linuxppc-dev; +Cc: masahiroy, linux-kbuild
Some of our phony targets are not marked as such. This can lead to
confusing errors, eg:
$ make clean
$ touch install
$ make install
make: 'install' is up to date.
$
Fix it by adding them to the PHONY variable which is marked phony in
the top-level Makefile, or in scripts/Makefile.build for the boot
Makefile.
Suggested-by: Masahiro Yamada <masahiroy@kernel.org>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/Makefile | 6 ++++++
arch/powerpc/boot/Makefile | 2 ++
2 files changed, 8 insertions(+)
v2: Use PHONY variable in boot/Makefile as well.
diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index f35730548e42..cbe5ca4f0ee5 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -298,6 +298,7 @@ $(BOOT_TARGETS2): vmlinux
$(Q)$(MAKE) $(build)=$(boot) $(patsubst %,$(boot)/%,$@)
+PHONY += bootwrapper_install
bootwrapper_install:
$(Q)$(MAKE) $(build)=$(boot) $(patsubst %,$(boot)/%,$@)
@@ -403,9 +404,11 @@ define archhelp
@echo ' (minus the .dts extension).'
endef
+PHONY += install
install:
$(Q)$(MAKE) $(build)=$(boot) install
+PHONY += vdso_install
vdso_install:
ifdef CONFIG_PPC64
$(Q)$(MAKE) $(build)=arch/$(ARCH)/kernel/vdso64 $@
@@ -425,6 +428,7 @@ archprepare: checkbin
ifdef CONFIG_STACKPROTECTOR
prepare: stack_protector_prepare
+PHONY += stack_protector_prepare
stack_protector_prepare: prepare0
ifdef CONFIG_PPC64
$(eval KBUILD_CFLAGS += -mstack-protector-guard-offset=$(shell awk '{if ($$2 == "PACA_CANARY") print $$3;}' include/generated/asm-offsets.h))
@@ -436,10 +440,12 @@ endif
ifdef CONFIG_SMP
prepare: task_cpu_prepare
+PHONY += task_cpu_prepare
task_cpu_prepare: prepare0
$(eval KBUILD_CFLAGS += -D_TASK_CPU=$(shell awk '{if ($$2 == "TASK_CPU") print $$3;}' include/generated/asm-offsets.h))
endif
+PHONY += checkbin
# Check toolchain versions:
# - gcc-4.6 is the minimum kernel-wide version so nothing required.
checkbin:
diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
index 0556bf4fc9e9..c53a1b8bba8b 100644
--- a/arch/powerpc/boot/Makefile
+++ b/arch/powerpc/boot/Makefile
@@ -445,6 +445,8 @@ install: $(CONFIGURE) $(addprefix $(obj)/, $(image-y))
zInstall: $(CONFIGURE) $(addprefix $(obj)/, $(image-y))
sh -x $(srctree)/$(src)/install.sh "$(KERNELRELEASE)" vmlinux System.map "$(INSTALL_PATH)" $^
+PHONY += install zInstall
+
# anything not in $(targets)
clean-files += $(image-) $(initrd-) cuImage.* dtbImage.* treeImage.* \
zImage zImage.initrd zImage.chrp zImage.coff zImage.holly \
--
2.21.1
^ permalink raw reply related
* Re: [PATCH v2 06/27] ocxl: Tally up the LPC memory on a link & allow it to be mapped
From: Alastair D'Silva @ 2020-02-19 0:01 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Masahiro Yamada,
Paul Mackerras, Mauro Carvalho Chehab, Ira Weiny, Thomas Gleixner,
Rob Herring, Dave Jiang, linux-nvdimm, Vishal Verma,
Krzysztof Kozlowski, Anju T Sudhakar, Mahesh Salgaonkar,
Andrew Donnellan, Arnd Bergmann, Greg Kurz, Nicholas Piggin,
Cédric Le Goater, Dan Williams, Hari Bathini, linux-mm,
Greg Kroah-Hartman, linux-kernel, Frederic Barrat, Andrew Morton,
linuxppc-dev, David S. Miller
In-Reply-To: <20200203123712.0000461a@Huawei.com>
On Mon, 2020-02-03 at 12:37 +0000, Jonathan Cameron wrote:
> On Tue, 3 Dec 2019 14:46:34 +1100
> Alastair D'Silva <alastair@au1.ibm.com> wrote:
>
> > From: Alastair D'Silva <alastair@d-silva.org>
> >
> > Tally up the LPC memory on an OpenCAPI link & allow it to be mapped
> >
> > Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> Hi Alastair,
>
> A few trivial comments inline.
>
> Jonathan
>
> > ---
> > drivers/misc/ocxl/core.c | 10 ++++++
> > drivers/misc/ocxl/link.c | 60
> > +++++++++++++++++++++++++++++++
> > drivers/misc/ocxl/ocxl_internal.h | 33 +++++++++++++++++
> > 3 files changed, 103 insertions(+)
> >
> > diff --git a/drivers/misc/ocxl/core.c b/drivers/misc/ocxl/core.c
> > index b7a09b21ab36..2531c6cf19a0 100644
> > --- a/drivers/misc/ocxl/core.c
> > +++ b/drivers/misc/ocxl/core.c
> > @@ -230,8 +230,18 @@ static int configure_afu(struct ocxl_afu *afu,
> > u8 afu_idx, struct pci_dev *dev)
> > if (rc)
> > goto err_free_pasid;
> >
> > + if (afu->config.lpc_mem_size || afu-
> > >config.special_purpose_mem_size) {
> > + rc = ocxl_link_add_lpc_mem(afu->fn->link, afu-
> > >config.lpc_mem_offset,
> > + afu->config.lpc_mem_size +
> > + afu-
> > >config.special_purpose_mem_size);
> > + if (rc)
> > + goto err_free_mmio;
> > + }
> > +
> > return 0;
> >
> > +err_free_mmio:
> > + unmap_mmio_areas(afu);
> > err_free_pasid:
> > reclaim_afu_pasid(afu);
> > err_free_actag:
> > diff --git a/drivers/misc/ocxl/link.c b/drivers/misc/ocxl/link.c
> > index 58d111afd9f6..d8503f0dc6ec 100644
> > --- a/drivers/misc/ocxl/link.c
> > +++ b/drivers/misc/ocxl/link.c
> > @@ -84,6 +84,11 @@ struct ocxl_link {
> > int dev;
> > atomic_t irq_available;
> > struct spa *spa;
> > + struct mutex lpc_mem_lock;
>
> Always a good idea to explicitly document what a lock is intended to
> protect.
>
Ok
> > + u64 lpc_mem_sz; /* Total amount of LPC memory presented on the
> > link */
> > + u64 lpc_mem;
> > + int lpc_consumers;
> > +
> > void *platform_data;
> > };
> > static struct list_head links_list = LIST_HEAD_INIT(links_list);
> > @@ -396,6 +401,8 @@ static int alloc_link(struct pci_dev *dev, int
> > PE_mask, struct ocxl_link **out_l
> > if (rc)
> > goto err_spa;
> >
> > + mutex_init(&link->lpc_mem_lock);
> > +
> > /* platform specific hook */
> > rc = pnv_ocxl_spa_setup(dev, link->spa->spa_mem, PE_mask,
> > &link->platform_data);
> > @@ -711,3 +718,56 @@ void ocxl_link_free_irq(void *link_handle, int
> > hw_irq)
> > atomic_inc(&link->irq_available);
> > }
> > EXPORT_SYMBOL_GPL(ocxl_link_free_irq);
> > +
> > +int ocxl_link_add_lpc_mem(void *link_handle, u64 offset, u64 size)
> > +{
> > + struct ocxl_link *link = (struct ocxl_link *) link_handle;
> > +
> > + // Check for overflow
>
> Stray c++ style comment.
>
This is permitted in powerpc.
> > + if (offset > (offset + size))
> > + return -EINVAL;
> > +
> > + mutex_lock(&link->lpc_mem_lock);
> > + link->lpc_mem_sz = max(link->lpc_mem_sz, offset + size);
> > +
> > + mutex_unlock(&link->lpc_mem_lock);
> > +
> > + return 0;
> > +}
> > +
> > +u64 ocxl_link_lpc_map(void *link_handle, struct pci_dev *pdev)
> > +{
> > + struct ocxl_link *link = (struct ocxl_link *) link_handle;
> > + u64 lpc_mem;
> > +
> > + mutex_lock(&link->lpc_mem_lock);
> > + if (link->lpc_mem) {
>
> If you don't modify this later in the series (I haven't read it all
> yet :),
> it rather feels like it would be more compact and just as readable as
> something like...
>
> if (!link->lpc_mem)
> link->lpc_mem = pnv_ocxl...
>
> if (link->lpc_mem)
> link->lpc_consumers++;
> mutex_unlock(&link->lpc_mem_lock);
>
> return link->lpc_mem;
>
Agreed, thanks.
> > + lpc_mem = link->lpc_mem;
> > +
> > + link->lpc_consumers++;
> > + mutex_unlock(&link->lpc_mem_lock);
> > + return lpc_mem;
> > + }
> > +
> > + link->lpc_mem = pnv_ocxl_platform_lpc_setup(pdev, link-
> > >lpc_mem_sz);
> > + if (link->lpc_mem)
> > + link->lpc_consumers++;
> > + lpc_mem = link->lpc_mem;
> > + mutex_unlock(&link->lpc_mem_lock);
> > +
> > + return lpc_mem;
> > +}
> > +
> > +void ocxl_link_lpc_release(void *link_handle, struct pci_dev
> > *pdev)
> > +{
> > + struct ocxl_link *link = (struct ocxl_link *) link_handle;
> > +
> > + mutex_lock(&link->lpc_mem_lock);
> > + WARN_ON(--link->lpc_consumers < 0);
> > + if (link->lpc_consumers == 0) {
> > + pnv_ocxl_platform_lpc_release(pdev);
> > + link->lpc_mem = 0;
> > + }
> > +
> > + mutex_unlock(&link->lpc_mem_lock);
> > +}
> > diff --git a/drivers/misc/ocxl/ocxl_internal.h
> > b/drivers/misc/ocxl/ocxl_internal.h
> > index 97415afd79f3..20b417e00949 100644
> > --- a/drivers/misc/ocxl/ocxl_internal.h
> > +++ b/drivers/misc/ocxl/ocxl_internal.h
> > @@ -141,4 +141,37 @@ int ocxl_irq_offset_to_id(struct ocxl_context
> > *ctx, u64 offset);
> > u64 ocxl_irq_id_to_offset(struct ocxl_context *ctx, int irq_id);
> > void ocxl_afu_irq_free_all(struct ocxl_context *ctx);
> >
> > +/**
> > + * ocxl_link_add_lpc_mem() - Increment the amount of memory
> > required by an OpenCAPI link
> > + *
> > + * @link_handle: The OpenCAPI link handle
> > + * @offset: The offset of the memory to add
> > + * @size: The amount of memory to increment by
> > + *
> > + * Return 0 on success, negative on overflow
> > + */
> > +int ocxl_link_add_lpc_mem(void *link_handle, u64 offset, u64
> > size);
> > +
> > +/**
> > + * ocxl_link_lpc_map() - Map the LPC memory for an OpenCAPI device
> > + *
> > + * Since LPC memory belongs to a link, the whole LPC memory
> > available
> > + * on the link bust be mapped in order to make it accessible to a
> > device.
>
> must
Whoops :)
>
> > + *
> > + * @link_handle: The OpenCAPI link handle
> > + * @pdev: A device that is on the link
> > + */
> > +u64 ocxl_link_lpc_map(void *link_handle, struct pci_dev *pdev);
> > +
> > +/**
> > + * ocxl_link_lpc_release() - Release the LPC memory device for an
> > OpenCAPI device
> > + *
> > + * Offlines LPC memory on an OpenCAPI link for a device. If this
> > is the
> > + * last device on the link to release the memory, unmap it from
> > the link.
> > + *
> > + * @link_handle: The OpenCAPI link handle
> > + * @pdev: A device that is on the link
> > + */
> > +void ocxl_link_lpc_release(void *link_handle, struct pci_dev
> > *pdev);
> > +
> > #endif /* _OCXL_INTERNAL_H_ */
--
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox