Re: [PATCH] tpm: introduce struct tpm_buf

[Jason Gunthorpe]

On Wed, Jun 03, 2015 at 03:23:31PM +0300, Jarkko Sakkinen wrote:
> On Tue, Jun 02, 2015 at 12:13:15PM -0600, Jason Gunthorpe wrote:
> > On Tue, Jun 02, 2015 at 04:04:22PM +0300, Jarkko Sakkinen wrote:
> > > +/* A string buffer type for constructing TPM commands. This is based on the
> > > + * code in security/keys/trusted.h.
> > > + */
> > > +
> > > +#define TPM_BUF_SIZE 512
> > > +
> > > +struct tpm_buf {
> > > +	u8 data[TPM_BUF_SIZE];
> > This should be u32 or u64 to guarentee correct alignment for the
> > casting.
> 
> Good catch. The functions where this might cause trouble are *_length()
> and *_tag().
> 
> In other places misalignment should not cause any regressions since data
> is not directly assigned to the buffer with pointer casting.
> 
> I would prefer to fix by changing *_length() and *_tag() to copy the
> value to a local variable and return that. It's a fail safe way and here
> the performance is not an issue.

I would change the type, that is very simple and will improve
performance of the memcpy as well.

It actually isn't a problem for the casts to tpm_input_header - any
structure marked __packed will cause the compiler to assume that the
entire structure is unaligned and code gen accordingly, so the cast
will always work, but on x86 it will be more efficient if the array
is aligned.

Ideally we could change to something like:
 __attribute__((packed,aligned(4))) 

Instead of __packed which will help the compiler minimize unaligned
load instructions..

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/