Patches to work with processor-passthru driver (v1).

RFC PATCH New Konrad Rzeszutek Wilk 4 participants 11 messages Feb 24, 2012 – Mar 12, 2012

Subsystems (2)

Xen hypervisor interface CPU frequency scaling framework

lore

Share & tools

lore

Konrad Rzeszutek Wilk

author

2012-02-24 06:43 UTC #0 lore raw

RFC PATCH 1/1 Patches to work with processor-passthru driver (v1).

These two patches provide the neccessary infrastructure changes
for the processor-passthru driver [www.spinics.net/lists/linux-acpi/msg34655.html] to properly function.

The first one is quite easy - we just modprobe the processor-passthru driver.

The second allows it to work under AMD machines by exposing the PM RDMSR to dom0. It has been tested with 2.6.32 kernel as well to make sure it does not negativly impact them and it does not.

Konrad Rzeszutek Wilk

author

2012-02-24 06:43 UTC #1 lore raw

PATCH 1/1 traps: AMD PM RDMSRs (MSR_K8_PSTATE_CTRL, etc)

# HG changeset patch
# User Konrad Rzeszutek Wilk <[email protected]> # Date 1330065828 18000
# Node ID a34b652afb0ca484b7416008c95fd36ffbbea334 # Parent a4d93d0e0df2fafe5b3e2dab3e34799498a875e2
traps: AMD PM RDMSRs (MSR_K8_PSTATE_CTRL, etc)

The restriction to read and write the AMD power management MSRs is gated if the domain 0 is the PM domain (so FREQCTL_dom0_kernel is set). But we can relax this restriction and allow the privileged domain to read the MSRs (but not write). This allows the priviliged domain to harvest the power management information (ACPI _PSS states) and send it to the hypervisor.

Signed-off-by Konrad Rzeszutek Wilk <[email protected]>

xen/arch/x86/traps.c +1 -1

diff -r a4d93d0e0df2 -r a34b652afb0c xen/arch/x86/traps.c
@@ -2610,7 +2610,7 @@ static int emulate_privileged_op(struct 
         case MSR_K8_PSTATE7:
             if ( boot_cpu_data.x86_vendor != X86_VENDOR_AMD )
                 goto fail;
-            if ( !is_cpufreq_controller(v->domain) )
+            if ( !is_cpufreq_controller(v->domain) && !IS_PRIV(v->domain) )
             {
                 regs->eax = regs->edx = 0;
                 break;

Jan Beulich

2012-02-24 10:38 UTC #2 lore raw

Re: [PATCH 1 of 2] traps: AMD PM RDMSRs (MSR_K8_PSTATE_CTRL, etc)

Quoted 21 lines

Quoted 1 line
Quoted 1 line · Konrad Rzeszutek Wilk
# HG changeset patch
# User Konrad Rzeszutek Wilk <[email protected]> # Date 1330065828 18000
# Node ID a34b652afb0ca484b7416008c95fd36ffbbea334 # Parent a4d93d0e0df2fafe5b3e2dab3e34799498a875e2
traps: AMD PM RDMSRs (MSR_K8_PSTATE_CTRL, etc)
The restriction to read and write the AMD power management MSRs is gated if the
domain 0 is the PM domain (so FREQCTL_dom0_kernel is set). But we can relax this restriction and allow the privileged domain to read the MSRs (but not write). This allows the priviliged domain to harvest the power management information (ACPI _PSS states) and send it to the hypervisor.
Signed-off-by Konrad Rzeszutek Wilk <[email protected]>
diff -r a4d93d0e0df2 -r a34b652afb0c xen/arch/x86/traps.c

xen/arch/x86/traps.c static int emulate_privileged_op(struct · line 2613

Quoted 5 lines

xen/arch/x86/traps.c +1 -1

@@ -2610,7 +2610,7 @@ static int emulate_privileged_op(struct 
         case MSR_K8_PSTATE7:
             if ( boot_cpu_data.x86_vendor != X86_VENDOR_AMD )
                 goto fail;
-            if ( !is_cpufreq_controller(v->domain) )
+            if ( !is_cpufreq_controller(v->domain) && !IS_PRIV(v->domain) )

As said already in response to your Linux side overview mail, while I don't view this as having potential to cause any problems, I also don't see the need.

And then if this indeed is wanted, replacing is_cpufreq_controller() by IS_PRIV() rather than adding the latter would seem preferable (because of being more strait forward).

Jan

Quoted 10 lines

xen/arch/x86/traps.c +0 -0
             {
                 regs->eax = regs->edx = 0;
                 break;
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

Keir Fraser

2012-03-01 16:33 UTC #3 lore raw

Re: [PATCH 1 of 2] traps: AMD PM RDMSRs (MSR_K8_PSTATE_CTRL, etc)

Quoted 31 lines · Jan Beulich

Quoted 21 lines
Quoted 1 line
Quoted 1 line · Konrad Rzeszutek Wilk
# HG changeset patch
# User Konrad Rzeszutek Wilk <[email protected]> # Date 1330065828 18000
# Node ID a34b652afb0ca484b7416008c95fd36ffbbea334 # Parent a4d93d0e0df2fafe5b3e2dab3e34799498a875e2
traps: AMD PM RDMSRs (MSR_K8_PSTATE_CTRL, etc)
The restriction to read and write the AMD power management MSRs is gated if the
domain 0 is the PM domain (so FREQCTL_dom0_kernel is set). But we can relax this restriction and allow the privileged domain to read the MSRs (but not write). This allows the priviliged domain to harvest the power management information (ACPI _PSS states) and send it to the hypervisor.
Signed-off-by Konrad Rzeszutek Wilk <[email protected]>
diff -r a4d93d0e0df2 -r a34b652afb0c xen/arch/x86/traps.c
xen/arch/x86/traps.c Fri Feb 24 01:43:48 2012 -0500 static int emulate_privileged_op(struct · line 2613
Quoted 5 lines
xen/arch/x86/traps.c Fri Feb 24 01:43:48 2012 -0500 renamed from xen/arch/x86/traps.c Wed Feb 22 14:33:24 2012 +0000 +1 -1
@@ -2610,7 +2610,7 @@ static int emulate_privileged_op(struct
         case MSR_K8_PSTATE7:
             if ( boot_cpu_data.x86_vendor != X86_VENDOR_AMD )
                 goto fail;
-            if ( !is_cpufreq_controller(v->domain) )
+            if ( !is_cpufreq_controller(v->domain) && !IS_PRIV(v->domain) )
As said already in response to your Linux side overview mail, while I don't view this as having potential to cause any problems, I also don't see the need.

Quoted 3 lines

And then if this indeed is wanted, replacing is_cpufreq_controller() by IS_PRIV() rather than adding the latter would seem preferable (because of being more strait forward).

Any response to this, Konrad?

Quoted 20 lines

Jan
Quoted 10 lines
xen/arch/x86/traps.c Fri Feb 24 01:43:48 2012 -0500 renamed from xen/arch/x86/traps.c Wed Feb 22 14:33:24 2012 +0000 +0 -0
             {
                 regs->eax = regs->edx = 0;
                 break;
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

Konrad Rzeszutek Wilk

author

2012-03-12 18:11 UTC #4 lore raw

Re: [PATCH 1 of 2] traps: AMD PM RDMSRs (MSR_K8_PSTATE_CTRL, etc)

Quoted 33 lines · Keir Fraser

Quoted 31 lines · Jan Beulich
Quoted 21 lines
Quoted 1 line
Quoted 1 line · Konrad Rzeszutek Wilk
# HG changeset patch
# User Konrad Rzeszutek Wilk <[email protected]> # Date 1330065828 18000
# Node ID a34b652afb0ca484b7416008c95fd36ffbbea334 # Parent a4d93d0e0df2fafe5b3e2dab3e34799498a875e2
traps: AMD PM RDMSRs (MSR_K8_PSTATE_CTRL, etc)
The restriction to read and write the AMD power management MSRs is gated if the
domain 0 is the PM domain (so FREQCTL_dom0_kernel is set). But we can relax this restriction and allow the privileged domain to read the MSRs (but not write). This allows the priviliged domain to harvest the power management information (ACPI _PSS states) and send it to the hypervisor.
Signed-off-by Konrad Rzeszutek Wilk <[email protected]>
diff -r a4d93d0e0df2 -r a34b652afb0c xen/arch/x86/traps.c
xen/arch/x86/traps.c Fri Feb 24 01:43:48 2012 -0500 static int emulate_privileged_op(struct · line 2613
Quoted 5 lines
xen/arch/x86/traps.c Fri Feb 24 01:43:48 2012 -0500 renamed from xen/arch/x86/traps.c Wed Feb 22 14:33:24 2012 +0000 +1 -1
@@ -2610,7 +2610,7 @@ static int emulate_privileged_op(struct
         case MSR_K8_PSTATE7:
             if ( boot_cpu_data.x86_vendor != X86_VENDOR_AMD )
                 goto fail;
-            if ( !is_cpufreq_controller(v->domain) )
+            if ( !is_cpufreq_controller(v->domain) && !IS_PRIV(v->domain) )
As said already in response to your Linux side overview mail, while I don't view this as having potential to cause any problems, I also don't see the need.

Quoted 5 lines

Quoted 3 lines
And then if this indeed is wanted, replacing is_cpufreq_controller() by IS_PRIV() rather than adding the latter would seem preferable (because of being more strait forward).
Any response to this, Konrad?

Ah, lets drop this patch. Jan got me thinking about doing it in a much nicer way without a need to do the MSRs.

Quoted 24 lines

K.
Quoted 20 lines
Jan
Quoted 10 lines
xen/arch/x86/traps.c Fri Feb 24 01:43:48 2012 -0500 renamed from xen/arch/x86/traps.c Wed Feb 22 14:33:24 2012 +0000 +0 -0
             {
                 regs->eax = regs->edx = 0;
                 break;
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

Konrad Rzeszutek Wilk

author

2012-02-24 06:43 UTC #5 lore raw

PATCH 1/1 linux-xencommons: Load processor-passthru

In reply to Konrad Rzeszutek Wilk

# HG changeset patch
# User Konrad Rzeszutek Wilk <[email protected]> # Date 1330065828 18000
# Node ID aa3a294327ed075bafbe3c070b39e3dbacbc49cd # Parent a34b652afb0ca484b7416008c95fd36ffbbea334
linux-xencommons: Load processor-passthru

The processor-passthru module is used in the upstream kernels to upload power management information to the hypervisor. We need to load it to work first.

Signed-off-by Konrad Rzeszutek Wilk <[email protected]>

tools/hotplug/Linux/init.d/xencommons +1 -0

diff -r a34b652afb0c -r aa3a294327ed tools/hotplug/Linux/init.d/xencommons
@@ -58,6 +58,7 @@ do_start () {
 	modprobe xen-gntdev 2>/dev/null
 	modprobe evtchn 2>/dev/null
 	modprobe gntdev 2>/dev/null
+	modprobe processor-passthru 2>/dev/null
 
 	if ! `xenstore-read -s / >/dev/null 2>&1`
 	then

Jan Beulich

2012-02-24 10:44 UTC #6 lore raw

Re: [PATCH 2 of 2] linux-xencommons: Load processor-passthru

Quoted 11 lines

Quoted 1 line
Quoted 1 line · Konrad Rzeszutek Wilk
# HG changeset patch
# User Konrad Rzeszutek Wilk <[email protected]> # Date 1330065828 18000
# Node ID aa3a294327ed075bafbe3c070b39e3dbacbc49cd # Parent a34b652afb0ca484b7416008c95fd36ffbbea334
linux-xencommons: Load processor-passthru
The processor-passthru module is used in the upstream kernels to upload power management information to the hypervisor. We need to load it to work first.

Hmm, I don't really like this (and prior) pvops-specific additions here, even if stderr gets directed into nowhere. I don't think this (and any other) script intended to target Linux in general should target just a specific implementation.

Furthermore, given the purpose of the driver, I'm thinking that this is too late in the boot process anyway, and the driver should rather be loaded at the point where other CPUFreq drivers would normally be by a particular distro (which would still be later than when the respective code gets run on traditional Xenified Linux).

Jan

Quoted 20 lines

Signed-off-by Konrad Rzeszutek Wilk <[email protected]>
diff -r a34b652afb0c -r aa3a294327ed tools/hotplug/Linux/init.d/xencommons
tools/hotplug/Linux/init.d/xencommons +1 -0
@@ -58,6 +58,7 @@ do_start () {
 	modprobe xen-gntdev 2>/dev/null
 	modprobe evtchn 2>/dev/null
 	modprobe gntdev 2>/dev/null
+	modprobe processor-passthru 2>/dev/null
 
 	if ! `xenstore-read -s / >/dev/null 2>&1`
 	then
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

Konrad Rzeszutek Wilk

author

2012-02-27 01:46 UTC #7 lore raw

Re: [PATCH 2 of 2] linux-xencommons: Load processor-passthru

Quoted 17 lines · Jan Beulich

Quoted 11 lines
Quoted 1 line
Quoted 1 line · Konrad Rzeszutek Wilk
# HG changeset patch
# User Konrad Rzeszutek Wilk <[email protected]> # Date 1330065828 18000
# Node ID aa3a294327ed075bafbe3c070b39e3dbacbc49cd # Parent a34b652afb0ca484b7416008c95fd36ffbbea334
linux-xencommons: Load processor-passthru
The processor-passthru module is used in the upstream kernels to upload power management information to the hypervisor. We need to load it to work first.
Hmm, I don't really like this (and prior) pvops-specific additions here, even if stderr gets directed into nowhere. I don't think this (and any other) script intended to target Linux in general should target just a specific implementation.

Quoted 5 lines

Furthermore, given the purpose of the driver, I'm thinking that this is too late in the boot process anyway, and the driver should rather be loaded at the point where other CPUFreq drivers would normally be by a particular distro (which would still be later than when the respective code gets run on traditional Xenified Linux).

My thinking is to keep the amount of changes to be within the Xen-ecosystem.

Doing the changes in the old-fashioned way in drivers/acpi has not been very productive - so I've been looking at just some form of uploading the PM information to the hypervisor. And I've been piggybacking on top of the cpufreq scaling drivers collecting the information. So with that in mind, the next idea I had was to provide a cpufreq governor, called 'xen' which would inhibit the cpufreq scaling drivers from changing anything in dom0 (so that the hypervisor can do it instead). Also it would be responsible for uploading the PM information to the hypervisor. See attached.

The fix to the tool-stack would be something along:
# HG changeset patch
# Parent 917f845f3e838dcc1efccb132abe2d1f254cb7b8

tools/hotplug/Linux/init.d/xencommons +1 -1

diff -r 917f845f3e83 tools/hotplug/Linux/init.d/xencommons
@@ -58,7 +58,7 @@ do_start () {
 	modprobe xen-gntdev 2>/dev/null
 	modprobe evtchn 2>/dev/null
 	modprobe gntdev 2>/dev/null
-
+	for file in /sys/devices/system/cpu/cpu*/cpufreq/scaling_governor; do echo xen > $file; done 2>/dev/null
 	if ! `xenstore-read -s / >/dev/null 2>&1`
 	then
 		test -z "$XENSTORED_ROOTDIR" || XENSTORED_ROOTDIR="/var/lib/xenstored"

Quoted 1 line

From 20e7a07fa0f8a0dbe30a0f732686d78849d29d96 Mon Sep 17 00:00:00 2001

From: Konrad Rzeszutek Wilk <[email protected]>
Date: Fri, 3 Feb 2012 16:03:20 -0500
Subject: [PATCH] [CPUFREQ] xen: governor for Xen hypervisor frequency
scaling.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This CPU freq governor leaves the frequency decision to the Xen hypervisor.

To do that the driver parses the Power Management data and uploads said information to the Xen hypervisor. Then the Xen hypervisor can select the proper Cx and Pxx states for the initial domain and all other domains.

To upload the information, this CPU frequency driver reads Power Management (PM) (_Pxx and _Cx) which are populated in the 'struct acpi_processor' structure. It simply reads the contents of that structure and pass it up the Xen hypervisor. For that to work we depend on the appropriate CPU frequency scaling driver to do the heavy-lifting - so that the contents is correct.

The CPU frequency governor it has been loaded also sets up a timer to check if the ACPI IDs count is different from the APIC ID count - which can happen if the user choose to use dom0_max_vcpu argument. In such a case a backup of the PM structure is used and uploaded to the hypervisor.

[v1-v2: Initial RFC implementations that were posted]

[v3: Changed the name to passthru suggested by Pasi Kärkkäinen <[email protected]>]

[v4: Added vCPU != pCPU support - aka dom0_max_vcpus support]

[v5: Cleaned up the driver, fix bug under Athlon XP]

[v6: Changed the driver to a CPU frequency governor]

Signed-off-by Konrad Rzeszutek Wilk <[email protected]>

---
 drivers/xen/Kconfig       |   15 ++
 drivers/xen/Makefile      |    2 +-
 drivers/xen/cpufreq_xen.c |  445 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 461 insertions(+), 1 deletions(-)
 create mode 100644 drivers/xen/cpufreq_xen.c

drivers/xen/Kconfig +15 -0

index a1ced52..28ba371 100644
@@ -178,4 +178,19 @@ config XEN_PRIVCMD
 	depends on XEN
 	default m
 
+config CPU_FREQ_GOV_XEN
+	tristate "'xen' governor for hypervisor scaling"
+	depends on XEN && X86 && ACPI_PROCESSOR && CPU_FREQ
+	default m
+	help
+          This cpufreq governor leaves the frequency decision to the Xen hypervisor.
+
+	  To do that the driver parses the Power Management data and uploads said
+	  information to the Xen hypervisor. Then the Xen hypervisor can select the
+          proper Cx and Pxx states.
+
+          To compile this driver as a module, choose M here: the
+          module will be called cpufreq_xen.  If you do not know what to choose,
+          select M here.
+
 endmenu

drivers/xen/Makefile +1 -1

index aa31337..5802220 100644
@@ -20,7 +20,7 @@ obj-$(CONFIG_SWIOTLB_XEN)		+= swiotlb-xen.o
 obj-$(CONFIG_XEN_DOM0)			+= pci.o
 obj-$(CONFIG_XEN_PCIDEV_BACKEND)	+= xen-pciback/
 obj-$(CONFIG_XEN_PRIVCMD)		+= xen-privcmd.o
-
+obj-$(CONFIG_CPU_FREQ_GOV_XEN)		+= cpufreq_xen.o
 xen-evtchn-y				:= evtchn.o
 xen-gntdev-y				:= gntdev.o
 xen-gntalloc-y				:= gntalloc.o

drivers/xen/cpufreq_xen.c new file +445 -0

445 lines not on this page · raw message

Signature · 1 line

1.7.9.48.g85da4d

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

Jan Beulich

2012-02-27 08:31 UTC #8 lore raw

Re: [PATCH 2 of 2] linux-xencommons: Load processor-passthru

Quoted 45 lines

Quoted 1 line
Quoted 1 line · Konrad Rzeszutek Wilk
Quoted 17 lines · Jan Beulich
Quoted 11 lines
Quoted 1 line
Quoted 1 line · Konrad Rzeszutek Wilk
# HG changeset patch
# User Konrad Rzeszutek Wilk <[email protected]> # Date 1330065828 18000
# Node ID aa3a294327ed075bafbe3c070b39e3dbacbc49cd # Parent a34b652afb0ca484b7416008c95fd36ffbbea334
linux-xencommons: Load processor-passthru
The processor-passthru module is used in the upstream kernels to upload power management information to the hypervisor. We need to load it to work first.
Hmm, I don't really like this (and prior) pvops-specific additions here, even if stderr gets directed into nowhere. I don't think this (and any other) script intended to target Linux in general should target just a specific implementation.
Quoted 5 lines
Furthermore, given the purpose of the driver, I'm thinking that this is too late in the boot process anyway, and the driver should rather be loaded at the point where other CPUFreq drivers would normally be by a particular distro (which would still be later than when the respective code gets run on traditional Xenified Linux).
My thinking is to keep the amount of changes to be within the Xen-ecosystem.
Doing the changes in the old-fashioned way in drivers/acpi has not been very productive - so I've been looking at just some form of uploading the PM information to the hypervisor. And I've been piggybacking on top of the cpufreq scaling drivers collecting the information. So with that in mind, the next idea I had was to provide a cpufreq governor, called 'xen' which would inhibit the cpufreq scaling drivers from changing anything in dom0
(so that the hypervisor can do it instead). Also it would be responsible for uploading the PM information to the hypervisor. See attached.
The fix to the tool-stack would be something along:
# HG changeset patch
# Parent 917f845f3e838dcc1efccb132abe2d1f254cb7b8
diff -r 917f845f3e83 tools/hotplug/Linux/init.d/xencommons

tools/hotplug/Linux/init.d/xencommons do_start () { · line 61

Quoted 5 lines

tools/hotplug/Linux/init.d/xencommons +1 -1

@@ -58,7 +58,7 @@ do_start () {
 	modprobe xen-gntdev 2>/dev/null
 	modprobe evtchn 2>/dev/null
 	modprobe gntdev 2>/dev/null
-
+	for file in /sys/devices/system/cpu/cpu*/cpufreq/scaling_governor; do echo xen > $file; done 2>/dev/null

While not an unreasonable idea (yes, I sort of contradict my other mail from a few minutes ago, given your explanation above), this won't work with subsequently onlined vCPU-s (and hence, consistent with my other mail, having the thing be a governor is likely the wrong choice - it ought to be a driver).

Jan

Quoted 3 lines

tools/hotplug/Linux/init.d/xencommons +0 -0

 	if ! `xenstore-read -s / >/dev/null 2>&1`
 	then
 		test -z "$XENSTORED_ROOTDIR" || XENSTORED_ROOTDIR="/var/lib/xenstored"

Konrad Rzeszutek Wilk

2012-03-05 02:49 UTC #9 lore raw

Re: [PATCH 2 of 2] linux-xencommons: Load processor-passthru

Quoted 4 lines

Quoted 4 lines
Quoted 4 lines
Furthermore, given the purpose of the driver, I'm thinking that this is too late in the boot process anyway, and the driver should rather be loaded at the point where other CPUFreq drivers would normally be by a particular distro (which would still be later than when the

In Ubuntu and Fedora they are set to:
CONFIG_X86_POWERNOW_K8=y
CONFIG_X86_ACPI_CPUFREQ=y

(thought previous to Fedora 16 POWERNOW_K8 was =m, per
https://bugzilla.redhat.com/show_bug.cgi?id=739159, " powernow-k8 transitions fail in xen dom0")

Looking at how the built is done with =y, the cpufreq drivers are loaded in the late_init stage and there is a strict order (drivers/cpufreq/Makefile):

  # x86 drivers.
  # Link order matters. K8 is preferred to ACPI because of firmware
bugs in early
  # K8 systems. ACPI is preferred to all other hardware-specific drivers.
  # speedstep-* is preferred over p4-clockmod.

Both of them (acpi-cpufreq.c and powernow-k8.c) have a symbol dependency on drivers/acpi/processor.c

Quoted 3 lines

Quoted 3 lines
Quoted 1 line
respective code gets run on traditional Xenified Linux).
My thinking is to keep the amount of changes to be within the Xen-ecosystem.

.. snip..

Quoted 1 line

Quoted 1 line
The fix to the tool-stack would be something along:

.. snip..

Quoted 5 lines

Quoted 5 lines
diff -r 917f845f3e83 tools/hotplug/Linux/init.d/xencommons
tools/hotplug/Linux/init.d/xencommons Fri Feb 24 21:42:20 2012 -0500 renamed from tools/hotplug/Linux/init.d/xencommons Thu Feb 23 13:29:00 2012 -0500 +0 -0
@@ -58,7 +58,7 @@ do_start () {
modprobe xen-gntdev 2>/dev/null

Quoted 8 lines

Quoted 4 lines
modprobe evtchn 2>/dev/null
modprobe gntdev 2>/dev/null
-
+ for file in /sys/devices/system/cpu/cpu*/cpufreq/scaling_governor; do echo xen > $file; done 2>/dev/null
While not an unreasonable idea (yes, I sort of contradict my other mail from a few minutes ago, given your explanation above), this won't work with subsequently onlined vCPU-s (and hence, consistent with my other

This udev rule could do it? [completely untested]

ACTION=="add", SUBSYSTEM=="cpu", RUN+="/bin/sh -c '[ ! -e /sys$devpath/online ] || echo xen > /sys$devpath/cpufreq/scaling_governor'"

Quoted 3 lines

mail, having the thing be a governor is likely the wrong choice - it ought to be a driver).

By driver, I think you mean either a) a new cpufreq scaling driver, or
b) the processor-core driver.

The b) is what the old Xen-O-Linux tree had, and a simplified version of this had been posted on LKML: https://lkml.org/lkml/2011/11/30/245
. There was no response from Len Brown about it and from the off-topic email conversations I got the hint that Len wasn't particularly thrilled about it.

For a), this would mean some form of unregistering the existing cpufreq scaling drivers.The reason
for that is we want to use the generic ones (acpi-cpufreq and powernow-k8) b/c they do all the filtering and parsing of the ACPI data instead of re-implementing it in our own cpufreq-xen-scaling. Thought one other option is to export both powernow-k8 and acpi-cpufreq functions that do this and use them within the cpufreq-xen-scaling-driver but that sounds icky.

I think the "unregistering" would be akin to "cpufreq_unregister_driver" but without any parameters. However, we would be the only user of this mechanism and I am not sure what Dave Jones feelings are about that (CC-ed here) Here is what I was thinking (this is of course completely untested/uncompiled):

drivers/cpufreq/cpufreq.c +0 -0

index 622013f..049fca9 100644
@@ -1881,9 +1881,12 @@ int cpufreq_unregister_driver(struct

cpufreq_driver *driver)
{
unsigned long flags;

- if (!cpufreq_driver || (driver != cpufreq_driver))
+ if (!cpufreq_driver)
return -EINVAL;

+ if (!driver)
+ driver = cpufreq_driver;
+
pr_debug("unregistering driver %s\n", driver->name);

subsys_interface_unregister(&cpufreq_interface);

There would also be the need to do some tweaking of the order of drivers to load (so that cpufreq-xen-scaling-driver gets loaded _after_ all of the other ones). And I am afraid
about the scenario where a cpufreq-gov-X tries to set a specific frequency and we yank it out
from within and might end up with a NULL pointer. But I think the "cpufreq_driver_lock" inhibits that. The other thing that strikes me as dangerous is that we don't call the module_exit of the cpufreq scaling driver.

While I was writing this up I thought about the overall picture. Right now, the APIs work in this way:

[acpi processors] -> [cpufreq-scaling] <- [cpufreq-gov-*]

And we want to leverage them to do two things:
1). Inhibit cpufreq-gov-* telling cpufreq-scaling drivers what to do [which is a bug
right now with Linux dom0 pvops - as the cpufreq-gov-* has no clue about the other domains and might set the P states to something incorrect]
2). Upload the power management information up to the hypervisor.

The 1) option can be either done by ignoring the WRMSR or outb in the hypervisor (but for that we need 2) to know _which_ ones to inhibit). Or it can be done by your
suggestion by introducing a cpufreq scaling driver that would nop all calls and unregister
the other drivers. Or lastly by the cpufreq-gov-xen driver which would leave it to the hypervisor to do it.

The 2). Can be done by either the proposed cpufreq-gov-xen, or the earlier patch set I sent called processor-passthru
[https://lwn.net/Articles/483668/] which is a simple driver that checks when cpufreq-scaling driver is loaded and then it upload the power management data. It does nothing
with the cpufreq-[scaling|gov] APIs. Granted it does need a 'modprobe processor-passthru' in the xencommons to take advantage of the data - and yes, it does deal with hot-cpu on lining.

Thoughts?

Jan Beulich

2012-03-05 11:43 UTC #10 lore raw

Re: [PATCH 2 of 2] linux-xencommons: Load processor-passthru

Quoted 17 lines

Quoted 5 lines
Quoted 5 lines · Konrad Rzeszutek Wilk
Quoted 4 lines
Furthermore, given the purpose of the driver, I'm thinking that this is too late in the boot process anyway, and the driver should rather be loaded at the point where other CPUFreq drivers would normally be by a particular distro (which would still be later than when the
In Ubuntu and Fedora they are set to:
CONFIG_X86_POWERNOW_K8=y
CONFIG_X86_ACPI_CPUFREQ=y
(thought previous to Fedora 16 POWERNOW_K8 was =m, per
https://bugzilla.redhat.com/show_bug.cgi?id=739159, " powernow-k8 transitions fail in xen dom0")
Looking at how the built is done with =y, the cpufreq drivers are loaded in the late_init stage and there is a strict order (drivers/cpufreq/Makefile):

Quoted 8 lines

  # x86 drivers.
  # Link order matters. K8 is preferred to ACPI because of firmware
bugs in early
  # K8 systems. ACPI is preferred to all other hardware-specific drivers.
  # speedstep-* is preferred over p4-clockmod.
Both of them (acpi-cpufreq.c and powernow-k8.c) have a symbol dependency on drivers/acpi/processor.c

But them being 'm' or 'y' shouldn't matter in the end.

Quoted 6 lines

Quoted 3 lines
mail, having the thing be a governor is likely the wrong choice - it ought to be a driver).
By driver, I think you mean either a) a new cpufreq scaling driver, or
b) the processor-core driver.

I really meant a).

Quoted 2 lines

For a), this would mean some form of unregistering the existing cpufreq scaling drivers.The reason

Or loading before them (and not depending on them), thus preventing them from loading successfully.

Quoted 6 lines

for that is we want to use the generic ones (acpi-cpufreq and powernow-k8) b/c they do all the filtering and parsing of the ACPI data instead of re-implementing it in our own cpufreq-xen-scaling. Thought one other option is to export both powernow-k8 and acpi-cpufreq functions that do this and use them within the cpufreq-xen-scaling-driver but that sounds icky.

Indeed.

Quoted 1 line

2). Upload the power management information up to the hypervisor.

Which doesn't require cpufreq drivers at all (in non-pv-ops we simply suppress the CPU_FREQ config option when XEN is set).

Jan