Messages in this thread | | | Date | Mon, 29 Aug 2022 07:30:00 +0200 | From | Greg Kroah-Hartman <> | Subject | Re: [PATCH v11 1/3] x86/tdx: Add TDX Guest attestation interface driver |
| |
On Sun, Aug 28, 2022 at 05:19:33PM -0700, Sathyanarayanan Kuppuswamy wrote: > Hi, > > On 8/28/22 1:14 PM, Dave Hansen wrote: > > On 8/26/22 08:06, Kuppuswamy Sathyanarayanan wrote: > >> +struct tdx_report_req { > >> + __u8 subtype; > >> + __u64 reportdata; > >> + __u32 rpd_len; > >> + __u64 tdreport; > >> + __u32 tdr_len; > >> +}; > > > > Please do take a look at how the compiler ends up building that structure. > > > > If you sized things to "save space", the way the compiler treats that > > structure may surprise you. You might also want to look at how a 32-bit > > compile deals with it versus a 64-bit one. > > Since it is only used in user/kernel ABI, I did not consider the size > issue.
That is _EXACTLY_ why you need to consider the size issues here.
> But I understand your point. The size of this struct in a 64-bit > system is 40 bytes. So we did not gain anything with using different member > sizes. In a 32-bit system, size due to padding is less compared to 64-bit.
That's not the issue here, please use a tool like pahole to see the problems with this definition.
> I will re-arrange the struct as below. With this change, the size will come > down to 32 bytes. > > struct tdx_report_req { > __u64 reportdata;; > __u64 tdreport; > __u32 rpd_len > __u32 tdr_len; > __u8 subtype; > };
That's better, but again, please use pahole.
thanks,
greg k-h
| |