Messages in this thread | | | From | Ashley Lai <> | Date | Tue, 19 May 2015 16:08:09 -0500 (CDT) | Subject | Re: [PATCH 3/3] vTPM: support little endian guests |
| |
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
| |