Re: [PATCH 1/2] powerpc/pseries: define driver for Platform KeyStore
Hi, Nayna.
Some comments below.
On 7/12/22 21:59, 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.
I think it would be nice to have some consistency as to spaces before
acronymous, e.g.: "Keystore(PKS)" vs. "Keystore (PKS)". The same could
be applied to "Keystore" vs. "KeyStore" vs. "Key Storage".
Define PLPKS driver using H_CALL interface to access PKS storage.
Signed-off-by: Nayna Jain
---
arch/powerpc/include/asm/hvcall.h | 9 +
arch/powerpc/include/asm/plpks.h | 90
arch/powerpc/platforms/pseries/Kconfig| 13 +
arch/powerpc/platforms/pseries/Makefile | 2 +
arch/powerpc/platforms/pseries/plpks/Makefile | 7 +
arch/powerpc/platforms/pseries/plpks/plpks.c | 509 ++
6 files changed, 630 insertions(+)
create mode 100644 arch/powerpc/include/asm/plpks.h
create mode 100644 arch/powerpc/platforms/pseries/plpks/Makefile
create mode 100644 arch/powerpc/platforms/pseries/plpks/plpks.c
diff --git a/arch/powerpc/include/asm/hvcall.h
b/arch/powerpc/include/asm/hvcall.h
index d92a20a85395..24b661b0717c 100644
--- a/arch/powerpc/include/asm/hvcall.h
+++ b/arch/powerpc/include/asm/hvcall.h
@@ -97,6 +97,7 @@
#define H_OP_MODE -73
#define H_COP_HW -74
#define H_STATE -75
+#define H_IN_USE -77
#define H_UNSUPPORTED_FLAG_START -256
#define H_UNSUPPORTED_FLAG_END-511
#define H_MULTI_THREADS_ACTIVE-9005
@@ -321,6 +322,14 @@
#define H_SCM_UNBIND_ALL0x3FC
#define H_SCM_HEALTH0x400
#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_OBJECT0x438
+#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/include/asm/plpks.h b/arch/powerpc/include/asm/plpks.h
new file mode 100644
index ..cf60e53e1f15
--- /dev/null
+++ b/arch/powerpc/include/asm/plpks.h
@@ -0,0 +1,90 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2022 IBM Corporation
+ * Author: Nayna Jain
+ *
+ * Platform keystore for pseries LPAR(PLPKS).
+ */
+
+#ifndef _PSERIES_PLPKS_H
+#define _PSERIES_PLPKS_H
+
+#include
+#include
+
+#define OSSECBOOTAUDIT 0x4000
+#define OSSECBOOTENFORCE 0x2000
+#define WORLDREADABLE 0x0800
+#define SIGNEDUPDATE 0x0100
+
+#define PLPKS_VAR_LINUX0x01
+#define PLPKS_VAR_COMMON 0x04
+
+struct plpks_var {
+ char *component;
+ u8 os;
+ u8 *name;
+ u16 namelen;
+ u32 policy;
+ u16 datalen;
+ u8 *data;
+};
+
+struct plpks_var_name {
+ u16 namelen;
+ u8 *name;
+};
+
+struct plpks_var_name_list {
+ u32 varcount;
+ struct plpks_var_name varlist[];
+};
+
+struct plpks_config {
+ u8 version;
+ u8 flags;
+ u32 rsvd0;
+ u16 maxpwsize;
+ u16 maxobjlabelsize;
+ u16 maxobjsize;
+ u32 totalsize;
+ u32 usedspace;
+ u32 supportedpolicies;
+ u64 rsvd1;
+} __packed;
+
+/**
+ * Successful return from this API implies PKS is available.
+ * This is used to initialize kernel driver and user interfaces.
+ */
+struct plpks_config *plpks_get_config(void);
+
+/**
+ * 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
diff --git a/arch/powerpc/platforms/pseries/Kconfig
b/arch/powerpc/platforms/pseries/Kconfig
index f7fd91d153a4..de6efe5d18c2 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
+ tristate "Support for the Platform Key Storage"
+ help
+ PowerVM provides an isolated Platform Keystore(PKS) storage
+ allocation for each
Re: [PATCH 1/2] powerpc/pseries: define driver for Platform KeyStore
On 7/12/22 7:59 PM, 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 PLPKS driver using H_CALL interface to access PKS storage.
>
> Signed-off-by: Nayna Jain
> ---
> arch/powerpc/include/asm/hvcall.h | 9 +
> arch/powerpc/include/asm/plpks.h | 90
> arch/powerpc/platforms/pseries/Kconfig| 13 +
> arch/powerpc/platforms/pseries/Makefile | 2 +
> arch/powerpc/platforms/pseries/plpks/Makefile | 7 +
> arch/powerpc/platforms/pseries/plpks/plpks.c | 509 ++
> 6 files changed, 630 insertions(+)
> create mode 100644 arch/powerpc/include/asm/plpks.h
> create mode 100644 arch/powerpc/platforms/pseries/plpks/Makefile
> create mode 100644 arch/powerpc/platforms/pseries/plpks/plpks.c
>
> diff --git a/arch/powerpc/include/asm/hvcall.h
> b/arch/powerpc/include/asm/hvcall.h
> index d92a20a85395..24b661b0717c 100644
> --- a/arch/powerpc/include/asm/hvcall.h
> +++ b/arch/powerpc/include/asm/hvcall.h
> @@ -97,6 +97,7 @@
> #define H_OP_MODE-73
> #define H_COP_HW -74
> #define H_STATE -75
> +#define H_IN_USE -77
> #define H_UNSUPPORTED_FLAG_START -256
> #define H_UNSUPPORTED_FLAG_END -511
> #define H_MULTI_THREADS_ACTIVE -9005
> @@ -321,6 +322,14 @@
> #define H_SCM_UNBIND_ALL0x3FC
> #define H_SCM_HEALTH0x400
> #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_KEY0x430
> +#define H_PKS_READ_OBJECT0x434
> +#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/include/asm/plpks.h
> b/arch/powerpc/include/asm/plpks.h
> new file mode 100644
> index ..cf60e53e1f15
> --- /dev/null
> +++ b/arch/powerpc/include/asm/plpks.h
> @@ -0,0 +1,90 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2022 IBM Corporation
> + * Author: Nayna Jain
> + *
> + * Platform keystore for pseries LPAR(PLPKS).
> + */
> +
> +#ifndef _PSERIES_PLPKS_H
> +#define _PSERIES_PLPKS_H
> +
> +#include
> +#include
> +
> +#define OSSECBOOTAUDIT 0x4000
> +#define OSSECBOOTENFORCE 0x2000
> +#define WORLDREADABLE 0x0800
> +#define SIGNEDUPDATE 0x0100
> +
> +#define PLPKS_VAR_LINUX 0x01
> +#define PLPKS_VAR_COMMON 0x04
> +
> +struct plpks_var {
> + char *component;
> + u8 os;
> + u8 *name;
> + u16 namelen;
> + u32 policy;
> + u16 datalen;
> + u8 *data;
> +};
> +
> +struct plpks_var_name {
> + u16 namelen;
> + u8 *name;
> +};
> +
> +struct plpks_var_name_list {
> + u32 varcount;
> + struct plpks_var_name varlist[];
> +};
> +
> +struct plpks_config {
> + u8 version;
> + u8 flags;
> + u32 rsvd0;
> + u16 maxpwsize;
> + u16 maxobjlabelsize;
> + u16 maxobjsize;
> + u32 totalsize;
> + u32 usedspace;
> + u32 supportedpolicies;
> + u64 rsvd1;
> +} __packed;
> +
> +/**
> + * Successful return from this API implies PKS is available.
> + * This is used to initialize kernel driver and user interfaces.
> + */
> +struct plpks_config *plpks_get_config(void);
> +
> +/**
> + * 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
> diff --git a/arch/powerpc/platforms/pseries/Kconfig
> b/arch/powerpc/platforms/pseries/Kconfig
> index f7fd91d153a4..de6efe5d18c2 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
> + tristate "Support for the Platform Key Storage"
> + help
> + PowerVM provides an isolated Platform Keystore(PKS) storage
> + allocation for each LPAR with individually
Re: [PATCH 1/2] powerpc/pseries: define driver for Platform KeyStore
Comments inline.
Reviewed-by: Greg Joyce
From: Nayna Jain
Sent: Tuesday, July 12, 2022 7:59 PM
To: linuxppc-dev@lists.ozlabs.org
Cc: Michael Ellerman ; Benjamin Herrenschmidt
; Paul Mackerras ; George Wilson
; Gregory Joyce ; Nayna Jain
Subject: [PATCH 1/2] powerpc/pseries: define driver for Platform KeyStore
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 PLPKS driver using H_CALL interface to access PKS storage.
Signed-off-by: Nayna Jain
---
+
+static int construct_auth(u8 consumer, struct plpks_auth **auth)
+{
+ pr_debug("max password size is %u\n", config->maxpwsize);
Are the pr_debugs in this function still needed?
+
+ if (!auth || consumer > 3)
+ return -EINVAL;
+
+ *auth = kmalloc(struct_size(*auth, password, config->maxpwsize),
+ GFP_KERNEL);
+ if (!*auth)
+ return -ENOMEM;
+
+ (*auth)->version = 1;
+ (*auth)->consumer = consumer;
+ (*auth)->rsvd0 = 0;
+ (*auth)->rsvd1 = 0;
+ if (consumer == PKS_FW_OWNER || consumer == PKS_BOOTLOADER_OWNER) {
+ pr_debug("consumer is bootloader or firmware\n");
+ (*auth)->passwordlength = 0;
+ return 0;
+ }
+
+ (*auth)->passwordlength = (__force __be16)ospasswordlength;
+
+ memcpy((*auth)->password, ospassword,
+ flex_array_size(*auth, password,
+ (__force u16)((*auth)->passwordlength)));
+ (*auth)->passwordlength = cpu_to_be16((__force
u16)((*auth)->passwordlength));
+
+ return 0;
+}
+
+/**
+ * Label is combination of label attributes + name.
+ * Label attributes are used internally by kernel and not exposed to the user.
+ */
+static int construct_label(char *component, u8 varos, u8 *name, u16 namelen,
u8 **label)
+{
+ int varlen;
+ int len = 0;
+ int llen = 0;
+ int i;
+ int rc = 0;
+ u8 labellength = MAX_LABEL_ATTR_SIZE;
+
+ if (!label)
+ return -EINVAL;
+
+ varlen = namelen + sizeof(struct label_attr);
+ *label = kzalloc(varlen, GFP_KERNEL);
+
+ if (!*label)
+ return -ENOMEM;
+
+ if (component) {
+ len = strlen(component);
+ memcpy(*label, component, len);
+ }
+ llen = len;
+
I guess the 8, 1, and 5 are field lengths. Could they be a define or sizeof?
+ if (component)
+ len = 8 - strlen(component);
+ else
+ len = 8;
+
+ memset(*label + llen, 0, len);
+ llen = llen + len;
+
+ ((*label)[llen]) = 0;
+ llen = llen + 1;
+
+ memcpy(*label + llen, , 1);
+ llen = llen + 1;
+
+ memcpy(*label + llen, , 1);
+ llen = llen + 1;
+
+ memset(*label + llen, 0, 5);
+ llen = llen + 5;
+
+ memcpy(*label + llen, name, namelen);
+ llen = llen + namelen;
+
+ for (i = 0; i < llen; i++)
+ pr_debug("%c", (*label)[i]);
+
+ rc = llen;
+ return rc;
+}
+
+static int _plpks_get_config(void)
+{
+ unsigned long retbuf[PLPAR_HCALL_BUFSIZE] = {0};
+ int rc;
+ size_t size = sizeof(struct plpks_config);
+
+ config = kzalloc(size, GFP_KERNEL);
+ if (!config)
+ return -ENOMEM;
+
+ rc = plpar_hcall(H_PKS_GET_CONFIG,
+retbuf,
+virt_to_phys(config),
+size);
+
+ if (rc != H_SUCCESS)
Free config before returning the error.
+ return pseries_status_to_err(rc);
+
+ config->rsvd0 = be32_to_cpu((__force __be32)config->rsvd0);
+ config->maxpwsize = be16_to_cpu((__force __be16)config->maxpwsize);
+ config->maxobjlabelsize = be16_to_cpu((__force
__be16)config->maxobjlabelsize);
+ config->maxobjsize = be16_to_cpu((__force __be16)config->maxobjsize);
+ config->totalsize = be32_to_cpu((__force __be32)config->totalsize);
+ config->usedspace = be32_to_cpu((__force __be32)config->usedspace);
+ config->supportedpolicies = be32_to_cpu((__force
__be32)config->supportedpolicies);
+ config->rsvd1 = be64_to_cpu((__force __be64)config->rsvd1);
+
+ configset = true;
+
+ return 0;
+}
+
+static int plpks_confirm_object_flushed(u8 *label, u16 labellen)
+{
+ unsigned long retbuf[PLPAR_HCALL_BUFSIZE] = {0};
+ int rc;
+ u64 timeout = 0;
+ struct plpks_auth *auth;
+ u8 status;
+ int i;
+
Deleted pr_debugs? I guess this is a general question for all pr_debugs in this
file.
+ rc = construct_auth(PKS_OS_OWNER, );
+ if (rc)
+ return rc;
+
+ for (i = 0; i < labellen; i++)
+ pr_debug("%02x ", label[i]);
+
+ do {
+
Re: [PATCH 1/2] powerpc/pseries: define driver for Platform KeyStore
Tested-by: Greg Joyce From: Nayna Jain Sent: Tuesday, July 12, 2022 7:59 PM To: linuxppc-dev@lists.ozlabs.org Cc: Michael Ellerman ; Benjamin Herrenschmidt ; Paul Mackerras ; George Wilson ; Gregory Joyce ; Nayna Jain Subject: [PATCH 1/2] powerpc/pseries: define driver for Platform KeyStore 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 PLPKS driver using H_CALL interface to access PKS storage. Tested-by: Greg Joyce Signed-off-by: Nayna Jain
