Re: [PATCH v4 1/2] vTPM: support little endian guests

["Hon Ching(Vicky) Lo" <honclo@linux.vnet.ibm.com>]

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 <honclo@linux.vnet.ibm.com>
> > > > Signed-off-by: Joy Latten <jmlatten@linux.vnet.ibm.com>
> > > > 
> > > > 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