Re: [tpmdd-devel] [PATCH v7 01/10] tpm: merge duplicate transmit_cmd() functions

From: Jarkko Sakkinen
Date: Thu Nov 27 2014 - 06:43:54 EST

Next message: Lee Jones: "[PATCH 0/4] spi: st: New driver for ST's SPI Controller"
Previous message: Arnd Bergmann: "Re: [PATCH 11/19] clk: samsung: exynos5433: Add clocks for CMU_BUS{0|1|2} domains"
In reply to: Stefan Berger: "Re: [tpmdd-devel] [PATCH v7 01/10] tpm: merge duplicate transmit_cmd() functions"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Tue, Nov 25, 2014 at 04:18:39PM -0500, Stefan Berger wrote:
> On 11/11/2014 08:45 AM, Jarkko Sakkinen wrote:
> >Merged transmit_cmd() functions in tpm-interface.c and tpm-sysfs.c.
> >Added "tpm_" prefix for consistency sake. Changed cmd parameter as
> >opaque. This enables to use separate command structures for TPM1
> >and TPM2 commands in future. Loose coupling works fine here.
> >
> >Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@xxxxxxxxxxxxxxx>
> >---
> > drivers/char/tpm/tpm-interface.c | 49 +++++++++++++++++++++-------------------
> > drivers/char/tpm/tpm-sysfs.c | 23 ++-----------------
> > drivers/char/tpm/tpm.h | 3 ++-
> > 3 files changed, 30 insertions(+), 45 deletions(-)
> >
> >diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> >index 6af1700..0150b7c 100644
> >--- a/drivers/char/tpm/tpm-interface.c
> >+++ b/drivers/char/tpm/tpm-interface.c
> >@@ -398,9 +398,10 @@ out:
> > #define TPM_DIGEST_SIZE 20
> > #define TPM_RET_CODE_IDX 6
> >
> >-static ssize_t transmit_cmd(struct tpm_chip *chip, struct tpm_cmd_t *cmd,
> >- int len, const char *desc)
> >+ssize_t tpm_transmit_cmd(struct tpm_chip *chip, void *cmd,
> >+ int len, const char *desc)
> > {
> >+ struct tpm_output_header *header;
> > int err;
> >
> > len = tpm_transmit(chip, (u8 *) cmd, len);
> >@@ -409,7 +410,9 @@ static ssize_t transmit_cmd(struct tpm_chip *chip, struct tpm_cmd_t *cmd,
> > else if (len < TPM_HEADER_SIZE)
> > return -EFAULT;
> >
> >- err = be32_to_cpu(cmd->header.out.return_code);
> >+ header = (struct tpm_output_header *) cmd;
>
> The cast should not be necessary -- and this change doesn't buy much...
>
> header = &cmd->header.out;
>
> Should do the trick without cast.

I changed the cmd parameter opaque to make this function work both with
TPM1 and TPM2 easily so that I do not have to drop everything to
struct tpm_cmd_t.

I can change this to

header = cmd;

> >+
> >+ err = be32_to_cpu(header->return_code);
> > if (err != 0 && desc)
> > dev_err(chip->dev, "A TPM error (%d) occurred %s\n", err, desc);
> >
> >@@ -448,7 +451,7 @@ ssize_t tpm_getcap(struct device *dev, __be32 subcap_id, cap_t *cap,
> > tpm_cmd.params.getcap_in.subcap_size = cpu_to_be32(4);
> > tpm_cmd.params.getcap_in.subcap = subcap_id;
> > }
> >- rc = transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, desc);
> >+ rc = tpm_transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, desc);
> > if (!rc)
> > *cap = tpm_cmd.params.getcap_out.cap;
> > return rc;
> >@@ -464,8 +467,8 @@ void tpm_gen_interrupt(struct tpm_chip *chip)
> > tpm_cmd.params.getcap_in.subcap_size = cpu_to_be32(4);
> > tpm_cmd.params.getcap_in.subcap = TPM_CAP_PROP_TIS_TIMEOUT;
> >
> >- rc = transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
> >- "attempting to determine the timeouts");
> >+ rc = tpm_transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
> >+ "attempting to determine the timeouts");
> > }
> > EXPORT_SYMBOL_GPL(tpm_gen_interrupt);
> >
> >@@ -484,8 +487,8 @@ static int tpm_startup(struct tpm_chip *chip, __be16 startup_type)
> > struct tpm_cmd_t start_cmd;
> > start_cmd.header.in = tpm_startup_header;
> > start_cmd.params.startup_in.startup_type = startup_type;
> >- return transmit_cmd(chip, &start_cmd, TPM_INTERNAL_RESULT_SIZE,
> >- "attempting to start the TPM");
> >+ return tpm_transmit_cmd(chip, &start_cmd, TPM_INTERNAL_RESULT_SIZE,
> >+ "attempting to start the TPM");
> > }
> >
> > int tpm_get_timeouts(struct tpm_chip *chip)
> >@@ -500,7 +503,7 @@ int tpm_get_timeouts(struct tpm_chip *chip)
> > tpm_cmd.params.getcap_in.cap = TPM_CAP_PROP;
> > tpm_cmd.params.getcap_in.subcap_size = cpu_to_be32(4);
> > tpm_cmd.params.getcap_in.subcap = TPM_CAP_PROP_TIS_TIMEOUT;
> >- rc = transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, NULL);
> >+ rc = tpm_transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, NULL);
> >
> > if (rc == TPM_ERR_INVALID_POSTINIT) {
> > /* The TPM is not started, we are the first to talk to it.
> >@@ -513,7 +516,7 @@ int tpm_get_timeouts(struct tpm_chip *chip)
> > tpm_cmd.params.getcap_in.cap = TPM_CAP_PROP;
> > tpm_cmd.params.getcap_in.subcap_size = cpu_to_be32(4);
> > tpm_cmd.params.getcap_in.subcap = TPM_CAP_PROP_TIS_TIMEOUT;
> >- rc = transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
> >+ rc = tpm_transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
> > NULL);
> > }
> > if (rc) {
> >@@ -575,8 +578,8 @@ duration:
> > tpm_cmd.params.getcap_in.subcap_size = cpu_to_be32(4);
> > tpm_cmd.params.getcap_in.subcap = TPM_CAP_PROP_TIS_DURATION;
> >
> >- rc = transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
> >- "attempting to determine the durations");
> >+ rc = tpm_transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
> >+ "attempting to determine the durations");
> > if (rc)
> > return rc;
> >
> >@@ -631,8 +634,8 @@ static int tpm_continue_selftest(struct tpm_chip *chip)
> > struct tpm_cmd_t cmd;
> >
> > cmd.header.in = continue_selftest_header;
> >- rc = transmit_cmd(chip, &cmd, CONTINUE_SELFTEST_RESULT_SIZE,
> >- "continue selftest");
> >+ rc = tpm_transmit_cmd(chip, &cmd, CONTINUE_SELFTEST_RESULT_SIZE,
> >+ "continue selftest");
> > return rc;
> > }
> >
> >@@ -672,8 +675,8 @@ int tpm_pcr_read_dev(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
> >
> > cmd.header.in = pcrread_header;
> > cmd.params.pcrread_in.pcr_idx = cpu_to_be32(pcr_idx);
> >- rc = transmit_cmd(chip, &cmd, READ_PCR_RESULT_SIZE,
> >- "attempting to read a pcr value");
> >+ rc = tpm_transmit_cmd(chip, &cmd, READ_PCR_RESULT_SIZE,
> >+ "attempting to read a pcr value");
> >
> > if (rc == 0)
> > memcpy(res_buf, cmd.params.pcrread_out.pcr_result,
> >@@ -737,8 +740,8 @@ int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash)
> > cmd.header.in = pcrextend_header;
> > cmd.params.pcrextend_in.pcr_idx = cpu_to_be32(pcr_idx);
> > memcpy(cmd.params.pcrextend_in.hash, hash, TPM_DIGEST_SIZE);
> >- rc = transmit_cmd(chip, &cmd, EXTEND_PCR_RESULT_SIZE,
> >- "attempting extend a PCR value");
> >+ rc = tpm_transmit_cmd(chip, &cmd, EXTEND_PCR_RESULT_SIZE,
> >+ "attempting extend a PCR value");
> >
> > tpm_chip_put(chip);
> > return rc;
> >@@ -817,7 +820,7 @@ int tpm_send(u32 chip_num, void *cmd, size_t buflen)
> > if (chip == NULL)
> > return -ENODEV;
> >
> >- rc = transmit_cmd(chip, cmd, buflen, "attempting tpm_cmd");
> >+ rc = tpm_transmit_cmd(chip, cmd, buflen, "attempting tpm_cmd");
> >
> > tpm_chip_put(chip);
> > return rc;
> >@@ -938,14 +941,14 @@ int tpm_pm_suspend(struct device *dev)
> > cmd.params.pcrextend_in.pcr_idx = cpu_to_be32(tpm_suspend_pcr);
> > memcpy(cmd.params.pcrextend_in.hash, dummy_hash,
> > TPM_DIGEST_SIZE);
> >- rc = transmit_cmd(chip, &cmd, EXTEND_PCR_RESULT_SIZE,
> >- "extending dummy pcr before suspend");
> >+ rc = tpm_transmit_cmd(chip, &cmd, EXTEND_PCR_RESULT_SIZE,
> >+ "extending dummy pcr before suspend");
> > }
> >
> > /* now do the actual savestate */
> > for (try = 0; try < TPM_RETRY; try++) {
> > cmd.header.in = savestate_header;
> >- rc = transmit_cmd(chip, &cmd, SAVESTATE_RESULT_SIZE, NULL);
> >+ rc = tpm_transmit_cmd(chip, &cmd, SAVESTATE_RESULT_SIZE, NULL);
> >
> > /*
> > * If the TPM indicates that it is too busy to respond to
> >@@ -1022,7 +1025,7 @@ int tpm_get_random(u32 chip_num, u8 *out, size_t max)
> > tpm_cmd.header.in = tpm_getrandom_header;
> > tpm_cmd.params.getrandom_in.num_bytes = cpu_to_be32(num_bytes);
> >
> >- err = transmit_cmd(chip, &tpm_cmd,
> >+ err = tpm_transmit_cmd(chip, &tpm_cmd,
> > TPM_GETRANDOM_RESULT_SIZE + num_bytes,
> > "attempting get random");
> > if (err)
> >diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
> >index 01730a2..8ecb052 100644
> >--- a/drivers/char/tpm/tpm-sysfs.c
> >+++ b/drivers/char/tpm/tpm-sysfs.c
> >@@ -20,25 +20,6 @@
> > #include <linux/device.h>
> > #include "tpm.h"
> >
> >-/* XXX for now this helper is duplicated in tpm-interface.c */
> >-static ssize_t transmit_cmd(struct tpm_chip *chip, struct tpm_cmd_t *cmd,
> >- int len, const char *desc)
> >-{
> >- int err;
> >-
> >- len = tpm_transmit(chip, (u8 *) cmd, len);
> >- if (len < 0)
> >- return len;
> >- else if (len < TPM_HEADER_SIZE)
> >- return -EFAULT;
> >-
> >- err = be32_to_cpu(cmd->header.out.return_code);
> >- if (err != 0 && desc)
> >- dev_err(chip->dev, "A TPM error (%d) occurred %s\n", err, desc);
> >-
> >- return err;
> >-}
> >-
> > #define READ_PUBEK_RESULT_SIZE 314
> > #define TPM_ORD_READPUBEK cpu_to_be32(124)
> > static struct tpm_input_header tpm_readpubek_header = {
> >@@ -58,8 +39,8 @@ static ssize_t pubek_show(struct device *dev, struct device_attribute *attr,
> > struct tpm_chip *chip = dev_get_drvdata(dev);
> >
> > tpm_cmd.header.in = tpm_readpubek_header;
> >- err = transmit_cmd(chip, &tpm_cmd, READ_PUBEK_RESULT_SIZE,
> >- "attempting to read the PUBEK");
> >+ err = tpm_transmit_cmd(chip, &tpm_cmd, READ_PUBEK_RESULT_SIZE,
> >+ "attempting to read the PUBEK");
> > if (err)
> > goto out;
> >
> >diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> >index e4d0888..e638eb0 100644
> >--- a/drivers/char/tpm/tpm.h
> >+++ b/drivers/char/tpm/tpm.h
> >@@ -314,9 +314,10 @@ struct tpm_cmd_t {
> > } __packed;
> >
> > ssize_t tpm_getcap(struct device *, __be32, cap_t *, const char *);
> >-
> > ssize_t tpm_transmit(struct tpm_chip *chip, const char *buf,
> > size_t bufsiz);
>
> Delete this prototype ?

tpm-dev.c uses tpm_transmit() and tpm_transmit_cmd() does unnecessary
steps for that use.

> >+ssize_t tpm_transmit_cmd(struct tpm_chip *chip, void *cmd, int len,
> >+ const char *desc);
> > extern int tpm_get_timeouts(struct tpm_chip *);
> > extern void tpm_gen_interrupt(struct tpm_chip *);
> > extern int tpm_do_selftest(struct tpm_chip *);
>
>
> Stefan

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Next message: Lee Jones: "[PATCH 0/4] spi: st: New driver for ST's SPI Controller"
Previous message: Arnd Bergmann: "Re: [PATCH 11/19] clk: samsung: exynos5433: Add clocks for CMU_BUS{0|1|2} domains"
In reply to: Stefan Berger: "Re: [tpmdd-devel] [PATCH v7 01/10] tpm: merge duplicate transmit_cmd() functions"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]