[PATCH 1/2] cpu: common: make clearcpuid option take bits list

[PATCH 1/2] cpu: common: make clearcpuid option take bits list

[Vladimir Davydov]

It is more convenient to write 'clearcpuid=147,148,...' than
'clearcpuid=147 clearcpuid=148 ...'
---
 arch/x86/kernel/cpu/common.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 6b9333b..8ffe1b9 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1012,10 +1012,10 @@ static __init int setup_disablecpuid(char *arg)
 {
 	int bit;
 
-	if (get_option(&arg, &bit) && bit < NCAPINTS*32)
-		setup_clear_cpu_cap(bit);
-	else
-		return 0;
+	while (get_option(&arg, &bit)) {
+		if (bit >= 0 && bit < NCAPINTS*32)
+			setup_clear_cpu_cap(bit);
+	}
 
 	return 1;
 }
-- 
1.7.1

[PATCH 2/2] cpu: intel, amd: mask cleared cpuid features

[Vladimir Davydov]

If 'clearcpuid=N' is specified in boot options, CPU feature #N won't be
reported in /proc/cpuinfo and used by the kernel. However, if a
userpsace process checks CPU features directly using the cpuid
instruction, it will be reported about all features supported by the CPU
irrespective of what features are cleared.

The patch makes the clearcpuid boot option not only clear CPU features
in kernel but also mask them in hardware for Intel and AMD CPUs that
support it so that the features cleared won't be reported even by the
cpuid instruction.

This can be useful for migration of virtual machines managed by
hypervisors that do not support/use Intel VT/AMD-V hardware-assisted
virtualization technology.

If CPUID masking is supported, this will be reported in
/proc/cpuinfo:flags as 'cpuidmask'.
---
 arch/x86/include/asm/cpufeature.h |    1 +
 arch/x86/kernel/cpu/amd.c         |   22 ++++++++++++++
 arch/x86/kernel/cpu/intel.c       |   59 +++++++++++++++++++++++++++++++++++++
 3 files changed, 82 insertions(+), 0 deletions(-)

diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index f91e80f..6f061ea 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -97,6 +97,7 @@
 #define X86_FEATURE_EXTD_APICID	(3*32+26) /* has extended APICID (8 bits) */
 #define X86_FEATURE_AMD_DCM     (3*32+27) /* multi-node processor */
 #define X86_FEATURE_APERFMPERF	(3*32+28) /* APERFMPERF */
+#define X86_FEATURE_CPUIDMASK	(3*32+29) /* CPUID feature masking */
 
 /* Intel-defined CPU features, CPUID level 0x00000001 (ecx), word 4 */
 #define X86_FEATURE_XMM3	(4*32+ 0) /* "pni" SSE-3 */
diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index 146bb62..ddc8ea2 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -482,6 +482,26 @@ static void __cpuinit early_init_amd(struct cpuinfo_x86 *c)
 #endif
 }
 
+#define MSR_AMD_CPUID_FEATURES_OVERRIDE		0xc0011004
+#define MSR_AMD_EXT_CPUID_FEATURES_OVERRIDE	0xc0011005
+
+static void __cpuinit amd_mask_cpu_caps(struct cpuinfo_x86 *c)
+{
+	u64 val;
+
+	if (c->x86 >= 0x0f) {
+		set_cpu_cap(c, X86_FEATURE_CPUIDMASK);
+
+		rdmsrl_amd_safe(MSR_AMD_CPUID_FEATURES_OVERRIDE, &val);
+		val &= ~((u64)cpu_caps_cleared[4] << 32 | cpu_caps_cleared[0]);
+		wrmsrl_amd_safe(MSR_AMD_CPUID_FEATURES_OVERRIDE, val);
+
+		rdmsrl_amd_safe(MSR_AMD_EXT_CPUID_FEATURES_OVERRIDE, &val);
+		val &= ~((u64)cpu_caps_cleared[6] << 32 | cpu_caps_cleared[1]);
+		wrmsrl_amd_safe(MSR_AMD_EXT_CPUID_FEATURES_OVERRIDE, val);
+	}
+}
+
 static void __cpuinit init_amd(struct cpuinfo_x86 *c)
 {
 	u32 dummy;
@@ -684,6 +704,8 @@ static void __cpuinit init_amd(struct cpuinfo_x86 *c)
 	}
 
 	rdmsr_safe(MSR_AMD64_PATCH_LEVEL, &c->microcode, &dummy);
+
+	amd_mask_cpu_caps(c);
 }
 
 #ifdef CONFIG_X86_32
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index 3e6ff6c..1df1da5 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -358,6 +358,63 @@ static void __cpuinit detect_vmx_virtcap(struct cpuinfo_x86 *c)
 	}
 }
 
+static unsigned int __cpuinit intel_cpuid_mask_msr(struct cpuinfo_x86 *c,
+					unsigned int *msr_ext_cpuid_mask)
+{
+	unsigned int msr, msr_ext;
+
+	msr = msr_ext = 0;
+
+	switch (c->x86_model) {
+	case 0x17:
+	case 0x1D:
+		msr = 0x478;
+		break;
+	case 0x1A:
+	case 0x1E:
+	case 0x1F:
+	case 0x25:
+	case 0x2C:
+	case 0x2E:
+	case 0x2F:
+		msr = 0x130;
+		msr_ext = 0x131;
+		break;
+	case 0x2A:
+		msr = 0x132;
+		msr_ext = 0x133;
+		break;
+	}
+
+	if (msr_ext_cpuid_mask)
+		*msr_ext_cpuid_mask = msr_ext;
+
+	return msr;
+}
+
+static void __cpuinit intel_mask_cpu_caps(struct cpuinfo_x86 *c)
+{
+	u32 low, high;
+	unsigned int msr_cpuid_mask, msr_ext_cpuid_mask;
+
+	msr_cpuid_mask = intel_cpuid_mask_msr(c, &msr_ext_cpuid_mask);
+	if (msr_cpuid_mask) {
+		set_cpu_cap(c, X86_FEATURE_CPUIDMASK);
+
+		rdmsr(msr_cpuid_mask, low, high);
+		low &= ~cpu_caps_cleared[4];
+		high &= ~cpu_caps_cleared[0];
+		wrmsr(msr_cpuid_mask, low, high);
+
+		if (msr_ext_cpuid_mask != 0) {
+			rdmsr(msr_ext_cpuid_mask, low, high);
+			low &= ~cpu_caps_cleared[6];
+			high &= ~cpu_caps_cleared[1];
+			wrmsr(msr_ext_cpuid_mask, low, high);
+		}
+	}
+}
+
 static void __cpuinit init_intel(struct cpuinfo_x86 *c)
 {
 	unsigned int l2 = 0;
@@ -474,6 +531,8 @@ static void __cpuinit init_intel(struct cpuinfo_x86 *c)
 			wrmsrl(MSR_IA32_ENERGY_PERF_BIAS, epb);
 		}
 	}
