x86 PMU broken in current Linus' tree

x86 PMU broken in current Linus' tree

[Jiri Kosina]

With current Linus' tree (HEAD == 731c7d3a20), I am getting bogus MSR 
write warning during bootup, and kernel panic when shutting PMUs down 
during poweroff.

The MSR warning is below, the camera capture of the poweroff panic can be 
found at

	http://www.jikos.cz/jikos/junk/pmu-panic.jpg

The last previous kernel version that I've booted on this particular 
machine was 4.7.0-rc4, and it had neither of those symptoms, so I can 
eventually bisect if needed.

=== [ snip ] ==
[    0.136000] smpboot: CPU0: Intel(R) Core(TM)2 Duo CPU     L9400  @ 1.86GHz (family: 0x6, model: 0x17, stepping: 0x6)
[    0.136000] Performance Events: PEBS fmt0+, Core2 events, Intel PMU driver.
[    0.136000] ... version:                2
[    0.136000] ... bit width:              40
[    0.136000] ... generic registers:      2
[    0.136000] ... value mask:             000000ffffffffff
[    0.136000] ... max period:             000000007fffffff
[    0.136000] ... fixed-purpose events:   3
[    0.136000] ... event mask:             0000000700000003
[    0.136000] NMI watchdog: enabled on all CPUs, permanently consumes one hw-PMU counter.
[    0.136000] unchecked MSR access error: WRMSR to 0xdf (tried to write 0x000000ff80000001) at rIP: 0xffffffff90004acc (x86_perf_event_set_period+0xdc/0x190)
[    0.136000]  ffff8e39f941b000 ffff8e39fbc0a440 000000000000001e ffff8e39fbc0a660
[    0.136000]  ffff8e39f9523b78 ffffffff90004bd0 ffff8e39f941b000 ffff8e39fbc0a760
[    0.136000]  ffff8e39fbc0a440 ffff8e39f9523bc0 ffffffff900053ef 00000000f951c2c0
[    0.136000] Call Trace:
[    0.136000]  [<ffffffff90004bd0>] x86_pmu_start+0x50/0x110
[    0.136000]  [<ffffffff900053ef>] x86_pmu_enable+0x27f/0x2f0
[    0.136000]  [<ffffffff90175642>] perf_pmu_enable+0x22/0x30
[    0.136000]  [<ffffffff901766b1>] ctx_resched+0x51/0x60
[    0.136000]  [<ffffffff901768a0>] __perf_event_enable+0x1e0/0x240
[    0.136000]  [<ffffffff9016e5f9>] event_function+0xa9/0x180
[    0.136000]  [<ffffffff901766c0>] ? ctx_resched+0x60/0x60
[    0.136000]  [<ffffffff9016fcef>] remote_function+0x3f/0x50
[    0.136000]  [<ffffffff901012b6>] generic_exec_single+0xb6/0x140
[    0.136000]  [<ffffffff9016fcb0>] ? perf_cgroup_attach+0x50/0x50
[    0.136000]  [<ffffffff9016fcb0>] ? perf_cgroup_attach+0x50/0x50
[    0.136000]  [<ffffffff901013f7>] smp_call_function_single+0xb7/0x110
[    0.136000]  [<ffffffff9016e984>] cpu_function_call+0x34/0x40
[    0.136000]  [<ffffffff9016e550>] ? list_del_event+0x150/0x150
[    0.136000]  [<ffffffff9016ecda>] event_function_call+0x11a/0x120
[    0.136000]  [<ffffffff901766c0>] ? ctx_resched+0x60/0x60
[    0.136000]  [<ffffffff9016ed79>] _perf_event_enable+0x49/0x70
[    0.136000]  [<ffffffff901736ac>] perf_event_enable+0x1c/0x40
[    0.136000]  [<ffffffff9013cad2>] watchdog_enable+0x132/0x1d0
[    0.136000]  [<ffffffff90092440>] smpboot_thread_fn+0xe0/0x1d0
[    0.136000]  [<ffffffff90092360>] ? sort_range+0x30/0x30
[    0.136000]  [<ffffffff9008e7e2>] kthread+0xf2/0x110
[    0.136000]  [<ffffffff9069e611>] ? wait_for_completion+0xe1/0x110
[    0.136000]  [<ffffffff906a3b2f>] ret_from_fork+0x1f/0x40
[    0.136000]  [<ffffffff9008e6f0>] ? kthread_create_on_node+0x220/0x220
=== [ snip ] ===

-- 
Jiri Kosina
SUSE Labs

Re: x86 PMU broken in current Linus' tree

[Peter Zijlstra]

On Tue, Aug 02, 2016 at 02:04:58PM +0200, Jiri Kosina wrote:
> With current Linus' tree (HEAD == 731c7d3a20), I am getting bogus MSR 
> write warning during bootup, and kernel panic when shutting PMUs down 
> during poweroff.
> 
> The MSR warning is below, the camera capture of the poweroff panic can be 
> found at
> 
> 	http://www.jikos.cz/jikos/junk/pmu-panic.jpg
> 
> The last previous kernel version that I've booted on this particular 
> machine was 4.7.0-rc4, and it had neither of those symptoms, so I can 
> eventually bisect if needed.
> 
> === [ snip ] ==
> [    0.136000] smpboot: CPU0: Intel(R) Core(TM)2 Duo CPU     L9400  @ 1.86GHz (family: 0x6, model: 0x17, stepping: 0x6)
> [    0.136000] Performance Events: PEBS fmt0+, Core2 events, Intel PMU driver.
> [    0.136000] ... version:                2
> [    0.136000] ... bit width:              40
> [    0.136000] ... generic registers:      2
> [    0.136000] ... value mask:             000000ffffffffff
> [    0.136000] ... max period:             000000007fffffff
> [    0.136000] ... fixed-purpose events:   3
> [    0.136000] ... event mask:             0000000700000003
> [    0.136000] NMI watchdog: enabled on all CPUs, permanently consumes one hw-PMU counter.
> [    0.136000] unchecked MSR access error: WRMSR to 0xdf (tried to write 0x000000ff80000001) at rIP: 0xffffffff90004acc (x86_perf_event_set_period+0xdc/0x190)

'Curious'.. :/

x86_perf_event_set_period() only does:

  wrmsrl(hwc->event_base, (u64)(-left) & x86_pmu.cntval_mask);

and hwc->event ends up being:

  MSR_ARCH_PERFMON_PERFCTR0 + index

>From which we can deduce that index = 0xdf - 0xc1 = 30, which is
somewhat larger than the max reported number of counters (2).

Lemme go see how that can happen.

Re: x86 PMU broken in current Linus' tree

[Jiri Kosina]

On Tue, 2 Aug 2016, Peter Zijlstra wrote:

> > With current Linus' tree (HEAD == 731c7d3a20), I am getting bogus MSR 
> > write warning during bootup, and kernel panic when shutting PMUs down 
> > during poweroff.
> > 
> > The MSR warning is below, the camera capture of the poweroff panic can be 
> > found at
> > 
> > 	http://www.jikos.cz/jikos/junk/pmu-panic.jpg
> > 
> > The last previous kernel version that I've booted on this particular 
> > machine was 4.7.0-rc4, and it had neither of those symptoms, so I can 
> > eventually bisect if needed.
> > 
> > === [ snip ] ==
> > [    0.136000] smpboot: CPU0: Intel(R) Core(TM)2 Duo CPU     L9400  @ 1.86GHz (family: 0x6, model: 0x17, stepping: 0x6)
> > [    0.136000] Performance Events: PEBS fmt0+, Core2 events, Intel PMU driver.
> > [    0.136000] ... version:                2
> > [    0.136000] ... bit width:              40
> > [    0.136000] ... generic registers:      2
> > [    0.136000] ... value mask:             000000ffffffffff
> > [    0.136000] ... max period:             000000007fffffff
> > [    0.136000] ... fixed-purpose events:   3
> > [    0.136000] ... event mask:             0000000700000003
> > [    0.136000] NMI watchdog: enabled on all CPUs, permanently consumes one hw-PMU counter.
> > [    0.136000] unchecked MSR access error: WRMSR to 0xdf (tried to write 0x000000ff80000001) at rIP: 0xffffffff90004acc (x86_perf_event_set_period+0xdc/0x190)
> 
> 'Curious'.. :/
> 
> x86_perf_event_set_period() only does:
> 
>   wrmsrl(hwc->event_base, (u64)(-left) & x86_pmu.cntval_mask);
> 
> and hwc->event ends up being:
> 
>   MSR_ARCH_PERFMON_PERFCTR0 + index
> 
> From which we can deduce that index = 0xdf - 0xc1 = 30, which is
> somewhat larger than the max reported number of counters (2).
> 
> Lemme go see how that can happen.

FTR, I tried the very same kernel on Xeon E5, and the issue didn't pop up. 
So it might be somehow specific to the older Core2, or somehow otherwise 
not really completely generic problem.

-- 
Jiri Kosina
SUSE Labs

Re: x86 PMU broken in current Linus' tree

[Peter Zijlstra]

On Tue, Aug 02, 2016 at 02:55:24PM +0200, Peter Zijlstra wrote:
> On Tue, Aug 02, 2016 at 02:04:58PM +0200, Jiri Kosina wrote:
> > With current Linus' tree (HEAD == 731c7d3a20), I am getting bogus MSR 
> > write warning during bootup, and kernel panic when shutting PMUs down 
> > during poweroff.
> > 
> > The MSR warning is below, the camera capture of the poweroff panic can be 
> > found at
> > 
> > 	http://www.jikos.cz/jikos/junk/pmu-panic.jpg
> > 
> > The last previous kernel version that I've booted on this particular 
> > machine was 4.7.0-rc4, and it had neither of those symptoms, so I can 
> > eventually bisect if needed.
> > 
> > === [ snip ] ==
> > [    0.136000] smpboot: CPU0: Intel(R) Core(TM)2 Duo CPU     L9400  @ 1.86GHz (family: 0x6, model: 0x17, stepping: 0x6)
> > [    0.136000] Performance Events: PEBS fmt0+, Core2 events, Intel PMU driver.
> > [    0.136000] ... version:                2
> > [    0.136000] ... bit width:              40
> > [    0.136000] ... generic registers:      2
> > [    0.136000] ... value mask:             000000ffffffffff
> > [    0.136000] ... max period:             000000007fffffff
> > [    0.136000] ... fixed-purpose events:   3
> > [    0.136000] ... event mask:             0000000700000003
> > [    0.136000] NMI watchdog: enabled on all CPUs, permanently consumes one hw-PMU counter.
> > [    0.136000] unchecked MSR access error: WRMSR to 0xdf (tried to write 0x000000ff80000001) at rIP: 0xffffffff90004acc (x86_perf_event_set_period+0xdc/0x190)
> 
> 'Curious'.. :/
> 
> x86_perf_event_set_period() only does:
> 
>   wrmsrl(hwc->event_base, (u64)(-left) & x86_pmu.cntval_mask);
> 
> and hwc->event ends up being:
> 
>   MSR_ARCH_PERFMON_PERFCTR0 + index
> 
> From which we can deduce that index = 0xdf - 0xc1 = 30, which is
> somewhat larger than the max reported number of counters (2).
> 
> Lemme go see how that can happen.

So I can reproduce on my Lenovo T500 which has a Core2 as well. By long
and tedious printk() it looks like the event constraint:

  FIXED_EVENT_CONSTRAINT(0x003c, 1), /* CPU_CLK_UNHALTED.CORE */

which is updated in:

  intel_pmu_init()

to include the generic counter masks, gets corrupted for some reason.
But the moment I put printk()s in there to print the idxmsk64 values,
everything works as expected again.

I'll go prod mode.

Re: x86 PMU broken in current Linus' tree

[Jiri Kosina]

On Mon, 8 Aug 2016, Peter Zijlstra wrote:

> So I can reproduce on my Lenovo T500 which has a Core2 as well. By long
> and tedious printk() it looks like the event constraint:
> 
>   FIXED_EVENT_CONSTRAINT(0x003c, 1), /* CPU_CLK_UNHALTED.CORE */

[ adding Boris to CC ]

This made me wonder, and it turned out that HWEIGHT() doesn't seem to be 
doing the right thing on machines that don't have popcnt instruction 
(which is probably the distinguishing factor here).

How about the patch below? It fixes the symptoms on my system.




From: Jiri Kosina <jkosina@suse.cz>
Subject: [PATCH] x86/hweight: fix open-coded versions of 32bit and 64bit hweight calculations

The open-coded version of __sw_hweight32() and __sw_hweight64() are broken;
the variable <-> register mapping in the comments doesn't match the registers
being used, resulting in wrong calculations.

This has a potential to demonstrate itself by various syptoms on machines
where this is actually being used to compute the hweight (due to lack of
popcnt insn). On my core2 system, this demonstrates itself as

 NMI watchdog: enabled on all CPUs, permanently consumes one hw-PMU counter.
 unchecked MSR access error: WRMSR to 0xdf (tried to write 0x000000ff80000001) at rIP: 0xffffffff90004acc
t_set_period+0xdc/0x190)
  ffff8e39f941b000 ffff8e39fbc0a440 000000000000001e ffff8e39fbc0a660
  ffff8e39f9523b78 ffffffff90004bd0 ffff8e39f941b000 ffff8e39fbc0a760
  ffff8e39fbc0a440 ffff8e39f9523bc0 ffffffff900053ef 00000000f951c2c0
 Call Trace:
  [<ffffffff90004bd0>] x86_pmu_start+0x50/0x110
  [<ffffffff900053ef>] x86_pmu_enable+0x27f/0x2f0
  [<ffffffff90175642>] perf_pmu_enable+0x22/0x30
  [<ffffffff901766b1>] ctx_resched+0x51/0x60
  [<ffffffff901768a0>] __perf_event_enable+0x1e0/0x240
  [<ffffffff9016e5f9>] event_function+0xa9/0x180
  [<ffffffff901766c0>] ? ctx_resched+0x60/0x60
  [<ffffffff9016fcef>] remote_function+0x3f/0x50
  [<ffffffff901012b6>] generic_exec_single+0xb6/0x140
  [<ffffffff9016fcb0>] ? perf_cgroup_attach+0x50/0x50
  [<ffffffff9016fcb0>] ? perf_cgroup_attach+0x50/0x50
  [<ffffffff901013f7>] smp_call_function_single+0xb7/0x110
  [<ffffffff9016e984>] cpu_function_call+0x34/0x40
  [<ffffffff9016e550>] ? list_del_event+0x150/0x150
  [<ffffffff9016ecda>] event_function_call+0x11a/0x120
  [<ffffffff901766c0>] ? ctx_resched+0x60/0x60
  [<ffffffff9016ed79>] _perf_event_enable+0x49/0x70
  [<ffffffff901736ac>] perf_event_enable+0x1c/0x40
  [<ffffffff9013cad2>] watchdog_enable+0x132/0x1d0
  [<ffffffff90092440>] smpboot_thread_fn+0xe0/0x1d0
  [<ffffffff90092360>] ? sort_range+0x30/0x30
  [<ffffffff9008e7e2>] kthread+0xf2/0x110
  [<ffffffff9069e611>] ? wait_for_completion+0xe1/0x110
  [<ffffffff906a3b2f>] ret_from_fork+0x1f/0x40
  [<ffffffff9008e6f0>] ? kthread_create_on_node+0x220/0x220

as FIXED_EVENT_CONSTRAINT() doesn't procude correct results, and followup panic
in x86_pmu_stop(). YMMV.

Fix this by rewriting the code so that it actually matches the math in
comments.

Fixes: f5967101e9d ("x86/hweight: Get rid of the special calling convention")
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
---
 arch/x86/lib/hweight.S | 43 +++++++++++++++++++++----------------------
 1 file changed, 21 insertions(+), 22 deletions(-)

diff --git a/arch/x86/lib/hweight.S b/arch/x86/lib/hweight.S
index 02de3d7..9d0540f 100644
--- a/arch/x86/lib/hweight.S
+++ b/arch/x86/lib/hweight.S
@@ -8,24 +8,23 @@
  */
 ENTRY(__sw_hweight32)
 
+	__ASM_SIZE(push,) %__ASM_REG(dx)
 #ifdef CONFIG_X86_64
-	movl %edi, %eax				# w
+	movl %edi, %edx				# w -> t
 #endif
-	__ASM_SIZE(push,) %__ASM_REG(dx)
-	movl %eax, %edx				# w -> t
-	shrl %edx				# t >>= 1
-	andl $0x55555555, %edx			# t &= 0x55555555
-	subl %edx, %eax				# w -= t
+	shrl %edx				# w >>= 1
+	andl $0x55555555, %edx			# w &= 0x55555555
+	subl %edx, %edi				# w -= t
+	movl %edi, %eax				# w -> t
 
