All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] x86, bitops: Move BIT_64 for a wider use
@ 2012-05-22 10:53 Borislav Petkov
  2012-05-22 10:53 ` [PATCH 2/2] x86, MCE: Fix 32-bit build Borislav Petkov
  2012-05-23 16:07 ` [tip:x86/mce] x86/bitops: Move BIT_64() for a wider use tip-bot for Borislav Petkov
  0 siblings, 2 replies; 28+ messages in thread
From: Borislav Petkov @ 2012-05-22 10:53 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: Frank Arnold, X86-ML, LKML, Borislav Petkov

From: Borislav Petkov <borislav.petkov@amd.com>

Needed for shifting 64-bit values on 32-bit, like MSR values, for
example.

Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
---
 arch/x86/include/asm/bitops.h |    2 ++
 drivers/edac/mce_amd.h        |    2 --
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/bitops.h b/arch/x86/include/asm/bitops.h
index b97596e2b68c..a6983b277220 100644
--- a/arch/x86/include/asm/bitops.h
+++ b/arch/x86/include/asm/bitops.h
@@ -15,6 +15,8 @@
 #include <linux/compiler.h>
 #include <asm/alternative.h>
 
+#define BIT_64(n)			(U64_C(1) << (n))
+
 /*
  * These have to be done with inline assembly: that way the bit-setting
  * is guaranteed to be atomic. All bit operations return 0 if the bit
diff --git a/drivers/edac/mce_amd.h b/drivers/edac/mce_amd.h
index c6074c5cd1ef..8c87a5e87057 100644
--- a/drivers/edac/mce_amd.h
+++ b/drivers/edac/mce_amd.h
@@ -5,8 +5,6 @@
 
 #include <asm/mce.h>
 
-#define BIT_64(n)			(U64_C(1) << (n))
-
 #define EC(x)				((x) & 0xffff)
 #define XEC(x, mask)			(((x) >> 16) & mask)
 
-- 
1.7.9.3.362.g71319


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 2/2] x86, MCE: Fix 32-bit build
  2012-05-22 10:53 [PATCH 1/2] x86, bitops: Move BIT_64 for a wider use Borislav Petkov
@ 2012-05-22 10:53 ` Borislav Petkov
  2012-05-23 16:08   ` [tip:x86/mce] x86/mce: " tip-bot for Borislav Petkov
  2012-05-23 16:07 ` [tip:x86/mce] x86/bitops: Move BIT_64() for a wider use tip-bot for Borislav Petkov
  1 sibling, 1 reply; 28+ messages in thread
From: Borislav Petkov @ 2012-05-22 10:53 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: Frank Arnold, X86-ML, LKML, Borislav Petkov

From: Borislav Petkov <borislav.petkov@amd.com>

Got bitten again by the BIT() macro:

arch/x86/kernel/cpu/mcheck/mce.c: In function '__mcheck_cpu_apply_quirks':
arch/x86/kernel/cpu/mcheck/mce.c:1453:6: warning: left shift count >= width of type
arch/x86/kernel/cpu/mcheck/mce.c:1454:7: warning: left shift count >= width of type

Fix it already.

Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
---
 arch/x86/kernel/cpu/mcheck/mce.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
index 888fbf9d0adf..0456b9a08086 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -1450,9 +1450,9 @@ static int __cpuinit __mcheck_cpu_apply_quirks(struct cpuinfo_x86 *c)
 				 rdmsrl(msrs[i], val);
 
 				 /* CntP bit set? */
-				 if (val & BIT(62)) {
-					 val &= ~BIT(62);
-					 wrmsrl(msrs[i], val);
+				 if (val & BIT_64(62)) {
+					val &= ~BIT_64(62);
+					wrmsrl(msrs[i], val);
 				 }
 			 }
 
