[PATCH 3/3] Xen physical cpus interface (V2)

[PATCH 3/3] Xen physical cpus interface (V2)

[Liu, Jinsong]

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

>From 7aa4cf7c1172e24611dc76163397b8708acacc59 Mon Sep 17 00:00:00 2001
From: Liu, Jinsong <jinsong.liu@intel.com>
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 <jinsong.liu@intel.com>
Signed-off-by: Jiang, Yunhong <yunhong.jiang@intel.com>
---
 .../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 <jinsong.liu@intel.com>
+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 <jinsong.liu@intel.com>
+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 <jinsong.liu@intel.com>
+ * Author: Jiang, Yunhong <yunhong.jiang@intel.com>
+ *
+ * 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

[-- Attachment #2: 0003-Xen-physical-cpus-interface.patch --]
[-- Type: application/octet-stream, Size: 12603 bytes --]

From 7aa4cf7c1172e24611dc76163397b8708acacc59 Mon Sep 17 00:00:00 2001
From: Liu, Jinsong <jinsong.liu@intel.com>
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 <jinsong.liu@intel.com>
Signed-off-by: Jiang, Yunhong <yunhong.jiang@intel.com>
---
 .../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 <jinsong.liu@intel.com>
+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 <jinsong.liu@intel.com>
+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 <jinsong.liu@intel.com>
+ * Author: Jiang, Yunhong <yunhong.jiang@intel.com>
+ *
+ * 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

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

[Konrad Rzeszutek Wilk]

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 <jinsong.liu@intel.com>
> 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 <jinsong.liu@intel.com>
> Signed-off-by: Jiang, Yunhong <yunhong.jiang@intel.com>
> ---
>  .../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 <jinsong.liu@intel.com>
> +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 <jinsong.liu@intel.com>
> +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 <jinsong.liu@intel.com>
> + * Author: Jiang, Yunhong <yunhong.jiang@intel.com>
> + *
> + * 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.

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

[Konrad Rzeszutek Wilk]

> +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

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

[Liu, Jinsong]

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 <jinsong.liu@intel.com>
>> 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 <jinsong.liu@intel.com>
>> Signed-off-by: Jiang, Yunhong <yunhong.jiang@intel.com> ---
>>  .../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 <jinsong.liu@intel.com>
>> +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 <jinsong.liu@intel.com>
>> +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 <jinsong.liu@intel.com>
>> + * Author: Jiang, Yunhong <yunhong.jiang@intel.com> + *
>> + * 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

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

[Liu, Jinsong]

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

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

[Konrad Rzeszutek Wilk]

> >> +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.