linux-kernel - Re: [PATCH] x86: mce: Honour bios-set CMCI threshold

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] [day] [month] [year] [list]

Date:	Mon, 27 Aug 2012 15:24:30 +0530
From:	"Naveen N. Rao" <naveen.n.rao@...ux.vnet.ibm.com>
To:	Borislav Petkov <bp@...64.org>
CC:	tony.luck@...el.com, andi@...stfloor.org,
	gong.chen@...ux.intel.com, x86@...nel.org,
	linux-kernel@...r.kernel.org, mingo@...hat.com, tglx@...utronix.de,
	linux-edac@...r.kernel.org, ananth@...ibm.com,
	Chris McDermott <lcm@...ibm.com>, masbock@...ux.vnet.ibm.com
Subject: Re: [PATCH] x86: mce: Honour bios-set CMCI threshold

On 08/27/2012 02:42 PM, Borislav Petkov wrote:
> On Thu, Aug 23, 2012 at 05:26:09PM +0530, Naveen N. Rao wrote:
>> Sure - sounds like a good idea. Further, a #define could eliminate
>> the need to change other references, but I'm not sure that's
>> GENERALLacceptable
>>
>> #define mce_bios_cmci_threshold boot_flags.mce_bios_cmci_threshold
>>
>> could eliminate the need to change other references, but I'm not sure
>> that's acceptable
>
> Yeah, that's kinda obfuscating it for no reason. As I said before, we
> can always add it later if it makes sense.

Ouch!
This was an old draft and I'm not sure how this ended up on the list!
Sorry for all the trouble.

>
>> But, I just had a quick look and it seems to me that these were
>> defined as integers since they are exposed via sysfs. For instance:
>>
>> static struct dev_ext_attribute dev_attr_cmci_disabled = {
>>          __ATTR(cmci_disabled, 0644, device_show_int, set_cmci_disabled),
>>          &mce_cmci_disabled
>> };
>>
>> Converting mce_cmci_disabled to a bit-field doesn't work since we
>> take its address above. We could ignore and not set the second field
>> at all (dev_ext_attribute->var) and define our own callbacks, but
>> that'll be more work and I'm not sure if we work fine without
>
> Right,
>
> but take a look at set_cmci_disabled(): it converts the newly read value
> to bool anyway:
>
>          if (mce_cmci_disabled ^ !!new) {
>
> so we can do later
>
> 	flags.mce_cmci_disabled = true;
>
> or
> 	flags.mce_cmci_disabled = false;
>
> instead of assigning 0 or 1 to it.
>
> And, about showing it with device_show_int, a simple test works:
>
> ---
> #include <stdio.h>
> #include <stdbool.h>
>
> int main()
> {
>          bool a = true;
>          printf("%d\n", a);
>          return 0;
> }
> --
>
> but even if there are troubles with that, we can change device_show_int
> to a locally defined function.
>
> But, anyway, long story short: I wasn't suggesting you go and change all
> of them - simply start by adding your flag mce_bios_cmci_threshold to a
> struct <something>_flags and I'll take care of the rest.
>
> Unless you really want to do it, of course :-)

Sure, I'll send a patch for this soon.


Regards,
Naveen

>
> Oh, and the more important thing is, Tony would need to review your
> Intel-specific changes so pls keep him CCed on your next iteration too.
>
> Thanks.
>

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