* [PATCH 2/7] tpm: reorganize sysfs files
[not found] <1143823488.2992.166.camel@localhost.localdomain>
@ 2006-04-03 16:42 ` Kylene Jo Hall
2006-04-03 16:54 ` Dave Hansen
0 siblings, 1 reply; 2+ messages in thread
From: Kylene Jo Hall @ 2006-04-03 16:42 UTC (permalink / raw)
To: linux-kernel; +Cc: akpm, TPM Device Driver List, Marcel Selhorst
Many of the sysfs files were calling the TPM_GetCapability command with
different options and each command layed out in its own static const
array. Since for 1.2 more sysfs files of this type are coming I am
generalizing the array so there can be one array and the unique parts
can be filled in just before the command is called.
Signed-off-by: Kylene Hall <kjhall@us.ibm.com>
---
drivers/char/tpm/tpm.c | 116 ++++++++++++++++++++++++---------------
1 files changed, 72 insertions(+), 44 deletions(-)
--- linux-2.6.16/drivers/char/tpm/tpm.c 2006-03-30 12:27:02.858478250 -0600
+++ linux-2.6.16-rc1-tpm/drivers/char/tpm/tpm.c 2006-03-29 14:17:14.421822250 -0600
@@ -119,17 +427,41 @@ out:
}
#define TPM_DIGEST_SIZE 20
-#define CAP_PCR_RESULT_SIZE 18
-static const u8 cap_pcr[] = {
+#define TPM_ERROR_SIZE 10
+#define TPM_RET_CODE_IDX 6
+#define TPM_GET_CAP_RET_SIZE_IDX 10
+#define TPM_GET_CAP_RET_UINT32_1_IDX 14
+#define TPM_GET_CAP_RET_UINT32_2_IDX 18
+#define TPM_GET_CAP_RET_UINT32_3_IDX 22
+#define TPM_GET_CAP_RET_UINT32_4_IDX 26
+
+#define TPM_CAP_IDX 13
+#define TPM_CAP_SUBCAP_IDX 21
+
+enum tpm_capabilities {
+ TPM_CAP_PROP = 5,
+};
+
+enum tpm_sub_capabilities {
+ TPM_CAP_PROP_PCR = 0x1,
+ TPM_CAP_PROP_MANUFACTURER = 0x3,
+};
+
+/*
+ * This is a semi generic GetCapability command for use
+ * with the capability type TPM_CAP_PROP or TPM_CAP_FLAG
+ * and their associated sub_capabilities.
+ */
+
+static const u8 tpm_cap[] = {
0, 193, /* TPM_TAG_RQU_COMMAND */
0, 0, 0, 22, /* length */
0, 0, 0, 101, /* TPM_ORD_GetCapability */
- 0, 0, 0, 5,
- 0, 0, 0, 4,
- 0, 0, 1, 1
+ 0, 0, 0, 0, /* TPM_CAP_<TYPE> */
+ 0, 0, 0, 4, /* TPM_CAP_SUB_<TYPE> size */
+ 0, 0, 1, 0 /* TPM_CAP_SUB_<TYPE> */
};
-#define READ_PCR_RESULT_SIZE 30
static const u8 pcrread[] = {
0, 193, /* TPM_TAG_RQU_COMMAND */
0, 0, 0, 14, /* length */
@@ -140,7 +649,7 @@ static const u8 pcrread[] = {
ssize_t tpm_show_pcrs(struct device *dev, struct device_attribute *attr,
char *buf)
{
- u8 data[READ_PCR_RESULT_SIZE];
+ u8 data[30];
ssize_t len;
int i, j, num_pcrs;
__be32 index;
@@ -150,26 +659,28 @@ ssize_t tpm_show_pcrs(struct device *dev
if (chip == NULL)
return -ENODEV;
- memcpy(data, cap_pcr, sizeof(cap_pcr));
+ memcpy(data, tpm_cap, sizeof(tpm_cap));
+ data[TPM_CAP_IDX] = TPM_CAP_PROP;
+ data[TPM_CAP_SUBCAP_IDX] = TPM_CAP_PROP_PCR;
+
if ((len = tpm_transmit(chip, data, sizeof(data)))
- < CAP_PCR_RESULT_SIZE) {
+ <= TPM_ERROR_SIZE) {
dev_dbg(chip->dev, "A TPM error (%d) occurred "
- "attempting to determine the number of PCRS\n",
- be32_to_cpu(*((__be32 *) (data + 6))));
+ "attempting to determine the number of PCRS\n",
+ be32_to_cpu(*((__be32 *) (data + TPM_RET_CODE_IDX))));
return 0;
}
num_pcrs = be32_to_cpu(*((__be32 *) (data + 14)));
-
for (i = 0; i < num_pcrs; i++) {
memcpy(data, pcrread, sizeof(pcrread));
index = cpu_to_be32(i);
memcpy(data + 10, &index, 4);
if ((len = tpm_transmit(chip, data, sizeof(data)))
- < READ_PCR_RESULT_SIZE){
+ <= TPM_ERROR_SIZE) {
dev_dbg(chip->dev, "A TPM error (%d) occurred"
" attempting to read PCR %d of %d\n",
- be32_to_cpu(*((__be32 *) (data + 6))),
+ be32_to_cpu(*((__be32 *) (data + TPM_RET_CODE_IDX))),
i, num_pcrs);
goto out;
}
@@ -208,11 +724,11 @@ ssize_t tpm_show_pubek(struct device *de
memcpy(data, readpubek, sizeof(readpubek));
- if ((len = tpm_transmit(chip, data, READ_PUBEK_RESULT_SIZE)) <
- READ_PUBEK_RESULT_SIZE) {
+ if ((len = tpm_transmit(chip, data, READ_PUBEK_RESULT_SIZE)) <=
+ TPM_ERROR_SIZE) {
dev_dbg(chip->dev, "A TPM error (%d) occurred "
- "attempting to read the PUBEK\n",
- be32_to_cpu(*((__be32 *) (data + 6))));
+ "attempting to read the PUBEK\n",
+ be32_to_cpu(*((__be32 *) (data + TPM_RET_CODE_IDX))));
rc = 0;
goto out;
}
@@ -250,29 +284,20 @@ out:
}
EXPORT_SYMBOL_GPL(tpm_show_pubek);
-#define CAP_VER_RESULT_SIZE 18
+#define CAP_VERSION_1_1 6
+#define CAP_VERSION_IDX 13
static const u8 cap_version[] = {
0, 193, /* TPM_TAG_RQU_COMMAND */
0, 0, 0, 18, /* length */
0, 0, 0, 101, /* TPM_ORD_GetCapability */
- 0, 0, 0, 6,
+ 0, 0, 0, 0,
0, 0, 0, 0
};
-#define CAP_MANUFACTURER_RESULT_SIZE 18
-static const u8 cap_manufacturer[] = {
- 0, 193, /* TPM_TAG_RQU_COMMAND */
- 0, 0, 0, 22, /* length */
- 0, 0, 0, 101, /* TPM_ORD_GetCapability */
- 0, 0, 0, 5,
- 0, 0, 0, 4,
- 0, 0, 1, 3
-};
-
ssize_t tpm_show_caps(struct device *dev, struct device_attribute *attr,
char *buf)
{
- u8 data[sizeof(cap_manufacturer)];
+ u8 data[30];
ssize_t len;
char *str = buf;
@@ -282,26 +793,37 @@ ssize_t tpm_show_caps(struct device *dev
if (chip == NULL)
return -ENODEV;
- memcpy(data, cap_manufacturer, sizeof(cap_manufacturer));
+ memcpy(data, tpm_cap, sizeof(tpm_cap));
+ data[TPM_CAP_IDX] = TPM_CAP_PROP;
+ data[TPM_CAP_SUBCAP_IDX] = TPM_CAP_PROP_MANUFACTURER;
- if ((len = tpm_transmit(chip, data, sizeof(data))) <
- CAP_MANUFACTURER_RESULT_SIZE)
- return len;
+ if ((len = tpm_transmit(chip, data, sizeof(data))) <=
+ TPM_ERROR_SIZE) {
+ dev_dbg(chip->dev, "A TPM error (%d) occurred "
+ "attempting to determine the manufacturer\n",
+ be32_to_cpu(*((__be32 *) (data + TPM_RET_CODE_IDX))));
+ return 0;
+ }
str += sprintf(str, "Manufacturer: 0x%x\n",
- be32_to_cpu(*((__be32 *) (data + 14))));
+ be32_to_cpu(*((__be32 *) (data + TPM_GET_CAP_RET_UINT32_1_IDX))));
memcpy(data, cap_version, sizeof(cap_version));
+ data[CAP_VERSION_IDX] = CAP_VERSION_1_1;
+ if ((len = tpm_transmit(chip, data, sizeof(data))) <=
+ TPM_ERROR_SIZE) {
+ dev_err(chip->dev, "A TPM error (%d) occurred "
+ "attempting to determine the 1.1 version\n",
+ be32_to_cpu(*((__be32 *) (data + TPM_RET_CODE_IDX))));
+ goto out;
+ }
- if ((len = tpm_transmit(chip, data, sizeof(data))) <
- CAP_VER_RESULT_SIZE)
- return len;
-
- str +=
- sprintf(str, "TCG version: %d.%d\nFirmware version: %d.%d\n",
- (int) data[14], (int) data[15], (int) data[16],
- (int) data[17]);
+ str += sprintf(str,
+ "TCG version: %d.%d\nFirmware version: %d.%d\n",
+ (int) data[14], (int) data[15], (int) data[16],
+ (int) data[17]);
+out:
return str - buf;
}
EXPORT_SYMBOL_GPL(tpm_show_caps);
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH 2/7] tpm: reorganize sysfs files
2006-04-03 16:42 ` [PATCH 2/7] tpm: reorganize sysfs files Kylene Jo Hall
@ 2006-04-03 16:54 ` Dave Hansen
0 siblings, 0 replies; 2+ messages in thread
From: Dave Hansen @ 2006-04-03 16:54 UTC (permalink / raw)
To: Kylene Jo Hall
Cc: linux-kernel, akpm, TPM Device Driver List, Marcel Selhorst
On Mon, 2006-04-03 at 11:42 -0500, Kylene Jo Hall wrote:
> ssize_t tpm_show_pcrs(struct device *dev, struct device_attribute *attr,
> char *buf)
> {
> - u8 data[READ_PCR_RESULT_SIZE];
> + u8 data[30];
Is this correct? Are you guaranteed that this data read will never,
ever exceed 30 bytes? Any reason it shouldn't be a variable?
> ssize_t len;
> int i, j, num_pcrs;
> __be32 index;
> @@ -150,26 +659,28 @@ ssize_t tpm_show_pcrs(struct device *dev
> if (chip == NULL)
> return -ENODEV;
>
> - memcpy(data, cap_pcr, sizeof(cap_pcr));
> + memcpy(data, tpm_cap, sizeof(tpm_cap));
> + data[TPM_CAP_IDX] = TPM_CAP_PROP;
> + data[TPM_CAP_SUBCAP_IDX] = TPM_CAP_PROP_PCR;
> +
> if ((len = tpm_transmit(chip, data, sizeof(data)))
> - < CAP_PCR_RESULT_SIZE) {
> + <= TPM_ERROR_SIZE) {
> dev_dbg(chip->dev, "A TPM error (%d) occurred "
> - "attempting to determine the number of PCRS\n",
> - be32_to_cpu(*((__be32 *) (data + 6))));
> + "attempting to determine the number of PCRS\n",
> + be32_to_cpu(*((__be32 *) (data + TPM_RET_CODE_IDX))));
> return 0;
> }
I know this is old code, but I see this little
be32_to_cpu(*((__be32 *) (data + TPM_RET_CODE_IDX))));
snippet at least twice. It is also a bit hard to read. Seems likt it
would be a great candidate for a little helper function.
Come to think of it, that entire if() sequence appears to be repeated
quite a few times.
> num_pcrs = be32_to_cpu(*((__be32 *) (data + 14)));
> -
> for (i = 0; i < num_pcrs; i++) {
> memcpy(data, pcrread, sizeof(pcrread));
> index = cpu_to_be32(i);
> memcpy(data + 10, &index, 4);
> if ((len = tpm_transmit(chip, data, sizeof(data)))
> - < READ_PCR_RESULT_SIZE){
> + <= TPM_ERROR_SIZE) {
> dev_dbg(chip->dev, "A TPM error (%d) occurred"
> " attempting to read PCR %d of %d\n",
> - be32_to_cpu(*((__be32 *) (data + 6))),
> + be32_to_cpu(*((__be32 *) (data + TPM_RET_CODE_IDX))),
> i, num_pcrs);
> goto out;
> }
> @@ -208,11 +724,11 @@ ssize_t tpm_show_pubek(struct device *de
>
> memcpy(data, readpubek, sizeof(readpubek));
>
> - if ((len = tpm_transmit(chip, data, READ_PUBEK_RESULT_SIZE)) <
> - READ_PUBEK_RESULT_SIZE) {
> + if ((len = tpm_transmit(chip, data, READ_PUBEK_RESULT_SIZE)) <=
> + TPM_ERROR_SIZE) {
> dev_dbg(chip->dev, "A TPM error (%d) occurred "
> - "attempting to read the PUBEK\n",
> - be32_to_cpu(*((__be32 *) (data + 6))));
> + "attempting to read the PUBEK\n",
> + be32_to_cpu(*((__be32 *) (data + TPM_RET_CODE_IDX))));
> rc = 0;
> goto out;
> }
> @@ -250,29 +284,20 @@ out:
> }
> EXPORT_SYMBOL_GPL(tpm_show_pubek);
>
> -#define CAP_VER_RESULT_SIZE 18
> +#define CAP_VERSION_1_1 6
> +#define CAP_VERSION_IDX 13
> static const u8 cap_version[] = {
> 0, 193, /* TPM_TAG_RQU_COMMAND */
> 0, 0, 0, 18, /* length */
> 0, 0, 0, 101, /* TPM_ORD_GetCapability */
> - 0, 0, 0, 6,
> + 0, 0, 0, 0,
> 0, 0, 0, 0
> };
>
> -#define CAP_MANUFACTURER_RESULT_SIZE 18
> -static const u8 cap_manufacturer[] = {
> - 0, 193, /* TPM_TAG_RQU_COMMAND */
> - 0, 0, 0, 22, /* length */
> - 0, 0, 0, 101, /* TPM_ORD_GetCapability */
> - 0, 0, 0, 5,
> - 0, 0, 0, 4,
> - 0, 0, 1, 3
> -};
> -
> ssize_t tpm_show_caps(struct device *dev, struct device_attribute *attr,
> char *buf)
> {
> - u8 data[sizeof(cap_manufacturer)];
> + u8 data[30];
> ssize_t len;
> char *str = buf;
>
> @@ -282,26 +793,37 @@ ssize_t tpm_show_caps(struct device *dev
> if (chip == NULL)
> return -ENODEV;
>
> - memcpy(data, cap_manufacturer, sizeof(cap_manufacturer));
> + memcpy(data, tpm_cap, sizeof(tpm_cap));
> + data[TPM_CAP_IDX] = TPM_CAP_PROP;
> + data[TPM_CAP_SUBCAP_IDX] = TPM_CAP_PROP_MANUFACTURER;
>
> - if ((len = tpm_transmit(chip, data, sizeof(data))) <
> - CAP_MANUFACTURER_RESULT_SIZE)
> - return len;
> + if ((len = tpm_transmit(chip, data, sizeof(data))) <=
> + TPM_ERROR_SIZE) {
> + dev_dbg(chip->dev, "A TPM error (%d) occurred "
> + "attempting to determine the manufacturer\n",
> + be32_to_cpu(*((__be32 *) (data + TPM_RET_CODE_IDX))));
> + return 0;
> + }
Since you're going through and modifying these, it might be nice to
change them to the more normal (and readable) style of
len = tpm_transmit(chip, data, sizeof(data));
if (len < CAP_MANUFACTURER_RESULT_SIZE)
return len;
Note that that doesn't even increase the number of lines of code.
-- Dave
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2006-04-03 16:54 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1143823488.2992.166.camel@localhost.localdomain>
2006-04-03 16:42 ` [PATCH 2/7] tpm: reorganize sysfs files Kylene Jo Hall
2006-04-03 16:54 ` Dave Hansen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox