[PATCH] xen/acpi-processor: C and P-state driver that uploads said data to hypervisor. [v7]

[PATCH] xen/acpi-processor: C and P-state driver that uploads said data to hypervisor. [v7]

[Konrad Rzeszutek Wilk]

The problem this patch is trying to solve is to provide ACPI power management
information to the hypervisor from the initial domain. The hypervisor lacks the
ACPI DSDT parser so it can't get that data without some help - and the initial
domain can provide that. One approach (https://lkml.org/lkml/2011/11/30/245)
augments the ACPI code to call an external PM code - but there were no comments
about it so I decided to see if another approach could solve it.

It also solves the other problem of CPUfreq scaling drivers running in
the initial domain, changing frequencies without consulting the hypervisor for
the appropiate load information. This means that both the hypervisor and the
initial domain might be changing the P-states at the same time.

This module (xen-acpi-processor)  collects the information the same way that
the cpufreq drivers would utilize ACPI processor code and save everything in
the 'struct acpi_processor' and then uploads it to the hypervisor.

The driver can be either an module or compiled in. If compiled in, it will
launch itself before the CPUfreq scaling drivers to inhibit them. If as a module
then further work is needed in the init script to allow this driver to be loaded
before powernow-k8 or acpi-cpufreq.

[PATCH] xen/acpi-processor: C and P-state driver that uploads said data to hypervisor.

[Konrad Rzeszutek Wilk]

This driver solves three problems:
 1). Parse and upload ACPI0007 (or PROCESSOR_TYPE) information to the
     hypervisor - aka P-states (cpufreq data).
 2). Upload the the Cx state information (cpuidle data).
 3). Inhibit CPU frequency scaling drivers from loading.

The reason for wanting to solve 1) and 2) is such that the Xen hypervisor
is the only one that knows the CPU usage of different guests and can
make the proper decision of when to put CPUs and packages in proper states.
Unfortunately the hypervisor has no support to parse ACPI DSDT tables, hence it
needs help from the initial domain to provide this information. The reason
for 3) is that we do not want the initial domain to change P-states while the
hypervisor is doing it as well - it causes rather some funny cases of P-states
transitions.

For this to work, the driver parses the Power Management data and uploads said
information to the Xen hypervisor. It also calls acpi_processor_notify_smm()
to inhibit the other CPU frequency scaling drivers from being loaded.

Everything revolves around the 'struct acpi_processor' structure which
gets updated during the bootup cycle in different stages. At the startup, when
the ACPI parser starts, the C-state information is processed (processor_idle)
and saved in said structure as 'power' element. Later on, the CPU frequency
scaling driver (powernow-k8 or acpi_cpufreq), would call the the
acpi_processor_* (processor_perflib functions) to parse P-states information
and populate in the said structure the 'performance' element.

Since we do not want the CPU frequency scaling drivers from loading
we have to call the acpi_processor_* functions to parse the P-states and
call "acpi_processor_notify_smm" to stop them from loading.

There is also one oddity in this driver which is that under Xen, the
physical online CPU count can be different from the virtual online CPU count.
Meaning that the macros 'for_[online|possible]_cpu' would process only
up to virtual online CPU count. We on the other hand want to process
the full amount of physical CPUs. For that, the driver checks 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 <pasik@iki.fi>]
[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]
[v7: Made it a semi-cpufreq scaling type driver suggested by Jan Beulich <jbeulich@suse.com>]
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 drivers/xen/Kconfig              |   17 ++
 drivers/xen/Makefile             |    2 +-
 drivers/xen/xen-acpi-processor.c |  557 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 575 insertions(+), 1 deletions(-)
 create mode 100644 drivers/xen/xen-acpi-processor.c

diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
index a1ced52..648bcd4 100644
--- a/drivers/xen/Kconfig
+++ b/drivers/xen/Kconfig
@@ -178,4 +178,21 @@ config XEN_PRIVCMD
 	depends on XEN
 	default m
 
+config XEN_ACPI_PROCESSOR
+	tristate "Xen ACPI processor"
+	depends on XEN && X86 && ACPI_PROCESSOR
+	default y if (X86_ACPI_CPUFREQ = y || X86_POWERNOW_K8 = y)
+	default m if (X86_ACPI_CPUFREQ = m || X86_POWERNOW_K8 = m)
+	help
+          This ACPI processor uploads Power Management information 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. It also registers itslef as the SMM so that
+          other drivers (such as ACPI cpufreq scaling driver) will not load.
+
+          To compile this driver as a module, choose M here: the
+          module will be called xen_acpi_processor  If you do not know what to choose,
+          select M here. If the CPUFREQ drivers are built in, select Y here.
+
 endmenu
diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
index aa31337..9adc5be 100644
--- a/drivers/xen/Makefile
+++ b/drivers/xen/Makefile
@@ -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_XEN_ACPI_PROCESSOR)	+= xen-acpi-processor.o
 xen-evtchn-y				:= evtchn.o
 xen-gntdev-y				:= gntdev.o
 xen-gntalloc-y				:= gntalloc.o
