linux-kernel - Re: [PATCH v4 0/9] i2c: piix4: Replace cd6h/cd7h port I/O accesses with MMIO accesses

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: <27e60021-30cb-3b1c-f429-2618bf891e5e@amd.com>
Date:   Tue, 8 Feb 2022 17:03:09 -0600
From:   Terry Bowman <Terry.Bowman@....com>
To:     Jean Delvare <jdelvare@...e.de>
Cc:     linux@...ck-us.net, linux-watchdog@...r.kernel.org,
        linux-i2c@...r.kernel.org, wsa@...nel.org,
        andy.shevchenko@...il.com, rafael.j.wysocki@...el.com,
        linux-kernel@...r.kernel.org, wim@...ux-watchdog.org,
        rrichter@....com, thomas.lendacky@....com, sudheesh.mavila@....com,
        Nehal-bakulchandra.Shah@....com, Basavaraj.Natikar@....com,
        Shyam-sundar.S-k@....com, Mario.Limonciello@....com
Subject: Re: [PATCH v4 0/9] i2c: piix4: Replace cd6h/cd7h port I/O accesses
 with MMIO accesses

Hi Jean,

On 2/8/22 15:46, Jean Delvare wrote:
> On Tue, 8 Feb 2022 13:36:55 -0600, Terry Bowman wrote:
>> On 2/8/22 11:11, Jean Delvare wrote:
>>> Unfortunately I'm not really able to test this series. While I do have
>>> an AMD-based laptop which uses the i2c-piix4 driver, the SMBus has
>>> never been usable on it. The driver creates 3 i2c buses (port 0 at
>>> 0b00, port 2 at 0b00 and port 1 at 0b20). The first 2 do not seem to
>>> have any device on them (i2cdetect returns empty). The last one must
>>> have some devices on it, I see address 0x1c answers the ping,
>>> unfortunately as soon as probing reaches address 0x2c, all later pings
>>> return success, regardless of the address. It seems that some I2C
>>> device (probably the one at 0x2c, but I don't know what it is) is
>>> holding SDA low forever, therefore preventing any use of the whole
>>> SMBus port until the next reboot.
>>
>> My understanding is the OEM may have different i2c devices because it
>> is mainboard specific.
> 
> Yes, the devices on the SMBus could be anything Lenovo decided to use.
> Tomorrow I'll try to scan the SMBus but skipping the problematic
> address, to see if it works around the problem.
> 
>>>> (...)
>>>> Changes in v4:
>>>> (...)
>>>>  - Removed iowrite32(ioread32(...), ...). Unnecessary because MMIO is
>>>>    already enabled. (Terry Bowman)  
>>>
>>> I'm curious, how can you be sure of that actually?
>>
>> The removed code was using a MMIO region to write 1 to 'mmioen'. This was 
>> using the MMIO region to enable same MMIO region.
> 
> Ah ah, I get it now. Nice chicken or the egg situation :-)
> 
>> Programmer's manual shows FCH::PM::ISACONTROL[mmioen] has a '1' reset value.
>> Per programmer's manual, FCH::PM::ISACONTROL[mmioen] enables MMIO mapping 
>> at FED8_0000h..FED8_1FFFh. The FCH::PM::ISACONTROL register is MMIO 
>> mapped at FED8_0304h. 'mmioen' was intended to be set using port I/O to 
>> enable MMIO but, port I/O access to these registers is now disabled.
> 
> Is my understanding correct that there is some overlapping of the
> access methods and there are certain chipsets where both legacy I/O and
> MMIO access is possible?
> 
Yes. 

> If so, while there's indeed nothing to be done for the most recent
> systems where only MMIO access is possible, you may still need to
> enable MMIO access through legacy I/O if you try to use MMIO on
> chipsets where both are possible. I'm not sure what exactly where you
> set the limit. In the last patch you say that 0x51 is the first
> revision of the family 17h CPUs, but is family 17h the first where MMIO
> is available, or the first where legacy I/O isn't?
> 
Family 17h, SMBus PCI ID >= 0x51 is the first where cd6h/cd7h port I/O is disabled. 
If SMBus PCI ID < 0x51 then cd6h/cd7h port I/O is used. 
  
Regards,
Terry