Re: [Xen-devel] [PATCH RFC 25/31] xen/x86: Common infrastructure for levelling context switching

2016-01-22 Thread Andrew Cooper 
On 22/01/16 08:56, Jan Beulich wrote:
 On 16.12.15 at 22:24,  wrote:
>> --- a/xen/arch/x86/cpu/common.c
>> +++ b/xen/arch/x86/cpu/common.c
>> @@ -35,6 +35,9 @@ integer_param("cpuid_mask_ext_edx", 
>> opt_cpuid_mask_ext_edx);
>>  unsigned int __initdata expected_levelling_cap;
>>  unsigned int __read_mostly levelling_caps;
>>  
>> +DEFINE_PER_CPU(struct cpumasks, cpumasks);
>> +struct cpumasks __read_mostly cpumask_defaults;
> Any reason these can't be introduced when first used? (The question
> really applies to the rest of this patch too, I guess.)

The subsequent two patches are sufficiently complicated in their own
right that I deliberately pulled these bits out.

>
>> --- a/xen/include/asm-x86/processor.h
>> +++ b/xen/include/asm-x86/processor.h
>> @@ -585,6 +585,21 @@ int microcode_resume_cpu(unsigned int cpu);
>>   */
>>  extern unsigned int expected_levelling_cap, levelling_caps;
>>  
>> +struct cpumasks
>> +{
>> +uint64_t _1cd;
>> +uint64_t e1cd;
>> +uint64_t Da1;
>> +uint64_t _6c;
>> +uint64_t _7ab0;
>> +};
> While I see the need for these underscore prefixes with the
> current naming scheme, perhaps it would be better to make
> this fully consistent with the acronym #define-s, by e.g.
> prefixing the base ones with 'b'? Such full naming consistency
> would allow for string concatenation in macros, should such a
> possibility ever arise, no matter whether manifest constants
> or structure members here are needed to be accessed.
>
> As to the name of the structure itself - perhaps better
> cpuidmasks?

Ok.

~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH RFC 25/31] xen/x86: Common infrastructure for levelling context switching

2016-01-22 Thread Jan Beulich 
>>> On 16.12.15 at 22:24,  wrote:
> --- a/xen/arch/x86/cpu/common.c
> +++ b/xen/arch/x86/cpu/common.c
> @@ -35,6 +35,9 @@ integer_param("cpuid_mask_ext_edx", opt_cpuid_mask_ext_edx);
>  unsigned int __initdata expected_levelling_cap;
>  unsigned int __read_mostly levelling_caps;
>  
> +DEFINE_PER_CPU(struct cpumasks, cpumasks);
> +struct cpumasks __read_mostly cpumask_defaults;

Any reason these can't be introduced when first used? (The question
really applies to the rest of this patch too, I guess.)

> --- a/xen/include/asm-x86/processor.h
> +++ b/xen/include/asm-x86/processor.h
> @@ -585,6 +585,21 @@ int microcode_resume_cpu(unsigned int cpu);
>   */
>  extern unsigned int expected_levelling_cap, levelling_caps;
>  
> +struct cpumasks
> +{
> +uint64_t _1cd;
> +uint64_t e1cd;
> +uint64_t Da1;
> +uint64_t _6c;
> +uint64_t _7ab0;
> +};

While I see the need for these underscore prefixes with the
current naming scheme, perhaps it would be better to make
this fully consistent with the acronym #define-s, by e.g.
prefixing the base ones with 'b'? Such full naming consistency
would allow for string concatenation in macros, should such a
possibility ever arise, no matter whether manifest constants
or structure members here are needed to be accessed.

As to the name of the structure itself - perhaps better
cpuidmasks?

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH RFC 25/31] xen/x86: Common infrastructure for levelling context switching

2015-12-16 Thread Andrew Cooper 
Future changes will make use of this to advertise availability of levelling
support, and to provide lazy context switching.

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
---
 xen/arch/x86/cpu/common.c|  3 +++
 xen/include/asm-x86/cpufeature.h |  1 +
 xen/include/asm-x86/processor.h  | 15 +++
 3 files changed, 19 insertions(+)

diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
index 2ad85e0..896a579 100644
--- a/xen/arch/x86/cpu/common.c
+++ b/xen/arch/x86/cpu/common.c
@@ -35,6 +35,9 @@ integer_param("cpuid_mask_ext_edx", opt_cpuid_mask_ext_edx);
 unsigned int __initdata expected_levelling_cap;
 unsigned int __read_mostly levelling_caps;
 
+DEFINE_PER_CPU(struct cpumasks, cpumasks);
+struct cpumasks __read_mostly cpumask_defaults;
+
 const struct cpu_dev *__read_mostly cpu_devs[X86_VENDOR_NUM] = {};
 
 unsigned int paddr_bits __read_mostly = 36;
diff --git a/xen/include/asm-x86/cpufeature.h b/xen/include/asm-x86/cpufeature.h
index 547ed7d..8d48290 100644
--- a/xen/include/asm-x86/cpufeature.h
+++ b/xen/include/asm-x86/cpufeature.h
@@ -111,6 +111,7 @@
 #define cpu_has_xsavec boot_cpu_has(X86_FEATURE_XSAVEC)
 #define cpu_has_xgetbv1boot_cpu_has(X86_FEATURE_XGETBV1)
 #define cpu_has_xsaves boot_cpu_has(X86_FEATURE_XSAVES)
+#define cpu_has_hypervisor boot_cpu_has(X86_FEATURE_HYPERVISOR)
 
 enum _cache_type {
 CACHE_TYPE_NULL = 0,
diff --git a/xen/include/asm-x86/processor.h b/xen/include/asm-x86/processor.h
index b2c3653..6d2f09b 100644
--- a/xen/include/asm-x86/processor.h
+++ b/xen/include/asm-x86/processor.h
@@ -585,6 +585,21 @@ int microcode_resume_cpu(unsigned int cpu);
  */
 extern unsigned int expected_levelling_cap, levelling_caps;
 
+struct cpumasks
+{
+uint64_t _1cd;
+uint64_t e1cd;
+uint64_t Da1;
+uint64_t _6c;
+uint64_t _7ab0;
+};
+
+/* Per CPU shadows of masking MSR values, for lazy context switching. */
+DECLARE_PER_CPU(struct cpumasks, cpumasks);
+
+/* Default masking MSR values, calculated at boot. */
+extern struct cpumasks cpumask_defaults;
+
 enum get_cpu_vendor {
gcv_host_early,
gcv_host_late,
-- 
2.1.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel