This patch makes the code endianness independent. We defined a
macro do_endian_conversion to apply endianness to raw integers
in the event entries so that they will be displayed properly.
tpm_binary_bios_measurements_show() is modified for the display.
Signed-off-by: Hon Ching(Vicky) Lo <[email protected]>
Signed-off-by: Joy Latten <[email protected]>
---
drivers/char/tpm/tpm_eventlog.c | 78 ++++++++++++++++++++++++++++-----------
drivers/char/tpm/tpm_eventlog.h | 6 +++
2 files changed, 62 insertions(+), 22 deletions(-)
diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c
index 3a56a13..bd72fb0 100644
--- a/drivers/char/tpm/tpm_eventlog.c
+++ b/drivers/char/tpm/tpm_eventlog.c
@@ -76,15 +76,25 @@ static void *tpm_bios_measurements_start(struct seq_file *m, loff_t *pos)
void *addr = log->bios_event_log;
void *limit = log->bios_event_log_end;
struct tcpa_event *event;
+ u32 converted_event_size;
+ u32 converted_event_type;
+
/* read over *pos measurements */
for (i = 0; i < *pos; i++) {
event = addr;
+ converted_event_size =
+ do_endian_conversion(event->event_size);
+ converted_event_type =
+ do_endian_conversion(event->event_type);
+
if ((addr + sizeof(struct tcpa_event)) < limit) {
- if (event->event_type == 0 && event->event_size == 0)
+ if ((converted_event_type == 0) &&
+ (converted_event_size == 0))
return NULL;
- addr += sizeof(struct tcpa_event) + event->event_size;
+ addr += (sizeof(struct tcpa_event) +
+ converted_event_size);
}
}
@@ -94,8 +104,12 @@ static void *tpm_bios_measurements_start(struct seq_file *m, loff_t *pos)
event = addr;
- if ((event->event_type == 0 && event->event_size == 0) ||
- ((addr + sizeof(struct tcpa_event) + event->event_size) >= limit))
+ converted_event_size = do_endian_conversion(event->event_size);
+ converted_event_type = do_endian_conversion(event->event_type);
+
+ if (((converted_event_type == 0) && (converted_event_size == 0))
+ || ((addr + sizeof(struct tcpa_event) + converted_event_size)
+ >= limit))
return NULL;
return addr;
@@ -107,8 +121,12 @@ static void *tpm_bios_measurements_next(struct seq_file *m, void *v,
struct tcpa_event *event = v;
struct tpm_bios_log *log = m->private;
void *limit = log->bios_event_log_end;
+ u32 converted_event_size;
+ u32 converted_event_type;
- v += sizeof(struct tcpa_event) + event->event_size;
+ converted_event_size = do_endian_conversion(event->event_size);
+
+ v += sizeof(struct tcpa_event) + converted_event_size;
/* now check if current entry is valid */
if ((v + sizeof(struct tcpa_event)) >= limit)
@@ -116,11 +134,11 @@ static void *tpm_bios_measurements_next(struct seq_file *m, void *v,
event = v;
- if (event->event_type == 0 && event->event_size == 0)
- return NULL;
+ converted_event_size = do_endian_conversion(event->event_size);
+ converted_event_type = do_endian_conversion(event->event_type);
- if ((event->event_type == 0 && event->event_size == 0) ||
- ((v + sizeof(struct tcpa_event) + event->event_size) >= limit))
+ if (((converted_event_type == 0) && (converted_event_size == 0)) ||
+ ((v + sizeof(struct tcpa_event) + converted_event_size) >= limit))
return NULL;
(*pos)++;
@@ -140,7 +158,7 @@ static int get_event_name(char *dest, struct tcpa_event *event,
int i, n_len = 0, d_len = 0;
struct tcpa_pc_event *pc_event;
- switch(event->event_type) {
+ switch (do_endian_conversion(event->event_type)) {
case PREBOOT:
case POST_CODE:
case UNUSED:
@@ -156,14 +174,16 @@ static int get_event_name(char *dest, struct tcpa_event *event,
case NONHOST_CODE:
case NONHOST_CONFIG:
case NONHOST_INFO:
- name = tcpa_event_type_strings[event->event_type];
+ name = tcpa_event_type_strings[do_endian_conversion
+ (event->event_type)];
n_len = strlen(name);
break;
case SEPARATOR:
case ACTION:
- if (MAX_TEXT_EVENT > event->event_size) {
+ if (MAX_TEXT_EVENT >
+ do_endian_conversion(event->event_size)) {
name = event_entry;
- n_len = event->event_size;
+ n_len = do_endian_conversion(event->event_size);
}
break;
case EVENT_TAG:
@@ -171,7 +191,7 @@ static int get_event_name(char *dest, struct tcpa_event *event,
/* ToDo Row data -> Base64 */
- switch (pc_event->event_id) {
+ switch (do_endian_conversion(pc_event->event_id)) {
case SMBIOS:
case BIS_CERT:
case CMOS:
@@ -179,7 +199,8 @@ static int get_event_name(char *dest, struct tcpa_event *event,
case OPTION_ROM_EXEC:
case OPTION_ROM_CONFIG:
case S_CRTM_VERSION:
- name = tcpa_pc_event_id_strings[pc_event->event_id];
+ name = tcpa_pc_event_id_strings[do_endian_conversion
+ (pc_event->event_id)];
n_len = strlen(name);
break;
/* hash data */
@@ -188,7 +209,8 @@ static int get_event_name(char *dest, struct tcpa_event *event,
case OPTION_ROM_MICROCODE:
case S_CRTM_CONTENTS:
case POST_CONTENTS:
- name = tcpa_pc_event_id_strings[pc_event->event_id];
+ name = tcpa_pc_event_id_strings[do_endian_conversion
+ (pc_event->event_id)];
n_len = strlen(name);
for (i = 0; i < 20; i++)
d_len += sprintf(&data[2*i], "%02x",
@@ -209,13 +231,24 @@ static int get_event_name(char *dest, struct tcpa_event *event,
static int tpm_binary_bios_measurements_show(struct seq_file *m, void *v)
{
struct tcpa_event *event = v;
- char *data = v;
+ struct tcpa_event temp_event;
+ char *tempPtr;
int i;
- for (i = 0; i < sizeof(struct tcpa_event) + event->event_size; i++)
- seq_putc(m, data[i]);
+ memcpy(&temp_event, event, sizeof(struct tcpa_event));
+
+ /* convert raw integers for endianness */
+ temp_event.pcr_index = do_endian_conversion(event->pcr_index);
+ temp_event.event_type = do_endian_conversion(event->event_type);
+ temp_event.event_size = do_endian_conversion(event->event_size);
+
+ tempPtr = (char *)&temp_event;
+
+ for (i = 0; i < sizeof(struct tcpa_event) + temp_event.event_size; i++)
+ seq_putc(m, tempPtr[i]);
return 0;
+
}
static int tpm_bios_measurements_release(struct inode *inode,
@@ -238,7 +271,7 @@ static int tpm_ascii_bios_measurements_show(struct seq_file *m, void *v)
char *eventname;
struct tcpa_event *event = v;
unsigned char *event_entry =
- (unsigned char *) (v + sizeof(struct tcpa_event));
+ (unsigned char *)(v + sizeof(struct tcpa_event));
eventname = kmalloc(MAX_TEXT_EVENT, GFP_KERNEL);
if (!eventname) {
@@ -247,13 +280,14 @@ static int tpm_ascii_bios_measurements_show(struct seq_file *m, void *v)
return -EFAULT;
}
- seq_printf(m, "%2d ", event->pcr_index);
+ /* 1st: PCR */
+ seq_printf(m, "%2d ", do_endian_conversion(event->pcr_index));
/* 2nd: SHA1 */
seq_printf(m, "%20phN", event->pcr_value);
/* 3rd: event type identifier */
- seq_printf(m, " %02x", event->event_type);
+ seq_printf(m, " %02x", do_endian_conversion(event->event_type));
len += get_event_name(eventname, event, event_entry);
diff --git a/drivers/char/tpm/tpm_eventlog.h b/drivers/char/tpm/tpm_eventlog.h
index e7da086..267bfbd 100644
--- a/drivers/char/tpm/tpm_eventlog.h
+++ b/drivers/char/tpm/tpm_eventlog.h
@@ -6,6 +6,12 @@
#define MAX_TEXT_EVENT 1000 /* Max event string length */
#define ACPI_TCPA_SIG "TCPA" /* 0x41504354 /'TCPA' */
+#ifdef CONFIG_PPC64
+#define do_endian_conversion(x) be32_to_cpu(x)
+#else
+#define do_endian_conversion(x) x
+#endif
+
enum bios_platform_class {
BIOS_CLIENT = 0x00,
BIOS_SERVER = 0x01,
--
1.7.1
The base pointer for the event log is allocated in the local
kernel (in prom_instantiate_sml()), therefore it is already in
the host's endian byte order and requires no conversion.
The content of the 'basep' pointer in read_log() stores the
base address of the log. This patch ensures that it is correctly
implemented.
Signed-off-by: Hon Ching(Vicky) Lo <[email protected]>
Signed-off-by: Joy Latten <[email protected]>
---
drivers/char/tpm/tpm_of.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/char/tpm/tpm_of.c b/drivers/char/tpm/tpm_of.c
index c002d1b..62a22ce 100644
--- a/drivers/char/tpm/tpm_of.c
+++ b/drivers/char/tpm/tpm_of.c
@@ -24,7 +24,7 @@ int read_log(struct tpm_bios_log *log)
{
struct device_node *np;
const u32 *sizep;
- const __be64 *basep;
+ const u64 *basep;
if (log->bios_event_log != NULL) {
pr_err("%s: ERROR - Eventlog already initialized\n", __func__);
@@ -63,7 +63,7 @@ int read_log(struct tpm_bios_log *log)
log->bios_event_log_end = log->bios_event_log + *sizep;
- memcpy(log->bios_event_log, __va(be64_to_cpup(basep)), *sizep);
+ memcpy(log->bios_event_log, __va(*basep), *sizep);
return 0;
--
1.7.1
Looks better. Thanks.
Reviewed-by: Ashley Lai <[email protected]>
Cheers,
Ashley Lai
Looks good.
Reviewed-by: Ashley Lai <[email protected]>
Ashley Lai
Hi Peter,
Can you please commit the patch in the next open window, if you
accept it as well?
Thanks,
Vicky
On Thu, 2015-06-18 at 08:23 -0500, Ashley Lai wrote:
Looks better. Thanks.
>
> Reviewed-by: Ashley Lai <[email protected]>
>
> Cheers,
> Ashley Lai
>
>
On Wed, 2015-06-17 at 18:17 -0400, Hon Ching(Vicky) Lo wrote:
> This patch makes the code endianness independent. We defined a
> macro do_endian_conversion to apply endianness to raw integers
> in the event entries so that they will be displayed properly.
> tpm_binary_bios_measurements_show() is modified for the display.
>
> Signed-off-by: Hon Ching(Vicky) Lo <[email protected]>
> Signed-off-by: Joy Latten <[email protected]>
> ---
> drivers/char/tpm/tpm_eventlog.c | 78 ++++++++++++++++++++++++++++-----------
> drivers/char/tpm/tpm_eventlog.h | 6 +++
> 2 files changed, 62 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c
> index 3a56a13..bd72fb0 100644
> --- a/drivers/char/tpm/tpm_eventlog.c
> +++ b/drivers/char/tpm/tpm_eventlog.c
> @@ -76,15 +76,25 @@ static void *tpm_bios_measurements_start(struct seq_file *m, loff_t *pos)
> void *addr = log->bios_event_log;
> void *limit = log->bios_event_log_end;
> struct tcpa_event *event;
> + u32 converted_event_size;
> + u32 converted_event_type;
> +
>
> /* read over *pos measurements */
> for (i = 0; i < *pos; i++) {
> event = addr;
>
> + converted_event_size =
> + do_endian_conversion(event->event_size);
> + converted_event_type =
> + do_endian_conversion(event->event_type);
> +
> if ((addr + sizeof(struct tcpa_event)) < limit) {
> - if (event->event_type == 0 && event->event_size == 0)
> + if ((converted_event_type == 0) &&
> + (converted_event_size == 0))
> return NULL;
> - addr += sizeof(struct tcpa_event) + event->event_size;
> + addr += (sizeof(struct tcpa_event) +
> + converted_event_size);
> }
> }
>
> @@ -94,8 +104,12 @@ static void *tpm_bios_measurements_start(struct seq_file *m, loff_t *pos)
>
> event = addr;
>
> - if ((event->event_type == 0 && event->event_size == 0) ||
> - ((addr + sizeof(struct tcpa_event) + event->event_size) >= limit))
> + converted_event_size = do_endian_conversion(event->event_size);
> + converted_event_type = do_endian_conversion(event->event_type);
> +
> + if (((converted_event_type == 0) && (converted_event_size == 0))
> + || ((addr + sizeof(struct tcpa_event) + converted_event_size)
> + >= limit))
> return NULL;
>
> return addr;
> @@ -107,8 +121,12 @@ static void *tpm_bios_measurements_next(struct seq_file *m, void *v,
> struct tcpa_event *event = v;
> struct tpm_bios_log *log = m->private;
> void *limit = log->bios_event_log_end;
> + u32 converted_event_size;
> + u32 converted_event_type;
>
> - v += sizeof(struct tcpa_event) + event->event_size;
> + converted_event_size = do_endian_conversion(event->event_size);
> +
> + v += sizeof(struct tcpa_event) + converted_event_size;
>
> /* now check if current entry is valid */
> if ((v + sizeof(struct tcpa_event)) >= limit)
> @@ -116,11 +134,11 @@ static void *tpm_bios_measurements_next(struct seq_file *m, void *v,
>
> event = v;
>
> - if (event->event_type == 0 && event->event_size == 0)
> - return NULL;
> + converted_event_size = do_endian_conversion(event->event_size);
> + converted_event_type = do_endian_conversion(event->event_type);
>
> - if ((event->event_type == 0 && event->event_size == 0) ||
> - ((v + sizeof(struct tcpa_event) + event->event_size) >= limit))
> + if (((converted_event_type == 0) && (converted_event_size == 0)) ||
> + ((v + sizeof(struct tcpa_event) + converted_event_size) >= limit))
> return NULL;
>
> (*pos)++;
> @@ -140,7 +158,7 @@ static int get_event_name(char *dest, struct tcpa_event *event,
> int i, n_len = 0, d_len = 0;
> struct tcpa_pc_event *pc_event;
>
> - switch(event->event_type) {
> + switch (do_endian_conversion(event->event_type)) {
> case PREBOOT:
> case POST_CODE:
> case UNUSED:
> @@ -156,14 +174,16 @@ static int get_event_name(char *dest, struct tcpa_event *event,
> case NONHOST_CODE:
> case NONHOST_CONFIG:
> case NONHOST_INFO:
> - name = tcpa_event_type_strings[event->event_type];
> + name = tcpa_event_type_strings[do_endian_conversion
> + (event->event_type)];
> n_len = strlen(name);
> break;
> case SEPARATOR:
> case ACTION:
> - if (MAX_TEXT_EVENT > event->event_size) {
> + if (MAX_TEXT_EVENT >
> + do_endian_conversion(event->event_size)) {
> name = event_entry;
> - n_len = event->event_size;
> + n_len = do_endian_conversion(event->event_size);
> }
> break;
> case EVENT_TAG:
> @@ -171,7 +191,7 @@ static int get_event_name(char *dest, struct tcpa_event *event,
>
> /* ToDo Row data -> Base64 */
>
> - switch (pc_event->event_id) {
> + switch (do_endian_conversion(pc_event->event_id)) {
> case SMBIOS:
> case BIS_CERT:
> case CMOS:
> @@ -179,7 +199,8 @@ static int get_event_name(char *dest, struct tcpa_event *event,
> case OPTION_ROM_EXEC:
> case OPTION_ROM_CONFIG:
> case S_CRTM_VERSION:
> - name = tcpa_pc_event_id_strings[pc_event->event_id];
> + name = tcpa_pc_event_id_strings[do_endian_conversion
> + (pc_event->event_id)];
> n_len = strlen(name);
> break;
> /* hash data */
> @@ -188,7 +209,8 @@ static int get_event_name(char *dest, struct tcpa_event *event,
> case OPTION_ROM_MICROCODE:
> case S_CRTM_CONTENTS:
> case POST_CONTENTS:
> - name = tcpa_pc_event_id_strings[pc_event->event_id];
> + name = tcpa_pc_event_id_strings[do_endian_conversion
> + (pc_event->event_id)];
> n_len = strlen(name);
> for (i = 0; i < 20; i++)
> d_len += sprintf(&data[2*i], "%02x",
> @@ -209,13 +231,24 @@ static int get_event_name(char *dest, struct tcpa_event *event,
> static int tpm_binary_bios_measurements_show(struct seq_file *m, void *v)
> {
> struct tcpa_event *event = v;
> - char *data = v;
> + struct tcpa_event temp_event;
> + char *tempPtr;
> int i;
>
> - for (i = 0; i < sizeof(struct tcpa_event) + event->event_size; i++)
> - seq_putc(m, data[i]);
> + memcpy(&temp_event, event, sizeof(struct tcpa_event));
> +
> + /* convert raw integers for endianness */
> + temp_event.pcr_index = do_endian_conversion(event->pcr_index);
> + temp_event.event_type = do_endian_conversion(event->event_type);
> + temp_event.event_size = do_endian_conversion(event->event_size);
> +
> + tempPtr = (char *)&temp_event;
> +
> + for (i = 0; i < sizeof(struct tcpa_event) + temp_event.event_size; i++)
> + seq_putc(m, tempPtr[i]);
>
> return 0;
> +
> }
>
> static int tpm_bios_measurements_release(struct inode *inode,
> @@ -238,7 +271,7 @@ static int tpm_ascii_bios_measurements_show(struct seq_file *m, void *v)
> char *eventname;
> struct tcpa_event *event = v;
> unsigned char *event_entry =
> - (unsigned char *) (v + sizeof(struct tcpa_event));
> + (unsigned char *)(v + sizeof(struct tcpa_event));
>
> eventname = kmalloc(MAX_TEXT_EVENT, GFP_KERNEL);
> if (!eventname) {
> @@ -247,13 +280,14 @@ static int tpm_ascii_bios_measurements_show(struct seq_file *m, void *v)
> return -EFAULT;
> }
>
> - seq_printf(m, "%2d ", event->pcr_index);
> + /* 1st: PCR */
> + seq_printf(m, "%2d ", do_endian_conversion(event->pcr_index));
>
> /* 2nd: SHA1 */
> seq_printf(m, "%20phN", event->pcr_value);
>
> /* 3rd: event type identifier */
> - seq_printf(m, " %02x", event->event_type);
> + seq_printf(m, " %02x", do_endian_conversion(event->event_type));
>
> len += get_event_name(eventname, event, event_entry);
>
> diff --git a/drivers/char/tpm/tpm_eventlog.h b/drivers/char/tpm/tpm_eventlog.h
> index e7da086..267bfbd 100644
> --- a/drivers/char/tpm/tpm_eventlog.h
> +++ b/drivers/char/tpm/tpm_eventlog.h
> @@ -6,6 +6,12 @@
> #define MAX_TEXT_EVENT 1000 /* Max event string length */
> #define ACPI_TCPA_SIG "TCPA" /* 0x41504354 /'TCPA' */
>
> +#ifdef CONFIG_PPC64
> +#define do_endian_conversion(x) be32_to_cpu(x)
> +#else
> +#define do_endian_conversion(x) x
> +#endif
> +
> enum bios_platform_class {
> BIOS_CLIENT = 0x00,
> BIOS_SERVER = 0x01,
Hi Peter,
Can you please commit the patch in the next open window,
if you accept it as well? Thanks!
Regards,
Vicky
-------- Forwarded Message --------
From: Ashley Lai <[email protected]>
To: Hon Ching(Vicky) Lo <[email protected]>
Cc: [email protected], Peter Huewe <[email protected]>,
Ashley Lai <[email protected]>, Vicky Lo <[email protected]>,
[email protected], Joy Latten <[email protected]>
Subject: Re: [PATCH v4 1/2] vTPM: support little endian guests
Date: Thu, 18 Jun 2015 08:23:01 -0500 (CDT)
Looks better. Thanks.
Reviewed-by: Ashley Lai <[email protected]>
Cheers,
Ashley Lai
On Wed, 2015-06-17 at 18:17 -0400, Hon Ching(Vicky) Lo wrote:
> This patch makes the code endianness independent. We defined a
> macro do_endian_conversion to apply endianness to raw integers
> in the event entries so that they will be displayed properly.
> tpm_binary_bios_measurements_show() is modified for the display.
>
> Signed-off-by: Hon Ching(Vicky) Lo <[email protected]>
> Signed-off-by: Joy Latten <[email protected]>
> ---
> drivers/char/tpm/tpm_eventlog.c | 78 ++++++++++++++++++++++++++++-----------
> drivers/char/tpm/tpm_eventlog.h | 6 +++
> 2 files changed, 62 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c
> index 3a56a13..bd72fb0 100644
> --- a/drivers/char/tpm/tpm_eventlog.c
> +++ b/drivers/char/tpm/tpm_eventlog.c
> @@ -76,15 +76,25 @@ static void *tpm_bios_measurements_start(struct seq_file *m, loff_t *pos)
> void *addr = log->bios_event_log;
> void *limit = log->bios_event_log_end;
> struct tcpa_event *event;
> + u32 converted_event_size;
> + u32 converted_event_type;
> +
>
> /* read over *pos measurements */
> for (i = 0; i < *pos; i++) {
> event = addr;
>
> + converted_event_size =
> + do_endian_conversion(event->event_size);
> + converted_event_type =
> + do_endian_conversion(event->event_type);
> +
> if ((addr + sizeof(struct tcpa_event)) < limit) {
> - if (event->event_type == 0 && event->event_size == 0)
> + if ((converted_event_type == 0) &&
> + (converted_event_size == 0))
> return NULL;
> - addr += sizeof(struct tcpa_event) + event->event_size;
> + addr += (sizeof(struct tcpa_event) +
> + converted_event_size);
> }
> }
>
> @@ -94,8 +104,12 @@ static void *tpm_bios_measurements_start(struct seq_file *m, loff_t *pos)
>
> event = addr;
>
> - if ((event->event_type == 0 && event->event_size == 0) ||
> - ((addr + sizeof(struct tcpa_event) + event->event_size) >= limit))
> + converted_event_size = do_endian_conversion(event->event_size);
> + converted_event_type = do_endian_conversion(event->event_type);
> +
> + if (((converted_event_type == 0) && (converted_event_size == 0))
> + || ((addr + sizeof(struct tcpa_event) + converted_event_size)
> + >= limit))
> return NULL;
>
> return addr;
> @@ -107,8 +121,12 @@ static void *tpm_bios_measurements_next(struct seq_file *m, void *v,
> struct tcpa_event *event = v;
> struct tpm_bios_log *log = m->private;
> void *limit = log->bios_event_log_end;
> + u32 converted_event_size;
> + u32 converted_event_type;
>
> - v += sizeof(struct tcpa_event) + event->event_size;
> + converted_event_size = do_endian_conversion(event->event_size);
> +
> + v += sizeof(struct tcpa_event) + converted_event_size;
>
> /* now check if current entry is valid */
> if ((v + sizeof(struct tcpa_event)) >= limit)
> @@ -116,11 +134,11 @@ static void *tpm_bios_measurements_next(struct seq_file *m, void *v,
>
> event = v;
>
> - if (event->event_type == 0 && event->event_size == 0)
> - return NULL;
> + converted_event_size = do_endian_conversion(event->event_size);
> + converted_event_type = do_endian_conversion(event->event_type);
>
> - if ((event->event_type == 0 && event->event_size == 0) ||
> - ((v + sizeof(struct tcpa_event) + event->event_size) >= limit))
> + if (((converted_event_type == 0) && (converted_event_size == 0)) ||
> + ((v + sizeof(struct tcpa_event) + converted_event_size) >= limit))
> return NULL;
>
> (*pos)++;
> @@ -140,7 +158,7 @@ static int get_event_name(char *dest, struct tcpa_event *event,
> int i, n_len = 0, d_len = 0;
> struct tcpa_pc_event *pc_event;
>
> - switch(event->event_type) {
> + switch (do_endian_conversion(event->event_type)) {
> case PREBOOT:
> case POST_CODE:
> case UNUSED:
> @@ -156,14 +174,16 @@ static int get_event_name(char *dest, struct tcpa_event *event,
> case NONHOST_CODE:
> case NONHOST_CONFIG:
> case NONHOST_INFO:
> - name = tcpa_event_type_strings[event->event_type];
> + name = tcpa_event_type_strings[do_endian_conversion
> + (event->event_type)];
> n_len = strlen(name);
> break;
> case SEPARATOR:
> case ACTION:
> - if (MAX_TEXT_EVENT > event->event_size) {
> + if (MAX_TEXT_EVENT >
> + do_endian_conversion(event->event_size)) {
> name = event_entry;
> - n_len = event->event_size;
> + n_len = do_endian_conversion(event->event_size);
> }
> break;
> case EVENT_TAG:
> @@ -171,7 +191,7 @@ static int get_event_name(char *dest, struct tcpa_event *event,
>
> /* ToDo Row data -> Base64 */
>
> - switch (pc_event->event_id) {
> + switch (do_endian_conversion(pc_event->event_id)) {
> case SMBIOS:
> case BIS_CERT:
> case CMOS:
> @@ -179,7 +199,8 @@ static int get_event_name(char *dest, struct tcpa_event *event,
> case OPTION_ROM_EXEC:
> case OPTION_ROM_CONFIG:
> case S_CRTM_VERSION:
> - name = tcpa_pc_event_id_strings[pc_event->event_id];
> + name = tcpa_pc_event_id_strings[do_endian_conversion
> + (pc_event->event_id)];
> n_len = strlen(name);
> break;
> /* hash data */
> @@ -188,7 +209,8 @@ static int get_event_name(char *dest, struct tcpa_event *event,
> case OPTION_ROM_MICROCODE:
> case S_CRTM_CONTENTS:
> case POST_CONTENTS:
> - name = tcpa_pc_event_id_strings[pc_event->event_id];
> + name = tcpa_pc_event_id_strings[do_endian_conversion
> + (pc_event->event_id)];
> n_len = strlen(name);
> for (i = 0; i < 20; i++)
> d_len += sprintf(&data[2*i], "%02x",
> @@ -209,13 +231,24 @@ static int get_event_name(char *dest, struct tcpa_event *event,
> static int tpm_binary_bios_measurements_show(struct seq_file *m, void *v)
> {
> struct tcpa_event *event = v;
> - char *data = v;
> + struct tcpa_event temp_event;
> + char *tempPtr;
> int i;
>
> - for (i = 0; i < sizeof(struct tcpa_event) + event->event_size; i++)
> - seq_putc(m, data[i]);
> + memcpy(&temp_event, event, sizeof(struct tcpa_event));
> +
> + /* convert raw integers for endianness */
> + temp_event.pcr_index = do_endian_conversion(event->pcr_index);
> + temp_event.event_type = do_endian_conversion(event->event_type);
> + temp_event.event_size = do_endian_conversion(event->event_size);
> +
> + tempPtr = (char *)&temp_event;
> +
> + for (i = 0; i < sizeof(struct tcpa_event) + temp_event.event_size; i++)
> + seq_putc(m, tempPtr[i]);
>
> return 0;
> +
> }
>
> static int tpm_bios_measurements_release(struct inode *inode,
> @@ -238,7 +271,7 @@ static int tpm_ascii_bios_measurements_show(struct seq_file *m, void *v)
> char *eventname;
> struct tcpa_event *event = v;
> unsigned char *event_entry =
> - (unsigned char *) (v + sizeof(struct tcpa_event));
> + (unsigned char *)(v + sizeof(struct tcpa_event));
>
> eventname = kmalloc(MAX_TEXT_EVENT, GFP_KERNEL);
> if (!eventname) {
> @@ -247,13 +280,14 @@ static int tpm_ascii_bios_measurements_show(struct seq_file *m, void *v)
> return -EFAULT;
> }
>
> - seq_printf(m, "%2d ", event->pcr_index);
> + /* 1st: PCR */
> + seq_printf(m, "%2d ", do_endian_conversion(event->pcr_index));
>
> /* 2nd: SHA1 */
> seq_printf(m, "%20phN", event->pcr_value);
>
> /* 3rd: event type identifier */
> - seq_printf(m, " %02x", event->event_type);
> + seq_printf(m, " %02x", do_endian_conversion(event->event_type));
>
> len += get_event_name(eventname, event, event_entry);
>
> diff --git a/drivers/char/tpm/tpm_eventlog.h b/drivers/char/tpm/tpm_eventlog.h
> index e7da086..267bfbd 100644
> --- a/drivers/char/tpm/tpm_eventlog.h
> +++ b/drivers/char/tpm/tpm_eventlog.h
> @@ -6,6 +6,12 @@
> #define MAX_TEXT_EVENT 1000 /* Max event string length */
> #define ACPI_TCPA_SIG "TCPA" /* 0x41504354 /'TCPA' */
>
> +#ifdef CONFIG_PPC64
> +#define do_endian_conversion(x) be32_to_cpu(x)
> +#else
> +#define do_endian_conversion(x) x
> +#endif
> +
> enum bios_platform_class {
> BIOS_CLIENT = 0x00,
> BIOS_SERVER = 0x01,
Hi Peter,
Please also commit this patch, if you accept it as well.
Thanks,
Vicky
-------- Forwarded Message --------
From: Ashley Lai <[email protected]>
To: Hon Ching(Vicky) Lo <[email protected]>
Cc: [email protected], Peter Huewe <[email protected]>,
Ashley Lai <[email protected]>, Vicky Lo <[email protected]>,
[email protected], Joy Latten <[email protected]>
>Subject: Re: [PATCH v4 2/2] TPM: remove unnecessary little endian
conversion
> Date: Thu, 18 Jun 2015 08:23:33 -0500 (CDT)
> Looks good.
> Reviewed-by: Ashley Lai <[email protected]>
> Ashley Lai
On Wed, 2015-06-17 at 18:17 -0400, Hon Ching(Vicky) Lo wrote:
> The base pointer for the event log is allocated in the local
> kernel (in prom_instantiate_sml()), therefore it is already in
> the host's endian byte order and requires no conversion.
>
> The content of the 'basep' pointer in read_log() stores the
> base address of the log. This patch ensures that it is correctly
> implemented.
>
> Signed-off-by: Hon Ching(Vicky) Lo <[email protected]>
> Signed-off-by: Joy Latten <[email protected]>
> ---
> drivers/char/tpm/tpm_of.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_of.c b/drivers/char/tpm/tpm_of.c
> index c002d1b..62a22ce 100644
> --- a/drivers/char/tpm/tpm_of.c
> +++ b/drivers/char/tpm/tpm_of.c
> @@ -24,7 +24,7 @@ int read_log(struct tpm_bios_log *log)
> {
> struct device_node *np;
> const u32 *sizep;
> - const __be64 *basep;
> + const u64 *basep;
>
> if (log->bios_event_log != NULL) {
> pr_err("%s: ERROR - Eventlog already initialized\n", __func__);
> @@ -63,7 +63,7 @@ int read_log(struct tpm_bios_log *log)
>
> log->bios_event_log_end = log->bios_event_log + *sizep;
>
> - memcpy(log->bios_event_log, __va(be64_to_cpup(basep)), *sizep);
> + memcpy(log->bios_event_log, __va(*basep), *sizep);
>
> return 0;
>
Hi Vicky,
sorry for the late reply
> This patch makes the code endianness independent. We defined a
> macro do_endian_conversion to apply endianness to raw integers
> in the event entries so that they will be displayed properly.
> tpm_binary_bios_measurements_show() is modified for the display.
>
> Signed-off-by: Hon Ching(Vicky) Lo <[email protected]>
> Signed-off-by: Joy Latten <[email protected]>
> b/drivers/char/tpm/tpm_eventlog.h index e7da086..267bfbd 100644
> --- a/drivers/char/tpm/tpm_eventlog.h
> +++ b/drivers/char/tpm/tpm_eventlog.h
> @@ -6,6 +6,12 @@
> #define MAX_TEXT_EVENT 1000 /* Max event string length */
> #define ACPI_TCPA_SIG "TCPA" /* 0x41504354 /'TCPA' */
>
> +#ifdef CONFIG_PPC64
> +#define do_endian_conversion(x) be32_to_cpu(x)
> +#else
> +#define do_endian_conversion(x) x
> +#endif
Why is this macro needed?
shouldn't the be32_to_cpu macro already do correct thing?
Or am I missing something here?
Thanks,
Peter
Hi Peter,
On Mon, 2015-07-13 at 23:08 +0200, Peter Hüwe wrote:
> Hi Vicky,
>
> sorry for the late reply
>
>
> > This patch makes the code endianness independent. We defined a
> > macro do_endian_conversion to apply endianness to raw integers
> > in the event entries so that they will be displayed properly.
> > tpm_binary_bios_measurements_show() is modified for the display.
> >
> > Signed-off-by: Hon Ching(Vicky) Lo <[email protected]>
> > Signed-off-by: Joy Latten <[email protected]>
>
> > b/drivers/char/tpm/tpm_eventlog.h index e7da086..267bfbd 100644
> > --- a/drivers/char/tpm/tpm_eventlog.h
> > +++ b/drivers/char/tpm/tpm_eventlog.h
> > @@ -6,6 +6,12 @@
> > #define MAX_TEXT_EVENT 1000 /* Max event string length */
> > #define ACPI_TCPA_SIG "TCPA" /* 0x41504354 /'TCPA' */
> >
> > +#ifdef CONFIG_PPC64
> > +#define do_endian_conversion(x) be32_to_cpu(x)
> > +#else
> > +#define do_endian_conversion(x) x
> > +#endif
>
>
> Why is this macro needed?
> shouldn't the be32_to_cpu macro already do correct thing?
> Or am I missing something here?
>
>
> Thanks,
> Peter
>
The macro is defined to not do the conversion in the architecture
that does not need it.
Regards,
Vicky
Hi Vicky,
Am Donnerstag, 16. Juli 2015, 19:54:15 schrieb Hon Ching(Vicky) Lo:
> Hi Peter,
>
> On Mon, 2015-07-13 at 23:08 +0200, Peter Hüwe wrote:
> > Hi Vicky,
> >
> > sorry for the late reply
> >
> > > This patch makes the code endianness independent. We defined a
> > > macro do_endian_conversion to apply endianness to raw integers
> > > in the event entries so that they will be displayed properly.
> > > tpm_binary_bios_measurements_show() is modified for the display.
> > >
> > > Signed-off-by: Hon Ching(Vicky) Lo <[email protected]>
> > > Signed-off-by: Joy Latten <[email protected]>
> > >
> > > b/drivers/char/tpm/tpm_eventlog.h index e7da086..267bfbd 100644
> > > --- a/drivers/char/tpm/tpm_eventlog.h
> > > +++ b/drivers/char/tpm/tpm_eventlog.h
> > > @@ -6,6 +6,12 @@
> > >
> > > #define MAX_TEXT_EVENT 1000 /* Max event string length */
> > > #define ACPI_TCPA_SIG "TCPA" /* 0x41504354 /'TCPA' */
> > >
> > > +#ifdef CONFIG_PPC64
> > > +#define do_endian_conversion(x) be32_to_cpu(x)
> > > +#else
> > > +#define do_endian_conversion(x) x
> > > +#endif
> >
> > Why is this macro needed?
> > shouldn't the be32_to_cpu macro already do correct thing?
> > Or am I missing something here?
> >
> >
> > Thanks,
> > Peter
>
> The macro is defined to not do the conversion in the architecture
> that does not need it.
Unfortunately I'm still not convinced this is needed?
be32_to_cpu(x)
should already do the right thing if no conversion is needed ? (being defined
as (x))
Or is it not?
Peter
On Thu, 2015-07-16 at 20:43 +0200, Peter Hüwe wrote:
> Hi Vicky,
> Am Donnerstag, 16. Juli 2015, 19:54:15 schrieb Hon Ching(Vicky) Lo:
> > Hi Peter,
> >
> > On Mon, 2015-07-13 at 23:08 +0200, Peter Hüwe wrote:
> > > Hi Vicky,
> > >
> > > sorry for the late reply
> > >
> > > > This patch makes the code endianness independent. We defined a
> > > > macro do_endian_conversion to apply endianness to raw integers
> > > > in the event entries so that they will be displayed properly.
> > > > tpm_binary_bios_measurements_show() is modified for the display.
> > > >
> > > > Signed-off-by: Hon Ching(Vicky) Lo <[email protected]>
> > > > Signed-off-by: Joy Latten <[email protected]>
> > > >
> > > > b/drivers/char/tpm/tpm_eventlog.h index e7da086..267bfbd 100644
> > > > --- a/drivers/char/tpm/tpm_eventlog.h
> > > > +++ b/drivers/char/tpm/tpm_eventlog.h
> > > > @@ -6,6 +6,12 @@
> > > >
> > > > #define MAX_TEXT_EVENT 1000 /* Max event string length */
> > > > #define ACPI_TCPA_SIG "TCPA" /* 0x41504354 /'TCPA' */
> > > >
> > > > +#ifdef CONFIG_PPC64
> > > > +#define do_endian_conversion(x) be32_to_cpu(x)
> > > > +#else
> > > > +#define do_endian_conversion(x) x
> > > > +#endif
> > >
> > > Why is this macro needed?
> > > shouldn't the be32_to_cpu macro already do correct thing?
> > > Or am I missing something here?
> > >
> > >
> > > Thanks,
> > > Peter
> >
> > The macro is defined to not do the conversion in the architecture
> > that does not need it.
>
> Unfortunately I'm still not convinced this is needed?
> be32_to_cpu(x)
> should already do the right thing if no conversion is needed ? (being defined
> as (x))
> Or is it not?
>
>
>
> Peter
>
include/linux/byteorder/generic.h:
#define be32_to_cpu __be32_to_cpu
include/uapi/linux/byteorder/little_endian.h:
#define __be32_to_cpu(x) __swab32((__force __u32)(__be32)(x))
include/uapi/linux/byteorder/big_endian.h:
#define __be32_to_cpu(x) ((__force __u32)(__be32)(x))
The above defines show that be32_to_cpu(x) would do a byte swap on x.
The defines for __be32_to_cpu(x) in both little_endian.h and
big_endian.h
are doing the "right thing" that you mentioned.
However, with the architecture differences between ppc64 and x86
for instance, we need the new macro to not do the conversion.
The power firmware will always be in BE. Therefore, firmware will
pass BE data to an LE operating system, so the conversion is needed.
Whereas for an x86 system, BIOS gives LE data to the LE OS. (i.e.
the macro is needed for LE support. Does it make sense?
Vicky