All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] x86/faulting: Use formal defines instead of opencoded bits
@ 2014-02-25 11:02 Andrew Cooper
  2014-02-25 11:21 ` Jan Beulich
  0 siblings, 1 reply; 6+ messages in thread
From: Andrew Cooper @ 2014-02-25 11:02 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Keir Fraser, Jan Beulich

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <JBeulich@suse.com>
---
 xen/arch/x86/cpu/intel.c        |    7 ++++---
 xen/include/asm-x86/msr-index.h |    5 +++++
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/cpu/intel.c b/xen/arch/x86/cpu/intel.c
index 27fe762..808ed8d 100644
--- a/xen/arch/x86/cpu/intel.c
+++ b/xen/arch/x86/cpu/intel.c
@@ -21,7 +21,8 @@
 static unsigned int probe_intel_cpuid_faulting(void)
 {
 	uint64_t x;
-	return !rdmsr_safe(MSR_INTEL_PLATFORM_INFO, x) && (x & (1u<<31));
+	return !rdmsr_safe(MSR_INTEL_PLATFORM_INFO, x) &&
+            (x & PLATFORM_INFO_CPUID_FAULTING);
 }
 
 static DEFINE_PER_CPU(bool_t, cpuid_faulting_enabled);
@@ -34,9 +35,9 @@ void set_cpuid_faulting(bool_t enable)
 		return;
 
 	rdmsr(MSR_INTEL_MISC_FEATURES_ENABLES, lo, hi);
-	lo &= ~1;
+	lo &= ~MISC_FEATURES_CPUID_FAULTING;
 	if (enable)
-		lo |= 1;
+		lo |= MISC_FEATURES_CPUID_FAULTING;
 	wrmsr(MSR_INTEL_MISC_FEATURES_ENABLES, lo, hi);
 
 	this_cpu(cpuid_faulting_enabled) = enable;
diff --git a/xen/include/asm-x86/msr-index.h b/xen/include/asm-x86/msr-index.h
index 86b7d64..15a7e13 100644
--- a/xen/include/asm-x86/msr-index.h
+++ b/xen/include/asm-x86/msr-index.h
@@ -486,7 +486,12 @@
 
 /* Intel cpuid faulting MSRs */
 #define MSR_INTEL_PLATFORM_INFO		0x000000ce
+#define _PLATFORM_INFO_CPUID_FAULTING	31
+#define PLATFORM_INFO_CPUID_FAULTING	(1ULL << _PLATFORM_INFO_CPUID_FAULTING)
+
 #define MSR_INTEL_MISC_FEATURES_ENABLES	0x00000140
+#define _MISC_FEATURES_CPUID_FAULTING	0
+#define MISC_FEATURES_CPUID_FAULTING	(1ULL << _MISC_FEATURES_CPUID_FAULTING)
 
 /* Geode defined MSRs */
 #define MSR_GEODE_BUSCONT_CONF0		0x00001900
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] x86/faulting: Use formal defines instead of opencoded bits
  2014-02-25 11:02 [PATCH] x86/faulting: Use formal defines instead of opencoded bits Andrew Cooper
@ 2014-02-25 11:21 ` Jan Beulich
  2014-02-25 11:23   ` Andrew Cooper
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2014-02-25 11:21 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Keir Fraser, Xen-devel

>>> On 25.02.14 at 12:02, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> --- a/xen/arch/x86/cpu/intel.c
> +++ b/xen/arch/x86/cpu/intel.c
> @@ -21,7 +21,8 @@
>  static unsigned int probe_intel_cpuid_faulting(void)
>  {
>  	uint64_t x;
> -	return !rdmsr_safe(MSR_INTEL_PLATFORM_INFO, x) && (x & (1u<<31));
> +	return !rdmsr_safe(MSR_INTEL_PLATFORM_INFO, x) &&
> +            (x & PLATFORM_INFO_CPUID_FAULTING);

Indentation (a single hard tab ought to come first at least).

> --- a/xen/include/asm-x86/msr-index.h
> +++ b/xen/include/asm-x86/msr-index.h
> @@ -486,7 +486,12 @@
>  
>  /* Intel cpuid faulting MSRs */
>  #define MSR_INTEL_PLATFORM_INFO		0x000000ce
> +#define _PLATFORM_INFO_CPUID_FAULTING	31
> +#define PLATFORM_INFO_CPUID_FAULTING	(1ULL << _PLATFORM_INFO_CPUID_FAULTING)
> +
>  #define MSR_INTEL_MISC_FEATURES_ENABLES	0x00000140
> +#define _MISC_FEATURES_CPUID_FAULTING	0
> +#define MISC_FEATURES_CPUID_FAULTING	(1ULL << _MISC_FEATURES_CPUID_FAULTING)

I wonder whether, from a name space pov, it wouldn't be better
if these new constants had at least MSR_ as additional prefix. Both
are rather generic without...

Jan

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] x86/faulting: Use formal defines instead of opencoded bits
  2014-02-25 11:21 ` Jan Beulich
@ 2014-02-25 11:23   ` Andrew Cooper
  2014-02-25 11:34     ` Jan Beulich
  0 siblings, 1 reply; 6+ messages in thread
From: Andrew Cooper @ 2014-02-25 11:23 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Keir Fraser, Xen-devel

On 25/02/14 11:21, Jan Beulich wrote:
>>>> On 25.02.14 at 12:02, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>> --- a/xen/arch/x86/cpu/intel.c
>> +++ b/xen/arch/x86/cpu/intel.c
>> @@ -21,7 +21,8 @@
>>  static unsigned int probe_intel_cpuid_faulting(void)
>>  {
>>  	uint64_t x;
>> -	return !rdmsr_safe(MSR_INTEL_PLATFORM_INFO, x) && (x & (1u<<31));
>> +	return !rdmsr_safe(MSR_INTEL_PLATFORM_INFO, x) &&
>> +            (x & PLATFORM_INFO_CPUID_FAULTING);
> Indentation (a single hard tab ought to come first at least).
>
>> --- a/xen/include/asm-x86/msr-index.h
>> +++ b/xen/include/asm-x86/msr-index.h
>> @@ -486,7 +486,12 @@
>>  
>>  /* Intel cpuid faulting MSRs */
>>  #define MSR_INTEL_PLATFORM_INFO		0x000000ce
>> +#define _PLATFORM_INFO_CPUID_FAULTING	31
>> +#define PLATFORM_INFO_CPUID_FAULTING	(1ULL << _PLATFORM_INFO_CPUID_FAULTING)
>> +
>>  #define MSR_INTEL_MISC_FEATURES_ENABLES	0x00000140
>> +#define _MISC_FEATURES_CPUID_FAULTING	0
>> +#define MISC_FEATURES_CPUID_FAULTING	(1ULL << _MISC_FEATURES_CPUID_FAULTING)
> I wonder whether, from a name space pov, it wouldn't be better
> if these new constants had at least MSR_ as additional prefix. Both
> are rather generic without...
>
> Jan
>

How about MSR_INTEL_ to match their MSR number names?

~Andrew

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] x86/faulting: Use formal defines instead of opencoded bits
  2014-02-25 11:23   ` Andrew Cooper
@ 2014-02-25 11:34     ` Jan Beulich
  2014-02-25 11:39       ` [PATCH v2] " Andrew Cooper
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2014-02-25 11:34 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Keir Fraser, Xen-devel

>>> On 25.02.14 at 12:23, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> On 25/02/14 11:21, Jan Beulich wrote:
>>>>> On 25.02.14 at 12:02, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>>> --- a/xen/arch/x86/cpu/intel.c
>>> +++ b/xen/arch/x86/cpu/intel.c
>>> @@ -21,7 +21,8 @@
>>>  static unsigned int probe_intel_cpuid_faulting(void)
>>>  {
>>>  	uint64_t x;
>>> -	return !rdmsr_safe(MSR_INTEL_PLATFORM_INFO, x) && (x & (1u<<31));
>>> +	return !rdmsr_safe(MSR_INTEL_PLATFORM_INFO, x) &&
>>> +            (x & PLATFORM_INFO_CPUID_FAULTING);
>> Indentation (a single hard tab ought to come first at least).
>>
>>> --- a/xen/include/asm-x86/msr-index.h
>>> +++ b/xen/include/asm-x86/msr-index.h
>>> @@ -486,7 +486,12 @@
>>>  
>>>  /* Intel cpuid faulting MSRs */
>>>  #define MSR_INTEL_PLATFORM_INFO		0x000000ce
>>> +#define _PLATFORM_INFO_CPUID_FAULTING	31
>>> +#define PLATFORM_INFO_CPUID_FAULTING	(1ULL << _PLATFORM_INFO_CPUID_FAULTING)
>>> +
>>>  #define MSR_INTEL_MISC_FEATURES_ENABLES	0x00000140
>>> +#define _MISC_FEATURES_CPUID_FAULTING	0
>>> +#define MISC_FEATURES_CPUID_FAULTING	(1ULL << _MISC_FEATURES_CPUID_FAULTING)
>> I wonder whether, from a name space pov, it wouldn't be better
>> if these new constants had at least MSR_ as additional prefix. Both
>> are rather generic without...
> 
> How about MSR_INTEL_ to match their MSR number names?

I'd be fine with that. I merely didn't require it to be the full name
because it gets rather long. But with only a single use site that's
probably acceptable.

Jan

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH v2] x86/faulting: Use formal defines instead of opencoded bits
  2014-02-25 11:34     ` Jan Beulich
@ 2014-02-25 11:39       ` Andrew Cooper
  2014-02-25 18:01         ` Keir Fraser
  0 siblings, 1 reply; 6+ messages in thread
From: Andrew Cooper @ 2014-02-25 11:39 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Keir Fraser, Jan Beulich

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <JBeulich@suse.com>

---

Changes in v2:
 * Use MSR_ prefix for names
---
 xen/arch/x86/cpu/intel.c        |    7 ++++---
 xen/include/asm-x86/msr-index.h |    5 +++++
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/cpu/intel.c b/xen/arch/x86/cpu/intel.c
index 27fe762..9e02cf6 100644
--- a/xen/arch/x86/cpu/intel.c
+++ b/xen/arch/x86/cpu/intel.c
@@ -21,7 +21,8 @@
 static unsigned int probe_intel_cpuid_faulting(void)
 {
 	uint64_t x;
-	return !rdmsr_safe(MSR_INTEL_PLATFORM_INFO, x) && (x & (1u<<31));
+	return !rdmsr_safe(MSR_INTEL_PLATFORM_INFO, x) &&
+		(x & MSR_PLATFORM_INFO_CPUID_FAULTING);
 }
 
 static DEFINE_PER_CPU(bool_t, cpuid_faulting_enabled);
@@ -34,9 +35,9 @@ void set_cpuid_faulting(bool_t enable)
 		return;
 
 	rdmsr(MSR_INTEL_MISC_FEATURES_ENABLES, lo, hi);
-	lo &= ~1;
+	lo &= ~MSR_MISC_FEATURES_CPUID_FAULTING;
 	if (enable)
-		lo |= 1;
+		lo |= MSR_MISC_FEATURES_CPUID_FAULTING;
 	wrmsr(MSR_INTEL_MISC_FEATURES_ENABLES, lo, hi);
 
 	this_cpu(cpuid_faulting_enabled) = enable;
diff --git a/xen/include/asm-x86/msr-index.h b/xen/include/asm-x86/msr-index.h
index 86b7d64..3269c43 100644
--- a/xen/include/asm-x86/msr-index.h
+++ b/xen/include/asm-x86/msr-index.h
@@ -486,7 +486,12 @@
 
 /* Intel cpuid faulting MSRs */
 #define MSR_INTEL_PLATFORM_INFO		0x000000ce
+#define _MSR_PLATFORM_INFO_CPUID_FAULTING	31
+#define MSR_PLATFORM_INFO_CPUID_FAULTING	(1ULL << _MSR_PLATFORM_INFO_CPUID_FAULTING)
+
 #define MSR_INTEL_MISC_FEATURES_ENABLES	0x00000140
+#define _MSR_MISC_FEATURES_CPUID_FAULTING	0
+#define MSR_MISC_FEATURES_CPUID_FAULTING	(1ULL << _MSR_MISC_FEATURES_CPUID_FAULTING)
 
 /* Geode defined MSRs */
 #define MSR_GEODE_BUSCONT_CONF0		0x00001900
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH v2] x86/faulting: Use formal defines instead of opencoded bits
  2014-02-25 11:39       ` [PATCH v2] " Andrew Cooper
@ 2014-02-25 18:01         ` Keir Fraser
  0 siblings, 0 replies; 6+ messages in thread
From: Keir Fraser @ 2014-02-25 18:01 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Jan Beulich, Xen-devel

Andrew Cooper wrote:
> Signed-off-by: Andrew Cooper<andrew.cooper3@citrix.com>
> CC: Keir Fraser<keir@xen.org>
> CC: Jan Beulich<JBeulich@suse.com>

Acked-by: Keir Fraser <keir@xen.org>

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-02-25 18:01 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-25 11:02 [PATCH] x86/faulting: Use formal defines instead of opencoded bits Andrew Cooper
2014-02-25 11:21 ` Jan Beulich
2014-02-25 11:23   ` Andrew Cooper
2014-02-25 11:34     ` Jan Beulich
2014-02-25 11:39       ` [PATCH v2] " Andrew Cooper
2014-02-25 18:01         ` Keir Fraser

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.