2012-05-10 15:07:24

by Liu, Jinsong

[permalink] [raw]
Subject: [PATCH 3/3] Xen physical cpus interface (V2)

>From 7aa4cf7c1172e24611dc76163397b8708acacc59 Mon Sep 17 00:00:00 2001
From: Liu, Jinsong <[email protected]>
Date: Fri, 11 May 2012 06:55:35 +0800
Subject: [PATCH 3/3] Xen physical cpus interface

Manage physical cpus in dom0, get physical cpus info and provide sys interface.

Signed-off-by: Liu, Jinsong <[email protected]>
Signed-off-by: Jiang, Yunhong <[email protected]>
---
.../ABI/testing/sysfs-devices-system-xen_cpu | 20 +
drivers/xen/Makefile | 1 +
drivers/xen/pcpu.c | 374 ++++++++++++++++++++
include/xen/interface/platform.h | 8 +
include/xen/interface/xen.h | 1 +
5 files changed, 404 insertions(+), 0 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-devices-system-xen_cpu
create mode 100644 drivers/xen/pcpu.c

diff --git a/Documentation/ABI/testing/sysfs-devices-system-xen_cpu b/Documentation/ABI/testing/sysfs-devices-system-xen_cpu
new file mode 100644
index 0000000..9ca02fb
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-devices-system-xen_cpu
@@ -0,0 +1,20 @@
+What: /sys/devices/system/xen_cpu/
+Date: May 2012
+Contact: Liu, Jinsong <[email protected]>
+Description:
+ A collection of global/individual Xen physical cpu attributes
+
+ Individual physical cpu attributes are contained in
+ subdirectories named by the Xen's logical cpu number, e.g.:
+ /sys/devices/system/xen_cpu/xen_cpu#/
+
+
+What: /sys/devices/system/xen_cpu/xen_cpu#/online
+Date: May 2012
+Contact: Liu, Jinsong <[email protected]>
+Description:
+ Interface to online/offline Xen physical cpus
+
+ When running under Xen platform, it provide user interface
+ to online/offline physical cpus, except cpu0 due to several
+ logic restrictions and assumptions.
diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
index 1d3e763..d12d14d 100644
--- a/drivers/xen/Makefile
+++ b/drivers/xen/Makefile
@@ -19,6 +19,7 @@ obj-$(CONFIG_XEN_PVHVM) += platform-pci.o
obj-$(CONFIG_XEN_TMEM) += tmem.o
obj-$(CONFIG_SWIOTLB_XEN) += swiotlb-xen.o
obj-$(CONFIG_XEN_DOM0) += pci.o
+obj-$(CONFIG_XEN_DOM0) += pcpu.o
obj-$(CONFIG_XEN_PCIDEV_BACKEND) += xen-pciback/
obj-$(CONFIG_XEN_PRIVCMD) += xen-privcmd.o
obj-$(CONFIG_XEN_ACPI_PROCESSOR) += xen-acpi-processor.o
diff --git a/drivers/xen/pcpu.c b/drivers/xen/pcpu.c
new file mode 100644
index 0000000..9014ff1
--- /dev/null
+++ b/drivers/xen/pcpu.c
@@ -0,0 +1,374 @@
+/******************************************************************************
+ * pcpu.c
+ * Management physical cpu in dom0, get pcpu info and provide sys interface
+ *
+ * Copyright (c) 2012 Intel Corporation
+ * Author: Liu, Jinsong <[email protected]>
+ * Author: Jiang, Yunhong <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation; or, when distributed
+ * separately from the Linux kernel or incorporated into other
+ * software packages, subject to the following license:
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this source file (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use, copy, modify,
+ * merge, publish, distribute, sublicense, and/or sell copies of the Software,
+ * and to permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#include <linux/interrupt.h>
+#include <linux/spinlock.h>
+#include <linux/cpu.h>
+#include <linux/stat.h>
+#include <linux/capability.h>
+
+#include <xen/xen.h>
+#include <xen/xenbus.h>
+#include <xen/events.h>
+#include <xen/interface/platform.h>
+#include <asm/xen/hypervisor.h>
+#include <asm/xen/hypercall.h>
+
+#define XEN_PCPU "xen_cpu: "
+
+/*
+ * @cpu_id: Xen physical cpu logic number
+ * @flags: Xen physical cpu status flag
+ * - XEN_PCPU_FLAGS_ONLINE: cpu is online
+ * - XEN_PCPU_FLAGS_INVALID: cpu is not present
+ */
+struct pcpu {
+ struct list_head list;
+ struct device dev;
+ uint32_t cpu_id;
+ uint32_t flags;
+};
+
+static struct bus_type xen_pcpu_subsys = {
+ .name = "xen_cpu",
+ .dev_name = "xen_cpu",
+};
+
+static DEFINE_MUTEX(xen_pcpu_lock);
+
+static LIST_HEAD(xen_pcpus);
+
+static int xen_pcpu_down(uint32_t cpu_id)
+{
+ struct xen_platform_op op = {
+ .cmd = XENPF_cpu_offline,
+ .interface_version = XENPF_INTERFACE_VERSION,
+ .u.cpu_ol.cpuid = cpu_id,
+ };
+
+ return HYPERVISOR_dom0_op(&op);
+}
+
+static int xen_pcpu_up(uint32_t cpu_id)
+{
+ struct xen_platform_op op = {
+ .cmd = XENPF_cpu_online,
+ .interface_version = XENPF_INTERFACE_VERSION,
+ .u.cpu_ol.cpuid = cpu_id,
+ };
+
+ return HYPERVISOR_dom0_op(&op);
+}
+
+static ssize_t show_online(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct pcpu *cpu = container_of(dev, struct pcpu, dev);
+
+ return sprintf(buf, "%u\n", !!(cpu->flags & XEN_PCPU_FLAGS_ONLINE));
+}
+
+static ssize_t __ref store_online(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct pcpu *pcpu = container_of(dev, struct pcpu, dev);
+ unsigned long long val;
+ ssize_t ret;
+
+ if (!capable(CAP_SYS_ADMIN))
+ return -EPERM;
+
+ if (kstrtoull(buf, 0, &val) < 0)
+ return -EINVAL;
+
+ switch (val) {
+ case 0:
+ ret = xen_pcpu_down(pcpu->cpu_id);
+ break;
+ case 1:
+ ret = xen_pcpu_up(pcpu->cpu_id);
+ break;
+ default:
+ ret = -EINVAL;
+ }
+
+ if (ret >= 0)
+ ret = count;
+ return ret;
+}
+static DEVICE_ATTR(online, S_IRUGO | S_IWUSR, show_online, store_online);
+
+static bool xen_pcpu_online(uint32_t flags)
+{
+ return !!(flags & XEN_PCPU_FLAGS_ONLINE);
+}
+
+static void pcpu_online_status(struct xenpf_pcpuinfo *info,
+ struct pcpu *pcpu)
+{
+ if (xen_pcpu_online(info->flags) &&
+ !xen_pcpu_online(pcpu->flags)) {
+ /* the pcpu is onlined */
+ pcpu->flags |= XEN_PCPU_FLAGS_ONLINE;
+ kobject_uevent(&pcpu->dev.kobj, KOBJ_ONLINE);
+ } else if (!xen_pcpu_online(info->flags) &&
+ xen_pcpu_online(pcpu->flags)) {
+ /* The pcpu is offlined */
+ pcpu->flags &= ~XEN_PCPU_FLAGS_ONLINE;
+ kobject_uevent(&pcpu->dev.kobj, KOBJ_OFFLINE);
+ }
+}
+
+static struct pcpu *get_pcpu(uint32_t cpu_id)
+{
+ struct list_head *cur;
+ struct pcpu *pcpu;
+
+ list_for_each(cur, &xen_pcpus) {
+ pcpu = list_entry(cur, struct pcpu, list);
+ if (pcpu->cpu_id == cpu_id)
+ return pcpu;
+ }
+
+ return NULL;
+}
+
+static void pcpu_sys_remove(struct pcpu *pcpu)
+{
+ struct device *dev;
+
+ if (!pcpu)
+ return;
+
+ dev = &pcpu->dev;
+ if (dev->id)
+ device_remove_file(dev, &dev_attr_online);
+
+ device_unregister(dev);
+}
+
+static void free_pcpu(struct pcpu *pcpu)
+{
+ if (!pcpu)
+ return;
+
+ pcpu_sys_remove(pcpu);
+
+ list_del(&pcpu->list);
+ kfree(pcpu);
+}
+
+static int pcpu_sys_create(struct pcpu *pcpu)
+{
+ struct device *dev;
+ int err = -EINVAL;
+
+ if (!pcpu)
+ return err;
+
+ dev = &pcpu->dev;
+ dev->bus = &xen_pcpu_subsys;
+ dev->id = pcpu->cpu_id;
+
+ err = device_register(dev);
+ if (err)
+ return err;
+
+ /*
+ * Xen never offline cpu0 due to several restrictions
+ * and assumptions. This basically doesn't add a sys control
+ * to user, one cannot attempt to offline BSP.
+ */
+ if (dev->id) {
+ err = device_create_file(dev, &dev_attr_online);
+ if (err) {
+ device_unregister(dev);
+ return err;
+ }
+ }
+
+ return 0;
+}
+
+static struct pcpu *init_pcpu(struct xenpf_pcpuinfo *info)
+{
+ struct pcpu *pcpu;
+ int err;
+
+ if (info->flags & XEN_PCPU_FLAGS_INVALID)
+ return ERR_PTR(-ENODEV);
+
+ pcpu = kzalloc(sizeof(struct pcpu), GFP_KERNEL);
+ if (!pcpu)
+ return ERR_PTR(-ENOMEM);
+
+ INIT_LIST_HEAD(&pcpu->list);
+ pcpu->cpu_id = info->xen_cpuid;
+ pcpu->flags = info->flags;
+
+ err = pcpu_sys_create(pcpu);
+ if (err) {
+ pr_warning(XEN_PCPU "Failed to register pcpu%u\n",
+ info->xen_cpuid);
+ kfree(pcpu);
+ return ERR_PTR(-ENOENT);
+ }
+
+ /* Need hold on xen_pcpu_lock before pcpu list manipulations */
+ list_add_tail(&pcpu->list, &xen_pcpus);
+ return pcpu;
+}
+
+/*
+ * Caller should hold the xen_pcpu_lock
+ */
+static int sync_pcpu(uint32_t cpu, uint32_t *max_cpu)
+{
+ int ret;
+ struct pcpu *pcpu = NULL;
+ struct xenpf_pcpuinfo *info;
+ struct xen_platform_op op = {
+ .cmd = XENPF_get_cpuinfo,
+ .interface_version = XENPF_INTERFACE_VERSION,
+ .u.pcpu_info.xen_cpuid = cpu,
+ };
+
+ ret = HYPERVISOR_dom0_op(&op);
+ if (ret)
+ return ret;
+
+ info = &op.u.pcpu_info;
+ if (max_cpu)
+ *max_cpu = info->max_present;
+
+ pcpu = get_pcpu(cpu);
+
+ if (info->flags & XEN_PCPU_FLAGS_INVALID) {
+ /* The pcpu has been removed */
+ if (pcpu)
+ free_pcpu(pcpu);
+ return 0;
+ }
+
+ if (!pcpu) {
+ pcpu = init_pcpu(info);
+ if (IS_ERR_OR_NULL(pcpu))
+ return -ENODEV;
+ } else
+ pcpu_online_status(info, pcpu);
+
+ return 0;
+}
+
+/*
+ * Sync dom0's pcpu information with xen hypervisor's
+ */
+static int xen_sync_pcpus(void)
+{
+ /*
+ * Boot cpu always have cpu_id 0 in xen
+ */
+ uint32_t cpu = 0, max_cpu = 0;
+ int err = 0;
+ struct list_head *cur, *tmp;
+ struct pcpu *pcpu;
+
+ mutex_lock(&xen_pcpu_lock);
+
+ while (!err && (cpu <= max_cpu)) {
+ err = sync_pcpu(cpu, &max_cpu);
+ cpu++;
+ }
+
+ if (err) {
+ list_for_each_safe(cur, tmp, &xen_pcpus) {
+ pcpu = list_entry(cur, struct pcpu, list);
+ free_pcpu(pcpu);
+ }
+ }
+
+ mutex_unlock(&xen_pcpu_lock);
+
+ return err;
+}
+
+static void xen_pcpu_work_fn(struct work_struct *work)
+{
+ xen_sync_pcpus();
+}
+static DECLARE_WORK(xen_pcpu_work, xen_pcpu_work_fn);
+
+static irqreturn_t xen_pcpu_interrupt(int irq, void *dev_id)
+{
+ schedule_work(&xen_pcpu_work);
+ return IRQ_HANDLED;
+}
+
+static int __init xen_pcpu_init(void)
+{
+ int irq, ret;
+
+ if (!xen_initial_domain())
+ return -ENODEV;
+
+ irq = bind_virq_to_irqhandler(VIRQ_PCPU_STATE, 0,
+ xen_pcpu_interrupt, 0,
+ "xen-pcpu", NULL);
+ if (irq < 0) {
+ pr_warning(XEN_PCPU "Failed to bind pcpu virq\n");
+ return irq;
+ }
+
+ ret = subsys_system_register(&xen_pcpu_subsys, NULL);
+ if (ret) {
+ pr_warning(XEN_PCPU "Failed to register pcpu subsys\n");
+ goto err1;
+ }
+
+ ret = xen_sync_pcpus();
+ if (ret) {
+ pr_warning(XEN_PCPU "Failed to sync pcpu info\n");
+ goto err2;
+ }
+
+ return 0;
+
+err2:
+ bus_unregister(&xen_pcpu_subsys);
+err1:
+ unbind_from_irqhandler(irq, NULL);
+ return ret;
+}
+arch_initcall(xen_pcpu_init);
diff --git a/include/xen/interface/platform.h b/include/xen/interface/platform.h
index 486653f..61fa661 100644
--- a/include/xen/interface/platform.h
+++ b/include/xen/interface/platform.h
@@ -314,6 +314,13 @@ struct xenpf_pcpuinfo {
};
DEFINE_GUEST_HANDLE_STRUCT(xenpf_pcpuinfo);

