[PATCH 0/4] PLPKS bugfixes and enhancements

[PATCH 0/4] PLPKS bugfixes and enhancements

[Andrew Donnellan]

This series fixes a few miscellaneous bugs in the plpks driver, and adds some
additional internal APIs that will be used by some patches that are coming
imminently.

This supersedes Nayna's earlier patch at:
https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20221106210744.603240-2-nayna@linux.ibm.com/

Many thanks to Russell Currey and Ben Gray for their help on this series.

Andrew Donnellan (2):
  powerpc/pseries: Fix handling of PLPKS object flushing timeout
  powerpc/pseries: Fix alignment of PLPKS structures and buffers

Nayna Jain (2):
  powerpc/pseries: Expose PLPKS config values, support additional fields
  powerpc/pseries: Implement signed update for PLPKS objects

 arch/powerpc/include/asm/hvcall.h      |   3 +-
 arch/powerpc/platforms/pseries/plpks.c | 220 ++++++++++++++++++++++---
 arch/powerpc/platforms/pseries/plpks.h |  63 +++++++
 3 files changed, 259 insertions(+), 27 deletions(-)

-- 
2.38.1

[PATCH 1/4] powerpc/pseries: Fix handling of PLPKS object flushing timeout

[Andrew Donnellan]

plpks_confirm_object_flushed() uses the H_PKS_CONFIRM_OBJECT_FLUSHED hcall
to check whether changes to an object in the Platform KeyStore have been
flushed to non-volatile storage.

The hcall returns two output values, the return code and the flush status.
plpks_confirm_object_flushed() polls the hcall until either the flush
status has updated, the return code is an error, or a timeout has been
exceeded.

While we're still polling, the hcall is returning H_SUCCESS (0) as the
return code. In the timeout case, this means that upon exiting the polling
loop, rc is 0, and therefore 0 is returned to the user.

Handle the timeout case separately and return ETIMEDOUT if triggered.

Fixes: 2454a7af0f2a ("powerpc/pseries: define driver for Platform KeyStore")
Reported-by: Benjamin Gray <bgray@linux.ibm.com>
Signed-off-by: Andrew Donnellan <ajd@linux.ibm.com>
---
 arch/powerpc/platforms/pseries/plpks.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/plpks.c b/arch/powerpc/platforms/pseries/plpks.c