-- 
1.7.9.3.362.g71319


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [tip:x86/mce] x86/bitops: Move BIT_64() for a wider use
  2012-05-22 10:53 [PATCH 1/2] x86, bitops: Move BIT_64 for a wider use Borislav Petkov
  2012-05-22 10:53 ` [PATCH 2/2] x86, MCE: Fix 32-bit build Borislav Petkov
@ 2012-05-23 16:07 ` tip-bot for Borislav Petkov
  2012-05-23 16:10   ` Peter Zijlstra
  1 sibling, 1 reply; 28+ messages in thread
From: tip-bot for Borislav Petkov @ 2012-05-23 16:07 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, a.p.zijlstra, torvalds, akpm,
	frank.arnold, tglx, borislav.petkov

Commit-ID:  e8f380e00840f694599e6ab42806639f7de26f11
Gitweb:     http://git.kernel.org/tip/e8f380e00840f694599e6ab42806639f7de26f11
Author:     Borislav Petkov <borislav.petkov@amd.com>
AuthorDate: Tue, 22 May 2012 12:53:45 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 23 May 2012 17:16:42 +0200

x86/bitops: Move BIT_64() for a wider use

Needed for shifting 64-bit values on 32-bit, like MSR values,
for example.

Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Frank Arnold <frank.arnold@amd.com>
Link: http://lkml.kernel.org/r/1337684026-19740-1-git-send-email-bp@amd64.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/include/asm/bitops.h |    2 ++
 drivers/edac/mce_amd.h        |    2 --
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/bitops.h b/arch/x86/include/asm/bitops.h
index b97596e..a6983b2 100644
--- a/arch/x86/include/asm/bitops.h
+++ b/arch/x86/include/asm/bitops.h
@@ -15,6 +15,8 @@
 #include <linux/compiler.h>
 #include <asm/alternative.h>
 
+#define BIT_64(n)			(U64_C(1) << (n))
+
 /*
  * These have to be done with inline assembly: that way the bit-setting
  * is guaranteed to be atomic. All bit operations return 0 if the bit
diff --git a/drivers/edac/mce_amd.h b/drivers/edac/mce_amd.h
index c6074c5..8c87a5e 100644
--- a/drivers/edac/mce_amd.h
+++ b/drivers/edac/mce_amd.h
@@ -5,8 +5,6 @@
 
 #include <asm/mce.h>
 
-#define BIT_64(n)			(U64_C(1) << (n))
-
 #define EC(x)				((x) & 0xffff)
 #define XEC(x, mask)			(((x) >> 16) & mask)
 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [tip:x86/mce] x86/mce: Fix 32-bit build
  2012-05-22 10:53 ` [PATCH 2/2] x86, MCE: Fix 32-bit build Borislav Petkov
@ 2012-05-23 16:08   ` tip-bot for Borislav Petkov
  2012-05-23 16:18     ` Peter Zijlstra
  0 siblings, 1 reply; 28+ messages in thread
From: tip-bot for Borislav Petkov @ 2012-05-23 16:08 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, a.p.zijlstra, torvalds, frank.arnold,
	akpm, tglx, borislav.petkov

Commit-ID:  80f033610fb968e75f5d470233d8d0260d7a72ed
Gitweb:     http://git.kernel.org/tip/80f033610fb968e75f5d470233d8d0260d7a72ed
Author:     Borislav Petkov <borislav.petkov@amd.com>
AuthorDate: Tue, 22 May 2012 12:53:46 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 23 May 2012 17:16:43 +0200

x86/mce: Fix 32-bit build

Got bitten again by the BIT() macro:

 arch/x86/kernel/cpu/mcheck/mce.c: In function '__mcheck_cpu_apply_quirks':
 arch/x86/kernel/cpu/mcheck/mce.c:1453:6: warning: left shift
 count >= width of type arch/x86/kernel/cpu/mcheck/mce.c:1454:7: warning: left shift count >= width of type

Fix it already.

Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
Cc: Frank Arnold <frank.arnold@amd.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1337684026-19740-2-git-send-email-bp@amd64.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/cpu/mcheck/mce.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
index 888fbf9..0456b9a 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -1450,9 +1450,9 @@ static int __cpuinit __mcheck_cpu_apply_quirks(struct cpuinfo_x86 *c)
 				 rdmsrl(msrs[i], val);
 
 				 /* CntP bit set? */
-				 if (val & BIT(62)) {
-					 val &= ~BIT(62);
-					 wrmsrl(msrs[i], val);
+				 if (val & BIT_64(62)) {
+					val &= ~BIT_64(62);
+					wrmsrl(msrs[i], val);
 				 }
 			 }
 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [tip:x86/mce] x86/bitops: Move BIT_64() for a wider use
  2012-05-23 16:07 ` [tip:x86/mce] x86/bitops: Move BIT_64() for a wider use tip-bot for Borislav Petkov
@ 2012-05-23 16:10   ` Peter Zijlstra
  2012-05-23 16:11     ` H. Peter Anvin
  0 siblings, 1 reply; 28+ messages in thread
From: Peter Zijlstra @ 2012-05-23 16:10 UTC (permalink / raw)
  To: mingo, hpa, linux-kernel, frank.arnold, akpm, torvalds, tglx,
	borislav.petkov
  Cc: linux-tip-commits

On Wed, 2012-05-23 at 09:07 -0700, tip-bot for Borislav Petkov wrote:
> +#define BIT_64(n)                      (U64_C(1) << (n))

Because writing 1ULL << n is too much work? 




^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [tip:x86/mce] x86/bitops: Move BIT_64() for a wider use
  2012-05-23 16:10   ` Peter Zijlstra
@ 2012-05-23 16:11     ` H. Peter Anvin
  2012-05-23 16:19       ` Borislav Petkov
  0 siblings, 1 reply; 28+ messages in thread
From: H. Peter Anvin @ 2012-05-23 16:11 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: mingo, linux-kernel, frank.arnold, akpm, torvalds, tglx,
	borislav.petkov, linux-tip-commits

On 05/23/2012 09:10 AM, Peter Zijlstra wrote:
> On Wed, 2012-05-23 at 09:07 -0700, tip-bot for Borislav Petkov wrote:
>> +#define BIT_64(n)                      (U64_C(1) << (n))
> 
> Because writing 1ULL << n is too much work? 
> 

Because writing 1ULL << n is broken in anything that needs to be used in
assembly, for example.

	-hpa



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [tip:x86/mce] x86/mce: Fix 32-bit build
  2012-05-23 16:08   ` [tip:x86/mce] x86/mce: " tip-bot for Borislav Petkov
@ 2012-05-23 16:18     ` Peter Zijlstra
  2012-05-23 16:23       ` Borislav Petkov
  0 siblings, 1 reply; 28+ messages in thread
From: Peter Zijlstra @ 2012-05-23 16:18 UTC (permalink / raw)
  To: mingo, hpa, linux-kernel, akpm, frank.arnold, torvalds, tglx,
	borislav.petkov
  Cc: linux-tip-commits

On Wed, 2012-05-23 at 09:08 -0700, tip-bot for Borislav Petkov wrote:
> +                                if (val & BIT_64(62)) {
> +                                       val &= ~BIT_64(62);
> +                                       wrmsrl(msrs[i], val);
>                                  } 

Wouldn't it be much better to name those things, BIT(62), BIT(18) etc.
aren't very descriptive at all.

MCE_MISC_COUNTER_PRESENT is much more descriptive than BIT(62).


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [tip:x86/mce] x86/bitops: Move BIT_64() for a wider use
  2012-05-23 16:11     ` H. Peter Anvin
@ 2012-05-23 16:19       ` Borislav Petkov
  2012-05-23 16:29         ` Peter Zijlstra
  2012-05-23 16:40         ` Linus Torvalds
  0 siblings, 2 replies; 28+ messages in thread
From: Borislav Petkov @ 2012-05-23 16:19 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Peter Zijlstra, mingo, linux-kernel, frank.arnold, akpm,
	torvalds, tglx, linux-tip-commits

On Wed, May 23, 2012 at 09:11:51AM -0700, H. Peter Anvin wrote:
> On 05/23/2012 09:10 AM, Peter Zijlstra wrote:
> > On Wed, 2012-05-23 at 09:07 -0700, tip-bot for Borislav Petkov wrote:
> >> +#define BIT_64(n)                      (U64_C(1) << (n))
> > 
> > Because writing 1ULL << n is too much work? 
> > 
> 
> Because writing 1ULL << n is broken in anything that needs to be used in
> assembly, for example.

Actually we need a BIT() macro that works both
on 32- and 64-bit. But that won't be that easy:
http://lkml.indiana.edu/hypermail/linux/kernel/1010.1/02335.html

And it should return UL for shift values < 32 and ULL otherwise.

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [tip:x86/mce] x86/mce: Fix 32-bit build
  2012-05-23 16:18     ` Peter Zijlstra
@ 2012-05-23 16:23       ` Borislav Petkov
  2012-05-23 16:31         ` Peter Zijlstra
  0 siblings, 1 reply; 28+ messages in thread
From: Borislav Petkov @ 2012-05-23 16:23 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: mingo, hpa, linux-kernel, akpm, frank.arnold, torvalds, tglx,
	linux-tip-commits

On Wed, May 23, 2012 at 06:18:18PM +0200, Peter Zijlstra wrote:
> On Wed, 2012-05-23 at 09:08 -0700, tip-bot for Borislav Petkov wrote:
> > +                                if (val & BIT_64(62)) {
> > +                                       val &= ~BIT_64(62);
> > +                                       wrmsrl(msrs[i], val);
> >                                  } 
> 
> Wouldn't it be much better to name those things, BIT(62), BIT(18) etc.
> aren't very descriptive at all.
> 
> MCE_MISC_COUNTER_PRESENT is much more descriptive than BIT(62).

I know but I have the exact bit name from the BKDG in the comment above
- CntP - and this is used only at one place - here - so no need for the
defines, IMHO.

But yeah, I see your point too.

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [tip:x86/mce] x86/bitops: Move BIT_64() for a wider use
  2012-05-23 16:19       ` Borislav Petkov
@ 2012-05-23 16:29         ` Peter Zijlstra
  2012-05-23 16:31           ` H. Peter Anvin
  2012-05-23 16:54           ` Peter Zijlstra
  2012-05-23 16:40         ` Linus Torvalds
  1 sibling, 2 replies; 28+ messages in thread
From: Peter Zijlstra @ 2012-05-23 16:29 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: H. Peter Anvin, mingo, linux-kernel, frank.arnold, akpm,
	torvalds, tglx, linux-tip-commits

On Wed, 2012-05-23 at 18:19 +0200, Borislav Petkov wrote:
> On Wed, May 23, 2012 at 09:11:51AM -0700, H. Peter Anvin wrote:
> > On 05/23/2012 09:10 AM, Peter Zijlstra wrote:
> > > On Wed, 2012-05-23 at 09:07 -0700, tip-bot for Borislav Petkov wrote:
> > >> +#define BIT_64(n)                      (U64_C(1) << (n))
> > > 
> > > Because writing 1ULL << n is too much work? 
> > > 
> > 
> > Because writing 1ULL << n is broken in anything that needs to be used in
> > assembly, for example.
> 
> Actually we need a BIT() macro that works both
> on 32- and 64-bit. But that won't be that easy:
> http://lkml.indiana.edu/hypermail/linux/kernel/1010.1/02335.html
> 
> And it should return UL for shift values < 32 and ULL otherwise.

If I remember my type rules correctly you'll get something like that
with:

#define BIT(n)	({ typeof(n) __n = (n); (__n < 32) ? (1UL << __n) : (1ULL << __n); })

That said, having it do this might be unexpected, also hpa mentioned
something about assembly magics which will obviously not work with the
above either.

Anyway, ignore me, I just thought the BIT_64() thing looked funny, but
apparently there's good reasons for it.


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [tip:x86/mce] x86/mce: Fix 32-bit build
  2012-05-23 16:23       ` Borislav Petkov
