linux-kernel - Re: [PATCH v2 02/10] x86/virt/tdx: Unbind global metadata read with 'struct tdx_tdmr

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <491e3134f4fb1a09436aceeefb4104c4e11275d7.camel@intel.com>
Date: Tue, 27 Aug 2024 07:22:38 +0000
From: "Huang, Kai" <kai.huang@...el.com>
To: "Hansen, Dave" <dave.hansen@...el.com>, "seanjc@...gle.com"
	<seanjc@...gle.com>, "bp@...en8.de" <bp@...en8.de>, "peterz@...radead.org"
	<peterz@...radead.org>, "hpa@...or.com" <hpa@...or.com>, "mingo@...hat.com"
	<mingo@...hat.com>, "Hunter, Adrian" <adrian.hunter@...el.com>, "Williams,
 Dan J" <dan.j.williams@...el.com>, "pbonzini@...hat.com"
	<pbonzini@...hat.com>, "kirill.shutemov@...ux.intel.com"
	<kirill.shutemov@...ux.intel.com>, "tglx@...utronix.de" <tglx@...utronix.de>
CC: "Gao, Chao" <chao.gao@...el.com>, "kvm@...r.kernel.org"
	<kvm@...r.kernel.org>, "binbin.wu@...ux.intel.com"
	<binbin.wu@...ux.intel.com>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "Edgecombe, Rick P"
	<rick.p.edgecombe@...el.com>, "Yamahata, Isaku" <isaku.yamahata@...el.com>,
	"x86@...nel.org" <x86@...nel.org>
Subject: Re: [PATCH v2 02/10] x86/virt/tdx: Unbind global metadata read with
 'struct tdx_tdmr_sysinfo'

On Tue, 2024-08-27 at 07:54 +0300, Adrian Hunter wrote:
> > > 
> > > BUILD_BUG_ON() requires a function, but it is still
> > > be possible to add a build time check in TD_SYSINFO_MAP
> > > e.g.
> > > 
> > > #define TD_SYSINFO_CHECK_SIZE(_field_id, _size)                       \
> > >        __builtin_choose_expr(MD_FIELD_ELE_SIZE(_field_id) == _size, _size, (void)0)
> > > 
> > > #define _TD_SYSINFO_MAP(_field_id, _offset, _size)            \
> > >        { .field_id = _field_id,                                \
> > >          .offset   = _offset,                                  \
> > >          .size     = TD_SYSINFO_CHECK_SIZE(_field_id, _size) }
> > > 
> > > #define TD_SYSINFO_MAP(_field_id, _struct, _member)           \
> > >        _TD_SYSINFO_MAP(MD_FIELD_ID_##_field_id,                \
> > >                        offsetof(_struct, _member),             \
> > >                        sizeof(typeof(((_struct *)0)->_member)))
> > > 
> > > 
> > 
> > Thanks for the comment, but I don't think this meets for our purpose.
> > 
> > We want a build time "error" when the "MD_FIELD_ELE_SIZE(_field_id) == _size"
> > fails, but not "still initializing the size to 0".
> 
> FWIW, it isn't 0, it is void.  Assignment to void is an error.  Could use
> anything that is correct syntax but would produce a compile-time error
> e.g. (1 / 0).

Ah I missed the '(void)'.  I didn't thought this way (and yet to try out). 
Thanks for the insight.

I already sent out the v3 based on Dan's suggestion.  Besides the pros
mentioned by Dan, I also found Dan's suggestion yields less LoC of the final
tdx.c despite it is trivial.  So let's continue on the v3.