LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 5/5] selftests/powerpc/cache_shape: Fix out-of-tree build
From: Michael Ellerman @ 2018-10-29 11:23 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: joel
In-Reply-To: <20181029112353.15805-1-mpe@ellerman.id.au>

Use TEST_GEN_PROGS and don't redefine all, this makes the out-of-tree
build work. We need to move the extra dependencies below the include
of lib.mk, because it adds the $(OUTPUT) prefix if it's defined.

We can also drop the clean rule, lib.mk does it for us.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 tools/testing/selftests/powerpc/cache_shape/Makefile | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/powerpc/cache_shape/Makefile b/tools/testing/selftests/powerpc/cache_shape/Makefile
index ede4d3dae750..689f6c8ebcd8 100644
--- a/tools/testing/selftests/powerpc/cache_shape/Makefile
+++ b/tools/testing/selftests/powerpc/cache_shape/Makefile
@@ -1,12 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
-TEST_PROGS := cache_shape
-
-all: $(TEST_PROGS)
-
-$(TEST_PROGS): ../harness.c ../utils.c
+TEST_GEN_PROGS := cache_shape
 
 top_srcdir = ../../../../..
 include ../../lib.mk
 
-clean:
-	rm -f $(TEST_PROGS) *.o
+$(TEST_GEN_PROGS): ../harness.c ../utils.c
-- 
2.17.2


^ permalink raw reply related

* Re: [PATCH 2/4] mm: speed up mremap by 500x on large regions (v2)
From: Will Deacon @ 2018-10-29 10:28 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: linux-mips, Rich Felker, linux-ia64, linux-sh, Peter Zijlstra,
	Catalin Marinas, Dave Hansen, mhocko, linux-mm, lokeshgidra,
	sparclinux, linux-riscv, elfring, Jonas Bonn, kvmarm, dancol,
	Yoshinori Sato, linux-xtensa, linux-hexagon, Helge Deller,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), hughd,
	James E.J. Bottomley, kasan-dev, anton.ivanov, Ingo Molnar,
	Geert Uytterhoeven, Andrey Ryabinin, linux-snps-arc, kernel-team,
	Sam Creasey, Fenghua Yu, linux-s390, Jeff Dike, linux-um,
	Stefan Kristiansson, Julia Lawall, linux-m68k, Borislav Petkov,
	Andy Lutomirski, nios2-dev, Kirill A. Shutemov, Stafford Horne,
	Guan Xuetao, Chris Zankel, Tony Luck, Richard Weinberger,
	linux-parisc, pantin, Max Filippov, linux-kernel, minchan,
	Thomas Gleixner, linux-alpha, Ley Foon Tan, akpm, linuxppc-dev,
	David S. Miller
In-Reply-To: <20181026211148.GA140716@joelaf.mtv.corp.google.com>

On Fri, Oct 26, 2018 at 02:11:48PM -0700, Joel Fernandes wrote:
> My thinking is to take it slow and get the patch in in its current state,
> since it improves x86. Then as a next step, look into why the arm64 tlb
> flushes are that expensive and look into optimizing that. On arm64 I am
> testing on a 4.9 kernel so I'm wondering there are any optimizations since
> 4.9 that can help speed it up there. After that, if all else fails about
> speeding up arm64, then I look into developing the cleanest possible solution
> where we can keep the lock held for longer and flush lesser.

We rewrote a good chunk of the arm64 TLB invalidation and core mmu_gather
code this merge window, so please do have another look at -rc1!

Will

^ permalink raw reply

* RE: [PATCH v7 2/6] soc/fsl/guts: Add definition for LX2160A
From: Poonam Aggrwal @ 2018-10-29 11:09 UTC (permalink / raw)
  To: Vabhav Sharma, sudeep.holla@arm.com, oss@buserror.net,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	robh+dt@kernel.org, mark.rutland@arm.com,
	linuxppc-dev@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org, mturquette@baylibre.com,
	sboyd@kernel.org, rjw@rjwysocki.net, viresh.kumar@linaro.org,
	linux-clk@vger.kernel.org, linux-pm@vger.kernel.org,
	linux-kernel-owner@vger.kernel.org, catalin.marinas@arm.com,
	will.deacon@arm.com, gregkh@linuxfoundation.org, arnd@arndb.de,
	kstewart@linuxfoundation.org, yamada.masahiro@socionext.com,
	Leo Li, shawnguo@kernel.org
  Cc: ulf.hansson@linaro.org, Udit Kumar, Pankaj Bansal,
	linux@armlinux.org.uk, adrian.hunter@intel.com, Varun Sethi,
	Vabhav Sharma, Yinbo Zhu
In-Reply-To: <1540759926-3641-3-git-send-email-vabhav.sharma@nxp.com>



> -----Original Message-----
> From: linux-arm-kernel [mailto:linux-arm-kernel-bounces@lists.infradead.org]
> On Behalf Of Vabhav Sharma
> Sent: Monday, October 29, 2018 2:28 PM
> To: sudeep.holla@arm.com; oss@buserror.net; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org; robh+dt@kernel.org; mark.rutland@arm.com;
> linuxppc-dev@lists.ozlabs.org; linux-arm-kernel@lists.infradead.org;
> mturquette@baylibre.com; sboyd@kernel.org; rjw@rjwysocki.net;
> viresh.kumar@linaro.org; linux-clk@vger.kernel.org; linux-pm@vger.kernel.org;
> linux-kernel-owner@vger.kernel.org; catalin.marinas@arm.com;
> will.deacon@arm.com; gregkh@linuxfoundation.org; arnd@arndb.de;
> kstewart@linuxfoundation.org; yamada.masahiro@socionext.com; Leo Li
> <leoyang.li@nxp.com>; shawnguo@kernel.org
> Cc: ulf.hansson@linaro.org; Udit Kumar <udit.kumar@nxp.com>; Pankaj Bansal
> <pankaj.bansal@nxp.com>; linux@armlinux.org.uk; adrian.hunter@intel.com;
> Varun Sethi <V.Sethi@nxp.com>; Vabhav Sharma <vabhav.sharma@nxp.com>;
> Yinbo Zhu <yinbo.zhu@nxp.com>
> Subject: [PATCH v7 2/6] soc/fsl/guts: Add definition for LX2160A
> 
> Adding compatible string "lx2160a-dcfg" to initialize guts driver for lx2160 and
> SoC die attribute definition for LX2160A
> 
> Signed-off-by: Vabhav Sharma <vabhav.sharma@nxp.com>
> Signed-off-by: Yinbo Zhu <yinbo.zhu@nxp.com>
> Acked-by: Li Yang <leoyang.li@nxp.com>
> ---
>  drivers/soc/fsl/guts.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c index
> 302e0c8..bcab1ee 100644
> --- a/drivers/soc/fsl/guts.c
> +++ b/drivers/soc/fsl/guts.c
> @@ -100,6 +100,11 @@ static const struct fsl_soc_die_attr fsl_soc_die[] = {
>  	  .svr		= 0x87000000,
>  	  .mask		= 0xfff70000,
>  	},
> +	/* Die: LX2160A, SoC: LX2160A/LX2120A/LX2080A */
> +	{ .die          = "LX2160A",
> +	  .svr          = 0x87360000,
Do all  the threevariants " LX2160A/LX2120A/LX2080A"  have same SVR?
> +	  .mask         = 0xff3f0000,
> +	},
>  	{ },
>  };
> 
> @@ -222,6 +227,7 @@ static const struct of_device_id fsl_guts_of_match[] = {
>  	{ .compatible = "fsl,ls1088a-dcfg", },
>  	{ .compatible = "fsl,ls1012a-dcfg", },
>  	{ .compatible = "fsl,ls1046a-dcfg", },
> +	{ .compatible = "fsl,lx2160a-dcfg", },
>  	{}
>  };
>  MODULE_DEVICE_TABLE(of, fsl_guts_of_match);
> --
> 2.7.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists.infr
> adead.org%2Fmailman%2Flistinfo%2Flinux-arm-
> kernel&amp;data=02%7C01%7Cpoonam.aggrwal%40nxp.com%7Cd9b7c36c786
> 54cd6dc4008d63d7fee63%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0
> %7C636764017001504288&amp;sdata=pqFMlw6yVrHrvn4YRhqj%2FmoOnSscrZu
> tcoYn3In8OJc%3D&amp;reserved=0

^ permalink raw reply

* [RFC PATCH] seccomp: Add protection keys into seccomp_data
From: Michael Sammler @ 2018-10-29 11:23 UTC (permalink / raw)
  Cc: Will Drewry, Kees Cook, linux-api, Ram Pai, Andy Lutomirski,
	Michael Sammler, linuxppc-dev

Add the current value of an architecture specific protection keys
register (currently PKRU on x86) to data available for seccomp-bpf
programs to work on. This allows filters based on the currently
enabled protection keys.

Support for protection keys on the POWER architecture is not part of
this patch since I do not have access to a PowerPC, but adding support
for it can be achieved by setting sd->pkeys to the AMR register in
populate_seccomp_data.

One use case for this patch is disabling unnecessary system calls for a
library (e.g. network i/o for a crypto library) while the library runs
without disabling the system calls for the whole program (by changing
the protection keys before and after the library executes). Using this
one could ensure that the library behaves a expected (e.g. the crypto
library not sending private keys to a malicious server).

This patch also enables lightweight sandboxing of untrusted code using
memory protection keys: Protection keys provide memory isolation but
for a sandbox system call isolation is needed as well. This patch
allows writing a seccomp filter to prevent system calls by the
untrusted code while still allowing system calls for the trusted code.

An alternative design would be to extend (c)BPF with a new instruction
to read the state of a protection key. This alternate design would
provide a simpler interface to the user space since the BPF program
would not need to deal with the architecture specific pkeys field in
seccomp_data, but the question is, how much of an advantage this would
be as the nr field in seccomp_data is already architecture specific.
Adding a new instruction for BPF programs is more complicated than
this patch and might be a breaking change.

Results of selftests/seccomp_benchmark.c on a x86 machine with pkeys
support:

With patch:
Benchmarking 33554432 samples...
28.019505558 - 18.676858522 = 9342647036
getpid native: 278 ns
42.279109885 - 28.019657031 = 14259452854
getpid RET_ALLOW: 424 ns
Estimated seccomp overhead per syscall: 146 ns

Without patch:
Benchmarking 33554432 samples...
28.059619466 - 18.706769155 = 9352850311
getpid native: 278 ns
42.299228279 - 28.059761804 = 14239466475
getpid RET_ALLOW: 424 ns
Estimated seccomp overhead per syscall: 146 ns

Cc: Kees Cook <keescook@chromium.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Will Drewry <wad@chromium.org>
Cc: Ram Pai <linuxram@us.ibm.com>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-api@vger.kernel.org
Signed-off-by: Michael Sammler <msammler@mpi-sws.org>
---
Changes to the previous version:
- added motivation, notes about POWER, alternative design and benchmark results to the commit log
- renamed pkru field in seccomp_data to pkeys
- changed size of pkru field to __u64 and removed reserved field
- added test for x86

 arch/mips/kernel/ptrace.c                     |   1 +
 arch/x86/entry/common.c                       |   1 +
 include/uapi/linux/seccomp.h                  |   3 +
 kernel/seccomp.c                              |   1 +
 tools/testing/selftests/seccomp/seccomp_bpf.c | 107 +++++++++++++++++++++++++-
 5 files changed, 112 insertions(+), 1 deletion(-)

diff --git a/arch/mips/kernel/ptrace.c b/arch/mips/kernel/ptrace.c
index e5ba56c0..a58dd04d 100644
--- a/arch/mips/kernel/ptrace.c
+++ b/arch/mips/kernel/ptrace.c
@@ -1277,6 +1277,7 @@ asmlinkage long syscall_trace_enter(struct pt_regs *regs, long syscall)
 		for (i = 0; i < 6; i++)
 			sd.args[i] = args[i];
 		sd.instruction_pointer = KSTK_EIP(current);
+		sd.pkeys = 0;

 		ret = __secure_computing(&sd);
 		if (ret == -1)
diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
index 3b2490b8..20c51bf2 100644
--- a/arch/x86/entry/common.c
+++ b/arch/x86/entry/common.c
@@ -98,6 +98,7 @@ static long syscall_trace_enter(struct pt_regs *regs)
 		sd.arch = arch;
 		sd.nr = regs->orig_ax;
 		sd.instruction_pointer = regs->ip;