@ 2012-05-23 16:31         ` Peter Zijlstra
  2012-05-23 16:44           ` Borislav Petkov
  0 siblings, 1 reply; 28+ messages in thread
From: Peter Zijlstra @ 2012-05-23 16:31 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: mingo, hpa, linux-kernel, akpm, frank.arnold, torvalds, tglx,
	linux-tip-commits

On Wed, 2012-05-23 at 18:23 +0200, Borislav Petkov wrote:
> I know but I have the exact bit name from the BKDG in the comment above
> - CntP - and this is used only at one place - here - so no need for the
> defines, IMHO. 

Yes, but CntP is such a good name that it actually forced me to look at
the BKDG to figure out wtf it meant ;-)


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [tip:x86/mce] x86/bitops: Move BIT_64() for a wider use
  2012-05-23 16:29         ` Peter Zijlstra
@ 2012-05-23 16:31           ` H. Peter Anvin
  2012-05-23 16:43             ` Linus Torvalds
  2012-05-23 16:53             ` Borislav Petkov
  2012-05-23 16:54           ` Peter Zijlstra
  1 sibling, 2 replies; 28+ messages in thread
From: H. Peter Anvin @ 2012-05-23 16:31 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Borislav Petkov, mingo, linux-kernel, frank.arnold, akpm,
	torvalds, tglx, linux-tip-commits

>>
>> Actually we need a BIT() macro that works both
>> on 32- and 64-bit. But that won't be that easy:
>> http://lkml.indiana.edu/hypermail/linux/kernel/1010.1/02335.html
>>
>> And it should return UL for shift values < 32 and ULL otherwise.
> 

Why do you want that behavior?  That seems bizarre...

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [tip:x86/mce] x86/bitops: Move BIT_64() for a wider use
  2012-05-23 16:19       ` Borislav Petkov
  2012-05-23 16:29         ` Peter Zijlstra
@ 2012-05-23 16:40         ` Linus Torvalds
  2012-05-23 16:42           ` H. Peter Anvin
  1 sibling, 1 reply; 28+ messages in thread
From: Linus Torvalds @ 2012-05-23 16:40 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: H. Peter Anvin, Peter Zijlstra, mingo, linux-kernel,
	frank.arnold, akpm, tglx, linux-tip-commits

On Wed, May 23, 2012 at 9:19 AM, Borislav Petkov
<borislav.petkov@amd.com> wrote:
>
> Actually we need a BIT() macro that works both
> on 32- and 64-bit. But that won't be that easy:

We could use __builtin_choose_expr(), but that *only* works with constants.

So we could do this:

  static inline unsigned long bit(unsigned int x)
  {
      return 1ul << x;
  }

  static inline u64 bit64(unsigned int x)
  {
      return 1ull << x;
  }

  #define BIT(x) \
    __builtin_choose_expr((x) < 8*sizeof(unsigned long), bit(x), bit64(x))

but then you *have* to use a plain constant for the BIT() macro.
Anything else will error out in a big way. Non-constant users would
have to be modified to use bit() and bit64() instead.

And no, I tested. You apparently can't do

  #define __is_longlongshift(x) \
        (__builtin_constant_p(x) && (x) < 8*(sizeof(long)))

because while that is a compile-time constant expression, it's not
"constant enough" for __builtin_choose_expr().

                   Linus

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [tip:x86/mce] x86/bitops: Move BIT_64() for a wider use
  2012-05-23 16:40         ` Linus Torvalds
@ 2012-05-23 16:42           ` H. Peter Anvin
  2012-05-23 16:45             ` Linus Torvalds
  0 siblings, 1 reply; 28+ messages in thread
From: H. Peter Anvin @ 2012-05-23 16:42 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Borislav Petkov, Peter Zijlstra, mingo, linux-kernel,
	frank.arnold, akpm, tglx, linux-tip-commits

On 05/23/2012 09:40 AM, Linus Torvalds wrote:
> On Wed, May 23, 2012 at 9:19 AM, Borislav Petkov
> <borislav.petkov@amd.com> wrote:
>>
>> Actually we need a BIT() macro that works both
>> on 32- and 64-bit. But that won't be that easy:
> 
> We could use __builtin_choose_expr(), but that *only* works with constants.
> 

