linux-kernel - Re: [REGRESSION] hwmon: (applesmc) avoid overlong udelay()

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: <20201104142057.62493c12@aktux>
Date:   Wed, 4 Nov 2020 14:20:57 +0100
From:   Andreas Kemnade <andreas@...nade.info>
To:     Brad Campbell <brad@...rfbargle.com>
Cc:     Guenter Roeck <linux@...ck-us.net>, Arnd Bergmann <arnd@...db.de>,
        rydberg@...math.org, Jean Delvare <jdelvare@...e.com>,
        linux-hwmon@...r.kernel.org,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        hns@...delico.com
Subject: Re: [REGRESSION] hwmon: (applesmc) avoid overlong udelay()

On Tue, 3 Nov 2020 16:56:32 +1100
Brad Campbell <brad@...rfbargle.com> wrote:

> On 3/11/20 10:56 am, Brad Campbell wrote:
> 
> > 
> > I've examined the code in VirtualSMC and I'm not convinced we were not waiting on the wrong bits.
> > 
> > #define SMC_STATUS_AWAITING_DATA  BIT0  ///< Ready to read data.
> > #define SMC_STATUS_IB_CLOSED      BIT1  /// A write is pending.
> > #define SMC_STATUS_BUSY           BIT2  ///< Busy processing a command.
> > #define SMC_STATUS_GOT_COMMAND    BIT3  ///< The last input was a command.
> > #define SMC_STATUS_UKN_0x16       BIT4
> > #define SMC_STATUS_KEY_DONE       BIT5
> > #define SMC_STATUS_READY          BIT6  // Ready to work
> > #define SMC_STATUS_UKN_0x80       BIT7  // error
> > 
> > Any chance you could try this patch? It's ugly, hacked together and currently fairly undocumented, but if it works I'll figure out how to clean it up (suggestions welcome).
> > It works on my MacbookPro 11,1.  
> 
> I had some time so I spent a bit of time refactoring and trying to clarify the magic numbers.
> 
> I also did some fuzzing of the SMC and figured out where we can loosen the masks.
> This has some debug code in it to identify if any wait loops exceed 1 loop and if the SMC is reporting anything other than a clear "I'm waiting" prior to each command.
> 
> You might see some of these :
> [   51.316202] applesmc: wait_status looping 2: 0x44, 0x40, 0x4e
> [   60.002547] applesmc: wait_status looping 2: 0x44, 0x40, 0x4e
> [   60.130754] applesmc: wait_status looping 2: 0x44, 0x40, 0x4e
> 
> I did some heavy tests and found that with the delays at the bottom of the loop about 50% of calls required no delay at all before a read or write and the other 50% require a single delay.
> I can count on one hand the number of times it's exceeded 1 loop, and the max thus far has been 5 loops.
> 
That matches my experience. The only delay is at the end of the
command. Critcal seems to be that there is not too much delay in between.

[...]
> If anyone with a Mac having a conventional SMC and seeing issues on 5.9 could test this it'd be appreciated. I'm not saying this code is "correct", but it "works for me".
> 
Seems to work here.
 dmesg  | grep applesmc

[    1.350782] applesmc: key=561 fan=1 temp=33 index=33 acc=0 lux=2 kbd=1
[    1.350922] applesmc applesmc.768: hwmon_device_register() is deprecated. Please convert the driver to use hwmon_device_register_with_info().
[   17.748504] applesmc: wait_status looping 2: 0x4a, 0x4c, 0x4f
[  212.008952] applesmc: wait_status looping 2: 0x44, 0x40, 0x4e
[  213.033930] applesmc: wait_status looping 2: 0x44, 0x40, 0x4e
[  213.167908] applesmc: wait_status looping 2: 0x44, 0x40, 0x4e
[  219.087854] applesmc: wait_status looping 2: 0x44, 0x40, 0x4e

Tested it on top of 5.9

Regards,
Andreas