linux-kernel - Re: [PATCH 4/5] platform/x86/intel/ifs: Implement Array BIST test

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:   Wed, 1 Feb 2023 11:53:46 -0800
From:   Dave Hansen <dave.hansen@...el.com>
To:     "Luck, Tony" <tony.luck@...el.com>,
        "Joseph, Jithu" <jithu.joseph@...el.com>,
        "hdegoede@...hat.com" <hdegoede@...hat.com>,
        "markgross@...nel.org" <markgross@...nel.org>
Cc:     "tglx@...utronix.de" <tglx@...utronix.de>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "bp@...en8.de" <bp@...en8.de>,
        "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
        "x86@...nel.org" <x86@...nel.org>, "hpa@...or.com" <hpa@...or.com>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "rostedt@...dmis.org" <rostedt@...dmis.org>,
        "Raj, Ashok" <ashok.raj@...el.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "platform-driver-x86@...r.kernel.org" 
        <platform-driver-x86@...r.kernel.org>,
        "patches@...ts.linux.dev" <patches@...ts.linux.dev>,
        "Shankar, Ravi V" <ravi.v.shankar@...el.com>,
        "Macieira, Thiago" <thiago.macieira@...el.com>,
        "Jimenez Gonzalez, Athenas" <athenas.jimenez.gonzalez@...el.com>,
        "Mehta, Sohil" <sohil.mehta@...el.com>
Subject: Re: [PATCH 4/5] platform/x86/intel/ifs: Implement Array BIST test

On 2/1/23 11:43, Luck, Tony wrote:
>> Why bother with a bitfield?  Just do:
> How much "bother" is a bitfield?
> 
>> union ifs_array {
>>       u64     data;
>>       struct {
>>               u32     array_bitmask;
>>               u16     array_bank;
>>               u16     flags;
>>       };
>> };
>>
>> Then you only need to mask 'ctrl_result' out of flags.  You don't need
>> any crazy macros.
> "only need" to special case this one field ... but that's extra
> code for humans to read (and humans aren't good at that)
> rather than the computer (compiler) which is really good at
> doing this.

I don't follow.

If you have:

	struct foo {
		u16	rsvd			:15;
		u16	ctrl_result		:1;
	};

versus:

	struct bar {
		u16	flags;
	};

and you do:

	if (foo->ctrl_result)
		something();

versus:

	if (bar->flags & CTRL_RESULT_MASK)
		something();

I think both of those are quite readable.  I'd argue that humans will be
less _surprised_ by 'bar'.  I also like to write portable code even if
it's going to be x86 only.  It helps people who are used to reading
portable generic code read and find bugs in x86 only code.