And it doesn't work in assembly.  However, I really question the assumption.

	-hpa


-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [tip:x86/mce] x86/bitops: Move BIT_64() for a wider use
  2012-05-23 16:31           ` H. Peter Anvin
@ 2012-05-23 16:43             ` Linus Torvalds
  2012-05-23 16:47               ` H. Peter Anvin
  2012-05-23 16:53             ` Borislav Petkov
  1 sibling, 1 reply; 28+ messages in thread
From: Linus Torvalds @ 2012-05-23 16:43 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Peter Zijlstra, Borislav Petkov, mingo, linux-kernel,
	frank.arnold, akpm, tglx, linux-tip-commits

On Wed, May 23, 2012 at 9:31 AM, H. Peter Anvin <hpa@zytor.com> wrote:
>>>
>>> And it should return UL for shift values < 32 and ULL otherwise.
>>
>
> Why do you want that behavior?  That seems bizarre...

We *have* to have that behavior.

A 64-bit value on a 32-bit architecture has fundamentally different
semantics than a 32-bit one.

It expands arithmetic, but it has other semantic differences too.
Think "printf()" etc. We don't want to force people to do 64-bit
arithmetic on x86-32 when they are working with BIT(0), for chrissake!

So if people make BIT(0) be a 64-bit value on a 32-bit architecture,
I'm going to run around naked with a chainsaw, and call people morons.
That's just not acceptable.

                  Linus

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [tip:x86/mce] x86/mce: Fix 32-bit build
  2012-05-23 16:31         ` Peter Zijlstra
@ 2012-05-23 16:44           ` Borislav Petkov
  2012-05-23 16:46             ` Peter Zijlstra
  0 siblings, 1 reply; 28+ messages in thread
From: Borislav Petkov @ 2012-05-23 16:44 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Borislav Petkov, mingo, hpa, linux-kernel, akpm, frank.arnold,
	torvalds, tglx, linux-tip-commits

On Wed, May 23, 2012 at 06:31:07PM +0200, Peter Zijlstra wrote:
> On Wed, 2012-05-23 at 18:23 +0200, Borislav Petkov wrote:
> > I know but I have the exact bit name from the BKDG in the comment above
> > - CntP - and this is used only at one place - here - so no need for the
> > defines, IMHO. 
> 
> Yes, but CntP is such a good name that it actually forced me to look at
> the BKDG to figure out wtf it meant ;-)

Yeah, this is quirks code - you're not supposed to look at it at all :-)

FWIW, I can change it if you feel too strongly about it, I don't really
care that much.

Thanks.

-- 
Regards/Gruss,
    Boris.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [tip:x86/mce] x86/bitops: Move BIT_64() for a wider use
  2012-05-23 16:42           ` H. Peter Anvin
@ 2012-05-23 16:45             ` Linus Torvalds
  0 siblings, 0 replies; 28+ messages in thread
From: Linus Torvalds @ 2012-05-23 16:45 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Borislav Petkov, Peter Zijlstra, mingo, linux-kernel,
	frank.arnold, akpm, tglx, linux-tip-commits

On Wed, May 23, 2012 at 9:42 AM, H. Peter Anvin <hpa@zytor.com> wrote:
> On 05/23/2012 09:40 AM, Linus Torvalds wrote:
>
> And it doesn't work in assembly.  However, I really question the assumption.

The asm case is *trivial*.

asm cannot handle anything but constant shifts anyway, so the BIT()
constness rules would remain. But since asm doesn't have any integer
types, you simply do

  #ifdef __ASSEMBLY__
    #define BIT(x) (1<<(x))
  #else
    .. the C type-morphing one ..
  #endif

As to questioning the assumption, you're simply wrong. BIT(0)
absolutely MUST NOT be a 64-bit value on a 32-bit kernel. End of
discussion. This isn't an "assumption", it's an axiom.

                    Linus

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [tip:x86/mce] x86/mce: Fix 32-bit build
  2012-05-23 16:44           ` Borislav Petkov
@ 2012-05-23 16:46             ` Peter Zijlstra
  2012-05-23 16:54               ` Borislav Petkov
  0 siblings, 1 reply; 28+ messages in thread
From: Peter Zijlstra @ 2012-05-23 16:46 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Borislav Petkov, mingo, hpa, linux-kernel, akpm, frank.arnold,
	torvalds, tglx, linux-tip-commits

On Wed, 2012-05-23 at 18:44 +0200, Borislav Petkov wrote:

> FWIW, I can change it if you feel too strongly about it, I don't really
> care that much.

I don't care much at the moment, I might if I'm ever forced to actually
read the MCE code, but until that time I'll crawl back into my
corner :-)


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [tip:x86/mce] x86/bitops: Move BIT_64() for a wider use
  2012-05-23 16:43             ` Linus Torvalds
@ 2012-05-23 16:47               ` H. Peter Anvin
  2012-05-23 16:50                 ` H. Peter Anvin
  2012-05-23 16:54                 ` Linus Torvalds
  0 siblings, 2 replies; 28+ messages in thread
