* [PATCH 0/3] additional little endian support
@ 2015-05-06 0:51 Hon Ching(Vicky) Lo
2015-05-06 0:51 ` [PATCH 1/3] vTPM: fixed the limit checking Hon Ching(Vicky) Lo
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Hon Ching(Vicky) Lo @ 2015-05-06 0:51 UTC (permalink / raw)
To: tpmdd-devel
Cc: Peter Huewe, Ashley Lai, Mimi Zohar, Vicky Lo, linux-kernel,
Hon Ching(Vicky) Lo
Hi,
The patch set converts big endian event log entries to guest format
in PowerPC, which supports both little endian and big endian guests.
It also contains a fix to make sure the last event entry wasn't skipped.
Hon Ching(Vicky) Lo (3):
vTPM: fixed the limit checking
TPM: remove unnecessary little endian conversion
vTPM: support little endian guests
drivers/char/tpm/tpm_eventlog.c | 95 ++++++++++++++++++++++++++++++---------
drivers/char/tpm/tpm_of.c | 4 +-
2 files changed, 75 insertions(+), 24 deletions(-)
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/3] vTPM: fixed the limit checking
2015-05-06 0:51 [PATCH 0/3] additional little endian support Hon Ching(Vicky) Lo
@ 2015-05-06 0:51 ` Hon Ching(Vicky) Lo
2015-05-11 13:02 ` [tpmdd-devel] " Stefan Berger
2015-05-06 0:51 ` [PATCH 2/3] TPM: remove unnecessary little endian conversion Hon Ching(Vicky) Lo
2015-05-06 0:51 ` [PATCH 3/3] vTPM: support little endian guests Hon Ching(Vicky) Lo
2 siblings, 1 reply; 12+ messages in thread
From: Hon Ching(Vicky) Lo @ 2015-05-06 0:51 UTC (permalink / raw)
To: tpmdd-devel
Cc: Peter Huewe, Ashley Lai, Mimi Zohar, Vicky Lo, linux-kernel,
Hon Ching(Vicky) Lo, Joy Latten
Do not skip the last entry of the event log.
Signed-off-by: Hon Ching(Vicky) Lo <honclo@linux.vnet.ibm.com>
Signed-off-by: Joy Latten <jmlatten@linux.vnet.ibm.com>
Changelog:
- remove redundant code
---
drivers/char/tpm/tpm_eventlog.c | 5 +----
1 files changed, 1 insertions(+), 4 deletions(-)
diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c
index 3a56a13..e77d8c1 100644
--- a/drivers/char/tpm/tpm_eventlog.c
+++ b/drivers/char/tpm/tpm_eventlog.c
@@ -116,11 +116,8 @@ 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;
-
if ((event->event_type == 0 && event->event_size == 0) ||
- ((v + sizeof(struct tcpa_event) + event->event_size) >= limit))
+ ((v + sizeof(struct tcpa_event) + event->event_size) > limit))
return NULL;
(*pos)++;
--
1.7.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/3] TPM: remove unnecessary little endian conversion
2015-05-06 0:51 [PATCH 0/3] additional little endian support Hon Ching(Vicky) Lo
2015-05-06 0:51 ` [PATCH 1/3] vTPM: fixed the limit checking Hon Ching(Vicky) Lo
@ 2015-05-06 0:51 ` Hon Ching(Vicky) Lo
2015-05-06 0:51 ` [PATCH 3/3] vTPM: support little endian guests Hon Ching(Vicky) Lo
2 siblings, 0 replies; 12+ messages in thread
From: Hon Ching(Vicky) Lo @ 2015-05-06 0:51 UTC (permalink / raw)
To: tpmdd-devel
Cc: Peter Huewe, Ashley Lai, Mimi Zohar, Vicky Lo, linux-kernel,
Hon Ching(Vicky) Lo, Joy Latten
prom_instantiate_sml() already converted the base pointer to little
endian. This patch removes this unnecessary additional conversion.
Signed-off-by: Hon Ching(Vicky) Lo <honclo@linux.vnet.ibm.com>
Signed-off-by: Joy Latten <jmlatten@linux.vnet.ibm.com>
---
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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 3/3] vTPM: support little endian guests
2015-05-06 0:51 [PATCH 0/3] additional little endian support Hon Ching(Vicky) Lo
2015-05-06 0:51 ` [PATCH 1/3] vTPM: fixed the limit checking Hon Ching(Vicky) Lo
2015-05-06 0:51 ` [PATCH 2/3] TPM: remove unnecessary little endian conversion Hon Ching(Vicky) Lo
@ 2015-05-06 0:51 ` Hon Ching(Vicky) Lo
2015-05-08 22:31 ` Ashley Lai
2015-05-19 21:08 ` Ashley Lai
2 siblings, 2 replies; 12+ messages in thread
From: Hon Ching(Vicky) Lo @ 2015-05-06 0:51 UTC (permalink / raw)
To: tpmdd-devel
Cc: Peter Huewe, Ashley Lai, Mimi Zohar, Vicky Lo, linux-kernel,
Hon Ching(Vicky) Lo, Joy Latten
The event log in ppc64 arch is always in big endian format. PowerPC
supports both little endian and big endian guests. This patch converts
the event log entries to guest format.
We defined a macro to convert to guest format. In addition,
tpm_binary_bios_measurements_show() is modified to parse the event
and print each field individually.
Signed-off-by: Hon Ching(Vicky) Lo <honclo@linux.vnet.ibm.com>
Signed-off-by: Joy Latten <jmlatten@linux.vnet.ibm.com>
---
drivers/char/tpm/tpm_eventlog.c | 92 +++++++++++++++++++++++++++++++--------
1 files changed, 73 insertions(+), 19 deletions(-)
diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c
index e77d8c1..1b62c52 100644
--- a/drivers/char/tpm/tpm_eventlog.c
+++ b/drivers/char/tpm/tpm_eventlog.c
@@ -28,6 +28,11 @@
#include "tpm.h"
#include "tpm_eventlog.h"
+#ifdef CONFIG_PPC64
+#define convert_to_host_format(x) be32_to_cpu(x)
+#else
+#define convert_to_host_format(x) x
+#endif
static const char* tcpa_event_type_strings[] = {
"PREBOOT",
@@ -82,9 +87,11 @@ static void *tpm_bios_measurements_start(struct seq_file *m, loff_t *pos)
event = addr;
if ((addr + sizeof(struct tcpa_event)) < limit) {
- if (event->event_type == 0 && event->event_size == 0)
+ if ((convert_to_host_format(event->event_type) == 0) &&
+ (convert_to_host_format(event->event_size) == 0))
return NULL;
- addr += sizeof(struct tcpa_event) + event->event_size;
+ addr += (sizeof(struct tcpa_event) +
+ convert_to_host_format(event->event_size));
}
}
@@ -94,8 +101,11 @@ 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))
+ if (((convert_to_host_format(event->event_type) == 0) &&
+ (convert_to_host_format(event->event_size) == 0))
+ ||
+ ((addr + sizeof(struct tcpa_event) +
+ convert_to_host_format(event->event_size)) >= limit))
return NULL;
return addr;
@@ -108,7 +118,8 @@ static void *tpm_bios_measurements_next(struct seq_file *m, void *v,
struct tpm_bios_log *log = m->private;
void *limit = log->bios_event_log_end;
- v += sizeof(struct tcpa_event) + event->event_size;
+ v += (sizeof(struct tcpa_event) +
+ convert_to_host_format(event->event_size));
/* now check if current entry is valid */
if ((v + sizeof(struct tcpa_event)) >= limit)
@@ -116,8 +127,11 @@ static void *tpm_bios_measurements_next(struct seq_file *m, void *v,
event = v;
- if ((event->event_type == 0 && event->event_size == 0) ||
- ((v + sizeof(struct tcpa_event) + event->event_size) > limit))
+ if (((convert_to_host_format(event->event_type) == 0) &&
+ (convert_to_host_format(event->event_size) == 0))
+ ||
+ ((v + sizeof(struct tcpa_event) +
+ convert_to_host_format(event->event_size)) > limit))
return NULL;
(*pos)++;
@@ -137,7 +151,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(convert_to_host_format(event->event_type)) {
case PREBOOT:
case POST_CODE:
case UNUSED:
@@ -153,14 +167,17 @@ 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[convert_to_host_format
+ (event->event_type)];
n_len = strlen(name);
break;
case SEPARATOR:
case ACTION:
- if (MAX_TEXT_EVENT > event->event_size) {
+ if (MAX_TEXT_EVENT >
+ convert_to_host_format(event->event_size)) {
name = event_entry;
- n_len = event->event_size;
+ n_len = convert_to_host_format(event->event_size);
}
break;
case EVENT_TAG:
@@ -168,7 +185,7 @@ static int get_event_name(char *dest, struct tcpa_event *event,
/* ToDo Row data -> Base64 */
- switch (pc_event->event_id) {
+ switch(convert_to_host_format(pc_event->event_id)) {
case SMBIOS:
case BIS_CERT:
case CMOS:
@@ -176,7 +193,9 @@ 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[convert_to_host_format
+ (pc_event->event_id)];
n_len = strlen(name);
break;
/* hash data */
@@ -185,7 +204,9 @@ 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[convert_to_host_format
+ (pc_event->event_id)];
n_len = strlen(name);
for (i = 0; i < 20; i++)
d_len += sprintf(&data[2*i], "%02x",
@@ -208,11 +229,43 @@ static int tpm_binary_bios_measurements_show(struct seq_file *m, void *v)
struct tcpa_event *event = v;
char *data = v;
int i;
-
- for (i = 0; i < sizeof(struct tcpa_event) + event->event_size; i++)
+ u32 x;
+ char tmp[4];
+
+ /* PCR */
+ x = convert_to_host_format(event->pcr_index);
+ memcpy(tmp, &x, 4);
+ for (i = 0; i < 4; i++)
+ seq_putc(m, tmp[i]);
+ data += 4;
+
+ /* Event Type */
+ x = convert_to_host_format(event->event_type);
+ memcpy(tmp, &x, 4);
+ for (i = 0; i < 4; i++)
+ seq_putc(m, tmp[i]);
+ data += 4;
+
+ /* HASH */
+ for (i = 0; i < 20; i++)
seq_putc(m, data[i]);
+ data += 20;
+
+ /* Size */
+ x = convert_to_host_format(event->event_size);
+ memcpy(tmp, &x, 4);
+ for (i = 0; i < 4; i++)
+ seq_putc(m, tmp[i]);
+ data += 4;
+
+ /* Data */
+ if (convert_to_host_format(event->event_size)) {
+ for (i = 0; i < convert_to_host_format(event->event_size); i++)
+ seq_putc(m, data[i]);
+ }
return 0;
+
}
static int tpm_bios_measurements_release(struct inode *inode,
@@ -235,7 +288,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) {
@@ -244,13 +297,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 ", convert_to_host_format(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", convert_to_host_format(event->event_type));
len += get_event_name(eventname, event, event_entry);
--
1.7.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] vTPM: support little endian guests
2015-05-06 0:51 ` [PATCH 3/3] vTPM: support little endian guests Hon Ching(Vicky) Lo
@ 2015-05-08 22:31 ` Ashley Lai
2015-05-08 23:10 ` Hon Ching (Vicky) Lo
2015-05-11 22:07 ` Joy M. Latten
2015-05-19 21:08 ` Ashley Lai
1 sibling, 2 replies; 12+ messages in thread
From: Ashley Lai @ 2015-05-08 22:31 UTC (permalink / raw)
To: Hon Ching(Vicky) Lo
Cc: tpmdd-devel, Peter Huewe, Ashley Lai, Mimi Zohar, Vicky Lo,
linux-kernel, Joy Latten
> The event log in ppc64 arch is always in big endian format. PowerPC
> supports both little endian and big endian guests. This patch converts
> the event log entries to guest format.
I'm a little confused here. If this patch is to convert the event log
entries why are we convert in the conditional statements? One example
below:
+ if (((convert_to_host_format(event->event_type) == 0) &&
+ (convert_to_host_format(event->event_size) == 0))
+ ||
+ ((v + sizeof(struct tcpa_event) +
+ convert_to_host_format(event->event_size)) > limit))
>
> We defined a macro to convert to guest format. In addition,
> tpm_binary_bios_measurements_show() is modified to parse the event
> and print each field individually.
It's nice to have human readable format but it may break existing tools
that parse or understand the machine readable format. Any comments on
this anyone?
>>
> diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c
> index e77d8c1..1b62c52 100644
> --- a/drivers/char/tpm/tpm_eventlog.c
> +++ b/drivers/char/tpm/tpm_eventlog.c
> @@ -28,6 +28,11 @@
> #include "tpm.h"
> #include "tpm_eventlog.h"
>
> +#ifdef CONFIG_PPC64
> +#define convert_to_host_format(x) be32_to_cpu(x)
> +#else
> +#define convert_to_host_format(x) x
> +#endif
This can go in the header file tpm_eventlog.h
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] vTPM: support little endian guests
2015-05-08 22:31 ` Ashley Lai
@ 2015-05-08 23:10 ` Hon Ching (Vicky) Lo
2015-05-11 22:07 ` Joy M. Latten
1 sibling, 0 replies; 12+ messages in thread
From: Hon Ching (Vicky) Lo @ 2015-05-08 23:10 UTC (permalink / raw)
To: Ashley Lai
Cc: tpmdd-devel, Peter Huewe, Mimi Zohar, Vicky Lo, linux-kernel, Joy Latten
Thanks Ashley!
> > The event log in ppc64 arch is always in big endian format. PowerPC
> > supports both little endian and big endian guests. This patch converts
> > the event log entries to guest format.
>
> I'm a little confused here. If this patch is to convert the event log
> entries why are we convert in the conditional statements? One example
> below:
>
> + if (((convert_to_host_format(event->event_type) == 0) &&
> + (convert_to_host_format(event->event_size) == 0))
> + ||
> + ((v + sizeof(struct tcpa_event) +
> + convert_to_host_format(event->event_size)) > limit))
>
> >
> > We defined a macro to convert to guest format. In addition,
> > tpm_binary_bios_measurements_show() is modified to parse the event
> > and print each field individually.
>
We do not convert the whole event entry. Instead, we're converting only
what's necessary such as pcr_index, event_type and event_size. pcr_value
and event_data are of type u8. They do not need LE conversion.
> It's nice to have human readable format but it may break existing tools
> that parse or understand the machine readable format. Any comments on
> this anyone?
I got comments on the format, so I tried to make that conditional
statement all in one line, but the 'Lindent' tool puts the lines back to
the above format..
Regards,
Vicky
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [tpmdd-devel] [PATCH 1/3] vTPM: fixed the limit checking
2015-05-06 0:51 ` [PATCH 1/3] vTPM: fixed the limit checking Hon Ching(Vicky) Lo
@ 2015-05-11 13:02 ` Stefan Berger
2015-05-12 21:19 ` Joy M. Latten
0 siblings, 1 reply; 12+ messages in thread
From: Stefan Berger @ 2015-05-11 13:02 UTC (permalink / raw)
To: Hon Ching(Vicky) Lo, tpmdd-devel
Cc: Joy Latten, Ashley Lai, linux-kernel, Peter Huewe
On 05/05/2015 08:51 PM, Hon Ching(Vicky) Lo wrote:
> Do not skip the last entry of the event log.
>
> Signed-off-by: Hon Ching(Vicky) Lo <honclo@linux.vnet.ibm.com>
> Signed-off-by: Joy Latten <jmlatten@linux.vnet.ibm.com>
>
> Changelog:
> - remove redundant code
> ---
> drivers/char/tpm/tpm_eventlog.c | 5 +----
> 1 files changed, 1 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c
> index 3a56a13..e77d8c1 100644
> --- a/drivers/char/tpm/tpm_eventlog.c
> +++ b/drivers/char/tpm/tpm_eventlog.c
> @@ -116,11 +116,8 @@ 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;
> -
> if ((event->event_type == 0 && event->event_size == 0) ||
> - ((v + sizeof(struct tcpa_event) + event->event_size) >= limit))
> + ((v + sizeof(struct tcpa_event) + event->event_size) > limit))
> return NULL;
>
> (*pos)++;
The limit stems from log->bios_event_log_end and is calculated as follows:
log->bios_event_log_end = log->bios_event_log + len;
The '>=' above is correct since the limit address itself is not part of
the log anymore.
The log is the following sequence of addresses:
[log->bios_event_log ... log->bios_event_log + len - 1 ]
or
[log->bios_event_log ... log->bios_event_log + len [
with ']' meaning inclusive and '[' meaning exclusive.
Stefan
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] vTPM: support little endian guests
2015-05-08 22:31 ` Ashley Lai
2015-05-08 23:10 ` Hon Ching (Vicky) Lo
@ 2015-05-11 22:07 ` Joy M. Latten
1 sibling, 0 replies; 12+ messages in thread
From: Joy M. Latten @ 2015-05-11 22:07 UTC (permalink / raw)
To: Ashley Lai
Cc: Hon Ching(Vicky) Lo, tpmdd-devel, Peter Huewe, Mimi Zohar,
Vicky Lo, linux-kernel
Hi Ashley,
On Fri, 2015-05-08 at 17:31 -0500, Ashley Lai wrote:
> > The event log in ppc64 arch is always in big endian format. PowerPC
> > supports both little endian and big endian guests. This patch converts
> > the event log entries to guest format.
>
> I'm a little confused here. If this patch is to convert the event log
> entries why are we convert in the conditional statements? One example
> below:
>
> + if (((convert_to_host_format(event->event_type) == 0) &&
> + (convert_to_host_format(event->event_size) == 0))
> + ||
> + ((v + sizeof(struct tcpa_event) +
> + convert_to_host_format(event->event_size)) > limit))
>
Ah, ok... I see what you are saying.. description may be a bit
confusing... we are not converting the event log entries.
We are just making sure that raw integers are correctly interpreted on
both big and little endian platforms.
Phype sends info (including logs) in big endian.
Thus any raw integers in that data, like event_size and
event_type may become "garbaged" when OS is ppc64-LE.
This will ensure raw integers are correctly interpreted for both
LE and BE platforms.
> >
> > We defined a macro to convert to guest format. In addition,
> > tpm_binary_bios_measurements_show() is modified to parse the event
> > and print each field individually.
>
> It's nice to have human readable format but it may break existing tools
> that parse or understand the machine readable format. Any comments on
> this anyone?
>
Again, description...
We have not changed the format. We are just making sure that raw
integers are correctly interpreted on both big and little endian
platforms for vtpm.
> >>
> > diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c
> > index e77d8c1..1b62c52 100644
> > --- a/drivers/char/tpm/tpm_eventlog.c
> > +++ b/drivers/char/tpm/tpm_eventlog.c
> > @@ -28,6 +28,11 @@
> > #include "tpm.h"
> > #include "tpm_eventlog.h"
> >
> > +#ifdef CONFIG_PPC64
> > +#define convert_to_host_format(x) be32_to_cpu(x)
> > +#else
> > +#define convert_to_host_format(x) x
> > +#endif
>
> This can go in the header file tpm_eventlog.h
>
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [tpmdd-devel] [PATCH 1/3] vTPM: fixed the limit checking
2015-05-11 13:02 ` [tpmdd-devel] " Stefan Berger
@ 2015-05-12 21:19 ` Joy M. Latten
0 siblings, 0 replies; 12+ messages in thread
From: Joy M. Latten @ 2015-05-12 21:19 UTC (permalink / raw)
To: Stefan Berger
Cc: Hon Ching(Vicky) Lo, tpmdd-devel, Ashley Lai, linux-kernel, Peter Huewe
Hi Stefan,
On Mon, 2015-05-11 at 09:02 -0400, Stefan Berger wrote:
> On 05/05/2015 08:51 PM, Hon Ching(Vicky) Lo wrote:
> > Do not skip the last entry of the event log.
> >
> > Signed-off-by: Hon Ching(Vicky) Lo <honclo@linux.vnet.ibm.com>
> > Signed-off-by: Joy Latten <jmlatten@linux.vnet.ibm.com>
> >
> > Changelog:
> > - remove redundant code
> > ---
> > drivers/char/tpm/tpm_eventlog.c | 5 +----
> > 1 files changed, 1 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c
> > index 3a56a13..e77d8c1 100644
> > --- a/drivers/char/tpm/tpm_eventlog.c
> > +++ b/drivers/char/tpm/tpm_eventlog.c
> > @@ -116,11 +116,8 @@ 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;
> > -
> > if ((event->event_type == 0 && event->event_size == 0) ||
> > - ((v + sizeof(struct tcpa_event) + event->event_size) >= limit))
> > + ((v + sizeof(struct tcpa_event) + event->event_size) > limit))
> > return NULL;
> >
> > (*pos)++;
>
> The limit stems from log->bios_event_log_end and is calculated as follows:
>
> log->bios_event_log_end = log->bios_event_log + len;
>
> The '>=' above is correct since the limit address itself is not part of
> the log anymore.
>
> The log is the following sequence of addresses:
>
> [log->bios_event_log ... log->bios_event_log + len - 1 ]
>
> or
>
> [log->bios_event_log ... log->bios_event_log + len [
>
> with ']' meaning inclusive and '[' meaning exclusive.
>
Thanks for pointing this out... you are correct.
We are just seeing where the spec describes this.
On powerpc(p8/powervm)) using vtpm, the last entry doesn't
get shown when ">=", so I suspect phype may be giving us
an "inclusive" len. Vicky and I will investigate this.
regards,
Joy
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] vTPM: support little endian guests
2015-05-06 0:51 ` [PATCH 3/3] vTPM: support little endian guests Hon Ching(Vicky) Lo
2015-05-08 22:31 ` Ashley Lai
@ 2015-05-19 21:08 ` Ashley Lai
2015-05-19 21:18 ` Ashley Lai
2015-05-20 21:23 ` Hon Ching (Vicky) Lo
1 sibling, 2 replies; 12+ messages in thread
From: Ashley Lai @ 2015-05-19 21:08 UTC (permalink / raw)
To: Hon Ching(Vicky) Lo
Cc: tpmdd-devel, Peter Huewe, Ashley Lai, Mimi Zohar, Vicky Lo,
linux-kernel, Joy Latten
Thank you Vicky and Joy for the clarification. This patch mainly
converts the fields in the tcpa_event structure. I see the code converts
everytime it accesses the event fields. Would it be more efficient if you
do the conversion once and reuse them when needed? Could
convert_to_host_format(x) takes x as a tcpa_event structure?
If not you still can convert individual fields and reuse them. I'm aware
that the pcr_value field is type u8 and it does not need the conversion
but if convert_to_host_format() can convert the structure it shouldn't convert
u8 type I think.
> static const char* tcpa_event_type_strings[] = {
> "PREBOOT",
> @@ -82,9 +87,11 @@ static void *tpm_bios_measurements_start(struct seq_file *m, loff_t *pos)
> event = addr;
>
> if ((addr + sizeof(struct tcpa_event)) < limit) {
> - if (event->event_type == 0 && event->event_size == 0)
> + if ((convert_to_host_format(event->event_type) == 0) &&
> + (convert_to_host_format(event->event_size) == 0))
> return NULL;
> - addr += sizeof(struct tcpa_event) + event->event_size;
> + addr += (sizeof(struct tcpa_event) +
> + convert_to_host_format(event->event_size));
> }
> }
>
> @@ -94,8 +101,11 @@ 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))
> + if (((convert_to_host_format(event->event_type) == 0) &&
> + (convert_to_host_format(event->event_size) == 0))
> + ||
> + ((addr + sizeof(struct tcpa_event) +
> + convert_to_host_format(event->event_size)) >= limit))
> return NULL;
>
> return addr;
In this function, event_type and event_size can be converted
once and reuse.
> case SEPARATOR:
> case ACTION:
> - if (MAX_TEXT_EVENT > event->event_size) {
> + if (MAX_TEXT_EVENT >
> + convert_to_host_format(event->event_size)) {
> name = event_entry;
> - n_len = event->event_size;
> + n_len = convert_to_host_format(event->event_size);
> }
> break;
> case EVENT_TAG:
Same here.
> @@ -208,11 +229,43 @@ static int tpm_binary_bios_measurements_show(struct seq_file *m, void *v)
> struct tcpa_event *event = v;
> char *data = v;
> int i;
> -
> - for (i = 0; i < sizeof(struct tcpa_event) + event->event_size; i++)
> + u32 x;
> + char tmp[4];
> +
> + /* PCR */
> + x = convert_to_host_format(event->pcr_index);
> + memcpy(tmp, &x, 4);
> + for (i = 0; i < 4; i++)
> + seq_putc(m, tmp[i]);
> + data += 4;
> +
> + /* Event Type */
> + x = convert_to_host_format(event->event_type);
> + memcpy(tmp, &x, 4);
> + for (i = 0; i < 4; i++)
> + seq_putc(m, tmp[i]);
> + data += 4;
> +
> + /* HASH */
> + for (i = 0; i < 20; i++)
> seq_putc(m, data[i]);
> + data += 20;
> +
> + /* Size */
> + x = convert_to_host_format(event->event_size);
> + memcpy(tmp, &x, 4);
> + for (i = 0; i < 4; i++)
> + seq_putc(m, tmp[i]);
> + data += 4;
> +
> + /* Data */
> + if (convert_to_host_format(event->event_size)) {
> + for (i = 0; i < convert_to_host_format(event->event_size); i++)
> + seq_putc(m, data[i]);
> + }
>
> return 0;
> +
> }
If the tcpa_event structure is converted, you may be able to get away
with memcpy and the for loop.
Thanks,
--Ashley Lai
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] vTPM: support little endian guests
2015-05-19 21:08 ` Ashley Lai
@ 2015-05-19 21:18 ` Ashley Lai
2015-05-20 21:23 ` Hon Ching (Vicky) Lo
1 sibling, 0 replies; 12+ messages in thread
From: Ashley Lai @ 2015-05-19 21:18 UTC (permalink / raw)
To: Ashley Lai
Cc: Hon Ching(Vicky) Lo, tpmdd-devel, Peter Huewe, Mimi Zohar,
Vicky Lo, linux-kernel, Joy Latten
>> @@ -208,11 +229,43 @@ static int tpm_binary_bios_measurements_show(struct
>> seq_file *m, void *v)
>> struct tcpa_event *event = v;
>> char *data = v;
>> int i;
>> -
>> - for (i = 0; i < sizeof(struct tcpa_event) + event->event_size; i++)
>> + u32 x;
>> + char tmp[4];
>> +
>> + /* PCR */
>> + x = convert_to_host_format(event->pcr_index);
>> + memcpy(tmp, &x, 4);
>> + for (i = 0; i < 4; i++)
>> + seq_putc(m, tmp[i]);
>> + data += 4;
>> +
>> + /* Event Type */
>> + x = convert_to_host_format(event->event_type);
>> + memcpy(tmp, &x, 4);
>> + for (i = 0; i < 4; i++)
>> + seq_putc(m, tmp[i]);
>> + data += 4;
>> +
>> + /* HASH */
>> + for (i = 0; i < 20; i++)
>> seq_putc(m, data[i]);
>> + data += 20;
>> +
>> + /* Size */
>> + x = convert_to_host_format(event->event_size);
>> + memcpy(tmp, &x, 4);
>> + for (i = 0; i < 4; i++)
>> + seq_putc(m, tmp[i]);
>> + data += 4;
>> +
>> + /* Data */
>> + if (convert_to_host_format(event->event_size)) {
>> + for (i = 0; i < convert_to_host_format(event->event_size);
>> i++)
>> + seq_putc(m, data[i]);
>> + }
>>
>> return 0;
>> +
>> }
> If the tcpa_event structure is converted, you may be able to get away with
> memcpy and the for loop.
You can get away with memcpy and use the original for loop.
for (i = 0; i < sizeof(struct tcpa_event) + event->event_size; i++)
seq_putc(m, data[i]);
Thanks,
--Ashley
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] vTPM: support little endian guests
2015-05-19 21:08 ` Ashley Lai
2015-05-19 21:18 ` Ashley Lai
@ 2015-05-20 21:23 ` Hon Ching (Vicky) Lo
1 sibling, 0 replies; 12+ messages in thread
From: Hon Ching (Vicky) Lo @ 2015-05-20 21:23 UTC (permalink / raw)
To: Ashley Lai
Cc: tpmdd-devel, Peter Huewe, Mimi Zohar, Vicky Lo, linux-kernel, Joy Latten
On Tue, 2015-05-19 at 16:08 -0500, Ashley Lai wrote:
> Thank you Vicky and Joy for the clarification. This patch mainly
> converts the fields in the tcpa_event structure. I see the code converts
> everytime it accesses the event fields. Would it be more efficient if you
> do the conversion once and reuse them when needed? Could
> convert_to_host_format(x) takes x as a tcpa_event structure?
> If not you still can convert individual fields and reuse them. I'm aware
> that the pcr_value field is type u8 and it does not need the conversion
> but if convert_to_host_format() can convert the structure it shouldn't convert
> u8 type I think.
>
> > static const char* tcpa_event_type_strings[] = {
> > "PREBOOT",
> > @@ -82,9 +87,11 @@ static void *tpm_bios_measurements_start(struct seq_file *m, loff_t *pos)
> > event = addr;
> >
> > if ((addr + sizeof(struct tcpa_event)) < limit) {
> > - if (event->event_type == 0 && event->event_size == 0)
> > + if ((convert_to_host_format(event->event_type) == 0) &&
> > + (convert_to_host_format(event->event_size) == 0))
> > return NULL;
> > - addr += sizeof(struct tcpa_event) + event->event_size;
> > + addr += (sizeof(struct tcpa_event) +
> > + convert_to_host_format(event->event_size));
> > }
> > }
> >
> > @@ -94,8 +101,11 @@ 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))
> > + if (((convert_to_host_format(event->event_type) == 0) &&
> > + (convert_to_host_format(event->event_size) == 0))
> > + ||
> > + ((addr + sizeof(struct tcpa_event) +
> > + convert_to_host_format(event->event_size)) >= limit))
> > return NULL;
> >
> > return addr;
>
> In this function, event_type and event_size can be converted
> once and reuse.
>
> > case SEPARATOR:
> > case ACTION:
> > - if (MAX_TEXT_EVENT > event->event_size) {
> > + if (MAX_TEXT_EVENT >
> > + convert_to_host_format(event->event_size)) {
> > name = event_entry;
> > - n_len = event->event_size;
> > + n_len = convert_to_host_format(event->event_size);
> > }
> > break;
> > case EVENT_TAG:
>
> Same here.
>
Agree. It's more efficient to do the conversion once and reuse.
convert_to_host_format(x) cannot take tcpa_event structure, as
be64_to_cpu() only converts raw integers.
> > @@ -208,11 +229,43 @@ static int tpm_binary_bios_measurements_show(struct seq_file *m, void *v)
> > struct tcpa_event *event = v;
> > char *data = v;
> > int i;
> > -
> > - for (i = 0; i < sizeof(struct tcpa_event) + event->event_size; i++)
> > + u32 x;
> > + char tmp[4];
> > +
> > + /* PCR */
> > + x = convert_to_host_format(event->pcr_index);
> > + memcpy(tmp, &x, 4);
> > + for (i = 0; i < 4; i++)
> > + seq_putc(m, tmp[i]);
> > + data += 4;
> > +
> > + /* Event Type */
> > + x = convert_to_host_format(event->event_type);
> > + memcpy(tmp, &x, 4);
> > + for (i = 0; i < 4; i++)
> > + seq_putc(m, tmp[i]);
> > + data += 4;
> > +
> > + /* HASH */
> > + for (i = 0; i < 20; i++)
> > seq_putc(m, data[i]);
> > + data += 20;
> > +
> > + /* Size */
> > + x = convert_to_host_format(event->event_size);
> > + memcpy(tmp, &x, 4);
> > + for (i = 0; i < 4; i++)
> > + seq_putc(m, tmp[i]);
> > + data += 4;
> > +
> > + /* Data */
> > + if (convert_to_host_format(event->event_size)) {
> > + for (i = 0; i < convert_to_host_format(event->event_size); i++)
> > + seq_putc(m, data[i]);
> > + }
> >
> > return 0;
> > +
> > }
> If the tcpa_event structure is converted, you may be able to get away
> with memcpy and the for loop.
>
> Thanks,
> --Ashley Lai
>
To simplify the code, we can try creating a struct that contains the
first 4 fields in the tcpa_event so to only copy over the header part.
Then we do conversion on it. That way, we can use a loop to print
byte-by-byte on the new struct. Then, we can use a small loop to
print byte-by-byte of the data part of the original event structure.
I'll try this and re-submit patch.
Thanks!
Vicky
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2015-05-20 21:24 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-06 0:51 [PATCH 0/3] additional little endian support Hon Ching(Vicky) Lo
2015-05-06 0:51 ` [PATCH 1/3] vTPM: fixed the limit checking Hon Ching(Vicky) Lo
2015-05-11 13:02 ` [tpmdd-devel] " Stefan Berger
2015-05-12 21:19 ` Joy M. Latten
2015-05-06 0:51 ` [PATCH 2/3] TPM: remove unnecessary little endian conversion Hon Ching(Vicky) Lo
2015-05-06 0:51 ` [PATCH 3/3] vTPM: support little endian guests Hon Ching(Vicky) Lo
2015-05-08 22:31 ` Ashley Lai
2015-05-08 23:10 ` Hon Ching (Vicky) Lo
2015-05-11 22:07 ` Joy M. Latten
2015-05-19 21:08 ` Ashley Lai
2015-05-19 21:18 ` Ashley Lai
2015-05-20 21:23 ` Hon Ching (Vicky) Lo
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.