linux-kernel - Re: [RESEND][PATCH v4 1/3] i2c: piix4: Fix port selection for AMD Family 16h Model 30h

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]

Message-ID: <8b8638e2-18a2-5865-4e7a-27092b053b1b@opengear.com>
Date:   Tue, 27 Feb 2018 07:21:37 +1000
From:   Andrew Cooks <andrew.cooks@...ngear.com>
To:     "Tobin C. Harding" <me@...in.cc>
Cc:     Jean Delvare <jdelvare@...e.com>, Wolfram Sang <wsa@...-dreams.de>,
        "open list:I2C/SMBUS CONTROLLER DRIVERS FOR PC" 
        <linux-i2c@...r.kernel.org>,
        open list <linux-kernel@...r.kernel.org>,
        Andrew Cooks <acooks@...ionali.st>, linux-acpi@...r.kernel.org,
        platypus-sw@...ngear.com
Subject: Re: [RESEND][PATCH v4 1/3] i2c: piix4: Fix port selection for AMD
 Family 16h Model 30h

Hi Tobin

On 26/02/18 18:58, Tobin C. Harding wrote:
> On Mon, Feb 26, 2018 at 10:28:43AM +1000, Andrew Cooks wrote:
>> Family 16h Model 30h SMBus controller needs the same port selection fix
>> as described and fixed in commit 0fe16195f891 ("i2c: piix4: Fix SMBus port
>> selection for AMD Family 17h chips")
>>
>> commit 6befa3fde65f ("i2c: piix4: Support alternative port selection
>> register") also fixed the port selection for Hudson2, but unfortunately
>> this is not the exact same device and the AMD naming and PCI Device IDs
>> aren't particularly helpful here.
>>
>> The SMBus port selection register is common to the following Families
>> and models, as documented in AMD's publicly available BIOS and Kernel
>> Developer Guides:
>>
>>  50742 - Family 15h Model 60h-6Fh (PCI_DEVICE_ID_AMD_KERNCZ_SMBUS)
>>  55072 - Family 15h Model 70h-7Fh (PCI_DEVICE_ID_AMD_KERNCZ_SMBUS)
>>  52740 - Family 16h Model 30h-3Fh (PCI_DEVICE_ID_AMD_HUDSON2_SMBUS)
>>
>> The Hudson2 PCI Device ID (PCI_DEVICE_ID_AMD_HUDSON2_SMBUS) is shared
>> between Bolton FCH and Family 16h Model 30h, but the location of the
>> SmBus0Sel port selection bits are different:
>>
>>  51192 - Bolton Register Reference Guide
>>
>> We distinguish between Bolton and Family 16h Model 30h using the PCI
>> Revision ID:
>>
>>   Bolton is device 0x780b, revision 0x15
>>   Family 16h Model 30h is device 0x780b, revision 0x1F
>>   Family 15h Model 60h and 70h are both device 0x790b, revision 0x4A.
>>
>> The following additional public AMD BKDG documents were checked and do
>> not share the same port selection register:
>>
>>  42301 - Family 15h Model 00h-0Fh doesn't mention any
>>  42300 - Family 15h Model 10h-1Fh doesn't mention any
>>  49125 - Family 15h Model 30h-3Fh doesn't mention any
>>
>>  48751 - Family 16h Model 00h-0Fh uses the previously supported
>>          index register SB800_PIIX4_PORT_IDX_ALT at 0x2e
>>
>> Signed-off-by: Andrew Cooks <andrew.cooks@...ngear.com>
>> ---
>>  drivers/i2c/busses/i2c-piix4.c | 12 +++++-------
>>  1 file changed, 5 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c
>> index 174579d..5c90a44 100644
>> --- a/drivers/i2c/busses/i2c-piix4.c
>> +++ b/drivers/i2c/busses/i2c-piix4.c
>> @@ -99,7 +99,7 @@
>>  #define SB800_PIIX4_PORT_IDX_MASK	0x06
>>  #define SB800_PIIX4_PORT_IDX_SHIFT	1
>>  
>> -/* On kerncz, SmBus0Sel is at bit 20:19 of PMx00 DecodeEn */
>> +/* On kerncz and Hudson2, SmBus0Sel is at bit 20:19 of PMx00 DecodeEn */
>>  #define SB800_PIIX4_PORT_IDX_KERNCZ		0x02
>>  #define SB800_PIIX4_PORT_IDX_MASK_KERNCZ	0x18
>>  #define SB800_PIIX4_PORT_IDX_SHIFT_KERNCZ	3
>> @@ -359,18 +359,16 @@ static int piix4_setup_sb800(struct pci_dev *PIIX4_dev,
>>  
>>  	/* Find which register is used for port selection */
>>  	if (PIIX4_dev->vendor == PCI_VENDOR_ID_AMD) {
>> -		switch (PIIX4_dev->device) {
>> -		case PCI_DEVICE_ID_AMD_KERNCZ_SMBUS:
>> +		if ((PIIX4_dev->device == PCI_DEVICE_ID_AMD_KERNCZ_SMBUS) ||
> 
> nit:		if (PIIX4_dev->device == PCI_DEVICE_ID_AMD_KERNCZ_SMBUS ||
> 
>> +		    (PIIX4_dev->device == PCI_DEVICE_ID_AMD_HUDSON2_SMBUS &&
>> +			   PIIX4_dev->revision >= 0x1F)) {
> 
> 
> Hope this helps,

It does, thanks. I'll roll this fix in with whatever other feedback I get before sending the next patch version.

a.