Re: [PATCH 3/3] vTPM: support little endian guests

[Ashley Lai <ashley@ashleylai.com>]

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