+#define XENPF_cpu_online 56
+#define XENPF_cpu_offline 57
+struct xenpf_cpu_ol {
+ uint32_t cpuid;
+};
+DEFINE_GUEST_HANDLE_STRUCT(xenpf_cpu_ol);
+
struct xen_platform_op {
uint32_t cmd;
uint32_t interface_version; /* XENPF_INTERFACE_VERSION */
@@ -330,6 +337,7 @@ struct xen_platform_op {
struct xenpf_getidletime getidletime;
struct xenpf_set_processor_pminfo set_pminfo;
struct xenpf_pcpuinfo pcpu_info;
+ struct xenpf_cpu_ol cpu_ol;
uint8_t pad[128];
} u;
};
diff --git a/include/xen/interface/xen.h b/include/xen/interface/xen.h
index a890804..0801468 100644
--- a/include/xen/interface/xen.h
+++ b/include/xen/interface/xen.h
@@ -80,6 +80,7 @@
#define VIRQ_CONSOLE 2 /* (DOM0) Bytes received on emergency console. */
#define VIRQ_DOM_EXC 3 /* (DOM0) Exceptional event for some domain. */
#define VIRQ_DEBUGGER 6 /* (DOM0) A domain has paused for debugging. */
+#define VIRQ_PCPU_STATE 9 /* (DOM0) PCPU state changed */