-	movl %eax, %edx				# w -> t
-	shrl $2, %eax				# w_tmp >>= 2
-	andl $0x33333333, %edx			# t	&= 0x33333333
-	andl $0x33333333, %eax			# w_tmp &= 0x33333333
-	addl %edx, %eax				# w = w_tmp + t
+	shrl $2, %edi				# w_tmp >>= 2
+	andl $0x33333333, %eax			# t	&= 0x33333333
+	andl $0x33333333, %edi			# w_tmp &= 0x33333333
+	addl %eax, %edi				# w = w_tmp + t
 
-	movl %eax, %edx				# w -> t
-	shrl $4, %edx				# t >>= 4
-	addl %edx, %eax				# w_tmp += t
+	movl %edi, %eax				# w -> t
+	shrl $4, %eax				# t >>= 4
+	addl %edi, %eax				# w_tmp += t
 	andl  $0x0f0f0f0f, %eax			# w_tmp &= 0x0f0f0f0f
 	imull $0x01010101, %eax, %eax		# w_tmp *= 0x01010101
 	shrl $24, %eax				# w = w_tmp >> 24
@@ -40,20 +39,20 @@ ENTRY(__sw_hweight64)
 	movq    %rdi, %rdx                      # w -> t
 	movabsq $0x5555555555555555, %rax
 	shrq    %rdx                            # t >>= 1
-	andq    %rdx, %rax                      # t &= 0x5555555555555555
+	andq    %rax, %rdx                      # t &= 0x5555555555555555
+	subq    %rdx, %rdi                      # w -= t
 	movabsq $0x3333333333333333, %rdx
-	subq    %rax, %rdi                      # w -= t
 
 	movq    %rdi, %rax                      # w -> t
 	shrq    $2, %rdi                        # w_tmp >>= 2
 	andq    %rdx, %rax                      # t     &= 0x3333333333333333
-	andq    %rdi, %rdx                      # w_tmp &= 0x3333333333333333
-	addq    %rdx, %rax                      # w = w_tmp + t
-
-	movq    %rax, %rdx                      # w -> t
-	shrq    $4, %rdx                        # t >>= 4
-	addq    %rdx, %rax                      # w_tmp += t
+	andq    %rdx, %rdi                      # w_tmp &= 0x3333333333333333
 	movabsq $0x0f0f0f0f0f0f0f0f, %rdx
+	addq    %rax, %rdi                      # w = w_tmp + t
+
+	movq    %rdi, %rax                      # w -> t
+	shrq    $4, %rax                        # t >>= 4
+	addq    %rdi, %rax                      # w_tmp += t
 	andq    %rdx, %rax                      # w_tmp &= 0x0f0f0f0f0f0f0f0f
 	movabsq $0x0101010101010101, %rdx
 	imulq   %rdx, %rax                      # w_tmp *= 0x0101010101010101

-- 
Jiri Kosina
SUSE Labs

[PATCH v2] x86/hweight: fix open-coded versions of 32bit and 64bit hweight calculations

[Jiri Kosina]

From: Jiri Kosina <jkosina@suse.cz>

The open-coded version of __sw_hweight32() and __sw_hweight64() are 
broken; the variable <-> register mapping in the comments doesn't match 
the registers being used, resulting in wrong calculations.