index 4edd1585e245..cb93062e8223 100644
--- a/arch/powerpc/platforms/pseries/plpks.c
+++ b/arch/powerpc/platforms/pseries/plpks.c
@@ -248,6 +248,7 @@ static int plpks_confirm_object_flushed(struct label *label,
 					struct plpks_auth *auth)
 {
 	unsigned long retbuf[PLPAR_HCALL_BUFSIZE] = { 0 };
+	bool timed_out = true;
 	u64 timeout = 0;
 	u8 status;
 	int rc;
@@ -259,20 +260,26 @@ static int plpks_confirm_object_flushed(struct label *label,
 
 		status = retbuf[0];
 		if (rc) {
+			timed_out = false;
 			if (rc == H_NOT_FOUND && status == 1)
 				rc = 0;
 			break;
 		}
 
-		if (!rc && status == 1)
+		if (!rc && status == 1) {
+			timed_out = false;
 			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);
+	if (timed_out)
+		rc = -ETIMEDOUT;
+	else
+		rc = pseries_status_to_err(rc);
 
 	return rc;
 }
-- 
2.38.1

[PATCH 2/4] powerpc/pseries: Fix alignment of PLPKS structures and buffers

[Andrew Donnellan]

A number of structures and buffers passed to PKS hcalls have alignment
requirements, which could on occasion cause problems:

- Authorisation structures must be 16-byte aligned and must not cross a
  page boundary

- Label structures must not cross page coundaries

- Password output buffers must not cross page boundaries

Round up the allocations of these structures/buffers to the next power of
2 to make sure this happens.

Reported-by: Benjamin Gray <bgray@linux.ibm.com>
Fixes: 2454a7af0f2a ("powerpc/pseries: define driver for Platform KeyStore")
Signed-off-by: Andrew Donnellan <ajd@linux.ibm.com>
---
 arch/powerpc/platforms/pseries/plpks.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/plpks.c b/arch/powerpc/platforms/pseries/plpks.c
index cb93062e8223..8ccc91143370 100644
--- a/arch/powerpc/platforms/pseries/plpks.c
+++ b/arch/powerpc/platforms/pseries/plpks.c
@@ -126,7 +126,8 @@ static int plpks_gen_password(void)
 	u8 *password, consumer = PKS_OS_OWNER;
 	int rc;
 
-	password = kzalloc(maxpwsize, GFP_KERNEL);
+	// The password must not cross a page boundary, so we align to the next power of 2
+	password = kzalloc(roundup_pow_of_two(maxpwsize), GFP_KERNEL);
 	if (!password)
 		return -ENOMEM;
 
@@ -162,7 +163,9 @@ static struct plpks_auth *construct_auth(u8 consumer)
 	if (consumer > PKS_OS_OWNER)
 		return ERR_PTR(-EINVAL);
 
-	auth = kzalloc(struct_size(auth, password, maxpwsize), GFP_KERNEL);
+	// The auth structure must not cross a page boundary and must be
+	// 16 byte aligned. We align to the next largest power of 2
+	auth = kzalloc(roundup_pow_of_two(struct_size(auth, password, maxpwsize)), GFP_KERNEL);
 	if (!auth)
 		return ERR_PTR(-ENOMEM);
 
@@ -196,7 +199,8 @@ static struct label *construct_label(char *component, u8 varos, u8 *name,
 	if (component && slen > sizeof(label->attr.prefix))
 		return ERR_PTR(-EINVAL);
 
-	label = kzalloc(sizeof(*label), GFP_KERNEL);
+	// The label structure must not cross a page boundary, so we align to the next power of 2
+	label = kzalloc(roundup_pow_of_two(sizeof(*label)), GFP_KERNEL);
 	if (!label)
 		return ERR_PTR(-ENOMEM);
 
-- 
2.38.1

[PATCH 3/4] powerpc/pseries: Expose PLPKS config values, support additional fields

[Andrew Donnellan]

From: Nayna Jain <nayna@linux.ibm.com>

The plpks driver uses the H_PKS_GET_CONFIG hcall to retrieve configuration
and status information about the PKS from the hypervisor.

Update _plpks_get_config() to handle some additional fields. Add getter
functions to allow the PKS configuration information to be accessed from
other files.

While we're here, move the config struct in _plpks_get_config() off the
stack - it's getting large and we also need to make sure it doesn't cross
a page boundary.

Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
[ajd: split patch, extend to support additional v3 API fields, minor fixes]
Co-developed-by: Andrew Donnellan <ajd@linux.ibm.com>
Signed-off-by: Andrew Donnellan <ajd@linux.ibm.com>
---
 arch/powerpc/platforms/pseries/plpks.c | 118 ++++++++++++++++++++++---
 arch/powerpc/platforms/pseries/plpks.h |  58 ++++++++++++
 2 files changed, 164 insertions(+), 12 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/plpks.c b/arch/powerpc/platforms/pseries/plpks.c
index 8ccc91143370..c5ae00a8a968 100644
--- a/arch/powerpc/platforms/pseries/plpks.c
+++ b/arch/powerpc/platforms/pseries/plpks.c
@@ -38,8 +38,16 @@ static u8 *ospassword;
 static u16 ospasswordlength;
 
 // Retrieved with H_PKS_GET_CONFIG
+static u8 version;
+static u16 objoverhead;
 static u16 maxpwsize;
 static u16 maxobjsize;
+static s16 maxobjlabelsize;
+static u32 totalsize;
+static u32 usedspace;
+static u32 supportedpolicies;
+static u32 maxlargeobjectsize;
+static u64 signedupdatealgorithms;
 
 struct plpks_auth {
 	u8 version;
@@ -220,32 +228,118 @@ static struct label *construct_label(char *component, u8 varos, u8 *name,
 static int _plpks_get_config(void)
 {
 	unsigned long retbuf[PLPAR_HCALL_BUFSIZE] = { 0 };
-	struct {
+	struct config {
 		u8 version;
 		u8 flags;
-		__be32 rsvd0;
+		__be16 rsvd0;
+		__be16 objoverhead;
 		__be16 maxpwsize;
 		__be16 maxobjlabelsize;
 		__be16 maxobjsize;
 		__be32 totalsize;
 		__be32 usedspace;
 		__be32 supportedpolicies;
-		__be64 rsvd1;
-	} __packed config;
+		__be32 maxlargeobjectsize;
+		__be64 signedupdatealgorithms;
+		u8 rsvd1[476];
+	} __packed *config;
 	size_t size;
-	int rc;
+	int rc = 0;
+
+	size = sizeof(*config);
+
+	// Config struct must not cross a page boundary. So long as the struct
+	// size is a power of 2, this should be fine as alignment is guaranteed
+	config = kzalloc(size, GFP_KERNEL);
+	if (!config) {
+		rc = -ENOMEM;
+		goto err;
+	}
 
-	size = sizeof(config);
+	rc = plpar_hcall(H_PKS_GET_CONFIG, retbuf, virt_to_phys(config), size);
 
-	rc = plpar_hcall(H_PKS_GET_CONFIG, retbuf, virt_to_phys(&config), size);
+	if (rc != H_SUCCESS) {
+		rc = pseries_status_to_err(rc);
+		goto err;
+	}
 
-	if (rc != H_SUCCESS)
-		return pseries_status_to_err(rc);
+	version = config->version;
+	objoverhead = be16_to_cpu(config->objoverhead);
+	maxpwsize = be16_to_cpu(config->maxpwsize);
+	maxobjsize = be16_to_cpu(config->maxobjsize);
+	maxobjlabelsize = be16_to_cpu(config->maxobjlabelsize) -
+			  MAX_LABEL_ATTR_SIZE;
+	maxobjlabelsize = maxobjlabelsize < 0 ? 0 : maxobjlabelsize;
+	totalsize = be32_to_cpu(config->totalsize);
+	usedspace = be32_to_cpu(config->usedspace);
+	supportedpolicies = be32_to_cpu(config->supportedpolicies);
+	maxlargeobjectsize = be32_to_cpu(config->maxlargeobjectsize);
+	signedupdatealgorithms = be64_to_cpu(config->signedupdatealgorithms);
+
+err:
+	kfree(config);
+	return rc;
+}
 
-	maxpwsize = be16_to_cpu(config.maxpwsize);
-	maxobjsize = be16_to_cpu(config.maxobjsize);
+u8 plpks_get_version(void)
+{
+	return version;
+}
+
+u16 plpks_get_objoverhead(void)
+{
+	return objoverhead;
+}
+
+u16 plpks_get_maxpwsize(void)
+{
+	return maxpwsize;
+}
+
+u16 plpks_get_maxobjectsize(void)
+{
+	return maxobjsize;
+}
+
+u16 plpks_get_maxobjectlabelsize(void)
+{
+	return maxobjlabelsize;
+}
+
+u32 plpks_get_totalsize(void)
+{
+	return totalsize;
+}
+
+u32 plpks_get_usedspace(void)
+{
+	return usedspace;
+}
+
+u32 plpks_get_supportedpolicies(void)
+{
+	return supportedpolicies;
+}
+
+u32 plpks_get_maxlargeobjectsize(void)
+{
+	return maxlargeobjectsize;
+}
+
+u64 plpks_get_signedupdatealgorithms(void)
+{
+	return signedupdatealgorithms;
+}
+
+bool plpks_is_available(void)
+{
+	int rc;
+
+	rc = _plpks_get_config();
+	if (rc)
+		return false;
 
-	return 0;
+	return true;
 }
 
 static int plpks_confirm_object_flushed(struct label *label,
diff --git a/arch/powerpc/platforms/pseries/plpks.h b/arch/powerpc/platforms/pseries/plpks.h
index 275ccd86bfb5..c89740796660 100644
--- a/arch/powerpc/platforms/pseries/plpks.h
+++ b/arch/powerpc/platforms/pseries/plpks.h
@@ -68,4 +68,62 @@ int plpks_read_fw_var(struct plpks_var *var);
  */
 int plpks_read_bootloader_var(struct plpks_var *var);
 
+/**
+ * Returns if PKS is available on this LPAR.
+ */
+bool plpks_is_available(void);
+
+/**
+ * Returns version of the Platform KeyStore.
+ */
+u8 plpks_get_version(void);
+
+/**
+ * Returns hypervisor storage overhead per object, not including the size of
+ * the object or label. Only valid for config version >= 2
+ */
+u16 plpks_get_objoverhead(void);
+
+/**
+ * Returns maximum password size. Must be >= 32 bytes
+ */
+u16 plpks_get_maxpwsize(void);
+
+/**
+ * Returns maximum object size supported by Platform KeyStore.
+ */
+u16 plpks_get_maxobjectsize(void);
+
+/**
+ * Returns maximum object label size supported by Platform KeyStore.
+ */
+u16 plpks_get_maxobjectlabelsize(void);
+
+/**
+ * Returns total size of the configured Platform KeyStore.
+ */
+u32 plpks_get_totalsize(void);
+
+/**
+ * Returns used space from the total size of the Platform KeyStore.
+ */
+u32 plpks_get_usedspace(void);
+
+/**
+ * Returns bitmask of policies supported by the hypervisor.
+ */
+u32 plpks_get_supportedpolicies(void);
+
+/**
+ * Returns maximum byte size of a single object supported by the hypervisor.
+ * Only valid for config version >= 3
+ */
+u32 plpks_get_maxlargeobjectsize(void);
+
+/**
+ * Returns bitmask of signature algorithms supported for signed updates.
+ * Only valid for config version >= 3
+ */
+u64 plpks_get_signedupdatealgorithms(void);
+
 #endif
-- 
2.38.1

[PATCH 4/4] powerpc/pseries: Implement signed update for PLPKS objects

[Andrew Donnellan]

From: Nayna Jain <nayna@linux.ibm.com>

The Platform Keystore provides a signed update interface which can be used
to create, replace or append to certain variables in the PKS in a secure
fashion, with the hypervisor requiring that the update be signed using the
Platform Key.

Implement an interface to the H_PKS_SIGNED_UPDATE hcall in the plpks
driver to allow signed updates to PKS objects.

(The plpks driver doesn't need to do any cryptography or otherwise handle
the actual signed variable contents - that will be handled by userspace
tooling.)

Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
[ajd: split patch, rewrite commit message, add timeout handling]
Co-developed-by: Andrew Donnellan <ajd@linux.ibm.com>
Signed-off-by: Andrew Donnellan <ajd@linux.ibm.com>
---
 arch/powerpc/include/asm/hvcall.h      |  3 +-
 arch/powerpc/platforms/pseries/plpks.c | 81 +++++++++++++++++++++++---
 arch/powerpc/platforms/pseries/plpks.h |  5 ++
 3 files changed, 79 insertions(+), 10 deletions(-)

diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h
index 95fd7f9485d5..33b26c0cb69b 100644
--- a/arch/powerpc/include/asm/hvcall.h
+++ b/arch/powerpc/include/asm/hvcall.h
@@ -336,7 +336,8 @@
 #define H_SCM_FLUSH		0x44C
 #define H_GET_ENERGY_SCALE_INFO	0x450
 #define H_WATCHDOG		0x45C
-#define MAX_HCALL_OPCODE	H_WATCHDOG
+#define H_PKS_SIGNED_UPDATE	0x454
+#define MAX_HCALL_OPCODE	H_PKS_SIGNED_UPDATE
 
 /* Scope args for H_SCM_UNBIND_ALL */
 #define H_UNBIND_SCOPE_ALL (0x1)
diff --git a/arch/powerpc/platforms/pseries/plpks.c b/arch/powerpc/platforms/pseries/plpks.c
index c5ae00a8a968..9e4401aabf4f 100644
--- a/arch/powerpc/platforms/pseries/plpks.c
+++ b/arch/powerpc/platforms/pseries/plpks.c
@@ -30,9 +30,9 @@
 #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
+#define PKS_MAX_TIMEOUT		5000 // msec
+#define PKS_FLUSH_SLEEP		10 // msec
+#define PKS_FLUSH_SLEEP_RANGE	400
 
 static u8 *ospassword;
 static u16 ospasswordlength;
@@ -95,6 +95,12 @@ static int pseries_status_to_err(int rc)
 		err = -ENOENT;
 		break;
 	case H_BUSY:
+	case H_LONG_BUSY_ORDER_1_MSEC:
+	case H_LONG_BUSY_ORDER_10_MSEC:
+	case H_LONG_BUSY_ORDER_100_MSEC:
+	case H_LONG_BUSY_ORDER_1_SEC:
+	case H_LONG_BUSY_ORDER_10_SEC:
+	case H_LONG_BUSY_ORDER_100_SEC:
 		err = -EBUSY;
 		break;
 	case H_AUTHORITY:
@@ -198,14 +204,17 @@ static struct label *construct_label(char *component, u8 varos, u8 *name,
 				     u16 namelen)
 {
 	struct label *label;
-	size_t slen;
+	size_t slen = 0;
 
 	if (!name || namelen > MAX_NAME_SIZE)
 		return ERR_PTR(-EINVAL);
 
-	slen = strlen(component);
-	if (component && slen > sizeof(label->attr.prefix))
-		return ERR_PTR(-EINVAL);
+	// Support NULL component for signed updates
+	if (component) {
+		slen = strlen(component);
+		if (slen > sizeof(label->attr.prefix))
+			return ERR_PTR(-EINVAL);
+	}
 
 	// The label structure must not cross a page boundary, so we align to the next power of 2
 	label = kzalloc(roundup_pow_of_two(sizeof(*label)), GFP_KERNEL);
@@ -372,7 +381,7 @@ static int plpks_confirm_object_flushed(struct label *label,
 		usleep_range(PKS_FLUSH_SLEEP,
 			     PKS_FLUSH_SLEEP + PKS_FLUSH_SLEEP_RANGE);
 		timeout = timeout + PKS_FLUSH_SLEEP;
-	} while (timeout < PKS_FLUSH_MAX_TIMEOUT);
+	} while (timeout < PKS_MAX_TIMEOUT);
 
 	if (timed_out)
 		rc = -ETIMEDOUT;
@@ -382,6 +391,60 @@ static int plpks_confirm_object_flushed(struct label *label,
 	return rc;
 }
 
+int plpks_signed_update_var(struct plpks_var var, u64 flags)
+{
+	unsigned long retbuf[PLPAR_HCALL9_BUFSIZE] = {0};
+	int rc;
+	struct label *label;
+	struct plpks_auth *auth;
+	u64 continuetoken = 0;
+	u64 timeout = 0;
+
+	if (!var.data || var.datalen <= 0 || var.namelen > MAX_NAME_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;
+	}
+
+	do {
+		rc = plpar_hcall9(H_PKS_SIGNED_UPDATE, retbuf,
+				  virt_to_phys(auth), virt_to_phys(label),
+				  label->size, var.policy, flags,
+				  virt_to_phys(var.data), var.datalen,
+				  continuetoken);
+
+		continuetoken = retbuf[0];
+		if (pseries_status_to_err(rc) == -EBUSY) {
+			int delay_ms = get_longbusy_msecs(rc);
+			mdelay(delay_ms);
+			timeout += delay_ms;
+		}
+		rc = pseries_status_to_err(rc);
+	} while (rc == -EBUSY && timeout < PKS_MAX_TIMEOUT);
+
+	if (!rc) {
+		rc = plpks_confirm_object_flushed(label, auth);
+		rc = pseries_status_to_err(rc);
+	}
+
+	kfree(label);
+out:
+	kfree(auth);
+
+	return rc;
+}
+EXPORT_SYMBOL(plpks_signed_update_var);
+
 int plpks_write_var(struct plpks_var var)
 {
 	unsigned long retbuf[PLPAR_HCALL_BUFSIZE] = { 0 };
@@ -428,7 +491,7 @@ int plpks_remove_var(char *component, u8 varos, struct plpks_var_name vname)
 	struct label *label;
 	int rc;
 
-	if (!component || vname.namelen > MAX_NAME_SIZE)
+	if (vname.namelen > MAX_NAME_SIZE)
 		return -EINVAL;
 
 	auth = construct_auth(PKS_OS_OWNER);
diff --git a/arch/powerpc/platforms/pseries/plpks.h b/arch/powerpc/platforms/pseries/plpks.h
index c89740796660..33cf12809392 100644
--- a/arch/powerpc/platforms/pseries/plpks.h
+++ b/arch/powerpc/platforms/pseries/plpks.h
@@ -40,6 +40,11 @@ struct plpks_var_name_list {
 	struct plpks_var_name varlist[];
 };
 
+/**
+ * Updates the authenticated variable. It expects NULL as the component.
+ */
+int plpks_signed_update_var(struct plpks_var var, u64 flags);
+
 /**
  * Writes the specified var and its data to PKS.
  * Any caller of PKS driver should present a valid component type for
-- 
2.38.1

Re: [PATCH 1/4] powerpc/pseries: Fix handling of PLPKS object flushing timeout

[Russell Currey]

On Tue, 2022-12-20 at 18:16 +1100, Andrew Donnellan wrote:
> plpks_confirm_object_flushed() uses the H_PKS_CONFIRM_OBJECT_FLUSHED
> hcall
> to check whether changes to an object in the Platform KeyStore have
> been
> flushed to non-volatile storage.
> 
> The hcall returns two output values, the return code and the flush
> status.
> plpks_confirm_object_flushed() polls the hcall until either the flush
> status has updated, the return code is an error, or a timeout has
> been
> exceeded.
> 
> While we're still polling, the hcall is returning H_SUCCESS (0) as
> the
> return code. In the timeout case, this means that upon exiting the
> polling
> loop, rc is 0, and therefore 0 is returned to the user.
> 
> Handle the timeout case separately and return ETIMEDOUT if triggered.
> 
> Fixes: 2454a7af0f2a ("powerpc/pseries: define driver for Platform
> KeyStore")
> Reported-by: Benjamin Gray <bgray@linux.ibm.com>
> Signed-off-by: Andrew Donnellan <ajd@linux.ibm.com>

Tested-by: Russell Currey <ruscur@russell.cc>
Reviewed-by: Russell Currey <ruscur@russell.cc>

Re: [PATCH 2/4] powerpc/pseries: Fix alignment of PLPKS structures and buffers

[Russell Currey]

On Tue, 2022-12-20 at 18:16 +1100, Andrew Donnellan wrote:
> A number of structures and buffers passed to PKS hcalls have
> alignment
> requirements, which could on occasion cause problems:
> 
> - Authorisation structures must be 16-byte aligned and must not cross
> a
>   page boundary
> 
> - Label structures must not cross page coundaries
> 
> - Password output buffers must not cross page boundaries
> 
> Round up the allocations of these structures/buffers to the next
> power of
> 2 to make sure this happens.
> 
> Reported-by: Benjamin Gray <bgray@linux.ibm.com>
> Fixes: 2454a7af0f2a ("powerpc/pseries: define driver for Platform
> KeyStore")
> Signed-off-by: Andrew Donnellan <ajd@linux.ibm.com>
> 
Reviewed-by: Russell Currey <ruscur@russell.cc>

Re: [PATCH 3/4] powerpc/pseries: Expose PLPKS config values, support additional fields

[Russell Currey]

On Tue, 2022-12-20 at 18:16 +1100, Andrew Donnellan wrote:
> From: Nayna Jain <nayna@linux.ibm.com>
> 
> The plpks driver uses the H_PKS_GET_CONFIG hcall to retrieve
> configuration
> and status information about the PKS from the hypervisor.
> 
> Update _plpks_get_config() to handle some additional fields. Add
> getter
> functions to allow the PKS configuration information to be accessed
> from
> other files.
> 
> While we're here, move the config struct in _plpks_get_config() off
> the
> stack - it's getting large and we also need to make sure it doesn't
> cross
> a page boundary.
> 
> Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
> [ajd: split patch, extend to support additional v3 API fields, minor
> fixes]
> Co-developed-by: Andrew Donnellan <ajd@linux.ibm.com>
> Signed-off-by: Andrew Donnellan <ajd@linux.ibm.com>
> ---
>  arch/powerpc/platforms/pseries/plpks.c | 118 ++++++++++++++++++++++-
> --
>  arch/powerpc/platforms/pseries/plpks.h |  58 ++++++++++++
>  2 files changed, 164 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/pseries/plpks.c
> b/arch/powerpc/platforms/pseries/plpks.c
> index 8ccc91143370..c5ae00a8a968 100644
> --- a/arch/powerpc/platforms/pseries/plpks.c
> +++ b/arch/powerpc/platforms/pseries/plpks.c
> @@ -38,8 +38,16 @@ static u8 *ospassword;
>  static u16 ospasswordlength;
>  
>  // Retrieved with H_PKS_GET_CONFIG
> +static u8 version;
> +static u16 objoverhead;
>  static u16 maxpwsize;
>  static u16 maxobjsize;
> +static s16 maxobjlabelsize;
> +static u32 totalsize;
> +static u32 usedspace;
> +static u32 supportedpolicies;
> +static u32 maxlargeobjectsize;
> +static u64 signedupdatealgorithms;
>  
>  struct plpks_auth {
>         u8 version;
> @@ -220,32 +228,118 @@ static struct label *construct_label(char
> *component, u8 varos, u8 *name,
>  static int _plpks_get_config(void)
>  {
>         unsigned long retbuf[PLPAR_HCALL_BUFSIZE] = { 0 };
> -       struct {
> +       struct config {
>                 u8 version;
>                 u8 flags;
> -               __be32 rsvd0;
> +               __be16 rsvd0;
> +               __be16 objoverhead;
>                 __be16 maxpwsize;
>                 __be16 maxobjlabelsize;
>                 __be16 maxobjsize;
>                 __be32 totalsize;
>                 __be32 usedspace;
>                 __be32 supportedpolicies;
> -               __be64 rsvd1;
> -       } __packed config;
> +               __be32 maxlargeobjectsize;
> +               __be64 signedupdatealgorithms;
> +               u8 rsvd1[476];
> +       } __packed *config;
>         size_t size;
> -       int rc;
> +       int rc = 0;
> +
> +       size = sizeof(*config);
> +
> +       // Config struct must not cross a page boundary. So long as
> the struct
> +       // size is a power of 2, this should be fine as alignment is
> guaranteed
> +       config = kzalloc(size, GFP_KERNEL);
> +       if (!config) {
> +               rc = -ENOMEM;
> +               goto err;
> +       }
>  
> -       size = sizeof(config);
> +       rc = plpar_hcall(H_PKS_GET_CONFIG, retbuf,
> virt_to_phys(config), size);
>  
> -       rc = plpar_hcall(H_PKS_GET_CONFIG, retbuf,
> virt_to_phys(&config), size);
> +       if (rc != H_SUCCESS) {
> +               rc = pseries_status_to_err(rc);
> +               goto err;
> +       }
>  
> -       if (rc != H_SUCCESS)
> -               return pseries_status_to_err(rc);
> +       version = config->version;
> +       objoverhead = be16_to_cpu(config->objoverhead);
> +       maxpwsize = be16_to_cpu(config->maxpwsize);
> +       maxobjsize = be16_to_cpu(config->maxobjsize);
> +       maxobjlabelsize = be16_to_cpu(config->maxobjlabelsize) -
> +                         MAX_LABEL_ATTR_SIZE;
> +       maxobjlabelsize = maxobjlabelsize < 0 ? 0 : maxobjlabelsize;

Isn't a bit of precision lost here?  There has to be a better way to
handle this.  We get a be16 from the hypervisor, turn it into a u16,
and assign that to an s16 in order to handle underflow.  Can we just
check if the size we're given is large enough?  The hypervisor
documentation also says this value must be at least 255, if we sanity
check that we don't have to worry about underflow.

> +       totalsize = be32_to_cpu(config->totalsize);
> +       usedspace = be32_to_cpu(config->usedspace);
> +       supportedpolicies = be32_to_cpu(config->supportedpolicies);
> +       maxlargeobjectsize = be32_to_cpu(config->maxlargeobjectsize);
> +       signedupdatealgorithms = be64_to_cpu(config-
> >signedupdatealgorithms);
> +
> +err:
> +       kfree(config);
> +       return rc;
> +}
>  
> -       maxpwsize = be16_to_cpu(config.maxpwsize);
> -       maxobjsize = be16_to_cpu(config.maxobjsize);
> +u8 plpks_get_version(void)
> +{
> +       return version;
> +}
> +
> +u16 plpks_get_objoverhead(void)
> +{
> +       return objoverhead;
> +}
> +
> +u16 plpks_get_maxpwsize(void)
> +{
> +       return maxpwsize;
> +}
> +
> +u16 plpks_get_maxobjectsize(void)
> +{
> +       return maxobjsize;
> +}
> +
> +u16 plpks_get_maxobjectlabelsize(void)

and it's returned as a u16 here.

> +{
> +       return maxobjlabelsize;
> +}
> +
> +u32 plpks_get_totalsize(void)
> +{
> +       return totalsize;
> +}
> +
> +u32 plpks_get_usedspace(void)
> +{
> +       return usedspace;
> +}
> +
> +u32 plpks_get_supportedpolicies(void)
> +{
> +       return supportedpolicies;
> +}
> +
> +u32 plpks_get_maxlargeobjectsize(void)
> +{
> +       return maxlargeobjectsize;
> +}
> +
> +u64 plpks_get_signedupdatealgorithms(void)
> +{
> +       return signedupdatealgorithms;
> +}
> +
> +bool plpks_is_available(void)
> +{
> +       int rc;
> +
> +       rc = _plpks_get_config();
> +       if (rc)
> +               return false;
>  
> -       return 0;
> +       return true;
>  }
>  
>  static int plpks_confirm_object_flushed(struct label *label,
> diff --git a/arch/powerpc/platforms/pseries/plpks.h
> b/arch/powerpc/platforms/pseries/plpks.h
> index 275ccd86bfb5..c89740796660 100644
> --- a/arch/powerpc/platforms/pseries/plpks.h
> +++ b/arch/powerpc/platforms/pseries/plpks.h
> @@ -68,4 +68,62 @@ int plpks_read_fw_var(struct plpks_var *var);
>   */
>  int plpks_read_bootloader_var(struct plpks_var *var);
>  
> +/**
> + * Returns if PKS is available on this LPAR.
> + */
> +bool plpks_is_available(void);
> +
> +/**
> + * Returns version of the Platform KeyStore.
> + */
> +u8 plpks_get_version(void);
> +
> +/**
> + * Returns hypervisor storage overhead per object, not including the
> size of
> + * the object or label. Only valid for config version >= 2
> + */
> +u16 plpks_get_objoverhead(void);
> +
> +/**
> + * Returns maximum password size. Must be >= 32 bytes
> + */
> +u16 plpks_get_maxpwsize(void);
> +
> +/**
> + * Returns maximum object size supported by Platform KeyStore.
> + */
> +u16 plpks_get_maxobjectsize(void);
> +
> +/**
> + * Returns maximum object label size supported by Platform KeyStore.
> + */
> +u16 plpks_get_maxobjectlabelsize(void);
> +
> +/**
> + * Returns total size of the configured Platform KeyStore.
> + */
> +u32 plpks_get_totalsize(void);
> +
> +/**
> + * Returns used space from the total size of the Platform KeyStore.
> + */
> +u32 plpks_get_usedspace(void);
> +
> +/**
> + * Returns bitmask of policies supported by the hypervisor.
> + */
> +u32 plpks_get_supportedpolicies(void);
> +
> +/**
> + * Returns maximum byte size of a single object supported by the
> hypervisor.
> + * Only valid for config version >= 3
> + */
> +u32 plpks_get_maxlargeobjectsize(void);
> +
> +/**
> + * Returns bitmask of signature algorithms supported for signed
> updates.
> + * Only valid for config version >= 3
> + */
> +u64 plpks_get_signedupdatealgorithms(void);
> +
>  #endif

Re: [PATCH 4/4] powerpc/pseries: Implement signed update for PLPKS objects

[Russell Currey]

On Tue, 2022-12-20 at 18:16 +1100, Andrew Donnellan wrote:
> From: Nayna Jain <nayna@linux.ibm.com>
> 
> The Platform Keystore provides a signed update interface which can be
> used
> to create, replace or append to certain variables in the PKS in a
> secure
> fashion, with the hypervisor requiring that the update be signed
> using the
> Platform Key.
> 
> Implement an interface to the H_PKS_SIGNED_UPDATE hcall in the plpks
> driver to allow signed updates to PKS objects.
> 
> (The plpks driver doesn't need to do any cryptography or otherwise
> handle
> the actual signed variable contents - that will be handled by
> userspace
> tooling.)
> 
> Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
> [ajd: split patch, rewrite commit message, add timeout handling]
> Co-developed-by: Andrew Donnellan <ajd@linux.ibm.com>
> Signed-off-by: Andrew Donnellan <ajd@linux.ibm.com>
> ---
>  arch/powerpc/include/asm/hvcall.h      |  3 +-
>  arch/powerpc/platforms/pseries/plpks.c | 81 +++++++++++++++++++++++-
> --
>  arch/powerpc/platforms/pseries/plpks.h |  5 ++
>  3 files changed, 79 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/hvcall.h
> b/arch/powerpc/include/asm/hvcall.h
> index 95fd7f9485d5..33b26c0cb69b 100644
> --- a/arch/powerpc/include/asm/hvcall.h
> +++ b/arch/powerpc/include/asm/hvcall.h
> @@ -336,7 +336,8 @@
>  #define H_SCM_FLUSH            0x44C
>  #define H_GET_ENERGY_SCALE_INFO        0x450
>  #define H_WATCHDOG             0x45C
> -#define MAX_HCALL_OPCODE       H_WATCHDOG
> +#define H_PKS_SIGNED_UPDATE    0x454
> +#define MAX_HCALL_OPCODE       H_PKS_SIGNED_UPDATE
>  
>  /* Scope args for H_SCM_UNBIND_ALL */
>  #define H_UNBIND_SCOPE_ALL (0x1)
> diff --git a/arch/powerpc/platforms/pseries/plpks.c
> b/arch/powerpc/platforms/pseries/plpks.c
> index c5ae00a8a968..9e4401aabf4f 100644
> --- a/arch/powerpc/platforms/pseries/plpks.c
> +++ b/arch/powerpc/platforms/pseries/plpks.c
> @@ -30,9 +30,9 @@
>  #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
> +#define PKS_MAX_TIMEOUT                5000 // msec
> +#define PKS_FLUSH_SLEEP                10 // msec
> +#define PKS_FLUSH_SLEEP_RANGE  400
>  
>  static u8 *ospassword;
>  static u16 ospasswordlength;
> @@ -95,6 +95,12 @@ static int pseries_status_to_err(int rc)
>                 err = -ENOENT;
>                 break;
>         case H_BUSY:
> +       case H_LONG_BUSY_ORDER_1_MSEC:
> +       case H_LONG_BUSY_ORDER_10_MSEC:
> +       case H_LONG_BUSY_ORDER_100_MSEC:
> +       case H_LONG_BUSY_ORDER_1_SEC:
> +       case H_LONG_BUSY_ORDER_10_SEC:
> +       case H_LONG_BUSY_ORDER_100_SEC:
>                 err = -EBUSY;
>                 break;
>         case H_AUTHORITY:
> @@ -198,14 +204,17 @@ static struct label *construct_label(char
> *component, u8 varos, u8 *name,
>                                      u16 namelen)
>  {
>         struct label *label;
> -       size_t slen;
> +       size_t slen = 0;
>  
>         if (!name || namelen > MAX_NAME_SIZE)
>                 return ERR_PTR(-EINVAL);
>  
> -       slen = strlen(component);
> -       if (component && slen > sizeof(label->attr.prefix))
> -               return ERR_PTR(-EINVAL);
> +       // Support NULL component for signed updates
> +       if (component) {
> +               slen = strlen(component);
> +               if (slen > sizeof(label->attr.prefix))
> +                       return ERR_PTR(-EINVAL);
> +       }
>  
>         // The label structure must not cross a page boundary, so we
> align to the next power of 2
>         label = kzalloc(roundup_pow_of_two(sizeof(*label)),
> GFP_KERNEL);
> @@ -372,7 +381,7 @@ static int plpks_confirm_object_flushed(struct
> label *label,
>                 usleep_range(PKS_FLUSH_SLEEP,
>                              PKS_FLUSH_SLEEP +
> PKS_FLUSH_SLEEP_RANGE);
>                 timeout = timeout + PKS_FLUSH_SLEEP;
> -       } while (timeout < PKS_FLUSH_MAX_TIMEOUT);
> +       } while (timeout < PKS_MAX_TIMEOUT);
>  
>         if (timed_out)
>                 rc = -ETIMEDOUT;
> @@ -382,6 +391,60 @@ static int plpks_confirm_object_flushed(struct
> label *label,
>         return rc;
>  }
>  
> +int plpks_signed_update_var(struct plpks_var var, u64 flags)
> +{
> +       unsigned long retbuf[PLPAR_HCALL9_BUFSIZE] = {0};
> +       int rc;
> +       struct label *label;
> +       struct plpks_auth *auth;
> +       u64 continuetoken = 0;
> +       u64 timeout = 0;
> +
> +       if (!var.data || var.datalen <= 0 || var.namelen >
> MAX_NAME_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;
> +       }
> +
> +       do {
> +               rc = plpar_hcall9(H_PKS_SIGNED_UPDATE, retbuf,
> +                                 virt_to_phys(auth),
> virt_to_phys(label),
> +                                 label->size, var.policy, flags,
> +                                 virt_to_phys(var.data),
> var.datalen,
> +                                 continuetoken);
> +
> +               continuetoken = retbuf[0];
> +               if (pseries_status_to_err(rc) == -EBUSY) {
> +                       int delay_ms = get_longbusy_msecs(rc);
> +                       mdelay(delay_ms);
> +                       timeout += delay_ms;
> +               }
> +               rc = pseries_status_to_err(rc);
> +       } while (rc == -EBUSY && timeout < PKS_MAX_TIMEOUT);
> +
> +       if (!rc) {
> +               rc = plpks_confirm_object_flushed(label, auth);
> +               rc = pseries_status_to_err(rc);

Doesn't plpks_confirm_object_flushed() already return a Linux-friendly
error code?  If I'm reading this right, we'd be replacing any non-zero
return code with -EINVAL.

> +       }
> +
> +       kfree(label);
> +out:
> +       kfree(auth);
> +
> +       return rc;
> +}
> +EXPORT_SYMBOL(plpks_signed_update_var);
> +
>  int plpks_write_var(struct plpks_var var)
>  {
>         unsigned long retbuf[PLPAR_HCALL_BUFSIZE] = { 0 };
> @@ -428,7 +491,7 @@ int plpks_remove_var(char *component, u8 varos,
> struct plpks_var_name vname)
>         struct label *label;
>         int rc;
>  
> -       if (!component || vname.namelen > MAX_NAME_SIZE)
> +       if (vname.namelen > MAX_NAME_SIZE)
>                 return -EINVAL;
>  
>         auth = construct_auth(PKS_OS_OWNER);
> diff --git a/arch/powerpc/platforms/pseries/plpks.h
> b/arch/powerpc/platforms/pseries/plpks.h
> index c89740796660..33cf12809392 100644
> --- a/arch/powerpc/platforms/pseries/plpks.h
> +++ b/arch/powerpc/platforms/pseries/plpks.h
> @@ -40,6 +40,11 @@ struct plpks_var_name_list {
>         struct plpks_var_name varlist[];
>  };
>  
> +/**
> + * Updates the authenticated variable. It expects NULL as the
> component.
> + */
> +int plpks_signed_update_var(struct plpks_var var, u64 flags);
> +
>  /**
>   * Writes the specified var and its data to PKS.
>   * Any caller of PKS driver should present a valid component type
> for

Re: [PATCH 4/4] powerpc/pseries: Implement signed update for PLPKS objects

[Andrew Donnellan]

On Wed, 2023-01-04 at 15:04 +1100, Russell Currey wrote:
> > +       if (!rc) {
> > +               rc = plpks_confirm_object_flushed(label, auth);
> > +               rc = pseries_status_to_err(rc);
> 
> Doesn't plpks_confirm_object_flushed() already return a Linux-
> friendly
> error code?  If I'm reading this right, we'd be replacing any non-
> zero
> return code with -EINVAL.

Good catch, will fix.


-- 
Andrew Donnellan    OzLabs, ADL Canberra
ajd@linux.ibm.com   IBM Australia Limited

Re: [PATCH 3/4] powerpc/pseries: Expose PLPKS config values, support additional fields

[Andrew Donnellan]

On Wed, 2023-01-04 at 14:57 +1100, Russell Currey wrote:
> > +       maxobjlabelsize = be16_to_cpu(config->maxobjlabelsize) -
> > +                         MAX_LABEL_ATTR_SIZE;
> > +       maxobjlabelsize = maxobjlabelsize < 0 ? 0 :
> > maxobjlabelsize;
> 
> Isn't a bit of precision lost here?  There has to be a better way to
> handle this.  We get a be16 from the hypervisor, turn it into a u16,
> and assign that to an s16 in order to handle underflow.  Can we just
> check if the size we're given is large enough?  The hypervisor
> documentation also says this value must be at least 255, if we sanity
> check that we don't have to worry about underflow.

Agreed, and it makes more sense for the value that we return to the
user to be the same as the number we actually get from the hypervisor.
I'll fix it in the next spin.

-- 
Andrew Donnellan    OzLabs, ADL Canberra
ajd@linux.ibm.com   IBM Australia Limited

Re: [PATCH 4/4] powerpc/pseries: Implement signed update for PLPKS objects

[Michael Ellerman]

Andrew Donnellan <ajd@linux.ibm.com> writes:
> From: Nayna Jain <nayna@linux.ibm.com>
>
> The Platform Keystore provides a signed update interface which can be used
> to create, replace or append to certain variables in the PKS in a secure
> fashion, with the hypervisor requiring that the update be signed using the
> Platform Key.
>
> Implement an interface to the H_PKS_SIGNED_UPDATE hcall in the plpks
> driver to allow signed updates to PKS objects.
>
> (The plpks driver doesn't need to do any cryptography or otherwise handle
> the actual signed variable contents - that will be handled by userspace
> tooling.)
>
> Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
> [ajd: split patch, rewrite commit message, add timeout handling]
> Co-developed-by: Andrew Donnellan <ajd@linux.ibm.com>
> Signed-off-by: Andrew Donnellan <ajd@linux.ibm.com>
> ---
>  arch/powerpc/include/asm/hvcall.h      |  3 +-
>  arch/powerpc/platforms/pseries/plpks.c | 81 +++++++++++++++++++++++---
>  arch/powerpc/platforms/pseries/plpks.h |  5 ++
>  3 files changed, 79 insertions(+), 10 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h
> index 95fd7f9485d5..33b26c0cb69b 100644
> --- a/arch/powerpc/include/asm/hvcall.h
> +++ b/arch/powerpc/include/asm/hvcall.h
> @@ -336,7 +336,8 @@
>  #define H_SCM_FLUSH		0x44C
>  #define H_GET_ENERGY_SCALE_INFO	0x450
>  #define H_WATCHDOG		0x45C
> -#define MAX_HCALL_OPCODE	H_WATCHDOG
> +#define H_PKS_SIGNED_UPDATE	0x454
> +#define MAX_HCALL_OPCODE	H_PKS_SIGNED_UPDATE
>  
>  /* Scope args for H_SCM_UNBIND_ALL */
>  #define H_UNBIND_SCOPE_ALL (0x1)
> diff --git a/arch/powerpc/platforms/pseries/plpks.c b/arch/powerpc/platforms/pseries/plpks.c
> index c5ae00a8a968..9e4401aabf4f 100644
> --- a/arch/powerpc/platforms/pseries/plpks.c
> +++ b/arch/powerpc/platforms/pseries/plpks.c
> @@ -30,9 +30,9 @@
>  #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
> +#define PKS_MAX_TIMEOUT		5000 // msec
> +#define PKS_FLUSH_SLEEP		10 // msec
> +#define PKS_FLUSH_SLEEP_RANGE	400
>  
>  static u8 *ospassword;
>  static u16 ospasswordlength;
> @@ -95,6 +95,12 @@ static int pseries_status_to_err(int rc)
>  		err = -ENOENT;
>  		break;
>  	case H_BUSY:
> +	case H_LONG_BUSY_ORDER_1_MSEC:
> +	case H_LONG_BUSY_ORDER_10_MSEC:
> +	case H_LONG_BUSY_ORDER_100_MSEC:
> +	case H_LONG_BUSY_ORDER_1_SEC:
> +	case H_LONG_BUSY_ORDER_10_SEC:
> +	case H_LONG_BUSY_ORDER_100_SEC:
>  		err = -EBUSY;
>  		break;
>  	case H_AUTHORITY:
> @@ -198,14 +204,17 @@ static struct label *construct_label(char *component, u8 varos, u8 *name,
>  				     u16 namelen)
>  {
>  	struct label *label;
> -	size_t slen;
> +	size_t slen = 0;
>  
>  	if (!name || namelen > MAX_NAME_SIZE)
>  		return ERR_PTR(-EINVAL);
>  
> -	slen = strlen(component);
> -	if (component && slen > sizeof(label->attr.prefix))
> -		return ERR_PTR(-EINVAL);
> +	// Support NULL component for signed updates
> +	if (component) {
> +		slen = strlen(component);
> +		if (slen > sizeof(label->attr.prefix))
> +			return ERR_PTR(-EINVAL);
> +	}
>  
>  	// The label structure must not cross a page boundary, so we align to the next power of 2
>  	label = kzalloc(roundup_pow_of_two(sizeof(*label)), GFP_KERNEL);
> @@ -372,7 +381,7 @@ static int plpks_confirm_object_flushed(struct label *label,
>  		usleep_range(PKS_FLUSH_SLEEP,
>  			     PKS_FLUSH_SLEEP + PKS_FLUSH_SLEEP_RANGE);
>  		timeout = timeout + PKS_FLUSH_SLEEP;
> -	} while (timeout < PKS_FLUSH_MAX_TIMEOUT);
> +	} while (timeout < PKS_MAX_TIMEOUT);
>  
>  	if (timed_out)
>  		rc = -ETIMEDOUT;
> @@ -382,6 +391,60 @@ static int plpks_confirm_object_flushed(struct label *label,
>  	return rc;
>  }
>  
> +int plpks_signed_update_var(struct plpks_var var, u64 flags)
> +{

I don't see a reason why var is passed by value here? A pointer would be
more typical.

cheers

Re: [PATCH 4/4] powerpc/pseries: Implement signed update for PLPKS objects

[Andrew Donnellan]

On Fri, 2023-01-06 at 21:54 +1100, Michael Ellerman wrote:
> > +int plpks_signed_update_var(struct plpks_var var, u64 flags)
> > +{
> 
> I don't see a reason why var is passed by value here? A pointer would
> be
> more typical.

Will change.

-- 
Andrew Donnellan    OzLabs, ADL Canberra
ajd@linux.ibm.com   IBM Australia Limited