From: H. Peter Anvin @ 2012-05-23 16:47 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Peter Zijlstra, Borislav Petkov, mingo, linux-kernel,
	frank.arnold, akpm, tglx, linux-tip-commits

On 05/23/2012 09:43 AM, Linus Torvalds wrote:
> On Wed, May 23, 2012 at 9:31 AM, H. Peter Anvin <hpa@zytor.com> wrote:
>>>>
>>>> And it should return UL for shift values < 32 and ULL otherwise.
>>>
>>
>> Why do you want that behavior?  That seems bizarre...
> 
> We *have* to have that behavior.
> 
> A 64-bit value on a 32-bit architecture has fundamentally different
> semantics than a 32-bit one.
> 
> It expands arithmetic, but it has other semantic differences too.
> Think "printf()" etc. We don't want to force people to do 64-bit
> arithmetic on x86-32 when they are working with BIT(0), for chrissake!
> 
> So if people make BIT(0) be a 64-bit value on a 32-bit architecture,
> I'm going to run around naked with a chainsaw, and call people morons.
> That's just not acceptable.
> 

BIT(0), okay.  I thought we were talking about BIT_64() here...

Any reason we can't just tell people to use BIT() for a native "unsigned
long" type (32/64 bits) and BIT_64() if they really want a 64-bit result?

There are good reasons for the latter.  Consider, for example:

	u64 msr;
	...
	msr &= ~BIT_64(1);

This *better* not be an unsigned 32 bit value, or we just chopped off
the upper half.  In this case and similar ones the 64-bitness of the
result really matters.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [tip:x86/mce] x86/bitops: Move BIT_64() for a wider use
  2012-05-23 16:47               ` H. Peter Anvin
@ 2012-05-23 16:50                 ` H. Peter Anvin
  2012-05-23 16:54                 ` Linus Torvalds
  1 sibling, 0 replies; 28+ messages in thread
From: H. Peter Anvin @ 2012-05-23 16:50 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Peter Zijlstra, Borislav Petkov, mingo, linux-kernel,
	frank.arnold, akpm, tglx, linux-tip-commits

On 05/23/2012 09:47 AM, H. Peter Anvin wrote:
> 
> BIT(0), okay.  I thought we were talking about BIT_64() here...
> 
> Any reason we can't just tell people to use BIT() for a native "unsigned
> long" type (32/64 bits) and BIT_64() if they really want a 64-bit result?
> 
> There are good reasons for the latter.  Consider, for example:
> 
> 	u64 msr;
> 	...
> 	msr &= ~BIT_64(1);
> 
> This *better* not be an unsigned 32 bit value, or we just chopped off
> the upper half.  In this case and similar ones the 64-bitness of the
> result really matters.
> 

To better clarify my concern: my concern is that if we make BIT() be a
DWIM type, it will appear to work in most situations.  As such, we'll
see things in headers like:

#define MSR_BLAH_FOO	BIT(31)
#define MSR_BLAH_BAR	BIT(32)

... and *almost all the time* the above will work.  But if you use
MSR_BLAH_FOO inverted, then you get truncation.

	-hpa


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [tip:x86/mce] x86/bitops: Move BIT_64() for a wider use
  2012-05-23 16:31           ` H. Peter Anvin
  2012-05-23 16:43             ` Linus Torvalds
@ 2012-05-23 16:53             ` Borislav Petkov
  2012-05-23 16:57               ` Linus Torvalds
  1 sibling, 1 reply; 28+ messages in thread
From: Borislav Petkov @ 2012-05-23 16:53 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Peter Zijlstra, Borislav Petkov, mingo, linux-kernel,
	frank.arnold, akpm, torvalds, tglx, linux-tip-commits

On Wed, May 23, 2012 at 09:31:17AM -0700, H. Peter Anvin wrote:
> >>
> >> Actually we need a BIT() macro that works both
> >> on 32- and 64-bit. But that won't be that easy:
> >> http://lkml.indiana.edu/hypermail/linux/kernel/1010.1/02335.html
> >>
> >> And it should return UL for shift values < 32 and ULL otherwise.
> > 
> 
> Why do you want that behavior?  That seems bizarre...

I forgot to say "on 32-bit" above. So the sentence should've been:

"And it should return UL for shift values < 32 and ULL otherwise on
32-bit."

How about the following completely untested chunk:

#ifdef CONFIG_64BIT
#define BIT(nr)		(UC_64(1) << (nr))
#else
#define BIT(nr)	\
({ \
	unsigned _shift = (nr);	\
 	((_shift > 32) ? (U64_C(1) << _shift) : (U32_C(1) << _shift)); \
})
#endif

Ok? Too ugly? It still changes the return type of unsigned long to ULL
for shift values >= 32 and probably Linus doesn't want that...

Hmm.

-- 
Regards/Gruss,
    Boris.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [tip:x86/mce] x86/bitops: Move BIT_64() for a wider use
  2012-05-23 16:47               ` H. Peter Anvin
  2012-05-23 16:50                 ` H. Peter Anvin
@ 2012-05-23 16:54                 ` Linus Torvalds
  2012-05-23 16:55                   ` H. Peter Anvin
  1 sibling, 1 reply; 28+ messages in thread
From: Linus Torvalds @ 2012-05-23 16:54 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Peter Zijlstra, Borislav Petkov, mingo, linux-kernel,
	frank.arnold, akpm, tglx, linux-tip-commits

On Wed, May 23, 2012 at 9:47 AM, H. Peter Anvin <hpa@zytor.com> wrote:
>
> Any reason we can't just tell people to use BIT() for a native "unsigned
> long" type (32/64 bits) and BIT_64() if they really want a 64-bit result?

Well, that's what we're doing now. And it seems to have resulted in
repeated bugs for architectures where most of the developers run on
64-bit machines, but the same code is actually supposed to work on
32-bit too (ie x86).

                   Linus

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [tip:x86/mce] x86/mce: Fix 32-bit build
  2012-05-23 16:46             ` Peter Zijlstra
@ 2012-05-23 16:54               ` Borislav Petkov
  0 siblings, 0 replies; 28+ messages in thread
From: Borislav Petkov @ 2012-05-23 16:54 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Borislav Petkov, mingo, hpa, linux-kernel, akpm, frank.arnold,
	torvalds, tglx, linux-tip-commits

On Wed, May 23, 2012 at 06:46:45PM +0200, Peter Zijlstra wrote:
> On Wed, 2012-05-23 at 18:44 +0200, Borislav Petkov wrote:
> 
> > FWIW, I can change it if you feel too strongly about it, I don't really
> > care that much.
> 
> I don't care much at the moment, I might if I'm ever forced to actually
> read the MCE code, but until that time I'll crawl back into my
> corner :-)

Well, our job is to make it as unreadable as possible until that time
comes.

:-)

-- 
Regards/Gruss,
    Boris.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [tip:x86/mce] x86/bitops: Move BIT_64() for a wider use
  2012-05-23 16:29         ` Peter Zijlstra
  2012-05-23 16:31           ` H. Peter Anvin
@ 2012-05-23 16:54           ` Peter Zijlstra
  1 sibling, 0 replies; 28+ messages in thread
From: Peter Zijlstra @ 2012-05-23 16:54 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: H. Peter Anvin, mingo, linux-kernel, frank.arnold, akpm,
	torvalds, tglx, linux-tip-commits

On Wed, 2012-05-23 at 18:29 +0200, Peter Zijlstra wrote:
> If I remember my type rules correctly you'll get something like that
> with:
> 
> #define BIT(n)  ({ typeof(n) __n = (n); (__n < 32) ? (1UL << __n) : (1ULL << __n); })

OK, that doesn't work. Memory played tricks on me. Now I'll keep
wondering what was special about ?: and how I've seen it abused.


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [tip:x86/mce] x86/bitops: Move BIT_64() for a wider use
  2012-05-23 16:54                 ` Linus Torvalds
@ 2012-05-23 16:55                   ` H. Peter Anvin
  0 siblings, 0 replies; 28+ messages in thread
From: H. Peter Anvin @ 2012-05-23 16:55 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Peter Zijlstra, Borislav Petkov, mingo, linux-kernel,
	frank.arnold, akpm, tglx, linux-tip-commits

On 05/23/2012 09:54 AM, Linus Torvalds wrote:
> On Wed, May 23, 2012 at 9:47 AM, H. Peter Anvin <hpa@zytor.com> wrote:
>>
>> Any reason we can't just tell people to use BIT() for a native "unsigned
>> long" type (32/64 bits) and BIT_64() if they really want a 64-bit result?
> 
> Well, that's what we're doing now. And it seems to have resulted in
> repeated bugs for architectures where most of the developers run on
> 64-bit machines, but the same code is actually supposed to work on
> 32-bit too (ie x86).
> 

Yes, but I fear that this will result in more subtle bugs which will
therefore be even harder to detect and diagnose.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [tip:x86/mce] x86/bitops: Move BIT_64() for a wider use
  2012-05-23 16:53             ` Borislav Petkov
@ 2012-05-23 16:57               ` Linus Torvalds
  2012-05-23 17:11                 ` Borislav Petkov
  0 siblings, 1 reply; 28+ messages in thread
From: Linus Torvalds @ 2012-05-23 16:57 UTC (permalink / raw)
  To: Borislav Petkov, H. Peter Anvin, Peter Zijlstra, Borislav Petkov,
	mingo, linux-kernel, frank.arnold, akpm, torvalds, tglx,
	linux-tip-commits

On Wed, May 23, 2012 at 9:53 AM, Borislav Petkov <bp@alien8.de> wrote:
>
> How about the following completely untested chunk:

No, you can't do that. All standard C operations will return *one*
type. That very much includes the ternary ?: operator.

The *only* ways I know of to get two types are

 - C preprocessor stuff, ie

     #define BIT(x)  __BIT_##x

   and then just enumerate all the 64 cases. This is portable, but it gets old.

 - using __builtin_choose_expr(), which actually allows the two
expressions to have different types, but requires a very strict
compile-time constant (ie you cannot rely on the optimizer making it a
constant - because it needs to choose the expression before the
optimizer runs)

There might be some other magic gcc extension, of course.

                 Linus

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [tip:x86/mce] x86/bitops: Move BIT_64() for a wider use
  2012-05-23 16:57               ` Linus Torvalds