diff --git a/drivers/xen/xen-acpi-processor.c b/drivers/xen/xen-acpi-processor.c
new file mode 100644
index 0000000..8c05fe7
--- /dev/null
+++ b/drivers/xen/xen-acpi-processor.c
@@ -0,0 +1,557 @@
+/*
+ * Copyright 2012 by Oracle Inc
+ * Author: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
+ *
+ * This code borrows ideas from https://lkml.org/lkml/2011/11/30/249
+ * so many thanks go to Kevin Tian <kevin.tian@intel.com>
+ * and Yu Ke <ke.yu@intel.com>.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ */
+
+#include <linux/cpumask.h>
+#include <linux/cpufreq.h>
+#include <linux/freezer.h>
+#include <linux/kernel.h>
+#include <linux/kthread.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/types.h>
+#include <acpi/acpi_bus.h>
+#include <acpi/acpi_drivers.h>
+#include <acpi/processor.h>
+
+#include <xen/interface/platform.h>
+#include <asm/xen/hypercall.h>
+
+#define DRV_NAME "xen-acpi-processor"
+
+static int no_hypercall;
+MODULE_PARM_DESC(off, "Inhibit the hypercall.");
+module_param_named(off, no_hypercall, int, 0400);
+
+/*
+ * Mutex to protect the acpi_ids_done.
+ */
+static DEFINE_MUTEX(acpi_ids_mutex);
+/*
+ * Don't think convert this to cpumask_var_t or use cpumask_bit - as those
+ * shrink to nr_cpu_bits (which is dependent on possible_cpu), which can be
+ * less than what we want to put in.
+ */
+#define NR_ACPI_CPUS	NR_CPUS
+#define MAX_ACPI_BITS	(BITS_TO_LONGS(NR_ACPI_CPUS))
+static unsigned long *acpi_ids_done;
+/*
+ * Again, don't convert to cpumask - as we are reading the raw ACPI CPU ids
+ * which can go beyond what we presently see.
+ */
+static unsigned long *acpi_id_present;
+static unsigned long *acpi_id_cst_present;
+
+static int push_cxx_to_hypervisor(struct acpi_processor *_pr)
+{
+	struct xen_platform_op op = {
+		.cmd			= XENPF_set_processor_pminfo,
+		.interface_version	= XENPF_INTERFACE_VERSION,
+		.u.set_pminfo.id	= _pr->acpi_id,
+		.u.set_pminfo.type	= XEN_PM_CX,
+	};
+	struct xen_processor_cx *dst_cx, *dst_cx_states = NULL;
+	struct acpi_processor_cx *cx;
+	unsigned int i, ok;
+	int ret = 0;
+
+	dst_cx_states = kcalloc(_pr->power.count,
+				sizeof(struct xen_processor_cx), GFP_KERNEL);
+	if (!dst_cx_states)
+		return -ENOMEM;
+
+	for (ok = 0, i = 1; i <= _pr->power.count; i++) {
+		cx = &_pr->power.states[i];
+		if (!cx->valid)
+			continue;
+
+		dst_cx = &(dst_cx_states[ok++]);
+
+		dst_cx->reg.space_id = ACPI_ADR_SPACE_SYSTEM_IO;
+		if (cx->entry_method == ACPI_CSTATE_SYSTEMIO) {
+			dst_cx->reg.bit_width = 8;
+			dst_cx->reg.bit_offset = 0;
+			dst_cx->reg.access_size = 1;
+		} else {
+			dst_cx->reg.space_id = ACPI_ADR_SPACE_FIXED_HARDWARE;
+			if (cx->entry_method == ACPI_CSTATE_FFH) {
+				/* NATIVE_CSTATE_BEYOND_HALT */
+				dst_cx->reg.bit_offset = 2;
+				dst_cx->reg.bit_width = 1; /* VENDOR_INTEL */
+			}
+			dst_cx->reg.access_size = 0;
+		}
+		dst_cx->reg.address = cx->address;
+
+		dst_cx->type = cx->type;
+		dst_cx->latency = cx->latency;
+		dst_cx->power = cx->power;
+
+		dst_cx->dpcnt = 0;
+		set_xen_guest_handle(dst_cx->dp, NULL);
+	}
+	if (!ok) {
+		pr_err(DRV_NAME ": No _Cx for ACPI CPU %u\n", _pr->acpi_id);
+		kfree(dst_cx_states);
+		return -EINVAL;
+	}
+	op.u.set_pminfo.power.count = ok;
+	op.u.set_pminfo.power.flags.bm_control = _pr->flags.bm_control;
+	op.u.set_pminfo.power.flags.bm_check = _pr->flags.bm_check;
+	op.u.set_pminfo.power.flags.has_cst = _pr->flags.has_cst;
+	op.u.set_pminfo.power.flags.power_setup_done =
+		_pr->flags.power_setup_done;
+
+	set_xen_guest_handle(op.u.set_pminfo.power.states, dst_cx_states);
+
+	if (!no_hypercall)
+		ret = HYPERVISOR_dom0_op(&op);
+
+	if (!ret) {
+		pr_debug("ACPI CPU%u - C-states uploaded.\n", _pr->acpi_id);
+		for (i = 1; i <= _pr->power.count; i++) {
+			cx = &_pr->power.states[i];
+			if (!cx->valid)
+				continue;
+			pr_debug("     C%d: %s %d uS\n",
+				 cx->type, cx->desc, (u32)cx->latency);
+		}
+	} else
+		pr_err(DRV_NAME "(CX): Hypervisor error (%d) for ACPI CPU%u\n",
+		       ret, _pr->acpi_id);
+
+	kfree(dst_cx_states);
+
+	return ret;
+}
+static struct xen_processor_px *
+xen_copy_pss_data(struct acpi_processor *_pr,
+		  struct xen_processor_performance *dst_perf)
+{
+	struct xen_processor_px *dst_states = NULL;
+	unsigned int i;
+
+	BUILD_BUG_ON(sizeof(struct xen_processor_px) !=
+		     sizeof(struct acpi_processor_px));
+
+	dst_states = kcalloc(_pr->performance->state_count,
+			     sizeof(struct xen_processor_px), GFP_KERNEL);
+	if (!dst_states)
+		return ERR_PTR(-ENOMEM);
+
+	dst_perf->state_count = _pr->performance->state_count;
+	for (i = 0; i < _pr->performance->state_count; i++) {
+		/* Fortunatly for us, they are both the same size */
+		memcpy(&(dst_states[i]), &(_pr->performance->states[i]),
+		       sizeof(struct acpi_processor_px));
+	}
+	return dst_states;
+}
+static int xen_copy_psd_data(struct acpi_processor *_pr,
+			     struct xen_processor_performance *dst)
+{
+	struct acpi_psd_package *pdomain;
+
+	BUILD_BUG_ON(sizeof(struct xen_psd_package) !=
+		     sizeof(struct acpi_psd_package));
+
+	/* This information is enumerated only if acpi_processor_preregister_performance
+	 * has been called.
+	 */
+	dst->shared_type = _pr->performance->shared_type;
+
+	pdomain = &(_pr->performance->domain_info);
+
+	/* 'acpi_processor_preregister_performance' does not parseif the
+	 * num_processors <= 1, but Xen still requires it. Do it manually here.
+	 */
+	if (pdomain->num_processors <= 1) {
+		if (pdomain->coord_type == DOMAIN_COORD_TYPE_SW_ALL)
+			dst->shared_type = CPUFREQ_SHARED_TYPE_ALL;
+		else if (pdomain->coord_type == DOMAIN_COORD_TYPE_HW_ALL)
+			dst->shared_type = CPUFREQ_SHARED_TYPE_HW;
+		else if (pdomain->coord_type == DOMAIN_COORD_TYPE_SW_ANY)
+			dst->shared_type = CPUFREQ_SHARED_TYPE_ANY;
+
+	}
+	memcpy(&(dst->domain_info), pdomain, sizeof(struct acpi_psd_package));
+	return 0;
+}
+static int xen_copy_pct_data(struct acpi_pct_register *pct,
+			     struct xen_pct_register *dst_pct)
+{
+	/* It would be nice if you could just do 'memcpy(pct, dst_pct') but
+	 * sadly the Xen structure did not have the proper padding so the
+	 * descriptor field takes two (dst_pct) bytes instead of one (pct).
+	 */
+	dst_pct->descriptor = pct->descriptor;
+	dst_pct->length = pct->length;
+	dst_pct->space_id = pct->space_id;
+	dst_pct->bit_width = pct->bit_width;
+	dst_pct->bit_offset = pct->bit_offset;
+	dst_pct->reserved = pct->reserved;
+	dst_pct->address = pct->address;
+	return 0;
+}
+static int push_pxx_to_hypervisor(struct acpi_processor *_pr)
+{
+	int ret = 0;
+	struct xen_platform_op op = {
+		.cmd			= XENPF_set_processor_pminfo,
+		.interface_version	= XENPF_INTERFACE_VERSION,
+		.u.set_pminfo.id	= _pr->acpi_id,
+		.u.set_pminfo.type	= XEN_PM_PX,
+	};
+	struct xen_processor_performance *dst_perf;
+	struct xen_processor_px *dst_states = NULL;
+
+	dst_perf = &op.u.set_pminfo.perf;
+
+	dst_perf->platform_limit = _pr->performance_platform_limit;
+	dst_perf->flags |= XEN_PX_PPC;
+	xen_copy_pct_data(&(_pr->performance->control_register),
+			  &dst_perf->control_register);
+	xen_copy_pct_data(&(_pr->performance->status_register),
+			  &dst_perf->status_register);
+	dst_perf->flags |= XEN_PX_PCT;
+	dst_states = xen_copy_pss_data(_pr, dst_perf);
+	if (!IS_ERR_OR_NULL(dst_states)) {
+		set_xen_guest_handle(dst_perf->states, dst_states);
+		dst_perf->flags |= XEN_PX_PSS;
+	}
+	if (!xen_copy_psd_data(_pr, dst_perf))
+		dst_perf->flags |= XEN_PX_PSD;
+
+	if (dst_perf->flags != (XEN_PX_PSD | XEN_PX_PSS | XEN_PX_PCT | XEN_PX_PPC)) {
+		pr_warn(DRV_NAME "ACPI CPU%u missing some P-state data (%x), skipping.\n",
+			_pr->acpi_id, dst_perf->flags);
+		ret = -ENODEV;
+		goto err_free;
+	}
+
+	if (!no_hypercall)
+		ret = HYPERVISOR_dom0_op(&op);
+
+	if (!ret) {
+		struct acpi_processor_performance *perf;
+		unsigned int i;
+
+		perf = _pr->performance;
+		pr_debug("ACPI CPU%u - P-states uploaded.\n", _pr->acpi_id);
+		for (i = 0; i < perf->state_count; i++) {
+			pr_debug("     %cP%d: %d MHz, %d mW, %d uS\n",
+			(i == perf->state ? '*' : ' '), i,
+			(u32) perf->states[i].core_frequency,
+			(u32) perf->states[i].power,
+			(u32) perf->states[i].transition_latency);
+		}
+	} else if (ret != -EINVAL)
+		/* EINVAL means the ACPI ID is incorrect - meaning the ACPI
+		 * table is referencing a non-existing CPU - which can happen
+		 * with broken ACPI tables. */
+		pr_warn(DRV_NAME "(_PXX): Hypervisor error (%d) for ACPI CPU%u\n",
+		       ret, _pr->acpi_id);
+err_free:
+	if (!IS_ERR_OR_NULL(dst_states))
+		kfree(dst_states);
+
+	return ret;
+}
+static int upload_pm_data(struct acpi_processor *_pr)
+{
+	int err = 0;
+
+	mutex_lock(&acpi_ids_mutex);
+	if (__test_and_set_bit(_pr->acpi_id, acpi_ids_done)) {
+		mutex_unlock(&acpi_ids_mutex);
+		return -EBUSY;
+	}
+	if (_pr->flags.power)
+		err = push_cxx_to_hypervisor(_pr);
+
+	if (_pr->performance && _pr->performance->states)
+		err |= push_pxx_to_hypervisor(_pr);
+
+	mutex_unlock(&acpi_ids_mutex);
+	return err;
+}
+static int xen_cpu_soft_notify(struct notifier_block *nfb,
+			       unsigned long action, void *hcpu)
+{
+	unsigned int cpu = (unsigned long)hcpu;
+	struct acpi_processor *_pr = per_cpu(processors, cpu);
+
+	/* The acpi_cpu_soft_notify is called before us so it populates
+	 * the structure for us. */
+	if (action == CPU_ONLINE && _pr)
+		(void)upload_pm_data(_pr);
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block xen_cpu_notifier = {
+	.notifier_call = xen_cpu_soft_notify,
+	.priority = -1, /* Be the last one */
+};
+
+/*
+ * The read_acpi_id and check_acpi_ids are there to support the Xen
+ * oddity of virtual CPUs != physical CPUs in the initial domain.
+ * The user can supply 'xen_max_vcpus=X' on the Xen hypervisor line
+ * which will band the amount of CPUs the initial domain can see.
+ * In general that is OK, except it plays havoc with any of the
+ * for_each_[present|online]_cpu macros which are banded to the virtual
+ * CPU amount.
+ */
+static acpi_status __init
+read_acpi_id(acpi_handle handle, u32 lvl, void *context, void **rv)
+{
+	u32 acpi_id;
+	acpi_status status;
+	acpi_object_type acpi_type;
+	unsigned long long tmp;
+	union acpi_object object = { 0 };
+	struct acpi_buffer buffer = { sizeof(union acpi_object), &object };
+	acpi_io_address pblk = 0;
+
+	status = acpi_get_type(handle, &acpi_type);
+	if (ACPI_FAILURE(status))
+		return AE_OK;
+
+	switch (acpi_type) {
+	case ACPI_TYPE_PROCESSOR:
+		status = acpi_evaluate_object(handle, NULL, NULL, &buffer);
+		if (ACPI_FAILURE(status))
+			return AE_OK;
+		acpi_id = object.processor.proc_id;
+		pblk = object.processor.pblk_address;
+		break;
+	case ACPI_TYPE_DEVICE:
+		status = acpi_evaluate_integer(handle, "_UID", NULL, &tmp);
+		if (ACPI_FAILURE(status))
+			return AE_OK;
+		acpi_id = tmp;
+		break;
+	default:
+		return AE_OK;
+	}
+	if (acpi_id > NR_ACPI_CPUS) {
+		WARN_ONCE(1, "There are %u ACPI CPUs, but we can only handle %d!\n",
+		     acpi_id, NR_ACPI_CPUS);
+		return AE_OK;
+	}
+	/* OK, There is a ACPI Processor object */
+	__set_bit(acpi_id, acpi_id_present);
+
+	status = acpi_evaluate_object(handle, "_CST", NULL, &buffer);
+	if (ACPI_FAILURE(status)) {
+		if (!pblk)
+			return AE_OK;
+	}
+	/* .. and it has a C-state */
+	__set_bit(acpi_id, acpi_id_cst_present);
+
+	return AE_OK;
+}
+static int __init check_acpi_ids(struct acpi_processor *pr_backup)
+{
+	if (!pr_backup)
+		return -ENODEV;
+
+	/* All online CPUs have been processed at this stage. Now verify
+	 * whether in fact "online CPUs" == physical CPUs.
+	 */
+	acpi_id_present = kcalloc(MAX_ACPI_BITS, sizeof(unsigned long), GFP_KERNEL);
+	if (!acpi_id_present)
+		return -ENOMEM;
+
+	memset(acpi_id_present, 0, MAX_ACPI_BITS * sizeof(unsigned long));
+
+	acpi_id_cst_present = kcalloc(MAX_ACPI_BITS, sizeof(unsigned long), GFP_KERNEL);
+	if (!acpi_id_cst_present) {
+		kfree(acpi_id_present);
+		return -ENOMEM;
+	}
+	memset(acpi_id_cst_present, 0, MAX_ACPI_BITS * sizeof(unsigned long));
+
+	acpi_walk_namespace(ACPI_TYPE_PROCESSOR, ACPI_ROOT_OBJECT,
+			    ACPI_UINT32_MAX,
+			    read_acpi_id, NULL, NULL, NULL);
+	acpi_get_devices("ACPI0007", read_acpi_id, NULL, NULL);
+
+	if (!bitmap_equal(acpi_id_present, acpi_ids_done, MAX_ACPI_BITS)) {
+		unsigned int i;
+
+		for_each_set_bit(i, acpi_id_present, MAX_ACPI_BITS) {
+			pr_backup->acpi_id = i;
+			/* Mask out C-states if there are no _CST */
+			pr_backup->flags.power = test_bit(i, acpi_id_cst_present);
+			(void)upload_pm_data(pr_backup);
+		}
+	}
+	kfree(acpi_id_present);
+	acpi_id_present = NULL;
+	kfree(acpi_id_cst_present);
+	acpi_id_cst_present = NULL;
+	return 0;
+}
+static int __init check_prereq(void)
+{
+	struct cpuinfo_x86 *c = &cpu_data(0);
+
+	if (!xen_initial_domain())
+		return -ENODEV;
+
+	if (!acpi_gbl_FADT.smi_command)
+		return -ENODEV;
+
+	if (c->x86_vendor == X86_VENDOR_INTEL) {
+		if (!cpu_has(c, X86_FEATURE_EST))
+			return -ENODEV;
+
+		return 0;
+	}
+	if (c->x86_vendor == X86_VENDOR_AMD) {
+		/* Copied from powernow-k8.h, can't include ../xen-acpi-processor/powernow
+		 * as we get compile warnings for the static functions.
+		 */
+#define CPUID_FREQ_VOLT_CAPABILITIES    0x80000007
+#define USE_HW_PSTATE                   0x00000080
+		u32 eax, ebx, ecx, edx;
+		cpuid(CPUID_FREQ_VOLT_CAPABILITIES, &eax, &ebx, &ecx, &edx);
+		if ((edx & USE_HW_PSTATE) != USE_HW_PSTATE)
+			return -ENODEV;
+		return 0;
+	}
+	return -ENODEV;
+}
+/* acpi_perf_data is a pointer to percpu data. */
+static struct acpi_processor_performance __percpu *acpi_perf_data;
+
+static void free_acpi_perf_data(void)
+{
+	unsigned int i;
+
+	/* Freeing a NULL pointer is OK, and alloc_percpu zeroes. */
+	for_each_possible_cpu(i)
+		free_cpumask_var(per_cpu_ptr(acpi_perf_data, i)
+				 ->shared_cpu_map);
+	free_percpu(acpi_perf_data);
+}
+
+static int __init xen_acpi_processor_init(void)
+{
+	struct acpi_processor *pr_backup = NULL;
+	unsigned int i;
+	int rc = check_prereq();
+
+	if (rc)
+		return rc;
+
+	acpi_ids_done = kcalloc(MAX_ACPI_BITS, sizeof(unsigned long), GFP_KERNEL);
+	if (!acpi_ids_done)
+		return -ENOMEM;
+
+	memset(acpi_ids_done, 0, MAX_ACPI_BITS * sizeof(unsigned long));
+
+	acpi_perf_data = alloc_percpu(struct acpi_processor_performance);
+	if (!acpi_perf_data) {
+		pr_debug(DRV_NAME "Memory allocation error for acpi_perf_data.\n");
+		kfree(acpi_ids_done);
+		return -ENOMEM;
+	}
+	for_each_possible_cpu(i) {
+		if (!zalloc_cpumask_var_node(
+			&per_cpu_ptr(acpi_perf_data, i)->shared_cpu_map,
+			GFP_KERNEL, cpu_to_node(i))) {
+			rc = -ENOMEM;
+			goto err_out;
+		}
+	}
+
+	/* Do initialization in ACPI core */
+	rc = acpi_processor_preregister_performance(acpi_perf_data);
+	if (WARN_ON(rc))
+		goto err_out;
+
+	for_each_possible_cpu(i) {
+		struct acpi_processor_performance *perf;
+
+		perf = per_cpu_ptr(acpi_perf_data, i);
+		rc = acpi_processor_register_performance(perf, i);
+		if (WARN_ON(rc))
+			goto err_out;
+	}
+	rc = acpi_processor_notify_smm(THIS_MODULE);
+	if (WARN_ON(rc))
+		goto err_unregister;
+
+	for_each_possible_cpu(i) {
+		struct acpi_processor *_pr;
+		_pr = per_cpu(processors, i /* APIC ID */);
+		if (!_pr)
+			continue;
+
+		if (!pr_backup) {
+			pr_backup = kzalloc(sizeof(struct acpi_processor), GFP_KERNEL);
+			memcpy(pr_backup, _pr, sizeof(struct acpi_processor));
+		}
+		(void)upload_pm_data(_pr);
+	}
+	rc = check_acpi_ids(pr_backup);
+	if (rc)
+		goto err_unregister;
+
+	kfree(pr_backup);
+	register_hotcpu_notifier(&xen_cpu_notifier);
+
+	return 0;
+err_unregister:
+	for_each_possible_cpu(i) {
+		struct acpi_processor_performance *perf;
+		perf = per_cpu_ptr(acpi_perf_data, i);
+		acpi_processor_unregister_performance(perf, i);
+	}
+err_out:
+	/* Freeing a NULL pointer is OK: alloc_percpu zeroes. */
+	free_acpi_perf_data();
+	kfree(acpi_ids_done);
+	return rc;
+}
+static void __exit xen_acpi_processor_exit(void)
+{
+	int i;
+
+	unregister_hotcpu_notifier(&xen_cpu_notifier);
+	kfree(acpi_ids_done);
+	for_each_possible_cpu(i) {
+		struct acpi_processor_performance *perf;
+		perf = per_cpu_ptr(acpi_perf_data, i);
+		acpi_processor_unregister_performance(perf, i);
+	}
+	free_acpi_perf_data();
+}
+
+MODULE_AUTHOR("Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>");
+MODULE_DESCRIPTION("Xen ACPI Processor P-states (and Cx) driver which uploads PM data to Xen hypervisor");
+MODULE_LICENSE("GPL");
+
+/* We want to be loaded before the CPU freq scaling drivers are loaded.
+ * They are loaded in late_initcall. */
+device_initcall(xen_acpi_processor_init);
+module_exit(xen_acpi_processor_exit);
-- 
1.7.9.48.g85da4d

Re: [PATCH] xen/acpi-processor: C and P-state driver that uploads said data to hypervisor.

[Jan Beulich]

>>> On 10.03.12 at 17:05, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
> [v7: Made it a semi-cpufreq scaling type driver suggested by Jan Beulich 

I don't see any registration or other hooking up that would match this
piece of the description.

> --- a/drivers/xen/Kconfig
> +++ b/drivers/xen/Kconfig
> @@ -178,4 +178,21 @@ config XEN_PRIVCMD
>  	depends on XEN
>  	default m
>  
> +config XEN_ACPI_PROCESSOR
> +	tristate "Xen ACPI processor"
> +	depends on XEN && X86 && ACPI_PROCESSOR
> +	default y if (X86_ACPI_CPUFREQ = y || X86_POWERNOW_K8 = y)
> +	default m if (X86_ACPI_CPUFREQ = m || X86_POWERNOW_K8 = m)

I don't think the code itself has any dependencies on this, and as long
as you interface directly with the "normal" ACPI processor driver there
also shouldn't be any implicit dependency. Is this hence may just a
leftover?

> +	help
> +          This ACPI processor uploads Power Management information 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. It also registers itslef as the SMM so that
> +          other drivers (such as ACPI cpufreq scaling driver) will not load.
> +
> +          To compile this driver as a module, choose M here: the
> +          module will be called xen_acpi_processor  If you do not know what to choose,
> +          select M here. If the CPUFREQ drivers are built in, select Y here.
> +
>  endmenu
> --- /dev/null
> +++ b/drivers/xen/xen-acpi-processor.c
> +#define NR_ACPI_CPUS	NR_CPUS
> +#define MAX_ACPI_BITS	(BITS_TO_LONGS(NR_ACPI_CPUS))

Rather than arbitrarily limiting this, you could call into Xen at startup
and scan over all CPUs' ACPI IDs to find the maximum. I think that
would even cover statically declared hotplug ones, but adding some
slack may still be necessary to cover dynamic hotplug ones (implying
that pCPU hotplug patches will make it in at some point).

>...
> +static int __init check_acpi_ids(struct acpi_processor *pr_backup)
> +{
> +	if (!pr_backup)
> +		return -ENODEV;
> +
> +	/* All online CPUs have been processed at this stage. Now verify
> +	 * whether in fact "online CPUs" == physical CPUs.
> +	 */
> +	acpi_id_present = kcalloc(MAX_ACPI_BITS, sizeof(unsigned long), GFP_KERNEL);
> +	if (!acpi_id_present)
> +		return -ENOMEM;
> +
> +	memset(acpi_id_present, 0, MAX_ACPI_BITS * sizeof(unsigned long));

kcalloc() already clears the allocated memory.

> +
> +	acpi_id_cst_present = kcalloc(MAX_ACPI_BITS, sizeof(unsigned long), GFP_KERNEL);
> +	if (!acpi_id_cst_present) {
> +		kfree(acpi_id_present);
> +		return -ENOMEM;
> +	}
> +	memset(acpi_id_cst_present, 0, MAX_ACPI_BITS * sizeof(unsigned long));

Same here.

> +
> +	acpi_walk_namespace(ACPI_TYPE_PROCESSOR, ACPI_ROOT_OBJECT,
> +			    ACPI_UINT32_MAX,
> +			    read_acpi_id, NULL, NULL, NULL);
> +	acpi_get_devices("ACPI0007", read_acpi_id, NULL, NULL);
> +
> +	if (!bitmap_equal(acpi_id_present, acpi_ids_done, MAX_ACPI_BITS)) {
> +		unsigned int i;
> +
> +		for_each_set_bit(i, acpi_id_present, MAX_ACPI_BITS) {
> +			pr_backup->acpi_id = i;
> +			/* Mask out C-states if there are no _CST */
> +			pr_backup->flags.power = test_bit(i, acpi_id_cst_present);
> +			(void)upload_pm_data(pr_backup);
> +		}
> +	}
> +	kfree(acpi_id_present);
> +	acpi_id_present = NULL;
> +	kfree(acpi_id_cst_present);
> +	acpi_id_cst_present = NULL;
> +	return 0;
> +}
> +static int __init check_prereq(void)
> +{
> +	struct cpuinfo_x86 *c = &cpu_data(0);
> +
> +	if (!xen_initial_domain())
> +		return -ENODEV;
> +
> +	if (!acpi_gbl_FADT.smi_command)
> +		return -ENODEV;
> +
> +	if (c->x86_vendor == X86_VENDOR_INTEL) {
> +		if (!cpu_has(c, X86_FEATURE_EST))
> +			return -ENODEV;
> +
> +		return 0;
> +	}
> +	if (c->x86_vendor == X86_VENDOR_AMD) {
> +		/* Copied from powernow-k8.h, can't include ../xen-acpi-processor/powernow
> +		 * as we get compile warnings for the static functions.
> +		 */

If drivers/cpufreq/powernow-k[78].h are supposed to be useful at all
(which they currently aren't as both get included in just a single place),
those static function declarations should be removed from the latter.

> +#define CPUID_FREQ_VOLT_CAPABILITIES    0x80000007
> +#define USE_HW_PSTATE                   0x00000080
> +		u32 eax, ebx, ecx, edx;
> +		cpuid(CPUID_FREQ_VOLT_CAPABILITIES, &eax, &ebx, &ecx, &edx);
> +		if ((edx & USE_HW_PSTATE) != USE_HW_PSTATE)
> +			return -ENODEV;
> +		return 0;
> +	}
> +	return -ENODEV;
> +}
>...
> +static int __init xen_acpi_processor_init(void)
> +{
> +	struct acpi_processor *pr_backup = NULL;
> +	unsigned int i;
> +	int rc = check_prereq();
> +
> +	if (rc)
> +		return rc;
> +
> +	acpi_ids_done = kcalloc(MAX_ACPI_BITS, sizeof(unsigned long), GFP_KERNEL);
> +	if (!acpi_ids_done)
> +		return -ENOMEM;
> +
> +	memset(acpi_ids_done, 0, MAX_ACPI_BITS * sizeof(unsigned long));

Pointless kcalloc() + memset() again.

> +
> +	acpi_perf_data = alloc_percpu(struct acpi_processor_performance);
> +	if (!acpi_perf_data) {
> +		pr_debug(DRV_NAME "Memory allocation error for acpi_perf_data.\n");
> +		kfree(acpi_ids_done);
> +		return -ENOMEM;
> +	}
> +	for_each_possible_cpu(i) {
> +		if (!zalloc_cpumask_var_node(
> +			&per_cpu_ptr(acpi_perf_data, i)->shared_cpu_map,
> +			GFP_KERNEL, cpu_to_node(i))) {
> +			rc = -ENOMEM;
> +			goto err_out;
> +		}
> +	}
> +
> +	/* Do initialization in ACPI core */
> +	rc = acpi_processor_preregister_performance(acpi_perf_data);
> +	if (WARN_ON(rc))
> +		goto err_out;
> +
> +	for_each_possible_cpu(i) {
> +		struct acpi_processor_performance *perf;
> +
> +		perf = per_cpu_ptr(acpi_perf_data, i);
> +		rc = acpi_processor_register_performance(perf, i);
> +		if (WARN_ON(rc))
> +			goto err_out;
> +	}
> +	rc = acpi_processor_notify_smm(THIS_MODULE);
> +	if (WARN_ON(rc))
> +		goto err_unregister;
> +
> +	for_each_possible_cpu(i) {
> +		struct acpi_processor *_pr;
> +		_pr = per_cpu(processors, i /* APIC ID */);
> +		if (!_pr)
> +			continue;
> +
> +		if (!pr_backup) {
> +			pr_backup = kzalloc(sizeof(struct acpi_processor), GFP_KERNEL);
> +			memcpy(pr_backup, _pr, sizeof(struct acpi_processor));
> +		}
> +		(void)upload_pm_data(_pr);
> +	}
> +	rc = check_acpi_ids(pr_backup);
> +	if (rc)
> +		goto err_unregister;
> +
> +	kfree(pr_backup);
> +	register_hotcpu_notifier(&xen_cpu_notifier);

This is pointless - you'd get notified of vCPU-s arrival/departure only.
Without pCPU hotplug code in place, there's just nothing you can (and
need to) do here.

> +
> +	return 0;
> +err_unregister:
> +	for_each_possible_cpu(i) {
> +		struct acpi_processor_performance *perf;
> +		perf = per_cpu_ptr(acpi_perf_data, i);
> +		acpi_processor_unregister_performance(perf, i);
> +	}
> +err_out:
> +	/* Freeing a NULL pointer is OK: alloc_percpu zeroes. */
> +	free_acpi_perf_data();
> +	kfree(acpi_ids_done);
> +	return rc;
> +}

Overall this version looks a lot better to me than the previous ones.

Jan

Re: [PATCH] xen/acpi-processor: C and P-state driver that uploads said data to hypervisor.

[Konrad Rzeszutek Wilk]

On Mon, Mar 12, 2012 at 11:01:49AM +0000, Jan Beulich wrote:
> >>> On 10.03.12 at 17:05, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
> > [v7: Made it a semi-cpufreq scaling type driver suggested by Jan Beulich 
> 
> I don't see any registration or other hooking up that would match this
> piece of the description.

Oh, a little stale. Should have said : "suggestion by Jan Beulich made me
think about not depending on CPU freq scaling drivers at all."
> 
> > --- a/drivers/xen/Kconfig
> > +++ b/drivers/xen/Kconfig
> > @@ -178,4 +178,21 @@ config XEN_PRIVCMD
> >  	depends on XEN
> >  	default m
> >  
> > +config XEN_ACPI_PROCESSOR
> > +	tristate "Xen ACPI processor"
> > +	depends on XEN && X86 && ACPI_PROCESSOR
> > +	default y if (X86_ACPI_CPUFREQ = y || X86_POWERNOW_K8 = y)
> > +	default m if (X86_ACPI_CPUFREQ = m || X86_POWERNOW_K8 = m)
> 
> I don't think the code itself has any dependencies on this, and as long
> as you interface directly with the "normal" ACPI processor driver there
> also shouldn't be any implicit dependency. Is this hence may just a
> leftover?

It is needed so that the CPU freq scaling drivers don't get called
before this driver is loaded.

Being that if the kernel was compiled with powernow-k8/acpi-cpufreq as
built in, we really want to be loaded before them - to thwart them
from loading. The one way this is done is by calling
 acpi_processor_notify_smm

which on subsequent calls will return -EBUSY for the cpufreq scaling drivers
and inhibit them from being called. Naturally it does not fix the case
if the drivers are built as modules - then the change to load xen-acpi-processor
should be done in the same init script as the one loading powernow-k8/acpi-cpufreq.,

It would be much easier if there was a cpuidle_disable variant in the cpufreq
scaling department - let me see if Dave Jones is open for this, as I can't
see an easy /clean way to make the cpuid checks in the acpi-cpufreq and powernow-k8
changed so that said drivers won't load.


> 
> > +	help
> > +          This ACPI processor uploads Power Management information 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. It also registers itslef as the SMM so that
> > +          other drivers (such as ACPI cpufreq scaling driver) will not load.
> > +
> > +          To compile this driver as a module, choose M here: the
> > +          module will be called xen_acpi_processor  If you do not know what to choose,
> > +          select M here. If the CPUFREQ drivers are built in, select Y here.
> > +
> >  endmenu
> > --- /dev/null
> > +++ b/drivers/xen/xen-acpi-processor.c
> > +#define NR_ACPI_CPUS	NR_CPUS
> > +#define MAX_ACPI_BITS	(BITS_TO_LONGS(NR_ACPI_CPUS))
> 
> Rather than arbitrarily limiting this, you could call into Xen at startup
> and scan over all CPUs' ACPI IDs to find the maximum. I think that
> would even cover statically declared hotplug ones, but adding some
> slack may still be necessary to cover dynamic hotplug ones (implying
> that pCPU hotplug patches will make it in at some point).

This would be the XENPF_get_cpuinfo call right? I do plan on looking at the
pCPU hotplug, but I think I need some fancy hardware to test it correctly?

> 
> >...
> > +static int __init check_acpi_ids(struct acpi_processor *pr_backup)
> > +{
> > +	if (!pr_backup)
> > +		return -ENODEV;
> > +
> > +	/* All online CPUs have been processed at this stage. Now verify
> > +	 * whether in fact "online CPUs" == physical CPUs.
> > +	 */
> > +	acpi_id_present = kcalloc(MAX_ACPI_BITS, sizeof(unsigned long), GFP_KERNEL);
> > +	if (!acpi_id_present)
> > +		return -ENOMEM;
> > +
> > +	memset(acpi_id_present, 0, MAX_ACPI_BITS * sizeof(unsigned long));
> 
> kcalloc() already clears the allocated memory.
> 
> > +
> > +	acpi_id_cst_present = kcalloc(MAX_ACPI_BITS, sizeof(unsigned long), GFP_KERNEL);
> > +	if (!acpi_id_cst_present) {
> > +		kfree(acpi_id_present);
> > +		return -ENOMEM;
> > +	}
> > +	memset(acpi_id_cst_present, 0, MAX_ACPI_BITS * sizeof(unsigned long));
> 
> Same here.

Ah yes, one of those paste-n-copy errors.
> 
> > +
> > +	acpi_walk_namespace(ACPI_TYPE_PROCESSOR, ACPI_ROOT_OBJECT,
> > +			    ACPI_UINT32_MAX,
> > +			    read_acpi_id, NULL, NULL, NULL);
> > +	acpi_get_devices("ACPI0007", read_acpi_id, NULL, NULL);
> > +
> > +	if (!bitmap_equal(acpi_id_present, acpi_ids_done, MAX_ACPI_BITS)) {
> > +		unsigned int i;
> > +
> > +		for_each_set_bit(i, acpi_id_present, MAX_ACPI_BITS) {
> > +			pr_backup->acpi_id = i;
> > +			/* Mask out C-states if there are no _CST */
> > +			pr_backup->flags.power = test_bit(i, acpi_id_cst_present);
> > +			(void)upload_pm_data(pr_backup);
> > +		}
> > +	}
> > +	kfree(acpi_id_present);
> > +	acpi_id_present = NULL;
> > +	kfree(acpi_id_cst_present);
> > +	acpi_id_cst_present = NULL;
> > +	return 0;
> > +}
> > +static int __init check_prereq(void)
> > +{
> > +	struct cpuinfo_x86 *c = &cpu_data(0);
> > +
> > +	if (!xen_initial_domain())
> > +		return -ENODEV;
> > +
> > +	if (!acpi_gbl_FADT.smi_command)
> > +		return -ENODEV;
> > +
> > +	if (c->x86_vendor == X86_VENDOR_INTEL) {
> > +		if (!cpu_has(c, X86_FEATURE_EST))
> > +			return -ENODEV;
> > +
> > +		return 0;
> > +	}
> > +	if (c->x86_vendor == X86_VENDOR_AMD) {
> > +		/* Copied from powernow-k8.h, can't include ../xen-acpi-processor/powernow
> > +		 * as we get compile warnings for the static functions.
> > +		 */
> 
> If drivers/cpufreq/powernow-k[78].h are supposed to be useful at all
> (which they currently aren't as both get included in just a single place),
> those static function declarations should be removed from the latter.

<nods> Will post a seperate patch for that.
> 
> > +#define CPUID_FREQ_VOLT_CAPABILITIES    0x80000007
> > +#define USE_HW_PSTATE                   0x00000080
> > +		u32 eax, ebx, ecx, edx;
> > +		cpuid(CPUID_FREQ_VOLT_CAPABILITIES, &eax, &ebx, &ecx, &edx);
> > +		if ((edx & USE_HW_PSTATE) != USE_HW_PSTATE)
> > +			return -ENODEV;
> > +		return 0;
> > +	}
> > +	return -ENODEV;
> > +}
> >...
> > +static int __init xen_acpi_processor_init(void)
> > +{
> > +	struct acpi_processor *pr_backup = NULL;
> > +	unsigned int i;
> > +	int rc = check_prereq();
> > +
> > +	if (rc)
> > +		return rc;
> > +
> > +	acpi_ids_done = kcalloc(MAX_ACPI_BITS, sizeof(unsigned long), GFP_KERNEL);
> > +	if (!acpi_ids_done)
> > +		return -ENOMEM;
> > +
> > +	memset(acpi_ids_done, 0, MAX_ACPI_BITS * sizeof(unsigned long));
> 
> Pointless kcalloc() + memset() again.
> 
> > +
> > +	acpi_perf_data = alloc_percpu(struct acpi_processor_performance);
> > +	if (!acpi_perf_data) {
> > +		pr_debug(DRV_NAME "Memory allocation error for acpi_perf_data.\n");
> > +		kfree(acpi_ids_done);
> > +		return -ENOMEM;
> > +	}
> > +	for_each_possible_cpu(i) {
> > +		if (!zalloc_cpumask_var_node(
> > +			&per_cpu_ptr(acpi_perf_data, i)->shared_cpu_map,
> > +			GFP_KERNEL, cpu_to_node(i))) {
> > +			rc = -ENOMEM;
> > +			goto err_out;
> > +		}
> > +	}
> > +
> > +	/* Do initialization in ACPI core */
> > +	rc = acpi_processor_preregister_performance(acpi_perf_data);
> > +	if (WARN_ON(rc))
> > +		goto err_out;
> > +
> > +	for_each_possible_cpu(i) {
> > +		struct acpi_processor_performance *perf;
> > +
> > +		perf = per_cpu_ptr(acpi_perf_data, i);
> > +		rc = acpi_processor_register_performance(perf, i);
> > +		if (WARN_ON(rc))
> > +			goto err_out;
> > +	}
> > +	rc = acpi_processor_notify_smm(THIS_MODULE);
> > +	if (WARN_ON(rc))
> > +		goto err_unregister;
> > +
> > +	for_each_possible_cpu(i) {
> > +		struct acpi_processor *_pr;
> > +		_pr = per_cpu(processors, i /* APIC ID */);
> > +		if (!_pr)
> > +			continue;
> > +
> > +		if (!pr_backup) {
> > +			pr_backup = kzalloc(sizeof(struct acpi_processor), GFP_KERNEL);
> > +			memcpy(pr_backup, _pr, sizeof(struct acpi_processor));
> > +		}
> > +		(void)upload_pm_data(_pr);
> > +	}
> > +	rc = check_acpi_ids(pr_backup);
> > +	if (rc)
> > +		goto err_unregister;
> > +
> > +	kfree(pr_backup);
> > +	register_hotcpu_notifier(&xen_cpu_notifier);
> 
> This is pointless - you'd get notified of vCPU-s arrival/departure only.
> Without pCPU hotplug code in place, there's just nothing you can (and
> need to) do here.

I hadn't actually looked in details on the pCPU hotplug code to see how it works.

I presume I need special hardware for this to work as well as the ACPI
is involved in triggering the hotplug CPU up calls?

> 
> > +
> > +	return 0;
> > +err_unregister:
> > +	for_each_possible_cpu(i) {
> > +		struct acpi_processor_performance *perf;
> > +		perf = per_cpu_ptr(acpi_perf_data, i);
> > +		acpi_processor_unregister_performance(perf, i);
> > +	}
> > +err_out:
> > +	/* Freeing a NULL pointer is OK: alloc_percpu zeroes. */
> > +	free_acpi_perf_data();
> > +	kfree(acpi_ids_done);
> > +	return rc;
> > +}
> 
> Overall this version looks a lot better to me than the previous ones.

Yeah, and it drops the dependency on those Xen patches I had sent earlier.
Thanks for taking a look!

> 
> Jan

Re: [PATCH] xen/acpi-processor: C and P-state driver that uploads said data to hypervisor.

[Jan Beulich]

>>> On 12.03.12 at 16:07, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
> On Mon, Mar 12, 2012 at 11:01:49AM +0000, Jan Beulich wrote:
>> >>> On 10.03.12 at 17:05, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
>> Rather than arbitrarily limiting this, you could call into Xen at startup
>> and scan over all CPUs' ACPI IDs to find the maximum. I think that
>> would even cover statically declared hotplug ones, but adding some
>> slack may still be necessary to cover dynamic hotplug ones (implying
>> that pCPU hotplug patches will make it in at some point).
> 
> This would be the XENPF_get_cpuinfo call right? I do plan on looking at the

Yes.

> pCPU hotplug, but I think I need some fancy hardware to test it correctly?

No, you don't really - for testing these code paths you can soft offline
and then online back CPUs in Xen.

>> > +	register_hotcpu_notifier(&xen_cpu_notifier);
>> 
>> This is pointless - you'd get notified of vCPU-s arrival/departure only.
>> Without pCPU hotplug code in place, there's just nothing you can (and
>> need to) do here.
> 
> I hadn't actually looked in details on the pCPU hotplug code to see how it 
> works.
> 
> I presume I need special hardware for this to work as well as the ACPI
> is involved in triggering the hotplug CPU up calls?

For exercising the ACPI code paths - yes, you'll need suitable
hardware. But for just exercising the notifier logic, as said above,
you don't.

Jan

Re: [PATCH] xen/acpi-processor: C and P-state driver that uploads said data to hypervisor.

[Konrad Rzeszutek Wilk]

> >> > +	register_hotcpu_notifier(&xen_cpu_notifier);
> >> 
> >> This is pointless - you'd get notified of vCPU-s arrival/departure only.
> >> Without pCPU hotplug code in place, there's just nothing you can (and
> >> need to) do here.
> > 
> > I hadn't actually looked in details on the pCPU hotplug code to see how it 
> > works.
> > 
> > I presume I need special hardware for this to work as well as the ACPI
> > is involved in triggering the hotplug CPU up calls?
> 
> For exercising the ACPI code paths - yes, you'll need suitable
> hardware. But for just exercising the notifier logic, as said above,
> you don't.

I think I can do the pCPU hotplug code later - as the driver to do the hypercall
is not yet in upstream. The TODO after this is to rework the powernow-k8.h to
not cause compile warnings.

So, per your suggestions I've reworked the driver some more. Please
see attached.

>From ac14b2f77270f3d2c771de71f4a34d06064b1375 Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Fri, 3 Feb 2012 16:03:20 -0500
Subject: [PATCH] xen/acpi-processor: C and P-state driver that uploads said
 data to hypervisor.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This driver solves three problems:
 1). Parse and upload ACPI0007 (or PROCESSOR_TYPE) information to the
     hypervisor - aka P-states (cpufreq data).
 2). Upload the the Cx state information (cpuidle data).
 3). Inhibit CPU frequency scaling drivers from loading.