This has a potential to demonstrate itself by various syptoms on machines 
where this is actually being used to compute the hweight (due to lack of 
popcnt insn). On my core2 system, this demonstrates itself as

 NMI watchdog: enabled on all CPUs, permanently consumes one hw-PMU counter.
 unchecked MSR access error: WRMSR to 0xdf (tried to write 0x000000ff80000001) at rIP: 0xffffffff90004acct_set_period+0xdc/0x190)
  ffff8e39f941b000 ffff8e39fbc0a440 000000000000001e ffff8e39fbc0a660
  ffff8e39f9523b78 ffffffff90004bd0 ffff8e39f941b000 ffff8e39fbc0a760
  ffff8e39fbc0a440 ffff8e39f9523bc0 ffffffff900053ef 00000000f951c2c0
 Call Trace:
  [<ffffffff90004bd0>] x86_pmu_start+0x50/0x110
  [<ffffffff900053ef>] x86_pmu_enable+0x27f/0x2f0
  [<ffffffff90175642>] perf_pmu_enable+0x22/0x30
  [<ffffffff901766b1>] ctx_resched+0x51/0x60
  [<ffffffff901768a0>] __perf_event_enable+0x1e0/0x240
  [<ffffffff9016e5f9>] event_function+0xa9/0x180
  [<ffffffff901766c0>] ? ctx_resched+0x60/0x60
  [<ffffffff9016fcef>] remote_function+0x3f/0x50
  [<ffffffff901012b6>] generic_exec_single+0xb6/0x140
  [<ffffffff9016fcb0>] ? perf_cgroup_attach+0x50/0x50
  [<ffffffff9016fcb0>] ? perf_cgroup_attach+0x50/0x50
  [<ffffffff901013f7>] smp_call_function_single+0xb7/0x110
  [<ffffffff9016e984>] cpu_function_call+0x34/0x40
  [<ffffffff9016e550>] ? list_del_event+0x150/0x150
  [<ffffffff9016ecda>] event_function_call+0x11a/0x120
  [<ffffffff901766c0>] ? ctx_resched+0x60/0x60
  [<ffffffff9016ed79>] _perf_event_enable+0x49/0x70
  [<ffffffff901736ac>] perf_event_enable+0x1c/0x40
  [<ffffffff9013cad2>] watchdog_enable+0x132/0x1d0
  [<ffffffff90092440>] smpboot_thread_fn+0xe0/0x1d0
  [<ffffffff90092360>] ? sort_range+0x30/0x30
  [<ffffffff9008e7e2>] kthread+0xf2/0x110
  [<ffffffff9069e611>] ? wait_for_completion+0xe1/0x110
  [<ffffffff906a3b2f>] ret_from_fork+0x1f/0x40
  [<ffffffff9008e6f0>] ? kthread_create_on_node+0x220/0x220

as FIXED_EVENT_CONSTRAINT() doesn't produce correct results, and followup 
panic in x86_pmu_stop(). YMMV.

Fix this by rewriting the code so that it actually matches the math 
lib/hweight.c. While at it, nuke the original comments altogether; the 
"variable" names don't really match what's being used in the C-version of 
the function, and I find them to be more misleading than helping.

This version of gcc:

	gcc (SUSE Linux) 4.8.5

produces identical assembly code from lib/hweight.c (modulo frame pointer 
maths).

Fixes: f5967101e9d ("x86/hweight: Get rid of the special calling convention")
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
---

v1 -> v2: remove the comments altogether; put "approved by gcc" note into 
          changelog

 arch/x86/lib/hweight.S | 69 +++++++++++++++++++++++++-------------------------
 1 file changed, 34 insertions(+), 35 deletions(-)

diff --git a/arch/x86/lib/hweight.S b/arch/x86/lib/hweight.S
index 02de3d7..e1cea44f 100644
--- a/arch/x86/lib/hweight.S
+++ b/arch/x86/lib/hweight.S
@@ -8,56 +8,55 @@
  */
 ENTRY(__sw_hweight32)
 
-#ifdef CONFIG_X86_64
-	movl %edi, %eax				# w
-#endif
 	__ASM_SIZE(push,) %__ASM_REG(dx)
-	movl %eax, %edx				# w -> t
-	shrl %edx				# t >>= 1
-	andl $0x55555555, %edx			# t &= 0x55555555
-	subl %edx, %eax				# w -= t
 
-	movl %eax, %edx				# w -> t
-	shrl $2, %eax				# w_tmp >>= 2
-	andl $0x33333333, %edx			# t	&= 0x33333333
-	andl $0x33333333, %eax			# w_tmp &= 0x33333333
-	addl %edx, %eax				# w = w_tmp + t
+	movl %edi, %edx
+
+	shrl %edx
+	andl $0x55555555, %edx
+	subl %edx, %edi
+	movl %edi, %eax
+
+	shrl $2, %edi
+	andl $0x33333333, %eax
+	andl $0x33333333, %edi
+	addl %eax, %edi
 
-	movl %eax, %edx				# w -> t
-	shrl $4, %edx				# t >>= 4
-	addl %edx, %eax				# w_tmp += t
-	andl  $0x0f0f0f0f, %eax			# w_tmp &= 0x0f0f0f0f
-	imull $0x01010101, %eax, %eax		# w_tmp *= 0x01010101
-	shrl $24, %eax				# w = w_tmp >> 24
+	movl %edi, %eax
+	shrl $4, %eax
+	addl %edi, %eax
+	andl  $0x0f0f0f0f, %eax
+	imull $0x01010101, %eax, %eax
+	shrl $24, %eax
 	__ASM_SIZE(pop,) %__ASM_REG(dx)
 	ret
 ENDPROC(__sw_hweight32)
 
 ENTRY(__sw_hweight64)
-#ifdef CONFIG_X86_64
+
 	pushq   %rdx
 
-	movq    %rdi, %rdx                      # w -> t
+	movq    %rdi, %rdx
 	movabsq $0x5555555555555555, %rax
-	shrq    %rdx                            # t >>= 1
-	andq    %rdx, %rax                      # t &= 0x5555555555555555
+	shrq    %rdx
+	andq    %rax, %rdx
+	subq    %rdx, %rdi
 	movabsq $0x3333333333333333, %rdx
-	subq    %rax, %rdi                      # w -= t
-
-	movq    %rdi, %rax                      # w -> t
-	shrq    $2, %rdi                        # w_tmp >>= 2
-	andq    %rdx, %rax                      # t     &= 0x3333333333333333
-	andq    %rdi, %rdx                      # w_tmp &= 0x3333333333333333
-	addq    %rdx, %rax                      # w = w_tmp + t
 
-	movq    %rax, %rdx                      # w -> t
-	shrq    $4, %rdx                        # t >>= 4
-	addq    %rdx, %rax                      # w_tmp += t
+	movq    %rdi, %rax
+	shrq    $2, %rdi
+	andq    %rdx, %rax
+	andq    %rdx, %rdi
 	movabsq $0x0f0f0f0f0f0f0f0f, %rdx
-	andq    %rdx, %rax                      # w_tmp &= 0x0f0f0f0f0f0f0f0f
+	addq    %rax, %rdi
+
+	movq    %rdi, %rax
+	shrq    $4, %rax
+	addq    %rdi, %rax
+	andq    %rdx, %rax
 	movabsq $0x0101010101010101, %rdx
-	imulq   %rdx, %rax                      # w_tmp *= 0x0101010101010101
-	shrq    $56, %rax                       # w = w_tmp >> 56
+	imulq   %rdx, %rax
+	shrq    $56, %rax
 
 	popq    %rdx
 	ret
-- 
Jiri Kosina
SUSE Labs

Re: [PATCH v2] x86/hweight: fix open-coded versions of 32bit and 64bit hweight calculations

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2538 bytes --]

Hi Jiri,

[auto build test ERROR on tip/auto-latest]
[also build test ERROR on v4.8-rc1 next-20160808]
[cannot apply to tip/x86/core]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Jiri-Kosina/x86-hweight-fix-open-coded-versions-of-32bit-and-64bit-hweight-calculations/20160809-001505
config: i386-tinyconfig (attached as .config)
compiler: gcc-6 (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

>> arch/x86/lib/hweight.S:63:2: error: #else without #if
    #else /* CONFIG_X86_32 */
     ^~~~
>> arch/x86/lib/hweight.S:75:2: error: #endif without #if
    #endif
     ^~~~~

vim +63 arch/x86/lib/hweight.S

f5967101 Borislav Petkov 2016-05-30  57  	movabsq $0x0101010101010101, %rdx
4792d6cc Jiri Kosina     2016-08-08  58  	imulq   %rdx, %rax
4792d6cc Jiri Kosina     2016-08-08  59  	shrq    $56, %rax
f5967101 Borislav Petkov 2016-05-30  60  
f5967101 Borislav Petkov 2016-05-30  61  	popq    %rdx
f5967101 Borislav Petkov 2016-05-30  62  	ret
f5967101 Borislav Petkov 2016-05-30 @63  #else /* CONFIG_X86_32 */
f5967101 Borislav Petkov 2016-05-30  64  	/* We're getting an u64 arg in (%eax,%edx): unsigned long hweight64(__u64 w) */
f5967101 Borislav Petkov 2016-05-30  65  	pushl   %ecx
f5967101 Borislav Petkov 2016-05-30  66  
f5967101 Borislav Petkov 2016-05-30  67  	call    __sw_hweight32
f5967101 Borislav Petkov 2016-05-30  68  	movl    %eax, %ecx                      # stash away result
f5967101 Borislav Petkov 2016-05-30  69  	movl    %edx, %eax                      # second part of input
f5967101 Borislav Petkov 2016-05-30  70  	call    __sw_hweight32
f5967101 Borislav Petkov 2016-05-30  71  	addl    %ecx, %eax                      # result
f5967101 Borislav Petkov 2016-05-30  72  
f5967101 Borislav Petkov 2016-05-30  73  	popl    %ecx
f5967101 Borislav Petkov 2016-05-30  74  	ret
f5967101 Borislav Petkov 2016-05-30 @75  #endif
f5967101 Borislav Petkov 2016-05-30  76  ENDPROC(__sw_hweight64)

:::::: The code at line 63 was first introduced by commit
:::::: f5967101e9de12addcda4510dfbac66d7c5779c3 x86/hweight: Get rid of the special calling convention

:::::: TO: Borislav Petkov <bp@suse.de>
:::::: CC: Ingo Molnar <mingo@kernel.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 6290 bytes --]

Re: [PATCH v2] x86/hweight: fix open-coded versions of 32bit and 64bit hweight calculations

[Jiri Kosina]

Okay, scratch this thread, it's been just covered (and Linus already 
applied it) in

	https://lkml.kernel.org/r/1470677729-10561-1-git-send-email-ville.syrjala@linux.intel.com

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH v2] x86/hweight: fix open-coded versions of 32bit and 64bit hweight calculations

[Borislav Petkov]

On Mon, Aug 08, 2016 at 08:48:44PM +0200, Jiri Kosina wrote:
> Okay, scratch this thread, it's been just covered (and Linus already 
> applied it) in
> 
> 	https://lkml.kernel.org/r/1470677729-10561-1-git-send-email-ville.syrjala@linux.intel.com

Can you confirm it fixes the issue on your machine too?

Thanks.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: [PATCH v2] x86/hweight: fix open-coded versions of 32bit and 64bit hweight calculations

[Jiri Kosina]

On Mon, 8 Aug 2016, Borislav Petkov wrote:

> > Okay, scratch this thread, it's been just covered (and Linus already 
> > applied it) in
> > 
> > 	https://lkml.kernel.org/r/1470677729-10561-1-git-send-email-ville.syrjala@linux.intel.com
> 
> Can you confirm it fixes the issue on your machine too?

Confirmed, it does. Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH v2] x86/hweight: fix open-coded versions of 32bit and 64bit hweight calculations

[Ingo Molnar]

* Jiri Kosina <jikos@kernel.org> wrote:

>  ENTRY(__sw_hweight64)
> -#ifdef CONFIG_X86_64
> +

So this removes an #ifdef without removing the #else branch.

Help?

Thanks,

	Ingo

Re: [PATCH v2] x86/hweight: fix open-coded versions of 32bit and 64bit hweight calculations

[Jiri Kosina]

On Wed, 10 Aug 2016, Ingo Molnar wrote:

> >  ENTRY(__sw_hweight64)
> > -#ifdef CONFIG_X86_64
> > +
> 
> So this removes an #ifdef without removing the #else branch.

Hi Ingo,

this patch is obsolete; the real issue has already been fixed by 65ea11ec6 
("x86/hweight: Don't clobber %rdi") in Linus' tree already.

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH v2] x86/hweight: fix open-coded versions of 32bit and 64bit hweight calculations

[Ingo Molnar]

* Jiri Kosina <jikos@kernel.org> wrote:

> On Wed, 10 Aug 2016, Ingo Molnar wrote:
> 
> > >  ENTRY(__sw_hweight64)
> > > -#ifdef CONFIG_X86_64
> > > +
> > 
> > So this removes an #ifdef without removing the #else branch.
> 
> Hi Ingo,
> 
> this patch is obsolete; the real issue has already been fixed by 65ea11ec6 
> ("x86/hweight: Don't clobber %rdi") in Linus' tree already.

Ok, great!

Thanks,

	Ingo