<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
<style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style>
</head>
<body dir="ltr">
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);" class="elementToProof">
Comments inline.</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);" class="elementToProof">
Reviewed-by: Greg Joyce <gjoyce@linux.vnet.ibm.com></div>
<div id="signature_bookmark"></div>
<div id="appendonsend"></div>
<div style="font-family:Calibri,Arial,Helvetica,sans-serif; font-size:12pt; color:rgb(0,0,0)">
<br>
</div>
<hr tabindex="-1" style="display:inline-block; width:98%">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" color="#000000" style="font-size:11pt"><b>From:</b> Nayna Jain <nayna@linux.ibm.com><br>
<b>Sent:</b> Tuesday, July 12, 2022 7:59 PM<br>
<b>To:</b> linuxppc-dev@lists.ozlabs.org <linuxppc-dev@lists.ozlabs.org><br>
<b>Cc:</b> Michael Ellerman <mpe@ellerman.id.au>; Benjamin Herrenschmidt <benh@kernel.crashing.org>; Paul Mackerras <paulus@samba.org>; George Wilson <gcwilson@linux.ibm.com>; Gregory Joyce <gjoyce@ibm.com>; Nayna Jain <nayna@linux.ibm.com><br>
<b>Subject:</b> [PATCH 1/2] powerpc/pseries: define driver for Platform KeyStore</font>
<div> </div>
</div>
<div class="BodyFragment"><font size="2"><span style="font-size:11pt">
<div class="PlainText elementToProof">PowerVM provides an isolated Platform Keystore(PKS) storage allocation<br>
for each LPAR with individually managed access controls to store<br>
sensitive information securely. It provides a new set of hypervisor<br>
calls for Linux kernel to access PKS storage.<br>
<br>
Define PLPKS driver using H_CALL interface to access PKS storage.<br>
<br>
Signed-off-by: Nayna Jain <nayna@linux.ibm.com><br>
---<br>
 <br>
