* [PATCH v2 0/4] powerpc: expose secure variables to the kernel and userspace
From: Nayna Jain @ 2019-08-21 15:08 UTC (permalink / raw)
To: linuxppc-dev, linux-efi, linux-integrity
Cc: Ard Biesheuvel, Eric Ricther, Nayna Jain, linux-kernel,
Mimi Zohar, Claudio Carvalho, Matthew Garret, Greg Kroah-Hartman,
Paul Mackerras, Jeremy Kerr, Elaine Palmer, Oliver O'Halloran,
George Wilson
In order to verify the OS kernel on PowerNV systems, secure boot requires
X.509 certificates trusted by the platform. These are stored in secure
variables controlled by OPAL, called OPAL secure variables. In order to
enable users to manage the keys, the secure variables need to be exposed
to userspace.
OPAL provides the runtime services for the kernel to be able to access the
secure variables[1]. This patchset defines the kernel interface for the
OPAL APIs. These APIs are used by the hooks, which load these variables
to the keyring and expose them to the userspace for reading/writing.
The previous version[2] of the patchset added support only for the sysfs
interface. This patch adds two more patches that involves loading of
the firmware trusted keys to the kernel keyring. This patchset is
dependent on the base CONFIG PPC_SECURE_BOOT added by ima arch specific
patches for POWER[3]
Overall, this patchset adds the following support:
* expose secure variables to the kernel via OPAL Runtime API interface
* expose secure variables to the userspace via kernel sysfs interface
* load kernel verification and revocation keys to .platform and
.blacklist keyring respectively.
The secure variables can be read/written using simple linux utilities
cat/hexdump.
For example:
Path to the secure variables is:
/sys/firmware/secvar/vars
Each secure variable is listed as directory.
$ ls -l
total 0
drwxr-xr-x. 2 root root 0 Aug 20 21:20 db
drwxr-xr-x. 2 root root 0 Aug 20 21:20 KEK
drwxr-xr-x. 2 root root 0 Aug 20 21:20 PK
The attributes of each of the secure variables are(for example: PK):
[PK]$ ls -l
total 0
-r--r--r--. 1 root root 32000 Aug 21 08:28 data
-r--r--r--. 1 root root 65536 Aug 21 08:28 name
-r--r--r--. 1 root root 65536 Aug 21 08:28 size
--w-------. 1 root root 32000 Aug 21 08:28 update
The "data" is used to read the existing variable value using hexdump. The
data is stored in ESL format.
The "update" is used to write a new value using cat. The update is
to be submitted as AUTH file.
[1] Depends on skiboot OPAL API changes which removes metadata from
the API. The new version with the changes are going to be posted soon.
[2] https://lkml.org/lkml/2019/6/13/1644
[3] https://lkml.org/lkml/2019/8/19/402
Changelog:
v2:
* removes complete efi-sms from the sysfs implementation and is simplified
* includes Greg's and Oliver's feedbacks:
* adds sysfs documentation
* moves sysfs code to arch/powerpc
* other code related feedbacks.
* adds two new patches to load keys to .platform and .blacklist keyring.
These patches are added to this series as they are also dependent on
OPAL APIs.
Nayna Jain (4):
powerpc/powernv: Add OPAL API interface to access secure variable
powerpc: expose secure variables to userspace via sysfs
x86/efi: move common keyring handler functions to new file
powerpc: load firmware trusted keys into kernel keyring
Documentation/ABI/testing/sysfs-secvar | 27 +++
arch/powerpc/Kconfig | 9 +
arch/powerpc/include/asm/opal-api.h | 5 +-
arch/powerpc/include/asm/opal.h | 6 +
arch/powerpc/include/asm/secvar.h | 55 +++++
arch/powerpc/kernel/Makefile | 3 +-
arch/powerpc/kernel/secvar-ops.c | 25 +++
arch/powerpc/kernel/secvar-sysfs.c | 210 ++++++++++++++++++
arch/powerpc/platforms/powernv/Kconfig | 6 +
arch/powerpc/platforms/powernv/Makefile | 1 +
arch/powerpc/platforms/powernv/opal-call.c | 3 +
arch/powerpc/platforms/powernv/opal-secvar.c | 102 +++++++++
arch/powerpc/platforms/powernv/opal.c | 5 +
security/integrity/Kconfig | 9 +
security/integrity/Makefile | 6 +-
.../platform_certs/keyring_handler.c | 80 +++++++
.../platform_certs/keyring_handler.h | 35 +++
.../integrity/platform_certs/load_powerpc.c | 94 ++++++++
security/integrity/platform_certs/load_uefi.c | 67 +-----
19 files changed, 679 insertions(+), 69 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-secvar
create mode 100644 arch/powerpc/include/asm/secvar.h
create mode 100644 arch/powerpc/kernel/secvar-ops.c
create mode 100644 arch/powerpc/kernel/secvar-sysfs.c
create mode 100644 arch/powerpc/platforms/powernv/opal-secvar.c
create mode 100644 security/integrity/platform_certs/keyring_handler.c
create mode 100644 security/integrity/platform_certs/keyring_handler.h
create mode 100644 security/integrity/platform_certs/load_powerpc.c
--
2.20.1
^ permalink raw reply
* [PATCH v2 1/4] powerpc/powernv: Add OPAL API interface to access secure variable
From: Nayna Jain @ 2019-08-21 15:08 UTC (permalink / raw)
To: linuxppc-dev, linux-efi, linux-integrity
Cc: Ard Biesheuvel, Eric Ricther, Nayna Jain, linux-kernel,
Mimi Zohar, Claudio Carvalho, Matthew Garret, Greg Kroah-Hartman,
Paul Mackerras, Jeremy Kerr, Elaine Palmer, Oliver O'Halloran,
George Wilson
In-Reply-To: <1566400103-18201-1-git-send-email-nayna@linux.ibm.com>
The X.509 certificates trusted by the platform and required to secure boot
the OS kernel are wrapped in secure variables, which are controlled by
OPAL.
This patch adds firmware/kernel interface to read and write OPAL secure
variables based on the unique key.
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>
---
arch/powerpc/include/asm/opal-api.h | 5 +-
arch/powerpc/include/asm/opal.h | 6 ++
arch/powerpc/include/asm/secvar.h | 55 ++++++++++
arch/powerpc/kernel/Makefile | 2 +-
arch/powerpc/kernel/secvar-ops.c | 25 +++++
arch/powerpc/platforms/powernv/Kconfig | 6 ++
arch/powerpc/platforms/powernv/Makefile | 1 +
arch/powerpc/platforms/powernv/opal-call.c | 3 +
arch/powerpc/platforms/powernv/opal-secvar.c | 102 +++++++++++++++++++
arch/powerpc/platforms/powernv/opal.c | 5 +
10 files changed, 208 insertions(+), 2 deletions(-)
create mode 100644 arch/powerpc/include/asm/secvar.h
create mode 100644 arch/powerpc/kernel/secvar-ops.c
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 383242eb0dea..b238b4f26c5b 100644
--- a/arch/powerpc/include/asm/opal-api.h
+++ b/arch/powerpc/include/asm/opal-api.h
@@ -208,7 +208,10 @@
#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_GET_NEXT 174
+#define OPAL_SECVAR_ENQUEUE_UPDATE 175
+#define OPAL_LAST 175
#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.h b/arch/powerpc/include/asm/opal.h
index 57bd029c715e..247adec2375f 100644
--- a/arch/powerpc/include/asm/opal.h
+++ b/arch/powerpc/include/asm/opal.h
@@ -388,6 +388,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_data, uint64_t k_data_size);
+extern int opal_secvar_get_next(uint64_t k_key, uint64_t k_key_len,
+ uint64_t k_key_size);
+extern int opal_secvar_enqueue_update(uint64_t k_key, uint64_t k_key_len,
+ uint64_t k_data, uint64_t k_data_size);
#endif /* __ASSEMBLY__ */
#endif /* _ASM_POWERPC_OPAL_H */
diff --git a/arch/powerpc/include/asm/secvar.h b/arch/powerpc/include/asm/secvar.h
new file mode 100644
index 000000000000..645654456265
--- /dev/null
+++ b/arch/powerpc/include/asm/secvar.h
@@ -0,0 +1,55 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * PowerPC secure variable operations.
+ *
+ * Copyright (C) 2019 IBM Corporation
+ * Author: Nayna Jain <nayna@linux.ibm.com>
+ *
+ */
+#ifndef SECVAR_OPS_H
+#define SECVAR_OPS_H
+
+#include<linux/types.h>
+#include<linux/errno.h>
+
+struct secvar_operations {
+ int (*get_variable)(const char *key, unsigned long key_len, u8 *data,
+ unsigned long *data_size);
+ int (*get_next_variable)(const char *key, unsigned long *key_len,
+ unsigned long keysize);
+ int (*set_variable)(const char *key, unsigned long key_len, u8 *data,
+ unsigned long data_size);
+};
+
+#ifdef CONFIG_PPC_SECURE_BOOT
+
+extern void set_secvar_ops(struct secvar_operations *ops);
+extern struct secvar_operations *get_secvar_ops(void);
+
+#else
+
+static inline void set_secvar_ops(struct secvar_operations *ops)
+{
+}
+
+static inline struct secvar_operations *get_secvar_ops(void)
+{
+ return NULL;
+}
+
+#endif
+
+#ifdef CONFIG_OPAL_SECVAR
+
+extern int secvar_init(void);
+
+#else
+
+static inline int secvar_init(void)
+{
+ return -EINVAL;
+}
+
+#endif
+
+#endif
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 520b1c814197..9041563f1c74 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -157,7 +157,7 @@ endif
obj-$(CONFIG_EPAPR_PARAVIRT) += epapr_paravirt.o epapr_hcalls.o
obj-$(CONFIG_KVM_GUEST) += kvm.o kvm_emul.o
-obj-$(CONFIG_PPC_SECURE_BOOT) += secboot.o ima_arch.o
+obj-$(CONFIG_PPC_SECURE_BOOT) += secboot.o ima_arch.o secvar-ops.o
# Disable GCOV, KCOV & sanitizers in odd or sensitive code
GCOV_PROFILE_prom_init.o := n
diff --git a/arch/powerpc/kernel/secvar-ops.c b/arch/powerpc/kernel/secvar-ops.c
new file mode 100644
index 000000000000..198222499848
--- /dev/null
+++ b/arch/powerpc/kernel/secvar-ops.c
@@ -0,0 +1,25 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2019 IBM Corporation
+ * Author: Nayna Jain <nayna@linux.ibm.com>
+ *
+ * secvar-ops.c
+ * - initialize secvar operations for PowerPC Secureboot
+ */
+
+#include<stddef.h>
+#include<asm/secvar.h>
+
+static struct secvar_operations *secvars_ops;
+
+void set_secvar_ops(struct secvar_operations *ops)
+{
+ if (!ops)
+ secvars_ops = NULL;
+ secvars_ops = ops;
+}
+
+struct secvar_operations *get_secvar_ops(void)
+{
+ return secvars_ops;
+}
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 29ca523c1c79..93106e867924 100644
--- a/arch/powerpc/platforms/powernv/opal-call.c
+++ b/arch/powerpc/platforms/powernv/opal-call.c
@@ -287,3 +287,6 @@ 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_get_next, OPAL_SECVAR_GET_NEXT);
+OPAL_CALL(opal_secvar_enqueue_update, OPAL_SECVAR_ENQUEUE_UPDATE);
diff --git a/arch/powerpc/platforms/powernv/opal-secvar.c b/arch/powerpc/platforms/powernv/opal-secvar.c
new file mode 100644
index 000000000000..b0f97cea7675
--- /dev/null
+++ b/arch/powerpc/platforms/powernv/opal-secvar.c
@@ -0,0 +1,102 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * PowerNV code for secure variables
+ *
+ * Copyright (C) 2019 IBM Corporation
+ * Author: Claudio Carvalho <cclaudio@linux.ibm.com>
+ *
+ * APIs to access secure variables managed by OPAL.
+ *
+ */
+
+#define pr_fmt(fmt) "secvar: "fmt
+
+#include <linux/types.h>
+#include <asm/opal.h>
+#include <asm/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_GET_NEXT)
+ || !opal_check_token(OPAL_SECVAR_ENQUEUE_UPDATE)) {
+ pr_err("OPAL doesn't support secure variables\n");
+ opal_secvar_supported = false;
+ } else {
+ opal_secvar_supported = true;
+ }
+
+ initialized = true;
+
+ return opal_secvar_supported;
+}
+
+static int opal_get_variable(const char *key, unsigned long ksize,
+ u8 *data, unsigned long *dsize)
+{
+ int rc;
+
+ if (!is_opal_secvar_supported())
+ return OPAL_UNSUPPORTED;
+
+ if (dsize)
+ *dsize = cpu_to_be64(*dsize);
+
+ rc = opal_secvar_get(__pa(key), ksize,
+ __pa(data), __pa(dsize));
+
+ if (dsize)
+ *dsize = be64_to_cpu(*dsize);
+
+ return rc;
+}
+
+static int opal_get_next_variable(const char *key, unsigned long *keylen,
+ unsigned long keysize)
+{
+ int rc;
+
+ if (!is_opal_secvar_supported())
+ return OPAL_UNSUPPORTED;
+
+ if (keylen)
+ *keylen = cpu_to_be64(*keylen);
+
+ rc = opal_secvar_get_next(__pa(key), __pa(keylen), keysize);
+
+ if (keylen)
+ *keylen = be64_to_cpu(*keylen);
+
+ return rc;
+}
+
+static int opal_set_variable(const char *key, unsigned long ksize, u8 *data,
+ unsigned long dsize)
+{
+ int rc;
+
+ if (!is_opal_secvar_supported())
+ return OPAL_UNSUPPORTED;
+
+ rc = opal_secvar_enqueue_update(__pa(key), ksize, __pa(data), dsize);
+
+ return rc;
+}
+
+static struct secvar_operations secvar_ops = {
+ .get_variable = opal_get_variable,
+ .get_next_variable = opal_get_next_variable,
+ .set_variable = opal_set_variable,
+};
+
+int secvar_init(void)
+{
+ set_secvar_ops(&secvar_ops);
+ return 0;
+}
diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
index aba443be7daa..ffe6f1cf0830 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c
@@ -32,6 +32,8 @@
#include <asm/mce.h>
#include <asm/imc-pmu.h>
#include <asm/bug.h>
+#include <asm/secvar.h>
+#include <asm/secboot.h>
#include "powernv.h"
@@ -988,6 +990,9 @@ static int __init opal_init(void)
/* Initialise OPAL Power control interface */
opal_power_control_init();
+ if (is_powerpc_secvar_supported())
+ secvar_init();
+
return 0;
}
machine_subsys_initcall(powernv, opal_init);
--
2.20.1
^ permalink raw reply related
* [PATCH v2 2/4] powerpc: expose secure variables to userspace via sysfs
From: Nayna Jain @ 2019-08-21 15:08 UTC (permalink / raw)
To: linuxppc-dev, linux-efi, linux-integrity
Cc: Ard Biesheuvel, Eric Ricther, Nayna Jain, linux-kernel,
Mimi Zohar, Claudio Carvalho, Matthew Garret, Greg Kroah-Hartman,
Paul Mackerras, Jeremy Kerr, Elaine Palmer, Oliver O'Halloran,
George Wilson
In-Reply-To: <1566400103-18201-1-git-send-email-nayna@linux.ibm.com>
PowerNV secure variables, which store the keys used for OS kernel
verification, are managed by the firmware. These secure variables need to
be accessed by the userspace for addition/deletion of the certificates.
This patch adds the sysfs interface to expose secure variables for PowerNV
secureboot. The users shall use this interface for manipulating
the keys stored in the secure variables.
Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
---
Documentation/ABI/testing/sysfs-secvar | 27 ++++
arch/powerpc/Kconfig | 9 ++
arch/powerpc/kernel/Makefile | 1 +
arch/powerpc/kernel/secvar-sysfs.c | 210 +++++++++++++++++++++++++
4 files changed, 247 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-secvar
create mode 100644 arch/powerpc/kernel/secvar-sysfs.c
diff --git a/Documentation/ABI/testing/sysfs-secvar b/Documentation/ABI/testing/sysfs-secvar
new file mode 100644
index 000000000000..68f0e03d873d
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-secvar
@@ -0,0 +1,27 @@
+What: /sys/firmware/secvar
+Date: August 2019
+Contact: Nayna Jain <nayna@linux.ibm.com>
+Description:
+ This directory exposes interfaces for interacting with
+ the secure variables managed by OPAL firmware.
+
+ This is only for the powerpc/powernv platform.
+
+ Directory:
+ vars: This directory lists all the variables that
+ are supported by the OPAL. The variables are
+ represented in the form of directories with
+ their variable names. The variable name is
+ unique and is in ASCII representation. The data
+ and size can be determined by reading their
+ respective attribute files.
+
+ Each variable directory has the following files:
+ name: An ASCII representation of the variable name
+ data: A read-only file containing the value of the
+ variable
+ size: An integer representation of the size of the
+ content of the variable. In other works, it
+ represents the size of the data
+ update: A write-only file that is used to submit the new
+ value for the variable.
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 42109682b727..b4bdf77837b2 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -925,6 +925,15 @@ config PPC_SECURE_BOOT
allows user to enable OS Secure Boot on PowerPC systems that
have firmware secure boot support.
+config SECVAR_SYSFS
+ tristate "Enable sysfs interface for POWER secure variables"
+ depends on PPC_SECURE_BOOT
+ help
+ POWER secure variables are managed and controlled by firmware.
+ These variables are exposed to userspace via sysfs to enable
+ read/write operations on these variables. Say Y if you have
+ secure boot enabled and want to expose variables to userspace.
+
endmenu
config ISA_DMA_API
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 9041563f1c74..4ea7b738c3a3 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -158,6 +158,7 @@ obj-$(CONFIG_EPAPR_PARAVIRT) += epapr_paravirt.o epapr_hcalls.o
obj-$(CONFIG_KVM_GUEST) += kvm.o kvm_emul.o
obj-$(CONFIG_PPC_SECURE_BOOT) += secboot.o ima_arch.o secvar-ops.o
+obj-$(CONFIG_SECVAR_SYSFS) += secvar-sysfs.o
# Disable GCOV, KCOV & sanitizers in odd or sensitive code
GCOV_PROFILE_prom_init.o := n
diff --git a/arch/powerpc/kernel/secvar-sysfs.c b/arch/powerpc/kernel/secvar-sysfs.c
new file mode 100644
index 000000000000..e46986bb29a0
--- /dev/null
+++ b/arch/powerpc/kernel/secvar-sysfs.c
@@ -0,0 +1,210 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2019 IBM Corporation <nayna@linux.ibm.com>
+ *
+ * This code exposes secure variables to user via sysfs
+ */
+
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/compat.h>
+#include <linux/string.h>
+#include <asm/opal.h>
+#include <asm/secvar.h>
+
+//Approximating it for now, it is bound to change.
+#define VARIABLE_MAX_SIZE 32000
+
+static struct kobject *powerpc_kobj;
+static struct secvar_operations *secvarops;
+struct kset *secvar_kset;
+
+static ssize_t name_show(struct kobject *kobj, struct kobj_attribute *attr,
+ char *buf)
+{
+ return sprintf(buf, "%s", kobj->name);
+}
+
+static ssize_t size_show(struct kobject *kobj, struct kobj_attribute *attr,
+ char *buf)
+{
+ unsigned long dsize;
+ int rc;
+
+ rc = secvarops->get_variable(kobj->name, strlen(kobj->name) + 1, NULL,
+ &dsize);
+ if (rc) {
+ pr_err("Error retrieving variable size %d\n", rc);
+ return rc;
+ }
+
+ rc = sprintf(buf, "%ld", dsize);
+
+ return rc;
+}
+
+static ssize_t data_read(struct file *filep, struct kobject *kobj,
+ struct bin_attribute *attr, char *buf, loff_t off,
+ size_t count)
+{
+ unsigned long dsize;
+ int rc;
+ char *data;
+
+ rc = secvarops->get_variable(kobj->name, strlen(kobj->name) + 1, NULL,
+ &dsize);
+ if (rc) {
+ pr_err("Error getting variable size %d\n", rc);
+ return rc;
+ }
+ pr_debug("dsize is %ld\n", dsize);
+
+ data = kzalloc(dsize, GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ rc = secvarops->get_variable(kobj->name, strlen(kobj->name)+1, data,
+ &dsize);
+ if (rc) {
+ pr_err("Error getting variable %d\n", rc);
+ goto data_fail;
+ }
+
+ rc = memory_read_from_buffer(buf, count, &off, data, dsize);
+
+data_fail:
+ kfree(data);
+ return rc;
+}
+
+static ssize_t update_write(struct file *filep, struct kobject *kobj,
+ struct bin_attribute *attr, char *buf, loff_t off,
+ size_t count)
+{
+ int rc;
+
+ pr_debug("count is %ld\n", count);
+ rc = secvarops->set_variable(kobj->name, strlen(kobj->name)+1, buf,
+ count);
+ if (rc) {
+ pr_err("Error setting the variable %s\n", kobj->name);
+ return rc;
+ }
+
+ return count;
+}
+
+static struct kobj_attribute name_attr =
+__ATTR(name, 0444, name_show, NULL);
+
+static struct kobj_attribute size_attr =
+__ATTR(size, 0444, size_show, NULL);
+
+static struct bin_attribute data_attr = {
+ .attr = {.name = "data", .mode = 0444},
+ .size = VARIABLE_MAX_SIZE,
+ .read = data_read,
+};
+
+
+static struct bin_attribute update_attr = {
+ .attr = {.name = "update", .mode = 0200},
+ .size = VARIABLE_MAX_SIZE,
+ .write = update_write,
+};
+
+static struct bin_attribute *secvar_bin_attrs[] = {
+ &data_attr,
+ &update_attr,
+ NULL,
+};
+
+static struct attribute *secvar_attrs[] = {
+ &name_attr.attr,
+ &size_attr.attr,
+ NULL,
+};
+
+const struct attribute_group secvar_attr_group = {
+ .attrs = secvar_attrs,
+ .bin_attrs = secvar_bin_attrs,
+};
+
+int secvar_sysfs_load(void)
+{
+
+ char *name;
+ unsigned long namesize;
+ struct kobject *kobj;
+ int status;
+ int rc = 0;
+
+ name = kzalloc(1024, GFP_KERNEL);
+ if (!name)
+ return -ENOMEM;
+
+ do {
+
+ status = secvarops->get_next_variable(name, &namesize, 1024);
+ if (status != OPAL_SUCCESS)
+ break;
+
+ pr_info("name is %s\n", name);
+ kobj = kobject_create_and_add(name, &(secvar_kset->kobj));
+ if (kobj) {
+ rc = sysfs_create_group(kobj, &secvar_attr_group);
+ if (rc)
+ pr_err("Error creating attributes for %s variable\n",
+ name);
+ } else {
+ pr_err("Error creating sysfs entry for %s variable\n",
+ name);
+ rc = -EINVAL;
+ }
+
+ } while ((status == OPAL_SUCCESS) && (rc == 0));
+
+ kfree(name);
+ return rc;
+}
+
+int secvar_sysfs_init(void)
+{
+ powerpc_kobj = kobject_create_and_add("secvar", firmware_kobj);
+ if (!powerpc_kobj) {
+ pr_err("secvar: Failed to create firmware kobj\n");
+ return -ENODEV;
+ }
+
+ secvar_kset = kset_create_and_add("vars", NULL, powerpc_kobj);
+ if (!secvar_kset) {
+ pr_err("secvar: sysfs kobject registration failed.\n");
+ return -ENODEV;
+ }
+
+ secvarops = get_secvar_ops();
+ if (!secvarops) {
+ kobject_put(powerpc_kobj);
+ pr_err("secvar: failed to retrieve secvar operations.\n");
+ return -ENODEV;
+ }
+
+ secvar_sysfs_load();
+ pr_info("Secure variables sysfs initialized");
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(secvar_sysfs_init);
+
+static void secvar_sysfs_exit(void)
+{
+ kobject_put(powerpc_kobj);
+}
+EXPORT_SYMBOL_GPL(secvar_sysfs_exit);
+
+module_init(secvar_sysfs_init);
+module_exit(secvar_sysfs_exit);
+
+MODULE_AUTHOR("Nayna Jain<nayna@linux.ibm.com>");
+MODULE_DESCRIPTION("sysfs interface to POWER secure variables");
+MODULE_LICENSE("GPL");
--
2.20.1
^ permalink raw reply related
* [PATCH v2 3/4] x86/efi: move common keyring handler functions to new file
From: Nayna Jain @ 2019-08-21 15:08 UTC (permalink / raw)
To: linuxppc-dev, linux-efi, linux-integrity
Cc: Ard Biesheuvel, Eric Ricther, Nayna Jain, linux-kernel,
Mimi Zohar, Claudio Carvalho, Matthew Garret, Greg Kroah-Hartman,
Paul Mackerras, Jeremy Kerr, Elaine Palmer, Oliver O'Halloran,
George Wilson
In-Reply-To: <1566400103-18201-1-git-send-email-nayna@linux.ibm.com>
This patch moves the common code to keyring_handler.c
Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
---
security/integrity/Makefile | 3 +-
.../platform_certs/keyring_handler.c | 80 +++++++++++++++++++
.../platform_certs/keyring_handler.h | 35 ++++++++
security/integrity/platform_certs/load_uefi.c | 67 +---------------
4 files changed, 118 insertions(+), 67 deletions(-)
create mode 100644 security/integrity/platform_certs/keyring_handler.c
create mode 100644 security/integrity/platform_certs/keyring_handler.h
diff --git a/security/integrity/Makefile b/security/integrity/Makefile
index 19faace69644..525bf1d6e0db 100644
--- a/security/integrity/Makefile
+++ b/security/integrity/Makefile
@@ -11,7 +11,8 @@ integrity-$(CONFIG_INTEGRITY_SIGNATURE) += digsig.o
integrity-$(CONFIG_INTEGRITY_ASYMMETRIC_KEYS) += digsig_asymmetric.o
integrity-$(CONFIG_INTEGRITY_PLATFORM_KEYRING) += platform_certs/platform_keyring.o
integrity-$(CONFIG_LOAD_UEFI_KEYS) += platform_certs/efi_parser.o \
- platform_certs/load_uefi.o
+ platform_certs/load_uefi.o \
+ platform_certs/keyring_handler.o
integrity-$(CONFIG_LOAD_IPL_KEYS) += platform_certs/load_ipl_s390.o
$(obj)/load_uefi.o: KBUILD_CFLAGS += -fshort-wchar
diff --git a/security/integrity/platform_certs/keyring_handler.c b/security/integrity/platform_certs/keyring_handler.c
new file mode 100644
index 000000000000..c5ba695c10e3
--- /dev/null
+++ b/security/integrity/platform_certs/keyring_handler.c
@@ -0,0 +1,80 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/kernel.h>
+#include <linux/sched.h>
+#include <linux/cred.h>
+#include <linux/err.h>
+#include <linux/efi.h>
+#include <linux/slab.h>
+#include <keys/asymmetric-type.h>
+#include <keys/system_keyring.h>
+#include "../integrity.h"
+
+static efi_guid_t efi_cert_x509_guid __initdata = EFI_CERT_X509_GUID;
+static efi_guid_t efi_cert_x509_sha256_guid __initdata =
+ EFI_CERT_X509_SHA256_GUID;
+static efi_guid_t efi_cert_sha256_guid __initdata = EFI_CERT_SHA256_GUID;
+
+/*
+ * Blacklist a hash.
+ */
+static __init void uefi_blacklist_hash(const char *source, const void *data,
+ size_t len, const char *type,
+ size_t type_len)
+{
+ char *hash, *p;
+
+ hash = kmalloc(type_len + len * 2 + 1, GFP_KERNEL);
+ if (!hash)
+ return;
+ p = memcpy(hash, type, type_len);
+ p += type_len;
+ bin2hex(p, data, len);
+ p += len * 2;
+ *p = 0;
+
+ mark_hash_blacklisted(hash);
+ kfree(hash);
+}
+
+/*
+ * Blacklist an X509 TBS hash.
+ */
+static __init void uefi_blacklist_x509_tbs(const char *source,
+ const void *data, size_t len)
+{
+ uefi_blacklist_hash(source, data, len, "tbs:", 4);
+}
+
+/*
+ * Blacklist the hash of an executable.
+ */
+static __init void uefi_blacklist_binary(const char *source,
+ const void *data, size_t len)
+{
+ uefi_blacklist_hash(source, data, len, "bin:", 4);
+}
+
+/*
+ * Return the appropriate handler for particular signature list types found in
+ * the UEFI db and MokListRT tables.
+ */
+__init efi_element_handler_t get_handler_for_db(const efi_guid_t *sig_type)
+{
+ if (efi_guidcmp(*sig_type, efi_cert_x509_guid) == 0)
+ return add_to_platform_keyring;
+ return 0;
+}
+
+/*
+ * Return the appropriate handler for particular signature list types found in
+ * the UEFI dbx and MokListXRT tables.
+ */
+__init efi_element_handler_t get_handler_for_dbx(const efi_guid_t *sig_type)
+{
+ if (efi_guidcmp(*sig_type, efi_cert_x509_sha256_guid) == 0)
+ return uefi_blacklist_x509_tbs;
+ if (efi_guidcmp(*sig_type, efi_cert_sha256_guid) == 0)
+ return uefi_blacklist_binary;
+ return 0;
+}
diff --git a/security/integrity/platform_certs/keyring_handler.h b/security/integrity/platform_certs/keyring_handler.h
new file mode 100644
index 000000000000..829a14b95218
--- /dev/null
+++ b/security/integrity/platform_certs/keyring_handler.h
@@ -0,0 +1,35 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2019 IBM Corporation
+ * Author: Nayna Jain <nayna@linux.ibm.com>
+ */
+#ifndef PLATFORM_CERTS_INTERNAL_H
+#define PLATFORM_CERTS_INTERNAL_H
+
+#include <linux/efi.h>
+
+void blacklist_hash(const char *source, const void *data,
+ size_t len, const char *type,
+ size_t type_len);
+
+/*
+ * Blacklist an X509 TBS hash.
+ */
+void blacklist_x509_tbs(const char *source, const void *data, size_t len);
+
+/*
+ * Blacklist the hash of an executable.
+ */
+void blacklist_binary(const char *source, const void *data, size_t len);
+
+/*
+ * Return the handler for particular signature list types found in the db.
+ */
+efi_element_handler_t get_handler_for_db(const efi_guid_t *sig_type);
+
+/*
+ * Return the handler for particular signature list types found in the dbx.
+ */
+efi_element_handler_t get_handler_for_dbx(const efi_guid_t *sig_type);
+
+#endif
diff --git a/security/integrity/platform_certs/load_uefi.c b/security/integrity/platform_certs/load_uefi.c
index 81b19c52832b..4369204a19cd 100644
--- a/security/integrity/platform_certs/load_uefi.c
+++ b/security/integrity/platform_certs/load_uefi.c
@@ -9,6 +9,7 @@
#include <keys/asymmetric-type.h>
#include <keys/system_keyring.h>
#include "../integrity.h"
+#include "keyring_handler.h"
static efi_guid_t efi_cert_x509_guid __initdata = EFI_CERT_X509_GUID;
static efi_guid_t efi_cert_x509_sha256_guid __initdata =
@@ -67,72 +68,6 @@ static __init void *get_cert_list(efi_char16_t *name, efi_guid_t *guid,
return db;
}
-/*
- * Blacklist a hash.
- */
-static __init void uefi_blacklist_hash(const char *source, const void *data,
- size_t len, const char *type,
- size_t type_len)
-{
- char *hash, *p;
-
- hash = kmalloc(type_len + len * 2 + 1, GFP_KERNEL);
- if (!hash)
- return;
- p = memcpy(hash, type, type_len);
- p += type_len;
- bin2hex(p, data, len);
- p += len * 2;
- *p = 0;
-
- mark_hash_blacklisted(hash);
- kfree(hash);
-}
-
-/*
- * Blacklist an X509 TBS hash.
- */
-static __init void uefi_blacklist_x509_tbs(const char *source,
- const void *data, size_t len)
-{
- uefi_blacklist_hash(source, data, len, "tbs:", 4);
-}
-
-/*
- * Blacklist the hash of an executable.
- */
-static __init void uefi_blacklist_binary(const char *source,
- const void *data, size_t len)
-{
- uefi_blacklist_hash(source, data, len, "bin:", 4);
-}
-
-/*
- * Return the appropriate handler for particular signature list types found in
- * the UEFI db and MokListRT tables.
- */
-static __init efi_element_handler_t get_handler_for_db(const efi_guid_t *
- sig_type)
-{
- if (efi_guidcmp(*sig_type, efi_cert_x509_guid) == 0)
- return add_to_platform_keyring;
- return 0;
-}
-
-/*
- * Return the appropriate handler for particular signature list types found in
- * the UEFI dbx and MokListXRT tables.
- */
-static __init efi_element_handler_t get_handler_for_dbx(const efi_guid_t *
- sig_type)
-{
- if (efi_guidcmp(*sig_type, efi_cert_x509_sha256_guid) == 0)
- return uefi_blacklist_x509_tbs;
- if (efi_guidcmp(*sig_type, efi_cert_sha256_guid) == 0)
- return uefi_blacklist_binary;
- return 0;
-}
-
/*
* Load the certs contained in the UEFI databases into the platform trusted
* keyring and the UEFI blacklisted X.509 cert SHA256 hashes into the blacklist
--
2.20.1
^ permalink raw reply related
* [PATCH v2 4/4] powerpc: load firmware trusted keys into kernel keyring
From: Nayna Jain @ 2019-08-21 15:08 UTC (permalink / raw)
To: linuxppc-dev, linux-efi, linux-integrity
Cc: Ard Biesheuvel, Eric Ricther, Nayna Jain, linux-kernel,
Mimi Zohar, Claudio Carvalho, Matthew Garret, Greg Kroah-Hartman,
Paul Mackerras, Jeremy Kerr, Elaine Palmer, Oliver O'Halloran,
George Wilson
In-Reply-To: <1566400103-18201-1-git-send-email-nayna@linux.ibm.com>
The keys used to verify the Host OS kernel are managed by OPAL as secure
variables. This patch loads the verification keys into the .platform
keyring and revocation keys into .blacklist keyring. This enables
verification and loading of the kernels signed by the boot time keys which
are trusted by firmware.
Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
---
security/integrity/Kconfig | 9 ++
security/integrity/Makefile | 3 +
.../integrity/platform_certs/load_powerpc.c | 94 +++++++++++++++++++
3 files changed, 106 insertions(+)
create mode 100644 security/integrity/platform_certs/load_powerpc.c
diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig
index 0bae6adb63a9..2b4109c157e2 100644
--- a/security/integrity/Kconfig
+++ b/security/integrity/Kconfig
@@ -72,6 +72,15 @@ config LOAD_IPL_KEYS
depends on S390
def_bool y
+config LOAD_PPC_KEYS
+ bool "Enable loading of platform and revocation keys for POWER"
+ depends on INTEGRITY_PLATFORM_KEYRING
+ depends on PPC_SECURE_BOOT
+ def_bool y
+ help
+ Enable loading of db keys to the .platform keyring and dbx keys to
+ the .blacklist keyring for powerpc based platforms.
+
config INTEGRITY_AUDIT
bool "Enables integrity auditing support "
depends on AUDIT
diff --git a/security/integrity/Makefile b/security/integrity/Makefile
index 525bf1d6e0db..9eeb6b053de3 100644
--- a/security/integrity/Makefile
+++ b/security/integrity/Makefile
@@ -14,6 +14,9 @@ integrity-$(CONFIG_LOAD_UEFI_KEYS) += platform_certs/efi_parser.o \
platform_certs/load_uefi.o \
platform_certs/keyring_handler.o
integrity-$(CONFIG_LOAD_IPL_KEYS) += platform_certs/load_ipl_s390.o
+integrity-$(CONFIG_LOAD_PPC_KEYS) += platform_certs/efi_parser.o \
+ platform_certs/load_powerpc.o \
+ platform_certs/keyring_handler.o
$(obj)/load_uefi.o: KBUILD_CFLAGS += -fshort-wchar
subdir-$(CONFIG_IMA) += ima
diff --git a/security/integrity/platform_certs/load_powerpc.c b/security/integrity/platform_certs/load_powerpc.c
new file mode 100644
index 000000000000..f4d869171062
--- /dev/null
+++ b/security/integrity/platform_certs/load_powerpc.c
@@ -0,0 +1,94 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2019 IBM Corporation
+ * Author: Nayna Jain <nayna@linux.ibm.com>
+ *
+ * load_powernv.c
+ * - loads keys and certs stored and controlled
+ * by the firmware.
+ */
+#include <linux/kernel.h>
+#include <linux/sched.h>
+#include <linux/cred.h>
+#include <linux/err.h>
+#include <linux/slab.h>
+#include <asm/secboot.h>
+#include <asm/secvar.h>
+#include "keyring_handler.h"
+
+static struct secvar_operations *secvarops;
+
+/*
+ * Get a certificate list blob from the named EFI variable.
+ */
+static __init void *get_cert_list(u8 *key, unsigned long keylen,
+ unsigned long *size)
+{
+ int rc;
+ void *db;
+
+ rc = secvarops->get_variable(key, keylen, NULL, size);
+ if (rc) {
+ pr_err("Couldn't get size: %d\n", rc);
+ return NULL;
+ }
+
+ db = kmalloc(*size, GFP_KERNEL);
+ if (!db)
+ return NULL;
+
+ rc = secvarops->get_variable(key, keylen, db, size);
+ if (rc) {
+ kfree(db);
+ pr_err("Error reading db var: %d\n", rc);
+ return NULL;
+ }
+
+ return db;
+}
+
+/*
+ * Load the certs contained in the UEFI databases into the platform trusted
+ * keyring and the UEFI blacklisted X.509 cert SHA256 hashes into the blacklist
+ * keyring.
+ */
+static int __init load_powerpc_certs(void)
+{
+ void *db = NULL, *dbx = NULL;
+ unsigned long dbsize = 0, dbxsize = 0;
+ int rc = 0;
+
+ secvarops = get_secvar_ops();
+ if (!secvarops)
+ return -ENOENT;
+
+ /* Get db, and dbx. They might not exist, so it isn't
+ * an error if we can't get them.
+ */
+ db = get_cert_list("db", 3, &dbsize);
+ if (!db) {
+ pr_err("Couldn't get db list from OPAL\n");
+ } else {
+ rc = parse_efi_signature_list("OPAL:db",
+ db, dbsize, get_handler_for_db);
+ if (rc)
+ pr_err("Couldn't parse db signatures: %d\n",
+ rc);
+ kfree(db);
+ }
+
+ dbx = get_cert_list("dbx", 3, &dbxsize);
+ if (!dbx) {
+ pr_info("Couldn't get dbx list from OPAL\n");
+ } else {
+ rc = parse_efi_signature_list("OPAL:dbx",
+ dbx, dbxsize,
+ get_handler_for_dbx);
+ if (rc)
+ pr_err("Couldn't parse dbx signatures: %d\n", rc);
+ kfree(dbx);
+ }
+
+ return rc;
+}
+late_initcall(load_powerpc_certs);
--
2.20.1
^ permalink raw reply related
* [PATCH] powerpc/mm: tell if a bad page fault on data is read or write.
From: Christophe Leroy @ 2019-08-21 15:21 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
DSISR has a bit to tell if the fault is due to a read or a write.
Display it.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/mm/fault.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
index 8432c281de92..b5047f9b5dec 100644
--- a/arch/powerpc/mm/fault.c
+++ b/arch/powerpc/mm/fault.c
@@ -645,6 +645,7 @@ NOKPROBE_SYMBOL(do_page_fault);
void bad_page_fault(struct pt_regs *regs, unsigned long address, int sig)
{
const struct exception_table_entry *entry;
+ int is_write = page_fault_is_write(regs->dsisr);
/* Are we prepared to handle this fault? */
if ((entry = search_exception_tables(regs->nip)) != NULL) {
@@ -658,9 +659,10 @@ void bad_page_fault(struct pt_regs *regs, unsigned long address, int sig)
case 0x300:
case 0x380:
case 0xe00:
- pr_alert("BUG: %s at 0x%08lx\n",
+ pr_alert("BUG: %s on %s at 0x%08lx\n",
regs->dar < PAGE_SIZE ? "Kernel NULL pointer dereference" :
- "Unable to handle kernel data access", regs->dar);
+ "Unable to handle kernel data access",
+ is_write ? "write" : "read", regs->dar);
break;
case 0x400:
case 0x480:
--
2.13.3
^ permalink raw reply related
* [PATCH v1 5/5] mm/memory_hotplug: Remove zone parameter from __remove_pages()
From: David Hildenbrand @ 2019-08-21 15:40 UTC (permalink / raw)
To: linux-kernel
Cc: Mark Rutland, linux-s390, Rich Felker, linux-ia64,
David Hildenbrand, Peter Zijlstra, Catalin Marinas, Dave Hansen,
Heiko Carstens, Wei Yang, linux-mm, Michal Hocko, Paul Mackerras,
H. Peter Anvin, Will Deacon, Dan Williams, Halil Pasic, Yu Zhao,
Yoshinori Sato, Jason Gunthorpe, linux-sh, x86,
Matthew Wilcox (Oracle), Mike Rapoport, Jun Yao,
Christian Borntraeger, Ingo Molnar, linux-arm-kernel, Ira Weiny,
Fenghua Yu, Pavel Tatashin, Vasily Gorbik, Anshuman Khandual,
Masahiro Yamada, linuxppc-dev, Borislav Petkov, Andy Lutomirski,
Thomas Gleixner, Gerald Schaefer, Oscar Salvador, Tony Luck,
Steve Capper, Greg Kroah-Hartman, Logan Gunthorpe, Tom Lendacky,
Aneesh Kumar K.V, Qian Cai, Andrew Morton, Robin Murphy
In-Reply-To: <20190821154006.1338-1-david@redhat.com>
No longer in use, let's drop it. We no longer access the zone of
possibly never onlined memory (and therefore don't read garabage in
these scenarios).
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Vasily Gorbik <gor@linux.ibm.com>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: Rich Felker <dalias@libc.org>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86@kernel.org
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Steve Capper <steve.capper@arm.com>
Cc: Mike Rapoport <rppt@linux.ibm.com>
Cc: Anshuman Khandual <anshuman.khandual@arm.com>
Cc: Yu Zhao <yuzhao@google.com>
Cc: Jun Yao <yaojun8558363@gmail.com>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Oscar Salvador <osalvador@suse.de>
Cc: "Matthew Wilcox (Oracle)" <willy@infradead.org>
Cc: Christophe Leroy <christophe.leroy@c-s.fr>
Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>
Cc: Pavel Tatashin <pasha.tatashin@soleen.com>
Cc: Gerald Schaefer <gerald.schaefer@de.ibm.com>
Cc: Halil Pasic <pasic@linux.ibm.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Wei Yang <richard.weiyang@gmail.com>
Cc: Qian Cai <cai@lca.pw>
Cc: Jason Gunthorpe <jgg@ziepe.ca>
Cc: Logan Gunthorpe <logang@deltatee.com>
Cc: Ira Weiny <ira.weiny@intel.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-ia64@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-s390@vger.kernel.org
Cc: linux-sh@vger.kernel.org
Signed-off-by: David Hildenbrand <david@redhat.com>
---
arch/arm64/mm/mmu.c | 4 +---
arch/ia64/mm/init.c | 4 +---
arch/powerpc/mm/mem.c | 3 +--
arch/s390/mm/init.c | 4 +---
arch/sh/mm/init.c | 4 +---
arch/x86/mm/init_32.c | 4 +---
arch/x86/mm/init_64.c | 4 +---
include/linux/memory_hotplug.h | 4 ++--
mm/memory_hotplug.c | 6 +++---
mm/memremap.c | 3 +--
10 files changed, 13 insertions(+), 27 deletions(-)
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index e67bab4d613e..b3843aff12bf 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -1080,7 +1080,6 @@ void arch_remove_memory(int nid, u64 start, u64 size,
{
unsigned long start_pfn = start >> PAGE_SHIFT;
unsigned long nr_pages = size >> PAGE_SHIFT;
- struct zone *zone;
/*
* FIXME: Cleanup page tables (also in arch_add_memory() in case
@@ -1089,7 +1088,6 @@ void arch_remove_memory(int nid, u64 start, u64 size,
* unplug. ARCH_ENABLE_MEMORY_HOTREMOVE must not be
* unlocked yet.
*/
- zone = page_zone(pfn_to_page(start_pfn));
- __remove_pages(zone, start_pfn, nr_pages, altmap);
+ __remove_pages(start_pfn, nr_pages, altmap);
}
#endif
diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c
index bf9df2625bc8..a6dd80a2c939 100644
--- a/arch/ia64/mm/init.c
+++ b/arch/ia64/mm/init.c
@@ -689,9 +689,7 @@ void arch_remove_memory(int nid, u64 start, u64 size,
{
unsigned long start_pfn = start >> PAGE_SHIFT;
unsigned long nr_pages = size >> PAGE_SHIFT;
- struct zone *zone;
- zone = page_zone(pfn_to_page(start_pfn));
- __remove_pages(zone, start_pfn, nr_pages, altmap);
+ __remove_pages(start_pfn, nr_pages, altmap);
}
#endif
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index 9191a66b3bc5..7351c44c435a 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -130,10 +130,9 @@ void __ref arch_remove_memory(int nid, u64 start, u64 size,
{
unsigned long start_pfn = start >> PAGE_SHIFT;
unsigned long nr_pages = size >> PAGE_SHIFT;
- struct page *page = pfn_to_page(start_pfn) + vmem_altmap_offset(altmap);
int ret;
- __remove_pages(page_zone(page), start_pfn, nr_pages, altmap);
+ __remove_pages(start_pfn, nr_pages, altmap);
/* Remove htab bolted mappings for this section of memory */
start = (unsigned long)__va(start);
diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c
index 20340a03ad90..6f13eb66e375 100644
--- a/arch/s390/mm/init.c
+++ b/arch/s390/mm/init.c
@@ -296,10 +296,8 @@ void arch_remove_memory(int nid, u64 start, u64 size,
{
unsigned long start_pfn = start >> PAGE_SHIFT;
unsigned long nr_pages = size >> PAGE_SHIFT;
- struct zone *zone;
- zone = page_zone(pfn_to_page(start_pfn));
- __remove_pages(zone, start_pfn, nr_pages, altmap);
+ __remove_pages(start_pfn, nr_pages, altmap);
vmem_remove_mapping(start, size);
}
#endif /* CONFIG_MEMORY_HOTPLUG */
diff --git a/arch/sh/mm/init.c b/arch/sh/mm/init.c
index dfdbaa50946e..d1b1ff2be17a 100644
--- a/arch/sh/mm/init.c
+++ b/arch/sh/mm/init.c
@@ -434,9 +434,7 @@ void arch_remove_memory(int nid, u64 start, u64 size,
{
unsigned long start_pfn = PFN_DOWN(start);
unsigned long nr_pages = size >> PAGE_SHIFT;
- struct zone *zone;
- zone = page_zone(pfn_to_page(start_pfn));
- __remove_pages(zone, start_pfn, nr_pages, altmap);
+ __remove_pages(start_pfn, nr_pages, altmap);
}
#endif /* CONFIG_MEMORY_HOTPLUG */
diff --git a/arch/x86/mm/init_32.c b/arch/x86/mm/init_32.c
index 4068abb9427f..9d036be27aaa 100644
--- a/arch/x86/mm/init_32.c
+++ b/arch/x86/mm/init_32.c
@@ -865,10 +865,8 @@ void arch_remove_memory(int nid, u64 start, u64 size,
{
unsigned long start_pfn = start >> PAGE_SHIFT;
unsigned long nr_pages = size >> PAGE_SHIFT;
- struct zone *zone;
- zone = page_zone(pfn_to_page(start_pfn));
- __remove_pages(zone, start_pfn, nr_pages, altmap);
+ __remove_pages(start_pfn, nr_pages, altmap);
}
#endif
diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
index a6b5c653727b..b8541d77452c 100644
--- a/arch/x86/mm/init_64.c
+++ b/arch/x86/mm/init_64.c
@@ -1212,10 +1212,8 @@ void __ref arch_remove_memory(int nid, u64 start, u64 size,
{
unsigned long start_pfn = start >> PAGE_SHIFT;
unsigned long nr_pages = size >> PAGE_SHIFT;
- struct page *page = pfn_to_page(start_pfn) + vmem_altmap_offset(altmap);
- struct zone *zone = page_zone(page);
- __remove_pages(zone, start_pfn, nr_pages, altmap);
+ __remove_pages(start_pfn, nr_pages, altmap);
kernel_physical_mapping_remove(start, start + size);
}
#endif /* CONFIG_MEMORY_HOTPLUG */
diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
index f46ea71b4ffd..f75d9483864f 100644
--- a/include/linux/memory_hotplug.h
+++ b/include/linux/memory_hotplug.h
@@ -125,8 +125,8 @@ static inline bool movable_node_is_enabled(void)
extern void arch_remove_memory(int nid, u64 start, u64 size,
struct vmem_altmap *altmap);
-extern void __remove_pages(struct zone *zone, unsigned long start_pfn,
- unsigned long nr_pages, struct vmem_altmap *altmap);
+extern void __remove_pages(unsigned long start_pfn, unsigned long nr_pages,
+ struct vmem_altmap *altmap);
/* reasonably generic interface to expand the physical pages */
extern int __add_pages(int nid, unsigned long start_pfn, unsigned long nr_pages,
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index e88c96cf9d77..7a9719a762fe 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -532,7 +532,6 @@ static void __remove_section(unsigned long pfn, unsigned long nr_pages,
/**
* __remove_pages() - remove sections of pages from a zone
- * @zone: zone from which pages need to be removed
* @pfn: starting pageframe (must be aligned to start of a section)
* @nr_pages: number of pages to remove (must be multiple of section size)
* @altmap: alternative device page map or %NULL if default memmap is used
@@ -542,12 +541,13 @@ static void __remove_section(unsigned long pfn, unsigned long nr_pages,
* sure that pages are marked reserved and zones are adjust properly by
* calling offline_pages().
*/
-void __remove_pages(struct zone *zone, unsigned long pfn,
- unsigned long nr_pages, struct vmem_altmap *altmap)
+void __remove_pages(unsigned long pfn, unsigned long nr_pages,
+ struct vmem_altmap *altmap)
{
const unsigned long end_pfn = pfn + nr_pages;
unsigned long cur_nr_pages;
unsigned long map_offset = 0;
+ struct zone *zone;
if (check_pfn_span(pfn, nr_pages, "remove"))
return;
diff --git a/mm/memremap.c b/mm/memremap.c
index 8a394552b5bd..7e34f42e5f5a 100644
--- a/mm/memremap.c
+++ b/mm/memremap.c
@@ -138,8 +138,7 @@ static void devm_memremap_pages_release(void *data)
mem_hotplug_begin();
if (pgmap->type == MEMORY_DEVICE_PRIVATE) {
pfn = PHYS_PFN(res->start);
- __remove_pages(page_zone(pfn_to_page(pfn)), pfn,
- PHYS_PFN(resource_size(res)), NULL);
+ __remove_pages(pfn, PHYS_PFN(resource_size(res)), NULL);
} else {
arch_remove_memory(nid, res->start, resource_size(res),
pgmap_altmap(pgmap));
--
2.21.0
^ permalink raw reply related
* Re: [PATCH] powerpc/vdso64: inline __get_datapage()
From: Nathan Lynch @ 2019-08-21 15:58 UTC (permalink / raw)
To: Christophe Leroy; +Cc: Santosh Sivaraj, linuxppc-dev
In-Reply-To: <3724e1ba-d0f8-7247-73c2-6d83a3dbd040@c-s.fr>
Christophe Leroy <christophe.leroy@c-s.fr> writes:
> Le 21/08/2019 à 11:29, Santosh Sivaraj a écrit :
>> __get_datapage() is only a few instructions to retrieve the
>> address of the page where the kernel stores data to the VDSO.
>>
>> By inlining this function into its users, a bl/blr pair and
>> a mflr/mtlr pair is avoided, plus a few reg moves.
>>
>> clock-gettime-monotonic: syscall: 514 nsec/call 396 nsec/call
>> clock-gettime-monotonic: libc: 25 nsec/call 24 nsec/call
>> clock-gettime-monotonic: vdso: 20 nsec/call 20 nsec/call
>> clock-getres-monotonic: syscall: 347 nsec/call 372 nsec/call
>> clock-getres-monotonic: libc: 19 nsec/call 19 nsec/call
>> clock-getres-monotonic: vdso: 10 nsec/call 10 nsec/call
>> clock-gettime-monotonic-coarse: syscall: 511 nsec/call 396 nsec/call
>> clock-gettime-monotonic-coarse: libc: 23 nsec/call 21 nsec/call
>> clock-gettime-monotonic-coarse: vdso: 15 nsec/call 13 nsec/call
>> clock-gettime-realtime: syscall: 526 nsec/call 405 nsec/call
>> clock-gettime-realtime: libc: 24 nsec/call 23 nsec/call
>> clock-gettime-realtime: vdso: 18 nsec/call 18 nsec/call
>> clock-getres-realtime: syscall: 342 nsec/call 372 nsec/call
>> clock-getres-realtime: libc: 19 nsec/call 19 nsec/call
>> clock-getres-realtime: vdso: 10 nsec/call 10 nsec/call
>> clock-gettime-realtime-coarse: syscall: 515 nsec/call 373 nsec/call
>> clock-gettime-realtime-coarse: libc: 23 nsec/call 22 nsec/call
>> clock-gettime-realtime-coarse: vdso: 14 nsec/call 13 nsec/call
>
> I think you should only put the measurements on vdso calls, and only the
> ones that are impacted by the change. For exemple, getres function
> doesn't use __get_datapage so showing it here is pointless.
I agree with this point, but also, I would caution against using
vdsotest's benchmark function for anything like rigorous performance
analysis. The intention was to roughly confirm the VDSO's relative
performance vs the in-kernel implementations. Not to compare one VDSO
implementation of (say) clock_gettime to another.
I suggest using perf to confirm the expected effects of the change, if
possible.
^ permalink raw reply
* Re: [PATCH] powerpc/vdso64: inline __get_datapage()
From: Christophe Leroy @ 2019-08-21 16:23 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: Santosh Sivaraj, linuxppc-dev
In-Reply-To: <20190821121517.GD31406@gate.crashing.org>
Le 21/08/2019 à 14:15, Segher Boessenkool a écrit :
> On Wed, Aug 21, 2019 at 01:50:52PM +0200, Christophe Leroy wrote:
>> Do you have any idea on how to avoid that bcl/mflr stuff ?
>
> Do a load from some fixed address? Maybe an absolute address, even?
> lwz r3,-12344(0) or similar (that address is in kernel space...)
>
> There aren't many options, and certainly not many *good* options!
>
IIUC, the VDSO is seen by apps the same way as a dynamic lib. Couldn't
the relocation be done only once when the app loads the VDSO as for a
regular .so lib ?
It looks like it is what others do, at least x86 and arm64, unless I
misunderstood their code.
Christophe
^ permalink raw reply
* Re: [PATCH v2 2/4] powerpc: expose secure variables to userspace via sysfs
From: Greg Kroah-Hartman @ 2019-08-21 16:30 UTC (permalink / raw)
To: Nayna Jain
Cc: linux-efi, Ard Biesheuvel, Eric Ricther, linux-kernel, Mimi Zohar,
Claudio Carvalho, Matthew Garret, linuxppc-dev, Paul Mackerras,
Jeremy Kerr, Elaine Palmer, Oliver O'Halloran,
linux-integrity, George Wilson
In-Reply-To: <1566400103-18201-3-git-send-email-nayna@linux.ibm.com>
On Wed, Aug 21, 2019 at 11:08:21AM -0400, Nayna Jain wrote:
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-secvar
> @@ -0,0 +1,27 @@
> +What: /sys/firmware/secvar
> +Date: August 2019
> +Contact: Nayna Jain <nayna@linux.ibm.com>
> +Description:
> + This directory exposes interfaces for interacting with
> + the secure variables managed by OPAL firmware.
> +
> + This is only for the powerpc/powernv platform.
> +
> + Directory:
> + vars: This directory lists all the variables that
> + are supported by the OPAL. The variables are
> + represented in the form of directories with
> + their variable names. The variable name is
> + unique and is in ASCII representation. The data
> + and size can be determined by reading their
> + respective attribute files.
> +
> + Each variable directory has the following files:
> + name: An ASCII representation of the variable name
> + data: A read-only file containing the value of the
> + variable
> + size: An integer representation of the size of the
> + content of the variable. In other works, it
> + represents the size of the data
> + update: A write-only file that is used to submit the new
> + value for the variable.
Can you break this out into one-entry-per-file like most other entries
are defined? That makes it easier for tools to parse (specifically the
tool in the tree right now...)
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 42109682b727..b4bdf77837b2 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -925,6 +925,15 @@ config PPC_SECURE_BOOT
> allows user to enable OS Secure Boot on PowerPC systems that
> have firmware secure boot support.
>
> +config SECVAR_SYSFS
> + tristate "Enable sysfs interface for POWER secure variables"
> + depends on PPC_SECURE_BOOT
No depends on SYSFS?
> + help
> + POWER secure variables are managed and controlled by firmware.
> + These variables are exposed to userspace via sysfs to enable
> + read/write operations on these variables. Say Y if you have
> + secure boot enabled and want to expose variables to userspace.
Mix of tabs and spaces :(
> +
> endmenu
>
> config ISA_DMA_API
> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
> index 9041563f1c74..4ea7b738c3a3 100644
> --- a/arch/powerpc/kernel/Makefile
> +++ b/arch/powerpc/kernel/Makefile
> @@ -158,6 +158,7 @@ obj-$(CONFIG_EPAPR_PARAVIRT) += epapr_paravirt.o epapr_hcalls.o
> obj-$(CONFIG_KVM_GUEST) += kvm.o kvm_emul.o
>
> obj-$(CONFIG_PPC_SECURE_BOOT) += secboot.o ima_arch.o secvar-ops.o
> +obj-$(CONFIG_SECVAR_SYSFS) += secvar-sysfs.o
No tab?
>
> # Disable GCOV, KCOV & sanitizers in odd or sensitive code
> GCOV_PROFILE_prom_init.o := n
> diff --git a/arch/powerpc/kernel/secvar-sysfs.c b/arch/powerpc/kernel/secvar-sysfs.c
> new file mode 100644
> index 000000000000..e46986bb29a0
> --- /dev/null
> +++ b/arch/powerpc/kernel/secvar-sysfs.c
> @@ -0,0 +1,210 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2019 IBM Corporation <nayna@linux.ibm.com>
> + *
> + * This code exposes secure variables to user via sysfs
> + */
> +
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/compat.h>
> +#include <linux/string.h>
> +#include <asm/opal.h>
> +#include <asm/secvar.h>
> +
> +//Approximating it for now, it is bound to change.
" " before "A" here please.
> +#define VARIABLE_MAX_SIZE 32000
> +
> +static struct kobject *powerpc_kobj;
> +static struct secvar_operations *secvarops;
> +struct kset *secvar_kset;
> +
> +static ssize_t name_show(struct kobject *kobj, struct kobj_attribute *attr,
> + char *buf)
> +{
> + return sprintf(buf, "%s", kobj->name);
> +}
Why do you need this entry as it is the directory name? Userspace
already "knows" it if they can open this file.
> +
> +static ssize_t size_show(struct kobject *kobj, struct kobj_attribute *attr,
> + char *buf)
> +{
> + unsigned long dsize;
> + int rc;
> +
> + rc = secvarops->get_variable(kobj->name, strlen(kobj->name) + 1, NULL,
> + &dsize);
> + if (rc) {
> + pr_err("Error retrieving variable size %d\n", rc);
> + return rc;
> + }
> +
> + rc = sprintf(buf, "%ld", dsize);
> +
> + return rc;
> +}
> +
> +static ssize_t data_read(struct file *filep, struct kobject *kobj,
> + struct bin_attribute *attr, char *buf, loff_t off,
> + size_t count)
> +{
> + unsigned long dsize;
> + int rc;
> + char *data;
> +
> + rc = secvarops->get_variable(kobj->name, strlen(kobj->name) + 1, NULL,
> + &dsize);
> + if (rc) {
> + pr_err("Error getting variable size %d\n", rc);
> + return rc;
> + }
> + pr_debug("dsize is %ld\n", dsize);
> +
> + data = kzalloc(dsize, GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + rc = secvarops->get_variable(kobj->name, strlen(kobj->name)+1, data,
> + &dsize);
> + if (rc) {
> + pr_err("Error getting variable %d\n", rc);
> + goto data_fail;
> + }
> +
> + rc = memory_read_from_buffer(buf, count, &off, data, dsize);
> +
> +data_fail:
> + kfree(data);
> + return rc;
> +}
> +
> +static ssize_t update_write(struct file *filep, struct kobject *kobj,
> + struct bin_attribute *attr, char *buf, loff_t off,
> + size_t count)
> +{
> + int rc;
> +
> + pr_debug("count is %ld\n", count);
> + rc = secvarops->set_variable(kobj->name, strlen(kobj->name)+1, buf,
> + count);
> + if (rc) {
> + pr_err("Error setting the variable %s\n", kobj->name);
> + return rc;
> + }
> +
> + return count;
> +}
> +
> +static struct kobj_attribute name_attr =
> +__ATTR(name, 0444, name_show, NULL);
__ATTR_RO()?
> +
> +static struct kobj_attribute size_attr =
> +__ATTR(size, 0444, size_show, NULL);
__ATTR_RO()?
> +
> +static struct bin_attribute data_attr = {
> + .attr = {.name = "data", .mode = 0444},
> + .size = VARIABLE_MAX_SIZE,
> + .read = data_read,
> +};
__BIN_ATTR_RO()?
> +
> +
> +static struct bin_attribute update_attr = {
> + .attr = {.name = "update", .mode = 0200},
> + .size = VARIABLE_MAX_SIZE,
> + .write = update_write,
> +};
__BIN_ATTR_RO()?
> +
> +static struct bin_attribute *secvar_bin_attrs[] = {
> + &data_attr,
> + &update_attr,
> + NULL,
> +};
> +
> +static struct attribute *secvar_attrs[] = {
> + &name_attr.attr,
> + &size_attr.attr,
> + NULL,
> +};
> +
> +const struct attribute_group secvar_attr_group = {
> + .attrs = secvar_attrs,
> + .bin_attrs = secvar_bin_attrs,
> +};
static?
> +
> +int secvar_sysfs_load(void)
> +{
> +
> + char *name;
No blank line. You didn't run this this through checkpatch, did you :(
> + unsigned long namesize;
> + struct kobject *kobj;
> + int status;
> + int rc = 0;
> +
> + name = kzalloc(1024, GFP_KERNEL);
Why 1024?
> + if (!name)
> + return -ENOMEM;
> +
> + do {
> +
> + status = secvarops->get_next_variable(name, &namesize, 1024);
> + if (status != OPAL_SUCCESS)
> + break;
> +
> + pr_info("name is %s\n", name);
Please delete debugging messages.
> + kobj = kobject_create_and_add(name, &(secvar_kset->kobj));
> + if (kobj) {
> + rc = sysfs_create_group(kobj, &secvar_attr_group);
You just raced userspace and lost :(
If you set your kobj_type to have the attribute group you will not race
and loose, the core will handle it for you.
> + if (rc)
> + pr_err("Error creating attributes for %s variable\n",
> + name);
> + } else {
> + pr_err("Error creating sysfs entry for %s variable\n",
> + name);
> + rc = -EINVAL;
> + }
> +
> + } while ((status == OPAL_SUCCESS) && (rc == 0));
> +
> + kfree(name);
> + return rc;
> +}
> +
> +int secvar_sysfs_init(void)
> +{
> + powerpc_kobj = kobject_create_and_add("secvar", firmware_kobj);
> + if (!powerpc_kobj) {
> + pr_err("secvar: Failed to create firmware kobj\n");
> + return -ENODEV;
> + }
> +
> + secvar_kset = kset_create_and_add("vars", NULL, powerpc_kobj);
> + if (!secvar_kset) {
> + pr_err("secvar: sysfs kobject registration failed.\n");
You juat leaked a kobject :(
> + return -ENODEV;
> + }
> +
> + secvarops = get_secvar_ops();
> + if (!secvarops) {
> + kobject_put(powerpc_kobj);
> + pr_err("secvar: failed to retrieve secvar operations.\n");
> + return -ENODEV;
You just leaked 2 things from above :(
> + }
> +
> + secvar_sysfs_load();
> + pr_info("Secure variables sysfs initialized");
Do not be noisy when all goes just fine. The kernel log should be quiet
when all goes well.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v2 3/4] x86/efi: move common keyring handler functions to new file
From: Greg Kroah-Hartman @ 2019-08-21 16:30 UTC (permalink / raw)
To: Nayna Jain
Cc: linux-efi, Ard Biesheuvel, Eric Ricther, linux-kernel, Mimi Zohar,
Claudio Carvalho, Matthew Garret, linuxppc-dev, Paul Mackerras,
Jeremy Kerr, Elaine Palmer, Oliver O'Halloran,
linux-integrity, George Wilson
In-Reply-To: <1566400103-18201-4-git-send-email-nayna@linux.ibm.com>
On Wed, Aug 21, 2019 at 11:08:22AM -0400, Nayna Jain wrote:
> This patch moves the common code to keyring_handler.c
That says _what_ you are doing, but not _why_ you are doing it. We have
no idea :(
^ permalink raw reply
* Re: [PATCH v2 4/4] powerpc: load firmware trusted keys into kernel keyring
From: Greg Kroah-Hartman @ 2019-08-21 16:32 UTC (permalink / raw)
To: Nayna Jain
Cc: linux-efi, Ard Biesheuvel, Eric Ricther, linux-kernel, Mimi Zohar,
Claudio Carvalho, Matthew Garret, linuxppc-dev, Paul Mackerras,
Jeremy Kerr, Elaine Palmer, Oliver O'Halloran,
linux-integrity, George Wilson
In-Reply-To: <1566400103-18201-5-git-send-email-nayna@linux.ibm.com>
On Wed, Aug 21, 2019 at 11:08:23AM -0400, Nayna Jain wrote:
> The keys used to verify the Host OS kernel are managed by OPAL as secure
> variables. This patch loads the verification keys into the .platform
> keyring and revocation keys into .blacklist keyring. This enables
> verification and loading of the kernels signed by the boot time keys which
> are trusted by firmware.
>
> Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
> ---
> security/integrity/Kconfig | 9 ++
> security/integrity/Makefile | 3 +
> .../integrity/platform_certs/load_powerpc.c | 94 +++++++++++++++++++
> 3 files changed, 106 insertions(+)
> create mode 100644 security/integrity/platform_certs/load_powerpc.c
>
> diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig
> index 0bae6adb63a9..2b4109c157e2 100644
> --- a/security/integrity/Kconfig
> +++ b/security/integrity/Kconfig
> @@ -72,6 +72,15 @@ config LOAD_IPL_KEYS
> depends on S390
> def_bool y
>
> +config LOAD_PPC_KEYS
> + bool "Enable loading of platform and revocation keys for POWER"
> + depends on INTEGRITY_PLATFORM_KEYRING
> + depends on PPC_SECURE_BOOT
> + def_bool y
def_bool y only for things that the system will not boot if it is not
enabled because you added a new feature. Otherwise just do not set the
default.
> + help
> + Enable loading of db keys to the .platform keyring and dbx keys to
> + the .blacklist keyring for powerpc based platforms.
> +
> config INTEGRITY_AUDIT
> bool "Enables integrity auditing support "
> depends on AUDIT
> diff --git a/security/integrity/Makefile b/security/integrity/Makefile
> index 525bf1d6e0db..9eeb6b053de3 100644
> --- a/security/integrity/Makefile
> +++ b/security/integrity/Makefile
> @@ -14,6 +14,9 @@ integrity-$(CONFIG_LOAD_UEFI_KEYS) += platform_certs/efi_parser.o \
> platform_certs/load_uefi.o \
> platform_certs/keyring_handler.o
> integrity-$(CONFIG_LOAD_IPL_KEYS) += platform_certs/load_ipl_s390.o
> +integrity-$(CONFIG_LOAD_PPC_KEYS) += platform_certs/efi_parser.o \
> + platform_certs/load_powerpc.o \
> + platform_certs/keyring_handler.o
> $(obj)/load_uefi.o: KBUILD_CFLAGS += -fshort-wchar
> subdir-$(CONFIG_IMA) += ima
> diff --git a/security/integrity/platform_certs/load_powerpc.c b/security/integrity/platform_certs/load_powerpc.c
> new file mode 100644
> index 000000000000..f4d869171062
> --- /dev/null
> +++ b/security/integrity/platform_certs/load_powerpc.c
> @@ -0,0 +1,94 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2019 IBM Corporation
> + * Author: Nayna Jain <nayna@linux.ibm.com>
> + *
> + * load_powernv.c
That's not the name of this file :(
And the perfect example of why you NEVER have the name of the file in
the file itself, as it's not needed and easy to get wrong :)
thanks,
greg k-h
^ permalink raw reply
* Re: Regression fix for bpf in v5.3 (was Re: [RFC PATCH] bpf: handle 32-bit zext during constant blinding)
From: Naveen N. Rao @ 2019-08-21 19:12 UTC (permalink / raw)
To: Jiong Wang, Michael Ellerman
Cc: Daniel Borkmann, netdev, linux-kernel, bpf, linuxppc-dev,
Alexei Starovoitov
In-Reply-To: <87k1b6yeh1.fsf@netronome.com>
Jiong Wang wrote:
>
> Michael Ellerman writes:
>
>> "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> writes:
>>> Since BPF constant blinding is performed after the verifier pass, there
>>> are certain ALU32 instructions inserted which don't have a corresponding
>>> zext instruction inserted after. This is causing a kernel oops on
>>> powerpc and can be reproduced by running 'test_cgroup_storage' with
>>> bpf_jit_harden=2.
>>>
>>> Fix this by emitting BPF_ZEXT during constant blinding if
>>> prog->aux->verifier_zext is set.
>>>
>>> Fixes: a4b1d3c1ddf6cb ("bpf: verifier: insert zero extension according to analysis result")
>>> Reported-by: Michael Ellerman <mpe@ellerman.id.au>
>>> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
>>> ---
>>> This approach (the location where zext is being introduced below, in
>>> particular) works for powerpc, but I am not entirely sure if this is
>>> sufficient for other architectures as well. This is broken on v5.3-rc4.
>>
>> Any comment on this?
>
> Have commented on https://marc.info/?l=linux-netdev&m=156637836024743&w=2
>
> The fix looks correct to me on "BPF_LD | BPF_IMM | BPF_DW", but looks
> unnecessary on two other places. It would be great if you or Naveen could
> confirm it.
Jiong,
Thanks for the review. I can now see why the other two changes are not
necessary. I will post a follow-on patch.
Thanks!
- Naveen
^ permalink raw reply
* [PATCH] bpf: handle 32-bit zext during constant blinding
From: Naveen N. Rao @ 2019-08-21 19:23 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Jiong Wang
Cc: bpf, linuxppc-dev, linux-kernel, netdev
Since BPF constant blinding is performed after the verifier pass, the
ALU32 instructions inserted for doubleword immediate loads don't have a
corresponding zext instruction. This is causing a kernel oops on powerpc
and can be reproduced by running 'test_cgroup_storage' with
bpf_jit_harden=2.
Fix this by emitting BPF_ZEXT during constant blinding if
prog->aux->verifier_zext is set.
Fixes: a4b1d3c1ddf6cb ("bpf: verifier: insert zero extension according to analysis result")
Reported-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
Changes since RFC:
- Removed changes to ALU32 and JMP32 ops since those don't alter program
execution, and the verifier would have already accounted for them.
kernel/bpf/core.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 8191a7db2777..66088a9e9b9e 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -890,7 +890,8 @@ int bpf_jit_get_func_addr(const struct bpf_prog *prog,
static int bpf_jit_blind_insn(const struct bpf_insn *from,
const struct bpf_insn *aux,
- struct bpf_insn *to_buff)
+ struct bpf_insn *to_buff,
+ bool emit_zext)
{
struct bpf_insn *to = to_buff;
u32 imm_rnd = get_random_int();
@@ -1005,6 +1006,8 @@ static int bpf_jit_blind_insn(const struct bpf_insn *from,
case 0: /* Part 2 of BPF_LD | BPF_IMM | BPF_DW. */
*to++ = BPF_ALU32_IMM(BPF_MOV, BPF_REG_AX, imm_rnd ^ aux[0].imm);
*to++ = BPF_ALU32_IMM(BPF_XOR, BPF_REG_AX, imm_rnd);
+ if (emit_zext)
+ *to++ = BPF_ZEXT_REG(BPF_REG_AX);
*to++ = BPF_ALU64_REG(BPF_OR, aux[0].dst_reg, BPF_REG_AX);
break;
@@ -1088,7 +1091,8 @@ struct bpf_prog *bpf_jit_blind_constants(struct bpf_prog *prog)
insn[1].code == 0)
memcpy(aux, insn, sizeof(aux));
- rewritten = bpf_jit_blind_insn(insn, aux, insn_buff);
+ rewritten = bpf_jit_blind_insn(insn, aux, insn_buff,
+ clone->aux->verifier_zext);
if (!rewritten)
continue;
--
2.22.0
^ permalink raw reply related
* Re: [PATCHv3 1/3] dt-bindings: pci: layerscape-pci: add compatible strings "fsl,ls1028a-pcie"
From: Rob Herring @ 2019-08-21 19:26 UTC (permalink / raw)
To: Xiaowei Bao
Cc: mark.rutland, kstewart, linux-pci, shawn.lin, minghuan.Lian,
shawnguo, lorenzo.pieralisi, kishon, mingkai.hu, devicetree, arnd,
Hou Zhiqiang, robh+dt, bhelgaas, linux-arm-kernel, roy.zang,
Xiaowei Bao, gregkh, linux-kernel, leoyang.li, pombredanne,
linuxppc-dev
In-Reply-To: <20190806061553.19934-1-xiaowei.bao@nxp.com>
On Tue, 6 Aug 2019 14:15:51 +0800, Xiaowei Bao wrote:
> Add the PCIe compatible string for LS1028A
>
> Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com>
> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> ---
> v2:
> - no change.
> v3:
> - no change.
>
> Documentation/devicetree/bindings/pci/layerscape-pci.txt | 1 +
> 1 file changed, 1 insertion(+)
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* [PATCH] powerpc/8xx: drop unused self-modifying code alternative to FixupDAR.
From: Christophe Leroy @ 2019-08-21 20:00 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
The code which fixups the DAR on TLB errors for dbcX instructions
has a self-modifying code alternative that has never been used.
Drop it.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/kernel/head_8xx.S | 24 ------------------------
1 file changed, 24 deletions(-)
diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S
index b8ca5b43e587..19f583e18402 100644
--- a/arch/powerpc/kernel/head_8xx.S
+++ b/arch/powerpc/kernel/head_8xx.S
@@ -575,8 +575,6 @@ InstructionBreakpoint:
* by decoding the registers used by the dcbx instruction and adding them.
* DAR is set to the calculated address.
*/
- /* define if you don't want to use self modifying code */
-#define NO_SELF_MODIFYING_CODE
FixupDAR:/* Entry point for dcbx workaround. */
mtspr SPRN_M_TW, r10
/* fetch instruction from memory. */
@@ -640,27 +638,6 @@ FixupDAR:/* Entry point for dcbx workaround. */
rlwinm r10, r10,0,7,5 /* Clear store bit for buggy dcbst insn */
mtspr SPRN_DSISR, r10
142: /* continue, it was a dcbx, dcbi instruction. */
-#ifndef NO_SELF_MODIFYING_CODE
- andis. r10,r11,0x1f /* test if reg RA is r0 */
- li r10,modified_instr@l
- dcbtst r0,r10 /* touch for store */
- rlwinm r11,r11,0,0,20 /* Zero lower 10 bits */
- oris r11,r11,640 /* Transform instr. to a "add r10,RA,RB" */
- ori r11,r11,532
- stw r11,0(r10) /* store add/and instruction */
- dcbf 0,r10 /* flush new instr. to memory. */
- icbi 0,r10 /* invalidate instr. cache line */
- mfspr r11, SPRN_SPRG_SCRATCH1 /* restore r11 */
- mfspr r10, SPRN_SPRG_SCRATCH0 /* restore r10 */
- isync /* Wait until new instr is loaded from memory */
-modified_instr:
- .space 4 /* this is where the add instr. is stored */
- bne+ 143f
- subf r10,r0,r10 /* r10=r10-r0, only if reg RA is r0 */
-143: mtdar r10 /* store faulting EA in DAR */
- mfspr r10,SPRN_M_TW
- b DARFixed /* Go back to normal TLB handling */
-#else
mfctr r10
mtdar r10 /* save ctr reg in DAR */
rlwinm r10, r11, 24, 24, 28 /* offset into jump table for reg RB */
@@ -724,7 +701,6 @@ modified_instr:
add r10, r10, r11 /* add it */
mfctr r11 /* restore r11 */
b 151b
-#endif
/*
* This is where the main kernel code starts.
--
2.13.3
^ permalink raw reply related
* Re: [PATCH v4 1/7] kexec: add KEXEC_ELF
From: Thiago Jung Bauermann @ 2019-08-21 20:15 UTC (permalink / raw)
To: kexec; +Cc: linuxppc-dev, deller, Sven Schnelle
In-Reply-To: <87zhk3mj0k.fsf@morokweng.localdomain>
Thiago Jung Bauermann <bauerman@linux.ibm.com> writes:
>> diff --git a/arch/powerpc/kernel/kexec_elf_64.c b/kernel/kexec_elf.c
>> similarity index 71%
>> copy from arch/powerpc/kernel/kexec_elf_64.c
>> copy to kernel/kexec_elf.c
>> index ba4f18a43ee8..6e9f52171ede 100644
>> --- a/arch/powerpc/kernel/kexec_elf_64.c
>> +++ b/kernel/kexec_elf.c
>> @@ -1,33 +1,10 @@
>> -/*
>> - * Load ELF vmlinux file for the kexec_file_load syscall.
>> - *
>> - * Copyright (C) 2004 Adam Litke (agl@us.ibm.com)
>> - * Copyright (C) 2004 IBM Corp.
>> - * Copyright (C) 2005 R Sharada (sharada@in.ibm.com)
>> - * Copyright (C) 2006 Mohan Kumar M (mohan@in.ibm.com)
>> - * Copyright (C) 2016 IBM Corporation
>> - *
>> - * Based on kexec-tools' kexec-elf-exec.c and kexec-elf-ppc64.c.
>> - * Heavily modified for the kernel by
>> - * Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>.
>> - *
>> - * This program is free software; you can redistribute it and/or modify
>> - * it under the terms of the GNU General Public License as published by
>> - * the Free Software Foundation (version 2 of the License).
>> - *
>> - * This program is distributed in the hope that it will be useful,
>> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> - * GNU General Public License for more details.
>> - */
>> +// SPDX-License-Identifier: GPL-2.0-only
>
> I may be wrong, but my understanding of the SPDX license identifier is
> that it substitutes the license text (i.e., the last two paragraphs
> above), but not the copyright statements. Is it ok to have a file with a
> SPDX license identifier but no copyright statement?
Answering my own question: I just came accross commit b24413180f56
("License cleanup: add SPDX GPL-2.0 license identifier to files with no
license") which adds SPDX license identifiers to a lot of files without
any copyright statement so I conclude that it is indeed ok to not have
copyright statements in a file.
In this instance the new file is heavily based on the old one though, so
IMHO it makes sense for it to inherit the copyright statements from the
original file.
--
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply
* Re: [RFC PATCH] powerpc: Convert ____flush_dcache_icache_phys() to C
From: Christophe Leroy @ 2019-08-21 20:27 UTC (permalink / raw)
To: Alastair D'Silva, Segher Boessenkool; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <9887dada07278cb39051941d1a47d50349d9fde0.camel@au1.ibm.com>
Le 20/08/2019 à 06:36, Alastair D'Silva a écrit :
> On Fri, 2019-08-16 at 15:52 +0000, Christophe Leroy wrote:
[...]
>
>
> Thanks Christophe,
>
> I'm trying a somewhat different approach that requires less knowledge
> of assembler. Handling of CPU_FTR_COHERENT_ICACHE is outside this
> function. The code below is not a patch as my tree is a bit messy,
> sorry:
Can we be 100% sure that GCC won't add any code accessing some global
data or stack while the Data MMU is OFF ?
Christophe
>
> /**
> * flush_dcache_icache_phys() - Flush a page by it's physical address
> * @addr: the physical address of the page
> */
> static void flush_dcache_icache_phys(unsigned long addr)
> {
> register unsigned long msr;
> register unsigned long dlines = PAGE_SIZE >> l1_dcache_shift();
> register unsigned long dbytes = l1_dcache_bytes();
> register unsigned long ilines = PAGE_SIZE >> l1_icache_shift();
> register unsigned long ibytes = l1_icache_bytes();
> register unsigned long i;
> register unsigned long address = addr;
>
> /*
> * Clear the DR bit so that we operate on physical
> * rather than virtual addresses
> */
> msr = mfmsr();
> mtmsr(msr & ~(MSR_DR));
>
> /* Write out the data cache */
> for (i = 0; i < dlines; i++, address += dbytes)
> dcbst((void *)address);
>
> /* Invalidate the instruction cache */
> address = addr;
> for (i = 0; i < ilines; i++, address += ibytes)
> icbi((void *)address);
>
> mtmsr(msr);
> }
>
> void test_flush_phys(unsigned long addr)
> {
> flush_dcache_icache_phys(addr);
> }
>
>
> This gives the following assembler (using pmac32_defconfig):
> 000003cc <test_flush_phys>:
> 3cc: 94 21 ff f0 stwu r1,-16(r1)
> 3d0: 7d 00 00 a6 mfmsr r8
> 3d4: 55 09 07 34 rlwinm r9,r8,0,28,26
> 3d8: 7d 20 01 24 mtmsr r9
> 3dc: 39 20 00 80 li r9,128
> 3e0: 7d 29 03 a6 mtctr r9
> 3e4: 39 43 10 00 addi r10,r3,4096
> 3e8: 7c 69 1b 78 mr r9,r3
> 3ec: 7c 00 48 6c dcbst 0,r9
> 3f0: 39 29 00 20 addi r9,r9,32
> 3f4: 42 00 ff f8 bdnz 3ec <test_flush_phys+0x20>
> 3f8: 7c 00 1f ac icbi 0,r3
> 3fc: 38 63 00 20 addi r3,r3,32
> 400: 7f 8a 18 40 cmplw cr7,r10,r3
> 404: 40 9e ff f4 bne cr7,3f8 <test_flush_phys+0x2c>
> 408: 7d 00 01 24 mtmsr r8
> 40c: 38 21 00 10 addi r1,r1,16
> 410: 4e 80 00 20 blr
>
>
^ permalink raw reply
* Re: [PATCH] bpf: handle 32-bit zext during constant blinding
From: Jiong Wang @ 2019-08-21 21:51 UTC (permalink / raw)
To: Naveen N. Rao
Cc: Daniel Borkmann, Jiong Wang, linux-kernel, netdev, bpf,
linuxppc-dev, Alexei Starovoitov
In-Reply-To: <20190821192358.31922-1-naveen.n.rao@linux.vnet.ibm.com>
Naveen N. Rao writes:
> Since BPF constant blinding is performed after the verifier pass, the
> ALU32 instructions inserted for doubleword immediate loads don't have a
> corresponding zext instruction. This is causing a kernel oops on powerpc
> and can be reproduced by running 'test_cgroup_storage' with
> bpf_jit_harden=2.
>
> Fix this by emitting BPF_ZEXT during constant blinding if
> prog->aux->verifier_zext is set.
>
> Fixes: a4b1d3c1ddf6cb ("bpf: verifier: insert zero extension according to analysis result")
> Reported-by: Michael Ellerman <mpe@ellerman.id.au>
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
Thanks for the fix.
Reviewed-by: Jiong Wang <jiong.wang@netronome.com>
Just two other comments during review in case I am wrong on somewhere.
- Use verifier_zext instead of bpf_jit_needs_zext() seems better, even
though the latter could avoid extending function argument.
Because JIT back-ends look at verifier_zext, true means zext inserted
by verifier so JITs won't do the code-gen.
Use verifier_zext is sort of keeping JIT blinding the same behaviour
has verifier even though blinding doesn't belong to verifier, but for
such insn patching, it could be seen as a extension of verifier,
therefore use verifier_zext seems better than bpf_jit_needs_zext() to
me.
- JIT blinding is also escaping the HI32 randomization which happens
inside verifier, otherwise x86-64 regression should have caught this issue.
Regards,
Jiong
> ---
> Changes since RFC:
> - Removed changes to ALU32 and JMP32 ops since those don't alter program
> execution, and the verifier would have already accounted for them.
>
>
> kernel/bpf/core.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> index 8191a7db2777..66088a9e9b9e 100644
> --- a/kernel/bpf/core.c
> +++ b/kernel/bpf/core.c
> @@ -890,7 +890,8 @@ int bpf_jit_get_func_addr(const struct bpf_prog *prog,
>
> static int bpf_jit_blind_insn(const struct bpf_insn *from,
> const struct bpf_insn *aux,
> - struct bpf_insn *to_buff)
> + struct bpf_insn *to_buff,
> + bool emit_zext)
> {
> struct bpf_insn *to = to_buff;
> u32 imm_rnd = get_random_int();
> @@ -1005,6 +1006,8 @@ static int bpf_jit_blind_insn(const struct bpf_insn *from,
> case 0: /* Part 2 of BPF_LD | BPF_IMM | BPF_DW. */
> *to++ = BPF_ALU32_IMM(BPF_MOV, BPF_REG_AX, imm_rnd ^ aux[0].imm);
> *to++ = BPF_ALU32_IMM(BPF_XOR, BPF_REG_AX, imm_rnd);
> + if (emit_zext)
> + *to++ = BPF_ZEXT_REG(BPF_REG_AX);
> *to++ = BPF_ALU64_REG(BPF_OR, aux[0].dst_reg, BPF_REG_AX);
> break;
>
> @@ -1088,7 +1091,8 @@ struct bpf_prog *bpf_jit_blind_constants(struct bpf_prog *prog)
> insn[1].code == 0)
> memcpy(aux, insn, sizeof(aux));
>
> - rewritten = bpf_jit_blind_insn(insn, aux, insn_buff);
> + rewritten = bpf_jit_blind_insn(insn, aux, insn_buff,
> + clone->aux->verifier_zext);
> if (!rewritten)
> continue;
^ permalink raw reply
* Re: [PATCH v5 1/7] Documentation/powerpc: Ultravisor API
From: Claudio Carvalho @ 2019-08-21 22:01 UTC (permalink / raw)
To: Fabiano Rosas, linuxppc-dev
Cc: Madhavan Srinivasan, Sukadev Bhattiprolu, Michael Anderson,
Ram Pai, kvm-ppc, Bharata B Rao, Ryan Grimm, Ram Pai,
Sukadev Bhattiprolu, Guerney Hunt, Thiago Bauermann
In-Reply-To: <877e7i74ua.fsf@linux.ibm.com>
On 8/12/19 12:58 PM, Fabiano Rosas wrote:
> Claudio Carvalho <cclaudio@linux.ibm.com> writes:
>
> Some small suggestions below:
>
>> +
>> + * The privilege of a process is now determined by three MSR bits,
>> + MSR(S, HV, PR). In each of the tables below the modes are listed
>> + from least privilege to highest privilege. The higher privilege
>> + modes can access all the resources of the lower privilege modes.
>> +
>> + **Secure Mode MSR Settings**
>> +
>> + +---+---+---+---------------+
>> + | S | HV| PR|Privilege |
>> + +===+===+===+===============+
>> + | 1 | 0 | 1 | Problem |
>> + +---+---+---+---------------+
>> + | 1 | 0 | 0 | Privileged(OS)|
>> + +---+---+---+---------------+
>> + | 1 | 1 | 0 | Ultravisor |
>> + +---+---+---+---------------+
>> + | 1 | 1 | 1 | Reserved |
>> + +---+---+---+---------------+
>> +
>> + **Normal Mode MSR Settings**
>> +
>> + +---+---+---+---------------+
>> + | S | HV| PR|Privilege |
>> + +===+===+===+===============+
>> + | 0 | 0 | 1 | Problem |
>> + +---+---+---+---------------+
>> + | 0 | 0 | 0 | Privileged(OS)|
>> + +---+---+---+---------------+
>> + | 0 | 1 | 0 | Hypervisor |
>> + +---+---+---+---------------+
>> + | 0 | 1 | 1 | Problem (HV) |
>> + +---+---+---+---------------+
> I find the use of '(HV)' in this last line a bit ambiguous. Since we are
> already using 'HV' to refer to MSR(HV). I'd suggest using '(Host)' or
> simply leaving it out.
>
>> +
>> + * Memory is partitioned into secure and normal memory. Only processes
>> + that are running in secure mode can access secure memory.
>> +
>> + * The hardware does not allow anything that is not running secure to
>> + access secure memory. This means that the Hypervisor cannot access
>> + the memory of the SVM without using an ultracall (asking the
>> + Ultravisor). The Ultravisor will only allow the hypervisor to see
>> + the SVM memory encrypted.
>> +
>> + * I/O systems are not allowed to directly address secure memory. This
>> + limits the SVMs to virtual I/O only.
>> +
>> + * The architecture allows the SVM to share pages of memory with the
>> + hypervisor that are not protected with encryption. However, this
>> + sharing must be initiated by the SVM.
>> +
>> + * When a process is running in secure mode all hypercalls
>> + (syscall lev=1) are reflected to the Ultravisor.
> Here 'reflected' refers to the Ultravisor. Later on, it is used as
> meaning the Ultravisor reflects hypercalls/interrupts to the
> Hypervisor. I suggest we use this term to mean the latter only.
>
>> +
>> + * When a process is in secure mode all interrupts go to the
>> + Ultravisor.
>> +
>> + * The following resources have become Ultravisor privileged and
>> + require an Ultravisor interface to manipulate:
>> +
>> + * Processor configurations registers (SCOMs).
>> +
>> + * Stop state information.
>> +
>> + * The debug registers CIABR, DAWR, and DAWRX become Ultravisor
>> + resources when SMFCTRL(D) is set. If SMFCTRL(D) is not set they do
> It looks like you could go without "become Ultravisor resources" since
> it is already mentioned in the parent item above (The following...).
>
>> + not work in secure mode. When set, reading and writing requires
>> + an Ultravisor call, otherwise that will cause a Hypervisor Emulation
>> + Assistance interrupt.
>> +
>> + * PTCR and partition table entries (partition table is in secure
>> + memory). An attempt to write to PTCR will cause a Hypervisor
>> + Emulation Assitance interrupt.
>> +
>> + * LDBAR (LD Base Address Register) and IMC (In-Memory Collection)
>> + non-architected registers. An attempt to write to them will cause a
>> + Hypervisor Emulation Assistance interrupt.
>> +
>> + * Paging for an SVM, sharing of memory with Hypervisor for an SVM.
>> + (Including Virtual Processor Area (VPA) and virtual I/O).
>> +
>> +
>> +Software/Microcode
>> +==================
>> +
>> + The software changes include:
>> +
>> + * SVMs are created from normal VM using (open source) tooling supplied
>> + by IBM.
>> +
>> + * All SVMs start as normal VMs and utilize an ultracall, UV_ESM
>> + (Enter Secure Mode), to make the transition.
>> +
>> + * When the UV_ESM ultracall is made the Ultravisor copies the VM into
>> + secure memory, decrypts the verification information, and checks the
>> + integrity of the SVM. If the integrity check passes the Ultravisor
>> + passes control in secure mode.
>> +
>> + * For external interrupts the Ultravisor saves the state of the SVM,
>> + and reflects the interrupt to the hypervisor for processing.
>> + For hypercalls, the Ultravisor inserts neutral state into all
>> + registers not needed for the hypercall then reflects the call to
>> + the hypervisor for processing. The H_RANDOM hypercall is performed
>> + by the Ultravisor and not reflected.
>> +
>> + * The verification information includes the pass phrase for the
>> + encrypted disk associated with the SVM. This pass phrase is given
>> + to the SVM when requested.
> This is the second mention of 'verification information'. Could we
> perhaps move this up after "SMVs are created..."
> and rephrase it in a way
> that introduces the concept? What/where/in what state is the
> verification information?
Thanks for reviewing the patch Fabiano. I addressed all your suggestions in
the next version, except the one above because the "verification
information" concept is big and it is not related to the kernel.
Claudio.
>> +
>> + * The Ultravisor is not involved in protecting the encrypted disk of
>> + the SVM while at rest.
>> +
>> + * For virtual I/O to work bounce buffering must be done.
>> +
>> + * The Ultravisor uses AES (IAPM) for protection of SVM memory. IAPM
>> + is a mode of AES that provides integrity and secrecy concurrently.
>> +
>> + * The movement of data between normal and secure pages is coordinated
>> + with the Ultravisor by a new HMM plug-in in the Hypervisor.
>> +
>> + The Ultravisor offers new services to the hypervisor and SVMs. These
>> + are accessed through ultracalls.
>> +
>> +Terminology
>> +===========
>> +
>> + * Hypercalls: special system calls used to request services from
>> + Hypervisor.
>> +
>> + * Normal memory: Memory that is accessible to Hypervisor.
>> +
>> + * Normal page: Page backed by normal memory and available to
>> + Hypervisor.
>> +
>> + * Shared page: A page backed by normal memory and available to both
>> + the Hypervisor/QEMU and the SVM (i.e page has mappings in SVM and
>> + Hypervisor/QEMU).
>> +
>> + * Secure memory: Memory that is accessible only to Ultravisor and
>> + SVMs.
>> +
>> + * Secure page: Page backed by secure memory and only available to
>> + Ultravisor and SVM.
>> +
>> + * SVM: Secure Virtual Machine.
>> +
>> + * Ultracalls: special system calls used to request services from
>> + Ultravisor.
>> +
>> +
>> +Ultravisor calls API
>> +####################
>> +
>> + This section describes Ultravisor calls (ultracalls) needed to
>> + support Secure Virtual Machines (SVM)s and Paravirtalized KVM. The
> Paravirtualized
>
>> + ultracalls allow the SVMs and Hypervisor to request services from the
>> + Ultravisor such as accessing a register or memory region that can only
>> + be accessed when running in Ultravisor-privileged mode.
>> +
>> + The specific service needed from an ultracall is specified in register
>> + R3 (the first parameter to the ultracall). Other parameters to the
>> + ultracall, if any, are specified in registers R4 through R12.
>> +
>> + Return value of all ultracalls is in register R3. Other output values
>> + from the ultracall, if any, are returned in registers R4 through R12.
>> + The only exception to this register usage is the ``UV_RETURN``
>> + ultracall described below.
>> +
>> + Each ultracall returns specific error codes, applicable in the context
>> + of the ultracall. However, like with the PowerPC Architecture Platform
>> + Reference (PAPR), if no specific error code is defined for a
>> + particular situation, then the ultracall will fallback to an erroneous
>> + parameter-position based code. i.e U_PARAMETER, U_P2, U_P3 etc
>> + depending on the ultracall parameter that may have caused the error.
>> +
>> + Some ultracalls involve transferring a page of data between Ultravisor
>> + and Hypervisor. Secure pages that are transferred from secure memory
>> + to normal memory may be encrypted using dynamically generated keys.
>> + When the secure pages are transferred back to secure memory, they may
>> + be decrypted using the same dynamically generated keys. Generation and
>> + management of these keys will be covered in a separate document.
>> +
>> + For now this only covers ultracalls currently implemented and being
>> + used by Hypervisor and SVMs but others can be added here when it
>> + makes sense.
>> +
>> + The full specification for all hypercalls/ultracalls will eventually
>> + be made available in the public/OpenPower version of the PAPR
>> + specification.
>> +
>> + **Note**
>> +
>> + If PEF is not enabled, the ultracalls will be redirected to the
>> + Hypervisor which must handle/fail the calls.
>> +
>> +Ultracalls used by Hypervisor
>> +=============================
>> +
>> + This section describes the virtual memory management ultracalls used
>> + by the Hypervisor to manage SVMs.
>> +
>> +UV_PAGE_OUT
>> +-----------
>> +
>> + Encrypt and move the contents of a page from secure memory to normal
>> + memory.
>> +
>> +Syntax
>> +~~~~~~
>> +
>> +.. code-block:: c
>> +
>> + uint64_t ultracall(const uint64_t UV_PAGE_OUT,
>> + uint16_t lpid, /* LPAR ID */
>> + uint64_t dest_ra, /* real address of destination page */
>> + uint64_t src_gpa, /* source guest-physical-address */
>> + uint8_t flags, /* flags */
>> + uint64_t order) /* page size order */
>> +
>> +Return values
>> +~~~~~~~~~~~~~
>> +
>> + One of the following values:
>> +
>> + * U_SUCCESS on success.
>> + * U_PARAMETER if ``lpid`` is invalid.
>> + * U_P2 if ``dest_ra`` is invalid.
>> + * U_P3 if the ``src_gpa`` address is invalid.
>> + * U_P4 if any bit in the ``flags`` is unrecognized
>> + * U_P5 if the ``order`` parameter is unsupported.
>> + * U_FUNCTION if functionality is not supported.
>> + * U_BUSY if page cannot be currently paged-out.
>> +
>> +Description
>> +~~~~~~~~~~~
>> +
>> + Encrypt the contents of a secure-page and make it available to
>> + Hypervisor in a normal page.
>> +
>> + By default, the source page is unmapped from the SVM's partition-
>> + scoped page table. But the Hypervisor can provide a hint to the
>> + Ultravisor to retain the page mapping by setting the ``UV_SNAPSHOT``
>> + flag in ``flags`` parameter.
>> +
>> + If the source page is already a shared page the call returns
>> + U_SUCCESS, without doing anything.
>> +
>> +Use cases
>> +~~~~~~~~~
>> +
>> + #. QEMU attempts to access an address belonging to the SVM but the
>> + page frame for that address is not mapped into QEMU's address
>> + space. In this case, the Hypervisor will allocate a page frame,
>> + map it into QEMU's address space and issue the ``UV_PAGE_OUT``
>> + call to retrieve the encrypted contents of the page.
>> +
>> + #. When Ultravisor runs low on secure memory and it needs to page-out
>> + an LRU page. In this case, Ultravisor will issue the
>> + ``H_SVM_PAGE_OUT`` hypercall to the Hypervisor. The Hypervisor will
>> + then allocate a normal page and issue the ``UV_PAGE_OUT`` ultracall
>> + and the Ultravisor will encrypt and move the contents of the secure
>> + page into the normal page.
>> +
>> +
>> +UV_PAGE_IN
>> +----------
>> +
>> + Move the contents of a page from normal memory to secure memory.
>> +
>> +Syntax
>> +~~~~~~
>> +
>> +.. code-block:: c
>> +
>> + uint64_t ultracall(const uint64_t UV_PAGE_IN,
>> + uint16_t lpid, /* the LPAR ID */
>> + uint64_t src_ra, /* source real address of page */
>> + uint64_t dest_gpa, /* destination guest physical address */
>> + uint64_t flags, /* flags */
>> + uint64_t order) /* page size order */
>> +
>> +Return values
>> +~~~~~~~~~~~~~
>> +
>> + One of the following values:
>> +
>> + * U_SUCCESS on success.
>> + * U_BUSY if page cannot be currently paged-in.
>> + * U_FUNCTION if functionality is not supported
>> + * U_PARAMETER if ``lpid`` is invalid.
>> + * U_P2 if ``src_ra`` is invalid.
>> + * U_P3 if the ``dest_gpa`` address is invalid.
>> + * U_P4 if any bit in the ``flags`` is unrecognized
>> + * U_P5 if the ``order`` parameter is unsupported.
>> +
>> +Description
>> +~~~~~~~~~~~
>> +
>> + Move the contents of the page identified by ``src_ra`` from normal
>> + memory to secure memory and map it to the guest physical address
>> + ``dest_gpa``.
>> +
>> + If `dest_gpa` refers to a shared address, map the page into the
>> + partition-scoped page-table of the SVM. If `dest_gpa` is not shared,
>> + copy the contents of the page into the corresponding secure page.
>> + Depending on the context, decrypt the page before being copied.
>> +
>> + The caller provides the attributes of the page through the ``flags``
>> + parameter. Valid values for ``flags`` are:
>> +
>> + * CACHE_INHIBITED
>> + * CACHE_ENABLED
>> + * WRITE_PROTECTION
>> +
>> + The Hypervisor must pin the page in memory before making
>> + ``UV_PAGE_IN`` ultracall.
>> +
>> +Use cases
>> +~~~~~~~~~
>> +
>> + #. When a normal VM switches to secure mode, all its pages residing
>> + in normal memory, are moved into secure memory.
>> +
>> + #. When an SVM requests to share a page with Hypervisor the Hypervisor
>> + allocates a page and informs the Ultravisor.
>> +
>> + #. When an SVM accesses a secure page that has been paged-out,
>> + Ultravisor invokes the Hypervisor to locate the page. After
>> + locating the page, the Hypervisor uses UV_PAGE_IN to make the
>> + page available to Ultravisor.
>> +
>> + #. When Hypervisor accesses SVM data, the Hypervisor requests the
>> + Ultravisor to transfer the corresponding page into a insecure page,
>> + which the Hypervisor can access. The data in the normal page will
>> + be encrypted though.
> This looks like it should be under UV_PAGE_OUT instead.
>
^ permalink raw reply
* Re: [PATCH v5 1/7] Documentation/powerpc: Ultravisor API
From: Claudio Carvalho @ 2019-08-21 22:30 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev
Cc: Madhavan Srinivasan, Michael Anderson, Ram Pai, kvm-ppc,
Bharata B Rao, Ryan Grimm, Ram Pai, Sukadev Bhattiprolu,
Sukadev Bhattiprolu, Guerney Hunt, Thiago Bauermann
In-Reply-To: <87v9v6se14.fsf@concordia.ellerman.id.au>
On 8/9/19 9:45 AM, Michael Ellerman wrote:
> Claudio Carvalho <cclaudio@linux.ibm.com> writes:
>> From: Sukadev Bhattiprolu <sukadev@linux.ibm.com>
>>
>> POWER9 processor includes support for Protected Execution Facility (PEF).
>> Which POWER9? Please be more precise.
>>
>> It's public knowledge that some versions of Power9 don't have PEF (or
>> have it broken / fused off).
>>
>> People are going to try and test this on various chip revisions that are
>> out in the wild, we need to make it clear where it's expected to work
>> and where it's not.
Updated this part of the commit message to:
Protected Execution Facility (PEF) is an architectural change for
POWER 9 that enables Secure Virtual Machines (SVMs). When enabled,
PEF adds a new higher privileged mode, called Ultravisor mode, to POWER
architecture. Along with the new mode there is new firmware called the
Protected Execution Ultravisor (or Ultravisor for short).
POWER 9 DD2.3 chips (PVR=0x004e1203) or greater will be PEF-capable.
Attached documentation ...
>
>> Attached documentation provides an overview of PEF and defines the API
>> for various interfaces that must be implemented in the Ultravisor
>> firmware as well as in the KVM Hypervisor.
>>
>> Based on input from Mike Anderson, Thiago Bauermann, Claudio Carvalho,
>> Ben Herrenschmidt, Guerney Hunt, Paul Mackerras.
>>
>> Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.ibm.com>
>> Signed-off-by: Ram Pai <linuxram@linux.ibm.com>
>> Signed-off-by: Guerney Hunt <gdhh@linux.ibm.com>
>> Reviewed-by: Claudio Carvalho <cclaudio@linux.ibm.com>
>> Reviewed-by: Michael Anderson <andmike@linux.ibm.com>
>> Reviewed-by: Thiago Bauermann <bauerman@linux.ibm.com>
>> Signed-off-by: Claudio Carvalho <cclaudio@linux.ibm.com>
>> ---
>> Documentation/powerpc/ultravisor.rst | 1055 ++++++++++++++++++++++++++
>> 1 file changed, 1055 insertions(+)
>> create mode 100644 Documentation/powerpc/ultravisor.rst
>>
>> diff --git a/Documentation/powerpc/ultravisor.rst b/Documentation/powerpc/ultravisor.rst
>> new file mode 100644
>> index 000000000000..8d5246585b66
>> --- /dev/null
>> +++ b/Documentation/powerpc/ultravisor.rst
>> @@ -0,0 +1,1055 @@
>> +.. SPDX-License-Identifier: GPL-2.0
>> +.. _ultravisor:
>> +
>> +============================
>> +Protected Execution Facility
>> +============================
>> +
>> +.. contents::
>> + :depth: 3
>> +
>> +.. sectnum::
>> + :depth: 3
>> +
>> +Protected Execution Facility
>> +############################
>> +
>> + Protected Execution Facility (PEF) is an architectural change for
>> + POWER 9 that enables Secure Virtual Machines (SVMs). When enabled,
> Ditto here.
>
> Also you don't mention which ISA version PEF is (will be) documented in.
> Do we know? Or can we at least reference the RFC number so folks can
> find it.
>
> Otherwise this looks really good. I'll try and find time to proof read
> it thoroughly.
>
> cheers
>
>> + PEF adds a new higher privileged mode, called Ultravisor mode, to
>> + POWER architecture. Along with the new mode there is new firmware
>> + called the Protected Execution Ultravisor (or Ultravisor for short).
>> + Ultravisor mode is the highest privileged mode in POWER architecture.
Updated this part to:
Protected Execution Facility (PEF) is an architectural change for
POWER 9 that enables Secure Virtual Machines (SVMs). DD2.3 chips
(PVR=0x004e1203) or greater will be PEF-capable. A new ISA release
will include the PEF RFC02487 changes.
When enabled, PEF adds a new higher privileged mode, called Ultravisor
mode, to POWER architecture. Along with the new mode there is new
firmware called the Protected Execution Ultravisor (or Ultravisor
for short). Ultravisor mode is the highest privileged mode in POWER
architecture.
Followed by the original table below.
Thanks Michael
Claudio.
>> +
>> + +------------------+
>> + | Privilege States |
>> + +==================+
>> + | Problem |
>> + +------------------+
>> + | Supervisor |
>> + +------------------+
>> + | Hypervisor |
>> + +------------------+
>> + | Ultravisor |
>> + +------------------+
>> +
>> + PEF protects SVMs from the hypervisor, privileged users, and other
>> + VMs in the system. SVMs are protected while at rest and can only be
>> + executed by an authorized machine. All virtual machines utilize
>> + hypervisor services. The Ultravisor filters calls between the SVMs
>> + and the hypervisor to assure that information does not accidentally
>> + leak. All hypercalls except H_RANDOM are reflected to the hypervisor.
>> + H_RANDOM is not reflected to prevent the hypervisor from influencing
>> + random values in the SVM.
>> +
>> + To support this there is a refactoring of the ownership of resources
>> + in the CPU. Some of the resources which were previously hypervisor
>> + privileged are now ultravisor privileged.
>> +
>> +Hardware
>> +========
>> +
>> + The hardware changes include the following:
>> +
>> + * There is a new bit in the MSR that determines whether the current
>> + process is running in secure mode, MSR(S) bit 41. MSR(S)=1, process
>> + is in secure mode, MSR(s)=0 process is in normal mode.
>> +
>> + * The MSR(S) bit can only be set by the Ultravisor.
>> +
>> + * HRFID cannot be used to set the MSR(S) bit. If the hypervisor needs
>> + to return to a SVM it must use an ultracall. It can determine if
>> + the VM it is returning to is secure.
>> +
>> + * There is a new Ultravisor privileged register, SMFCTRL, which has an
>> + enable/disable bit SMFCTRL(E).
>> +
>> + * The privilege of a process is now determined by three MSR bits,
>> + MSR(S, HV, PR). In each of the tables below the modes are listed
>> + from least privilege to highest privilege. The higher privilege
>> + modes can access all the resources of the lower privilege modes.
>> +
>> + **Secure Mode MSR Settings**
>> +
>> + +---+---+---+---------------+
>> + | S | HV| PR|Privilege |
>> + +===+===+===+===============+
>> + | 1 | 0 | 1 | Problem |
>> + +---+---+---+---------------+
>> + | 1 | 0 | 0 | Privileged(OS)|
>> + +---+---+---+---------------+
>> + | 1 | 1 | 0 | Ultravisor |
>> + +---+---+---+---------------+
>> + | 1 | 1 | 1 | Reserved |
>> + +---+---+---+---------------+
>> +
>> + **Normal Mode MSR Settings**
>> +
>> + +---+---+---+---------------+
>> + | S | HV| PR|Privilege |
>> + +===+===+===+===============+
>> + | 0 | 0 | 1 | Problem |
>> + +---+---+---+---------------+
>> + | 0 | 0 | 0 | Privileged(OS)|
>> + +---+---+---+---------------+
>> + | 0 | 1 | 0 | Hypervisor |
>> + +---+---+---+---------------+
>> + | 0 | 1 | 1 | Problem (HV) |
>> + +---+---+---+---------------+
>> +
>> + * Memory is partitioned into secure and normal memory. Only processes
>> + that are running in secure mode can access secure memory.
>> +
>> + * The hardware does not allow anything that is not running secure to
>> + access secure memory. This means that the Hypervisor cannot access
>> + the memory of the SVM without using an ultracall (asking the
>> + Ultravisor). The Ultravisor will only allow the hypervisor to see
>> + the SVM memory encrypted.
>> +
>> + * I/O systems are not allowed to directly address secure memory. This
>> + limits the SVMs to virtual I/O only.
>> +
>> + * The architecture allows the SVM to share pages of memory with the
>> + hypervisor that are not protected with encryption. However, this
>> + sharing must be initiated by the SVM.
>> +
>> + * When a process is running in secure mode all hypercalls
>> + (syscall lev=1) are reflected to the Ultravisor.
>> +
>> + * When a process is in secure mode all interrupts go to the
>> + Ultravisor.
>> +
>> + * The following resources have become Ultravisor privileged and
>> + require an Ultravisor interface to manipulate:
>> +
>> + * Processor configurations registers (SCOMs).
>> +
>> + * Stop state information.
>> +
>> + * The debug registers CIABR, DAWR, and DAWRX become Ultravisor
>> + resources when SMFCTRL(D) is set. If SMFCTRL(D) is not set they do
>> + not work in secure mode. When set, reading and writing requires
>> + an Ultravisor call, otherwise that will cause a Hypervisor Emulation
>> + Assistance interrupt.
>> +
>> + * PTCR and partition table entries (partition table is in secure
>> + memory). An attempt to write to PTCR will cause a Hypervisor
>> + Emulation Assitance interrupt.
>> +
>> + * LDBAR (LD Base Address Register) and IMC (In-Memory Collection)
>> + non-architected registers. An attempt to write to them will cause a
>> + Hypervisor Emulation Assistance interrupt.
>> +
>> + * Paging for an SVM, sharing of memory with Hypervisor for an SVM.
>> + (Including Virtual Processor Area (VPA) and virtual I/O).
>> +
>> +
>> +Software/Microcode
>> +==================
>> +
>> + The software changes include:
>> +
>> + * SVMs are created from normal VM using (open source) tooling supplied
>> + by IBM.
>> +
>> + * All SVMs start as normal VMs and utilize an ultracall, UV_ESM
>> + (Enter Secure Mode), to make the transition.
>> +
>> + * When the UV_ESM ultracall is made the Ultravisor copies the VM into
>> + secure memory, decrypts the verification information, and checks the
>> + integrity of the SVM. If the integrity check passes the Ultravisor
>> + passes control in secure mode.
>> +
>> + * For external interrupts the Ultravisor saves the state of the SVM,
>> + and reflects the interrupt to the hypervisor for processing.
>> + For hypercalls, the Ultravisor inserts neutral state into all
>> + registers not needed for the hypercall then reflects the call to
>> + the hypervisor for processing. The H_RANDOM hypercall is performed
>> + by the Ultravisor and not reflected.
>> +
>> + * The verification information includes the pass phrase for the
>> + encrypted disk associated with the SVM. This pass phrase is given
>> + to the SVM when requested.
>> +
>> + * The Ultravisor is not involved in protecting the encrypted disk of
>> + the SVM while at rest.
>> +
>> + * For virtual I/O to work bounce buffering must be done.
>> +
>> + * The Ultravisor uses AES (IAPM) for protection of SVM memory. IAPM
>> + is a mode of AES that provides integrity and secrecy concurrently.
>> +
>> + * The movement of data between normal and secure pages is coordinated
>> + with the Ultravisor by a new HMM plug-in in the Hypervisor.
>> +
>> + The Ultravisor offers new services to the hypervisor and SVMs. These
>> + are accessed through ultracalls.
>> +
>> +Terminology
>> +===========
>> +
>> + * Hypercalls: special system calls used to request services from
>> + Hypervisor.
>> +
>> + * Normal memory: Memory that is accessible to Hypervisor.
>> +
>> + * Normal page: Page backed by normal memory and available to
>> + Hypervisor.
>> +
>> + * Shared page: A page backed by normal memory and available to both
>> + the Hypervisor/QEMU and the SVM (i.e page has mappings in SVM and
>> + Hypervisor/QEMU).
>> +
>> + * Secure memory: Memory that is accessible only to Ultravisor and
>> + SVMs.
>> +
>> + * Secure page: Page backed by secure memory and only available to
>> + Ultravisor and SVM.
>> +
>> + * SVM: Secure Virtual Machine.
>> +
>> + * Ultracalls: special system calls used to request services from
>> + Ultravisor.
>> +
>> +
>> +Ultravisor calls API
>> +####################
>> +
>> + This section describes Ultravisor calls (ultracalls) needed to
>> + support Secure Virtual Machines (SVM)s and Paravirtalized KVM. The
>> + ultracalls allow the SVMs and Hypervisor to request services from the
>> + Ultravisor such as accessing a register or memory region that can only
>> + be accessed when running in Ultravisor-privileged mode.
>> +
>> + The specific service needed from an ultracall is specified in register
>> + R3 (the first parameter to the ultracall). Other parameters to the
>> + ultracall, if any, are specified in registers R4 through R12.
>> +
>> + Return value of all ultracalls is in register R3. Other output values
>> + from the ultracall, if any, are returned in registers R4 through R12.
>> + The only exception to this register usage is the ``UV_RETURN``
>> + ultracall described below.
>> +
>> + Each ultracall returns specific error codes, applicable in the context
>> + of the ultracall. However, like with the PowerPC Architecture Platform
>> + Reference (PAPR), if no specific error code is defined for a
>> + particular situation, then the ultracall will fallback to an erroneous
>> + parameter-position based code. i.e U_PARAMETER, U_P2, U_P3 etc
>> + depending on the ultracall parameter that may have caused the error.
>> +
>> + Some ultracalls involve transferring a page of data between Ultravisor
>> + and Hypervisor. Secure pages that are transferred from secure memory
>> + to normal memory may be encrypted using dynamically generated keys.
>> + When the secure pages are transferred back to secure memory, they may
>> + be decrypted using the same dynamically generated keys. Generation and
>> + management of these keys will be covered in a separate document.
>> +
>> + For now this only covers ultracalls currently implemented and being
>> + used by Hypervisor and SVMs but others can be added here when it
>> + makes sense.
>> +
>> + The full specification for all hypercalls/ultracalls will eventually
>> + be made available in the public/OpenPower version of the PAPR
>> + specification.
>> +
>> + **Note**
>> +
>> + If PEF is not enabled, the ultracalls will be redirected to the
>> + Hypervisor which must handle/fail the calls.
>> +
>> +Ultracalls used by Hypervisor
>> +=============================
>> +
>> + This section describes the virtual memory management ultracalls used
>> + by the Hypervisor to manage SVMs.
>> +
>> +UV_PAGE_OUT
>> +-----------
>> +
>> + Encrypt and move the contents of a page from secure memory to normal
>> + memory.
>> +
>> +Syntax
>> +~~~~~~
>> +
>> +.. code-block:: c
>> +
>> + uint64_t ultracall(const uint64_t UV_PAGE_OUT,
>> + uint16_t lpid, /* LPAR ID */
>> + uint64_t dest_ra, /* real address of destination page */
>> + uint64_t src_gpa, /* source guest-physical-address */
>> + uint8_t flags, /* flags */
>> + uint64_t order) /* page size order */
>> +
>> +Return values
>> +~~~~~~~~~~~~~
>> +
>> + One of the following values:
>> +
>> + * U_SUCCESS on success.
>> + * U_PARAMETER if ``lpid`` is invalid.
>> + * U_P2 if ``dest_ra`` is invalid.
>> + * U_P3 if the ``src_gpa`` address is invalid.
>> + * U_P4 if any bit in the ``flags`` is unrecognized
>> + * U_P5 if the ``order`` parameter is unsupported.
>> + * U_FUNCTION if functionality is not supported.
>> + * U_BUSY if page cannot be currently paged-out.
>> +
>> +Description
>> +~~~~~~~~~~~
>> +
>> + Encrypt the contents of a secure-page and make it available to
>> + Hypervisor in a normal page.
>> +
>> + By default, the source page is unmapped from the SVM's partition-
>> + scoped page table. But the Hypervisor can provide a hint to the
>> + Ultravisor to retain the page mapping by setting the ``UV_SNAPSHOT``
>> + flag in ``flags`` parameter.
>> +
>> + If the source page is already a shared page the call returns
>> + U_SUCCESS, without doing anything.
>> +
>> +Use cases
>> +~~~~~~~~~
>> +
>> + #. QEMU attempts to access an address belonging to the SVM but the
>> + page frame for that address is not mapped into QEMU's address
>> + space. In this case, the Hypervisor will allocate a page frame,
>> + map it into QEMU's address space and issue the ``UV_PAGE_OUT``
>> + call to retrieve the encrypted contents of the page.
>> +
>> + #. When Ultravisor runs low on secure memory and it needs to page-out
>> + an LRU page. In this case, Ultravisor will issue the
>> + ``H_SVM_PAGE_OUT`` hypercall to the Hypervisor. The Hypervisor will
>> + then allocate a normal page and issue the ``UV_PAGE_OUT`` ultracall
>> + and the Ultravisor will encrypt and move the contents of the secure
>> + page into the normal page.
>> +
>> +
>> +UV_PAGE_IN
>> +----------
>> +
>> + Move the contents of a page from normal memory to secure memory.
>> +
>> +Syntax
>> +~~~~~~
>> +
>> +.. code-block:: c
>> +
>> + uint64_t ultracall(const uint64_t UV_PAGE_IN,
>> + uint16_t lpid, /* the LPAR ID */
>> + uint64_t src_ra, /* source real address of page */
>> + uint64_t dest_gpa, /* destination guest physical address */
>> + uint64_t flags, /* flags */
>> + uint64_t order) /* page size order */
>> +
>> +Return values
>> +~~~~~~~~~~~~~
>> +
>> + One of the following values:
>> +
>> + * U_SUCCESS on success.
>> + * U_BUSY if page cannot be currently paged-in.
>> + * U_FUNCTION if functionality is not supported
>> + * U_PARAMETER if ``lpid`` is invalid.
>> + * U_P2 if ``src_ra`` is invalid.
>> + * U_P3 if the ``dest_gpa`` address is invalid.
>> + * U_P4 if any bit in the ``flags`` is unrecognized
>> + * U_P5 if the ``order`` parameter is unsupported.
>> +
>> +Description
>> +~~~~~~~~~~~
>> +
>> + Move the contents of the page identified by ``src_ra`` from normal
>> + memory to secure memory and map it to the guest physical address
>> + ``dest_gpa``.
>> +
>> + If `dest_gpa` refers to a shared address, map the page into the
>> + partition-scoped page-table of the SVM. If `dest_gpa` is not shared,
>> + copy the contents of the page into the corresponding secure page.
>> + Depending on the context, decrypt the page before being copied.
>> +
>> + The caller provides the attributes of the page through the ``flags``
>> + parameter. Valid values for ``flags`` are:
>> +
>> + * CACHE_INHIBITED
>> + * CACHE_ENABLED
>> + * WRITE_PROTECTION
>> +
>> + The Hypervisor must pin the page in memory before making
>> + ``UV_PAGE_IN`` ultracall.
>> +
>> +Use cases
>> +~~~~~~~~~
>> +
>> + #. When a normal VM switches to secure mode, all its pages residing
>> + in normal memory, are moved into secure memory.
>> +
>> + #. When an SVM requests to share a page with Hypervisor the Hypervisor
>> + allocates a page and informs the Ultravisor.
>> +
>> + #. When an SVM accesses a secure page that has been paged-out,
>> + Ultravisor invokes the Hypervisor to locate the page. After
>> + locating the page, the Hypervisor uses UV_PAGE_IN to make the
>> + page available to Ultravisor.
>> +
>> + #. When Hypervisor accesses SVM data, the Hypervisor requests the
>> + Ultravisor to transfer the corresponding page into a insecure page,
>> + which the Hypervisor can access. The data in the normal page will
>> + be encrypted though.
>> +
>> +UV_PAGE_INVAL
>> +-------------
>> +
>> + Invalidate the Ultravisor mapping of a page.
>> +
>> +Syntax
>> +~~~~~~
>> +
>> +.. code-block:: c
>> +
>> + uint64_t ultracall(const uint64_t UV_PAGE_INVAL,
>> + uint16_t lpid, /* the LPAR ID */
>> + uint64_t guest_pa, /* destination guest-physical-address */
>> + uint64_t order) /* page size order */
>> +
>> +Return values
>> +~~~~~~~~~~~~~
>> +
>> + One of the following values:
>> +
>> + * U_SUCCESS on success.
>> + * U_PARAMETER if ``lpid`` is invalid.
>> + * U_P2 if ``guest_pa`` is invalid (or corresponds to a secure
>> + page mapping).
>> + * U_P3 if the ``order`` is invalid.
>> + * U_FUNCTION if functionality is not supported.
>> + * U_BUSY if page cannot be currently invalidated.
>> +
>> +Description
>> +~~~~~~~~~~~
>> +
>> + This ultracall informs Ultravisor that the page mapping in Hypervisor
>> + corresponding to the given guest physical address has been invalidated
>> + and that the Ultravisor should not access the page. If the specified
>> + ``guest_pa`` corresponds to a secure page, Ultravisor will ignore the
>> + attempt to invalidate the page and return U_P2.
>> +
>> +Use cases
>> +~~~~~~~~~
>> +
>> + #. When a shared page is unmapped from the QEMU's page table, possibly
>> + because it is paged-out to disk, Ultravisor needs to know that the
>> + page should not be accessed from its side too.
>> +
>> +
>> +UV_WRITE_PATE
>> +-------------
>> +
>> + Validate and write the partition table entry (PATE) for a given
>> + partition.
>> +
>> +Syntax
>> +~~~~~~
>> +
>> +.. code-block:: c
>> +
>> + uint64_t ultracall(const uint64_t UV_WRITE_PATE,
>> + uint32_t lpid, /* the LPAR ID */
>> + uint64_t dw0 /* the first double word to write */
>> + uint64_t dw1) /* the second double word to write */
>> +
>> +Return values
>> +~~~~~~~~~~~~~
>> +
>> + One of the following values:
>> +
>> + * U_SUCCESS on success.
>> + * U_BUSY if PATE cannot be currently written to.
>> + * U_FUNCTION if functionality is not supported.
>> + * U_PARAMETER if ``lpid`` is invalid.
>> + * U_P2 if ``dw0`` is invalid.
>> + * U_P3 if the ``dw1`` address is invalid.
>> + * U_PERMISSION if the Hypervisor is attempting to change the PATE
>> + of a secure virtual machine or if called from a
>> + context other than Hypervisor.
>> +
>> +Description
>> +~~~~~~~~~~~
>> +
>> + Validate and write a LPID and its partition-table-entry for the given
>> + LPID. If the LPID is already allocated and initialized, this call
>> + results in changing the partition table entry.
>> +
>> +Use cases
>> +~~~~~~~~~
>> +
>> + #. The Partition table resides in Secure memory and its entries,
>> + called PATE (Partition Table Entries), point to the partition-
>> + scoped page tables for the Hypervisor as well as each of the
>> + virtual machines (both secure and normal). The Hypervisor
>> + operates in partition 0 and its partition-scoped page tables
>> + reside in normal memory.
>> +
>> + #. This ultracall allows the Hypervisor to register the partition-
>> + scoped and process-scoped page table entries for the Hypervisor
>> + and other partitions (virtual machines) with the Ultravisor.
>> +
>> + #. If the value of the PATE for an existing partition (VM) changes,
>> + the TLB cache for the partition is flushed.
>> +
>> + #. The Hypervisor is responsible for allocating LPID. The LPID and
>> + its PATE entry are registered together. The Hypervisor manages
>> + the PATE entries for a normal VM and can change the PATE entry
>> + anytime. Ultravisor manages the PATE entries for an SVM and
>> + Hypervisor is not allowed to modify them.
>> +
>> +UV_RETURN
>> +---------
>> +
>> + Return control from the Hypervisor back to the Ultravisor after
>> + processing an hypercall or interrupt that was forwarded (aka
>> + *reflected*) to the Hypervisor.
>> +
>> +Syntax
>> +~~~~~~
>> +
>> +.. code-block:: c
>> +
>> + uint64_t ultracall(const uint64_t UV_RETURN)
>> +
>> +Return values
>> +~~~~~~~~~~~~~
>> +
>> + This call never returns to Hypervisor on success. It returns
>> + U_INVALID if ultracall is not made from a Hypervisor context.
>> +
>> +Description
>> +~~~~~~~~~~~
>> +
>> + When an SVM makes an hypercall or incurs some other exception, the
>> + Ultravisor usually forwards (aka *reflects*) the exceptions to the
>> + Hypervisor. After processing the exception, Hypervisor uses the
>> + ``UV_RETURN`` ultracall to return control back to the SVM.
>> +
>> + The expected register state on entry to this ultracall is:
>> +
>> + * Non-volatile registers are restored to their original values.
>> + * If returning from an hypercall, register R0 contains the return
>> + value (**unlike other ultracalls**) and, registers R4 through R12
>> + contain any output values of the hypercall.
>> + * R3 contains the ultracall number, i.e UV_RETURN.
>> + * If returning with a synthesized interrupt, R2 contains the
>> + synthesized interrupt number.
>> +
>> +Use cases
>> +~~~~~~~~~
>> +
>> + #. Ultravisor relies on the Hypervisor to provide several services to
>> + the SVM such as processing hypercall and other exceptions. After
>> + processing the exception, Hypervisor uses UV_RETURN to return
>> + control back to the Ultravisor.
>> +
>> + #. Hypervisor has to use this ultracall to return control to the SVM.
>> +
>> +
>> +UV_REGISTER_MEM_SLOT
>> +--------------------
>> +
>> + Register an SVM address-range with specified properties.
>> +
>> +Syntax
>> +~~~~~~
>> +
>> +.. code-block:: c
>> +
>> + uint64_t ultracall(const uint64_t UV_REGISTER_MEM_SLOT,
>> + uint64_t lpid, /* LPAR ID of the SVM */
>> + uint64_t start_gpa, /* start guest physical address */
>> + uint64_t size, /* size of address range in bytes */
>> + uint64_t flags /* reserved for future expansion */
>> + uint16_t slotid) /* slot identifier */
>> +
>> +Return values
>> +~~~~~~~~~~~~~
>> +
>> + One of the following values:
>> +
>> + * U_SUCCESS on success.
>> + * U_PARAMETER if ``lpid`` is invalid.
>> + * U_P2 if ``start_gpa`` is invalid.
>> + * U_P3 if ``size`` is invalid.
>> + * U_P4 if any bit in the ``flags`` is unrecognized.
>> + * U_P5 if the ``slotid`` parameter is unsupported.
>> + * U_PERMISSION if called from context other than Hypervisor.
>> + * U_FUNCTION if functionality is not supported.
>> +
>> +
>> +Description
>> +~~~~~~~~~~~
>> +
>> + Register a memory range for an SVM. The memory range starts at the
>> + guest physical address ``start_gpa`` and is ``size`` bytes long.
>> +
>> +Use cases
>> +~~~~~~~~~
>> +
>> +
>> + #. When a virtual machine goes secure, all the memory slots managed by
>> + the Hypervisor move into secure memory. The Hypervisor iterates
>> + through each of memory slots, and registers the slot with
>> + Ultravisor. Hypervisor may discard some slots such as those used
>> + for firmware (SLOF).
>> +
>> + #. When new memory is hot-plugged, a new memory slot gets registered.
>> +
>> +
>> +UV_UNREGISTER_MEM_SLOT
>> +----------------------
>> +
>> + Unregister an SVM address-range that was previously registered using
>> + UV_REGISTER_MEM_SLOT.
>> +
>> +Syntax
>> +~~~~~~
>> +
>> +.. code-block:: c
>> +
>> + uint64_t ultracall(const uint64_t UV_UNREGISTER_MEM_SLOT,
>> + uint64_t lpid, /* LPAR ID of the SVM */
>> + uint64_t slotid) /* reservation slotid */
>> +
>> +Return values
>> +~~~~~~~~~~~~~
>> +
>> + One of the following values:
>> +
>> + * U_SUCCESS on success.
>> + * U_FUNCTION if functionality is not supported.
>> + * U_PARAMETER if ``lpid`` is invalid.
>> + * U_P2 if ``slotid`` is invalid.
>> + * U_PERMISSION if called from context other than Hypervisor.
>> +
>> +Description
>> +~~~~~~~~~~~
>> +
>> + Release the memory slot identified by ``slotid`` and free any
>> + resources allocated towards the reservation.
>> +
>> +Use cases
>> +~~~~~~~~~
>> +
>> + #. Memory hot-remove.
>> +
>> +
>> +UV_SVM_TERMINATE
>> +----------------
>> +
>> + Terminate an SVM and release its resources.
>> +
>> +Syntax
>> +~~~~~~
>> +
>> +.. code-block:: c
>> +
>> + uint64_t ultracall(const uint64_t UV_SVM_TERMINATE,
>> + uint64_t lpid, /* LPAR ID of the SVM */)
>> +
>> +Return values
>> +~~~~~~~~~~~~~
>> +
>> + One of the following values:
>> +
>> + * U_SUCCESS on success.
>> + * U_FUNCTION if functionality is not supported.
>> + * U_PARAMETER if ``lpid`` is invalid.
>> + * U_INVALID if VM is not secure.
>> + * U_PERMISSION if not called from a Hypervisor context.
>> +
>> +Description
>> +~~~~~~~~~~~
>> +
>> + Terminate an SVM and release all its resources.
>> +
>> +Use cases
>> +~~~~~~~~~
>> +
>> + #. Called by Hypervisor when terminating an SVM.
>> +
>> +
>> +Ultracalls used by SVM
>> +======================
>> +
>> +UV_SHARE_PAGE
>> +-------------
>> +
>> + Share a set of guest physical pages with the Hypervisor.
>> +
>> +Syntax
>> +~~~~~~
>> +
>> +.. code-block:: c
>> +
>> + uint64_t ultracall(const uint64_t UV_SHARE_PAGE,
>> + uint64_t gfn, /* guest page frame number */
>> + uint64_t num) /* number of pages of size PAGE_SIZE */
>> +
>> +Return values
>> +~~~~~~~~~~~~~
>> +
>> + One of the following values:
>> +
>> + * U_SUCCESS on success.
>> + * U_FUNCTION if functionality is not supported.
>> + * U_INVALID if the VM is not secure.
>> + * U_PARAMETER if ``gfn`` is invalid.
>> + * U_P2 if ``num`` is invalid.
>> +
>> +Description
>> +~~~~~~~~~~~
>> +
>> + Share the ``num`` pages starting at guest physical frame number ``gfn``
>> + with the Hypervisor. Assume page size is PAGE_SIZE bytes. Zero the
>> + pages before returning.
>> +
>> + If the address is already backed by a secure page, unmap the page and
>> + back it with an insecure page, with the help of the Hypervisor. If it
>> + is not backed by any page yet, mark the PTE as insecure and back it
>> + with an insecure page when the address is accessed. If it is already
>> + backed by an insecure page, zero the page and return.
>> +
>> +Use cases
>> +~~~~~~~~~
>> +
>> + #. The Hypervisor cannot access the SVM pages since they are backed by
>> + secure pages. Hence an SVM must explicitly request Ultravisor for
>> + pages it can share with Hypervisor.
>> +
>> + #. Shared pages are needed to support virtio and Virtual Processor Area
>> + (VPA) in SVMs.
>> +
>> +
>> +UV_UNSHARE_PAGE
>> +---------------
>> +
>> + Restore a shared SVM page to its initial state.
>> +
>> +Syntax
>> +~~~~~~
>> +
>> +.. code-block:: c
>> +
>> + uint64_t ultracall(const uint64_t UV_UNSHARE_PAGE,
>> + uint64_t gfn, /* guest page frame number */
>> + uint73 num) /* number of pages of size PAGE_SIZE*/
>> +
>> +Return values
>> +~~~~~~~~~~~~~
>> +
>> + One of the following values:
>> +
>> + * U_SUCCESS on success.
>> + * U_FUNCTION if functionality is not supported.
>> + * U_INVALID if VM is not secure.
>> + * U_PARAMETER if ``gfn`` is invalid.
>> + * U_P2 if ``num`` is invalid.
>> +
>> +Description
>> +~~~~~~~~~~~
>> +
>> + Stop sharing ``num`` pages starting at ``gfn`` with the Hypervisor.
>> + Assume that the page size is PAGE_SIZE. Zero the pages before
>> + returning.
>> +
>> + If the address is already backed by an insecure page, unmap the page
>> + and back it with a secure page. Inform the Hypervisor to release
>> + reference to its shared page. If the address is not backed by a page
>> + yet, mark the PTE as secure and back it with a secure page when that
>> + address is accessed. If it is already backed by an secure page zero
>> + the page and return.
>> +
>> +Use cases
>> +~~~~~~~~~
>> +
>> + #. The SVM may decide to unshare a page from the Hypervisor.
>> +
>> +
>> +UV_UNSHARE_ALL_PAGES
>> +--------------------
>> +
>> + Unshare all pages the SVM has shared with Hypervisor.
>> +
>> +Syntax
>> +~~~~~~
>> +
>> +.. code-block:: c
>> +
>> + uint64_t ultracall(const uint64_t UV_UNSHARE_ALL_PAGES)
>> +
>> +Return values
>> +~~~~~~~~~~~~~
>> +
>> + One of the following values:
>> +
>> + * U_SUCCESS on success.
>> + * U_FUNCTION if functionality is not supported.
>> + * U_INVAL if VM is not secure.
>> +
>> +Description
>> +~~~~~~~~~~~
>> +
>> + Unshare all shared pages from the Hypervisor. All unshared pages are
>> + zeroed on return. Only pages explicitly shared by the SVM with the
>> + Hypervisor (using UV_SHARE_PAGE ultracall) are unshared. Ultravisor
>> + may internally share some pages with the Hypervisor without explicit
>> + request from the SVM. These pages will not be unshared by this
>> + ultracall.
>> +
>> +Use cases
>> +~~~~~~~~~
>> +
>> + #. This call is needed when ``kexec`` is used to boot a different
>> + kernel. It may also be needed during SVM reset.
>> +
>> +UV_ESM
>> +------
>> +
>> + Secure the virtual machine (*enter secure mode*).
>> +
>> +Syntax
>> +~~~~~~
>> +
>> +.. code-block:: c
>> +
>> + uint64_t ultracall(const uint64_t UV_ESM,
>> + uint64_t esm_blob_addr, /* location of the ESM blob */
>> + unint64_t fdt) /* Flattened device tree */
>> +
>> +Return values
>> +~~~~~~~~~~~~~
>> +
>> + One of the following values:
>> +
>> + * U_SUCCESS on success (including if VM is already secure).
>> + * U_FUNCTION if functionality is not supported.
>> + * U_INVALID if VM is not secure.
>> + * U_PARAMETER if ``esm_blob_addr`` is invalid.
>> + * U_P2 if ``fdt`` is invalid.
>> + * U_PERMISSION if any integrity checks fail.
>> + * U_RETRY insufficient memory to create SVM.
>> + * U_NO_KEY symmetric key unavailable.
>> +
>> +Description
>> +~~~~~~~~~~~
>> +
>> + Secure the virtual machine. On successful completion, return
>> + control to the virtual machine at the address specified in the
>> + ESM blob.
>> +
>> +Use cases
>> +~~~~~~~~~
>> +
>> + #. A normal virtual machine can choose to switch to a secure mode.
>> +
>> +Hypervisor Calls API
>> +####################
>> +
>> + This document describes the Hypervisor calls (hypercalls) that are
>> + needed to support the Ultravisor. Hypercalls are services provided by
>> + the Hypervisor to virtual machines and Ultravisor.
>> +
>> + Register usage for these hypercalls is identical to that of the other
>> + hypercalls defined in the Power Architecture Platform Reference (PAPR)
>> + document. i.e on input, register R3 identifies the specific service
>> + that is being requested and registers R4 through R11 contain
>> + additional parameters to the hypercall, if any. On output, register
>> + R3 contains the return value and registers R4 through R9 contain any
>> + other output values from the hypercall.
>> +
>> + This document only covers hypercalls currently implemented/planned
>> + for Ultravisor usage but others can be added here when it makes sense.
>> +
>> + The full specification for all hypercalls/ultracalls will eventually
>> + be made available in the public/OpenPower version of the PAPR
>> + specification.
>> +
>> +Hypervisor calls to support Ultravisor
>> +======================================
>> +
>> + Following are the set of hypercalls needed to support Ultravisor.
>> +
>> +H_SVM_INIT_START
>> +----------------
>> +
>> + Begin the process of converting a normal virtual machine into an SVM.
>> +
>> +Syntax
>> +~~~~~~
>> +
>> +.. code-block:: c
>> +
>> + uint64_t hypercall(const uint64_t H_SVM_INIT_START)
>> +
>> +Return values
>> +~~~~~~~~~~~~~
>> +
>> + One of the following values:
>> +
>> + * H_SUCCESS on success.
>> +
>> +Description
>> +~~~~~~~~~~~
>> +
>> + Initiate the process of securing a virtual machine. This involves
>> + coordinating with the Ultravisor, using ultracalls, to allocate
>> + resources in the Ultravisor for the new SVM, transferring the VM's
>> + pages from normal to secure memory etc. When the process is
>> + completed, Ultravisor issues the H_SVM_INIT_DONE hypercall.
>> +
>> +Use cases
>> +~~~~~~~~~
>> +
>> + #. Ultravisor uses this hypercall to inform Hypervisor that a VM
>> + has initiated the process of switching to secure mode.
>> +
>> +
>> +H_SVM_INIT_DONE
>> +---------------
>> +
>> + Complete the process of securing an SVM.
>> +
>> +Syntax
>> +~~~~~~
>> +
>> +.. code-block:: c
>> +
>> + uint64_t hypercall(const uint64_t H_SVM_INIT_DONE)
>> +
>> +Return values
>> +~~~~~~~~~~~~~
>> +
>> + One of the following values:
>> +
>> + * H_SUCCESS on success.
>> + * H_UNSUPPORTED if called from the wrong context (e.g.
>> + from an SVM or before an H_SVM_INIT_START
>> + hypercall).
>> +
>> +Description
>> +~~~~~~~~~~~
>> +
>> + Complete the process of securing a virtual machine. This call must
>> + be made after a prior call to ``H_SVM_INIT_START`` hypercall.
>> +
>> +Use cases
>> +~~~~~~~~~
>> +
>> + On successfully securing a virtual machine, the Ultravisor informs
>> + Hypervisor about it. Hypervisor can use this call to finish setting
>> + up its internal state for this virtual machine.
>> +
>> +
>> +H_SVM_PAGE_IN
>> +-------------
>> +
>> + Move the contents of a page from normal memory to secure memory.
>> +
>> +Syntax
>> +~~~~~~
>> +
>> +.. code-block:: c
>> +
>> + uint64_t hypercall(const uint64_t H_SVM_PAGE_IN,
>> + uint64_t guest_pa, /* guest-physical-address */
>> + uint64_t flags, /* flags */
>> + uint64_t order) /* page size order */
>> +
>> +Return values
>> +~~~~~~~~~~~~~
>> +
>> + One of the following values:
>> +
>> + * H_SUCCESS on success.
>> + * H_PARAMETER if ``guest_pa`` is invalid.
>> + * H_P2 if ``flags`` is invalid.
>> + * H_P3 if ``order`` of page is invalid.
>> +
>> +Description
>> +~~~~~~~~~~~
>> +
>> + Retrieve the content of the page, belonging to the VM at the specified
>> + guest physical address.
>> +
>> + Only valid value(s) in ``flags`` are:
>> +
>> + * H_PAGE_IN_SHARED which indicates that the page is to be shared
>> + with the Ultravisor.
>> +
>> + * H_PAGE_IN_NONSHARED indicates that the UV is not anymore
>> + interested in the page. Applicable if the page is a shared page.
>> +
>> + The ``order`` parameter must correspond to the configured page size.
>> +
>> +Use cases
>> +~~~~~~~~~
>> +
>> + #. When a normal VM becomes a secure VM (using the UV_ESM ultracall),
>> + the Ultravisor uses this hypercall to move contents of each page of
>> + the VM from normal memory to secure memory.
>> +
>> + #. Ultravisor uses this hypercall to ask Hypervisor to provide a page
>> + in normal memory that can be shared between the SVM and Hypervisor.
>> +
>> + #. Ultravisor uses this hypercall to page-in a paged-out page. This
>> + can happen when the SVM touches a paged-out page.
>> +
>> + #. If SVM wants to disable sharing of pages with Hypervisor, it can
>> + inform Ultravisor to do so. Ultravisor will then use this hypercall
>> + and inform Hypervisor that it has released access to the normal
>> + page.
>> +
>> +H_SVM_PAGE_OUT
>> +---------------
>> +
>> + Move the contents of the page to normal memory.
>> +
>> +Syntax
>> +~~~~~~
>> +
>> +.. code-block:: c
>> +
>> + uint64_t hypercall(const uint64_t H_SVM_PAGE_OUT,
>> + uint64_t guest_pa, /* guest-physical-address */
>> + uint64_t flags, /* flags (currently none) */
>> + uint64_t order) /* page size order */
>> +
>> +Return values
>> +~~~~~~~~~~~~~
>> +
>> + One of the following values:
>> +
>> + * H_SUCCESS on success.
>> + * H_PARAMETER if ``guest_pa`` is invalid.
>> + * H_P2 if ``flags`` is invalid.
>> + * H_P3 if ``order`` is invalid.
>> +
>> +Description
>> +~~~~~~~~~~~
>> +
>> + Move the contents of the page identified by ``guest_pa`` to normal
>> + memory.
>> +
>> + Currently ``flags`` is unused and must be set to 0. The ``order``
>> + parameter must correspond to the configured page size.
>> +
>> +Use cases
>> +~~~~~~~~~
>> +
>> + #. If Ultravisor is running low on secure pages, it can move the
>> + contents of some secure pages, into normal pages using this
>> + hypercall. The content will be encrypted.
>> +
>> +References
>> +##########
>> +
>> +.. [1] `Supporting Protected Computing on IBM Power Architecture <https://developer.ibm.com/articles/l-support-protected-computing/>`_
>> --
>> 2.20.1
^ permalink raw reply
* RE: [RFC PATCH] powerpc: Convert ____flush_dcache_icache_phys() to C
From: Alastair D'Silva @ 2019-08-22 0:27 UTC (permalink / raw)
To: Christophe Leroy, Segher Boessenkool, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <a0ad8dd8-2f5d-256d-9e88-e9c236335bb8@c-s.fr>
On Wed, 2019-08-21 at 22:27 +0200, Christophe Leroy wrote:
>
> Le 20/08/2019 à 06:36, Alastair D'Silva a écrit :
> > On Fri, 2019-08-16 at 15:52 +0000, Christophe Leroy wrote:
>
> [...]
>
> >
> > Thanks Christophe,
> >
> > I'm trying a somewhat different approach that requires less
> > knowledge
> > of assembler. Handling of CPU_FTR_COHERENT_ICACHE is outside this
> > function. The code below is not a patch as my tree is a bit messy,
> > sorry:
>
> Can we be 100% sure that GCC won't add any code accessing some
> global
> data or stack while the Data MMU is OFF ?
>
> Christophe
>
+mpe
I'm not sure how we would go about making such a guarantee, but I've
tied every variable used to a register and addr is passed in a
register, so there is no stack usage, and every call in there only
operates on it's operands.
The calls to the inline cache helpers (for the PPC32 case) are all
constants, so I can't see a reasonable scenario where there would be a
function call and reordered to after the DR bit is turned off, but I
guess if we want to be paranoid, we could always add an mb() call
before the DR bit is manipulated to prevent the compiler from
reordering across the section where the data MMU is disabled.
>
> > /**
> > * flush_dcache_icache_phys() - Flush a page by it's physical
> > address
> > * @addr: the physical address of the page
> > */
> > static void flush_dcache_icache_phys(unsigned long addr)
> > {
> > register unsigned long msr;
> > register unsigned long dlines = PAGE_SIZE >> l1_dcache_shift();
> > register unsigned long dbytes = l1_dcache_bytes();
> > register unsigned long ilines = PAGE_SIZE >> l1_icache_shift();
> > register unsigned long ibytes = l1_icache_bytes();
> > register unsigned long i;
> > register unsigned long address = addr;
> >
> > /*
> > * Clear the DR bit so that we operate on physical
> > * rather than virtual addresses
> > */
> > msr = mfmsr();
> > mtmsr(msr & ~(MSR_DR));
> >
> > /* Write out the data cache */
> > for (i = 0; i < dlines; i++, address += dbytes)
> > dcbst((void *)address);
> >
> > /* Invalidate the instruction cache */
> > address = addr;
> > for (i = 0; i < ilines; i++, address += ibytes)
> > icbi((void *)address);
> >
> > mtmsr(msr);
> > }
> >
> > void test_flush_phys(unsigned long addr)
> > {
> > flush_dcache_icache_phys(addr);
> > }
> >
> >
> > This gives the following assembler (using pmac32_defconfig):
> > 000003cc <test_flush_phys>:
> > 3cc: 94 21 ff f0 stwu r1,-16(r1)
> > 3d0: 7d 00 00 a6 mfmsr r8
> > 3d4: 55 09 07 34 rlwinm r9,r8,0,28,26
> > 3d8: 7d 20 01 24 mtmsr r9
> > 3dc: 39 20 00 80 li r9,128
> > 3e0: 7d 29 03 a6 mtctr r9
> > 3e4: 39 43 10 00 addi r10,r3,4096
> > 3e8: 7c 69 1b 78 mr r9,r3
> > 3ec: 7c 00 48 6c dcbst 0,r9
> > 3f0: 39 29 00 20 addi r9,r9,32
> > 3f4: 42 00 ff f8 bdnz 3ec <test_flush_phys+0x20>
> > 3f8: 7c 00 1f ac icbi 0,r3
> > 3fc: 38 63 00 20 addi r3,r3,32
> > 400: 7f 8a 18 40 cmplw cr7,r10,r3
> > 404: 40 9e ff f4 bne cr7,3f8 <test_flush_phys+0x2c>
> > 408: 7d 00 01 24 mtmsr r8
> > 40c: 38 21 00 10 addi r1,r1,16
> > 410: 4e 80 00 20 blr
> >
> >
--
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819
^ permalink raw reply
* [PATCH] powerpc/64s/exception: most SRR type interrupts need only test KVM for PR-KVM
From: Nicholas Piggin @ 2019-08-22 0:48 UTC (permalink / raw)
To: linuxppc-dev; +Cc: kvm-ppc, Nicholas Piggin
Apart from SRESET, MCE, and syscall (hcall variant), SRR (EXC_STD) type
interrupts are not escalated to hypervisor mode, so delivered to the OS.
When running PR-KVM, the OS is the hypervisor, and the guest runs with
MSR[PR]=1, so these interrupts must test if a guest was running when
interrupted. These tests are done at the real-mode entry points only
because PR-KVM runs with LPCR[AIL]=0, so no virt-mode interrupt entry.
In HV KVM and nested HV KVM, the guest always receives these interrupts,
so there is no need for the host to make this test.
Therefore, remove these tests if PR KVM is not configured.
Improve the KVM interrupt comments, which explains this change and other
KVm interrupt delivery issues.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kernel/exceptions-64s.S | 76 ++++++++++++++++++++++------
1 file changed, 61 insertions(+), 15 deletions(-)
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index 2963b46f9580..b4d39330789d 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -215,23 +215,37 @@ do_define_int n
#ifdef CONFIG_KVM_BOOK3S_64_HANDLER
#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
/*
- * All interrupts which set HSRR registers (EXC_HV) as well as SRESET and
- * MCE and syscall when invoked with "sc 1" switch to the hypervisor to
- * be taken, so all generally need to test whether they were taken in guest
+ * All interrupts which set HSRR registers (EXC_HV) as well as SRESET and MCE
+ * and syscall when invoked with "sc 1" switch to MSR[HV]=1 (HVMODE) to be
+ * taken, so they all generally need to test whether they were taken in guest
* context.
*
- * SRESET and MCE may also be sent to the guest by the hypervisor.
- *
- * Interrupts which set SRR registers except SRESET and MCE and sc 1 are not
- * elevated to the hypervisor, though many can be taken when running in
- * hypervisor mode (e.g., bare metal kernel and userspace). These generally
- * need to test whether taken in guest context for PR KVM guests. PR KVM
- * does not enable AIL interrupts, so always takes them in real mode, which
- * is why these generally only need test the real-mode case.
- *
- * If hv is possible, interrupts come into to the hv version
- * of the kvmppc_interrupt code, which then jumps to the PR handler,
- * kvmppc_interrupt_pr, if the guest is a PR guest.
+ * Note: SRESET and MCE may also be sent to the guest by the hypervisor, and be
+ * taken with MSR[HV]=0.
+ *
+ * Interrupts which set SRR registers (with the above exceptions) do not
+ * elevate to MSR[HV]=1 mode, though most can be taken when running with
+ * MSR[HV]=1 (e.g., bare metal kernel and userspace). So these interrupts do
+ * not need to test whether a guest is running because they get delivered to
+ * the guest directly, including nested HV KVM guests.
+ *
+ * The exception is PR KVM, where the guest runs with MSR[PR]=1 and the host
+ * runs with MSR[HV]=0, so the host takes all interrupts on behalf of the
+ * guest. PR KVM runs with LPCR[AIL]=0 which causes interrupts to always be
+ * delivered to the real-mode entry point, therefore such interrupts only test
+ * KVM in their real mode handlers, and only when PR KVM is possible.
+ *
+ * Interrupts that are taken in MSR[HV]=0 and escalate to MSR[HV]=1 are always
+ * delivered in real-mode when the MMU is in hash mode because the MMU
+ * registers are not set appropriately to translate host addresses. In nested
+ * radix mode these can be delivered in virt-mode as the host translations are
+ * used implicitly (see: effective LPID, effective PID).
+ */
+
+/*
+ * If an interrupt is taken while a guest is running, it is immediately routed
+ * to KVM to handle. If both HV and PR KVM arepossible, KVM interrupts go first
+ * to kvmppc_interrupt_hv, which handles the PR guest case.
*/
#define kvmppc_interrupt kvmppc_interrupt_hv
#else
@@ -1277,8 +1291,10 @@ INT_DEFINE_BEGIN(data_access)
IAREA=PACA_EXGEN
IDAR=1
IDSISR=1
+#ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE
IKVM_SKIP=1
IKVM_REAL=1
+#endif
INT_DEFINE_END(data_access)
EXC_REAL_BEGIN(data_access, 0x300, 0x80)
@@ -1326,8 +1342,10 @@ INT_DEFINE_BEGIN(data_access_slb)
IAREA=PACA_EXSLB
IRECONCILE=0
IDAR=1
+#ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE
IKVM_SKIP=1
IKVM_REAL=1
+#endif
INT_DEFINE_END(data_access_slb)
EXC_REAL_BEGIN(data_access_slb, 0x380, 0x80)
@@ -1379,7 +1397,9 @@ INT_DEFINE_BEGIN(instruction_access)
IISIDE=1
IDAR=1
IDSISR=1
+#ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE
IKVM_REAL=1
+#endif
INT_DEFINE_END(instruction_access)
EXC_REAL_BEGIN(instruction_access, 0x400, 0x80)
@@ -1419,7 +1439,9 @@ INT_DEFINE_BEGIN(instruction_access_slb)
IRECONCILE=0
IISIDE=1
IDAR=1
+#ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE
IKVM_REAL=1
+#endif
INT_DEFINE_END(instruction_access_slb)
EXC_REAL_BEGIN(instruction_access_slb, 0x480, 0x80)
@@ -1514,7 +1536,9 @@ INT_DEFINE_BEGIN(alignment)
IAREA=PACA_EXGEN
IDAR=1
IDSISR=1
+#ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE
IKVM_REAL=1
+#endif
INT_DEFINE_END(alignment)
EXC_REAL_BEGIN(alignment, 0x600, 0x100)
@@ -1546,7 +1570,9 @@ INT_DEFINE_BEGIN(program_check)
IVEC=0x700
IHSRR=EXC_STD
IAREA=PACA_EXGEN
+#ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE
IKVM_REAL=1
+#endif
INT_DEFINE_END(program_check)
EXC_REAL_BEGIN(program_check, 0x700, 0x100)
@@ -1611,7 +1637,9 @@ INT_DEFINE_BEGIN(fp_unavailable)
IHSRR=EXC_STD
IAREA=PACA_EXGEN
IRECONCILE=0
+#ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE
IKVM_REAL=1
+#endif
INT_DEFINE_END(fp_unavailable)
EXC_REAL_BEGIN(fp_unavailable, 0x800, 0x100)
@@ -1674,7 +1702,9 @@ INT_DEFINE_BEGIN(decrementer)
IHSRR=EXC_STD
IAREA=PACA_EXGEN
IMASK=IRQS_DISABLED
+#ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE
IKVM_REAL=1
+#endif
INT_DEFINE_END(decrementer)
EXC_REAL_BEGIN(decrementer, 0x900, 0x80)
@@ -1762,7 +1792,9 @@ INT_DEFINE_BEGIN(doorbell_super)
IHSRR=EXC_STD
IAREA=PACA_EXGEN
IMASK=IRQS_DISABLED
+#ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE
IKVM_REAL=1
+#endif
INT_DEFINE_END(doorbell_super)
EXC_REAL_BEGIN(doorbell_super, 0xa00, 0x100)
@@ -1960,7 +1992,9 @@ INT_DEFINE_BEGIN(single_step)
IVEC=0xd00
IHSRR=EXC_STD
IAREA=PACA_EXGEN
+#ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE
IKVM_REAL=1
+#endif
INT_DEFINE_END(single_step)
EXC_REAL_BEGIN(single_step, 0xd00, 0x100)
@@ -2260,7 +2294,9 @@ INT_DEFINE_BEGIN(performance_monitor)
IHSRR=EXC_STD
IAREA=PACA_EXGEN
IMASK=IRQS_PMI_DISABLED
+#ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE
IKVM_REAL=1
+#endif
INT_DEFINE_END(performance_monitor)
EXC_REAL_BEGIN(performance_monitor, 0xf00, 0x20)
@@ -2291,7 +2327,9 @@ INT_DEFINE_BEGIN(altivec_unavailable)
IHSRR=EXC_STD
IAREA=PACA_EXGEN
IRECONCILE=0
+#ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE
IKVM_REAL=1
+#endif
INT_DEFINE_END(altivec_unavailable)
EXC_REAL_BEGIN(altivec_unavailable, 0xf20, 0x20)
@@ -2347,7 +2385,9 @@ INT_DEFINE_BEGIN(vsx_unavailable)
IHSRR=EXC_STD
IAREA=PACA_EXGEN
IRECONCILE=0
+#ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE
IKVM_REAL=1
+#endif
INT_DEFINE_END(vsx_unavailable)
EXC_REAL_BEGIN(vsx_unavailable, 0xf40, 0x20)
@@ -2402,7 +2442,9 @@ INT_DEFINE_BEGIN(facility_unavailable)
IVEC=0xf60
IHSRR=EXC_STD
IAREA=PACA_EXGEN
+#ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE
IKVM_REAL=1
+#endif
INT_DEFINE_END(facility_unavailable)
EXC_REAL_BEGIN(facility_unavailable, 0xf60, 0x20)
@@ -2496,8 +2538,10 @@ INT_DEFINE_BEGIN(instruction_breakpoint)
IVEC=0x1300
IHSRR=EXC_STD
IAREA=PACA_EXGEN
+#ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE
IKVM_SKIP=1
IKVM_REAL=1
+#endif
INT_DEFINE_END(instruction_breakpoint)
EXC_REAL_BEGIN(instruction_breakpoint, 0x1300, 0x100)
@@ -2672,7 +2716,9 @@ INT_DEFINE_BEGIN(altivec_assist)
IVEC=0x1700
IHSRR=EXC_STD
IAREA=PACA_EXGEN
+#ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE
IKVM_REAL=1
+#endif
INT_DEFINE_END(altivec_assist)
EXC_REAL_BEGIN(altivec_assist, 0x1700, 0x100)
--
2.22.0
^ permalink raw reply related
* Re: [PATCH v5 2/7] powerpc/kernel: Add ucall_norets() ultravisor call handler
From: Claudio Carvalho @ 2019-08-22 1:24 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev
Cc: Madhavan Srinivasan, Michael Anderson, Ram Pai, kvm-ppc,
Bharata B Rao, Ryan Grimm, Sukadev Bhattiprolu, Guerney Hunt,
Thiago Bauermann
In-Reply-To: <87wofgqb2g.fsf@concordia.ellerman.id.au>
On 8/14/19 7:46 AM, Michael Ellerman wrote:
> Claudio Carvalho <cclaudio@linux.ibm.com> writes:
>> diff --git a/arch/powerpc/kernel/ucall.S b/arch/powerpc/kernel/ucall.S
>> new file mode 100644
>> index 000000000000..de9133e45d21
>> --- /dev/null
>> +++ b/arch/powerpc/kernel/ucall.S
>> @@ -0,0 +1,20 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * Generic code to perform an ultravisor call.
>> + *
>> + * Copyright 2019, IBM Corporation.
>> + *
>> + */
>> +#include <asm/ppc_asm.h>
>> +#include <asm/export.h>
>> +
>> +_GLOBAL(ucall_norets)
>> +EXPORT_SYMBOL_GPL(ucall_norets)
>> + mfcr r0
>> + stw r0,8(r1)
>> +
>> + sc 2 /* Invoke the ultravisor */
>> +
>> + lwz r0,8(r1)
>> + mtcrf 0xff,r0
>> + blr /* Return r3 = status */
> Paulus points that we shouldn't need to save CR here. Our caller will
> have already saved it if it needed to, and we don't use CR in this
> function so we don't need to save it.
Dropped the CR save/restore in the next patchset version:
_GLOBAL(ucall_norets)
EXPORT_SYMBOL_GPL(ucall_norets)
sc 2 /* Invoke the ultravisor */
blr /* Return r3 = status */
Thanks,
Claudio
>
> That's assuming the Ultravisor follows the hcall ABI in which CR2-4 are
> non-volatile (PAPR § 14.5.3).
>
> I know plpar_hcall_norets() does save CR, but it shouldn't need to, that
> seems to be historical. aka. no one knows why it does it but it always
> has.
>
> cheers
>
^ permalink raw reply
* Re: [PATCH v5 2/7] powerpc/kernel: Add ucall_norets() ultravisor call handler
From: Claudio Carvalho @ 2019-08-22 1:26 UTC (permalink / raw)
To: Segher Boessenkool, Michael Ellerman
Cc: Madhavan Srinivasan, Michael Anderson, Ram Pai, kvm-ppc,
Bharata B Rao, linuxppc-dev, Ryan Grimm, Sukadev Bhattiprolu,
Guerney Hunt, Thiago Bauermann
In-Reply-To: <20190814183432.GG31406@gate.crashing.org>
On 8/14/19 3:34 PM, Segher Boessenkool wrote:
> On Wed, Aug 14, 2019 at 08:46:15PM +1000, Michael Ellerman wrote:
>> Claudio Carvalho <cclaudio@linux.ibm.com> writes:
>>> +_GLOBAL(ucall_norets)
>>> +EXPORT_SYMBOL_GPL(ucall_norets)
>>> + mfcr r0
>>> + stw r0,8(r1)
>>> +
>>> + sc 2 /* Invoke the ultravisor */
>>> +
>>> + lwz r0,8(r1)
>>> + mtcrf 0xff,r0
>>> + blr /* Return r3 = status */
>> Paulus points that we shouldn't need to save CR here. Our caller will
>> have already saved it if it needed to, and we don't use CR in this
>> function so we don't need to save it.
>>
>> That's assuming the Ultravisor follows the hcall ABI in which CR2-4 are
>> non-volatile (PAPR § 14.5.3).
> And assuming the ultravisor already clears (or sets, or whatever) all CR
> fields it does not want to leak the contents of (which it also should,
> of course).
Thanks Segher. We are working on that in the ultravisor source code.
Claudio.
>
>> I know plpar_hcall_norets() does save CR, but it shouldn't need to, that
>> seems to be historical. aka. no one knows why it does it but it always
>> has.
>
> Segher
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox