[PATCH v4 1/6] xen: sync xen header

[PATCH v4 1/6] xen: sync xen header

[Juergen Gross]

Import the actual version of include/xen/interface/sched.h from Xen.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 include/xen/interface/sched.h | 100 ++++++++++++++++++++++++++++++++++--------
 1 file changed, 82 insertions(+), 18 deletions(-)

diff --git a/include/xen/interface/sched.h b/include/xen/interface/sched.h
index f184909..a4c4d73 100644
--- a/include/xen/interface/sched.h
+++ b/include/xen/interface/sched.h
@@ -3,6 +3,24 @@
  *
  * Scheduler state interactions
  *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (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.
+ *
  * Copyright (c) 2005, Keir Fraser <keir@xensource.com>
  */
 
@@ -12,18 +30,30 @@
 #include <xen/interface/event_channel.h>
 
 /*
+ * Guest Scheduler Operations
+ *
+ * The SCHEDOP interface provides mechanisms for a guest to interact
+ * with the scheduler, including yield, blocking and shutting itself
+ * down.
+ */
+
+/*
  * The prototype for this hypercall is:
- *  long sched_op_new(int cmd, void *arg)
+ * long HYPERVISOR_sched_op(enum sched_op cmd, void *arg, ...)
+ *
  * @cmd == SCHEDOP_??? (scheduler operation).
  * @arg == Operation-specific extra argument(s), as described below.
+ * ...  == Additional Operation-specific extra arguments, described below.
  *
- * **NOTE**:
- * Versions of Xen prior to 3.0.2 provide only the following legacy version
+ * Versions of Xen prior to 3.0.2 provided only the following legacy version
  * of this hypercall, supporting only the commands yield, block and shutdown:
  *  long sched_op(int cmd, unsigned long arg)
  * @cmd == SCHEDOP_??? (scheduler operation).
  * @arg == 0               (SCHEDOP_yield and SCHEDOP_block)
  *      == SHUTDOWN_* code (SCHEDOP_shutdown)
+ *
+ * This legacy version is available to new guests as:
+ * long HYPERVISOR_sched_op_compat(enum sched_op cmd, unsigned long arg)
  */
 
 /*
@@ -44,12 +74,17 @@
 /*
  * Halt execution of this domain (all VCPUs) and notify the system controller.
  * @arg == pointer to sched_shutdown structure.
+ *
+ * If the sched_shutdown_t reason is SHUTDOWN_suspend then
+ * x86 PV guests must also set RDX (EDX for 32-bit guests) to the MFN
+ * of the guest's start info page.  RDX/EDX is the third hypercall
+ * argument.
+ *
+ * In addition, which reason is SHUTDOWN_suspend this hypercall
+ * returns 1 if suspend was cancelled or the domain was merely
+ * checkpointed, and 0 if it is resuming in a new domain.
  */
 #define SCHEDOP_shutdown    2
-struct sched_shutdown {
-    unsigned int reason; /* SHUTDOWN_* */
-};
-DEFINE_GUEST_HANDLE_STRUCT(sched_shutdown);
 
 /*
  * Poll a set of event-channel ports. Return when one or more are pending. An
@@ -57,12 +92,6 @@ DEFINE_GUEST_HANDLE_STRUCT(sched_shutdown);
  * @arg == pointer to sched_poll structure.
  */
 #define SCHEDOP_poll        3
-struct sched_poll {
-    GUEST_HANDLE(evtchn_port_t) ports;
-    unsigned int nr_ports;
-    uint64_t timeout;
-};
-DEFINE_GUEST_HANDLE_STRUCT(sched_poll);
 
 /*
  * Declare a shutdown for another domain. The main use of this function is
@@ -71,15 +100,11 @@ DEFINE_GUEST_HANDLE_STRUCT(sched_poll);
  * @arg == pointer to sched_remote_shutdown structure.
  */
 #define SCHEDOP_remote_shutdown        4
-struct sched_remote_shutdown {
-    domid_t domain_id;         /* Remote domain ID */
-    unsigned int reason;       /* SHUTDOWN_xxx reason */
-};
 
 /*
  * Latch a shutdown code, so that when the domain later shuts down it
  * reports this code to the control tools.
- * @arg == as for SCHEDOP_shutdown.
+ * @arg == sched_shutdown, as for SCHEDOP_shutdown.
  */
 #define SCHEDOP_shutdown_code 5
 
@@ -92,10 +117,47 @@ struct sched_remote_shutdown {
  * With id != 0 and timeout != 0, poke watchdog timer and set new timeout.
  */
 #define SCHEDOP_watchdog    6
+
+/*
+ * Override the current vcpu affinity by pinning it to one physical cpu or
+ * undo this override restoring the previous affinity.
+ * @arg == pointer to sched_pin_override structure.
+ *
+ * A negative pcpu value will undo a previous pin override and restore the
+ * previous cpu affinity.
+ * This call is allowed for the hardware domain only and requires the cpu
+ * to be part of the domain's cpupool.
+ */
+#define SCHEDOP_pin_override 7
+
+struct sched_shutdown {
+    unsigned int reason; /* SHUTDOWN_* => shutdown reason */
+};
+DEFINE_GUEST_HANDLE_STRUCT(sched_shutdown);
+
+struct sched_poll {
+    GUEST_HANDLE(evtchn_port_t) ports;
+    unsigned int nr_ports;
+    uint64_t timeout;
+};
+DEFINE_GUEST_HANDLE_STRUCT(sched_poll);
+
+struct sched_remote_shutdown {
+    domid_t domain_id;         /* Remote domain ID */
+    unsigned int reason;       /* SHUTDOWN_* => shutdown reason */
+};
+DEFINE_GUEST_HANDLE_STRUCT(sched_remote_shutdown);
+
 struct sched_watchdog {
     uint32_t id;                /* watchdog ID */
     uint32_t timeout;           /* timeout */
 };
+DEFINE_GUEST_HANDLE_STRUCT(sched_watchdog);
+
+struct sched_pin_override {
+    int32_t pcpu;
+};
+DEFINE_GUEST_HANDLE_STRUCT(sched_pin_override);
 
 /*
  * Reason codes for SCHEDOP_shutdown. These may be interpreted by control
@@ -107,6 +169,7 @@ struct sched_watchdog {
 #define SHUTDOWN_suspend    2  /* Clean up, save suspend info, kill.         */
 #define SHUTDOWN_crash      3  /* Tell controller we've crashed.             */
 #define SHUTDOWN_watchdog   4  /* Restart because watchdog time expired.     */
+
 /*
  * Domain asked to perform 'soft reset' for it. The expected behavior is to
  * reset internal Xen state for the domain returning it to the point where it
@@ -115,5 +178,6 @@ struct sched_watchdog {
  * interfaces again.
  */
 #define SHUTDOWN_soft_reset 5
+#define SHUTDOWN_MAX        5  /* Maximum valid shutdown reason.             */
 
 #endif /* __XEN_PUBLIC_SCHED_H__ */
-- 
2.6.2


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[PATCH v4 2/6] virt, sched: add generic vcpu pinning support

[Juergen Gross]

Add generic virtualization support for pinning the current vcpu to a
specified physical cpu. As this operation isn't performance critical
(a very limited set of operations like BIOS calls and SMIs is expected
to need this) just add a hypervisor specific indirection.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
V4: move this patch some places up in the series
    WARN_ONCE in case platform doesn't support pinning as requested by
    Peter Zijlstra

V3: use getc_cpu()/put_cpu() as suggested by David Vrabel

V2: adapt to using workqueues
    add include/linux/hypervisor.h to hide architecture specific stuff
    from generic kernel code

In case paravirt maintainers don't want to be responsible for
include/linux/hypervisor.h I could take it.
---
 MAINTAINERS                       |  1 +
 arch/x86/include/asm/hypervisor.h |  4 ++++
 arch/x86/kernel/cpu/hypervisor.c  | 11 +++++++++++
 include/linux/hypervisor.h        | 17 +++++++++++++++++
 kernel/smp.c                      |  1 +
 kernel/up.c                       |  1 +
 6 files changed, 35 insertions(+)
 create mode 100644 include/linux/hypervisor.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 2ec5079..959173e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8330,6 +8330,7 @@ S:	Supported
 F:	Documentation/virtual/paravirt_ops.txt
 F:	arch/*/kernel/paravirt*
 F:	arch/*/include/asm/paravirt.h
+F:	include/linux/hypervisor.h
 
 PARIDE DRIVERS FOR PARALLEL PORT IDE DEVICES
 M:	Tim Waugh <tim@cyberelk.net>
diff --git a/arch/x86/include/asm/hypervisor.h b/arch/x86/include/asm/hypervisor.h
index 055ea99..67942b6 100644
--- a/arch/x86/include/asm/hypervisor.h
+++ b/arch/x86/include/asm/hypervisor.h
@@ -43,6 +43,9 @@ struct hypervisor_x86 {
 
 	/* X2APIC detection (run once per boot) */
 	bool		(*x2apic_available)(void);
+
+	/* pin current vcpu to specified physical cpu (run rarely) */
+	void		(*pin_vcpu)(int);
 };
 
 extern const struct hypervisor_x86 *x86_hyper;
@@ -56,6 +59,7 @@ extern const struct hypervisor_x86 x86_hyper_kvm;
 extern void init_hypervisor(struct cpuinfo_x86 *c);
 extern void init_hypervisor_platform(void);
 extern bool hypervisor_x2apic_available(void);
+extern void hypervisor_pin_vcpu(int cpu);
 #else
 static inline void init_hypervisor(struct cpuinfo_x86 *c) { }
 static inline void init_hypervisor_platform(void) { }
diff --git a/arch/x86/kernel/cpu/hypervisor.c b/arch/x86/kernel/cpu/hypervisor.c
index 73d391a..ff108f8 100644
--- a/arch/x86/kernel/cpu/hypervisor.c
+++ b/arch/x86/kernel/cpu/hypervisor.c
@@ -85,3 +85,14 @@ bool __init hypervisor_x2apic_available(void)
 	       x86_hyper->x2apic_available &&
 	       x86_hyper->x2apic_available();
 }
+
+void hypervisor_pin_vcpu(int cpu)
+{
+	if (!x86_hyper)
+		return;
+
+	if (x86_hyper->pin_vcpu)
+		x86_hyper->pin_vcpu(cpu);
+	else
+		WARN_ONCE(1, "vcpu pinning requested but not supported!\n");
+}
diff --git a/include/linux/hypervisor.h b/include/linux/hypervisor.h
new file mode 100644
index 0000000..3fa5ef2
--- /dev/null
+++ b/include/linux/hypervisor.h
@@ -0,0 +1,17 @@
+#ifndef __LINUX_HYPEVISOR_H
+#define __LINUX_HYPEVISOR_H
+
+/*
+ *	Generic Hypervisor support
+ *		Juergen Gross <jgross@suse.com>
+ */
+
+#ifdef CONFIG_HYPERVISOR_GUEST
+#include <asm/hypervisor.h>
+#else
+static inline void hypervisor_pin_vcpu(int cpu)
+{
+}
+#endif
+
+#endif /* __LINUX_HYPEVISOR_H */
diff --git a/kernel/smp.c b/kernel/smp.c
index 7416544..9388064 100644
--- a/kernel/smp.c
+++ b/kernel/smp.c
@@ -14,6 +14,7 @@
 #include <linux/smp.h>
 #include <linux/cpu.h>
 #include <linux/sched.h>
+#include <linux/hypervisor.h>
 
 #include "smpboot.h"
 
diff --git a/kernel/up.c b/kernel/up.c
index 1760bf3..3ccee2b 100644
--- a/kernel/up.c
+++ b/kernel/up.c
@@ -6,6 +6,7 @@
 #include <linux/kernel.h>
 #include <linux/export.h>
 #include <linux/smp.h>
+#include <linux/hypervisor.h>
 
 int smp_call_function_single(int cpu, void (*func) (void *info), void *info,
 				int wait)
-- 
2.6.2


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[PATCH v4 3/6] smp: add function to execute a function synchronously on a cpu

[Juergen Gross]

On some hardware models (e.g. Dell Studio 1555 laptop) some hardware
related functions (e.g. SMIs) are to be executed on physical cpu 0
only. Instead of open coding such a functionality multiple times in
the kernel add a service function for this purpose. This will enable
the possibility to take special measures in virtualized environments
like Xen, too.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
V4: change return value in case of illegal cpu as requested by Peter Zijlstra
    make pinning of vcpu an option as suggested by Peter Zijlstra

V2: instead of manipulating the allowed set of cpus use cpu specific
    workqueue as requested by Peter Zijlstra
---
 include/linux/smp.h |  2 ++
 kernel/smp.c        | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 kernel/up.c         | 17 +++++++++++++++++
 3 files changed, 69 insertions(+)

diff --git a/include/linux/smp.h b/include/linux/smp.h
index c441407..3b5813b 100644
--- a/include/linux/smp.h
+++ b/include/linux/smp.h
@@ -196,4 +196,6 @@ extern void arch_enable_nonboot_cpus_end(void);
 
 void smp_setup_processor_id(void);
 
+int smp_call_on_cpu(unsigned int cpu, bool pin, int (*func)(void *), void *par);
+
 #endif /* __LINUX_SMP_H */
diff --git a/kernel/smp.c b/kernel/smp.c
index 9388064..357458b 100644
--- a/kernel/smp.c
+++ b/kernel/smp.c
@@ -740,3 +740,53 @@ void wake_up_all_idle_cpus(void)
 	preempt_enable();
 }
 EXPORT_SYMBOL_GPL(wake_up_all_idle_cpus);
+
+/**
+ * smp_call_on_cpu - Call a function on a specific cpu
+ *
+ * Used to call a function on a specific cpu and wait for it to return.
+ * Optionally make sure the call is done on a specified physical cpu via vcpu
+ * pinning in order to support virtualized environments.
+ */
+struct smp_call_on_cpu_struct {
+	struct work_struct	work;
+	struct completion	done;
+	int			(*func)(void *);
+	void			*data;
+	int			ret;
+	int			cpu;
+};
+
+static void smp_call_on_cpu_callback(struct work_struct *work)
+{
+	struct smp_call_on_cpu_struct *sscs;
+
+	sscs = container_of(work, struct smp_call_on_cpu_struct, work);
+	if (sscs->cpu >= 0)
+		hypervisor_pin_vcpu(sscs->cpu);
+	sscs->ret = sscs->func(sscs->data);
+	if (sscs->cpu >= 0)
+		hypervisor_pin_vcpu(-1);
+
+	complete(&sscs->done);
+}
+
+int smp_call_on_cpu(unsigned int cpu, bool pin, int (*func)(void *), void *par)
+{
+	struct smp_call_on_cpu_struct sscs = {
+		.work = __WORK_INITIALIZER(sscs.work, smp_call_on_cpu_callback),
+		.done = COMPLETION_INITIALIZER_ONSTACK(sscs.done),
+		.func = func,
+		.data = par,
+		.cpu  = pin ? cpu : -1,
+	};
+
+	if (cpu >= nr_cpu_ids)
+		return -ENXIO;
+
+	queue_work_on(cpu, system_wq, &sscs.work);
+	wait_for_completion(&sscs.done);
+
+	return sscs.ret;
+}
+EXPORT_SYMBOL_GPL(smp_call_on_cpu);
diff --git a/kernel/up.c b/kernel/up.c
index 3ccee2b..8266810b 100644
--- a/kernel/up.c
+++ b/kernel/up.c
@@ -83,3 +83,20 @@ void on_each_cpu_cond(bool (*cond_func)(int cpu, void *info),
 	preempt_enable();
 }
 EXPORT_SYMBOL(on_each_cpu_cond);
+
+int smp_call_on_cpu(unsigned int cpu, bool pin, int (*func)(void *), void *par)
+{
+	int ret;
+
+	if (cpu != 0)
+		return -ENXIO;
+
+	if (pin)
+		hypervisor_pin_vcpu(0);
+	ret = func(par);
+	if (pin)
+		hypervisor_pin_vcpu(-1);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(smp_call_on_cpu);
-- 
2.6.2


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[PATCH v4 4/6] xen: add xen_pin_vcpu() to support calling functions on a dedicated pcpu

[Juergen Gross]

Some hardware models (e.g. Dell Studio 1555 laptops) require calls to
the firmware to be issued on cpu 0 only. As Dom0 might have to use
these calls, add xen_pin_vcpu() to achieve this functionality.

In case either the domain doesn't have the privilege to make the
related hypercall or the hypervisor isn't supporting it, issue a
warning once and disable further pinning attempts.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 arch/x86/xen/enlighten.c | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 880862c..7907bcf8 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1885,6 +1885,45 @@ static void xen_set_cpu_features(struct cpuinfo_x86 *c)
 	}
 }
 
+static void xen_pin_vcpu(int cpu)
+{
+	static bool disable_pinning;
+	struct sched_pin_override pin_override;
+	int ret;
+
+	if (disable_pinning)
+		return;
+
+	pin_override.pcpu = cpu;
+	ret = HYPERVISOR_sched_op(SCHEDOP_pin_override, &pin_override);
+	if (cpu < 0)
+		return;
+
+	switch (ret) {
+	case -ENOSYS:
+		pr_warn("The kernel tried to call a function on physical cpu %d, but Xen isn't\n"
+			"supporting this. In case of problems you might consider vcpu pinning.\n",
+			cpu);
+		disable_pinning = true;
+		break;
+	case -EPERM:
+		WARN(1, "Trying to pin vcpu without having privilege to do so\n");
+		disable_pinning = true;
+		break;
+	case -EINVAL:
+	case -EBUSY:
+		pr_warn("The kernel tried to call a function on physical cpu %d, but this cpu\n"
+			"seems not to be available. Please check your Xen cpu configuration.\n",
+			cpu);
+		break;
+	case 0:
+		break;
+	default:
+		WARN(1, "rc %d while trying to pin vcpu\n", ret);
+		disable_pinning = true;
+	}
+}
+
 const struct hypervisor_x86 x86_hyper_xen = {
 	.name			= "Xen",
 	.detect			= xen_platform,
@@ -1893,6 +1932,7 @@ const struct hypervisor_x86 x86_hyper_xen = {
 #endif
 	.x2apic_available	= xen_x2apic_para_available,
 	.set_cpu_features       = xen_set_cpu_features,
+	.pin_vcpu               = xen_pin_vcpu,
 };
 EXPORT_SYMBOL(x86_hyper_xen);
 
-- 
2.6.2


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[PATCH v4 5/6] dcdbas: make use of smp_call_on_cpu()

[Juergen Gross]

Use smp_call_on_cpu() to raise SMI on cpu 0.
Make call secure by adding get_online_cpus() to avoid e.g. suspend
resume cycles in between.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
V4: add call to get_online_cpus()
---
 drivers/firmware/dcdbas.c | 51 ++++++++++++++++++++++++-----------------------
 1 file changed, 26 insertions(+), 25 deletions(-)

diff --git a/drivers/firmware/dcdbas.c b/drivers/firmware/dcdbas.c
index 829eec8..68e1d38 100644
--- a/drivers/firmware/dcdbas.c
+++ b/drivers/firmware/dcdbas.c
@@ -23,6 +23,7 @@
 #include <linux/platform_device.h>
 #include <linux/dma-mapping.h>
 #include <linux/errno.h>
+#include <linux/cpu.h>
 #include <linux/gfp.h>
 #include <linux/init.h>
 #include <linux/kernel.h>
@@ -238,33 +239,14 @@ static ssize_t host_control_on_shutdown_store(struct device *dev,
 	return count;
 }
 
-/**
- * dcdbas_smi_request: generate SMI request
- *
- * Called with smi_data_lock.
- */
-int dcdbas_smi_request(struct smi_cmd *smi_cmd)
+static int raise_smi(void *par)
 {
-	cpumask_var_t old_mask;
-	int ret = 0;
+	struct smi_cmd *smi_cmd = par;
 
-	if (smi_cmd->magic != SMI_CMD_MAGIC) {
-		dev_info(&dcdbas_pdev->dev, "%s: invalid magic value\n",
-			 __func__);
-		return -EBADR;
-	}
-
-	/* SMI requires CPU 0 */
-	if (!alloc_cpumask_var(&old_mask, GFP_KERNEL))
-		return -ENOMEM;
-
-	cpumask_copy(old_mask, &current->cpus_allowed);
-	set_cpus_allowed_ptr(current, cpumask_of(0));
 	if (smp_processor_id() != 0) {
 		dev_dbg(&dcdbas_pdev->dev, "%s: failed to get CPU 0\n",
 			__func__);
-		ret = -EBUSY;
-		goto out;
+		return -EBUSY;
 	}
 
 	/* generate SMI */
@@ -280,9 +262,28 @@ int dcdbas_smi_request(struct smi_cmd *smi_cmd)
 		: "memory"
 	);
 
-out:
-	set_cpus_allowed_ptr(current, old_mask);
-	free_cpumask_var(old_mask);
+	return 0;
+}
+/**
+ * dcdbas_smi_request: generate SMI request
+ *
+ * Called with smi_data_lock.
+ */
+int dcdbas_smi_request(struct smi_cmd *smi_cmd)
+{
+	int ret;
+
+	if (smi_cmd->magic != SMI_CMD_MAGIC) {
+		dev_info(&dcdbas_pdev->dev, "%s: invalid magic value\n",
+			 __func__);
+		return -EBADR;
+	}
+
+	/* SMI requires CPU 0 */
+	get_online_cpus();
+	ret = smp_call_on_cpu(0, true, raise_smi, smi_cmd);
+	put_online_cpus();
+
 	return ret;
 }
 