<br>
+<br>
+static int construct_auth(u8 consumer, struct plpks_auth **auth)<br>
+{<br>
+       pr_debug("max password size is %u\n", config->maxpwsize);</div>
<div class="PlainText elementToProof"><br>
</div>
<div class="PlainText elementToProof">Are the pr_debugs in this function still needed?</div>
<div class="PlainText elementToProof"><br>
+<br>
+       if (!auth || consumer > 3)<br>
+               return -EINVAL;<br>
+<br>
+       *auth = kmalloc(struct_size(*auth, password, config->maxpwsize),<br>
+                       GFP_KERNEL);<br>
+       if (!*auth)<br>
+               return -ENOMEM;<br>
+<br>
+       (*auth)->version = 1;<br>
+       (*auth)->consumer = consumer;<br>
+       (*auth)->rsvd0 = 0;<br>
+       (*auth)->rsvd1 = 0;<br>
+       if (consumer == PKS_FW_OWNER || consumer == PKS_BOOTLOADER_OWNER) {<br>
+               pr_debug("consumer is bootloader or firmware\n");<br>
+               (*auth)->passwordlength = 0;<br>
+               return 0;<br>
+       }<br>
+<br>
+       (*auth)->passwordlength = (__force __be16)ospasswordlength;<br>
+<br>
+       memcpy((*auth)->password, ospassword,<br>
+              flex_array_size(*auth, password,<br>
+              (__force u16)((*auth)->passwordlength)));<br>
+       (*auth)->passwordlength = cpu_to_be16((__force u16)((*auth)->passwordlength));<br>
+<br>
+       return 0;<br>
+}<br>
+<br>
+/**<br>
+ * Label is combination of label attributes + name.<br>
+ * Label attributes are used internally by kernel and not exposed to the user.<br>
+ */<br>
+static int construct_label(char *component, u8 varos, u8 *name, u16 namelen, u8 **label)<br>
+{<br>
+       int varlen;<br>
+       int len = 0;<br>
+       int llen = 0;<br>
+       int i;<br>
+       int rc = 0;<br>
+       u8 labellength = MAX_LABEL_ATTR_SIZE;<br>
+<br>
+       if (!label)<br>
+               return -EINVAL;<br>
+<br>
+       varlen = namelen + sizeof(struct label_attr);<br>
+       *label = kzalloc(varlen, GFP_KERNEL);<br>
+<br>
+       if (!*label)<br>
+               return -ENOMEM;<br>
+<br>
+       if (component) {<br>
+               len = strlen(component);<br>
+               memcpy(*label, component, len);<br>
+       }<br>
+       llen = len;<br>
+</div>
<div class="PlainText elementToProof"><br>
</div>
<div class="PlainText elementToProof">I guess the 8, 1, and 5 are field lengths. Could they be a define or sizeof?</div>
<div class="PlainText elementToProof"><br>
+       if (component)<br>
+               len = 8 - strlen(component);<br>
+       else<br>
+               len = 8;<br>
+<br>
+       memset(*label + llen, 0, len);<br>
+       llen = llen + len;<br>
+<br>
+       ((*label)[llen]) = 0;<br>
+       llen = llen + 1;<br>
+<br>
+       memcpy(*label + llen, &varos, 1);<br>
+       llen = llen + 1;<br>
+<br>
+       memcpy(*label + llen, &labellength, 1);<br>
+       llen = llen + 1;<br>
+<br>
+       memset(*label + llen, 0, 5);<br>
+       llen = llen + 5;<br>
+<br>
+       memcpy(*label + llen, name, namelen);<br>
+       llen = llen + namelen;<br>
+<br>
+       for (i = 0; i < llen; i++)<br>
+               pr_debug("%c", (*label)[i]);<br>
+<br>
+       rc = llen;<br>
+       return rc;<br>
+}<br>
+<br>
+static int _plpks_get_config(void)<br>
+{<br>
+       unsigned long retbuf[PLPAR_HCALL_BUFSIZE] = {0};<br>
+       int rc;<br>
+       size_t size = sizeof(struct plpks_config);<br>
+<br>
+       config = kzalloc(size, GFP_KERNEL);<br>
+       if (!config)<br>
+               return -ENOMEM;<br>
+<br>
+       rc = plpar_hcall(H_PKS_GET_CONFIG,<br>
+                        retbuf,<br>
+                        virt_to_phys(config),<br>
+                        size);<br>
+<br>
+       if (rc != H_SUCCESS)</div>
<div class="PlainText elementToProof"><br>
</div>
<div class="PlainText elementToProof">Free config before returning the error.</div>
<div class="PlainText elementToProof"><br>
+               return pseries_status_to_err(rc);<br>
+<br>
+       config->rsvd0 = be32_to_cpu((__force __be32)config->rsvd0);<br>
+       config->maxpwsize = be16_to_cpu((__force __be16)config->maxpwsize);<br>
+       config->maxobjlabelsize = be16_to_cpu((__force __be16)config->maxobjlabelsize);<br>
+       config->maxobjsize = be16_to_cpu((__force __be16)config->maxobjsize);<br>
+       config->totalsize = be32_to_cpu((__force __be32)config->totalsize);<br>
+       config->usedspace = be32_to_cpu((__force __be32)config->usedspace);<br>
+       config->supportedpolicies = be32_to_cpu((__force __be32)config->supportedpolicies);<br>
+       config->rsvd1 = be64_to_cpu((__force __be64)config->rsvd1);<br>
+<br>
+       configset = true;<br>
+<br>
+       return 0;<br>
+}<br>
+<br>
+static int plpks_confirm_object_flushed(u8 *label, u16 labellen)<br>
+{<br>
+       unsigned long retbuf[PLPAR_HCALL_BUFSIZE] = {0};<br>
+       int rc;<br>
+       u64 timeout = 0;<br>
+       struct plpks_auth *auth;<br>
+       u8 status;<br>
+       int i;<br>
+</div>
<div class="PlainText elementToProof"><br>
</div>
<div class="PlainText elementToProof">Deleted pr_debugs? I guess this is a general question for all pr_debugs in this file.</div>
<div class="PlainText elementToProof"><br>
+       rc = construct_auth(PKS_OS_OWNER, &auth);<br>
+       if (rc)<br>
+               return rc;<br>
+<br>
+       for (i = 0; i < labellen; i++)<br>
+               pr_debug("%02x ", label[i]);<br>
+<br>
+       do {<br>
+               rc = plpar_hcall(H_PKS_CONFIRM_OBJECT_FLUSHED,<br>
+                                retbuf,<br>
+                                virt_to_phys(auth),<br>
+                                virt_to_phys(label),<br>
+                                labellen);<br>
+<br>
+               status = retbuf[0];<br>
+               if (rc) {<br>
+                       pr_info("rc is %d, status is %d\n", rc, status);<br>
+                       if (rc == H_NOT_FOUND && status == 1)<br>
+                               rc = 0;<br>
+                       break;<br>
+               }<br>
+<br>
+               pr_debug("rc is %d, status is %d\n", rc, status);<br>
+<br>
+               if (!rc && status == 1)<br>
+                       break;<br>
+<br>
+               usleep_range(PKS_FLUSH_SLEEP, PKS_FLUSH_SLEEP + PKS_FLUSH_SLEEP_RANGE);<br>
+               timeout = timeout + PKS_FLUSH_SLEEP;<br>
+               pr_debug("timeout is %llu\n", timeout);<br>
+<br>
+       } while (timeout < PKS_FLUSH_MAX_TIMEOUT);<br>
+<br>
+       rc = pseries_status_to_err(rc);<br>
+<br>
+       kfree(auth);<br>
+<br>
+       return rc;<br>
+}<br>
+<br>
<br>
+EXPORT_SYMBOL(plpks_remove_var);<br>
XPORT_SYMBOL(plpks_get_config);<br>
+<br>
+static __init int pseries_plpks_init(void)<br>
+{<br>
+       int rc = 0;<br>
+<br>
+       rc = _plpks_get_config();<br>
+<br>
+       if (rc) {</div>
<div class="PlainText elementToProof"><br>
</div>
<div class="PlainText elementToProof">I think this pr_err would be better if provided more info like the pr_info below.</div>
<div class="PlainText elementToProof"><br>
+               pr_err("Error initializing plpks\n");<br>
+               return rc;<br>
+       }<br>
+<br>
+       rc = plpks_gen_password();<br>
+       if (rc) {<br>
+               if (rc == H_IN_USE) {<br>
+                       rc = 0;<br>
+               } else {</div>
<div class="PlainText elementToProof"><br>
</div>
<div class="PlainText elementToProof"><span style="background-color:rgb(255, 255, 255);display:inline !important">I think this pr_err would be better if provided more info like the pr_info below.</span></div>
<div class="PlainText elementToProof"><br>
+                       pr_err("Failed setting password %d\n", rc);<br>
+                       rc = pseries_status_to_err(rc);<br>
+                       return rc;<br>
+               }<br>
+       }<br>
+<br>
+       pr_info("POWER LPAR Platform Keystore initialized successfully\n");<br>
+<br>
+       return rc;<br>
+}<br>
+arch_initcall(pseries_plpks_init);<br>
-- <br>
2.27.0<br>
<br>
</div>
</span></font></div>
</body>
</html>