+		sd.pkeys = read_pkru();
 #ifdef CONFIG_X86_64
 		if (arch == AUDIT_ARCH_X86_64) {
 			sd.args[0] = regs->di;
diff --git a/include/uapi/linux/seccomp.h b/include/uapi/linux/seccomp.h
index 9efc0e73..3aa2d934 100644
--- a/include/uapi/linux/seccomp.h
+++ b/include/uapi/linux/seccomp.h
@@ -52,12 +52,15 @@
  * @instruction_pointer: at the time of the system call.
  * @args: up to 6 system call arguments always stored as 64-bit values
  *        regardless of the architecture.
+ * @pkeys: value of an architecture specific protection keys register
+ *         (currently PKRU on x86)
  */
 struct seccomp_data {
 	int nr;
 	__u32 arch;
 	__u64 instruction_pointer;
 	__u64 args[6];
+	__u64 pkeys;
 };

 #endif /* _UAPI_LINUX_SECCOMP_H */
diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index fd023ac2..dfb8b0d6 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -91,6 +91,7 @@ static void populate_seccomp_data(struct seccomp_data *sd)
 	sd->args[4] = args[4];
 	sd->args[5] = args[5];
 	sd->instruction_pointer = KSTK_EIP(task);
+	sd->pkeys = 0;
 }

 /**
diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c
index e1473234..f7f8fa6f 100644
--- a/tools/testing/selftests/seccomp/seccomp_bpf.c
+++ b/tools/testing/selftests/seccomp/seccomp_bpf.c
@@ -82,6 +82,7 @@ struct seccomp_data {
 	__u32 arch;
 	__u64 instruction_pointer;
 	__u64 args[6];
+	__u64 pkeys;
 };
 #endif

@@ -732,7 +733,9 @@ TEST(KILL_process)
 TEST(arg_out_of_range)
 {
 	struct sock_filter filter[] = {
-		BPF_STMT(BPF_LD|BPF_W|BPF_ABS, syscall_arg(6)),
+		BPF_STMT(BPF_LD|BPF_W|BPF_ABS,
+			offsetof(struct seccomp_data, pkeys)
+				+ sizeof(__u64)),
 		BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ALLOW),
 	};
 	struct sock_fprog prog = {
@@ -2933,6 +2936,108 @@ skip:
 	ASSERT_EQ(0, kill(pid, SIGKILL));
 }

+#if defined(__i386__) || defined(__x86_64__)
+static inline void __cpuid(unsigned int *eax, unsigned int *ebx,
+		unsigned int *ecx, unsigned int *edx)
+{
+	/* ecx is often an input as well as an output. */
+	asm volatile(
+		"cpuid;"
+		: "=a" (*eax),
+		  "=b" (*ebx),
+		  "=c" (*ecx),
+		  "=d" (*edx)
+		: "0" (*eax), "2" (*ecx));
+}
+
+/* Intel-defined CPU features, CPUID level 0x00000007:0 (ecx) */
+#define X86_FEATURE_PKU        (1<<3) /* Protection Keys for Userspace */
+#define X86_FEATURE_OSPKE      (1<<4) /* OS Protection Keys Enable */
+
+static inline int cpu_has_pku(void)
+{
+	unsigned int eax;
+	unsigned int ebx;
+	unsigned int ecx;
+	unsigned int edx;
+
+	eax = 0x7;
+	ecx = 0x0;
+	__cpuid(&eax, &ebx, &ecx, &edx);
+
+	if (!(ecx & X86_FEATURE_PKU))
+		return 0;
+	if (!(ecx & X86_FEATURE_OSPKE))
+		return 0;
+	return 1;
+}
+
+static inline __u32 read_pkru(void)
+{
+	if (!cpu_has_pku())
+		return 0;
+
+	__u32 ecx = 0;
+	__u32 edx, pkru;
+
+	/*
+	 * "rdpkru" instruction.  Places PKRU contents in to EAX,
+	 * clears EDX and requires that ecx=0.
+	 */
+	asm volatile(".byte 0x0f,0x01,0xee\n\t"
+		     : "=a" (pkru), "=d" (edx)
+		     : "c" (ecx));
+	return pkru;
+}
+
+static inline void write_pkru(__u32 pkru)
+{
+	if (!cpu_has_pku())
+		return;
+
+	__u32 ecx = 0, edx = 0;
+
+	/*
+	 * "wrpkru" instruction.  Loads contents in EAX to PKRU,
+	 * requires that ecx = edx = 0.
+	 */
+	asm volatile(".byte 0x0f,0x01,0xef\n\t"
+		     : : "a" (pkru), "c"(ecx), "d"(edx));
+}
+
+#define TEST_PKRU 0x55555550
+
+TEST_SIGNAL(pkeys_set, SIGSYS)
+{
+	write_pkru(TEST_PKRU);
+	/* read back the written value because pkru might not be supported */
+	__u32 pkru = read_pkru();
+
+	struct sock_filter filter[] = {
+		BPF_STMT(BPF_LD|BPF_W|BPF_ABS,
+			offsetof(struct seccomp_data, pkeys)),
+		BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, pkru, 1, 0),
+		BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ALLOW),
+		BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_KILL),
+	};
+	struct sock_fprog prog = {
+		.len = (unsigned short)ARRAY_SIZE(filter),
+		.filter = filter,
+	};
+	long ret;
+
+	ret = prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0);
+	ASSERT_EQ(0, ret);
+
+	ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog);
+	ASSERT_EQ(0, ret);
+
+	/* should never return. */
+	EXPECT_EQ(0, syscall(__NR_getpid));
+}
+#endif
+
+
 /*
  * TODO:
  * - add microbenchmarks
--
2.11.0

^ permalink raw reply related

* Re: [PATCH 1/5] powerpc/64s: Guarded Userspace Access Prevention
From: kbuild test robot @ 2018-10-29 13:27 UTC (permalink / raw)
  To: Russell Currey; +Cc: mikey, linuxppc-dev, kbuild-all, npiggin
In-Reply-To: <20181026063513.30806-2-ruscur@russell.cc>

[-- Attachment #1: Type: text/plain, Size: 2873 bytes --]

Hi Russell,

I love your patch! Yet something to improve:

[auto build test ERROR on powerpc/next]
[also build test ERROR on next-20181029]
[cannot apply to v4.19]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Russell-Currey/Guarded-Userspace-Access-Prevention-on-Radix/20181026-145017
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-ppc64e_defconfig (attached as .config)
compiler: powerpc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.2.0 make.cross ARCH=powerpc 

All error/warnings (new ones prefixed by >>):

   In file included from arch/powerpc/kernel/asm-offsets.c:31:0:
   arch/powerpc/kernel/asm-offsets.c: In function 'main':
>> include/linux/compiler_types.h:229:35: error: 'struct paca_struct' has no member named 'user_access_allowed'
    #define __compiler_offsetof(a, b) __builtin_offsetof(a, b)
                                      ^
   include/linux/kbuild.h:6:62: note: in definition of macro 'DEFINE'
     asm volatile("\n.ascii \"->" #sym " %0 " #val "\"" : : "i" (val))
                                                                 ^~~
   include/linux/stddef.h:17:32: note: in expansion of macro '__compiler_offsetof'
    #define offsetof(TYPE, MEMBER) __compiler_offsetof(TYPE, MEMBER)
                                   ^~~~~~~~~~~~~~~~~~~
>> include/linux/kbuild.h:11:14: note: in expansion of macro 'offsetof'
     DEFINE(sym, offsetof(struct str, mem))
                 ^~~~~~~~
>> arch/powerpc/kernel/asm-offsets.c:263:2: note: in expansion of macro 'OFFSET'
     OFFSET(PACA_USER_ACCESS_ALLOWED, paca_struct, user_access_allowed);
     ^~~~~~
   make[2]: *** [arch/powerpc/kernel/asm-offsets.s] Error 1
   make[2]: Target '__build' not remade because of errors.
   make[1]: *** [prepare0] Error 2
   make[1]: Target 'prepare' not remade because of errors.
   make: *** [sub-make] Error 2

vim +229 include/linux/compiler_types.h

815f0ddb Nick Desaulniers 2018-08-22  228  
815f0ddb Nick Desaulniers 2018-08-22 @229  #define __compiler_offsetof(a, b)	__builtin_offsetof(a, b)
815f0ddb Nick Desaulniers 2018-08-22  230  

:::::: The code at line 229 was first introduced by commit
:::::: 815f0ddb346c196018d4d8f8f55c12b83da1de3f include/linux/compiler*.h: make compiler-*.h mutually exclusive

:::::: TO: Nick Desaulniers <ndesaulniers@google.com>
:::::: CC: Linus Torvalds <torvalds@linux-foundation.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 21124 bytes --]

^ permalink raw reply

* Re: [PATCH 3/7] dmaengine: fsl-qdma: Add qDMA controller driver for Layerscape SoCs
From: Li Yang @ 2018-10-29 14:45 UTC (permalink / raw)
  To: Peng Ma
  Cc: Mark Rutland,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Wen He, linuxppc-dev, lkml, Leo Li, Vinod, Rob Herring,
	dmaengine@vger.kernel.org, Dan Williams, Shawn Guo, Jiaheng Fan,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <VI1PR04MB443154EF740DCE49CCA6043BEDF30@VI1PR04MB4431.eurprd04.prod.outlook.com>



> On Oct 29, 2018, at 4:51 AM, Peng Ma <peng.ma@nxp.com> wrote:
> 
> 
> 
>> -----Original Message-----
>> From: Li Yang <leoyang.li@nxp.com>
>> Sent: 2018年10月27日 4:48
>> To: Peng Ma <peng.ma@nxp.com>
>> Cc: Vinod <vkoul@kernel.org>; Rob Herring <robh+dt@kernel.org>; Mark
>> Rutland <mark.rutland@arm.com>; Shawn Guo <shawnguo@kernel.org>; Dan
>> Williams <dan.j.williams@intel.com>; dmaengine@vger.kernel.org; open
>> list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
>> <devicetree@vger.kernel.org>; lkml <linux-kernel@vger.kernel.org>;
>> moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
>> <linux-arm-kernel@lists.infradead.org>; linuxppc-dev
>> <linuxppc-dev@lists.ozlabs.org>; Wen He <wen.he_1@nxp.com>; Jiaheng Fan
>> <jiaheng.fan@nxp.com>
>> Subject: Re: [PATCH 3/7] dmaengine: fsl-qdma: Add qDMA controller driver for
>> Layerscape SoCs
>> 
>>> On Fri, Oct 26, 2018 at 4:57 AM Peng Ma <peng.ma@nxp.com> wrote:
>>> 
>>> NXP Queue DMA controller(qDMA) on Layerscape SoCs supports channel
>>> virtuallization by allowing DMA jobs to be enqueued into different
>>> command queues.
>>> 
>>> Note that this module depends on NXP DPAA.
>> 
>> It is not clear if you are saying that the driver can only work on
>> SoCs with a DPAA hardware block, or the driver is actually depending
>> on the DPAA drivers also.  If it is the later case, you also should
>> express that in the Kconfig you added below.
>> 
> [Peng Ma] Ok, I will express it in the Kconfig.
>>> 
>>> Signed-off-by: Wen He <wen.he_1@nxp.com>
>>> Signed-off-by: Jiaheng Fan <jiaheng.fan@nxp.com>
>>> Signed-off-by: Peng Ma <peng.ma@nxp.com>
>>> ---
>>> change in v10:
>>>        - no
>>> 
>>> drivers/dma/Kconfig    |   13 +
>>> drivers/dma/Makefile   |    1 +
>>> drivers/dma/fsl-qdma.c | 1257
>> ++++++++++++++++++++++++++++++++++++++++++++++++
>>> 3 files changed, 1271 insertions(+), 0 deletions(-)
>>> create mode 100644 drivers/dma/fsl-qdma.c
>>> 
>>> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
>>> index dacf3f4..50e19d7 100644
>>> --- a/drivers/dma/Kconfig
>>> +++ b/drivers/dma/Kconfig
>>> @@ -218,6 +218,19 @@ config FSL_EDMA
>>>          multiplexing capability for DMA request sources(slot).
>>>          This module can be found on Freescale Vybrid and LS-1 SoCs.
>>> 
>>> +config FSL_QDMA
>>> +       tristate "NXP Layerscape qDMA engine support"
>>> +       depends on ARM || ARM64
>>> +       select DMA_ENGINE
>>> +       select DMA_VIRTUAL_CHANNELS
>>> +       select DMA_ENGINE_RAID
>>> +       select ASYNC_TX_ENABLE_CHANNEL_SWITCH
>>> +       help
>>> +         Support the NXP Layerscape qDMA engine with command queue
>> and legacy mode.
>>> +         Channel virtualization is supported through enqueuing of DMA
>> jobs to,
>>> +         or dequeuing DMA jobs from, different work queues.
>>> +         This module can be found on NXP Layerscape SoCs.
>>> +
>>> config FSL_RAID
>>>         tristate "Freescale RAID engine Support"
>>>         depends on FSL_SOC
>> && !ASYNC_TX_ENABLE_CHANNEL_SWITCH
>>> diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
>>> index c91702d..2d1b586 100644
>>> --- a/drivers/dma/Makefile
>>> +++ b/drivers/dma/Makefile
>>> @@ -32,6 +32,7 @@ obj-$(CONFIG_DW_DMAC_CORE) += dw/
>>> obj-$(CONFIG_EP93XX_DMA) += ep93xx_dma.o
>>> obj-$(CONFIG_FSL_DMA) += fsldma.o
>>> obj-$(CONFIG_FSL_EDMA) += fsl-edma.o
>>> +obj-$(CONFIG_FSL_QDMA) += fsl-qdma.o
>>> obj-$(CONFIG_FSL_RAID) += fsl_raid.o
>>> obj-$(CONFIG_HSU_DMA) += hsu/
>>> obj-$(CONFIG_IMG_MDC_DMA) += img-mdc-dma.o
>>> diff --git a/drivers/dma/fsl-qdma.c b/drivers/dma/fsl-qdma.c
>>> new file mode 100644
>>> index 0000000..404869e
>>> --- /dev/null
>>> +++ b/drivers/dma/fsl-qdma.c
>>> @@ -0,0 +1,1257 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +// Copyright 2018 NXP
>> 
>> I'm not sure if this is really the case.  The driver at least has been
>> sent out in 2015.  We should keep these copyright claims, even the
>> legacy Freescale copyright claims.
>> 
> [Peng Ma] 
>    I am not sure this patch sent out in 2015, but the git log earliest shows the patch cteated at Dec 20 2017. so if i changed the "Copyright 2018 NXP" to " Copyright 2017-2018 NXP "

You can find early versions of this patch in previous SDK releases or the upstream versions with a google search.  These existing copyright claims should not be removed in the first place.

> 
> Best regards
>    Peng Ma
>>> +
>>> +/*
>>> + * Driver for NXP Layerscape Queue Direct Memory Access Controller
>>> + *
>>> + * Author:
>>> + *  Wen He <wen.he_1@nxp.com>
>>> + *  Jiaheng Fan <jiaheng.fan@nxp.com>
>>> + *
>>> + */
>>> +
>>> +#include <linux/module.h>
>>> +#include <linux/delay.h>
>>> +#include <linux/of_irq.h>
>>> +#include <linux/of_platform.h>
>>> +#include <linux/of_dma.h>
>>> +#include <linux/dma-mapping.h>
>>> +
>>> +#include "virt-dma.h"
>>> +#include "fsldma.h"
>>> +
>>> +/* Register related definition */
>>> +#define FSL_QDMA_DMR                   0x0
>>> +#define FSL_QDMA_DSR                   0x4
>>> +#define FSL_QDMA_DEIER                 0xe00
>>> +#define FSL_QDMA_DEDR                  0xe04
>>> +#define FSL_QDMA_DECFDW0R              0xe10
>>> +#define FSL_QDMA_DECFDW1R              0xe14
>>> +#define FSL_QDMA_DECFDW2R              0xe18
>>> +#define FSL_QDMA_DECFDW3R              0xe1c
>>> +#define FSL_QDMA_DECFQIDR              0xe30
>>> +#define FSL_QDMA_DECBR                 0xe34
>>> +
>>> +#define FSL_QDMA_BCQMR(x)              (0xc0 + 0x100 * (x))
>>> +#define FSL_QDMA_BCQSR(x)              (0xc4 + 0x100 * (x))
>>> +#define FSL_QDMA_BCQEDPA_SADDR(x)      (0xc8 + 0x100 * (x))
>>> +#define FSL_QDMA_BCQDPA_SADDR(x)       (0xcc + 0x100 * (x))
>>> +#define FSL_QDMA_BCQEEPA_SADDR(x)      (0xd0 + 0x100 * (x))
>>> +#define FSL_QDMA_BCQEPA_SADDR(x)       (0xd4 + 0x100 * (x))
>>> +#define FSL_QDMA_BCQIER(x)             (0xe0 + 0x100 * (x))
>>> +#define FSL_QDMA_BCQIDR(x)             (0xe4 + 0x100 * (x))
>>> +
>>> +#define FSL_QDMA_SQDPAR                        0x80c
>>> +#define FSL_QDMA_SQEPAR                        0x814
>>> +#define FSL_QDMA_BSQMR                 0x800
>>> +#define FSL_QDMA_BSQSR                 0x804
>>> +#define FSL_QDMA_BSQICR                        0x828
>>> +#define FSL_QDMA_CQMR                  0xa00
>>> +#define FSL_QDMA_CQDSCR1               0xa08
>>> +#define FSL_QDMA_CQDSCR2                0xa0c
>>> +#define FSL_QDMA_CQIER                 0xa10
>>> +#define FSL_QDMA_CQEDR                 0xa14
>>> +#define FSL_QDMA_SQCCMR                        0xa20
>>> +
>>> +/* Registers for bit and genmask */
>>> +#define FSL_QDMA_CQIDR_SQT             BIT(15)
>>> +#define QDMA_CCDF_FOTMAT               BIT(29)
>>> +#define QDMA_CCDF_SER                  BIT(30)
>>> +#define QDMA_SG_FIN                    BIT(30)
>>> +#define QDMA_SG_LEN_MASK               GENMASK(29, 0)
>>> +#define QDMA_CCDF_MASK                 GENMASK(28, 20)
>>> +
>>> +#define FSL_QDMA_DEDR_CLEAR            GENMASK(31, 0)
>>> +#define FSL_QDMA_BCQIDR_CLEAR          GENMASK(31, 0)
>>> +#define FSL_QDMA_DEIER_CLEAR           GENMASK(31, 0)
>>> +
>>> +#define FSL_QDMA_BCQIER_CQTIE          BIT(15)
>>> +#define FSL_QDMA_BCQIER_CQPEIE         BIT(23)
>>> +#define FSL_QDMA_BSQICR_ICEN           BIT(31)
>>> +
>>> +#define FSL_QDMA_BSQICR_ICST(x)                ((x) << 16)
>>> +#define FSL_QDMA_CQIER_MEIE            BIT(31)
>>> +#define FSL_QDMA_CQIER_TEIE            BIT(0)
>>> +#define FSL_QDMA_SQCCMR_ENTER_WM       BIT(21)
>>> +
>>> +#define FSL_QDMA_BCQMR_EN              BIT(31)
>>> +#define FSL_QDMA_BCQMR_EI              BIT(30)
>>> +#define FSL_QDMA_BCQMR_CD_THLD(x)      ((x) << 20)
>>> +#define FSL_QDMA_BCQMR_CQ_SIZE(x)      ((x) << 16)
>>> +
>>> +#define FSL_QDMA_BCQSR_QF              BIT(16)
>>> +#define FSL_QDMA_BCQSR_XOFF            BIT(0)
>>> +
>>> +#define FSL_QDMA_BSQMR_EN              BIT(31)
>>> +#define FSL_QDMA_BSQMR_DI              BIT(30)
>>> +#define FSL_QDMA_BSQMR_CQ_SIZE(x)      ((x) << 16)
>>> +
>>> +#define FSL_QDMA_BSQSR_QE              BIT(17)
>>> +
>>> +#define FSL_QDMA_DMR_DQD               BIT(30)
>>> +#define FSL_QDMA_DSR_DB                BIT(31)
>>> +
>>> +/* Size related definition */
>>> +#define FSL_QDMA_QUEUE_MAX             8
>>> +#define FSL_QDMA_COMMAND_BUFFER_SIZE   64
>>> +#define FSL_QDMA_DESCRIPTOR_BUFFER_SIZE 32
>>> +#define FSL_QDMA_CIRCULAR_DESC_SIZE_MIN        64
>>> +#define FSL_QDMA_CIRCULAR_DESC_SIZE_MAX        16384
>>> +#define FSL_QDMA_QUEUE_NUM_MAX         8
>>> +
>>> +/* Field definition for CMD */
>>> +#define FSL_QDMA_CMD_RWTTYPE           0x4
>>> +#define FSL_QDMA_CMD_LWC                0x2
>>> +#define FSL_QDMA_CMD_RWTTYPE_OFFSET    28
>>> +#define FSL_QDMA_CMD_NS_OFFSET         27
>>> +#define FSL_QDMA_CMD_DQOS_OFFSET       24
>>> +#define FSL_QDMA_CMD_WTHROTL_OFFSET    20
>>> +#define FSL_QDMA_CMD_DSEN_OFFSET       19
>>> +#define FSL_QDMA_CMD_LWC_OFFSET                16
>>> +
>>> +/* Field definition for Descriptor offset */
>>> +#define QDMA_CCDF_STATUS               20
>>> +#define QDMA_CCDF_OFFSET               20
>>> +
>>> +/* Field definition for safe loop count*/
>>> +#define FSL_QDMA_HALT_COUNT            1500
>>> +#define FSL_QDMA_MAX_SIZE              16385
>>> +#define        FSL_QDMA_COMP_TIMEOUT           1000
>>> +#define FSL_COMMAND_QUEUE_OVERFLLOW    10
>>> +
>>> +#define FSL_QDMA_BLOCK_BASE_OFFSET(fsl_qdma_engine, x)
>> \
>>> +       (((fsl_qdma_engine)->block_offset) * (x))
>>> +
>>> +/**
>>> + * struct fsl_qdma_format - This is the struct holding describing compound
>>> + *                         descriptor format with qDMA.
>>> + * @status:                Command status and enqueue status
>> notification.
>>> + * @cfg:                   Frame offset and frame format.
>>> + * @addr_lo:               Holding the compound descriptor of the
>> lower
>>> + *                         32-bits address in memory 40-bit address.
>>> + * @addr_hi:               Same as above member, but point high
>> 8-bits in
>>> + *                         memory 40-bit address.
>>> + * @__reserved1:           Reserved field.
>>> + * @cfg8b_w1:              Compound descriptor command queue
>> origin produced
>>> + *                         by qDMA and dynamic debug field.
>>> + * @data                   Pointer to the memory 40-bit address,
>> describes DMA
>>> + *                         source information and DMA destination
>> information.
>>> + */
>>> +struct fsl_qdma_format {
>>> +       __le32 status;
>>> +       __le32 cfg;
>>> +       union {
>>> +               struct {
>>> +                       __le32 addr_lo;
>>> +                       u8 addr_hi;
>>> +                       u8 __reserved1[2];
>>> +                       u8 cfg8b_w1;
>>> +               } __packed;
>>> +               __le64 data;
>>> +       };
>>> +} __packed;
>>> +
>>> +/* qDMA status notification pre information */
>>> +struct fsl_pre_status {
>>> +       u64 addr;
>>> +       u8 queue;
>>> +};
>>> +
>>> +static DEFINE_PER_CPU(struct fsl_pre_status, pre);
>>> +
>>> +struct fsl_qdma_chan {
>>> +       struct virt_dma_chan            vchan;
>>> +       struct virt_dma_desc            vdesc;
>>> +       enum dma_status                 status;
>>> +       struct fsl_qdma_engine          *qdma;
>>> +       struct fsl_qdma_queue           *queue;
>>> +};
>>> +
>>> +struct fsl_qdma_queue {
>>> +       struct fsl_qdma_format  *virt_head;
>>> +       struct fsl_qdma_format  *virt_tail;
>>> +       struct list_head        comp_used;
>>> +       struct list_head        comp_free;
>>> +       struct dma_pool         *comp_pool;
>>> +       struct dma_pool         *desc_pool;
>>> +       spinlock_t              queue_lock;
>>> +       dma_addr_t              bus_addr;
>>> +       u32                     n_cq;
>>> +       u32                     id;
>>> +       struct fsl_qdma_format  *cq;
>>> +       void __iomem            *block_base;
>>> +};
>>> +
>>> +struct fsl_qdma_comp {
>>> +       dma_addr_t              bus_addr;
>>> +       dma_addr_t              desc_bus_addr;
>>> +       struct fsl_qdma_format  *virt_addr;
>>> +       struct fsl_qdma_format  *desc_virt_addr;
>>> +       struct fsl_qdma_chan    *qchan;
>>> +       struct virt_dma_desc    vdesc;
>>> +       struct list_head        list;
>>> +};
>>> +
>>> +struct fsl_qdma_engine {
>>> +       struct dma_device       dma_dev;
>>> +       void __iomem            *ctrl_base;
>>> +       void __iomem            *status_base;
>>> +       void __iomem            *block_base;
>>> +       u32                     n_chans;
>>> +       u32                     n_queues;
>>> +       struct mutex            fsl_qdma_mutex;
>>> +       int                     error_irq;
>>> +       int                     *queue_irq;
>>> +       u32                     feature;
>>> +       struct fsl_qdma_queue   *queue;
>>> +       struct fsl_qdma_queue   **status;
>>> +       struct fsl_qdma_chan    *chans;
>>> +       int                     block_number;
>>> +       int                     block_offset;
>>> +       int                     irq_base;
>>> +       int                     desc_allocated;
>>> +
>>> +};
>>> +
>>> +static inline u64
>>> +qdma_ccdf_addr_get64(const struct fsl_qdma_format *ccdf)
>>> +{
>>> +       return le64_to_cpu(ccdf->data) & (U64_MAX >> 24);
>>> +}
>>> +
>>> +static inline void
>>> +qdma_desc_addr_set64(struct fsl_qdma_format *ccdf, u64 addr)
>>> +{
>>> +       ccdf->addr_hi = upper_32_bits(addr);
>>> +       ccdf->addr_lo = cpu_to_le32(lower_32_bits(addr));
>>> +}
>>> +
>>> +static inline u8
>>> +qdma_ccdf_get_queue(const struct fsl_qdma_format *ccdf)
>>> +{
>>> +       return ccdf->cfg8b_w1 & U8_MAX;
>>> +}
>>> +
>>> +static inline int
>>> +qdma_ccdf_get_offset(const struct fsl_qdma_format *ccdf)
>>> +{
>>> +       return (le32_to_cpu(ccdf->cfg) & QDMA_CCDF_MASK) >>
>> QDMA_CCDF_OFFSET;
>>> +}
>>> +
>>> +static inline void
>>> +qdma_ccdf_set_format(struct fsl_qdma_format *ccdf, int offset)
>>> +{
>>> +       ccdf->cfg = cpu_to_le32(QDMA_CCDF_FOTMAT | offset);
>>> +}
>>> +
>>> +static inline int
>>> +qdma_ccdf_get_status(const struct fsl_qdma_format *ccdf)
>>> +{
>>> +       return (le32_to_cpu(ccdf->status) & QDMA_CCDF_MASK) >>
>> QDMA_CCDF_STATUS;
>>> +}
>>> +
>>> +static inline void
>>> +qdma_ccdf_set_ser(struct fsl_qdma_format *ccdf, int status)
>>> +{
>>> +       ccdf->status = cpu_to_le32(QDMA_CCDF_SER | status);
>>> +}
>>> +
>>> +static inline void qdma_csgf_set_len(struct fsl_qdma_format *csgf, int len)
>>> +{
>>> +       csgf->cfg = cpu_to_le32(len & QDMA_SG_LEN_MASK);
>>> +}
>>> +
>>> +static inline void qdma_csgf_set_f(struct fsl_qdma_format *csgf, int len)
>>> +{
>>> +       csgf->cfg = cpu_to_le32(QDMA_SG_FIN | (len &
>> QDMA_SG_LEN_MASK));
>>> +}
>>> +
>>> +static u32 qdma_readl(struct fsl_qdma_engine *qdma, void __iomem
>> *addr)
>>> +{
>>> +       return FSL_DMA_IN(qdma, addr, 32);
>>> +}
>>> +
>>> +static void qdma_writel(struct fsl_qdma_engine *qdma, u32 val,
>>> +                       void __iomem *addr)
>>> +{
>>> +       FSL_DMA_OUT(qdma, addr, val, 32);
>>> +}
>>> +
>>> +static struct fsl_qdma_chan *to_fsl_qdma_chan(struct dma_chan *chan)
>>> +{
>>> +       return container_of(chan, struct fsl_qdma_chan, vchan.chan);
>>> +}
>>> +
>>> +static struct fsl_qdma_comp *to_fsl_qdma_comp(struct virt_dma_desc *vd)
>>> +{
>>> +       return container_of(vd, struct fsl_qdma_comp, vdesc);
>>> +}
>>> +
>>> +static void fsl_qdma_free_chan_resources(struct dma_chan *chan)
>>> +{
>>> +       struct fsl_qdma_chan *fsl_chan = to_fsl_qdma_chan(chan);
>>> +       struct fsl_qdma_queue *fsl_queue = fsl_chan->queue;
>>> +       struct fsl_qdma_engine *fsl_qdma = fsl_chan->qdma;
>>> +       struct fsl_qdma_comp *comp_temp, *_comp_temp;
>>> +       unsigned long flags;
>>> +       LIST_HEAD(head);
>>> +
>>> +       spin_lock_irqsave(&fsl_chan->vchan.lock, flags);
>>> +       vchan_get_all_descriptors(&fsl_chan->vchan, &head);
>>> +       spin_unlock_irqrestore(&fsl_chan->vchan.lock, flags);
>>> +
>>> +       vchan_dma_desc_free_list(&fsl_chan->vchan, &head);
>>> +
>>> +       if (!fsl_queue->comp_pool && !fsl_queue->comp_pool)
>>> +               return;
>>> +
>>> +       list_for_each_entry_safe(comp_temp, _comp_temp,
>>> +                                &fsl_queue->comp_used, list) {
>>> +               dma_pool_free(fsl_queue->comp_pool,
>>> +                             comp_temp->virt_addr,
>>> +                             comp_temp->bus_addr);
>>> +               dma_pool_free(fsl_queue->desc_pool,
>>> +                             comp_temp->desc_virt_addr,
>>> +                             comp_temp->desc_bus_addr);
>>> +               list_del(&comp_temp->list);
>>> +               kfree(comp_temp);
>>> +       }
>>> +
>>> +       list_for_each_entry_safe(comp_temp, _comp_temp,
>>> +                                &fsl_queue->comp_free, list) {
>>> +               dma_pool_free(fsl_queue->comp_pool,
>>> +                             comp_temp->virt_addr,
>>> +                             comp_temp->bus_addr);
>>> +               dma_pool_free(fsl_queue->desc_pool,
>>> +                             comp_temp->desc_virt_addr,
>>> +                             comp_temp->desc_bus_addr);
>>> +               list_del(&comp_temp->list);
>>> +               kfree(comp_temp);
>>> +       }
>>> +
>>> +       dma_pool_destroy(fsl_queue->comp_pool);
>>> +       dma_pool_destroy(fsl_queue->desc_pool);
>>> +
>>> +       fsl_qdma->desc_allocated--;
>>> +       fsl_queue->comp_pool = NULL;
>>> +       fsl_queue->desc_pool = NULL;
>>> +}
>>> +
>>> +static void fsl_qdma_comp_fill_memcpy(struct fsl_qdma_comp *fsl_comp,
>>> +                                     dma_addr_t dst, dma_addr_t
>> src, u32 len)
>>> +{
>>> +       struct fsl_qdma_format *sdf, *ddf;
>>> +       struct fsl_qdma_format *ccdf, *csgf_desc, *csgf_src, *csgf_dest;
>>> +
>>> +       ccdf = fsl_comp->virt_addr;
>>> +       csgf_desc = fsl_comp->virt_addr + 1;
>>> +       csgf_src = fsl_comp->virt_addr + 2;
>>> +       csgf_dest = fsl_comp->virt_addr + 3;
>>> +       sdf = fsl_comp->desc_virt_addr;
>>> +       ddf = fsl_comp->desc_virt_addr + 1;
>>> +
>>> +       memset(fsl_comp->virt_addr, 0,
>> FSL_QDMA_COMMAND_BUFFER_SIZE);
>>> +       memset(fsl_comp->desc_virt_addr, 0,
>> FSL_QDMA_DESCRIPTOR_BUFFER_SIZE);
>>> +       /* Head Command Descriptor(Frame Descriptor) */
>>> +       qdma_desc_addr_set64(ccdf, fsl_comp->bus_addr + 16);
>>> +       qdma_ccdf_set_format(ccdf, qdma_ccdf_get_offset(ccdf));
>>> +       qdma_ccdf_set_ser(ccdf, qdma_ccdf_get_status(ccdf));
>>> +       /* Status notification is enqueued to status queue. */
>>> +       /* Compound Command Descriptor(Frame List Table) */
>>> +       qdma_desc_addr_set64(csgf_desc, fsl_comp->desc_bus_addr);
>>> +       /* It must be 32 as Compound S/G Descriptor */
>>> +       qdma_csgf_set_len(csgf_desc, 32);
>>> +       qdma_desc_addr_set64(csgf_src, src);
>>> +       qdma_csgf_set_len(csgf_src, len);
>>> +       qdma_desc_addr_set64(csgf_dest, dst);
>>> +       qdma_csgf_set_len(csgf_dest, len);
>>> +       /* This entry is the last entry. */
>>> +       qdma_csgf_set_f(csgf_dest, len);
>>> +       /* Descriptor Buffer */
>>> +       sdf->data =
>>> +               cpu_to_le64(FSL_QDMA_CMD_RWTTYPE <<
>>> +                           FSL_QDMA_CMD_RWTTYPE_OFFSET);
>>> +       ddf->data =
>>> +               cpu_to_le64(FSL_QDMA_CMD_RWTTYPE <<
>>> +                           FSL_QDMA_CMD_RWTTYPE_OFFSET);
>>> +       ddf->data |=
>>> +               cpu_to_le64(FSL_QDMA_CMD_LWC <<
>> FSL_QDMA_CMD_LWC_OFFSET);
>>> +}
>>> +
>>> +/*
>>> + * Pre-request full command descriptor for enqueue.
>>> + */
>>> +static int fsl_qdma_pre_request_enqueue_desc(struct fsl_qdma_queue
>> *queue)
>>> +{
>>> +       int i;
>>> +       struct fsl_qdma_comp *comp_temp, *_comp_temp;
>>> +
>>> +       for (i = 0; i < queue->n_cq + FSL_COMMAND_QUEUE_OVERFLLOW;
>> i++) {
>>> +               comp_temp = kzalloc(sizeof(*comp_temp),
>> GFP_KERNEL);
>>> +               if (!comp_temp)
>>> +                       goto err_alloc;
>>> +               comp_temp->virt_addr =
>>> +                       dma_pool_alloc(queue->comp_pool,
>> GFP_KERNEL,
>>> +                                      &comp_temp->bus_addr);
>>> +               if (!comp_temp->virt_addr)
>>> +                       goto err_dma_alloc;
>>> +
>>> +               comp_temp->desc_virt_addr =
>>> +                       dma_pool_alloc(queue->desc_pool,
>> GFP_KERNEL,
>>> +
>> &comp_temp->desc_bus_addr);
>>> +               if (!comp_temp->desc_virt_addr)
>>> +                       goto err_desc_dma_alloc;
>>> +
>>> +               list_add_tail(&comp_temp->list, &queue->comp_free);
>>> +       }
>>> +
>>> +       return 0;
>>> +
>>> +err_desc_dma_alloc:
>>> +       dma_pool_free(queue->comp_pool, comp_temp->virt_addr,
>>> +                     comp_temp->bus_addr);
>>> +
>>> +err_dma_alloc:
>>> +       kfree(comp_temp);
>>> +
>>> +err_alloc:
>>> +       list_for_each_entry_safe(comp_temp, _comp_temp,
>>> +                                &queue->comp_free, list) {
>>> +               if (comp_temp->virt_addr)
>>> +                       dma_pool_free(queue->comp_pool,
>>> +                                     comp_temp->virt_addr,
>>> +                                     comp_temp->bus_addr);
>>> +               if (comp_temp->desc_virt_addr)
>>> +                       dma_pool_free(queue->desc_pool,
>>> +                                     comp_temp->desc_virt_addr,
>>> +
>> comp_temp->desc_bus_addr);
>>> +
>>> +               list_del(&comp_temp->list);
>>> +               kfree(comp_temp);
>>> +       }
>>> +
>>> +       return -ENOMEM;
>>> +}
>>> +
>>> +/*
>>> + * Request a command descriptor for enqueue.
>>> + */
>>> +static struct fsl_qdma_comp
>>> +*fsl_qdma_request_enqueue_desc(struct fsl_qdma_chan *fsl_chan)
>>> +{
>>> +       unsigned long flags;
>>> +       struct fsl_qdma_comp *comp_temp;
>>> +       int timeout = FSL_QDMA_COMP_TIMEOUT;
>>> +       struct fsl_qdma_queue *queue = fsl_chan->queue;
>>> +
>>> +       while (timeout--) {
>>> +               spin_lock_irqsave(&queue->queue_lock, flags);
>>> +               if (!list_empty(&queue->comp_free)) {
>>> +                       comp_temp =
>> list_first_entry(&queue->comp_free,
>>> +                                                    struct
>> fsl_qdma_comp,
>>> +                                                    list);
>>> +                       list_del(&comp_temp->list);
>>> +
>>> +                       spin_unlock_irqrestore(&queue->queue_lock,
>> flags);
>>> +                       comp_temp->qchan = fsl_chan;
>>> +                       return comp_temp;
>>> +               }
>>> +               spin_unlock_irqrestore(&queue->queue_lock, flags);
>>> +               udelay(1);
>>> +       }
>>> +
>>> +       return NULL;
>>> +}
>>> +
>>> +static struct fsl_qdma_queue
>>> +*fsl_qdma_alloc_queue_resources(struct platform_device *pdev,
>>> +                               struct fsl_qdma_engine *fsl_qdma)
>>> +{
>>> +       int ret, len, i, j;
>>> +       int queue_num, block_number;
>>> +       unsigned int queue_size[FSL_QDMA_QUEUE_MAX];
>>> +       struct fsl_qdma_queue *queue_head, *queue_temp;
>>> +
>>> +       queue_num = fsl_qdma->n_queues;
>>> +       block_number = fsl_qdma->block_number;
>>> +
>>> +       if (queue_num > FSL_QDMA_QUEUE_MAX)
>>> +               queue_num = FSL_QDMA_QUEUE_MAX;
>>> +       len = sizeof(*queue_head) * queue_num * block_number;
>>> +       queue_head = devm_kzalloc(&pdev->dev, len, GFP_KERNEL);
>>> +       if (!queue_head)
>>> +               return NULL;
>>> +
>>> +       ret = device_property_read_u32_array(&pdev->dev, "queue-sizes",
>>> +                                            queue_size,
>> queue_num);
>>> +       if (ret) {
>>> +               dev_err(&pdev->dev, "Can't get queue-sizes.\n");
>>> +               return NULL;
>>> +       }
>>> +       for (j = 0; j < block_number; j++) {
>>> +               for (i = 0; i < queue_num; i++) {
>>> +                       if (queue_size[i] >
>> FSL_QDMA_CIRCULAR_DESC_SIZE_MAX ||
>>> +                           queue_size[i] <
>> FSL_QDMA_CIRCULAR_DESC_SIZE_MIN) {
>>> +                               dev_err(&pdev->dev,
>>> +                                       "Get wrong
>> queue-sizes.\n");
>>> +                               return NULL;
>>> +                       }
>>> +                       queue_temp = queue_head + i + (j *
>> queue_num);
>>> +
>>> +                       queue_temp->cq =
>>> +                       dma_alloc_coherent(&pdev->dev,
>>> +                                          sizeof(struct
>> fsl_qdma_format) *
>>> +                                          queue_size[i],
>>> +
>> &queue_temp->bus_addr,
>>> +                                          GFP_KERNEL);
>>> +                       if (!queue_temp->cq)
>>> +                               return NULL;
>>> +                       queue_temp->block_base =
>> fsl_qdma->block_base +
>>> +
>> FSL_QDMA_BLOCK_BASE_OFFSET(fsl_qdma, j);
>>> +                       queue_temp->n_cq = queue_size[i];
>>> +                       queue_temp->id = i;
>>> +                       queue_temp->virt_head = queue_temp->cq;
>>> +                       queue_temp->virt_tail = queue_temp->cq;
>>> +                       /*
>>> +                        * List for queue command buffer
>>> +                        */
>>> +                       INIT_LIST_HEAD(&queue_temp->comp_used);
>>> +                       spin_lock_init(&queue_temp->queue_lock);
>>> +               }
>>> +       }
>>> +       return queue_head;
>>> +}
>>> +
>>> +static struct fsl_qdma_queue
>>> +*fsl_qdma_prep_status_queue(struct platform_device *pdev)
>>> +{
>>> +       int ret;
>>> +       unsigned int status_size;
>>> +       struct fsl_qdma_queue *status_head;
>>> +       struct device_node *np = pdev->dev.of_node;
>>> +
>>> +       ret = of_property_read_u32(np, "status-sizes", &status_size);
>>> +       if (ret) {
>>> +               dev_err(&pdev->dev, "Can't get status-sizes.\n");
>>> +               return NULL;
>>> +       }
>>> +       if (status_size > FSL_QDMA_CIRCULAR_DESC_SIZE_MAX ||
>>> +           status_size < FSL_QDMA_CIRCULAR_DESC_SIZE_MIN) {
>>> +               dev_err(&pdev->dev, "Get wrong status_size.\n");
>>> +               return NULL;
>>> +       }
>>> +       status_head = devm_kzalloc(&pdev->dev,
>>> +                                  sizeof(*status_head),
>> GFP_KERNEL);
>>> +       if (!status_head)
>>> +               return NULL;
>>> +
>>> +       /*
>>> +        * Buffer for queue command
>>> +        */
>>> +       status_head->cq = dma_alloc_coherent(&pdev->dev,
>>> +                                            sizeof(struct
>> fsl_qdma_format) *
>>> +                                            status_size,
>>> +
>> &status_head->bus_addr,
>>> +                                            GFP_KERNEL);
>>> +       if (!status_head->cq) {
>>> +               devm_kfree(&pdev->dev, status_head);
>>> +               return NULL;
>>> +       }
>>> +       status_head->n_cq = status_size;
>>> +       status_head->virt_head = status_head->cq;
>>> +       status_head->virt_tail = status_head->cq;
>>> +       status_head->comp_pool = NULL;
>>> +
>>> +       return status_head;
>>> +}
>>> +
>>> +static int fsl_qdma_halt(struct fsl_qdma_engine *fsl_qdma)
>>> +{
>>> +       u32 reg;
>>> +       int i, j, count = FSL_QDMA_HALT_COUNT;
>>> +       void __iomem *block, *ctrl = fsl_qdma->ctrl_base;
>>> +
>>> +       /* Disable the command queue and wait for idle state. */
>>> +       reg = qdma_readl(fsl_qdma, ctrl + FSL_QDMA_DMR);
>>> +       reg |= FSL_QDMA_DMR_DQD;
>>> +       qdma_writel(fsl_qdma, reg, ctrl + FSL_QDMA_DMR);
>>> +       for (j = 0; j < fsl_qdma->block_number; j++) {
>>> +               block = fsl_qdma->block_base +
>>> +                       FSL_QDMA_BLOCK_BASE_OFFSET(fsl_qdma, j);
>>> +               for (i = 0; i < FSL_QDMA_QUEUE_NUM_MAX; i++)
>>> +                       qdma_writel(fsl_qdma, 0, block +
>> FSL_QDMA_BCQMR(i));
>>> +       }
>>> +       while (1) {
>>> +               reg = qdma_readl(fsl_qdma, ctrl + FSL_QDMA_DSR);
>>> +               if (!(reg & FSL_QDMA_DSR_DB))
>>> +                       break;
>>> +               if (count-- < 0)
>>> +                       return -EBUSY;
>>> +               udelay(100);
>>> +       }
>>> +
>>> +       for (j = 0; j < fsl_qdma->block_number; j++) {
>>> +               block = fsl_qdma->block_base +
>>> +                       FSL_QDMA_BLOCK_BASE_OFFSET(fsl_qdma, j);
>>> +
>>> +               /* Disable status queue. */
>>> +               qdma_writel(fsl_qdma, 0, block + FSL_QDMA_BSQMR);
>>> +
>>> +               /*
>>> +                * clear the command queue interrupt detect register for
>>> +                * all queues.
>>> +                */
>>> +               qdma_writel(fsl_qdma, FSL_QDMA_BCQIDR_CLEAR,
>>> +                           block + FSL_QDMA_BCQIDR(0));
>>> +       }
>>> +
>>> +       return 0;
>>> +}
>>> +
>>> +static int
>>> +fsl_qdma_queue_transfer_complete(struct fsl_qdma_engine *fsl_qdma,
>>> +                                void *block,
>>> +                                int id)
>>> +{
>>> +       bool duplicate;
>>> +       u32 reg, i, count;
>>> +       struct fsl_qdma_queue *temp_queue;
>>> +       struct fsl_qdma_format *status_addr;
>>> +       struct fsl_qdma_comp *fsl_comp = NULL;
>>> +       struct fsl_qdma_queue *fsl_queue = fsl_qdma->queue;
>>> +       struct fsl_qdma_queue *fsl_status = fsl_qdma->status[id];
>>> +
>>> +       count = FSL_QDMA_MAX_SIZE;
>>> +
>>> +       while (count--) {
>>> +               duplicate = 0;
>>> +               reg = qdma_readl(fsl_qdma, block + FSL_QDMA_BSQSR);
>>> +               if (reg & FSL_QDMA_BSQSR_QE)
>>> +                       return 0;
>>> +
>>> +               status_addr = fsl_status->virt_head;
>>> +
>>> +               if (qdma_ccdf_get_queue(status_addr) ==
>>> +                  __this_cpu_read(pre.queue) &&
>>> +                       qdma_ccdf_addr_get64(status_addr) ==
>>> +                       __this_cpu_read(pre.addr))
>>> +                       duplicate = 1;
>>> +               i = qdma_ccdf_get_queue(status_addr) +
>>> +                       id * fsl_qdma->n_queues;
>>> +               __this_cpu_write(pre.addr,
>> qdma_ccdf_addr_get64(status_addr));
>>> +               __this_cpu_write(pre.queue,
>> qdma_ccdf_get_queue(status_addr));
>>> +               temp_queue = fsl_queue + i;
>>> +
>>> +               spin_lock(&temp_queue->queue_lock);
>>> +               if (list_empty(&temp_queue->comp_used)) {
>>> +                       if (!duplicate) {
>>> +
>> spin_unlock(&temp_queue->queue_lock);
>>> +                               return -EAGAIN;
>>> +                       }
>>> +               } else {
>>> +                       fsl_comp =
>> list_first_entry(&temp_queue->comp_used,
>>> +                                                   struct
>> fsl_qdma_comp, list);
>>> +                       if (fsl_comp->bus_addr + 16 !=
>>> +                               __this_cpu_read(pre.addr)) {
>>> +                               if (!duplicate) {
>>> +
>> spin_unlock(&temp_queue->queue_lock);
>>> +                                       return -EAGAIN;
>>> +                               }
>>> +                       }
>>> +               }
>>> +
>>> +               if (duplicate) {
>>> +                       reg = qdma_readl(fsl_qdma, block +
>> FSL_QDMA_BSQMR);
>>> +                       reg |= FSL_QDMA_BSQMR_DI;
>>> +                       qdma_desc_addr_set64(status_addr, 0x0);
>>> +                       fsl_status->virt_head++;
>>> +                       if (fsl_status->virt_head == fsl_status->cq
>>> +                                                  +
>> fsl_status->n_cq)
>>> +                               fsl_status->virt_head =
>> fsl_status->cq;
>>> +                       qdma_writel(fsl_qdma, reg, block +
>> FSL_QDMA_BSQMR);
>>> +                       spin_unlock(&temp_queue->queue_lock);
>>> +                       continue;
>>> +               }
>>> +               list_del(&fsl_comp->list);
>>> +
>>> +               reg = qdma_readl(fsl_qdma, block +
>> FSL_QDMA_BSQMR);
>>> +               reg |= FSL_QDMA_BSQMR_DI;
>>> +               qdma_desc_addr_set64(status_addr, 0x0);
>>> +               fsl_status->virt_head++;
>>> +               if (fsl_status->virt_head == fsl_status->cq +
>> fsl_status->n_cq)
>>> +                       fsl_status->virt_head = fsl_status->cq;
>>> +               qdma_writel(fsl_qdma, reg, block +
>> FSL_QDMA_BSQMR);
>>> +               spin_unlock(&temp_queue->queue_lock);
>>> +
>>> +               spin_lock(&fsl_comp->qchan->vchan.lock);
>>> +               vchan_cookie_complete(&fsl_comp->vdesc);
>>> +               fsl_comp->qchan->status = DMA_COMPLETE;
>>> +               spin_unlock(&fsl_comp->qchan->vchan.lock);
>>> +       }
>>> +
>>> +       return 0;
>>> +}
>>> +
>>> +static irqreturn_t fsl_qdma_error_handler(int irq, void *dev_id)
>>> +{
>>> +       unsigned int intr;
>>> +       struct fsl_qdma_engine *fsl_qdma = dev_id;
>>> +       void __iomem *status = fsl_qdma->status_base;
>>> +
>>> +       intr = qdma_readl(fsl_qdma, status + FSL_QDMA_DEDR);
>>> +
>>> +       if (intr) {
>>> +               dev_err(fsl_qdma->dma_dev.dev, "DMA transaction
>> error!\n");
>>> +               return IRQ_NONE;
>>> +       }
>>> +
>>> +       qdma_writel(fsl_qdma, FSL_QDMA_DEDR_CLEAR, status +
>> FSL_QDMA_DEDR);
>>> +       return IRQ_HANDLED;
>>> +}
>>> +
>>> +static irqreturn_t fsl_qdma_queue_handler(int irq, void *dev_id)
>>> +{
>>> +       int id;
>>> +       unsigned int intr, reg;
>>> +       struct fsl_qdma_engine *fsl_qdma = dev_id;
>>> +       void __iomem *block, *ctrl = fsl_qdma->ctrl_base;
>>> +
>>> +       id = irq - fsl_qdma->irq_base;
>>> +       if (id < 0 && id > fsl_qdma->block_number) {
>>> +               dev_err(fsl_qdma->dma_dev.dev,
>>> +                       "irq %d is wrong irq_base is %d\n",
>>> +                       irq, fsl_qdma->irq_base);
>>> +       }
>>> +
>>> +       block = fsl_qdma->block_base +
>>> +               FSL_QDMA_BLOCK_BASE_OFFSET(fsl_qdma, id);
>>> +
>>> +       intr = qdma_readl(fsl_qdma, block + FSL_QDMA_BCQIDR(0));
>>> +
>>> +       if ((intr & FSL_QDMA_CQIDR_SQT) != 0)
>>> +               intr = fsl_qdma_queue_transfer_complete(fsl_qdma,
>> block, id);
>>> +
>>> +       if (intr != 0) {
>>> +               reg = qdma_readl(fsl_qdma, ctrl + FSL_QDMA_DMR);
>>> +               reg |= FSL_QDMA_DMR_DQD;
>>> +               qdma_writel(fsl_qdma, reg, ctrl + FSL_QDMA_DMR);
>>> +               qdma_writel(fsl_qdma, 0, block +
>> FSL_QDMA_BCQIER(0));
>>> +               dev_err(fsl_qdma->dma_dev.dev, "QDMA: status
>> err!\n");
>>> +       }
>>> +
>>> +       /* Clear all detected events and interrupts. */
>>> +       qdma_writel(fsl_qdma, FSL_QDMA_BCQIDR_CLEAR,
>>> +                   block + FSL_QDMA_BCQIDR(0));
>>> +
>>> +       return IRQ_HANDLED;
>>> +}
>>> +
>>> +static int
>>> +fsl_qdma_irq_init(struct platform_device *pdev,
>>> +                 struct fsl_qdma_engine *fsl_qdma)
>>> +{
>>> +       int i;
>>> +       int cpu;
>>> +       int ret;
>>> +       char irq_name[20];
>>> +
>>> +       fsl_qdma->error_irq =
>>> +               platform_get_irq_byname(pdev, "qdma-error");
>>> +       if (fsl_qdma->error_irq < 0) {
>>> +               dev_err(&pdev->dev, "Can't get qdma controller irq.\n");
>>> +               return fsl_qdma->error_irq;
>>> +       }
>>> +
>>> +       ret = devm_request_irq(&pdev->dev, fsl_qdma->error_irq,
>>> +                              fsl_qdma_error_handler, 0,
>>> +                              "qDMA error", fsl_qdma);
>>> +       if (ret) {
>>> +               dev_err(&pdev->dev, "Can't register qDMA controller
>> IRQ.\n");
>>> +               return  ret;
>>> +       }
>>> +
>>> +       for (i = 0; i < fsl_qdma->block_number; i++) {
>>> +               sprintf(irq_name, "qdma-queue%d", i);
>>> +               fsl_qdma->queue_irq[i] =
>>> +                               platform_get_irq_byname(pdev,
>> irq_name);
>>> +
>>> +               if (fsl_qdma->queue_irq[i] < 0) {
>>> +                       dev_err(&pdev->dev,
>>> +                               "Can't get qdma queue %d irq.\n", i);
>>> +                       return fsl_qdma->queue_irq[i];
>>> +               }
>>> +
>>> +               ret = devm_request_irq(&pdev->dev,
>>> +                                      fsl_qdma->queue_irq[i],
>>> +                                      fsl_qdma_queue_handler,
>>> +                                      0,
>>> +                                      "qDMA queue",
>>> +                                      fsl_qdma);
>>> +               if (ret) {
>>> +                       dev_err(&pdev->dev,
>>> +                               "Can't register qDMA queue
>> IRQ.\n");
>>> +                       return  ret;
>>> +               }
>>> +
>>> +               cpu = i % num_online_cpus();
>>> +               ret = irq_set_affinity_hint(fsl_qdma->queue_irq[i],
>>> +                                           get_cpu_mask(cpu));
>>> +               if (ret) {
>>> +                       dev_err(&pdev->dev,
>>> +                               "Can't set cpu %d affinity to
>> IRQ %d.\n",
>>> +                               cpu,
>>> +                               fsl_qdma->queue_irq[i]);
>>> +                       return  ret;
>>> +               }
>>> +       }
>>> +
>>> +       return 0;
>>> +}
>>> +
>>> +static void fsl_qdma_irq_exit(struct platform_device *pdev,
>>> +                             struct fsl_qdma_engine *fsl_qdma)
>>> +{
>>> +       int i;
>>> +
>>> +       devm_free_irq(&pdev->dev, fsl_qdma->error_irq, fsl_qdma);
>>> +       for (i = 0; i < fsl_qdma->block_number; i++)
>>> +               devm_free_irq(&pdev->dev, fsl_qdma->queue_irq[i],
>> fsl_qdma);
>>> +}
>>> +
>>> +static int fsl_qdma_reg_init(struct fsl_qdma_engine *fsl_qdma)
>>> +{
>>> +       u32 reg;
>>> +       int i, j, ret;
>>> +       struct fsl_qdma_queue *temp;
>>> +       void __iomem *status = fsl_qdma->status_base;
>>> +       void __iomem *block, *ctrl = fsl_qdma->ctrl_base;
>>> +       struct fsl_qdma_queue *fsl_queue = fsl_qdma->queue;
>>> +
>>> +       /* Try to halt the qDMA engine first. */
>>> +       ret = fsl_qdma_halt(fsl_qdma);
>>> +       if (ret) {
>>> +               dev_err(fsl_qdma->dma_dev.dev, "DMA halt failed!");
>>> +               return ret;
>>> +       }
>>> +
>>> +       for (i = 0; i < fsl_qdma->block_number; i++) {
>>> +               /*
>>> +                * Clear the command queue interrupt detect register
>> for
>>> +                * all queues.
>>> +                */
>>> +
>>> +               block = fsl_qdma->block_base +
>>> +                       FSL_QDMA_BLOCK_BASE_OFFSET(fsl_qdma, i);
>>> +               qdma_writel(fsl_qdma, FSL_QDMA_BCQIDR_CLEAR,
>>> +                           block + FSL_QDMA_BCQIDR(0));
>>> +       }
>>> +
>>> +       for (j = 0; j < fsl_qdma->block_number; j++) {
>>> +               block = fsl_qdma->block_base +
>>> +                       FSL_QDMA_BLOCK_BASE_OFFSET(fsl_qdma, j);
>>> +               for (i = 0; i < fsl_qdma->n_queues; i++) {
>>> +                       temp = fsl_queue + i + (j *
>> fsl_qdma->n_queues);
>>> +                       /*
>>> +                        * Initialize Command Queue registers to
>>> +                        * point to the first
>>> +                        * command descriptor in memory.
>>> +                        * Dequeue Pointer Address Registers
>>> +                        * Enqueue Pointer Address Registers
>>> +                        */
>>> +
>>> +                       qdma_writel(fsl_qdma, temp->bus_addr,
>>> +                                   block +
>> FSL_QDMA_BCQDPA_SADDR(i));
>>> +                       qdma_writel(fsl_qdma, temp->bus_addr,
>>> +                                   block +
>> FSL_QDMA_BCQEPA_SADDR(i));
>>> +
>>> +                       /* Initialize the queue mode. */
>>> +                       reg = FSL_QDMA_BCQMR_EN;
>>> +                       reg |=
>> FSL_QDMA_BCQMR_CD_THLD(ilog2(temp->n_cq) - 4);
>>> +                       reg |=
>> FSL_QDMA_BCQMR_CQ_SIZE(ilog2(temp->n_cq) - 6);
>>> +                       qdma_writel(fsl_qdma, reg, block +
>> FSL_QDMA_BCQMR(i));
>>> +               }
>>> +
>>> +               /*
>>> +                * Workaround for erratum: ERR010812.
>>> +                * We must enable XOFF to avoid the enqueue rejection
>> occurs.
>>> +                * Setting SQCCMR ENTER_WM to 0x20.
>>> +                */
>>> +
>>> +               qdma_writel(fsl_qdma,
>> FSL_QDMA_SQCCMR_ENTER_WM,
>>> +                           block + FSL_QDMA_SQCCMR);
>>> +
>>> +               /*
>>> +                * Initialize status queue registers to point to the first
>>> +                * command descriptor in memory.
>>> +                * Dequeue Pointer Address Registers
>>> +                * Enqueue Pointer Address Registers
>>> +                */
>>> +
>>> +               qdma_writel(fsl_qdma, fsl_qdma->status[j]->bus_addr,
>>> +                           block + FSL_QDMA_SQEPAR);
>>> +               qdma_writel(fsl_qdma, fsl_qdma->status[j]->bus_addr,
>>> +                           block + FSL_QDMA_SQDPAR);
>>> +               /* Initialize status queue interrupt. */
>>> +               qdma_writel(fsl_qdma, FSL_QDMA_BCQIER_CQTIE,
>>> +                           block + FSL_QDMA_BCQIER(0));
>>> +               qdma_writel(fsl_qdma, FSL_QDMA_BSQICR_ICEN |
>>> +                                  FSL_QDMA_BSQICR_ICST(5) |
>> 0x8000,
>>> +                                  block + FSL_QDMA_BSQICR);
>>> +               qdma_writel(fsl_qdma, FSL_QDMA_CQIER_MEIE |
>>> +                                  FSL_QDMA_CQIER_TEIE,
>>> +                                  block + FSL_QDMA_CQIER);
>>> +
>>> +               /* Initialize the status queue mode. */
>>> +               reg = FSL_QDMA_BSQMR_EN;
>>> +               reg |= FSL_QDMA_BSQMR_CQ_SIZE(ilog2
>>> +                       (fsl_qdma->status[j]->n_cq) - 6);
>>> +
>>> +               qdma_writel(fsl_qdma, reg, block +
>> FSL_QDMA_BSQMR);
>>> +               reg = qdma_readl(fsl_qdma, block +
>> FSL_QDMA_BSQMR);
>>> +       }
>>> +
>>> +       /* Initialize controller interrupt register. */
>>> +       qdma_writel(fsl_qdma, FSL_QDMA_DEDR_CLEAR, status +
>> FSL_QDMA_DEDR);
>>> +       qdma_writel(fsl_qdma, FSL_QDMA_DEIER_CLEAR, status +
>> FSL_QDMA_DEIER);
>>> +
>>> +       reg = qdma_readl(fsl_qdma, ctrl + FSL_QDMA_DMR);
>>> +       reg &= ~FSL_QDMA_DMR_DQD;
>>> +       qdma_writel(fsl_qdma, reg, ctrl + FSL_QDMA_DMR);
>>> +
>>> +       return 0;
>>> +}
>>> +
>>> +static struct dma_async_tx_descriptor *
>>> +fsl_qdma_prep_memcpy(struct dma_chan *chan, dma_addr_t dst,
>>> +                    dma_addr_t src, size_t len, unsigned long flags)
>>> +{
>>> +       struct fsl_qdma_comp *fsl_comp;
>>> +       struct fsl_qdma_chan *fsl_chan = to_fsl_qdma_chan(chan);
>>> +
>>> +       fsl_comp = fsl_qdma_request_enqueue_desc(fsl_chan);
>>> +
>>> +       if (!fsl_comp)
>>> +               return NULL;
>>> +
>>> +       fsl_qdma_comp_fill_memcpy(fsl_comp, dst, src, len);
>>> +
>>> +       return vchan_tx_prep(&fsl_chan->vchan, &fsl_comp->vdesc, flags);
>>> +}
>>> +
>>> +static void fsl_qdma_enqueue_desc(struct fsl_qdma_chan *fsl_chan)
>>> +{
>>> +       u32 reg;
>>> +       struct virt_dma_desc *vdesc;
>>> +       struct fsl_qdma_comp *fsl_comp;
>>> +       struct fsl_qdma_queue *fsl_queue = fsl_chan->queue;
>>> +       void __iomem *block = fsl_queue->block_base;
>>> +
>>> +       reg = qdma_readl(fsl_chan->qdma, block +
>> FSL_QDMA_BCQSR(fsl_queue->id));
>>> +       if (reg & (FSL_QDMA_BCQSR_QF | FSL_QDMA_BCQSR_XOFF))
>>> +               return;
>>> +       vdesc = vchan_next_desc(&fsl_chan->vchan);
>>> +       if (!vdesc)
>>> +               return;
>>> +       list_del(&vdesc->node);
>>> +       fsl_comp = to_fsl_qdma_comp(vdesc);
>>> +
>>> +       memcpy(fsl_queue->virt_head++,
>>> +              fsl_comp->virt_addr, sizeof(struct fsl_qdma_format));
>>> +       if (fsl_queue->virt_head == fsl_queue->cq + fsl_queue->n_cq)
>>> +               fsl_queue->virt_head = fsl_queue->cq;
>>> +
>>> +       list_add_tail(&fsl_comp->list, &fsl_queue->comp_used);
>>> +       barrier();
>>> +       reg = qdma_readl(fsl_chan->qdma, block +
>> FSL_QDMA_BCQMR(fsl_queue->id));
>>> +       reg |= FSL_QDMA_BCQMR_EI;
>>> +       qdma_writel(fsl_chan->qdma, reg, block +
>> FSL_QDMA_BCQMR(fsl_queue->id));
>>> +       fsl_chan->status = DMA_IN_PROGRESS;
>>> +}
>>> +
>>> +static void fsl_qdma_free_desc(struct virt_dma_desc *vdesc)
>>> +{
>>> +       unsigned long flags;
>>> +       struct fsl_qdma_comp *fsl_comp;
>>> +       struct fsl_qdma_queue *fsl_queue;
>>> +
>>> +       fsl_comp = to_fsl_qdma_comp(vdesc);
>>> +       fsl_queue = fsl_comp->qchan->queue;
>>> +
>>> +       spin_lock_irqsave(&fsl_queue->queue_lock, flags);
>>> +       list_add_tail(&fsl_comp->list, &fsl_queue->comp_free);
>>> +       spin_unlock_irqrestore(&fsl_queue->queue_lock, flags);
>>> +}
>>> +
>>> +static void fsl_qdma_issue_pending(struct dma_chan *chan)
>>> +{
>>> +       unsigned long flags;
>>> +       struct fsl_qdma_chan *fsl_chan = to_fsl_qdma_chan(chan);
>>> +       struct fsl_qdma_queue *fsl_queue = fsl_chan->queue;
>>> +
>>> +       spin_lock_irqsave(&fsl_queue->queue_lock, flags);
>>> +       spin_lock(&fsl_chan->vchan.lock);
>>> +       if (vchan_issue_pending(&fsl_chan->vchan))
>>> +               fsl_qdma_enqueue_desc(fsl_chan);
>>> +       spin_unlock(&fsl_chan->vchan.lock);
>>> +       spin_unlock_irqrestore(&fsl_queue->queue_lock, flags);
>>> +}
>>> +
>>> +static void fsl_qdma_synchronize(struct dma_chan *chan)
>>> +{
>>> +       struct fsl_qdma_chan *fsl_chan = to_fsl_qdma_chan(chan);
>>> +
>>> +       vchan_synchronize(&fsl_chan->vchan);
>>> +}
>>> +
>>> +static int fsl_qdma_terminate_all(struct dma_chan *chan)
>>> +{
>>> +       LIST_HEAD(head);
>>> +       unsigned long flags;
>>> +       struct fsl_qdma_chan *fsl_chan = to_fsl_qdma_chan(chan);
>>> +
>>> +       spin_lock_irqsave(&fsl_chan->vchan.lock, flags);
>>> +       vchan_get_all_descriptors(&fsl_chan->vchan, &head);
>>> +       spin_unlock_irqrestore(&fsl_chan->vchan.lock, flags);
>>> +       vchan_dma_desc_free_list(&fsl_chan->vchan, &head);
>>> +       return 0;
>>> +}
>>> +
>>> +static int fsl_qdma_alloc_chan_resources(struct dma_chan *chan)
>>> +{
>>> +       int ret;
>>> +       struct fsl_qdma_chan *fsl_chan = to_fsl_qdma_chan(chan);
>>> +       struct fsl_qdma_engine *fsl_qdma = fsl_chan->qdma;
>>> +       struct fsl_qdma_queue *fsl_queue = fsl_chan->queue;
>>> +
>>> +       if (fsl_queue->comp_pool && fsl_queue->desc_pool)
>>> +               return fsl_qdma->desc_allocated;
>>> +
>>> +       INIT_LIST_HEAD(&fsl_queue->comp_free);
>>> +
>>> +       /*
>>> +        * The dma pool for queue command buffer
>>> +        */
>>> +       fsl_queue->comp_pool =
>>> +       dma_pool_create("comp_pool",
>>> +                       chan->device->dev,
>>> +                       FSL_QDMA_COMMAND_BUFFER_SIZE,
>>> +                       64, 0);
>>> +       if (!fsl_queue->comp_pool)
>>> +               return -ENOMEM;
>>> +
>>> +       /*
>>> +        * The dma pool for Descriptor(SD/DD) buffer
>>> +        */
>>> +       fsl_queue->desc_pool =
>>> +       dma_pool_create("desc_pool",
>>> +                       chan->device->dev,
>>> +                       FSL_QDMA_DESCRIPTOR_BUFFER_SIZE,
>>> +                       32, 0);
>>> +       if (!fsl_queue->desc_pool)
>>> +               goto err_desc_pool;
>>> +
>>> +       ret = fsl_qdma_pre_request_enqueue_desc(fsl_queue);
>>> +       if (ret) {
>>> +               dev_err(chan->device->dev,
>>> +                       "failed to alloc dma buffer for S/G
>> descriptor\n");
>>> +               goto err_mem;
>>> +       }
>>> +
>>> +       fsl_qdma->desc_allocated++;
>>> +       return fsl_qdma->desc_allocated;
>>> +
>>> +err_mem:
>>> +       dma_pool_destroy(fsl_queue->desc_pool);
>>> +err_desc_pool:
>>> +       dma_pool_destroy(fsl_queue->comp_pool);
>>> +       return -ENOMEM;
>>> +}
>>> +
>>> +static int fsl_qdma_probe(struct platform_device *pdev)
>>> +{
>>> +       int ret, i;
>>> +       int blk_num, blk_off;
>>> +       u32 len, chans, queues;
>>> +       struct resource *res;
>>> +       struct fsl_qdma_chan *fsl_chan;
>>> +       struct fsl_qdma_engine *fsl_qdma;
>>> +       struct device_node *np = pdev->dev.of_node;
>>> +
>>> +       ret = of_property_read_u32(np, "dma-channels", &chans);
>>> +       if (ret) {
>>> +               dev_err(&pdev->dev, "Can't get dma-channels.\n");
>>> +               return ret;
>>> +       }
>>> +
>>> +       ret = of_property_read_u32(np, "block-offset", &blk_off);
>>> +       if (ret) {
>>> +               dev_err(&pdev->dev, "Can't get block-offset.\n");
>>> +               return ret;
>>> +       }
>>> +
>>> +       ret = of_property_read_u32(np, "block-number", &blk_num);
>>> +       if (ret) {
>>> +               dev_err(&pdev->dev, "Can't get block-number.\n");
>>> +               return ret;
>>> +       }
>>> +
>>> +       blk_num = min_t(int, blk_num, num_online_cpus());
>>> +
>>> +       len = sizeof(*fsl_qdma);
>>> +       fsl_qdma = devm_kzalloc(&pdev->dev, len, GFP_KERNEL);
>>> +       if (!fsl_qdma)
>>> +               return -ENOMEM;
>>> +
>>> +       len = sizeof(*fsl_chan) * chans;
>>> +       fsl_qdma->chans = devm_kzalloc(&pdev->dev, len, GFP_KERNEL);
>>> +       if (!fsl_qdma->chans)
>>> +               return -ENOMEM;
>>> +
>>> +       len = sizeof(struct fsl_qdma_queue *) * blk_num;
>>> +       fsl_qdma->status = devm_kzalloc(&pdev->dev, len, GFP_KERNEL);
>>> +       if (!fsl_qdma->status)
>>> +               return -ENOMEM;
>>> +
>>> +       len = sizeof(int) * blk_num;
>>> +       fsl_qdma->queue_irq = devm_kzalloc(&pdev->dev, len,
>> GFP_KERNEL);
>>> +       if (!fsl_qdma->queue_irq)
>>> +               return -ENOMEM;
>>> +
>>> +       ret = of_property_read_u32(np, "fsl,dma-queues", &queues);
>>> +       if (ret) {
>>> +               dev_err(&pdev->dev, "Can't get queues.\n");
>>> +               return ret;
>>> +       }
>>> +
>>> +       fsl_qdma->desc_allocated = 0;
>>> +       fsl_qdma->n_chans = chans;
>>> +       fsl_qdma->n_queues = queues;
>>> +       fsl_qdma->block_number = blk_num;
>>> +       fsl_qdma->block_offset = blk_off;
>>> +
>>> +       mutex_init(&fsl_qdma->fsl_qdma_mutex);
>>> +
>>> +       for (i = 0; i < fsl_qdma->block_number; i++) {
>>> +               fsl_qdma->status[i] =
>> fsl_qdma_prep_status_queue(pdev);
>>> +               if (!fsl_qdma->status[i])
>>> +                       return -ENOMEM;
>>> +       }
>>> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> +       fsl_qdma->ctrl_base = devm_ioremap_resource(&pdev->dev, res);
>>> +       if (IS_ERR(fsl_qdma->ctrl_base))
>>> +               return PTR_ERR(fsl_qdma->ctrl_base);
>>> +
>>> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>>> +       fsl_qdma->status_base = devm_ioremap_resource(&pdev->dev,
>> res);
>>> +       if (IS_ERR(fsl_qdma->status_base))
>>> +               return PTR_ERR(fsl_qdma->status_base);
>>> +
>>> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
>>> +       fsl_qdma->block_base = devm_ioremap_resource(&pdev->dev,
>> res);
>>> +       if (IS_ERR(fsl_qdma->block_base))
>>> +               return PTR_ERR(fsl_qdma->block_base);
>>> +       fsl_qdma->queue = fsl_qdma_alloc_queue_resources(pdev,
>> fsl_qdma);
>>> +       if (!fsl_qdma->queue)
>>> +               return -ENOMEM;
>>> +
>>> +       ret = fsl_qdma_irq_init(pdev, fsl_qdma);
>>> +       if (ret)
>>> +               return ret;
>>> +
>>> +       fsl_qdma->irq_base = platform_get_irq_byname(pdev,
>> "qdma-queue0");
>>> +       fsl_qdma->feature = of_property_read_bool(np, "big-endian");
>>> +       INIT_LIST_HEAD(&fsl_qdma->dma_dev.channels);
>>> +
>>> +       for (i = 0; i < fsl_qdma->n_chans; i++) {
>>> +               struct fsl_qdma_chan *fsl_chan = &fsl_qdma->chans[i];
>>> +
>>> +               fsl_chan->qdma = fsl_qdma;
>>> +               fsl_chan->queue = fsl_qdma->queue + i %
>> (fsl_qdma->n_queues *
>>> +
>> fsl_qdma->block_number);
>>> +               fsl_chan->vchan.desc_free = fsl_qdma_free_desc;
>>> +               vchan_init(&fsl_chan->vchan, &fsl_qdma->dma_dev);
>>> +       }
>>> +
>>> +       dma_cap_set(DMA_MEMCPY, fsl_qdma->dma_dev.cap_mask);
>>> +
>>> +       fsl_qdma->dma_dev.dev = &pdev->dev;
>>> +       fsl_qdma->dma_dev.device_free_chan_resources =
>>> +               fsl_qdma_free_chan_resources;
>>> +       fsl_qdma->dma_dev.device_alloc_chan_resources =
>>> +               fsl_qdma_alloc_chan_resources;
>>> +       fsl_qdma->dma_dev.device_tx_status = dma_cookie_status;
>>> +       fsl_qdma->dma_dev.device_prep_dma_memcpy =
>> fsl_qdma_prep_memcpy;
>>> +       fsl_qdma->dma_dev.device_issue_pending =
>> fsl_qdma_issue_pending;
>>> +       fsl_qdma->dma_dev.device_synchronize = fsl_qdma_synchronize;
>>> +       fsl_qdma->dma_dev.device_terminate_all =
>> fsl_qdma_terminate_all;
>>> +
>>> +       dma_set_mask(&pdev->dev, DMA_BIT_MASK(40));
>>> +
>>> +       platform_set_drvdata(pdev, fsl_qdma);
>>> +
>>> +       ret = dma_async_device_register(&fsl_qdma->dma_dev);
>>> +       if (ret) {
>>> +               dev_err(&pdev->dev,
>>> +                       "Can't register NXP Layerscape qDMA
>> engine.\n");
>>> +               return ret;
>>> +       }
>>> +
>>> +       ret = fsl_qdma_reg_init(fsl_qdma);
>>> +       if (ret) {
>>> +               dev_err(&pdev->dev, "Can't Initialize the qDMA
>> engine.\n");
>>> +               return ret;
>>> +       }
>>> +
>>> +       return 0;
>>> +}
>>> +
>>> +static void fsl_qdma_cleanup_vchan(struct dma_device *dmadev)
>>> +{
>>> +       struct fsl_qdma_chan *chan, *_chan;
>>> +
>>> +       list_for_each_entry_safe(chan, _chan,
>>> +                                &dmadev->channels,
>> vchan.chan.device_node) {
>>> +               list_del(&chan->vchan.chan.device_node);
>>> +               tasklet_kill(&chan->vchan.task);
>>> +       }
>>> +}
>>> +
>>> +static int fsl_qdma_remove(struct platform_device *pdev)
>>> +{
>>> +       int i;
>>> +       struct fsl_qdma_queue *status;
>>> +       struct device_node *np = pdev->dev.of_node;
>>> +       struct fsl_qdma_engine *fsl_qdma = platform_get_drvdata(pdev);
>>> +
>>> +       fsl_qdma_irq_exit(pdev, fsl_qdma);
>>> +       fsl_qdma_cleanup_vchan(&fsl_qdma->dma_dev);
>>> +       of_dma_controller_free(np);
>>> +       dma_async_device_unregister(&fsl_qdma->dma_dev);
>>> +
>>> +       for (i = 0; i < fsl_qdma->block_number; i++) {
>>> +               status = fsl_qdma->status[i];
>>> +               dma_free_coherent(&pdev->dev, sizeof(struct
>> fsl_qdma_format) *
>>> +                               status->n_cq, status->cq,
>> status->bus_addr);
>>> +       }
>>> +       return 0;
>>> +}
>>> +
>>> +static const struct of_device_id fsl_qdma_dt_ids[] = {
>>> +       { .compatible = "fsl,ls1021a-qdma", },
>>> +       { /* sentinel */ }
>>> +};
>>> +MODULE_DEVICE_TABLE(of, fsl_qdma_dt_ids);
>>> +
>>> +static struct platform_driver fsl_qdma_driver = {
>>> +       .driver         = {
>>> +               .name   = "fsl-qdma",
>>> +               .of_match_table = fsl_qdma_dt_ids,
>>> +       },
>>> +       .probe          = fsl_qdma_probe,
>>> +       .remove         = fsl_qdma_remove,
>>> +};
>>> +
>>> +module_platform_driver(fsl_qdma_driver);
>>> +
>>> +MODULE_ALIAS("platform:fsl-qdma");
>>> +MODULE_DESCRIPTION("NXP Layerscape qDMA engine driver");
>>> --
>>> 1.7.1
>>> 

^ permalink raw reply

* Re: [RFC PATCH] seccomp: Add protection keys into seccomp_data
From: Kees Cook @ 2018-10-29 16:25 UTC (permalink / raw)
  To: Michael Sammler
  Cc: Will Drewry, Linux API, Dave Hansen, Ram Pai, Andy Lutomirski,
	PowerPC
In-Reply-To: <20181029112343.27454-1-msammler@mpi-sws.org>

On Mon, Oct 29, 2018 at 4:23 AM, Michael Sammler <msammler@mpi-sws.org> wrote:
> Add the current value of an architecture specific protection keys
> register (currently PKRU on x86) to data available for seccomp-bpf
> programs to work on. This allows filters based on the currently
> enabled protection keys.
>
> Support for protection keys on the POWER architecture is not part of
> this patch since I do not have access to a PowerPC, but adding support
> for it can be achieved by setting sd->pkeys to the AMR register in
> populate_seccomp_data.
>
> One use case for this patch is disabling unnecessary system calls for a
> library (e.g. network i/o for a crypto library) while the library runs
> without disabling the system calls for the whole program (by changing
> the protection keys before and after the library executes). Using this
> one could ensure that the library behaves a expected (e.g. the crypto
> library not sending private keys to a malicious server).
>
> This patch also enables lightweight sandboxing of untrusted code using
> memory protection keys: Protection keys provide memory isolation but
> for a sandbox system call isolation is needed as well. This patch
> allows writing a seccomp filter to prevent system calls by the
> untrusted code while still allowing system calls for the trusted code.

Isn't PKU instruction based? Couldn't a malicious library just change
the state of the MPK registers? This seems like an easy way to bypass
any filters that used PKU. I'm not convinced this is a meaningful
barrier that should be enforced by seccomp.

This can also be done with a signal handler with SECCOMP_RET_TRAP and
check instruction pointer vs PKU state.

-Kees

>
> An alternative design would be to extend (c)BPF with a new instruction
> to read the state of a protection key. This alternate design would
> provide a simpler interface to the user space since the BPF program
> would not need to deal with the architecture specific pkeys field in
> seccomp_data, but the question is, how much of an advantage this would
> be as the nr field in seccomp_data is already architecture specific.
> Adding a new instruction for BPF programs is more complicated than
> this patch and might be a breaking change.
>
> Results of selftests/seccomp_benchmark.c on a x86 machine with pkeys
> support:
>
> With patch:
> Benchmarking 33554432 samples...
> 28.019505558 - 18.676858522 = 9342647036
> getpid native: 278 ns
> 42.279109885 - 28.019657031 = 14259452854
> getpid RET_ALLOW: 424 ns
> Estimated seccomp overhead per syscall: 146 ns
>
> Without patch:
> Benchmarking 33554432 samples...
> 28.059619466 - 18.706769155 = 9352850311
> getpid native: 278 ns
> 42.299228279 - 28.059761804 = 14239466475
> getpid RET_ALLOW: 424 ns
> Estimated seccomp overhead per syscall: 146 ns
>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Andy Lutomirski <luto@amacapital.net>
> Cc: Will Drewry <wad@chromium.org>
> Cc: Ram Pai <linuxram@us.ibm.com>
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux-api@vger.kernel.org
> Signed-off-by: Michael Sammler <msammler@mpi-sws.org>
> ---
> Changes to the previous version:
> - added motivation, notes about POWER, alternative design and benchmark results to the commit log
> - renamed pkru field in seccomp_data to pkeys
> - changed size of pkru field to __u64 and removed reserved field
> - added test for x86
>
>  arch/mips/kernel/ptrace.c                     |   1 +
>  arch/x86/entry/common.c                       |   1 +
>  include/uapi/linux/seccomp.h                  |   3 +
>  kernel/seccomp.c                              |   1 +
>  tools/testing/selftests/seccomp/seccomp_bpf.c | 107 +++++++++++++++++++++++++-
>  5 files changed, 112 insertions(+), 1 deletion(-)
>
> diff --git a/arch/mips/kernel/ptrace.c b/arch/mips/kernel/ptrace.c
> index e5ba56c0..a58dd04d 100644
> --- a/arch/mips/kernel/ptrace.c
> +++ b/arch/mips/kernel/ptrace.c
> @@ -1277,6 +1277,7 @@ asmlinkage long syscall_trace_enter(struct pt_regs *regs, long syscall)
>                 for (i = 0; i < 6; i++)
>                         sd.args[i] = args[i];
>                 sd.instruction_pointer = KSTK_EIP(current);
> +               sd.pkeys = 0;
>
>                 ret = __secure_computing(&sd);
>                 if (ret == -1)
> diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
> index 3b2490b8..20c51bf2 100644
> --- a/arch/x86/entry/common.c
> +++ b/arch/x86/entry/common.c
> @@ -98,6 +98,7 @@ static long syscall_trace_enter(struct pt_regs *regs)
>                 sd.arch = arch;
>                 sd.nr = regs->orig_ax;
>                 sd.instruction_pointer = regs->ip;
> +               sd.pkeys = read_pkru();
>  #ifdef CONFIG_X86_64
>                 if (arch == AUDIT_ARCH_X86_64) {
>                         sd.args[0] = regs->di;
> diff --git a/include/uapi/linux/seccomp.h b/include/uapi/linux/seccomp.h
> index 9efc0e73..3aa2d934 100644
> --- a/include/uapi/linux/seccomp.h
> +++ b/include/uapi/linux/seccomp.h
> @@ -52,12 +52,15 @@
>   * @instruction_pointer: at the time of the system call.
>   * @args: up to 6 system call arguments always stored as 64-bit values
>   *        regardless of the architecture.
> + * @pkeys: value of an architecture specific protection keys register
> + *         (currently PKRU on x86)
>   */
>  struct seccomp_data {
>         int nr;
>         __u32 arch;
>         __u64 instruction_pointer;
>         __u64 args[6];
> +       __u64 pkeys;
>  };
>
>  #endif /* _UAPI_LINUX_SECCOMP_H */
> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> index fd023ac2..dfb8b0d6 100644
> --- a/kernel/seccomp.c
> +++ b/kernel/seccomp.c
> @@ -91,6 +91,7 @@ static void populate_seccomp_data(struct seccomp_data *sd)
>         sd->args[4] = args[4];
>         sd->args[5] = args[5];
>         sd->instruction_pointer = KSTK_EIP(task);
> +       sd->pkeys = 0;
>  }
>
>  /**
> diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c
> index e1473234..f7f8fa6f 100644
> --- a/tools/testing/selftests/seccomp/seccomp_bpf.c
> +++ b/tools/testing/selftests/seccomp/seccomp_bpf.c
> @@ -82,6 +82,7 @@ struct seccomp_data {
>         __u32 arch;
>         __u64 instruction_pointer;
>         __u64 args[6];
> +       __u64 pkeys;
>  };
>  #endif
>
> @@ -732,7 +733,9 @@ TEST(KILL_process)
>  TEST(arg_out_of_range)
>  {
>         struct sock_filter filter[] = {
> -               BPF_STMT(BPF_LD|BPF_W|BPF_ABS, syscall_arg(6)),
> +               BPF_STMT(BPF_LD|BPF_W|BPF_ABS,
> +                       offsetof(struct seccomp_data, pkeys)
> +                               + sizeof(__u64)),
>                 BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ALLOW),
>         };
>         struct sock_fprog prog = {
> @@ -2933,6 +2936,108 @@ skip:
>         ASSERT_EQ(0, kill(pid, SIGKILL));
>  }
>
> +#if defined(__i386__) || defined(__x86_64__)
> +static inline void __cpuid(unsigned int *eax, unsigned int *ebx,
> +               unsigned int *ecx, unsigned int *edx)
> +{
> +       /* ecx is often an input as well as an output. */
> +       asm volatile(
> +               "cpuid;"
> +               : "=a" (*eax),
> +                 "=b" (*ebx),
> +                 "=c" (*ecx),
> +                 "=d" (*edx)
> +               : "0" (*eax), "2" (*ecx));
> +}
> +
> +/* Intel-defined CPU features, CPUID level 0x00000007:0 (ecx) */
> +#define X86_FEATURE_PKU        (1<<3) /* Protection Keys for Userspace */
> +#define X86_FEATURE_OSPKE      (1<<4) /* OS Protection Keys Enable */
> +
> +static inline int cpu_has_pku(void)
> +{
> +       unsigned int eax;
> +       unsigned int ebx;
> +       unsigned int ecx;
> +       unsigned int edx;
> +
> +       eax = 0x7;
> +       ecx = 0x0;
> +       __cpuid(&eax, &ebx, &ecx, &edx);
> +
> +       if (!(ecx & X86_FEATURE_PKU))
> +               return 0;
> +       if (!(ecx & X86_FEATURE_OSPKE))
> +               return 0;
> +       return 1;
> +}
> +
> +static inline __u32 read_pkru(void)
> +{
> +       if (!cpu_has_pku())
> +               return 0;
> +
> +       __u32 ecx = 0;
> +       __u32 edx, pkru;
> +
> +       /*
> +        * "rdpkru" instruction.  Places PKRU contents in to EAX,
> +        * clears EDX and requires that ecx=0.
> +        */
> +       asm volatile(".byte 0x0f,0x01,0xee\n\t"
> +                    : "=a" (pkru), "=d" (edx)
> +                    : "c" (ecx));
> +       return pkru;
> +}
> +
> +static inline void write_pkru(__u32 pkru)
> +{
> +       if (!cpu_has_pku())
> +               return;
> +
> +       __u32 ecx = 0, edx = 0;
> +
> +       /*
> +        * "wrpkru" instruction.  Loads contents in EAX to PKRU,
> +        * requires that ecx = edx = 0.
> +        */
> +       asm volatile(".byte 0x0f,0x01,0xef\n\t"
> +                    : : "a" (pkru), "c"(ecx), "d"(edx));
> +}
> +
> +#define TEST_PKRU 0x55555550
> +
> +TEST_SIGNAL(pkeys_set, SIGSYS)
> +{
> +       write_pkru(TEST_PKRU);
> +       /* read back the written value because pkru might not be supported */
> +       __u32 pkru = read_pkru();
> +
> +       struct sock_filter filter[] = {
> +               BPF_STMT(BPF_LD|BPF_W|BPF_ABS,
> +                       offsetof(struct seccomp_data, pkeys)),
> +               BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, pkru, 1, 0),
> +               BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ALLOW),
> +               BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_KILL),
> +       };
> +       struct sock_fprog prog = {
> +               .len = (unsigned short)ARRAY_SIZE(filter),
> +               .filter = filter,
> +       };
> +       long ret;
> +
> +       ret = prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0);
> +       ASSERT_EQ(0, ret);
> +
> +       ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog);
> +       ASSERT_EQ(0, ret);
> +
> +       /* should never return. */
> +       EXPECT_EQ(0, syscall(__NR_getpid));
> +}
> +#endif
> +
> +
>  /*
>   * TODO:
>   * - add microbenchmarks
> --
> 2.11.0



-- 
Kees Cook

^ permalink raw reply

* Re: [RFC PATCH] seccomp: Add protection keys into seccomp_data
From: Dave Hansen @ 2018-10-29 16:37 UTC (permalink / raw)
  To: Kees Cook, Michael Sammler
  Cc: Will Drewry, Linux API, Dave Hansen, Ram Pai, Andy Lutomirski,
	PowerPC
In-Reply-To: <CAGXu5jJ2fFmEXfUzga3XBXcsHPuPWWf5zLeyn8+Z1oZVa8oTuQ@mail.gmail.com>

On 10/29/18 9:25 AM, Kees Cook wrote:
> On Mon, Oct 29, 2018 at 4:23 AM, Michael Sammler <msammler@mpi-sws.org> wrote:
>> Add the current value of an architecture specific protection keys
>> register (currently PKRU on x86) to data available for seccomp-bpf
>> programs to work on. This allows filters based on the currently
>> enabled protection keys.

How does the current "assignment" of protection keys to the various uses
get communicated to the filter?

I'm not sure this is a great use for PKRU.  I *think* the basic problem
is that you want to communicate some rights information down into a
filter, and you want to communicate it with PKRU.  While it's handy to
have an extra register that nobody (generally) mucks with, I'm not quite
convinced that we want to repurpose it this way.

Also, I'm not sure the kernel provides the PKRU guarantees you want at
the moment.  Our implementation *probably* works, but it's mostly by
accident.

^ permalink raw reply

* Re: [RFC PATCH] seccomp: Add protection keys into seccomp_data
From: Jann Horn @ 2018-10-29 16:42 UTC (permalink / raw)
  To: Kees Cook
  Cc: wad, Linux API, Dave Hansen, linuxram, Andy Lutomirski, msammler,
	linuxppc-dev
In-Reply-To: <CAGXu5jJ2fFmEXfUzga3XBXcsHPuPWWf5zLeyn8+Z1oZVa8oTuQ@mail.gmail.com>

On Mon, Oct 29, 2018 at 5:25 PM Kees Cook <keescook@chromium.org> wrote:
> On Mon, Oct 29, 2018 at 4:23 AM, Michael Sammler <msammler@mpi-sws.org> wrote:
> > Add the current value of an architecture specific protection keys
> > register (currently PKRU on x86) to data available for seccomp-bpf
> > programs to work on. This allows filters based on the currently
> > enabled protection keys.
> >
> > Support for protection keys on the POWER architecture is not part of
> > this patch since I do not have access to a PowerPC, but adding support
> > for it can be achieved by setting sd->pkeys to the AMR register in
> > populate_seccomp_data.
> >
> > One use case for this patch is disabling unnecessary system calls for a
> > library (e.g. network i/o for a crypto library) while the library runs
> > without disabling the system calls for the whole program (by changing
> > the protection keys before and after the library executes). Using this
> > one could ensure that the library behaves a expected (e.g. the crypto
> > library not sending private keys to a malicious server).
> >
> > This patch also enables lightweight sandboxing of untrusted code using
> > memory protection keys: Protection keys provide memory isolation but
> > for a sandbox system call isolation is needed as well. This patch
> > allows writing a seccomp filter to prevent system calls by the
> > untrusted code while still allowing system calls for the trusted code.
>
> Isn't PKU instruction based? Couldn't a malicious library just change
> the state of the MPK registers? This seems like an easy way to bypass
> any filters that used PKU. I'm not convinced this is a meaningful
> barrier that should be enforced by seccomp.

The idea is that if you know that there are no unwanted WRPKRU
instructions in your code, and all WRPKRU instructions in your code
are directly followed by code that verifies correctness of the
protection key, then you can implement lightweight sandboxes inside a
single process. See https://arxiv.org/pdf/1801.06822.pdf for one
description of this approach.

> This can also be done with a signal handler with SECCOMP_RET_TRAP and
> check instruction pointer vs PKU state.

But then you can't prevent an attacker from just jumping directly to
your syscall instruction. And it's slow.

^ permalink raw reply

* Re: [RFC PATCH] seccomp: Add protection keys into seccomp_data
From: Ram Pai @ 2018-10-29 16:48 UTC (permalink / raw)
  To: Kees Cook
  Cc: Will Drewry, Linux API, Dave Hansen, Andy Lutomirski,
	Michael Sammler, PowerPC
In-Reply-To: <CAGXu5jJ2fFmEXfUzga3XBXcsHPuPWWf5zLeyn8+Z1oZVa8oTuQ@mail.gmail.com>

On Mon, Oct 29, 2018 at 09:25:15AM -0700, Kees Cook wrote:
> On Mon, Oct 29, 2018 at 4:23 AM, Michael Sammler <msammler@mpi-sws.org> wrote:
> > Add the current value of an architecture specific protection keys
> > register (currently PKRU on x86) to data available for seccomp-bpf
> > programs to work on. This allows filters based on the currently
> > enabled protection keys.
> >
> > Support for protection keys on the POWER architecture is not part of
> > this patch since I do not have access to a PowerPC, but adding support
> > for it can be achieved by setting sd->pkeys to the AMR register in
> > populate_seccomp_data.

Maybe you can use a generic read_pkey() function, and each arch can
provide their own implementation.  In the case of powerpc it is
mfspr(SPRN_AMR);

something like the code below might help?

diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
index 92a9962..d79efe3 100644
--- a/arch/powerpc/include/asm/pkeys.h
+++ b/arch/powerpc/include/asm/pkeys.h
@@ -89,6 +89,11 @@ static inline u16 pte_to_pkey_bits(u64 pteflags)
 #define __mm_pkey_is_reserved(pkey) (reserved_allocation_mask & \
 				       pkey_alloc_mask(pkey))
 
+static inline u64 read_pkey()
+{
+        return mfspr(SPRN_AMR);
+}
+
 static inline bool mm_pkey_is_allocated(struct mm_struct *mm, int pkey)
 {
 	if (pkey < 0 || pkey >= arch_max_pkey())
diff --git a/arch/x86/include/asm/pkeys.h b/arch/x86/include/asm/pkeys.h
index 19b137f..feea114 100644
--- a/arch/x86/include/asm/pkeys.h
+++ b/arch/x86/include/asm/pkeys.h
@@ -9,6 +9,11 @@
 extern int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
 		unsigned long init_val);
 
+static inline u64 read_pkey()
+{
+        return read_pkru();
+}
+
 static inline bool arch_pkeys_enabled(void)
 {
 	return boot_cpu_has(X86_FEATURE_OSPKE);
diff --git a/include/linux/pkeys.h b/include/linux/pkeys.h
index 2955ba97..f1538c7 100644
--- a/include/linux/pkeys.h
+++ b/include/linux/pkeys.h
@@ -13,6 +13,11 @@
 #define PKEY_DEDICATED_EXECUTE_ONLY 0
 #define ARCH_VM_PKEY_FLAGS 0
 
+static inline u64 read_pkey()
+{
+	return 0;
+}
+
 static inline int vma_pkey(struct vm_area_struct *vma)
 {
 	return 0;

RP

> >
> > One use case for this patch is disabling unnecessary system calls for a
> > library (e.g. network i/o for a crypto library) while the library runs
> > without disabling the system calls for the whole program (by changing
> > the protection keys before and after the library executes). Using this
> > one could ensure that the library behaves a expected (e.g. the crypto
> > library not sending private keys to a malicious server).
> >
> > This patch also enables lightweight sandboxing of untrusted code using
> > memory protection keys: Protection keys provide memory isolation but
> > for a sandbox system call isolation is needed as well. This patch
> > allows writing a seccomp filter to prevent system calls by the
> > untrusted code while still allowing system calls for the trusted code.
> 
> Isn't PKU instruction based? Couldn't a malicious library just change
> the state of the MPK registers? This seems like an easy way to bypass
> any filters that used PKU. I'm not convinced this is a meaningful
> barrier that should be enforced by seccomp.
> 
> This can also be done with a signal handler with SECCOMP_RET_TRAP and
> check instruction pointer vs PKU state.
> 
> -Kees
> 
> >
> > An alternative design would be to extend (c)BPF with a new instruction
> > to read the state of a protection key. This alternate design would
> > provide a simpler interface to the user space since the BPF program
> > would not need to deal with the architecture specific pkeys field in
> > seccomp_data, but the question is, how much of an advantage this would
> > be as the nr field in seccomp_data is already architecture specific.
> > Adding a new instruction for BPF programs is more complicated than
> > this patch and might be a breaking change.
> >
> > Results of selftests/seccomp_benchmark.c on a x86 machine with pkeys
> > support:
> >
> > With patch:
> > Benchmarking 33554432 samples...
> > 28.019505558 - 18.676858522 = 9342647036
> > getpid native: 278 ns
> > 42.279109885 - 28.019657031 = 14259452854
> > getpid RET_ALLOW: 424 ns
> > Estimated seccomp overhead per syscall: 146 ns
> >
> > Without patch:
> > Benchmarking 33554432 samples...
> > 28.059619466 - 18.706769155 = 9352850311
> > getpid native: 278 ns
> > 42.299228279 - 28.059761804 = 14239466475
> > getpid RET_ALLOW: 424 ns
> > Estimated seccomp overhead per syscall: 146 ns
> >
> > Cc: Kees Cook <keescook@chromium.org>
> > Cc: Andy Lutomirski <luto@amacapital.net>
> > Cc: Will Drewry <wad@chromium.org>
> > Cc: Ram Pai <linuxram@us.ibm.com>
> > Cc: linuxppc-dev@lists.ozlabs.org
> > Cc: linux-api@vger.kernel.org
> > Signed-off-by: Michael Sammler <msammler@mpi-sws.org>
> > ---
> > Changes to the previous version:
> > - added motivation, notes about POWER, alternative design and benchmark results to the commit log
> > - renamed pkru field in seccomp_data to pkeys
> > - changed size of pkru field to __u64 and removed reserved field
> > - added test for x86
> >
> >  arch/mips/kernel/ptrace.c                     |   1 +
> >  arch/x86/entry/common.c                       |   1 +
> >  include/uapi/linux/seccomp.h                  |   3 +
> >  kernel/seccomp.c                              |   1 +
> >  tools/testing/selftests/seccomp/seccomp_bpf.c | 107 +++++++++++++++++++++++++-
> >  5 files changed, 112 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/mips/kernel/ptrace.c b/arch/mips/kernel/ptrace.c
> > index e5ba56c0..a58dd04d 100644
> > --- a/arch/mips/kernel/ptrace.c
> > +++ b/arch/mips/kernel/ptrace.c
> > @@ -1277,6 +1277,7 @@ asmlinkage long syscall_trace_enter(struct pt_regs *regs, long syscall)
> >                 for (i = 0; i < 6; i++)
> >                         sd.args[i] = args[i];
> >                 sd.instruction_pointer = KSTK_EIP(current);
> > +               sd.pkeys = 0;
> >
> >                 ret = __secure_computing(&sd);
> >                 if (ret == -1)
> > diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
> > index 3b2490b8..20c51bf2 100644
> > --- a/arch/x86/entry/common.c
> > +++ b/arch/x86/entry/common.c
> > @@ -98,6 +98,7 @@ static long syscall_trace_enter(struct pt_regs *regs)
> >                 sd.arch = arch;
> >                 sd.nr = regs->orig_ax;
> >                 sd.instruction_pointer = regs->ip;
> > +               sd.pkeys = read_pkru();
> >  #ifdef CONFIG_X86_64
> >                 if (arch == AUDIT_ARCH_X86_64) {
> >                         sd.args[0] = regs->di;
> > diff --git a/include/uapi/linux/seccomp.h b/include/uapi/linux/seccomp.h
> > index 9efc0e73..3aa2d934 100644
> > --- a/include/uapi/linux/seccomp.h
> > +++ b/include/uapi/linux/seccomp.h
> > @@ -52,12 +52,15 @@
> >   * @instruction_pointer: at the time of the system call.
> >   * @args: up to 6 system call arguments always stored as 64-bit values
> >   *        regardless of the architecture.
> > + * @pkeys: value of an architecture specific protection keys register
> > + *         (currently PKRU on x86)
> >   */
> >  struct seccomp_data {
> >         int nr;
> >         __u32 arch;
> >         __u64 instruction_pointer;
> >         __u64 args[6];
> > +       __u64 pkeys;
> >  };
> >
> >  #endif /* _UAPI_LINUX_SECCOMP_H */
> > diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> > index fd023ac2..dfb8b0d6 100644
> > --- a/kernel/seccomp.c
> > +++ b/kernel/seccomp.c
> > @@ -91,6 +91,7 @@ static void populate_seccomp_data(struct seccomp_data *sd)
> >         sd->args[4] = args[4];
> >         sd->args[5] = args[5];
> >         sd->instruction_pointer = KSTK_EIP(task);
> > +       sd->pkeys = 0;
> >  }
> >
> >  /**
> > diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c
> > index e1473234..f7f8fa6f 100644
> > --- a/tools/testing/selftests/seccomp/seccomp_bpf.c
> > +++ b/tools/testing/selftests/seccomp/seccomp_bpf.c
> > @@ -82,6 +82,7 @@ struct seccomp_data {
> >         __u32 arch;
> >         __u64 instruction_pointer;
> >         __u64 args[6];
> > +       __u64 pkeys;
> >  };
> >  #endif
> >
> > @@ -732,7 +733,9 @@ TEST(KILL_process)
> >  TEST(arg_out_of_range)
> >  {
> >         struct sock_filter filter[] = {
> > -               BPF_STMT(BPF_LD|BPF_W|BPF_ABS, syscall_arg(6)),
> > +               BPF_STMT(BPF_LD|BPF_W|BPF_ABS,
> > +                       offsetof(struct seccomp_data, pkeys)
> > +                               + sizeof(__u64)),
> >                 BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ALLOW),
> >         };
> >         struct sock_fprog prog = {
> > @@ -2933,6 +2936,108 @@ skip:
> >         ASSERT_EQ(0, kill(pid, SIGKILL));
> >  }
> >
> > +#if defined(__i386__) || defined(__x86_64__)
> > +static inline void __cpuid(unsigned int *eax, unsigned int *ebx,
> > +               unsigned int *ecx, unsigned int *edx)
> > +{
> > +       /* ecx is often an input as well as an output. */
> > +       asm volatile(
> > +               "cpuid;"
> > +               : "=a" (*eax),
> > +                 "=b" (*ebx),
> > +                 "=c" (*ecx),
> > +                 "=d" (*edx)
> > +               : "0" (*eax), "2" (*ecx));
> > +}
> > +
> > +/* Intel-defined CPU features, CPUID level 0x00000007:0 (ecx) */
> > +#define X86_FEATURE_PKU        (1<<3) /* Protection Keys for Userspace */
> > +#define X86_FEATURE_OSPKE      (1<<4) /* OS Protection Keys Enable */
> > +
> > +static inline int cpu_has_pku(void)
> > +{
> > +       unsigned int eax;
> > +       unsigned int ebx;
> > +       unsigned int ecx;
> > +       unsigned int edx;
> > +
> > +       eax = 0x7;
> > +       ecx = 0x0;
> > +       __cpuid(&eax, &ebx, &ecx, &edx);
> > +
> > +       if (!(ecx & X86_FEATURE_PKU))
> > +               return 0;
> > +       if (!(ecx & X86_FEATURE_OSPKE))
> > +               return 0;
> > +       return 1;
> > +}
> > +
> > +static inline __u32 read_pkru(void)
> > +{
> > +       if (!cpu_has_pku())
> > +               return 0;
> > +
> > +       __u32 ecx = 0;
> > +       __u32 edx, pkru;
> > +
> > +       /*
> > +        * "rdpkru" instruction.  Places PKRU contents in to EAX,
> > +        * clears EDX and requires that ecx=0.
> > +        */
> > +       asm volatile(".byte 0x0f,0x01,0xee\n\t"
> > +                    : "=a" (pkru), "=d" (edx)
> > +                    : "c" (ecx));
> > +       return pkru;
> > +}
> > +
> > +static inline void write_pkru(__u32 pkru)
> > +{
> > +       if (!cpu_has_pku())
> > +               return;
> > +
> > +       __u32 ecx = 0, edx = 0;
> > +
> > +       /*
> > +        * "wrpkru" instruction.  Loads contents in EAX to PKRU,
> > +        * requires that ecx = edx = 0.
> > +        */
> > +       asm volatile(".byte 0x0f,0x01,0xef\n\t"
> > +                    : : "a" (pkru), "c"(ecx), "d"(edx));
> > +}
> > +
> > +#define TEST_PKRU 0x55555550
> > +
> > +TEST_SIGNAL(pkeys_set, SIGSYS)
> > +{
> > +       write_pkru(TEST_PKRU);
> > +       /* read back the written value because pkru might not be supported */
> > +       __u32 pkru = read_pkru();
> > +
> > +       struct sock_filter filter[] = {
> > +               BPF_STMT(BPF_LD|BPF_W|BPF_ABS,
> > +                       offsetof(struct seccomp_data, pkeys)),
> > +               BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, pkru, 1, 0),
> > +               BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ALLOW),
> > +               BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_KILL),
> > +       };
> > +       struct sock_fprog prog = {
> > +               .len = (unsigned short)ARRAY_SIZE(filter),
> > +               .filter = filter,
> > +       };
> > +       long ret;
> > +
> > +       ret = prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0);
> > +       ASSERT_EQ(0, ret);
> > +
> > +       ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog);
> > +       ASSERT_EQ(0, ret);
> > +
> > +       /* should never return. */
> > +       EXPECT_EQ(0, syscall(__NR_getpid));
> > +}
> > +#endif
> > +
> > +
> >  /*
> >   * TODO:
> >   * - add microbenchmarks
> > --
> > 2.11.0
> 
> 
> 
> -- 
> Kees Cook

-- 
Ram Pai


^ permalink raw reply related

* Re: [RFC PATCH] seccomp: Add protection keys into seccomp_data
From: Jann Horn @ 2018-10-29 16:48 UTC (permalink / raw)
  To: Dave Hansen
  Cc: wad, Kees Cook, Linux API, Dave Hansen, linuxram, Andy Lutomirski,
	msammler, linuxppc-dev
In-Reply-To: <7d93080b-68bd-7563-bd3b-e7ee1545e367@intel.com>

On Mon, Oct 29, 2018 at 5:37 PM Dave Hansen <dave.hansen@intel.com> wrote:
> On 10/29/18 9:25 AM, Kees Cook wrote:
> > On Mon, Oct 29, 2018 at 4:23 AM, Michael Sammler <msammler@mpi-sws.org> wrote:
> >> Add the current value of an architecture specific protection keys
> >> register (currently PKRU on x86) to data available for seccomp-bpf
> >> programs to work on. This allows filters based on the currently
> >> enabled protection keys.
>
> How does the current "assignment" of protection keys to the various uses
> get communicated to the filter?

I assume that you first allocate your protection keys, then install the filter?

> I'm not sure this is a great use for PKRU.  I *think* the basic problem
> is that you want to communicate some rights information down into a
> filter, and you want to communicate it with PKRU.  While it's handy to
> have an extra register that nobody (generally) mucks with, I'm not quite
> convinced that we want to repurpose it this way.

That's not how I understand it; I believe that the context is probably
https://arxiv.org/pdf/1801.06822.pdf ?
My understanding is that PKRU is used for lightweight in-process
sandboxing, and to extend this sandbox protection to the syscall
interface, it is necessary to expose PKRU state to seccomp filters.
In other words, this isn't using PKRU exclusively for passing rights
into a filter, but it has to use PKRU anyway.

> Also, I'm not sure the kernel provides the PKRU guarantees you want at
> the moment.  Our implementation *probably* works, but it's mostly by
> accident.

^ permalink raw reply

* Re: NXP P50XX/e5500: SMP doesn't work anymore with the latest Git kernel
From: Christian Zigotzky @ 2018-10-29 17:00 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <99266ac6-640c-b2a9-eef3-4e89ee1e0ad5@xenosoft.de>

Hello,

I figured out that the problem is in the OF source code of the commit: 
Merge tag devicetree-for-4.20. [1]

I reverted the following OF files and SMP works!

drivers/of/base.c
drivers/of/device.c
drivers/of/of_mdio.c
drivers/of/of_numa.c
drivers/of/of_private.h
drivers/of/overlay.c
drivers/of/platform.c
drivers/of/unittest-data/overlay_15.dts
drivers/of/unittest-data/tests-overlay.dtsi
drivers/of/unittest.c
include/linux/of.h

Cheers,
Christian

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b27186abb37b7bd19e0ca434f4f425c807dbd708


On 29 October 2018 at 10:56AM, Christian Zigotzky wrote:
> Hello,
>
> I have figured out that the commit 'devicetree-for-4.20' [1] is 
> responsible for the SMP problem. I was able to revert this commit with 
> 'git revert b27186abb37b7bd19e0ca434f4f425c807dbd708 -m 1' today.
>
> [master ec81438] Revert "Merge tag 'devicetree-for-4.20' of 
> git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux"
> 138 files changed, 931 insertions(+), 1538 deletions(-)
> rename Documentation/devicetree/bindings/arm/{atmel-sysregs.txt => 
> atmel-at91.txt} (67%)
> delete mode 100644 
> Documentation/devicetree/bindings/arm/freescale/fsl,layerscape-dcfg.txt
> delete mode 100644 
> Documentation/devicetree/bindings/arm/freescale/fsl,layerscape-scfg.txt
> rename Documentation/devicetree/bindings/arm/{zte,sysctrl.txt => 
> zte.txt} (62%)
> delete mode 100644 Documentation/devicetree/bindings/misc/lwn-bk4.txt
> create mode 100644 arch/c6x/boot/dts/linked_dtb.S
> delete mode 100644 arch/nios2/boot/dts/Makefile
> create mode 100644 arch/nios2/boot/linked_dtb.S
> delete mode 100644 arch/powerpc/boot/dts/Makefile
> delete mode 100644 arch/powerpc/boot/dts/fsl/Makefile
> delete mode 100644 scripts/dtc/yamltree.c
>
> It solves the SMP problem! SMP works again on my P5020 board and on 
> virtual e5500 QEMU machines.
>
> QEMU command: ./qemu-system-ppc64 -M ppce500 -cpu e5500 -m 2048 
> -kernel /home/christian/Downloads/uImage-4.20-alpha5 -drive 
> format=raw,file=/home/christian/Dokumente/ubuntu_MATE_16.04.3_LTS_PowerPC_QEMU/ubuntu_MATE_16.04_PowerPC.img,index=0,if=virtio 
> -nic user,model=e1000 -append "rw root=/dev/vda3" -device virtio-vga 
> -device virtio-mouse-pci -device virtio-keyboard-pci -soundhw es1370 
> -smp 4
>
> Screenshot: 
> https://plus.google.com/u/0/photos/photo/115515624056477014971/6617705776207990082
>
> Do we need a new dtb file or is it a bug?
>
> Thanks,
> Christian
>
> [1] 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b27186abb37b7bd19e0ca434f4f425c807dbd708
>
>
> On 28 October 2018 at 5:35PM, Christian Zigotzky wrote:
>> Hello,
>>
>> SMP doesn't work anymore with the latest Git kernel (28/10/18 11:12AM 
>> GMT) on my P5020 board and on virtual e5500 QEMU machines.
>>
>> Board with P5020 dual core CPU:
>>
>> [    0.000000] -----------------------------------------------------
>> [    0.000000] phys_mem_size     = 0x200000000
>> [    0.000000] dcache_bsize      = 0x40
>> [    0.000000] icache_bsize      = 0x40
>> [    0.000000] cpu_features      = 0x00000003008003b4
>> [    0.000000]   possible        = 0x00000003009003b4
>> [    0.000000]   always          = 0x00000003008003b4
>> [    0.000000] cpu_user_features = 0xcc008000 0x08000000
>> [    0.000000] mmu_features      = 0x000a0010
>> [    0.000000] firmware_features = 0x0000000000000000
>> [    0.000000] -----------------------------------------------------
>> [    0.000000] CoreNet Generic board
>>
>>     ...
>>
>> [    0.002161] smp: Bringing up secondary CPUs ...
>> [    0.002339] No cpu-release-addr for cpu 1
>> [    0.002347] smp: failed starting cpu 1 (rc -2)
>> [    0.002401] smp: Brought up 1 node, 1 CPU
>>
>> Virtual e5500 quad core QEMU machine:
>>
>> [    0.026394] smp: Bringing up secondary CPUs ...
>> [    0.027831] No cpu-release-addr for cpu 1
>> [    0.027989] smp: failed starting cpu 1 (rc -2)
>> [    0.030143] No cpu-release-addr for cpu 2
>> [    0.030304] smp: failed starting cpu 2 (rc -2)
>> [    0.032400] No cpu-release-addr for cpu 3
>> [    0.032533] smp: failed starting cpu 3 (rc -2)
>> [    0.033117] smp: Brought up 1 node, 1 CPU
>>
>> QEMU command: ./qemu-system-ppc64 -M ppce500 -cpu e5500 -m 2048 
>> -kernel 
>> /home/christian/Downloads/vmlinux-4.20-alpha4-AmigaOne_X1000_X5000/X5000_and_QEMU_e5500/uImage-4.20 
>> -drive 
>> format=raw,file=/home/christian/Downloads/MATE_PowerPC_Remix_2017_0.9.img,index=0,if=virtio 
>> -nic user,model=e1000 -append "rw root=/dev/vda" -device virtio-vga 
>> -device virtio-mouse-pci -device virtio-keyboard-pci -usb -soundhw 
>> es1370 -smp 4
>>
>> .config:
>>
>> ...
>> CONFIG_SMP=y
>> CONFIG_NR_CPUS=4
>> ...
>>
>> Please test the latest Git kernel on your NXP P50XX boards.
>>
>> Thanks,
>> Christian
>>
>
>


^ permalink raw reply

* Re: [RFC PATCH] seccomp: Add protection keys into seccomp_data
From: Michael Sammler @ 2018-10-29 17:02 UTC (permalink / raw)
  To: Jann Horn, Dave Hansen
  Cc: wad, Kees Cook, Linux API, Dave Hansen, linuxram, Andy Lutomirski,
	linuxppc-dev
In-Reply-To: <CAG48ez1mY7Osdsc4iJ1JKq=C60wb9bQF=aHbi1fb=pb_V2XMJA@mail.gmail.com>

On 10/29/2018 05:48 PM, Jann Horn wrote:
> On Mon, Oct 29, 2018 at 5:37 PM Dave Hansen <dave.hansen@intel.com> wrote:
>> On 10/29/18 9:25 AM, Kees Cook wrote:
>>> On Mon, Oct 29, 2018 at 4:23 AM, Michael Sammler <msammler@mpi-sws.org> wrote:
>>>> Add the current value of an architecture specific protection keys
>>>> register (currently PKRU on x86) to data available for seccomp-bpf
>>>> programs to work on. This allows filters based on the currently
>>>> enabled protection keys.
>> How does the current "assignment" of protection keys to the various uses
>> get communicated to the filter?
> I assume that you first allocate your protection keys, then install the filter?
>
Yes, but I agree that it should probably be documented, that the filter 
should only look at the parts of the PKRU, which belong to pkeys the 
user space program allocated (if the kernel wants to use some parts of 
the PKRU for its own purposes).
>> I'm not sure this is a great use for PKRU.  I *think* the basic problem
>> is that you want to communicate some rights information down into a
>> filter, and you want to communicate it with PKRU.  While it's handy to
>> have an extra register that nobody (generally) mucks with, I'm not quite
>> convinced that we want to repurpose it this way.
> That's not how I understand it; I believe that the context is probably
> https://arxiv.org/pdf/1801.06822.pdf ?
> My understanding is that PKRU is used for lightweight in-process
> sandboxing, and to extend this sandbox protection to the syscall
> interface, it is necessary to expose PKRU state to seccomp filters.
> In other words, this isn't using PKRU exclusively for passing rights
> into a filter, but it has to use PKRU anyway.
Yes, https://arxiv.org/pdf/1801.06822.pdf is indeed the context and what 
you say is correct.
>> Also, I'm not sure the kernel provides the PKRU guarantees you want at
>> the moment.  Our implementation *probably* works, but it's mostly by
>> accident.
I don't know, which guarantees about the PKRU are provided at the 
moment, but the only guarantee needed for this patch is, that the kernel 
does not change the bits of the PKRU register, which belong to pkeys 
allocated by the user program, between the syscall entry and the call to 
secure_computing(). Is there are use case where the kernel would like to 
modify these bits of the PKRU?

-- MIchael

^ permalink raw reply

* Re: [RFC PATCH] seccomp: Add protection keys into seccomp_data
From: Michael Sammler @ 2018-10-29 17:05 UTC (permalink / raw)
  To: Ram Pai, Kees Cook
  Cc: PowerPC, Linux API, Will Drewry, Dave Hansen, Andy Lutomirski
In-Reply-To: <20181029164806.GA5494@ram.oc3035372033.ibm.com>

On 10/29/2018 05:48 PM, Ram Pai wrote:

> On Mon, Oct 29, 2018 at 09:25:15AM -0700, Kees Cook wrote:
>> On Mon, Oct 29, 2018 at 4:23 AM, Michael Sammler <msammler@mpi-sws.org> wrote:
>>> Add the current value of an architecture specific protection keys
>>> register (currently PKRU on x86) to data available for seccomp-bpf
>>> programs to work on. This allows filters based on the currently
>>> enabled protection keys.
>>>
>>> Support for protection keys on the POWER architecture is not part of
>>> this patch since I do not have access to a PowerPC, but adding support
>>> for it can be achieved by setting sd->pkeys to the AMR register in
>>> populate_seccomp_data.
> Maybe you can use a generic read_pkey() function, and each arch can
> provide their own implementation.  In the case of powerpc it is
> mfspr(SPRN_AMR);
>
> something like the code below might help?
>
> diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
> index 92a9962..d79efe3 100644
> --- a/arch/powerpc/include/asm/pkeys.h
> +++ b/arch/powerpc/include/asm/pkeys.h
> @@ -89,6 +89,11 @@ static inline u16 pte_to_pkey_bits(u64 pteflags)
>   #define __mm_pkey_is_reserved(pkey) (reserved_allocation_mask & \
>   				       pkey_alloc_mask(pkey))
>   
> +static inline u64 read_pkey()
> +{
> +        return mfspr(SPRN_AMR);
> +}
> +
>   static inline bool mm_pkey_is_allocated(struct mm_struct *mm, int pkey)
>   {
>   	if (pkey < 0 || pkey >= arch_max_pkey())
> diff --git a/arch/x86/include/asm/pkeys.h b/arch/x86/include/asm/pkeys.h
> index 19b137f..feea114 100644
> --- a/arch/x86/include/asm/pkeys.h
> +++ b/arch/x86/include/asm/pkeys.h
> @@ -9,6 +9,11 @@
>   extern int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
>   		unsigned long init_val);
>   
> +static inline u64 read_pkey()
> +{
> +        return read_pkru();
> +}
> +
>   static inline bool arch_pkeys_enabled(void)
>   {
>   	return boot_cpu_has(X86_FEATURE_OSPKE);
> diff --git a/include/linux/pkeys.h b/include/linux/pkeys.h
> index 2955ba97..f1538c7 100644
> --- a/include/linux/pkeys.h
> +++ b/include/linux/pkeys.h
> @@ -13,6 +13,11 @@
>   #define PKEY_DEDICATED_EXECUTE_ONLY 0
>   #define ARCH_VM_PKEY_FLAGS 0
>   
> +static inline u64 read_pkey()
> +{
> +	return 0;
> +}
> +
>   static inline int vma_pkey(struct vm_area_struct *vma)
>   {
>   	return 0;
>
> RP
Thank you very much, this helps indeed. I will add this to the next 
version of this patch.

>>> One use case for this patch is disabling unnecessary system calls for a
>>> library (e.g. network i/o for a crypto library) while the library runs
>>> without disabling the system calls for the whole program (by changing
>>> the protection keys before and after the library executes). Using this
>>> one could ensure that the library behaves a expected (e.g. the crypto
>>> library not sending private keys to a malicious server).
>>>
>>> This patch also enables lightweight sandboxing of untrusted code using
>>> memory protection keys: Protection keys provide memory isolation but
>>> for a sandbox system call isolation is needed as well. This patch
>>> allows writing a seccomp filter to prevent system calls by the
>>> untrusted code while still allowing system calls for the trusted code.
>> Isn't PKU instruction based? Couldn't a malicious library just change
>> the state of the MPK registers? This seems like an easy way to bypass
>> any filters that used PKU. I'm not convinced this is a meaningful
>> barrier that should be enforced by seccomp.
>>
>> This can also be done with a signal handler with SECCOMP_RET_TRAP and
>> check instruction pointer vs PKU state.
>>
>> -Kees
>>
>>> An alternative design would be to extend (c)BPF with a new instruction
>>> to read the state of a protection key. This alternate design would
>>> provide a simpler interface to the user space since the BPF program
>>> would not need to deal with the architecture specific pkeys field in
>>> seccomp_data, but the question is, how much of an advantage this would
>>> be as the nr field in seccomp_data is already architecture specific.
>>> Adding a new instruction for BPF programs is more complicated than
>>> this patch and might be a breaking change.
>>>
>>> Results of selftests/seccomp_benchmark.c on a x86 machine with pkeys
>>> support:
>>>
>>> With patch:
>>> Benchmarking 33554432 samples...
>>> 28.019505558 - 18.676858522 = 9342647036
>>> getpid native: 278 ns
>>> 42.279109885 - 28.019657031 = 14259452854
>>> getpid RET_ALLOW: 424 ns
>>> Estimated seccomp overhead per syscall: 146 ns
>>>
>>> Without patch:
>>> Benchmarking 33554432 samples...
>>> 28.059619466 - 18.706769155 = 9352850311
>>> getpid native: 278 ns
>>> 42.299228279 - 28.059761804 = 14239466475
>>> getpid RET_ALLOW: 424 ns
>>> Estimated seccomp overhead per syscall: 146 ns
>>>
>>> Cc: Kees Cook <keescook@chromium.org>
>>> Cc: Andy Lutomirski <luto@amacapital.net>
>>> Cc: Will Drewry <wad@chromium.org>
>>> Cc: Ram Pai <linuxram@us.ibm.com>
>>> Cc: linuxppc-dev@lists.ozlabs.org
>>> Cc: linux-api@vger.kernel.org
>>> Signed-off-by: Michael Sammler <msammler@mpi-sws.org>
>>> ---
>>> Changes to the previous version:
>>> - added motivation, notes about POWER, alternative design and benchmark results to the commit log
>>> - renamed pkru field in seccomp_data to pkeys
>>> - changed size of pkru field to __u64 and removed reserved field
>>> - added test for x86
>>>
>>>   arch/mips/kernel/ptrace.c                     |   1 +
>>>   arch/x86/entry/common.c                       |   1 +
>>>   include/uapi/linux/seccomp.h                  |   3 +
>>>   kernel/seccomp.c                              |   1 +
>>>   tools/testing/selftests/seccomp/seccomp_bpf.c | 107 +++++++++++++++++++++++++-
>>>   5 files changed, 112 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/mips/kernel/ptrace.c b/arch/mips/kernel/ptrace.c
>>> index e5ba56c0..a58dd04d 100644
>>> --- a/arch/mips/kernel/ptrace.c
>>> +++ b/arch/mips/kernel/ptrace.c
>>> @@ -1277,6 +1277,7 @@ asmlinkage long syscall_trace_enter(struct pt_regs *regs, long syscall)
>>>                  for (i = 0; i < 6; i++)
>>>                          sd.args[i] = args[i];
>>>                  sd.instruction_pointer = KSTK_EIP(current);
>>> +               sd.pkeys = 0;
>>>
>>>                  ret = __secure_computing(&sd);
>>>                  if (ret == -1)
>>> diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
>>> index 3b2490b8..20c51bf2 100644
>>> --- a/arch/x86/entry/common.c
>>> +++ b/arch/x86/entry/common.c
>>> @@ -98,6 +98,7 @@ static long syscall_trace_enter(struct pt_regs *regs)
>>>                  sd.arch = arch;
>>>                  sd.nr = regs->orig_ax;
>>>                  sd.instruction_pointer = regs->ip;
>>> +               sd.pkeys = read_pkru();
>>>   #ifdef CONFIG_X86_64
>>>                  if (arch == AUDIT_ARCH_X86_64) {
>>>                          sd.args[0] = regs->di;
>>> diff --git a/include/uapi/linux/seccomp.h b/include/uapi/linux/seccomp.h
>>> index 9efc0e73..3aa2d934 100644
>>> --- a/include/uapi/linux/seccomp.h
>>> +++ b/include/uapi/linux/seccomp.h
>>> @@ -52,12 +52,15 @@
>>>    * @instruction_pointer: at the time of the system call.
>>>    * @args: up to 6 system call arguments always stored as 64-bit values
>>>    *        regardless of the architecture.
>>> + * @pkeys: value of an architecture specific protection keys register
>>> + *         (currently PKRU on x86)
>>>    */
>>>   struct seccomp_data {
>>>          int nr;
>>>          __u32 arch;
>>>          __u64 instruction_pointer;
>>>          __u64 args[6];
>>> +       __u64 pkeys;
>>>   };
>>>
>>>   #endif /* _UAPI_LINUX_SECCOMP_H */
>>> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
>>> index fd023ac2..dfb8b0d6 100644
>>> --- a/kernel/seccomp.c
>>> +++ b/kernel/seccomp.c
>>> @@ -91,6 +91,7 @@ static void populate_seccomp_data(struct seccomp_data *sd)
>>>          sd->args[4] = args[4];
>>>          sd->args[5] = args[5];
>>>          sd->instruction_pointer = KSTK_EIP(task);
>>> +       sd->pkeys = 0;
>>>   }
>>>
>>>   /**
>>> diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c
>>> index e1473234..f7f8fa6f 100644
>>> --- a/tools/testing/selftests/seccomp/seccomp_bpf.c
>>> +++ b/tools/testing/selftests/seccomp/seccomp_bpf.c
>>> @@ -82,6 +82,7 @@ struct seccomp_data {
>>>          __u32 arch;
>>>          __u64 instruction_pointer;
>>>          __u64 args[6];
>>> +       __u64 pkeys;
>>>   };
>>>   #endif
>>>
>>> @@ -732,7 +733,9 @@ TEST(KILL_process)
>>>   TEST(arg_out_of_range)
>>>   {
>>>          struct sock_filter filter[] = {
>>> -               BPF_STMT(BPF_LD|BPF_W|BPF_ABS, syscall_arg(6)),
>>> +               BPF_STMT(BPF_LD|BPF_W|BPF_ABS,
>>> +                       offsetof(struct seccomp_data, pkeys)
>>> +                               + sizeof(__u64)),
>>>                  BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ALLOW),
>>>          };
>>>          struct sock_fprog prog = {
>>> @@ -2933,6 +2936,108 @@ skip:
>>>          ASSERT_EQ(0, kill(pid, SIGKILL));
>>>   }
>>>
>>> +#if defined(__i386__) || defined(__x86_64__)
>>> +static inline void __cpuid(unsigned int *eax, unsigned int *ebx,
>>> +               unsigned int *ecx, unsigned int *edx)
>>> +{
>>> +       /* ecx is often an input as well as an output. */
>>> +       asm volatile(
>>> +               "cpuid;"
>>> +               : "=a" (*eax),
>>> +                 "=b" (*ebx),
>>> +                 "=c" (*ecx),
>>> +                 "=d" (*edx)
>>> +               : "0" (*eax), "2" (*ecx));
>>> +}
>>> +
>>> +/* Intel-defined CPU features, CPUID level 0x00000007:0 (ecx) */
>>> +#define X86_FEATURE_PKU        (1<<3) /* Protection Keys for Userspace */
>>> +#define X86_FEATURE_OSPKE      (1<<4) /* OS Protection Keys Enable */
>>> +
>>> +static inline int cpu_has_pku(void)
>>> +{
>>> +       unsigned int eax;
>>> +       unsigned int ebx;
>>> +       unsigned int ecx;
>>> +       unsigned int edx;
>>> +
>>> +       eax = 0x7;
>>> +       ecx = 0x0;
>>> +       __cpuid(&eax, &ebx, &ecx, &edx);
>>> +
>>> +       if (!(ecx & X86_FEATURE_PKU))
>>> +               return 0;
>>> +       if (!(ecx & X86_FEATURE_OSPKE))
>>> +               return 0;
>>> +       return 1;
>>> +}
>>> +
>>> +static inline __u32 read_pkru(void)
>>> +{
>>> +       if (!cpu_has_pku())
>>> +               return 0;
>>> +
>>> +       __u32 ecx = 0;
>>> +       __u32 edx, pkru;
>>> +
>>> +       /*
>>> +        * "rdpkru" instruction.  Places PKRU contents in to EAX,
>>> +        * clears EDX and requires that ecx=0.
>>> +        */
>>> +       asm volatile(".byte 0x0f,0x01,0xee\n\t"
>>> +                    : "=a" (pkru), "=d" (edx)
>>> +                    : "c" (ecx));
>>> +       return pkru;
>>> +}
>>> +
>>> +static inline void write_pkru(__u32 pkru)
>>> +{
>>> +       if (!cpu_has_pku())
>>> +               return;
>>> +
>>> +       __u32 ecx = 0, edx = 0;
>>> +
>>> +       /*
>>> +        * "wrpkru" instruction.  Loads contents in EAX to PKRU,
>>> +        * requires that ecx = edx = 0.
>>> +        */
>>> +       asm volatile(".byte 0x0f,0x01,0xef\n\t"
>>> +                    : : "a" (pkru), "c"(ecx), "d"(edx));
>>> +}
>>> +
>>> +#define TEST_PKRU 0x55555550
>>> +
>>> +TEST_SIGNAL(pkeys_set, SIGSYS)
>>> +{
>>> +       write_pkru(TEST_PKRU);
>>> +       /* read back the written value because pkru might not be supported */
>>> +       __u32 pkru = read_pkru();
>>> +
>>> +       struct sock_filter filter[] = {
>>> +               BPF_STMT(BPF_LD|BPF_W|BPF_ABS,
>>> +                       offsetof(struct seccomp_data, pkeys)),
>>> +               BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, pkru, 1, 0),
>>> +               BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ALLOW),
>>> +               BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_KILL),
>>> +       };
>>> +       struct sock_fprog prog = {
>>> +               .len = (unsigned short)ARRAY_SIZE(filter),
>>> +               .filter = filter,
>>> +       };
>>> +       long ret;
>>> +
>>> +       ret = prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0);
>>> +       ASSERT_EQ(0, ret);
>>> +
>>> +       ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog);
>>> +       ASSERT_EQ(0, ret);
>>> +
>>> +       /* should never return. */
>>> +       EXPECT_EQ(0, syscall(__NR_getpid));
>>> +}
>>> +#endif
>>> +
>>> +
>>>   /*
>>>    * TODO:
>>>    * - add microbenchmarks
>>> --
>>> 2.11.0
>>
>>
>> -- 
>> Kees Cook


^ permalink raw reply

* Re: [RFC PATCH] seccomp: Add protection keys into seccomp_data
From: Dave Hansen @ 2018-10-29 17:07 UTC (permalink / raw)
  To: Michael Sammler, Jann Horn
  Cc: wad, Kees Cook, Rik van Riel, Linux API, Dave Hansen, linuxram,
	Andy Lutomirski, linuxppc-dev
In-Reply-To: <24afa443-92e8-8202-845a-c1e1df1a3226@mpi-sws.org>

On 10/29/18 10:02 AM, Michael Sammler wrote:
>>> Also, I'm not sure the kernel provides the PKRU guarantees you want at
>>> the moment.  Our implementation *probably* works, but it's mostly by
>>> accident.
> I don't know, which guarantees about the PKRU are provided at the
> moment, but the only guarantee needed for this patch is, that the kernel
> does not change the bits of the PKRU register, which belong to pkeys
> allocated by the user program, between the syscall entry and the call to
> secure_computing(). Is there are use case where the kernel would like to
> modify these bits of the PKRU?

We've been talking about doing more lax save/restore of the XSAVE
content (PKRU is part of this content).  We would, for instance, only
restore it when returning to userspace, but PKRU might not be up-to-date
with the value in current->fpu.

It's not a deal-breaker with your approach, it's just something to be
careful of and make sure PKRU is up-to-date before you go use it.

^ permalink raw reply

* Re: [RFC PATCH] seccomp: Add protection keys into seccomp_data
From: Dave Hansen @ 2018-10-29 17:29 UTC (permalink / raw)
  To: Jann Horn
  Cc: wad, Kees Cook, Linux API, Dave Hansen, linuxram, Andy Lutomirski,
	msammler, linuxppc-dev
In-Reply-To: <CAG48ez1mY7Osdsc4iJ1JKq=C60wb9bQF=aHbi1fb=pb_V2XMJA@mail.gmail.com>

On 10/29/18 9:48 AM, Jann Horn wrote:
> On Mon, Oct 29, 2018 at 5:37 PM Dave Hansen <dave.hansen@intel.com> wrote:
>> I'm not sure this is a great use for PKRU.  I *think* the basic problem
>> is that you want to communicate some rights information down into a
>> filter, and you want to communicate it with PKRU.  While it's handy to
>> have an extra register that nobody (generally) mucks with, I'm not quite
>> convinced that we want to repurpose it this way.
> 
> That's not how I understand it; I believe that the context is probably
> https://arxiv.org/pdf/1801.06822.pdf ?
> My understanding is that PKRU is used for lightweight in-process
> sandboxing, and to extend this sandbox protection to the syscall
> interface, it is necessary to expose PKRU state to seccomp filters.
> In other words, this isn't using PKRU exclusively for passing rights
> into a filter, but it has to use PKRU anyway.

PKRU gives information about rights to various bits of application data.
 From that, a seccomp filter can infer the context, and thus the ability
for the code to call a given syscall at a certain point in time.

This makes PKRU an opt-in part of the syscall ABI, which is pretty
interesting.  We _could_ do the same kind of thing with any callee-saved
general purpose register, but PKRU is particularly attractive because
there is only one instruction that writes to it (well, outside of
XSAVE*), and random library code is very unlikely at this point to be
using it.

PKRU getting reset on signals, and the requirement now that it *can't*
be changed if you make syscalls probably needs to get thought about very
carefully before we do this, though.

^ permalink raw reply

* [PATCH 6/7] smp: Don't yell about IRQs disabled in kgdb_roundup_cpus()
From: Douglas Anderson @ 2018-10-29 18:07 UTC (permalink / raw)
  To: Jason Wessel, Daniel Thompson, tglx, mingo, gregkh
  Cc: linux-mips, linux-sh, peterz, linux-arm-msm, frederic, riel,
	Douglas Anderson, linux-kernel, luto, kgdb-bugreport, sparclinux,
	linux-hexagon, linux-snps-arc, linuxppc-dev, linux-arm-kernel
In-Reply-To: <20181029180707.207546-1-dianders@chromium.org>

In kgdb_roundup_cpus() we've got code that looks like:
  local_irq_enable();
  smp_call_function(kgdb_call_nmi_hook, NULL, 0);
  local_irq_disable();

In certain cases when we drop into kgdb (like with sysrq-g on a serial
console) we'll get a big yell that looks like:

  sysrq: SysRq : DEBUG
  ------------[ cut here ]------------
  DEBUG_LOCKS_WARN_ON(current->hardirq_context)
  WARNING: CPU: 0 PID: 0 at .../kernel/locking/lockdep.c:2875 lockdep_hardirqs_on+0xf0/0x160
  CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.19.0 #27
  pstate: 604003c9 (nZCv DAIF +PAN -UAO)
  pc : lockdep_hardirqs_on+0xf0/0x160
  ...
  Call trace:
   lockdep_hardirqs_on+0xf0/0x160
   trace_hardirqs_on+0x188/0x1ac
   kgdb_roundup_cpus+0x14/0x3c
   kgdb_cpu_enter+0x53c/0x5cc
   kgdb_handle_exception+0x180/0x1d4
   kgdb_compiled_brk_fn+0x30/0x3c
   brk_handler+0x134/0x178
   do_debug_exception+0xfc/0x178
   el1_dbg+0x18/0x78
   kgdb_breakpoint+0x34/0x58
   sysrq_handle_dbg+0x54/0x5c
   __handle_sysrq+0x114/0x21c
   handle_sysrq+0x30/0x3c
   qcom_geni_serial_isr+0x2dc/0x30c
  ...
  ...
  irq event stamp: ...45
  hardirqs last  enabled at (...44): [...] __do_softirq+0xd8/0x4e4
  hardirqs last disabled at (...45): [...] el1_irq+0x74/0x130
  softirqs last  enabled at (...42): [...] _local_bh_enable+0x2c/0x34
  softirqs last disabled at (...43): [...] irq_exit+0xa8/0x100
  ---[ end trace adf21f830c46e638 ]---

Let's add kgdb to the list of reasons not to warn in
smp_call_function_many().  That will allow us (in a future patch) to
stop calling local_irq_enable() which will get rid of the original
splat.

NOTE: with this change comes the obvious question: will we start
deadlocking more often now when we drop into the debugger.  I can't
say that for sure one way or the other, but the fact that we do the
same logic for "oops_in_progress" makes me feel like it shouldn't
happen too often.  Also note that the old logic of turning on
interrupts temporarily wasn't exactly safe since (I presume) that
could have violated spin_lock_irqsave() semantics and ended up with a
deadlock of its own.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

 kernel/smp.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/kernel/smp.c b/kernel/smp.c
index 163c451af42e..bb581e58c8dc 100644
--- a/kernel/smp.c
+++ b/kernel/smp.c
@@ -19,6 +19,7 @@
 #include <linux/sched.h>
 #include <linux/sched/idle.h>
 #include <linux/hypervisor.h>
+#include <linux/kgdb.h>
 
 #include "smpboot.h"
 
@@ -413,7 +414,8 @@ void smp_call_function_many(const struct cpumask *mask,
 	 * can't happen.
 	 */
 	WARN_ON_ONCE(cpu_online(this_cpu) && irqs_disabled()
-		     && !oops_in_progress && !early_boot_irqs_disabled);
+		     && !oops_in_progress && !early_boot_irqs_disabled
+		     && !in_dbg_master());
 
 	/* Try to fastpath.  So, what's a CPU they want? Ignoring this one. */
 	cpu = cpumask_first_and(mask, cpu_online_mask);
