From: Guenter Roeck <linux@roeck-us.net>
To: Armin Wolf <W_Armin@gmx.de>
Cc: pali@kernel.org, jdelvare@suse.com, David.Laight@ACULAB.COM,
linux-hwmon@vger.kernel.org, linux-assembly@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4] hwmon: (dell-smm) Improve assembly code
Date: Tue, 22 Feb 2022 08:54:32 -0800 [thread overview]
Message-ID: <20220222165432.GA255373@roeck-us.net> (raw)
In-Reply-To: <20220220190851.17965-1-W_Armin@gmx.de>
On Sun, Feb 20, 2022 at 08:08:51PM +0100, Armin Wolf wrote:
> The new assembly code works on both 32 bit and 64 bit
> cpus and allows for more compiler optimisations.
> Since clang runs out of registers on 32 bit x86 when
> using CC_OUT, we need to execute "setc" ourself.
> Also modify the debug message so we can still see
> the result (eax) when the carry flag was set.
>
> Tested with 32 bit and 64 bit kernels on a Dell Inspiron 3505.
>
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
It would be great if I can get some Tested-by/Acked-by/Reviewed-by
tags for this patch.
Thanks,
Guenter
> ---
> Changes in v4:
> - reword commit message
>
> Changes in v3:
> - make carry an unsigned char
> - use "+a", ... for output registers
> - drop "cc" from clobbered list
>
> Changes in v2:
> - fix clang running out of registers on 32 bit x86
> - modify debug message
> ---
> drivers/hwmon/dell-smm-hwmon.c | 78 ++++++++--------------------------
> 1 file changed, 18 insertions(+), 60 deletions(-)
>
> --
> 2.30.2
>
> diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
> index c5939e68586d..38d23a8e83f2 100644
> --- a/drivers/hwmon/dell-smm-hwmon.c
> +++ b/drivers/hwmon/dell-smm-hwmon.c
> @@ -119,7 +119,7 @@ struct smm_regs {
> unsigned int edx;
> unsigned int esi;
> unsigned int edi;
> -} __packed;
> +};
>
> static const char * const temp_labels[] = {
> "CPU",
> @@ -164,74 +164,32 @@ static int i8k_smm_func(void *par)
> struct smm_regs *regs = par;
> int eax = regs->eax;
> int ebx = regs->ebx;
> + unsigned char carry;
> long long duration;
> - int rc;
>
> /* SMM requires CPU 0 */
> if (smp_processor_id() != 0)
> return -EBUSY;
>
> -#if defined(CONFIG_X86_64)
> - asm volatile("pushq %%rax\n\t"
> - "movl 0(%%rax),%%edx\n\t"
> - "pushq %%rdx\n\t"
> - "movl 4(%%rax),%%ebx\n\t"
> - "movl 8(%%rax),%%ecx\n\t"
> - "movl 12(%%rax),%%edx\n\t"
> - "movl 16(%%rax),%%esi\n\t"
> - "movl 20(%%rax),%%edi\n\t"
> - "popq %%rax\n\t"
> - "out %%al,$0xb2\n\t"
> - "out %%al,$0x84\n\t"
> - "xchgq %%rax,(%%rsp)\n\t"
> - "movl %%ebx,4(%%rax)\n\t"
> - "movl %%ecx,8(%%rax)\n\t"
> - "movl %%edx,12(%%rax)\n\t"
> - "movl %%esi,16(%%rax)\n\t"
> - "movl %%edi,20(%%rax)\n\t"
> - "popq %%rdx\n\t"
> - "movl %%edx,0(%%rax)\n\t"
> - "pushfq\n\t"
> - "popq %%rax\n\t"
> - "andl $1,%%eax\n"
> - : "=a"(rc)
> - : "a"(regs)
> - : "%ebx", "%ecx", "%edx", "%esi", "%edi", "memory");
> -#else
> - asm volatile("pushl %%eax\n\t"
> - "movl 0(%%eax),%%edx\n\t"
> - "push %%edx\n\t"
> - "movl 4(%%eax),%%ebx\n\t"
> - "movl 8(%%eax),%%ecx\n\t"
> - "movl 12(%%eax),%%edx\n\t"
> - "movl 16(%%eax),%%esi\n\t"
> - "movl 20(%%eax),%%edi\n\t"
> - "popl %%eax\n\t"
> - "out %%al,$0xb2\n\t"
> - "out %%al,$0x84\n\t"
> - "xchgl %%eax,(%%esp)\n\t"
> - "movl %%ebx,4(%%eax)\n\t"
> - "movl %%ecx,8(%%eax)\n\t"
> - "movl %%edx,12(%%eax)\n\t"
> - "movl %%esi,16(%%eax)\n\t"
> - "movl %%edi,20(%%eax)\n\t"
> - "popl %%edx\n\t"
> - "movl %%edx,0(%%eax)\n\t"
> - "lahf\n\t"
> - "shrl $8,%%eax\n\t"
> - "andl $1,%%eax\n"
> - : "=a"(rc)
> - : "a"(regs)
> - : "%ebx", "%ecx", "%edx", "%esi", "%edi", "memory");
> -#endif
> - if (rc != 0 || (regs->eax & 0xffff) == 0xffff || regs->eax == eax)
> - rc = -EINVAL;
> + asm volatile("out %%al,$0xb2\n\t"
> + "out %%al,$0x84\n\t"
> + "setc %0\n"
> + : "=mr" (carry),
> + "+a" (regs->eax),
> + "+b" (regs->ebx),
> + "+c" (regs->ecx),
> + "+d" (regs->edx),
> + "+S" (regs->esi),
> + "+D" (regs->edi));
>
> duration = ktime_us_delta(ktime_get(), calltime);
> - pr_debug("smm(0x%.4x 0x%.4x) = 0x%.4x (took %7lld usecs)\n", eax, ebx,
> - (rc ? 0xffff : regs->eax & 0xffff), duration);
> + pr_debug("smm(0x%.4x 0x%.4x) = 0x%.4x carry: %d (took %7lld usecs)\n",
> + eax, ebx, regs->eax & 0xffff, carry, duration);
>
> - return rc;
> + if (carry || (regs->eax & 0xffff) == 0xffff || regs->eax == eax)
> + return -EINVAL;
> +
> + return 0;
> }
>
> /*
next prev parent reply other threads:[~2022-02-22 16:54 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-20 19:08 [PATCH v4] hwmon: (dell-smm) Improve assembly code Armin Wolf
2022-02-22 16:54 ` Guenter Roeck [this message]
2022-02-22 17:51 ` Pali Rohár
2022-02-22 19:46 ` David Laight
2022-02-23 21:39 ` Armin Wolf
2022-02-23 22:35 ` David Laight
2022-02-24 0:45 ` Guenter Roeck
2022-03-02 18:07 ` Guenter Roeck
2022-06-20 15:21 ` Guenter Roeck
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220222165432.GA255373@roeck-us.net \
--to=linux@roeck-us.net \
--cc=David.Laight@ACULAB.COM \
--cc=W_Armin@gmx.de \
--cc=jdelvare@suse.com \
--cc=linux-assembly@vger.kernel.org \
--cc=linux-hwmon@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=pali@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).