-- 
2.6.2


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[PATCH v4 6/6] hwmon: use smp_call_on_cpu() for dell-smm i8k

[Juergen Gross]

Use the smp_call_on_cpu() function to call system management
mode on cpu 0.
Make call secure by adding get_online_cpus() to avoid e.g. suspend
resume cycles in between.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
V4: add call to get_online_cpus()
---
 drivers/hwmon/dell-smm-hwmon.c | 35 ++++++++++++++++++++---------------
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
index c43318d..643f3a1 100644
--- a/drivers/hwmon/dell-smm-hwmon.c
+++ b/drivers/hwmon/dell-smm-hwmon.c
@@ -21,6 +21,7 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+#include <linux/cpu.h>
 #include <linux/delay.h>
 #include <linux/module.h>
 #include <linux/types.h>
@@ -35,6 +36,7 @@
 #include <linux/uaccess.h>
 #include <linux/io.h>
 #include <linux/sched.h>
+#include <linux/smp.h>
 
 #include <linux/i8k.h>
 
@@ -130,23 +132,15 @@ static inline const char *i8k_get_dmi_data(int field)
 /*
  * Call the System Management Mode BIOS. Code provided by Jonathan Buzzard.
  */
-static int i8k_smm(struct smm_regs *regs)
+static int i8k_smm_func(void *par)
 {
 	int rc;
+	struct smm_regs *regs = par;
 	int eax = regs->eax;
-	cpumask_var_t old_mask;
 
 	/* SMM requires CPU 0 */
-	if (!alloc_cpumask_var(&old_mask, GFP_KERNEL))
-		return -ENOMEM;
-	cpumask_copy(old_mask, &current->cpus_allowed);
-	rc = set_cpus_allowed_ptr(current, cpumask_of(0));
-	if (rc)
-		goto out;
-	if (smp_processor_id() != 0) {
-		rc = -EBUSY;
-		goto out;
-	}
+	if (smp_processor_id() != 0)
+		return -EBUSY;
 
 #if defined(CONFIG_X86_64)
 	asm volatile("pushq %%rax\n\t"
@@ -204,13 +198,24 @@ static int i8k_smm(struct smm_regs *regs)
 	if (rc != 0 || (regs->eax & 0xffff) == 0xffff || regs->eax == eax)
 		rc = -EINVAL;
 
-out:
-	set_cpus_allowed_ptr(current, old_mask);
-	free_cpumask_var(old_mask);
 	return rc;
 }
 
 /*
+ * Call the System Management Mode BIOS.
+ */
+static int i8k_smm(struct smm_regs *regs)
+{
+	int ret;
+
+	get_online_cpus();
+	ret = smp_call_on_cpu(0, true, i8k_smm_func, regs);
+	put_online_cpus();
+
+	return ret;
+}
+
+/*
  * Read the fan status.
  */
 static int i8k_get_fan_status(int fan)
-- 
2.6.2


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v4 3/6] smp: add function to execute a function synchronously on a cpu

