diff mbox series

[v2,5/5] tpm: ppi: Enable submission of optional command parameter for PPI 1.3

Message ID 20190117174135.2117937-6-stefanb@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series Extend TPM PPI interface to support revision 1.3 | expand

Commit Message

Stefan Berger Jan. 17, 2019, 5:41 p.m. UTC 
This patch enables a user to specify the additional optional command
parameter by writing it into the request file:

   # echo "23 16" > request
   # cat request
   23 16

For backwards compatibility:

If only 1 parameter is given then we assume this is the operation request
number.

   # echo "5" > request
   # cat request
   5

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Tested-by: David Safford <david.safford@ge.com>
---
 drivers/char/tpm/tpm_ppi.c | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

Comments

Jarkko Sakkinen Jan. 22, 2019, 12:01 p.m. UTC | #1 
On Thu, Jan 17, 2019 at 12:41:35PM -0500, Stefan Berger wrote:
> This patch enables a user to specify the additional optional command
> parameter by writing it into the request file:
> 
>    # echo "23 16" > request
>    # cat request
>    23 16
> 
> For backwards compatibility:
> 
> If only 1 parameter is given then we assume this is the operation request
> number.
> 
>    # echo "5" > request
>    # cat request
>    5
> 
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> Tested-by: David Safford <david.safford@ge.com>

Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

/Jarkko
Jarkko Sakkinen Jan. 22, 2019, 12:10 p.m. UTC | #2 
On Tue, Jan 22, 2019 at 02:01:59PM +0200, Jarkko Sakkinen wrote:
> On Thu, Jan 17, 2019 at 12:41:35PM -0500, Stefan Berger wrote:
> > This patch enables a user to specify the additional optional command
> > parameter by writing it into the request file:
> > 
> >    # echo "23 16" > request
> >    # cat request
> >    23 16
> > 
> > For backwards compatibility:
> > 
> > If only 1 parameter is given then we assume this is the operation request
> > number.
> > 
> >    # echo "5" > request
> >    # cat request
> >    5
> > 
> > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> > Tested-by: David Safford <david.safford@ge.com>
> 
> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

Oops, sorry, this was meant for 4/5, so please ignore that.

The goto statement in tpm_store_ppi_request() is a bit messy as you
jump between branches. Definitely works, but very unreadable.

To get around it, you could change the branching as

if (strcmp(chip->ppi_version, "1.3") == 0 &&
    sscanf(buf, "%llu %llu", &tmp[0].integer.value,
	   &tmp[1].integer.value) == 2) {
	/* ... */
} else if (strcmp(chip->ppi_version, "1.2") == 0) {
	/* ... */
} else {
	/* ... */
}

/Jarkko
diff mbox series

Patch

diff --git a/drivers/char/tpm/tpm_ppi.c b/drivers/char/tpm/tpm_ppi.c
index 942a2f79e9da..75e7a856177c 100644
--- a/drivers/char/tpm/tpm_ppi.c
+++ b/drivers/char/tpm/tpm_ppi.c
@@ -120,9 +120,10 @@  static ssize_t tpm_store_ppi_request(struct device *dev,
 	u32 req;
 	u64 ret;
 	int func = TPM_PPI_FN_SUBREQ;
-	union acpi_object *obj, tmp;
-	union acpi_object argv4 = ACPI_INIT_DSM_ARGV4(1, &tmp);
+	union acpi_object *obj, tmp[2];
+	union acpi_object argv4 = ACPI_INIT_DSM_ARGV4(2, tmp);
 	struct tpm_chip *chip = to_tpm_chip(dev);
+	u64 rev = TPM_PPI_REVISION_ID_1;
 
 	/*
 	 * the function to submit TPM operation request to pre-os environment
@@ -139,20 +140,29 @@  static ssize_t tpm_store_ppi_request(struct device *dev,
 	 * string/package type. For PPI version 1.0 and 1.1, use buffer type
 	 * for compatibility, and use package type since 1.2 according to spec.
 	 */
-	if (strcmp(chip->ppi_version, "1.2") < 0) {
+	if (strcmp(chip->ppi_version, "1.3") == 0) {
+		if (sscanf(buf, "%llu %llu", &tmp[0].integer.value,
+			   &tmp[1].integer.value) != 2)
+			goto ppi12;
+		rev = TPM_PPI_REVISION_ID_2;
+		tmp[0].type = ACPI_TYPE_INTEGER;
+		tmp[1].type = ACPI_TYPE_INTEGER;
+	} else if (strcmp(chip->ppi_version, "1.2") < 0) {
 		if (sscanf(buf, "%d", &req) != 1)
 			return -EINVAL;
 		argv4.type = ACPI_TYPE_BUFFER;
 		argv4.buffer.length = sizeof(req);
 		argv4.buffer.pointer = (u8 *)&req;
 	} else {
-		tmp.type = ACPI_TYPE_INTEGER;
-		if (sscanf(buf, "%llu", &tmp.integer.value) != 1)
+ppi12:
+		argv4.package.count = 1;
+		tmp[0].type = ACPI_TYPE_INTEGER;
+		if (sscanf(buf, "%llu", &tmp[0].integer.value) != 1)
 			return -EINVAL;
 	}
 
 	obj = tpm_eval_dsm(chip->acpi_dev_handle, func, ACPI_TYPE_INTEGER,
-			   &argv4, TPM_PPI_REVISION_ID_1);
+			   &argv4, rev);
 	if (!obj) {
 		return -ENXIO;
 	} else {