+
+	intel_mask_cpu_caps(c);
 }
 
 #ifdef CONFIG_X86_32
-- 
1.7.1

Re: [PATCH 2/2] cpu: intel, amd: mask cleared cpuid features

[Andi Kleen]

> +					unsigned int *msr_ext_cpuid_mask)
> +{
> +	unsigned int msr, msr_ext;
> +
> +	msr = msr_ext = 0;
> +
> +	switch (c->x86_model) {

You have to check the family too.

> +
> +	return msr;
> +}
> +
> +static void __cpuinit intel_mask_cpu_caps(struct cpuinfo_x86 *c)
> +{
> +	u32 low, high;
> +	unsigned int msr_cpuid_mask, msr_ext_cpuid_mask;
> +
> +	msr_cpuid_mask = intel_cpuid_mask_msr(c, &msr_ext_cpuid_mask);
> +	if (msr_cpuid_mask) {
> +		set_cpu_cap(c, X86_FEATURE_CPUIDMASK);
> +
> +		rdmsr(msr_cpuid_mask, low, high);

And please use rdmsrl(), rdmsr() is so 90ies

Other than that patch looks good. I presume it will be useful for
other things than just migration too.

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only

Re: [PATCH 2/2] cpu: intel, amd: mask cleared cpuid features

[H. Peter Anvin]

On 07/20/2012 10:10 AM, Andi Kleen wrote:
> 
> And please use rdmsrl(), rdmsr() is so 90ies
> 

rdmsr()/wrmsr() make more sense in cases like this where the upper and
lower halves of the MSR actually contain separate data.

	-hpa

Re: [PATCH 2/2] cpu: intel, amd: mask cleared cpuid features

[Vladimir Davydov]

On 07/20/2012 09:10 PM, Andi Kleen wrote:
>> +					unsigned int *msr_ext_cpuid_mask)
>> +{
>> +	unsigned int msr, msr_ext;
>> +
>> +	msr = msr_ext = 0;
>> +
>> +	switch (c->x86_model) {
> You have to check the family too.
>
>> +
>> +	return msr;
>> +}
>> +
>> +static void __cpuinit intel_mask_cpu_caps(struct cpuinfo_x86 *c)
>> +{
>> +	u32 low, high;
>> +	unsigned int msr_cpuid_mask, msr_ext_cpuid_mask;
>> +
>> +	msr_cpuid_mask = intel_cpuid_mask_msr(c,&msr_ext_cpuid_mask);
>> +	if (msr_cpuid_mask) {
>> +		set_cpu_cap(c, X86_FEATURE_CPUIDMASK);
>> +
>> +		rdmsr(msr_cpuid_mask, low, high);
> And please use rdmsrl(), rdmsr() is so 90ies
>
> Other than that patch looks good. I presume it will be useful for
> other things than just migration too.
>
> -Andi
>

Thank you for review, but we've decided to drop this since the kernel 
does not seem to be a suitable place for such "tweaking".

Re: [PATCH 2/2] cpu: intel, amd: mask cleared cpuid features

[H. Peter Anvin]

On 07/20/2012 09:37 AM, Vladimir Davydov wrote:
> If 'clearcpuid=N' is specified in boot options, CPU feature #N won't be
> reported in /proc/cpuinfo and used by the kernel. However, if a
> userpsace process checks CPU features directly using the cpuid
> instruction, it will be reported about all features supported by the CPU
> irrespective of what features are cleared.
> 
> The patch makes the clearcpuid boot option not only clear CPU features
> in kernel but also mask them in hardware for Intel and AMD CPUs that
> support it so that the features cleared won't be reported even by the
> cpuid instruction.
> 
> This can be useful for migration of virtual machines managed by
> hypervisors that do not support/use Intel VT/AMD-V hardware-assisted
> virtualization technology.
> 
> If CPUID masking is supported, this will be reported in
> /proc/cpuinfo:flags as 'cpuidmask'.

I am a bit concerned about this patch:

1. it silently changes existing behavior.
2. even on enabled hardware, only some of the bits are maskable.

	-hpa

Re: [PATCH 2/2] cpu: intel, amd: mask cleared cpuid features

[Vladimir Davydov]

On Jul 20, 2012, at 9:20 PM, H. Peter Anvin wrote:

> On 07/20/2012 09:37 AM, Vladimir Davydov wrote:
>> If 'clearcpuid=N' is specified in boot options, CPU feature #N won't be
>> reported in /proc/cpuinfo and used by the kernel. However, if a
>> userpsace process checks CPU features directly using the cpuid
>> instruction, it will be reported about all features supported by the CPU
>> irrespective of what features are cleared.
>> 
>> The patch makes the clearcpuid boot option not only clear CPU features
>> in kernel but also mask them in hardware for Intel and AMD CPUs that
>> support it so that the features cleared won't be reported even by the
>> cpuid instruction.
>> 
>> This can be useful for migration of virtual machines managed by
>> hypervisors that do not support/use Intel VT/AMD-V hardware-assisted
>> virtualization technology.
>> 
>> If CPUID masking is supported, this will be reported in
>> /proc/cpuinfo:flags as 'cpuidmask'.
> 
> I am a bit concerned about this patch:
> 
> 1. it silently changes existing behavior.

Yes, but who needs the current implementation of 'clearcpuid' which, in fact, just hides flags in /proc/cpuinfo while userspace apps will see and consequently use all CPU features?

So, I think it logically extends the existing behavior.

> 2. even on enabled hardware, only some of the bits are maskable.

The patch makes only words 0, 1, 4, 6 maskable, but words 3, 7, 8 are Linux-defined, words 2 and 5 are Transmeta-, Centaur-, etc- defined, and word 9 contains some bizarre Intel CPU features. Thus, it is words 0, 1, 4, 6 that contain useful information for most hardware models.

If you ask about some Intel CPUs that can't mask CPUID function 0x80000001, this function describes AMD-specific features, and I bet those Intel CPUs just don't have them at all and thus have nothing to mask.

> 
> 	-hpa
> 
> 

Re: [PATCH 2/2] cpu: intel, amd: mask cleared cpuid features

[H. Peter Anvin]

On 07/20/2012 11:21 AM, Vladimir Davydov wrote:
>>
>> I am a bit concerned about this patch:
>>
>> 1. it silently changes existing behavior.
>
> Yes, but who needs the current implementation of 'clearcpuid' which,
> in fact, just hides flags in /proc/cpuinfo while userspace apps will
> see and consequently use all CPU features?

Anyone who wants to disable a feature from the kernel, specifically.

> So, I think it logically extends the existing behavior.
>
>> 2. even on enabled hardware, only some of the bits are maskable.
>
> The patch makes only words 0, 1, 4, 6 maskable, but words 3, 7, 8 are Linux-defined, words 2 and 5 are Transmeta-, Centaur-, etc- defined, and word 9 contains some bizarre Intel CPU features. Thus, it is words 0, 1, 4, 6 that contain useful information for most hardware models.

"Bizarre"?  New features, perhaps.

> If you ask about some Intel CPUs that can't mask CPUID function 0x80000001, this function describes AMD-specific features, and I bet those Intel CPUs just don't have them at all and thus have nothing to mask.

Not quite.

	-hpa



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

Re: [PATCH 2/2] cpu: intel, amd: mask cleared cpuid features

[Vladimir Davydov]

On Jul 21, 2012, at 12:19 AM, H. Peter Anvin wrote:

> On 07/20/2012 11:21 AM, Vladimir Davydov wrote:
>>> 
>>> I am a bit concerned about this patch:
>>> 
>>> 1. it silently changes existing behavior.
>> 
>> Yes, but who needs the current implementation of 'clearcpuid' which,
>> in fact, just hides flags in /proc/cpuinfo while userspace apps will
>> see and consequently use all CPU features?
> 
> Anyone who wants to disable a feature from the kernel, specifically.
> 

Another option then?

>> So, I think it logically extends the existing behavior.
>> 
>>> 2. even on enabled hardware, only some of the bits are maskable.
>> 
>> The patch makes only words 0, 1, 4, 6 maskable, but words 3, 7, 8 are Linux-defined, words 2 and 5 are Transmeta-, Centaur-, etc- defined, and word 9 contains some bizarre Intel CPU features. Thus, it is words 0, 1, 4, 6 that contain useful information for most hardware models.
> 
> "Bizarre"?  New features, perhaps.

All right, new :-)

If the features are widely used, they'll provide a way of masking them either I guess.

AFAIK, Intel added CPUID faulting for their newest 'Ivy Bridge' models, which allows masking all CPUID functions. Unfortunately, I don't have such a CPU. But later this feature can be utilized and wired into the code.

Anyway, masking of at least some of the features would be better than lacking of the ability at all, wouldn't it? Another question whether the kernel should report errors/warnings if a particular feature can't be masked.

> 
>> If you ask about some Intel CPUs that can't mask CPUID function 0x80000001, this function describes AMD-specific features, and I bet those Intel CPUs just don't have them at all and thus have nothing to mask.
> 
> Not quite.
> 
> 	-hpa
> 
> 
> 
> -- 
> H. Peter Anvin, Intel Open Source Technology Center
> I work for Intel.  I don't speak on their behalf.
> 
> 
> 

Re: [PATCH 2/2] cpu: intel, amd: mask cleared cpuid features

[Borislav Petkov]

(+ Andre who's been doing some cross vendor stuff)

On Fri, Jul 20, 2012 at 08:37:33PM +0400, Vladimir Davydov wrote:
> If 'clearcpuid=N' is specified in boot options, CPU feature #N won't be
> reported in /proc/cpuinfo and used by the kernel. However, if a
> userpsace process checks CPU features directly using the cpuid
> instruction, it will be reported about all features supported by the CPU
> irrespective of what features are cleared.
> 
> The patch makes the clearcpuid boot option not only clear CPU features
> in kernel but also mask them in hardware for Intel and AMD CPUs that
> support it so that the features cleared won't be reported even by the
> cpuid instruction.
> 
> This can be useful for migration of virtual machines managed by
> hypervisors that do not support/use Intel VT/AMD-V hardware-assisted
> virtualization technology.

As they say in Star Wars: "I have a bad feeling about this."

So opening the floodgates to people fiddling with this (not only
migrators) makes me feel pretty uneasy. And I won't wonder if all of
a sudden strange failures start to appear because code is querying
cpuid features but some funny distro has disabled it in its kernel boot
options.

Or some other obscure case where the culprit is hidden in kernel command
line options.

If it were only needed for migration, then I'd say you guys can use
msr-tools and run a script as root on the target machine to which you
want to migrate to and toggle the feature bits you want.

I don't think cross vendor migration alone justifies having a generic
kernel feature like that.

Thanks.

-- 
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

Re: [PATCH 2/2] cpu: intel, amd: mask cleared cpuid features

[Vladimir Davydov]

On 07/21/2012 02:37 PM, Borislav Petkov wrote:
> (+ Andre who's been doing some cross vendor stuff)
>
> On Fri, Jul 20, 2012 at 08:37:33PM +0400, Vladimir Davydov wrote:
>> If 'clearcpuid=N' is specified in boot options, CPU feature #N won't be
>> reported in /proc/cpuinfo and used by the kernel. However, if a
>> userpsace process checks CPU features directly using the cpuid
>> instruction, it will be reported about all features supported by the CPU
>> irrespective of what features are cleared.
>>
>> The patch makes the clearcpuid boot option not only clear CPU features
>> in kernel but also mask them in hardware for Intel and AMD CPUs that
>> support it so that the features cleared won't be reported even by the
>> cpuid instruction.
>>
>> This can be useful for migration of virtual machines managed by
>> hypervisors that do not support/use Intel VT/AMD-V hardware-assisted
>> virtualization technology.
> As they say in Star Wars: "I have a bad feeling about this."
>
> So opening the floodgates to people fiddling with this (not only
> migrators) makes me feel pretty uneasy. And I won't wonder if all of
> a sudden strange failures start to appear because code is querying
> cpuid features but some funny distro has disabled it in its kernel boot
> options.
>
> Or some other obscure case where the culprit is hidden in kernel command
> line options.
>
> If it were only needed for migration, then I'd say you guys can use
> msr-tools and run a script as root on the target machine to which you
> want to migrate to and toggle the feature bits you want.

If msr-tools are used for cpuid masking, we will either get 
inconsistency between /proc/cpuinfo:flags and the output of the cpuid 
instruction or have to "synchronize" the clearcpuid boot option and the 
userspace app using msr-tools, which seems to be inconvenient. So, IMO, 
it would be better to have such functionality implemented in the kernel.

> I don't think cross vendor migration alone justifies having a generic
> kernel feature like that.
>
> Thanks.
>

Re: [PATCH 2/2] cpu: intel, amd: mask cleared cpuid features

[Borislav Petkov]

On Tue, Jul 24, 2012 at 11:06:08AM +0400, Vladimir Davydov wrote:
> If msr-tools are used for cpuid masking, we will either get
> inconsistency between /proc/cpuinfo:flags and the output of the cpuid
> instruction or have to "synchronize" the clearcpuid boot option and
> the userspace app using msr-tools, which seems to be inconvenient.

Right, so IMO what you want to do is not very kosher as a generic kernel
feature for reasons described earlier.

And yes, "synchronizing" /proc/cpuinfo's flags and CPUID output is a lot
less pain compared to having a generic abusable feature.

And also much better suited for your niche use case than opening this up
as a generic functionality to the wide public which will most definitely
find productive ways to abuse this.

Simply ask yourself this: is this something which the majority of
x86 linux users will benefit from or is it only something which is
nice-to-have solely for our single use case?

Thanks.

-- 
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

Re: [PATCH 2/2] cpu: intel, amd: mask cleared cpuid features

[Andre Przywara]

On 07/24/2012 09:06 AM, Vladimir Davydov wrote:
> On 07/21/2012 02:37 PM, Borislav Petkov wrote:
>> (+ Andre who's been doing some cross vendor stuff)
>>
>> On Fri, Jul 20, 2012 at 08:37:33PM +0400, Vladimir Davydov wrote:
>>> If 'clearcpuid=N' is specified in boot options, CPU feature #N won't be
>>> reported in /proc/cpuinfo and used by the kernel. However, if a
>>> userpsace process checks CPU features directly using the cpuid
>>> instruction, it will be reported about all features supported by the CPU
>>> irrespective of what features are cleared.
>>>
>>> The patch makes the clearcpuid boot option not only clear CPU features
>>> in kernel but also mask them in hardware for Intel and AMD CPUs that
>>> support it so that the features cleared won't be reported even by the
>>> cpuid instruction.
>>>
>>> This can be useful for migration of virtual machines managed by
>>> hypervisors that do not support/use Intel VT/AMD-V hardware-assisted
>>> virtualization technology.

But for this case you want it more fine-grained, say on a pre-process or 
per-container level, right?
For hardware-assisted virtualization you simply don't need it, and for 
Xen PV guests for instance this can be more safely done by the 
hypervisor. I assume Parallels is similar in this respect, so you may 
want to switch the MSRs on the guest's entry and exit by the VMM.
Also if you want to restrict a guest's CPUID features, you don't want to 
do this at the guest's discretion, but better one level below where the 
guest cannot revert this, right?

In general I am not reluctant to have this feature with a sane 
interface, but I simply don't see the usefulness of having it per kernel.
Also note that AFAIK this masking only helps with the basic CPUID 
features, namely leaf 1 and 0x80000001 for ECX and EDX. This does not 
cover the more advanced features and not the new ones at leaf 7.

>> So opening the floodgates to people fiddling with this (not only
>> migrators) makes me feel pretty uneasy. And I won't wonder if all of
>> a sudden strange failures start to appear because code is querying
>> cpuid features but some funny distro has disabled it in its kernel boot
>> options.

Actually these "strange failures" would be a bug then. If CPUID is not 
there, the feature is not there. Full stop. In the past we had had 
already some trouble with people ignoring CPUID and stating some funny 
things like: "Every XYZ processor has this feature."
If someone disables MCE, then on purpose. Let the code cope with it.

And Boris: I don't like this "majority of users" argument. If there is 
some sense in this feature, why not have it (unless it significantly 
hurts the code base)? Remember, this is Linux: If you want to shoot 
yourself in the foot, we will not prevent you.

Regards,
Andre.

-- 
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany

Re: [PATCH 2/2] cpu: intel, amd: mask cleared cpuid features

[Vladimir Davydov]

On 07/24/2012 12:14 PM, Andre Przywara wrote:
> On 07/24/2012 09:06 AM, Vladimir Davydov wrote:
>> On 07/21/2012 02:37 PM, Borislav Petkov wrote:
>>> (+ Andre who's been doing some cross vendor stuff)
>>>
>>> On Fri, Jul 20, 2012 at 08:37:33PM +0400, Vladimir Davydov wrote:
>>>> If 'clearcpuid=N' is specified in boot options, CPU feature #N won't be
>>>> reported in /proc/cpuinfo and used by the kernel. However, if a
>>>> userpsace process checks CPU features directly using the cpuid
>>>> instruction, it will be reported about all features supported by the CPU
>>>> irrespective of what features are cleared.
>>>>
>>>> The patch makes the clearcpuid boot option not only clear CPU features
>>>> in kernel but also mask them in hardware for Intel and AMD CPUs that
>>>> support it so that the features cleared won't be reported even by the
>>>> cpuid instruction.
>>>>
>>>> This can be useful for migration of virtual machines managed by
>>>> hypervisors that do not support/use Intel VT/AMD-V hardware-assisted
>>>> virtualization technology.
> But for this case you want it more fine-grained, say on a pre-process or
> per-container level, right?
> For hardware-assisted virtualization you simply don't need it, and for
> Xen PV guests for instance this can be more safely done by the
> hypervisor. I assume Parallels is similar in this respect, so you may
> want to switch the MSRs on the guest's entry and exit by the VMM.
> Also if you want to restrict a guest's CPUID features, you don't want to
> do this at the guest's discretion, but better one level below where the
> guest cannot revert this, right?

Actually I meant OS-level virtualization (no hypervisors) based on the 
linux cgroup subsystem and namespaces like OpenVZ or LXC . Although the 
latter does not have the container migration ability at present, there 
is a project that will hopefully allow this soon (criu.org). For such 
virtualization systems, per-kernel option would be enough because all 
guests share the same kernel.

>
> In general I am not reluctant to have this feature with a sane
> interface, but I simply don't see the usefulness of having it per kernel.
> Also note that AFAIK this masking only helps with the basic CPUID
> features, namely leaf 1 and 0x80000001 for ECX and EDX. This does not
> cover the more advanced features and not the new ones at leaf 7.

I guess that when the more advanced features become widely-used, vendors 
will offer new MSRs and/or CPUID faulting.

>>> So opening the floodgates to people fiddling with this (not only
>>> migrators) makes me feel pretty uneasy. And I won't wonder if all of
>>> a sudden strange failures start to appear because code is querying
>>> cpuid features but some funny distro has disabled it in its kernel boot
>>> options.
> Actually these "strange failures" would be a bug then. If CPUID is not
> there, the feature is not there. Full stop. In the past we had had
> already some trouble with people ignoring CPUID and stating some funny
> things like: "Every XYZ processor has this feature."
> If someone disables MCE, then on purpose. Let the code cope with it.
>
> And Boris: I don't like this "majority of users" argument. If there is
> some sense in this feature, why not have it (unless it significantly
> hurts the code base)? Remember, this is Linux: If you want to shoot
> yourself in the foot, we will not prevent you.
>
> Regards,
> Andre.
>

Re: [PATCH 2/2] cpu: intel, amd: mask cleared cpuid features

[Borislav Petkov]

On Tue, Jul 24, 2012 at 12:29:19PM +0400, Vladimir Davydov wrote:
> I guess that when the more advanced features become widely-used,
> vendors will offer new MSRs and/or CPUID faulting.

And this right there is the dealbreaker:

So what are you doing for cpus which have the advanced CPUID features
leafs but there are no MSRs to turn those bits off?

You surely need some software-only solution for the migration to work,
no?

If so, why not apply that solution to your hypervisor without touching
the kernel at all?

Thanks.

-- 
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

Re: [PATCH 2/2] cpu: intel, amd: mask cleared cpuid features

[Vladimir Davydov]

On 07/24/2012 02:10 PM, Borislav Petkov wrote:
> On Tue, Jul 24, 2012 at 12:29:19PM +0400, Vladimir Davydov wrote:
>> I guess that when the more advanced features become widely-used,
>> vendors will offer new MSRs and/or CPUID faulting.
> And this right there is the dealbreaker:
>
> So what are you doing for cpus which have the advanced CPUID features
> leafs but there are no MSRs to turn those bits off?

We have not encountered this situation in our environments and I hope we 
won't :-)

But look, these CPUID functions cover majority of CPU features, don't 
they? So, most of "normal" apps inside VM will survive migration. 
Perhaps, some low-level utils won't. I guess that's why there are no 
MSRs for other levels provided by vendors.

> You surely need some software-only solution for the migration to work,
> no?

Yes.

> If so, why not apply that solution to your hypervisor without touching
> the kernel at all?

In most hypervisor-based virtualization products, this is already 
implemented using VMM-exits, so that each VM can have arbitrary CPUID 
mask set by the admin.

The problem is that we have no hypervisor. "Virtualization" we want this 
feature for is based on cgroups and namespaces (examples are OpenVZ and 
mainstream LXC). Tasks are just grouped into virtual environments and 
share the same kernel, which is proved to be more memory usage efficient 
than traditional hypervisor-based approaches.

> Thanks.
>

Re: [PATCH 2/2] cpu: intel, amd: mask cleared cpuid features

[Andre Przywara]

On 07/24/2012 01:09 PM, Vladimir Davydov wrote:
> On 07/24/2012 02:10 PM, Borislav Petkov wrote:
>> On Tue, Jul 24, 2012 at 12:29:19PM +0400, Vladimir Davydov wrote:
>>> I guess that when the more advanced features become widely-used,
>>> vendors will offer new MSRs and/or CPUID faulting.
>> And this right there is the dealbreaker:
>>
>> So what are you doing for cpus which have the advanced CPUID features
>> leafs but there are no MSRs to turn those bits off?
>
> We have not encountered this situation in our environments and I hope we
> won't :-)
>
> But look, these CPUID functions cover majority of CPU features, don't
> they? So, most of "normal" apps inside VM will survive migration.
> Perhaps, some low-level utils won't. I guess that's why there are no
> MSRs for other levels provided by vendors.

You have the new feature leaf at EAX=7. This contains things like BMI 
and AVX2 and probably more upcoming features.
So you may be safe for a while, but you need a solution in the long run.

>> You surely need some software-only solution for the migration to work,
>> no?
>
> Yes.
>
>> If so, why not apply that solution to your hypervisor without touching
>> the kernel at all?
>
> In most hypervisor-based virtualization products, this is already
> implemented using VMM-exits, so that each VM can have arbitrary CPUID
> mask set by the admin.
>
> The problem is that we have no hypervisor. "Virtualization" we want this
> feature for is based on cgroups and namespaces (examples are OpenVZ and
> mainstream LXC). Tasks are just grouped into virtual environments and
> share the same kernel, which is proved to be more memory usage efficient
> than traditional hypervisor-based approaches.

So for this single kernel approach I'd understand it that way:
1. You boot up the kernel on the host, it should detect and enable all 
the features, say MCA.
2. After boot, you use /src/msr-tools/wrmsr to mask CPUID bits, again 
MCA for instance or AVX/AES or the like.
Since the (host side of the) kernel already detected it, this does not 
hurt the kernel features like MCA. But AVX will not be available to 
applications running in the "host container", which is probably OK since 
these are mostly management applications, right?
3. Then you start guests. The guest's libc will not detect the features 
because of the MSR masking. All you need now is /proc/cpuinfo filtering 
to make this bullet-proof, preferably through the container 
functionality. I see that you do already massive sysfs filtering and 
also /proc/<pid> filtering, so this maybe an option?

This approach does not need any kernel support (except for the 
/proc/cpuinfo filtering). Does this address the issues you have?

Regards,
Andre.

-- 
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany

Re: [PATCH 2/2] cpu: intel, amd: mask cleared cpuid features

[Alan Cox]

> This approach does not need any kernel support (except for the 
> /proc/cpuinfo filtering). Does this address the issues you have?

You can do the /proc/cpuinfo filtering in user space too

Re: [PATCH 2/2] cpu: intel, amd: mask cleared cpuid features

[Vladimir Davydov]

On 07/24/2012 04:44 PM, Alan Cox wrote:
>> This approach does not need any kernel support (except for the
>> /proc/cpuinfo filtering). Does this address the issues you have?
> You can do the /proc/cpuinfo filtering in user space too
>

How?

Re: [PATCH 2/2] cpu: intel, amd: mask cleared cpuid features

[Andre Przywara]

On 07/25/2012 12:31 PM, Vladimir Davydov wrote:
> On 07/24/2012 04:44 PM, Alan Cox wrote:
>>> This approach does not need any kernel support (except for the
>>> /proc/cpuinfo filtering). Does this address the issues you have?
>> You can do the /proc/cpuinfo filtering in user space too
>>
>
> How?

I wanted to write the same reply yesterday, but followed the hint in 
Alan's previous mail:
# mount --bind /dev/shm/faked_cpuinfo /somepath/proc/cpuinfo

I checked it, it works even with chroots and is not visible from within.

Regards,
Andre.

-- 
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany

Re: [PATCH 2/2] cpu: intel, amd: mask cleared cpuid features

[Vladimir Davydov]

On 07/25/2012 02:58 PM, Andre Przywara wrote:
> On 07/25/2012 12:31 PM, Vladimir Davydov wrote:
>> On 07/24/2012 04:44 PM, Alan Cox wrote:
>>>> This approach does not need any kernel support (except for the
>>>> /proc/cpuinfo filtering). Does this address the issues you have?
>>> You can do the /proc/cpuinfo filtering in user space too
>>>
>> How?
> I wanted to write the same reply yesterday, but followed the hint in
> Alan's previous mail:
> # mount --bind /dev/shm/faked_cpuinfo /somepath/proc/cpuinfo
>
> I checked it, it works even with chroots and is not visible from within.

If CPUs go online/offline?

>
> Regards,
> Andre.
>

Re: [PATCH 2/2] cpu: intel, amd: mask cleared cpuid features

[Andre Przywara]

On 07/25/2012 01:02 PM, Vladimir Davydov wrote:
> On 07/25/2012 02:58 PM, Andre Przywara wrote:
>> On 07/25/2012 12:31 PM, Vladimir Davydov wrote:
>>> On 07/24/2012 04:44 PM, Alan Cox wrote:
>>>>> This approach does not need any kernel support (except for the
>>>>> /proc/cpuinfo filtering). Does this address the issues you have?
>>>> You can do the /proc/cpuinfo filtering in user space too
>>>>
>>> How?
>> I wanted to write the same reply yesterday, but followed the hint in
>> Alan's previous mail:
>> # mount --bind /dev/shm/faked_cpuinfo /somepath/proc/cpuinfo
>>
>> I checked it, it works even with chroots and is not visible from within.
>
> If CPUs go online/offline?

Do you support CPU offlining from within the guest? My OpenVZ guest only 
has /sys/class and nothing else, so I cannot offline any CPU.

So you setup a "hand-crafted" cpuinfo for the guest and this should stay 
valid for the whole guest's runtime, right?

And since it is a dumped file, "host" CPU off/onlining does not affect 
it. Or do you want to propagate this to the guests?
(Sorry, but my thinking is more Xen/KVM oriented, where guests can only 
do most things if they are explicitly allowed to do it and separation 
between guests and host is much stricter).


Regards,
Andre.

-- 
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany

Re: [PATCH 2/2] cpu: intel, amd: mask cleared cpuid features

[Vladimir Davydov]

On 07/25/2012 03:17 PM, Andre Przywara wrote:
> On 07/25/2012 01:02 PM, Vladimir Davydov wrote:
>> On 07/25/2012 02:58 PM, Andre Przywara wrote:
>>> On 07/25/2012 12:31 PM, Vladimir Davydov wrote:
>>>> On 07/24/2012 04:44 PM, Alan Cox wrote:
>>>>>> This approach does not need any kernel support (except for the
>>>>>> /proc/cpuinfo filtering). Does this address the issues you have?
>>>>> You can do the /proc/cpuinfo filtering in user space too
>>>>>
>>>> How?
>>> I wanted to write the same reply yesterday, but followed the hint in
>>> Alan's previous mail:
>>> # mount --bind /dev/shm/faked_cpuinfo /somepath/proc/cpuinfo
>>>
>>> I checked it, it works even with chroots and is not visible from within.
>> If CPUs go online/offline?
> Do you support CPU offlining from within the guest? My OpenVZ guest only
> has /sys/class and nothing else, so I cannot offline any CPU.
>
> So you setup a "hand-crafted" cpuinfo for the guest and this should stay
> valid for the whole guest's runtime, right?
>
> And since it is a dumped file, "host" CPU off/onlining does not affect
> it. Or do you want to propagate this to the guests?

A guest cannot have more CPUs than the host in container virtualization 
(at least in OpenVZ). So yes, we will have to propagate.

Anyway, we've agreed that you were right and are going to think about 
/proc/cpuinfo virtualization.

Thank you for your time and feedback.

> (Sorry, but my thinking is more Xen/KVM oriented, where guests can only
> do most things if they are explicitly allowed to do it and separation
> between guests and host is much stricter).
>
>
> Regards,
> Andre.
>

Re: [PATCH 2/2] cpu: intel, amd: mask cleared cpuid features

[Alan Cox]

On Wed, 25 Jul 2012 14:31:30 +0400
Vladimir Davydov <vdavydov@parallels.com> wrote:

> On 07/24/2012 04:44 PM, Alan Cox wrote:
> >> This approach does not need any kernel support (except for the
> >> /proc/cpuinfo filtering). Does this address the issues you have?
> > You can do the /proc/cpuinfo filtering in user space too
> >
> 
> How?

bind mount your own normal file over the top or a FUSE file

Re: [PATCH 2/2] cpu: intel, amd: mask cleared cpuid features

[Vladimir Davydov]

On 07/25/2012 03:31 PM, Alan Cox wrote:
> On Wed, 25 Jul 2012 14:31:30 +0400
> Vladimir Davydov<vdavydov@parallels.com>  wrote:
>
>> On 07/24/2012 04:44 PM, Alan Cox wrote:
>>>> This approach does not need any kernel support (except for the
>>>> /proc/cpuinfo filtering). Does this address the issues you have?
>>> You can do the /proc/cpuinfo filtering in user space too
>>>
>> How?
> bind mount your own normal file over the top or a FUSE file

In general, we've agreed that wiring CPUID masking into the kernel was 
not a good idea.

We will try to virtualize /proc/cpuinfo as well as other /proc stuff in 
mainstream.

Thank you for your time and feedback.

Re: [PATCH 2/2] cpu: intel, amd: mask cleared cpuid features

[Vladimir Davydov]

On 07/24/2012 04:34 PM, Andre Przywara wrote:
> On 07/24/2012 01:09 PM, Vladimir Davydov wrote:
>> On 07/24/2012 02:10 PM, Borislav Petkov wrote:
>>> On Tue, Jul 24, 2012 at 12:29:19PM +0400, Vladimir Davydov wrote:
>>>> I guess that when the more advanced features become widely-used,
>>>> vendors will offer new MSRs and/or CPUID faulting.
>>> And this right there is the dealbreaker:
>>>
>>> So what are you doing for cpus which have the advanced CPUID features
>>> leafs but there are no MSRs to turn those bits off?
>> We have not encountered this situation in our environments and I hope we
>> won't :-)
>>
>> But look, these CPUID functions cover majority of CPU features, don't
>> they? So, most of "normal" apps inside VM will survive migration.
>> Perhaps, some low-level utils won't. I guess that's why there are no
>> MSRs for other levels provided by vendors.
> You have the new feature leaf at EAX=7. This contains things like BMI
> and AVX2 and probably more upcoming features.
> So you may be safe for a while, but you need a solution in the long run.
>
>>> You surely need some software-only solution for the migration to work,
>>> no?
>> Yes.
>>
>>> If so, why not apply that solution to your hypervisor without touching
>>> the kernel at all?
>> In most hypervisor-based virtualization products, this is already
>> implemented using VMM-exits, so that each VM can have arbitrary CPUID
>> mask set by the admin.
>>
>> The problem is that we have no hypervisor. "Virtualization" we want this
>> feature for is based on cgroups and namespaces (examples are OpenVZ and
>> mainstream LXC). Tasks are just grouped into virtual environments and
>> share the same kernel, which is proved to be more memory usage efficient
>> than traditional hypervisor-based approaches.
> So for this single kernel approach I'd understand it that way:
> 1. You boot up the kernel on the host, it should detect and enable all
> the features, say MCA.
> 2. After boot, you use /src/msr-tools/wrmsr to mask CPUID bits, again
> MCA for instance or AVX/AES or the like.
> Since the (host side of the) kernel already detected it, this does not
> hurt the kernel features like MCA. But AVX will not be available to
> applications running in the "host container", which is probably OK since
> these are mostly management applications, right?
> 3. Then you start guests. The guest's libc will not detect the features
> because of the MSR masking. All you need now is /proc/cpuinfo filtering
> to make this bullet-proof, preferably through the container
> functionality. I see that you do already massive sysfs filtering and
> also /proc/<pid>  filtering, so this maybe an option?

Yes, we do filter /proc/cpuinfo output in our product (OpenVZ), but 
there is also the LXC project that is based completely on the mainstream 
kernel. LXC developers have not faced the cross-vendor migration problem 
yet because currently they don't have migration at all, but they will 
surely face it in future since the work on migration is in progress now 
(CRIU).

So, you prefer adding some filtering of /proc/cpuinfo into the 
mainstream kernel (not now, later, for LXC to work) instead of enabling 
clearcpuid boot option to mask CPUID features? IMO, the latter would 
look clearer.

>
> This approach does not need any kernel support (except for the
> /proc/cpuinfo filtering). Does this address the issues you have?
>
> Regards,
> Andre.
>

Re: [PATCH 2/2] cpu: intel, amd: mask cleared cpuid features

[Borislav Petkov]

On Wed, Jul 25, 2012 at 02:31:23PM +0400, Vladimir Davydov wrote:
> So, you prefer adding some filtering of /proc/cpuinfo into the
> mainstream kernel

That's already there right? And your 1/2 patch was making toggling those
bits easier.

> (not now, later, for LXC to work) instead of enabling clearcpuid boot
> option to mask CPUID features? IMO, the latter would look clearer.

Yes, but for reasons noted earlier, you cannot tweak all hardware CPUID
features as you want them.

So, having a software-only layer of tweaking /proc/cpuinfo or something
different you can come up with is the only option you have.

And even in that case, applications running in the container which
execute CPUID might fail in a strange manner when the corresponding
/proc/cpuinfo flag was cleared by you intentionally but the hardware
CPUID flag is there. In such situations, issues like that should
probably be sorted on a case-by-case basis I guess.

Thanks.

-- 
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

Re: [PATCH 2/2] cpu: intel, amd: mask cleared cpuid features

[Vladimir Davydov]

On 07/25/2012 02:43 PM, Borislav Petkov wrote:
> On Wed, Jul 25, 2012 at 02:31:23PM +0400, Vladimir Davydov wrote:
>> So, you prefer adding some filtering of /proc/cpuinfo into the
>> mainstream kernel
> That's already there right? And your 1/2 patch was making toggling those
> bits easier.
>
>> (not now, later, for LXC to work) instead of enabling clearcpuid boot
>> option to mask CPUID features? IMO, the latter would look clearer.
> Yes, but for reasons noted earlier, you cannot tweak all hardware CPUID
> features as you want them.
>
> So, having a software-only layer of tweaking /proc/cpuinfo or something
> different you can come up with is the only option you have.
>
> And even in that case, applications running in the container which
> execute CPUID might fail in a strange manner when the corresponding
> /proc/cpuinfo flag was cleared by you intentionally but the hardware
> CPUID flag is there. In such situations, issues like that should
> probably be sorted on a case-by-case basis I guess.
>
> Thanks.
>

We've agreed that tweaking CPUID bits in kernel is not a good idea and 
it is better to think about virtualization of /proc/cpuinfo and using 
msr-tools.

Thank you for your time and feedback.

Re: [PATCH 2/2] cpu: intel, amd: mask cleared cpuid features

[H. Peter Anvin]

On 07/24/2012 04:09 AM, Vladimir Davydov wrote:
> 
> We have not encountered this situation in our environments and I hope we
> won't :-)
> 
> But look, these CPUID functions cover majority of CPU features, don't
> they? So, most of "normal" apps inside VM will survive migration.
> Perhaps, some low-level utils won't. I guess that's why there are no
> MSRs for other levels provided by vendors.
> 

You will once Ivy Bridge becomes common.

	-hpa

Re: [PATCH 2/2] cpu: intel, amd: mask cleared cpuid features

[Vladimir Davydov]

On 07/25/2012 04:57 AM, H. Peter Anvin wrote:
> On 07/24/2012 04:09 AM, Vladimir Davydov wrote:
>> We have not encountered this situation in our environments and I hope we
>> won't :-)
>>
>> But look, these CPUID functions cover majority of CPU features, don't
>> they? So, most of "normal" apps inside VM will survive migration.
>> Perhaps, some low-level utils won't. I guess that's why there are no
>> MSRs for other levels provided by vendors.
>>
> You will once Ivy Bridge becomes common.

Ivy Bridge has CPUID faulting, which allows masking all CPUID 
levels/functions.

> 	-hpa
>

Re: [PATCH 2/2] cpu: intel, amd: mask cleared cpuid features

[H. Peter Anvin]

With the necessary infrastructure, yes.  Trap and emulate is montrivial work.

Vladimir Davydov <vdavydov@parallels.com> wrote:

>On 07/25/2012 04:57 AM, H. Peter Anvin wrote:
>> On 07/24/2012 04:09 AM, Vladimir Davydov wrote:
>>> We have not encountered this situation in our environments and I
>hope we
>>> won't :-)
>>>
>>> But look, these CPUID functions cover majority of CPU features,
>don't
>>> they? So, most of "normal" apps inside VM will survive migration.
>>> Perhaps, some low-level utils won't. I guess that's why there are no
>>> MSRs for other levels provided by vendors.
>>>
>> You will once Ivy Bridge becomes common.
>
>Ivy Bridge has CPUID faulting, which allows masking all CPUID 
>levels/functions.
>
>> 	-hpa
>>

-- 
Sent from my mobile phone. Please excuse brevity and lack of formatting.

Re: [PATCH 2/2] cpu: intel, amd: mask cleared cpuid features

[Vladimir Davydov]

On 07/25/2012 04:57 AM, H. Peter Anvin wrote:
> On 07/24/2012 04:09 AM, Vladimir Davydov wrote:
>> We have not encountered this situation in our environments and I hope we
>> won't :-)
>>
>> But look, these CPUID functions cover majority of CPU features, don't
>> they? So, most of "normal" apps inside VM will survive migration.
>> Perhaps, some low-level utils won't. I guess that's why there are no
>> MSRs for other levels provided by vendors.
>>
> You will once Ivy Bridge becomes common.
>
> 	-hpa
>

You're right here. We will try to find other ways for sorting the 
migration problem out.

Thank you for your time and feedback.

Re: [PATCH 2/2] cpu: intel, amd: mask cleared cpuid features

[Borislav Petkov]

On Tue, Jul 24, 2012 at 10:14:09AM +0200, Andre Przywara wrote:
> Actually these "strange failures" would be a bug then. If CPUID is
> not there, the feature is not there. Full stop.

That's full of b*llshit and you know it. The feature is not there
*because* some luser has disabled it with a command line option. Or the
distro kernel has done it for some other idiotic reason.

Then this unsuspecting user comes to lkml and complains that cpupower
utils doesn't show boosting information, for example (yep, for that
example, we've erroneously disabled CPUID_8000_0007_EAX[9]).

And then, after a long debugging session someone finally accidentally
looks at the kernel command line options and realizes that it was
disabled there but the cpu actually had it from the get-go. Bummer, all
that wasted time.

And then I can even imagine such tools going the extra mile of checking
f/m/s and telling the user that the cpu actually supports the feature
but someone has disabled it.

And this is just a simple example.

So actually, making it straightforward to disable CPUID feature bits
just for every whim is the bug.

> In the past we had had already some trouble with people ignoring CPUID
> and stating some funny things like: "Every XYZ processor has this
> feature."

You'll get more of those with a feature like that.

> If someone disables MCE, then on purpose. Let the code cope with it.

I'd like to see a real valid reason why someone would even think that.
Except virtualization folks who are crazy anyway, so that doesn't count :).

> And Boris: I don't like this "majority of users" argument. If there is
> some sense in this feature, why not have it (unless it significantly
> hurts the code base)?

Really??! There's some sense in having a coffee machine daemon in the
kernel, for a certain definition of sense and for a certain number of
users. Why not add it to the kernel too then?

Majority of users is majority of users no matter how you look at it!

> Remember, this is Linux: If you want to shoot yourself in the foot, we
> will not prevent you.

Right, and how is giving the user a heavy, well-oiled AK-47 to do that,
user-friendly?

Btw, this was exactly one of the topics last year at the kernel summit:
Linux maintainers should be more conservative and accept new features
only when it really makes sense and there's verifiable usability to the
majority of users.

And this is exactly what I'm questioning: the usability, or rather, the
mis-usability of such a feature.

-- 
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

Re: [PATCH 2/2] cpu: intel, amd: mask cleared cpuid features

[Alan Cox]

> So actually, making it straightforward to disable CPUID feature bits
> just for every whim is the bug.

Sometimes its needed to make stuff work. Expecting user space to go
digging in odd places isn't good either but exposing *both* true/apparent
cpuid bits might not be a bad idea.

> I'd like to see a real valid reason why someone would even think that.
> Except virtualization folks who are crazy anyway, so that doesn't count :).

Which is a very large part of the x86 market. So they most definitely do
count. Virtualisation is somewhat different though. There you are trying
to define a subset of the features that all the systems in your
environment have so you can do migrations. Virtualisation you have rather
more different control of the cpuid and msrs anyway.

> Majority of users is majority of users no matter how you look at it!

That's not a good argument. The majority of users don't have SCSI,
certain processors and so on ...

> Right, and how is giving the user a heavy, well-oiled AK-47 to do that,
> user-friendly?

It's a point and click interface

> And this is exactly what I'm questioning: the usability, or rather, the
> mis-usability of such a feature.

What goes with that is "so how do you do it otherwise". Distros can
certainly add patches for such features if needed but that just makes it
even more fun to debug.
 
Does "bind mount your own cpuid file" cover this ?

Alan

Re: [PATCH 2/2] cpu: intel, amd: mask cleared cpuid features

[Borislav Petkov]

On Tue, Jul 24, 2012 at 11:32:04AM +0100, Alan Cox wrote:
> > So actually, making it straightforward to disable CPUID feature bits
> > just for every whim is the bug.
> 
> Sometimes its needed to make stuff work. Expecting user space to go
> digging in odd places

Nah, not odd places. Simply doing "wrmsr... " as root would suffice.

> isn't good either but exposing *both* true/apparent cpuid bits might
> not be a bad idea.

I'm fine with the "might not be a bad idea" thing. I'm saying that it
seems like a bad idea in certain cases...

> > I'd like to see a real valid reason why someone would even think that.
> > Except virtualization folks who are crazy anyway, so that doesn't count :).
> 
> Which is a very large part of the x86 market. So they most definitely do
> count. Virtualisation is somewhat different though. There you are trying
> to define a subset of the features that all the systems in your
> environment have so you can do migrations. Virtualisation you have rather
> more different control of the cpuid and msrs anyway.

Right and best it would be if *only* virtualization had access to those
MSRs and CPUID bits. IOW, you #GP when accessing them in the normal case
and access is granted when in VMRUN context.

> > Majority of users is majority of users no matter how you look at it!
> 
> That's not a good argument. The majority of users don't have SCSI,
> certain processors and so on ...

or coffee machines ... :-)

> > Right, and how is giving the user a heavy, well-oiled AK-47 to do that,
> > user-friendly?
> 
> It's a point and click interface

Or rather, aim and squeeze :-)

> > And this is exactly what I'm questioning: the usability, or rather, the
> > mis-usability of such a feature.
> 
> What goes with that is "so how do you do it otherwise".

Not much more harder using msr-tools and easily scriptable. See above.

> Distros can certainly add patches for such features if needed but that
> just makes it even more fun to debug.

That's easy: the first question we ask from the bug reporter is (and you
do that too, btw - I've seen you dozens of times :-)) "can you reproduce
it with mainline"?

> Does "bind mount your own cpuid file" cover this ?

Well, AFAICU, the writes to the MSRs are globally visible. If you're
asking whether through bind-mounting your own cpuid file we're making
the process of toggling CPUID bits more involved versus using simply
kernel command line options then this is probably a step in the right
direction IMHO, BUT (!)...

... there's still a software-only solution needed for CPUID leafs which
cannot be toggled through MSR writes simply because there are no such
MSRs.

The solution to that situation could cover all issues without touching
the kernel.

-- 
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