The reason for wanting to solve 1) and 2) is such that the Xen hypervisor
is the only one that knows the CPU usage of different guests and can
make the proper decision of when to put CPUs and packages in proper states.
Unfortunately the hypervisor has no support to parse ACPI DSDT tables, hence it
needs help from the initial domain to provide this information. The reason
for 3) is that we do not want the initial domain to change P-states while the
hypervisor is doing it as well - it causes rather some funny cases of P-states
transitions.

For this to work, the driver parses the Power Management data and uploads said
information to the Xen hypervisor. It also calls acpi_processor_notify_smm()
to inhibit the other CPU frequency scaling drivers from being loaded.

Everything revolves around the 'struct acpi_processor' structure which
gets updated during the bootup cycle in different stages. At the startup, when
the ACPI parser starts, the C-state information is processed (processor_idle)
and saved in said structure as 'power' element. Later on, the CPU frequency
scaling driver (powernow-k8 or acpi_cpufreq), would call the the
acpi_processor_* (processor_perflib functions) to parse P-states information
and populate in the said structure the 'performance' element.

Since we do not want the CPU frequency scaling drivers from loading
we have to call the acpi_processor_* functions to parse the P-states and
call "acpi_processor_notify_smm" to stop them from loading.

There is also one oddity in this driver which is that under Xen, the
physical online CPU count can be different from the virtual online CPU count.
Meaning that the macros 'for_[online|possible]_cpu' would process only
up to virtual online CPU count. We on the other hand want to process
the full amount of physical CPUs. For that, the driver checks 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 <pasik@iki.fi>]
[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]
[v7: Jan Beulich <jbeulich@suse.com> suggestion to make it a cpufreq scaling driver
     made me rework it as driver that inhibits cpufreq scaling driver]
[v8: Per Jan's review comments, fixed up the driver]
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 drivers/xen/Kconfig              |   17 ++
 drivers/xen/Makefile             |    2 +-
 drivers/xen/xen-acpi-processor.c |  565 ++++++++++++++++++++++++++++++++++++++
 include/xen/interface/platform.h |   17 ++
 4 files changed, 600 insertions(+), 1 deletions(-)
 create mode 100644 drivers/xen/xen-acpi-processor.c

diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
index a1ced52..648bcd4 100644
--- a/drivers/xen/Kconfig
+++ b/drivers/xen/Kconfig
@@ -178,4 +178,21 @@ config XEN_PRIVCMD
 	depends on XEN
 	default m
 
+config XEN_ACPI_PROCESSOR
+	tristate "Xen ACPI processor"
+	depends on XEN && X86 && ACPI_PROCESSOR
+	default y if (X86_ACPI_CPUFREQ = y || X86_POWERNOW_K8 = y)
+	default m if (X86_ACPI_CPUFREQ = m || X86_POWERNOW_K8 = m)
+	help
+          This ACPI processor uploads Power Management information 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. It also registers itslef as the SMM so that
+          other drivers (such as ACPI cpufreq scaling driver) will not load.
+
+          To compile this driver as a module, choose M here: the
+          module will be called xen_acpi_processor  If you do not know what to choose,
+          select M here. If the CPUFREQ drivers are built in, select Y here.
+
 endmenu
diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
index aa31337..9adc5be 100644
--- a/drivers/xen/Makefile
+++ b/drivers/xen/Makefile
@@ -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_XEN_ACPI_PROCESSOR)	+= xen-acpi-processor.o
 xen-evtchn-y				:= evtchn.o
 xen-gntdev-y				:= gntdev.o
 xen-gntalloc-y				:= gntalloc.o