-- 
2.19.1.568.g152ad8e336-goog


^ permalink raw reply related

* [PATCH 7/7] kgdb: Remove irq flags and local_irq_enable/disable from roundup
From: Douglas Anderson @ 2018-10-29 18:07 UTC (permalink / raw)
  To: Jason Wessel, Daniel Thompson, tglx, mingo, gregkh
  Cc: kstewart, linux-mips, dalias, catalin.marinas, linux-sh,
	will.deacon, linux-kernel, mhocko, paulus, hpa, sparclinux, sfr,
	ysato, x86, linux, mingo, kgdb-bugreport, jhogan, linux-snps-arc,
	linux-arm-msm, ying.huang, rppt, bp, linux-arm-kernel,
	pombredanne, vgupta, Douglas Anderson, ralf, rkuo, paul.burton,
	linux-hexagon, akpm, linuxppc-dev, davem
In-Reply-To: <20181029180707.207546-1-dianders@chromium.org>

The function kgdb_roundup_cpus() was passed a parameter that was
documented as:

> the flags that will be used when restoring the interrupts. There is
> local_irq_save() call before kgdb_roundup_cpus().

Nobody used those flags.  Anyone who wanted to temporarily turn on
interrupts just did local_irq_enable() and local_irq_disable() without
looking at them.  So we can definitely remove the flags.