[Peter Zijlstra]

On Tue, Apr 05, 2016 at 07:10:04AM +0200, Juergen Gross wrote:
> +int smp_call_on_cpu(unsigned int cpu, bool pin, int (*func)(void *), void *par)

Why .pin and not .phys? .pin does not (to me) reflect the
hypervisor/physical-cpu thing.

Also, as per smp_call_function_single() would it not be more consistent
to make this the last argument?

> +{
> +	struct smp_call_on_cpu_struct sscs = {
> +		.work = __WORK_INITIALIZER(sscs.work, smp_call_on_cpu_callback),
> +		.done = COMPLETION_INITIALIZER_ONSTACK(sscs.done),
> +		.func = func,
> +		.data = par,
> +		.cpu  = pin ? cpu : -1,
> +	};
> +
> +	if (cpu >= nr_cpu_ids)

You might want to also include cpu_online().

	if (cpu >= nr_cpu_ids || !cpu_online(cpu))
> +		return -ENXIO;

Seeing how its fairly hard to schedule work on a cpu that's not actually
there.

> +
> +	queue_work_on(cpu, system_wq, &sscs.work);
> +	wait_for_completion(&sscs.done);
> +
> +	return sscs.ret;
> +}



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v4 3/6] smp: add function to execute a function synchronously on a cpu

[Juergen Gross]

On 05/04/16 10:11, Peter Zijlstra wrote:
> On Tue, Apr 05, 2016 at 07:10:04AM +0200, Juergen Gross wrote:
>> +int smp_call_on_cpu(unsigned int cpu, bool pin, int (*func)(void *), void *par)
> 
> Why .pin and not .phys? .pin does not (to me) reflect the
> hypervisor/physical-cpu thing.

I don't mind either way. As you don't like .pin, lets name it .phys.

> Also, as per smp_call_function_single() would it not be more consistent
> to make this the last argument?

Okay, I'll change it.

> 
>> +{
>> +	struct smp_call_on_cpu_struct sscs = {
>> +		.work = __WORK_INITIALIZER(sscs.work, smp_call_on_cpu_callback),
>> +		.done = COMPLETION_INITIALIZER_ONSTACK(sscs.done),
>> +		.func = func,
>> +		.data = par,
>> +		.cpu  = pin ? cpu : -1,
>> +	};
>> +
>> +	if (cpu >= nr_cpu_ids)
> 
> You might want to also include cpu_online().
> 
> 	if (cpu >= nr_cpu_ids || !cpu_online(cpu))

Indeed, good idea.

>> +		return -ENXIO;
> 
> Seeing how its fairly hard to schedule work on a cpu that's not actually
> there.

Really? ;-)


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v4 1/6] xen: sync xen header

[David Vrabel]

On 05/04/16 06:10, Juergen Gross wrote:
> Import the actual version of include/xen/interface/sched.h from Xen.

Acked-by: David Vrabel <david.vrabel@citrix.com>

David

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v4 4/6] xen: add xen_pin_vcpu() to support calling functions on a dedicated pcpu

[David Vrabel]

On 05/04/16 06:10, Juergen Gross wrote:
> Some hardware models (e.g. Dell Studio 1555 laptops) require calls to
> the firmware to be issued on cpu 0 only. As Dom0 might have to use
> these calls, add xen_pin_vcpu() to achieve this functionality.
> 
> In case either the domain doesn't have the privilege to make the
> related hypercall or the hypervisor isn't supporting it, issue a
> warning once and disable further pinning attempts.
[...]
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
> @@ -1885,6 +1885,45 @@ static void xen_set_cpu_features(struct cpuinfo_x86 *c)
>  	}
>  }
>  
> +static void xen_pin_vcpu(int cpu)
> +{
> +	static bool disable_pinning;
> +	struct sched_pin_override pin_override;
> +	int ret;
> +
> +	if (disable_pinning)
> +		return;
> +
> +	pin_override.pcpu = cpu;
> +	ret = HYPERVISOR_sched_op(SCHEDOP_pin_override, &pin_override);

	/* Ignore errors when removing override. */
> +	if (cpu < 0)
> +		return;
> +
> +	switch (ret) {
> +	case -ENOSYS:
> +		pr_warn("The kernel tried to call a function on physical cpu %d, but Xen isn't\n"
> +			"supporting this. In case of problems you might consider vcpu pinning.\n",
> +			cpu);
> +		disable_pinning = true;
> +		break;
> +	case -EPERM:
> +		WARN(1, "Trying to pin vcpu without having privilege to do so\n");
> +		disable_pinning = true;
> +		break;
> +	case -EINVAL:
> +	case -EBUSY:
> +		pr_warn("The kernel tried to call a function on physical cpu %d, but this cpu\n"
> +			"seems not to be available. Please check your Xen cpu configuration.\n",
> +			cpu);
> +		break;
> +	case 0:
> +		break;
> +	default:
> +		WARN(1, "rc %d while trying to pin vcpu\n", ret);
> +		disable_pinning = true;
> +	}

These messages are a bit wordy for my taste and since they don't say
what function failed or what driver is affected they're not as useful as
they could be.  I'd probably turn these all into:

	if (cpu >= 0 && ret < 0) {
		pr_warn("Failed to pin VCPU %d to physical CPU %d: %d",
		        smp_processor_id(), cpu, ret);
		disable_pinning = true;
	}

And look at getting the user of this API to print a more useful error.

"i8k: unable to call SMM BIOS on physical CPU %d: %d"

Or whatever.

David

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v4 4/6] xen: add xen_pin_vcpu() to support calling functions on a dedicated pcpu

[Juergen Gross]

On 05/04/16 11:45, David Vrabel wrote:
> On 05/04/16 06:10, Juergen Gross wrote:
>> Some hardware models (e.g. Dell Studio 1555 laptops) require calls to
>> the firmware to be issued on cpu 0 only. As Dom0 might have to use
>> these calls, add xen_pin_vcpu() to achieve this functionality.
>>
>> In case either the domain doesn't have the privilege to make the
>> related hypercall or the hypervisor isn't supporting it, issue a
>> warning once and disable further pinning attempts.
> [...]
>> --- a/arch/x86/xen/enlighten.c
>> +++ b/arch/x86/xen/enlighten.c
>> @@ -1885,6 +1885,45 @@ static void xen_set_cpu_features(struct cpuinfo_x86 *c)
>>  	}
>>  }
>>  
>> +static void xen_pin_vcpu(int cpu)
>> +{
>> +	static bool disable_pinning;
>> +	struct sched_pin_override pin_override;
>> +	int ret;
>> +
>> +	if (disable_pinning)
>> +		return;
>> +
>> +	pin_override.pcpu = cpu;
>> +	ret = HYPERVISOR_sched_op(SCHEDOP_pin_override, &pin_override);
> 
> 	/* Ignore errors when removing override. */

Okay.

>> +	if (cpu < 0)
>> +		return;
>> +
>> +	switch (ret) {
>> +	case -ENOSYS:
>> +		pr_warn("The kernel tried to call a function on physical cpu %d, but Xen isn't\n"
>> +			"supporting this. In case of problems you might consider vcpu pinning.\n",
>> +			cpu);
>> +		disable_pinning = true;
>> +		break;
>> +	case -EPERM:
>> +		WARN(1, "Trying to pin vcpu without having privilege to do so\n");
>> +		disable_pinning = true;
>> +		break;
>> +	case -EINVAL:
>> +	case -EBUSY:
>> +		pr_warn("The kernel tried to call a function on physical cpu %d, but this cpu\n"
>> +			"seems not to be available. Please check your Xen cpu configuration.\n",
>> +			cpu);
>> +		break;
>> +	case 0:
>> +		break;
>> +	default:
>> +		WARN(1, "rc %d while trying to pin vcpu\n", ret);
>> +		disable_pinning = true;
>> +	}
> 
> These messages are a bit wordy for my taste and since they don't say
> what function failed or what driver is affected they're not as useful as

Did you notice I used WARN() for the cases where a usage error is to
be suspected? This will print a stack backtrace helping to identify the
driver.

I can work on the message text, of course.

> they could be.  I'd probably turn these all into:
> 
> 	if (cpu >= 0 && ret < 0) {
> 		pr_warn("Failed to pin VCPU %d to physical CPU %d: %d",
> 		        smp_processor_id(), cpu, ret);
> 		disable_pinning = true;
> 	}

No, I don't think this is a good idea. In the EINVAL or EBUSY case a
simple Xen admin command might be enough to make the next call succeed.
I don't want to disable pinning in this case.

> And look at getting the user of this API to print a more useful error.
> 
> "i8k: unable to call SMM BIOS on physical CPU %d: %d"

TBH: I think this should be done by another patch. This is something
the maintainers of the callers' code should decide.


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v4 4/6] xen: add xen_pin_vcpu() to support calling functions on a dedicated pcpu

[David Vrabel]

On 05/04/16 11:01, Juergen Gross wrote:
> 
> No, I don't think this is a good idea. In the EINVAL or EBUSY case a
> simple Xen admin command might be enough to make the next call succeed.
> I don't want to disable pinning in this case.

Ok.

Acked-by: David Vrabel <david.vrabel@citrix.com>

David

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v4 6/6] hwmon: use smp_call_on_cpu() for dell-smm i8k

[Guenter Roeck]

On Tue, Apr 05, 2016 at 07:10:07AM +0200, Juergen Gross wrote:
> Use the smp_call_on_cpu() function to call system management
> mode on cpu 0.
> Make call secure by adding get_online_cpus() to avoid e.g. suspend
> resume cycles in between.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
> V4: add call to get_online_cpus()

Pali, any chance to test this ?

Thanks,
Guenter

> ---
>  drivers/hwmon/dell-smm-hwmon.c | 35 ++++++++++++++++++++---------------
>  1 file changed, 20 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
> index c43318d..643f3a1 100644
> --- a/drivers/hwmon/dell-smm-hwmon.c
> +++ b/drivers/hwmon/dell-smm-hwmon.c
> @@ -21,6 +21,7 @@
>  
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>  
> +#include <linux/cpu.h>
>  #include <linux/delay.h>
>  #include <linux/module.h>
>  #include <linux/types.h>
> @@ -35,6 +36,7 @@
>  #include <linux/uaccess.h>
>  #include <linux/io.h>
>  #include <linux/sched.h>
> +#include <linux/smp.h>
>  
>  #include <linux/i8k.h>
>  
> @@ -130,23 +132,15 @@ static inline const char *i8k_get_dmi_data(int field)
>  /*
>   * Call the System Management Mode BIOS. Code provided by Jonathan Buzzard.
>   */
> -static int i8k_smm(struct smm_regs *regs)
> +static int i8k_smm_func(void *par)
>  {
>  	int rc;
> +	struct smm_regs *regs = par;
>  	int eax = regs->eax;
> -	cpumask_var_t old_mask;
>  
>  	/* SMM requires CPU 0 */
> -	if (!alloc_cpumask_var(&old_mask, GFP_KERNEL))
> -		return -ENOMEM;
> -	cpumask_copy(old_mask, &current->cpus_allowed);
> -	rc = set_cpus_allowed_ptr(current, cpumask_of(0));
> -	if (rc)
> -		goto out;
> -	if (smp_processor_id() != 0) {
> -		rc = -EBUSY;
> -		goto out;
> -	}
> +	if (smp_processor_id() != 0)
> +		return -EBUSY;
>  
>  #if defined(CONFIG_X86_64)
>  	asm volatile("pushq %%rax\n\t"
> @@ -204,13 +198,24 @@ static int i8k_smm(struct smm_regs *regs)
>  	if (rc != 0 || (regs->eax & 0xffff) == 0xffff || regs->eax == eax)
>  		rc = -EINVAL;
>  
> -out:
> -	set_cpus_allowed_ptr(current, old_mask);
> -	free_cpumask_var(old_mask);
>  	return rc;
>  }
>  
>  /*
> + * Call the System Management Mode BIOS.
> + */
> +static int i8k_smm(struct smm_regs *regs)
> +{
> +	int ret;
> +
> +	get_online_cpus();
> +	ret = smp_call_on_cpu(0, true, i8k_smm_func, regs);
> +	put_online_cpus();
> +
> +	return ret;
> +}
> +
> +/*
>   * Read the fan status.
>   */
>  static int i8k_get_fan_status(int fan)
> -- 
> 2.6.2
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v4 6/6] hwmon: use smp_call_on_cpu() for dell-smm i8k

[Pali Rohár]

[-- Attachment #1.1: Type: Text/Plain, Size: 674 bytes --]

On Tuesday 05 April 2016 16:54:14 Guenter Roeck wrote:
> On Tue, Apr 05, 2016 at 07:10:07AM +0200, Juergen Gross wrote:
> > Use the smp_call_on_cpu() function to call system management
> > mode on cpu 0.
> > Make call secure by adding get_online_cpus() to avoid e.g. suspend
> > resume cycles in between.
> > 
> > Signed-off-by: Juergen Gross <jgross@suse.com>
> > ---
> > V4: add call to get_online_cpus()
> 
> Pali, any chance to test this ?

I can test it, but just on machine where (probably) smm calls can be 
send from any cpu... Need some time for testing and I believe I can do 
that at the end of the week.

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v4 6/6] hwmon: use smp_call_on_cpu() for dell-smm i8k

[Pali Rohár]

On Tuesday 05 April 2016 21:31:52 Pali Rohár wrote:
> On Tuesday 05 April 2016 16:54:14 Guenter Roeck wrote:
> > On Tue, Apr 05, 2016 at 07:10:07AM +0200, Juergen Gross wrote:
> > > Use the smp_call_on_cpu() function to call system management
> > > mode on cpu 0.
> > > Make call secure by adding get_online_cpus() to avoid e.g. suspend
> > > resume cycles in between.
> > > 
> > > Signed-off-by: Juergen Gross <jgross@suse.com>
> > > ---
> > > V4: add call to get_online_cpus()
> > 
> > Pali, any chance to test this ?
> 
> I can test it, but just on machine where (probably) smm calls can be 
> send from any cpu... Need some time for testing and I believe I can do 
> that at the end of the week.

Sorry I had absolutely no more free time last weekend :-( And same
prediction is for this weekend and also next one...

-- 
Pali Rohár
pali.rohar@gmail.com

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v4 6/6] hwmon: use smp_call_on_cpu() for dell-smm i8k

[Juergen Gross]

On 21/04/16 12:57, Pali Rohár wrote:
> On Tuesday 05 April 2016 21:31:52 Pali Rohár wrote:
>> On Tuesday 05 April 2016 16:54:14 Guenter Roeck wrote:
>>> On Tue, Apr 05, 2016 at 07:10:07AM +0200, Juergen Gross wrote:
>>>> Use the smp_call_on_cpu() function to call system management
>>>> mode on cpu 0.
>>>> Make call secure by adding get_online_cpus() to avoid e.g. suspend
>>>> resume cycles in between.
>>>>
>>>> Signed-off-by: Juergen Gross <jgross@suse.com>
>>>> ---
>>>> V4: add call to get_online_cpus()
>>>
>>> Pali, any chance to test this ?
>>
>> I can test it, but just on machine where (probably) smm calls can be 
>> send from any cpu... Need some time for testing and I believe I can do 
>> that at the end of the week.
> 
> Sorry I had absolutely no more free time last weekend :-( And same
> prediction is for this weekend and also next one...

Pali, I've got a Dell laptop (Latitude E6440) here. Would this device be
okay for a test? What would you do for testing? In case you can give me
some hints how to do a sensible test I'd do it.

I've verified by adding a printk() to smp_call_on_cpu() that at least
one of the modified drivers has been used during system boot.


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v4 6/6] hwmon: use smp_call_on_cpu() for dell-smm i8k

[Pali Rohár]

On Thursday 21 April 2016 15:12:52 Juergen Gross wrote:
> On 21/04/16 12:57, Pali Rohár wrote:
> > On Tuesday 05 April 2016 21:31:52 Pali Rohár wrote:
> >> On Tuesday 05 April 2016 16:54:14 Guenter Roeck wrote:
> >>> On Tue, Apr 05, 2016 at 07:10:07AM +0200, Juergen Gross wrote:
> >>>> Use the smp_call_on_cpu() function to call system management
> >>>> mode on cpu 0.
> >>>> Make call secure by adding get_online_cpus() to avoid e.g. suspend
> >>>> resume cycles in between.
> >>>>
> >>>> Signed-off-by: Juergen Gross <jgross@suse.com>
> >>>> ---
> >>>> V4: add call to get_online_cpus()
> >>>
> >>> Pali, any chance to test this ?
> >>
> >> I can test it, but just on machine where (probably) smm calls can be 
> >> send from any cpu... Need some time for testing and I believe I can do 
> >> that at the end of the week.
> > 
> > Sorry I had absolutely no more free time last weekend :-( And same
> > prediction is for this weekend and also next one...
> 
> Pali, I've got a Dell laptop (Latitude E6440) here. Would this device be
> okay for a test?

Hi!

Proper regression test should check if this patch does not break any
function or drivers dependent on dcdbas.ko. And should be done on both
notebook devices: which needs to issue that smm call on cpu 0 and also
on which it is not needed.

Some notebooks which needs smm call to issued from cpu 0 can be found in
git commit messages of i8k, dell-laptop or dcdbas kernel drivers.

> What would you do for testing? In case you can give me
> some hints how to do a sensible test I'd do it.

Test e.g. dell-laptop.ko driver. It provides /sys interface for changing
keyboard backlight or changing rfkill switches (bluetooth wifi).

Also test tools from libsmbios (userspace) package.

There must be no difference in output/functionality with or without your
patches.

> I've verified by adding a printk() to smp_call_on_cpu() that at least
> one of the modified drivers has been used during system boot.

Also you can patch i8k/dcdbas smm function to print cpu number on which
is code running (to verify that it was really called on cpu 0 as
needed).

-- 
Pali Rohár
pali.rohar@gmail.com

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v4 6/6] hwmon: use smp_call_on_cpu() for dell-smm i8k

