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]
Date: Fri, 3 May 2024 12:52:07 +1200
From: "Huang, Kai" <kai.huang@...el.com>
To: "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>, "kvm@...r.kernel.org"
	<kvm@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>
CC: "Hansen, Dave" <dave.hansen@...el.com>, "seanjc@...gle.com"
	<seanjc@...gle.com>, "bp@...en8.de" <bp@...en8.de>, "x86@...nel.org"
	<x86@...nel.org>, "peterz@...radead.org" <peterz@...radead.org>,
	"hpa@...or.com" <hpa@...or.com>, "mingo@...hat.com" <mingo@...hat.com>,
	"kirill.shutemov@...ux.intel.com" <kirill.shutemov@...ux.intel.com>,
	"tglx@...utronix.de" <tglx@...utronix.de>, "pbonzini@...hat.com"
	<pbonzini@...hat.com>, "jgross@...e.com" <jgross@...e.com>, "Yamahata, Isaku"
	<isaku.yamahata@...el.com>
Subject: Re: [PATCH 3/5] x86/virt/tdx: Unbind global metadata read with
 'struct tdx_tdmr_sysinfo'



On 3/05/2024 12:12 pm, Edgecombe, Rick P wrote:
> On Sat, 2024-03-02 at 00:20 +1300, Kai Huang wrote:
>> +#define TD_SYSINFO_MAP_TDMR_INFO(_field_id, _member)   \
>> +       TD_SYSINFO_MAP(_field_id, struct tdx_tdmr_sysinfo, _member)
>>   
>>   static int get_tdx_tdmr_sysinfo(struct tdx_tdmr_sysinfo *tdmr_sysinfo)
>>   {
>>          /* Map TD_SYSINFO fields into 'struct tdx_tdmr_sysinfo': */
>>          const struct field_mapping fields[] = {
>> -               TD_SYSINFO_MAP(MAX_TDMRS,             max_tdmrs),
>> -               TD_SYSINFO_MAP(MAX_RESERVED_PER_TDMR, max_reserved_per_tdmr),
>> -               TD_SYSINFO_MAP(PAMT_4K_ENTRY_SIZE,
>> pamt_entry_size[TDX_PS_4K]),
>> -               TD_SYSINFO_MAP(PAMT_2M_ENTRY_SIZE,
>> pamt_entry_size[TDX_PS_2M]),
>> -               TD_SYSINFO_MAP(PAMT_1G_ENTRY_SIZE,
>> pamt_entry_size[TDX_PS_1G]),
>> +               TD_SYSINFO_MAP_TDMR_INFO(MAX_TDMRS,             max_tdmrs),
>> +               TD_SYSINFO_MAP_TDMR_INFO(MAX_RESERVED_PER_TDMR,
>> max_reserved_per_tdmr),
>> +               TD_SYSINFO_MAP_TDMR_INFO(PAMT_4K_ENTRY_SIZE,
>> pamt_entry_size[TDX_PS_4K]),
>> +               TD_SYSINFO_MAP_TDMR_INFO(PAMT_2M_ENTRY_SIZE,
>> pamt_entry_size[TDX_PS_2M]),
>> +               TD_SYSINFO_MAP_TDMR_INFO(PAMT_1G_ENTRY_SIZE,
>> pamt_entry_size[TDX_PS_1G]),
> 
> The creation of TD_SYSINFO_MAP_TDMR_INFO part is not strictly needed, but makes
> sense in the context of the signature change in read_sys_metadata_field16(). It
> might be worth justifying it in the log.


I see your point.  How about adding below paragraph to the end of this 
changelog?

"
The metadata reading code uses the TD_SYSINFO_MAP() macro to describe 
the mapping between the metadata fields and the members of the 'struct 
tdx_tdmr_sysinfo'.  I.e., it hard-codes the 'struct tdx_tdmr_sysinfo' 
inside the macro.

As part of unbinding metadata read with 'struct tdx_tdmr_sysinfo', the 
TD_SYSINFO_MAP() macro needs to be changed to additionally take the 
structure as argument so it can accept any structure.  That would make 
the current code to read TDMR related metadata fields longer if using 
TD_SYSINFO_MAP() directly.

Define a wrapper macro for reading TDMR related metadata fields to make 
the code shorter.
"

By typing, it reminds me that I kinda need to learn how to separate the 
"high level design" vs "low level implementation details".  I think the 
latter can be seen easily in the code, and probably can be avoided in 
the changelog.

I am not sure whether adding the TD_SYSINFO_MAP_TDMR_INFO() macro belong 
to which category, especially when I needed a lot text to justify this 
change (thus I wonder whether it is worth to do).

Or any shorter version that you can suggest?

Thanks.
	

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