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, &varos, 1);
+ llen = llen + 1;
+
+ memcpy(*label + llen, &labellength, 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, &
