linux-kernel - Re: [PATCH 4/4] x86/mce/AMD: Add comments for easier understanding

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 for Android: free password hash cracker in your pocket

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

Message-ID: <56D0A2A4.6030406@amd.com>
Date:	Fri, 26 Feb 2016 13:08:20 -0600
From:	Aravind Gopalakrishnan <aravind.gopalakrishnan@....com>
To:	Borislav Petkov <bp@...en8.de>
CC:	<tony.luck@...el.com>, <hpa@...or.com>, <mingo@...hat.com>,
	<tglx@...utronix.de>, <dougthompson@...ssion.com>,
	<mchehab@....samsung.com>, <x86@...nel.org>,
	<linux-edac@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<ashok.raj@...el.com>, <gong.chen@...ux.intel.com>,
	<len.brown@...el.com>, <peterz@...radead.org>,
	<ak@...ux.intel.com>, <alexander.shishkin@...ux.intel.com>
Subject: Re: [PATCH 4/4] x86/mce/AMD: Add comments for easier understanding

On 2/26/2016 11:44 AM, Borislav Petkov wrote:
>
> threshold_restart_bank() reprograms the MISC MSR after sanity-checking
> the fields supplied for that MSR. store_threshold_limit() sets the error
> count, store_interrupt_enable() enables/disables the interrupt and both
> call threshold_restart_bank() to do that.
>
> But this is basically spelling the code now - I don't think we need to
> comment in that detail.

Ok, Have dropped this for V2.

> /*
>   * Called via smp_call_function_single(), must be called with correct
>   * cpu affinity.
>   */
>
> is also useless.

Will remove these as well.

>> "This function provides user with capabilities to re-program the
>> 'thresold_limit' and 'interrupt_enable' sysfs attributes"
> No sorry, I don't want to be explaining every line. Just say: "Reprogram
> the MISC MSR behind this threshold bank."
>

Ok, Will do that.

Btw, included comments around struct threshold_block to describethe members.
Do let me know if this seems OK-

  struct threshold_block {
-       unsigned int            block;
-       unsigned int            bank;
-       unsigned int            cpu;
-       u32                     address;
-       u16                     interrupt_enable;
-       bool                    interrupt_capable;
-       u16                     threshold_limit;
-       struct kobject          kobj;
-       struct list_head        miscj;
+       unsigned int            block;                  /* Threshold 
block number within bank */
+       unsigned int            bank;                   /* MCA bank the 
block belongs to */
+       unsigned int            cpu;                    /* CPU which 
controls the MCA bank */
+       u32                     address;                /* MSR address 
for the block */
+       u16                     interrupt_enable;       /* Enable/ 
Disable APIC interrupt upon threshold error */
+       bool                    interrupt_capable;      /* Specifies if 
interrupt is possible from the block */
+       u16                     threshold_limit;        /* Value upon 
which threshold interrupt is generated */
+       struct kobject          kobj;                   /* sysfs object */
+       struct list_head        miscj;                  /* Add multiple 
threshold blocks within a bank to the list */
  };

Thanks,
-Aravind.