diff --git a/drivers/xen/xen-acpi-processor.c b/drivers/xen/xen-acpi-processor.c
new file mode 100644
index 0000000..fd49aab
--- /dev/null
+++ b/drivers/xen/xen-acpi-processor.c
@@ -0,0 +1,565 @@
+/*
+ * Copyright 2012 by Oracle Inc
+ * Author: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
+ *
+ * This code borrows ideas from https://lkml.org/lkml/2011/11/30/249
+ * so many thanks go to Kevin Tian <kevin.tian@intel.com>
+ * and Yu Ke <ke.yu@intel.com>.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ */
+
+#include <linux/cpumask.h>
+#include <linux/cpufreq.h>
+#include <linux/freezer.h>
+#include <linux/kernel.h>
+#include <linux/kthread.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/types.h>
+#include <acpi/acpi_bus.h>
+#include <acpi/acpi_drivers.h>
+#include <acpi/processor.h>
+
+#include <xen/interface/platform.h>
+#include <asm/xen/hypercall.h>
+
+#define DRV_NAME "xen-acpi-processor: "
+
+static int no_hypercall;
+MODULE_PARM_DESC(off, "Inhibit the hypercall.");
+module_param_named(off, no_hypercall, int, 0400);
+
+/*
+ * Mutex to protect the acpi_ids_done.
+ */
+static DEFINE_MUTEX(acpi_ids_mutex);
+/*
+ * Note: Do not convert the acpi_id* below to cpumask_var_t or use cpumask_bit
+ * - as those shrink to nr_cpu_bits (which is dependent on possible_cpu), which
+ * can be less than what we want to put in. Instead use the 'nr_acpi_bits'
+ * which is dynamically computed based on the MADT or x2APIC table.
+ */
+static unsigned int nr_acpi_bits;
+/* Which ACPI ID we have processed from 'struct acpi_processor'. */
+static unsigned long *acpi_ids_done;
+/* Which ACPI ID exist in the SSDT/DSDT processor definitions. */
+static unsigned long __initdata *acpi_id_present;
+/* And if there is an _CST definition (or a PBLK) for the ACPI IDs */
+static unsigned long __initdata *acpi_id_cst_present;
+
+static int push_cxx_to_hypervisor(struct acpi_processor *_pr)
+{
+	struct xen_platform_op op = {
+		.cmd			= XENPF_set_processor_pminfo,
+		.interface_version	= XENPF_INTERFACE_VERSION,
+		.u.set_pminfo.id	= _pr->acpi_id,
+		.u.set_pminfo.type	= XEN_PM_CX,
+	};
+	struct xen_processor_cx *dst_cx, *dst_cx_states = NULL;
+	struct acpi_processor_cx *cx;
+	unsigned int i, ok;
+	int ret = 0;
+
+	dst_cx_states = kcalloc(_pr->power.count,
+				sizeof(struct xen_processor_cx), GFP_KERNEL);
+	if (!dst_cx_states)
+		return -ENOMEM;
+
+	for (ok = 0, i = 1; i <= _pr->power.count; i++) {
+		cx = &_pr->power.states[i];
+		if (!cx->valid)
+			continue;
+
+		dst_cx = &(dst_cx_states[ok++]);
+
+		dst_cx->reg.space_id = ACPI_ADR_SPACE_SYSTEM_IO;
+		if (cx->entry_method == ACPI_CSTATE_SYSTEMIO) {
+			dst_cx->reg.bit_width = 8;
+			dst_cx->reg.bit_offset = 0;
+			dst_cx->reg.access_size = 1;
+		} else {
+			dst_cx->reg.space_id = ACPI_ADR_SPACE_FIXED_HARDWARE;
+			if (cx->entry_method == ACPI_CSTATE_FFH) {
+				/* NATIVE_CSTATE_BEYOND_HALT */
+				dst_cx->reg.bit_offset = 2;
+				dst_cx->reg.bit_width = 1; /* VENDOR_INTEL */
+			}
+			dst_cx->reg.access_size = 0;
+		}
+		dst_cx->reg.address = cx->address;
+
+		dst_cx->type = cx->type;
+		dst_cx->latency = cx->latency;
+		dst_cx->power = cx->power;
+
+		dst_cx->dpcnt = 0;
+		set_xen_guest_handle(dst_cx->dp, NULL);
+	}
+	if (!ok) {
+		pr_err(DRV_NAME "No _Cx for ACPI CPU %u\n", _pr->acpi_id);
+		kfree(dst_cx_states);
+		return -EINVAL;
+	}
+	op.u.set_pminfo.power.count = ok;
+	op.u.set_pminfo.power.flags.bm_control = _pr->flags.bm_control;
+	op.u.set_pminfo.power.flags.bm_check = _pr->flags.bm_check;
+	op.u.set_pminfo.power.flags.has_cst = _pr->flags.has_cst;
+	op.u.set_pminfo.power.flags.power_setup_done =
+		_pr->flags.power_setup_done;
+
+	set_xen_guest_handle(op.u.set_pminfo.power.states, dst_cx_states);
+
+	if (!no_hypercall)
+		ret = HYPERVISOR_dom0_op(&op);
+
+	if (!ret) {
+		pr_debug("ACPI CPU%u - C-states uploaded.\n", _pr->acpi_id);
+		for (i = 1; i <= _pr->power.count; i++) {
+			cx = &_pr->power.states[i];
+			if (!cx->valid)
+				continue;
+			pr_debug("     C%d: %s %d uS\n",
+				 cx->type, cx->desc, (u32)cx->latency);
+		}
+	} else
+		pr_err(DRV_NAME "(CX): Hypervisor error (%d) for ACPI CPU%u\n",
+		       ret, _pr->acpi_id);
+
+	kfree(dst_cx_states);
+
+	return ret;
+}
+static struct xen_processor_px *
+xen_copy_pss_data(struct acpi_processor *_pr,
+		  struct xen_processor_performance *dst_perf)
+{
+	struct xen_processor_px *dst_states = NULL;
+	unsigned int i;
+
+	BUILD_BUG_ON(sizeof(struct xen_processor_px) !=
+		     sizeof(struct acpi_processor_px));
+
+	dst_states = kcalloc(_pr->performance->state_count,
+			     sizeof(struct xen_processor_px), GFP_KERNEL);
+	if (!dst_states)
+		return ERR_PTR(-ENOMEM);
+
+	dst_perf->state_count = _pr->performance->state_count;
+	for (i = 0; i < _pr->performance->state_count; i++) {
+		/* Fortunatly for us, they are both the same size */
+		memcpy(&(dst_states[i]), &(_pr->performance->states[i]),
+		       sizeof(struct acpi_processor_px));
+	}
+	return dst_states;
+}
+static int xen_copy_psd_data(struct acpi_processor *_pr,
+			     struct xen_processor_performance *dst)
+{
+	struct acpi_psd_package *pdomain;
+
+	BUILD_BUG_ON(sizeof(struct xen_psd_package) !=
+		     sizeof(struct acpi_psd_package));
+
+	/* This information is enumerated only if acpi_processor_preregister_performance
+	 * has been called.
+	 */
+	dst->shared_type = _pr->performance->shared_type;
+
+	pdomain = &(_pr->performance->domain_info);
+
+	/* 'acpi_processor_preregister_performance' does not parse if the
+	 * num_processors <= 1, but Xen still requires it. Do it manually here.
+	 */
+	if (pdomain->num_processors <= 1) {
+		if (pdomain->coord_type == DOMAIN_COORD_TYPE_SW_ALL)
+			dst->shared_type = CPUFREQ_SHARED_TYPE_ALL;
+		else if (pdomain->coord_type == DOMAIN_COORD_TYPE_HW_ALL)
+			dst->shared_type = CPUFREQ_SHARED_TYPE_HW;
+		else if (pdomain->coord_type == DOMAIN_COORD_TYPE_SW_ANY)
+			dst->shared_type = CPUFREQ_SHARED_TYPE_ANY;
+
+	}
+	memcpy(&(dst->domain_info), pdomain, sizeof(struct acpi_psd_package));
+	return 0;
+}
+static int xen_copy_pct_data(struct acpi_pct_register *pct,
+			     struct xen_pct_register *dst_pct)
+{
+	/* It would be nice if you could just do 'memcpy(pct, dst_pct') but
+	 * sadly the Xen structure did not have the proper padding so the
+	 * descriptor field takes two (dst_pct) bytes instead of one (pct).
+	 */
+	dst_pct->descriptor = pct->descriptor;
+	dst_pct->length = pct->length;
+	dst_pct->space_id = pct->space_id;
+	dst_pct->bit_width = pct->bit_width;
+	dst_pct->bit_offset = pct->bit_offset;
+	dst_pct->reserved = pct->reserved;
+	dst_pct->address = pct->address;
+	return 0;
+}
+static int push_pxx_to_hypervisor(struct acpi_processor *_pr)
+{
+	int ret = 0;
+	struct xen_platform_op op = {
+		.cmd			= XENPF_set_processor_pminfo,
+		.interface_version	= XENPF_INTERFACE_VERSION,
+		.u.set_pminfo.id	= _pr->acpi_id,
+		.u.set_pminfo.type	= XEN_PM_PX,
+	};
+	struct xen_processor_performance *dst_perf;
+	struct xen_processor_px *dst_states = NULL;
+
+	dst_perf = &op.u.set_pminfo.perf;
+
+	dst_perf->platform_limit = _pr->performance_platform_limit;
+	dst_perf->flags |= XEN_PX_PPC;
+	xen_copy_pct_data(&(_pr->performance->control_register),
+			  &dst_perf->control_register);
+	xen_copy_pct_data(&(_pr->performance->status_register),
+			  &dst_perf->status_register);
+	dst_perf->flags |= XEN_PX_PCT;
+	dst_states = xen_copy_pss_data(_pr, dst_perf);
+	if (!IS_ERR_OR_NULL(dst_states)) {
+		set_xen_guest_handle(dst_perf->states, dst_states);
+		dst_perf->flags |= XEN_PX_PSS;
+	}
+	if (!xen_copy_psd_data(_pr, dst_perf))
+		dst_perf->flags |= XEN_PX_PSD;
+
+	if (dst_perf->flags != (XEN_PX_PSD | XEN_PX_PSS | XEN_PX_PCT | XEN_PX_PPC)) {
+		pr_warn(DRV_NAME "ACPI CPU%u missing some P-state data (%x), skipping.\n",
+			_pr->acpi_id, dst_perf->flags);
+		ret = -ENODEV;
+		goto err_free;
+	}
+
+	if (!no_hypercall)
+		ret = HYPERVISOR_dom0_op(&op);
+
+	if (!ret) {
+		struct acpi_processor_performance *perf;
+		unsigned int i;
+
+		perf = _pr->performance;
+		pr_debug("ACPI CPU%u - P-states uploaded.\n", _pr->acpi_id);
+		for (i = 0; i < perf->state_count; i++) {
+			pr_debug("     %cP%d: %d MHz, %d mW, %d uS\n",
+			(i == perf->state ? '*' : ' '), i,
+			(u32) perf->states[i].core_frequency,
+			(u32) perf->states[i].power,
+			(u32) perf->states[i].transition_latency);
+		}
+	} else if (ret != -EINVAL)
+		/* EINVAL means the ACPI ID is incorrect - meaning the ACPI
+		 * table is referencing a non-existing CPU - which can happen
+		 * with broken ACPI tables. */
+		pr_warn(DRV_NAME "(_PXX): Hypervisor error (%d) for ACPI CPU%u\n",
+		       ret, _pr->acpi_id);
+err_free:
+	if (!IS_ERR_OR_NULL(dst_states))
+		kfree(dst_states);
+
+	return ret;
+}
+static int upload_pm_data(struct acpi_processor *_pr)
+{
+	int err = 0;
+
+	mutex_lock(&acpi_ids_mutex);
+	if (__test_and_set_bit(_pr->acpi_id, acpi_ids_done)) {
+		mutex_unlock(&acpi_ids_mutex);
+		return -EBUSY;
+	}
+	if (_pr->flags.power)
+		err = push_cxx_to_hypervisor(_pr);
+
+	if (_pr->performance && _pr->performance->states)
+		err |= push_pxx_to_hypervisor(_pr);
+
+	mutex_unlock(&acpi_ids_mutex);
+	return err;
+}
+static unsigned int __init get_max_acpi_id(void)
+{
+	struct xenpf_pcpuinfo *info;
+	struct xen_platform_op op = {
+		.cmd = XENPF_get_cpuinfo,
+		.interface_version = XENPF_INTERFACE_VERSION,
+	};
+	int ret = 0;
+	unsigned int i, max_apic_id, max_acpi_id = 0;
+
+	info = &op.u.pcpu_info;
+	info->xen_cpuid = 0;
+
+	ret = HYPERVISOR_dom0_op(&op);
+	if (ret)
+		return max_acpi_id;
+
+	/* The max_present is the same irregardless of the xen_cpuid */
+	max_apic_id = op.u.pcpu_info.max_present;
+	for (i = 0; i <= max_apic_id; i++) {
+		info->xen_cpuid = i;
+		ret = HYPERVISOR_dom0_op(&op);
+		if (ret)
+			continue;
+		max_acpi_id = max(info->acpi_id, max_acpi_id);
+	}
+	pr_debug(DRV_NAME "Max ACPI ID: %u\n", max_acpi_id);
+	return max_acpi_id;
+}
+/*
+ * The read_acpi_id and check_acpi_ids are there to support the Xen
+ * oddity of virtual CPUs != physical CPUs in the initial domain.
+ * The user can supply 'xen_max_vcpus=X' on the Xen hypervisor line
+ * which will band the amount of CPUs the initial domain can see.
+ * In general that is OK, except it plays havoc with any of the
+ * for_each_[present|online]_cpu macros which are banded to the virtual
+ * CPU amount.
+ */
+static acpi_status __init
+read_acpi_id(acpi_handle handle, u32 lvl, void *context, void **rv)
+{
+	u32 acpi_id;
+	acpi_status status;
+	acpi_object_type acpi_type;
+	unsigned long long tmp;
+	union acpi_object object = { 0 };
+	struct acpi_buffer buffer = { sizeof(union acpi_object), &object };
+	acpi_io_address pblk = 0;
+
+	status = acpi_get_type(handle, &acpi_type);
+	if (ACPI_FAILURE(status))
+		return AE_OK;
+
+	switch (acpi_type) {
+	case ACPI_TYPE_PROCESSOR:
+		status = acpi_evaluate_object(handle, NULL, NULL, &buffer);
+		if (ACPI_FAILURE(status))
+			return AE_OK;
+		acpi_id = object.processor.proc_id;
+		pblk = object.processor.pblk_address;
+		break;
+	case ACPI_TYPE_DEVICE:
+		status = acpi_evaluate_integer(handle, "_UID", NULL, &tmp);
+		if (ACPI_FAILURE(status))
+			return AE_OK;
+		acpi_id = tmp;
+		break;
+	default:
+		return AE_OK;
+	}
+	/* There are more ACPI Processor objects than in x2APIC or MADT.
+	 * This can happen with incorrect ACPI SSDT declerations. */
+	if (acpi_id > nr_acpi_bits) {
+		pr_debug(DRV_NAME "We only have %u, trying to set %u\n",
+			 nr_acpi_bits, acpi_id);
+		return AE_OK;
+	}
+	/* OK, There is a ACPI Processor object */
+	__set_bit(acpi_id, acpi_id_present);
+
+	pr_debug(DRV_NAME "ACPI CPU%u w/ PBLK:0x%lx\n", acpi_id,
+		 (unsigned long)pblk);
+
+	status = acpi_evaluate_object(handle, "_CST", NULL, &buffer);
+	if (ACPI_FAILURE(status)) {
+		if (!pblk)
+			return AE_OK;
+	}
+	/* .. and it has a C-state */
+	__set_bit(acpi_id, acpi_id_cst_present);
+
+	return AE_OK;
+}
+static int __init check_acpi_ids(struct acpi_processor *pr_backup)
+{
+
+	if (!pr_backup)
+		return -ENODEV;
+
+	/* All online CPUs have been processed at this stage. Now verify
+	 * whether in fact "online CPUs" == physical CPUs.
+	 */
+	acpi_id_present = kcalloc(BITS_TO_LONGS(nr_acpi_bits), sizeof(unsigned long), GFP_KERNEL);
+	if (!acpi_id_present)
+		return -ENOMEM;
+
+	acpi_id_cst_present = kcalloc(BITS_TO_LONGS(nr_acpi_bits), sizeof(unsigned long), GFP_KERNEL);
+	if (!acpi_id_cst_present) {
+		kfree(acpi_id_present);
+		return -ENOMEM;
+	}
+
+	acpi_walk_namespace(ACPI_TYPE_PROCESSOR, ACPI_ROOT_OBJECT,
+			    ACPI_UINT32_MAX,
+			    read_acpi_id, NULL, NULL, NULL);
+	acpi_get_devices("ACPI0007", read_acpi_id, NULL, NULL);
+
+	if (!bitmap_equal(acpi_id_present, acpi_ids_done, nr_acpi_bits)) {
+		unsigned int i;
+		for_each_set_bit(i, acpi_id_present, nr_acpi_bits) {
+			pr_backup->acpi_id = i;
+			/* Mask out C-states if there are no _CST or PBLK */
+			pr_backup->flags.power = test_bit(i, acpi_id_cst_present);
+			(void)upload_pm_data(pr_backup);
+		}
+	}
+	kfree(acpi_id_present);
+	acpi_id_present = NULL;
+	kfree(acpi_id_cst_present);
+	acpi_id_cst_present = NULL;
+	return 0;
+}
+static int __init check_prereq(void)
+{
+	struct cpuinfo_x86 *c = &cpu_data(0);
+
+	if (!xen_initial_domain())
+		return -ENODEV;
+
+	if (!acpi_gbl_FADT.smi_command)
+		return -ENODEV;
+
+	if (c->x86_vendor == X86_VENDOR_INTEL) {
+		if (!cpu_has(c, X86_FEATURE_EST))
+			return -ENODEV;
+
+		return 0;
+	}
+	if (c->x86_vendor == X86_VENDOR_AMD) {
+		/* Copied from powernow-k8.h, can't include ../cpufreq/powernow
+		 * as we get compile warnings for the static functions.
+		 */
+#define CPUID_FREQ_VOLT_CAPABILITIES    0x80000007
+#define USE_HW_PSTATE                   0x00000080
+		u32 eax, ebx, ecx, edx;
+		cpuid(CPUID_FREQ_VOLT_CAPABILITIES, &eax, &ebx, &ecx, &edx);
+		if ((edx & USE_HW_PSTATE) != USE_HW_PSTATE)
+			return -ENODEV;
+		return 0;
+	}
+	return -ENODEV;
+}
+/* acpi_perf_data is a pointer to percpu data. */
+static struct acpi_processor_performance __percpu *acpi_perf_data;
+
+static void free_acpi_perf_data(void)
+{
+	unsigned int i;
+
+	/* Freeing a NULL pointer is OK, and alloc_percpu zeroes. */
+	for_each_possible_cpu(i)
+		free_cpumask_var(per_cpu_ptr(acpi_perf_data, i)
+				 ->shared_cpu_map);
+	free_percpu(acpi_perf_data);
+}
+
+static int __init xen_acpi_processor_init(void)
+{
+	struct acpi_processor *pr_backup = NULL;
+	unsigned int i;
+	int rc = check_prereq();
+
+	if (rc)
+		return rc;
+
+	nr_acpi_bits = get_max_acpi_id() + 1;
+	acpi_ids_done = kcalloc(BITS_TO_LONGS(nr_acpi_bits), sizeof(unsigned long), GFP_KERNEL);
+	if (!acpi_ids_done)
+		return -ENOMEM;
+
+	acpi_perf_data = alloc_percpu(struct acpi_processor_performance);
+	if (!acpi_perf_data) {
+		pr_debug(DRV_NAME "Memory allocation error for acpi_perf_data.\n");
+		kfree(acpi_ids_done);
+		return -ENOMEM;
+	}
+	for_each_possible_cpu(i) {
+		if (!zalloc_cpumask_var_node(
+			&per_cpu_ptr(acpi_perf_data, i)->shared_cpu_map,
+			GFP_KERNEL, cpu_to_node(i))) {
+			rc = -ENOMEM;
+			goto err_out;
+		}
+	}
+
+	/* Do initialization in ACPI core */
+	rc = acpi_processor_preregister_performance(acpi_perf_data);
+	if (WARN_ON(rc))
+		goto err_out;
+
+	for_each_possible_cpu(i) {
+		struct acpi_processor_performance *perf;
+
+		perf = per_cpu_ptr(acpi_perf_data, i);
+		rc = acpi_processor_register_performance(perf, i);
+		if (WARN_ON(rc))
+			goto err_out;
+	}
+	rc = acpi_processor_notify_smm(THIS_MODULE);
+	if (WARN_ON(rc))
+		goto err_unregister;
+
+	for_each_possible_cpu(i) {
+		struct acpi_processor *_pr;
+		_pr = per_cpu(processors, i /* APIC ID */);
+		if (!_pr)
+			continue;
+
+		if (!pr_backup) {
+			pr_backup = kzalloc(sizeof(struct acpi_processor), GFP_KERNEL);
+			memcpy(pr_backup, _pr, sizeof(struct acpi_processor));
+		}
+		(void)upload_pm_data(_pr);
+	}
+	rc = check_acpi_ids(pr_backup);
+	if (rc)
+		goto err_unregister;
+
+	kfree(pr_backup);
+
+	return 0;
+err_unregister:
+	for_each_possible_cpu(i) {
+		struct acpi_processor_performance *perf;
+		perf = per_cpu_ptr(acpi_perf_data, i);
+		acpi_processor_unregister_performance(perf, i);
+	}
+err_out:
+	/* Freeing a NULL pointer is OK: alloc_percpu zeroes. */
+	free_acpi_perf_data();
+	kfree(acpi_ids_done);
+	return rc;
+}
+static void __exit xen_acpi_processor_exit(void)
+{
+	int i;
+
+	kfree(acpi_ids_done);
+	for_each_possible_cpu(i) {
+		struct acpi_processor_performance *perf;
+		perf = per_cpu_ptr(acpi_perf_data, i);
+		acpi_processor_unregister_performance(perf, i);
+	}
+	free_acpi_perf_data();
+}
+
+MODULE_AUTHOR("Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>");
+MODULE_DESCRIPTION("Xen ACPI Processor P-states (and Cx) driver which uploads PM data to Xen hypervisor");
+MODULE_LICENSE("GPL");
+
+/* We want to be loaded before the CPU freq scaling drivers are loaded.
+ * They are loaded in late_initcall. */
+device_initcall(xen_acpi_processor_init);
+module_exit(xen_acpi_processor_exit);
diff --git a/include/xen/interface/platform.h b/include/xen/interface/platform.h
index 861b359b..486653f 100644
--- a/include/xen/interface/platform.h
+++ b/include/xen/interface/platform.h
@@ -298,6 +298,22 @@ struct xenpf_set_processor_pminfo {
 };
 DEFINE_GUEST_HANDLE_STRUCT(xenpf_set_processor_pminfo);
 