/* Architecture-specific VIRQ definitions. */
#define VIRQ_ARCH_0 16
--
1.7.1


Attachments:
0003-Xen-physical-cpus-interface.patch (12.31 kB)
0003-Xen-physical-cpus-interface.patch

2012-05-11 14:11:07

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: Re: [PATCH 3/3] Xen physical cpus interface (V2)

On Thu, May 10, 2012 at 03:06:14PM +0000, Liu, Jinsong wrote:
> >From 7aa4cf7c1172e24611dc76163397b8708acacc59 Mon Sep 17 00:00:00 2001
> From: Liu, Jinsong <[email protected]>
> Date: Fri, 11 May 2012 06:55:35 +0800
> Subject: [PATCH 3/3] Xen physical cpus interface
>
> Manage physical cpus in dom0, get physical cpus info and provide sys interface.
>
> Signed-off-by: Liu, Jinsong <[email protected]>
> Signed-off-by: Jiang, Yunhong <[email protected]>
> ---
> .../ABI/testing/sysfs-devices-system-xen_cpu | 20 +
> drivers/xen/Makefile | 1 +
> drivers/xen/pcpu.c | 374 ++++++++++++++++++++
> include/xen/interface/platform.h | 8 +
> include/xen/interface/xen.h | 1 +
> 5 files changed, 404 insertions(+), 0 deletions(-)
> create mode 100644 Documentation/ABI/testing/sysfs-devices-system-xen_cpu
> create mode 100644 drivers/xen/pcpu.c
>
> diff --git a/Documentation/ABI/testing/sysfs-devices-system-xen_cpu b/Documentation/ABI/testing/sysfs-devices-system-xen_cpu
> new file mode 100644
> index 0000000..9ca02fb
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-devices-system-xen_cpu
> @@ -0,0 +1,20 @@
> +What: /sys/devices/system/xen_cpu/
> +Date: May 2012
> +Contact: Liu, Jinsong <[email protected]>
> +Description:
> + A collection of global/individual Xen physical cpu attributes
> +
> + Individual physical cpu attributes are contained in
> + subdirectories named by the Xen's logical cpu number, e.g.:
> + /sys/devices/system/xen_cpu/xen_cpu#/
> +
> +
> +What: /sys/devices/system/xen_cpu/xen_cpu#/online
> +Date: May 2012
> +Contact: Liu, Jinsong <[email protected]>
> +Description:
> + Interface to online/offline Xen physical cpus
> +
> + When running under Xen platform, it provide user interface
> + to online/offline physical cpus, except cpu0 due to several
> + logic restrictions and assumptions.
> diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
> index 1d3e763..d12d14d 100644
> --- a/drivers/xen/Makefile
> +++ b/drivers/xen/Makefile
> @@ -19,6 +19,7 @@ obj-$(CONFIG_XEN_PVHVM) += platform-pci.o
> obj-$(CONFIG_XEN_TMEM) += tmem.o
> obj-$(CONFIG_SWIOTLB_XEN) += swiotlb-xen.o
> obj-$(CONFIG_XEN_DOM0) += pci.o
> +obj-$(CONFIG_XEN_DOM0) += pcpu.o
> obj-$(CONFIG_XEN_PCIDEV_BACKEND) += xen-pciback/
> obj-$(CONFIG_XEN_PRIVCMD) += xen-privcmd.o
> obj-$(CONFIG_XEN_ACPI_PROCESSOR) += xen-acpi-processor.o
> diff --git a/drivers/xen/pcpu.c b/drivers/xen/pcpu.c
> new file mode 100644
> index 0000000..9014ff1
> --- /dev/null
> +++ b/drivers/xen/pcpu.c
> @@ -0,0 +1,374 @@
> +/******************************************************************************
> + * pcpu.c
> + * Management physical cpu in dom0, get pcpu info and provide sys interface
> + *
> + * Copyright (c) 2012 Intel Corporation
> + * Author: Liu, Jinsong <[email protected]>
> + * Author: Jiang, Yunhong <[email protected]>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License version 2
> + * as published by the Free Software Foundation; or, when distributed
> + * separately from the Linux kernel or incorporated into other
> + * software packages, subject to the following license:
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this source file (the "Software"), to deal in the Software without
> + * restriction, including without limitation the rights to use, copy, modify,
> + * merge, publish, distribute, sublicense, and/or sell copies of the Software,
> + * and to permit persons to whom the Software is furnished to do so, subject to
> + * the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + */
> +
> +#include <linux/interrupt.h>
> +#include <linux/spinlock.h>
> +#include <linux/cpu.h>
> +#include <linux/stat.h>
> +#include <linux/capability.h>
> +
> +#include <xen/xen.h>
> +#include <xen/xenbus.h>
> +#include <xen/events.h>
> +#include <xen/interface/platform.h>
> +#include <asm/xen/hypervisor.h>
> +#include <asm/xen/hypercall.h>
> +
> +#define XEN_PCPU "xen_cpu: "
> +
> +/*
> + * @cpu_id: Xen physical cpu logic number
> + * @flags: Xen physical cpu status flag
> + * - XEN_PCPU_FLAGS_ONLINE: cpu is online
> + * - XEN_PCPU_FLAGS_INVALID: cpu is not present
> + */
> +struct pcpu {
> + struct list_head list;
> + struct device dev;
> + uint32_t cpu_id;
> + uint32_t flags;
> +};
> +
> +static struct bus_type xen_pcpu_subsys = {
> + .name = "xen_cpu",
> + .dev_name = "xen_cpu",
> +};
> +
> +static DEFINE_MUTEX(xen_pcpu_lock);
> +
> +static LIST_HEAD(xen_pcpus);