Speaking of calling local_irq_enable(), it seems like a bad idea and
it caused a nice splat on my system with lockdep turned on.
Specifically it hit:
  DEBUG_LOCKS_WARN_ON(current->hardirq_context)

See the previous patch in this series ("smp: Don't yell about IRQs
disabled in kgdb_roundup_cpus()") for more details, but the last few
things on the stack were this on my arm64 board:
  lockdep_hardirqs_on+0xf0/0x160
  trace_hardirqs_on+0x188/0x1ac
  kgdb_roundup_cpus+0x14/0x3c

As agrued in the the commit text of ("smp: Don't yell about IRQs
disabled in kgdb_roundup_cpus()"), it seems better to make
smp_call_function() lenient about kgdb than to locally turn on IRQs
here.  Thus let's totally remove all the local_irq_enable() and
local_irq_disable() calls from all of the kgdb_roundup_cpus() calls.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

 arch/arc/kernel/kgdb.c     |  4 +---
 arch/arm/kernel/kgdb.c     |  4 +---
 arch/arm64/kernel/kgdb.c   |  4 +---
 arch/hexagon/kernel/kgdb.c | 11 ++---------
 arch/mips/kernel/kgdb.c    |  4 +---
 arch/powerpc/kernel/kgdb.c |  2 +-
 arch/sh/kernel/kgdb.c      |  4 +---
 arch/sparc/kernel/smp_64.c |  2 +-
 arch/x86/kernel/kgdb.c     |  9 ++-------
 include/linux/kgdb.h       |  9 ++-------
 kernel/debug/debug_core.c  |  2 +-
 11 files changed, 14 insertions(+), 41 deletions(-)

diff --git a/arch/arc/kernel/kgdb.c b/arch/arc/kernel/kgdb.c
index 9a3c34af2ae8..d94d3cb7f9e8 100644
--- a/arch/arc/kernel/kgdb.c
+++ b/arch/arc/kernel/kgdb.c
@@ -197,11 +197,9 @@ static void kgdb_call_nmi_hook(void *ignored)
 	kgdb_nmicallback(raw_smp_processor_id(), NULL);
 }
 
-void kgdb_roundup_cpus(unsigned long flags)
+void kgdb_roundup_cpus(void)
 {
-	local_irq_enable();
 	smp_call_function(kgdb_call_nmi_hook, NULL, 0);
-	local_irq_disable();
 }
 
 struct kgdb_arch arch_kgdb_ops = {
diff --git a/arch/arm/kernel/kgdb.c b/arch/arm/kernel/kgdb.c
index caa0dbe3dc61..a80e9259f7e9 100644
--- a/arch/arm/kernel/kgdb.c
+++ b/arch/arm/kernel/kgdb.c
@@ -175,11 +175,9 @@ static void kgdb_call_nmi_hook(void *ignored)
        kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs());
 }
 
