[PATCH] Allow dom0 to write MSR IA32_ENERGY_PERF_BIAS

[PATCH] Allow dom0 to write MSR IA32_ENERGY_PERF_BIAS

[Wei, Gang]

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

Allow dom0 to write MSR IA32_ENERGY_PERF_BIAS

There is a new hardware feature, which lets system software to set Energy Performance Preference. This is a opaque knob in the form of IA32_ENERGY_PERF_BIAS MSR, which has a 4 bit Energy Performance Preference Hint.

The support for this feature is indicated by CPUID.06H.ECX.bit3. Refer to Intel Architectures Software Developer's Manual for more info.

Let dom0 tools to control it.

Signed-off-by: Wei Gang <gang.wei@intel.com>

diff -r 4e108cf56d07 xen/arch/x86/traps.c
--- a/xen/arch/x86/traps.c	Mon Dec 27 08:00:09 2010 +0000
+++ b/xen/arch/x86/traps.c	Sat Jan 01 20:01:43 2011 +0800
@@ -2333,6 +2333,7 @@ static int emulate_privileged_op(struct 
                 goto fail;
             break;
         case MSR_IA32_THERM_CONTROL:
+        case MSR_IA32_ENERGY_PERF_BIAS:
             if ( boot_cpu_data.x86_vendor != X86_VENDOR_INTEL )
                 goto fail;
             if ( (v->domain->domain_id != 0) || !v->domain->is_pinned )
diff -r 4e108cf56d07 xen/include/asm-x86/msr-index.h
--- a/xen/include/asm-x86/msr-index.h	Mon Dec 27 08:00:09 2010 +0000
+++ b/xen/include/asm-x86/msr-index.h	Sat Jan 01 19:57:58 2011 +0800
@@ -330,6 +330,7 @@
 #define MSR_IA32_MISC_ENABLE_XTPR_DISABLE (1<<23)
 
 #define MSR_IA32_TSC_DEADLINE		0x000006E0
+#define MSR_IA32_ENERGY_PERF_BIAS	0x000001b0
 
 /* Intel Model 6 */
 #define MSR_P6_EVNTSEL0			0x00000186

[-- Attachment #2: msr-energy_perf_bias.patch --]
[-- Type: application/octet-stream, Size: 1407 bytes --]

Allow dom0 to write MSR IA32_ENERGY_PERF_BIAS

There is a new hardware feature, which lets system software to set Energy Performance Preference. This is a opaque knob in the form of IA32_ENERGY_PERF_BIAS MSR, which has a 4 bit Energy Performance Preference Hint.

The support for this feature is indicated by CPUID.06H.ECX.bit3. Refer to Intel Architectures Software Developer's Manual for more info.

Let dom0 tools to control it.

Signed-off-by: Wei Gang <gang.wei@intel.com>

diff -r 4e108cf56d07 xen/arch/x86/traps.c
--- a/xen/arch/x86/traps.c	Mon Dec 27 08:00:09 2010 +0000
+++ b/xen/arch/x86/traps.c	Sat Jan 01 20:01:43 2011 +0800
@@ -2333,6 +2333,7 @@ static int emulate_privileged_op(struct 
                 goto fail;
             break;
         case MSR_IA32_THERM_CONTROL:
+        case MSR_IA32_ENERGY_PERF_BIAS:
             if ( boot_cpu_data.x86_vendor != X86_VENDOR_INTEL )
                 goto fail;
             if ( (v->domain->domain_id != 0) || !v->domain->is_pinned )
diff -r 4e108cf56d07 xen/include/asm-x86/msr-index.h
--- a/xen/include/asm-x86/msr-index.h	Mon Dec 27 08:00:09 2010 +0000
+++ b/xen/include/asm-x86/msr-index.h	Sat Jan 01 19:57:58 2011 +0800
@@ -330,6 +330,7 @@
 #define MSR_IA32_MISC_ENABLE_XTPR_DISABLE (1<<23)
 
 #define MSR_IA32_TSC_DEADLINE		0x000006E0
+#define MSR_IA32_ENERGY_PERF_BIAS	0x000001b0
 
 /* Intel Model 6 */
 #define MSR_P6_EVNTSEL0			0x00000186

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] Allow dom0 to write MSR IA32_ENERGY_PERF_BIAS

[Jan Beulich]

>>> On 04.01.11 at 04:04, "Wei, Gang" <gang.wei@intel.com> wrote:
> Allow dom0 to write MSR IA32_ENERGY_PERF_BIAS
> 
> There is a new hardware feature, which lets system software to set Energy 
> Performance Preference. This is a opaque knob in the form of 
> IA32_ENERGY_PERF_BIAS MSR, which has a 4 bit Energy Performance Preference 
> Hint.
> 
> The support for this feature is indicated by CPUID.06H.ECX.bit3. Refer to 
> Intel Architectures Software Developer's Manual for more info.
> 
> Let dom0 tools to control it.
> 
> Signed-off-by: Wei Gang <gang.wei@intel.com>
> 
> diff -r 4e108cf56d07 xen/arch/x86/traps.c
> --- a/xen/arch/x86/traps.c	Mon Dec 27 08:00:09 2010 +0000
> +++ b/xen/arch/x86/traps.c	Sat Jan 01 20:01:43 2011 +0800
> @@ -2333,6 +2333,7 @@ static int emulate_privileged_op(struct 
>                  goto fail;
>              break;
>          case MSR_IA32_THERM_CONTROL:
> +        case MSR_IA32_ENERGY_PERF_BIAS:
>              if ( boot_cpu_data.x86_vendor != X86_VENDOR_INTEL )
>                  goto fail;
>              if ( (v->domain->domain_id != 0) || !v->domain->is_pinned )

Why would you allow this only if Dom0 has its vcpus pinned?

Jan

RE: [PATCH] Allow dom0 to write MSR IA32_ENERGY_PERF_BIAS

[Wei, Gang]

Jan Beulich wrote on 2011-01-04:
>> diff -r 4e108cf56d07 xen/arch/x86/traps.c
>> --- a/xen/arch/x86/traps.c	Mon Dec 27 08:00:09 2010 +0000
>> +++ b/xen/arch/x86/traps.c	Sat Jan 01 20:01:43 2011 +0800
>> @@ -2333,6 +2333,7 @@ static int emulate_privileged_op(struct
>>                  goto fail;
>>              break;
>>          case MSR_IA32_THERM_CONTROL:
>> +        case MSR_IA32_ENERGY_PERF_BIAS:
>>              if ( boot_cpu_data.x86_vendor != X86_VENDOR_INTEL )
>>                  goto fail;
>>              if ( (v->domain->domain_id != 0) ||
>> !v->domain->is_pinned
>> )
> 
> Why would you allow this only if Dom0 has its vcpus pinned?

It is meaningless if dom0 can't control all pcpus exactly. Only in case of dom0 vcpus pinned, it makes sense.

Jimmy

RE: [PATCH] Allow dom0 to write MSR IA32_ENERGY_PERF_BIAS

[Jan Beulich]

>>> On 05.01.11 at 03:08, "Wei, Gang" <gang.wei@intel.com> wrote:
> Jan Beulich wrote on 2011-01-04:
>>> diff -r 4e108cf56d07 xen/arch/x86/traps.c
>>> --- a/xen/arch/x86/traps.c	Mon Dec 27 08:00:09 2010 +0000
>>> +++ b/xen/arch/x86/traps.c	Sat Jan 01 20:01:43 2011 +0800
>>> @@ -2333,6 +2333,7 @@ static int emulate_privileged_op(struct
>>>                  goto fail;
>>>              break;
>>>          case MSR_IA32_THERM_CONTROL:
>>> +        case MSR_IA32_ENERGY_PERF_BIAS:
>>>              if ( boot_cpu_data.x86_vendor != X86_VENDOR_INTEL )
>>>                  goto fail;
>>>              if ( (v->domain->domain_id != 0) ||
>>> !v->domain->is_pinned
>>> )
>> 
>> Why would you allow this only if Dom0 has its vcpus pinned?
> 
> It is meaningless if dom0 can't control all pcpus exactly. Only in case of 
> dom0 vcpus pinned, it makes sense.

Disagree. The user mode tool could set its own affinity (virtual and
physical) and then issue the MSR write. Please don't enforce
restrictions where not really needed (I actually suppose that the
restriction should be removed for MSR_IA32_THERM_CONTROL too).

Jan

Re: [PATCH] Allow dom0 to write MSR IA32_ENERGY_PERF_BIAS

[Keir Fraser]

On 05/01/2011 07:59, "Jan Beulich" <JBeulich@novell.com> wrote:

>>> Why would you allow this only if Dom0 has its vcpus pinned?
>> 
>> It is meaningless if dom0 can't control all pcpus exactly. Only in case of
>> dom0 vcpus pinned, it makes sense.
> 
> Disagree. The user mode tool could set its own affinity (virtual and
> physical) and then issue the MSR write. Please don't enforce
> restrictions where not really needed (I actually suppose that the
> restriction should be removed for MSR_IA32_THERM_CONTROL too).

If so, it deserves a separate patch to strip out *all* the is_pinned checks
at the same time.

 -- Keir

Re: [PATCH] Allow dom0 to write MSR IA32_ENERGY_PERF_BIAS

[Jan Beulich]

>>> On 05.01.11 at 09:13, Keir Fraser <keir.fraser@eu.citrix.com> wrote:
> On 05/01/2011 07:59, "Jan Beulich" <JBeulich@novell.com> wrote:
> 
>>>> Why would you allow this only if Dom0 has its vcpus pinned?
>>> 
>>> It is meaningless if dom0 can't control all pcpus exactly. Only in case of
>>> dom0 vcpus pinned, it makes sense.
>> 
>> Disagree. The user mode tool could set its own affinity (virtual and
>> physical) and then issue the MSR write. Please don't enforce
>> restrictions where not really needed (I actually suppose that the
>> restriction should be removed for MSR_IA32_THERM_CONTROL too).
> 
> If so, it deserves a separate patch to strip out *all* the is_pinned checks
> at the same time.

You certainly don't mean *all*, but yes, I'm intending to submit such
a patch.

Jan

Re: [PATCH] Allow dom0 to write MSR IA32_ENERGY_PERF_BIAS

[Keir Fraser]

On 05/01/2011 08:17, "Jan Beulich" <JBeulich@novell.com> wrote:

>>>> On 05.01.11 at 09:13, Keir Fraser <keir.fraser@eu.citrix.com> wrote:
>> On 05/01/2011 07:59, "Jan Beulich" <JBeulich@novell.com> wrote:
>> 
>>>>> Why would you allow this only if Dom0 has its vcpus pinned?
>>>> 
>>>> It is meaningless if dom0 can't control all pcpus exactly. Only in case of
>>>> dom0 vcpus pinned, it makes sense.
>>> 
>>> Disagree. The user mode tool could set its own affinity (virtual and
>>> physical) and then issue the MSR write. Please don't enforce
>>> restrictions where not really needed (I actually suppose that the
>>> restriction should be removed for MSR_IA32_THERM_CONTROL too).
>> 
>> If so, it deserves a separate patch to strip out *all* the is_pinned checks
>> at the same time.
> 
> You certainly don't mean *all*, but yes, I'm intending to submit such
> a patch.

The ones in x86/traps.c (WRMSR emulation) and x86/domain.c
(VCPUOP_get_physid) are both unnecessary, at least.

 -- Keir

> Jan
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [PATCH] Allow dom0 to write MSR IA32_ENERGY_PERF_BIAS

[Jan Beulich]

>>> On 05.01.11 at 09:22, Keir Fraser <keir@xen.org> wrote:
> On 05/01/2011 08:17, "Jan Beulich" <JBeulich@novell.com> wrote:
> 
>>>>> On 05.01.11 at 09:13, Keir Fraser <keir.fraser@eu.citrix.com> wrote:
>>> On 05/01/2011 07:59, "Jan Beulich" <JBeulich@novell.com> wrote:
>>> 
>>>>>> Why would you allow this only if Dom0 has its vcpus pinned?
>>>>> 
>>>>> It is meaningless if dom0 can't control all pcpus exactly. Only in case of
>>>>> dom0 vcpus pinned, it makes sense.
>>>> 
>>>> Disagree. The user mode tool could set its own affinity (virtual and
>>>> physical) and then issue the MSR write. Please don't enforce
>>>> restrictions where not really needed (I actually suppose that the
>>>> restriction should be removed for MSR_IA32_THERM_CONTROL too).
>>> 
>>> If so, it deserves a separate patch to strip out *all* the is_pinned checks
>>> at the same time.
>> 
>> You certainly don't mean *all*, but yes, I'm intending to submit such
>> a patch.
> 
> The ones in x86/traps.c (WRMSR emulation) and x86/domain.c
> (VCPUOP_get_physid) are both unnecessary, at least.

They aren't outright unnecessary I'd say, they just need some
relaxing (as the code makes sense also when the vCPU is
constrained to a single pCPU). That's the change I'm going to
send shortly.

Jan

RE: [PATCH] Allow dom0 to write MSR IA32_ENERGY_PERF_BIAS

[Wei, Gang]

Jan Beulich wrote on 2011-01-05:
>>>> On 05.01.11 at 09:22, Keir Fraser <keir@xen.org> wrote:
>> On 05/01/2011 08:17, "Jan Beulich" <JBeulich@novell.com> wrote:
>> 
>>>>>> On 05.01.11 at 09:13, Keir Fraser <keir.fraser@eu.citrix.com> wrote:
>>>> On 05/01/2011 07:59, "Jan Beulich" <JBeulich@novell.com> wrote:
>>>> 
>>>>>>> Why would you allow this only if Dom0 has its vcpus pinned?
>>>>>> 
>>>>>> It is meaningless if dom0 can't control all pcpus exactly. Only
>>>>>> in case of dom0 vcpus pinned, it makes sense.
>>>>> 
>>>>> Disagree. The user mode tool could set its own affinity (virtual
>>>>> and
>>>>> physical) and then issue the MSR write. Please don't enforce
>>>>> restrictions where not really needed (I actually suppose that the
>>>>> restriction should be removed for MSR_IA32_THERM_CONTROL too).

Ok, I accept such kind of usages. So how about simply check in my patch and do remove these restrictions in your following patches?

>>>> 
>>>> If so, it deserves a separate patch to strip out *all* the
>>>> is_pinned checks at the same time.
>>> 
>>> You certainly don't mean *all*, but yes, I'm intending to submit
>>> such a patch.
>> 
>> The ones in x86/traps.c (WRMSR emulation) and x86/domain.c
>> (VCPUOP_get_physid) are both unnecessary, at least.
> 
> They aren't outright unnecessary I'd say, they just need some relaxing
> (as the code makes sense also when the vCPU is constrained to a single
> pCPU). That's the change I'm going to send shortly.


Jimmy

RE: [PATCH] Allow dom0 to write MSR IA32_ENERGY_PERF_BIAS

[Jan Beulich]

>>> On 05.01.11 at 09:55, "Wei, Gang" <gang.wei@intel.com> wrote:
> Jan Beulich wrote on 2011-01-05:
>>>>> On 05.01.11 at 09:22, Keir Fraser <keir@xen.org> wrote:
>>> On 05/01/2011 08:17, "Jan Beulich" <JBeulich@novell.com> wrote:
>>> 
>>>>>>> On 05.01.11 at 09:13, Keir Fraser <keir.fraser@eu.citrix.com> wrote:
>>>>> On 05/01/2011 07:59, "Jan Beulich" <JBeulich@novell.com> wrote:
>>>>> 
>>>>>>>> Why would you allow this only if Dom0 has its vcpus pinned?
>>>>>>> 
>>>>>>> It is meaningless if dom0 can't control all pcpus exactly. Only
>>>>>>> in case of dom0 vcpus pinned, it makes sense.
>>>>>> 
>>>>>> Disagree. The user mode tool could set its own affinity (virtual
>>>>>> and
>>>>>> physical) and then issue the MSR write. Please don't enforce
>>>>>> restrictions where not really needed (I actually suppose that the
>>>>>> restriction should be removed for MSR_IA32_THERM_CONTROL too).
> 
> Ok, I accept such kind of usages. So how about simply check in my patch and 
> do remove these restrictions in your following patches?

Yes, that's what we all seem to agree to now (and I just sent out
that other patch).

Jan