So what about the recommendation to get rid of that and
instead do

struct pcpu *xen_cpu;

and use that as a list? Meaning
.. snip..
> +{
> + struct list_head *cur;
> + struct pcpu *pcpu;
> +
> + list_for_each(cur, &xen_pcpus) {
> + pcpu = list_entry(cur, struct pcpu, list);
> + if (pcpu->cpu_id == cpu_id)
> + return pcpu;
> + }

do:
struct pcpu *pcpu;

list_for_each_entry(pcpu, xen_pcpus, list)
if (pcpu->cpu_id == cpu_id)
return pcpu;
?

and such.

2012-05-11 14:24:28

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: Re: [PATCH 3/3] Xen physical cpus interface (V2)

> +static void pcpu_sys_remove(struct pcpu *pcpu)
> +{
> + struct device *dev;
> +
> + if (!pcpu)
> + return;
> +
> + dev = &pcpu->dev;
> + if (dev->id)
> + device_remove_file(dev, &dev_attr_online);
> +
> + device_unregister(dev);
> +}
> +
> +static void free_pcpu(struct pcpu *pcpu)

1) So this isn't just freeing the PCPU, it also unregisters
the SysFS.

As such you should call this differently:

unregister_and_free(struct pcpu *pcpu)

or do the unregistration part seperatly.

2) But there is also another issue - a possiblity of race.

The user might be running:
while (true)
do
echo 1 > online
echo 0 > online
done

and the the sync_pcpu will end up calling pcpu_sys_remove and
free the pcpu. The SysFS aren't deleted until all of the
object references (kref's) have been put. So when you get to
'kfree(pcpu)' you might be still holding a reference (meaning
the user is doing 'echo 0 > online') and crash.

I think you need to hold the mutex in the store_online() function
(not good), or better yet, make a device_release function
that will be called when the last kref has been put.


3) Third the name. These functions all depend on the mutex lock being
held. As such add a postfix to the name of the function of
"_locked" or a prefix of "__' so it is known that the caller holds
the mutex.

