* Re: [PATCH v3 5/6] powerpc/fsl_booke/64: clear the original kernel if randomized
From: Christophe Leroy @ 2020-02-20 13:49 UTC (permalink / raw)
To: Jason Yan, mpe, linuxppc-dev, diana.craciun, benh, paulus,
npiggin, keescook, kernel-hardening, oss
Cc: linux-kernel, zhaohongjiang
In-Reply-To: <20200206025825.22934-6-yanaijie@huawei.com>
Le 06/02/2020 à 03:58, Jason Yan a écrit :
> The original kernel still exists in the memory, clear it now.
No such problem with PPC32 ? Or is that common ?
Christophe
>
> Signed-off-by: Jason Yan <yanaijie@huawei.com>
> Cc: Scott Wood <oss@buserror.net>
> Cc: Diana Craciun <diana.craciun@nxp.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Christophe Leroy <christophe.leroy@c-s.fr>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Nicholas Piggin <npiggin@gmail.com>
> Cc: Kees Cook <keescook@chromium.org>
> ---
> arch/powerpc/mm/nohash/kaslr_booke.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/mm/nohash/kaslr_booke.c b/arch/powerpc/mm/nohash/kaslr_booke.c
> index c6f5c1db1394..ed1277059368 100644
> --- a/arch/powerpc/mm/nohash/kaslr_booke.c
> +++ b/arch/powerpc/mm/nohash/kaslr_booke.c
> @@ -378,8 +378,10 @@ notrace void __init kaslr_early_init(void *dt_ptr, phys_addr_t size)
> unsigned int *__kaslr_offset = (unsigned int *)(KERNELBASE + 0x58);
> unsigned int *__run_at_load = (unsigned int *)(KERNELBASE + 0x5c);
>
> - if (*__run_at_load == 1)
> + if (*__run_at_load == 1) {
> + kaslr_late_init();
> return;
> + }
>
> /* Setup flat device-tree pointer */
> initial_boot_params = dt_ptr;
>
^ permalink raw reply
* Re: [PATCH v3 3/6] powerpc/fsl_booke/64: implement KASLR for fsl_booke64
From: Christophe Leroy @ 2020-02-20 13:48 UTC (permalink / raw)
To: Jason Yan, mpe, linuxppc-dev, diana.craciun, benh, paulus,
npiggin, keescook, kernel-hardening, oss
Cc: linux-kernel, zhaohongjiang
In-Reply-To: <20200206025825.22934-4-yanaijie@huawei.com>
Le 06/02/2020 à 03:58, Jason Yan a écrit :
> The implementation for Freescale BookE64 is similar as BookE32. One
> difference is that Freescale BookE64 set up a TLB mapping of 1G during
> booting. Another difference is that ppc64 needs the kernel to be
> 64K-aligned. So we can randomize the kernel in this 1G mapping and make
> it 64K-aligned. This can save some code to creat another TLB map at
> early boot. The disadvantage is that we only have about 1G/64K = 16384
> slots to put the kernel in.
>
> To support secondary cpu boot up, a variable __kaslr_offset was added in
> first_256B section. This can help secondary cpu get the kaslr offset
> before the 1:1 mapping has been setup.
>
> Signed-off-by: Jason Yan <yanaijie@huawei.com>
> Cc: Scott Wood <oss@buserror.net>
> Cc: Diana Craciun <diana.craciun@nxp.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Christophe Leroy <christophe.leroy@c-s.fr>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Nicholas Piggin <npiggin@gmail.com>
> Cc: Kees Cook <keescook@chromium.org>
> ---
> arch/powerpc/Kconfig | 2 +-
> arch/powerpc/kernel/exceptions-64e.S | 10 +++++++++
> arch/powerpc/kernel/head_64.S | 7 ++++++
> arch/powerpc/kernel/setup_64.c | 4 +++-
> arch/powerpc/mm/mmu_decl.h | 16 +++++++-------
> arch/powerpc/mm/nohash/kaslr_booke.c | 33 +++++++++++++++++++++++++---
> 6 files changed, 59 insertions(+), 13 deletions(-)
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index c150a9d49343..754aeb96bb1c 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -568,7 +568,7 @@ config RELOCATABLE
>
> config RANDOMIZE_BASE
> bool "Randomize the address of the kernel image"
> - depends on (FSL_BOOKE && FLATMEM && PPC32)
> + depends on (PPC_FSL_BOOK3E && FLATMEM)
> depends on RELOCATABLE
> help
> Randomizes the virtual address at which the kernel image is
> diff --git a/arch/powerpc/kernel/exceptions-64e.S b/arch/powerpc/kernel/exceptions-64e.S
> index 1b9b174bee86..c1c05b8684ca 100644
> --- a/arch/powerpc/kernel/exceptions-64e.S
> +++ b/arch/powerpc/kernel/exceptions-64e.S
> @@ -1378,6 +1378,7 @@ skpinv: addi r6,r6,1 /* Increment */
> 1: mflr r6
> addi r6,r6,(2f - 1b)
> tovirt(r6,r6)
> + add r6,r6,r19
> lis r7,MSR_KERNEL@h
> ori r7,r7,MSR_KERNEL@l
> mtspr SPRN_SRR0,r6
> @@ -1400,6 +1401,7 @@ skpinv: addi r6,r6,1 /* Increment */
>
> /* We translate LR and return */
> tovirt(r8,r8)
> + add r8,r8,r19
> mtlr r8
> blr
>
> @@ -1528,6 +1530,7 @@ a2_tlbinit_code_end:
> */
> _GLOBAL(start_initialization_book3e)
> mflr r28
> + li r19, 0
>
> /* First, we need to setup some initial TLBs to map the kernel
> * text, data and bss at PAGE_OFFSET. We don't have a real mode
> @@ -1570,6 +1573,12 @@ _GLOBAL(book3e_secondary_core_init)
> cmplwi r4,0
> bne 2f
>
> + li r19, 0
> +#ifdef CONFIG_RANDOMIZE_BASE
> + LOAD_REG_ADDR_PIC(r19, __kaslr_offset)
> + lwz r19,0(r19)
> + rlwinm r19,r19,0,0,5
> +#endif
> /* Setup TLB for this core */
> bl initial_tlb_book3e
>
> @@ -1602,6 +1611,7 @@ _GLOBAL(book3e_secondary_core_init)
> lis r3,PAGE_OFFSET@highest
> sldi r3,r3,32
> or r28,r28,r3
> + add r28,r28,r19
> 1: mtlr r28
> blr
>
> diff --git a/arch/powerpc/kernel/head_64.S b/arch/powerpc/kernel/head_64.S
> index ad79fddb974d..744624140fb8 100644
> --- a/arch/powerpc/kernel/head_64.S
> +++ b/arch/powerpc/kernel/head_64.S
> @@ -104,6 +104,13 @@ __secondary_hold_acknowledge:
> .8byte 0x0
>
> #ifdef CONFIG_RELOCATABLE
> +#ifdef CONFIG_RANDOMIZE_BASE
> + . = 0x58
> + .globl __kaslr_offset
> +__kaslr_offset:
> +DEFINE_FIXED_SYMBOL(__kaslr_offset)
> + .long 0
> +#endif
> /* This flag is set to 1 by a loader if the kernel should run
> * at the loaded address instead of the linked address. This
> * is used by kexec-tools to keep the the kdump kernel in the
> diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
> index 6104917a282d..a16b970a8d1a 100644
> --- a/arch/powerpc/kernel/setup_64.c
> +++ b/arch/powerpc/kernel/setup_64.c
> @@ -66,7 +66,7 @@
> #include <asm/feature-fixups.h>
> #include <asm/kup.h>
> #include <asm/early_ioremap.h>
> -
Why remove this new line which clearly separates things in asm/ and
things in local dir ?
> +#include <mm/mmu_decl.h>
> #include "setup.h"
>
> int spinning_secondaries;
> @@ -300,6 +300,8 @@ void __init early_setup(unsigned long dt_ptr)
> /* Enable early debugging if any specified (see udbg.h) */
> udbg_early_init();
>
> + kaslr_early_init(__va(dt_ptr), 0);
> +
> udbg_printf(" -> %s(), dt_ptr: 0x%lx\n", __func__, dt_ptr);
>
> /*
> diff --git a/arch/powerpc/mm/mmu_decl.h b/arch/powerpc/mm/mmu_decl.h
> index 3e1c85c7d10b..bbd721d1e3d7 100644
> --- a/arch/powerpc/mm/mmu_decl.h
> +++ b/arch/powerpc/mm/mmu_decl.h
> @@ -147,14 +147,6 @@ void reloc_kernel_entry(void *fdt, long addr);
> extern void loadcam_entry(unsigned int index);
> extern void loadcam_multi(int first_idx, int num, int tmp_idx);
>
> -#ifdef CONFIG_RANDOMIZE_BASE
> -void kaslr_early_init(void *dt_ptr, phys_addr_t size);
> -void kaslr_late_init(void);
> -#else
> -static inline void kaslr_early_init(void *dt_ptr, phys_addr_t size) {}
> -static inline void kaslr_late_init(void) {}
> -#endif
> -
> struct tlbcam {
> u32 MAS0;
> u32 MAS1;
> @@ -164,6 +156,14 @@ struct tlbcam {
> };
> #endif
>
> +#ifdef CONFIG_RANDOMIZE_BASE
> +void kaslr_early_init(void *dt_ptr, phys_addr_t size);
> +void kaslr_late_init(void);
> +#else
> +static inline void kaslr_early_init(void *dt_ptr, phys_addr_t size) {}
> +static inline void kaslr_late_init(void) {}
> +#endif
> +
> #if defined(CONFIG_PPC_BOOK3S_32) || defined(CONFIG_FSL_BOOKE) || defined(CONFIG_PPC_8xx)
> /* 6xx have BATS */
> /* FSL_BOOKE have TLBCAM */
> diff --git a/arch/powerpc/mm/nohash/kaslr_booke.c b/arch/powerpc/mm/nohash/kaslr_booke.c
> index 07b036e98353..c6f5c1db1394 100644
> --- a/arch/powerpc/mm/nohash/kaslr_booke.c
> +++ b/arch/powerpc/mm/nohash/kaslr_booke.c
> @@ -231,7 +231,7 @@ static __init unsigned long get_usable_address(const void *fdt,
> unsigned long pa;
> unsigned long pa_end;
>
> - for (pa = offset; (long)pa > (long)start; pa -= SZ_16K) {
> + for (pa = offset; (long)pa > (long)start; pa -= SZ_64K) {
Doesn't this modify the behaviour for PPC32 too ?
> pa_end = pa + regions.kernel_size;
> if (overlaps_region(fdt, pa, pa_end))
> continue;
> @@ -265,14 +265,14 @@ static unsigned long __init kaslr_legal_offset(void *dt_ptr, unsigned long rando
> {
> unsigned long koffset = 0;
> unsigned long start;
> - unsigned long index;
> unsigned long offset;
>
> +#ifdef CONFIG_PPC32
Can we use
if (IS_ENABLED(CONFIG_PPC32)) {
/* 32 bits stuff */
} else {
/* 64 bits stuff */
}
> /*
> * Decide which 64M we want to start
> * Only use the low 8 bits of the random seed
> */
> - index = random & 0xFF;
> + unsigned long index = random & 0xFF;
That's not good in terms of readability, index declaration should remain
at the top of the function, should be possible if using IS_ENABLED() instead
> index %= regions.linear_sz / SZ_64M;
>
> /* Decide offset inside 64M */
> @@ -287,6 +287,15 @@ static unsigned long __init kaslr_legal_offset(void *dt_ptr, unsigned long rando
> break;
> index--;
> }
> +#else
> + /* Decide kernel offset inside 1G */
> + offset = random % (SZ_1G - regions.kernel_size);
> + offset = round_down(offset, SZ_64K);
> +
> + start = memstart_addr;
> + offset = memstart_addr + offset;
> + koffset = get_usable_address(dt_ptr, start, offset);
> +#endif
>
> if (koffset != 0)
> koffset -= memstart_addr;
> @@ -325,6 +334,7 @@ static unsigned long __init kaslr_choose_location(void *dt_ptr, phys_addr_t size
> else
> pr_warn("KASLR: No safe seed for randomizing the kernel base.\n");
>
> +#ifdef CONFIG_PPC32
> ram = min_t(phys_addr_t, __max_low_memory, size);
> ram = map_mem_in_cams(ram, CONFIG_LOWMEM_CAM_NUM, true);
> linear_sz = min_t(unsigned long, ram, SZ_512M);
> @@ -332,6 +342,7 @@ static unsigned long __init kaslr_choose_location(void *dt_ptr, phys_addr_t size
> /* If the linear size is smaller than 64M, do not randmize */
> if (linear_sz < SZ_64M)
> return 0;
> +#endif
>
> /* check for a reserved-memory node and record its cell sizes */
> regions.reserved_mem = fdt_path_offset(dt_ptr, "/reserved-memory");
> @@ -363,6 +374,17 @@ notrace void __init kaslr_early_init(void *dt_ptr, phys_addr_t size)
> unsigned long offset;
> unsigned long kernel_sz;
>
> +#ifdef CONFIG_PPC64
Same, can we use a standard C if/else sequence with
IS_ENABLED(CONFIG_PPC64) ?
> + unsigned int *__kaslr_offset = (unsigned int *)(KERNELBASE + 0x58);
> + unsigned int *__run_at_load = (unsigned int *)(KERNELBASE + 0x5c);
> +
> + if (*__run_at_load == 1)
> + return;
> +
> + /* Setup flat device-tree pointer */
> + initial_boot_params = dt_ptr;
> +#endif
> +
> kernel_sz = (unsigned long)_end - (unsigned long)_stext;
>
> offset = kaslr_choose_location(dt_ptr, size, kernel_sz);
> @@ -372,6 +394,7 @@ notrace void __init kaslr_early_init(void *dt_ptr, phys_addr_t size)
> kernstart_virt_addr += offset;
> kernstart_addr += offset;
>
> +#ifdef CONFIG_PPC32
> is_second_reloc = 1;
>
> if (offset >= SZ_64M) {
> @@ -381,6 +404,10 @@ notrace void __init kaslr_early_init(void *dt_ptr, phys_addr_t size)
> /* Create kernel map to relocate in */
> create_kaslr_tlb_entry(1, tlb_virt, tlb_phys);
> }
> +#else
> + *__kaslr_offset = kernstart_virt_addr - KERNELBASE;
> + *__run_at_load = 1;
> +#endif
>
> /* Copy the kernel to it's new location and run */
> memcpy((void *)kernstart_virt_addr, (void *)_stext, kernel_sz);
>
Christophe
^ permalink raw reply
* Re: [PATCH v3 2/6] powerpc/fsl_booke/64: introduce reloc_kernel_entry() helper
From: Christophe Leroy @ 2020-02-20 13:41 UTC (permalink / raw)
To: Jason Yan, mpe, linuxppc-dev, diana.craciun, benh, paulus,
npiggin, keescook, kernel-hardening, oss
Cc: linux-kernel, zhaohongjiang
In-Reply-To: <20200206025825.22934-3-yanaijie@huawei.com>
Le 06/02/2020 à 03:58, Jason Yan a écrit :
> Like the 32bit code, we introduce reloc_kernel_entry() helper to prepare
> for the KASLR 64bit version. And move the C declaration of this function
> out of CONFIG_PPC32 and use long instead of int for the parameter 'addr'.
>
> Signed-off-by: Jason Yan <yanaijie@huawei.com>
> Cc: Scott Wood <oss@buserror.net>
> Cc: Diana Craciun <diana.craciun@nxp.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Christophe Leroy <christophe.leroy@c-s.fr>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Nicholas Piggin <npiggin@gmail.com>
> Cc: Kees Cook <keescook@chromium.org>
Reviewed-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
> arch/powerpc/kernel/exceptions-64e.S | 13 +++++++++++++
> arch/powerpc/mm/mmu_decl.h | 3 ++-
> 2 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/exceptions-64e.S b/arch/powerpc/kernel/exceptions-64e.S
> index e4076e3c072d..1b9b174bee86 100644
> --- a/arch/powerpc/kernel/exceptions-64e.S
> +++ b/arch/powerpc/kernel/exceptions-64e.S
> @@ -1679,3 +1679,16 @@ _GLOBAL(setup_ehv_ivors)
> _GLOBAL(setup_lrat_ivor)
> SET_IVOR(42, 0x340) /* LRAT Error */
> blr
> +
> +/*
> + * Return to the start of the relocated kernel and run again
> + * r3 - virtual address of fdt
> + * r4 - entry of the kernel
> + */
> +_GLOBAL(reloc_kernel_entry)
> + mfmsr r7
> + rlwinm r7, r7, 0, ~(MSR_IS | MSR_DS)
> +
> + mtspr SPRN_SRR0,r4
> + mtspr SPRN_SRR1,r7
> + rfi
> diff --git a/arch/powerpc/mm/mmu_decl.h b/arch/powerpc/mm/mmu_decl.h
> index 8e99649c24fc..3e1c85c7d10b 100644
> --- a/arch/powerpc/mm/mmu_decl.h
> +++ b/arch/powerpc/mm/mmu_decl.h
> @@ -140,9 +140,10 @@ extern void adjust_total_lowmem(void);
> extern int switch_to_as1(void);
> extern void restore_to_as0(int esel, int offset, void *dt_ptr, int bootcpu);
> void create_kaslr_tlb_entry(int entry, unsigned long virt, phys_addr_t phys);
> -void reloc_kernel_entry(void *fdt, int addr);
> extern int is_second_reloc;
> #endif
> +
> +void reloc_kernel_entry(void *fdt, long addr);
> extern void loadcam_entry(unsigned int index);
> extern void loadcam_multi(int first_idx, int num, int tmp_idx);
>
>
^ permalink raw reply
* Re: [PATCH v3 1/6] powerpc/fsl_booke/kaslr: refactor kaslr_legal_offset() and kaslr_early_init()
From: Christophe Leroy @ 2020-02-20 13:40 UTC (permalink / raw)
To: Jason Yan, mpe, linuxppc-dev, diana.craciun, benh, paulus,
npiggin, keescook, kernel-hardening, oss
Cc: linux-kernel, zhaohongjiang
In-Reply-To: <20200206025825.22934-2-yanaijie@huawei.com>
Le 06/02/2020 à 03:58, Jason Yan a écrit :
> Some code refactor in kaslr_legal_offset() and kaslr_early_init(). No
> functional change. This is a preparation for KASLR fsl_booke64.
>
> Signed-off-by: Jason Yan <yanaijie@huawei.com>
> Cc: Scott Wood <oss@buserror.net>
> Cc: Diana Craciun <diana.craciun@nxp.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Christophe Leroy <christophe.leroy@c-s.fr>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Nicholas Piggin <npiggin@gmail.com>
> Cc: Kees Cook <keescook@chromium.org>
> ---
> arch/powerpc/mm/nohash/kaslr_booke.c | 40 ++++++++++++++--------------
> 1 file changed, 20 insertions(+), 20 deletions(-)
>
> diff --git a/arch/powerpc/mm/nohash/kaslr_booke.c b/arch/powerpc/mm/nohash/kaslr_booke.c
> index 4a75f2d9bf0e..07b036e98353 100644
> --- a/arch/powerpc/mm/nohash/kaslr_booke.c
> +++ b/arch/powerpc/mm/nohash/kaslr_booke.c
> @@ -25,6 +25,7 @@ struct regions {
> unsigned long pa_start;
> unsigned long pa_end;
> unsigned long kernel_size;
> + unsigned long linear_sz;
> unsigned long dtb_start;
> unsigned long dtb_end;
> unsigned long initrd_start;
> @@ -260,11 +261,23 @@ static __init void get_cell_sizes(const void *fdt, int node, int *addr_cells,
> *size_cells = fdt32_to_cpu(*prop);
> }
>
> -static unsigned long __init kaslr_legal_offset(void *dt_ptr, unsigned long index,
> - unsigned long offset)
> +static unsigned long __init kaslr_legal_offset(void *dt_ptr, unsigned long random)
> {
> unsigned long koffset = 0;
> unsigned long start;
> + unsigned long index;
> + unsigned long offset;
> +
> + /*
> + * Decide which 64M we want to start
> + * Only use the low 8 bits of the random seed
> + */
> + index = random & 0xFF;
> + index %= regions.linear_sz / SZ_64M;
> +
> + /* Decide offset inside 64M */
> + offset = random % (SZ_64M - regions.kernel_size);
> + offset = round_down(offset, SZ_16K);
>
> while ((long)index >= 0) {
> offset = memstart_addr + index * SZ_64M + offset;
> @@ -289,10 +302,9 @@ static inline __init bool kaslr_disabled(void)
> static unsigned long __init kaslr_choose_location(void *dt_ptr, phys_addr_t size,
> unsigned long kernel_sz)
> {
> - unsigned long offset, random;
> + unsigned long random;
> unsigned long ram, linear_sz;
> u64 seed;
> - unsigned long index;
>
> kaslr_get_cmdline(dt_ptr);
> if (kaslr_disabled())
> @@ -333,22 +345,12 @@ static unsigned long __init kaslr_choose_location(void *dt_ptr, phys_addr_t size
> regions.dtb_start = __pa(dt_ptr);
> regions.dtb_end = __pa(dt_ptr) + fdt_totalsize(dt_ptr);
> regions.kernel_size = kernel_sz;
> + regions.linear_sz = linear_sz;
>
> get_initrd_range(dt_ptr);
> get_crash_kernel(dt_ptr, ram);
>
> - /*
> - * Decide which 64M we want to start
> - * Only use the low 8 bits of the random seed
> - */
> - index = random & 0xFF;
> - index %= linear_sz / SZ_64M;
> -
> - /* Decide offset inside 64M */
> - offset = random % (SZ_64M - kernel_sz);
> - offset = round_down(offset, SZ_16K);
> -
> - return kaslr_legal_offset(dt_ptr, index, offset);
> + return kaslr_legal_offset(dt_ptr, random);
> }
>
> /*
> @@ -358,8 +360,6 @@ static unsigned long __init kaslr_choose_location(void *dt_ptr, phys_addr_t size
> */
> notrace void __init kaslr_early_init(void *dt_ptr, phys_addr_t size)
> {
> - unsigned long tlb_virt;
> - phys_addr_t tlb_phys;
> unsigned long offset;
> unsigned long kernel_sz;
>
> @@ -375,8 +375,8 @@ notrace void __init kaslr_early_init(void *dt_ptr, phys_addr_t size)
> is_second_reloc = 1;
>
> if (offset >= SZ_64M) {
> - tlb_virt = round_down(kernstart_virt_addr, SZ_64M);
> - tlb_phys = round_down(kernstart_addr, SZ_64M);
> + unsigned long tlb_virt = round_down(kernstart_virt_addr, SZ_64M);
> + phys_addr_t tlb_phys = round_down(kernstart_addr, SZ_64M);
That looks like cleanup unrelated to the patch itself.
>
> /* Create kernel map to relocate in */
> create_kaslr_tlb_entry(1, tlb_virt, tlb_phys);
>
Christophe
^ permalink raw reply
* Re: [PATCH v3 1/5] powerpc: Rename current_stack_pointer() to current_stack_frame()
From: Christophe Leroy @ 2020-02-20 13:26 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev
In-Reply-To: <20200220115141.2707-1-mpe@ellerman.id.au>
Le 20/02/2020 à 12:51, Michael Ellerman a écrit :
> current_stack_pointer(), which was called __get_SP(), used to just
> return the value in r1.
>
> But that caused problems in some cases, so it was turned into a
> function in commit bfe9a2cfe91a ("powerpc: Reimplement __get_SP() as a
> function not a define").
>
> Because it's a function in a separate compilation unit to all its
> callers, it has the effect of causing a stack frame to be created, and
> then returns the address of that frame. This is good in some cases
> like those described in the above commit, but in other cases it's
> overkill, we just need to know what stack page we're on.
>
> On some other arches current_stack_pointer is just a register global
> giving the stack pointer, and we'd like to do that too. So rename our
> current_stack_pointer() to current_stack_frame() to make that
> possible.
>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
LGTM
I was afraid to do that and risk invisible conflicts hence bugs by
reusing the same name for different purpose, but that's the best
solution for sure.
Reviewed-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
> arch/powerpc/include/asm/perf_event.h | 2 +-
> arch/powerpc/include/asm/reg.h | 2 +-
> arch/powerpc/kernel/irq.c | 4 ++--
> arch/powerpc/kernel/misc.S | 4 ++--
> arch/powerpc/kernel/process.c | 2 +-
> arch/powerpc/kernel/stacktrace.c | 6 +++---
> 6 files changed, 10 insertions(+), 10 deletions(-)
>
> v3: New.
>
> diff --git a/arch/powerpc/include/asm/perf_event.h b/arch/powerpc/include/asm/perf_event.h
> index 7426d7a90e1e..eed3954082fa 100644
> --- a/arch/powerpc/include/asm/perf_event.h
> +++ b/arch/powerpc/include/asm/perf_event.h
> @@ -32,7 +32,7 @@
> do { \
> (regs)->result = 0; \
> (regs)->nip = __ip; \
> - (regs)->gpr[1] = current_stack_pointer(); \
> + (regs)->gpr[1] = current_stack_frame(); \
> asm volatile("mfmsr %0" : "=r" ((regs)->msr)); \
> } while (0)
>
> diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
> index 1aa46dff0957..1b1ffdba6097 100644
> --- a/arch/powerpc/include/asm/reg.h
> +++ b/arch/powerpc/include/asm/reg.h
> @@ -1448,7 +1448,7 @@ static inline void mtsrin(u32 val, u32 idx)
>
> #define proc_trap() asm volatile("trap")
>
> -extern unsigned long current_stack_pointer(void);
> +extern unsigned long current_stack_frame(void);
>
> extern unsigned long scom970_read(unsigned int address);
> extern void scom970_write(unsigned int address, unsigned long value);
> diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
> index 5c9b11878555..02118c18434d 100644
> --- a/arch/powerpc/kernel/irq.c
> +++ b/arch/powerpc/kernel/irq.c
> @@ -602,7 +602,7 @@ static inline void check_stack_overflow(void)
> #ifdef CONFIG_DEBUG_STACKOVERFLOW
> long sp;
>
> - sp = current_stack_pointer() & (THREAD_SIZE-1);
> + sp = current_stack_frame() & (THREAD_SIZE-1);
>
> /* check for stack overflow: is there less than 2KB free? */
> if (unlikely(sp < 2048)) {
> @@ -647,7 +647,7 @@ void do_IRQ(struct pt_regs *regs)
> void *cursp, *irqsp, *sirqsp;
>
> /* Switch to the irq stack to handle this */
> - cursp = (void *)(current_stack_pointer() & ~(THREAD_SIZE - 1));
> + cursp = (void *)(current_stack_frame() & ~(THREAD_SIZE - 1));
> irqsp = hardirq_ctx[raw_smp_processor_id()];
> sirqsp = softirq_ctx[raw_smp_processor_id()];
>
> diff --git a/arch/powerpc/kernel/misc.S b/arch/powerpc/kernel/misc.S
> index 974f65f79a8e..65f9f731c229 100644
> --- a/arch/powerpc/kernel/misc.S
> +++ b/arch/powerpc/kernel/misc.S
> @@ -110,7 +110,7 @@ _GLOBAL(longjmp)
> li r3, 1
> blr
>
> -_GLOBAL(current_stack_pointer)
> +_GLOBAL(current_stack_frame)
> PPC_LL r3,0(r1)
> blr
> -EXPORT_SYMBOL(current_stack_pointer)
> +EXPORT_SYMBOL(current_stack_frame)
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index e730b8e522b0..110db94cdf3c 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -2051,7 +2051,7 @@ void show_stack(struct task_struct *tsk, unsigned long *stack)
> sp = (unsigned long) stack;
> if (sp == 0) {
> if (tsk == current)
> - sp = current_stack_pointer();
> + sp = current_stack_frame();
> else
> sp = tsk->thread.ksp;
> }
> diff --git a/arch/powerpc/kernel/stacktrace.c b/arch/powerpc/kernel/stacktrace.c
> index e2a46cfed5fd..c477b8585a29 100644
> --- a/arch/powerpc/kernel/stacktrace.c
> +++ b/arch/powerpc/kernel/stacktrace.c
> @@ -57,7 +57,7 @@ void save_stack_trace(struct stack_trace *trace)
> {
> unsigned long sp;
>
> - sp = current_stack_pointer();
> + sp = current_stack_frame();
>
> save_context_stack(trace, sp, current, 1);
> }
> @@ -71,7 +71,7 @@ void save_stack_trace_tsk(struct task_struct *tsk, struct stack_trace *trace)
> return;
>
> if (tsk == current)
> - sp = current_stack_pointer();
> + sp = current_stack_frame();
> else
> sp = tsk->thread.ksp;
>
> @@ -131,7 +131,7 @@ static int __save_stack_trace_tsk_reliable(struct task_struct *tsk,
> }
>
> if (tsk == current)
> - sp = current_stack_pointer();
> + sp = current_stack_frame();
> else
> sp = tsk->thread.ksp;
>
>
^ permalink raw reply
* Re: [PATCH v5 01/10] capabilities: introduce CAP_PERFMON to kernel and user space
From: Alexey Budankov @ 2020-02-20 13:05 UTC (permalink / raw)
To: Thomas Gleixner, Stephen Smalley, Serge Hallyn, James Morris
Cc: Mark Rutland, Song Liu, Peter Zijlstra,
joonas.lahtinen@linux.intel.com, Will Deacon, Alexei Starovoitov,
Lionel Landwerlin, Paul Mackerras, Jiri Olsa, Alexei Starovoitov,
Andi Kleen, Igor Lubashev, Alexander Shishkin, Ingo Molnar,
oprofile-list, linux-arm-kernel, Robert Richter,
selinux@vger.kernel.org, intel-gfx@lists.freedesktop.org,
jani.nikula@linux.intel.com, Arnaldo Carvalho de Melo,
rodrigo.vivi@intel.com, Namhyung Kim, Stephane Eranian,
linux-parisc@vger.kernel.org, linux-kernel, Andy Lutomirski,
linux-perf-users@vger.kernel.org,
linux-security-module@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <7d6f4210-423f-e454-3910-9f8e17dff1aa@linux.intel.com>
On 07.02.2020 16:39, Alexey Budankov wrote:
>
> On 07.02.2020 14:38, Thomas Gleixner wrote:
>> Alexey Budankov <alexey.budankov@linux.intel.com> writes:
>>> On 22.01.2020 17:25, Alexey Budankov wrote:
>>>> On 22.01.2020 17:07, Stephen Smalley wrote:
>>>>>> It keeps the implementation simple and readable. The implementation is more
>>>>>> performant in the sense of calling the API - one capable() call for CAP_PERFMON
>>>>>> privileged process.
>>>>>>
>>>>>> Yes, it bloats audit log for CAP_SYS_ADMIN privileged and unprivileged processes,
>>>>>> but this bloating also advertises and leverages using more secure CAP_PERFMON
>>>>>> based approach to use perf_event_open system call.
>>>>>
>>>>> I can live with that. We just need to document that when you see
>>>>> both a CAP_PERFMON and a CAP_SYS_ADMIN audit message for a process,
>>>>> try only allowing CAP_PERFMON first and see if that resolves the
>>>>> issue. We have a similar issue with CAP_DAC_READ_SEARCH versus
>>>>> CAP_DAC_OVERRIDE.
>>>>
>>>> perf security [1] document can be updated, at least, to align and document
>>>> this audit logging specifics.
>>>
>>> And I plan to update the document right after this patch set is accepted.
>>> Feel free to let me know of the places in the kernel docs that also
>>> require update w.r.t CAP_PERFMON extension.
>>
>> The documentation update wants be part of the patch set and not planned
>> to be done _after_ the patch set is merged.
>
> Well, accepted. It is going to make patches #11 and beyond.
Patches #11 and #12 of v7 [1] contain information on CAP_PERFMON intention and usage.
Patch for man-pages [2] extends perf_event_open.2 documentation.
Thanks,
Alexey
---
[1] https://lore.kernel.org/lkml/c8de937a-0b3a-7147-f5ef-69f467e87a13@linux.intel.com/
[2] https://lore.kernel.org/lkml/18d1083d-efe5-f5f8-c531-d142c0e5c1a8@linux.intel.com/
^ permalink raw reply
* [Bug 206525] BUG: KASAN: stack-out-of-bounds in test_bit+0x30/0x44 (kernel 5.6-rc1)
From: bugzilla-daemon @ 2020-02-20 12:19 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-206525-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=206525
--- Comment #6 from Nikolay Aleksandrov (nikolay@cumulusnetworks.com) ---
Note that the bug wasn't introduced by my commit, but instead has been there
since:
commit 4f520900522f
Author: Richard Guy Briggs <rgb@redhat.com>
Date: Tue Apr 22 21:31:54 2014 -0400
netlink: have netlink per-protocol bind function return an error code.
which moved the ngroups test_bit() to a local variable. My commit only exposed
the bug since it added the 33rd group. I'm currently preparing a fix and will
post it to netdev after verifying and testing it.
--
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* [PATCH v3 5/5] powerpc/irq: Use current_stack_pointer in do_IRQ()
From: Michael Ellerman @ 2020-02-20 11:51 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <20200220115141.2707-1-mpe@ellerman.id.au>
From: Christophe Leroy <christophe.leroy@c-s.fr>
Until commit 7306e83ccf5c ("powerpc: Don't use CURRENT_THREAD_INFO to
find the stack"), the current stack base address was obtained by
calling current_thread_info(). That inline function was simply masking
out the value of r1.
In that commit, it was changed to using current_stack_pointer() (since
renamed current_stack_frame()), which is a heavier function as it is
an outline assembly function which cannot be inlined and which reads
the content of the stack at 0(r1).
Convert to using current_stack_pointer for geting r1 and masking out
its value to obtain the base address of the stack pointer as before.
Fixes: 7306e83ccf5c ("powerpc: Don't use CURRENT_THREAD_INFO to find the stack")
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/a37e699e7ab897742c07b6838a08af33bc9217e3.1579849665.git.christophe.leroy@c-s.fr
---
arch/powerpc/kernel/irq.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
index 46d5852fb00a..1bed18b7229e 100644
--- a/arch/powerpc/kernel/irq.c
+++ b/arch/powerpc/kernel/irq.c
@@ -648,7 +648,7 @@ void do_IRQ(struct pt_regs *regs)
void *cursp, *irqsp, *sirqsp;
/* Switch to the irq stack to handle this */
- cursp = (void *)(current_stack_frame() & ~(THREAD_SIZE - 1));
+ cursp = (void *)(current_stack_pointer & ~(THREAD_SIZE - 1));
irqsp = hardirq_ctx[raw_smp_processor_id()];
sirqsp = softirq_ctx[raw_smp_processor_id()];
--
2.21.1
v3: s/get_sp()/current_stack_pointer/
^ permalink raw reply related
* [PATCH v3 4/5] powerpc/irq: use IS_ENABLED() in check_stack_overflow()
From: Michael Ellerman @ 2020-02-20 11:51 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <20200220115141.2707-1-mpe@ellerman.id.au>
From: Christophe Leroy <christophe.leroy@c-s.fr>
Instead of #ifdef, use IS_ENABLED(CONFIG_DEBUG_STACKOVERFLOW).
This enable GCC to check for code validity even when the option
is not selected.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/98855694e9e8993673af08cc2e97e16e0cf50f4a.1579849665.git.christophe.leroy@c-s.fr
---
arch/powerpc/kernel/irq.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
index c7d6f5cdffdb..46d5852fb00a 100644
--- a/arch/powerpc/kernel/irq.c
+++ b/arch/powerpc/kernel/irq.c
@@ -599,9 +599,11 @@ u64 arch_irq_stat_cpu(unsigned int cpu)
static inline void check_stack_overflow(void)
{
-#ifdef CONFIG_DEBUG_STACKOVERFLOW
long sp;
+ if (!IS_ENABLED(CONFIG_DEBUG_STACKOVERFLOW))
+ return;
+
sp = current_stack_pointer & (THREAD_SIZE - 1);
/* check for stack overflow: is there less than 2KB free? */
@@ -609,7 +611,6 @@ static inline void check_stack_overflow(void)
pr_err("do_IRQ: stack overflow: %ld\n", sp);
dump_stack();
}
-#endif
}
void __do_irq(struct pt_regs *regs)
--
2.21.1
^ permalink raw reply related
* [PATCH v3 3/5] powerpc/irq: Use current_stack_pointer in check_stack_overflow()
From: Michael Ellerman @ 2020-02-20 11:51 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <20200220115141.2707-1-mpe@ellerman.id.au>
From: Christophe Leroy <christophe.leroy@c-s.fr>
The purpose of check_stack_overflow() is to verify that the stack has
not overflowed.
To really know whether the stack pointer is still within boundaries,
the check must be done directly on the value of r1.
So use current_stack_pointer, which returns the current value of r1,
rather than current_stack_frame() which causes a frame to be created
and then returns that value.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/435e0030e942507766cbef5bc95f906262d2ccf2.1579849665.git.christophe.leroy@c-s.fr
---
arch/powerpc/kernel/irq.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
index 02118c18434d..c7d6f5cdffdb 100644
--- a/arch/powerpc/kernel/irq.c
+++ b/arch/powerpc/kernel/irq.c
@@ -602,7 +602,7 @@ static inline void check_stack_overflow(void)
#ifdef CONFIG_DEBUG_STACKOVERFLOW
long sp;
- sp = current_stack_frame() & (THREAD_SIZE-1);
+ sp = current_stack_pointer & (THREAD_SIZE - 1);
/* check for stack overflow: is there less than 2KB free? */
if (unlikely(sp < 2048)) {
--
2.21.1
v3: s/get_sp()/current_stack_pointer/
^ permalink raw reply related
* [PATCH v3 2/5] powerpc: Add current_stack_pointer as a register global
From: Michael Ellerman @ 2020-02-20 11:51 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <20200220115141.2707-1-mpe@ellerman.id.au>
From: Christophe Leroy <christophe.leroy@c-s.fr>
current_stack_frame() doesn't return the stack pointer, but the
caller's stack frame. See commit bfe9a2cfe91a ("powerpc: Reimplement
__get_SP() as a function not a define") and commit
acf620ecf56c ("powerpc: Rename __get_SP() to current_stack_pointer()")
for details.
In some cases this is overkill or incorrect, as it doesn't return the
current value of r1.
So add a current_stack_pointer register global to get the value of r1
directly.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
[mpe: Split out of other patch, tweak change log]
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/435e0030e942507766cbef5bc95f906262d2ccf2.1579849665.git.christophe.leroy@c-s.fr
---
arch/powerpc/include/asm/reg.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index 1b1ffdba6097..da5cab038e25 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -1450,6 +1450,8 @@ static inline void mtsrin(u32 val, u32 idx)
extern unsigned long current_stack_frame(void);
+register unsigned long current_stack_pointer asm("r1");
+
extern unsigned long scom970_read(unsigned int address);
extern void scom970_write(unsigned int address, unsigned long value);
--
2.21.1
v3: Split out, and use current_stack_pointer not get_sp()
^ permalink raw reply related
* [PATCH v3 1/5] powerpc: Rename current_stack_pointer() to current_stack_frame()
From: Michael Ellerman @ 2020-02-20 11:51 UTC (permalink / raw)
To: linuxppc-dev
current_stack_pointer(), which was called __get_SP(), used to just
return the value in r1.
But that caused problems in some cases, so it was turned into a
function in commit bfe9a2cfe91a ("powerpc: Reimplement __get_SP() as a
function not a define").
Because it's a function in a separate compilation unit to all its
callers, it has the effect of causing a stack frame to be created, and
then returns the address of that frame. This is good in some cases
like those described in the above commit, but in other cases it's
overkill, we just need to know what stack page we're on.
On some other arches current_stack_pointer is just a register global
giving the stack pointer, and we'd like to do that too. So rename our
current_stack_pointer() to current_stack_frame() to make that
possible.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/include/asm/perf_event.h | 2 +-
arch/powerpc/include/asm/reg.h | 2 +-
arch/powerpc/kernel/irq.c | 4 ++--
arch/powerpc/kernel/misc.S | 4 ++--
arch/powerpc/kernel/process.c | 2 +-
arch/powerpc/kernel/stacktrace.c | 6 +++---
6 files changed, 10 insertions(+), 10 deletions(-)
v3: New.
diff --git a/arch/powerpc/include/asm/perf_event.h b/arch/powerpc/include/asm/perf_event.h
index 7426d7a90e1e..eed3954082fa 100644
--- a/arch/powerpc/include/asm/perf_event.h
+++ b/arch/powerpc/include/asm/perf_event.h
@@ -32,7 +32,7 @@
do { \
(regs)->result = 0; \
(regs)->nip = __ip; \
- (regs)->gpr[1] = current_stack_pointer(); \
+ (regs)->gpr[1] = current_stack_frame(); \
asm volatile("mfmsr %0" : "=r" ((regs)->msr)); \
} while (0)
diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index 1aa46dff0957..1b1ffdba6097 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -1448,7 +1448,7 @@ static inline void mtsrin(u32 val, u32 idx)
#define proc_trap() asm volatile("trap")
-extern unsigned long current_stack_pointer(void);
+extern unsigned long current_stack_frame(void);
extern unsigned long scom970_read(unsigned int address);
extern void scom970_write(unsigned int address, unsigned long value);
diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
index 5c9b11878555..02118c18434d 100644
--- a/arch/powerpc/kernel/irq.c
+++ b/arch/powerpc/kernel/irq.c
@@ -602,7 +602,7 @@ static inline void check_stack_overflow(void)
#ifdef CONFIG_DEBUG_STACKOVERFLOW
long sp;
- sp = current_stack_pointer() & (THREAD_SIZE-1);
+ sp = current_stack_frame() & (THREAD_SIZE-1);
/* check for stack overflow: is there less than 2KB free? */
if (unlikely(sp < 2048)) {
@@ -647,7 +647,7 @@ void do_IRQ(struct pt_regs *regs)
void *cursp, *irqsp, *sirqsp;
/* Switch to the irq stack to handle this */
- cursp = (void *)(current_stack_pointer() & ~(THREAD_SIZE - 1));
+ cursp = (void *)(current_stack_frame() & ~(THREAD_SIZE - 1));
irqsp = hardirq_ctx[raw_smp_processor_id()];
sirqsp = softirq_ctx[raw_smp_processor_id()];
diff --git a/arch/powerpc/kernel/misc.S b/arch/powerpc/kernel/misc.S
index 974f65f79a8e..65f9f731c229 100644
--- a/arch/powerpc/kernel/misc.S
+++ b/arch/powerpc/kernel/misc.S
@@ -110,7 +110,7 @@ _GLOBAL(longjmp)
li r3, 1
blr
-_GLOBAL(current_stack_pointer)
+_GLOBAL(current_stack_frame)
PPC_LL r3,0(r1)
blr
-EXPORT_SYMBOL(current_stack_pointer)
+EXPORT_SYMBOL(current_stack_frame)
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index e730b8e522b0..110db94cdf3c 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -2051,7 +2051,7 @@ void show_stack(struct task_struct *tsk, unsigned long *stack)
sp = (unsigned long) stack;
if (sp == 0) {
if (tsk == current)
- sp = current_stack_pointer();
+ sp = current_stack_frame();
else
sp = tsk->thread.ksp;
}
diff --git a/arch/powerpc/kernel/stacktrace.c b/arch/powerpc/kernel/stacktrace.c
index e2a46cfed5fd..c477b8585a29 100644
--- a/arch/powerpc/kernel/stacktrace.c
+++ b/arch/powerpc/kernel/stacktrace.c
@@ -57,7 +57,7 @@ void save_stack_trace(struct stack_trace *trace)
{
unsigned long sp;
- sp = current_stack_pointer();
+ sp = current_stack_frame();
save_context_stack(trace, sp, current, 1);
}
@@ -71,7 +71,7 @@ void save_stack_trace_tsk(struct task_struct *tsk, struct stack_trace *trace)
return;
if (tsk == current)
- sp = current_stack_pointer();
+ sp = current_stack_frame();
else
sp = tsk->thread.ksp;
@@ -131,7 +131,7 @@ static int __save_stack_trace_tsk_reliable(struct task_struct *tsk,
}
if (tsk == current)
- sp = current_stack_pointer();
+ sp = current_stack_frame();
else
sp = tsk->thread.ksp;
--
2.21.1
^ permalink raw reply related
* [PATCH] powerpc: Include .BTF section
From: Naveen N. Rao @ 2020-02-20 11:31 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
Selecting CONFIG_DEBUG_INFO_BTF results in the below warning from ld:
ld: warning: orphan section `.BTF' from `.btf.vmlinux.bin.o' being placed in section `.BTF'
Include .BTF section in vmlinux explicitly to fix the same.
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
arch/powerpc/kernel/vmlinux.lds.S | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arch/powerpc/kernel/vmlinux.lds.S b/arch/powerpc/kernel/vmlinux.lds.S
index b4c89a1acebb..a32d478a7f41 100644
--- a/arch/powerpc/kernel/vmlinux.lds.S
+++ b/arch/powerpc/kernel/vmlinux.lds.S
@@ -303,6 +303,12 @@ SECTIONS
*(.branch_lt)
}
+#ifdef CONFIG_DEBUG_INFO_BTF
+ .BTF : AT(ADDR(.BTF) - LOAD_OFFSET) {
+ *(.BTF)
+ }
+#endif
+
.opd : AT(ADDR(.opd) - LOAD_OFFSET) {
__start_opd = .;
KEEP(*(.opd))
--
2.24.1
^ permalink raw reply related
* [PATCH 5/8] powerpc/uapi: Introduce uapi header 'papr_scm_dsm.h' for papr_scm DSMs
From: Vaibhav Jain @ 2020-02-20 9:58 UTC (permalink / raw)
To: linuxppc-dev
Cc: Vaibhav Jain, Michael Ellerman, Oliver O'Halloran,
Alastair D'Silva, Aneesh Kumar K . V
In-Reply-To: <20200220095805.197229-1-vaibhav@linux.ibm.com>
Define and add a new uapi header for papr_scm describing device
specific methods (DSMs) and structs for libndctl. PAPR-SCM specific
implementation in libndctl will use these commands/structs to interact
with papr_scm kernel module. Currently only DSMs to retrieve health and
performance statistics information of a dimm are defined.
DSM Envelope
=============
The ioctl ND_CMD_CALL transfers data between user-space and kernel via
'envelopes'. An envelope consists of a header and user-defined payload
section. The primary structure describing this envelope is 'struct
nd_papr_scm_cmd_pkg' which expects a payload at the end of the envelop
pointed to by 'nd_papr_scm_cmd_pkg.payload_offset'. Currently two
payloads are defined 'struct nd_papr_scm_dimm_health_stat' and 'struct
nd_papr_scm_perf_stats'. These can be used to retrieve dimm-health and
performance stats respectively.
The header is defined as 'struct nd_cmd_pkg' which in return is
wrapped in a user defined struct called 'struct
nd_papr_scm_cmd_pkg'. This relationship is illustrated below:
64-Bytes 8-Bytes
+-------------+-------------------+------------------------------+
| nd_family | | |
| | | |
| nd_size_out | cmd_status | |
| | | |
| nd_size_in | payload_version | PAYLOAD |
| | | |
| nd_command | payload_offset | |
| | | | |
| nd_fw_size | +----------> | |
+-------------+-------------------+------------------------------+
\ nd_cmd_pkg / / /
\----------/ / /
\ nd_papr_scm_cmd_pkg / /
\--------------------------/ /
\ Envelope /
\-----------------------------------------------------/
Important fields to note in above illustration are:
* 'nd_command' : DSM command sent by libndctl
* 'nd_family' : Id for newly introduced DSM family NVDIMM_FAMILY_PAPR_SCM
* 'nd_fw_size' : Number of bytes that kernel wanted to copy to the
payload but may not have copied due to limited size of the envelope.
* 'nd_size_in/out' : Number of bytes that kernel needs to copy from
user-space (in) and copy-back to user-space (out).
* 'cmd_status' : Out variable indicating any error encountered while
servicing the DSM.
* 'payload_version': Version number associated with the payload.
* 'payload_offset': Offset of the payload from start of the envelope.
libnvdimm enforces a hard limit of 256 bytes on the envelope size,
which leaves around 184 bytes for the envelope payload (ignoring any
padding that the compiler may silently introduce).
Envelope Payload Layout
=======================
The layout of the DSM Payload is defined by various structs defined in
'papr_scm_dsm.h'. Definition of these structs are shared between
papr_scm and libndctl so that contents of payload can be
interpreted. This patch-set introduces two such structs namely
'nd_papr_scm_dimm_health_stat' and 'nd_papr_scm_perf_stats' that can
be used to exchange dimm health and performance stats between papr_scm
and libndctl.
During servicing of a DSM the papr_scm module will read input args
from the payload field by casting its contents to an appropriate
struct pointer based on the DSM command. Similarly the output of
servicing the DSM command will be copied to the payload field using
the same struct.
Payload Version
===============
Since the structs associated with each DSM can evolve over time adding
more data and the definitions of these structs known to papr_scm and
libndctl may differ, hence the version number is associated with each
iteration of the struct. This version number is exchanged between
papr_scm <-> libndctl via the 'payload_version' of the DSM envelope.
When libndctl sends an envelope to papr_scm it populates the
'payload_version' field with the version number of the struct it had
copied and/or expects in the payload area. The papr_scm module when
servicing the DSM envelop checks the 'payload_version', if required
changes it to a different version number that it knows about and then
use the DSM struct associated with the new version number to process
the DSM (i.e read the args and copy the results to the payload
area). Libndctl on receiving the envelop back from papr_scm again
checks the 'payload_version' field and based on it use the appropriate
version dsm struct to parse the results.
Above scheme of exchanging different versioned DSM struct between
libndctl and papr_scm should work until following two assumptions
hold:
Let T(X) = { set of attributes of DSM struct 'T' versioned X }
1. T(X) is a proper subset of T(Y) if X > Y.
i.e Each new version of DSM struct should retain existing struct
attributes.
2. If an entity (libndctl or papr_scm) supports a DSM struct T(X) then
it should also support T(1), T(2)...T(X - 1).
i.e When adding support for new version of a DSM struct, libndctl
and papr_scm should retain support of the existing DSM struct
version they support.
Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
---
arch/powerpc/include/uapi/asm/papr_scm_dsm.h | 143 +++++++++++++++++++
1 file changed, 143 insertions(+)
create mode 100644 arch/powerpc/include/uapi/asm/papr_scm_dsm.h
diff --git a/arch/powerpc/include/uapi/asm/papr_scm_dsm.h b/arch/powerpc/include/uapi/asm/papr_scm_dsm.h
new file mode 100644
index 000000000000..aacced453579
--- /dev/null
+++ b/arch/powerpc/include/uapi/asm/papr_scm_dsm.h
@@ -0,0 +1,143 @@
+/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
+/*
+ * PAPR SCM Device specific methods for libndctl and ndctl
+ *
+ * (C) Copyright IBM 2020
+ *
+ * Author: Vaibhav Jain <vaibhav at linux.ibm.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2, or (at your option)
+ * any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef _UAPI_ASM_POWERPC_PAPR_SCM_DSM_H_
+#define _UAPI_ASM_POWERPC_PAPR_SCM_DSM_H_
+
+#include <linux/types.h>
+
+#ifdef __KERNEL__
+#include <linux/ndctl.h>
+#else
+#include <ndctl.h>
+#endif
+
+/*
+ * Sub commands for ND_CMD_CALL. To prevent overlap from ND_CMD_*, values for
+ * these enums start at 0x10000. These values are then returned from
+ * cmd_to_func() making it easy to implement the switch-case block in
+ * papr_scm_ndctl()
+ */
+enum dsm_papr_scm {
+ DSM_PAPR_SCM_MIN = 0x10000,
+ DSM_PAPR_SCM_HEALTH,
+ DSM_PAPR_SCM_STATS,
+ DSM_PAPR_SCM_MAX,
+};
+
+enum dsm_papr_scm_dimm_health {
+ DSM_PAPR_SCM_DIMM_HEALTHY,
+ DSM_PAPR_SCM_DIMM_UNHEALTHY,
+ DSM_PAPR_SCM_DIMM_CRITICAL,
+ DSM_PAPR_SCM_DIMM_FATAL,
+};
+
+/* Papr-scm-header + payload expected with ND_CMD_CALL ioctl from libnvdimm */
+struct nd_papr_scm_cmd_pkg {
+ struct nd_cmd_pkg hdr; /* Package header containing sub-cmd */
+ int32_t cmd_status; /* Out: Sub-cmd status returned back */
+ uint16_t payload_offset; /* In: offset from start of struct */
+ uint16_t payload_version; /* In/Out: version of the payload */
+ uint8_t payload[]; /* In/Out: Sub-cmd data buffer */
+};
+
+/* Helpers to evaluate the size of PAPR_SCM envelope */
+/* Calculate the papr_scm-header size */
+#define ND_PAPR_SCM_ENVELOPE_CONTENT_HDR_SIZE \
+ (sizeof(struct nd_papr_scm_cmd_pkg) - sizeof(struct nd_cmd_pkg))
+/*
+ * Given a type calculate the envelope size
+ * (nd-header + papr_scm-header + payload)
+ */
+#define ND_PAPR_SCM_ENVELOPE_SIZE(_type_) \
+ (sizeof(_type_) + sizeof(struct nd_papr_scm_cmd_pkg))
+
+/* Given a type envelope-content size (papr_scm-header + payload) */
+#define ND_PAPR_SCM_ENVELOPE_CONTENT_SIZE(_type_) \
+ (sizeof(_type_) + ND_PAPR_SCM_ENVELOPE_CONTENT_HDR_SIZE)
+
+/*
+ * Struct exchanged between kernel & ndctl in for PAPR_DSM_PAPR_SMART_HEALTH
+ * Various bitflags indicate the health status of the dimm.
+ */
+struct nd_papr_scm_dimm_health_stat_v1 {
+ /* Dimm not armed. So contents wont persist */
+ bool dimm_unarmed;
+ /* Previous shutdown did not persist contents */
+ bool dimm_bad_shutdown;
+ /* Contents from previous shutdown werent restored */
+ bool dimm_bad_restore;
+ /* Contents of the dimm have been scrubbed */
+ bool dimm_scrubbed;
+ /* Contents of the dimm cant be modified until CEC reboot */
+ bool dimm_locked;
+ /* Contents of dimm are encrypted */
+ bool dimm_encrypted;
+
+ enum dsm_papr_scm_dimm_health dimm_health;
+};
+
+/*
+ * Typedef the current struct for dimm_health so that any application
+ * or kernel recompiled after introducing a new version autometically
+ * supports the new version.
+ */
+#define nd_papr_scm_dimm_health_stat nd_papr_scm_dimm_health_stat_v1
+
+/* Current version number for the dimm health struct */
+#define ND_PAPR_SCM_DIMM_HEALTH_VERSION 1
+
+/* Struct holding a single performance metric */
+struct nd_papr_scm_perf_stat {
+ u64 statistic_id;
+ u64 statistic_value;
+};
+
+/* Struct exchanged between kernel and ndctl reporting drc perf stats */
+struct nd_papr_scm_perf_stats_v1 {
+ /* Number of stats following */
+ u32 num_statistics;
+
+ /* zero or more performance matrics */
+ struct nd_papr_scm_perf_stat scm_statistics[];
+};
+
+/*
+ * Typedef the current struct for dimm_stats so that any application
+ * or kernel recompiled after introducing a new version autometically
+ * supports the new version.
+ */
+#define nd_papr_scm_perf_stats nd_papr_scm_perf_stats_v1
+#define ND_PAPR_SCM_DIMM_PERF_STATS_VERSION 1
+
+/* Convert a libnvdimm nd_cmd_pkg to papr_scm specific pkg */
+static struct nd_papr_scm_cmd_pkg *nd_to_papr_cmd_pkg(struct nd_cmd_pkg *cmd)
+{
+ return (struct nd_papr_scm_cmd_pkg *) cmd;
+}
+
+/* Return the payload pointer for a given pcmd */
+static void *papr_scm_pcmd_to_payload(struct nd_papr_scm_cmd_pkg *pcmd)
+{
+ if (pcmd->hdr.nd_size_in == 0 && pcmd->hdr.nd_size_out == 0)
+ return NULL;
+ else
+ return (void *)((u8 *) pcmd + pcmd->payload_offset);
+}
+#endif /* _UAPI_ASM_POWERPC_PAPR_SCM_DSM_H_ */
--
2.24.1
^ permalink raw reply related
* [PATCH 8/8] powerpc/papr_scm: Implement support for DSM_PAPR_SCM_HEALTH
From: Vaibhav Jain @ 2020-02-20 9:58 UTC (permalink / raw)
To: linuxppc-dev
Cc: Vaibhav Jain, Michael Ellerman, Oliver O'Halloran,
Alastair D'Silva, Aneesh Kumar K . V
In-Reply-To: <20200220095805.197229-1-vaibhav@linux.ibm.com>
The DSM 'DSM_PAPR_SCM_HEALTH' should return a 'struct
nd_papr_scm_dimm_health_stat' containing information in dimm health back
to user space in response to ND_CMD_CALL. We implement this DSM by
implementing a new function papr_scm_get_health() that queries the
DIMM health information and then copies these bitmaps to the package
payload whose layout is defined by 'struct papr_scm_ndctl_health'.
The patch also handle cases where in future versions of 'struct
papr_scm_ndctl_health' may want to return more health
information. Such payload envelops will contain appropriate version
information in 'struct nd_papr_scm_cmd_pkg.payload_version'. The patch
takes care of only returning the sub-data corresponding to the payload
version requested. Please see the comments in papr_scm_get_health()
for how this is done.
Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
---
arch/powerpc/platforms/pseries/papr_scm.c | 73 +++++++++++++++++++++++
1 file changed, 73 insertions(+)
diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
index 29f38246c59f..bf81acb0bf3f 100644
--- a/arch/powerpc/platforms/pseries/papr_scm.c
+++ b/arch/powerpc/platforms/pseries/papr_scm.c
@@ -415,6 +415,74 @@ static int cmd_to_func(struct nvdimm *nvdimm, unsigned int cmd, void *buf,
return pkg->hdr.nd_command;
}
+/*
+ * Fetch the DIMM health info and populate it in provided papr_scm package.
+ * Since the caller can request a different version of payload and each new
+ * version of struct nd_papr_scm_dimm_health_stat is a proper-subset of
+ * previous version hence we return a subset of the cached 'struct
+ * nd_papr_scm_dimm_health_stat' depending on the payload version requested.
+ */
+static int papr_scm_get_health(struct papr_scm_priv *p,
+ struct nd_papr_scm_cmd_pkg *pkg)
+{
+ int rc;
+ size_t copysize;
+ /* Map version to number of bytes to be copied to payload */
+ const size_t copysizes[] = {
+ [1] =
+ sizeof(struct nd_papr_scm_dimm_health_stat_v1),
+
+ /* This should always be preset */
+ [ND_PAPR_SCM_DIMM_HEALTH_VERSION] =
+ sizeof(struct nd_papr_scm_dimm_health_stat),
+ };
+
+ rc = drc_pmem_query_health(p);
+ if (rc)
+ goto out;
+ /*
+ * If the requested payload version is greater than one we know
+ * aboute, return the payload version we know about and let
+ * caller/userspace handle the mess.
+ */
+ if (pkg->payload_version > ND_PAPR_SCM_DIMM_HEALTH_VERSION)
+ pkg->payload_version = ND_PAPR_SCM_DIMM_HEALTH_VERSION;
+
+ copysize = copysizes[pkg->payload_version];
+ if (!copysize) {
+ dev_dbg(&p->pdev->dev, "%s Unsupported payload version=0x%x\n",
+ __func__, pkg->payload_version);
+ rc = -ENOSPC;
+ goto out;
+ }
+
+ if (pkg->hdr.nd_size_out < copysize) {
+ dev_dbg(&p->pdev->dev, "%s Payload not large enough\n",
+ __func__);
+ dev_dbg(&p->pdev->dev, "%s Expected %lu, available %u\n",
+ __func__, copysize, pkg->hdr.nd_size_out);
+ rc = -ENOSPC;
+ goto out;
+ }
+
+ dev_dbg(&p->pdev->dev, "%s Copying payload size=%lu version=0x%x\n",
+ __func__, copysize, pkg->payload_version);
+
+ /* Copy a subset of health struct based on copysize */
+ memcpy(papr_scm_pcmd_to_payload(pkg), &p->health, copysize);
+ pkg->hdr.nd_fw_size = copysize;
+
+out:
+ /*
+ * Put the error in out package and return success from function
+ * so that errors if any are propogated back to userspace.
+ */
+ pkg->cmd_status = rc;
+ dev_dbg(&p->pdev->dev, "%s completion code = %d\n", __func__, rc);
+
+ return 0;
+}
+
int papr_scm_ndctl(struct nvdimm_bus_descriptor *nd_desc, struct nvdimm *nvdimm,
unsigned int cmd, void *buf, unsigned int buf_len, int *cmd_rc)
{
@@ -460,6 +528,11 @@ int papr_scm_ndctl(struct nvdimm_bus_descriptor *nd_desc, struct nvdimm *nvdimm,
*cmd_rc = 0;
break;
+ case DSM_PAPR_SCM_HEALTH:
+ call_pkg = nd_to_papr_cmd_pkg(buf);
+ *cmd_rc = papr_scm_get_health(p, call_pkg);
+ break;
+
default:
dev_dbg(&p->pdev->dev, "Unknown command = %d\n", cmd_in);
*cmd_rc = -EINVAL;
--
2.24.1
^ permalink raw reply related
* [PATCH 7/8] powerpc/papr_scm: Re-implement 'papr_flags' using 'nd_papr_scm_dimm_health_stat'
From: Vaibhav Jain @ 2020-02-20 9:58 UTC (permalink / raw)
To: linuxppc-dev
Cc: Vaibhav Jain, Michael Ellerman, Oliver O'Halloran,
Alastair D'Silva, Aneesh Kumar K . V
In-Reply-To: <20200220095805.197229-1-vaibhav@linux.ibm.com>
Previous commit [1] introduced 'struct nd_papr_scm_dimm_health_stat' for
communicating health status of an nvdimm to libndctl. This struct
however can also be used to cache the nvdimm health information in
'struct papr_scm_priv' instead of two '__be64' values. Benefit of this
re-factoring will be apparent when support for libndctl being able to
request nvdimm health stats is implemented where we can simply memcpy
this struct over to the user-space provided payload envelope.
Hence this patch introduces a new member 'struct papr_scm_priv.health'
that caches the health information of a dimm. This member is populated
inside drc_pmem_query_health() which checks for the various bit flags
returned from H_SCM_HEALTH and sets them in this struct. We also
re-factor 'papr_flags' sysfs attribute show function papr_flags_show()
to use the flags in 'struct papr_scm_priv.health' to return
appropriate status strings pertaining to dimm health.
This patch shouldn't introduce any behavioral change.
Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
---
arch/powerpc/platforms/pseries/papr_scm.c | 61 ++++++++++++++++-------
1 file changed, 44 insertions(+), 17 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
index d5eea2f38fa6..29f38246c59f 100644
--- a/arch/powerpc/platforms/pseries/papr_scm.c
+++ b/arch/powerpc/platforms/pseries/papr_scm.c
@@ -47,8 +47,7 @@ struct papr_scm_priv {
struct mutex dimm_mutex;
/* Health information for the dimm */
- __be64 health_bitmap;
- __be64 health_bitmap_valid;
+ struct nd_papr_scm_dimm_health_stat health;
/* length of the stat buffer as expected by phyp */
size_t len_stat_buffer;
@@ -205,6 +204,7 @@ static int drc_pmem_query_health(struct papr_scm_priv *p)
{
unsigned long ret[PLPAR_HCALL_BUFSIZE];
int64_t rc;
+ __be64 health;
rc = plpar_hcall(H_SCM_HEALTH, ret, p->drc_index);
if (rc != H_SUCCESS) {
@@ -219,13 +219,41 @@ static int drc_pmem_query_health(struct papr_scm_priv *p)
return rc;
/* Store the retrieved health information in dimm platform data */
- p->health_bitmap = ret[0];
- p->health_bitmap_valid = ret[1];
+ health = ret[0] & ret[1];
dev_dbg(&p->pdev->dev,
"Queried dimm health info. Bitmap:0x%016llx Mask:0x%016llx\n",
- be64_to_cpu(p->health_bitmap),
- be64_to_cpu(p->health_bitmap_valid));
+ be64_to_cpu(ret[0]),
+ be64_to_cpu(ret[1]));
+
+ memset(&p->health, 0, sizeof(p->health));
+
+ /* Check for various masks in bitmap and set the buffer */
+ if (health & PAPR_SCM_DIMM_UNARMED_MASK)
+ p->health.dimm_unarmed = true;
+
+ if (health & PAPR_SCM_DIMM_BAD_SHUTDOWN_MASK)
+ p->health.dimm_bad_shutdown = true;
+
+ if (health & PAPR_SCM_DIMM_BAD_RESTORE_MASK)
+ p->health.dimm_bad_restore = true;
+
+ if (health & PAPR_SCM_DIMM_ENCRYPTED)
+ p->health.dimm_encrypted = true;
+
+ if (health & PAPR_SCM_DIMM_SCRUBBED_AND_LOCKED) {
+ p->health.dimm_locked = true;
+ p->health.dimm_scrubbed = true;
+ }
+
+ if (health & PAPR_SCM_DIMM_HEALTH_UNHEALTHY)
+ p->health.dimm_health = DSM_PAPR_SCM_DIMM_UNHEALTHY;
+
+ if (health & PAPR_SCM_DIMM_HEALTH_CRITICAL)
+ p->health.dimm_health = DSM_PAPR_SCM_DIMM_CRITICAL;
+
+ if (health & PAPR_SCM_DIMM_HEALTH_FATAL)
+ p->health.dimm_health = DSM_PAPR_SCM_DIMM_FATAL;
mutex_unlock(&p->dimm_mutex);
return 0;
@@ -513,7 +541,6 @@ static ssize_t papr_flags_show(struct device *dev,
{
struct nvdimm *dimm = to_nvdimm(dev);
struct papr_scm_priv *p = nvdimm_provider_data(dimm);
- __be64 health;
int rc;
rc = drc_pmem_query_health(p);
@@ -525,26 +552,26 @@ static ssize_t papr_flags_show(struct device *dev,
if (rc)
return rc;
- health = p->health_bitmap & p->health_bitmap_valid;
-
- /* Check for various masks in bitmap and set the buffer */
- if (health & PAPR_SCM_DIMM_UNARMED_MASK)
+ if (p->health.dimm_unarmed)
rc += sprintf(buf, "not_armed ");
- if (health & PAPR_SCM_DIMM_BAD_SHUTDOWN_MASK)
+ if (p->health.dimm_bad_shutdown)
rc += sprintf(buf + rc, "save_fail ");
- if (health & PAPR_SCM_DIMM_BAD_RESTORE_MASK)
+ if (p->health.dimm_bad_restore)
rc += sprintf(buf + rc, "restore_fail ");
- if (health & PAPR_SCM_DIMM_ENCRYPTED)
+ if (p->health.dimm_encrypted)
rc += sprintf(buf + rc, "encrypted ");
- if (health & PAPR_SCM_DIMM_SMART_EVENT_MASK)
+ if (p->health.dimm_health)
rc += sprintf(buf + rc, "smart_notify ");
- if (health & PAPR_SCM_DIMM_SCRUBBED_AND_LOCKED)
- rc += sprintf(buf + rc, "scrubbed locked ");
+ if (p->health.dimm_scrubbed)
+ rc += sprintf(buf + rc, "scrubbed ");
+
+ if (p->health.dimm_locked)
+ rc += sprintf(buf + rc, "locked ");
if (rc > 0)
rc += sprintf(buf + rc, "\n");
--
2.24.1
^ permalink raw reply related
* [PATCH 6/8] powerpc/papr_scm: Add support for handling PAPR DSM commands
From: Vaibhav Jain @ 2020-02-20 9:58 UTC (permalink / raw)
To: linuxppc-dev
Cc: Vaibhav Jain, Michael Ellerman, Oliver O'Halloran,
Alastair D'Silva, Aneesh Kumar K . V
In-Reply-To: <20200220095805.197229-1-vaibhav@linux.ibm.com>
Implement support for handling PAPR DSM commands in papr_scm
module. We advertise support for ND_CMD_CALL for the dimm command mask
and implement necessary scaffolding in the module to handle ND_CMD_CALL
ioctl and DSM commands that we receive.
The layout of the DSM commands as we expect from libnvdimm/libndctl is
defined in 'struct nd_pkg_papr_scm' which contains a 'struct
nd_cmd_pkg' as header. This header is used to communicate the DSM
command via 'nd_pkg_papr_scm->nd_command' and size of payload that
need to be sent/received for servicing the DSM.
The PAPR DSM commands are assigned indexes started from 0x10000 to
prevent them from overlapping ND_CMD_* values and also makes handling
dimm commands in papr_scm_ndctl() easier via a simplified switch-case
block. For this a new function cmd_to_func() is implemented that reads
the args to papr_scm_ndctl() , performs sanity tests on them and
converts them to PAPR DSM commands which can then be handled via the
switch-case block.
Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
---
Change-log:
* Added a 'reserved' field in 'struct nd_pkg_papr_scm' to ensure
'payload' falls on a 8-Byte aligned boundary.
---
arch/powerpc/platforms/pseries/papr_scm.c | 87 +++++++++++++++++++++--
1 file changed, 80 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
index 28143a681aa2..d5eea2f38fa6 100644
--- a/arch/powerpc/platforms/pseries/papr_scm.c
+++ b/arch/powerpc/platforms/pseries/papr_scm.c
@@ -15,13 +15,15 @@
#include <asm/plpar_wrappers.h>
#include <asm/papr_scm.h>
+#include <asm/papr_scm_dsm.h>
#define BIND_ANY_ADDR (~0ul)
#define PAPR_SCM_DIMM_CMD_MASK \
((1ul << ND_CMD_GET_CONFIG_SIZE) | \
(1ul << ND_CMD_GET_CONFIG_DATA) | \
- (1ul << ND_CMD_SET_CONFIG_DATA))
+ (1ul << ND_CMD_SET_CONFIG_DATA) | \
+ (1ul << ND_CMD_CALL))
struct papr_scm_priv {
struct platform_device *pdev;
@@ -330,19 +332,82 @@ static int papr_scm_meta_set(struct papr_scm_priv *p,
return 0;
}
+/*
+ * Validate the input to dimm-control function and return papr_scm specific
+ * commands. This does sanity validation to ND_CMD_CALL sub-command packages.
+ */
+static int cmd_to_func(struct nvdimm *nvdimm, unsigned int cmd, void *buf,
+ unsigned int buf_len)
+{
+ unsigned long cmd_mask = PAPR_SCM_DIMM_CMD_MASK;
+ struct nd_papr_scm_cmd_pkg *pkg = nd_to_papr_cmd_pkg(buf);
+
+ /* Only dimm-specific calls are supported atm */
+ if (!nvdimm)
+ return -EINVAL;
+
+ if (!test_bit(cmd, &cmd_mask)) {
+ pr_debug("%s: Unsupported cmd=%u\n", __func__, cmd);
+ return -EINVAL;
+ } else if (cmd != ND_CMD_CALL) {
+ return cmd;
+ }
+
+ /* cmd == ND_CMD_CALL so verify the envelop package */
+
+ if (!buf || buf_len < sizeof(struct nd_papr_scm_cmd_pkg)) {
+ pr_debug("%s: Invalid pkg size=%u\n", __func__, buf_len);
+ return -EINVAL;
+ }
+
+ if (pkg->hdr.nd_family != NVDIMM_FAMILY_PAPR_SCM) {
+ pr_debug("%s: Invalid pkg family=0x%llx\n", __func__,
+ pkg->hdr.nd_family);
+ return -EINVAL;
+
+ }
+
+ if (pkg->hdr.nd_command <= DSM_PAPR_SCM_MIN ||
+ pkg->hdr.nd_command >= DSM_PAPR_SCM_MAX) {
+
+ /* for unknown subcommands return ND_CMD_CALL */
+ pr_debug("%s: Unknown sub-command=0x%llx\n", __func__,
+ pkg->hdr.nd_command);
+ return ND_CMD_CALL;
+ }
+
+ /* We except a payload with all DSM commands */
+ if (papr_scm_pcmd_to_payload(pkg) == NULL) {
+ pr_debug("%s: Empty patload for sub-command=0x%llx\n", __func__,
+ pkg->hdr.nd_command);
+ return -EINVAL;
+ }
+
+ /* Return the DSM_PAPR_SCM_* command */
+ return pkg->hdr.nd_command;
+}
+
int papr_scm_ndctl(struct nvdimm_bus_descriptor *nd_desc, struct nvdimm *nvdimm,
unsigned int cmd, void *buf, unsigned int buf_len, int *cmd_rc)
{
struct nd_cmd_get_config_size *get_size_hdr;
struct papr_scm_priv *p;
+ struct nd_papr_scm_cmd_pkg *call_pkg = NULL;
+ int cmd_in, rc;
- /* Only dimm-specific calls are supported atm */
- if (!nvdimm)
- return -EINVAL;
+ /* Use a local variable in case cmd_rc pointer is NULL */
+ if (cmd_rc == NULL)
+ cmd_rc = &rc;
+
+ cmd_in = cmd_to_func(nvdimm, cmd, buf, buf_len);
+ if (cmd_in < 0) {
+ pr_debug("%s: Invalid cmd=%u. Err=%d\n", __func__, cmd, cmd_in);
+ return cmd_in;
+ }
p = nvdimm_provider_data(nvdimm);
- switch (cmd) {
+ switch (cmd_in) {
case ND_CMD_GET_CONFIG_SIZE:
get_size_hdr = buf;
@@ -360,13 +425,21 @@ int papr_scm_ndctl(struct nvdimm_bus_descriptor *nd_desc, struct nvdimm *nvdimm,
*cmd_rc = papr_scm_meta_set(p, buf);
break;
+ case ND_CMD_CALL:
+ /* This happens if subcommand package sanity fails */
+ call_pkg = nd_to_papr_cmd_pkg(buf);
+ call_pkg->cmd_status = -ENOENT;
+ *cmd_rc = 0;
+ break;
+
default:
- return -EINVAL;
+ dev_dbg(&p->pdev->dev, "Unknown command = %d\n", cmd_in);
+ *cmd_rc = -EINVAL;
}
dev_dbg(&p->pdev->dev, "returned with cmd_rc = %d\n", *cmd_rc);
- return 0;
+ return *cmd_rc;
}
static inline int papr_scm_node(int node)
--
2.24.1
^ permalink raw reply related
* [PATCH 4/8] UAPI: ndctl: Introduce NVDIMM_FAMILY_PAPR_SCM as a new NVDIMM DSM family
From: Vaibhav Jain @ 2020-02-20 9:58 UTC (permalink / raw)
To: linuxppc-dev, linux-nvdimm
Cc: Alastair D'Silva, Aneesh Kumar K . V, Oliver O'Halloran,
Vishal Verma, Vaibhav Jain, Michael Ellerman, Dan Williams
In-Reply-To: <20200220095805.197229-1-vaibhav@linux.ibm.com>
Add PAPR-scm family of DSM command-set to the white list of NVDIMM
command sets.
Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
---
include/uapi/linux/ndctl.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/uapi/linux/ndctl.h b/include/uapi/linux/ndctl.h
index de5d90212409..99fb60600ef8 100644
--- a/include/uapi/linux/ndctl.h
+++ b/include/uapi/linux/ndctl.h
@@ -244,6 +244,7 @@ struct nd_cmd_pkg {
#define NVDIMM_FAMILY_HPE2 2
#define NVDIMM_FAMILY_MSFT 3
#define NVDIMM_FAMILY_HYPERV 4
+#define NVDIMM_FAMILY_PAPR_SCM 5
#define ND_IOCTL_CALL _IOWR(ND_IOCTL, ND_CMD_CALL,\
struct nd_cmd_pkg)
--
2.24.1
^ permalink raw reply related
* [PATCH 3/8] powerpc/papr_scm: Fetch dimm performance stats from PHYP
From: Vaibhav Jain @ 2020-02-20 9:58 UTC (permalink / raw)
To: linuxppc-dev
Cc: Vaibhav Jain, Michael Ellerman, Oliver O'Halloran,
Alastair D'Silva, Aneesh Kumar K . V
In-Reply-To: <20200220095805.197229-1-vaibhav@linux.ibm.com>
Implement support for fetching dimm performance metrics via
H_SCM_PERFORMANCE_HEALTH hcall as documented in Ref[1]. The hcall
returns a structure as described in Ref[1] and defined as newly
introduced 'struct papr_scm_perf_stats'. The struct has a header
followed by key-value pairs of performance attributes. The 'key' part
is a 8-byte char array naming the attribute encoded as a __be64
integer. This makes the output buffer format for the hcall self
describing and can be easily interpreted.
This patch implements functionality to fetch these performance stats
and reporting them via a nvdimm sysfs attribute named
'papr_perf_stats'.
A new function drc_pmem_query_stats() is implemented that issues hcall
H_SCM_PERFORMANCE_HEALTH ,requesting PHYP to store performance stats
in pre-allocated 'struct papr_scm_perf_stats' buffer. During nvdimm
initialization in papr_scm_nvdimm_init() this function is called with
an empty buffer to know the max buffer size needed for issuing the
H_SCM_PERFORMANCE_HEALTH hcall. The buffer size retrieved is stored in
newly introduced 'struct papc_scm_priv.len_stat_buffer' for later
retrival.
[1]: commit 58b278f568f0 ("powerpc: Provide initial documentation for
PAPR hcalls")
Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
---
arch/powerpc/platforms/pseries/papr_scm.c | 107 ++++++++++++++++++++++
1 file changed, 107 insertions(+)
diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
index aaf2e4ab1f75..28143a681aa2 100644
--- a/arch/powerpc/platforms/pseries/papr_scm.c
+++ b/arch/powerpc/platforms/pseries/papr_scm.c
@@ -47,6 +47,9 @@ struct papr_scm_priv {
/* Health information for the dimm */
__be64 health_bitmap;
__be64 health_bitmap_valid;
+
+ /* length of the stat buffer as expected by phyp */
+ size_t len_stat_buffer;
};
static int drc_pmem_bind(struct papr_scm_priv *p)
@@ -152,6 +155,50 @@ static int drc_pmem_query_n_bind(struct papr_scm_priv *p)
return drc_pmem_bind(p);
}
+static int drc_pmem_query_stats(struct papr_scm_priv *p,
+ struct papr_scm_perf_stats *stats,
+ size_t size, uint64_t *out)
+{
+ unsigned long ret[PLPAR_HCALL_BUFSIZE];
+ int64_t rc;
+
+ /* In case of no out buffer ignore the size */
+ if (!stats)
+ size = 0;
+
+ /*
+ * Do the HCALL asking PHYP for info and if R4 was requested
+ * return its value in 'out' variable.
+ */
+ rc = plpar_hcall(H_SCM_PERFORMANCE_STATS, ret, p->drc_index,
+ __pa(stats), size);
+ if (out)
+ *out = be64_to_cpu(ret[0]);
+
+ switch (rc) {
+ case H_SUCCESS:
+ /* Handle the case where size of stat buffer was requested */
+ if (size != 0)
+ dev_dbg(&p->pdev->dev,
+ "Performance stats returned %d stats\n",
+ be32_to_cpu(stats->num_statistics));
+ else
+ dev_dbg(&p->pdev->dev,
+ "Performance stats size %lld\n",
+ be64_to_cpu(ret[0]));
+ return 0;
+ case H_PARTIAL:
+ dev_err(&p->pdev->dev,
+ "Unknown performance stats, Err:0x%016llX\n",
+ be64_to_cpu(ret[0]));
+ return -ENOENT;
+ default:
+ dev_err(&p->pdev->dev,
+ "Failed to query performance stats, Err:%lld\n", rc);
+ return -ENXIO;
+ }
+}
+
static int drc_pmem_query_health(struct papr_scm_priv *p)
{
unsigned long ret[PLPAR_HCALL_BUFSIZE];
@@ -341,6 +388,53 @@ static inline int papr_scm_node(int node)
return min_node;
}
+static ssize_t papr_perf_stats_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct nvdimm *dimm = to_nvdimm(dev);
+ struct papr_scm_priv *p = nvdimm_provider_data(dimm);
+ struct papr_scm_perf_stats *retbuffer;
+ struct papr_scm_perf_stat *stat;
+ uint64_t statid, val;
+ int rc, i;
+
+ if (!p->len_stat_buffer)
+ return -ENOENT;
+
+ /* Return buffer for phyp where stats are written */
+ retbuffer = kzalloc(p->len_stat_buffer, GFP_KERNEL);
+ if (!retbuffer)
+ return -ENOMEM;
+
+ /* Setup the buffer */
+ memcpy(retbuffer->eye_catcher, PAPR_SCM_PERF_STATS_EYECATCHER,
+ sizeof(retbuffer->eye_catcher));
+ retbuffer->stats_version = cpu_to_be32(0x1);
+ retbuffer->num_statistics = 0;
+
+ rc = drc_pmem_query_stats(p, retbuffer, p->len_stat_buffer, NULL);
+ if (rc)
+ goto out;
+
+ /*
+ * Go through the returned output buffer and print stats and values.
+ * Since statistic_id is essentially a char string of 8 bytes encoded
+ * as a __be64, simply use the string format specifier to print it.
+ */
+ for (i = 0, stat = retbuffer->scm_statistics;
+ i < be32_to_cpu(retbuffer->num_statistics); ++i, ++stat) {
+ statid = be64_to_cpu(stat->statistic_id);
+ val = be64_to_cpu(stat->statistic_value);
+ rc += sprintf(buf + rc, "%.8s => 0x%016llX\n",
+ (char *) &(statid), val);
+ }
+out:
+ kfree(retbuffer);
+ return rc;
+
+}
+DEVICE_ATTR_RO(papr_perf_stats);
+
static ssize_t papr_flags_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -390,6 +484,7 @@ DEVICE_ATTR_RO(papr_flags);
/* papr_scm specific dimm attributes */
static struct attribute *papr_scm_nd_attributes[] = {
&dev_attr_papr_flags.attr,
+ &dev_attr_papr_perf_stats.attr,
NULL,
};
@@ -409,6 +504,7 @@ static int papr_scm_nvdimm_init(struct papr_scm_priv *p)
struct nd_region_desc ndr_desc;
unsigned long dimm_flags;
int target_nid, online_nid;
+ uint64_t stat_size;
p->bus_desc.ndctl = papr_scm_ndctl;
p->bus_desc.module = THIS_MODULE;
@@ -470,6 +566,17 @@ static int papr_scm_nvdimm_init(struct papr_scm_priv *p)
dev_info(dev, "Region registered with target node %d and online node %d",
target_nid, online_nid);
+ /* Try retriving the stat buffer and see if its supported */
+ if (!drc_pmem_query_stats(p, NULL, 0, &stat_size)) {
+ p->len_stat_buffer = (size_t)stat_size;
+ dev_dbg(&p->pdev->dev, "Max dimm perf stats size %ld bytes\n",
+ p->len_stat_buffer);
+ } else {
+ p->len_stat_buffer = 0;
+ dev_dbg(&p->pdev->dev, "Unable to retrieve performace stats\n");
+ dev_info(&p->pdev->dev, "Limited dimm info available\n");
+ }
+
return 0;
err: nvdimm_bus_unregister(p->bus);
--
2.24.1
^ permalink raw reply related
* [PATCH 2/8] powerpc/papr_scm: Provide support for fetching dimm health information
From: Vaibhav Jain @ 2020-02-20 9:57 UTC (permalink / raw)
To: linuxppc-dev
Cc: Vaibhav Jain, Michael Ellerman, Oliver O'Halloran,
Alastair D'Silva, Aneesh Kumar K . V
In-Reply-To: <20200220095805.197229-1-vaibhav@linux.ibm.com>
Implement support for fetching dimm health information via
H_SCM_HEALTH hcall as documented in Ref[1]. The hcall returns a pair of
64-bit big-endian integers which are then stored in 'struct
papr_scm_priv' and subsequently exposed to userspace via dimm
attribute 'papr_flags'.
'papr_flags' sysfs attribute reports space separated string flags
indicating various health state an nvdimm can be. These are:
* "not_armed" : Indicating that nvdimm contents wont survive a power
cycle.
* "save_fail" : Indicating that nvdimm contents couldn't be flushed
during last shutdown event.
* "restore_fail": Indicating that nvdimm contents couldn't be restored
during dimm initialization.
* "encrypted" : Dimm contents are encrypted.
* "smart_notify": There is health event for the nvdimm.
* "scrubbed" : Indicating that contents of the nvdimm have been
scrubbed.
* "locked" : Indicating that nvdimm contents cant be modified
until next power cycle.
[1]: commit 58b278f568f0 ("powerpc: Provide initial documentation for
PAPR hcalls")
Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
---
arch/powerpc/platforms/pseries/papr_scm.c | 105 +++++++++++++++++++++-
1 file changed, 103 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
index 0b4467e378e5..aaf2e4ab1f75 100644
--- a/arch/powerpc/platforms/pseries/papr_scm.c
+++ b/arch/powerpc/platforms/pseries/papr_scm.c
@@ -14,6 +14,7 @@
#include <linux/delay.h>
#include <asm/plpar_wrappers.h>
+#include <asm/papr_scm.h>
#define BIND_ANY_ADDR (~0ul)
@@ -39,6 +40,13 @@ struct papr_scm_priv {
struct resource res;
struct nd_region *region;
struct nd_interleave_set nd_set;
+
+ /* Protect dimm data from concurrent access */
+ struct mutex dimm_mutex;
+
+ /* Health information for the dimm */
+ __be64 health_bitmap;
+ __be64 health_bitmap_valid;
};
static int drc_pmem_bind(struct papr_scm_priv *p)
@@ -144,6 +152,35 @@ static int drc_pmem_query_n_bind(struct papr_scm_priv *p)
return drc_pmem_bind(p);
}
+static int drc_pmem_query_health(struct papr_scm_priv *p)
+{
+ unsigned long ret[PLPAR_HCALL_BUFSIZE];
+ int64_t rc;
+
+ rc = plpar_hcall(H_SCM_HEALTH, ret, p->drc_index);
+ if (rc != H_SUCCESS) {
+ dev_err(&p->pdev->dev,
+ "Failed to query health information, Err:%lld\n", rc);
+ return -ENXIO;
+ }
+
+ /* Protect modifications to papr_scm_priv with the mutex */
+ rc = mutex_lock_interruptible(&p->dimm_mutex);
+ if (rc)
+ return rc;
+
+ /* Store the retrieved health information in dimm platform data */
+ p->health_bitmap = ret[0];
+ p->health_bitmap_valid = ret[1];
+
+ dev_dbg(&p->pdev->dev,
+ "Queried dimm health info. Bitmap:0x%016llx Mask:0x%016llx\n",
+ be64_to_cpu(p->health_bitmap),
+ be64_to_cpu(p->health_bitmap_valid));
+
+ mutex_unlock(&p->dimm_mutex);
+ return 0;
+}
static int papr_scm_meta_get(struct papr_scm_priv *p,
struct nd_cmd_get_config_data_hdr *hdr)
@@ -304,6 +341,67 @@ static inline int papr_scm_node(int node)
return min_node;
}
+static ssize_t papr_flags_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct nvdimm *dimm = to_nvdimm(dev);
+ struct papr_scm_priv *p = nvdimm_provider_data(dimm);
+ __be64 health;
+ int rc;
+
+ rc = drc_pmem_query_health(p);
+ if (rc)
+ return rc;
+
+ /* Protect against modifications to papr_scm_priv with the mutex */
+ rc = mutex_lock_interruptible(&p->dimm_mutex);
+ if (rc)
+ return rc;
+
+ health = p->health_bitmap & p->health_bitmap_valid;
+
+ /* Check for various masks in bitmap and set the buffer */
+ if (health & PAPR_SCM_DIMM_UNARMED_MASK)
+ rc += sprintf(buf, "not_armed ");
+
+ if (health & PAPR_SCM_DIMM_BAD_SHUTDOWN_MASK)
+ rc += sprintf(buf + rc, "save_fail ");
+
+ if (health & PAPR_SCM_DIMM_BAD_RESTORE_MASK)
+ rc += sprintf(buf + rc, "restore_fail ");
+
+ if (health & PAPR_SCM_DIMM_ENCRYPTED)
+ rc += sprintf(buf + rc, "encrypted ");
+
+ if (health & PAPR_SCM_DIMM_SMART_EVENT_MASK)
+ rc += sprintf(buf + rc, "smart_notify ");
+
+ if (health & PAPR_SCM_DIMM_SCRUBBED_AND_LOCKED)
+ rc += sprintf(buf + rc, "scrubbed locked ");
+
+ if (rc > 0)
+ rc += sprintf(buf + rc, "\n");
+
+ mutex_unlock(&p->dimm_mutex);
+ return rc;
+}
+DEVICE_ATTR_RO(papr_flags);
+
+/* papr_scm specific dimm attributes */
+static struct attribute *papr_scm_nd_attributes[] = {
+ &dev_attr_papr_flags.attr,
+ NULL,
+};
+
+static struct attribute_group papr_scm_nd_attribute_group = {
+ .attrs = papr_scm_nd_attributes,
+};
+
+static const struct attribute_group *papr_scm_dimm_attr_groups[] = {
+ &papr_scm_nd_attribute_group,
+ NULL,
+};
+
static int papr_scm_nvdimm_init(struct papr_scm_priv *p)
{
struct device *dev = &p->pdev->dev;
@@ -330,8 +428,8 @@ static int papr_scm_nvdimm_init(struct papr_scm_priv *p)
dimm_flags = 0;
set_bit(NDD_ALIASING, &dimm_flags);
- p->nvdimm = nvdimm_create(p->bus, p, NULL, dimm_flags,
- PAPR_SCM_DIMM_CMD_MASK, 0, NULL);
+ p->nvdimm = nvdimm_create(p->bus, p, papr_scm_dimm_attr_groups,
+ dimm_flags, PAPR_SCM_DIMM_CMD_MASK, 0, NULL);
if (!p->nvdimm) {
dev_err(dev, "Error creating DIMM object for %pOF\n", p->dn);
goto err;
@@ -415,6 +513,9 @@ static int papr_scm_probe(struct platform_device *pdev)
if (!p)
return -ENOMEM;
+ /* Initialize the dimm mutex */
+ mutex_init(&p->dimm_mutex);
+
/* optional DT properties */
of_property_read_u32(dn, "ibm,metadata-size", &metadata_size);
--
2.24.1
^ permalink raw reply related
* [PATCH 1/8] powerpc: Add asm header 'papr_scm.h' describing the papr-scm interface
From: Vaibhav Jain @ 2020-02-20 9:57 UTC (permalink / raw)
To: linuxppc-dev
Cc: Vaibhav Jain, Michael Ellerman, Oliver O'Halloran,
Alastair D'Silva, Aneesh Kumar K . V
In-Reply-To: <20200220095805.197229-1-vaibhav@linux.ibm.com>
Add a new powerpc specific asm header named 'papr-scm.h' that descibes
the interface between PHYP and guest kernel running as an LPAR.
The HCALLs specific to managing SCM are descibed in Ref[1]. The asm
header introduced by this patch however describes the data structures
exchanged between PHYP and kernel during those HCALLs.
Future patches will use these structures to provide support for
retriving nvdimm health and performance stats in papr_scm kernel
module.
[1]: commit 58b278f568f0 ("powerpc: Provide initial documentation for
PAPR hcalls")
Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
---
arch/powerpc/include/asm/papr_scm.h | 68 +++++++++++++++++++++++++++++
1 file changed, 68 insertions(+)
create mode 100644 arch/powerpc/include/asm/papr_scm.h
diff --git a/arch/powerpc/include/asm/papr_scm.h b/arch/powerpc/include/asm/papr_scm.h
new file mode 100644
index 000000000000..d893621063f3
--- /dev/null
+++ b/arch/powerpc/include/asm/papr_scm.h
@@ -0,0 +1,68 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Structures and defines needed to manage nvdimms for spapr guests.
+ */
+#ifndef _ASM_POWERPC_PAPR_SCM_H_
+#define _ASM_POWERPC_PAPR_SCM_H_
+
+#include <linux/types.h>
+#include <asm/bitsperlong.h>
+#include <linux/stringify.h>
+
+/* DIMM health bitmap bitmap indicators */
+/* SCM device is unable to persist memory contents */
+#define PAPR_SCM_DIMM_UNARMED PPC_BIT(0)
+/* SCM device failed to persist memory contents */
+#define PAPR_SCM_DIMM_SHUTDOWN_DIRTY PPC_BIT(1)
+/* SCM device contents are persisted from previous IPL */
+#define PAPR_SCM_DIMM_SHUTDOWN_CLEAN PPC_BIT(2)
+/* SCM device contents are not persisted from previous IPL */
+#define PAPR_SCM_DIMM_EMPTY PPC_BIT(3)
+/* SCM device memory life remaining is critically low */
+#define PAPR_SCM_DIMM_HEALTH_CRITICAL PPC_BIT(4)
+/* SCM device will be garded off next IPL due to failure */
+#define PAPR_SCM_DIMM_HEALTH_FATAL PPC_BIT(5)
+/* SCM contents cannot persist due to current platform health status */
+#define PAPR_SCM_DIMM_HEALTH_UNHEALTHY PPC_BIT(6)
+/* SCM device is unable to persist memory contents in certain conditions */
+#define PAPR_SCM_DIMM_HEALTH_NON_CRITICAL PPC_BIT(7)
+/* SCM device is encrypted */
+#define PAPR_SCM_DIMM_ENCRYPTED PPC_BIT(8)
+/* SCM device has been scrubbed and locked */
+#define PAPR_SCM_DIMM_SCRUBBED_AND_LOCKED PPC_BIT(9)
+
+/* Bits status indicators for health bitmap indicating unarmed dimm */
+#define PAPR_SCM_DIMM_UNARMED_MASK (PAPR_SCM_DIMM_UNARMED | \
+ PAPR_SCM_DIMM_HEALTH_UNHEALTHY | \
+ PAPR_SCM_DIMM_HEALTH_NON_CRITICAL)
+
+/* Bits status indicators for health bitmap indicating unflushed dimm */
+#define PAPR_SCM_DIMM_BAD_SHUTDOWN_MASK (PAPR_SCM_DIMM_SHUTDOWN_DIRTY)
+
+/* Bits status indicators for health bitmap indicating unrestored dimm */
+#define PAPR_SCM_DIMM_BAD_RESTORE_MASK (PAPR_SCM_DIMM_EMPTY)
+
+/* Bit status indicators for smart event notification */
+#define PAPR_SCM_DIMM_SMART_EVENT_MASK (PAPR_SCM_DIMM_HEALTH_CRITICAL | \
+ PAPR_SCM_DIMM_HEALTH_FATAL | \
+ PAPR_SCM_DIMM_HEALTH_UNHEALTHY | \
+ PAPR_SCM_DIMM_HEALTH_NON_CRITICAL)
+
+#define PAPR_SCM_PERF_STATS_EYECATCHER __stringify(SCMSTATS)
+
+/* Struct holding a single performance metric */
+struct papr_scm_perf_stat {
+ __be64 statistic_id;
+ __be64 statistic_value;
+};
+
+/* Struct exchanged between kernel and ndctl reporting drc perf stats */
+struct papr_scm_perf_stats {
+ uint8_t eye_catcher[8];
+ __be32 stats_version; /* Should be 0x01 */
+ __be32 num_statistics; /* Number of stats following */
+ /* zero or more performance matrics */
+ struct papr_scm_perf_stat scm_statistics[];
+} __packed;
+
+#endif
--
2.24.1
^ permalink raw reply related
* [PATCH 0/7] powerpc/papr_scm: Add support for reporting nvdimm health
From: Vaibhav Jain @ 2020-02-20 9:57 UTC (permalink / raw)
To: linuxppc-dev, linux-nvdimm
Cc: Alastair D'Silva, Aneesh Kumar K . V, Oliver O'Halloran,
Vishal Verma, Vaibhav Jain, Michael Ellerman, Dan Williams
The PAPR standard[1][3] provides suitable mechanisms to query the health and
performance stats of an NVDIMM via various hcalls as described in Ref[2]. Until
now these stats were never available nor exposed to the user-space tools like
'ndctl'. This is partly due to PAPR platform not having support for ACPI and
NFIT. Hence 'ndctl' is unable to query and report the dimm health status and a
user had no way to determine the current health status of a NDVIMM.
To overcome this limitation this patch-set updates papr_scm kernel module to
query and fetch nvdimm health and performance stats using hcalls described in
Ref[2]. This health and performance stats are then exposed to userspace via
syfs and Dimm-Specific-Methods(DSM) issued by libndctl.
These changes coupled with proposed ndtcl changes located at Ref[4] should
provide a way for the user to retrieve NVDIMM health status using ndtcl. Below
is a sample output using proposed kernel + ndctl for PAPR NVDIMM in an
emulation environment:
# ndctl list -DH
[
{
"dev":"nmem0",
"health":{
"health_state":"fatal",
"shutdown_state":"dirty"
}
}
]
PAPR Dimm-Specific-Methods(DSM)
================================
As the name suggests DSMs are used by vendor specific code in libndctl to
execute certain operations or fetch certain information for NVDIMMS. DSMs
can be sent to papr_scm module via libndctl (userspace) and libnvdimm(kernel)
using the ND_CMD_CALL ioctl which can be handled in the dimm control function
papr_scm_ndctl(). For PAPR this patchset proposes two DSMs defined in the newly
introduced uapi header named 'papr_scm_dsm.h', that directly map to hcalls
provided by PHYP to query NVDIMM health and stats. These DSMs are:
* DSM_PAPR_SCM_HEALTH: Which map to hcall H_SCM_HEALTH and returns dimm health.
* DSM_PAPR_SCM_STATS: Which map to hcall H_SCM_PERFORMANCE_STATS and returns
dimm performance stats.
P.S: The current patch-set only provides an implementation for servicing
DSM_PAPR_SCM_HEALTH and a future patch will add support for DSM_PAPR_SCM_STATS.
The ioctl ND_CMD_CALL can also transfer data between user-space and kernel via
'envelopes'. The envelop is part of a 'struct nd_cmd_pkg' which in return is
wrapped in a user defined struct which in our case is a newly introduced
'struct nd_papr_scm_cmd_pkg'. Apart from 'envelope header' this struct holds
'payload', 'payload offset', 'payload version' and 'command status'.
The 'payload' field of the envelop holds a struct depending on the DSM method
used and should be one of the structs defined in newly introduced uapi header
'papr_scm_dsm.h'. This makes it possible for libndctl/kernel to share the same
definitions for these DSM structs.
Earlier Work
============
An earlier RFC patch set titled "powerpc/papr_scm: Implement support for
reporting DIMM health and stats" [5] was proposed which tried to achieve
same functionality albeit with a different approach i.e papr_scm module
acted as a pass-through for the DSM calls from libndctl.
This patch-set however departs from that design by decoupling the
libndctl <--> papr_scm and papr_scm <--> phyp interfaces. This provides
more flexibility compared to earlier approach were these two interfaces were
coupled with each other.
Structure of the patch-set
==========================
The initial 3 patches of the patch-set add functionality of issuing necessary
HCALLs to PHYP to retrieve the dimm health/performance stats information and
exposing them to user-space via sysfs attributes.
Subsequent patches deal with defining and implementing support for
NVDIMM_FAMILY_PAPR_SCM DSM command family and implementing the payload
versioning scheme as mentioned above.
References:
[1]: "Power Architecture Platform Reference"
https://en.wikipedia.org/wiki/Power_Architecture_Platform_Reference
[2]: "[DOC,v2] powerpc: Provide initial documentation for PAPR hcalls"
https://patchwork.ozlabs.org/patch/1154292/
[3]: "Linux on Power Architecture Platform Reference"
https://members.openpowerfoundation.org/document/dl/469
[4]: https://github.com/vaibhav92/ndctl/tree/papr_scm_health_v1
[5]: https://lore.kernel.org/linuxppc-dev/20200129152844.71286-1-vaibhav@linux.ibm.com/
Vaibhav Jain (8):
powerpc: Add asm header 'papr_scm.h' describing the papr-scm interface
powerpc/papr_scm: Provide support for fetching dimm health information
powerpc/papr_scm: Fetch dimm performance stats from PHYP
UAPI: ndctl: Introduce NVDIMM_FAMILY_PAPR_SCM as a new NVDIMM DSM
family
powerpc/uapi: Introduce uapi header 'papr_scm_dsm.h' for papr_scm DSMs
powerpc/papr_scm: Add support for handling PAPR DSM commands
powerpc/papr_scm: Re-implement 'papr_flags' using
'nd_papr_scm_dimm_health_stat'
powerpc/papr_scm: Implement support for DSM_PAPR_SCM_HEALTH
arch/powerpc/include/asm/papr_scm.h | 68 ++++
arch/powerpc/include/uapi/asm/papr_scm_dsm.h | 143 +++++++
arch/powerpc/platforms/pseries/papr_scm.c | 399 ++++++++++++++++++-
include/uapi/linux/ndctl.h | 1 +
4 files changed, 602 insertions(+), 9 deletions(-)
create mode 100644 arch/powerpc/include/asm/papr_scm.h
create mode 100644 arch/powerpc/include/uapi/asm/papr_scm_dsm.h
--
2.24.1
^ permalink raw reply
* Re: [PATCH 8/8] perf/tools/pmu-events/powerpc: Add hv_24x7 socket/chip level metric events
From: maddy @ 2020-02-20 9:53 UTC (permalink / raw)
To: Kajol Jain, acme, linuxppc-dev
Cc: ravi.bangoria, maddy, jmario, mpetlan, peterz, gregkh,
linux-kernel, alexander.shishkin, linux-perf-users, ak, yao.jin,
anju, jolsa, namhyung, mingo, kan.liang
In-Reply-To: <20200214110335.31483-9-kjain@linux.ibm.com>
On 2/14/20 4:33 PM, Kajol Jain wrote:
> The hv_24×7 feature in IBM® POWER9™ processor-based servers provide the
> facility to continuously collect large numbers of hardware performance
> metrics efficiently and accurately.
> This patch adds hv_24x7 json metric file for different Socket/chip
> resources.
>
> Result:
>
> power9 platform:
>
> command:# ./perf stat --metric-only -M Memory_RD_BW_Chip -C 0
> -I 1000 sleep 1
>
> time MB Memory_RD_BW_Chip_0 MB Memory_RD_BW_Chip_1 MB
> 1.000192635 0.4 0.0
> 1.001695883 0.0 0.0
>
> Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
> ---
> .../arch/powerpc/power9/hv_24x7_metrics.json | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
> create mode 100644 tools/perf/pmu-events/arch/powerpc/power9/hv_24x7_metrics.json
>
> diff --git a/tools/perf/pmu-events/arch/powerpc/power9/hv_24x7_metrics.json b/tools/perf/pmu-events/arch/powerpc/power9/hv_24x7_metrics.json
> new file mode 100644
> index 000000000000..ac38f5540ac6
> --- /dev/null
> +++ b/tools/perf/pmu-events/arch/powerpc/power9/hv_24x7_metrics.json
Better to have it as nest_metrics.json instead. Rest looks fine
Reviewed-by: Madhavan Srinivasan <maddy@linux.ibm.com>
> @@ -0,0 +1,19 @@
> +[
> + {
> + "MetricExpr": "(hv_24x7@PM_MCS01_128B_RD_DISP_PORT01\\,chip\\=?@ + hv_24x7@PM_MCS01_128B_RD_DISP_PORT23\\,chip\\=?@ + hv_24x7@PM_MCS23_128B_RD_DISP_PORT01\\,chip\\=?@ + hv_24x7@PM_MCS23_128B_RD_DISP_PORT23\\,chip\\=?@)",
> + "MetricName": "Memory_RD_BW_Chip",
> + "MetricGroup": "Memory_BW",
> + "ScaleUnit": "1.6e-2MB"
> + },
> + {
> + "MetricExpr": "(hv_24x7@PM_MCS01_128B_WR_DISP_PORT01\\,chip\\=?@ + hv_24x7@PM_MCS01_128B_WR_DISP_PORT23\\,chip\\=?@ + hv_24x7@PM_MCS23_128B_WR_DISP_PORT01\\,chip\\=?@ + hv_24x7@PM_MCS23_128B_WR_DISP_PORT23\\,chip\\=?@ )",
> + "MetricName": "Memory_WR_BW_Chip",
> + "MetricGroup": "Memory_BW",
> + "ScaleUnit": "1.6e-2MB"
> + },
> + {
> + "MetricExpr": "(hv_24x7@PM_PB_CYC\\,chip\\=?@ )",
> + "MetricName": "PowerBUS_Frequency",
> + "ScaleUnit": "2.5e-7GHz"
> + }
> +]
^ permalink raw reply
* Re: [RESEND PATCH v2 9/9] ath5k: Constify ioreadX() iomem argument (as in generic implementation)
From: Jiri Slaby @ 2020-02-20 9:48 UTC (permalink / raw)
To: Krzysztof Kozlowski, Richard Henderson, Ivan Kokshaysky,
Matt Turner, Alexey Brodkin, Vineet Gupta, James E.J. Bottomley,
Helge Deller, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman, Yoshinori Sato, Rich Felker, Dave Airlie,
David Airlie, Daniel Vetter, Ben Skeggs, Mauro Carvalho Chehab,
Nick Kossifidis, Luis Chamberlain, Kalle Valo, David S. Miller,
Dave Jiang, Jon Mason, Allen Hubbe, Michael S. Tsirkin,
Jason Wang, Arnd Bergmann, Geert Uytterhoeven, Andrew Morton,
Thomas Gleixner, linux-alpha, linux-kernel, linux-snps-arc,
linux-parisc, linuxppc-dev, linux-sh, dri-devel, nouveau,
linux-media, linux-wireless, netdev, linux-ntb, virtualization,
linux-arch
In-Reply-To: <20200219175007.13627-10-krzk@kernel.org>
On 19. 02. 20, 18:50, Krzysztof Kozlowski wrote:
> The ioreadX() helpers have inconsistent interface. On some architectures
> void *__iomem address argument is a pointer to const, on some not.
>
> Implementations of ioreadX() do not modify the memory under the address
> so they can be converted to a "const" version for const-safety and
> consistency among architectures.
>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> Acked-by: Kalle Valo <kvalo@codeaurora.org>
> ---
> drivers/net/wireless/ath/ath5k/ahb.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath5k/ahb.c b/drivers/net/wireless/ath/ath5k/ahb.c
> index 2c9cec8b53d9..8bd01df369fb 100644
> --- a/drivers/net/wireless/ath/ath5k/ahb.c
> +++ b/drivers/net/wireless/ath/ath5k/ahb.c
> @@ -138,18 +138,18 @@ static int ath_ahb_probe(struct platform_device *pdev)
>
> if (bcfg->devid >= AR5K_SREV_AR2315_R6) {
> /* Enable WMAC AHB arbitration */
> - reg = ioread32((void __iomem *) AR5K_AR2315_AHB_ARB_CTL);
> + reg = ioread32((const void __iomem *) AR5K_AR2315_AHB_ARB_CTL);
While I understand why the parameter of ioread32 should be const, I
don't see a reason for these casts on the users' side. What does it
bring except longer code to read?
thanks,
--
js
^ permalink raw reply
* [PATCH 6/6] asm-generic/tlb: avoid potential double flush
From: Santosh Sivaraj @ 2020-02-20 8:42 UTC (permalink / raw)
To: linuxppc-dev, stable; +Cc: peterz, aneesh.kumar, akshay.adiga, gregkh
In-Reply-To: <20200220084229.1278137-1-santosh@fossix.org>
From: Peter Zijlstra <peterz@infradead.org>
Aneesh reported that:
tlb_flush_mmu()
tlb_flush_mmu_tlbonly()
tlb_flush() <-- #1
tlb_flush_mmu_free()
tlb_table_flush()
tlb_table_invalidate()
tlb_flush_mmu_tlbonly()
tlb_flush() <-- #2
does two TLBIs when tlb->fullmm, because __tlb_reset_range() will not
clear tlb->end in that case.
Observe that any caller to __tlb_adjust_range() also sets at least one of
the tlb->freed_tables || tlb->cleared_p* bits, and those are
unconditionally cleared by __tlb_reset_range().
Change the condition for actually issuing TLBI to having one of those bits
set, as opposed to having tlb->end != 0.
0758cd830494 in upstream.
Link: http://lkml.kernel.org/r/20200116064531.483522-4-aneesh.kumar@linux.ibm.com
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Reported-by: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>
Cc: <stable@vger.kernel.org> # 4.19
Signed-off-by: Santosh Sivaraj <santosh@fossix.org>
[santosh: backported to 4.19 stable]
---
include/asm-generic/tlb.h | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/include/asm-generic/tlb.h b/include/asm-generic/tlb.h
index 19934cdd143e..427a70c56ddd 100644
--- a/include/asm-generic/tlb.h
+++ b/include/asm-generic/tlb.h
@@ -179,7 +179,12 @@ static inline void __tlb_reset_range(struct mmu_gather *tlb)
static inline void tlb_flush_mmu_tlbonly(struct mmu_gather *tlb)
{
- if (!tlb->end)
+ /*
+ * Anything calling __tlb_adjust_range() also sets at least one of
+ * these bits.
+ */
+ if (!(tlb->freed_tables || tlb->cleared_ptes || tlb->cleared_pmds ||
+ tlb->cleared_puds || tlb->cleared_p4ds))
return;
tlb_flush(tlb);
--
2.24.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox