* [PATCH v4 0/3] powerpc: Enabling IMA arch specific secure boot policies
From: Nayna Jain @ 2019-06-11 17:06 UTC (permalink / raw)
To: linuxppc-dev, linux-efi, linux-integrity, linux-kernel
Cc: Ard Biesheuvel, Nayna Jain, Claudio Carvalho, Mimi Zohar,
Matthew Garret, Paul Mackerras, Jeremy Kerr, Satheesh Rajendran
This patch set, previously named "powerpc: Enabling secure boot on powernv
systems - Part 1", is part of a series that implements secure boot on
PowerNV systems.
In order to verify the OS kernel on PowerNV, secure boot requires X.509
certificates trusted by the platform, the secure boot modes, and several
other pieces of information. These are stored in secure variables
controlled by OPAL, also known as OPAL secure variables.
The IMA architecture specific policy support on POWER is dependent on OPAL
runtime services to access secure variables. OPAL APIs in skiboot are
modified to define generic interface compatible to any backend. This
patchset is consequently updated to be compatible with new OPAL API
interface. This has cleaned up any EFIsms in the arch specific code.
Further, the ima arch specific policies are updated to be able to support
appended signatures. They also now use per policy template.
Exposing the OPAL secure variables to userspace will be posted as a
separate patch set, allowing the IMA architecture specific policy on POWER
to be upstreamed independently.
This patch set adds the following features:
1. Add support for OPAL Runtime API to access secure variables controlled
by OPAL.
2. Define IMA arch-specific policies based on the secure boot state and
mode of the system. On secure boot enabled PowerNV systems, the OS kernel
signature will be verified by IMA appraisal.
Pre-requisites for this patchset are:
1. OPAL APIs in Skiboot[1]
2. Appended signature support in IMA [2]
3. Per policy template support in IMA [3]
[1] https://patchwork.ozlabs.org/project/skiboot/list/?series=112868
[2] https://patchwork.ozlabs.org/cover/1087361/. Updated version will be
posted soon
[3] Repo: https://kernel.googlesource.com/pub/scm/linux/kernel/git/zohar/linux-integrity
Branch: next-queued-testing. Commit: f241bb1f42aa95
----------------------------------------------------------------------------------
Original Cover Letter:
This patch set is part of a series that implements secure boot on PowerNV
systems.
In order to verify the OS kernel on PowerNV, secure boot requires X.509
certificates trusted by the platform, the secure boot modes, and several
other pieces of information. These are stored in secure variables
controlled by OPAL, also known as OPAL secure variables.
The IMA architecture specific policy support on Power is dependent on OPAL
runtime services to access secure variables. Instead of directly accessing
the OPAL runtime services, version 3 of this patch set relied upon the
EFI hooks. This version drops that dependency and calls the OPAL runtime
services directly. Skiboot OPAL APIs are due to be posted soon.
Exposing the OPAL secure variables to userspace will be posted as a
separate patch set, allowing the IMA architecture specific policy on Power
to be upstreamed independently.
This patch set adds the following features:
1. Add support for OPAL Runtime API to access secure variables controlled
by OPAL.
2. Define IMA arch-specific policies based on the secure boot state and
mode of the system. On secure boot enabled powernv systems, the OS kernel
signature will be verified by IMA appraisal.
[1] https://patchwork.kernel.org/cover/10882149/
Changelog:
v4:
* Fixed the build issue as reported by Satheesh Rajendran.
v3:
* OPAL APIs in Patch 1 are updated to provide generic interface based on
key/keylen. This patchset updates kernel OPAL APIs to be compatible with
generic interface.
* Patch 2 is cleaned up to use new OPAL APIs.
* Since OPAL can support different types of backend which can vary in the
variable interpretation, the Patch 2 is updated to add a check for the
backend version
* OPAL API now expects consumer to first check the supported backend version
before calling other secvar OPAL APIs. This check is now added in patch 2.
* IMA policies in Patch 3 is updated to specify appended signature and
per policy template.
* The patches now are free of any EFIisms.
v2:
* Removed Patch 1: powerpc/include: Override unneeded early ioremap
functions
* Updated Subject line and patch description of the Patch 1 of this series
* Removed dependency of OPAL_SECVAR on EFI, CPU_BIG_ENDIAN and UCS2_STRING
* Changed OPAL APIs from static to non-static. Added opal-secvar.h for the
same
* Removed EFI hooks from opal_secvar.c
* Removed opal_secvar_get_next(), opal_secvar_enqueue() and
opal_query_variable_info() function
* get_powerpc_sb_mode() in secboot.c now directly calls OPAL Runtime API
rather than via EFI hooks.
* Fixed log messages in get_powerpc_sb_mode() function.
* Added dependency for PPC_SECURE_BOOT on configs PPC64 and OPAL_SECVAR
* Replaced obj-$(CONFIG_IMA) with obj-$(CONFIG_PPC_SECURE_BOOT) in
arch/powerpc/kernel/Makefile
Claudio Carvalho (1):
powerpc/powernv: Add OPAL API interface to get secureboot state
Nayna Jain (2):
powerpc/powernv: detect the secure boot mode of the system
powerpc: Add support to initialize ima policy rules
arch/powerpc/Kconfig | 14 ++++
arch/powerpc/include/asm/opal-api.h | 4 +-
arch/powerpc/include/asm/opal-secvar.h | 23 ++++++
arch/powerpc/include/asm/opal.h | 6 ++
arch/powerpc/include/asm/secboot.h | 21 +++++
arch/powerpc/kernel/Makefile | 1 +
arch/powerpc/kernel/ima_arch.c | 54 +++++++++++++
arch/powerpc/platforms/powernv/Kconfig | 6 ++
arch/powerpc/platforms/powernv/Makefile | 1 +
arch/powerpc/platforms/powernv/opal-call.c | 2 +
arch/powerpc/platforms/powernv/opal-secvar.c | 85 ++++++++++++++++++++
arch/powerpc/platforms/powernv/secboot.c | 61 ++++++++++++++
include/linux/ima.h | 3 +-
13 files changed, 279 insertions(+), 2 deletions(-)
create mode 100644 arch/powerpc/include/asm/opal-secvar.h
create mode 100644 arch/powerpc/include/asm/secboot.h
create mode 100644 arch/powerpc/kernel/ima_arch.c
create mode 100644 arch/powerpc/platforms/powernv/opal-secvar.c
create mode 100644 arch/powerpc/platforms/powernv/secboot.c
--
2.20.1
^ permalink raw reply
* [PATCH v4 1/3] powerpc/powernv: Add OPAL API interface to get secureboot state
From: Nayna Jain @ 2019-06-11 17:06 UTC (permalink / raw)
To: linuxppc-dev, linux-efi, linux-integrity, linux-kernel
Cc: Ard Biesheuvel, Nayna Jain, Claudio Carvalho, Mimi Zohar,
Matthew Garret, Paul Mackerras, Jeremy Kerr, Satheesh Rajendran
In-Reply-To: <1560272765-5768-1-git-send-email-nayna@linux.ibm.com>
From: Claudio Carvalho <cclaudio@linux.ibm.com>
The X.509 certificates trusted by the platform and other information
required to secure boot the OS kernel are wrapped in secure variables,
which are controlled by OPAL.
This patch adds support to read OPAL secure variables through
OPAL_SECVAR_GET call. It returns the metadata and data for a given secure
variable based on the unique key.
Since OPAL can support different types of backend which can vary in the
variable interpretation, a new OPAL API call named OPAL_SECVAR_BACKEND, is
added to retrieve the supported backend version. This helps the consumer
to know how to interpret the variable.
This support can be enabled using CONFIG_OPAL_SECVAR
Signed-off-by: Claudio Carvalho <cclaudio@linux.ibm.com>
Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
---
This patch depends on a new OPAL call that is being added to skiboot.
The patch set that implements the new call has been posted to
https://patchwork.ozlabs.org/project/skiboot/list/?series=112868
arch/powerpc/include/asm/opal-api.h | 4 +-
arch/powerpc/include/asm/opal-secvar.h | 23 ++++++
arch/powerpc/include/asm/opal.h | 6 ++
arch/powerpc/platforms/powernv/Kconfig | 6 ++
arch/powerpc/platforms/powernv/Makefile | 1 +
arch/powerpc/platforms/powernv/opal-call.c | 2 +
arch/powerpc/platforms/powernv/opal-secvar.c | 85 ++++++++++++++++++++
7 files changed, 126 insertions(+), 1 deletion(-)
create mode 100644 arch/powerpc/include/asm/opal-secvar.h
create mode 100644 arch/powerpc/platforms/powernv/opal-secvar.c
diff --git a/arch/powerpc/include/asm/opal-api.h b/arch/powerpc/include/asm/opal-api.h
index e1577cfa7186..a505e669b4b6 100644
--- a/arch/powerpc/include/asm/opal-api.h
+++ b/arch/powerpc/include/asm/opal-api.h
@@ -212,7 +212,9 @@
#define OPAL_HANDLE_HMI2 166
#define OPAL_NX_COPROC_INIT 167
#define OPAL_XIVE_GET_VP_STATE 170
-#define OPAL_LAST 170
+#define OPAL_SECVAR_GET 173
+#define OPAL_SECVAR_BACKEND 177
+#define OPAL_LAST 177
#define QUIESCE_HOLD 1 /* Spin all calls at entry */
#define QUIESCE_REJECT 2 /* Fail all calls with OPAL_BUSY */
diff --git a/arch/powerpc/include/asm/opal-secvar.h b/arch/powerpc/include/asm/opal-secvar.h
new file mode 100644
index 000000000000..b677171a0368
--- /dev/null
+++ b/arch/powerpc/include/asm/opal-secvar.h
@@ -0,0 +1,23 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * PowerNV definitions for secure variables OPAL API.
+ *
+ * Copyright (C) 2019 IBM Corporation
+ * Author: Claudio Carvalho <cclaudio@linux.ibm.com>
+ *
+ */
+#ifndef OPAL_SECVAR_H
+#define OPAL_SECVAR_H
+
+enum {
+ BACKEND_NONE = 0,
+ BACKEND_TC_COMPAT_V1,
+};
+
+extern int opal_get_variable(u8 *key, unsigned long ksize,
+ u8 *metadata, unsigned long *mdsize,
+ u8 *data, unsigned long *dsize);
+
+extern int opal_variable_version(unsigned long *backend);
+
+#endif
diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
index 4cc37e708bc7..57d2c2356eda 100644
--- a/arch/powerpc/include/asm/opal.h
+++ b/arch/powerpc/include/asm/opal.h
@@ -394,6 +394,12 @@ void opal_powercap_init(void);
void opal_psr_init(void);
void opal_sensor_groups_init(void);
+extern int opal_secvar_get(uint64_t k_key, uint64_t k_key_len,
+ uint64_t k_metadata, uint64_t k_metadata_size,
+ uint64_t k_data, uint64_t k_data_size);
+
+extern int opal_secvar_backend(uint64_t k_backend);
+
#endif /* __ASSEMBLY__ */
#endif /* _ASM_POWERPC_OPAL_H */
diff --git a/arch/powerpc/platforms/powernv/Kconfig b/arch/powerpc/platforms/powernv/Kconfig
index 850eee860cf2..65b060539b5c 100644
--- a/arch/powerpc/platforms/powernv/Kconfig
+++ b/arch/powerpc/platforms/powernv/Kconfig
@@ -47,3 +47,9 @@ config PPC_VAS
VAS adapters are found in POWER9 based systems.
If unsure, say N.
+
+config OPAL_SECVAR
+ bool "OPAL Secure Variables"
+ depends on PPC_POWERNV
+ help
+ This enables the kernel to access OPAL secure variables.
diff --git a/arch/powerpc/platforms/powernv/Makefile b/arch/powerpc/platforms/powernv/Makefile
index da2e99efbd04..6651c742e530 100644
--- a/arch/powerpc/platforms/powernv/Makefile
+++ b/arch/powerpc/platforms/powernv/Makefile
@@ -16,3 +16,4 @@ obj-$(CONFIG_PERF_EVENTS) += opal-imc.o
obj-$(CONFIG_PPC_MEMTRACE) += memtrace.o
obj-$(CONFIG_PPC_VAS) += vas.o vas-window.o vas-debug.o
obj-$(CONFIG_OCXL_BASE) += ocxl.o
+obj-$(CONFIG_OPAL_SECVAR) += opal-secvar.o
diff --git a/arch/powerpc/platforms/powernv/opal-call.c b/arch/powerpc/platforms/powernv/opal-call.c
index 36c8fa3647a2..0445980f294f 100644
--- a/arch/powerpc/platforms/powernv/opal-call.c
+++ b/arch/powerpc/platforms/powernv/opal-call.c
@@ -288,3 +288,5 @@ OPAL_CALL(opal_pci_set_pbcq_tunnel_bar, OPAL_PCI_SET_PBCQ_TUNNEL_BAR);
OPAL_CALL(opal_sensor_read_u64, OPAL_SENSOR_READ_U64);
OPAL_CALL(opal_sensor_group_enable, OPAL_SENSOR_GROUP_ENABLE);
OPAL_CALL(opal_nx_coproc_init, OPAL_NX_COPROC_INIT);
+OPAL_CALL(opal_secvar_get, OPAL_SECVAR_GET);
+OPAL_CALL(opal_secvar_backend, OPAL_SECVAR_BACKEND);
diff --git a/arch/powerpc/platforms/powernv/opal-secvar.c b/arch/powerpc/platforms/powernv/opal-secvar.c
new file mode 100644
index 000000000000..dba441dd5af1
--- /dev/null
+++ b/arch/powerpc/platforms/powernv/opal-secvar.c
@@ -0,0 +1,85 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * PowerNV code for secure variables
+ *
+ * Copyright (C) 2019 IBM Corporation
+ * Author: Claudio Carvalho <cclaudio@linux.ibm.com>
+ *
+ */
+
+/*
+ * The opal wrappers in this file treat the @name, @vendor, and @data
+ * parameters as little endian blobs.
+ * @name is a ucs2 string
+ * @vendor is the vendor GUID. It is converted to LE in the kernel
+ * @data variable data, which layout may be different for each variable
+ */
+
+#define pr_fmt(fmt) "secvar: "fmt
+
+#include <linux/types.h>
+#include <asm/opal.h>
+#include <asm/opal-secvar.h>
+
+static bool is_opal_secvar_supported(void)
+{
+ static bool opal_secvar_supported;
+ static bool initialized;
+
+ if (initialized)
+ return opal_secvar_supported;
+
+ if (!opal_check_token(OPAL_SECVAR_GET)
+ || !opal_check_token(OPAL_SECVAR_BACKEND)) {
+ pr_err("OPAL doesn't support secure variables\n");
+ opal_secvar_supported = false;
+ } else {
+ opal_secvar_supported = true;
+ }
+
+ initialized = true;
+
+ return opal_secvar_supported;
+}
+
+int opal_get_variable(u8 *key, unsigned long ksize, u8 *metadata,
+ unsigned long *mdsize, u8 *data, unsigned long *dsize)
+{
+ int rc;
+
+ if (!is_opal_secvar_supported())
+ return OPAL_UNSUPPORTED;
+
+ if (mdsize)
+ *mdsize = cpu_to_be64(*mdsize);
+ if (dsize)
+ *dsize = cpu_to_be64(*dsize);
+
+ rc = opal_secvar_get(__pa(key), ksize, __pa(metadata), __pa(mdsize),
+ __pa(data), __pa(dsize));
+
+ if (mdsize)
+ *mdsize = be64_to_cpu(*mdsize);
+ if (dsize)
+ *dsize = be64_to_cpu(*dsize);
+
+ return rc;
+}
+
+int opal_variable_version(unsigned long *backend)
+{
+ int rc;
+
+ if (!is_opal_secvar_supported())
+ return OPAL_UNSUPPORTED;
+
+ if (backend)
+ *backend = cpu_to_be64(*backend);
+
+ rc = opal_secvar_backend(__pa(backend));
+
+ if (backend)
+ *backend = be64_to_cpu(*backend);
+
+ return rc;
+}
--
2.20.1
^ permalink raw reply related
* [PATCH v4 3/3] powerpc: Add support to initialize ima policy rules
From: Nayna Jain @ 2019-06-11 17:06 UTC (permalink / raw)
To: linuxppc-dev, linux-efi, linux-integrity, linux-kernel
Cc: Ard Biesheuvel, Nayna Jain, Claudio Carvalho, Mimi Zohar,
Matthew Garret, Paul Mackerras, Jeremy Kerr, Satheesh Rajendran
In-Reply-To: <1560272765-5768-1-git-send-email-nayna@linux.ibm.com>
PowerNV secure boot relies on the kernel IMA security subsystem to
perform the OS kernel image signature verification. Since each secure
boot mode has different IMA policy requirements, dynamic definition of
the policy rules based on the runtime secure boot mode of the system is
required. On systems that support secure boot, but have it disabled,
only measurement policy rules of the kernel image and modules are
defined.
This patch defines the arch-specific implementation to retrieve the
secure boot mode of the system and accordingly configures the IMA policy
rules.
This patch provides arch-specific IMA policies if PPC_SECURE_BOOT
config is enabled.
Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
---
arch/powerpc/Kconfig | 14 +++++++++
arch/powerpc/kernel/Makefile | 1 +
arch/powerpc/kernel/ima_arch.c | 54 ++++++++++++++++++++++++++++++++++
include/linux/ima.h | 3 +-
4 files changed, 71 insertions(+), 1 deletion(-)
create mode 100644 arch/powerpc/kernel/ima_arch.c
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 8c1c636308c8..9de77bb14f54 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -902,6 +902,20 @@ config PPC_MEM_KEYS
If unsure, say y.
+config PPC_SECURE_BOOT
+ prompt "Enable PowerPC Secure Boot"
+ bool
+ default n
+ depends on PPC64
+ depends on OPAL_SECVAR
+ depends on IMA
+ depends on IMA_ARCH_POLICY
+ help
+ Linux on POWER with firmware secure boot enabled needs to define
+ security policies to extend secure boot to the OS.This config
+ allows user to enable OS Secure Boot on PowerPC systems that
+ have firmware secure boot support.
+
endmenu
config ISA_DMA_API
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 0ea6c4aa3a20..75c929b41341 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -131,6 +131,7 @@ ifdef CONFIG_IMA
obj-y += ima_kexec.o
endif
endif
+obj-$(CONFIG_PPC_SECURE_BOOT) += ima_arch.o
obj-$(CONFIG_AUDIT) += audit.o
obj64-$(CONFIG_AUDIT) += compat_audit.o
diff --git a/arch/powerpc/kernel/ima_arch.c b/arch/powerpc/kernel/ima_arch.c
new file mode 100644
index 000000000000..1767bf6e6550
--- /dev/null
+++ b/arch/powerpc/kernel/ima_arch.c
@@ -0,0 +1,54 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2019 IBM Corporation
+ * Author: Nayna Jain <nayna@linux.ibm.com>
+ *
+ * ima_arch.c
+ * - initialize ima policies for PowerPC Secure Boot
+ */
+
+#include <linux/ima.h>
+#include <asm/secboot.h>
+
+bool arch_ima_get_secureboot(void)
+{
+ bool sb_mode;
+
+ sb_mode = get_powerpc_sb_mode();
+ if (sb_mode)
+ return true;
+ else
+ return false;
+}
+
+/*
+ * File signature verification is not needed, include only measurements
+ */
+static const char *const default_arch_rules[] = {
+ "measure func=KEXEC_KERNEL_CHECK template=ima-modsig",
+ "measure func=MODULE_CHECK template=ima-modsig",
+ NULL
+};
+
+/* Both file signature verification and measurements are needed */
+static const char *const sb_arch_rules[] = {
+ "measure func=KEXEC_KERNEL_CHECK template=ima-modsig",
+ "measure func=MODULE_CHECK template=ima-modsig",
+ "appraise func=KEXEC_KERNEL_CHECK appraise_type=imasig|modsig template=ima-modsig",
+#if !IS_ENABLED(CONFIG_MODULE_SIG)
+ "appraise func=MODULE_CHECK appraise_type=imasig|modsig template=ima-modsig",
+#endif
+ NULL
+};
+
+/*
+ * On PowerPC, file measurements are to be added to the IMA measurement list
+ * irrespective of the secure boot state of the system. Signature verification
+ * is conditionally enabled based on the secure boot state.
+ */
+const char *const *arch_get_ima_policy(void)
+{
+ if (IS_ENABLED(CONFIG_IMA_ARCH_POLICY) && arch_ima_get_secureboot())
+ return sb_arch_rules;
+ return default_arch_rules;
+}
diff --git a/include/linux/ima.h b/include/linux/ima.h
index fd9f7cf4cdf5..a01df076ecae 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -31,7 +31,8 @@ extern void ima_post_path_mknod(struct dentry *dentry);
extern void ima_add_kexec_buffer(struct kimage *image);
#endif
-#if (defined(CONFIG_X86) && defined(CONFIG_EFI)) || defined(CONFIG_S390)
+#if (defined(CONFIG_X86) && defined(CONFIG_EFI)) || defined(CONFIG_S390) \
+ || defined(CONFIG_PPC_SECURE_BOOT)
extern bool arch_ima_get_secureboot(void);
extern const char * const *arch_get_ima_policy(void);
#else
--
2.20.1
^ permalink raw reply related
* [PATCH v4 2/3] powerpc/powernv: detect the secure boot mode of the system
From: Nayna Jain @ 2019-06-11 17:06 UTC (permalink / raw)
To: linuxppc-dev, linux-efi, linux-integrity, linux-kernel
Cc: Ard Biesheuvel, Nayna Jain, Claudio Carvalho, Mimi Zohar,
Matthew Garret, Paul Mackerras, Jeremy Kerr, Satheesh Rajendran
In-Reply-To: <1560272765-5768-1-git-send-email-nayna@linux.ibm.com>
PowerNV secure boot defines different IMA policies based on the secure
boot state of the system.
This patch defines a function to detect the secure boot state of the
system.
Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
---
arch/powerpc/include/asm/secboot.h | 21 ++++++++
arch/powerpc/platforms/powernv/Makefile | 2 +-
arch/powerpc/platforms/powernv/secboot.c | 61 ++++++++++++++++++++++++
3 files changed, 83 insertions(+), 1 deletion(-)
create mode 100644 arch/powerpc/include/asm/secboot.h
create mode 100644 arch/powerpc/platforms/powernv/secboot.c
diff --git a/arch/powerpc/include/asm/secboot.h b/arch/powerpc/include/asm/secboot.h
new file mode 100644
index 000000000000..1904fb4a3352
--- /dev/null
+++ b/arch/powerpc/include/asm/secboot.h
@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * PowerPC secure boot definitions
+ *
+ * Copyright (C) 2019 IBM Corporation
+ * Author: Nayna Jain <nayna@linux.ibm.com>
+ *
+ */
+#ifndef POWERPC_SECBOOT_H
+#define POWERPC_SECBOOT_H
+
+#if defined(CONFIG_OPAL_SECVAR)
+extern bool get_powerpc_sb_mode(void);
+#else
+static inline bool get_powerpc_sb_mode(void)
+{
+ return false;
+}
+#endif
+
+#endif
diff --git a/arch/powerpc/platforms/powernv/Makefile b/arch/powerpc/platforms/powernv/Makefile
index 6651c742e530..6f4af607a915 100644
--- a/arch/powerpc/platforms/powernv/Makefile
+++ b/arch/powerpc/platforms/powernv/Makefile
@@ -16,4 +16,4 @@ obj-$(CONFIG_PERF_EVENTS) += opal-imc.o
obj-$(CONFIG_PPC_MEMTRACE) += memtrace.o
obj-$(CONFIG_PPC_VAS) += vas.o vas-window.o vas-debug.o
obj-$(CONFIG_OCXL_BASE) += ocxl.o
-obj-$(CONFIG_OPAL_SECVAR) += opal-secvar.o
+obj-$(CONFIG_OPAL_SECVAR) += opal-secvar.o secboot.o
diff --git a/arch/powerpc/platforms/powernv/secboot.c b/arch/powerpc/platforms/powernv/secboot.c
new file mode 100644
index 000000000000..9199e520ebed
--- /dev/null
+++ b/arch/powerpc/platforms/powernv/secboot.c
@@ -0,0 +1,61 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2019 IBM Corporation
+ * Author: Nayna Jain <nayna@linux.ibm.com>
+ *
+ * secboot.c
+ * - util function to get powerpc secboot state
+ */
+#include <linux/uuid.h>
+#include <asm/opal.h>
+#include <asm/secboot.h>
+#include <asm/opal-secvar.h>
+
+bool get_powerpc_sb_mode(void)
+{
+ u8 secure_boot_name[] = "SecureBoot";
+ u8 setup_mode_name[] = "SetupMode";
+ u8 secboot, setupmode;
+ unsigned long size = sizeof(secboot);
+ int status;
+ unsigned long version;
+
+ status = opal_variable_version(&version);
+ if ((status != OPAL_SUCCESS) || (version != BACKEND_TC_COMPAT_V1)) {
+ pr_info("secboot: error retrieving compatible backend\n");
+ return false;
+ }
+
+ status = opal_get_variable(secure_boot_name, sizeof(secure_boot_name),
+ NULL, NULL, &secboot, &size);
+
+ /*
+ * For now assume all failures reading the SecureBoot variable implies
+ * secure boot is not enabled. Later differentiate failure types.
+ */
+ if (status != OPAL_SUCCESS) {
+ secboot = 0;
+ setupmode = 0;
+ goto out;
+ }
+
+ size = sizeof(setupmode);
+ status = opal_get_variable(setup_mode_name, sizeof(setup_mode_name),
+ NULL, NULL, &setupmode, &size);
+
+ /*
+ * Failure to read the SetupMode variable does not prevent
+ * secure boot mode
+ */
+ if (status != OPAL_SUCCESS)
+ setupmode = 0;
+
+out:
+ if ((secboot == 0) || (setupmode == 1)) {
+ pr_info("secboot: secureboot mode disabled\n");
+ return false;
+ }
+
+ pr_info("secboot: secureboot mode enabled\n");
+ return true;
+}
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v3 3/3] powerpc: Add support to initialize ima policy rules
From: Nayna @ 2019-06-11 17:07 UTC (permalink / raw)
To: Satheesh Rajendran, Nayna Jain
Cc: linux-efi, Ard Biesheuvel, linux-kernel, Mimi Zohar,
Claudio Carvalho, Matthew Garret, linuxppc-dev, Paul Mackerras,
Jeremy Kerr, linux-integrity
In-Reply-To: <20190611051943.GA7516@sathnaga86.in.ibm.com>
On 06/11/2019 01:19 AM, Satheesh Rajendran wrote:
> On Mon, Jun 10, 2019 at 04:33:57PM -0400, Nayna Jain wrote:
>> PowerNV secure boot relies on the kernel IMA security subsystem to
>> perform the OS kernel image signature verification. Since each secure
>> boot mode has different IMA policy requirements, dynamic definition of
>> the policy rules based on the runtime secure boot mode of the system is
>> required. On systems that support secure boot, but have it disabled,
>> only measurement policy rules of the kernel image and modules are
>> defined.
>>
>> This patch defines the arch-specific implementation to retrieve the
>> secure boot mode of the system and accordingly configures the IMA policy
>> rules.
>>
>> This patch provides arch-specific IMA policies if PPC_SECURE_BOOT
>> config is enabled.
>>
>> Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
>> ---
>> arch/powerpc/Kconfig | 14 +++++++++
>> arch/powerpc/kernel/Makefile | 1 +
>> arch/powerpc/kernel/ima_arch.c | 54 ++++++++++++++++++++++++++++++++++
>> include/linux/ima.h | 3 +-
>> 4 files changed, 71 insertions(+), 1 deletion(-)
>> create mode 100644 arch/powerpc/kernel/ima_arch.c
> Hi,
>
> This series failed to build against linuxppc/merge tree with `ppc64le_defconfig`,
>
> arch/powerpc/platforms/powernv/secboot.c:14:6: error: redefinition of 'get_powerpc_sb_mode'
> 14 | bool get_powerpc_sb_mode(void)
> | ^~~~~~~~~~~~~~~~~~~
> In file included from arch/powerpc/platforms/powernv/secboot.c:11:
> ./arch/powerpc/include/asm/secboot.h:15:20: note: previous definition of 'get_powerpc_sb_mode' was here
> 15 | static inline bool get_powerpc_sb_mode(void)
> | ^~~~~~~~~~~~~~~~~~~
> make[3]: *** [scripts/Makefile.build:278: arch/powerpc/platforms/powernv/secboot.o] Error 1
> make[3]: *** Waiting for unfinished jobs....
> make[2]: *** [scripts/Makefile.build:489: arch/powerpc/platforms/powernv] Error 2
> make[1]: *** [scripts/Makefile.build:489: arch/powerpc/platforms] Error 2
> make: *** [Makefile:1071: arch/powerpc] Error 2
> make: *** Waiting for unfinished jobs....
Thanks for reporting. I have fixed it and reposted as v4.
Please retry.
Thanks & Regards,
- Nayna
^ permalink raw reply
* Re: [PATCH v2 0/4] Additional fixes on Talitos driver
From: Christophe Leroy @ 2019-06-11 17:16 UTC (permalink / raw)
To: Horia Geanta, Herbert Xu, David S. Miller
Cc: linuxppc-dev@lists.ozlabs.org, linux-crypto@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <VI1PR0402MB3485AD965F36709F27EFB72698ED0@VI1PR0402MB3485.eurprd04.prod.outlook.com>
Le 11/06/2019 à 18:30, Horia Geanta a écrit :
> On 6/11/2019 6:40 PM, Christophe Leroy wrote:
>>
>>
>> Le 11/06/2019 à 17:37, Horia Geanta a écrit :
>>> On 6/11/2019 5:39 PM, Christophe Leroy wrote:
>>>> This series is the last set of fixes for the Talitos driver.
>>>>
>>>> We now get a fully clean boot on both SEC1 (SEC1.2 on mpc885) and
>>>> SEC2 (SEC2.2 on mpc8321E) with CONFIG_CRYPTO_MANAGER_EXTRA_TESTS:
>>>>
>>> I am getting below failures on a sec 3.3.2 (p1020rdb) for hmac(sha384) and
>>> hmac(sha512):
>>
>> Is that new with this series or did you already have it before ?
>>
> Looks like this happens with or without this series.
>
> I haven't checked the state of this driver for quite some time.
> Since I've noticed increased activity, I thought it would be worth
> actually testing the changes.
>
> Are changes in patch 2/4 ("crypto: talitos - fix hash on SEC1.")
> strictly for sec 1.x or they affect all revisions?
They are strictly for sec 1.x
>
>> What do you mean by "fuzz testing" enabled ? Is that
>> CONFIG_CRYPTO_MANAGER_EXTRA_TESTS or something else ?
>>
> Yes, it's this config symbol.
Indeed SEC 2.2 only supports up to SHA-256.
Christophe
>
> Horia
>
^ permalink raw reply
* Re: Question - check in runtime which architecture am I running on
From: Oded Gabbay @ 2019-06-11 17:21 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linuxppc-dev, Linux-Kernel@Vger. Kernel. Org, Greg Kroah-Hartman
In-Reply-To: <20190611140725.GA28902@infradead.org>
On Tue, Jun 11, 2019 at 5:07 PM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Tue, Jun 11, 2019 at 03:30:08PM +0300, Oded Gabbay wrote:
> > Hello POWER developers,
> >
> > I'm trying to find out if there is an internal kernel API so that a
> > PCI driver can call it to check if its PCI device is running inside a
> > POWER9 machine. Alternatively, if that's not available, if it is
> > running on a machine with powerpc architecture.
>
> Your driver has absolutely not business knowing this.
>
> >
> > I need this information as my device (Goya AI accelerator)
> > unfortunately needs a slightly different configuration of its PCIe
> > controller in case of POWER9 (need to set bit 59 to be 1 in all
> > outbound transactions).
>
> No, it doesn't. You can query the output from dma_get_required_mask
> to optimize for the DMA addresses you get, and otherwise you simply
> set the maximum dma mask you support. That is about the control you
> get, and nothing else is a drivers business.
I don't want to conduct two discussions as I saw you answered on my patch.
I'll add the ppc mailing list to my patch.
Oded
^ permalink raw reply
* Re: [PATCH v2 8/8] habanalabs: enable 64-bit DMA mask in POWER9
From: Oded Gabbay @ 2019-06-11 17:22 UTC (permalink / raw)
To: Greg KH, linuxppc-dev, Christoph Hellwig; +Cc: Linux-Kernel@Vger. Kernel. Org
In-Reply-To: <CAFCwf13A73AxKzaa7Dk3tU-1NDgTFs4+xCO2os7SuSyUHZ9Z3Q@mail.gmail.com>
On Tue, Jun 11, 2019 at 8:03 PM Oded Gabbay <oded.gabbay@gmail.com> wrote:
>
> On Tue, Jun 11, 2019 at 6:26 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Tue, Jun 11, 2019 at 08:17:53AM -0700, Christoph Hellwig wrote:
> > > On Tue, Jun 11, 2019 at 11:58:57AM +0200, Greg KH wrote:
> > > > That feels like a big hack. ppc doesn't have any "what arch am I
> > > > running on?" runtime call? Did you ask on the ppc64 mailing list? I'm
> > > > ok to take this for now, but odds are you need a better fix for this
> > > > sometime...
> > >
> > > That isn't the worst part of it. The whole idea of checking what I'm
> > > running to set a dma mask just doesn't make any sense at all.
> >
> > Oded, I thought I asked if there was a dma call you should be making to
> > keep this type of check from being needed. What happened to that? As
> > Christoph points out, none of this should be needed, which is what I
> > thought I originally said :)
> >
> > thanks,
> >
> > greg k-h
>
> I'm sorry, but it seems I can't explain what's my problem because you
> and Christoph keep mentioning the pci_set_dma_mask() but it doesn't
> help me.
> I'll try again to explain.
>
> The main problem specifically for Goya device, is that I can't call
> this function with *the same parameter* for POWER9 and x86-64, because
> x86-64 supports dma mask of 48-bits while POWER9 supports only 32-bits
> or 64-bits.
>
> The main limitation in my Goya device is that it can generate PCI
> outbound transactions with addresses from 0 to (2^50 - 1).
> That's why when we first integrated it in x86-64, we used a DMA mask
> of 48-bits, by calling pci_set_dma_mask(pdev, 48). That way, the
> kernel ensures me that all the DMA addresses are from 0 to (2^48 - 1),
> and that address range is accessible by my device.
>
> If for some reason, the x86-64 machine doesn't support 48-bits, the
> standard fallback code in ALL the drivers I have seen is to set the
> DMA mask to 32-bits. And that's how my current driver's code is
> written.
>
> Now, when I tried to integrate Goya into a POWER9 machine, I got a
> reject from the call to pci_set_dma_mask(pdev, 48). The standard code,
> as I wrote above, is to call the same function with 32-bits. That
> works BUT it is not practical, as our applications require much more
> memory mapped then 32-bits. In addition, once you add more cards which
> are all mapped to the same range, it is simply not usable at all.
>
> Therefore, I consulted with POWER people and they told me I can call
> to pci_set_dma_mask with the mask as 64, but I must make sure that ALL
> outbound transactions from Goya will be with bit 59 set in the
> address.
> I can achieve that with a dedicated configuration I make in Goya's
> PCIe controller. That's what I did and that works.
>
> So, to summarize:
> If I call pci_set_dma_mask with 48, then it fails on POWER9. However,
> in runtime, I don't know if its POWER9 or not, so upon failure I will
> call it again with 32, which makes our device pretty much unusable.
> If I call pci_set_dma_mask with 64, and do the dedicated configuration
> in Goya's PCIe controller, then it won't work on x86-64, because bit
> 59 will be set and the host won't like it (I checked it). In addition,
> I might get addresses above 50 bits, which my device can't generate.
>
> I hope this makes things more clear. Now, please explain to me how I
> can call pci_set_dma_mask without any regard to whether I run on
> x86-64 or POWER9, considering what I wrote above ?
>
> Thanks,
> Oded
Adding ppc mailing list.
Oded
^ permalink raw reply
* Re: [RFC V3] mm: Generalize and rename notify_page_fault() as kprobe_page_fault()
From: Leonardo Bras @ 2019-06-11 17:31 UTC (permalink / raw)
To: Anshuman Khandual, Christophe Leroy, linux-kernel, linux-mm
Cc: Mark Rutland, Michal Hocko, linux-ia64, linux-sh, Peter Zijlstra,
Catalin Marinas, Dave Hansen, Heiko Carstens, Paul Mackerras,
sparclinux, linux-s390, Yoshinori Sato, x86, Russell King,
Matthew Wilcox, Ingo Molnar, Andrey Konovalov, Fenghua Yu,
Stephen Rothwell, Will Deacon, Andy Lutomirski, Thomas Gleixner,
linux-arm-kernel, Tony Luck, Martin Schwidefsky, Andrew Morton,
linuxppc-dev, David S. Miller
In-Reply-To: <7b0a7afd-2776-0d95-19c5-3e15959744eb@arm.com>
[-- Attachment #1: Type: text/plain, Size: 1165 bytes --]
On Tue, 2019-06-11 at 10:44 +0530, Anshuman Khandual wrote:
>
> On 06/10/2019 08:57 PM, Leonardo Bras wrote:
> > On Mon, 2019-06-10 at 08:09 +0530, Anshuman Khandual wrote:
> > > > > + /*
> > > > > + * To be potentially processing a kprobe fault and to be allowed
> > > > > + * to call kprobe_running(), we have to be non-preemptible.
> > > > > + */
> > > > > + if (kprobes_built_in() && !preemptible() && !user_mode(regs)) {
> > > > > + if (kprobe_running() && kprobe_fault_handler(regs, trap))
> > > >
> > > > don't need an 'if A if B', can do 'if A && B'
> > >
> > > Which will make it a very lengthy condition check.
> >
> > Well, is there any problem line-breaking the if condition?
> >
> > if (A && B && C &&
> > D && E )
> >
> > Also, if it's used only to decide the return value, maybe would be fine
> > to do somethink like that:
> >
> > return (A && B && C &&
> > D && E );
>
> Got it. But as Dave and Matthew had pointed out earlier, the current x86
> implementation has better readability. Hence will probably stick with it.
>
Sure, I agree with them. It's way more readable.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook
From: Andreas Schwab @ 2019-06-11 17:48 UTC (permalink / raw)
To: Larry Finger
Cc: Aaro Koskinen, linux-wireless, linux-kernel, Christian Zigotzky,
linuxppc-dev, Christoph Hellwig
In-Reply-To: <153c13f5-a829-1eab-a3c5-fecfb84127ff@lwfinger.net>
On Jun 10 2019, Larry Finger <Larry.Finger@lwfinger.net> wrote:
> I do not understand why the if statement returns true as neither of the
> values is zero.
That's because the format string does not make any sense. You are
printing garbage.
> diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
> index f7afdad..ba2489d 100644
> --- a/kernel/dma/mapping.c
> +++ b/kernel/dma/mapping.c
> @@ -317,9 +317,12 @@ int dma_supported(struct device *dev, u64 mask)
>
> int dma_set_mask(struct device *dev, u64 mask)
> {
> + pr_info("mask 0x%llx, dma_mask 0x%llx, dma_supported 0x%llx\n",
> mask, dev->dma_mask,
> + dma_supported(dev, mask));
None of the format directives match the type of the arguments.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
"And now for something completely different."
^ permalink raw reply
* [PATCH] cxl: no need to check return value of debugfs_create functions
From: Greg Kroah-Hartman @ 2019-06-11 18:13 UTC (permalink / raw)
To: Frederic Barrat, Andrew Donnellan, Arnd Bergmann; +Cc: linuxppc-dev
When calling debugfs functions, there is no need to ever check the
return value. The function can work or not, but the code logic should
never do something different based on this.
Because there's no need to check, also make the return value of the
local debugfs_create_io_x64() call void, as no one ever did anything
with the return value (as they did not need to.)
Cc: Frederic Barrat <fbarrat@linux.ibm.com>
Cc: Andrew Donnellan <ajd@linux.ibm.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/misc/cxl/debugfs.c | 19 +++++--------------
1 file changed, 5 insertions(+), 14 deletions(-)
diff --git a/drivers/misc/cxl/debugfs.c b/drivers/misc/cxl/debugfs.c
index 1fda22c24c93..27f3bcb7d939 100644
--- a/drivers/misc/cxl/debugfs.c
+++ b/drivers/misc/cxl/debugfs.c
@@ -26,11 +26,11 @@ static int debugfs_io_u64_set(void *data, u64 val)
DEFINE_DEBUGFS_ATTRIBUTE(fops_io_x64, debugfs_io_u64_get, debugfs_io_u64_set,
"0x%016llx\n");
-static struct dentry *debugfs_create_io_x64(const char *name, umode_t mode,
- struct dentry *parent, u64 __iomem *value)
+static void debugfs_create_io_x64(const char *name, umode_t mode,
+ struct dentry *parent, u64 __iomem *value)
{
- return debugfs_create_file_unsafe(name, mode, parent,
- (void __force *)value, &fops_io_x64);
+ debugfs_create_file_unsafe(name, mode, parent, (void __force *)value,
+ &fops_io_x64);
}
void cxl_debugfs_add_adapter_regs_psl9(struct cxl *adapter, struct dentry *dir)
@@ -64,8 +64,6 @@ int cxl_debugfs_adapter_add(struct cxl *adapter)
snprintf(buf, 32, "card%i", adapter->adapter_num);
dir = debugfs_create_dir(buf, cxl_debugfs);
- if (IS_ERR(dir))
- return PTR_ERR(dir);
adapter->debugfs = dir;
debugfs_create_io_x64("err_ivte", S_IRUSR, dir, _cxl_p1_addr(adapter, CXL_PSL_ErrIVTE));
@@ -106,8 +104,6 @@ int cxl_debugfs_afu_add(struct cxl_afu *afu)
snprintf(buf, 32, "psl%i.%i", afu->adapter->adapter_num, afu->slice);
dir = debugfs_create_dir(buf, afu->adapter->debugfs);
- if (IS_ERR(dir))
- return PTR_ERR(dir);
afu->debugfs = dir;
debugfs_create_io_x64("sr", S_IRUSR, dir, _cxl_p1n_addr(afu, CXL_PSL_SR_An));
@@ -129,15 +125,10 @@ void cxl_debugfs_afu_remove(struct cxl_afu *afu)
int __init cxl_debugfs_init(void)
{
- struct dentry *ent;
-
if (!cpu_has_feature(CPU_FTR_HVMODE))
return 0;
- ent = debugfs_create_dir("cxl", NULL);
- if (IS_ERR(ent))
- return PTR_ERR(ent);
- cxl_debugfs = ent;
+ cxl_debugfs = debugfs_create_dir("cxl", NULL);
return 0;
}
--
2.22.0
^ permalink raw reply related
* Re: [PATCH v3 06/20] docs: mark orphan documents as such
From: Andy Shevchenko @ 2019-06-11 18:23 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Linux Doc Mailing List, David Airlie, dri-devel, Platform Driver,
Paul Mackerras, linux-stm32, Alexandre Torgue, Jonathan Corbet,
Maxime Ripard, Andrew Donnellan, Linux PM, Maarten Lankhorst,
Matan Ziv-Av, Mauro Carvalho Chehab, Daniel Vetter, Sean Paul,
linux-arm Mailing List, Linux Kernel Mailing List,
Maxime Coquelin, Frederic Barrat,
open list:LINUX FOR POWERPC PA SEMI PWRFICIENT, Georgi Djakov
In-Reply-To: <20190611140501.11ba091b@coco.lan>
On Tue, Jun 11, 2019 at 8:05 PM Mauro Carvalho Chehab
<mchehab+samsung@kernel.org> wrote:
>
> Em Tue, 11 Jun 2019 19:52:04 +0300
> Andy Shevchenko <andy.shevchenko@gmail.com> escreveu:
>
> > On Fri, Jun 7, 2019 at 10:04 PM Mauro Carvalho Chehab
> > <mchehab+samsung@kernel.org> wrote:
> > > Sphinx doesn't like orphan documents:
> >
> > > Documentation/laptops/lg-laptop.rst: WARNING: document isn't included in any toctree
> >
> > > Documentation/laptops/lg-laptop.rst | 2 ++
> >
> > > diff --git a/Documentation/laptops/lg-laptop.rst b/Documentation/laptops/lg-laptop.rst
> > > index aa503ee9b3bc..f2c2ffe31101 100644
> > > --- a/Documentation/laptops/lg-laptop.rst
> > > +++ b/Documentation/laptops/lg-laptop.rst
> > > @@ -1,5 +1,7 @@
> > > .. SPDX-License-Identifier: GPL-2.0+
> > >
> > > +:orphan:
> > > +
> > > LG Gram laptop extra features
> > > =============================
> > >
> >
> > Can we rather create a toc tree there?
> > It was a first document in reST format in that folder.
>
> Sure, but:
>
> 1) I have a patch converting the other files on this dir to rst:
>
> https://git.linuxtv.org/mchehab/experimental.git/commit/?h=convert_rst_renames_v4.1&id=abc13233035fdfdbc5ef2f2fbd3d127a1ab15530
>
> 2) It probably makes sense to move the entire dir to
> Documentation/admin-guide.
>
> So, I would prefer to have the :orphan: here while (1) is not merged.
Fine to me as long as you will drop it by the mentioned effort.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH 01/16] mm: use untagged_addr() for get_user_pages_fast addresses
From: Khalid Aziz @ 2019-06-11 19:22 UTC (permalink / raw)
To: Christoph Hellwig, Linus Torvalds, Paul Burton, James Hogan,
Yoshinori Sato, Rich Felker, David S. Miller
Cc: linux-sh, Andrey Konovalov, x86, linux-mips, Nicholas Piggin,
linux-kernel, linux-mm, Paul Mackerras, sparclinux, linuxppc-dev
In-Reply-To: <20190611144102.8848-2-hch@lst.de>
On 6/11/19 8:40 AM, Christoph Hellwig wrote:
> This will allow sparc64 to override its ADI tags for
> get_user_pages and get_user_pages_fast.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
Commit message is sparc64 specific but the goal here is to allow any
architecture with memory tagging to use this. So I would suggest
rewording the commit log. Other than that:
Reviewed-by: Khalid Aziz <khalid.aziz@oracle.com>
> mm/gup.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/mm/gup.c b/mm/gup.c
> index ddde097cf9e4..6bb521db67ec 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -2146,7 +2146,7 @@ int __get_user_pages_fast(unsigned long start, int nr_pages, int write,
> unsigned long flags;
> int nr = 0;
>
> - start &= PAGE_MASK;
> + start = untagged_addr(start) & PAGE_MASK;
> len = (unsigned long) nr_pages << PAGE_SHIFT;
> end = start + len;
>
> @@ -2219,7 +2219,7 @@ int get_user_pages_fast(unsigned long start, int nr_pages,
> unsigned long addr, len, end;
> int nr = 0, ret = 0;
>
> - start &= PAGE_MASK;
> + start = untagged_addr(start) & PAGE_MASK;
> addr = start;
> len = (unsigned long) nr_pages << PAGE_SHIFT;
> end = start + len;
>
^ permalink raw reply
* Re: [PATCH 08/16] sparc64: define untagged_addr()
From: Khalid Aziz @ 2019-06-11 19:23 UTC (permalink / raw)
To: Christoph Hellwig, Linus Torvalds, Paul Burton, James Hogan,
Yoshinori Sato, Rich Felker, David S. Miller
Cc: linux-sh, Andrey Konovalov, x86, linux-mips, Nicholas Piggin,
linux-kernel, linux-mm, Paul Mackerras, sparclinux, linuxppc-dev
In-Reply-To: <20190611144102.8848-9-hch@lst.de>
On 6/11/19 8:40 AM, Christoph Hellwig wrote:
> Add a helper to untag a user pointer. This is needed for ADI support
> in get_user_pages_fast.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> arch/sparc/include/asm/pgtable_64.h | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
Looks good to me.
Reviewed-by: Khalid Aziz <khalid.aziz@oracle.com>
>
> diff --git a/arch/sparc/include/asm/pgtable_64.h b/arch/sparc/include/asm/pgtable_64.h
> index f0dcf991d27f..1904782dcd39 100644
> --- a/arch/sparc/include/asm/pgtable_64.h
> +++ b/arch/sparc/include/asm/pgtable_64.h
> @@ -1076,6 +1076,28 @@ static inline int io_remap_pfn_range(struct vm_area_struct *vma,
> }
> #define io_remap_pfn_range io_remap_pfn_range
>
> +static inline unsigned long untagged_addr(unsigned long start)
> +{
> + if (adi_capable()) {
> + long addr = start;
> +
> + /* If userspace has passed a versioned address, kernel
> + * will not find it in the VMAs since it does not store
> + * the version tags in the list of VMAs. Storing version
> + * tags in list of VMAs is impractical since they can be
> + * changed any time from userspace without dropping into
> + * kernel. Any address search in VMAs will be done with
> + * non-versioned addresses. Ensure the ADI version bits
> + * are dropped here by sign extending the last bit before
> + * ADI bits. IOMMU does not implement version tags.
> + */
> + return (addr << (long)adi_nbits()) >> (long)adi_nbits();
> + }
> +
> + return start;
> +}
> +#define untagged_addr untagged_addr
> +
> #include <asm/tlbflush.h>
> #include <asm-generic/pgtable.h>
>
>
^ permalink raw reply
* Re: [PATCH 09/16] sparc64: use the generic get_user_pages_fast code
From: Khalid Aziz @ 2019-06-11 19:35 UTC (permalink / raw)
To: Christoph Hellwig, Linus Torvalds, Paul Burton, James Hogan,
Yoshinori Sato, Rich Felker, David S. Miller
Cc: linux-sh, Andrey Konovalov, x86, linux-mips, Nicholas Piggin,
linux-kernel, linux-mm, Paul Mackerras, sparclinux, linuxppc-dev
In-Reply-To: <20190611144102.8848-10-hch@lst.de>
On 6/11/19 8:40 AM, Christoph Hellwig wrote:
> The sparc64 code is mostly equivalent to the generic one, minus various
> bugfixes and two arch overrides that this patch adds to pgtable.h.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> arch/sparc/Kconfig | 1 +
> arch/sparc/include/asm/pgtable_64.h | 18 ++
> arch/sparc/mm/Makefile | 2 +-
> arch/sparc/mm/gup.c | 340 ----------------------------
> 4 files changed, 20 insertions(+), 341 deletions(-)
> delete mode 100644 arch/sparc/mm/gup.c
>
Reviewed-by: Khalid Aziz <khalid.aziz@oracle.com>
^ permalink raw reply
* Re: [PATCH 10/16] mm: rename CONFIG_HAVE_GENERIC_GUP to CONFIG_HAVE_FAST_GUP
From: Khalid Aziz @ 2019-06-11 19:35 UTC (permalink / raw)
To: Christoph Hellwig, Linus Torvalds, Paul Burton, James Hogan,
Yoshinori Sato, Rich Felker, David S. Miller
Cc: linux-sh, Andrey Konovalov, x86, linux-mips, Nicholas Piggin,
linux-kernel, linux-mm, Paul Mackerras, sparclinux, linuxppc-dev
In-Reply-To: <20190611144102.8848-11-hch@lst.de>
On 6/11/19 8:40 AM, Christoph Hellwig wrote:
> We only support the generic GUP now, so rename the config option to
> be more clear, and always use the mm/Kconfig definition of the
> symbol and select it from the arch Kconfigs.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> arch/arm/Kconfig | 5 +----
> arch/arm64/Kconfig | 4 +---
> arch/mips/Kconfig | 2 +-
> arch/powerpc/Kconfig | 2 +-
> arch/s390/Kconfig | 2 +-
> arch/sh/Kconfig | 2 +-
> arch/sparc/Kconfig | 2 +-
> arch/x86/Kconfig | 4 +---
> mm/Kconfig | 2 +-
> mm/gup.c | 4 ++--
> 10 files changed, 11 insertions(+), 18 deletions(-)
>
Looks good.
Reviewed-by: Khalid Aziz <khalid.aziz@oracle.com>
^ permalink raw reply
* [Bug 203837] Booting kernel under KVM immediately freezes host
From: bugzilla-daemon @ 2019-06-11 19:42 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-203837-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=203837
--- Comment #4 from Shawn Anastasio (shawn@anastas.io) ---
I have applied Nick's patchset to 5.1.7 but the issue still occurs.
As for using pdbg, I'm aware of the tool's existence but I'm not sure how
I would effectively use it to diagnose this issue. If anybody has some
pointers, it'd be appreciated.
--
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook
From: Larry Finger @ 2019-06-11 22:20 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Aaro Koskinen, linux-wireless, linux-kernel, Christian Zigotzky,
linuxppc-dev
In-Reply-To: <20190611060521.GA19512@lst.de>
On 6/11/19 1:05 AM, Christoph Hellwig wrote:
> On Mon, Jun 10, 2019 at 11:09:47AM -0500, Larry Finger wrote:
>
> What might be confusing in your output is that dev->dma_mask is a pointer,
> and we are setting it in dma_set_mask. That is before we only check
> if the pointer is set, and later we override it. Of course this doesn't
> actually explain the failure. But what is even more strange to me
> is that you get a return value from dma_supported() that isn't 0 or 1,
> as that function is supposed to return a boolean, and I really can't see
> how mask >= __phys_to_dma(dev, min_mask), would return anything but 0
> or 1. Does the output change if you use the correct printk specifiers?
>
> i.e. with a debug patch like this:
>
>
> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> index 2c2772e9702a..9e5b30b12b10 100644
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -378,6 +378,7 @@ EXPORT_SYMBOL(dma_direct_map_resource);
> int dma_direct_supported(struct device *dev, u64 mask)
> {
> u64 min_mask;
> + bool ret;
>
> if (IS_ENABLED(CONFIG_ZONE_DMA))
> min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS);
> @@ -391,7 +392,12 @@ int dma_direct_supported(struct device *dev, u64 mask)
> * use __phys_to_dma() here so that the SME encryption mask isn't
> * part of the check.
> */
> - return mask >= __phys_to_dma(dev, min_mask);
> + ret = (mask >= __phys_to_dma(dev, min_mask));
> + if (!ret)
> + dev_info(dev,
> + "%s: failed (mask = 0x%llx, min_mask = 0x%llx/0x%llx, dma bits = %d\n",
> + __func__, mask, min_mask, __phys_to_dma(dev, min_mask), ARCH_ZONE_DMA_BITS);
> + return ret;
> }
>
> size_t dma_direct_max_mapping_size(struct device *dev)
> diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
> index f7afdadb6770..6c57ccdee2ae 100644
> --- a/kernel/dma/mapping.c
> +++ b/kernel/dma/mapping.c
> @@ -317,8 +317,14 @@ void arch_dma_set_mask(struct device *dev, u64 mask);
>
> int dma_set_mask(struct device *dev, u64 mask)
> {
> - if (!dev->dma_mask || !dma_supported(dev, mask))
> + if (!dev->dma_mask) {
> + dev_info(dev, "no DMA mask set!\n");
> return -EIO;
> + }
> + if (!dma_supported(dev, mask)) {
> + printk("DMA not supported\n");
> + return -EIO;
> + }
>
> arch_dma_set_mask(dev, mask);
> dma_check_mask(dev, mask);
>
After I got the correct formatting, the output with this patch only gives the
following in dmesg:
b43-pci-bridge 0001:11:00.0: dma_direct_supported: failed (mask = 0x3fffffff,
min_mask = 0x5ffff000/0x5ffff000, dma bits = 0x1f
DMA not supported
b43legacy-phy0 ERROR: The machine/kernel does not support the required 30-bit
DMA mask
Your first patch did not work as the configuration does not have
CONFIG_ZONE_DMA. As a result, the initial value of min_mask always starts at 32
bits and is taken down to 31 with the maximum pfn minimization. When I forced
the initial value of min_mask to 30 bits, the device worked.
It is obvious that the case of a mask smaller than min_mask should be handled by
the IOMMU. In my system, CONFIG_IOMMU_SUPPORT is selected. All other CONFIG
variables containing IOMMU are not selected. When dma_direct_supported() fails,
should the system not try for an IOMMU solution? Is the driver asking for the
wrong type of memory? It is doing a dma_and_set_mask_coherent() call.
Larry
^ permalink raw reply
* Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook
From: Aaro Koskinen @ 2019-06-11 22:46 UTC (permalink / raw)
To: Larry Finger
Cc: linux-wireless, linux-kernel, Christian Zigotzky, linuxppc-dev,
Christoph Hellwig
In-Reply-To: <5aaa600b-5b59-1f68-454f-20403c318f1a@lwfinger.net>
Hi,
On Tue, Jun 11, 2019 at 05:20:12PM -0500, Larry Finger wrote:
> It is obvious that the case of a mask smaller than min_mask should be
> handled by the IOMMU. In my system, CONFIG_IOMMU_SUPPORT is selected. All
> other CONFIG variables containing IOMMU are not selected. When
> dma_direct_supported() fails, should the system not try for an IOMMU
> solution? Is the driver asking for the wrong type of memory? It is doing a
> dma_and_set_mask_coherent() call.
I don't think we have IOMMU on G4. On G5 it should work (I remember fixing
b43 issue on G5, see 4c374af5fdee, unfortunately all my G5 Macs with b43
are dead and waiting for re-capping).
A.
^ permalink raw reply
* Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook
From: Benjamin Herrenschmidt @ 2019-06-11 22:46 UTC (permalink / raw)
To: Larry Finger, Christoph Hellwig
Cc: linuxppc-dev, Christian Zigotzky, linux-wireless, linux-kernel,
Aaro Koskinen
In-Reply-To: <5aaa600b-5b59-1f68-454f-20403c318f1a@lwfinger.net>
On Tue, 2019-06-11 at 17:20 -0500, Larry Finger wrote:
> b43-pci-bridge 0001:11:00.0: dma_direct_supported: failed (mask =
> 0x3fffffff,
> min_mask = 0x5ffff000/0x5ffff000, dma bits = 0x1f
Ugh ? A mask with holes in it ? That's very wrong... That min_mask is
bogus.
Ben.
^ permalink raw reply
* Re: [PATCH v2 8/8] habanalabs: enable 64-bit DMA mask in POWER9
From: Benjamin Herrenschmidt @ 2019-06-11 22:53 UTC (permalink / raw)
To: Oded Gabbay, Greg KH, linuxppc-dev, Christoph Hellwig
Cc: Russell Currey, Oliver OHalloran, Linux-Kernel@Vger. Kernel. Org
In-Reply-To: <CAFCwf134nTD4FM_9Q+THQ7ZAZzGxhs15O6EheaRJMqM5wxi+aA@mail.gmail.com>
On Tue, 2019-06-11 at 20:22 +0300, Oded Gabbay wrote:
>
> > So, to summarize:
> > If I call pci_set_dma_mask with 48, then it fails on POWER9. However,
> > in runtime, I don't know if its POWER9 or not, so upon failure I will
> > call it again with 32, which makes our device pretty much unusable.
> > If I call pci_set_dma_mask with 64, and do the dedicated configuration
> > in Goya's PCIe controller, then it won't work on x86-64, because bit
> > 59 will be set and the host won't like it (I checked it). In addition,
> > I might get addresses above 50 bits, which my device can't generate.
> >
> > I hope this makes things more clear. Now, please explain to me how I
> > can call pci_set_dma_mask without any regard to whether I run on
> > x86-64 or POWER9, considering what I wrote above ?
> >
> > Thanks,
> > Oded
>
> Adding ppc mailing list.
You can't. Your device is broken. Devices that don't support DMAing to
the full 64-bit deserve to be added to the trash pile.
As a result, getting it to work will require hacks. Some GPUs have
similar issues and require similar hacks, it's unfortunate.
Added a couple of guys on CC who might be able to help get those hacks
right.
It's still very fishy .. the idea is to detect the case where setting a
64-bit mask will give your system memory mapped at a fixed high address
(1 << 59 in our case) and program that in your chip in the "Fixed high
bits" register that you seem to have (also make sure it doesn't affect
MSIs or it will break them).
This will only work as long as all of the system memory can be
addressed at an offset from that fixed address that itself fits your
device addressing capabilities (50 bits in this case). It may or may
not be the case but there's no way to check since the DMA mask logic
won't really apply.
You might want to consider fixing your HW in the next iteration... This
is going to bite you when x86 increases the max physical memory for
example, or on other architectures.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH 16/16] mm: pass get_user_pages_fast iterator arguments in a structure
From: Nicholas Piggin @ 2019-06-12 0:52 UTC (permalink / raw)
To: Rich Felker, David S. Miller, Christoph Hellwig, James Hogan,
Paul Burton, Linus Torvalds, Yoshinori Sato
Cc: linux-sh, Andrey Konovalov, x86, linux-mips, linux-kernel,
linux-mm, Khalid Aziz, Paul Mackerras, sparclinux, linuxppc-dev
In-Reply-To: <20190611144102.8848-17-hch@lst.de>
Christoph Hellwig's on June 12, 2019 12:41 am:
> Instead of passing a set of always repeated arguments down the
> get_user_pages_fast iterators, create a struct gup_args to hold them and
> pass that by reference. This leads to an over 100 byte .text size
> reduction for x86-64.
What does this do for performance? I've found this pattern can be
bad for store aliasing detection.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH 16/16] mm: pass get_user_pages_fast iterator arguments in a structure
From: Linus Torvalds @ 2019-06-12 1:09 UTC (permalink / raw)
To: Nicholas Piggin
Cc: Rich Felker, Yoshinori Sato, Linux-sh list, James Hogan,
the arch/x86 maintainers, Khalid Aziz, Christoph Hellwig,
Linux-MM, Paul Burton, Paul Mackerras, Andrey Konovalov,
sparclinux, linux-mips, linuxppc-dev, David S. Miller,
Linux List Kernel Mailing
In-Reply-To: <1560300464.nijubslu3h.astroid@bobo.none>
On Tue, Jun 11, 2019 at 2:55 PM Nicholas Piggin <npiggin@gmail.com> wrote:
>
> What does this do for performance? I've found this pattern can be
> bad for store aliasing detection.
I wouldn't expect it to be noticeable, and the lack of argument
reloading etc should make up for it. Plus inlining makes it a
non-issue when that happens.
But I guess we could also at least look at using "restrict", if that
ends up helping. Unlike the completely bogus type-based aliasing rules
(that we disable because I think the C people were on some bad bad
drugs when they came up with them), restricted pointers are a real
thing that makes sense.
That said, we haven't traditionally used it, and I don't know how much
it helps gcc. Maybe gcc ignores it entirely? S
Linus
^ permalink raw reply
* Re: [PATCH 16/16] mm: pass get_user_pages_fast iterator arguments in a structure
From: Nadav Amit @ 2019-06-12 1:27 UTC (permalink / raw)
To: Nicholas Piggin, Christoph Hellwig
Cc: the arch/x86 maintainers, Rich Felker, Yoshinori Sato, linux-sh,
James Hogan, linuxppc-dev, Khalid Aziz, LKML, Linux-MM,
Paul Burton, Paul Mackerras, Andrey Konovalov, sparclinux,
linux-mips, Linus Torvalds, David S. Miller
In-Reply-To: <1560300464.nijubslu3h.astroid@bobo.none>
> On Jun 11, 2019, at 5:52 PM, Nicholas Piggin <npiggin@gmail.com> wrote:
>
> Christoph Hellwig's on June 12, 2019 12:41 am:
>> Instead of passing a set of always repeated arguments down the
>> get_user_pages_fast iterators, create a struct gup_args to hold them and
>> pass that by reference. This leads to an over 100 byte .text size
>> reduction for x86-64.
>
> What does this do for performance? I've found this pattern can be
> bad for store aliasing detection.
Note that sometimes such an optimization can also have adverse effect due to
stack protector code that gcc emits when you use such structs.
Matthew Wilcox encountered such a case:
https://patchwork.kernel.org/patch/10702741/
^ permalink raw reply
* Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook
From: Larry Finger @ 2019-06-12 1:52 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Christoph Hellwig
Cc: linuxppc-dev, Christian Zigotzky, linux-wireless, linux-kernel,
Aaro Koskinen
In-Reply-To: <0b257651bb7ac4a6f0a8dce5470120b7701720b9.camel@kernel.crashing.org>
On 6/11/19 5:46 PM, Benjamin Herrenschmidt wrote:
> On Tue, 2019-06-11 at 17:20 -0500, Larry Finger wrote:
>> b43-pci-bridge 0001:11:00.0: dma_direct_supported: failed (mask =
>> 0x3fffffff,
>> min_mask = 0x5ffff000/0x5ffff000, dma bits = 0x1f
>
> Ugh ? A mask with holes in it ? That's very wrong... That min_mask is
> bogus.
I agree, but that is not likely serious as most systems will have enough memory
that the max_pfn term will be much larger than the initial min_mask, and
min_mask will be unchanged by the min function. In addition, min_mask is not
used beyond this routine, and then only to decide if direct dma is supported.
The following patch generates masks with no holes, but I cannot see that it is
needed.
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 2c2772e9702a..e3edd4f29e80 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -384,7 +384,8 @@ int dma_direct_supported(struct device *dev, u64 mask)
else
min_mask = DMA_BIT_MASK(32);
- min_mask = min_t(u64, min_mask, (max_pfn - 1) << PAGE_SHIFT);
+ min_mask = min_t(u64, min_mask, ((max_pfn - 1) << PAGE_SHIFT) |
+ DMA_BIT_MASK(PAGE_SHIFT));
/*
* This check needs to be against the actual bit mask value, so
Larry
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox