linux-kernel - Re: [PATCH] applesmc: Re-work SMC comms v2

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]

Date:   Sun, 8 Nov 2020 09:22:14 +0100
From:   Henrik Rydberg <rydberg@...math.org>
To:     Brad Campbell <brad@...rfbargle.com>
Cc:     Arnd Bergmann <arnd@...db.de>, linux-hwmon@...r.kernel.org,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        hns@...delico.com, Guenter Roeck <linux@...ck-us.net>,
        Andreas Kemnade <andreas@...nade.info>,
        Jean Delvare <jdelvare@...e.com>
Subject: Re: [PATCH] applesmc: Re-work SMC comms v2

Hi Brad,

> G'day Henrik,
> 
> Which kernel was this based on? It won't apply to my 5.9 tree.

I was being lazy and applied the diff to linus/master on top of my 
current stable branch. More importantly, I sent the mail out from an 
email client that may not format the patch properly; I'll fix that.

> I assume the sprinkling of udelay(APPLESMC_MIN_WAIT) means the SMC is
> slow in getting its status register set up. Could we instead just put
> a single one of those up-front in wait_status?

That works fine, just a matter of taste.

> Any chance you could try this one? I've added a retry to send_command and
> added a single global APPLESMC_MIN_WAIT before each status read.
> 
>  From looking at your modified send_command, it appears the trigger for a
> retry is sending a command and the SMC doing absolutely nothing. This
> should do the same thing.

Not quite, unfortunately. The patch that works waits for a drop of 
IB_CLOSED, then checks the BUSY status. If not seen, it resends 
immediately, never expecting to see it. The patch in this email creates 
a dreadfully sluggish probe, and the occasional failure.

> Interestingly enough, by adding the udelay to wait_status on my machine I've
> gone from 24 reads/s to 50 reads/s.

Yep, I experience the same positive effect.

> I've left out the remainder of the cleanups. Once we get a minimally working
> patch I was going to look at a few cleanups, and I have some patches pending
> to allow writing to the SMC from userspace (for setting BCLM and BFCL mainly)

All fine. I will respond to the v3 mail separately.

Henrik