>From f233ad06cf924116693d7d38be9ae9d8c11f8a9b Mon Sep 17 00:00:00 2001
From: Liu, Jinsong <[email protected]>
Date: Fri, 26 Oct 2012 02:32:48 +0800
Subject: [PATCH 1/2] Xen acpi pad implement
PAD is acpi Processor Aggregator Device which provides a control point
that enables the platform to perform specific processor configuration
and control that applies to all processors in the platform.
This patch is to implement Xen acpi pad logic. When running under Xen
virt platform, native pad driver would not work. Instead Xen pad driver,
a self-contained and very thin logic level, would take over acpi pad staff.
When acpi pad notify OSPM, xen pad logic intercept and parse _PUR object
and then hypercall to hyervisor for the rest work, say, core parking.
Signed-off-by: Liu, Jinsong <[email protected]>
---
drivers/xen/Makefile | 1 +
drivers/xen/xen_acpi_pad.c | 173 ++++++++++++++++++++++++++++++++++++++
include/xen/interface/platform.h | 17 ++++
3 files changed, 191 insertions(+), 0 deletions(-)
create mode 100644 drivers/xen/xen_acpi_pad.c
diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
index 0e86370..a2af622 100644
--- a/drivers/xen/Makefile
+++ b/drivers/xen/Makefile
@@ -29,6 +29,7 @@ obj-$(CONFIG_XEN_MCE_LOG) += mcelog.o
obj-$(CONFIG_XEN_PCIDEV_BACKEND) += xen-pciback/
obj-$(CONFIG_XEN_PRIVCMD) += xen-privcmd.o
obj-$(CONFIG_XEN_ACPI_PROCESSOR) += xen-acpi-processor.o
+obj-$(CONFIG_XEN_DOM0) += xen_acpi_pad.o
xen-evtchn-y := evtchn.o
xen-gntdev-y := gntdev.o
xen-gntalloc-y := gntalloc.o
diff --git a/drivers/xen/xen_acpi_pad.c b/drivers/xen/xen_acpi_pad.c
new file mode 100644
index 0000000..e7b7dca
--- /dev/null
+++ b/drivers/xen/xen_acpi_pad.c
@@ -0,0 +1,173 @@
+/*
+ * xen_acpi_pad.c - Xen pad interface
+ *
+ * Copyright (c) 2012, Intel Corporation.
+ * Author: Liu, Jinsong <[email protected]>
+ *
+ * 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/kernel.h>
+#include <linux/types.h>
+#include <acpi/acpi_bus.h>
+#include <acpi/acpi_drivers.h>
+#include <asm/xen/hypercall.h>
+
+#if defined(CONFIG_ACPI_PROCESSOR_AGGREGATOR) || \
+ defined(CONFIG_ACPI_PROCESSOR_AGGREGATOR_MODULE)
+
+#define ACPI_PROCESSOR_AGGREGATOR_CLASS "acpi_pad"
+#define ACPI_PROCESSOR_AGGREGATOR_DEVICE_NAME "Processor Aggregator"
+#define ACPI_PROCESSOR_AGGREGATOR_NOTIFY 0x80
+
+static int xen_acpi_pad_idle_cpus(int *num_cpus)
+{
+ int ret;
+
+ struct xen_platform_op op = {
+ .cmd = XENPF_core_parking,
+ .interface_version = XENPF_INTERFACE_VERSION,
+ };
+
+ /* set cpu nums expected to be idled */
+ op.u.core_parking.type = XEN_CORE_PARKING_SET;
+ op.u.core_parking.idle_nums = (uint32_t)*num_cpus;
+ ret = HYPERVISOR_dom0_op(&op);
+ if (ret)
+ return ret;
+
+ /*
+ * get cpu nums actually be idled
+ * cannot get it by using hypercall once (shared with _SET)
+ * because of the characteristic of Xen continue_hypercall_on_cpu
+ */
+ op.u.core_parking.type = XEN_CORE_PARKING_GET;
+ ret = HYPERVISOR_dom0_op(&op);
+ if (ret)
+ return ret;
+
+ *num_cpus = op.u.core_parking.idle_nums;
+ return 0;
+}
+
+/*
+ * Query firmware how many CPUs should be idle
+ * return -1 on failure
+ */
+static int xen_acpi_pad_pur(acpi_handle handle)
+{
+ struct acpi_buffer buffer = {ACPI_ALLOCATE_BUFFER, NULL};
+ union acpi_object *package;
+ int num = -1;
+
+ if (ACPI_FAILURE(acpi_evaluate_object(handle, "_PUR", NULL, &buffer)))
+ return num;
+
+ if (!buffer.length || !buffer.pointer)
+ return num;
+
+ package = buffer.pointer;
+
+ if (package->type == ACPI_TYPE_PACKAGE &&
+ package->package.count == 2 &&
+ package->package.elements[0].integer.value == 1) /* rev 1 */
+
+ num = package->package.elements[1].integer.value;
+
+ kfree(buffer.pointer);
+ return num;
+}
+
+/* Notify firmware how many CPUs are idle */
+static void xen_acpi_pad_ost(acpi_handle handle, int stat,
+ uint32_t idle_cpus)
+{
+ union acpi_object params[3] = {
+ {.type = ACPI_TYPE_INTEGER,},
+ {.type = ACPI_TYPE_INTEGER,},
+ {.type = ACPI_TYPE_BUFFER,},
+ };
+ struct acpi_object_list arg_list = {3, params};
+
+ params[0].integer.value = ACPI_PROCESSOR_AGGREGATOR_NOTIFY;
+ params[1].integer.value = stat;
+ params[2].buffer.length = 4;
+ params[2].buffer.pointer = (void *)&idle_cpus;
+ acpi_evaluate_object(handle, "_OST", &arg_list, NULL);
+}
+
+static void xen_acpi_pad_handle_notify(acpi_handle handle)
+{
+ int ret, num_cpus;
+
+ num_cpus = xen_acpi_pad_pur(handle);
+ if (num_cpus < 0)
+ return;
+
+ ret = xen_acpi_pad_idle_cpus(&num_cpus);
+ if (ret)
+ return;
+
+ xen_acpi_pad_ost(handle, 0, num_cpus);
+}
+
+static void xen_acpi_pad_notify(acpi_handle handle, u32 event,
+ void *data)
+{
+ switch (event) {
+ case ACPI_PROCESSOR_AGGREGATOR_NOTIFY:
+ xen_acpi_pad_handle_notify(handle);
+ break;
+ default:
+ pr_warn("Unsupported event [0x%x]\n", event);
+ break;
+ }
+}
+
+static int xen_acpi_pad_add(struct acpi_device *device)
+{
+ acpi_status status;
+
+ strcpy(acpi_device_name(device), ACPI_PROCESSOR_AGGREGATOR_DEVICE_NAME);
+ strcpy(acpi_device_class(device), ACPI_PROCESSOR_AGGREGATOR_CLASS);
+
+ status = acpi_install_notify_handler(device->handle,
+ ACPI_DEVICE_NOTIFY, xen_acpi_pad_notify, device);
+ if (ACPI_FAILURE(status))
+ return -ENODEV;
+
+ return 0;
+}
+
+static const struct acpi_device_id pad_device_ids[] = {
+ {"ACPI000C", 0},
+ {"", 0},
+};
+
+static struct acpi_driver xen_acpi_pad_driver = {
+ .name = "processor_aggregator",
+ .class = ACPI_PROCESSOR_AGGREGATOR_CLASS,
+ .ids = pad_device_ids,
+ .ops = {
+ .add = xen_acpi_pad_add,
+ },
+};
+
+static int __init xen_acpi_pad_init(void)
+{
+ /* Only DOM0 is responsible for Xen acpi pad */
+ if (xen_initial_domain())
+ return acpi_bus_register_driver(&xen_acpi_pad_driver);
+
+ return -ENODEV;
+}
+subsys_initcall(xen_acpi_pad_init);
+
+#endif
diff --git a/include/xen/interface/platform.h b/include/xen/interface/platform.h
index 4755b5f..0f44376 100644
--- a/include/xen/interface/platform.h
+++ b/include/xen/interface/platform.h
@@ -324,6 +324,22 @@ struct xenpf_cpu_ol {
};
DEFINE_GUEST_HANDLE_STRUCT(xenpf_cpu_ol);
+/*
+ * CMD 58 and 59 are reserved for cpu hotadd and memory hotadd,
+ * which already occupied at Xen hypervisor side.
+ */
+#define XENPF_core_parking 60
+struct xenpf_core_parking {
+ /* IN variables */
+#define XEN_CORE_PARKING_SET 1
+#define XEN_CORE_PARKING_GET 2
+ uint32_t type;
+ /* IN variables: set cpu nums expected to be idled */
+ /* OUT variables: get cpu nums actually be idled */
+ uint32_t idle_nums;
+};
+DEFINE_GUEST_HANDLE_STRUCT(xenpf_core_parking);
+
struct xen_platform_op {
uint32_t cmd;
uint32_t interface_version; /* XENPF_INTERFACE_VERSION */
@@ -341,6 +357,7 @@ struct xen_platform_op {
struct xenpf_set_processor_pminfo set_pminfo;
struct xenpf_pcpuinfo pcpu_info;
struct xenpf_cpu_ol cpu_ol;
+ struct xenpf_core_parking core_parking;
uint8_t pad[128];
} u;
};
--
1.7.1
Thanks! updated accordingly, w/ 2 comments left as below:
> +static const struct acpi_device_id pad_device_ids[] = {
> + {"ACPI000C", 0},
> + {"", 0},
> +};
> +
> +static struct acpi_driver xen_acpi_pad_driver = {
> + .name = "processor_aggregator",
> + .class = ACPI_PROCESSOR_AGGREGATOR_CLASS,
> + .ids = pad_device_ids,
> + .ops = {
> + .add = xen_acpi_pad_add,
.remove?
[Jinsong] .remove method not used by any logic now (any possible point use it?), so we remove it from our former patch.
> + },
> +};
> +
> +static int __init xen_acpi_pad_init(void)
> +{
> + /* Only DOM0 is responsible for Xen acpi pad */
> + if (xen_initial_domain())
> + return acpi_bus_register_driver(&xen_acpi_pad_driver);
> +
> + return -ENODEV;
> +}
> +subsys_initcall(xen_acpi_pad_init);
> +
> +#endif
Overall I'd recommend taking a look at the cleaned up driver in
our kernels.
[Jinsong] What's your point here?
Thanks,
Jinsong
>>> "Liu, Jinsong" <[email protected]> 10/26/12 8:18 AM >>>
>> +static struct acpi_driver xen_acpi_pad_driver = {
>> + .name = "processor_aggregator",
>> + .class = ACPI_PROCESSOR_AGGREGATOR_CLASS,
>> + .ids = pad_device_ids,
>> + .ops = {
>> + .add = xen_acpi_pad_add,
>
>.remove?
>
>[Jinsong] .remove method not used by any logic now (any possible point use it?), so we remove it from our former patch.
Unless there is technical difficulty implementing it, I wouldn't defer adding that code until the point where something doesn't work anymore.
>Overall I'd recommend taking a look at the cleaned up driver in
>our kernels.
>
>[Jinsong] What's your point here?
There's quite a bit of cleanup/simplification potential here, and rather than pointing the pieces out individually I would think comparing with what we have in production use might be worthwhile. But that's up to you of course.
Jan
-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Jan Beulich
Sent: Friday, October 26, 2012 8:02 PM
To: Liu, Jinsong
Cc: [email protected]; [email protected]; [email protected]
Subject: Re: [Xen-devel] [PATCH 1/2] Xen acpi pad implement
>>> "Liu, Jinsong" <[email protected]> 10/26/12 8:18 AM >>>
>> +static struct acpi_driver xen_acpi_pad_driver = {
>> + .name = "processor_aggregator",
>> + .class = ACPI_PROCESSOR_AGGREGATOR_CLASS,
>> + .ids = pad_device_ids,
>> + .ops = {
>> + .add = xen_acpi_pad_add,
>
>.remove?
>
>[Jinsong] .remove method not used by any logic now (any possible point use it?), so we remove it from our former patch.
Unless there is technical difficulty implementing it, I wouldn't defer adding that code until the point where something doesn't work anymore.
[Jinsong] No technical difficulty at all, in fact at last version it has .remove method. I will re-add it.
>Overall I'd recommend taking a look at the cleaned up driver in
>our kernels.
>
>[Jinsong] What's your point here?
There's quite a bit of cleanup/simplification potential here, and rather than pointing the pieces out individually I would think comparing with what we have in production use might be worthwhile. But that's up to you of course.
[Jinsong] I know your concern now -- we can cleanup/simplify xen pad logic by piggyback on native acpi pad code -- technically it's true. However, we intentionally do so in order to keep xen pad logic self-contained, just like what xen mcelog logic did before.
Thanks,
Jinsong-
>>> On 25.10.12 at 14:19, "Liu, Jinsong" <[email protected]> wrote:
> --- /dev/null
> +++ b/drivers/xen/xen_acpi_pad.c
> @@ -0,0 +1,173 @@
> +/*
> + * xen_acpi_pad.c - Xen pad interface
> + *
> + * Copyright (c) 2012, Intel Corporation.
> + * Author: Liu, Jinsong <[email protected]>
> + *
> + * 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/kernel.h>
> +#include <linux/types.h>
> +#include <acpi/acpi_bus.h>
> +#include <acpi/acpi_drivers.h>
> +#include <asm/xen/hypercall.h>
> +
> +#if defined(CONFIG_ACPI_PROCESSOR_AGGREGATOR) || \
> + defined(CONFIG_ACPI_PROCESSOR_AGGREGATOR_MODULE)
> +
> +#define ACPI_PROCESSOR_AGGREGATOR_CLASS "acpi_pad"
> +#define ACPI_PROCESSOR_AGGREGATOR_DEVICE_NAME "Processor Aggregator"
> +#define ACPI_PROCESSOR_AGGREGATOR_NOTIFY 0x80
> +
> +static int xen_acpi_pad_idle_cpus(int *num_cpus)
> +{
> + int ret;
> +
Stray blank line.
> + struct xen_platform_op op = {
> + .cmd = XENPF_core_parking,
> + .interface_version = XENPF_INTERFACE_VERSION,
This is redundant with HYPERVISOR_dom0_op().
> + };
> +
> + /* set cpu nums expected to be idled */
> + op.u.core_parking.type = XEN_CORE_PARKING_SET;
> + op.u.core_parking.idle_nums = (uint32_t)*num_cpus;
It is quite a bit more efficient in terms of generated code to
initialize all fields using assignments (the use of an initializer
setting only a subset of fields will cause all other fields to get
zero initialized). In any case I think it is bad style to mix both
approaches.
> + ret = HYPERVISOR_dom0_op(&op);
> + if (ret)
> + return ret;
> +
> + /*
> + * get cpu nums actually be idled
> + * cannot get it by using hypercall once (shared with _SET)
> + * because of the characteristic of Xen continue_hypercall_on_cpu
> + */
> + op.u.core_parking.type = XEN_CORE_PARKING_GET;
> + ret = HYPERVISOR_dom0_op(&op);
> + if (ret)
> + return ret;
> +
> + *num_cpus = op.u.core_parking.idle_nums;
"num_cpus" doesn't need to be a pointer, and you don't need to
call _GET here either - callers that care for the count in effect
after the call can invoke the corresponding _GET on their own.
> + return 0;
> +}
> +
> +/*
> + * Query firmware how many CPUs should be idle
> + * return -1 on failure
> + */
> +static int xen_acpi_pad_pur(acpi_handle handle)
> +{
> + struct acpi_buffer buffer = {ACPI_ALLOCATE_BUFFER, NULL};
> + union acpi_object *package;
> + int num = -1;
> +
> + if (ACPI_FAILURE(acpi_evaluate_object(handle, "_PUR", NULL, &buffer)))
> + return num;
> +
> + if (!buffer.length || !buffer.pointer)
> + return num;
> +
> + package = buffer.pointer;
> +
> + if (package->type == ACPI_TYPE_PACKAGE &&
> + package->package.count == 2 &&
> + package->package.elements[0].integer.value == 1) /* rev 1 */
> +
> + num = package->package.elements[1].integer.value;
> +
> + kfree(buffer.pointer);
> + return num;
> +}
> +
> +/* Notify firmware how many CPUs are idle */
> +static void xen_acpi_pad_ost(acpi_handle handle, int stat,
> + uint32_t idle_cpus)
> +{
> + union acpi_object params[3] = {
> + {.type = ACPI_TYPE_INTEGER,},
> + {.type = ACPI_TYPE_INTEGER,},
> + {.type = ACPI_TYPE_BUFFER,},
> + };
> + struct acpi_object_list arg_list = {3, params};
> +
> + params[0].integer.value = ACPI_PROCESSOR_AGGREGATOR_NOTIFY;
> + params[1].integer.value = stat;
> + params[2].buffer.length = 4;
> + params[2].buffer.pointer = (void *)&idle_cpus;
> + acpi_evaluate_object(handle, "_OST", &arg_list, NULL);
> +}
> +
> +static void xen_acpi_pad_handle_notify(acpi_handle handle)
> +{
> + int ret, num_cpus;
> +
> + num_cpus = xen_acpi_pad_pur(handle);
> + if (num_cpus < 0)
> + return;
> +
> + ret = xen_acpi_pad_idle_cpus(&num_cpus);
> + if (ret)
> + return;
> +
> + xen_acpi_pad_ost(handle, 0, num_cpus);
> +}
> +
> +static void xen_acpi_pad_notify(acpi_handle handle, u32 event,
> + void *data)
> +{
> + switch (event) {
> + case ACPI_PROCESSOR_AGGREGATOR_NOTIFY:
> + xen_acpi_pad_handle_notify(handle);
> + break;
> + default:
> + pr_warn("Unsupported event [0x%x]\n", event);
> + break;
> + }
> +}
> +
> +static int xen_acpi_pad_add(struct acpi_device *device)
> +{
> + acpi_status status;
> +
> + strcpy(acpi_device_name(device), ACPI_PROCESSOR_AGGREGATOR_DEVICE_NAME);
> + strcpy(acpi_device_class(device), ACPI_PROCESSOR_AGGREGATOR_CLASS);
> +
> + status = acpi_install_notify_handler(device->handle,
> + ACPI_DEVICE_NOTIFY, xen_acpi_pad_notify, device);
> + if (ACPI_FAILURE(status))
> + return -ENODEV;
> +
> + return 0;
> +}
> +
> +static const struct acpi_device_id pad_device_ids[] = {
> + {"ACPI000C", 0},
> + {"", 0},
> +};
> +
> +static struct acpi_driver xen_acpi_pad_driver = {
> + .name = "processor_aggregator",
> + .class = ACPI_PROCESSOR_AGGREGATOR_CLASS,
> + .ids = pad_device_ids,
> + .ops = {
> + .add = xen_acpi_pad_add,
.remove?
> + },
> +};
> +
> +static int __init xen_acpi_pad_init(void)
> +{
> + /* Only DOM0 is responsible for Xen acpi pad */
> + if (xen_initial_domain())
> + return acpi_bus_register_driver(&xen_acpi_pad_driver);
> +
> + return -ENODEV;
> +}
> +subsys_initcall(xen_acpi_pad_init);
> +
> +#endif
Overall I'd recommend taking a look at the cleaned up driver in
our kernels.
Jan
Updated per Jan's comments.
Thanks,
Jinsong
===============
>From e6d4eceba9538c6873f9c970425e65e257fd9bf0 Mon Sep 17 00:00:00 2001
From: Liu, Jinsong <[email protected]>
Date: Sat, 27 Oct 2012 05:34:50 +0800
Subject: [PATCH 1/2] Xen acpi pad implement
PAD is acpi Processor Aggregator Device which provides a control point
that enables the platform to perform specific processor configuration
and control that applies to all processors in the platform.
This patch is to implement Xen acpi pad logic. When running under Xen
virt platform, native pad driver would not work. Instead Xen pad driver,
a self-contained and very thin logic level, would take over acpi pad staff.
When acpi pad notify OSPM, xen pad logic intercept and parse _PUR object
and then hypercall to hyervisor for the rest work, say, core parking.
Signed-off-by: Liu, Jinsong <[email protected]>
---
drivers/xen/Makefile | 1 +
drivers/xen/xen_acpi_pad.c | 201 ++++++++++++++++++++++++++++++++++++++
include/xen/interface/platform.h | 17 +++
3 files changed, 219 insertions(+), 0 deletions(-)
create mode 100644 drivers/xen/xen_acpi_pad.c
diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
index 0e86370..a2af622 100644
--- a/drivers/xen/Makefile
+++ b/drivers/xen/Makefile
@@ -29,6 +29,7 @@ obj-$(CONFIG_XEN_MCE_LOG) += mcelog.o
obj-$(CONFIG_XEN_PCIDEV_BACKEND) += xen-pciback/
obj-$(CONFIG_XEN_PRIVCMD) += xen-privcmd.o
obj-$(CONFIG_XEN_ACPI_PROCESSOR) += xen-acpi-processor.o
+obj-$(CONFIG_XEN_DOM0) += xen_acpi_pad.o
xen-evtchn-y := evtchn.o
xen-gntdev-y := gntdev.o
xen-gntalloc-y := gntalloc.o
diff --git a/drivers/xen/xen_acpi_pad.c b/drivers/xen/xen_acpi_pad.c
new file mode 100644
index 0000000..0f1a1e6
--- /dev/null
+++ b/drivers/xen/xen_acpi_pad.c
@@ -0,0 +1,201 @@
+/*
+ * xen_acpi_pad.c - Xen pad interface
+ *
+ * Copyright (c) 2012, Intel Corporation.
+ * Author: Liu, Jinsong <[email protected]>
+ *
+ * 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/kernel.h>
+#include <linux/types.h>
+#include <acpi/acpi_bus.h>
+#include <acpi/acpi_drivers.h>
+#include <asm/xen/hypercall.h>
+
+#if defined(CONFIG_ACPI_PROCESSOR_AGGREGATOR) || \
+ defined(CONFIG_ACPI_PROCESSOR_AGGREGATOR_MODULE)
+
+#define ACPI_PROCESSOR_AGGREGATOR_CLASS "acpi_pad"
+#define ACPI_PROCESSOR_AGGREGATOR_DEVICE_NAME "Processor Aggregator"
+#define ACPI_PROCESSOR_AGGREGATOR_NOTIFY 0x80
+
+static DEFINE_MUTEX(xen_pad_lock);
+
+static int xen_pad_set_idle_cpus(int num_cpus)
+{
+ struct xen_platform_op op;
+
+ if (num_cpus < 0)
+ return -EINVAL;
+
+ /* set cpu nums expected to be idled */
+ op.cmd = XENPF_core_parking;
+ op.u.core_parking.type = XEN_CORE_PARKING_SET;
+ op.u.core_parking.idle_nums = num_cpus;
+
+ return HYPERVISOR_dom0_op(&op);
+}
+
+/*
+ * Cannot get idle cpus by using hypercall once (shared with _SET)
+ * because of the characteristic of Xen continue_hypercall_on_cpu
+ */
+static int xen_pad_get_idle_cpus(void)
+{
+ int ret;
+ struct xen_platform_op op;
+
+ /* get cpu nums actually be idled */
+ op.cmd = XENPF_core_parking;
+ op.u.core_parking.type = XEN_CORE_PARKING_GET;
+ ret = HYPERVISOR_dom0_op(&op);
+ if (ret < 0)
+ return ret;
+
+ return op.u.core_parking.idle_nums;
+}
+
+/*
+ * Query firmware how many CPUs should be idle
+ * return -1 on failure
+ */
+static int xen_acpi_pad_pur(acpi_handle handle)
+{
+ struct acpi_buffer buffer = {ACPI_ALLOCATE_BUFFER, NULL};
+ union acpi_object *package;
+ int num = -1;
+
+ if (ACPI_FAILURE(acpi_evaluate_object(handle, "_PUR", NULL, &buffer)))
+ return num;
+
+ if (!buffer.length || !buffer.pointer)
+ return num;
+
+ package = buffer.pointer;
+
+ if (package->type == ACPI_TYPE_PACKAGE &&
+ package->package.count == 2 &&
+ package->package.elements[0].integer.value == 1) /* rev 1 */
+
+ num = package->package.elements[1].integer.value;
+
+ kfree(buffer.pointer);
+ return num;
+}
+
+/* Notify firmware how many CPUs are idle */
+static void xen_acpi_pad_ost(acpi_handle handle, int stat,
+ uint32_t idle_cpus)
+{
+ union acpi_object params[3] = {
+ {.type = ACPI_TYPE_INTEGER,},
+ {.type = ACPI_TYPE_INTEGER,},
+ {.type = ACPI_TYPE_BUFFER,},
+ };
+ struct acpi_object_list arg_list = {3, params};
+
+ params[0].integer.value = ACPI_PROCESSOR_AGGREGATOR_NOTIFY;
+ params[1].integer.value = stat;
+ params[2].buffer.length = 4;
+ params[2].buffer.pointer = (void *)&idle_cpus;
+ acpi_evaluate_object(handle, "_OST", &arg_list, NULL);
+}
+
+static void xen_acpi_pad_handle_notify(acpi_handle handle)
+{
+ int num_cpus;
+
+ num_cpus = xen_acpi_pad_pur(handle);
+ if (num_cpus < 0)
+ return;
+
+ mutex_lock(&xen_pad_lock);
+ if (xen_pad_set_idle_cpus(num_cpus)) {
+ mutex_unlock(&xen_pad_lock);
+ return;
+ }
+
+ num_cpus = xen_pad_get_idle_cpus();
+ if (num_cpus < 0) {
+ mutex_unlock(&xen_pad_lock);
+ return;
+ }
+ mutex_unlock(&xen_pad_lock);
+
+ xen_acpi_pad_ost(handle, 0, num_cpus);
+}
+
+static void xen_acpi_pad_notify(acpi_handle handle, u32 event,
+ void *data)
+{
+ switch (event) {
+ case ACPI_PROCESSOR_AGGREGATOR_NOTIFY:
+ xen_acpi_pad_handle_notify(handle);
+ break;
+ default:
+ pr_warn("Unsupported event [0x%x]\n", event);
+ break;
+ }
+}
+
+static int xen_acpi_pad_add(struct acpi_device *device)
+{
+ acpi_status status;
+
+ strcpy(acpi_device_name(device), ACPI_PROCESSOR_AGGREGATOR_DEVICE_NAME);
+ strcpy(acpi_device_class(device), ACPI_PROCESSOR_AGGREGATOR_CLASS);
+
+ status = acpi_install_notify_handler(device->handle,
+ ACPI_DEVICE_NOTIFY, xen_acpi_pad_notify, device);
+ if (ACPI_FAILURE(status))
+ return -ENODEV;
+
+ return 0;
+}
+
+static int xen_acpi_pad_remove(struct acpi_device *device,
+ int type)
+{
+ mutex_lock(&xen_pad_lock);
+ xen_pad_set_idle_cpus(0);
+ mutex_unlock(&xen_pad_lock);
+
+ acpi_remove_notify_handler(device->handle,
+ ACPI_DEVICE_NOTIFY, xen_acpi_pad_notify);
+ return 0;
+}
+
+static const struct acpi_device_id pad_device_ids[] = {
+ {"ACPI000C", 0},
+ {"", 0},
+};
+
+static struct acpi_driver xen_acpi_pad_driver = {
+ .name = "processor_aggregator",
+ .class = ACPI_PROCESSOR_AGGREGATOR_CLASS,
+ .ids = pad_device_ids,
+ .ops = {
+ .add = xen_acpi_pad_add,
+ .remove = xen_acpi_pad_remove,
+ },
+};
+
+static int __init xen_acpi_pad_init(void)
+{
+ /* Only DOM0 is responsible for Xen acpi pad */
+ if (xen_initial_domain())
+ return acpi_bus_register_driver(&xen_acpi_pad_driver);
+
+ return -ENODEV;
+}
+subsys_initcall(xen_acpi_pad_init);
+
+#endif
diff --git a/include/xen/interface/platform.h b/include/xen/interface/platform.h
index 4755b5f..0f44376 100644
--- a/include/xen/interface/platform.h
+++ b/include/xen/interface/platform.h
@@ -324,6 +324,22 @@ struct xenpf_cpu_ol {
};
DEFINE_GUEST_HANDLE_STRUCT(xenpf_cpu_ol);
+/*
+ * CMD 58 and 59 are reserved for cpu hotadd and memory hotadd,
+ * which already occupied at Xen hypervisor side.
+ */
+#define XENPF_core_parking 60
+struct xenpf_core_parking {
+ /* IN variables */
+#define XEN_CORE_PARKING_SET 1
+#define XEN_CORE_PARKING_GET 2
+ uint32_t type;
+ /* IN variables: set cpu nums expected to be idled */
+ /* OUT variables: get cpu nums actually be idled */
+ uint32_t idle_nums;
+};
+DEFINE_GUEST_HANDLE_STRUCT(xenpf_core_parking);
+
struct xen_platform_op {
uint32_t cmd;
uint32_t interface_version; /* XENPF_INTERFACE_VERSION */
@@ -341,6 +357,7 @@ struct xen_platform_op {
struct xenpf_set_processor_pminfo set_pminfo;
struct xenpf_pcpuinfo pcpu_info;
struct xenpf_cpu_ol cpu_ol;
+ struct xenpf_core_parking core_parking;
uint8_t pad[128];
} u;
};
--
1.7.1
On Fri, Oct 26, 2012 at 02:00:36PM +0000, Liu, Jinsong wrote:
> Updated per Jan's comments.
>
> Thanks,
> Jinsong
>
> ===============
>
> >From e6d4eceba9538c6873f9c970425e65e257fd9bf0 Mon Sep 17 00:00:00 2001
> From: Liu, Jinsong <[email protected]>
> Date: Sat, 27 Oct 2012 05:34:50 +0800
> Subject: [PATCH 1/2] Xen acpi pad implement
>
> PAD is acpi Processor Aggregator Device which provides a control point
> that enables the platform to perform specific processor configuration
> and control that applies to all processors in the platform.
>
> This patch is to implement Xen acpi pad logic. When running under Xen
> virt platform, native pad driver would not work. Instead Xen pad driver,
> a self-contained and very thin logic level, would take over acpi pad staff.
> When acpi pad notify OSPM, xen pad logic intercept and parse _PUR object
> and then hypercall to hyervisor for the rest work, say, core parking.
>
> Signed-off-by: Liu, Jinsong <[email protected]>
> ---
> drivers/xen/Makefile | 1 +
> drivers/xen/xen_acpi_pad.c | 201 ++++++++++++++++++++++++++++++++++++++
> include/xen/interface/platform.h | 17 +++
> 3 files changed, 219 insertions(+), 0 deletions(-)
> create mode 100644 drivers/xen/xen_acpi_pad.c
>
> diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
> index 0e86370..a2af622 100644
> --- a/drivers/xen/Makefile
> +++ b/drivers/xen/Makefile
> @@ -29,6 +29,7 @@ obj-$(CONFIG_XEN_MCE_LOG) += mcelog.o
> obj-$(CONFIG_XEN_PCIDEV_BACKEND) += xen-pciback/
> obj-$(CONFIG_XEN_PRIVCMD) += xen-privcmd.o
> obj-$(CONFIG_XEN_ACPI_PROCESSOR) += xen-acpi-processor.o
> +obj-$(CONFIG_XEN_DOM0) += xen_acpi_pad.o
We should have a Kconfig option. In it, please mention what
version of hypervisor supports this functionality.
> xen-evtchn-y := evtchn.o
> xen-gntdev-y := gntdev.o
> xen-gntalloc-y := gntalloc.o
> diff --git a/drivers/xen/xen_acpi_pad.c b/drivers/xen/xen_acpi_pad.c
> new file mode 100644
> index 0000000..0f1a1e6
> --- /dev/null
> +++ b/drivers/xen/xen_acpi_pad.c
> @@ -0,0 +1,201 @@
> +/*
> + * xen_acpi_pad.c - Xen pad interface
> + *
> + * Copyright (c) 2012, Intel Corporation.
> + * Author: Liu, Jinsong <[email protected]>
> + *
> + * 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/kernel.h>
> +#include <linux/types.h>
> +#include <acpi/acpi_bus.h>
> +#include <acpi/acpi_drivers.h>
> +#include <asm/xen/hypercall.h>
> +
> +#if defined(CONFIG_ACPI_PROCESSOR_AGGREGATOR) || \
> + defined(CONFIG_ACPI_PROCESSOR_AGGREGATOR_MODULE)
I don't think you need that.
> +
> +#define ACPI_PROCESSOR_AGGREGATOR_CLASS "acpi_pad"
> +#define ACPI_PROCESSOR_AGGREGATOR_DEVICE_NAME "Processor Aggregator"
> +#define ACPI_PROCESSOR_AGGREGATOR_NOTIFY 0x80
> +
> +static DEFINE_MUTEX(xen_pad_lock);
> +
> +static int xen_pad_set_idle_cpus(int num_cpus)
> +{
> + struct xen_platform_op op;
> +
> + if (num_cpus < 0)
> + return -EINVAL;
> +
> + /* set cpu nums expected to be idled */
> + op.cmd = XENPF_core_parking;
> + op.u.core_parking.type = XEN_CORE_PARKING_SET;
> + op.u.core_parking.idle_nums = num_cpus;
> +
> + return HYPERVISOR_dom0_op(&op);
> +}
> +
> +/*
> + * Cannot get idle cpus by using hypercall once (shared with _SET)
> + * because of the characteristic of Xen continue_hypercall_on_cpu
> + */
> +static int xen_pad_get_idle_cpus(void)
> +{
> + int ret;
> + struct xen_platform_op op;
> +
> + /* get cpu nums actually be idled */
> + op.cmd = XENPF_core_parking;
> + op.u.core_parking.type = XEN_CORE_PARKING_GET;
> + ret = HYPERVISOR_dom0_op(&op);
> + if (ret < 0)
> + return ret;
> +
> + return op.u.core_parking.idle_nums;
> +}
> +
> +/*
> + * Query firmware how many CPUs should be idle
> + * return -1 on failure
> + */
> +static int xen_acpi_pad_pur(acpi_handle handle)
> +{
> + struct acpi_buffer buffer = {ACPI_ALLOCATE_BUFFER, NULL};
> + union acpi_object *package;
> + int num = -1;
> +
> + if (ACPI_FAILURE(acpi_evaluate_object(handle, "_PUR", NULL, &buffer)))
> + return num;
> +
> + if (!buffer.length || !buffer.pointer)
> + return num;
> +
> + package = buffer.pointer;
> +
> + if (package->type == ACPI_TYPE_PACKAGE &&
> + package->package.count == 2 &&
> + package->package.elements[0].integer.value == 1) /* rev 1 */
> +
> + num = package->package.elements[1].integer.value;
> +
> + kfree(buffer.pointer);
> + return num;
> +}
> +
> +/* Notify firmware how many CPUs are idle */
> +static void xen_acpi_pad_ost(acpi_handle handle, int stat,
> + uint32_t idle_cpus)
> +{
> + union acpi_object params[3] = {
> + {.type = ACPI_TYPE_INTEGER,},
> + {.type = ACPI_TYPE_INTEGER,},
> + {.type = ACPI_TYPE_BUFFER,},
> + };
> + struct acpi_object_list arg_list = {3, params};
> +
> + params[0].integer.value = ACPI_PROCESSOR_AGGREGATOR_NOTIFY;
> + params[1].integer.value = stat;
> + params[2].buffer.length = 4;
> + params[2].buffer.pointer = (void *)&idle_cpus;
> + acpi_evaluate_object(handle, "_OST", &arg_list, NULL);
> +}
> +
> +static void xen_acpi_pad_handle_notify(acpi_handle handle)
> +{
> + int num_cpus;
> +
> + num_cpus = xen_acpi_pad_pur(handle);
> + if (num_cpus < 0)
> + return;
> +
> + mutex_lock(&xen_pad_lock);
> + if (xen_pad_set_idle_cpus(num_cpus)) {
> + mutex_unlock(&xen_pad_lock);
> + return;
> + }
> +
> + num_cpus = xen_pad_get_idle_cpus();
> + if (num_cpus < 0) {
> + mutex_unlock(&xen_pad_lock);
> + return;
> + }
> + mutex_unlock(&xen_pad_lock);
> +
> + xen_acpi_pad_ost(handle, 0, num_cpus);
> +}
> +
> +static void xen_acpi_pad_notify(acpi_handle handle, u32 event,
> + void *data)
> +{
> + switch (event) {
> + case ACPI_PROCESSOR_AGGREGATOR_NOTIFY:
> + xen_acpi_pad_handle_notify(handle);
> + break;
> + default:
> + pr_warn("Unsupported event [0x%x]\n", event);
> + break;
> + }
> +}
> +
> +static int xen_acpi_pad_add(struct acpi_device *device)
> +{
> + acpi_status status;
> +
> + strcpy(acpi_device_name(device), ACPI_PROCESSOR_AGGREGATOR_DEVICE_NAME);
> + strcpy(acpi_device_class(device), ACPI_PROCESSOR_AGGREGATOR_CLASS);
> +
> + status = acpi_install_notify_handler(device->handle,
> + ACPI_DEVICE_NOTIFY, xen_acpi_pad_notify, device);
> + if (ACPI_FAILURE(status))
> + return -ENODEV;
> +
> + return 0;
> +}
> +
> +static int xen_acpi_pad_remove(struct acpi_device *device,
> + int type)
> +{
> + mutex_lock(&xen_pad_lock);
> + xen_pad_set_idle_cpus(0);
> + mutex_unlock(&xen_pad_lock);
> +
> + acpi_remove_notify_handler(device->handle,
> + ACPI_DEVICE_NOTIFY, xen_acpi_pad_notify);
> + return 0;
> +}
> +
> +static const struct acpi_device_id pad_device_ids[] = {
> + {"ACPI000C", 0},
> + {"", 0},
> +};
> +
> +static struct acpi_driver xen_acpi_pad_driver = {
> + .name = "processor_aggregator",
> + .class = ACPI_PROCESSOR_AGGREGATOR_CLASS,
> + .ids = pad_device_ids,
> + .ops = {
> + .add = xen_acpi_pad_add,
> + .remove = xen_acpi_pad_remove,
> + },
> +};
> +
> +static int __init xen_acpi_pad_init(void)
> +{
> + /* Only DOM0 is responsible for Xen acpi pad */
> + if (xen_initial_domain())
> + return acpi_bus_register_driver(&xen_acpi_pad_driver);
> +
> + return -ENODEV;
> +}
> +subsys_initcall(xen_acpi_pad_init);
No way of making this a module? It would be nice - perhaps have a stub
function that registers the bus but does not do anything until this
proper module is loaded?
> +
> +#endif
> diff --git a/include/xen/interface/platform.h b/include/xen/interface/platform.h
> index 4755b5f..0f44376 100644
> --- a/include/xen/interface/platform.h
> +++ b/include/xen/interface/platform.h
> @@ -324,6 +324,22 @@ struct xenpf_cpu_ol {
> };
> DEFINE_GUEST_HANDLE_STRUCT(xenpf_cpu_ol);
>
> +/*
> + * CMD 58 and 59 are reserved for cpu hotadd and memory hotadd,
> + * which already occupied at Xen hypervisor side.
^- are
> + */
> +#define XENPF_core_parking 60
> +struct xenpf_core_parking {
> + /* IN variables */
> +#define XEN_CORE_PARKING_SET 1
> +#define XEN_CORE_PARKING_GET 2
> + uint32_t type;
> + /* IN variables: set cpu nums expected to be idled */
> + /* OUT variables: get cpu nums actually be idled */
> + uint32_t idle_nums;
> +};
> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_core_parking);
> +
> struct xen_platform_op {
> uint32_t cmd;
> uint32_t interface_version; /* XENPF_INTERFACE_VERSION */
> @@ -341,6 +357,7 @@ struct xen_platform_op {
> struct xenpf_set_processor_pminfo set_pminfo;
> struct xenpf_pcpuinfo pcpu_info;
> struct xenpf_cpu_ol cpu_ol;
> + struct xenpf_core_parking core_parking;
> uint8_t pad[128];
> } u;
> };
> --
> 1.7.1
It's doable to register a stub for xen. However, it's not preferred, say, to make xen pad as module, considering the case 'rmmod xen_acpi_pad' then 'insmod acpi_pad'? Under such case there is risk of mwait #UD, if we want to remove mwait check as 'patch 2/2: Revert-pad-config-check-in-xen_check_mwait.patch' did, advantages of which is to enjoy deep Cx.
IMHO to prevent mwait #UD, native pad should never have chance to register successfully when running under Xen. So it's better never unregister/disable xen pad.
Konrad Rzeszutek Wilk wrote:
>> --- a/drivers/xen/Makefile
>> +++ b/drivers/xen/Makefile
>> @@ -29,6 +29,7 @@ obj-$(CONFIG_XEN_MCE_LOG) += mcelog.o
>> obj-$(CONFIG_XEN_PCIDEV_BACKEND) += xen-pciback/
>> obj-$(CONFIG_XEN_PRIVCMD) += xen-privcmd.o
>> obj-$(CONFIG_XEN_ACPI_PROCESSOR) += xen-acpi-processor.o
>> +obj-$(CONFIG_XEN_DOM0) += xen_acpi_pad.o
>
> We should have a Kconfig option. In it, please mention what
> version of hypervisor supports this functionality.
>
Kconfig option for xen pad is not preferred, considering if we disable xen pad and then register native pad driver? (of course the precondition is that we do not register stub for xen).
>> +#include <linux/kernel.h>
>> +#include <linux/types.h>
>> +#include <acpi/acpi_bus.h>
>> +#include <acpi/acpi_drivers.h>
>> +#include <asm/xen/hypercall.h>
>> +
>> +#if defined(CONFIG_ACPI_PROCESSOR_AGGREGATOR) || \
>> + defined(CONFIG_ACPI_PROCESSOR_AGGREGATOR_MODULE)
>
> I don't think you need that.
>
OK.
>> +
>> +static int __init xen_acpi_pad_init(void)
>> +{
>> + /* Only DOM0 is responsible for Xen acpi pad */
>> + if (xen_initial_domain())
>> + return acpi_bus_register_driver(&xen_acpi_pad_driver); +
>> + return -ENODEV;
>> +}
>> +subsys_initcall(xen_acpi_pad_init);
>
>
> No way of making this a module? It would be nice - perhaps have a stub
> function that registers the bus but does not do anything until this
> proper module is loaded?
>
It's doable but not preferred, reason as above.
>> +
>> +#endif
>> diff --git a/include/xen/interface/platform.h
>> b/include/xen/interface/platform.h index 4755b5f..0f44376 100644 ---
>> a/include/xen/interface/platform.h +++
>> b/include/xen/interface/platform.h @@ -324,6 +324,22 @@ struct
>> xenpf_cpu_ol { };
>> DEFINE_GUEST_HANDLE_STRUCT(xenpf_cpu_ol);
>>
>> +/*
>> + * CMD 58 and 59 are reserved for cpu hotadd and memory hotadd,
>> + * which already occupied at Xen hypervisor side.
> ^- are
>
Sure.
Thanks,
Jinsong
On Mon, Oct 29, 2012 at 03:38:14AM +0000, Liu, Jinsong wrote:
> It's doable to register a stub for xen. However, it's not preferred, say, to make xen pad as module, considering the case 'rmmod xen_acpi_pad' then 'insmod acpi_pad'? Under such case there is risk of mwait #UD, if we want to remove mwait check as 'patch 2/2: Revert-pad-config-check-in-xen_check_mwait.patch' did, advantages of which is to enjoy deep Cx.
You are missing my point. This is what I was thinking off:
>From 5f30320b8a1c21d60a2c22e98bcf013b7773938b Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <[email protected]>
Date: Mon, 29 Oct 2012 08:38:22 -0400
Subject: [PATCH] xen/acpi: Provide a stub function to register for ACPI PAD
module.
TODO: Give more details.
Signed-off-by: Konrad Rzeszutek Wilk <[email protected]>
---
drivers/xen/Kconfig | 5 ++
drivers/xen/Makefile | 3 +-
drivers/xen/xen-acpi-pad-stub.c | 128 +++++++++++++++++++++++++++++++++++++++
drivers/xen/xen-acpi.h | 2 +
4 files changed, 137 insertions(+), 1 deletions(-)
create mode 100644 drivers/xen/xen-acpi-pad-stub.c
create mode 100644 drivers/xen/xen-acpi.h
diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
index d4dffcd..9156a08 100644
--- a/drivers/xen/Kconfig
+++ b/drivers/xen/Kconfig
@@ -204,4 +204,9 @@ config XEN_MCE_LOG
Allow kernel fetching MCE error from Xen platform and
converting it into Linux mcelog format for mcelog tools
+config XEN_ACPI_PAD_STUB
+ bool
+ depends on XEN_DOM0 && X86_64 && ACPI
+ default n
+
endmenu
diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
index 0e863703..d1895d9 100644
--- a/drivers/xen/Makefile
+++ b/drivers/xen/Makefile
@@ -9,9 +9,10 @@ nostackp := $(call cc-option, -fno-stack-protector)
CFLAGS_features.o := $(nostackp)
dom0-$(CONFIG_PCI) += pci.o
-dom0-$(CONFIG_USB_SUPPORT) += dbgp.o
+dom0-$(CONFIG_USB_EHCI_HCD) += dbgp.o
dom0-$(CONFIG_ACPI) += acpi.o
dom0-$(CONFIG_X86) += pcpu.o
+dom0-$(CONFIG_XEN_ACPI_PAD_STUB) += xen-acpi-pad-stub.o
obj-$(CONFIG_XEN_DOM0) += $(dom0-y)
obj-$(CONFIG_BLOCK) += biomerge.o
obj-$(CONFIG_XEN_XENCOMM) += xencomm.o
diff --git a/drivers/xen/xen-acpi-pad-stub.c b/drivers/xen/xen-acpi-pad-stub.c
new file mode 100644
index 0000000..cac9a39
--- /dev/null
+++ b/drivers/xen/xen-acpi-pad-stub.c
@@ -0,0 +1,128 @@
+/*
+ * Copyright 2012 by Oracle Inc
+ * Author: Konrad Rzeszutek Wilk <[email protected]>
+ *
+ * This code borrows ideas from https://lkml.org/lkml/2011/11/30/249
+ * so many thanks go to Kevin Tian <[email protected]>
+ * and Yu Ke <[email protected]>.
+ *
+ * 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/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/xen.h>
+#include <xen/interface/platform.h>
+#include <xen/interface/version.h>
+#include <asm/xen/hypercall.h>
+
+/* TODO: Needs comments. */
+static struct acpi_device_ops *xen_acpi_pad_ops;
+static bool __read_mostly xen_acpi_pad_registered;
+static DEFINE_SPINLOCK(xen_acpi_pad_spinlock);
+
+int xen_register_acpi_pad_owner(struct acpi_device_ops *ops)
+{
+ if (xen_acpi_pad_ops)
+ return -EEXIST;
+
+ spin_lock(&xen_acpi_pad_spinlock);
+ xen_acpi_pad_ops = ops;
+ spin_unlock(&xen_acpi_pad_spinlock);
+ return 0;
+}
+int xen_unregister_acpi_pad_owner(struct acpi_device_ops *ops)
+{
+ spin_lock(&xen_acpi_pad_spinlock);
+ if (xen_acpi_pad_ops != ops) {
+ spin_unlock(&xen_acpi_pad_spinlock);
+ return -ENODEV;
+ }
+ xen_acpi_pad_ops = NULL;
+ spin_unlock(&xen_acpi_pad_spinlock);
+ return 0;
+}
+
+static int xen_acpi_pad_remove(struct acpi_device *device, int type)
+{
+ int ret = -ENOSYS;
+
+ spin_lock(&xen_acpi_pad_spinlock);
+ if (xen_acpi_pad_ops && xen_acpi_pad_ops->remove)
+ ret = xen_acpi_pad_ops->remove(device, type);
+ spin_unlock(&xen_acpi_pad_spinlock);
+ return ret;
+}
+static int xen_acpi_pad_add(struct acpi_device *device)
+{
+ int ret = -ENOSYS;
+ spin_lock(&xen_acpi_pad_spinlock);
+ if (xen_acpi_pad_ops && xen_acpi_pad_ops->add)
+ ret = xen_acpi_pad_ops->add(device);
+ spin_unlock(&xen_acpi_pad_spinlock);
+ return ret;
+}
+
+static const struct acpi_device_id pad_device_ids[] = {
+ {"ACPI000C", 0},
+ {"", 0},
+};
+MODULE_DEVICE_TABLE(acpi, pad_device_ids);
+#define ACPI_PROCESSOR_AGGREGATOR_CLASS "acpi_pad"
+
+static struct acpi_driver xen_acpi_pad_driver = {
+ .name = "processor_aggregator",
+ .class = ACPI_PROCESSOR_AGGREGATOR_CLASS,
+ .ids = pad_device_ids,
+ .ops = {
+ .add = xen_acpi_pad_add,
+ .remove = xen_acpi_pad_remove,
+ },
+};
+
+static int __init xen_acpi_pad_stub_init(void)
+{
+ int ret = -ENOSYS;
+ unsigned version;
+
+ if (!xen_initial_domain())
+ return -ENODEV;
+
+ version = HYPERVISOR_xen_version(XENVER_version, NULL);
+
+ if ((version >> 16 >= 4) && ((version & 0xffff) >= 2)) {
+ ret = acpi_bus_register_driver(&xen_acpi_pad_driver);
+ if (!ret)
+ xen_acpi_pad_registered = true;
+ }
+ return ret;
+}
+#if 0
+/* For testing purposes. */
+static void __exit xen_acpi_pad_stub_exit(void)
+{
+ if (xen_acpi_pad_registered)
+ acpi_bus_unregister_driver(&xen_acpi_pad_driver);
+ /* The driver should have unregistered first ! */
+ if (WARN_ON(xen_acpi_pad_ops))
+ xen_acpi_pad_ops = NULL;
+}
+#endif
+subsys_initcall(xen_acpi_pad_stub_init);
diff --git a/drivers/xen/xen-acpi.h b/drivers/xen/xen-acpi.h
new file mode 100644
index 0000000..a0867b2
--- /dev/null
+++ b/drivers/xen/xen-acpi.h
@@ -0,0 +1,2 @@
+int xen_register_acpi_pad_owner(struct acpi_device_ops *ops);
+int xen_unregister_acpi_pad_owner(struct acpi_device_ops *ops);
--
1.7.7.6
>
> IMHO to prevent mwait #UD, native pad should never have chance to register successfully when running under Xen. So it's better never unregister/disable xen pad.
With the registration stub I mentioned that won't be a problem.
>
> Konrad Rzeszutek Wilk wrote:
> >> --- a/drivers/xen/Makefile
> >> +++ b/drivers/xen/Makefile
> >> @@ -29,6 +29,7 @@ obj-$(CONFIG_XEN_MCE_LOG) += mcelog.o
> >> obj-$(CONFIG_XEN_PCIDEV_BACKEND) += xen-pciback/
> >> obj-$(CONFIG_XEN_PRIVCMD) += xen-privcmd.o
> >> obj-$(CONFIG_XEN_ACPI_PROCESSOR) += xen-acpi-processor.o
> >> +obj-$(CONFIG_XEN_DOM0) += xen_acpi_pad.o
> >
> > We should have a Kconfig option. In it, please mention what
> > version of hypervisor supports this functionality.
> >
>
> Kconfig option for xen pad is not preferred, considering if we disable xen pad and then register native pad driver? (of course the precondition is that we do not register stub for xen).
That should be OK with the stub.
>
> >> +#include <linux/kernel.h>
> >> +#include <linux/types.h>
> >> +#include <acpi/acpi_bus.h>
> >> +#include <acpi/acpi_drivers.h>
> >> +#include <asm/xen/hypercall.h>
> >> +
> >> +#if defined(CONFIG_ACPI_PROCESSOR_AGGREGATOR) || \
> >> + defined(CONFIG_ACPI_PROCESSOR_AGGREGATOR_MODULE)
> >
> > I don't think you need that.
> >
>
> OK.
>
> >> +
> >> +static int __init xen_acpi_pad_init(void)
> >> +{
> >> + /* Only DOM0 is responsible for Xen acpi pad */
> >> + if (xen_initial_domain())
> >> + return acpi_bus_register_driver(&xen_acpi_pad_driver); +
> >> + return -ENODEV;
> >> +}
> >> +subsys_initcall(xen_acpi_pad_init);
> >
> >
> > No way of making this a module? It would be nice - perhaps have a stub
> > function that registers the bus but does not do anything until this
> > proper module is loaded?
> >
>
> It's doable but not preferred, reason as above.
>
> >> +
> >> +#endif
> >> diff --git a/include/xen/interface/platform.h
> >> b/include/xen/interface/platform.h index 4755b5f..0f44376 100644 ---
> >> a/include/xen/interface/platform.h +++
> >> b/include/xen/interface/platform.h @@ -324,6 +324,22 @@ struct
> >> xenpf_cpu_ol { };
> >> DEFINE_GUEST_HANDLE_STRUCT(xenpf_cpu_ol);
> >>
> >> +/*
> >> + * CMD 58 and 59 are reserved for cpu hotadd and memory hotadd,
> >> + * which already occupied at Xen hypervisor side.
> > ^- are
> >
>
> Sure.
>
> Thanks,
> Jinsong
Konrad Rzeszutek Wilk wrote:
> On Mon, Oct 29, 2012 at 03:38:14AM +0000, Liu, Jinsong wrote:
>> It's doable to register a stub for xen. However, it's not preferred,
>> say, to make xen pad as module, considering the case 'rmmod
>> xen_acpi_pad' then 'insmod acpi_pad'? Under such case there is risk
>> of mwait #UD, if we want to remove mwait check as 'patch 2/2:
>> Revert-pad-config-check-in-xen_check_mwait.patch' did, advantages of
>> which is to enjoy deep Cx.
>
> You are missing my point. This is what I was thinking off:
>
>
> From 5f30320b8a1c21d60a2c22e98bcf013b7773938b Mon Sep 17 00:00:00 2001
> From: Konrad Rzeszutek Wilk <[email protected]>
> Date: Mon, 29 Oct 2012 08:38:22 -0400
> Subject: [PATCH] xen/acpi: Provide a stub function to register for
> ACPI PAD module.
>
> TODO: Give more details.
>
> Signed-off-by: Konrad Rzeszutek Wilk <[email protected]>
> ---
> drivers/xen/Kconfig | 5 ++
> drivers/xen/Makefile | 3 +-
> drivers/xen/xen-acpi-pad-stub.c | 128
> +++++++++++++++++++++++++++++++++++++++ drivers/xen/xen-acpi.h
> | 2 + 4 files changed, 137 insertions(+), 1 deletions(-)
> create mode 100644 drivers/xen/xen-acpi-pad-stub.c
> create mode 100644 drivers/xen/xen-acpi.h
>
> diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
> index d4dffcd..9156a08 100644
> --- a/drivers/xen/Kconfig
> +++ b/drivers/xen/Kconfig
> @@ -204,4 +204,9 @@ config XEN_MCE_LOG
> Allow kernel fetching MCE error from Xen platform and
> converting it into Linux mcelog format for mcelog tools
>
> +config XEN_ACPI_PAD_STUB
> + bool
> + depends on XEN_DOM0 && X86_64 && ACPI
> + default n
> +
This Kconfig is pointless, if CONFIG_XEN_ACPI_PAD_STUB = n, native pad would successfully registerred, and then mwait #UD (we would revert df88b2d96e36d9a9e325bfcd12eb45671cbbc937, right?). So xen stub logic should unconditionally built-in kernel.
> endmenu
> diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
> index 0e863703..d1895d9 100644
> --- a/drivers/xen/Makefile
> +++ b/drivers/xen/Makefile
> @@ -9,9 +9,10 @@ nostackp := $(call cc-option, -fno-stack-protector)
> CFLAGS_features.o := $(nostackp)
>
> dom0-$(CONFIG_PCI) += pci.o
> -dom0-$(CONFIG_USB_SUPPORT) += dbgp.o
> +dom0-$(CONFIG_USB_EHCI_HCD) += dbgp.o
> dom0-$(CONFIG_ACPI) += acpi.o
> dom0-$(CONFIG_X86) += pcpu.o
> +dom0-$(CONFIG_XEN_ACPI_PAD_STUB) += xen-acpi-pad-stub.o
> obj-$(CONFIG_XEN_DOM0) += $(dom0-y)
> obj-$(CONFIG_BLOCK) += biomerge.o
> obj-$(CONFIG_XEN_XENCOMM) += xencomm.o
> diff --git a/drivers/xen/xen-acpi-pad-stub.c
> b/drivers/xen/xen-acpi-pad-stub.c new file mode 100644
> index 0000000..cac9a39
> --- /dev/null
> +++ b/drivers/xen/xen-acpi-pad-stub.c
> @@ -0,0 +1,128 @@
> +/*
> + * Copyright 2012 by Oracle Inc
> + * Author: Konrad Rzeszutek Wilk <[email protected]>
> + *
> + * This code borrows ideas from https://lkml.org/lkml/2011/11/30/249
> + * so many thanks go to Kevin Tian <[email protected]>
> + * and Yu Ke <[email protected]>.
> + *
> + * 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/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/xen.h>
> +#include <xen/interface/platform.h>
> +#include <xen/interface/version.h>
> +#include <asm/xen/hypercall.h>
> +
> +/* TODO: Needs comments. */
> +static struct acpi_device_ops *xen_acpi_pad_ops;
> +static bool __read_mostly xen_acpi_pad_registered;
> +static DEFINE_SPINLOCK(xen_acpi_pad_spinlock);
> +
> +int xen_register_acpi_pad_owner(struct acpi_device_ops *ops)
> +{
> + if (xen_acpi_pad_ops)
> + return -EEXIST;
> +
> + spin_lock(&xen_acpi_pad_spinlock);
> + xen_acpi_pad_ops = ops;
> + spin_unlock(&xen_acpi_pad_spinlock);
> + return 0;
> +}
> +int xen_unregister_acpi_pad_owner(struct acpi_device_ops *ops)
> +{
> + spin_lock(&xen_acpi_pad_spinlock);
> + if (xen_acpi_pad_ops != ops) {
> + spin_unlock(&xen_acpi_pad_spinlock);
> + return -ENODEV;
> + }
> + xen_acpi_pad_ops = NULL;
> + spin_unlock(&xen_acpi_pad_spinlock);
> + return 0;
> +}
> +
> +static int xen_acpi_pad_remove(struct acpi_device *device, int type)
> +{
> + int ret = -ENOSYS;
> +
> + spin_lock(&xen_acpi_pad_spinlock);
> + if (xen_acpi_pad_ops && xen_acpi_pad_ops->remove)
> + ret = xen_acpi_pad_ops->remove(device, type);
> + spin_unlock(&xen_acpi_pad_spinlock);
> + return ret;
> +}
> +static int xen_acpi_pad_add(struct acpi_device *device)
> +{
> + int ret = -ENOSYS;
> + spin_lock(&xen_acpi_pad_spinlock);
> + if (xen_acpi_pad_ops && xen_acpi_pad_ops->add)
> + ret = xen_acpi_pad_ops->add(device);
> + spin_unlock(&xen_acpi_pad_spinlock);
> + return ret;
> +}
> +
> +static const struct acpi_device_id pad_device_ids[] = {
> + {"ACPI000C", 0},
> + {"", 0},
> +};
> +MODULE_DEVICE_TABLE(acpi, pad_device_ids);
> +#define ACPI_PROCESSOR_AGGREGATOR_CLASS "acpi_pad"
> +
> +static struct acpi_driver xen_acpi_pad_driver = {
> + .name = "processor_aggregator",
> + .class = ACPI_PROCESSOR_AGGREGATOR_CLASS,
> + .ids = pad_device_ids,
> + .ops = {
> + .add = xen_acpi_pad_add,
> + .remove = xen_acpi_pad_remove,
> + },
> +};
> +
> +static int __init xen_acpi_pad_stub_init(void)
> +{
> + int ret = -ENOSYS;
> + unsigned version;
> +
> + if (!xen_initial_domain())
> + return -ENODEV;
> +
> + version = HYPERVISOR_xen_version(XENVER_version, NULL);
> +
> + if ((version >> 16 >= 4) && ((version & 0xffff) >= 2)) {
> + ret = acpi_bus_register_driver(&xen_acpi_pad_driver);
> + if (!ret)
> + xen_acpi_pad_registered = true;
> + }
> + return ret;
> +}
> +#if 0
> +/* For testing purposes. */
> +static void __exit xen_acpi_pad_stub_exit(void)
> +{
> + if (xen_acpi_pad_registered)
> + acpi_bus_unregister_driver(&xen_acpi_pad_driver);
> + /* The driver should have unregistered first ! */
> + if (WARN_ON(xen_acpi_pad_ops))
> + xen_acpi_pad_ops = NULL;
> +}
> +#endif
> +subsys_initcall(xen_acpi_pad_stub_init);
I'm still confused. In this way there are xen-acpi-pad-stub.c and xen-acpi-pad.c, and you want to let xen-acpi-pad loaded as module, right? how can xen-acpi-pad logic work when it was insmoded?
Thanks,
Jinsong-
> > +config XEN_ACPI_PAD_STUB
> > + bool
> > + depends on XEN_DOM0 && X86_64 && ACPI
> > + default n
> > +
>
> This Kconfig is pointless, if CONFIG_XEN_ACPI_PAD_STUB = n, native pad would successfully registerred, and then mwait #UD (we would revert df88b2d96e36d9a9e325bfcd12eb45671cbbc937, right?). So xen stub logic should unconditionally built-in kernel.
Potentially. Keep in mind that there is no need to built this if the
kernel is not built with ACPI.
.. snip.
> > +subsys_initcall(xen_acpi_pad_stub_init);
>
> I'm still confused. In this way there are xen-acpi-pad-stub.c and xen-acpi-pad.c, and you want to let xen-acpi-pad loaded as module, right? how can xen-acpi-pad logic work when it was insmoded?
Via the register/unregister calls that this provides? Or does ACPI bus
drivers get immediately called once the call acpi_bus_register_driver?
Or can one 'poke' the 'add' and 'remove' calls so that once the "true"
PAD driver is loaded it will restart the ops->add call?
Konrad Rzeszutek Wilk wrote:
>>> +config XEN_ACPI_PAD_STUB
>>> + bool
>>> + depends on XEN_DOM0 && X86_64 && ACPI
>>> + default n
>>> +
>>
>> This Kconfig is pointless, if CONFIG_XEN_ACPI_PAD_STUB = n, native
>> pad would successfully registerred, and then mwait #UD (we would
>> revert df88b2d96e36d9a9e325bfcd12eb45671cbbc937, right?). So xen
>> stub logic should unconditionally built-in kernel.
>
>
> Potentially. Keep in mind that there is no need to built this if the
> kernel is not built with ACPI.
Sure, 'obj-$(CONFIG_XEN_DOM0) +=' is enough.
(XEN_DOM0 depends on ACPI).
>>> +subsys_initcall(xen_acpi_pad_stub_init);
>>
>> I'm still confused. In this way there are xen-acpi-pad-stub.c and
>> xen-acpi-pad.c, and you want to let xen-acpi-pad loaded as module,
>> right? how can xen-acpi-pad logic work when it was insmoded?
>
> Via the register/unregister calls that this provides? Or does ACPI bus
> drivers get immediately called once the call acpi_bus_register_driver?
But when xen stub driver registerred, real xen pad ops has not been hooked to stub ops.
>
> Or can one 'poke' the 'add' and 'remove' calls so that once the "true"
> PAD driver is loaded it will restart the ops->add call?
I think we'd better not to use xen pad stub approach. Technically it's complicated, say, how to match xen_acpi_pad driver w/ pad device? when and how to invoke .add method? how to avoid native pad loading risk? etc. I didn't find its obivous advantages, so how about keep simpler approach?
Thanks,
Jinsong
On Tue, Oct 30, 2012 at 03:18:59PM +0000, Liu, Jinsong wrote:
> Konrad Rzeszutek Wilk wrote:
> >>> +config XEN_ACPI_PAD_STUB
> >>> + bool
> >>> + depends on XEN_DOM0 && X86_64 && ACPI
> >>> + default n
> >>> +
> >>
> >> This Kconfig is pointless, if CONFIG_XEN_ACPI_PAD_STUB = n, native
> >> pad would successfully registerred, and then mwait #UD (we would
> >> revert df88b2d96e36d9a9e325bfcd12eb45671cbbc937, right?). So xen
> >> stub logic should unconditionally built-in kernel.
> >
> >
> > Potentially. Keep in mind that there is no need to built this if the
> > kernel is not built with ACPI.
>
> Sure, 'obj-$(CONFIG_XEN_DOM0) +=' is enough.
> (XEN_DOM0 depends on ACPI).
>
> >>> +subsys_initcall(xen_acpi_pad_stub_init);
> >>
> >> I'm still confused. In this way there are xen-acpi-pad-stub.c and
> >> xen-acpi-pad.c, and you want to let xen-acpi-pad loaded as module,
> >> right? how can xen-acpi-pad logic work when it was insmoded?
> >
> > Via the register/unregister calls that this provides? Or does ACPI bus
> > drivers get immediately called once the call acpi_bus_register_driver?
>
> But when xen stub driver registerred, real xen pad ops has not been hooked to stub ops.
>
> >
> > Or can one 'poke' the 'add' and 'remove' calls so that once the "true"
> > PAD driver is loaded it will restart the ops->add call?
>
> I think we'd better not to use xen pad stub approach. Technically it's complicated, say, how to match xen_acpi_pad driver w/ pad device? when and how to invoke .add method? how to avoid native pad loading risk? etc. I didn't find its obivous advantages, so how about keep simpler approach?
OK. Lets go with that one for right now. The one thing I don't like about it is
that it is built-in. It would be so much better if it was a module, but I am just
not sure how to make that work with the acpi_pad. Unless the mwait CPUID capability
is not exported (so drop your patch 1 that reverts a commit).
Perhaps there is a way to make it a module/built-in with some Kconfig
magic options? Say if ACPI_PROCESSOR_AGGREGATOR is not set, then we can
make it a module. But if ACPI_PROCESSOR_AGGREGATOR=m|y then we do it
as built-in?
Konrad Rzeszutek Wilk wrote:
> On Tue, Oct 30, 2012 at 03:18:59PM +0000, Liu, Jinsong wrote:
>> Konrad Rzeszutek Wilk wrote:
>>>>> +config XEN_ACPI_PAD_STUB
>>>>> + bool
>>>>> + depends on XEN_DOM0 && X86_64 && ACPI
>>>>> + default n
>>>>> +
>>>>
>>>> This Kconfig is pointless, if CONFIG_XEN_ACPI_PAD_STUB = n, native
>>>> pad would successfully registerred, and then mwait #UD (we would
>>>> revert df88b2d96e36d9a9e325bfcd12eb45671cbbc937, right?). So xen
>>>> stub logic should unconditionally built-in kernel.
>>>
>>>
>>> Potentially. Keep in mind that there is no need to built this if the
>>> kernel is not built with ACPI.
>>
>> Sure, 'obj-$(CONFIG_XEN_DOM0) +=' is enough.
>> (XEN_DOM0 depends on ACPI).
>>
>>>>> +subsys_initcall(xen_acpi_pad_stub_init);
>>>>
>>>> I'm still confused. In this way there are xen-acpi-pad-stub.c and
>>>> xen-acpi-pad.c, and you want to let xen-acpi-pad loaded as module,
>>>> right? how can xen-acpi-pad logic work when it was insmoded?
>>>
>>> Via the register/unregister calls that this provides? Or does ACPI
>>> bus drivers get immediately called once the call
>>> acpi_bus_register_driver?
>>
>> But when xen stub driver registerred, real xen pad ops has not been
>> hooked to stub ops.
>>
>>>
>>> Or can one 'poke' the 'add' and 'remove' calls so that once the
>>> "true" PAD driver is loaded it will restart the ops->add call?
>>
>> I think we'd better not to use xen pad stub approach. Technically
>> it's complicated, say, how to match xen_acpi_pad driver w/ pad
>> device? when and how to invoke .add method? how to avoid native pad
>> loading risk? etc. I didn't find its obivous advantages, so how
>> about keep simpler approach?
>
> OK. Lets go with that one for right now. The one thing I don't like
> about it is that it is built-in. It would be so much better if it was
> a module, but I am just not sure how to make that work with the
> acpi_pad. Unless the mwait CPUID capability is not exported (so drop
> your patch 1 that reverts a commit).
Yes, but considering mwait case and its benefit to hypervisor, xen pad built-in kernel seems to be the price we have to pay. At least currently I didn't find better way.
>
> Perhaps there is a way to make it a module/built-in with some Kconfig
> magic options? Say if ACPI_PROCESSOR_AGGREGATOR is not set, then we
> can
> make it a module. But if ACPI_PROCESSOR_AGGREGATOR=m|y then we do it
> as built-in?
Hmm, seems it's hard to express this logic in Kconfig, and that would make xen pad logic very complicated :)
Thanks,
Jinsong
Thanks! updated as attached.
Jinsong
=====================
>From f514b97628945cfac00efb0d456f133d44754c9d Mon Sep 17 00:00:00 2001
From: Liu, Jinsong <[email protected]>
Date: Thu, 1 Nov 2012 21:02:36 +0800
Subject: [PATCH 1/2] Xen acpi pad implement
PAD is acpi Processor Aggregator Device which provides a control point
that enables the platform to perform specific processor configuration
and control that applies to all processors in the platform.
This patch is to implement Xen acpi pad logic. When running under Xen
virt platform, native pad driver would not work. Instead Xen pad driver,
a self-contained and very thin logic level, would take over acpi pad staff.
When acpi pad notify OSPM, xen pad logic intercept and parse _PUR object
and then hypercall to hyervisor for the rest work, say, core parking.
Signed-off-by: Liu, Jinsong <[email protected]>
---
drivers/xen/Makefile | 1 +
drivers/xen/xen_acpi_pad.c | 206 ++++++++++++++++++++++++++++++++++++++
include/xen/interface/platform.h | 17 +++
3 files changed, 224 insertions(+), 0 deletions(-)
create mode 100644 drivers/xen/xen_acpi_pad.c
diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
index 0e86370..a2af622 100644
--- a/drivers/xen/Makefile
+++ b/drivers/xen/Makefile
@@ -29,6 +29,7 @@ obj-$(CONFIG_XEN_MCE_LOG) += mcelog.o
obj-$(CONFIG_XEN_PCIDEV_BACKEND) += xen-pciback/
obj-$(CONFIG_XEN_PRIVCMD) += xen-privcmd.o
obj-$(CONFIG_XEN_ACPI_PROCESSOR) += xen-acpi-processor.o
+obj-$(CONFIG_XEN_DOM0) += xen_acpi_pad.o
xen-evtchn-y := evtchn.o
xen-gntdev-y := gntdev.o
xen-gntalloc-y := gntalloc.o
diff --git a/drivers/xen/xen_acpi_pad.c b/drivers/xen/xen_acpi_pad.c
new file mode 100644
index 0000000..e8c26a4
--- /dev/null
+++ b/drivers/xen/xen_acpi_pad.c
@@ -0,0 +1,206 @@
+/*
+ * xen_acpi_pad.c - Xen pad interface
+ *
+ * Copyright (c) 2012, Intel Corporation.
+ * Author: Liu, Jinsong <[email protected]>
+ *
+ * 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/kernel.h>
+#include <linux/types.h>
+#include <acpi/acpi_bus.h>
+#include <acpi/acpi_drivers.h>
+#include <asm/xen/hypercall.h>
+#include <xen/interface/version.h>
+
+#define ACPI_PROCESSOR_AGGREGATOR_CLASS "acpi_pad"
+#define ACPI_PROCESSOR_AGGREGATOR_DEVICE_NAME "Processor Aggregator"
+#define ACPI_PROCESSOR_AGGREGATOR_NOTIFY 0x80
+
+static DEFINE_MUTEX(xen_pad_lock);
+
+static int xen_pad_set_idle_cpus(int num_cpus)
+{
+ struct xen_platform_op op;
+
+ if (num_cpus < 0)
+ return -EINVAL;
+
+ /* set cpu nums expected to be idled */
+ op.cmd = XENPF_core_parking;
+ op.u.core_parking.type = XEN_CORE_PARKING_SET;
+ op.u.core_parking.idle_nums = num_cpus;
+
+ return HYPERVISOR_dom0_op(&op);
+}
+
+/*
+ * Cannot get idle cpus by using hypercall once (shared with _SET)
+ * because of the characteristic of Xen continue_hypercall_on_cpu
+ */
+static int xen_pad_get_idle_cpus(void)
+{
+ int ret;
+ struct xen_platform_op op;
+
+ /* get cpu nums actually be idled */
+ op.cmd = XENPF_core_parking;
+ op.u.core_parking.type = XEN_CORE_PARKING_GET;
+ ret = HYPERVISOR_dom0_op(&op);
+ if (ret < 0)
+ return ret;
+
+ return op.u.core_parking.idle_nums;
+}
+
+/*
+ * Query firmware how many CPUs should be idle
+ * return -1 on failure
+ */
+static int xen_acpi_pad_pur(acpi_handle handle)
+{
+ struct acpi_buffer buffer = {ACPI_ALLOCATE_BUFFER, NULL};
+ union acpi_object *package;
+ int num = -1;
+
+ if (ACPI_FAILURE(acpi_evaluate_object(handle, "_PUR", NULL, &buffer)))
+ return num;
+
+ if (!buffer.length || !buffer.pointer)
+ return num;
+
+ package = buffer.pointer;
+
+ if (package->type == ACPI_TYPE_PACKAGE &&
+ package->package.count == 2 &&
+ package->package.elements[0].integer.value == 1) /* rev 1 */
+
+ num = package->package.elements[1].integer.value;
+
+ kfree(buffer.pointer);
+ return num;
+}
+
+/* Notify firmware how many CPUs are idle */
+static void xen_acpi_pad_ost(acpi_handle handle, int stat,
+ uint32_t idle_cpus)
+{
+ union acpi_object params[3] = {
+ {.type = ACPI_TYPE_INTEGER,},
+ {.type = ACPI_TYPE_INTEGER,},
+ {.type = ACPI_TYPE_BUFFER,},
+ };
+ struct acpi_object_list arg_list = {3, params};
+
+ params[0].integer.value = ACPI_PROCESSOR_AGGREGATOR_NOTIFY;
+ params[1].integer.value = stat;
+ params[2].buffer.length = 4;
+ params[2].buffer.pointer = (void *)&idle_cpus;
+ acpi_evaluate_object(handle, "_OST", &arg_list, NULL);
+}
+
+static void xen_acpi_pad_handle_notify(acpi_handle handle)
+{
+ int num_cpus;
+
+ num_cpus = xen_acpi_pad_pur(handle);
+ if (num_cpus < 0)
+ return;
+
+ mutex_lock(&xen_pad_lock);
+ if (xen_pad_set_idle_cpus(num_cpus)) {
+ mutex_unlock(&xen_pad_lock);
+ return;
+ }
+
+ num_cpus = xen_pad_get_idle_cpus();
+ if (num_cpus < 0) {
+ mutex_unlock(&xen_pad_lock);
+ return;
+ }
+ mutex_unlock(&xen_pad_lock);
+
+ xen_acpi_pad_ost(handle, 0, num_cpus);
+}
+
+static void xen_acpi_pad_notify(acpi_handle handle, u32 event,
+ void *data)
+{
+ switch (event) {
+ case ACPI_PROCESSOR_AGGREGATOR_NOTIFY:
+ xen_acpi_pad_handle_notify(handle);
+ break;
+ default:
+ pr_warn("Unsupported event [0x%x]\n", event);
+ break;
+ }
+}
+
+static int xen_acpi_pad_add(struct acpi_device *device)
+{
+ acpi_status status;
+
+ strcpy(acpi_device_name(device), ACPI_PROCESSOR_AGGREGATOR_DEVICE_NAME);
+ strcpy(acpi_device_class(device), ACPI_PROCESSOR_AGGREGATOR_CLASS);
+
+ status = acpi_install_notify_handler(device->handle,
+ ACPI_DEVICE_NOTIFY, xen_acpi_pad_notify, device);
+ if (ACPI_FAILURE(status))
+ return -ENODEV;
+
+ return 0;
+}
+
+static int xen_acpi_pad_remove(struct acpi_device *device,
+ int type)
+{
+ mutex_lock(&xen_pad_lock);
+ xen_pad_set_idle_cpus(0);
+ mutex_unlock(&xen_pad_lock);
+
+ acpi_remove_notify_handler(device->handle,
+ ACPI_DEVICE_NOTIFY, xen_acpi_pad_notify);
+ return 0;
+}
+
+static const struct acpi_device_id pad_device_ids[] = {
+ {"ACPI000C", 0},
+ {"", 0},
+};
+
+static struct acpi_driver xen_acpi_pad_driver = {
+ .name = "processor_aggregator",
+ .class = ACPI_PROCESSOR_AGGREGATOR_CLASS,
+ .ids = pad_device_ids,
+ .ops = {
+ .add = xen_acpi_pad_add,
+ .remove = xen_acpi_pad_remove,
+ },
+};
+
+static int __init xen_acpi_pad_init(void)
+{
+ int ret = -ENOSYS;
+ unsigned int version = HYPERVISOR_xen_version(XENVER_version, NULL);
+ unsigned int major = version >> 16;
+ unsigned int minor = version & 0xffff;
+
+ /* Only DOM0 is responsible for Xen acpi pad */
+ if (!xen_initial_domain())
+ return -ENODEV;
+
+ /* Only Xen4.2 or later support Xen acpi pad */
+ if (((major == 4) && (minor >= 2)) || (major > 4))
+ ret = acpi_bus_register_driver(&xen_acpi_pad_driver);
+
+ return ret;
+}
+subsys_initcall(xen_acpi_pad_init);
diff --git a/include/xen/interface/platform.h b/include/xen/interface/platform.h
index 4755b5f..a3be54c 100644
--- a/include/xen/interface/platform.h
+++ b/include/xen/interface/platform.h
@@ -324,6 +324,22 @@ struct xenpf_cpu_ol {
};
DEFINE_GUEST_HANDLE_STRUCT(xenpf_cpu_ol);
+/*
+ * CMD 58 and 59 are reserved for cpu hotadd and memory hotadd,
+ * which are already occupied at Xen hypervisor side.
+ */
+#define XENPF_core_parking 60
+struct xenpf_core_parking {
+ /* IN variables */
+#define XEN_CORE_PARKING_SET 1
+#define XEN_CORE_PARKING_GET 2
+ uint32_t type;
+ /* IN variables: set cpu nums expected to be idled */
+ /* OUT variables: get cpu nums actually be idled */
+ uint32_t idle_nums;
+};
+DEFINE_GUEST_HANDLE_STRUCT(xenpf_core_parking);
+
struct xen_platform_op {
uint32_t cmd;
uint32_t interface_version; /* XENPF_INTERFACE_VERSION */
@@ -341,6 +357,7 @@ struct xen_platform_op {
struct xenpf_set_processor_pminfo set_pminfo;
struct xenpf_pcpuinfo pcpu_info;
struct xenpf_cpu_ol cpu_ol;
+ struct xenpf_core_parking core_parking;
uint8_t pad[128];
} u;
};
--
1.7.1
On Thu, Nov 01, 2012 at 06:34:45AM +0000, Liu, Jinsong wrote:
> Thanks! updated as attached.
>
> Jinsong
>
> =====================
> >From f514b97628945cfac00efb0d456f133d44754c9d Mon Sep 17 00:00:00 2001
> From: Liu, Jinsong <[email protected]>
> Date: Thu, 1 Nov 2012 21:02:36 +0800
> Subject: [PATCH 1/2] Xen acpi pad implement
>
> PAD is acpi Processor Aggregator Device which provides a control point
> that enables the platform to perform specific processor configuration
> and control that applies to all processors in the platform.
>
> This patch is to implement Xen acpi pad logic. When running under Xen
> virt platform, native pad driver would not work. Instead Xen pad driver,
> a self-contained and very thin logic level, would take over acpi pad staff.
> When acpi pad notify OSPM, xen pad logic intercept and parse _PUR object
> and then hypercall to hyervisor for the rest work, say, core parking.
Two comments:
- Did you look at the SuSE tree? Jan mentioned that they did some
fixes? Did you carry them over?
- The init function should not make hypercalls before checking if it
in facts run under Xen.
>
> Signed-off-by: Liu, Jinsong <[email protected]>
> ---
> drivers/xen/Makefile | 1 +
> drivers/xen/xen_acpi_pad.c | 206 ++++++++++++++++++++++++++++++++++++++
> include/xen/interface/platform.h | 17 +++
> 3 files changed, 224 insertions(+), 0 deletions(-)
> create mode 100644 drivers/xen/xen_acpi_pad.c
>
> diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
> index 0e86370..a2af622 100644
> --- a/drivers/xen/Makefile
> +++ b/drivers/xen/Makefile
> @@ -29,6 +29,7 @@ obj-$(CONFIG_XEN_MCE_LOG) += mcelog.o
> obj-$(CONFIG_XEN_PCIDEV_BACKEND) += xen-pciback/
> obj-$(CONFIG_XEN_PRIVCMD) += xen-privcmd.o
> obj-$(CONFIG_XEN_ACPI_PROCESSOR) += xen-acpi-processor.o
> +obj-$(CONFIG_XEN_DOM0) += xen_acpi_pad.o
> xen-evtchn-y := evtchn.o
> xen-gntdev-y := gntdev.o
> xen-gntalloc-y := gntalloc.o
> diff --git a/drivers/xen/xen_acpi_pad.c b/drivers/xen/xen_acpi_pad.c
> new file mode 100644
> index 0000000..e8c26a4
> --- /dev/null
> +++ b/drivers/xen/xen_acpi_pad.c
> @@ -0,0 +1,206 @@
> +/*
> + * xen_acpi_pad.c - Xen pad interface
> + *
> + * Copyright (c) 2012, Intel Corporation.
> + * Author: Liu, Jinsong <[email protected]>
> + *
> + * 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/kernel.h>
> +#include <linux/types.h>
> +#include <acpi/acpi_bus.h>
> +#include <acpi/acpi_drivers.h>
> +#include <asm/xen/hypercall.h>
> +#include <xen/interface/version.h>
> +
> +#define ACPI_PROCESSOR_AGGREGATOR_CLASS "acpi_pad"
> +#define ACPI_PROCESSOR_AGGREGATOR_DEVICE_NAME "Processor Aggregator"
> +#define ACPI_PROCESSOR_AGGREGATOR_NOTIFY 0x80
> +
> +static DEFINE_MUTEX(xen_pad_lock);
> +
> +static int xen_pad_set_idle_cpus(int num_cpus)
> +{
> + struct xen_platform_op op;
> +
> + if (num_cpus < 0)
> + return -EINVAL;
> +
> + /* set cpu nums expected to be idled */
> + op.cmd = XENPF_core_parking;
> + op.u.core_parking.type = XEN_CORE_PARKING_SET;
> + op.u.core_parking.idle_nums = num_cpus;
> +
> + return HYPERVISOR_dom0_op(&op);
> +}
> +
> +/*
> + * Cannot get idle cpus by using hypercall once (shared with _SET)
> + * because of the characteristic of Xen continue_hypercall_on_cpu
> + */
> +static int xen_pad_get_idle_cpus(void)
> +{
> + int ret;
> + struct xen_platform_op op;
> +
> + /* get cpu nums actually be idled */
> + op.cmd = XENPF_core_parking;
> + op.u.core_parking.type = XEN_CORE_PARKING_GET;
> + ret = HYPERVISOR_dom0_op(&op);
> + if (ret < 0)
> + return ret;
> +
> + return op.u.core_parking.idle_nums;
> +}
> +
> +/*
> + * Query firmware how many CPUs should be idle
> + * return -1 on failure
> + */
> +static int xen_acpi_pad_pur(acpi_handle handle)
> +{
> + struct acpi_buffer buffer = {ACPI_ALLOCATE_BUFFER, NULL};
> + union acpi_object *package;
> + int num = -1;
> +
> + if (ACPI_FAILURE(acpi_evaluate_object(handle, "_PUR", NULL, &buffer)))
> + return num;
> +
> + if (!buffer.length || !buffer.pointer)
> + return num;
> +
> + package = buffer.pointer;
> +
> + if (package->type == ACPI_TYPE_PACKAGE &&
> + package->package.count == 2 &&
> + package->package.elements[0].integer.value == 1) /* rev 1 */
> +
> + num = package->package.elements[1].integer.value;
> +
> + kfree(buffer.pointer);
> + return num;
> +}
> +
> +/* Notify firmware how many CPUs are idle */
> +static void xen_acpi_pad_ost(acpi_handle handle, int stat,
> + uint32_t idle_cpus)
> +{
> + union acpi_object params[3] = {
> + {.type = ACPI_TYPE_INTEGER,},
> + {.type = ACPI_TYPE_INTEGER,},
> + {.type = ACPI_TYPE_BUFFER,},
> + };
> + struct acpi_object_list arg_list = {3, params};
> +
> + params[0].integer.value = ACPI_PROCESSOR_AGGREGATOR_NOTIFY;
> + params[1].integer.value = stat;
> + params[2].buffer.length = 4;
> + params[2].buffer.pointer = (void *)&idle_cpus;
> + acpi_evaluate_object(handle, "_OST", &arg_list, NULL);
> +}
> +
> +static void xen_acpi_pad_handle_notify(acpi_handle handle)
> +{
> + int num_cpus;
> +
> + num_cpus = xen_acpi_pad_pur(handle);
> + if (num_cpus < 0)
> + return;
> +
> + mutex_lock(&xen_pad_lock);
> + if (xen_pad_set_idle_cpus(num_cpus)) {
> + mutex_unlock(&xen_pad_lock);
> + return;
> + }
> +
> + num_cpus = xen_pad_get_idle_cpus();
> + if (num_cpus < 0) {
> + mutex_unlock(&xen_pad_lock);
> + return;
> + }
> + mutex_unlock(&xen_pad_lock);
> +
> + xen_acpi_pad_ost(handle, 0, num_cpus);
> +}
> +
> +static void xen_acpi_pad_notify(acpi_handle handle, u32 event,
> + void *data)
> +{
> + switch (event) {
> + case ACPI_PROCESSOR_AGGREGATOR_NOTIFY:
> + xen_acpi_pad_handle_notify(handle);
> + break;
> + default:
> + pr_warn("Unsupported event [0x%x]\n", event);
> + break;
> + }
> +}
> +
> +static int xen_acpi_pad_add(struct acpi_device *device)
> +{
> + acpi_status status;
> +
> + strcpy(acpi_device_name(device), ACPI_PROCESSOR_AGGREGATOR_DEVICE_NAME);
> + strcpy(acpi_device_class(device), ACPI_PROCESSOR_AGGREGATOR_CLASS);
> +
> + status = acpi_install_notify_handler(device->handle,
> + ACPI_DEVICE_NOTIFY, xen_acpi_pad_notify, device);
> + if (ACPI_FAILURE(status))
> + return -ENODEV;
> +
> + return 0;
> +}
> +
> +static int xen_acpi_pad_remove(struct acpi_device *device,
> + int type)
> +{
> + mutex_lock(&xen_pad_lock);
> + xen_pad_set_idle_cpus(0);
> + mutex_unlock(&xen_pad_lock);
> +
> + acpi_remove_notify_handler(device->handle,
> + ACPI_DEVICE_NOTIFY, xen_acpi_pad_notify);
> + return 0;
> +}
> +
> +static const struct acpi_device_id pad_device_ids[] = {
> + {"ACPI000C", 0},
> + {"", 0},
> +};
> +
> +static struct acpi_driver xen_acpi_pad_driver = {
> + .name = "processor_aggregator",
> + .class = ACPI_PROCESSOR_AGGREGATOR_CLASS,
> + .ids = pad_device_ids,
> + .ops = {
> + .add = xen_acpi_pad_add,
> + .remove = xen_acpi_pad_remove,
> + },
> +};
> +
> +static int __init xen_acpi_pad_init(void)
> +{
> + int ret = -ENOSYS;
> + unsigned int version = HYPERVISOR_xen_version(XENVER_version, NULL);
Heh. so if you run this on baremetal I wonder what will happen.
> + unsigned int major = version >> 16;
> + unsigned int minor = version & 0xffff;
> +
> + /* Only DOM0 is responsible for Xen acpi pad */
> + if (!xen_initial_domain())
> + return -ENODEV;
> +
I think the check for version should happen here.
> + /* Only Xen4.2 or later support Xen acpi pad */
> + if (((major == 4) && (minor >= 2)) || (major > 4))
> + ret = acpi_bus_register_driver(&xen_acpi_pad_driver);
> +
> + return ret;
> +}
> +subsys_initcall(xen_acpi_pad_init);
> diff --git a/include/xen/interface/platform.h b/include/xen/interface/platform.h
> index 4755b5f..a3be54c 100644
> --- a/include/xen/interface/platform.h
> +++ b/include/xen/interface/platform.h
> @@ -324,6 +324,22 @@ struct xenpf_cpu_ol {
> };
> DEFINE_GUEST_HANDLE_STRUCT(xenpf_cpu_ol);
>
> +/*
> + * CMD 58 and 59 are reserved for cpu hotadd and memory hotadd,
> + * which are already occupied at Xen hypervisor side.
> + */
> +#define XENPF_core_parking 60
> +struct xenpf_core_parking {
> + /* IN variables */
> +#define XEN_CORE_PARKING_SET 1
> +#define XEN_CORE_PARKING_GET 2
> + uint32_t type;
> + /* IN variables: set cpu nums expected to be idled */
> + /* OUT variables: get cpu nums actually be idled */
> + uint32_t idle_nums;
> +};
> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_core_parking);
> +
> struct xen_platform_op {
> uint32_t cmd;
> uint32_t interface_version; /* XENPF_INTERFACE_VERSION */
> @@ -341,6 +357,7 @@ struct xen_platform_op {
> struct xenpf_set_processor_pminfo set_pminfo;
> struct xenpf_pcpuinfo pcpu_info;
> struct xenpf_cpu_ol cpu_ol;
> + struct xenpf_core_parking core_parking;
> uint8_t pad[128];
> } u;
> };
> --
> 1.7.1
> _______________________________________________
> Xen-devel mailing list
> [email protected]
> http://lists.xen.org/xen-devel
Konrad Rzeszutek Wilk wrote:
>>> From f514b97628945cfac00efb0d456f133d44754c9d Mon Sep 17 00:00:00
>>> 2001
>> From: Liu, Jinsong <[email protected]>
>> Date: Thu, 1 Nov 2012 21:02:36 +0800
>> Subject: [PATCH 1/2] Xen acpi pad implement
>>
>> PAD is acpi Processor Aggregator Device which provides a control
>> point
>> that enables the platform to perform specific processor configuration
>> and control that applies to all processors in the platform.
>>
>> This patch is to implement Xen acpi pad logic. When running under Xen
>> virt platform, native pad driver would not work. Instead Xen pad
>> driver,
>> a self-contained and very thin logic level, would take over acpi pad
>> staff. When acpi pad notify OSPM, xen pad logic intercept and parse
>> _PUR object
>> and then hypercall to hyervisor for the rest work, say, core parking.
>
> Two comments:
> - Did you look at the SuSE tree? Jan mentioned that they did some
> fixes? Did you carry them over?
I didn't look at Suse tree.
Jan, would you please tell me where can I pull Suse tree? have you implemented xen-acpi-pad logic at Suse dom0 and which points you have fixed?
> - The init function should not make hypercalls before checking if it
> in facts run under Xen.
OK.
>> +
>> +static const struct acpi_device_id pad_device_ids[] = {
>> + {"ACPI000C", 0}, + {"", 0},
>> +};
>> +
>> +static struct acpi_driver xen_acpi_pad_driver = {
>> + .name = "processor_aggregator",
>> + .class = ACPI_PROCESSOR_AGGREGATOR_CLASS,
>> + .ids = pad_device_ids,
>> + .ops = {
>> + .add = xen_acpi_pad_add,
>> + .remove = xen_acpi_pad_remove,
>> + },
>> +};
>> +
>> +static int __init xen_acpi_pad_init(void)
>> +{
>> + int ret = -ENOSYS;
>> + unsigned int version = HYPERVISOR_xen_version(XENVER_version,
>> NULL);
>
> Heh. so if you run this on baremetal I wonder what will happen.
>
>> + unsigned int major = version >> 16;
>> + unsigned int minor = version & 0xffff;
>> +
>> + /* Only DOM0 is responsible for Xen acpi pad */
>> + if (!xen_initial_domain())
>> + return -ENODEV;
>> +
>
> I think the check for version should happen here.
Got it, will update later.
Thanks,
Jinsong
>>> On 03.11.12 at 13:16, "Liu, Jinsong" <[email protected]> wrote:
> Konrad Rzeszutek Wilk wrote:
>>>> From f514b97628945cfac00efb0d456f133d44754c9d Mon Sep 17 00:00:00
>>>> 2001
>>> From: Liu, Jinsong <[email protected]>
>>> Date: Thu, 1 Nov 2012 21:02:36 +0800
>>> Subject: [PATCH 1/2] Xen acpi pad implement
>>>
>>> PAD is acpi Processor Aggregator Device which provides a control
>>> point
>>> that enables the platform to perform specific processor configuration
>>> and control that applies to all processors in the platform.
>>>
>>> This patch is to implement Xen acpi pad logic. When running under Xen
>>> virt platform, native pad driver would not work. Instead Xen pad
>>> driver,
>>> a self-contained and very thin logic level, would take over acpi pad
>>> staff. When acpi pad notify OSPM, xen pad logic intercept and parse
>>> _PUR object
>>> and then hypercall to hyervisor for the rest work, say, core parking.
>>
>> Two comments:
>> - Did you look at the SuSE tree? Jan mentioned that they did some
>> fixes? Did you carry them over?
>
> I didn't look at Suse tree.
>
> Jan, would you please tell me where can I pull Suse tree?
http://kernel.opensuse.org/git
> have you
> implemented xen-acpi-pad logic at Suse dom0 and which points you have fixed?
Yes, we've been carrying this for quite some time. I don't recall
the specific cleanups to the code I did, so I can only refer you
to the trees above.
Jan
> > diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
> > index 0e86370..a2af622 100644
> > --- a/drivers/xen/Makefile
> > +++ b/drivers/xen/Makefile
> > @@ -29,6 +29,7 @@ obj-$(CONFIG_XEN_MCE_LOG) += mcelog.o
> > obj-$(CONFIG_XEN_PCIDEV_BACKEND) += xen-pciback/
> > obj-$(CONFIG_XEN_PRIVCMD) += xen-privcmd.o
> > obj-$(CONFIG_XEN_ACPI_PROCESSOR) += xen-acpi-processor.o
> > +obj-$(CONFIG_XEN_DOM0) += xen_acpi_pad.o
> > xen-evtchn-y := evtchn.o
> > xen-gntdev-y := gntdev.o
> > xen-gntalloc-y := gntalloc.o
it should really depend on ACPI and maybe also X86, otherwise it is
going to break the ARM build
> > diff --git a/drivers/xen/xen_acpi_pad.c b/drivers/xen/xen_acpi_pad.c
> > new file mode 100644
> > index 0000000..e8c26a4
> > --- /dev/null
> > +++ b/drivers/xen/xen_acpi_pad.c
> > @@ -0,0 +1,206 @@
> > +/*
> > + * xen_acpi_pad.c - Xen pad interface
> > + *
> > + * Copyright (c) 2012, Intel Corporation.
> > + * Author: Liu, Jinsong <[email protected]>
> > + *
> > + * 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/kernel.h>
> > +#include <linux/types.h>
> > +#include <acpi/acpi_bus.h>
> > +#include <acpi/acpi_drivers.h>
> > +#include <asm/xen/hypercall.h>
> > +#include <xen/interface/version.h>
> > +
> > +#define ACPI_PROCESSOR_AGGREGATOR_CLASS "acpi_pad"
> > +#define ACPI_PROCESSOR_AGGREGATOR_DEVICE_NAME "Processor Aggregator"
> > +#define ACPI_PROCESSOR_AGGREGATOR_NOTIFY 0x80
> > +
> > +static DEFINE_MUTEX(xen_pad_lock);
> > +
> > +static int xen_pad_set_idle_cpus(int num_cpus)
> > +{
> > + struct xen_platform_op op;
> > +
> > + if (num_cpus < 0)
> > + return -EINVAL;
> > +
> > + /* set cpu nums expected to be idled */
> > + op.cmd = XENPF_core_parking;
> > + op.u.core_parking.type = XEN_CORE_PARKING_SET;
> > + op.u.core_parking.idle_nums = num_cpus;
> > +
> > + return HYPERVISOR_dom0_op(&op);
> > +}
> > +
> > +/*
> > + * Cannot get idle cpus by using hypercall once (shared with _SET)
> > + * because of the characteristic of Xen continue_hypercall_on_cpu
> > + */
> > +static int xen_pad_get_idle_cpus(void)
> > +{
> > + int ret;
> > + struct xen_platform_op op;
> > +
> > + /* get cpu nums actually be idled */
> > + op.cmd = XENPF_core_parking;
> > + op.u.core_parking.type = XEN_CORE_PARKING_GET;
> > + ret = HYPERVISOR_dom0_op(&op);
> > + if (ret < 0)
> > + return ret;
> > +
> > + return op.u.core_parking.idle_nums;
> > +}
> > +
> > +/*
> > + * Query firmware how many CPUs should be idle
> > + * return -1 on failure
> > + */
> > +static int xen_acpi_pad_pur(acpi_handle handle)
> > +{
> > + struct acpi_buffer buffer = {ACPI_ALLOCATE_BUFFER, NULL};
> > + union acpi_object *package;
> > + int num = -1;
> > +
> > + if (ACPI_FAILURE(acpi_evaluate_object(handle, "_PUR", NULL, &buffer)))
> > + return num;
> > +
> > + if (!buffer.length || !buffer.pointer)
> > + return num;
> > +
> > + package = buffer.pointer;
> > +
> > + if (package->type == ACPI_TYPE_PACKAGE &&
> > + package->package.count == 2 &&
> > + package->package.elements[0].integer.value == 1) /* rev 1 */
> > +
> > + num = package->package.elements[1].integer.value;
> > +
> > + kfree(buffer.pointer);
> > + return num;
> > +}
> > +
> > +/* Notify firmware how many CPUs are idle */
> > +static void xen_acpi_pad_ost(acpi_handle handle, int stat,
> > + uint32_t idle_cpus)
> > +{
> > + union acpi_object params[3] = {
> > + {.type = ACPI_TYPE_INTEGER,},
> > + {.type = ACPI_TYPE_INTEGER,},
> > + {.type = ACPI_TYPE_BUFFER,},
> > + };
> > + struct acpi_object_list arg_list = {3, params};
> > +
> > + params[0].integer.value = ACPI_PROCESSOR_AGGREGATOR_NOTIFY;
> > + params[1].integer.value = stat;
> > + params[2].buffer.length = 4;
> > + params[2].buffer.pointer = (void *)&idle_cpus;
> > + acpi_evaluate_object(handle, "_OST", &arg_list, NULL);
> > +}
> > +
> > +static void xen_acpi_pad_handle_notify(acpi_handle handle)
> > +{
> > + int num_cpus;
> > +
> > + num_cpus = xen_acpi_pad_pur(handle);
> > + if (num_cpus < 0)
> > + return;
> > +
> > + mutex_lock(&xen_pad_lock);
> > + if (xen_pad_set_idle_cpus(num_cpus)) {
> > + mutex_unlock(&xen_pad_lock);
> > + return;
> > + }
> > +
> > + num_cpus = xen_pad_get_idle_cpus();
> > + if (num_cpus < 0) {
> > + mutex_unlock(&xen_pad_lock);
> > + return;
> > + }
> > + mutex_unlock(&xen_pad_lock);
> > +
> > + xen_acpi_pad_ost(handle, 0, num_cpus);
> > +}
> > +
> > +static void xen_acpi_pad_notify(acpi_handle handle, u32 event,
> > + void *data)
> > +{
> > + switch (event) {
> > + case ACPI_PROCESSOR_AGGREGATOR_NOTIFY:
> > + xen_acpi_pad_handle_notify(handle);
> > + break;
> > + default:
> > + pr_warn("Unsupported event [0x%x]\n", event);
> > + break;
> > + }
> > +}
> > +
> > +static int xen_acpi_pad_add(struct acpi_device *device)
> > +{
> > + acpi_status status;
> > +
> > + strcpy(acpi_device_name(device), ACPI_PROCESSOR_AGGREGATOR_DEVICE_NAME);
> > + strcpy(acpi_device_class(device), ACPI_PROCESSOR_AGGREGATOR_CLASS);
> > +
> > + status = acpi_install_notify_handler(device->handle,
> > + ACPI_DEVICE_NOTIFY, xen_acpi_pad_notify, device);
> > + if (ACPI_FAILURE(status))
> > + return -ENODEV;
> > +
> > + return 0;
> > +}
> > +
> > +static int xen_acpi_pad_remove(struct acpi_device *device,
> > + int type)
> > +{
> > + mutex_lock(&xen_pad_lock);
> > + xen_pad_set_idle_cpus(0);
> > + mutex_unlock(&xen_pad_lock);
> > +
> > + acpi_remove_notify_handler(device->handle,
> > + ACPI_DEVICE_NOTIFY, xen_acpi_pad_notify);
> > + return 0;
> > +}
> > +
> > +static const struct acpi_device_id pad_device_ids[] = {
> > + {"ACPI000C", 0},
> > + {"", 0},
> > +};
> > +
> > +static struct acpi_driver xen_acpi_pad_driver = {
> > + .name = "processor_aggregator",
> > + .class = ACPI_PROCESSOR_AGGREGATOR_CLASS,
> > + .ids = pad_device_ids,
> > + .ops = {
> > + .add = xen_acpi_pad_add,
> > + .remove = xen_acpi_pad_remove,
> > + },
> > +};
> > +
> > +static int __init xen_acpi_pad_init(void)
> > +{
> > + int ret = -ENOSYS;
> > + unsigned int version = HYPERVISOR_xen_version(XENVER_version, NULL);
>
> Heh. so if you run this on baremetal I wonder what will happen.
>
> > + unsigned int major = version >> 16;
> > + unsigned int minor = version & 0xffff;
> > +
> > + /* Only DOM0 is responsible for Xen acpi pad */
> > + if (!xen_initial_domain())
> > + return -ENODEV;
> > +
>
> I think the check for version should happen here.
>
> > + /* Only Xen4.2 or later support Xen acpi pad */
> > + if (((major == 4) && (minor >= 2)) || (major > 4))
> > + ret = acpi_bus_register_driver(&xen_acpi_pad_driver);
> > +
> > + return ret;
> > +}
> > +subsys_initcall(xen_acpi_pad_init);
> > diff --git a/include/xen/interface/platform.h b/include/xen/interface/platform.h
> > index 4755b5f..a3be54c 100644
> > --- a/include/xen/interface/platform.h
> > +++ b/include/xen/interface/platform.h
> > @@ -324,6 +324,22 @@ struct xenpf_cpu_ol {
> > };
> > DEFINE_GUEST_HANDLE_STRUCT(xenpf_cpu_ol);
> >
> > +/*
> > + * CMD 58 and 59 are reserved for cpu hotadd and memory hotadd,
> > + * which are already occupied at Xen hypervisor side.
> > + */
> > +#define XENPF_core_parking 60
> > +struct xenpf_core_parking {
> > + /* IN variables */
> > +#define XEN_CORE_PARKING_SET 1
> > +#define XEN_CORE_PARKING_GET 2
> > + uint32_t type;
> > + /* IN variables: set cpu nums expected to be idled */
> > + /* OUT variables: get cpu nums actually be idled */
> > + uint32_t idle_nums;
> > +};
> > +DEFINE_GUEST_HANDLE_STRUCT(xenpf_core_parking);
> > +
> > struct xen_platform_op {
> > uint32_t cmd;
> > uint32_t interface_version; /* XENPF_INTERFACE_VERSION */
> > @@ -341,6 +357,7 @@ struct xen_platform_op {
> > struct xenpf_set_processor_pminfo set_pminfo;
> > struct xenpf_pcpuinfo pcpu_info;
> > struct xenpf_cpu_ol cpu_ol;
> > + struct xenpf_core_parking core_parking;
> > uint8_t pad[128];
> > } u;
> > };
> > --
> > 1.7.1
>
>
> > _______________________________________________
> > Xen-devel mailing list
> > [email protected]
> > http://lists.xen.org/xen-devel
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
Stefano Stabellini wrote:
>>> diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
>>> index 0e86370..a2af622 100644
>>> --- a/drivers/xen/Makefile
>>> +++ b/drivers/xen/Makefile
>>> @@ -29,6 +29,7 @@ obj-$(CONFIG_XEN_MCE_LOG) += mcelog.o
>>> obj-$(CONFIG_XEN_PCIDEV_BACKEND) += xen-pciback/
>>> obj-$(CONFIG_XEN_PRIVCMD) += xen-privcmd.o
>>> obj-$(CONFIG_XEN_ACPI_PROCESSOR) += xen-acpi-processor.o
>>> +obj-$(CONFIG_XEN_DOM0) += xen_acpi_pad.o
>>> xen-evtchn-y := evtchn.o
>>> xen-gntdev-y := gntdev.o
>>> xen-gntalloc-y := gntalloc.o
>
> it should really depend on ACPI and maybe also X86, otherwise it is
> going to break the ARM build
>
>
Hmm, XEN_DOM0 has already depended on ACPI and X86_LOCAL_APIC (which depends on X86_64).
Thanks,
Jinsong
>>> diff --git a/drivers/xen/xen_acpi_pad.c b/drivers/xen/xen_acpi_pad.c
>>> new file mode 100644
>>> index 0000000..e8c26a4
>>> --- /dev/null
>>> +++ b/drivers/xen/xen_acpi_pad.c
>>> @@ -0,0 +1,206 @@
>>> +/*
>>> + * xen_acpi_pad.c - Xen pad interface
>>> + *
>>> + * Copyright (c) 2012, Intel Corporation.
>>> + * Author: Liu, Jinsong <[email protected]> + *
>>> + * 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/kernel.h>
>>> +#include <linux/types.h>
>>> +#include <acpi/acpi_bus.h>
>>> +#include <acpi/acpi_drivers.h>
>>> +#include <asm/xen/hypercall.h>
>>> +#include <xen/interface/version.h>
>>> +
>>> +#define ACPI_PROCESSOR_AGGREGATOR_CLASS "acpi_pad"
>>> +#define ACPI_PROCESSOR_AGGREGATOR_DEVICE_NAME "Processor
>>> Aggregator" +#define ACPI_PROCESSOR_AGGREGATOR_NOTIFY 0x80
>>> +
>>> +static DEFINE_MUTEX(xen_pad_lock);
>>> +
>>> +static int xen_pad_set_idle_cpus(int num_cpus)
>>> +{
>>> + struct xen_platform_op op;
>>> +
>>> + if (num_cpus < 0)
>>> + return -EINVAL;
>>> +
>>> + /* set cpu nums expected to be idled */
>>> + op.cmd = XENPF_core_parking;
>>> + op.u.core_parking.type = XEN_CORE_PARKING_SET;
>>> + op.u.core_parking.idle_nums = num_cpus;
>>> +
>>> + return HYPERVISOR_dom0_op(&op);
>>> +}
>>> +
>>> +/*
>>> + * Cannot get idle cpus by using hypercall once (shared with _SET)
>>> + * because of the characteristic of Xen continue_hypercall_on_cpu
>>> + */ +static int xen_pad_get_idle_cpus(void)
>>> +{
>>> + int ret;
>>> + struct xen_platform_op op;
>>> +
>>> + /* get cpu nums actually be idled */
>>> + op.cmd = XENPF_core_parking;
>>> + op.u.core_parking.type = XEN_CORE_PARKING_GET;
>>> + ret = HYPERVISOR_dom0_op(&op);
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + return op.u.core_parking.idle_nums;
>>> +}
>>> +
>>> +/*
>>> + * Query firmware how many CPUs should be idle
>>> + * return -1 on failure
>>> + */
>>> +static int xen_acpi_pad_pur(acpi_handle handle)
>>> +{
>>> + struct acpi_buffer buffer = {ACPI_ALLOCATE_BUFFER, NULL};
>>> + union acpi_object *package;
>>> + int num = -1;
>>> +
>>> + if (ACPI_FAILURE(acpi_evaluate_object(handle, "_PUR", NULL,
>>> &buffer))) + return num; +
>>> + if (!buffer.length || !buffer.pointer)
>>> + return num;
>>> +
>>> + package = buffer.pointer;
>>> +
>>> + if (package->type == ACPI_TYPE_PACKAGE &&
>>> + package->package.count == 2 &&
>>> + package->package.elements[0].integer.value == 1) /*
>>> rev 1 */ + + num =
>>> package->package.elements[1].integer.value; + +
>>> kfree(buffer.pointer); + return num;
>>> +}
>>> +
>>> +/* Notify firmware how many CPUs are idle */
>>> +static void xen_acpi_pad_ost(acpi_handle handle, int stat, +
>>> uint32_t idle_cpus) +{
>>> + union acpi_object params[3] = {
>>> + {.type = ACPI_TYPE_INTEGER,},
>>> + {.type = ACPI_TYPE_INTEGER,},
>>> + {.type = ACPI_TYPE_BUFFER,},
>>> + };
>>> + struct acpi_object_list arg_list = {3, params}; +
>>> + params[0].integer.value = ACPI_PROCESSOR_AGGREGATOR_NOTIFY;
>>> + params[1].integer.value = stat;
>>> + params[2].buffer.length = 4;
>>> + params[2].buffer.pointer = (void *)&idle_cpus;
>>> + acpi_evaluate_object(handle, "_OST", &arg_list, NULL); +}
>>> +
>>> +static void xen_acpi_pad_handle_notify(acpi_handle handle) +{
>>> + int num_cpus;
>>> +
>>> + num_cpus = xen_acpi_pad_pur(handle);
>>> + if (num_cpus < 0)
>>> + return;
>>> +
>>> + mutex_lock(&xen_pad_lock);
>>> + if (xen_pad_set_idle_cpus(num_cpus)) {
>>> + mutex_unlock(&xen_pad_lock);
>>> + return;
>>> + }
>>> +
>>> + num_cpus = xen_pad_get_idle_cpus();
>>> + if (num_cpus < 0) {
>>> + mutex_unlock(&xen_pad_lock);
>>> + return;
>>> + }
>>> + mutex_unlock(&xen_pad_lock);
>>> +
>>> + xen_acpi_pad_ost(handle, 0, num_cpus);
>>> +}
>>> +
>>> +static void xen_acpi_pad_notify(acpi_handle handle, u32 event, +
>>> void *data) +{
>>> + switch (event) {
>>> + case ACPI_PROCESSOR_AGGREGATOR_NOTIFY:
>>> + xen_acpi_pad_handle_notify(handle);
>>> + break;
>>> + default:
>>> + pr_warn("Unsupported event [0x%x]\n", event); +
>>> break; + }
>>> +}
>>> +
>>> +static int xen_acpi_pad_add(struct acpi_device *device) +{
>>> + acpi_status status;
>>> +
>>> + strcpy(acpi_device_name(device),
>>> ACPI_PROCESSOR_AGGREGATOR_DEVICE_NAME); +
>>> strcpy(acpi_device_class(device), ACPI_PROCESSOR_AGGREGATOR_CLASS);
>>> + + status = acpi_install_notify_handler(device->handle, +
>>> ACPI_DEVICE_NOTIFY, xen_acpi_pad_notify, device); + if
>>> (ACPI_FAILURE(status)) + return -ENODEV;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int xen_acpi_pad_remove(struct acpi_device *device, +
>>> int type) +{
>>> + mutex_lock(&xen_pad_lock);
>>> + xen_pad_set_idle_cpus(0);
>>> + mutex_unlock(&xen_pad_lock);
>>> +
>>> + acpi_remove_notify_handler(device->handle,
>>> + ACPI_DEVICE_NOTIFY, xen_acpi_pad_notify); +
>>> return 0; +}
>>> +
>>> +static const struct acpi_device_id pad_device_ids[] = { +
>>> {"ACPI000C", 0}, + {"", 0},
>>> +};
>>> +
>>> +static struct acpi_driver xen_acpi_pad_driver = {
>>> + .name = "processor_aggregator",
>>> + .class = ACPI_PROCESSOR_AGGREGATOR_CLASS,
>>> + .ids = pad_device_ids,
>>> + .ops = {
>>> + .add = xen_acpi_pad_add,
>>> + .remove = xen_acpi_pad_remove,
>>> + },
>>> +};
>>> +
>>> +static int __init xen_acpi_pad_init(void)
>>> +{
>>> + int ret = -ENOSYS;
>>> + unsigned int version = HYPERVISOR_xen_version(XENVER_version,
>>> NULL);
>>
>> Heh. so if you run this on baremetal I wonder what will happen.
>>
>>> + unsigned int major = version >> 16;
>>> + unsigned int minor = version & 0xffff;
>>> +
>>> + /* Only DOM0 is responsible for Xen acpi pad */
>>> + if (!xen_initial_domain())
>>> + return -ENODEV;
>>> +
>>
>> I think the check for version should happen here.
>>
>>> + /* Only Xen4.2 or later support Xen acpi pad */
>>> + if (((major == 4) && (minor >= 2)) || (major > 4))
>>> + ret = acpi_bus_register_driver(&xen_acpi_pad_driver);
>>> + + return ret;
>>> +}
>>> +subsys_initcall(xen_acpi_pad_init);
>>> diff --git a/include/xen/interface/platform.h
>>> b/include/xen/interface/platform.h index 4755b5f..a3be54c 100644
>>> --- a/include/xen/interface/platform.h +++
>>> b/include/xen/interface/platform.h @@ -324,6 +324,22 @@ struct
>>> xenpf_cpu_ol { };
>>> DEFINE_GUEST_HANDLE_STRUCT(xenpf_cpu_ol);
>>>
>>> +/*
>>> + * CMD 58 and 59 are reserved for cpu hotadd and memory hotadd,
>>> + * which are already occupied at Xen hypervisor side. + */
>>> +#define XENPF_core_parking 60
>>> +struct xenpf_core_parking {
>>> + /* IN variables */
>>> +#define XEN_CORE_PARKING_SET 1
>>> +#define XEN_CORE_PARKING_GET 2
>>> + uint32_t type;
>>> + /* IN variables: set cpu nums expected to be idled */
>>> + /* OUT variables: get cpu nums actually be idled */ +
>>> uint32_t idle_nums; +};
>>> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_core_parking);
>>> +
>>> struct xen_platform_op {
>>> uint32_t cmd;
>>> uint32_t interface_version; /* XENPF_INTERFACE_VERSION */
>>> @@ -341,6 +357,7 @@ struct xen_platform_op {
>>> struct xenpf_set_processor_pminfo set_pminfo;
>>> struct xenpf_pcpuinfo pcpu_info;
>>> struct xenpf_cpu_ol cpu_ol;
>>> + struct xenpf_core_parking core_parking;
>>> uint8_t pad[128]; } u;
>>> };
>>> --
>>> 1.7.1
>>
>>
>>> _______________________________________________
>>> Xen-devel mailing list
>>> [email protected]
>>> http://lists.xen.org/xen-devel
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe
>> linux-kernel" in the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at http://www.tux.org/lkml/
Liu, Jinsong wrote:
> Stefano Stabellini wrote:
>>>> diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
>>>> index 0e86370..a2af622 100644
>>>> --- a/drivers/xen/Makefile
>>>> +++ b/drivers/xen/Makefile
>>>> @@ -29,6 +29,7 @@ obj-$(CONFIG_XEN_MCE_LOG) += mcelog.o
>>>> obj-$(CONFIG_XEN_PCIDEV_BACKEND) += xen-pciback/
>>>> obj-$(CONFIG_XEN_PRIVCMD) += xen-privcmd.o
>>>> obj-$(CONFIG_XEN_ACPI_PROCESSOR) += xen-acpi-processor.o
>>>> +obj-$(CONFIG_XEN_DOM0) += xen_acpi_pad.o
>>>> xen-evtchn-y := evtchn.o
>>>> xen-gntdev-y := gntdev.o
>>>> xen-gntalloc-y := gntalloc.o
>>
>> it should really depend on ACPI and maybe also X86, otherwise it is
>> going to break the ARM build
>>
>>
>
> Hmm, XEN_DOM0 has already depended on ACPI and X86_LOCAL_APIC (which
> depends on X86_64).
>
Ah, you and Konrad are right. I ignore XEN_DOM0 varies under different arch.
(But seems it not depends on X86 since it's logically an acpi stuff?).
Thanks,
Jinsong-
On Tue, 6 Nov 2012, Liu, Jinsong wrote:
> Liu, Jinsong wrote:
> > Stefano Stabellini wrote:
> >>>> diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
> >>>> index 0e86370..a2af622 100644
> >>>> --- a/drivers/xen/Makefile
> >>>> +++ b/drivers/xen/Makefile
> >>>> @@ -29,6 +29,7 @@ obj-$(CONFIG_XEN_MCE_LOG) += mcelog.o
> >>>> obj-$(CONFIG_XEN_PCIDEV_BACKEND) += xen-pciback/
> >>>> obj-$(CONFIG_XEN_PRIVCMD) += xen-privcmd.o
> >>>> obj-$(CONFIG_XEN_ACPI_PROCESSOR) += xen-acpi-processor.o
> >>>> +obj-$(CONFIG_XEN_DOM0) += xen_acpi_pad.o
> >>>> xen-evtchn-y := evtchn.o
> >>>> xen-gntdev-y := gntdev.o
> >>>> xen-gntalloc-y := gntalloc.o
> >>
> >> it should really depend on ACPI and maybe also X86, otherwise it is
> >> going to break the ARM build
> >>
> >>
> >
> > Hmm, XEN_DOM0 has already depended on ACPI and X86_LOCAL_APIC (which
> > depends on X86_64).
> >
>
> Ah, you and Konrad are right. I ignore XEN_DOM0 varies under different arch.
> (But seems it not depends on X86 since it's logically an acpi stuff?).
If it is generic ACPI code, than it can depend only on ACPI.
If it is ACPI code that contains X86 specific info, than it needs to
depend on X86 too.
Stefano Stabellini wrote:
> On Tue, 6 Nov 2012, Liu, Jinsong wrote:
>> Liu, Jinsong wrote:
>>> Stefano Stabellini wrote:
>>>>>> diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
>>>>>> index 0e86370..a2af622 100644
>>>>>> --- a/drivers/xen/Makefile
>>>>>> +++ b/drivers/xen/Makefile
>>>>>> @@ -29,6 +29,7 @@ obj-$(CONFIG_XEN_MCE_LOG) += mcelog.o
>>>>>> obj-$(CONFIG_XEN_PCIDEV_BACKEND) += xen-pciback/
>>>>>> obj-$(CONFIG_XEN_PRIVCMD) += xen-privcmd.o
>>>>>> obj-$(CONFIG_XEN_ACPI_PROCESSOR) += xen-acpi-processor.o
>>>>>> +obj-$(CONFIG_XEN_DOM0) += xen_acpi_pad.o
>>>>>> xen-evtchn-y := evtchn.o
>>>>>> xen-gntdev-y := gntdev.o
>>>>>> xen-gntalloc-y := gntalloc.o
>>>>
>>>> it should really depend on ACPI and maybe also X86, otherwise it
>>>> is going to break the ARM build
>>>>
>>>>
>>>
>>> Hmm, XEN_DOM0 has already depended on ACPI and X86_LOCAL_APIC
>>> (which depends on X86_64).
>>>
>>
>> Ah, you and Konrad are right. I ignore XEN_DOM0 varies under
>> different arch. (But seems it not depends on X86 since it's
>> logically an acpi stuff?).
>
> If it is generic ACPI code, than it can depend only on ACPI.
> If it is ACPI code that contains X86 specific info, than it needs to
> depend on X86 too.
No x86 specific so let's depend on ACPI.
Thanks,
Jinsong-
On Tue, Nov 06, 2012 at 04:23:57PM +0000, Liu, Jinsong wrote:
> Stefano Stabellini wrote:
> > On Tue, 6 Nov 2012, Liu, Jinsong wrote:
> >> Liu, Jinsong wrote:
> >>> Stefano Stabellini wrote:
> >>>>>> diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
> >>>>>> index 0e86370..a2af622 100644
> >>>>>> --- a/drivers/xen/Makefile
> >>>>>> +++ b/drivers/xen/Makefile
> >>>>>> @@ -29,6 +29,7 @@ obj-$(CONFIG_XEN_MCE_LOG) += mcelog.o
> >>>>>> obj-$(CONFIG_XEN_PCIDEV_BACKEND) += xen-pciback/
> >>>>>> obj-$(CONFIG_XEN_PRIVCMD) += xen-privcmd.o
> >>>>>> obj-$(CONFIG_XEN_ACPI_PROCESSOR) += xen-acpi-processor.o
> >>>>>> +obj-$(CONFIG_XEN_DOM0) += xen_acpi_pad.o
> >>>>>> xen-evtchn-y := evtchn.o
> >>>>>> xen-gntdev-y := gntdev.o
> >>>>>> xen-gntalloc-y := gntalloc.o
> >>>>
> >>>> it should really depend on ACPI and maybe also X86, otherwise it
> >>>> is going to break the ARM build
> >>>>
> >>>>
> >>>
> >>> Hmm, XEN_DOM0 has already depended on ACPI and X86_LOCAL_APIC
> >>> (which depends on X86_64).
> >>>
> >>
> >> Ah, you and Konrad are right. I ignore XEN_DOM0 varies under
> >> different arch. (But seems it not depends on X86 since it's
> >> logically an acpi stuff?).
> >
> > If it is generic ACPI code, than it can depend only on ACPI.
> > If it is ACPI code that contains X86 specific info, than it needs to
> > depend on X86 too.
>
> No x86 specific so let's depend on ACPI.
Huh? This feature is x86 specific isn't it? I mean it is in the ACPI spec, but
only x86 does it right?
On Thu, Oct 25, 2012 at 6:19 AM, Liu, Jinsong <[email protected]> wrote:
> From f233ad06cf924116693d7d38be9ae9d8c11f8a9b Mon Sep 17 00:00:00 2001
> From: Liu, Jinsong <[email protected]>
> Date: Fri, 26 Oct 2012 02:32:48 +0800
> Subject: [PATCH 1/2] Xen acpi pad implement
>
> PAD is acpi Processor Aggregator Device which provides a control point
> that enables the platform to perform specific processor configuration
> and control that applies to all processors in the platform.
>
> This patch is to implement Xen acpi pad logic. When running under Xen
> virt platform, native pad driver would not work. Instead Xen pad driver,
> a self-contained and very thin logic level, would take over acpi pad staff.
"logic" instead of staff (guessing this is supposed to stuff)
> When acpi pad notify OSPM, xen pad logic intercept and parse _PUR object
> and then hypercall to hyervisor for the rest work, say, core parking.
hypervisor
rest of the work? - could you add more details on what is handled by
this think PAD driver and what would be left to the host pad driver.
>
> Signed-off-by: Liu, Jinsong <[email protected]>
> ---
> drivers/xen/Makefile | 1 +
> drivers/xen/xen_acpi_pad.c | 173 ++++++++++++++++++++++++++++++++++++++
> include/xen/interface/platform.h | 17 ++++
> 3 files changed, 191 insertions(+), 0 deletions(-)
> create mode 100644 drivers/xen/xen_acpi_pad.c
>
> diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
> index 0e86370..a2af622 100644
> --- a/drivers/xen/Makefile
> +++ b/drivers/xen/Makefile
> @@ -29,6 +29,7 @@ obj-$(CONFIG_XEN_MCE_LOG) += mcelog.o
> obj-$(CONFIG_XEN_PCIDEV_BACKEND) += xen-pciback/
> obj-$(CONFIG_XEN_PRIVCMD) += xen-privcmd.o
> obj-$(CONFIG_XEN_ACPI_PROCESSOR) += xen-acpi-processor.o
> +obj-$(CONFIG_XEN_DOM0) += xen_acpi_pad.o
> xen-evtchn-y := evtchn.o
> xen-gntdev-y := gntdev.o
> xen-gntalloc-y := gntalloc.o
> diff --git a/drivers/xen/xen_acpi_pad.c b/drivers/xen/xen_acpi_pad.c
> new file mode 100644
> index 0000000..e7b7dca
> --- /dev/null
> +++ b/drivers/xen/xen_acpi_pad.c
> @@ -0,0 +1,173 @@
> +/*
> + * xen_acpi_pad.c - Xen pad interface
> + *
> + * Copyright (c) 2012, Intel Corporation.
> + * Author: Liu, Jinsong <[email protected]>
> + *
> + * 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/kernel.h>
> +#include <linux/types.h>
> +#include <acpi/acpi_bus.h>
> +#include <acpi/acpi_drivers.h>
> +#include <asm/xen/hypercall.h>
> +
> +#if defined(CONFIG_ACPI_PROCESSOR_AGGREGATOR) || \
> + defined(CONFIG_ACPI_PROCESSOR_AGGREGATOR_MODULE)
> +
> +#define ACPI_PROCESSOR_AGGREGATOR_CLASS "acpi_pad"
> +#define ACPI_PROCESSOR_AGGREGATOR_DEVICE_NAME "Processor Aggregator"
> +#define ACPI_PROCESSOR_AGGREGATOR_NOTIFY 0x80
> +
> +static int xen_acpi_pad_idle_cpus(int *num_cpus)
> +{
> + int ret;
> +
> + struct xen_platform_op op = {
> + .cmd = XENPF_core_parking,
> + .interface_version = XENPF_INTERFACE_VERSION,
> + };
> +
> + /* set cpu nums expected to be idled */
> + op.u.core_parking.type = XEN_CORE_PARKING_SET;
> + op.u.core_parking.idle_nums = (uint32_t)*num_cpus;
> + ret = HYPERVISOR_dom0_op(&op);
> + if (ret)
> + return ret;
> +
> + /*
> + * get cpu nums actually be idled
> + * cannot get it by using hypercall once (shared with _SET)
> + * because of the characteristic of Xen continue_hypercall_on_cpu
> + */
> + op.u.core_parking.type = XEN_CORE_PARKING_GET;
> + ret = HYPERVISOR_dom0_op(&op);
> + if (ret)
> + return ret;
> +
> + *num_cpus = op.u.core_parking.idle_nums;
> + return 0;
> +}
> +
> +/*
> + * Query firmware how many CPUs should be idle
> + * return -1 on failure
> + */
> +static int xen_acpi_pad_pur(acpi_handle handle)
> +{
> + struct acpi_buffer buffer = {ACPI_ALLOCATE_BUFFER, NULL};
> + union acpi_object *package;
> + int num = -1;
> +
> + if (ACPI_FAILURE(acpi_evaluate_object(handle, "_PUR", NULL, &buffer)))
> + return num;
> +
> + if (!buffer.length || !buffer.pointer)
> + return num;
> +
> + package = buffer.pointer;
> +
> + if (package->type == ACPI_TYPE_PACKAGE &&
> + package->package.count == 2 &&
> + package->package.elements[0].integer.value == 1) /* rev 1 */
> +
> + num = package->package.elements[1].integer.value;
> +
> + kfree(buffer.pointer);
> + return num;
> +}
> +
> +/* Notify firmware how many CPUs are idle */
> +static void xen_acpi_pad_ost(acpi_handle handle, int stat,
> + uint32_t idle_cpus)
> +{
> + union acpi_object params[3] = {
> + {.type = ACPI_TYPE_INTEGER,},
> + {.type = ACPI_TYPE_INTEGER,},
> + {.type = ACPI_TYPE_BUFFER,},
> + };
> + struct acpi_object_list arg_list = {3, params};
> +
> + params[0].integer.value = ACPI_PROCESSOR_AGGREGATOR_NOTIFY;
> + params[1].integer.value = stat;
> + params[2].buffer.length = 4;
> + params[2].buffer.pointer = (void *)&idle_cpus;
> + acpi_evaluate_object(handle, "_OST", &arg_list, NULL);
> +}
> +
> +static void xen_acpi_pad_handle_notify(acpi_handle handle)
> +{
> + int ret, num_cpus;
> +
> + num_cpus = xen_acpi_pad_pur(handle);
> + if (num_cpus < 0)
> + return;
> +
> + ret = xen_acpi_pad_idle_cpus(&num_cpus);
> + if (ret)
> + return;
> +
> + xen_acpi_pad_ost(handle, 0, num_cpus);
> +}
> +
> +static void xen_acpi_pad_notify(acpi_handle handle, u32 event,
> + void *data)
> +{
> + switch (event) {
> + case ACPI_PROCESSOR_AGGREGATOR_NOTIFY:
> + xen_acpi_pad_handle_notify(handle);
> + break;
> + default:
> + pr_warn("Unsupported event [0x%x]\n", event);
> + break;
> + }
> +}
> +
> +static int xen_acpi_pad_add(struct acpi_device *device)
> +{
> + acpi_status status;
> +
> + strcpy(acpi_device_name(device), ACPI_PROCESSOR_AGGREGATOR_DEVICE_NAME);
> + strcpy(acpi_device_class(device), ACPI_PROCESSOR_AGGREGATOR_CLASS);
> +
> + status = acpi_install_notify_handler(device->handle,
> + ACPI_DEVICE_NOTIFY, xen_acpi_pad_notify, device);
> + if (ACPI_FAILURE(status))
> + return -ENODEV;
> +
> + return 0;
> +}
> +
> +static const struct acpi_device_id pad_device_ids[] = {
> + {"ACPI000C", 0},
> + {"", 0},
> +};
> +
> +static struct acpi_driver xen_acpi_pad_driver = {
> + .name = "processor_aggregator",
> + .class = ACPI_PROCESSOR_AGGREGATOR_CLASS,
> + .ids = pad_device_ids,
> + .ops = {
> + .add = xen_acpi_pad_add,
> + },
> +};
> +
> +static int __init xen_acpi_pad_init(void)
> +{
> + /* Only DOM0 is responsible for Xen acpi pad */
> + if (xen_initial_domain())
> + return acpi_bus_register_driver(&xen_acpi_pad_driver);
> +
> + return -ENODEV;
> +}
> +subsys_initcall(xen_acpi_pad_init);
> +
> +#endif
> diff --git a/include/xen/interface/platform.h b/include/xen/interface/platform.h
> index 4755b5f..0f44376 100644
> --- a/include/xen/interface/platform.h
> +++ b/include/xen/interface/platform.h
> @@ -324,6 +324,22 @@ struct xenpf_cpu_ol {
> };
> DEFINE_GUEST_HANDLE_STRUCT(xenpf_cpu_ol);
>
> +/*
> + * CMD 58 and 59 are reserved for cpu hotadd and memory hotadd,
> + * which already occupied at Xen hypervisor side.
> + */
> +#define XENPF_core_parking 60
> +struct xenpf_core_parking {
> + /* IN variables */
> +#define XEN_CORE_PARKING_SET 1
> +#define XEN_CORE_PARKING_GET 2
> + uint32_t type;
> + /* IN variables: set cpu nums expected to be idled */
> + /* OUT variables: get cpu nums actually be idled */
> + uint32_t idle_nums;
> +};
> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_core_parking);
> +
> struct xen_platform_op {
> uint32_t cmd;
> uint32_t interface_version; /* XENPF_INTERFACE_VERSION */
> @@ -341,6 +357,7 @@ struct xen_platform_op {
> struct xenpf_set_processor_pminfo set_pminfo;
> struct xenpf_pcpuinfo pcpu_info;
> struct xenpf_cpu_ol cpu_ol;
> + struct xenpf_core_parking core_parking;
> uint8_t pad[128];
> } u;
> };
> --
> 1.7.1
>>>
>>> If it is generic ACPI code, than it can depend only on ACPI.
>>> If it is ACPI code that contains X86 specific info, than it needs to
>>> depend on X86 too.
>>
>> No x86 specific so let's depend on ACPI.
>
> Huh? This feature is x86 specific isn't it? I mean it is in the ACPI
> spec, but only x86 does it right?
OK, updated w/ ACPI & X86 dependency, and per other comments.
Thanks,
Jinsong
=================
>From d5f462cc8029863955a574f1541bece72aafb986 Mon Sep 17 00:00:00 2001
From: Liu, Jinsong <[email protected]>
Date: Thu, 8 Nov 2012 04:29:07 +0800
Subject: [PATCH 1/2] Xen acpi pad implement
PAD is acpi Processor Aggregator Device which provides a control point
that enables the platform to perform specific processor configuration
and control that applies to all processors in the platform.
This patch is to implement Xen acpi pad logic. When running under Xen
virt platform, native pad driver would not work. Instead Xen pad driver,
a self-contained and thin logic level, would take over acpi pad logic.
When acpi pad notify OSPM, xen pad logic intercept and parse _PUR object
to get the expected idle cpu number, and then hypercall to hypervisor.
Xen hypervisor would then do the rest work, say, core parking, to idle
specific number of cpus on its own policy.
Signed-off-by: Jan Beulich <[email protected]>
Signed-off-by: Liu Jinsong <[email protected]>
---
drivers/xen/Makefile | 3 +-
drivers/xen/xen-acpi-pad.c | 186 ++++++++++++++++++++++++++++++++++++++
include/xen/interface/platform.h | 17 ++++
include/xen/interface/version.h | 15 +++
4 files changed, 220 insertions(+), 1 deletions(-)
create mode 100644 drivers/xen/xen-acpi-pad.c
diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
index 0e86370..3c39717 100644
--- a/drivers/xen/Makefile
+++ b/drivers/xen/Makefile
@@ -10,7 +10,8 @@ CFLAGS_features.o := $(nostackp)
dom0-$(CONFIG_PCI) += pci.o
dom0-$(CONFIG_USB_SUPPORT) += dbgp.o
-dom0-$(CONFIG_ACPI) += acpi.o
+dom0-$(CONFIG_ACPI) += acpi.o $(xen-pad-y)
+xen-pad-$(CONFIG_X86) += xen-acpi-pad.o
dom0-$(CONFIG_X86) += pcpu.o
obj-$(CONFIG_XEN_DOM0) += $(dom0-y)
obj-$(CONFIG_BLOCK) += biomerge.o
diff --git a/drivers/xen/xen-acpi-pad.c b/drivers/xen/xen-acpi-pad.c
new file mode 100644
index 0000000..f02d3ff
--- /dev/null
+++ b/drivers/xen/xen-acpi-pad.c
@@ -0,0 +1,186 @@
+/*
+ * xen-acpi-pad.c - Xen pad interface
+ *
+ * Copyright (c) 2012, Intel Corporation.
+ * Author: Liu, Jinsong <[email protected]>
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/types.h>
+#include <acpi/acpi_bus.h>
+#include <acpi/acpi_drivers.h>
+#include <asm/xen/hypercall.h>
+#include <xen/interface/version.h>
+
+#define ACPI_PROCESSOR_AGGREGATOR_CLASS "acpi_pad"
+#define ACPI_PROCESSOR_AGGREGATOR_DEVICE_NAME "Processor Aggregator"
+#define ACPI_PROCESSOR_AGGREGATOR_NOTIFY 0x80
+
+static DEFINE_MUTEX(xen_cpu_lock);
+
+static int xen_acpi_pad_idle_cpus(unsigned int idle_nums)
+{
+ struct xen_platform_op op;
+
+ op.cmd = XENPF_core_parking;
+ op.u.core_parking.type = XEN_CORE_PARKING_SET;
+ op.u.core_parking.idle_nums = idle_nums;
+
+ return HYPERVISOR_dom0_op(&op);
+}
+
+static int xen_acpi_pad_idle_cpus_num(void)
+{
+ struct xen_platform_op op;
+
+ op.cmd = XENPF_core_parking;
+ op.u.core_parking.type = XEN_CORE_PARKING_GET;
+
+ return HYPERVISOR_dom0_op(&op)
+ ?: op.u.core_parking.idle_nums;
+}
+
+/*
+ * Query firmware how many CPUs should be idle
+ * return -1 on failure
+ */
+static int acpi_pad_pur(acpi_handle handle)
+{
+ struct acpi_buffer buffer = {ACPI_ALLOCATE_BUFFER, NULL};
+ union acpi_object *package;
+ int num = -1;
+
+ if (ACPI_FAILURE(acpi_evaluate_object(handle, "_PUR", NULL, &buffer)))
+ return num;
+
+ if (!buffer.length || !buffer.pointer)
+ return num;
+
+ package = buffer.pointer;
+
+ if (package->type == ACPI_TYPE_PACKAGE &&
+ package->package.count == 2 &&
+ package->package.elements[0].integer.value == 1) /* rev 1 */
+
+ num = package->package.elements[1].integer.value;
+
+ kfree(buffer.pointer);
+ return num;
+}
+
+/* Notify firmware how many CPUs are idle */
+static void acpi_pad_ost(acpi_handle handle, int stat,
+ uint32_t idle_nums)
+{
+ union acpi_object params[3] = {
+ {.type = ACPI_TYPE_INTEGER,},
+ {.type = ACPI_TYPE_INTEGER,},
+ {.type = ACPI_TYPE_BUFFER,},
+ };
+ struct acpi_object_list arg_list = {3, params};
+
+ params[0].integer.value = ACPI_PROCESSOR_AGGREGATOR_NOTIFY;
+ params[1].integer.value = stat;
+ params[2].buffer.length = 4;
+ params[2].buffer.pointer = (void *)&idle_nums;
+ acpi_evaluate_object(handle, "_OST", &arg_list, NULL);
+}
+
+static void acpi_pad_handle_notify(acpi_handle handle)
+{
+ int idle_nums;
+
+ mutex_lock(&xen_cpu_lock);
+ idle_nums = acpi_pad_pur(handle);
+ if (idle_nums < 0) {
+ mutex_unlock(&xen_cpu_lock);
+ return;
+ }
+
+ idle_nums = xen_acpi_pad_idle_cpus(idle_nums)
+ ?: xen_acpi_pad_idle_cpus_num();
+ if (idle_nums >= 0)
+ acpi_pad_ost(handle, 0, idle_nums);
+ mutex_unlock(&xen_cpu_lock);
+}
+
+static void acpi_pad_notify(acpi_handle handle, u32 event,
+ void *data)
+{
+ switch (event) {
+ case ACPI_PROCESSOR_AGGREGATOR_NOTIFY:
+ acpi_pad_handle_notify(handle);
+ break;
+ default:
+ pr_warn("Unsupported event [0x%x]\n", event);
+ break;
+ }
+}
+
+static int acpi_pad_add(struct acpi_device *device)
+{
+ acpi_status status;
+
+ strcpy(acpi_device_name(device), ACPI_PROCESSOR_AGGREGATOR_DEVICE_NAME);
+ strcpy(acpi_device_class(device), ACPI_PROCESSOR_AGGREGATOR_CLASS);
+
+ status = acpi_install_notify_handler(device->handle,
+ ACPI_DEVICE_NOTIFY, acpi_pad_notify, device);
+ if (ACPI_FAILURE(status))
+ return -ENODEV;
+
+ return 0;
+}
+
+static int acpi_pad_remove(struct acpi_device *device,
+ int type)
+{
+ mutex_lock(&xen_cpu_lock);
+ xen_acpi_pad_idle_cpus(0);
+ mutex_unlock(&xen_cpu_lock);
+
+ acpi_remove_notify_handler(device->handle,
+ ACPI_DEVICE_NOTIFY, acpi_pad_notify);
+ return 0;
+}
+
+static const struct acpi_device_id pad_device_ids[] = {
+ {"ACPI000C", 0},
+ {"", 0},
+};
+
+static struct acpi_driver acpi_pad_driver = {
+ .name = "processor_aggregator",
+ .class = ACPI_PROCESSOR_AGGREGATOR_CLASS,
+ .ids = pad_device_ids,
+ .ops = {
+ .add = acpi_pad_add,
+ .remove = acpi_pad_remove,
+ },
+};
+
+static int __init xen_acpi_pad_init(void)
+{
+ /* Only DOM0 and Xen4.2 or later support Xen acpi pad */
+ if (!xen_initial_domain() ||
+ !xen_running_on_version_or_later(4, 2))
+ return -ENODEV;
+
+ return acpi_bus_register_driver(&acpi_pad_driver);
+}
+subsys_initcall(xen_acpi_pad_init);
+
diff --git a/include/xen/interface/platform.h b/include/xen/interface/platform.h
index 4755b5f..5e36932 100644
--- a/include/xen/interface/platform.h
+++ b/include/xen/interface/platform.h
@@ -324,6 +324,22 @@ struct xenpf_cpu_ol {
};
DEFINE_GUEST_HANDLE_STRUCT(xenpf_cpu_ol);
+/*
+ * CMD 58 and 59 are reserved for cpu hotadd and memory hotadd,
+ * which are already occupied at Xen hypervisor side.
+ */
+#define XENPF_core_parking 60
+struct xenpf_core_parking {
+ /* IN variables */
+#define XEN_CORE_PARKING_SET 1
+#define XEN_CORE_PARKING_GET 2
+ uint32_t type;
+ /* IN variables: set cpu nums expected to be idled */
+ /* OUT variables: get cpu nums actually be idled */
+ uint32_t idle_nums;
+};
+DEFINE_GUEST_HANDLE_STRUCT(xenpf_core_parking);
+
struct xen_platform_op {
uint32_t cmd;
uint32_t interface_version; /* XENPF_INTERFACE_VERSION */
@@ -341,6 +357,7 @@ struct xen_platform_op {
struct xenpf_set_processor_pminfo set_pminfo;
struct xenpf_pcpuinfo pcpu_info;
struct xenpf_cpu_ol cpu_ol;
+ struct xenpf_core_parking core_parking;
uint8_t pad[128];
} u;
};
diff --git a/include/xen/interface/version.h b/include/xen/interface/version.h
index 7ff6498..96d8d3d 100644
--- a/include/xen/interface/version.h
+++ b/include/xen/interface/version.h
@@ -63,4 +63,19 @@ struct xen_feature_info {
/* arg == xen_domain_handle_t. */
#define XENVER_guest_handle 8
+/* Check if running on Xen version (major, minor) or later */
+static inline bool
+xen_running_on_version_or_later(unsigned int major, unsigned int minor)
+{
+ unsigned int version;
+
+ if (!xen_domain())
+ return false;
+
+ version = HYPERVISOR_xen_version(XENVER_version, NULL);
+ if ((((version >> 16) == major) && ((version & 0xffff) >= minor)) ||
+ ((version >> 16) > major))
+ return true;
+ return false;
+}
#endif /* __XEN_PUBLIC_VERSION_H__ */
--
1.7.1
Shuah Khan wrote:
> On Thu, Oct 25, 2012 at 6:19 AM, Liu, Jinsong <[email protected]>
> wrote:
>> From f233ad06cf924116693d7d38be9ae9d8c11f8a9b Mon Sep 17 00:00:00
>> 2001
>> From: Liu, Jinsong <[email protected]>
>> Date: Fri, 26 Oct 2012 02:32:48 +0800
>> Subject: [PATCH 1/2] Xen acpi pad implement
>>
>> PAD is acpi Processor Aggregator Device which provides a control
>> point
>> that enables the platform to perform specific processor configuration
>> and control that applies to all processors in the platform.
>>
>> This patch is to implement Xen acpi pad logic. When running under Xen
>> virt platform, native pad driver would not work. Instead Xen pad
>> driver,
>> a self-contained and very thin logic level, would take over acpi pad
>> staff.
>
> "logic" instead of staff (guessing this is supposed to stuff)
>
Yes.
>> When acpi pad notify OSPM, xen pad logic intercept and parse _PUR
>> object
>> and then hypercall to hyervisor for the rest work, say, core parking.
> hypervisor
> rest of the work? - could you add more details on what is handled by
> this think PAD driver and what would be left to the host pad driver.
OK, add at updated patch.
Thanks,
Jinsong-
On Wed, Nov 07, 2012 at 12:58:19PM +0000, Liu, Jinsong wrote:
> >>>
> >>> If it is generic ACPI code, than it can depend only on ACPI.
> >>> If it is ACPI code that contains X86 specific info, than it needs to
> >>> depend on X86 too.
> >>
> >> No x86 specific so let's depend on ACPI.
> >
> > Huh? This feature is x86 specific isn't it? I mean it is in the ACPI
> > spec, but only x86 does it right?
>
> OK, updated w/ ACPI & X86 dependency, and per other comments.
Ok. Could you send all the patchset in one go. I am bit lost of which
one to review as none of them seem to have the version information
[vX: Did this yyy]
right above your name or in the title:
[PATCH vX 1/2]: yyyy
anyhow, some comments below.
>
> Thanks,
> Jinsong
>
> =================
>
> >From d5f462cc8029863955a574f1541bece72aafb986 Mon Sep 17 00:00:00 2001
> From: Liu, Jinsong <[email protected]>
> Date: Thu, 8 Nov 2012 04:29:07 +0800
> Subject: [PATCH 1/2] Xen acpi pad implement
The title should be: xen/acpi: ACPI PAD driver
>
> PAD is acpi Processor Aggregator Device which provides a control point
> that enables the platform to perform specific processor configuration
> and control that applies to all processors in the platform.
>
> This patch is to implement Xen acpi pad logic. When running under Xen
> virt platform, native pad driver would not work. Instead Xen pad driver,
> a self-contained and thin logic level, would take over acpi pad logic.
>
> When acpi pad notify OSPM, xen pad logic intercept and parse _PUR object
> to get the expected idle cpu number, and then hypercall to hypervisor.
> Xen hypervisor would then do the rest work, say, core parking, to idle
> specific number of cpus on its own policy.
>
> Signed-off-by: Jan Beulich <[email protected]>
> Signed-off-by: Liu Jinsong <[email protected]>
> ---
> drivers/xen/Makefile | 3 +-
> drivers/xen/xen-acpi-pad.c | 186 ++++++++++++++++++++++++++++++++++++++
> include/xen/interface/platform.h | 17 ++++
> include/xen/interface/version.h | 15 +++
> 4 files changed, 220 insertions(+), 1 deletions(-)
> create mode 100644 drivers/xen/xen-acpi-pad.c
>
> diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
> index 0e86370..3c39717 100644
> --- a/drivers/xen/Makefile
> +++ b/drivers/xen/Makefile
> @@ -10,7 +10,8 @@ CFLAGS_features.o := $(nostackp)
>
> dom0-$(CONFIG_PCI) += pci.o
> dom0-$(CONFIG_USB_SUPPORT) += dbgp.o
> -dom0-$(CONFIG_ACPI) += acpi.o
> +dom0-$(CONFIG_ACPI) += acpi.o $(xen-pad-y)
> +xen-pad-$(CONFIG_X86) += xen-acpi-pad.o
> dom0-$(CONFIG_X86) += pcpu.o
> obj-$(CONFIG_XEN_DOM0) += $(dom0-y)
> obj-$(CONFIG_BLOCK) += biomerge.o
> diff --git a/drivers/xen/xen-acpi-pad.c b/drivers/xen/xen-acpi-pad.c
> new file mode 100644
> index 0000000..f02d3ff
> --- /dev/null
> +++ b/drivers/xen/xen-acpi-pad.c
> @@ -0,0 +1,186 @@
> +/*
> + * xen-acpi-pad.c - Xen pad interface
> + *
> + * Copyright (c) 2012, Intel Corporation.
> + * Author: Liu, Jinsong <[email protected]>
> + *
> + * 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.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program; if not, write to the Free Software Foundation, Inc.,
> + * 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
No address pls.
> + *
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/types.h>
> +#include <acpi/acpi_bus.h>
> +#include <acpi/acpi_drivers.h>
> +#include <asm/xen/hypercall.h>
> +#include <xen/interface/version.h>
> +
> +#define ACPI_PROCESSOR_AGGREGATOR_CLASS "acpi_pad"
> +#define ACPI_PROCESSOR_AGGREGATOR_DEVICE_NAME "Processor Aggregator"
> +#define ACPI_PROCESSOR_AGGREGATOR_NOTIFY 0x80
> +
> +static DEFINE_MUTEX(xen_cpu_lock);
> +
> +static int xen_acpi_pad_idle_cpus(unsigned int idle_nums)
> +{
> + struct xen_platform_op op;
> +
> + op.cmd = XENPF_core_parking;
> + op.u.core_parking.type = XEN_CORE_PARKING_SET;
> + op.u.core_parking.idle_nums = idle_nums;
> +
> + return HYPERVISOR_dom0_op(&op);
> +}
> +
> +static int xen_acpi_pad_idle_cpus_num(void)
> +{
> + struct xen_platform_op op;
> +
> + op.cmd = XENPF_core_parking;
> + op.u.core_parking.type = XEN_CORE_PARKING_GET;
> +
> + return HYPERVISOR_dom0_op(&op)
> + ?: op.u.core_parking.idle_nums;
> +}
> +
> +/*
> + * Query firmware how many CPUs should be idle
> + * return -1 on failure
> + */
> +static int acpi_pad_pur(acpi_handle handle)
> +{
> + struct acpi_buffer buffer = {ACPI_ALLOCATE_BUFFER, NULL};
> + union acpi_object *package;
> + int num = -1;
> +
> + if (ACPI_FAILURE(acpi_evaluate_object(handle, "_PUR", NULL, &buffer)))
> + return num;
> +
> + if (!buffer.length || !buffer.pointer)
> + return num;
> +
> + package = buffer.pointer;
> +
> + if (package->type == ACPI_TYPE_PACKAGE &&
> + package->package.count == 2 &&
> + package->package.elements[0].integer.value == 1) /* rev 1 */
> +
> + num = package->package.elements[1].integer.value;
> +
> + kfree(buffer.pointer);
> + return num;
> +}
> +
> +/* Notify firmware how many CPUs are idle */
> +static void acpi_pad_ost(acpi_handle handle, int stat,
> + uint32_t idle_nums)
> +{
> + union acpi_object params[3] = {
> + {.type = ACPI_TYPE_INTEGER,},
> + {.type = ACPI_TYPE_INTEGER,},
> + {.type = ACPI_TYPE_BUFFER,},
> + };
> + struct acpi_object_list arg_list = {3, params};
> +
> + params[0].integer.value = ACPI_PROCESSOR_AGGREGATOR_NOTIFY;
> + params[1].integer.value = stat;
> + params[2].buffer.length = 4;
> + params[2].buffer.pointer = (void *)&idle_nums;
> + acpi_evaluate_object(handle, "_OST", &arg_list, NULL);
> +}
> +
> +static void acpi_pad_handle_notify(acpi_handle handle)
> +{
> + int idle_nums;
> +
> + mutex_lock(&xen_cpu_lock);
> + idle_nums = acpi_pad_pur(handle);
> + if (idle_nums < 0) {
> + mutex_unlock(&xen_cpu_lock);
> + return;
> + }
> +
> + idle_nums = xen_acpi_pad_idle_cpus(idle_nums)
> + ?: xen_acpi_pad_idle_cpus_num();
> + if (idle_nums >= 0)
> + acpi_pad_ost(handle, 0, idle_nums);
> + mutex_unlock(&xen_cpu_lock);
> +}
> +
> +static void acpi_pad_notify(acpi_handle handle, u32 event,
> + void *data)
> +{
> + switch (event) {
> + case ACPI_PROCESSOR_AGGREGATOR_NOTIFY:
> + acpi_pad_handle_notify(handle);
> + break;
> + default:
> + pr_warn("Unsupported event [0x%x]\n", event);
> + break;
> + }
> +}
> +
> +static int acpi_pad_add(struct acpi_device *device)
> +{
> + acpi_status status;
> +
> + strcpy(acpi_device_name(device), ACPI_PROCESSOR_AGGREGATOR_DEVICE_NAME);
> + strcpy(acpi_device_class(device), ACPI_PROCESSOR_AGGREGATOR_CLASS);
> +
> + status = acpi_install_notify_handler(device->handle,
> + ACPI_DEVICE_NOTIFY, acpi_pad_notify, device);
> + if (ACPI_FAILURE(status))
> + return -ENODEV;
> +
> + return 0;
> +}
> +
> +static int acpi_pad_remove(struct acpi_device *device,
> + int type)
> +{
> + mutex_lock(&xen_cpu_lock);
> + xen_acpi_pad_idle_cpus(0);
> + mutex_unlock(&xen_cpu_lock);
> +
> + acpi_remove_notify_handler(device->handle,
> + ACPI_DEVICE_NOTIFY, acpi_pad_notify);
> + return 0;
> +}
> +
> +static const struct acpi_device_id pad_device_ids[] = {
> + {"ACPI000C", 0},
> + {"", 0},
> +};
> +
> +static struct acpi_driver acpi_pad_driver = {
> + .name = "processor_aggregator",
> + .class = ACPI_PROCESSOR_AGGREGATOR_CLASS,
> + .ids = pad_device_ids,
> + .ops = {
> + .add = acpi_pad_add,
> + .remove = acpi_pad_remove,
> + },
> +};
> +
> +static int __init xen_acpi_pad_init(void)
> +{
> + /* Only DOM0 and Xen4.2 or later support Xen acpi pad */
> + if (!xen_initial_domain() ||
> + !xen_running_on_version_or_later(4, 2))
> + return -ENODEV;
Please restructure this. Just make it
if (!xen_initial_domain())
return -ENODEV;
if (!xen_running..)
return -ENODEV;
I know the compiler will end up with code that looks exactly
the same but it is easier to read than the '!' and '||' logic.
> +
> + return acpi_bus_register_driver(&acpi_pad_driver);
> +}
> +subsys_initcall(xen_acpi_pad_init);
> +
> diff --git a/include/xen/interface/platform.h b/include/xen/interface/platform.h
> index 4755b5f..5e36932 100644
> --- a/include/xen/interface/platform.h
> +++ b/include/xen/interface/platform.h
> @@ -324,6 +324,22 @@ struct xenpf_cpu_ol {
> };
> DEFINE_GUEST_HANDLE_STRUCT(xenpf_cpu_ol);
>
> +/*
> + * CMD 58 and 59 are reserved for cpu hotadd and memory hotadd,
> + * which are already occupied at Xen hypervisor side.
> + */
> +#define XENPF_core_parking 60
> +struct xenpf_core_parking {
> + /* IN variables */
> +#define XEN_CORE_PARKING_SET 1
> +#define XEN_CORE_PARKING_GET 2
> + uint32_t type;
> + /* IN variables: set cpu nums expected to be idled */
> + /* OUT variables: get cpu nums actually be idled */
> + uint32_t idle_nums;
> +};
> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_core_parking);
> +
> struct xen_platform_op {
> uint32_t cmd;
> uint32_t interface_version; /* XENPF_INTERFACE_VERSION */
> @@ -341,6 +357,7 @@ struct xen_platform_op {
> struct xenpf_set_processor_pminfo set_pminfo;
> struct xenpf_pcpuinfo pcpu_info;
> struct xenpf_cpu_ol cpu_ol;
> + struct xenpf_core_parking core_parking;
> uint8_t pad[128];
> } u;
> };
> diff --git a/include/xen/interface/version.h b/include/xen/interface/version.h
> index 7ff6498..96d8d3d 100644
> --- a/include/xen/interface/version.h
> +++ b/include/xen/interface/version.h
> @@ -63,4 +63,19 @@ struct xen_feature_info {
> /* arg == xen_domain_handle_t. */
> #define XENVER_guest_handle 8
>
> +/* Check if running on Xen version (major, minor) or later */
> +static inline bool
> +xen_running_on_version_or_later(unsigned int major, unsigned int minor)
> +{
> + unsigned int version;
> +
> + if (!xen_domain())
> + return false;
> +
> + version = HYPERVISOR_xen_version(XENVER_version, NULL);
> + if ((((version >> 16) == major) && ((version & 0xffff) >= minor)) ||
> + ((version >> 16) > major))
> + return true;
> + return false;
> +}
> #endif /* __XEN_PUBLIC_VERSION_H__ */
> --
> 1.7.1
Konrad Rzeszutek Wilk wrote:
> On Wed, Nov 07, 2012 at 12:58:19PM +0000, Liu, Jinsong wrote:
>>>>>
>>>>> If it is generic ACPI code, than it can depend only on ACPI.
>>>>> If it is ACPI code that contains X86 specific info, than it needs
>>>>> to depend on X86 too.
>>>>
>>>> No x86 specific so let's depend on ACPI.
>>>
>>> Huh? This feature is x86 specific isn't it? I mean it is in the ACPI
>>> spec, but only x86 does it right?
>>
>> OK, updated w/ ACPI & X86 dependency, and per other comments.
>
> Ok. Could you send all the patchset in one go. I am bit lost of which
> one to review as none of them seem to have the version information
> [vX: Did this yyy]
Not quite clear your meaning, do you mean not reply old email and start a new thread with title like
[PATCH vX 1/2]: xen/acpi: ACPI PAD driver
[PATCH vX 2/2]: ...
?
>
> right above your name or in the title:
> [PATCH vX 1/2]: yyyy
>
> anyhow, some comments below.
>
OK.
Thanks,
Jinsong
On Wed, Nov 07, 2012 at 04:52:50PM +0000, Liu, Jinsong wrote:
> Konrad Rzeszutek Wilk wrote:
> > On Wed, Nov 07, 2012 at 12:58:19PM +0000, Liu, Jinsong wrote:
> >>>>>
> >>>>> If it is generic ACPI code, than it can depend only on ACPI.
> >>>>> If it is ACPI code that contains X86 specific info, than it needs
> >>>>> to depend on X86 too.
> >>>>
> >>>> No x86 specific so let's depend on ACPI.
> >>>
> >>> Huh? This feature is x86 specific isn't it? I mean it is in the ACPI
> >>> spec, but only x86 does it right?
> >>
> >> OK, updated w/ ACPI & X86 dependency, and per other comments.
> >
> > Ok. Could you send all the patchset in one go. I am bit lost of which
> > one to review as none of them seem to have the version information
> > [vX: Did this yyy]
>
> Not quite clear your meaning, do you mean not reply old email and start a new thread with title like
> [PATCH vX 1/2]: xen/acpi: ACPI PAD driver
> [PATCH vX 2/2]: ...
Have you ever used git-send email? If you repost them all it comes out threaded
and it makes it easy to apply the whole patchset in one go.
> ?
>
> >
> > right above your name or in the title:
> > [PATCH vX 1/2]: yyyy
> >
> > anyhow, some comments below.
> >
>
> OK.
>
> Thanks,
> Jinsong