* [PATCH] tpm: Remove duplicate code from caps_show() in tpm-sysfs.c
@ 2019-08-29 14:38 Jarkko Sakkinen
2019-08-29 20:27 ` Jerry Snitselaar
2019-08-30 16:09 ` Jerry Snitselaar
0 siblings, 2 replies; 4+ messages in thread
From: Jarkko Sakkinen @ 2019-08-29 14:38 UTC (permalink / raw)
To: linux-integrity
Cc: Jarkko Sakkinen, Alexey Klimov, Jerry Snitselaar, Peter Huewe,
Jason Gunthorpe, Arnd Bergmann, Greg Kroah-Hartman, open list
Replace existing TPM 1.x version structs with new structs that consolidate
the common parts into a single struct so that code duplication is no longer
needed in caps_show().
Cc: Alexey Klimov <aklimov@redhat.com>
Cc: Jerry Snitselaar <jsnitsel@redhat.com>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
Jerry, Alexey: Plese include this to the next version of your patches.
This a low priority patch alone so it does not need to be merge upfront.
drivers/char/tpm/tpm-sysfs.c | 44 ++++++++++++++++++------------------
drivers/char/tpm/tpm.h | 23 ++++++++-----------
2 files changed, 32 insertions(+), 35 deletions(-)
diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index edfa89160010..8064fea2de59 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -217,6 +217,7 @@ static ssize_t caps_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
struct tpm_chip *chip = to_tpm_chip(dev);
+ struct tpm1_version *version;
ssize_t rc = 0;
char *str = buf;
cap_t cap;
@@ -232,31 +233,30 @@ static ssize_t caps_show(struct device *dev, struct device_attribute *attr,
str += sprintf(str, "Manufacturer: 0x%x\n",
be32_to_cpu(cap.manufacturer_id));
- /* Try to get a TPM version 1.2 TPM_CAP_VERSION_INFO */
- rc = tpm1_getcap(chip, TPM_CAP_VERSION_1_2, &cap,
+ /* TPM 1.2 */
+ if (!tpm1_getcap(chip, TPM_CAP_VERSION_1_2, &cap,
"attempting to determine the 1.2 version",
- sizeof(cap.tpm_version_1_2));
- if (!rc) {
- str += sprintf(str,
- "TCG version: %d.%d\nFirmware version: %d.%d\n",
- cap.tpm_version_1_2.Major,
- cap.tpm_version_1_2.Minor,
- cap.tpm_version_1_2.revMajor,
- cap.tpm_version_1_2.revMinor);
- } else {
- /* Otherwise just use TPM_STRUCT_VER */
- if (tpm1_getcap(chip, TPM_CAP_VERSION_1_1, &cap,
- "attempting to determine the 1.1 version",
- sizeof(cap.tpm_version)))
- goto out_ops;
- str += sprintf(str,
- "TCG version: %d.%d\nFirmware version: %d.%d\n",
- cap.tpm_version.Major,
- cap.tpm_version.Minor,
- cap.tpm_version.revMajor,
- cap.tpm_version.revMinor);
+ sizeof(cap.version2))) {
+ version = &cap.version2.version;
+ goto out_print;
}
+
+ /* TPM 1.1 */
+ if (tpm1_getcap(chip, TPM_CAP_VERSION_1_1, &cap,
+ "attempting to determine the 1.1 version",
+ sizeof(cap.version1))) {
+ version = &cap.version1;
+ goto out_ops;
+ }
+
+out_print:
+ str += sprintf(str,
+ "TCG version: %d.%d\nFirmware version: %d.%d\n",
+ version->major, version->minor,
+ version->rev_major, version->rev_minor);
+
rc = str - buf;
+
out_ops:
tpm_put_ops(chip);
return rc;
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index a7fea3e0ca86..a4f74dd02a35 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -186,19 +186,16 @@ struct stclear_flags_t {
u8 bGlobalLock;
} __packed;
-struct tpm_version_t {
- u8 Major;
- u8 Minor;
- u8 revMajor;
- u8 revMinor;
+struct tpm1_version {
+ u8 major;
+ u8 minor;
+ u8 rev_major;
+ u8 rev_minor;
} __packed;
-struct tpm_version_1_2_t {
- __be16 tag;
- u8 Major;
- u8 Minor;
- u8 revMajor;
- u8 revMinor;
+struct tpm1_version2 {
+ __be16 tag;
+ struct tpm1_version version;
} __packed;
struct timeout_t {
@@ -243,8 +240,8 @@ typedef union {
struct stclear_flags_t stclear_flags;
__u8 owned;
__be32 num_pcrs;
- struct tpm_version_t tpm_version;
- struct tpm_version_1_2_t tpm_version_1_2;
+ struct tpm1_version version1;
+ struct tpm1_version2 version2;
__be32 manufacturer_id;
struct timeout_t timeout;
struct duration_t duration;
--
2.20.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] tpm: Remove duplicate code from caps_show() in tpm-sysfs.c
2019-08-29 14:38 [PATCH] tpm: Remove duplicate code from caps_show() in tpm-sysfs.c Jarkko Sakkinen
@ 2019-08-29 20:27 ` Jerry Snitselaar
2019-08-30 16:09 ` Jerry Snitselaar
1 sibling, 0 replies; 4+ messages in thread
From: Jerry Snitselaar @ 2019-08-29 20:27 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: linux-integrity, Alexey Klimov, Peter Huewe, Jason Gunthorpe,
Arnd Bergmann, Greg Kroah-Hartman, open list
On Thu Aug 29 19, Jarkko Sakkinen wrote:
>Replace existing TPM 1.x version structs with new structs that consolidate
>the common parts into a single struct so that code duplication is no longer
>needed in caps_show().
>
>Cc: Alexey Klimov <aklimov@redhat.com>
>Cc: Jerry Snitselaar <jsnitsel@redhat.com>
>Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
>---
>Jerry, Alexey: Plese include this to the next version of your patches.
>This a low priority patch alone so it does not need to be merge upfront.
Will do. I'll add my Reviewed-by and Tested-by to it and submit it with v3.
> drivers/char/tpm/tpm-sysfs.c | 44 ++++++++++++++++++------------------
> drivers/char/tpm/tpm.h | 23 ++++++++-----------
> 2 files changed, 32 insertions(+), 35 deletions(-)
>
>diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
>index edfa89160010..8064fea2de59 100644
>--- a/drivers/char/tpm/tpm-sysfs.c
>+++ b/drivers/char/tpm/tpm-sysfs.c
>@@ -217,6 +217,7 @@ static ssize_t caps_show(struct device *dev, struct device_attribute *attr,
> char *buf)
> {
> struct tpm_chip *chip = to_tpm_chip(dev);
>+ struct tpm1_version *version;
> ssize_t rc = 0;
> char *str = buf;
> cap_t cap;
>@@ -232,31 +233,30 @@ static ssize_t caps_show(struct device *dev, struct device_attribute *attr,
> str += sprintf(str, "Manufacturer: 0x%x\n",
> be32_to_cpu(cap.manufacturer_id));
>
>- /* Try to get a TPM version 1.2 TPM_CAP_VERSION_INFO */
>- rc = tpm1_getcap(chip, TPM_CAP_VERSION_1_2, &cap,
>+ /* TPM 1.2 */
>+ if (!tpm1_getcap(chip, TPM_CAP_VERSION_1_2, &cap,
> "attempting to determine the 1.2 version",
>- sizeof(cap.tpm_version_1_2));
>- if (!rc) {
>- str += sprintf(str,
>- "TCG version: %d.%d\nFirmware version: %d.%d\n",
>- cap.tpm_version_1_2.Major,
>- cap.tpm_version_1_2.Minor,
>- cap.tpm_version_1_2.revMajor,
>- cap.tpm_version_1_2.revMinor);
>- } else {
>- /* Otherwise just use TPM_STRUCT_VER */
>- if (tpm1_getcap(chip, TPM_CAP_VERSION_1_1, &cap,
>- "attempting to determine the 1.1 version",
>- sizeof(cap.tpm_version)))
>- goto out_ops;
>- str += sprintf(str,
>- "TCG version: %d.%d\nFirmware version: %d.%d\n",
>- cap.tpm_version.Major,
>- cap.tpm_version.Minor,
>- cap.tpm_version.revMajor,
>- cap.tpm_version.revMinor);
>+ sizeof(cap.version2))) {
>+ version = &cap.version2.version;
>+ goto out_print;
> }
>+
>+ /* TPM 1.1 */
>+ if (tpm1_getcap(chip, TPM_CAP_VERSION_1_1, &cap,
>+ "attempting to determine the 1.1 version",
>+ sizeof(cap.version1))) {
>+ version = &cap.version1;
>+ goto out_ops;
>+ }
>+
>+out_print:
>+ str += sprintf(str,
>+ "TCG version: %d.%d\nFirmware version: %d.%d\n",
>+ version->major, version->minor,
>+ version->rev_major, version->rev_minor);
>+
> rc = str - buf;
>+
> out_ops:
> tpm_put_ops(chip);
> return rc;
>diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
>index a7fea3e0ca86..a4f74dd02a35 100644
>--- a/drivers/char/tpm/tpm.h
>+++ b/drivers/char/tpm/tpm.h
>@@ -186,19 +186,16 @@ struct stclear_flags_t {
> u8 bGlobalLock;
> } __packed;
>
>-struct tpm_version_t {
>- u8 Major;
>- u8 Minor;
>- u8 revMajor;
>- u8 revMinor;
>+struct tpm1_version {
>+ u8 major;
>+ u8 minor;
>+ u8 rev_major;
>+ u8 rev_minor;
> } __packed;
>
>-struct tpm_version_1_2_t {
>- __be16 tag;
>- u8 Major;
>- u8 Minor;
>- u8 revMajor;
>- u8 revMinor;
>+struct tpm1_version2 {
>+ __be16 tag;
>+ struct tpm1_version version;
> } __packed;
>
> struct timeout_t {
>@@ -243,8 +240,8 @@ typedef union {
> struct stclear_flags_t stclear_flags;
> __u8 owned;
> __be32 num_pcrs;
>- struct tpm_version_t tpm_version;
>- struct tpm_version_1_2_t tpm_version_1_2;
>+ struct tpm1_version version1;
>+ struct tpm1_version2 version2;
> __be32 manufacturer_id;
> struct timeout_t timeout;
> struct duration_t duration;
>--
>2.20.1
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] tpm: Remove duplicate code from caps_show() in tpm-sysfs.c
2019-08-29 14:38 [PATCH] tpm: Remove duplicate code from caps_show() in tpm-sysfs.c Jarkko Sakkinen
2019-08-29 20:27 ` Jerry Snitselaar
@ 2019-08-30 16:09 ` Jerry Snitselaar
2019-09-02 13:35 ` Jarkko Sakkinen
1 sibling, 1 reply; 4+ messages in thread
From: Jerry Snitselaar @ 2019-08-30 16:09 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: linux-integrity, Alexey Klimov, Peter Huewe, Jason Gunthorpe,
Arnd Bergmann, Greg Kroah-Hartman, open list
On Thu Aug 29 19, Jarkko Sakkinen wrote:
>Replace existing TPM 1.x version structs with new structs that consolidate
>the common parts into a single struct so that code duplication is no longer
>needed in caps_show().
>
>Cc: Alexey Klimov <aklimov@redhat.com>
>Cc: Jerry Snitselaar <jsnitsel@redhat.com>
>Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
>---
>Jerry, Alexey: Plese include this to the next version of your patches.
>This a low priority patch alone so it does not need to be merge upfront.
> drivers/char/tpm/tpm-sysfs.c | 44 ++++++++++++++++++------------------
> drivers/char/tpm/tpm.h | 23 ++++++++-----------
> 2 files changed, 32 insertions(+), 35 deletions(-)
>
>diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
>index edfa89160010..8064fea2de59 100644
>--- a/drivers/char/tpm/tpm-sysfs.c
>+++ b/drivers/char/tpm/tpm-sysfs.c
>@@ -217,6 +217,7 @@ static ssize_t caps_show(struct device *dev, struct device_attribute *attr,
> char *buf)
> {
> struct tpm_chip *chip = to_tpm_chip(dev);
>+ struct tpm1_version *version;
> ssize_t rc = 0;
> char *str = buf;
> cap_t cap;
>@@ -232,31 +233,30 @@ static ssize_t caps_show(struct device *dev, struct device_attribute *attr,
> str += sprintf(str, "Manufacturer: 0x%x\n",
> be32_to_cpu(cap.manufacturer_id));
>
>- /* Try to get a TPM version 1.2 TPM_CAP_VERSION_INFO */
>- rc = tpm1_getcap(chip, TPM_CAP_VERSION_1_2, &cap,
>+ /* TPM 1.2 */
>+ if (!tpm1_getcap(chip, TPM_CAP_VERSION_1_2, &cap,
> "attempting to determine the 1.2 version",
>- sizeof(cap.tpm_version_1_2));
>- if (!rc) {
>- str += sprintf(str,
>- "TCG version: %d.%d\nFirmware version: %d.%d\n",
>- cap.tpm_version_1_2.Major,
>- cap.tpm_version_1_2.Minor,
>- cap.tpm_version_1_2.revMajor,
>- cap.tpm_version_1_2.revMinor);
>- } else {
>- /* Otherwise just use TPM_STRUCT_VER */
>- if (tpm1_getcap(chip, TPM_CAP_VERSION_1_1, &cap,
>- "attempting to determine the 1.1 version",
>- sizeof(cap.tpm_version)))
>- goto out_ops;
>- str += sprintf(str,
>- "TCG version: %d.%d\nFirmware version: %d.%d\n",
>- cap.tpm_version.Major,
>- cap.tpm_version.Minor,
>- cap.tpm_version.revMajor,
>- cap.tpm_version.revMinor);
>+ sizeof(cap.version2))) {
>+ version = &cap.version2.version;
>+ goto out_print;
> }
>+
>+ /* TPM 1.1 */
>+ if (tpm1_getcap(chip, TPM_CAP_VERSION_1_1, &cap,
>+ "attempting to determine the 1.1 version",
>+ sizeof(cap.version1))) {
>+ version = &cap.version1;
Actually looking at this again, this seems wrong. The version assignment here should be below this if, or in an else block, yes?
>+ goto out_ops;
>+ }
>+
>+out_print:
>+ str += sprintf(str,
>+ "TCG version: %d.%d\nFirmware version: %d.%d\n",
>+ version->major, version->minor,
>+ version->rev_major, version->rev_minor);
>+
> rc = str - buf;
>+
> out_ops:
> tpm_put_ops(chip);
> return rc;
>diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
>index a7fea3e0ca86..a4f74dd02a35 100644
>--- a/drivers/char/tpm/tpm.h
>+++ b/drivers/char/tpm/tpm.h
>@@ -186,19 +186,16 @@ struct stclear_flags_t {
> u8 bGlobalLock;
> } __packed;
>
>-struct tpm_version_t {
>- u8 Major;
>- u8 Minor;
>- u8 revMajor;
>- u8 revMinor;
>+struct tpm1_version {
>+ u8 major;
>+ u8 minor;
>+ u8 rev_major;
>+ u8 rev_minor;
> } __packed;
>
>-struct tpm_version_1_2_t {
>- __be16 tag;
>- u8 Major;
>- u8 Minor;
>- u8 revMajor;
>- u8 revMinor;
>+struct tpm1_version2 {
>+ __be16 tag;
>+ struct tpm1_version version;
> } __packed;
>
> struct timeout_t {
>@@ -243,8 +240,8 @@ typedef union {
> struct stclear_flags_t stclear_flags;
> __u8 owned;
> __be32 num_pcrs;
>- struct tpm_version_t tpm_version;
>- struct tpm_version_1_2_t tpm_version_1_2;
>+ struct tpm1_version version1;
>+ struct tpm1_version2 version2;
> __be32 manufacturer_id;
> struct timeout_t timeout;
> struct duration_t duration;
>--
>2.20.1
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] tpm: Remove duplicate code from caps_show() in tpm-sysfs.c
2019-08-30 16:09 ` Jerry Snitselaar
@ 2019-09-02 13:35 ` Jarkko Sakkinen
0 siblings, 0 replies; 4+ messages in thread
From: Jarkko Sakkinen @ 2019-09-02 13:35 UTC (permalink / raw)
To: linux-integrity, Alexey Klimov, Peter Huewe, Jason Gunthorpe,
Arnd Bergmann, Greg Kroah-Hartman, open list
On Fri, Aug 30, 2019 at 09:09:02AM -0700, Jerry Snitselaar wrote:
> On Thu Aug 29 19, Jarkko Sakkinen wrote:
> + /* TPM 1.1 */
> > + if (tpm1_getcap(chip, TPM_CAP_VERSION_1_1, &cap,
> > + "attempting to determine the 1.1 version",
> > + sizeof(cap.version1))) {
> > + version = &cap.version1;
:>
> Actually looking at this again, this seems wrong. The version
> assignment here should be below this if, or in an else block, yes?
Absolutely is in a wrong place.
I sent an updated patch:
https://patchwork.kernel.org/patch/11126741/
/Jarkko
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-09-02 13:35 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-29 14:38 [PATCH] tpm: Remove duplicate code from caps_show() in tpm-sysfs.c Jarkko Sakkinen
2019-08-29 20:27 ` Jerry Snitselaar
2019-08-30 16:09 ` Jerry Snitselaar
2019-09-02 13:35 ` Jarkko Sakkinen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).