linux-kernel - Re: [PATCH V4] edac, amd64_edac: Modify usage of amd64_read_dct_pci

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 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, 15 Sep 2014 10:19:58 -0500
From:	Aravind Gopalakrishnan <aravind.gopalakrishnan@....com>
To:	Borislav Petkov <bp@...en8.de>
CC:	<dougthompson@...ssion.com>, <m.chehab@...sung.com>,
	<linux-edac@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH V4] edac, amd64_edac: Modify usage of amd64_read_dct_pci_cfg()

On 9/15/2014 10:08 AM, Borislav Petkov wrote:
>>>>   	amd64_read_pci_cfg(pvt->F3, F10_ONLINE_SPARE, &pvt->online_spare);
>>>> -	amd64_read_dct_pci_cfg(pvt, DCLR0, &pvt->dclr0);
>>>> -	amd64_read_dct_pci_cfg(pvt, DCHR0, &pvt->dchr0);
>>>> -
>>>> -	if (!dct_ganging_enabled(pvt)) {
>>>> -		amd64_read_dct_pci_cfg(pvt, DCLR1, &pvt->dclr1);
>>>> -		amd64_read_dct_pci_cfg(pvt, DCHR1, &pvt->dchr1);
>>>> -	}
>>>> +	amd64_read_dct_pci_cfg(pvt, 0, DCLR0, &pvt->dclr0);
>>>> +	amd64_read_dct_pci_cfg(pvt, 0, DCHR0, &pvt->dchr0);
>>>>   	pvt->ecc_sym_sz = 4;
>>>>   	if (pvt->fam >= 0x10) {
>>>> +		amd64_read_dct_pci_cfg(pvt, 1, DCLR0, &pvt->dclr1);
>>>> +		amd64_read_dct_pci_cfg(pvt, 1, DCHR0, &pvt->dchr1);
>>> This doesn't look equivalent - above we're checking whether we're ganged
>>> now you're doing it for >= F10h. Why?
>>>
>>> Because only F10h supports ganging?
>> That's right.
>> If ganging is enabled (which is a condition we check for in
>> amd64_read_dct_pci_cfg();
>> Then we return 0.
> Right, but you're reading them now even if you don't have to. So why are
> you even changing this? What's wrong with doing:
>
> 	if (!dct_ganging_enabled(pvt)) {
> 		amd64_read_dct_pci_cfg(pvt, 1, DCLR0, &pvt->dclr1);
> 		amd64_read_dct_pci_cfg(pvt, 1, DCHR0, &pvt->dchr1);
> 	}
>
> which is much clearer than doing the family check.
>

Just thought we could remove one more condition..
Allright, I'll change this.

-Aravind.
--
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/