| Message ID | 20220215191113.16640-6-W_Armin@gmx.de (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | hwmon: (dell-smm) Miscellaneous improvements | expand |
Hi Armin,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on groeck-staging/hwmon-next]
[also build test ERROR on next-20220215]
[cannot apply to linus/master v5.17-rc4]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Armin-Wolf/hwmon-dell-smm-Miscellaneous-improvements/20220216-031722
base: https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
config: i386-randconfig-a004-20220214 (https://download.01.org/0day-ci/archive/20220216/202202160842.6EYIVsJn-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 37f422f4ac31c8b8041c6b62065263314282dab6)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/e03bd707be4885b219afdfd7a24778fb0a8129e1
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Armin-Wolf/hwmon-dell-smm-Miscellaneous-improvements/20220216-031722
git checkout e03bd707be4885b219afdfd7a24778fb0a8129e1
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> drivers/hwmon/dell-smm-hwmon.c:178:15: error: inline assembly requires more registers than available
asm volatile("out %%al,$0xb2\n\t"
^
1 error generated.
vim +178 drivers/hwmon/dell-smm-hwmon.c
161
162 /*
163 * Call the System Management Mode BIOS. Code provided by Jonathan Buzzard.
164 */
165 static int i8k_smm_func(void *par)
166 {
167 ktime_t calltime = ktime_get();
168 struct smm_regs *regs = par;
169 int eax = regs->eax;
170 int ebx = regs->ebx;
171 long long duration;
172 int rc;
173
174 /* SMM requires CPU 0 */
175 if (smp_processor_id() != 0)
176 return -EBUSY;
177
> 178 asm volatile("out %%al,$0xb2\n\t"
179 "out %%al,$0x84\n"
180 : "=a" (regs->eax),
181 "=b" (regs->ebx),
182 "=c" (regs->ecx),
183 "=d" (regs->edx),
184 "=S" (regs->esi),
185 "=D" (regs->edi),
186 CC_OUT(c) (rc)
187 : "a" (regs->eax),
188 "b" (regs->ebx),
189 "c" (regs->ecx),
190 "d" (regs->edx),
191 "S" (regs->esi),
192 "D" (regs->edi));
193
194 if (rc != 0 || (regs->eax & 0xffff) == 0xffff || regs->eax == eax)
195 rc = -EINVAL;
196
197 duration = ktime_us_delta(ktime_get(), calltime);
198 pr_debug("smm(0x%.4x 0x%.4x) = 0x%.4x (took %7lld usecs)\n", eax, ebx,
199 (rc ? 0xffff : regs->eax & 0xffff), duration);
200
201 return rc;
202 }
203
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c index b7016971bb2e..04a41d59da60 100644 --- a/drivers/hwmon/dell-smm-hwmon.c +++ b/drivers/hwmon/dell-smm-hwmon.c @@ -123,7 +123,7 @@ struct smm_regs { unsigned int edx; unsigned int esi; unsigned int edi; -} __packed; +}; static const char * const temp_labels[] = { "CPU", @@ -175,59 +175,22 @@ static int i8k_smm_func(void *par) 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 + asm volatile("out %%al,$0xb2\n\t" + "out %%al,$0x84\n" + : "=a" (regs->eax), + "=b" (regs->ebx), + "=c" (regs->ecx), + "=d" (regs->edx), + "=S" (regs->esi), + "=D" (regs->edi), + CC_OUT(c) (rc) + : "a" (regs->eax), + "b" (regs->ebx), + "c" (regs->ecx), + "d" (regs->edx), + "S" (regs->esi), + "D" (regs->edi)); + if (rc != 0 || (regs->eax & 0xffff) == 0xffff || regs->eax == eax) rc = -EINVAL;
The new assembly code works on both 32bit and 64bit cpus and allows for more compiler optimisations by not requiring smm_regs to be packed. Also since the SMM handler seems to modify the carry flag, the new code informs the compiler that the flags register needs to be saved/restored. Tested with 32 bit and 64 bit kernels on a Dell Inspiron 3505. Signed-off-by: Armin Wolf <W_Armin@gmx.de> --- drivers/hwmon/dell-smm-hwmon.c | 71 ++++++++-------------------------- 1 file changed, 17 insertions(+), 54 deletions(-) -- 2.30.2