-void kgdb_roundup_cpus(unsigned long flags)
+void kgdb_roundup_cpus(void)
 {
-       local_irq_enable();
        smp_call_function(kgdb_call_nmi_hook, NULL, 0);
-       local_irq_disable();
 }
 
 static int __kgdb_notify(struct die_args *args, unsigned long cmd)
diff --git a/arch/arm64/kernel/kgdb.c b/arch/arm64/kernel/kgdb.c
index a20de58061a8..5d171c26788f 100644
--- a/arch/arm64/kernel/kgdb.c
+++ b/arch/arm64/kernel/kgdb.c
@@ -289,11 +289,9 @@ static void kgdb_call_nmi_hook(void *ignored)
 	kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs());
 }
 
-void kgdb_roundup_cpus(unsigned long flags)
+void kgdb_roundup_cpus(void)
 {
-	local_irq_enable();
 	smp_call_function(kgdb_call_nmi_hook, NULL, 0);
-	local_irq_disable();
 }
 
 static int __kgdb_notify(struct die_args *args, unsigned long cmd)
diff --git a/arch/hexagon/kernel/kgdb.c b/arch/hexagon/kernel/kgdb.c
index 16c24b22d0b2..30fbc491cf45 100644
--- a/arch/hexagon/kernel/kgdb.c
+++ b/arch/hexagon/kernel/kgdb.c
@@ -119,17 +119,12 @@ void kgdb_arch_set_pc(struct pt_regs *regs, unsigned long pc)
 
 /**
  * kgdb_roundup_cpus - Get other CPUs into a holding pattern
- * @flags: Current IRQ state
  *
  * On SMP systems, we need to get the attention of the other CPUs
  * and get them be in a known state.  This should do what is needed
  * to get the other CPUs to call kgdb_wait(). Note that on some arches,
  * the NMI approach is not used for rounding up all the CPUs. For example,
- * in case of MIPS, smp_call_function() is used to roundup CPUs. In
- * this case, we have to make sure that interrupts are enabled before
- * calling smp_call_function(). The argument to this function is
- * the flags that will be used when restoring the interrupts. There is
- * local_irq_save() call before kgdb_roundup_cpus().
+ * in case of MIPS, smp_call_function() is used to roundup CPUs.
  *
  * On non-SMP systems, this is not called.
  */
