On Wed, Feb 04, 2026 at 09:44:34AM -0800, [email protected] wrote:
> On 2/1/26 8:23 AM, Daniel P. Smith wrote:
> > On 1/19/26 18:57, Jarkko Sakkinen wrote:
> > > On Mon, Dec 15, 2025 at 03:32:50PM -0800, Ross Philipson wrote:
> > > > This gathers all the TPM1 definitions and structures into two separate
> > > > header files (public tpm1.h and private tpm1_structs.h). The definitions
> > > > moved to these files correspond to the TCG specification for TPM 1
> > > > family:
> > > >
> > > > TPM 1.2 Main Specification
> > > > -
> > > > https://urldefense.com/v3/__https://trustedcomputinggroup.org/resource/tpm-main-specification/__;!!ACWV5N9M2RV99hQ!OivTxO-R0nvBJYXIV-T0n0fq0wY64MTgdaecPzAauxrFkRxTcQ6CuBEcXmZZYY1KfZlqmfLTZBbPaEr7Y8k7hJk9_mQ$
> > > > Note that the structures were pulled into tpm1_structs.h to allow their
> > > > external reuse.
> > > >
> > > > Signed-off-by: Daniel P. Smith <[email protected]>
> > > > Signed-off-by: Ross Philipson <[email protected]>
> > > > ---
> > > > drivers/char/tpm/tpm.h | 98 +--------------------------------
> > > > drivers/char/tpm/tpm1-cmd.c | 5 --
> > > > drivers/char/tpm/tpm1_structs.h | 97 ++++++++++++++++++++++++++++++++
> > >
> > > I think you are overcomplicating the patch set and doing more
> > > than you really need to do.
> > >
> > > I.e. structs could go also just as well to tpm_command.h. We
> > > will deal with if/when that file ever grows too large. It's
> > > absolutely not a priority for this patch set.
> >
> > Ack.
> >
> > > > include/linux/tpm1.h | 34 +++++++++++-
> > > > 4 files changed, 132 insertions(+), 102 deletions(-)
> > > > create mode 100644 drivers/char/tpm/tpm1_structs.h
> > > >
> > > > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> > > > index ca391b2a211c..1f9f8540eede 100644
> > > > --- a/drivers/char/tpm/tpm.h
> > > > +++ b/drivers/char/tpm/tpm.h
> > > > @@ -50,105 +50,9 @@ enum tpm_addr {
> > > > TPM_ADDR = 0x4E,
> > > > };
> > > > -#define TPM_WARN_RETRY 0x800
> > > > -#define TPM_WARN_DOING_SELFTEST 0x802
> > > > -#define TPM_ERR_DEACTIVATED 0x6
> > > > -#define TPM_ERR_DISABLED 0x7
> > > > -#define TPM_ERR_FAILEDSELFTEST 0x1C
> > > > -#define TPM_ERR_INVALID_POSTINIT 38
> > > > -
> > > > -#define TPM_TAG_RQU_COMMAND 193
> > > > -
> > > > /* TPM2 specific constants. */
> > > > #define TPM2_SPACE_BUFFER_SIZE 16384 /* 16 kB */
> > > > -struct stclear_flags_t {
> > > > - __be16 tag;
> > > > - u8 deactivated;
> > > > - u8 disableForceClear;
> > > > - u8 physicalPresence;
> > > > - u8 physicalPresenceLock;
> > > > - u8 bGlobalLock;
> > > > -} __packed;
> > > > -
> > > > -struct tpm1_version {
> > > > - u8 major;
> > > > - u8 minor;
> > > > - u8 rev_major;
> > > > - u8 rev_minor;
> > > > -} __packed;
> > > > -
> > > > -struct tpm1_version2 {
> > > > - __be16 tag;
> > > > - struct tpm1_version version;
> > > > -} __packed;
> > > > -
> > > > -struct timeout_t {
> > > > - __be32 a;
> > > > - __be32 b;
> > > > - __be32 c;
> > > > - __be32 d;
> > > > -} __packed;
> > > > -
> > > > -struct duration_t {
> > > > - __be32 tpm_short;
> > > > - __be32 tpm_medium;
> > > > - __be32 tpm_long;
> > > > -} __packed;
> > > > -
> > > > -struct permanent_flags_t {
> > > > - __be16 tag;
> > > > - u8 disable;
> > > > - u8 ownership;
> > > > - u8 deactivated;
> > > > - u8 readPubek;
> > > > - u8 disableOwnerClear;
> > > > - u8 allowMaintenance;
> > > > - u8 physicalPresenceLifetimeLock;
> > > > - u8 physicalPresenceHWEnable;
> > > > - u8 physicalPresenceCMDEnable;
> > > > - u8 CEKPUsed;
> > > > - u8 TPMpost;
> > > > - u8 TPMpostLock;
> > > > - u8 FIPS;
> > > > - u8 operator;
> > > > - u8 enableRevokeEK;
> > > > - u8 nvLocked;
> > > > - u8 readSRKPub;
> > > > - u8 tpmEstablished;
> > > > - u8 maintenanceDone;
> > > > - u8 disableFullDALogicInfo;
> > > > -} __packed;
> > > > -
> > > > -typedef union {
> > > > - struct permanent_flags_t perm_flags;
> > > > - struct stclear_flags_t stclear_flags;
> > > > - __u8 owned;
> > > > - __be32 num_pcrs;
> > > > - struct tpm1_version version1;
> > > > - struct tpm1_version2 version2;
> > > > - __be32 manufacturer_id;
> > > > - struct timeout_t timeout;
> > > > - struct duration_t duration;
> > > > -} cap_t;
> > > > -
> > > > -enum tpm_capabilities {
> > > > - TPM_CAP_FLAG = 4,
> > > > - TPM_CAP_PROP = 5,
> > > > - TPM_CAP_VERSION_1_1 = 0x06,
> > > > - TPM_CAP_VERSION_1_2 = 0x1A,
> > > > -};
> > > > -
> > > > -enum tpm_sub_capabilities {
> > > > - TPM_CAP_PROP_PCR = 0x101,
> > > > - TPM_CAP_PROP_MANUFACTURER = 0x103,
> > > > - TPM_CAP_FLAG_PERM = 0x108,
> > > > - TPM_CAP_FLAG_VOL = 0x109,
> > > > - TPM_CAP_PROP_OWNER = 0x111,
> > > > - TPM_CAP_PROP_TIS_TIMEOUT = 0x115,
> > > > - TPM_CAP_PROP_TIS_DURATION = 0x120,
> > > > -};
> > > > -
> > > > enum tpm2_pt_props {
> > > > TPM2_PT_NONE = 0x00000000,
> > > > TPM2_PT_GROUP = 0x00000100,
> > > > @@ -229,6 +133,8 @@ enum tpm2_pt_props {
> > > > * compiler warnings about stack frame size. */
> > > > #define TPM_MAX_RNG_DATA 128
> > > > +#include "tpm1_structs.h"
> > > > +
> > > > extern const struct class tpm_class;
> > > > extern const struct class tpmrm_class;
> > > > extern dev_t tpm_devt;
> > > > diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c
> > > > index f29827b454d2..02f20a0aa37d 100644
> > > > --- a/drivers/char/tpm/tpm1-cmd.c
> > > > +++ b/drivers/char/tpm/tpm1-cmd.c
> > > > @@ -505,11 +505,6 @@ ssize_t tpm1_getcap(struct tpm_chip *chip, u32
> > > > subcap_id, cap_t *cap,
> > > > }
> > > > EXPORT_SYMBOL_GPL(tpm1_getcap);
> > > > -struct tpm1_get_random_out {
> > > > - __be32 rng_data_len;
> > > > - u8 rng_data[TPM_MAX_RNG_DATA];
> > > > -} __packed;
> > > > -
> > > > /**
> > > > * tpm1_get_random() - get random bytes from the TPM's RNG
> > > > * @chip: a &struct tpm_chip instance
> > > > diff --git a/drivers/char/tpm/tpm1_structs.h
> > > > b/drivers/char/tpm/tpm1_structs.h
> > > > new file mode 100644
> > > > index 000000000000..ad21376af5ab
> > > > --- /dev/null
> > > > +++ b/drivers/char/tpm/tpm1_structs.h
> > > > @@ -0,0 +1,97 @@
> > > > +/* SPDX-License-Identifier: GPL-2.0-only */
> > > > +/*
> > > > + * Copyright (C) 2004 IBM Corporation
> > > > + * Copyright (C) 2015 Intel Corporation
> > > > + *
> > > > + * Authors:
> > > > + * Leendert van Doorn <[email protected]>
> > > > + * Dave Safford <[email protected]>
> > > > + * Reiner Sailer <[email protected]>
> > > > + * Kylene Hall <[email protected]>
> > > > + *
> > > > + * Maintained by: <[email protected]>
> > > > + *
> > > > + * Device driver for TCG/TCPA TPM (trusted platform module).
> > > > + * Specifications at
> > > > https://urldefense.com/v3/__http://www.trustedcomputinggroup.org__;!!ACWV5N9M2RV99hQ!OivTxO-R0nvBJYXIV-T0n0fq0wY64MTgdaecPzAauxrFkRxTcQ6CuBEcXmZZYY1KfZlqmfLTZBbPaEr7Y8k7HW5iz_w$
> > > > + */
> > > > +
> > > > +#ifndef __TPM1_STRUCTS_H__
> > > > +#define __TPM1_STRUCTS_H__
> > > > +
> > > > +struct stclear_flags_t {
> > > > + __be16 tag;
> > > > + u8 deactivated;
> > > > + u8 disableForceClear;
> > > > + u8 physicalPresence;
> > > > + u8 physicalPresenceLock;
> > > > + u8 bGlobalLock;
> > > > +} __packed;
> > >
> > >
> > > Don't retain alignment.
> > >
> >
> > Okay.
>
> I wanted to clarify what you are asking here. Do you mean to just not pack
> the structs? It seems some of these structs define the responses from the TPM
> chip and I would assume they are packed this way.
>
> >
> > > > +
> > > > +struct tpm1_version {
> > > > + u8 major;
> > > > + u8 minor;
> > > > + u8 rev_major;
> > > > + u8 rev_minor;
> > > > +} __packed;
> > > > +
> > > > +struct tpm1_version2 {
> > > > + __be16 tag;
> > > > + struct tpm1_version version;
> > > > +} __packed;
> > > > +
> > > > +struct timeout_t {
> > > > + __be32 a;
> > > > + __be32 b;
> > > > + __be32 c;
> > > > + __be32 d;
> > > > +} __packed;
> > > > +
> > > > +struct duration_t {
> > > > + __be32 tpm_short;
> > > > + __be32 tpm_medium;
> > > > + __be32 tpm_long;
> > > > +} __packed;
> > > > +
> > > > +struct permanent_flags_t {
> > > > + __be16 tag;
> > > > + u8 disable;
> > > > + u8 ownership;
> > > > + u8 deactivated;
> > > > + u8 readPubek;
> > > > + u8 disableOwnerClear;
> > > > + u8 allowMaintenance;
> > > > + u8 physicalPresenceLifetimeLock;
> > > > + u8 physicalPresenceHWEnable;
> > > > + u8 physicalPresenceCMDEnable;
> > > > + u8 CEKPUsed;
> > > > + u8 TPMpost;
> > > > + u8 TPMpostLock;
> > > > + u8 FIPS;
> > > > + u8 operator;
> > > > + u8 enableRevokeEK;
> > > > + u8 nvLocked;
> > > > + u8 readSRKPub;
> > > > + u8 tpmEstablished;
> > > > + u8 maintenanceDone;
> > > > + u8 disableFullDALogicInfo;
> > > > +} __packed;
> > > > +
> > > > +/* Gather all capabilities related information info one type */
> > > > +typedef union {
> > > > + struct permanent_flags_t perm_flags;
> > > > + struct stclear_flags_t stclear_flags;
> > > > + __u8 owned;
> > > > + __be32 num_pcrs;
> > > > + struct tpm1_version version1;
> > > > + struct tpm1_version2 version2;
> > > > + __be32 manufacturer_id;
> > > > + struct timeout_t timeout;
> > > > + struct duration_t duration;
> > > > +} cap_t;
> > >
> > > Don't retain alignment here.
>
> Again to clarify, do you mean none of these structs should be packed (and
> thus the union won't be either)?
I only meant the syntactical (is that even a word) alignment :-)
I don't know what purpose does it serve here, as struct members already
convolute, and alignment then sort of add up to the convolution.
E.g., "__be32 manufacturer_id" with just one spacebar is what I mean.
>
> Thanks
> Ross
>
> > >
> >
> > Okay.
> >
> > > > +
> > > > +struct tpm1_get_random_out {
> > > > + __be32 rng_data_len;
> > > > + u8 rng_data[TPM_MAX_RNG_DATA];
> > > > +} __packed;
> > > > +
> > > > +#endif
> > > > diff --git a/include/linux/tpm1.h b/include/linux/tpm1.h
> > > > index 54c6c211eb9e..5fad94ac8d15 100644
> > > > --- a/include/linux/tpm1.h
> > > > +++ b/include/linux/tpm1.h
> > > > @@ -47,7 +47,39 @@ enum tpm_command_ordinals {
> > > > TPM_ORD_UNSEAL = 24,
> > > > };
> > > > -/* Other constants */
> > > > +enum tpm_capabilities {
> > > > + TPM_CAP_FLAG = 4,
> > > > + TPM_CAP_PROP = 5,
> > > > + TPM_CAP_VERSION_1_1 = 0x06,
> > > > + TPM_CAP_VERSION_1_2 = 0x1A,
> > > > +};
> > > > +
> > > > +enum tpm_sub_capabilities {
> > > > + TPM_CAP_PROP_PCR = 0x101,
> > > > + TPM_CAP_PROP_MANUFACTURER = 0x103,
> > > > + TPM_CAP_FLAG_PERM = 0x108,
> > > > + TPM_CAP_FLAG_VOL = 0x109,
> > > > + TPM_CAP_PROP_OWNER = 0x111,
> > > > + TPM_CAP_PROP_TIS_TIMEOUT = 0x115,
> > > > + TPM_CAP_PROP_TIS_DURATION = 0x120,
> > > > +};
> > > > +
> > > > +/* Return Codes */
> > > > +enum tpm_return_codes {
> > > > + TPM_BASE_MASK = 0,
> > > > + TPM_NON_FATAL_MASK = 0x00000800,
> > > > + TPM_SUCCESS = TPM_BASE_MASK + 0,
> > > > + TPM_ERR_DEACTIVATED = TPM_BASE_MASK + 6,
> > > > + TPM_ERR_DISABLED = TPM_BASE_MASK + 7,
> > > > + TPM_ERR_FAIL = TPM_BASE_MASK + 9,
> > > > + TPM_ERR_FAILEDSELFTEST = TPM_BASE_MASK + 28,
> > > > + TPM_ERR_INVALID_POSTINIT = TPM_BASE_MASK + 38,
> > > > + TPM_ERR_INVALID_FAMILY = TPM_BASE_MASK + 55,
> > > > + TPM_WARN_RETRY = TPM_BASE_MASK + TPM_NON_FATAL_MASK + 0,
> > > > + TPM_WARN_DOING_SELFTEST = TPM_BASE_MASK +
> > > > TPM_NON_FATAL_MASK + 2,
> > > > +};
> > > > +
> > > > +/* Misc. constants */
> > >
> > > These constants should be relocated in a separate patch.
> > >
> >
> > Okay.
> >
> > > > #define SRKHANDLE 0x40000000
> > > > #define TPM_NONCE_SIZE 20
> > > > #define TPM_ST_CLEAR 1
> > > > --
> > > > 2.43.7
> > > >
> > >
> > > BR, Jarkko
> >
>
BR, Jarkko