[Juergen Gross]

On 21/04/16 15:27, Pali Rohár wrote:
> On Thursday 21 April 2016 15:12:52 Juergen Gross wrote:
>> On 21/04/16 12:57, Pali Rohár wrote:
>>> On Tuesday 05 April 2016 21:31:52 Pali Rohár wrote:
>>>> On Tuesday 05 April 2016 16:54:14 Guenter Roeck wrote:
>>>>> On Tue, Apr 05, 2016 at 07:10:07AM +0200, Juergen Gross wrote:
>>>>>> Use the smp_call_on_cpu() function to call system management
>>>>>> mode on cpu 0.
>>>>>> Make call secure by adding get_online_cpus() to avoid e.g. suspend
>>>>>> resume cycles in between.
>>>>>>
>>>>>> Signed-off-by: Juergen Gross <jgross@suse.com>
>>>>>> ---
>>>>>> V4: add call to get_online_cpus()
>>>>>
>>>>> Pali, any chance to test this ?
>>>>
>>>> I can test it, but just on machine where (probably) smm calls can be 
>>>> send from any cpu... Need some time for testing and I believe I can do 
>>>> that at the end of the week.
>>>
>>> Sorry I had absolutely no more free time last weekend :-( And same
>>> prediction is for this weekend and also next one...
>>
>> Pali, I've got a Dell laptop (Latitude E6440) here. Would this device be
>> okay for a test?
> 
> Hi!
> 
> Proper regression test should check if this patch does not break any
> function or drivers dependent on dcdbas.ko. And should be done on both
> notebook devices: which needs to issue that smm call on cpu 0 and also
> on which it is not needed.

Hmm, couldn't get one which needs smm to be called on cpu 0.
OTOH I've done various tests and added a printk() in raise_smm()
and i8k_smm_func() issuing the cpu number it was called on.

> Some notebooks which needs smm call to issued from cpu 0 can be found in
> git commit messages of i8k, dell-laptop or dcdbas kernel drivers.
> 
>> What would you do for testing? In case you can give me
>> some hints how to do a sensible test I'd do it.
> 
> Test e.g. dell-laptop.ko driver. It provides /sys interface for changing
> keyboard backlight or changing rfkill switches (bluetooth wifi).

Done.

> Also test tools from libsmbios (userspace) package.

Done.

> There must be no difference in output/functionality with or without your
> patches.

Verified.

>> I've verified by adding a printk() to smp_call_on_cpu() that at least
>> one of the modified drivers has been used during system boot.
> 
> Also you can patch i8k/dcdbas smm function to print cpu number on which
> is code running (to verify that it was really called on cpu 0 as
> needed).

Done.

I tested suspend/resume, too, as adding get_online_cpus() might have
changed behavior. Worked like a charm. :-)


Juergen


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v4 6/6] hwmon: use smp_call_on_cpu() for dell-smm i8k

[Pali Rohár]

On Monday 09 May 2016 16:37:01 Juergen Gross wrote:
> On 21/04/16 15:27, Pali Rohár wrote:
> > On Thursday 21 April 2016 15:12:52 Juergen Gross wrote:
> >> On 21/04/16 12:57, Pali Rohár wrote:
> >>> On Tuesday 05 April 2016 21:31:52 Pali Rohár wrote:
> >>>> On Tuesday 05 April 2016 16:54:14 Guenter Roeck wrote:
> >>>>> On Tue, Apr 05, 2016 at 07:10:07AM +0200, Juergen Gross wrote:
> >>>>>> Use the smp_call_on_cpu() function to call system management
> >>>>>> mode on cpu 0.
> >>>>>> Make call secure by adding get_online_cpus() to avoid e.g. suspend
> >>>>>> resume cycles in between.
> >>>>>>
> >>>>>> Signed-off-by: Juergen Gross <jgross@suse.com>
> >>>>>> ---
> >>>>>> V4: add call to get_online_cpus()
> >>>>>
> >>>>> Pali, any chance to test this ?
> >>>>
> >>>> I can test it, but just on machine where (probably) smm calls can be 
> >>>> send from any cpu... Need some time for testing and I believe I can do 
> >>>> that at the end of the week.
> >>>
> >>> Sorry I had absolutely no more free time last weekend :-( And same
> >>> prediction is for this weekend and also next one...
> >>
> >> Pali, I've got a Dell laptop (Latitude E6440) here. Would this device be
> >> okay for a test?
> > 
> > Hi!
> > 
> > Proper regression test should check if this patch does not break any
> > function or drivers dependent on dcdbas.ko. And should be done on both
> > notebook devices: which needs to issue that smm call on cpu 0 and also
> > on which it is not needed.
> 
> Hmm, couldn't get one which needs smm to be called on cpu 0.
> OTOH I've done various tests and added a printk() in raise_smm()
> and i8k_smm_func() issuing the cpu number it was called on.

Understood, those machines are old and probably rare now.

> > Some notebooks which needs smm call to issued from cpu 0 can be found in
> > git commit messages of i8k, dell-laptop or dcdbas kernel drivers.
> > 
> >> What would you do for testing? In case you can give me
> >> some hints how to do a sensible test I'd do it.
> > 
> > Test e.g. dell-laptop.ko driver. It provides /sys interface for changing
> > keyboard backlight or changing rfkill switches (bluetooth wifi).
> 
> Done.
> 
> > Also test tools from libsmbios (userspace) package.
> 
> Done.
> 
> > There must be no difference in output/functionality with or without your
> > patches.
> 
> Verified.
> 
> >> I've verified by adding a printk() to smp_call_on_cpu() that at least
> >> one of the modified drivers has been used during system boot.
> > 
> > Also you can patch i8k/dcdbas smm function to print cpu number on which
> > is code running (to verify that it was really called on cpu 0 as
> > needed).
> 
> Done.
> 
> I tested suspend/resume, too, as adding get_online_cpus() might have
> changed behavior. Worked like a charm. :-)

Ok, I think this should be enough. You can add my Acked-by.

-- 
Pali Rohár
pali.rohar@gmail.com

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel