All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/3] Provide PowerVM LPAR Platform KeyStore driver for Self Encrypting Drives
@ 2022-07-23 11:30 Nayna Jain
  2022-07-23 11:30 ` [PATCH v2 1/3] powerpc/pseries: define driver for Platform KeyStore Nayna Jain
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Nayna Jain @ 2022-07-23 11:30 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: gjoyce, erichte, Nayna Jain, npiggin, muriloo, George Wilson, bjking1

PowerVM provides an isolated Platform KeyStore(PKS)[1] storage allocation
for each partition(LPAR) with individually managed access controls to store
sensitive information securely. The Linux Kernel can access this storage by
interfacing with the hypervisor using a new set of hypervisor calls. 

This storage can be used for multiple purposes. The current two usecases
are:

1. Guest Secure Boot on PowerVM[2]
2. Self Encrypting Drives(SED) on PowerVM[3]

Initially, the PowerVM LPAR Platform KeyStore(PLPKS) driver was defined
as part of RFC patches which included the user interface design for guest
secure boot[2]. While this interface is still in progress, the same driver
is also required for Self Encrypting Drives(SED) support. For this reason,
the driver is being split from the patchset[1] and is now separately posted
with SED arch-specific code.

This patchset provides driver for PowerVM LPAR Platform KeyStore and also
arch-specific code for SED to make use of it.

The dependency patch from patch series[3] is moved to this patchset. This
patchset now builds completely of its own.

[1]https://community.ibm.com/community/user/power/blogs/chris-engel1/2020/11/20/powervm-introduces-the-platform-keystore
[2]https://lore.kernel.org/linuxppc-dev/20220622215648.96723-1-nayna@linux.ibm.com/
[3]https://lore.kernel.org/keyrings/20220718210156.1535955-1-gjoyce@linux.vnet.ibm.com/T/#m8e7b2cbbd26ee1de711bd70967fd0124c85c479f

Changelog:

v2:

* Include feedback from Gregory Joyce, Eric Richter and Murilo Opsfelder Araújo.
* Include suggestions from Michael Ellerman.
* Moved a dependency from generic SED code to this patchset. This patchset now
builds of its own.

Greg Joyce (2):
  lib: define generic accessor functions for arch specific keystore
  powerpc/pseries: Override lib/arch_vars.c with PowerPC architecture
    specific version

Nayna Jain (1):
  powerpc/pseries: define driver for Platform KeyStore

 arch/powerpc/include/asm/hvcall.h             |  11 +
 arch/powerpc/platforms/pseries/Kconfig        |  13 +
 arch/powerpc/platforms/pseries/Makefile       |   2 +
 arch/powerpc/platforms/pseries/plpks.c        | 460 ++++++++++++++++++
 arch/powerpc/platforms/pseries/plpks.h        |  71 +++
 .../platforms/pseries/plpks_arch_ops.c        | 166 +++++++
 include/linux/arch_vars.h                     |  23 +
 lib/Makefile                                  |   2 +-
 lib/arch_vars.c                               |  25 +
 9 files changed, 772 insertions(+), 1 deletion(-)
 create mode 100644 arch/powerpc/platforms/pseries/plpks.c
 create mode 100644 arch/powerpc/platforms/pseries/plpks.h
 create mode 100644 arch/powerpc/platforms/pseries/plpks_arch_ops.c
 create mode 100644 include/linux/arch_vars.h
 create mode 100644 lib/arch_vars.c

-- 
2.27.0

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 1/3] powerpc/pseries: define driver for Platform KeyStore
  2022-07-23 11:30 [PATCH v2 0/3] Provide PowerVM LPAR Platform KeyStore driver for Self Encrypting Drives Nayna Jain
@ 2022-07-23 11:30 ` Nayna Jain
  2022-07-28 14:14   ` Greg Joyce
  2022-09-06 21:00   ` Nathan Chancellor
  2022-07-23 11:30 ` [PATCH v2 2/3] lib: define generic accessor functions for arch specific keystore Nayna Jain
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 10+ messages in thread
From: Nayna Jain @ 2022-07-23 11:30 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: gjoyce, erichte, Nayna Jain, npiggin, muriloo, George Wilson, bjking1

PowerVM provides an isolated Platform Keystore(PKS) storage allocation
for each LPAR with individually managed access controls to store
sensitive information securely. It provides a new set of hypervisor
calls for Linux kernel to access PKS storage.

Define POWER LPAR Platform KeyStore(PLPKS) driver using H_CALL interface
to access PKS storage.

Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
---
 arch/powerpc/include/asm/hvcall.h       |  11 +
 arch/powerpc/platforms/pseries/Kconfig  |  13 +
 arch/powerpc/platforms/pseries/Makefile |   1 +
 arch/powerpc/platforms/pseries/plpks.c  | 460 ++++++++++++++++++++++++
 arch/powerpc/platforms/pseries/plpks.h  |  71 ++++
 5 files changed, 556 insertions(+)
 create mode 100644 arch/powerpc/platforms/pseries/plpks.c
 create mode 100644 arch/powerpc/platforms/pseries/plpks.h

diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h
index d92a20a85395..9f707974af1a 100644
--- a/arch/powerpc/include/asm/hvcall.h
+++ b/arch/powerpc/include/asm/hvcall.h
@@ -79,6 +79,7 @@
 #define H_NOT_ENOUGH_RESOURCES -44
 #define H_R_STATE       -45
 #define H_RESCINDED     -46
+#define H_P1		-54
 #define H_P2		-55
 #define H_P3		-56
 #define H_P4		-57
@@ -97,6 +98,8 @@
 #define H_OP_MODE	-73
 #define H_COP_HW	-74
 #define H_STATE		-75
+#define H_IN_USE	-77
+#define H_ABORTED	-78
 #define H_UNSUPPORTED_FLAG_START	-256
 #define H_UNSUPPORTED_FLAG_END		-511
 #define H_MULTI_THREADS_ACTIVE	-9005
@@ -321,6 +324,14 @@
 #define H_SCM_UNBIND_ALL        0x3FC
 #define H_SCM_HEALTH            0x400
 #define H_SCM_PERFORMANCE_STATS 0x418
+#define H_PKS_GET_CONFIG	0x41C
+#define H_PKS_SET_PASSWORD	0x420
+#define H_PKS_GEN_PASSWORD	0x424
+#define H_PKS_WRITE_OBJECT	0x42C
+#define H_PKS_GEN_KEY		0x430
+#define H_PKS_READ_OBJECT	0x434
+#define H_PKS_REMOVE_OBJECT	0x438
+#define H_PKS_CONFIRM_OBJECT_FLUSHED	0x43C
 #define H_RPT_INVALIDATE	0x448
 #define H_SCM_FLUSH		0x44C
 #define H_GET_ENERGY_SCALE_INFO	0x450
diff --git a/arch/powerpc/platforms/pseries/Kconfig b/arch/powerpc/platforms/pseries/Kconfig
index f7fd91d153a4..c4a6d4083a7a 100644
--- a/arch/powerpc/platforms/pseries/Kconfig
+++ b/arch/powerpc/platforms/pseries/Kconfig
@@ -142,6 +142,19 @@ config IBMEBUS
 	help
 	  Bus device driver for GX bus based adapters.
 
+config PSERIES_PLPKS
+	depends on PPC_PSERIES
+	bool "Support for the Platform Key Storage"
+	help
+	  PowerVM provides an isolated Platform Keystore(PKS) storage
+	  allocation for each LPAR with individually managed access
+	  controls to store sensitive information securely. It can be
+	  used to store asymmetric public keys or secrets as required
+	  by different usecases. Select this config to enable
+	  operating system interface to hypervisor to access this space.
+
+	  If unsure, select N.
+
 config PAPR_SCM
 	depends on PPC_PSERIES && MEMORY_HOTPLUG && LIBNVDIMM
 	tristate "Support for the PAPR Storage Class Memory interface"
diff --git a/arch/powerpc/platforms/pseries/Makefile b/arch/powerpc/platforms/pseries/Makefile
index 7aaff5323544..14e143b946a3 100644
--- a/arch/powerpc/platforms/pseries/Makefile
+++ b/arch/powerpc/platforms/pseries/Makefile
@@ -28,6 +28,7 @@ obj-$(CONFIG_PAPR_SCM)		+= papr_scm.o
 obj-$(CONFIG_PPC_SPLPAR)	+= vphn.o
 obj-$(CONFIG_PPC_SVM)		+= svm.o
 obj-$(CONFIG_FA_DUMP)		+= rtas-fadump.o
+obj-$(CONFIG_PSERIES_PLPKS) += plpks.o
 
 obj-$(CONFIG_SUSPEND)		+= suspend.o
 obj-$(CONFIG_PPC_VAS)		+= vas.o vas-sysfs.o
diff --git a/arch/powerpc/platforms/pseries/plpks.c b/arch/powerpc/platforms/pseries/plpks.c
new file mode 100644
index 000000000000..52aaa2894606
--- /dev/null
+++ b/arch/powerpc/platforms/pseries/plpks.c
@@ -0,0 +1,460 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * POWER LPAR Platform KeyStore(PLPKS)
+ * Copyright (C) 2022 IBM Corporation
+ * Author: Nayna Jain <nayna@linux.ibm.com>
+ *
+ * Provides access to variables stored in Power LPAR Platform KeyStore(PLPKS).
+ */
+
+#define pr_fmt(fmt) "plpks: " fmt
+
+#include <linux/delay.h>
+#include <linux/errno.h>
+#include <linux/io.h>
+#include <linux/printk.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/types.h>
+#include <asm/hvcall.h>
+
+#include "plpks.h"
+
+#define PKS_FW_OWNER	     0x1
+#define PKS_BOOTLOADER_OWNER 0x2
+#define PKS_OS_OWNER	     0x3
+
+#define LABEL_VERSION	    0
+#define MAX_LABEL_ATTR_SIZE 16
+#define MAX_NAME_SIZE	    239
+#define MAX_DATA_SIZE	    4000
+
+#define PKS_FLUSH_MAX_TIMEOUT 5000 //msec
+#define PKS_FLUSH_SLEEP	      10 //msec
+#define PKS_FLUSH_SLEEP_RANGE 400
+
+static u8 *ospassword;
+static u16 ospasswordlength;
+
+// Retrieved with H_PKS_GET_CONFIG
+static u16 maxpwsize;
+static u16 maxobjsize;
+
+struct plpks_auth {
+	u8 version;
+	u8 consumer;
+	__be64 rsvd0;
+	__be32 rsvd1;
+	__be16 passwordlength;
+	u8 password[];
+} __packed __aligned(16);
+
+struct label_attr {
+	u8 prefix[8];
+	u8 version;
+	u8 os;
+	u8 length;
+	u8 reserved[5];
+};
+
+struct label {
+	struct label_attr attr;
+	u8 name[MAX_NAME_SIZE];
+	size_t size;
+};
+
+static int pseries_status_to_err(int rc)
+{
+	int err;
+
+	switch (rc) {
+	case H_SUCCESS:
+		err = 0;
+		break;
+	case H_FUNCTION:
+		err = -ENXIO;
+		break;
+	case H_P1:
+	case H_P2:
+	case H_P3:
+	case H_P4:
+	case H_P5:
+	case H_P6:
+		err = -EINVAL;
+		break;
+	case H_NOT_FOUND:
+		err = -ENOENT;
+		break;
+	case H_BUSY:
+		err = -EBUSY;
+		break;
+	case H_AUTHORITY:
+		err = -EPERM;
+		break;
+	case H_NO_MEM:
+		err = -ENOMEM;
+		break;
+	case H_RESOURCE:
+		err = -EEXIST;
+		break;
+	case H_TOO_BIG:
+		err = -EFBIG;
+		break;
+	case H_STATE:
+		err = -EIO;
+		break;
+	case H_R_STATE:
+		err = -EIO;
+		break;
+	case H_IN_USE:
+		err = -EEXIST;
+		break;
+	case H_ABORTED:
+		err = -EINTR;
+		break;
+	default:
+		err = -EINVAL;
+	}
+
+	return err;
+}
+
+static int plpks_gen_password(void)
+{
+	unsigned long retbuf[PLPAR_HCALL_BUFSIZE] = { 0 };
+	u8 *password, consumer = PKS_OS_OWNER;
+	int rc;
+
+	password = kzalloc(maxpwsize, GFP_KERNEL);
+	if (!password)
+		return -ENOMEM;
+
+	rc = plpar_hcall(H_PKS_GEN_PASSWORD, retbuf, consumer, 0,
+			 virt_to_phys(password), maxpwsize);
+
+	if (!rc) {
+		ospasswordlength = maxpwsize;
+		ospassword = kzalloc(maxpwsize, GFP_KERNEL);
+		if (!ospassword) {
+			kfree(password);
+			return -ENOMEM;
+		}
+		memcpy(ospassword, password, ospasswordlength);
+	} else {
+		if (rc == H_IN_USE) {
+			pr_warn("Password is already set for POWER LPAR Platform KeyStore\n");
+			rc = 0;
+		} else {
+			goto out;
+		}
+	}
+out:
+	kfree(password);
+
+	return pseries_status_to_err(rc);
+}
+
+static struct plpks_auth *construct_auth(u8 consumer)
+{
+	struct plpks_auth *auth;
+
+	if (consumer > PKS_OS_OWNER)
+		return ERR_PTR(-EINVAL);
+
+	auth = kmalloc(struct_size(auth, password, maxpwsize), GFP_KERNEL);
+	if (!auth)
+		return ERR_PTR(-ENOMEM);
+
+	auth->version = 1;
+	auth->consumer = consumer;
+	auth->rsvd0 = 0;
+	auth->rsvd1 = 0;
+
+	if (consumer == PKS_FW_OWNER || consumer == PKS_BOOTLOADER_OWNER) {
+		auth->passwordlength = 0;
+		return auth;
+	}
+
+	memcpy(auth->password, ospassword, ospasswordlength);
+
+	auth->passwordlength = cpu_to_be16(ospasswordlength);
+
+	return auth;
+}
+
+/**
+ * Label is combination of label attributes + name.
+ * Label attributes are used internally by kernel and not exposed to the user.
+ */
+static struct label *construct_label(char *component, u8 varos, u8 *name,
+				     u16 namelen)
+{
+	struct label *label;
+	size_t slen;
+
+	if (!name || namelen > MAX_NAME_SIZE)
+		return ERR_PTR(-EINVAL);
+
+	slen = strlen(component);
+	if (component && slen > sizeof(label->attr.prefix))
+		return ERR_PTR(-EINVAL);
+
+	label = kzalloc(sizeof(*label), GFP_KERNEL);
+	if (!label)
+		return ERR_PTR(-ENOMEM);
+
+	if (component)
+		memcpy(&label->attr.prefix, component, slen);
+
+	label->attr.version = LABEL_VERSION;
+	label->attr.os = varos;
+	label->attr.length = MAX_LABEL_ATTR_SIZE;
+	memcpy(&label->name, name, namelen);
+
+	label->size = sizeof(struct label_attr) + namelen;
+
+	return label;
+}
+
+static int _plpks_get_config(void)
+{
+	unsigned long retbuf[PLPAR_HCALL_BUFSIZE] = { 0 };
+	struct {
+		u8 version;
+		u8 flags;
+		__be32 rsvd0;
+		__be16 maxpwsize;
+		__be16 maxobjlabelsize;
+		__be16 maxobjsize;
+		__be32 totalsize;
+		__be32 usedspace;
+		__be32 supportedpolicies;
+		__be64 rsvd1;
+	} __packed config;
+	size_t size;
+	int rc;
+
+	size = sizeof(config);
+
+	rc = plpar_hcall(H_PKS_GET_CONFIG, retbuf, virt_to_phys(&config), size);
+
+	if (rc != H_SUCCESS)
+		return pseries_status_to_err(rc);
+
+	maxpwsize = be16_to_cpu(config.maxpwsize);
+	maxobjsize = be16_to_cpu(config.maxobjsize);
+
+	return 0;
+}
+
+static int plpks_confirm_object_flushed(struct label *label,
+					struct plpks_auth *auth)
+{
+	unsigned long retbuf[PLPAR_HCALL_BUFSIZE] = { 0 };
+	u64 timeout = 0;
+	u8 status;
+	int rc;
+
+	do {
+		rc = plpar_hcall(H_PKS_CONFIRM_OBJECT_FLUSHED, retbuf,
+				 virt_to_phys(auth), virt_to_phys(label),
+				 label->size);
+
+		status = retbuf[0];
+		if (rc) {
+			if (rc == H_NOT_FOUND && status == 1)
+				rc = 0;
+			break;
+		}
+
+		if (!rc && status == 1)
+			break;
+
+		usleep_range(PKS_FLUSH_SLEEP,
+			     PKS_FLUSH_SLEEP + PKS_FLUSH_SLEEP_RANGE);
+		timeout = timeout + PKS_FLUSH_SLEEP;
+	} while (timeout < PKS_FLUSH_MAX_TIMEOUT);
+
+	rc = pseries_status_to_err(rc);
+
+	return rc;
+}
+
+int plpks_write_var(struct plpks_var var)
+{
+	unsigned long retbuf[PLPAR_HCALL_BUFSIZE] = { 0 };
+	struct plpks_auth *auth;
+	struct label *label;
+	int rc;
+
+	if (!var.component || !var.data || var.datalen <= 0 ||
+	    var.namelen > MAX_NAME_SIZE || var.datalen > MAX_DATA_SIZE)
+		return -EINVAL;
+
+	if (var.policy & SIGNEDUPDATE)
+		return -EINVAL;
+
+	auth = construct_auth(PKS_OS_OWNER);
+	if (IS_ERR(auth))
+		return PTR_ERR(auth);
+
+	label = construct_label(var.component, var.os, var.name, var.namelen);
+	if (IS_ERR(label)) {
+		rc = PTR_ERR(label);
+		goto out;
+	}
+
+	rc = plpar_hcall(H_PKS_WRITE_OBJECT, retbuf, virt_to_phys(auth),
+			 virt_to_phys(label), label->size, var.policy,
+			 virt_to_phys(var.data), var.datalen);
+
+	if (!rc)
+		rc = plpks_confirm_object_flushed(label, auth);
+
+	if (rc)
+		pr_err("Failed to write variable %s for component %s with error %d\n",
+		       var.name, var.component, rc);
+
+	rc = pseries_status_to_err(rc);
+	kfree(label);
+out:
+	kfree(auth);
+
+	return rc;
+}
+
+int plpks_remove_var(char *component, u8 varos, struct plpks_var_name vname)
+{
+	unsigned long retbuf[PLPAR_HCALL_BUFSIZE] = { 0 };
+	struct plpks_auth *auth;
+	struct label *label;
+	int rc;
+
+	if (!component || vname.namelen > MAX_NAME_SIZE)
+		return -EINVAL;
+
+	auth = construct_auth(PKS_OS_OWNER);
+	if (IS_ERR(auth))
+		return PTR_ERR(auth);
+
+	label = construct_label(component, varos, vname.name, vname.namelen);
+	if (IS_ERR(label)) {
+		rc = PTR_ERR(label);
+		goto out;
+	}
+
+	rc = plpar_hcall(H_PKS_REMOVE_OBJECT, retbuf, virt_to_phys(auth),
+			 virt_to_phys(label), label->size);
+
+	if (!rc)
+		rc = plpks_confirm_object_flushed(label, auth);
+
+	if (rc)
+		pr_err("Failed to remove variable %s for component %s with error %d\n",
+		       vname.name, component, rc);
+
+	rc = pseries_status_to_err(rc);
+	kfree(label);
+out:
+	kfree(auth);
+
+	return rc;
+}
+
+static int plpks_read_var(u8 consumer, struct plpks_var *var)
+{
+	unsigned long retbuf[PLPAR_HCALL_BUFSIZE] = { 0 };
+	struct plpks_auth *auth;
+	struct label *label;
+	u8 *output;
+	int rc;
+
+	if (var->namelen > MAX_NAME_SIZE)
+		return -EINVAL;
+
+	auth = construct_auth(PKS_OS_OWNER);
+	if (IS_ERR(auth))
+		return PTR_ERR(auth);
+
+	label = construct_label(var->component, var->os, var->name,
+				var->namelen);
+	if (IS_ERR(label)) {
+		rc = PTR_ERR(label);
+		goto out_free_auth;
+	}
+
+	output = kzalloc(maxobjsize, GFP_KERNEL);
+	if (!output) {
+		rc = -ENOMEM;
+		goto out_free_label;
+	}
+
+	rc = plpar_hcall(H_PKS_READ_OBJECT, retbuf, virt_to_phys(auth),
+			 virt_to_phys(label), label->size, virt_to_phys(output),
+			 maxobjsize);
+
+	if (rc != H_SUCCESS) {
+		pr_err("Failed to read variable %s for component %s with error %d\n",
+		       var->name, var->component, rc);
+		rc = pseries_status_to_err(rc);
+		goto out_free_output;
+	}
+
+	if (var->datalen == 0 || var->datalen > retbuf[0])
+		var->datalen = retbuf[0];
+
+	var->data = kzalloc(var->datalen, GFP_KERNEL);
+	if (!var->data) {
+		rc = -ENOMEM;
+		goto out_free_output;
+	}
+	var->policy = retbuf[1];
+
+	memcpy(var->data, output, var->datalen);
+	rc = 0;
+
+out_free_output:
+	kfree(output);
+out_free_label:
+	kfree(label);
+out_free_auth:
+	kfree(auth);
+
+	return rc;
+}
+
+int plpks_read_os_var(struct plpks_var *var)
+{
+	return plpks_read_var(PKS_OS_OWNER, var);
+}
+
+int plpks_read_fw_var(struct plpks_var *var)
+{
+	return plpks_read_var(PKS_FW_OWNER, var);
+}
+
+int plpks_read_bootloader_var(struct plpks_var *var)
+{
+	return plpks_read_var(PKS_BOOTLOADER_OWNER, var);
+}
+
+static __init int pseries_plpks_init(void)
+{
+	int rc;
+
+	rc = _plpks_get_config();
+
+	if (rc) {
+		pr_err("POWER LPAR Platform KeyStore is not supported or enabled\n");
+		return rc;
+	}
+
+	rc = plpks_gen_password();
+	if (rc)
+		pr_err("Failed setting POWER LPAR Platform KeyStore Password\n");
+	else
+		pr_info("POWER LPAR Platform KeyStore initialized successfully\n");
+
+	return rc;
+}
+arch_initcall(pseries_plpks_init);
diff --git a/arch/powerpc/platforms/pseries/plpks.h b/arch/powerpc/platforms/pseries/plpks.h
new file mode 100644
index 000000000000..c6a291367bb1
--- /dev/null
+++ b/arch/powerpc/platforms/pseries/plpks.h
@@ -0,0 +1,71 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2022 IBM Corporation
+ * Author: Nayna Jain <nayna@linux.ibm.com>
+ *
+ * Platform keystore for pseries LPAR(PLPKS).
+ */
+
+#ifndef _PSERIES_PLPKS_H
+#define _PSERIES_PLPKS_H
+
+#include <linux/types.h>
+#include <linux/list.h>
+
+#define OSSECBOOTAUDIT 0x40000000
+#define OSSECBOOTENFORCE 0x20000000
+#define WORLDREADABLE 0x08000000
+#define SIGNEDUPDATE 0x01000000
+
+#define PLPKS_VAR_LINUX	0x01
+#define PLPKS_VAR_COMMON	0x04
+
+struct plpks_var {
+	char *component;
+	u8 *name;
+	u8 *data;
+	u32 policy;
+	u16 namelen;
+	u16 datalen;
+	u8 os;
+};
+
+struct plpks_var_name {
+	u8  *name;
+	u16 namelen;
+};
+
+struct plpks_var_name_list {
+	u32 varcount;
+	struct plpks_var_name varlist[];
+};
+
+/**
+ * Writes the specified var and its data to PKS.
+ * Any caller of PKS driver should present a valid component type for
+ * their variable.
+ */
+int plpks_write_var(struct plpks_var var);
+
+/**
+ * Removes the specified var and its data from PKS.
+ */
+int plpks_remove_var(char *component, u8 varos,
+		     struct plpks_var_name vname);
+
+/**
+ * Returns the data for the specified os variable.
+ */
+int plpks_read_os_var(struct plpks_var *var);
+
+/**
+ * Returns the data for the specified firmware variable.
+ */
+int plpks_read_fw_var(struct plpks_var *var);
+
+/**
+ * Returns the data for the specified bootloader variable.
+ */
+int plpks_read_bootloader_var(struct plpks_var *var);
+
+#endif
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 2/3] lib: define generic accessor functions for arch specific keystore
  2022-07-23 11:30 [PATCH v2 0/3] Provide PowerVM LPAR Platform KeyStore driver for Self Encrypting Drives Nayna Jain
  2022-07-23 11:30 ` [PATCH v2 1/3] powerpc/pseries: define driver for Platform KeyStore Nayna Jain
@ 2022-07-23 11:30 ` Nayna Jain
  2022-07-23 11:30 ` [PATCH v2 3/3] powerpc/pseries: Override lib/arch_vars.c with PowerPC architecture specific version Nayna Jain
  2022-07-29 13:02 ` [PATCH v2 0/3] Provide PowerVM LPAR Platform KeyStore driver for Self Encrypting Drives Michael Ellerman
  3 siblings, 0 replies; 10+ messages in thread
From: Nayna Jain @ 2022-07-23 11:30 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: gjoyce, erichte, npiggin, muriloo, George Wilson, bjking1

From: Greg Joyce <gjoyce@linux.vnet.ibm.com>

Generic kernel subsystems may rely on platform specific persistent
KeyStore to store objects containing sensitive key material. In such case,
they need to access architecture specific functions to perform read/write
operations on these variables.

Define the generic variable read/write prototypes to be implemented by
architecture specific versions. The default(weak) implementations of
these prototypes return -EOPNOTSUPP unless overridden by architecture
versions.

Signed-off-by: Greg Joyce <gjoyce@linux.vnet.ibm.com>
---
 include/linux/arch_vars.h | 23 +++++++++++++++++++++++
 lib/Makefile              |  2 +-
 lib/arch_vars.c           | 25 +++++++++++++++++++++++++
 3 files changed, 49 insertions(+), 1 deletion(-)
 create mode 100644 include/linux/arch_vars.h
 create mode 100644 lib/arch_vars.c

diff --git a/include/linux/arch_vars.h b/include/linux/arch_vars.h
new file mode 100644
index 000000000000..9c280ff9432e
--- /dev/null
+++ b/include/linux/arch_vars.h
@@ -0,0 +1,23 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Platform variable opearations.
+ *
+ * Copyright (C) 2022 IBM Corporation
+ *
+ * These are the accessor functions (read/write) for architecture specific
+ * variables. Specific architectures can provide overrides.
+ *
+ */
+
+#include <linux/kernel.h>
+
+enum arch_variable_type {
+	ARCH_VAR_OPAL_KEY      = 0,     /* SED Opal Authentication Key */
+	ARCH_VAR_OTHER         = 1,     /* Other type of variable */
+	ARCH_VAR_MAX           = 1,     /* Maximum type value */
+};
+
+int arch_read_variable(enum arch_variable_type type, char *varname,
+		       void *varbuf, u_int *varlen);
+int arch_write_variable(enum arch_variable_type type, char *varname,
+			void *varbuf, u_int varlen);
diff --git a/lib/Makefile b/lib/Makefile
index f99bf61f8bbc..b90c4cb0dbbb 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -48,7 +48,7 @@ obj-y += bcd.o sort.o parser.o debug_locks.o random32.o \
 	 bsearch.o find_bit.o llist.o memweight.o kfifo.o \
 	 percpu-refcount.o rhashtable.o \
 	 once.o refcount.o usercopy.o errseq.o bucket_locks.o \
-	 generic-radix-tree.o
+	 generic-radix-tree.o arch_vars.o
 obj-$(CONFIG_STRING_SELFTEST) += test_string.o
 obj-y += string_helpers.o
 obj-$(CONFIG_TEST_STRING_HELPERS) += test-string_helpers.o
diff --git a/lib/arch_vars.c b/lib/arch_vars.c
new file mode 100644
index 000000000000..e6f16d7d09c1
--- /dev/null
+++ b/lib/arch_vars.c
@@ -0,0 +1,25 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Platform variable operations.
+ *
+ * Copyright (C) 2022 IBM Corporation
+ *
+ * These are the accessor functions (read/write) for architecture specific
+ * variables. Specific architectures can provide overrides.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/arch_vars.h>
+
+int __weak arch_read_variable(enum arch_variable_type type, char *varname,
+			      void *varbuf, u_int *varlen)
+{
+	return -EOPNOTSUPP;
+}
+
+int __weak arch_write_variable(enum arch_variable_type type, char *varname,
+			       void *varbuf, u_int varlen)
+{
+	return -EOPNOTSUPP;
+}
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 3/3] powerpc/pseries: Override lib/arch_vars.c with PowerPC architecture specific version
  2022-07-23 11:30 [PATCH v2 0/3] Provide PowerVM LPAR Platform KeyStore driver for Self Encrypting Drives Nayna Jain
  2022-07-23 11:30 ` [PATCH v2 1/3] powerpc/pseries: define driver for Platform KeyStore Nayna Jain
  2022-07-23 11:30 ` [PATCH v2 2/3] lib: define generic accessor functions for arch specific keystore Nayna Jain
@ 2022-07-23 11:30 ` Nayna Jain
  2022-07-29 13:02 ` [PATCH v2 0/3] Provide PowerVM LPAR Platform KeyStore driver for Self Encrypting Drives Michael Ellerman
  3 siblings, 0 replies; 10+ messages in thread
From: Nayna Jain @ 2022-07-23 11:30 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: gjoyce, erichte, npiggin, muriloo, George Wilson, bjking1

From: Greg Joyce <gjoyce@linux.vnet.ibm.com>

Self Encrypting Drives(SED) make use of POWER LPAR Platform KeyStore for
storing its variables. Thus the block subsystem needs to access
PowerPC specific functions to read/write objects in PLPKS.

Override the default implementations in lib/arch_vars.c file with
PowerPC specific versions.

Signed-off-by: Greg Joyce <gjoyce@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/pseries/Makefile       |   1 +
 .../platforms/pseries/plpks_arch_ops.c        | 166 ++++++++++++++++++
 2 files changed, 167 insertions(+)
 create mode 100644 arch/powerpc/platforms/pseries/plpks_arch_ops.c

diff --git a/arch/powerpc/platforms/pseries/Makefile b/arch/powerpc/platforms/pseries/Makefile
index 14e143b946a3..3a545422eae5 100644
--- a/arch/powerpc/platforms/pseries/Makefile
+++ b/arch/powerpc/platforms/pseries/Makefile
@@ -29,6 +29,7 @@ obj-$(CONFIG_PPC_SPLPAR)	+= vphn.o
 obj-$(CONFIG_PPC_SVM)		+= svm.o
 obj-$(CONFIG_FA_DUMP)		+= rtas-fadump.o
 obj-$(CONFIG_PSERIES_PLPKS) += plpks.o
+obj-$(CONFIG_PSERIES_PLPKS) += plpks_arch_ops.o
 
 obj-$(CONFIG_SUSPEND)		+= suspend.o
 obj-$(CONFIG_PPC_VAS)		+= vas.o vas-sysfs.o
diff --git a/arch/powerpc/platforms/pseries/plpks_arch_ops.c b/arch/powerpc/platforms/pseries/plpks_arch_ops.c
new file mode 100644
index 000000000000..48fa19f0c9c5
--- /dev/null
+++ b/arch/powerpc/platforms/pseries/plpks_arch_ops.c
@@ -0,0 +1,166 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * POWER Platform arch specific code for SED
+ * Copyright (C) 2022 IBM Corporation
+ *
+ * Define operations for generic kernel subsystems to read/write keys from
+ * POWER LPAR Platform KeyStore(PLPKS).
+ *
+ * List of subsystems/usecase using PLPKS:
+ * - Self Encrypting Drives(SED)
+ */
+
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/ioctl.h>
+#include <uapi/linux/sed-opal.h>
+#include <linux/sed-opal.h>
+#include <linux/arch_vars.h>
+#include "plpks.h"
+
+/*
+ * variable structure that contains all SED data
+ */
+struct plpks_sed_object_data {
+	u_char version;
+	u_char pad1[7];
+	u_long authority;
+	u_long range;
+	u_int  key_len;
+	u_char key[32];
+};
+
+/*
+ * ext_type values
+ *	00        no extension exists
+ *	01-1F     common
+ *	20-3F     AIX
+ *	40-5F     Linux
+ *	60-7F     IBMi
+ */
+
+/*
+ * This extension is optional for version 1 sed_object_data
+ */
+struct sed_object_extension {
+	u8 ext_type;
+	u8 rsvd[3];
+	u8 ext_data[64];
+};
+
+#define PKS_SED_OBJECT_DATA_V1          1
+#define PKS_SED_MANGLED_LABEL           "/default/pri"
+#define PLPKS_SED_COMPONENT             "sed-opal"
+#define PLPKS_SED_POLICY                WORLDREADABLE
+#define PLPKS_SED_OS_COMMON             4
+
+#ifndef CONFIG_BLK_SED_OPAL
+#define	OPAL_AUTH_KEY                   ""
+#endif
+
+/*
+ * Read the variable data from PKS given the label
+ */
+int arch_read_variable(enum arch_variable_type type, char *varname,
+		       void *varbuf, u_int *varlen)
+{
+	struct plpks_var var;
+	struct plpks_sed_object_data *data;
+	u_int offset = 0;
+	char *buf = (char *)varbuf;
+	int ret;
+
+	var.name = varname;
+	var.namelen = strlen(varname);
+	var.policy = PLPKS_SED_POLICY;
+	var.os = PLPKS_SED_OS_COMMON;
+	var.data = NULL;
+	var.datalen = 0;
+
+	switch (type) {
+	case ARCH_VAR_OPAL_KEY:
+		var.component = PLPKS_SED_COMPONENT;
+		if (strcmp(OPAL_AUTH_KEY, varname) == 0) {
+			var.name = PKS_SED_MANGLED_LABEL;
+			var.namelen = strlen(varname);
+		}
+		offset = offsetof(struct plpks_sed_object_data, key);
+		break;
+	case ARCH_VAR_OTHER:
+		var.component = "";
+		break;
+	}
+
+	ret = plpks_read_os_var(&var);
+	if (ret != 0)
+		return ret;
+
+	if (offset > var.datalen)
+		offset = 0;
+
+	switch (type) {
+	case ARCH_VAR_OPAL_KEY:
+		data = (struct plpks_sed_object_data *)var.data;
+		*varlen = data->key_len;
+		break;
+	case ARCH_VAR_OTHER:
+		*varlen = var.datalen;
+		break;
+	}
+
+	if (var.data) {
+		memcpy(varbuf, var.data + offset, var.datalen - offset);
+		buf[*varlen] = '\0';
+		kfree(var.data);
+	}
+
+	return 0;
+}
+
+/*
+ * Write the variable data to PKS given the label
+ */
+int arch_write_variable(enum arch_variable_type type, char *varname,
+			void *varbuf, u_int varlen)
+{
+	struct plpks_var var;
+	struct plpks_sed_object_data data;
+	struct plpks_var_name vname;
+
+	var.name = varname;
+	var.namelen = strlen(varname);
+	var.policy = PLPKS_SED_POLICY;
+	var.os = PLPKS_SED_OS_COMMON;
+	var.datalen = varlen;
+	var.data = varbuf;
+
+	switch (type) {
+	case ARCH_VAR_OPAL_KEY:
+		var.component = PLPKS_SED_COMPONENT;
+		if (strcmp(OPAL_AUTH_KEY, varname) == 0) {
+			var.name = PKS_SED_MANGLED_LABEL;
+			var.namelen = strlen(varname);
+		}
+		var.datalen = sizeof(struct plpks_sed_object_data);
+		var.data = (u8 *)&data;
+
+		/* initialize SED object */
+		data.version = PKS_SED_OBJECT_DATA_V1;
+		data.authority = 0;
+		data.range = 0;
+		data.key_len = varlen;
+		memcpy(data.key, varbuf, varlen);
+		break;
+	case ARCH_VAR_OTHER:
+		var.component = "";
+		break;
+	}
+
+	/* variable update requires delete first */
+	vname.namelen = var.namelen;
+	vname.name = var.name;
+	(void)plpks_remove_var(PLPKS_SED_COMPONENT, PLPKS_SED_OS_COMMON, vname);
+
+	return plpks_write_var(var);
+}
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/3] powerpc/pseries: define driver for Platform KeyStore
  2022-07-23 11:30 ` [PATCH v2 1/3] powerpc/pseries: define driver for Platform KeyStore Nayna Jain
@ 2022-07-28 14:14   ` Greg Joyce
  2022-09-06 21:00   ` Nathan Chancellor
  1 sibling, 0 replies; 10+ messages in thread
From: Greg Joyce @ 2022-07-28 14:14 UTC (permalink / raw)
  To: Nayna Jain, linuxppc-dev
  Cc: erichte, npiggin, muriloo, George Wilson, bjking1

Reviewed-by: Greg Joyce <gjoyce@linux.vnet.ibm.com>
Tested-by: Greg Joyce <gjoyce@linux.vnet.ibm.com>

On Sat, 2022-07-23 at 07:30 -0400, Nayna Jain wrote:
> PowerVM provides an isolated Platform Keystore(PKS) storage
> allocation
> for each LPAR with individually managed access controls to store
> sensitive information securely. It provides a new set of hypervisor
> calls for Linux kernel to access PKS storage.
> 
> Define POWER LPAR Platform KeyStore(PLPKS) driver using H_CALL
> interface
> to access PKS storage.
> 
> Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
> ---
>  arch/powerpc/include/asm/hvcall.h       |  11 +
>  arch/powerpc/platforms/pseries/Kconfig  |  13 +
>  arch/powerpc/platforms/pseries/Makefile |   1 +
>  arch/powerpc/platforms/pseries/plpks.c  | 460
> ++++++++++++++++++++++++
>  arch/powerpc/platforms/pseries/plpks.h  |  71 ++++
>  5 files changed, 556 insertions(+)
>  create mode 100644 arch/powerpc/platforms/pseries/plpks.c
>  create mode 100644 arch/powerpc/platforms/pseries/plpks.h
> 
> diff --git a/arch/powerpc/include/asm/hvcall.h
> b/arch/powerpc/include/asm/hvcall.h
> index d92a20a85395..9f707974af1a 100644
> --- a/arch/powerpc/include/asm/hvcall.h
> +++ b/arch/powerpc/include/asm/hvcall.h
> @@ -79,6 +79,7 @@
>  #define H_NOT_ENOUGH_RESOURCES -44
>  #define H_R_STATE       -45
>  #define H_RESCINDED     -46
> +#define H_P1		-54
>  #define H_P2		-55
>  #define H_P3		-56
>  #define H_P4		-57
> @@ -97,6 +98,8 @@
>  #define H_OP_MODE	-73
>  #define H_COP_HW	-74
>  #define H_STATE		-75
> +#define H_IN_USE	-77
> +#define H_ABORTED	-78
>  #define H_UNSUPPORTED_FLAG_START	-256
>  #define H_UNSUPPORTED_FLAG_END		-511
>  #define H_MULTI_THREADS_ACTIVE	-9005
> @@ -321,6 +324,14 @@
>  #define H_SCM_UNBIND_ALL        0x3FC
>  #define H_SCM_HEALTH            0x400
>  #define H_SCM_PERFORMANCE_STATS 0x418
> +#define H_PKS_GET_CONFIG	0x41C
> +#define H_PKS_SET_PASSWORD	0x420
> +#define H_PKS_GEN_PASSWORD	0x424
> +#define H_PKS_WRITE_OBJECT	0x42C
> +#define H_PKS_GEN_KEY		0x430
> +#define H_PKS_READ_OBJECT	0x434
> +#define H_PKS_REMOVE_OBJECT	0x438
> +#define H_PKS_CONFIRM_OBJECT_FLUSHED	0x43C
>  #define H_RPT_INVALIDATE	0x448
>  #define H_SCM_FLUSH		0x44C
>  #define H_GET_ENERGY_SCALE_INFO	0x450
> diff --git a/arch/powerpc/platforms/pseries/Kconfig
> b/arch/powerpc/platforms/pseries/Kconfig
> index f7fd91d153a4..c4a6d4083a7a 100644
> --- a/arch/powerpc/platforms/pseries/Kconfig
> +++ b/arch/powerpc/platforms/pseries/Kconfig
> @@ -142,6 +142,19 @@ config IBMEBUS
>  	help
>  	  Bus device driver for GX bus based adapters.
> 
> +config PSERIES_PLPKS
> +	depends on PPC_PSERIES
> +	bool "Support for the Platform Key Storage"
> +	help
> +	  PowerVM provides an isolated Platform Keystore(PKS) storage
> +	  allocation for each LPAR with individually managed access
> +	  controls to store sensitive information securely. It can be
> +	  used to store asymmetric public keys or secrets as required
> +	  by different usecases. Select this config to enable
> +	  operating system interface to hypervisor to access this
> space.
> +
> +	  If unsure, select N.
> +
>  config PAPR_SCM
>  	depends on PPC_PSERIES && MEMORY_HOTPLUG && LIBNVDIMM
>  	tristate "Support for the PAPR Storage Class Memory interface"
> diff --git a/arch/powerpc/platforms/pseries/Makefile
> b/arch/powerpc/platforms/pseries/Makefile
> index 7aaff5323544..14e143b946a3 100644
> --- a/arch/powerpc/platforms/pseries/Makefile
> +++ b/arch/powerpc/platforms/pseries/Makefile
> @@ -28,6 +28,7 @@ obj-$(CONFIG_PAPR_SCM)		+= papr_scm.o
>  obj-$(CONFIG_PPC_SPLPAR)	+= vphn.o
>  obj-$(CONFIG_PPC_SVM)		+= svm.o
>  obj-$(CONFIG_FA_DUMP)		+= rtas-fadump.o
> +obj-$(CONFIG_PSERIES_PLPKS) += plpks.o
> 
>  obj-$(CONFIG_SUSPEND)		+= suspend.o
>  obj-$(CONFIG_PPC_VAS)		+= vas.o vas-sysfs.o
> diff --git a/arch/powerpc/platforms/pseries/plpks.c
> b/arch/powerpc/platforms/pseries/plpks.c
> new file mode 100644
> index 000000000000..52aaa2894606
> --- /dev/null
> +++ b/arch/powerpc/platforms/pseries/plpks.c
> @@ -0,0 +1,460 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * POWER LPAR Platform KeyStore(PLPKS)
> + * Copyright (C) 2022 IBM Corporation
> + * Author: Nayna Jain <nayna@linux.ibm.com>
> + *
> + * Provides access to variables stored in Power LPAR Platform
> KeyStore(PLPKS).
> + */
> +
> +#define pr_fmt(fmt) "plpks: " fmt
> +
> +#include <linux/delay.h>
> +#include <linux/errno.h>
> +#include <linux/io.h>
> +#include <linux/printk.h>
> +#include <linux/slab.h>
> +#include <linux/string.h>
> +#include <linux/types.h>
> +#include <asm/hvcall.h>
> +
> +#include "plpks.h"
> +
> +#define PKS_FW_OWNER	     0x1
> +#define PKS_BOOTLOADER_OWNER 0x2
> +#define PKS_OS_OWNER	     0x3
> +
> +#define LABEL_VERSION	    0
> +#define MAX_LABEL_ATTR_SIZE 16
> +#define MAX_NAME_SIZE	    239
> +#define MAX_DATA_SIZE	    4000
> +
> +#define PKS_FLUSH_MAX_TIMEOUT 5000 //msec
> +#define PKS_FLUSH_SLEEP	      10 //msec
> +#define PKS_FLUSH_SLEEP_RANGE 400
> +
> +static u8 *ospassword;
> +static u16 ospasswordlength;
> +
> +// Retrieved with H_PKS_GET_CONFIG
> +static u16 maxpwsize;
> +static u16 maxobjsize;
> +
> +struct plpks_auth {
> +	u8 version;
> +	u8 consumer;
> +	__be64 rsvd0;
> +	__be32 rsvd1;
> +	__be16 passwordlength;
> +	u8 password[];
> +} __packed __aligned(16);
> +
> +struct label_attr {
> +	u8 prefix[8];
> +	u8 version;
> +	u8 os;
> +	u8 length;
> +	u8 reserved[5];
> +};
> +
> +struct label {
> +	struct label_attr attr;
> +	u8 name[MAX_NAME_SIZE];
> +	size_t size;
> +};
> +
> +static int pseries_status_to_err(int rc)
> +{
> +	int err;
> +
> +	switch (rc) {
> +	case H_SUCCESS:
> +		err = 0;
> +		break;
> +	case H_FUNCTION:
> +		err = -ENXIO;
> +		break;
> +	case H_P1:
> +	case H_P2:
> +	case H_P3:
> +	case H_P4:
> +	case H_P5:
> +	case H_P6:
> +		err = -EINVAL;
> +		break;
> +	case H_NOT_FOUND:
> +		err = -ENOENT;
> +		break;
> +	case H_BUSY:
> +		err = -EBUSY;
> +		break;
> +	case H_AUTHORITY:
> +		err = -EPERM;
> +		break;
> +	case H_NO_MEM:
> +		err = -ENOMEM;
> +		break;
> +	case H_RESOURCE:
> +		err = -EEXIST;
> +		break;
> +	case H_TOO_BIG:
> +		err = -EFBIG;
> +		break;
> +	case H_STATE:
> +		err = -EIO;
> +		break;
> +	case H_R_STATE:
> +		err = -EIO;
> +		break;
> +	case H_IN_USE:
> +		err = -EEXIST;
> +		break;
> +	case H_ABORTED:
> +		err = -EINTR;
> +		break;
> +	default:
> +		err = -EINVAL;
> +	}
> +
> +	return err;
> +}
> +
> +static int plpks_gen_password(void)
> +{
> +	unsigned long retbuf[PLPAR_HCALL_BUFSIZE] = { 0 };
> +	u8 *password, consumer = PKS_OS_OWNER;
> +	int rc;
> +
> +	password = kzalloc(maxpwsize, GFP_KERNEL);
> +	if (!password)
> +		return -ENOMEM;
> +
> +	rc = plpar_hcall(H_PKS_GEN_PASSWORD, retbuf, consumer, 0,
> +			 virt_to_phys(password), maxpwsize);
> +
> +	if (!rc) {
> +		ospasswordlength = maxpwsize;
> +		ospassword = kzalloc(maxpwsize, GFP_KERNEL);
> +		if (!ospassword) {
> +			kfree(password);
> +			return -ENOMEM;
> +		}
> +		memcpy(ospassword, password, ospasswordlength);
> +	} else {
> +		if (rc == H_IN_USE) {
> +			pr_warn("Password is already set for POWER LPAR
> Platform KeyStore\n");
> +			rc = 0;
> +		} else {
> +			goto out;
> +		}
> +	}
> +out:
> +	kfree(password);
> +
> +	return pseries_status_to_err(rc);
> +}
> +
> +static struct plpks_auth *construct_auth(u8 consumer)
> +{
> +	struct plpks_auth *auth;
> +
> +	if (consumer > PKS_OS_OWNER)
> +		return ERR_PTR(-EINVAL);
> +
> +	auth = kmalloc(struct_size(auth, password, maxpwsize),
> GFP_KERNEL);
> +	if (!auth)
> +		return ERR_PTR(-ENOMEM);
> +
> +	auth->version = 1;
> +	auth->consumer = consumer;
> +	auth->rsvd0 = 0;
> +	auth->rsvd1 = 0;
> +
> +	if (consumer == PKS_FW_OWNER || consumer ==
> PKS_BOOTLOADER_OWNER) {
> +		auth->passwordlength = 0;
> +		return auth;
> +	}
> +
> +	memcpy(auth->password, ospassword, ospasswordlength);
> +
> +	auth->passwordlength = cpu_to_be16(ospasswordlength);
> +
> +	return auth;
> +}
> +
> +/**
> + * Label is combination of label attributes + name.
> + * Label attributes are used internally by kernel and not exposed to
> the user.
> + */
> +static struct label *construct_label(char *component, u8 varos, u8
> *name,
> +				     u16 namelen)
> +{
> +	struct label *label;
> +	size_t slen;
> +
> +	if (!name || namelen > MAX_NAME_SIZE)
> +		return ERR_PTR(-EINVAL);
> +
> +	slen = strlen(component);
> +	if (component && slen > sizeof(label->attr.prefix))
> +		return ERR_PTR(-EINVAL);
> +
> +	label = kzalloc(sizeof(*label), GFP_KERNEL);
> +	if (!label)
> +		return ERR_PTR(-ENOMEM);
> +
> +	if (component)
> +		memcpy(&label->attr.prefix, component, slen);
> +
> +	label->attr.version = LABEL_VERSION;
> +	label->attr.os = varos;
> +	label->attr.length = MAX_LABEL_ATTR_SIZE;
> +	memcpy(&label->name, name, namelen);
> +
> +	label->size = sizeof(struct label_attr) + namelen;
> +
> +	return label;
> +}
> +
> +static int _plpks_get_config(void)
> +{
> +	unsigned long retbuf[PLPAR_HCALL_BUFSIZE] = { 0 };
> +	struct {
> +		u8 version;
> +		u8 flags;
> +		__be32 rsvd0;
> +		__be16 maxpwsize;
> +		__be16 maxobjlabelsize;
> +		__be16 maxobjsize;
> +		__be32 totalsize;
> +		__be32 usedspace;
> +		__be32 supportedpolicies;
> +		__be64 rsvd1;
> +	} __packed config;
> +	size_t size;
> +	int rc;
> +
> +	size = sizeof(config);
> +
> +	rc = plpar_hcall(H_PKS_GET_CONFIG, retbuf,
> virt_to_phys(&config), size);
> +
> +	if (rc != H_SUCCESS)
> +		return pseries_status_to_err(rc);
> +
> +	maxpwsize = be16_to_cpu(config.maxpwsize);
> +	maxobjsize = be16_to_cpu(config.maxobjsize);
> +
> +	return 0;
> +}
> +
> +static int plpks_confirm_object_flushed(struct label *label,
> +					struct plpks_auth *auth)
> +{
> +	unsigned long retbuf[PLPAR_HCALL_BUFSIZE] = { 0 };
> +	u64 timeout = 0;
> +	u8 status;
> +	int rc;
> +
> +	do {
> +		rc = plpar_hcall(H_PKS_CONFIRM_OBJECT_FLUSHED, retbuf,
> +				 virt_to_phys(auth),
> virt_to_phys(label),
> +				 label->size);
> +
> +		status = retbuf[0];
> +		if (rc) {
> +			if (rc == H_NOT_FOUND && status == 1)
> +				rc = 0;
> +			break;
> +		}
> +
> +		if (!rc && status == 1)
> +			break;
> +
> +		usleep_range(PKS_FLUSH_SLEEP,
> +			     PKS_FLUSH_SLEEP + PKS_FLUSH_SLEEP_RANGE);
> +		timeout = timeout + PKS_FLUSH_SLEEP;
> +	} while (timeout < PKS_FLUSH_MAX_TIMEOUT);
> +
> +	rc = pseries_status_to_err(rc);
> +
> +	return rc;
> +}
> +
> +int plpks_write_var(struct plpks_var var)
> +{
> +	unsigned long retbuf[PLPAR_HCALL_BUFSIZE] = { 0 };
> +	struct plpks_auth *auth;
> +	struct label *label;
> +	int rc;
> +
> +	if (!var.component || !var.data || var.datalen <= 0 ||
> +	    var.namelen > MAX_NAME_SIZE || var.datalen > MAX_DATA_SIZE)
> +		return -EINVAL;
> +
> +	if (var.policy & SIGNEDUPDATE)
> +		return -EINVAL;
> +
> +	auth = construct_auth(PKS_OS_OWNER);
> +	if (IS_ERR(auth))
> +		return PTR_ERR(auth);
> +
> +	label = construct_label(var.component, var.os, var.name,
> var.namelen);
> +	if (IS_ERR(label)) {
> +		rc = PTR_ERR(label);
> +		goto out;
> +	}
> +
> +	rc = plpar_hcall(H_PKS_WRITE_OBJECT, retbuf,
> virt_to_phys(auth),
> +			 virt_to_phys(label), label->size, var.policy,
> +			 virt_to_phys(var.data), var.datalen);
> +
> +	if (!rc)
> +		rc = plpks_confirm_object_flushed(label, auth);
> +
> +	if (rc)
> +		pr_err("Failed to write variable %s for component %s
> with error %d\n",
> +		       var.name, var.component, rc);
> +
> +	rc = pseries_status_to_err(rc);
> +	kfree(label);
> +out:
> +	kfree(auth);
> +
> +	return rc;
> +}
> +
> +int plpks_remove_var(char *component, u8 varos, struct
> plpks_var_name vname)
> +{
> +	unsigned long retbuf[PLPAR_HCALL_BUFSIZE] = { 0 };
> +	struct plpks_auth *auth;
> +	struct label *label;
> +	int rc;
> +
> +	if (!component || vname.namelen > MAX_NAME_SIZE)
> +		return -EINVAL;
> +
> +	auth = construct_auth(PKS_OS_OWNER);
> +	if (IS_ERR(auth))
> +		return PTR_ERR(auth);
> +
> +	label = construct_label(component, varos, vname.name,
> vname.namelen);
> +	if (IS_ERR(label)) {
> +		rc = PTR_ERR(label);
> +		goto out;
> +	}
> +
> +	rc = plpar_hcall(H_PKS_REMOVE_OBJECT, retbuf,
> virt_to_phys(auth),
> +			 virt_to_phys(label), label->size);
> +
> +	if (!rc)
> +		rc = plpks_confirm_object_flushed(label, auth);
> +
> +	if (rc)
> +		pr_err("Failed to remove variable %s for component %s
> with error %d\n",
> +		       vname.name, component, rc);
> +
> +	rc = pseries_status_to_err(rc);
> +	kfree(label);
> +out:
> +	kfree(auth);
> +
> +	return rc;
> +}
> +
> +static int plpks_read_var(u8 consumer, struct plpks_var *var)
> +{
> +	unsigned long retbuf[PLPAR_HCALL_BUFSIZE] = { 0 };
> +	struct plpks_auth *auth;
> +	struct label *label;
> +	u8 *output;
> +	int rc;
> +
> +	if (var->namelen > MAX_NAME_SIZE)
> +		return -EINVAL;
> +
> +	auth = construct_auth(PKS_OS_OWNER);
> +	if (IS_ERR(auth))
> +		return PTR_ERR(auth);
> +
> +	label = construct_label(var->component, var->os, var->name,
> +				var->namelen);
> +	if (IS_ERR(label)) {
> +		rc = PTR_ERR(label);
> +		goto out_free_auth;
> +	}
> +
> +	output = kzalloc(maxobjsize, GFP_KERNEL);
> +	if (!output) {
> +		rc = -ENOMEM;
> +		goto out_free_label;
> +	}
> +
> +	rc = plpar_hcall(H_PKS_READ_OBJECT, retbuf, virt_to_phys(auth),
> +			 virt_to_phys(label), label->size,
> virt_to_phys(output),
> +			 maxobjsize);
> +
> +	if (rc != H_SUCCESS) {
> +		pr_err("Failed to read variable %s for component %s
> with error %d\n",
> +		       var->name, var->component, rc);
> +		rc = pseries_status_to_err(rc);
> +		goto out_free_output;
> +	}
> +
> +	if (var->datalen == 0 || var->datalen > retbuf[0])
> +		var->datalen = retbuf[0];
> +
> +	var->data = kzalloc(var->datalen, GFP_KERNEL);
> +	if (!var->data) {
> +		rc = -ENOMEM;
> +		goto out_free_output;
> +	}
> +	var->policy = retbuf[1];
> +
> +	memcpy(var->data, output, var->datalen);
> +	rc = 0;
> +
> +out_free_output:
> +	kfree(output);
> +out_free_label:
> +	kfree(label);
> +out_free_auth:
> +	kfree(auth);
> +
> +	return rc;
> +}
> +
> +int plpks_read_os_var(struct plpks_var *var)
> +{
> +	return plpks_read_var(PKS_OS_OWNER, var);
> +}
> +
> +int plpks_read_fw_var(struct plpks_var *var)
> +{
> +	return plpks_read_var(PKS_FW_OWNER, var);
> +}
> +
> +int plpks_read_bootloader_var(struct plpks_var *var)
> +{
> +	return plpks_read_var(PKS_BOOTLOADER_OWNER, var);
> +}
> +
> +static __init int pseries_plpks_init(void)
> +{
> +	int rc;
> +
> +	rc = _plpks_get_config();
> +
> +	if (rc) {
> +		pr_err("POWER LPAR Platform KeyStore is not supported
> or enabled\n");
> +		return rc;
> +	}
> +
> +	rc = plpks_gen_password();
> +	if (rc)
> +		pr_err("Failed setting POWER LPAR Platform KeyStore
> Password\n");
> +	else
> +		pr_info("POWER LPAR Platform KeyStore initialized
> successfully\n");
> +
> +	return rc;
> +}
> +arch_initcall(pseries_plpks_init);
> diff --git a/arch/powerpc/platforms/pseries/plpks.h
> b/arch/powerpc/platforms/pseries/plpks.h
> new file mode 100644
> index 000000000000..c6a291367bb1
> --- /dev/null
> +++ b/arch/powerpc/platforms/pseries/plpks.h
> @@ -0,0 +1,71 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2022 IBM Corporation
> + * Author: Nayna Jain <nayna@linux.ibm.com>
> + *
> + * Platform keystore for pseries LPAR(PLPKS).
> + */
> +
> +#ifndef _PSERIES_PLPKS_H
> +#define _PSERIES_PLPKS_H
> +
> +#include <linux/types.h>
> +#include <linux/list.h>
> +
> +#define OSSECBOOTAUDIT 0x40000000
> +#define OSSECBOOTENFORCE 0x20000000
> +#define WORLDREADABLE 0x08000000
> +#define SIGNEDUPDATE 0x01000000
> +
> +#define PLPKS_VAR_LINUX	0x01
> +#define PLPKS_VAR_COMMON	0x04
> +
> +struct plpks_var {
> +	char *component;
> +	u8 *name;
> +	u8 *data;
> +	u32 policy;
> +	u16 namelen;
> +	u16 datalen;
> +	u8 os;
> +};
> +
> +struct plpks_var_name {
> +	u8  *name;
> +	u16 namelen;
> +};
> +
> +struct plpks_var_name_list {
> +	u32 varcount;
> +	struct plpks_var_name varlist[];
> +};
> +
> +/**
> + * Writes the specified var and its data to PKS.
> + * Any caller of PKS driver should present a valid component type
> for
> + * their variable.
> + */
> +int plpks_write_var(struct plpks_var var);
> +
> +/**
> + * Removes the specified var and its data from PKS.
> + */
> +int plpks_remove_var(char *component, u8 varos,
> +		     struct plpks_var_name vname);
> +
> +/**
> + * Returns the data for the specified os variable.
> + */
> +int plpks_read_os_var(struct plpks_var *var);
> +
> +/**
> + * Returns the data for the specified firmware variable.
> + */
> +int plpks_read_fw_var(struct plpks_var *var);
> +
> +/**
> + * Returns the data for the specified bootloader variable.
> + */
> +int plpks_read_bootloader_var(struct plpks_var *var);
> +
> +#endif


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 0/3] Provide PowerVM LPAR Platform KeyStore driver for Self Encrypting Drives
  2022-07-23 11:30 [PATCH v2 0/3] Provide PowerVM LPAR Platform KeyStore driver for Self Encrypting Drives Nayna Jain
                   ` (2 preceding siblings ...)
  2022-07-23 11:30 ` [PATCH v2 3/3] powerpc/pseries: Override lib/arch_vars.c with PowerPC architecture specific version Nayna Jain
@ 2022-07-29 13:02 ` Michael Ellerman
  3 siblings, 0 replies; 10+ messages in thread
From: Michael Ellerman @ 2022-07-29 13:02 UTC (permalink / raw)
  To: Nayna Jain, linuxppc-dev; +Cc: erichte, George Wilson, gjoyce, npiggin, muriloo

On Sat, 23 Jul 2022 07:30:45 -0400, Nayna Jain wrote:
> PowerVM provides an isolated Platform KeyStore(PKS)[1] storage allocation
> for each partition(LPAR) with individually managed access controls to store
> sensitive information securely. The Linux Kernel can access this storage by
> interfacing with the hypervisor using a new set of hypervisor calls.
> 
> This storage can be used for multiple purposes. The current two usecases
> are:
> 
> [...]

Patch 1 applied to powerpc/next.

[1/3] powerpc/pseries: define driver for Platform KeyStore
      https://git.kernel.org/powerpc/c/2454a7af0f2a42918aa972147a0bec38e6656cd8

cheers

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/3] powerpc/pseries: define driver for Platform KeyStore
  2022-07-23 11:30 ` [PATCH v2 1/3] powerpc/pseries: define driver for Platform KeyStore Nayna Jain
  2022-07-28 14:14   ` Greg Joyce
@ 2022-09-06 21:00   ` Nathan Chancellor
  2022-09-06 23:23     ` Michael Ellerman
  1 sibling, 1 reply; 10+ messages in thread
From: Nathan Chancellor @ 2022-09-06 21:00 UTC (permalink / raw)
  To: Nayna Jain
  Cc: bjking1, gjoyce, erichte, npiggin, muriloo, George Wilson, linuxppc-dev

Hi all,

On Sat, Jul 23, 2022 at 07:30:46AM -0400, Nayna Jain wrote:
> PowerVM provides an isolated Platform Keystore(PKS) storage allocation
> for each LPAR with individually managed access controls to store
> sensitive information securely. It provides a new set of hypervisor
> calls for Linux kernel to access PKS storage.
> 
> Define POWER LPAR Platform KeyStore(PLPKS) driver using H_CALL interface
> to access PKS storage.
> 
> Signed-off-by: Nayna Jain <nayna@linux.ibm.com>

This commit is now in mainline as commit 2454a7af0f2a ("powerpc/pseries:
define driver for Platform KeyStore") and I just bisected a crash while
boot testing Fedora's configuration [1] in QEMU to it. I initially
noticed this in ClangBuiltLinux's CI but this doesn't appear to be clang
specific since I can reproduce with GCC 12.2.1 from Fedora. I can
reproduce with just powernv_defconfig + CONFIG_PPC_PSERIES=y +
CONFIG_PSERIES_PLPKS=y. Our firmware and rootfs are available in our
boot-utils repository [2].

$ qemu-system-ppc64 \
-device ipmi-bmc-sim,id=bmc0 \
-device isa-ipmi-bt,bmc=bmc0,irq=10 \
-L .../boot-utils/images/ppc64le \
-bios skiboot.lid \
-machine powernv8 \
-kernel arch/powerpc/boot/zImage.epapr \
-display none \
-initrd .../boot-utils/images/ppc64le/rootfs.cpio \
-m 2G \
-nodefaults \
-no-reboot \
-serial mon:stdio
...
[    0.000000][    T0] Linux version 5.19.0-rc2-00179-g2454a7af0f2a (tuxmake@tuxmake) (powerpc64le-linux-gnu-gcc (GCC) 12.2.1 20220819 (Red Hat Cross 12.2.1-1), GNU ld version 2.38-4.fc37) #1 SMP @1658989333
...
[    0.144318][    T1] EEH: PowerNV platform initialized
[    0.145204][    T1] ------------[ cut here ]------------
[    0.145400][    T1] kernel BUG at arch/powerpc/kernel/interrupt.c:96!
[    0.145674][    T1] Oops: Exception in kernel mode, sig: 5 [#1]
[    0.147691][    T1] LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA PowerNV
[    0.148177][    T1] Modules linked in:
[    0.148619][    T1] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.19.0-rc2-00179-g2454a7af0f2a #1
[    0.149328][    T1] NIP:  c00000000002ea2c LR: c00000000000c63c CTR: c00000000000c540
[    0.149851][    T1] REGS: c000000002a03b10 TRAP: 0700   Not tainted  (5.19.0-rc2-00179-g2454a7af0f2a)
[    0.150478][    T1] MSR:  9000000000029033 <SF,HV,EE,ME,IR,DR,RI,LE>  CR: 24000282  XER: 20000000
[    0.151240][    T1] CFAR: c00000000000c638 IRQMASK: 3
[    0.151240][    T1] GPR00: c00000000000c63c c000000002a03db0 c00000000240ba00 000000000000041c
[    0.151240][    T1] GPR04: 0000000002a03b98 0000000000000020 000000007dcf0000 0000000000000000
[    0.151240][    T1] GPR08: 0000000000000000 0000000000000000 0000000000000001 0000000000000003
[    0.151240][    T1] GPR12: ffffffffffffffff c0000000025c0000 c0000000000125f8 0000000000000000
[    0.151240][    T1] GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[    0.151240][    T1] GPR20: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[    0.151240][    T1] GPR24: 0000000000000000 0000000000000000 000000007dcf0000 0000000000000020
[    0.151240][    T1] GPR28: 0000000002a03b98 000000000000041c 0000000024000282 c000000002a03e80
[    0.156459][    T1] NIP [c00000000002ea2c] system_call_exception+0x7c/0x370
[    0.157366][    T1] LR [c00000000000c63c] system_call_common+0xec/0x250
[    0.157991][    T1] Call Trace:
[    0.158255][    T1] [c000000002a03db0] [c000000000012620] kernel_init+0x30/0x1a0 (unreliable)
[    0.158936][    T1] [c000000002a03e10] [c00000000000c63c] system_call_common+0xec/0x250
[    0.159514][    T1] --- interrupt: c00 at plpar_hcall+0x38/0x60
[    0.159956][    T1] NIP:  c0000000000d4bc0 LR: c000000002021664 CTR: 0000000000000000
[    0.160469][    T1] REGS: c000000002a03e80 TRAP: 0c00   Not tainted  (5.19.0-rc2-00179-g2454a7af0f2a)
[    0.161068][    T1] MSR:  9000000002009033 <SF,HV,VEC,EE,ME,IR,DR,RI,LE>  CR: 24000282  XER: 00000000
[    0.161792][    T1] IRQMASK: 0
[    0.161792][    T1] GPR00: 0000000024000282 c000000002a03b30 c00000000240ba00 000000000000041c
[    0.161792][    T1] GPR04: 0000000002a03b98 0000000000000020 000000007dcf0000 0000000000000000
[    0.161792][    T1] GPR08: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[    0.161792][    T1] GPR12: 0000000000000000 c0000000025c0000 c0000000000125f8 0000000000000000
[    0.161792][    T1] GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[    0.161792][    T1] GPR20: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[    0.161792][    T1] GPR24: 0000000000000000 c00000000200015c cccccccccccccccd c000000002071048
[    0.161792][    T1] GPR28: 0000000000000000 c000000002071088 0000000000000003 c0000000020215f0
[    0.166796][    T1] NIP [c0000000000d4bc0] plpar_hcall+0x38/0x60
[    0.167215][    T1] LR [c000000002021664] pseries_plpks_init+0x74/0x268
[    0.167705][    T1] --- interrupt: c00
[    0.167959][    T1] [c000000002a03b30] [000000007dcf0000] 0x7dcf0000 (unreliable)
[    0.168763][    T1] Instruction dump:
[    0.169099][    T1] f8010010 f821ffa1 60000000 fbbf0110 39200000 0b090000 e95f0108 69490002
[    0.169741][    T1] 7929ffe2 0b090000 694a4000 794a97e2 <0b0a0000> e93f0138 792907e0 0b090000
[    0.170731][    T1] ---[ end trace 0000000000000000 ]---
...

If there is any more information I can provide or patches I can test, I
am more than happy to do so (although I may be slower to respond through
Plumbers).

[1]: https://src.fedoraproject.org/rpms/kernel/raw/rawhide/f/kernel-ppc64le-fedora.config
[2]: https://github.com/ClangBuiltLinux/boot-utils

Cheers,
Nathan

# bad: [568035b01cfb107af8d2e4bd2fb9aea22cf5b868] Linux 6.0-rc1
# good: [3d7cb6b04c3f3115719235cc6866b10326de34cd] Linux 5.19
git bisect start 'v6.0-rc1' 'v5.19'
# good: [b44f2fd87919b5ae6e1756d4c7ba2cbba22238e1] Merge tag 'drm-next-2022-08-03' of git://anongit.freedesktop.org/drm/drm
git bisect good b44f2fd87919b5ae6e1756d4c7ba2cbba22238e1
# good: [6614a3c3164a5df2b54abb0b3559f51041cf705b] Merge tag 'mm-stable-2022-08-03' of git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
git bisect good 6614a3c3164a5df2b54abb0b3559f51041cf705b
# bad: [eb5699ba31558bdb2cee6ebde3d0a68091e47dce] Merge tag 'mm-nonmm-stable-2022-08-06-2' of git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
git bisect bad eb5699ba31558bdb2cee6ebde3d0a68091e47dce
# good: [24df5428ef9d1ca1edd54eca7eb667110f2dfae3] ALSA: hda/realtek: Add quirk for HP Spectre x360 15-eb0xxx
git bisect good 24df5428ef9d1ca1edd54eca7eb667110f2dfae3
# good: [c993e07be023acdeec8e84e2e0743c52adb5fc94] Merge tag 'dma-mapping-5.20-2022-08-06' of git://git.infradead.org/users/hch/dma-mapping
git bisect good c993e07be023acdeec8e84e2e0743c52adb5fc94
# bad: [4cfa6ff24a9744ba484521c38bea613134fbfcb3] powerpc/64e: Fix kexec build error
git bisect bad 4cfa6ff24a9744ba484521c38bea613134fbfcb3
# good: [78988b273d592ce74c8aecdd5d748906c38a9e9d] powerpc/perf: Give generic PMU a nice name
git bisect good 78988b273d592ce74c8aecdd5d748906c38a9e9d
# good: [de40303b54bc458d7df0d4b4ee1d296df7fe98c7] powerpc/ppc-opcode: Define and use PPC_RAW_SETB()
git bisect good de40303b54bc458d7df0d4b4ee1d296df7fe98c7
# bad: [738f9dca0df3bb630e6f06a19573ab4e31bd443a] powerpc/sysdev: Fix comment typo
git bisect bad 738f9dca0df3bb630e6f06a19573ab4e31bd443a
# good: [4d5c5bad51935482437528f7fa4dffdcb3330d8b] powerpc: Remove asm/prom.h from asm/mpc52xx.h and asm/pci.h
git bisect good 4d5c5bad51935482437528f7fa4dffdcb3330d8b
# good: [d80f6de9d601c30b53c17f00cb7cfe3169f2ddad] powerpc/iommu: Fix iommu_table_in_use for a small default DMA window case
git bisect good d80f6de9d601c30b53c17f00cb7cfe3169f2ddad
# bad: [0fe1e96fef0a5c53b4c0d1500d356f3906000f81] powerpc/pci: Prefer PCI domain assignment via DT 'linux,pci-domain' and alias
git bisect bad 0fe1e96fef0a5c53b4c0d1500d356f3906000f81
# bad: [d20c96deb3e2c1cedc47d2be9fc110ffed81b1af] powerpc/85xx: Fix description of MPC85xx and P1/P2 boards options
git bisect bad d20c96deb3e2c1cedc47d2be9fc110ffed81b1af
# bad: [2454a7af0f2a42918aa972147a0bec38e6656cd8] powerpc/pseries: define driver for Platform KeyStore
git bisect bad 2454a7af0f2a42918aa972147a0bec38e6656cd8
# first bad commit: [2454a7af0f2a42918aa972147a0bec38e6656cd8] powerpc/pseries: define driver for Platform KeyStore

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/3] powerpc/pseries: define driver for Platform KeyStore
  2022-09-06 21:00   ` Nathan Chancellor
@ 2022-09-06 23:23     ` Michael Ellerman
  2022-09-06 23:32       ` Nathan Chancellor
  0 siblings, 1 reply; 10+ messages in thread
From: Michael Ellerman @ 2022-09-06 23:23 UTC (permalink / raw)
  To: Nathan Chancellor, Nayna Jain
  Cc: bjking1, gjoyce, erichte, npiggin, muriloo, George Wilson, linuxppc-dev

Nathan Chancellor <nathan@kernel.org> writes:
> Hi all,
>
> On Sat, Jul 23, 2022 at 07:30:46AM -0400, Nayna Jain wrote:
>> PowerVM provides an isolated Platform Keystore(PKS) storage allocation
>> for each LPAR with individually managed access controls to store
>> sensitive information securely. It provides a new set of hypervisor
>> calls for Linux kernel to access PKS storage.
>> 
>> Define POWER LPAR Platform KeyStore(PLPKS) driver using H_CALL interface
>> to access PKS storage.
>> 
>> Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
>
> This commit is now in mainline as commit 2454a7af0f2a ("powerpc/pseries:
> define driver for Platform KeyStore") and I just bisected a crash while
> boot testing Fedora's configuration [1] in QEMU to it. I initially
> noticed this in ClangBuiltLinux's CI but this doesn't appear to be clang
> specific since I can reproduce with GCC 12.2.1 from Fedora. I can
> reproduce with just powernv_defconfig + CONFIG_PPC_PSERIES=y +
> CONFIG_PSERIES_PLPKS=y. Our firmware and rootfs are available in our
> boot-utils repository [2].

Thanks, classic bug I should have spotted.

I didn't catch it in my testing because PLPKS isn't enabled in
our defconfigs.

Does your CI enable new options by default? Or are you booting
allyesconfig?

I'll send a fix.

cheers

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/3] powerpc/pseries: define driver for Platform KeyStore
  2022-09-06 23:23     ` Michael Ellerman
@ 2022-09-06 23:32       ` Nathan Chancellor
  2022-09-07  8:39         ` Michael Ellerman
  0 siblings, 1 reply; 10+ messages in thread
From: Nathan Chancellor @ 2022-09-06 23:32 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: bjking1, gjoyce, erichte, Nayna Jain, npiggin, muriloo,
	George Wilson, linuxppc-dev

On Wed, Sep 07, 2022 at 09:23:02AM +1000, Michael Ellerman wrote:
> Nathan Chancellor <nathan@kernel.org> writes:
> > Hi all,
> >
> > On Sat, Jul 23, 2022 at 07:30:46AM -0400, Nayna Jain wrote:
> >> PowerVM provides an isolated Platform Keystore(PKS) storage allocation
> >> for each LPAR with individually managed access controls to store
> >> sensitive information securely. It provides a new set of hypervisor
> >> calls for Linux kernel to access PKS storage.
> >> 
> >> Define POWER LPAR Platform KeyStore(PLPKS) driver using H_CALL interface
> >> to access PKS storage.
> >> 
> >> Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
> >
> > This commit is now in mainline as commit 2454a7af0f2a ("powerpc/pseries:
> > define driver for Platform KeyStore") and I just bisected a crash while
> > boot testing Fedora's configuration [1] in QEMU to it. I initially
> > noticed this in ClangBuiltLinux's CI but this doesn't appear to be clang
> > specific since I can reproduce with GCC 12.2.1 from Fedora. I can
> > reproduce with just powernv_defconfig + CONFIG_PPC_PSERIES=y +
> > CONFIG_PSERIES_PLPKS=y. Our firmware and rootfs are available in our
> > boot-utils repository [2].
> 
> Thanks, classic bug I should have spotted.
> 
> I didn't catch it in my testing because PLPKS isn't enabled in
> our defconfigs.
> 
> Does your CI enable new options by default? Or are you booting
> allyesconfig?

Neither actually. We just test a bunch of in-tree and distribution
configurations. The distribution configurations are fetched straight
from their URLs on gitweb so we get any updates that they do, which is
how we noticed this (CONFIG_PSERIES_PLPKS was recently enabled in
Fedora):

https://src.fedoraproject.org/rpms/kernel/c/a73f6858a2cbd16bbcc6d305d6c43aab6f59d0b1

> I'll send a fix.

Thanks for the quick response!

Cheers,
Nathan

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/3] powerpc/pseries: define driver for Platform KeyStore
  2022-09-06 23:32       ` Nathan Chancellor
@ 2022-09-07  8:39         ` Michael Ellerman
  0 siblings, 0 replies; 10+ messages in thread
From: Michael Ellerman @ 2022-09-07  8:39 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: bjking1, gjoyce, erichte, Nayna Jain, npiggin, muriloo,
	George Wilson, linuxppc-dev

Nathan Chancellor <nathan@kernel.org> writes:
> On Wed, Sep 07, 2022 at 09:23:02AM +1000, Michael Ellerman wrote:
>> Nathan Chancellor <nathan@kernel.org> writes:
>> > On Sat, Jul 23, 2022 at 07:30:46AM -0400, Nayna Jain wrote:
>> >> PowerVM provides an isolated Platform Keystore(PKS) storage allocation
>> >> for each LPAR with individually managed access controls to store
>> >> sensitive information securely. It provides a new set of hypervisor
>> >> calls for Linux kernel to access PKS storage.
>> >> 
>> >> Define POWER LPAR Platform KeyStore(PLPKS) driver using H_CALL interface
>> >> to access PKS storage.
>> >> 
>> >> Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
>> >
>> > This commit is now in mainline as commit 2454a7af0f2a ("powerpc/pseries:
>> > define driver for Platform KeyStore") and I just bisected a crash while
>> > boot testing Fedora's configuration [1] in QEMU to it. I initially
>> > noticed this in ClangBuiltLinux's CI but this doesn't appear to be clang
>> > specific since I can reproduce with GCC 12.2.1 from Fedora. I can
>> > reproduce with just powernv_defconfig + CONFIG_PPC_PSERIES=y +
>> > CONFIG_PSERIES_PLPKS=y. Our firmware and rootfs are available in our
>> > boot-utils repository [2].
>> 
>> Thanks, classic bug I should have spotted.
>> 
>> I didn't catch it in my testing because PLPKS isn't enabled in
>> our defconfigs.
>> 
>> Does your CI enable new options by default? Or are you booting
>> allyesconfig?
>
> Neither actually. We just test a bunch of in-tree and distribution
> configurations. The distribution configurations are fetched straight
> from their URLs on gitweb so we get any updates that they do, which is
> how we noticed this (CONFIG_PSERIES_PLPKS was recently enabled in
> Fedora):
>
> https://src.fedoraproject.org/rpms/kernel/c/a73f6858a2cbd16bbcc6d305d6c43aab6f59d0b1

Aha, neat trick.

>> I'll send a fix.
>
> Thanks for the quick response!

Thanks for the bug report :)

cheers

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2022-09-07  8:40 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-23 11:30 [PATCH v2 0/3] Provide PowerVM LPAR Platform KeyStore driver for Self Encrypting Drives Nayna Jain
2022-07-23 11:30 ` [PATCH v2 1/3] powerpc/pseries: define driver for Platform KeyStore Nayna Jain
2022-07-28 14:14   ` Greg Joyce
2022-09-06 21:00   ` Nathan Chancellor
2022-09-06 23:23     ` Michael Ellerman
2022-09-06 23:32       ` Nathan Chancellor
2022-09-07  8:39         ` Michael Ellerman
2022-07-23 11:30 ` [PATCH v2 2/3] lib: define generic accessor functions for arch specific keystore Nayna Jain
2022-07-23 11:30 ` [PATCH v2 3/3] powerpc/pseries: Override lib/arch_vars.c with PowerPC architecture specific version Nayna Jain
2022-07-29 13:02 ` [PATCH v2 0/3] Provide PowerVM LPAR Platform KeyStore driver for Self Encrypting Drives Michael Ellerman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.