@ 2012-05-23 17:11                 ` Borislav Petkov
  2012-05-23 17:37                   ` Linus Torvalds
  0 siblings, 1 reply; 28+ messages in thread
From: Borislav Petkov @ 2012-05-23 17:11 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: H. Peter Anvin, Peter Zijlstra, Borislav Petkov, mingo,
	linux-kernel, frank.arnold, akpm, tglx, linux-tip-commits

On Wed, May 23, 2012 at 09:57:47AM -0700, Linus Torvalds wrote:
> On Wed, May 23, 2012 at 9:53 AM, Borislav Petkov <bp@alien8.de> wrote:
> >
> > How about the following completely untested chunk:
> 
> No, you can't do that. All standard C operations will return *one*
> type. That very much includes the ternary ?: operator.

And, in addition, hpa's example won't work too:

	u64 msr = ~BIT(1);

> The *only* ways I know of to get two types are
> 
>  - C preprocessor stuff, ie
> 
>      #define BIT(x)  __BIT_##x
> 
>    and then just enumerate all the 64 cases. This is portable, but it gets old.

Nah, that's ugly.

>  - using __builtin_choose_expr(), which actually allows the two
> expressions to have different types, but requires a very strict
> compile-time constant (ie you cannot rely on the optimizer making it a
> constant - because it needs to choose the expression before the
> optimizer runs)
> 
> There might be some other magic gcc extension, of course.

Hmm and if not, it looks like BIT_64 is the easiest and most readable
thing we can do.

Oh well.

-- 
Regards/Gruss,
    Boris.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [tip:x86/mce] x86/bitops: Move BIT_64() for a wider use
  2012-05-23 17:11                 ` Borislav Petkov
@ 2012-05-23 17:37                   ` Linus Torvalds
  0 siblings, 0 replies; 28+ messages in thread
From: Linus Torvalds @ 2012-05-23 17:37 UTC (permalink / raw)
  To: Borislav Petkov, Linus Torvalds, H. Peter Anvin, Peter Zijlstra,
	Borislav Petkov, mingo, linux-kernel, frank.arnold, akpm, tglx,
	linux-tip-commits

On Wed, May 23, 2012 at 10:11 AM, Borislav Petkov <bp@alien8.de> wrote:
>
> And, in addition, hpa's example won't work too:
>
>        u64 msr = ~BIT(1);

Yes, that's a good example of something that can be problematic, and
is fundamentally different on 32-bit and 64-bit.

So maybe BIT() unconditionally being long, and BIT_64() being u64
simply is the right model.

                    Linus

^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2012-05-23 17:37 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-05-22 10:53 [PATCH 1/2] x86, bitops: Move BIT_64 for a wider use Borislav Petkov
2012-05-22 10:53 ` [PATCH 2/2] x86, MCE: Fix 32-bit build Borislav Petkov
2012-05-23 16:08   ` [tip:x86/mce] x86/mce: " tip-bot for Borislav Petkov
2012-05-23 16:18     ` Peter Zijlstra
2012-05-23 16:23       ` Borislav Petkov
2012-05-23 16:31         ` Peter Zijlstra
2012-05-23 16:44           ` Borislav Petkov
2012-05-23 16:46             ` Peter Zijlstra
2012-05-23 16:54               ` Borislav Petkov
2012-05-23 16:07 ` [tip:x86/mce] x86/bitops: Move BIT_64() for a wider use tip-bot for Borislav Petkov
2012-05-23 16:10   ` Peter Zijlstra
2012-05-23 16:11     ` H. Peter Anvin
2012-05-23 16:19       ` Borislav Petkov
2012-05-23 16:29         ` Peter Zijlstra
2012-05-23 16:31           ` H. Peter Anvin
2012-05-23 16:43             ` Linus Torvalds
2012-05-23 16:47               ` H. Peter Anvin
2012-05-23 16:50                 ` H. Peter Anvin
2012-05-23 16:54                 ` Linus Torvalds
2012-05-23 16:55                   ` H. Peter Anvin
2012-05-23 16:53             ` Borislav Petkov
2012-05-23 16:57               ` Linus Torvalds
2012-05-23 17:11                 ` Borislav Petkov
2012-05-23 17:37                   ` Linus Torvalds
2012-05-23 16:54           ` Peter Zijlstra
2012-05-23 16:40         ` Linus Torvalds
2012-05-23 16:42           ` H. Peter Anvin
2012-05-23 16:45             ` Linus Torvalds

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.