> +{
> + if (!pcpu)
> + return;
> +
> + pcpu_sys_remove(pcpu);
> +
> + list_del(&pcpu->list);
> + kfree(pcpu);
> +}
> +
> +static int pcpu_sys_create(struct pcpu *pcpu)
> +{
> + struct device *dev;
> + int err = -EINVAL;
> +
> + if (!pcpu)
> + return err;
> +
> + dev = &pcpu->dev;
> + dev->bus = &xen_pcpu_subsys;
> + dev->id = pcpu->cpu_id;
> +
> + err = device_register(dev);
> + if (err)
> + return err;
> +
> + /*
> + * Xen never offline cpu0 due to several restrictions
> + * and assumptions. This basically doesn't add a sys control
> + * to user, one cannot attempt to offline BSP.
> + */
> + if (dev->id) {
> + err = device_create_file(dev, &dev_attr_online);
> + if (err) {
> + device_unregister(dev);
> + return err;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static struct pcpu *init_pcpu(struct xenpf_pcpuinfo *info)
> +{
> + struct pcpu *pcpu;
> + int err;
> +
> + if (info->flags & XEN_PCPU_FLAGS_INVALID)
> + return ERR_PTR(-ENODEV);
> +
> + pcpu = kzalloc(sizeof(struct pcpu), GFP_KERNEL);
> + if (!pcpu)
> + return ERR_PTR(-ENOMEM);
> +
> + INIT_LIST_HEAD(&pcpu->list);
> + pcpu->cpu_id = info->xen_cpuid;
> + pcpu->flags = info->flags;
> +
> + err = pcpu_sys_create(pcpu);
> + if (err) {
> + pr_warning(XEN_PCPU "Failed to register pcpu%u\n",
> + info->xen_cpuid);
> + kfree(pcpu);
> + return ERR_PTR(-ENOENT);
> + }
> +
> + /* Need hold on xen_pcpu_lock before pcpu list manipulations */
> + list_add_tail(&pcpu->list, &xen_pcpus);
> + return pcpu;
> +}
> +
> +/*
> + * Caller should hold the xen_pcpu_lock
> + */
> +static int sync_pcpu(uint32_t cpu, uint32_t *max_cpu)
> +{
> + int ret;
> + struct pcpu *pcpu = NULL;
> + struct xenpf_pcpuinfo *info;
> + struct xen_platform_op op = {
> + .cmd = XENPF_get_cpuinfo,
> + .interface_version = XENPF_INTERFACE_VERSION,
> + .u.pcpu_info.xen_cpuid = cpu,
> + };
> +
> + ret = HYPERVISOR_dom0_op(&op);
> + if (ret)
> + return ret;
> +
> + info = &op.u.pcpu_info;
> + if (max_cpu)
> + *max_cpu = info->max_present;
> +
> + pcpu = get_pcpu(cpu);
> +
> + if (info->flags & XEN_PCPU_FLAGS_INVALID) {
> + /* The pcpu has been removed */
> + if (pcpu)
> + free_pcpu(pcpu);
> + return 0;
> + }
> +
> + if (!pcpu) {
> + pcpu = init_pcpu(info);
> + if (IS_ERR_OR_NULL(pcpu))
> + return -ENODEV;
> + } else
> + pcpu_online_status(info, pcpu);
> +
> + return 0;
> +}
> +
> +/*
> + * Sync dom0's pcpu information with xen hypervisor's
> + */
> +static int xen_sync_pcpus(void)
> +{
> + /*
> + * Boot cpu always have cpu_id 0 in xen
> + */
> + uint32_t cpu = 0, max_cpu = 0;
> + int err = 0;
> + struct list_head *cur, *tmp;
> + struct pcpu *pcpu;
> +
> + mutex_lock(&xen_pcpu_lock);
> +
> + while (!err && (cpu <= max_cpu)) {
> + err = sync_pcpu(cpu, &max_cpu);
> + cpu++;
> + }
> +
> + if (err) {
> + list_for_each_safe(cur, tmp, &xen_pcpus) {
> + pcpu = list_entry(cur, struct pcpu, list);
> + free_pcpu(pcpu);
> + }
> + }
> +
> + mutex_unlock(&xen_pcpu_lock);
> +
> + return err;
> +}
> +
> +static void xen_pcpu_work_fn(struct work_struct *work)
> +{
> + xen_sync_pcpus();
> +}
> +static DECLARE_WORK(xen_pcpu_work, xen_pcpu_work_fn);
> +
> +static irqreturn_t xen_pcpu_interrupt(int irq, void *dev_id)
> +{
> + schedule_work(&xen_pcpu_work);
> + return IRQ_HANDLED;
> +}
> +
> +static int __init xen_pcpu_init(void)
> +{
> + int irq, ret;
> +
> + if (!xen_initial_domain())
> + return -ENODEV;
> +
> + irq = bind_virq_to_irqhandler(VIRQ_PCPU_STATE, 0,
> + xen_pcpu_interrupt, 0,
> + "xen-pcpu", NULL);
> + if (irq < 0) {
> + pr_warning(XEN_PCPU "Failed to bind pcpu virq\n");
> + return irq;
> + }
> +
> + ret = subsys_system_register(&xen_pcpu_subsys, NULL);
> + if (ret) {
> + pr_warning(XEN_PCPU "Failed to register pcpu subsys\n");
> + goto err1;
> + }
> +
> + ret = xen_sync_pcpus();
> + if (ret) {
> + pr_warning(XEN_PCPU "Failed to sync pcpu info\n");
> + goto err2;
> + }
> +
> + return 0;
> +
> +err2:
> + bus_unregister(&xen_pcpu_subsys);
> +err1:
> + unbind_from_irqhandler(irq, NULL);
> + return ret;
> +}
> +arch_initcall(xen_pcpu_init);
> diff --git a/include/xen/interface/platform.h b/include/xen/interface/platform.h
> index 486653f..61fa661 100644
> --- a/include/xen/interface/platform.h
> +++ b/include/xen/interface/platform.h
> @@ -314,6 +314,13 @@ struct xenpf_pcpuinfo {
> };
> DEFINE_GUEST_HANDLE_STRUCT(xenpf_pcpuinfo);
>
> +#define XENPF_cpu_online 56
> +#define XENPF_cpu_offline 57
> +struct xenpf_cpu_ol {
> + uint32_t cpuid;
> +};
> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_cpu_ol);
> +
> struct xen_platform_op {
> uint32_t cmd;
> uint32_t interface_version; /* XENPF_INTERFACE_VERSION */
> @@ -330,6 +337,7 @@ struct xen_platform_op {
> struct xenpf_getidletime getidletime;
> struct xenpf_set_processor_pminfo set_pminfo;
> struct xenpf_pcpuinfo pcpu_info;
> + struct xenpf_cpu_ol cpu_ol;
> uint8_t pad[128];
> } u;
> };
> diff --git a/include/xen/interface/xen.h b/include/xen/interface/xen.h
> index a890804..0801468 100644
> --- a/include/xen/interface/xen.h
> +++ b/include/xen/interface/xen.h
> @@ -80,6 +80,7 @@
> #define VIRQ_CONSOLE 2 /* (DOM0) Bytes received on emergency console. */
> #define VIRQ_DOM_EXC 3 /* (DOM0) Exceptional event for some domain. */
> #define VIRQ_DEBUGGER 6 /* (DOM0) A domain has paused for debugging. */
> +#define VIRQ_PCPU_STATE 9 /* (DOM0) PCPU state changed */
>
> /* Architecture-specific VIRQ definitions. */
> #define VIRQ_ARCH_0 16
> --
> 1.7.1

2012-05-11 16:58:27

by Liu, Jinsong

[permalink] [raw]
Subject: RE: [PATCH 3/3] Xen physical cpus interface (V2)

Konrad Rzeszutek Wilk wrote:
> On Thu, May 10, 2012 at 03:06:14PM +0000, Liu, Jinsong wrote:
>>> From 7aa4cf7c1172e24611dc76163397b8708acacc59 Mon Sep 17 00:00:00
>>> 2001
>> From: Liu, Jinsong <[email protected]>
>> Date: Fri, 11 May 2012 06:55:35 +0800
>> Subject: [PATCH 3/3] Xen physical cpus interface
>>
>> Manage physical cpus in dom0, get physical cpus info and provide sys
>> interface.
>>
>> Signed-off-by: Liu, Jinsong <[email protected]>
>> Signed-off-by: Jiang, Yunhong <[email protected]> ---
>> .../ABI/testing/sysfs-devices-system-xen_cpu | 20 +
>> drivers/xen/Makefile | 1 +
>> drivers/xen/pcpu.c | 374
>> ++++++++++++++++++++ include/xen/interface/platform.h
>> | 8 + include/xen/interface/xen.h | 1 +
>> 5 files changed, 404 insertions(+), 0 deletions(-)
>> create mode 100644
>> Documentation/ABI/testing/sysfs-devices-system-xen_cpu create mode
>> 100644 drivers/xen/pcpu.c
>>
>> diff --git a/Documentation/ABI/testing/sysfs-devices-system-xen_cpu
>> b/Documentation/ABI/testing/sysfs-devices-system-xen_cpu new file
>> mode 100644 index 0000000..9ca02fb --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-devices-system-xen_cpu @@ -0,0
>> +1,20 @@ +What: /sys/devices/system/xen_cpu/
>> +Date: May 2012
>> +Contact: Liu, Jinsong <[email protected]>
>> +Description:
>> + A collection of global/individual Xen physical cpu attributes +
>> + Individual physical cpu attributes are contained in
>> + subdirectories named by the Xen's logical cpu number, e.g.:
>> + /sys/devices/system/xen_cpu/xen_cpu#/
>> +
>> +
>> +What: /sys/devices/system/xen_cpu/xen_cpu#/online +Date: May 2012
>> +Contact: Liu, Jinsong <[email protected]>
>> +Description:
>> + Interface to online/offline Xen physical cpus
>> +
>> + When running under Xen platform, it provide user interface
>> + to online/offline physical cpus, except cpu0 due to several
>> + logic restrictions and assumptions.
>> diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
>> index 1d3e763..d12d14d 100644
>> --- a/drivers/xen/Makefile
>> +++ b/drivers/xen/Makefile
>> @@ -19,6 +19,7 @@ obj-$(CONFIG_XEN_PVHVM) += platform-pci.o
>> obj-$(CONFIG_XEN_TMEM) += tmem.o
>> obj-$(CONFIG_SWIOTLB_XEN) += swiotlb-xen.o
>> obj-$(CONFIG_XEN_DOM0) += pci.o
>> +obj-$(CONFIG_XEN_DOM0) += pcpu.o
>> obj-$(CONFIG_XEN_PCIDEV_BACKEND) += xen-pciback/
>> obj-$(CONFIG_XEN_PRIVCMD) += xen-privcmd.o
>> obj-$(CONFIG_XEN_ACPI_PROCESSOR) += xen-acpi-processor.o
>> diff --git a/drivers/xen/pcpu.c b/drivers/xen/pcpu.c
>> new file mode 100644
>> index 0000000..9014ff1
>> --- /dev/null
>> +++ b/drivers/xen/pcpu.c
>> @@ -0,0 +1,374 @@
>> +/******************************************************************************
>> + * pcpu.c + * Management physical cpu in dom0, get pcpu info and
>> provide sys interface + * + * Copyright (c) 2012 Intel Corporation
>> + * Author: Liu, Jinsong <[email protected]>
>> + * Author: Jiang, Yunhong <[email protected]> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License
>> version 2 + * as published by the Free Software Foundation; or, when
>> distributed + * separately from the Linux kernel or incorporated
>> into other + * software packages, subject to the following license:
>> + * + * Permission is hereby granted, free of charge, to any person
>> obtaining a copy + * of this source file (the "Software"), to deal
>> in the Software without + * restriction, including without
>> limitation the rights to use, copy, modify, + * merge, publish,
>> distribute, sublicense, and/or sell copies of the Software, + * and
>> to permit persons to whom the Software is furnished to do so,
>> subject to + * the following conditions: + * + * The above copyright
>> notice and this permission notice shall be included in + * all
>> copies or substantial portions of the Software. + * + * THE SOFTWARE
>> IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + *
>> IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>> MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND
>> NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT
>> HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY,
>> WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + *
>> FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
>> DEALINGS + * IN THE SOFTWARE. + */ + +#include <linux/interrupt.h>
>> +#include <linux/spinlock.h>
>> +#include <linux/cpu.h>
>> +#include <linux/stat.h>
>> +#include <linux/capability.h>
>> +
>> +#include <xen/xen.h>
>> +#include <xen/xenbus.h>
>> +#include <xen/events.h>
>> +#include <xen/interface/platform.h>
>> +#include <asm/xen/hypervisor.h>
>> +#include <asm/xen/hypercall.h>
>> +
>> +#define XEN_PCPU "xen_cpu: "
>> +
>> +/*
>> + * @cpu_id: Xen physical cpu logic number
>> + * @flags: Xen physical cpu status flag
>> + * - XEN_PCPU_FLAGS_ONLINE: cpu is online
>> + * - XEN_PCPU_FLAGS_INVALID: cpu is not present
>> + */
>> +struct pcpu {
>> + struct list_head list;
>> + struct device dev;
>> + uint32_t cpu_id;
>> + uint32_t flags;
>> +};
>> +
>> +static struct bus_type xen_pcpu_subsys = {
>> + .name = "xen_cpu",
>> + .dev_name = "xen_cpu",
>> +};
>> +
>> +static DEFINE_MUTEX(xen_pcpu_lock);
>> +
>> +static LIST_HEAD(xen_pcpus);
>
> So what about the recommendation to get rid of that and
> instead do
>
> struct pcpu *xen_cpu;

I'm not quite clear your meaning here, do you mean 'LIST_HEAD(xen_pcpus)' instead of 'struct pcpu *xen_cpu'?


>
> and use that as a list? Meaning
> .. snip..
>> +{
>> + struct list_head *cur;
>> + struct pcpu *pcpu;
>> +
>> + list_for_each(cur, &xen_pcpus) {
>> + pcpu = list_entry(cur, struct pcpu, list);
>> + if (pcpu->cpu_id == cpu_id)
>> + return pcpu;
>> + }
>
> do:
> struct pcpu *pcpu;
>
> list_for_each_entry(pcpu, xen_pcpus, list)
> if (pcpu->cpu_id == cpu_id)
> return pcpu;
> ?
>
> and such.

OK for me to update.

Thanks,
Jinsong

2012-05-11 17:28:43

by Liu, Jinsong

[permalink] [raw]
Subject: RE: [PATCH 3/3] Xen physical cpus interface (V2)

Konrad Rzeszutek Wilk wrote:
>> +static void pcpu_sys_remove(struct pcpu *pcpu)
>> +{
>> + struct device *dev;
>> +
>> + if (!pcpu)
>> + return;
>> +
>> + dev = &pcpu->dev;
>> + if (dev->id)
>> + device_remove_file(dev, &dev_attr_online);
>> +
>> + device_unregister(dev);
>> +}
>> +
>> +static void free_pcpu(struct pcpu *pcpu)
>
> 1) So this isn't just freeing the PCPU, it also unregisters
> the SysFS.
>
> As such you should call this differently:
>
> unregister_and_free(struct pcpu *pcpu)

Fine, rename 2 funcs:
init_pcpu --> int_and_register_pcpu
free_pcpu --> unregister_and_free_pcpu

>
> or do the unregistration part seperatly.
>
> 2) But there is also another issue - a possiblity of race.
>
> The user might be running:
> while (true)
> do
> echo 1 > online
> echo 0 > online
> done
>
> and the the sync_pcpu will end up calling pcpu_sys_remove and
> free the pcpu. The SysFS aren't deleted until all of the

No race here. echo x > online would not change cpu present map.

> object references (kref's) have been put. So when you get to
> 'kfree(pcpu)' you might be still holding a reference (meaning
> the user is doing 'echo 0 > online') and crash.
>
> I think you need to hold the mutex in the store_online() function
> (not good), or better yet, make a device_release function
> that will be called when the last kref has been put.
>
>
> 3) Third the name. These functions all depend on the mutex lock being
> held. As such add a postfix to the name of the function of
> "_locked" or a prefix of "__' so it is known that the caller holds
> the mutex.

OK, add _locked postfix to these functions.

Thanks,
Jinsong

>
>> +{
>> + if (!pcpu)
>> + return;
>> +
>> + pcpu_sys_remove(pcpu);
>> +
>> + list_del(&pcpu->list);
>> + kfree(pcpu);
>> +}
>> +
>> +static int pcpu_sys_create(struct pcpu *pcpu)
>> +{
>> + struct device *dev;
>> + int err = -EINVAL;
>> +
>> + if (!pcpu)
>> + return err;
>> +
>> + dev = &pcpu->dev;
>> + dev->bus = &xen_pcpu_subsys;
>> + dev->id = pcpu->cpu_id;
>> +
>> + err = device_register(dev);
>> + if (err)
>> + return err;
>> +
>> + /*
>> + * Xen never offline cpu0 due to several restrictions
>> + * and assumptions. This basically doesn't add a sys control
>> + * to user, one cannot attempt to offline BSP.
>> + */
>> + if (dev->id) {
>> + err = device_create_file(dev, &dev_attr_online); + if (err) {
>> + device_unregister(dev);
>> + return err;
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static struct pcpu *init_pcpu(struct xenpf_pcpuinfo *info) +{
>> + struct pcpu *pcpu;
>> + int err;
>> +
>> + if (info->flags & XEN_PCPU_FLAGS_INVALID)
>> + return ERR_PTR(-ENODEV);
>> +
>> + pcpu = kzalloc(sizeof(struct pcpu), GFP_KERNEL);
>> + if (!pcpu)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + INIT_LIST_HEAD(&pcpu->list);
>> + pcpu->cpu_id = info->xen_cpuid;
>> + pcpu->flags = info->flags;
>> +
>> + err = pcpu_sys_create(pcpu);
>> + if (err) {
>> + pr_warning(XEN_PCPU "Failed to register pcpu%u\n", +
>> info->xen_cpuid); + kfree(pcpu);
>> + return ERR_PTR(-ENOENT);
>> + }
>> +
>> + /* Need hold on xen_pcpu_lock before pcpu list manipulations */
>> + list_add_tail(&pcpu->list, &xen_pcpus);
>> + return pcpu;
>> +}
>> +
>> +/*
>> + * Caller should hold the xen_pcpu_lock
>> + */
>> +static int sync_pcpu(uint32_t cpu, uint32_t *max_cpu) +{
>> + int ret;
>> + struct pcpu *pcpu = NULL;
>> + struct xenpf_pcpuinfo *info;
>> + struct xen_platform_op op = {
>> + .cmd = XENPF_get_cpuinfo,
>> + .interface_version = XENPF_INTERFACE_VERSION,
>> + .u.pcpu_info.xen_cpuid = cpu,
>> + };
>> +
>> + ret = HYPERVISOR_dom0_op(&op);
>> + if (ret)
>> + return ret;
>> +
>> + info = &op.u.pcpu_info;
>> + if (max_cpu)
>> + *max_cpu = info->max_present;
>> +
>> + pcpu = get_pcpu(cpu);
>> +
>> + if (info->flags & XEN_PCPU_FLAGS_INVALID) {
>> + /* The pcpu has been removed */
>> + if (pcpu)
>> + free_pcpu(pcpu);
>> + return 0;
>> + }
>> +
>> + if (!pcpu) {
>> + pcpu = init_pcpu(info);
>> + if (IS_ERR_OR_NULL(pcpu))
>> + return -ENODEV;
>> + } else
>> + pcpu_online_status(info, pcpu);
>> +
>> + return 0;
>> +}
>> +
>> +/*
>> + * Sync dom0's pcpu information with xen hypervisor's + */
>> +static int xen_sync_pcpus(void)
>> +{
>> + /*
>> + * Boot cpu always have cpu_id 0 in xen
>> + */
>> + uint32_t cpu = 0, max_cpu = 0;
>> + int err = 0;
>> + struct list_head *cur, *tmp;
>> + struct pcpu *pcpu;
>> +
>> + mutex_lock(&xen_pcpu_lock);
>> +
>> + while (!err && (cpu <= max_cpu)) {
>> + err = sync_pcpu(cpu, &max_cpu);
>> + cpu++;
>> + }
>> +
>> + if (err) {
>> + list_for_each_safe(cur, tmp, &xen_pcpus) {
>> + pcpu = list_entry(cur, struct pcpu, list);
>> + free_pcpu(pcpu);
>> + }
>> + }
>> +
>> + mutex_unlock(&xen_pcpu_lock);
>> +
>> + return err;
>> +}
>> +
>> +static void xen_pcpu_work_fn(struct work_struct *work) +{
>> + xen_sync_pcpus();
>> +}
>> +static DECLARE_WORK(xen_pcpu_work, xen_pcpu_work_fn); +
>> +static irqreturn_t xen_pcpu_interrupt(int irq, void *dev_id) +{
>> + schedule_work(&xen_pcpu_work);
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static int __init xen_pcpu_init(void)
>> +{
>> + int irq, ret;
>> +
>> + if (!xen_initial_domain())
>> + return -ENODEV;
>> +
>> + irq = bind_virq_to_irqhandler(VIRQ_PCPU_STATE, 0,
>> + xen_pcpu_interrupt, 0,
>> + "xen-pcpu", NULL);
>> + if (irq < 0) {
>> + pr_warning(XEN_PCPU "Failed to bind pcpu virq\n"); + return irq;
>> + }
>> +
>> + ret = subsys_system_register(&xen_pcpu_subsys, NULL); + if (ret) {
>> + pr_warning(XEN_PCPU "Failed to register pcpu subsys\n"); + goto
>> err1; + }
>> +
>> + ret = xen_sync_pcpus();
>> + if (ret) {
>> + pr_warning(XEN_PCPU "Failed to sync pcpu info\n"); + goto err2;
>> + }
>> +
>> + return 0;
>> +
>> +err2:
>> + bus_unregister(&xen_pcpu_subsys);
>> +err1:
>> + unbind_from_irqhandler(irq, NULL);
>> + return ret;
>> +}
>> +arch_initcall(xen_pcpu_init);
>> diff --git a/include/xen/interface/platform.h
>> b/include/xen/interface/platform.h index 486653f..61fa661 100644 ---
>> a/include/xen/interface/platform.h +++
>> b/include/xen/interface/platform.h @@ -314,6 +314,13 @@ struct
>> xenpf_pcpuinfo { };
>> DEFINE_GUEST_HANDLE_STRUCT(xenpf_pcpuinfo);
>>
>> +#define XENPF_cpu_online 56
>> +#define XENPF_cpu_offline 57
>> +struct xenpf_cpu_ol {
>> + uint32_t cpuid;
>> +};
>> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_cpu_ol);
>> +
>> struct xen_platform_op {
>> uint32_t cmd;
>> uint32_t interface_version; /* XENPF_INTERFACE_VERSION */
>> @@ -330,6 +337,7 @@ struct xen_platform_op {
>> struct xenpf_getidletime getidletime;
>> struct xenpf_set_processor_pminfo set_pminfo;
>> struct xenpf_pcpuinfo pcpu_info;
>> + struct xenpf_cpu_ol cpu_ol;
>> uint8_t pad[128];
>> } u;
>> };
>> diff --git a/include/xen/interface/xen.h
>> b/include/xen/interface/xen.h
>> index a890804..0801468 100644
>> --- a/include/xen/interface/xen.h
>> +++ b/include/xen/interface/xen.h
>> @@ -80,6 +80,7 @@
>> #define VIRQ_CONSOLE 2 /* (DOM0) Bytes received on emergency
>> console. */ #define VIRQ_DOM_EXC 3 /* (DOM0) Exceptional event
>> for some domain. */ #define VIRQ_DEBUGGER 6 /* (DOM0) A domain
>> has paused for debugging. */ +#define VIRQ_PCPU_STATE 9 /* (DOM0)
>> PCPU state changed */
>>
>> /* Architecture-specific VIRQ definitions. */
>> #define VIRQ_ARCH_0 16
>> --
>> 1.7.1

2012-05-11 19:37:49

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: Re: [PATCH 3/3] Xen physical cpus interface (V2)

> >> +struct pcpu {
> >> + struct list_head list;
> >> + struct device dev;
> >> + uint32_t cpu_id;
> >> + uint32_t flags;
> >> +};
> >> +
> >> +static struct bus_type xen_pcpu_subsys = {
> >> + .name = "xen_cpu",
> >> + .dev_name = "xen_cpu",
> >> +};
> >> +
> >> +static DEFINE_MUTEX(xen_pcpu_lock);
> >> +
> >> +static LIST_HEAD(xen_pcpus);
> >
> > So what about the recommendation to get rid of that and
> > instead do
> >
> > struct pcpu *xen_cpu;
>
> I'm not quite clear your meaning here, do you mean 'LIST_HEAD(xen_pcpus)' instead of 'struct pcpu *xen_cpu'?

No. Just use the embedded 'struct list_head' inside of 'struct pcpu'
as your iterator.

And your first 'struct pcpu' won't ever be deleted (as it is for
CPU0), so you can iterate from that.