+#define XENPF_get_cpuinfo 55
+struct xenpf_pcpuinfo {
+	/* IN */
+	uint32_t xen_cpuid;
+	/* OUT */
+	/* The maxium cpu_id that is present */
+	uint32_t max_present;
+#define XEN_PCPU_FLAGS_ONLINE   1
+	/* Correponding xen_cpuid is not present*/
+#define XEN_PCPU_FLAGS_INVALID  2
+	uint32_t flags;
+	uint32_t apic_id;
+	uint32_t acpi_id;
+};
+DEFINE_GUEST_HANDLE_STRUCT(xenpf_pcpuinfo);
+
 struct xen_platform_op {
 	uint32_t cmd;
 	uint32_t interface_version; /* XENPF_INTERFACE_VERSION */
@@ -313,6 +329,7 @@ struct xen_platform_op {
 		struct xenpf_change_freq       change_freq;
 		struct xenpf_getidletime       getidletime;
 		struct xenpf_set_processor_pminfo set_pminfo;
+		struct xenpf_pcpuinfo          pcpu_info;
 		uint8_t                        pad[128];
 	} u;
 };
-- 
1.7.7.5

Re: [PATCH] xen/acpi-processor: C and P-state driver that uploads said data to hypervisor.

[Jan Beulich]

>>> On 13.03.12 at 04:35, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
> +static unsigned int __init get_max_acpi_id(void)
> +{
> +	struct xenpf_pcpuinfo *info;
> +	struct xen_platform_op op = {
> +		.cmd = XENPF_get_cpuinfo,
> +		.interface_version = XENPF_INTERFACE_VERSION,
> +	};
> +	int ret = 0;
> +	unsigned int i, max_apic_id, max_acpi_id = 0;
> +
> +	info = &op.u.pcpu_info;
> +	info->xen_cpuid = 0;
> +
> +	ret = HYPERVISOR_dom0_op(&op);
> +	if (ret)
> +		return max_acpi_id;
> +
> +	/* The max_present is the same irregardless of the xen_cpuid */
> +	max_apic_id = op.u.pcpu_info.max_present;

The 'apic' in the name here is confusing, as this isn't an APIC ID, but
(Xen) ID of the last present CPU (i.e. one less than the maximum
count).

> +	for (i = 0; i <= max_apic_id; i++) {
> +		info->xen_cpuid = i;
> +		ret = HYPERVISOR_dom0_op(&op);
> +		if (ret)
> +			continue;
> +		max_acpi_id = max(info->acpi_id, max_acpi_id);
> +	}
> +	pr_debug(DRV_NAME "Max ACPI ID: %u\n", max_acpi_id);
> +	return max_acpi_id;

Iirc hotplug CPUs may not be represented in MADT, and hence I'd
suggest adding some slack here (e.g. doubling the value).

> +}

Now, if the whole thing could become a true extension to processor.ko,
the fishy (and too weak in the 'm' case, as you noted yourself) Kconfig
dependency on the cpufreq drivers could go away, since both
acpi-cpufreq and powernow-k8 depend on ACPI_PROCESSOR. Which
would at once put it on par with the XenoLinux placement (in terms of
the point in time when it gets executed) of the code.

Jan