On Mon, Apr 9, 2018 at 12:48 PM, Jason Andryuk <jandryuk@xxxxxxxxx> wrote:
> On Mon, Apr 9, 2018 at 12:20 PM, Jean Delvare <jdelvare@xxxxxxx> wrote:
>> On Mon, 9 Apr 2018 09:41:49 -0400, Jason Andryuk wrote:
>>> On Sat, Apr 7, 2018 at 4:32 AM, Jean Delvare <jdelvare@xxxxxxx> wrote:
>>> > On Fri, 6 Apr 2018 11:18:02 -0400, Jason Andryuk wrote:
>>> >> [root@localhost ~]# diff lspci.during lspci.after
>>> >> [root@localhost ~]# #No difference
>>> >>
>>> >> The full session of commands is below. I haven't analyzed any of
>>> >> these values since I am unfamiliar with them.
>>> >>
>>> >> I hope this helps.
>>> >
>>> > Not really, I'm afraid. I hopped for something more obviously
>>> > related to what you observe.
>>> >
>>> > For clarity, were the tests above done with or without
>>> > disable_features=0x20? Does it make any difference if you change?
>>>
>>> The previous data was from running without disable_features, so Host
>>> Notify was active. Below is the diff of data from a run with
>>> disable_features=0x20.
>>>
>>> The isadump changed part of 0xefa2-0xefa4 and 0xefb0-0xefbf to 0s.
>>>
>>> [root@localhost ~]# diff isadump.before isadump.during
>>> 2,3c2,3
>>> < efa0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>>> < efb0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>>> ---
>>> > efa0: 00 00 00 00 00 00 00 00 00 44 00 00 1c 00 07 07
>>> > efb0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>
>> 0xefa2 is Host Control, 0xefa3 is Host Command, 0xefa4 is Transmit
>> Slave Address. In the previous dump we could see a read from offset
>> 0x75 of the SPD EEPROM at I2C address 0x53, which is the kind of thing
>> the BIOS would do to configure the memory controller timings. It's not
>> related to Host Notify. I wonder if this difference is reproducible. It
>> could depend on something else, like cold boot vs. reboot, or entering
>> the BIOS setup screen.
>>
>> 0xefb1 is Slave Command, 0xefb4 is Notify Device Address. Both are
>> directly related to the Host Notify feature, so it is expected that
>> they are no longer set with disable_features=0x20.
>>
>> Note that the Notify Device Address was set to I2C device at address
>> 0x2C in the previous dump. So most likely, you DO have a device
>> triggering Host Notify interrupts on your laptop.
>>
>> What is pretty interesting in your previous post, and I failed to
>> notice before, is that bit 0 of register 0xefb1 was still set after the
>> module was unloaded. The original register value is supposed to be
>> restored in i801_disable_host_notify(), and the dump you just provided
>> indicates that Host Notify is not enabled by the BIOS on your machine,
>> so the value of this bit should have been saved, and then restored, to
>> 0. This is not happening.
>>
>> I think it would be worth instrumenting the i2c-i801 driver to check
>> what exactly is going on. Something like:
>>
>> ---
>> drivers/i2c/busses/i2c-i801.c | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>>
>> --- linux-4.16.orig/drivers/i2c/busses/i2c-i801.c 2018-04-09 14:54:05.179509697 +0200
>> +++ linux-4.16/drivers/i2c/busses/i2c-i801.c 2018-04-09 18:09:28.843867439 +0200
>> @@ -967,6 +967,8 @@ static void i801_enable_host_notify(stru
>> return;
>>
>> priv->original_slvcmd = inb_p(SMBSLVCMD(priv));
>> + dev_info(&adapter->dev, "Original SMBSLVCMD value: %02X\n",
>> + (unsigned int)priv->original_slvcmd);
>>
>> if (!(SMBSLVCMD_HST_NTFY_INTREN & priv->original_slvcmd))
>> outb_p(SMBSLVCMD_HST_NTFY_INTREN | priv->original_slvcmd,
>> @@ -974,6 +976,9 @@ static void i801_enable_host_notify(stru
>>
>> /* clear Host Notify bit to allow a new notification */
>> outb_p(SMBSLVSTS_HST_NTFY_STS, SMBSLVSTS(priv));
>> +
>> + dev_info(&adapter->dev, "SMBSLVCMD value after initialization: %02X\n",
>> + (unsigned int)inb_p(SMBSLVCMD(priv)));
>> }
>>
>> static void i801_disable_host_notify(struct i801_priv *priv)
>> @@ -981,7 +986,11 @@ static void i801_disable_host_notify(str
>> if (!(priv->features & FEATURE_HOST_NOTIFY))
>> return;
>>
>> + dev_info(&priv->adapter.dev, "SMBSLVCMD value at removal time: %02X\n",
>> + (unsigned int)inb_p(SMBSLVCMD(priv)));
>> outb_p(priv->original_slvcmd, SMBSLVCMD(priv));
>> + dev_info(&priv->adapter.dev, "SMBSLVCMD reset to: %02X\n",
>> + (unsigned int)inb_p(SMBSLVCMD(priv)));
>> }
>>
>> static const struct i2c_algorithm smbus_algorithm = {
>>
>> Could you try this with the Host Notify feature enabled, and report the
>> values that are logged?
>
> It'll take me a little time to get to this. I've been running with
> the blacklist patch and doing the dumping from a fedora live image.
>
>>> [root@localhost ~]# diff isadump.during isadump.after
>>> 2c2
>>> < efa0: 00 00 00 00 00 00 00 00 00 44 00 00 1c 00 07 07
>>> ---
>>> > efa0: 40 00 00 00 00 00 00 00 00 44 00 00 1c 00 07 07
>>>
>>> The PCI config space not displayed in the diff output is identical to
>>> the Host Notify run.
>>>
>>> [root@localhost ~]# diff lspci.before lspci.during
>>> 2c2
>>> < 00: 86 80 23 9d 00 00 80 02 21 00 05 0c 00 00 00 00
>>> ---
>>> > 00: 86 80 23 9d 03 00 80 02 21 00 05 0c 00 00 00 00
>>
>> Interrupt Status bit isn't set here. It was set in the Host Notify
>> enabled case. That's interesting.
>>
>>> [root@localhost ~]# diff lspci.during lspci.after
>>> [root@localhost ~]# # No Difference
>
> I made some progress by zeroing out ioport values. I booted up with
> Host Notify active and then removed i2c-i801.ko
>
> isadump looked like:
> efa0: 40 00 08 75 a7 00 00 00 00 44 00 00 1c 00 07 07
> efb0: 00 01 00 00 58 00 00 00 00 00 00 00 00 00 00 00
>
> Then I used `isaset` to zero out fields to:
> efa0: 40 00 00 00 00 00 00 00 00 44 00 00 1c 00 07 07
> efb0: 00 00 00 00 58 00 00 00 00 00 00 00 00 00 00 00
>
> Power off. Bug not observed - no hang on subsequent boot!
>
> Note, 0xefb4 would not let me write 0. Something about data mismatch
> and 0x58 was read back.
Just writing 0 to 0xefb1 is sufficient to avoid the boot hang bug.
Regards,
Jason
