linux-kernel - Re: [PATCH] hwmon: (pmbus) Clear pmbus fault/warning bits before read

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: <cfc47708-26a3-8836-2eef-5b5bfb809e71@roeck-us.net>
Date:   Thu, 10 Feb 2022 11:57:39 -0800
From:   Guenter Roeck <linux@...ck-us.net>
To:     Vikash Chandola <vikash.chandola@...ux.intel.com>,
        iwona.winiarska@...el.com, jdelvare@...e.com,
        linux-hwmon@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] hwmon: (pmbus) Clear pmbus fault/warning bits before read

On 2/10/22 11:55, Guenter Roeck wrote:
> On 2/10/22 10:29, Vikash Chandola wrote:
>>
>>
>> On 2/10/2022 10:55 PM, Guenter Roeck wrote:
>>> On 2/10/22 07:57, Vikash Chandola wrote:
>>>>
>>>>
>>>> On 2/10/2022 8:14 PM, Guenter Roeck wrote:
>>>>> On 2/10/22 04:41, Vikash Chandola wrote:
>>>>>> pmbus fault and warning bits are not cleared by itself once fault/warning
>>>>>> condition is not valid anymore. As per pmbus datasheet faults must be
>>>>>> cleared by user.
>>>>>> Modify hwmon behavior to clear latched status bytes if any bit in status
>>>>>> register is high prior to returning fresh data to userspace. If
>>>>>> fault/warning conditions are still applicable fault/warning bits will be
>>>>>> set and we will get updated data in second read.
>>>>>>
>>>>>> Hwmon behavior is changed here. Now sysfs reads will reflect latest
>>>>>> values from pmbus slave, not latched values.
>>>>>> In case a transient warning/fault has happened in the past, it will no
>>>>>> longer be reported to userspace.
>>>>>>
>>>>>
>>>>>
>>>>> NACK.
>>>>>
>>>>> Reporting that information is exactly the point of the current code.
>>>>> We _do_ want to report at least once that a problem occurred in the past,
>>>>> and only clear the warning flag(s) afterwards.
>>>>>
>>>>> Guenter
>>>>>
>>>> But as per current implementation we will continue to report fault even after fault condition is cleared. I could not find sysfs entry or any other means by which user/kernel can clear the faults/warnings after it is reported. Please point if I am missing anything.
>>>>
>>>
>>> Normally a chip should clear the status after the fault condition cleared
>>> and the register was read. At least that was my experience so far.
>>> What chip (or chips) don't do that ?
>>>
>>> Guenter
>>
>> I see that pmbus spec says that once fault/warning bits are set
>> they won't be cleared by itself. Section 10.2.3 of
>> "PMBus Specification Rev. 1.3.1 Part II 2015-03-13" from
>> https://pmbus.org/specification-archives/   says
>>
>> "
>> Almost all of the warning or fault bits set in the status registers remain set, even if the fault or warning condition is removed
>> or corrected, until one of the following occur:
>> * The bit is individually cleared (Section 10.2.4),
>> * The device receives a CLEAR_FAULTS command (Section 15.1),
>> * A RESET signal (if one exists) is asserted,
>> * The output is commanded through the CONTROL pin, the OPERATION command, or the combined action of the CONTROL pin and OPERATION
>> command, to turn off and then to turn back on, or
>> * Bias power is removed from the PMBus device...
>> "
>>
>>  From this I concluded that slave(PSU) following pmbus spec will not
>> clear the fault/warning in status registers.
>> I don't have exact chip details handy but I can get it by tomorrow.
>> However this looks to be issue not related to specific chip ?
>>
> 
> Good point. Interesting that I have not seen this before.
> 
>> If this is a problem, how should I approach this ? Shall I create a new
>> sysfs entry that user space application can invoke and clear all faults
>> or provide sysfs entry to clear individual fault/warning bits or
>> something else ?
>>
> 
> No. What we need to do is to add code to pmbus_get_boolean() and selectively
> write the reported status bit back into the status register. Something like
>      ...
>      regval = status & mask;
>      if (regval) {
>          ret = pmbus_write_status(client, page, reg, regval);
>                        ^^^^^^^^^^^^^^^^^^ new function
>          if (ret)
>              goto unlock;
>      }
> 
> and that will need to be tested on a variety of real hardware.
> 

Plus, of course, we need to confirm that there is at least one chip which
does follow the specification and does not auto-clear alarms.

Guenter