@@ -139,11 +134,9 @@ static void hexagon_kgdb_nmi_hook(void *ignored)
 	kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs());
 }
 
-void kgdb_roundup_cpus(unsigned long flags)
+void kgdb_roundup_cpus(void)
 {
-	local_irq_enable();
 	smp_call_function(hexagon_kgdb_nmi_hook, NULL, 0);
-	local_irq_disable();
 }
 #endif
 
diff --git a/arch/mips/kernel/kgdb.c b/arch/mips/kernel/kgdb.c
index eb6c0d582626..6671a279966f 100644
--- a/arch/mips/kernel/kgdb.c
+++ b/arch/mips/kernel/kgdb.c
@@ -219,11 +219,9 @@ static void kgdb_call_nmi_hook(void *ignored)
 	set_fs(old_fs);
 }
 
-void kgdb_roundup_cpus(unsigned long flags)
+void kgdb_roundup_cpus(void)
 {
-	local_irq_enable();
 	smp_call_function(kgdb_call_nmi_hook, NULL, 0);
-	local_irq_disable();
 }
 
 static int compute_signal(int tt)
diff --git a/arch/powerpc/kernel/kgdb.c b/arch/powerpc/kernel/kgdb.c
index 59c578f865aa..b0e804844be0 100644
--- a/arch/powerpc/kernel/kgdb.c
+++ b/arch/powerpc/kernel/kgdb.c
@@ -124,7 +124,7 @@ static int kgdb_call_nmi_hook(struct pt_regs *regs)
 }
 
 #ifdef CONFIG_SMP
-void kgdb_roundup_cpus(unsigned long flags)
+void kgdb_roundup_cpus(void)
 {
 	smp_send_debugger_break();
 }
diff --git a/arch/sh/kernel/kgdb.c b/arch/sh/kernel/kgdb.c
index 4f04c6638a4d..86b3ea927e42 100644
--- a/arch/sh/kernel/kgdb.c
+++ b/arch/sh/kernel/kgdb.c
@@ -319,11 +319,9 @@ static void kgdb_call_nmi_hook(void *ignored)
 	kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs());
 }
 
-void kgdb_roundup_cpus(unsigned long flags)
+void kgdb_roundup_cpus(void)
 {
-	local_irq_enable();
 	smp_call_function(kgdb_call_nmi_hook, NULL, 0);
-	local_irq_disable();
 }
 
 static int __kgdb_notify(struct die_args *args, unsigned long cmd)
diff --git a/arch/sparc/kernel/smp_64.c b/arch/sparc/kernel/smp_64.c
index 4792e08ad36b..f45d876983f1 100644
--- a/arch/sparc/kernel/smp_64.c
+++ b/arch/sparc/kernel/smp_64.c
@@ -1014,7 +1014,7 @@ void flush_dcache_page_all(struct mm_struct *mm, struct page *page)
 }
 
 #ifdef CONFIG_KGDB
-void kgdb_roundup_cpus(unsigned long flags)
+void kgdb_roundup_cpus(void)
 {
 	smp_cross_call(&xcall_kgdb_capture, 0, 0, 0);
 }
diff --git a/arch/x86/kernel/kgdb.c b/arch/x86/kernel/kgdb.c
index 8e36f249646e..ac6291a4178d 100644
--- a/arch/x86/kernel/kgdb.c
+++ b/arch/x86/kernel/kgdb.c
@@ -422,21 +422,16 @@ static void kgdb_disable_hw_debug(struct pt_regs *regs)
 #ifdef CONFIG_SMP
 /**
  *	kgdb_roundup_cpus - Get other CPUs into a holding pattern
- *	@flags: Current IRQ state
  *
  *	On SMP systems, we need to get the attention of the other CPUs
  *	and get them be in a known state.  This should do what is needed
  *	to get the other CPUs to call kgdb_wait(). Note that on some arches,
  *	the NMI approach is not used for rounding up all the CPUs. For example,
- *	in case of MIPS, smp_call_function() is used to roundup CPUs. In
- *	this case, we have to make sure that interrupts are enabled before
- *	calling smp_call_function(). The argument to this function is
- *	the flags that will be used when restoring the interrupts. There is
- *	local_irq_save() call before kgdb_roundup_cpus().
+ *	in case of MIPS, smp_call_function() is used to roundup CPUs.
  *
  *	On non-SMP systems, this is not called.
  */
-void kgdb_roundup_cpus(unsigned long flags)
+void kgdb_roundup_cpus(void)
 {
 	apic->send_IPI_allbutself(APIC_DM_NMI);
 }
diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h
index e465bb15912d..05e5b2eb0d32 100644
--- a/include/linux/kgdb.h
+++ b/include/linux/kgdb.h
@@ -178,21 +178,16 @@ kgdb_arch_handle_exception(int vector, int signo, int err_code,
 
 /**
  *	kgdb_roundup_cpus - Get other CPUs into a holding pattern
- *	@flags: Current IRQ state
  *
  *	On SMP systems, we need to get the attention of the other CPUs
  *	and get them into a known state.  This should do what is needed
  *	to get the other CPUs to call kgdb_wait(). Note that on some arches,
  *	the NMI approach is not used for rounding up all the CPUs. For example,
- *	in case of MIPS, smp_call_function() is used to roundup CPUs. In
- *	this case, we have to make sure that interrupts are enabled before
- *	calling smp_call_function(). The argument to this function is
- *	the flags that will be used when restoring the interrupts. There is
- *	local_irq_save() call before kgdb_roundup_cpus().
+ *	in case of MIPS, smp_call_function() is used to roundup CPUs.
  *
  *	On non-SMP systems, this is not called.
  */
-extern void kgdb_roundup_cpus(unsigned long flags);
+extern void kgdb_roundup_cpus(void);
 
 /**
  *	kgdb_arch_set_pc - Generic call back to the program counter
diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c
index 65c0f1363788..f3cadda45f07 100644
--- a/kernel/debug/debug_core.c
+++ b/kernel/debug/debug_core.c
@@ -593,7 +593,7 @@ static int kgdb_cpu_enter(struct kgdb_state *ks, struct pt_regs *regs,
 
 	/* Signal the other CPUs to enter kgdb_wait() */
 	else if ((!kgdb_single_step) && kgdb_do_roundup)
-		kgdb_roundup_cpus(flags);
+		kgdb_roundup_cpus();
 #endif
 
 	/*
-- 
2.19.1.568.g152ad8e336-goog


^ permalink raw reply related

* Re: NXP P50XX/e5500: SMP doesn't work anymore with the latest Git kernel
From: LEROY Christophe @ 2018-10-29 18:13 UTC (permalink / raw)
  To: Christian Zigotzky; +Cc: linuxppc-dev
In-Reply-To: <f6ca1728-d312-1267-d9d7-18d2369f1bf7@xenosoft.de>

Christian Zigotzky <chzigotzky@xenosoft.de> a écrit :

> Hello,
>
> I figured out that the problem is in the OF source code of the  
> commit: Merge tag devicetree-for-4.20. [1]

That's a merge commit. Can you bisect the branch and identify the  
faulting commit ?

Christophe

>
> I reverted the following OF files and SMP works!
>
> drivers/of/base.c
> drivers/of/device.c
> drivers/of/of_mdio.c
> drivers/of/of_numa.c
> drivers/of/of_private.h
> drivers/of/overlay.c
> drivers/of/platform.c
> drivers/of/unittest-data/overlay_15.dts
> drivers/of/unittest-data/tests-overlay.dtsi
> drivers/of/unittest.c
> include/linux/of.h
>
> Cheers,
> Christian
>
> [1]  
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b27186abb37b7bd19e0ca434f4f425c807dbd708
>
>
> On 29 October 2018 at 10:56AM, Christian Zigotzky wrote:
>> Hello,
>>
>> I have figured out that the commit 'devicetree-for-4.20' [1] is  
>> responsible for the SMP problem. I was able to revert this commit  
>> with 'git revert b27186abb37b7bd19e0ca434f4f425c807dbd708 -m 1'  
>> today.
>>
>> [master ec81438] Revert "Merge tag 'devicetree-for-4.20' of  
>> git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux"
>> 138 files changed, 931 insertions(+), 1538 deletions(-)
>> rename Documentation/devicetree/bindings/arm/{atmel-sysregs.txt =>  
>> atmel-at91.txt} (67%)
>> delete mode 100644  
>> Documentation/devicetree/bindings/arm/freescale/fsl,layerscape-dcfg.txt
>> delete mode 100644  
>> Documentation/devicetree/bindings/arm/freescale/fsl,layerscape-scfg.txt
>> rename Documentation/devicetree/bindings/arm/{zte,sysctrl.txt =>  
>> zte.txt} (62%)
>> delete mode 100644 Documentation/devicetree/bindings/misc/lwn-bk4.txt
>> create mode 100644 arch/c6x/boot/dts/linked_dtb.S
>> delete mode 100644 arch/nios2/boot/dts/Makefile
>> create mode 100644 arch/nios2/boot/linked_dtb.S
>> delete mode 100644 arch/powerpc/boot/dts/Makefile
>> delete mode 100644 arch/powerpc/boot/dts/fsl/Makefile
>> delete mode 100644 scripts/dtc/yamltree.c
>>
>> It solves the SMP problem! SMP works again on my P5020 board and on  
>> virtual e5500 QEMU machines.
>>
>> QEMU command: ./qemu-system-ppc64 -M ppce500 -cpu e5500 -m 2048  
>> -kernel /home/christian/Downloads/uImage-4.20-alpha5 -drive  
>> format=raw,file=/home/christian/Dokumente/ubuntu_MATE_16.04.3_LTS_PowerPC_QEMU/ubuntu_MATE_16.04_PowerPC.img,index=0,if=virtio -nic user,model=e1000 -append "rw root=/dev/vda3" -device virtio-vga -device virtio-mouse-pci -device virtio-keyboard-pci -soundhw es1370 -smp  
>> 4
>>
>> Screenshot:  
>> https://plus.google.com/u/0/photos/photo/115515624056477014971/6617705776207990082
>>
>> Do we need a new dtb file or is it a bug?
>>
>> Thanks,
>> Christian
>>
>> [1]  
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b27186abb37b7bd19e0ca434f4f425c807dbd708
>>
>>
>> On 28 October 2018 at 5:35PM, Christian Zigotzky wrote:
>>> Hello,
>>>
>>> SMP doesn't work anymore with the latest Git kernel (28/10/18  
>>> 11:12AM GMT) on my P5020 board and on virtual e5500 QEMU machines.
>>>
>>> Board with P5020 dual core CPU:
>>>
>>> [    0.000000] -----------------------------------------------------
>>> [    0.000000] phys_mem_size     = 0x200000000
>>> [    0.000000] dcache_bsize      = 0x40
>>> [    0.000000] icache_bsize      = 0x40
>>> [    0.000000] cpu_features      = 0x00000003008003b4
>>> [    0.000000]   possible        = 0x00000003009003b4
>>> [    0.000000]   always          = 0x00000003008003b4
>>> [    0.000000] cpu_user_features = 0xcc008000 0x08000000
>>> [    0.000000] mmu_features      = 0x000a0010
>>> [    0.000000] firmware_features = 0x0000000000000000
>>> [    0.000000] -----------------------------------------------------
>>> [    0.000000] CoreNet Generic board
>>>
>>>     ...
>>>
>>> [    0.002161] smp: Bringing up secondary CPUs ...
>>> [    0.002339] No cpu-release-addr for cpu 1
>>> [    0.002347] smp: failed starting cpu 1 (rc -2)
>>> [    0.002401] smp: Brought up 1 node, 1 CPU
>>>
>>> Virtual e5500 quad core QEMU machine:
>>>
>>> [    0.026394] smp: Bringing up secondary CPUs ...
>>> [    0.027831] No cpu-release-addr for cpu 1
>>> [    0.027989] smp: failed starting cpu 1 (rc -2)
>>> [    0.030143] No cpu-release-addr for cpu 2
>>> [    0.030304] smp: failed starting cpu 2 (rc -2)
>>> [    0.032400] No cpu-release-addr for cpu 3
>>> [    0.032533] smp: failed starting cpu 3 (rc -2)
>>> [    0.033117] smp: Brought up 1 node, 1 CPU
>>>
>>> QEMU command: ./qemu-system-ppc64 -M ppce500 -cpu e5500 -m 2048  
>>> -kernel  
>>> /home/christian/Downloads/vmlinux-4.20-alpha4-AmigaOne_X1000_X5000/X5000_and_QEMU_e5500/uImage-4.20 -drive format=raw,file=/home/christian/Downloads/MATE_PowerPC_Remix_2017_0.9.img,index=0,if=virtio -nic user,model=e1000 -append "rw root=/dev/vda" -device virtio-vga -device virtio-mouse-pci -device virtio-keyboard-pci -usb -soundhw es1370 -smp  
>>> 4
>>>
>>> .config:
>>>
>>> ...
>>> CONFIG_SMP=y
>>> CONFIG_NR_CPUS=4
>>> ...
>>>
>>> Please test the latest Git kernel on your NXP P50XX boards.
>>>
>>> Thanks,
>>> Christian
>>>
>>
>>



^ permalink raw reply

* [PATCH] powerpc/pseries: Perform full re-add of CPU for topology update post-migration
From: Nathan Fontenot @ 2018-10-29 18:43 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: ldufour

On pseries systems, performing a partition migration can result in
altering the nodes a CPU is assigned to on the destination system. For
exampl, pre-migration on the source system CPUs are in node 1 and 3,
post-migration on the destination system CPUs are in nodes 2 and 3.

Handling the node change for a CPU can cause corruption in the slab
cache if we hit a timing where a CPUs node is changed while cache_reap()
is invoked. The corruption occurs because the slab cache code appears
to rely on the CPU and slab cache pages being on the same node.

The current dynamic updating of a CPUs node done in arch/powerpc/mm/numa.c
does not prevent us from hitting this scenario.

Changing the device tree property update notification handler that
recognizes an affinity change for a CPU to do a full DLPAR remove and
add of the CPU instead of dynamically changing its node resolves this
issue.

Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/topology.h          |    2 ++
 arch/powerpc/mm/numa.c                       |    9 +--------
 arch/powerpc/platforms/pseries/hotplug-cpu.c |   19 +++++++++++++++++++
 3 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/include/asm/topology.h b/arch/powerpc/include/asm/topology.h
index a4a718dbfec6..f85e2b01c3df 100644
--- a/arch/powerpc/include/asm/topology.h
+++ b/arch/powerpc/include/asm/topology.h
@@ -132,6 +132,8 @@ static inline void shared_proc_topology_init(void) {}
 #define topology_sibling_cpumask(cpu)	(per_cpu(cpu_sibling_map, cpu))
 #define topology_core_cpumask(cpu)	(per_cpu(cpu_core_map, cpu))
 #define topology_core_id(cpu)		(cpu_to_core_id(cpu))
+
+int dlpar_cpu_readd(int cpu);
 #endif
 #endif
 
diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index 693ae1c1acba..bb6a7b56bef7 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -1461,13 +1461,6 @@ static void reset_topology_timer(void)
 
 #ifdef CONFIG_SMP
 
-static void stage_topology_update(int core_id)
-{
-	cpumask_or(&cpu_associativity_changes_mask,
-		&cpu_associativity_changes_mask, cpu_sibling_mask(core_id));
-	reset_topology_timer();
-}
-
 static int dt_update_callback(struct notifier_block *nb,
 				unsigned long action, void *data)
 {
@@ -1480,7 +1473,7 @@ static int dt_update_callback(struct notifier_block *nb,
 		    !of_prop_cmp(update->prop->name, "ibm,associativity")) {
 			u32 core_id;
 			of_property_read_u32(update->dn, "reg", &core_id);
-			stage_topology_update(core_id);
+			rc = dlpar_cpu_readd(core_id);
 			rc = NOTIFY_OK;
 		}
 		break;
diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
index 2f8e62163602..97feb6e79f1a 100644
--- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
+++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
@@ -802,6 +802,25 @@ static int dlpar_cpu_add_by_count(u32 cpus_to_add)
 	return rc;
 }
 
+int dlpar_cpu_readd(int cpu)
+{
+	struct device_node *dn;
+	struct device *dev;
+	u32 drc_index;
+	int rc;
+
+	dev = get_cpu_device(cpu);
+	dn = dev->of_node;
+
+	rc = of_property_read_u32(dn, "ibm,my-drc-index", &drc_index);
+
+	rc = dlpar_cpu_remove_by_index(drc_index);
+	if (!rc)
+		rc = dlpar_cpu_add(drc_index);
+
+	return rc;
+}
+
 int dlpar_cpu(struct pseries_hp_errorlog *hp_elog)
 {
 	u32 count, drc_index;


^ permalink raw reply related

* Re: [PATCH v08 0/5] powerpc/hotplug: Update affinity for migrated CPUs
From: Michal Suchánek @ 2018-10-29 19:51 UTC (permalink / raw)
  To: Michael Bringmann
  Cc: Nathan Fontenot, linuxppc-dev, Tyrel Datwyler, Thomas Falcon,
	John Allen
In-Reply-To: <9573dc30-01ad-2650-c247-28df08875cef@linux.vnet.ibm.com>

On Sun, 29 Jul 2018 08:18:34 -0500
Michael Bringmann <mwb@linux.vnet.ibm.com> wrote:

> The migration of LPARs across Power systems affects many attributes
> including that of the associativity of CPUs.  The patches in this
> set execute when a system is coming up fresh upon a migration target.
> They are intended to,
> 
> * Recognize changes to the associativity of CPUs recorded in internal
>   data structures when compared to the latest copies in the device
> tree.
> * Generate calls to other code layers to reset the data structures
>   related to associativity of the CPUs.
> * Re-register the 'changed' entities into the target system.
>   Re-registration of CPUs mostly entails acting as if they have been
>   newly hot-added into the target system.
> 
> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
> 
Hello,

what is the status of this patchset other than it no longer applies?

Thanks

Michal

^ permalink raw reply

* Re: NXP P50XX/e5500: SMP doesn't work anymore with the latest Git kernel
From: Christian Zigotzky @ 2018-10-29 20:12 UTC (permalink / raw)
  To: LEROY Christophe; +Cc: linuxppc-dev
In-Reply-To: <20181029191321.Horde.fxo4MJUhTstCq8zaw1lK7Q3@messagerie.si.c-s.fr>

Hello Christophe,

Bisecting isn’t necessary because the problem is in the following files.

drivers/of/base.c
drivers/of/device.c
drivers/of/of_mdio.c
drivers/of/of_numa.c
drivers/of/of_private.h
drivers/of/overlay.c
drivers/of/platform.c
drivers/of/unittest-data/overlay_15.dts
drivers/of/unittest-data/tests-overlay.dtsi
drivers/of/unittest.c
include/linux/of.h

I removed the OF modifications from the merge commit 'devicetree-for-4.20' and SMP works again.

Cheers,
Christian

Sent from my iPhone

> On 29. Oct 2018, at 19:13, LEROY Christophe <christophe.leroy@c-s.fr> wrote:
> 
> Christian Zigotzky <chzigotzky@xenosoft.de> a écrit :
> 
>> Hello,
>> 
>> I figured out that the problem is in the OF source code of the commit: Merge tag devicetree-for-4.20. [1]
> 
> That's a merge commit. Can you bisect the branch and identify the faulting commit ?
> 
> Christophe
> 
>> 
>> I reverted the following OF files and SMP works!
>> 
>> drivers/of/base.c
>> drivers/of/device.c
>> drivers/of/of_mdio.c
>> drivers/of/of_numa.c
>> drivers/of/of_private.h
>> drivers/of/overlay.c
>> drivers/of/platform.c
>> drivers/of/unittest-data/overlay_15.dts
>> drivers/of/unittest-data/tests-overlay.dtsi
>> drivers/of/unittest.c
>> include/linux/of.h
>> 
>> Cheers,
>> Christian
>> 
>> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b27186abb37b7bd19e0ca434f4f425c807dbd708
>> 
>> 
>>> On 29 October 2018 at 10:56AM, Christian Zigotzky wrote:
>>> Hello,
>>> 
>>> I have figured out that the commit 'devicetree-for-4.20' [1] is responsible for the SMP problem. I was able to revert this commit with 'git revert b27186abb37b7bd19e0ca434f4f425c807dbd708 -m 1' today.
>>> 
>>> [master ec81438] Revert "Merge tag 'devicetree-for-4.20' of git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux"
>>> 138 files changed, 931 insertions(+), 1538 deletions(-)
>>> rename Documentation/devicetree/bindings/arm/{atmel-sysregs.txt => atmel-at91.txt} (67%)
>>> delete mode 100644 Documentation/devicetree/bindings/arm/freescale/fsl,layerscape-dcfg.txt
>>> delete mode 100644 Documentation/devicetree/bindings/arm/freescale/fsl,layerscape-scfg.txt
>>> rename Documentation/devicetree/bindings/arm/{zte,sysctrl.txt => zte.txt} (62%)
>>> delete mode 100644 Documentation/devicetree/bindings/misc/lwn-bk4.txt
>>> create mode 100644 arch/c6x/boot/dts/linked_dtb.S
>>> delete mode 100644 arch/nios2/boot/dts/Makefile
>>> create mode 100644 arch/nios2/boot/linked_dtb.S
>>> delete mode 100644 arch/powerpc/boot/dts/Makefile
>>> delete mode 100644 arch/powerpc/boot/dts/fsl/Makefile
>>> delete mode 100644 scripts/dtc/yamltree.c
>>> 
>>> It solves the SMP problem! SMP works again on my P5020 board and on virtual e5500 QEMU machines.
>>> 
>>> QEMU command: ./qemu-system-ppc64 -M ppce500 -cpu e5500 -m 2048 -kernel /home/christian/Downloads/uImage-4.20-alpha5 -drive format=raw,file=/home/christian/Dokumente/ubuntu_MATE_16.04.3_LTS_PowerPC_QEMU/ubuntu_MATE_16.04_PowerPC.img,index=0,if=virtio -nic user,model=e1000 -append "rw root=/dev/vda3" -device virtio-vga -device virtio-mouse-pci -device virtio-keyboard-pci -soundhw es1370 -smp 4
>>> 
>>> Screenshot: https://plus.google.com/u/0/photos/photo/115515624056477014971/6617705776207990082
>>> 
>>> Do we need a new dtb file or is it a bug?
>>> 
>>> Thanks,
>>> Christian
>>> 
>>> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b27186abb37b7bd19e0ca434f4f425c807dbd708
>>> 
>>> 
>>>> On 28 October 2018 at 5:35PM, Christian Zigotzky wrote:
>>>> Hello,
>>>> 
>>>> SMP doesn't work anymore with the latest Git kernel (28/10/18 11:12AM GMT) on my P5020 board and on virtual e5500 QEMU machines.
>>>> 
>>>> Board with P5020 dual core CPU:
>>>> 
>>>> [    0.000000] -----------------------------------------------------
>>>> [    0.000000] phys_mem_size     = 0x200000000
>>>> [    0.000000] dcache_bsize      = 0x40
>>>> [    0.000000] icache_bsize      = 0x40
>>>> [    0.000000] cpu_features      = 0x00000003008003b4
>>>> [    0.000000]   possible        = 0x00000003009003b4
>>>> [    0.000000]   always          = 0x00000003008003b4
>>>> [    0.000000] cpu_user_features = 0xcc008000 0x08000000
>>>> [    0.000000] mmu_features      = 0x000a0010
>>>> [    0.000000] firmware_features = 0x0000000000000000
>>>> [    0.000000] -----------------------------------------------------
>>>> [    0.000000] CoreNet Generic board
>>>> 
>>>>     ...
>>>> 
>>>> [    0.002161] smp: Bringing up secondary CPUs ...
>>>> [    0.002339] No cpu-release-addr for cpu 1
>>>> [    0.002347] smp: failed starting cpu 1 (rc -2)
>>>> [    0.002401] smp: Brought up 1 node, 1 CPU
>>>> 
>>>> Virtual e5500 quad core QEMU machine:
>>>> 
>>>> [    0.026394] smp: Bringing up secondary CPUs ...
>>>> [    0.027831] No cpu-release-addr for cpu 1
>>>> [    0.027989] smp: failed starting cpu 1 (rc -2)
>>>> [    0.030143] No cpu-release-addr for cpu 2
>>>> [    0.030304] smp: failed starting cpu 2 (rc -2)
>>>> [    0.032400] No cpu-release-addr for cpu 3
>>>> [    0.032533] smp: failed starting cpu 3 (rc -2)
>>>> [    0.033117] smp: Brought up 1 node, 1 CPU
>>>> 
>>>> QEMU command: ./qemu-system-ppc64 -M ppce500 -cpu e5500 -m 2048 -kernel /home/christian/Downloads/vmlinux-4.20-alpha4-AmigaOne_X1000_X5000/X5000_and_QEMU_e5500/uImage-4.20 -drive format=raw,file=/home/christian/Downloads/MATE_PowerPC_Remix_2017_0.9.img,index=0,if=virtio -nic user,model=e1000 -append "rw root=/dev/vda" -device virtio-vga -device virtio-mouse-pci -device virtio-keyboard-pci -usb -soundhw es1370 -smp 4
>>>> 
>>>> .config:
>>>> 
>>>> ...
>>>> CONFIG_SMP=y
>>>> CONFIG_NR_CPUS=4
>>>> ...
>>>> 
>>>> Please test the latest Git kernel on your NXP P50XX boards.
>>>> 
>>>> Thanks,
>>>> Christian
>>>> 
>>> 
>>> 
> 
> 

^ permalink raw reply

* Re: [RFC PATCH] seccomp: Add protection keys into seccomp_data
From: Michael Sammler @ 2018-10-29 21:55 UTC (permalink / raw)
  To: Dave Hansen, Jann Horn
  Cc: wad, Kees Cook, Linux API, Dave Hansen, linuxram, Andy Lutomirski,
	linuxppc-dev
In-Reply-To: <62e09400-0443-8db9-a389-ba4f4201226b@intel.com>

Am 29.10.2018 um 18:29 schrieb Dave Hansen:

> On 10/29/18 9:48 AM, Jann Horn wrote:
>> On Mon, Oct 29, 2018 at 5:37 PM Dave Hansen <dave.hansen@intel.com> wrote:
>>> I'm not sure this is a great use for PKRU.  I *think* the basic problem
>>> is that you want to communicate some rights information down into a
>>> filter, and you want to communicate it with PKRU.  While it's handy to
>>> have an extra register that nobody (generally) mucks with, I'm not quite
>>> convinced that we want to repurpose it this way.
>> That's not how I understand it; I believe that the context is probably
>> https://arxiv.org/pdf/1801.06822.pdf ?
>> My understanding is that PKRU is used for lightweight in-process
>> sandboxing, and to extend this sandbox protection to the syscall
>> interface, it is necessary to expose PKRU state to seccomp filters.
>> In other words, this isn't using PKRU exclusively for passing rights
>> into a filter, but it has to use PKRU anyway.
> PKRU gives information about rights to various bits of application data.
>   From that, a seccomp filter can infer the context, and thus the ability
> for the code to call a given syscall at a certain point in time.
>
> This makes PKRU an opt-in part of the syscall ABI, which is pretty
> interesting.  We _could_ do the same kind of thing with any callee-saved
> general purpose register, but PKRU is particularly attractive because
> there is only one instruction that writes to it (well, outside of
> XSAVE*), and random library code is very unlikely at this point to be
> using it.
I agree with you on the points, why PKRU is particularly attractive but 
I think the most important point is that PKRU is _not_ a general purpose 
register, but is already used to control access to some resource 
(memory). This patch would allow to also control access to another 
resource (system calls) using the PKRU. This is why it makes sense to 
use the PKRU in this patch instead of another callee-saved register.
> PKRU getting reset on signals, and the requirement now that it *can't*
> be changed if you make syscalls probably needs to get thought about very
> carefully before we do this, though.
I am not sure, whether I follow you. Are you saying, that PKRU is 
currently not guaranteed to be preserved across system calls?
This would make it very hard to use protection keys if libc does not 
save and restore the PKRU before/after systemcalls (and I am not aware 
of this).

Or do you mean, that the kernel might want to use the PKRU register for 
its own purposes while it is executing?
Then the solution you proposed in another email in this thread would 
work: instead of providing the seccomp filter with the current value of 
the PKRU (which might be different from what the user space expects) use 
the user space value which must have been saved somewhere (otherwise it 
would not be possible to restore it).

Or are you afraid, that one part of a user space program installs a 
seccomp filter, which blocks system calls based on the PKRU, and another 
part of the same program (maybe a library) changes the PKRU in a way, 
which the first part did not expect and the program dies because it 
tries to do a forbidden system call?
I don't know whether the kernel can (and wants) do anything against 
this. This problem also exists without this patch if you replace system 
call with memory access.

-- Michael

^ permalink raw reply

* Re: [PATCH] seccomp: Add pkru into seccomp_data
From: Kees Cook @ 2018-10-29 22:01 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Florian Weimer, Will Drewry, Linux API, Ram Pai, Michael Sammler,
	linuxppc-dev
In-Reply-To: <7DC76493-28C9-4CAA-9262-E809F52459DB@amacapital.net>

On Thu, Oct 25, 2018 at 5:49 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>> On Oct 25, 2018, at 5:35 PM, Kees Cook <keescook@chromium.org> wrote:
>>
>>> On Fri, Oct 26, 2018 at 12:00 AM, Andy Lutomirski <luto@amacapital.net> wrote:
>>> You could bite the bullet and add seccomp eBPF support :)
>>
>> I'm not convinced this is a good enough reason for gaining the eBPF
>> attack surface yet.
>
> Is it an interesting attack surface?  It’s certainly scarier if you’re worried about attacks from the sandbox creator, but the security inside the sandbox should be more or less equivalent, no?

Yeah, I'm more worried about the creator: I don't want kernel exploits
via seccomp APIs. :P There have been kind of a long list of
eBPF-related flaws, so I'd really rather not tie seccomp to it. As for
sandbox escapes, I don't think there's too much concern, but I do
worry about "unexpected" situations exposed by eBPF (i.e. maps or
functions not doing what was expected, or allowing map manipulation
from outside). seccomp cBPF is pretty self-contained right now, so I'd
like a strong reason to justify the increase in code complexity and
related attack surface.

-- 
Kees Cook

^ permalink raw reply

* Re: [PATCH 2/2] selftests/powerpc: Skip test instead of failing
From: Thiago Jung Bauermann @ 2018-10-29 22:08 UTC (permalink / raw)
  To: Breno Leitao; +Cc: linuxppc-dev, Tyrel Datwyler
In-Reply-To: <132d7e4f-153f-8ba4-95f2-f0d9c1a66f28@debian.org>


Breno Leitao <leitao@debian.org> writes:

> Hi Tyrel,
>
> On 10/23/2018 05:41 PM, Tyrel Datwyler wrote:
>>> --- a/tools/testing/selftests/powerpc/ptrace/core-pkey.c
>>> +++ b/tools/testing/selftests/powerpc/ptrace/core-pkey.c
>>> @@ -352,17 +352,11 @@ static int write_core_pattern(const char *core_pattern)
>>>  	FILE *f;
>>>
>>>  	f = fopen(core_pattern_file, "w");
>>> -	if (!f) {
>>> -		perror("Error writing to core_pattern file");
>>> -		return TEST_FAIL;
>>> -	}
>>> +	SKIP_IF(!f);
>>>
>>>  	ret = fwrite(core_pattern, 1, len, f);
>>>  	fclose(f);
>>> -	if (ret != len) {
>>> -		perror("Error writing to core_pattern file");
>>> -		return TEST_FAIL;
>>> -	}
>>> +	SKIP_IF(ret != len);
>
>> If we don't have proper privileges we should fail on the open, right?
>> So wouldn't we still want to fail on the write if something goes
>> wrong?
>
> That is a good point. Should the test fail or skip if it is not possible
> to create the infrastructure to run the core test?
>
> Trying to find the answer in the current test sets, I find tests where
> the self test skips if the test environment is not able to be set up, as
> for example, when a memory allocation fails.
>
> File: tools/testing/selftests/powerpc/alignment/alignment_handler.c
>
>         ci1 = mmap(NULL, bufsize, PROT_WRITE, MAP_SHARED,
>                    fd, bufsize);
>         if ((ci0 == MAP_FAILED) || (ci1 == MAP_FAILED)) {
>                 printf("\n");
>                 perror("mmap failed");
>                 SKIP_IF(1);
>         }

I think TEST_FAIL means the test was able to exercise the feature
and found a problem with it. In this case, the test wasn't able to
exercise the feature so it's not appropriate.

Ideally, there should be a TEST_ERROR result for a case like this where
an unexpected problem prevented the testcase from exercising the
feature.

If we're to use the an existing result then I vote for SKIP_IF.

For reference, here are the test results that DejaGnu supports (it is
the test harness used by some GNU projects):

https://www.gnu.org/software/dejagnu/manual/Output-States.html

I would say that SKIP_IF corresponds to UNSUPPORTED in DejaGnu.

--
Thiago Jung Bauermann
IBM Linux Technology Center


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox