linuxppc-dev.lists.ozlabs.org archive mirror
 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).