[PATCH 01/12] Add support for hvm_op

[PATCH 01/12] Add support for hvm_op

[stefano.stabellini]

From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>

Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
Signed-off-by: Sheng Yang <sheng@linux.intel.com>
Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 arch/x86/include/asm/xen/hypercall.h |    6 ++
 include/xen/hvm.h                    |   24 +++++++++
 include/xen/interface/hvm/hvm_op.h   |   35 ++++++++++++
 include/xen/interface/hvm/params.h   |   95 ++++++++++++++++++++++++++++++++++
 4 files changed, 160 insertions(+), 0 deletions(-)
 create mode 100644 include/xen/hvm.h
 create mode 100644 include/xen/interface/hvm/hvm_op.h
 create mode 100644 include/xen/interface/hvm/params.h

diff --git a/arch/x86/include/asm/xen/hypercall.h b/arch/x86/include/asm/xen/hypercall.h
index 9c371e4..7fda040 100644
--- a/arch/x86/include/asm/xen/hypercall.h
+++ b/arch/x86/include/asm/xen/hypercall.h
@@ -417,6 +417,12 @@ HYPERVISOR_nmi_op(unsigned long op, unsigned long arg)
 	return _hypercall2(int, nmi_op, op, arg);
 }
 
+static inline unsigned long __must_check
+HYPERVISOR_hvm_op(int op, void *arg)
+{
+       return _hypercall2(unsigned long, hvm_op, op, arg);
+}
+
 static inline void
 MULTI_fpu_taskswitch(struct multicall_entry *mcl, int set)
 {
diff --git a/include/xen/hvm.h b/include/xen/hvm.h
new file mode 100644
index 0000000..6b0d418
--- /dev/null
+++ b/include/xen/hvm.h
@@ -0,0 +1,24 @@
+/* Simple wrappers around HVM functions */
+#ifndef XEN_HVM_H__
+#define XEN_HVM_H__
+
+#include <xen/interface/hvm/params.h>
+
+static inline int hvm_get_parameter(int idx, uint64_t *value)
+{
+       struct xen_hvm_param xhv;
+       int r;
+
+       xhv.domid = DOMID_SELF;
+       xhv.index = idx;
+       r = HYPERVISOR_hvm_op(HVMOP_get_param, &xhv);
+       if (r < 0) {
+               printk(KERN_ERR "cannot get hvm parameter %d: %d.\n",
+                      idx, r);
+               return r;
+       }
+       *value = xhv.value;
+       return r;
+}
+
+#endif /* XEN_HVM_H__ */
diff --git a/include/xen/interface/hvm/hvm_op.h b/include/xen/interface/hvm/hvm_op.h
new file mode 100644
index 0000000..73c8c7e
--- /dev/null
+++ b/include/xen/interface/hvm/hvm_op.h
@@ -0,0 +1,35 @@
+/*
+ * 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.
+ */
+
+#ifndef __XEN_PUBLIC_HVM_HVM_OP_H__
+#define __XEN_PUBLIC_HVM_HVM_OP_H__
+
+/* Get/set subcommands: the second argument of the hypercall is a
+ * pointer to a xen_hvm_param struct. */
+#define HVMOP_set_param           0
+#define HVMOP_get_param           1
+struct xen_hvm_param {
+    domid_t  domid;    /* IN */
+    uint32_t index;    /* IN */
+    uint64_t value;    /* IN/OUT */
+};
+DEFINE_GUEST_HANDLE_STRUCT(xen_hvm_param);
+
+#endif /* __XEN_PUBLIC_HVM_HVM_OP_H__ */
diff --git a/include/xen/interface/hvm/params.h b/include/xen/interface/hvm/params.h
new file mode 100644
index 0000000..1888d8c
--- /dev/null
+++ b/include/xen/interface/hvm/params.h
@@ -0,0 +1,95 @@
+/*
+ * 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.
+ */
+
+#ifndef __XEN_PUBLIC_HVM_PARAMS_H__
+#define __XEN_PUBLIC_HVM_PARAMS_H__
+
+#include "hvm_op.h"
+
+/*
+ * Parameter space for HVMOP_{set,get}_param.
+ */
+
+/*
+ * How should CPU0 event-channel notifications be delivered?
+ * val[63:56] == 0: val[55:0] is a delivery GSI (Global System Interrupt).
+ * val[63:56] == 1: val[55:0] is a delivery PCI INTx line, as follows:
+ *                  Domain = val[47:32], Bus  = val[31:16],
+ *                  DevFn  = val[15: 8], IntX = val[ 1: 0]
+ * val[63:56] == 2: val[7:0] is a vector number.
+ * If val == 0 then CPU0 event-channel notifications are not delivered.
+ */
+#define HVM_PARAM_CALLBACK_IRQ 0
+
+#define HVM_PARAM_STORE_PFN    1
+#define HVM_PARAM_STORE_EVTCHN 2
+
+#define HVM_PARAM_PAE_ENABLED  4
+
+#define HVM_PARAM_IOREQ_PFN    5
+
+#define HVM_PARAM_BUFIOREQ_PFN 6
+
+/*
+ * Set mode for virtual timers (currently x86 only):
+ *  delay_for_missed_ticks (default):
+ *   Do not advance a vcpu's time beyond the correct delivery time for
+ *   interrupts that have been missed due to preemption. Deliver missed
+ *   interrupts when the vcpu is rescheduled and advance the vcpu's virtual
+ *   time stepwise for each one.
+ *  no_delay_for_missed_ticks:
+ *   As above, missed interrupts are delivered, but guest time always tracks
+ *   wallclock (i.e., real) time while doing so.
+ *  no_missed_ticks_pending:
+ *   No missed interrupts are held pending. Instead, to ensure ticks are
+ *   delivered at some non-zero rate, if we detect missed ticks then the
+ *   internal tick alarm is not disabled if the VCPU is preempted during the
+ *   next tick period.
+ *  one_missed_tick_pending:
+ *   Missed interrupts are collapsed together and delivered as one 'late tick'.
+ *   Guest time always tracks wallclock (i.e., real) time.
+ */
+#define HVM_PARAM_TIMER_MODE   10
+#define HVMPTM_delay_for_missed_ticks    0
+#define HVMPTM_no_delay_for_missed_ticks 1
+#define HVMPTM_no_missed_ticks_pending   2
+#define HVMPTM_one_missed_tick_pending   3
+
+/* Boolean: Enable virtual HPET (high-precision event timer)? (x86-only) */
+#define HVM_PARAM_HPET_ENABLED 11
+
+/* Identity-map page directory used by Intel EPT when CR0.PG=0. */
+#define HVM_PARAM_IDENT_PT     12
+
+/* Device Model domain, defaults to 0. */
+#define HVM_PARAM_DM_DOMAIN    13
+
+/* ACPI S state: currently support S0 and S3 on x86. */
+#define HVM_PARAM_ACPI_S_STATE 14
+
+/* TSS used on Intel when CR0.PE=0. */
+#define HVM_PARAM_VM86_TSS     15
+
+/* Boolean: Enable aligning all periodic vpts to reduce interrupts */
+#define HVM_PARAM_VPT_ALIGN    16
+
+#define HVM_NR_PARAMS          17
+
+#endif /* __XEN_PUBLIC_HVM_PARAMS_H__ */
-- 
1.7.0.4

[PATCH 02/12] early PV on HVM

[stefano.stabellini]

From: Sheng Yang <sheng@linux.intel.com>

Initialize basic pv on hvm features in xen_guest_init.

The hook in arch/x86/kernel/setup.c can easily be removed using the new
generic hypervisor independent initialization infrastructure present in
the linux-2.6-tip tree.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Signed-off-by: Sheng Yang <sheng@linux.intel.com>
Signed-off-by: Yaozu (Eddie) Dong <eddie.dong@intel.com>
---
 arch/x86/kernel/setup.c           |    2 +
 arch/x86/xen/enlighten.c          |   86 +++++++++++++++++++++++++++++++++++++
 drivers/input/xen-kbdfront.c      |    2 +-
 drivers/video/xen-fbfront.c       |    2 +-
 drivers/xen/xenbus/xenbus_probe.c |   21 ++++++++-
 include/xen/xen.h                 |    2 +
 6 files changed, 110 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index c4851ef..ae9b6cb 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -69,6 +69,7 @@
 #include <linux/tboot.h>
 
 #include <video/edid.h>
+#include <xen/xen.h>
 
 #include <asm/mtrr.h>
 #include <asm/apic.h>
@@ -1032,6 +1033,7 @@ void __init setup_arch(char **cmdline_p)
 	probe_nr_irqs_gsi();
 
 	kvm_guest_init();
+	xen_guest_init();
 
 	e820_reserve_resources();
 	e820_mark_nosave_regions(max_low_pfn);
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 65d8d79..c1f6545 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -35,6 +35,7 @@
 #include <xen/interface/version.h>
 #include <xen/interface/physdev.h>
 #include <xen/interface/vcpu.h>
+#include <xen/interface/memory.h>
 #include <xen/features.h>
 #include <xen/page.h>
 #include <xen/hvc-console.h>
@@ -56,6 +57,7 @@
 #include <asm/tlbflush.h>
 #include <asm/reboot.h>
 #include <asm/stackprotector.h>
+#include <asm/hypervisor.h>
 
 #include "xen-ops.h"
 #include "mmu.h"
@@ -1206,3 +1208,87 @@ asmlinkage void __init xen_start_kernel(void)
 	x86_64_start_reservations((char *)__pa_symbol(&boot_params));
 #endif
 }
+
+static uint32_t xen_cpuid_base(void)
+{
+	uint32_t base, eax, ebx, ecx, edx;
+	char signature[13];
+
+	for (base = 0x40000000; base < 0x40010000; base += 0x100) {
+		cpuid(base, &eax, &ebx, &ecx, &edx);
+		*(uint32_t*)(signature + 0) = ebx;
+		*(uint32_t*)(signature + 4) = ecx;
+		*(uint32_t*)(signature + 8) = edx;
+		signature[12] = 0;
+
+		if (!strcmp("XenVMMXenVMM", signature) && ((eax - base) >= 2))
+			return base;
+	}
+
+	return 0;
+}
+
+static int init_hvm_pv_info(int *major, int *minor)
+{
+	uint32_t eax, ebx, ecx, edx, pages, msr, base;
+	u64 pfn;
+
+	base = xen_cpuid_base();
+	if (!base)
+		return -EINVAL;
+
+	cpuid(base + 1, &eax, &ebx, &ecx, &edx);
+
+	*major = eax >> 16;
+	*minor = eax & 0xffff;
+	printk(KERN_INFO "Xen version %d.%d.\n", *major, *minor);
+
+	cpuid(base + 2, &pages, &msr, &ecx, &edx);
+
+	pfn = __pa(hypercall_page);
+	wrmsr_safe(msr, (u32)pfn, (u32)(pfn >> 32));
+
+	xen_setup_features();
+
+	pv_info = xen_info;
+	pv_info.kernel_rpl = 0;
+
+	xen_domain_type = XEN_HVM_DOMAIN;
+
+	return 0;
+}
+
+static void __init init_shared_info(void)
+{
+	struct xen_add_to_physmap xatp;
+	struct shared_info *shared_info_page;
+
+	shared_info_page = (struct shared_info *) alloc_bootmem_pages(PAGE_SIZE);
+	xatp.domid = DOMID_SELF;
+	xatp.idx = 0;
+	xatp.space = XENMAPSPACE_shared_info;
+	xatp.gpfn = __pa(shared_info_page) >> PAGE_SHIFT;
+	if (HYPERVISOR_memory_op(XENMEM_add_to_physmap, &xatp))
+		BUG();
+
+	HYPERVISOR_shared_info = (struct shared_info *)shared_info_page;
+
+	/* Don't do the full vcpu_info placement stuff until we have a
+	   possible map and a non-dummy shared_info. */
+	per_cpu(xen_vcpu, 0) = &HYPERVISOR_shared_info->vcpu_info[0];
+}
+
+void __init xen_guest_init(void)
+{
+	int r;
+	int major, minor;
+
+	if (xen_pv_domain())
+		return;
+
+	r = init_hvm_pv_info(&major, &minor);
+	if (r < 0)
+		return;
+
+	init_shared_info();
+}
diff --git a/drivers/input/xen-kbdfront.c b/drivers/input/xen-kbdfront.c
index e140816..7451d78 100644
--- a/drivers/input/xen-kbdfront.c
+++ b/drivers/input/xen-kbdfront.c
@@ -339,7 +339,7 @@ static struct xenbus_driver xenkbd_driver = {
 
 static int __init xenkbd_init(void)
 {
-	if (!xen_domain())
+	if (!xen_domain() || xen_hvm_domain())
 		return -ENODEV;
 
 	/* Nothing to do if running in dom0. */
diff --git a/drivers/video/xen-fbfront.c b/drivers/video/xen-fbfront.c
index fa97d3e..a105a19 100644
--- a/drivers/video/xen-fbfront.c
+++ b/drivers/video/xen-fbfront.c
@@ -684,7 +684,7 @@ static struct xenbus_driver xenfb_driver = {
 
 static int __init xenfb_init(void)
 {
-	if (!xen_domain())
+	if (!xen_domain() || xen_hvm_domain())
 		return -ENODEV;
 
 	/* Nothing to do if running in dom0. */
diff --git a/drivers/xen/xenbus/xenbus_probe.c b/drivers/xen/xenbus/xenbus_probe.c
index 3479332..0b05b62 100644
--- a/drivers/xen/xenbus/xenbus_probe.c
+++ b/drivers/xen/xenbus/xenbus_probe.c
@@ -56,6 +56,8 @@
 #include <xen/events.h>
 #include <xen/page.h>
 
+#include <xen/hvm.h>
+
 #include "xenbus_comms.h"
 #include "xenbus_probe.h"
 
@@ -806,10 +808,23 @@ static int __init xenbus_probe_init(void)
 		/* dom0 not yet supported */
 	} else {
 		xenstored_ready = 1;
-		xen_store_evtchn = xen_start_info->store_evtchn;
-		xen_store_mfn = xen_start_info->store_mfn;
+		if (xen_hvm_domain()) {
+			uint64_t v = 0;
+			err = hvm_get_parameter(HVM_PARAM_STORE_EVTCHN, &v);
+			if (err)
+				goto out_error;
+			xen_store_evtchn = (int)v;
+			err = hvm_get_parameter(HVM_PARAM_STORE_PFN, &v);
+			if (err)
+				goto out_error;
+			xen_store_mfn = (unsigned long)v;
+			xen_store_interface = ioremap(xen_store_mfn << PAGE_SHIFT, PAGE_SIZE);
+		} else {
+			xen_store_evtchn = xen_start_info->store_evtchn;
+			xen_store_mfn = xen_start_info->store_mfn;
+			xen_store_interface = mfn_to_virt(xen_store_mfn);
+		}
 	}
-	xen_store_interface = mfn_to_virt(xen_store_mfn);
 
 	/* Initialize the interface to xenstore. */
 	err = xs_init();
diff --git a/include/xen/xen.h b/include/xen/xen.h
index a164024..cb8c48b 100644
--- a/include/xen/xen.h
+++ b/include/xen/xen.h
@@ -9,8 +9,10 @@ enum xen_domain_type {
 
 #ifdef CONFIG_XEN
 extern enum xen_domain_type xen_domain_type;
+extern void xen_guest_init(void);
 #else
 #define xen_domain_type		XEN_NATIVE
+#define xen_guest_init() do { } while (0)
 #endif
 
 #define xen_domain()		(xen_domain_type != XEN_NATIVE)
-- 
1.7.0.4

Re: [Xen-devel] [PATCH 02/12] early PV on HVM

[Konrad Rzeszutek Wilk]

On Thu, Jun 03, 2010 at 02:10:35PM +0100, stefano.stabellini@eu.citrix.com wrote:
> From: Sheng Yang <sheng@linux.intel.com>
> 
> Initialize basic pv on hvm features in xen_guest_init.
> 
> The hook in arch/x86/kernel/setup.c can easily be removed using the new
> generic hypervisor independent initialization infrastructure present in
> the linux-2.6-tip tree.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Signed-off-by: Sheng Yang <sheng@linux.intel.com>
> Signed-off-by: Yaozu (Eddie) Dong <eddie.dong@intel.com>
> ---
>  arch/x86/kernel/setup.c           |    2 +
>  arch/x86/xen/enlighten.c          |   86 +++++++++++++++++++++++++++++++++++++
>  drivers/input/xen-kbdfront.c      |    2 +-
>  drivers/video/xen-fbfront.c       |    2 +-
>  drivers/xen/xenbus/xenbus_probe.c |   21 ++++++++-
>  include/xen/xen.h                 |    2 +
>  6 files changed, 110 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index c4851ef..ae9b6cb 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -69,6 +69,7 @@
>  #include <linux/tboot.h>
>  
>  #include <video/edid.h>
> +#include <xen/xen.h>
>  
>  #include <asm/mtrr.h>
>  #include <asm/apic.h>
> @@ -1032,6 +1033,7 @@ void __init setup_arch(char **cmdline_p)
>  	probe_nr_irqs_gsi();
>  
>  	kvm_guest_init();
> +	xen_guest_init();
>  
>  	e820_reserve_resources();
>  	e820_mark_nosave_regions(max_low_pfn);
> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> index 65d8d79..c1f6545 100644
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
> @@ -35,6 +35,7 @@
>  #include <xen/interface/version.h>
>  #include <xen/interface/physdev.h>
>  #include <xen/interface/vcpu.h>
> +#include <xen/interface/memory.h>
>  #include <xen/features.h>
>  #include <xen/page.h>
>  #include <xen/hvc-console.h>
> @@ -56,6 +57,7 @@
>  #include <asm/tlbflush.h>
>  #include <asm/reboot.h>
>  #include <asm/stackprotector.h>
> +#include <asm/hypervisor.h>
>  
>  #include "xen-ops.h"
>  #include "mmu.h"
> @@ -1206,3 +1208,87 @@ asmlinkage void __init xen_start_kernel(void)
>  	x86_64_start_reservations((char *)__pa_symbol(&boot_params));
>  #endif
>  }
> +
> +static uint32_t xen_cpuid_base(void)
> +{
> +	uint32_t base, eax, ebx, ecx, edx;
> +	char signature[13];
> +
> +	for (base = 0x40000000; base < 0x40010000; base += 0x100) {
> +		cpuid(base, &eax, &ebx, &ecx, &edx);
> +		*(uint32_t*)(signature + 0) = ebx;
> +		*(uint32_t*)(signature + 4) = ecx;
> +		*(uint32_t*)(signature + 8) = edx;
> +		signature[12] = 0;
> +
> +		if (!strcmp("XenVMMXenVMM", signature) && ((eax - base) >= 2))
> +			return base;
> +	}
> +
> +	return 0;
> +}
> +
> +static int init_hvm_pv_info(int *major, int *minor)
> +{
> +	uint32_t eax, ebx, ecx, edx, pages, msr, base;
> +	u64 pfn;
> +
> +	base = xen_cpuid_base();
> +	if (!base)
> +		return -EINVAL;
> +
> +	cpuid(base + 1, &eax, &ebx, &ecx, &edx);
> +
> +	*major = eax >> 16;
> +	*minor = eax & 0xffff;
> +	printk(KERN_INFO "Xen version %d.%d.\n", *major, *minor);
> +
> +	cpuid(base + 2, &pages, &msr, &ecx, &edx);
> +
> +	pfn = __pa(hypercall_page);
> +	wrmsr_safe(msr, (u32)pfn, (u32)(pfn >> 32));
> +
> +	xen_setup_features();
> +
> +	pv_info = xen_info;
> +	pv_info.kernel_rpl = 0;
> +
> +	xen_domain_type = XEN_HVM_DOMAIN;
> +
> +	return 0;
> +}
> +
> +static void __init init_shared_info(void)
> +{
> +	struct xen_add_to_physmap xatp;
> +	struct shared_info *shared_info_page;
> +
> +	shared_info_page = (struct shared_info *) alloc_bootmem_pages(PAGE_SIZE);
> +	xatp.domid = DOMID_SELF;
> +	xatp.idx = 0;
> +	xatp.space = XENMAPSPACE_shared_info;
> +	xatp.gpfn = __pa(shared_info_page) >> PAGE_SHIFT;
> +	if (HYPERVISOR_memory_op(XENMEM_add_to_physmap, &xatp))
> +		BUG();
> +
> +	HYPERVISOR_shared_info = (struct shared_info *)shared_info_page;
> +
> +	/* Don't do the full vcpu_info placement stuff until we have a
> +	   possible map and a non-dummy shared_info. */

Might want to mention where the full vpcu placement is done.
> +	per_cpu(xen_vcpu, 0) = &HYPERVISOR_shared_info->vcpu_info[0];

Otherwise looks good to me. In other words, Reviewed-by: Konrad
Rzeszutek Wilk <konrad.wilk@oracle.com>

Re: [Xen-devel] [PATCH 02/12] early PV on HVM

[Stefano Stabellini]

On Fri, 4 Jun 2010, Konrad Rzeszutek Wilk wrote:
> On Thu, Jun 03, 2010 at 02:10:35PM +0100, stefano.stabellini@eu.citrix.com wrote:
> > From: Sheng Yang <sheng@linux.intel.com>
> > 
> > Initialize basic pv on hvm features in xen_guest_init.
> > 
> > The hook in arch/x86/kernel/setup.c can easily be removed using the new
> > generic hypervisor independent initialization infrastructure present in
> > the linux-2.6-tip tree.
> > 
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > Signed-off-by: Sheng Yang <sheng@linux.intel.com>
> > Signed-off-by: Yaozu (Eddie) Dong <eddie.dong@intel.com>
> > ---
> >  arch/x86/kernel/setup.c           |    2 +
> >  arch/x86/xen/enlighten.c          |   86 +++++++++++++++++++++++++++++++++++++
> >  drivers/input/xen-kbdfront.c      |    2 +-
> >  drivers/video/xen-fbfront.c       |    2 +-
> >  drivers/xen/xenbus/xenbus_probe.c |   21 ++++++++-
> >  include/xen/xen.h                 |    2 +
> >  6 files changed, 110 insertions(+), 5 deletions(-)
> > 
> > diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> > index c4851ef..ae9b6cb 100644
> > --- a/arch/x86/kernel/setup.c
> > +++ b/arch/x86/kernel/setup.c
> > @@ -69,6 +69,7 @@
> >  #include <linux/tboot.h>
> >  
> >  #include <video/edid.h>
> > +#include <xen/xen.h>
> >  
> >  #include <asm/mtrr.h>
> >  #include <asm/apic.h>
> > @@ -1032,6 +1033,7 @@ void __init setup_arch(char **cmdline_p)
> >  	probe_nr_irqs_gsi();
> >  
> >  	kvm_guest_init();
> > +	xen_guest_init();
> >  
> >  	e820_reserve_resources();
> >  	e820_mark_nosave_regions(max_low_pfn);
> > diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> > index 65d8d79..c1f6545 100644
> > --- a/arch/x86/xen/enlighten.c
> > +++ b/arch/x86/xen/enlighten.c
> > @@ -35,6 +35,7 @@
> >  #include <xen/interface/version.h>
> >  #include <xen/interface/physdev.h>
> >  #include <xen/interface/vcpu.h>
> > +#include <xen/interface/memory.h>
> >  #include <xen/features.h>
> >  #include <xen/page.h>
> >  #include <xen/hvc-console.h>
> > @@ -56,6 +57,7 @@
> >  #include <asm/tlbflush.h>
> >  #include <asm/reboot.h>
> >  #include <asm/stackprotector.h>
> > +#include <asm/hypervisor.h>
> >  
> >  #include "xen-ops.h"
> >  #include "mmu.h"
> > @@ -1206,3 +1208,87 @@ asmlinkage void __init xen_start_kernel(void)
> >  	x86_64_start_reservations((char *)__pa_symbol(&boot_params));
> >  #endif
> >  }
> > +
> > +static uint32_t xen_cpuid_base(void)
> > +{
> > +	uint32_t base, eax, ebx, ecx, edx;
> > +	char signature[13];
> > +
> > +	for (base = 0x40000000; base < 0x40010000; base += 0x100) {
> > +		cpuid(base, &eax, &ebx, &ecx, &edx);
> > +		*(uint32_t*)(signature + 0) = ebx;
> > +		*(uint32_t*)(signature + 4) = ecx;
> > +		*(uint32_t*)(signature + 8) = edx;
> > +		signature[12] = 0;
> > +
> > +		if (!strcmp("XenVMMXenVMM", signature) && ((eax - base) >= 2))
> > +			return base;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int init_hvm_pv_info(int *major, int *minor)
> > +{
> > +	uint32_t eax, ebx, ecx, edx, pages, msr, base;
> > +	u64 pfn;
> > +
> > +	base = xen_cpuid_base();
> > +	if (!base)
> > +		return -EINVAL;
> > +
> > +	cpuid(base + 1, &eax, &ebx, &ecx, &edx);
> > +
> > +	*major = eax >> 16;
> > +	*minor = eax & 0xffff;
> > +	printk(KERN_INFO "Xen version %d.%d.\n", *major, *minor);
> > +
> > +	cpuid(base + 2, &pages, &msr, &ecx, &edx);
> > +
> > +	pfn = __pa(hypercall_page);
> > +	wrmsr_safe(msr, (u32)pfn, (u32)(pfn >> 32));
> > +
> > +	xen_setup_features();
> > +
> > +	pv_info = xen_info;
> > +	pv_info.kernel_rpl = 0;
> > +
> > +	xen_domain_type = XEN_HVM_DOMAIN;
> > +
> > +	return 0;
> > +}
> > +
> > +static void __init init_shared_info(void)
> > +{
> > +	struct xen_add_to_physmap xatp;
> > +	struct shared_info *shared_info_page;
> > +
> > +	shared_info_page = (struct shared_info *) alloc_bootmem_pages(PAGE_SIZE);
> > +	xatp.domid = DOMID_SELF;
> > +	xatp.idx = 0;
> > +	xatp.space = XENMAPSPACE_shared_info;
> > +	xatp.gpfn = __pa(shared_info_page) >> PAGE_SHIFT;
> > +	if (HYPERVISOR_memory_op(XENMEM_add_to_physmap, &xatp))
> > +		BUG();
> > +
> > +	HYPERVISOR_shared_info = (struct shared_info *)shared_info_page;
> > +
> > +	/* Don't do the full vcpu_info placement stuff until we have a
> > +	   possible map and a non-dummy shared_info. */
> 
> Might want to mention where the full vpcu placement is done.

The comment is not accurate, we actually don't do any vcpu_info
placement on hvm because it is not very useful there.
Better just to remove the comment (I have done so in my tree).

> > +	per_cpu(xen_vcpu, 0) = &HYPERVISOR_shared_info->vcpu_info[0];
> 
> Otherwise looks good to me. In other words, Reviewed-by: Konrad
> Rzeszutek Wilk <konrad.wilk@oracle.com>
> 

thanks!

Re: [PATCH 02/12] early PV on HVM

[Konrad Rzeszutek Wilk]

> > > +	HYPERVISOR_shared_info = (struct shared_info *)shared_info_page;
> > > +
> > > +	/* Don't do the full vcpu_info placement stuff until we have a
> > > +	   possible map and a non-dummy shared_info. */
> > 
> > Might want to mention where the full vpcu placement is done.
> 
> The comment is not accurate, we actually don't do any vcpu_info
> placement on hvm because it is not very useful there.
> Better just to remove the comment (I have done so in my tree).
> 
> > > +	per_cpu(xen_vcpu, 0) = &HYPERVISOR_shared_info->vcpu_info[0];
> > 
So.. what is the purpose of the per_cpu(xen_vcpu, 0) then?

Re: [Xen-devel] [PATCH 02/12] early PV on HVM

[Stefano Stabellini]

On Tue, 8 Jun 2010, Konrad Rzeszutek Wilk wrote:
> > > > +	HYPERVISOR_shared_info = (struct shared_info *)shared_info_page;
> > > > +
> > > > +	/* Don't do the full vcpu_info placement stuff until we have a
> > > > +	   possible map and a non-dummy shared_info. */
> > > 
> > > Might want to mention where the full vpcu placement is done.
> > 
> > The comment is not accurate, we actually don't do any vcpu_info
> > placement on hvm because it is not very useful there.
> > Better just to remove the comment (I have done so in my tree).
> > 
> > > > +	per_cpu(xen_vcpu, 0) = &HYPERVISOR_shared_info->vcpu_info[0];
> > > 
> So.. what is the purpose of the per_cpu(xen_vcpu, 0) then?
> 

the vcpu info placement memory area is stored in per_cpu(xen_vcpu_info, cpu);
per_cpu(xen_vcpu, cpu) is just a pointer to that area if it is
available, otherwise it points to the vcpu_info struct in the shared
info page.

Re: [Xen-devel] [PATCH 02/12] early PV on HVM

[Konrad Rzeszutek Wilk]

On Tue, Jun 08, 2010 at 04:55:33PM +0100, Stefano Stabellini wrote:
> On Tue, 8 Jun 2010, Konrad Rzeszutek Wilk wrote:
> > > > > +	HYPERVISOR_shared_info = (struct shared_info *)shared_info_page;
> > > > > +
> > > > > +	/* Don't do the full vcpu_info placement stuff until we have a
> > > > > +	   possible map and a non-dummy shared_info. */
> > > > 
> > > > Might want to mention where the full vpcu placement is done.
> > > 
> > > The comment is not accurate, we actually don't do any vcpu_info
> > > placement on hvm because it is not very useful there.
> > > Better just to remove the comment (I have done so in my tree).
> > > 
> > > > > +	per_cpu(xen_vcpu, 0) = &HYPERVISOR_shared_info->vcpu_info[0];
> > > > 
> > So.. what is the purpose of the per_cpu(xen_vcpu, 0) then?
> > 
> 
> the vcpu info placement memory area is stored in per_cpu(xen_vcpu_info, cpu);
> per_cpu(xen_vcpu, cpu) is just a pointer to that area if it is
> available, otherwise it points to the vcpu_info struct in the shared
> info page.

I was just wondering why are we doing this when you say:
" don't do any vcpu_info placement on hvm because it is not very useful there."

So if it is not useful, why do it?

Re: [PATCH 02/12] early PV on HVM

[Stefano Stabellini]

On Tue, 8 Jun 2010, Konrad Rzeszutek Wilk wrote:
> On Tue, Jun 08, 2010 at 04:55:33PM +0100, Stefano Stabellini wrote:
> > On Tue, 8 Jun 2010, Konrad Rzeszutek Wilk wrote:
> > > > > > +	HYPERVISOR_shared_info = (struct shared_info *)shared_info_page;
> > > > > > +
> > > > > > +	/* Don't do the full vcpu_info placement stuff until we have a
> > > > > > +	   possible map and a non-dummy shared_info. */
> > > > > 
> > > > > Might want to mention where the full vpcu placement is done.
> > > > 
> > > > The comment is not accurate, we actually don't do any vcpu_info
> > > > placement on hvm because it is not very useful there.
> > > > Better just to remove the comment (I have done so in my tree).
> > > > 
> > > > > > +	per_cpu(xen_vcpu, 0) = &HYPERVISOR_shared_info->vcpu_info[0];
> > > > > 
> > > So.. what is the purpose of the per_cpu(xen_vcpu, 0) then?
> > > 
> > 
> > the vcpu info placement memory area is stored in per_cpu(xen_vcpu_info, cpu);
> > per_cpu(xen_vcpu, cpu) is just a pointer to that area if it is
> > available, otherwise it points to the vcpu_info struct in the shared
> > info page.
> 
> I was just wondering why are we doing this when you say:
> " don't do any vcpu_info placement on hvm because it is not very useful there."
> 
> So if it is not useful, why do it?
> 

I think Jeremy replied to your question better than me: we still need
the vcpu_info stuff for the timer and event channels, but we don't need
it to be at a specific address in kernel memory.
That is useful only for the following pvops operations:

pv_irq_ops.save_fl = __PV_IS_CALLEE_SAVE(xen_save_fl_direct);
pv_irq_ops.restore_fl = __PV_IS_CALLEE_SAVE(xen_restore_fl_direct);
pv_irq_ops.irq_disable = __PV_IS_CALLEE_SAVE(xen_irq_disable_direct);
pv_irq_ops.irq_enable = __PV_IS_CALLEE_SAVE(xen_irq_enable_direct);
pv_mmu_ops.read_cr2 = xen_read_cr2_direct;

none of which are used in the hvm case.

Re: [PATCH 02/12] early PV on HVM

[Konrad Rzeszutek Wilk]

On Tue, Jun 08, 2010 at 05:25:52PM +0100, Stefano Stabellini wrote:
> On Tue, 8 Jun 2010, Konrad Rzeszutek Wilk wrote:
> > On Tue, Jun 08, 2010 at 04:55:33PM +0100, Stefano Stabellini wrote:
> > > On Tue, 8 Jun 2010, Konrad Rzeszutek Wilk wrote:
> > > > > > > +	HYPERVISOR_shared_info = (struct shared_info *)shared_info_page;
> > > > > > > +
> > > > > > > +	/* Don't do the full vcpu_info placement stuff until we have a
> > > > > > > +	   possible map and a non-dummy shared_info. */
> > > > > > 
> > > > > > Might want to mention where the full vpcu placement is done.
> > > > > 
> > > > > The comment is not accurate, we actually don't do any vcpu_info
> > > > > placement on hvm because it is not very useful there.
> > > > > Better just to remove the comment (I have done so in my tree).
> > > > > 
> > > > > > > +	per_cpu(xen_vcpu, 0) = &HYPERVISOR_shared_info->vcpu_info[0];
> > > > > > 
> > > > So.. what is the purpose of the per_cpu(xen_vcpu, 0) then?
> > > > 
> > > 
> > > the vcpu info placement memory area is stored in per_cpu(xen_vcpu_info, cpu);
> > > per_cpu(xen_vcpu, cpu) is just a pointer to that area if it is
> > > available, otherwise it points to the vcpu_info struct in the shared
> > > info page.
> > 
> > I was just wondering why are we doing this when you say:
> > " don't do any vcpu_info placement on hvm because it is not very useful there."
> > 
> > So if it is not useful, why do it?
> > 
> 
> I think Jeremy replied to your question better than me: we still need
> the vcpu_info stuff for the timer and event channels, but we don't need
> it to be at a specific address in kernel memory.

Ok, can you add that comment for the usage of the per_cpu(xen_vcpu,0)
and mention that this is bootstrap code - hence only starting at CPU 0.

Re: [PATCH 02/12] early PV on HVM

[Stefano Stabellini]

On Tue, 8 Jun 2010, Konrad Rzeszutek Wilk wrote:
> On Tue, Jun 08, 2010 at 05:25:52PM +0100, Stefano Stabellini wrote:
> > On Tue, 8 Jun 2010, Konrad Rzeszutek Wilk wrote:
> > > On Tue, Jun 08, 2010 at 04:55:33PM +0100, Stefano Stabellini wrote:
> > > > On Tue, 8 Jun 2010, Konrad Rzeszutek Wilk wrote:
> > > > > > > > +	HYPERVISOR_shared_info = (struct shared_info *)shared_info_page;
> > > > > > > > +
> > > > > > > > +	/* Don't do the full vcpu_info placement stuff until we have a
> > > > > > > > +	   possible map and a non-dummy shared_info. */
> > > > > > > 
> > > > > > > Might want to mention where the full vpcu placement is done.
> > > > > > 
> > > > > > The comment is not accurate, we actually don't do any vcpu_info
> > > > > > placement on hvm because it is not very useful there.
> > > > > > Better just to remove the comment (I have done so in my tree).
> > > > > > 
> > > > > > > > +	per_cpu(xen_vcpu, 0) = &HYPERVISOR_shared_info->vcpu_info[0];
> > > > > > > 
> > > > > So.. what is the purpose of the per_cpu(xen_vcpu, 0) then?
> > > > > 
> > > > 
> > > > the vcpu info placement memory area is stored in per_cpu(xen_vcpu_info, cpu);
> > > > per_cpu(xen_vcpu, cpu) is just a pointer to that area if it is
> > > > available, otherwise it points to the vcpu_info struct in the shared
> > > > info page.
> > > 
> > > I was just wondering why are we doing this when you say:
> > > " don't do any vcpu_info placement on hvm because it is not very useful there."
> > > 
> > > So if it is not useful, why do it?
> > > 
> > 
> > I think Jeremy replied to your question better than me: we still need
> > the vcpu_info stuff for the timer and event channels, but we don't need
> > it to be at a specific address in kernel memory.
> 
> Ok, can you add that comment for the usage of the per_cpu(xen_vcpu,0)
> and mention that this is bootstrap code - hence only starting at CPU 0.
> 

All right, I'll do that.

Re: [Xen-devel] [PATCH 02/12] early PV on HVM

[Jeremy Fitzhardinge]

On 06/08/2010 06:46 AM, Konrad Rzeszutek Wilk wrote:
>>>> +	per_cpu(xen_vcpu, 0) = &HYPERVISOR_shared_info->vcpu_info[0];
>>>>         
>>>       
> So.. what is the purpose of the per_cpu(xen_vcpu, 0) then?
>   

We still need the percpu info for things like time, but it is mapped out
of the platform device rather than placed at any address we want in
kernel memory (vcpu info placement), where we can access it directly as
a percpu variable.  This is only really relevent for the implementation
of the interrupt enable/disable pvops, so it doesn't matter for hvm.

    J

Re: [Xen-devel] [PATCH 02/12] early PV on HVM

[Konrad Rzeszutek Wilk]

> +static uint32_t xen_cpuid_base(void)
> +{
> +	uint32_t base, eax, ebx, ecx, edx;
> +	char signature[13];
> +
> +	for (base = 0x40000000; base < 0x40010000; base += 0x100) {
> +		cpuid(base, &eax, &ebx, &ecx, &edx);
> +		*(uint32_t*)(signature + 0) = ebx;
> +		*(uint32_t*)(signature + 4) = ecx;
> +		*(uint32_t*)(signature + 8) = edx;
> +		signature[12] = 0;
> +
> +		if (!strcmp("XenVMMXenVMM", signature) && ((eax - base) >= 2))
> +			return base;
> +	}
> +
> +	return 0;
> +}

I forgot to mention this in the previous e-mail, but if you are rebasing
this against 2.6.34 it might be worth taking a look at the Microsoft HyperV code.
It also utilizes the cpuid code and this code could be made more generic.

Re: [Xen-devel] [PATCH 02/12] early PV on HVM

[Stefano Stabellini]

On Fri, 4 Jun 2010, Konrad Rzeszutek Wilk wrote:
> > +static uint32_t xen_cpuid_base(void)
> > +{
> > +	uint32_t base, eax, ebx, ecx, edx;
> > +	char signature[13];
> > +
> > +	for (base = 0x40000000; base < 0x40010000; base += 0x100) {
> > +		cpuid(base, &eax, &ebx, &ecx, &edx);
> > +		*(uint32_t*)(signature + 0) = ebx;
> > +		*(uint32_t*)(signature + 4) = ecx;
> > +		*(uint32_t*)(signature + 8) = edx;
> > +		signature[12] = 0;
> > +
> > +		if (!strcmp("XenVMMXenVMM", signature) && ((eax - base) >= 2))
> > +			return base;
> > +	}
> > +
> > +	return 0;
> > +}
> 
> I forgot to mention this in the previous e-mail, but if you are rebasing
> this against 2.6.34 it might be worth taking a look at the Microsoft HyperV code.
> It also utilizes the cpuid code and this code could be made more generic.
> 

these patches are already based on 2.6.34, I didn't use the generic
hypervisor setup code because it is not available in 2.6.34, however
in the future rebasing to use that is going to be quite easy.

[PATCH 03/12] evtchn delivery on HVM

[stefano.stabellini]

From: Sheng Yang <sheng@linux.intel.com>

Set the callback to receive evtchns from Xen, using the
callback vector delivery mechanism.

The traditional way for receiving event channel notifications from Xen
is interrupts from the platform PCI device.
The callback vector is a newer alternative that allow us to receive
notifications on any vcpu and doesn't need any PCI support: we allocate
a vector exclusively to receive events, in the vector handler we don't
need to interact with the vlapic, therefore we avoid a VMEXIT.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Signed-off-by: Sheng Yang <sheng@linux.intel.com>
---
 arch/x86/include/asm/irq_vectors.h |    5 ++
 arch/x86/kernel/entry_32.S         |    2 +
 arch/x86/kernel/entry_64.S         |    3 +
 arch/x86/xen/enlighten.c           |   29 +++++++++++++
 arch/x86/xen/xen-ops.h             |    2 +
 drivers/xen/events.c               |   82 ++++++++++++++++++++++++++++++++---
 include/xen/events.h               |    7 +++
 include/xen/hvm.h                  |    6 +++
 include/xen/interface/features.h   |    3 +
 9 files changed, 132 insertions(+), 7 deletions(-)

diff --git a/arch/x86/include/asm/irq_vectors.h b/arch/x86/include/asm/irq_vectors.h
index 8767d99..ab6ac1b 100644
--- a/arch/x86/include/asm/irq_vectors.h
+++ b/arch/x86/include/asm/irq_vectors.h
@@ -125,6 +125,11 @@
  */
 #define MCE_SELF_VECTOR			0xeb
 
+#ifdef CONFIG_XEN
+/* Xen vector callback to receive events in a HVM domain */
+#define XEN_HVM_EVTCHN_CALLBACK		0xe9
+#endif
+
 #define NR_VECTORS			 256
 
 #define FPU_IRQ				  13
diff --git a/arch/x86/kernel/entry_32.S b/arch/x86/kernel/entry_32.S
index 44a8e0d..88ed128 100644
--- a/arch/x86/kernel/entry_32.S
+++ b/arch/x86/kernel/entry_32.S
@@ -1147,6 +1147,8 @@ ENTRY(xen_failsafe_callback)
 .previous
 ENDPROC(xen_failsafe_callback)
 
+BUILD_INTERRUPT(xen_hvm_callback_vector, XEN_HVM_EVTCHN_CALLBACK)
+
 #endif	/* CONFIG_XEN */
 
 #ifdef CONFIG_FUNCTION_TRACER
diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index 0697ff1..d40c1e1 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -1329,6 +1329,9 @@ ENTRY(xen_failsafe_callback)
 	CFI_ENDPROC
 END(xen_failsafe_callback)
 
+apicinterrupt XEN_HVM_EVTCHN_CALLBACK \
+	xen_hvm_callback_vector smp_xen_hvm_callback_vector
+
 #endif /* CONFIG_XEN */
 
 /*
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index c1f6545..ed34b30 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -11,6 +11,7 @@
  * Jeremy Fitzhardinge <jeremy@xensource.com>, XenSource Inc, 2007
  */
 
+#include <linux/cpu.h>
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/smp.h>
@@ -38,6 +39,7 @@
 #include <xen/interface/memory.h>
 #include <xen/features.h>
 #include <xen/page.h>
+#include <xen/hvm.h>
 #include <xen/hvc-console.h>
 
 #include <asm/paravirt.h>
@@ -78,6 +80,9 @@ struct shared_info xen_dummy_shared_info;
 
 void *xen_initial_gdt;
 
+__read_mostly int xen_have_vector_callback;
+EXPORT_SYMBOL_GPL(xen_have_vector_callback);
+
 /*
  * Point at some empty memory to start with. We map the real shared_info
  * page as soon as fixmap is up and running.
@@ -1278,6 +1283,24 @@ static void __init init_shared_info(void)
 	per_cpu(xen_vcpu, 0) = &HYPERVISOR_shared_info->vcpu_info[0];
 }
 
+static int __cpuinit xen_hvm_cpu_notify(struct notifier_block *self,
+				    unsigned long action, void *hcpu)
+{
+	int cpu = (long)hcpu;
+	switch (action) {
+	case CPU_UP_PREPARE:
+		per_cpu(xen_vcpu, cpu) = &HYPERVISOR_shared_info->vcpu_info[cpu];
+		break;
+	default:
+		break;
+	}
+	return NOTIFY_OK;
+}
+
+static struct notifier_block __cpuinitdata xen_hvm_cpu_notifier = {
+	.notifier_call	= xen_hvm_cpu_notify,
+};
+
 void __init xen_guest_init(void)
 {
 	int r;
@@ -1291,4 +1314,10 @@ void __init xen_guest_init(void)
 		return;
 
 	init_shared_info();
+
+	if (xen_feature(XENFEAT_hvm_callback_vector))
+		xen_have_vector_callback = 1;
+	register_cpu_notifier(&xen_hvm_cpu_notifier);
+	have_vcpu_info_placement = 0;
+	x86_init.irqs.intr_init = xen_init_IRQ;
 }
diff --git a/arch/x86/xen/xen-ops.h b/arch/x86/xen/xen-ops.h
index f9153a3..0d0e0e6 100644
--- a/arch/x86/xen/xen-ops.h
+++ b/arch/x86/xen/xen-ops.h
@@ -38,6 +38,8 @@ void xen_enable_sysenter(void);
 void xen_enable_syscall(void);
 void xen_vcpu_restore(void);
 
+void xen_callback_vector(void);
+
 void __init xen_build_dynamic_phys_to_machine(void);
 
 void xen_init_irq_ops(void);
diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index db8f506..8604604 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -29,6 +29,7 @@
 #include <linux/bootmem.h>
 #include <linux/slab.h>
 
+#include <asm/desc.h>
 #include <asm/ptrace.h>
 #include <asm/irq.h>
 #include <asm/idle.h>
@@ -36,10 +37,14 @@
 #include <asm/xen/hypercall.h>
 #include <asm/xen/hypervisor.h>
 
+#include <xen/xen.h>
+#include <xen/hvm.h>
 #include <xen/xen-ops.h>
 #include <xen/events.h>
 #include <xen/interface/xen.h>
 #include <xen/interface/event_channel.h>
+#include <xen/interface/hvm/hvm_op.h>
+#include <xen/interface/hvm/params.h>
 
 /*
  * This lock protects updates to the following mapping and reference-count
@@ -617,17 +622,13 @@ static DEFINE_PER_CPU(unsigned, xed_nesting_count);
  * a bitset of words which contain pending event bits.  The second
  * level is a bitset of pending events themselves.
  */
-void xen_evtchn_do_upcall(struct pt_regs *regs)
+static void __xen_evtchn_do_upcall(void)
 {
 	int cpu = get_cpu();
-	struct pt_regs *old_regs = set_irq_regs(regs);
 	struct shared_info *s = HYPERVISOR_shared_info;
 	struct vcpu_info *vcpu_info = __get_cpu_var(xen_vcpu);
  	unsigned count;
 
-	exit_idle();
-	irq_enter();
-
 	do {
 		unsigned long pending_words;
 
@@ -667,10 +668,26 @@ void xen_evtchn_do_upcall(struct pt_regs *regs)
 	} while(count != 1);
 
 out:
+
+	put_cpu();
+}
+
+void xen_evtchn_do_upcall(struct pt_regs *regs)
+{
+	struct pt_regs *old_regs = set_irq_regs(regs);
+
+	exit_idle();
+	irq_enter();
+
+	__xen_evtchn_do_upcall();
+
 	irq_exit();
 	set_irq_regs(old_regs);
+}
 
-	put_cpu();
+void xen_hvm_evtchn_do_upcall(void)
+{
+	__xen_evtchn_do_upcall();
 }
 
 /* Rebind a new event channel to an existing irq. */
@@ -933,6 +950,52 @@ static struct irq_chip xen_dynamic_chip __read_mostly = {
 	.retrigger	= retrigger_dynirq,
 };
 
+int xen_set_callback_via(uint64_t via)
+{
+	struct xen_hvm_param a;
+	a.domid = DOMID_SELF;
+	a.index = HVM_PARAM_CALLBACK_IRQ;
+	a.value = via;
+	return HYPERVISOR_hvm_op(HVMOP_set_param, &a);
+}
+EXPORT_SYMBOL_GPL(xen_set_callback_via);
+
+void smp_xen_hvm_callback_vector(struct pt_regs *regs)
+{
+	struct pt_regs *old_regs = set_irq_regs(regs);
+
+	exit_idle();
+
+	irq_enter();
+
+	__xen_evtchn_do_upcall();
+
+	irq_exit();
+
+	set_irq_regs(old_regs);
+}
+
+/* the callback vector mechanism is a newer alternative way of receiving
+ * event channel notifications from Xen: we can receive vector callbacks
+ * on any vcpus and we don't need any PCI or IO APIC support */
+void xen_callback_vector(void)
+{
+	int rc;
+	uint64_t callback_via;
+	if (xen_have_vector_callback) {
+		callback_via = HVM_CALLBACK_VECTOR(XEN_HVM_EVTCHN_CALLBACK);
+		rc = xen_set_callback_via(callback_via);
+		if (rc) {
+			printk(KERN_ERR "request for callback vector failed\n");
+			xen_have_vector_callback = 0;
+			return;
+		}
+		printk(KERN_INFO "Xen HVM callback vector for event delivery is "
+				"enabled\n");
+		alloc_intr_gate(XEN_HVM_EVTCHN_CALLBACK, xen_hvm_callback_vector);
+	}
+}
+
 void __init xen_init_IRQ(void)
 {
 	int i;
@@ -947,5 +1010,10 @@ void __init xen_init_IRQ(void)
 	for (i = 0; i < NR_EVENT_CHANNELS; i++)
 		mask_evtchn(i);
 
-	irq_ctx_init(smp_processor_id());
+	if (xen_hvm_domain()) {
+		xen_callback_vector();
+		native_init_IRQ();
+	} else {
+		irq_ctx_init(smp_processor_id());
+	}
 }
diff --git a/include/xen/events.h b/include/xen/events.h
index e68d59a..a15d932 100644
--- a/include/xen/events.h
+++ b/include/xen/events.h
@@ -56,4 +56,11 @@ void xen_poll_irq(int irq);
 /* Determine the IRQ which is bound to an event channel */
 unsigned irq_from_evtchn(unsigned int evtchn);
 
+/* Xen HVM evtchn vector callback */
+extern void xen_hvm_callback_vector(void);
+extern int xen_have_vector_callback;
+int xen_set_callback_via(uint64_t via);
+void xen_evtchn_do_upcall(struct pt_regs *regs);
+void xen_hvm_evtchn_do_upcall(void);
+
 #endif	/* _XEN_EVENTS_H */
diff --git a/include/xen/hvm.h b/include/xen/hvm.h
index 6b0d418..d0cf7a6 100644
--- a/include/xen/hvm.h
+++ b/include/xen/hvm.h
@@ -3,6 +3,7 @@
 #define XEN_HVM_H__
 
 #include <xen/interface/hvm/params.h>
+#include <asm/xen/hypercall.h>
 
 static inline int hvm_get_parameter(int idx, uint64_t *value)
 {
@@ -21,4 +22,9 @@ static inline int hvm_get_parameter(int idx, uint64_t *value)
        return r;
 }
 
+#define HVM_CALLBACK_VIA_TYPE_VECTOR 0x2
+#define HVM_CALLBACK_VIA_TYPE_SHIFT 56
+#define HVM_CALLBACK_VECTOR(x) (((uint64_t)HVM_CALLBACK_VIA_TYPE_VECTOR)<<\
+                               HVM_CALLBACK_VIA_TYPE_SHIFT | (x))
+
 #endif /* XEN_HVM_H__ */
diff --git a/include/xen/interface/features.h b/include/xen/interface/features.h
index f51b641..8ab08b9 100644
--- a/include/xen/interface/features.h
+++ b/include/xen/interface/features.h
@@ -41,6 +41,9 @@
 /* x86: Does this Xen host support the MMU_PT_UPDATE_PRESERVE_AD hypercall? */
 #define XENFEAT_mmu_pt_update_preserve_ad  5
 
+/* x86: Does this Xen host support the HVM callback vector type? */
+#define XENFEAT_hvm_callback_vector        8
+
 #define XENFEAT_NR_SUBMAPS 1
 
 #endif /* __XEN_PUBLIC_FEATURES_H__ */
-- 
1.7.0.4

Re: [Xen-devel] [PATCH 03/12] evtchn delivery on HVM

[Konrad Rzeszutek Wilk]

> +static int __cpuinit xen_hvm_cpu_notify(struct notifier_block *self,
> +				    unsigned long action, void *hcpu)
> +{
> +	int cpu = (long)hcpu;
> +	switch (action) {
> +	case CPU_UP_PREPARE:
> +		per_cpu(xen_vcpu, cpu) = &HYPERVISOR_shared_info->vcpu_info[cpu];
> +		break;

Is there no need to do anything if the CPUs are brought down?

.. snip..

> +/* the callback vector mechanism is a newer alternative way of receiving
> + * event channel notifications from Xen: we can receive vector callbacks
> + * on any vcpus and we don't need any PCI or IO APIC support */

I am not an expert in English, but I think you need to revise this
comment.
> +void xen_callback_vector(void)
> +{
> +	int rc;
> +	uint64_t callback_via;
> +	if (xen_have_vector_callback) {
> +		callback_via = HVM_CALLBACK_VECTOR(XEN_HVM_EVTCHN_CALLBACK);
> +		rc = xen_set_callback_via(callback_via);
> +		if (rc) {
> +			printk(KERN_ERR "request for callback vector failed\n");

Perhaps mention which type? Say 'Request for Xen HVM callback vector
failed.\n' ?

Re: [Xen-devel] [PATCH 03/12] evtchn delivery on HVM

[Stefano Stabellini]

On Mon, 14 Jun 2010, Konrad Rzeszutek Wilk wrote:
> > +static int __cpuinit xen_hvm_cpu_notify(struct notifier_block *self,
> > +				    unsigned long action, void *hcpu)
> > +{
> > +	int cpu = (long)hcpu;
> > +	switch (action) {
> > +	case CPU_UP_PREPARE:
> > +		per_cpu(xen_vcpu, cpu) = &HYPERVISOR_shared_info->vcpu_info[cpu];
> > +		break;
> 
> Is there no need to do anything if the CPUs are brought down?
> 

nope


> > +/* the callback vector mechanism is a newer alternative way of receiving
> > + * event channel notifications from Xen: we can receive vector callbacks
> > + * on any vcpus and we don't need any PCI or IO APIC support */
> 
> I am not an expert in English, but I think you need to revise this
> comment.


/* vector callbacks are better than PCI interrupts to receive event
 * channel notifications because we can receive vector callbacks on any
 * vcpu and we don't need PCI support or APIC interactions */

is it good enough?


> > +void xen_callback_vector(void)
> > +{
> > +	int rc;
> > +	uint64_t callback_via;
> > +	if (xen_have_vector_callback) {
> > +		callback_via = HVM_CALLBACK_VECTOR(XEN_HVM_EVTCHN_CALLBACK);
> > +		rc = xen_set_callback_via(callback_via);
> > +		if (rc) {
> > +			printk(KERN_ERR "request for callback vector failed\n");
> 
> Perhaps mention which type? Say 'Request for Xen HVM callback vector
> failed.\n' ?
> 

good idea, I'll do that

Re: [PATCH 03/12] evtchn delivery on HVM

[Konrad Rzeszutek Wilk]

> /* vector callbacks are better than PCI interrupts to receive event
     ^V
>  * channel notifications because we can receive vector callbacks on any
>  * vcpu and we don't need PCI support or APIC interactions */
                                                            ^.
> 
> is it good enough?
Just two things..

Re: [Xen-devel] [PATCH 03/12] evtchn delivery on HVM

[Stefano Stabellini]

On Thu, 17 Jun 2010, Konrad Rzeszutek Wilk wrote:
> > /* vector callbacks are better than PCI interrupts to receive event
>      ^V
> >  * channel notifications because we can receive vector callbacks on any
> >  * vcpu and we don't need PCI support or APIC interactions */
>                                                             ^.
> > 
> > is it good enough?
> Just two things..
> 

Ok

[PATCH 04/12] Xen PCI platform device driver

[stefano.stabellini]

From: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

Add the xen pci platform device driver that is responsible
for initializing the grant table and xenbus in PV on HVM mode.
Few changes to xenbus and grant table are necessary to allow the delayed
initialization in HVM mode.
Grant table needs few additional modifications to work in HVM mode.

The Xen PCI platform device raises an irq every time an event has been
delivered to us. However these interrupts are only delivered to vcpu 0.
The Xen PCI platform interrupt handler calls xen_hvm_evtchn_do_upcall
that is a little wrapper around __xen_evtchn_do_upcall, the traditional
Xen upcall handler, the very same used with traditional PV guests.

When running on HVM the event channel upcall is never called while in
progress because it is a normal Linux irq handler, therefore we cannot
be sure that evtchn_upcall_pending is 0 when returning.
For this reason if evtchn_upcall_pending is set by Xen we need to loop
again on the event channels set pending otherwise we might loose some
event channel deliveries.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Signed-off-by: Sheng Yang <sheng@linux.intel.com>
---
 drivers/xen/Kconfig                 |    9 ++
 drivers/xen/Makefile                |    3 +-
 drivers/xen/events.c                |    7 +-
 drivers/xen/grant-table.c           |   69 ++++++++++++--
 drivers/xen/platform-pci.c          |  186 +++++++++++++++++++++++++++++++++++
 drivers/xen/xenbus/xenbus_probe.c   |   20 +++-
 include/linux/pci_ids.h             |    3 +
 include/xen/grant_table.h           |    1 +
 include/xen/interface/grant_table.h |    1 +
 include/xen/xenbus.h                |    1 +
 10 files changed, 284 insertions(+), 16 deletions(-)
 create mode 100644 drivers/xen/platform-pci.c

diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
index fad3df2..8819202 100644
--- a/drivers/xen/Kconfig
+++ b/drivers/xen/Kconfig
@@ -62,4 +62,13 @@ config XEN_SYS_HYPERVISOR
 	 virtual environment, /sys/hypervisor will still be present,
 	 but will have no xen contents.
 
+config XEN_PLATFORM_PCI
+	tristate "xen platform pci device driver"
+	depends on XEN
+	default y
+	help
+	  Driver for the Xen PCI Platform device: it is responsible for
+	  initializing xenbus and grant_table when running in a Xen HVM
+	  domain. As a consequence this driver is required to run any Xen PV
+	  frontend on Xen HVM.
 endmenu
diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
index 7c28434..e392fb7 100644
--- a/drivers/xen/Makefile
+++ b/drivers/xen/Makefile
@@ -9,4 +9,5 @@ obj-$(CONFIG_XEN_XENCOMM)	+= xencomm.o
 obj-$(CONFIG_XEN_BALLOON)	+= balloon.o
 obj-$(CONFIG_XEN_DEV_EVTCHN)	+= evtchn.o
 obj-$(CONFIG_XENFS)		+= xenfs/
-obj-$(CONFIG_XEN_SYS_HYPERVISOR)	+= sys-hypervisor.o
\ No newline at end of file
+obj-$(CONFIG_XEN_SYS_HYPERVISOR)	+= sys-hypervisor.o
+obj-$(CONFIG_XEN_PLATFORM_PCI)	+= platform-pci.o
diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index 8604604..be378e1 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -665,7 +665,7 @@ static void __xen_evtchn_do_upcall(void)
 
 		count = __get_cpu_var(xed_nesting_count);
 		__get_cpu_var(xed_nesting_count) = 0;
-	} while(count != 1);
+	} while(count != 1 || vcpu_info->evtchn_upcall_pending);
 
 out:
 
@@ -725,7 +725,10 @@ static int rebind_irq_to_cpu(unsigned irq, unsigned tcpu)
 	struct evtchn_bind_vcpu bind_vcpu;
 	int evtchn = evtchn_from_irq(irq);
 
-	if (!VALID_EVTCHN(evtchn))
+	/* events delivered via platform PCI interrupts are always
+	 * routed to vcpu 0 */
+	if (!VALID_EVTCHN(evtchn) ||
+		(xen_hvm_domain() && !xen_have_vector_callback))
 		return -1;
 
 	/* Send future instances of this interrupt to other vcpu. */
diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
index f66db3b..4c959a5 100644
--- a/drivers/xen/grant-table.c
+++ b/drivers/xen/grant-table.c
@@ -37,11 +37,13 @@
 #include <linux/slab.h>
 #include <linux/vmalloc.h>
 #include <linux/uaccess.h>
+#include <linux/io.h>
 
 #include <xen/xen.h>
 #include <xen/interface/xen.h>
 #include <xen/page.h>
 #include <xen/grant_table.h>
+#include <xen/interface/memory.h>
 #include <asm/xen/hypercall.h>
 
 #include <asm/pgtable.h>
@@ -59,6 +61,7 @@ static unsigned int boot_max_nr_grant_frames;
 static int gnttab_free_count;
 static grant_ref_t gnttab_free_head;
 static DEFINE_SPINLOCK(gnttab_list_lock);
+static unsigned long hvm_pv_resume_frames;
 
 static struct grant_entry *shared;
 
@@ -449,6 +452,30 @@ static int gnttab_map(unsigned int start_idx, unsigned int end_idx)
 	unsigned int nr_gframes = end_idx + 1;
 	int rc;
 
+	if (xen_hvm_domain()) {
+		struct xen_add_to_physmap xatp;
+		unsigned int i = end_idx;
+		rc = 0;
+		/*
+		 * Loop backwards, so that the first hypercall has the largest
+		 * index, ensuring that the table will grow only once.
+		 */
+		do {
+			xatp.domid = DOMID_SELF;
+			xatp.idx = i;
+			xatp.space = XENMAPSPACE_grant_table;
+			xatp.gpfn = (hvm_pv_resume_frames >> PAGE_SHIFT) + i;
+			rc = HYPERVISOR_memory_op(XENMEM_add_to_physmap, &xatp);
+			if (rc != 0) {
+				printk(KERN_WARNING
+						"grant table add_to_physmap failed, err=%d\n", rc);
+				break;
+			}
+		} while (i-- > start_idx);
+
+		return rc;
+	}
+
 	frames = kmalloc(nr_gframes * sizeof(unsigned long), GFP_ATOMIC);
 	if (!frames)
 		return -ENOMEM;
@@ -476,9 +503,28 @@ static int gnttab_map(unsigned int start_idx, unsigned int end_idx)
 
 int gnttab_resume(void)
 {
-	if (max_nr_grant_frames() < nr_grant_frames)
+	unsigned int max_nr_gframes;
+
+	max_nr_gframes = max_nr_grant_frames();
+	if (max_nr_gframes < nr_grant_frames)
 		return -ENOSYS;
-	return gnttab_map(0, nr_grant_frames - 1);
+
+	if (xen_pv_domain())
+		return gnttab_map(0, nr_grant_frames - 1);
+
+	if (!hvm_pv_resume_frames) {
+		hvm_pv_resume_frames = alloc_xen_mmio(PAGE_SIZE * max_nr_gframes);
+		shared = ioremap(hvm_pv_resume_frames, PAGE_SIZE * max_nr_gframes);
+		if (shared == NULL) {
+			printk(KERN_WARNING
+					"Fail to ioremap gnttab share frames\n");
+			return -ENOMEM;
+		}
+	}
+
+	gnttab_map(0, nr_grant_frames - 1);
+
+	return 0;
 }
 
 int gnttab_suspend(void)
@@ -505,15 +551,12 @@ static int gnttab_expand(unsigned int req_entries)
 	return rc;
 }
 
-static int __devinit gnttab_init(void)
+int gnttab_init(void)
 {
 	int i;
 	unsigned int max_nr_glist_frames, nr_glist_frames;
 	unsigned int nr_init_grefs;
 
-	if (!xen_domain())
-		return -ENODEV;
-
 	nr_grant_frames = 1;
 	boot_max_nr_grant_frames = __max_nr_grant_frames();
 
@@ -557,4 +600,16 @@ static int __devinit gnttab_init(void)
 	return -ENOMEM;
 }
 
-core_initcall(gnttab_init);
+static int __devinit __gnttab_init(void)
+{
+	/* Delay grant-table initialization in the PV on HVM case */
+	if (xen_hvm_domain())
+		return 0;
+
+	if (!xen_pv_domain())
+		return -ENODEV;
+
+	return gnttab_init();
+}
+
+core_initcall(__gnttab_init);
diff --git a/drivers/xen/platform-pci.c b/drivers/xen/platform-pci.c
new file mode 100644
index 0000000..9cb7242
--- /dev/null
+++ b/drivers/xen/platform-pci.c
@@ -0,0 +1,186 @@
+/******************************************************************************
+ * platform-pci.c
+ *
+ * Xen platform PCI device driver
+ * Copyright (c) 2005, Intel Corporation.
+ * Copyright (c) 2007, XenSource Inc.
+ * Copyright (c) 2010, Citrix
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; if not, write to the Free Software Foundation, Inc., 59 Temple
+ * Place - Suite 330, Boston, MA 02111-1307 USA.
+ *
+ */
+
+#include <asm/io.h>
+
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+
+#include <xen/grant_table.h>
+#include <xen/xenbus.h>
+#include <xen/events.h>
+#include <xen/hvm.h>
+
+#define DRV_NAME    "xen-platform-pci"
+
+MODULE_AUTHOR("ssmith@xensource.com and stefano.stabellini@eu.citrix.com");
+MODULE_DESCRIPTION("Xen platform PCI device");
+MODULE_LICENSE("GPL");
+
+static unsigned long platform_mmio;
+static unsigned long platform_mmio_alloc;
+static unsigned long platform_mmiolen;
+
+unsigned long alloc_xen_mmio(unsigned long len)
+{
+	unsigned long addr;
+
+	addr = platform_mmio + platform_mmio_alloc;
+	platform_mmio_alloc += len;
+	BUG_ON(platform_mmio_alloc > platform_mmiolen);
+
+	return addr;
+}
+
+static uint64_t get_callback_via(struct pci_dev *pdev)
+{
+	u8 pin;
+	int irq;
+
+	irq = pdev->irq;
+	if (irq < 16)
+		return irq; /* ISA IRQ */
+
+	pin = pdev->pin;
+
+	/* We don't know the GSI. Specify the PCI INTx line instead. */
+	return ((uint64_t)0x01 << 56) | /* PCI INTx identifier */
+		((uint64_t)pci_domain_nr(pdev->bus) << 32) |
+		((uint64_t)pdev->bus->number << 16) |
+		((uint64_t)(pdev->devfn & 0xff) << 8) |
+		((uint64_t)(pin - 1) & 3);
+}
+
+static irqreturn_t do_hvm_evtchn_intr(int irq, void *dev_id)
+{
+	xen_hvm_evtchn_do_upcall();
+	return IRQ_HANDLED;
+}
+
+static int xen_allocate_irq(struct pci_dev *pdev)
+{
+	return request_irq(pdev->irq, do_hvm_evtchn_intr,
+			IRQF_DISABLED | IRQF_NOBALANCING | IRQF_TRIGGER_RISING,
+			"xen-platform-pci", pdev);
+}
+
+static int __devinit platform_pci_init(struct pci_dev *pdev,
+				       const struct pci_device_id *ent)
+{
+	int i, ret;
+	long ioaddr, iolen;
+	long mmio_addr, mmio_len;
+	uint64_t callback_via;
+
+	i = pci_enable_device(pdev);
+	if (i)
+		return i;
+
+	ioaddr = pci_resource_start(pdev, 0);
+	iolen = pci_resource_len(pdev, 0);
+
+	mmio_addr = pci_resource_start(pdev, 1);
+	mmio_len = pci_resource_len(pdev, 1);
+
+	if (mmio_addr == 0 || ioaddr == 0) {
+		dev_err(&pdev->dev, "no resources found\n");
+		ret = -ENOENT;
+	}
+
+	if (request_mem_region(mmio_addr, mmio_len, DRV_NAME) == NULL) {
+		dev_err(&pdev->dev, "MEM I/O resource 0x%lx @ 0x%lx busy\n",
+		       mmio_addr, mmio_len);
+		ret = -EBUSY;
+	}
+
+	if (request_region(ioaddr, iolen, DRV_NAME) == NULL) {
+		dev_err(&pdev->dev, "I/O resource 0x%lx @ 0x%lx busy\n",
+		       iolen, ioaddr);
+		ret = -EBUSY;
+		goto out;
+	}
+
+	platform_mmio = mmio_addr;
+	platform_mmiolen = mmio_len;
+
+	if (!xen_have_vector_callback) {
+		ret = xen_allocate_irq(pdev);
+		if (ret) {
+			printk(KERN_WARNING "request_irq failed err=%d\n", ret);
+			goto out;
+		}
+		callback_via = get_callback_via(pdev);
+		ret = xen_set_callback_via(callback_via);
+		if (ret) {
+			printk(KERN_WARNING
+					"Unable to set the evtchn callback err=%d\n", ret);
+			goto out;
+		}
+	}
+
+	ret = gnttab_init();
+	if (ret)
+		goto out;
+	ret = xenbus_probe_init();
+	if (ret)
+		goto out;
+
+out:
+	if (ret) {
+		release_mem_region(mmio_addr, mmio_len);
+		release_region(ioaddr, iolen);
+		pci_disable_device(pdev);
+	}
+
+	return ret;
+}
+
+static struct pci_device_id platform_pci_tbl[] __devinitdata = {
+	{PCI_VENDOR_ID_XEN, PCI_DEVICE_ID_XEN_PLATFORM,
+		PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
+	{0,}
+};
+
+MODULE_DEVICE_TABLE(pci, platform_pci_tbl);
+
+static struct pci_driver platform_driver = {
+	.name =           DRV_NAME,
+	.probe =          platform_pci_init,
+	.id_table =       platform_pci_tbl,
+};
+
+static int __init platform_pci_module_init(void)
+{
+	int rc;
+
+	rc = pci_register_driver(&platform_driver);
+	if (rc) {
+		printk(KERN_INFO DRV_NAME
+		       ": No platform pci device model found\n");
+		return rc;
+	}
+	return 0;
+}
+
+module_init(platform_pci_module_init);
diff --git a/drivers/xen/xenbus/xenbus_probe.c b/drivers/xen/xenbus/xenbus_probe.c
index 0b05b62..dc6ed06 100644
--- a/drivers/xen/xenbus/xenbus_probe.c
+++ b/drivers/xen/xenbus/xenbus_probe.c
@@ -782,16 +782,24 @@ void xenbus_probe(struct work_struct *unused)
 	blocking_notifier_call_chain(&xenstore_chain, 0, NULL);
 }
 
-static int __init xenbus_probe_init(void)
+static int __init __xenbus_probe_init(void)
+{
+	/* Delay initialization in the PV on HVM case */
+	if (xen_hvm_domain())
+		return 0;
+
+	if (!xen_pv_domain())
+		return -ENODEV;
+
+	return xenbus_probe_init();
+}
+
+int xenbus_probe_init(void)
 {
 	int err = 0;
 
 	DPRINTK("");
 
-	err = -ENODEV;
-	if (!xen_domain())
-		goto out_error;
-
 	/* Register ourselves with the kernel bus subsystem */
 	err = bus_register(&xenbus_frontend.bus);
 	if (err)
@@ -857,7 +865,7 @@ static int __init xenbus_probe_init(void)
 	return err;
 }
 
-postcore_initcall(xenbus_probe_init);
+postcore_initcall(__xenbus_probe_init);
 
 MODULE_LICENSE("GPL");
 
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 9f688d2..64b528d 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -2718,3 +2718,6 @@
 #define PCI_DEVICE_ID_RME_DIGI32	0x9896
 #define PCI_DEVICE_ID_RME_DIGI32_PRO	0x9897
 #define PCI_DEVICE_ID_RME_DIGI32_8	0x9898
+
+#define PCI_VENDOR_ID_XEN	0x5853
+#define PCI_DEVICE_ID_XEN_PLATFORM	0x0001
diff --git a/include/xen/grant_table.h b/include/xen/grant_table.h
index a40f1cd..811cda5 100644
--- a/include/xen/grant_table.h
+++ b/include/xen/grant_table.h
@@ -51,6 +51,7 @@ struct gnttab_free_callback {
 	u16 count;
 };
 
+int gnttab_init(void);
 int gnttab_suspend(void);
 int gnttab_resume(void);
 
diff --git a/include/xen/interface/grant_table.h b/include/xen/interface/grant_table.h
index 39da93c..39e5717 100644
--- a/include/xen/interface/grant_table.h
+++ b/include/xen/interface/grant_table.h
@@ -28,6 +28,7 @@
 #ifndef __XEN_PUBLIC_GRANT_TABLE_H__
 #define __XEN_PUBLIC_GRANT_TABLE_H__
 
+#include <xen/interface/xen.h>
 
 /***********************************
  * GRANT TABLE REPRESENTATION
diff --git a/include/xen/xenbus.h b/include/xen/xenbus.h
index 43e2d7d..ffa97de 100644
--- a/include/xen/xenbus.h
+++ b/include/xen/xenbus.h
@@ -174,6 +174,7 @@ void unregister_xenbus_watch(struct xenbus_watch *watch);
 void xs_suspend(void);
 void xs_resume(void);
 void xs_suspend_cancel(void);
+int xenbus_probe_init(void);
 
 /* Used by xenbus_dev to borrow kernel's store connection. */
 void *xenbus_dev_request_and_reply(struct xsd_sockmsg *msg);
-- 
1.7.0.4

[PATCH 05/12] Add suspend\resume support for PV on HVM guests.

[stefano.stabellini]

From: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

Suspend\resume requires few different thing on HVM: the suspend
hypercall is different, we don't need to save\restore any memory related
setting, but we need to reinitialize the shared info page and the
callback mechanism.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 arch/x86/xen/enlighten.c   |   16 ++++++++++----
 arch/x86/xen/suspend.c     |    6 +++++
 arch/x86/xen/xen-ops.h     |    1 +
 drivers/xen/manage.c       |   45 ++++++++++++++++++++++++++++++++++++++++---
 drivers/xen/platform-pci.c |   23 +++++++++++++++++++++-
 include/xen/xen-ops.h      |    3 ++
 6 files changed, 84 insertions(+), 10 deletions(-)

diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index ed34b30..c71b0fa 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1263,12 +1263,14 @@ static int init_hvm_pv_info(int *major, int *minor)
 	return 0;
 }
 
-static void __init init_shared_info(void)
+void xen_hvm_init_shared_info(void)
 {
+	int cpu;
 	struct xen_add_to_physmap xatp;
-	struct shared_info *shared_info_page;
+	static struct shared_info *shared_info_page = 0;
 
-	shared_info_page = (struct shared_info *) alloc_bootmem_pages(PAGE_SIZE);
+	if (!shared_info_page)
+		shared_info_page = (struct shared_info *) alloc_bootmem_pages(PAGE_SIZE);
 	xatp.domid = DOMID_SELF;
 	xatp.idx = 0;
 	xatp.space = XENMAPSPACE_shared_info;
@@ -1280,7 +1282,11 @@ static void __init init_shared_info(void)
 
 	/* Don't do the full vcpu_info placement stuff until we have a
 	   possible map and a non-dummy shared_info. */
-	per_cpu(xen_vcpu, 0) = &HYPERVISOR_shared_info->vcpu_info[0];
+	/* xen_hvm_init_shared_info is run at resume time too, in that case
+	 * multiple vcpus might be online */
+ 	for_each_online_cpu(cpu) {
+		per_cpu(xen_vcpu, cpu) = &HYPERVISOR_shared_info->vcpu_info[cpu];
+	}
 }
 
 static int __cpuinit xen_hvm_cpu_notify(struct notifier_block *self,
@@ -1313,7 +1319,7 @@ void __init xen_guest_init(void)
 	if (r < 0)
 		return;
 
-	init_shared_info();
+	xen_hvm_init_shared_info();
 
 	if (xen_feature(XENFEAT_hvm_callback_vector))
 		xen_have_vector_callback = 1;
diff --git a/arch/x86/xen/suspend.c b/arch/x86/xen/suspend.c
index 987267f..6ff9665 100644
--- a/arch/x86/xen/suspend.c
+++ b/arch/x86/xen/suspend.c
@@ -26,6 +26,12 @@ void xen_pre_suspend(void)
 		BUG();
 }
 
+void xen_hvm_post_suspend(int suspend_cancelled)
+{
+	xen_hvm_init_shared_info();
+	xen_callback_vector();
+}
+
 void xen_post_suspend(int suspend_cancelled)
 {
 	xen_build_mfn_list_list();
diff --git a/arch/x86/xen/xen-ops.h b/arch/x86/xen/xen-ops.h
index 0d0e0e6..01c9dd3 100644
--- a/arch/x86/xen/xen-ops.h
+++ b/arch/x86/xen/xen-ops.h
@@ -39,6 +39,7 @@ void xen_enable_syscall(void);
 void xen_vcpu_restore(void);
 
 void xen_callback_vector(void);
+void xen_hvm_init_shared_info(void);
 
 void __init xen_build_dynamic_phys_to_machine(void);
 
diff --git a/drivers/xen/manage.c b/drivers/xen/manage.c
index 2ac4440..0716ba6 100644
--- a/drivers/xen/manage.c
+++ b/drivers/xen/manage.c
@@ -9,6 +9,7 @@
 #include <linux/stop_machine.h>
 #include <linux/freezer.h>
 
+#include <xen/xen.h>
 #include <xen/xenbus.h>
 #include <xen/grant_table.h>
 #include <xen/events.h>
@@ -17,6 +18,7 @@
 
 #include <asm/xen/hypercall.h>
 #include <asm/xen/page.h>
+#include <asm/xen/hypervisor.h>
 
 enum shutdown_state {
 	SHUTDOWN_INVALID = -1,
@@ -33,10 +35,30 @@ enum shutdown_state {
 static enum shutdown_state shutting_down = SHUTDOWN_INVALID;
 
 #ifdef CONFIG_PM_SLEEP
-static int xen_suspend(void *data)
+static int xen_hvm_suspend(void *data)
 {
+	struct sched_shutdown r = { .reason = SHUTDOWN_suspend };
 	int *cancelled = data;
+
+	BUG_ON(!irqs_disabled());
+
+	*cancelled = HYPERVISOR_sched_op(SCHEDOP_shutdown, &r);
+
+	xen_hvm_post_suspend(*cancelled);
+	gnttab_resume();
+
+	if (!*cancelled) {
+		xen_irq_resume();
+		xen_timer_resume();
+	}
+
+	return 0;
+}
+
+static int xen_suspend(void *data)
+{
 	int err;
+	int *cancelled = data;
 
 	BUG_ON(!irqs_disabled());
 
@@ -112,7 +134,10 @@ static void do_suspend(void)
 		goto out_resume;
 	}
 
-	err = stop_machine(xen_suspend, &cancelled, cpumask_of(0));
+	if (xen_hvm_domain())
+		err = stop_machine(xen_hvm_suspend, &cancelled, cpumask_of(0));
+	else
+		err = stop_machine(xen_suspend, &cancelled, cpumask_of(0));
 
 	dpm_resume_noirq(PMSG_RESUME);
 
@@ -261,7 +286,19 @@ static int shutdown_event(struct notifier_block *notifier,
 	return NOTIFY_DONE;
 }
 
-static int __init setup_shutdown_event(void)
+static int __init __setup_shutdown_event(void)
+{
+	/* Delay initialization in the PV on HVM case */
+	if (xen_hvm_domain())
+		return 0;
+
+	if (!xen_pv_domain())
+		return -ENODEV;
+
+	return xen_setup_shutdown_event();
+}
+
+int xen_setup_shutdown_event(void)
 {
 	static struct notifier_block xenstore_notifier = {
 		.notifier_call = shutdown_event
@@ -271,4 +308,4 @@ static int __init setup_shutdown_event(void)
 	return 0;
 }
 
-subsys_initcall(setup_shutdown_event);
+subsys_initcall(__setup_shutdown_event);
diff --git a/drivers/xen/platform-pci.c b/drivers/xen/platform-pci.c
index 9cb7242..43f5aa8 100644
--- a/drivers/xen/platform-pci.c
+++ b/drivers/xen/platform-pci.c
@@ -31,6 +31,7 @@
 #include <xen/xenbus.h>
 #include <xen/events.h>
 #include <xen/hvm.h>
+#include <xen/xen-ops.h>
 
 #define DRV_NAME    "xen-platform-pci"
 
@@ -41,6 +42,7 @@ MODULE_LICENSE("GPL");
 static unsigned long platform_mmio;
 static unsigned long platform_mmio_alloc;
 static unsigned long platform_mmiolen;
+static uint64_t callback_via;
 
 unsigned long alloc_xen_mmio(unsigned long len)
 {
@@ -85,13 +87,25 @@ static int xen_allocate_irq(struct pci_dev *pdev)
 			"xen-platform-pci", pdev);
 }
 
+static int platform_pci_resume(struct pci_dev *pdev)
+{
+	int err;
+	if (xen_have_vector_callback)
+		return 0;
+	err = xen_set_callback_via(callback_via);
+	if (err) {
+		printk("platform_pci_resume failure!\n");
+		return err;
+	}
+	return 0;
+}
+
 static int __devinit platform_pci_init(struct pci_dev *pdev,
 				       const struct pci_device_id *ent)
 {
 	int i, ret;
 	long ioaddr, iolen;
 	long mmio_addr, mmio_len;
-	uint64_t callback_via;
 
 	i = pci_enable_device(pdev);
 	if (i)
@@ -145,6 +159,10 @@ static int __devinit platform_pci_init(struct pci_dev *pdev,
 	ret = xenbus_probe_init();
 	if (ret)
 		goto out;
+	ret = xen_setup_shutdown_event();
+	if (ret)
+		goto out;
+
 
 out:
 	if (ret) {
@@ -168,6 +186,9 @@ static struct pci_driver platform_driver = {
 	.name =           DRV_NAME,
 	.probe =          platform_pci_init,
 	.id_table =       platform_pci_tbl,
+#ifdef CONFIG_PM
+	.resume_early =   platform_pci_resume,
+#endif
 };
 
 static int __init platform_pci_module_init(void)
diff --git a/include/xen/xen-ops.h b/include/xen/xen-ops.h
index 883a21b..46bc81e 100644
--- a/include/xen/xen-ops.h
+++ b/include/xen/xen-ops.h
@@ -7,6 +7,7 @@ DECLARE_PER_CPU(struct vcpu_info *, xen_vcpu);
 
 void xen_pre_suspend(void);
 void xen_post_suspend(int suspend_cancelled);
+void xen_hvm_post_suspend(int suspend_cancelled);
 
 void xen_mm_pin_all(void);
 void xen_mm_unpin_all(void);
@@ -14,4 +15,6 @@ void xen_mm_unpin_all(void);
 void xen_timer_resume(void);
 void xen_arch_resume(void);
 
+int xen_setup_shutdown_event(void);
+
 #endif /* INCLUDE_XEN_OPS_H */
-- 
1.7.0.4

Re: [Xen-devel] [PATCH 05/12] Add suspend\resume support for PV on HVM guests.

[Konrad Rzeszutek Wilk]

> +static int platform_pci_resume(struct pci_dev *pdev)
> +{
> +	int err;
> +	if (xen_have_vector_callback)
> +		return 0;
> +	err = xen_set_callback_via(callback_via);
> +	if (err) {
> +		printk("platform_pci_resume failure!\n");

How did you scripts/checkpatch.pl --strict let you do that?
You might want to use dev_err instead.

Re: [Xen-devel] [PATCH 05/12] Add suspend\resume support for PV on HVM guests.

[Stefano Stabellini]

On Mon, 14 Jun 2010, Konrad Rzeszutek Wilk wrote:
> > +static int platform_pci_resume(struct pci_dev *pdev)
> > +{
> > +	int err;
> > +	if (xen_have_vector_callback)
> > +		return 0;
> > +	err = xen_set_callback_via(callback_via);
> > +	if (err) {
> > +		printk("platform_pci_resume failure!\n");
> 
> How did you scripts/checkpatch.pl --strict let you do that?
> You might want to use dev_err instead.
> 

I'll do that and re-run checkpatch.pl on the whole series

[PATCH 06/12] Allow xen platform pci device to be compiled as a module

[stefano.stabellini]

From: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 drivers/xen/events.c              |    1 +
 drivers/xen/grant-table.c         |   20 +++++++++++---------
 drivers/xen/manage.c              |    1 +
 drivers/xen/platform-pci.c        |    3 +++
 drivers/xen/xenbus/xenbus_probe.c |    1 +
 include/xen/grant_table.h         |    3 +++
 6 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index be378e1..4bd0ea3 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -689,6 +689,7 @@ void xen_hvm_evtchn_do_upcall(void)
 {
 	__xen_evtchn_do_upcall();
 }
+EXPORT_SYMBOL_GPL(xen_hvm_evtchn_do_upcall);
 
 /* Rebind a new event channel to an existing irq. */
 void rebind_evtchn_irq(int evtchn, int irq)
diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
index 4c959a5..64bcad1 100644
--- a/drivers/xen/grant-table.c
+++ b/drivers/xen/grant-table.c
@@ -61,7 +61,8 @@ static unsigned int boot_max_nr_grant_frames;
 static int gnttab_free_count;
 static grant_ref_t gnttab_free_head;
 static DEFINE_SPINLOCK(gnttab_list_lock);
-static unsigned long hvm_pv_resume_frames;
+unsigned long xen_hvm_resume_frames;
+EXPORT_SYMBOL_GPL(xen_hvm_resume_frames);
 
 static struct grant_entry *shared;
 
@@ -436,7 +437,7 @@ static unsigned int __max_nr_grant_frames(void)
 	return query.max_nr_frames;
 }
 
-static inline unsigned int max_nr_grant_frames(void)
+unsigned int gnttab_max_nr_grant_frames(void)
 {
 	unsigned int xen_max = __max_nr_grant_frames();
 
@@ -444,6 +445,7 @@ static inline unsigned int max_nr_grant_frames(void)
 		return boot_max_nr_grant_frames;
 	return xen_max;
 }
+EXPORT_SYMBOL_GPL(gnttab_max_nr_grant_frames);
 
 static int gnttab_map(unsigned int start_idx, unsigned int end_idx)
 {
@@ -464,7 +466,7 @@ static int gnttab_map(unsigned int start_idx, unsigned int end_idx)
 			xatp.domid = DOMID_SELF;
 			xatp.idx = i;
 			xatp.space = XENMAPSPACE_grant_table;
-			xatp.gpfn = (hvm_pv_resume_frames >> PAGE_SHIFT) + i;
+			xatp.gpfn = (xen_hvm_resume_frames >> PAGE_SHIFT) + i;
 			rc = HYPERVISOR_memory_op(XENMEM_add_to_physmap, &xatp);
 			if (rc != 0) {
 				printk(KERN_WARNING
@@ -492,7 +494,7 @@ static int gnttab_map(unsigned int start_idx, unsigned int end_idx)
 
 	BUG_ON(rc || setup.status);
 
-	rc = arch_gnttab_map_shared(frames, nr_gframes, max_nr_grant_frames(),
+	rc = arch_gnttab_map_shared(frames, nr_gframes, gnttab_max_nr_grant_frames(),
 				    &shared);
 	BUG_ON(rc);
 
@@ -505,16 +507,15 @@ int gnttab_resume(void)
 {
 	unsigned int max_nr_gframes;
 
-	max_nr_gframes = max_nr_grant_frames();
+	max_nr_gframes = gnttab_max_nr_grant_frames();
 	if (max_nr_gframes < nr_grant_frames)
 		return -ENOSYS;
 
 	if (xen_pv_domain())
 		return gnttab_map(0, nr_grant_frames - 1);
 
-	if (!hvm_pv_resume_frames) {
-		hvm_pv_resume_frames = alloc_xen_mmio(PAGE_SIZE * max_nr_gframes);
-		shared = ioremap(hvm_pv_resume_frames, PAGE_SIZE * max_nr_gframes);
+	if (!shared) {
+		shared = ioremap(xen_hvm_resume_frames, PAGE_SIZE * max_nr_gframes);
 		if (shared == NULL) {
 			printk(KERN_WARNING
 					"Fail to ioremap gnttab share frames\n");
@@ -541,7 +542,7 @@ static int gnttab_expand(unsigned int req_entries)
 	cur = nr_grant_frames;
 	extra = ((req_entries + (GREFS_PER_GRANT_FRAME-1)) /
 		 GREFS_PER_GRANT_FRAME);
-	if (cur + extra > max_nr_grant_frames())
+	if (cur + extra > gnttab_max_nr_grant_frames())
 		return -ENOSPC;
 
 	rc = gnttab_map(cur, cur + extra - 1);
@@ -599,6 +600,7 @@ int gnttab_init(void)
 	kfree(gnttab_list);
 	return -ENOMEM;
 }
+EXPORT_SYMBOL_GPL(gnttab_init);
 
 static int __devinit __gnttab_init(void)
 {
diff --git a/drivers/xen/manage.c b/drivers/xen/manage.c
index 0716ba6..f5162e4 100644
--- a/drivers/xen/manage.c
+++ b/drivers/xen/manage.c
@@ -307,5 +307,6 @@ int xen_setup_shutdown_event(void)
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(xen_setup_shutdown_event);
 
 subsys_initcall(__setup_shutdown_event);
diff --git a/drivers/xen/platform-pci.c b/drivers/xen/platform-pci.c
index 43f5aa8..6285bd9 100644
--- a/drivers/xen/platform-pci.c
+++ b/drivers/xen/platform-pci.c
@@ -106,6 +106,7 @@ static int __devinit platform_pci_init(struct pci_dev *pdev,
 	int i, ret;
 	long ioaddr, iolen;
 	long mmio_addr, mmio_len;
+	unsigned int max_nr_gframes;
 
 	i = pci_enable_device(pdev);
 	if (i)
@@ -153,6 +154,8 @@ static int __devinit platform_pci_init(struct pci_dev *pdev,
 		}
 	}
 
+	max_nr_gframes = gnttab_max_nr_grant_frames();
+	xen_hvm_resume_frames = alloc_xen_mmio(PAGE_SIZE * max_nr_gframes);
 	ret = gnttab_init();
 	if (ret)
 		goto out;
diff --git a/drivers/xen/xenbus/xenbus_probe.c b/drivers/xen/xenbus/xenbus_probe.c
index dc6ed06..cf971da 100644
--- a/drivers/xen/xenbus/xenbus_probe.c
+++ b/drivers/xen/xenbus/xenbus_probe.c
@@ -864,6 +864,7 @@ int xenbus_probe_init(void)
   out_error:
 	return err;
 }
+EXPORT_SYMBOL_GPL(xenbus_probe_init);
 
 postcore_initcall(__xenbus_probe_init);
 
diff --git a/include/xen/grant_table.h b/include/xen/grant_table.h
index 811cda5..2a58ee0 100644
--- a/include/xen/grant_table.h
+++ b/include/xen/grant_table.h
@@ -113,6 +113,9 @@ int arch_gnttab_map_shared(unsigned long *frames, unsigned long nr_gframes,
 void arch_gnttab_unmap_shared(struct grant_entry *shared,
 			      unsigned long nr_gframes);
 
+extern unsigned long xen_hvm_resume_frames;
+unsigned int gnttab_max_nr_grant_frames(void);
+
 #define gnttab_map_vaddr(map) ((void *)(map.host_virt_addr))
 
 #endif /* __ASM_GNTTAB_H__ */
-- 
1.7.0.4

Re: [PATCH 06/12] Allow xen platform pci device to be compiled as a module

[Konrad Rzeszutek Wilk]

> -static inline unsigned int max_nr_grant_frames(void)
> +unsigned int gnttab_max_nr_grant_frames(void)
>  {
>  	unsigned int xen_max = __max_nr_grant_frames();
>  
> @@ -444,6 +445,7 @@ static inline unsigned int max_nr_grant_frames(void)
>  		return boot_max_nr_grant_frames;
>  	return xen_max;
>  }
> +EXPORT_SYMBOL_GPL(gnttab_max_nr_grant_frames);

Prefix the name with 'xen_' ? That is a mouthfull thought:

Xen Grant Table Max Nr Grant Frames. How about 'xen_gnttab_max_frames' ?

Re: [PATCH 06/12] Allow xen platform pci device to be compiled as a module

[Jeremy Fitzhardinge]

On 06/14/2010 10:20 PM, Konrad Rzeszutek Wilk wrote:
>> -static inline unsigned int max_nr_grant_frames(void)
>> +unsigned int gnttab_max_nr_grant_frames(void)
>>  {
>>  	unsigned int xen_max = __max_nr_grant_frames();
>>  
>> @@ -444,6 +445,7 @@ static inline unsigned int max_nr_grant_frames(void)
>>  		return boot_max_nr_grant_frames;
>>  	return xen_max;
>>  }
>> +EXPORT_SYMBOL_GPL(gnttab_max_nr_grant_frames);
>>     
> Prefix the name with 'xen_' ? That is a mouthfull thought:
>
> Xen Grant Table Max Nr Grant Frames. How about 'xen_gnttab_max_frames' ?
>   

Unfortunately the gnttab API is generally lacking xen_ prefixes.  It's
better to keep this consistent, and then maybe sort them all out together.

    J

Re: [Xen-devel] [PATCH 06/12] Allow xen platform pci device to be compiled as a module

[Stefano Stabellini]

On Tue, 15 Jun 2010, Jeremy Fitzhardinge wrote:
> On 06/14/2010 10:20 PM, Konrad Rzeszutek Wilk wrote:
> >> -static inline unsigned int max_nr_grant_frames(void)
> >> +unsigned int gnttab_max_nr_grant_frames(void)
> >>  {
> >>  	unsigned int xen_max = __max_nr_grant_frames();
> >>  
> >> @@ -444,6 +445,7 @@ static inline unsigned int max_nr_grant_frames(void)
> >>  		return boot_max_nr_grant_frames;
> >>  	return xen_max;
> >>  }
> >> +EXPORT_SYMBOL_GPL(gnttab_max_nr_grant_frames);
> >>     
> > Prefix the name with 'xen_' ? That is a mouthfull thought:
> >
> > Xen Grant Table Max Nr Grant Frames. How about 'xen_gnttab_max_frames' ?
> >   
> 
> Unfortunately the gnttab API is generally lacking xen_ prefixes.  It's
> better to keep this consistent, and then maybe sort them all out together.
> 
 

I renamed it to gnttab_max_grant_frames for now.

[PATCH 07/12] Fix find_unbound_irq in presence of ioapic irqs.

[stefano.stabellini]

From: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

Don't break the assumption that the first 16 irqs are ISA irqs;
make sure that the irq is actually free before using it.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 drivers/xen/events.c |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index 4bd0ea3..b233564 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -340,9 +340,18 @@ static int find_unbound_irq(void)
 	int irq;
 	struct irq_desc *desc;
 
-	for (irq = 0; irq < nr_irqs; irq++)
+	for (irq = 0; irq < nr_irqs; irq++) {
+		desc = irq_to_desc(irq);
+		/* only 0->15 have init'd desc; handle irq > 16 */
+		if (desc == NULL)
+			break;
+		if (desc->chip == &no_irq_chip)
+			break;
+		if (desc->chip != &xen_dynamic_chip)
+			continue;
 		if (irq_info[irq].type == IRQT_UNBOUND)
 			break;
+	}
 
 	if (irq == nr_irqs)
 		panic("No available IRQ to bind to: increase nr_irqs!\n");
-- 
1.7.0.4

[PATCH 08/12] Fix possible NULL pointer dereference in print_IO_APIC

[stefano.stabellini]

From: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

Make sure chip_data is not NULL before accessing it (the VIRQ_TIMER
handler and virq handlers in general don't have any chip_data).

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 arch/x86/kernel/apic/io_apic.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index eb2789c..c64499c 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -1732,6 +1732,8 @@ __apicdebuginit(void) print_IO_APIC(void)
 		struct irq_pin_list *entry;
 
 		cfg = desc->chip_data;
+		if (!cfg)
+			continue;
 		entry = cfg->irq_2_pin;
 		if (!entry)
 			continue;
-- 
1.7.0.4

[PATCH 09/12] __setup_vector_irq: handle NULL chip_data

[stefano.stabellini]

From: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

the VIRQ_TIMER handler and virq handlers in general don't have any
chip_data.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 arch/x86/kernel/apic/io_apic.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index c64499c..4d3d391 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -1269,6 +1269,8 @@ void __setup_vector_irq(int cpu)
 	/* Mark the inuse vectors */
 	for_each_irq_desc(irq, desc) {
 		cfg = desc->chip_data;
+		if (!cfg)
+			continue;
 
 		/*
 		 * If it is a legacy IRQ handled by the legacy PIC, this cpu
-- 
1.7.0.4

[PATCH 10/12] Do not try to disable hpet if it hasn't been initialized before

[stefano.stabellini]

From: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

hpet_disable is called unconditionally on machine reboot if hpet support
is compiled in the kernel.
hpet_disable only checks if the machine is hpet capable but doesn't make
sure that hpet has been initialized.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 arch/x86/kernel/hpet.c |   18 ++++++++++--------
 1 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kernel/hpet.c b/arch/x86/kernel/hpet.c
index 23b4ecd..2b299da 100644
--- a/arch/x86/kernel/hpet.c
+++ b/arch/x86/kernel/hpet.c
@@ -959,16 +959,18 @@ fs_initcall(hpet_late_init);
 
 void hpet_disable(void)
 {
-	if (is_hpet_capable()) {
-		unsigned int cfg = hpet_readl(HPET_CFG);
+	unsigned int cfg;
 
-		if (hpet_legacy_int_enabled) {
-			cfg &= ~HPET_CFG_LEGACY;
-			hpet_legacy_int_enabled = 0;
-		}
-		cfg &= ~HPET_CFG_ENABLE;
-		hpet_writel(cfg, HPET_CFG);
+	if (!is_hpet_capable() || !hpet_address || !hpet_virt_address)
+		return;
+
+	cfg = hpet_readl(HPET_CFG);
+	if (hpet_legacy_int_enabled) {
+		cfg &= ~HPET_CFG_LEGACY;
+		hpet_legacy_int_enabled = 0;
 	}
+	cfg &= ~HPET_CFG_ENABLE;
+	hpet_writel(cfg, HPET_CFG);
 }
 
 #ifdef CONFIG_HPET_EMULATE_RTC
-- 
1.7.0.4

[PATCH 11/12] Use xen_vcpuop_clockevent, xen_clocksource and xen wallclock.

[stefano.stabellini]

From: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

Use xen_vcpuop_clockevent instead of hpet and APIC timers as main
clockevent device on all vcpus, use the xen wallclock time as wallclock
instead of rtc and use xen_clocksource as clocksource.
The pv clock algorithm needs to work correctly for the xen_clocksource
and xen wallclock to be usable, only modern Xen versions offer a
reliable pv clock in HVM guests (XENFEAT_hvm_safe_pvclock).

Using the hpet as clocksource means a VMEXIT every time we read/write to
the hpet mmio addresses, pvclock give us a better rating without
VMEXITs. Same goes for the xen wallclock and xen_vcpuop_clockevent

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 arch/x86/xen/enlighten.c         |   31 +++++++++++++++++++++++++++++++
 arch/x86/xen/suspend.c           |    4 ++++
 include/xen/interface/features.h |    3 +++
 3 files changed, 38 insertions(+), 0 deletions(-)

diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index c71b0fa..e90dfcd 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1307,6 +1307,36 @@ static struct notifier_block __cpuinitdata xen_hvm_cpu_notifier = {
 	.notifier_call	= xen_hvm_cpu_notify,
 };
 
+static void xen_hvm_setup_cpu_clockevents(void)
+{
+	int cpu = smp_processor_id();
+	xen_setup_runstate_info(cpu);
+	xen_setup_timer(cpu);
+	xen_setup_cpu_clockevents();
+}
+
+static void init_hvm_time(void)
+{
+	/* vector callback is needed otherwise we cannot receive interrupts
+	 * on cpu > 0 */
+	if (!xen_have_vector_callback && num_present_cpus() > 1)
+		return;
+	if (!xen_feature(XENFEAT_hvm_safe_pvclock)) {
+		printk(KERN_INFO "Xen doesn't support pvclock on HVM,"
+				"disable pv timer\n");
+		return;
+	}
+
+	pv_time_ops = xen_time_ops;
+	x86_init.timers.timer_init = xen_time_init;
+	x86_init.timers.setup_percpu_clockev = x86_init_noop;
+	x86_cpuinit.setup_percpu_clockev = xen_hvm_setup_cpu_clockevents;
+
+	x86_platform.calibrate_tsc = xen_tsc_khz;
+	x86_platform.get_wallclock = xen_get_wallclock;
+	x86_platform.set_wallclock = xen_set_wallclock;
+}
+
 void __init xen_guest_init(void)
 {
 	int r;
@@ -1326,4 +1356,5 @@ void __init xen_guest_init(void)
 	register_cpu_notifier(&xen_hvm_cpu_notifier);
 	have_vcpu_info_placement = 0;
 	x86_init.irqs.intr_init = xen_init_IRQ;
+	init_hvm_time();
 }
diff --git a/arch/x86/xen/suspend.c b/arch/x86/xen/suspend.c
index 6ff9665..0774c67 100644
--- a/arch/x86/xen/suspend.c
+++ b/arch/x86/xen/suspend.c
@@ -28,8 +28,12 @@ void xen_pre_suspend(void)
 
 void xen_hvm_post_suspend(int suspend_cancelled)
 {
+	int cpu;
 	xen_hvm_init_shared_info();
 	xen_callback_vector();
+	for_each_online_cpu(cpu) {
+		xen_setup_runstate_info(cpu);
+	}
 }
 
 void xen_post_suspend(int suspend_cancelled)
diff --git a/include/xen/interface/features.h b/include/xen/interface/features.h
index 8ab08b9..70d2563 100644
--- a/include/xen/interface/features.h
+++ b/include/xen/interface/features.h
@@ -44,6 +44,9 @@
 /* x86: Does this Xen host support the HVM callback vector type? */
 #define XENFEAT_hvm_callback_vector        8
 
+/* x86: pvclock algorithm is safe to use on HVM */
+#define XENFEAT_hvm_safe_pvclock           9
+
 #define XENFEAT_NR_SUBMAPS 1
 
 #endif /* __XEN_PUBLIC_FEATURES_H__ */
-- 
1.7.0.4

[PATCH 12/12] Unplug emulated disks and nics

[stefano.stabellini]

From: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

Add a xen_emul_unplug command line option to the kernel to unplug
xen emulated disks and nics.

Set the default value of xen_emul_unplug depending on whether or
not the Xen PV frontends and the Xen platform PCI driver have
been compiled for this kernel (modules or built-in are both OK).

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 Documentation/kernel-parameters.txt |   11 +++
 arch/x86/xen/Makefile               |    2 +-
 arch/x86/xen/enlighten.c            |    1 +
 arch/x86/xen/platform-pci-unplug.c  |  131 +++++++++++++++++++++++++++++++++++
 arch/x86/xen/xen-ops.h              |    1 +
 drivers/xen/platform-pci.c          |    4 +
 drivers/xen/xenbus/xenbus_probe.c   |    4 +
 include/xen/platform_pci.h          |   27 +++++++
 8 files changed, 180 insertions(+), 1 deletions(-)
 create mode 100644 arch/x86/xen/platform-pci-unplug.c
 create mode 100644 include/xen/platform_pci.h

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 839b21b..716eea8 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -113,6 +113,7 @@ parameter is applicable:
 			More X86-64 boot options can be found in
 			Documentation/x86/x86_64/boot-options.txt .
 	X86	Either 32bit or 64bit x86 (same as X86-32+X86-64)
+	XEN	Xen support is enabled
 
 In addition, the following text indicates that the option:
 
@@ -2834,6 +2835,16 @@ and is between 256 and 4096 characters. It is defined in the file
 	xd=		[HW,XT] Original XT pre-IDE (RLL encoded) disks.
 	xd_geo=		See header of drivers/block/xd.c.
 
+	xen_emul_unplug=		[HW,X86,XEN]
+			Unplug Xen emulated devices
+			Format: [unplug0,][unplug1]
+			ide-disks -- unplug primary master IDE devices
+			aux-ide-disks -- unplug non-primary-master IDE devices
+			nics -- unplug network devices
+			all -- unplug all emulated devices (NICs and IDE disks)
+			ignore -- continue loading the Xen platform PCI driver even
+				if the version check failed
+
 	xirc2ps_cs=	[NET,PCMCIA]
 			Format:
 			<irq>,<irq_mask>,<io>,<full_duplex>,<do_sound>,<lockup_hack>[,<irq2>[,<irq3>[,<irq4>]]]
diff --git a/arch/x86/xen/Makefile b/arch/x86/xen/Makefile
index 3bb4fc2..9309546 100644
--- a/arch/x86/xen/Makefile
+++ b/arch/x86/xen/Makefile
@@ -12,7 +12,7 @@ CFLAGS_mmu.o			:= $(nostackp)
 
 obj-y		:= enlighten.o setup.o multicalls.o mmu.o irq.o \
 			time.o xen-asm.o xen-asm_$(BITS).o \
-			grant-table.o suspend.o
+			grant-table.o suspend.o platform-pci-unplug.o
 
 obj-$(CONFIG_SMP)		+= smp.o
 obj-$(CONFIG_PARAVIRT_SPINLOCKS)+= spinlock.o
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index e90dfcd..209cfd9 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1354,6 +1354,7 @@ void __init xen_guest_init(void)
 	if (xen_feature(XENFEAT_hvm_callback_vector))
 		xen_have_vector_callback = 1;
 	register_cpu_notifier(&xen_hvm_cpu_notifier);
+	xen_unplug_emulated_devices();
 	have_vcpu_info_placement = 0;
 	x86_init.irqs.intr_init = xen_init_IRQ;
 	init_hvm_time();
diff --git a/arch/x86/xen/platform-pci-unplug.c b/arch/x86/xen/platform-pci-unplug.c
new file mode 100644
index 0000000..1de8e6d
--- /dev/null
+++ b/arch/x86/xen/platform-pci-unplug.c
@@ -0,0 +1,131 @@
+/******************************************************************************
+ * platform-pci-unplug.c
+ *
+ * Xen platform PCI device driver
+ * Copyright (c) 2010, Citrix
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; if not, write to the Free Software Foundation, Inc., 59 Temple
+ * Place - Suite 330, Boston, MA 02111-1307 USA.
+ *
+ */
+
+#include <asm/io.h>
+
+#include <linux/init.h>
+#include <linux/module.h>
+
+#include <xen/platform_pci.h>
+
+/* boolean to signal that the platform pci device can be used */
+bool xen_platform_pci_enabled;
+EXPORT_SYMBOL_GPL(xen_platform_pci_enabled);
+static int xen_emul_unplug;
+
+static int __init check_platform_magic(void)
+{
+	short magic;
+	char protocol;
+
+	magic = inw(XEN_IOPORT_MAGIC);
+	if (magic != XEN_IOPORT_MAGIC_VAL) {
+		printk(KERN_ERR "Xen Platform PCI: unrecognised magic value\n");
+		return -1;
+	}
+
+	protocol = inb(XEN_IOPORT_PROTOVER);
+
+	printk(KERN_DEBUG "Xen Platform PCI: I/O protocol version %d\n",
+			protocol);
+
+	switch (protocol) {
+	case 1:
+		outw(XEN_IOPORT_LINUX_PRODNUM, XEN_IOPORT_PRODNUM);
+		outl(XEN_IOPORT_LINUX_DRVVER, XEN_IOPORT_DRVVER);
+		if (inw(XEN_IOPORT_MAGIC) != XEN_IOPORT_MAGIC_VAL) {
+			printk(KERN_ERR "Xen Platform: blacklisted by host\n");
+			return -3;
+		}
+		break;
+	default:
+		printk(KERN_WARNING "Xen Platform PCI: unknown I/O protocol version");
+		return -2;
+	}
+
+	return 0;
+}
+
+void __init xen_unplug_emulated_devices(void)
+{
+	int r;
+
+	/* check the version of the xen platform PCI device */
+	r = check_platform_magic();
+	/* If the version matches enable the Xen platform PCI driver.
+	 * Also enable the Xen platform PCI driver if the version is really old
+	 * and the user told us to ignore it. */
+	if (!r || (r == -1 && (xen_emul_unplug & XEN_UNPLUG_IGNORE)))
+		xen_platform_pci_enabled = 1;
+	/* Set the default value of xen_emul_unplug depending on whether or
+	 * not the Xen PV frontends and the Xen platform PCI driver have
+	 * been compiled for this kernel (modules or built-in are both OK). */
+	if (xen_platform_pci_enabled && !xen_emul_unplug) {
+#if (defined(CONFIG_XEN_NETDEV_FRONTEND) || \
+		defined(CONFIG_XEN_NETDEV_FRONTEND_MODULE)) && \
+		(defined(CONFIG_XEN_PLATFORM_PCI) || \
+		 defined(CONFIG_XEN_PLATFORM_PCI_MODULE))
+		printk(KERN_INFO "Netfront and the Xen platform PCI driver have "
+				"been compiled for this kernel: unplug emulated NICs.\n");
+		xen_emul_unplug |= XEN_UNPLUG_ALL_NICS;
+#endif
+#if (defined(CONFIG_XEN_BLKDEV_FRONTEND) || \
+		defined(CONFIG_XEN_BLKDEV_FRONTEND_MODULE)) && \
+		(defined(CONFIG_XEN_PLATFORM_PCI) || \
+		 defined(CONFIG_XEN_PLATFORM_PCI_MODULE))
+		printk(KERN_INFO "Blkfront and the Xen platform PCI driver have "
+				"been compiled for this kernel: unplug emulated disks.\n"
+				"You might have to change the root device\n"
+				"from /dev/hd[a-d] to /dev/xvd[a-d]\n"
+				"in your root= kernel command line option\n");
+		xen_emul_unplug |= XEN_UNPLUG_ALL_IDE_DISKS;
+#endif
+	}
+	/* Now unplug the emulated devices */
+	if (xen_platform_pci_enabled && !(xen_emul_unplug & XEN_UNPLUG_IGNORE))
+		outw(xen_emul_unplug, XEN_IOPORT_UNPLUG);
+}
+
+static int __init parse_xen_emul_unplug(char *arg)
+{
+	char *p, *q;
+
+	for (p = arg; p; p = q) {
+		q = strchr(arg, ',');
+		if (q)
+			*q++ = '\0';
+		if (!strcmp(p, "all"))
+			xen_emul_unplug |= XEN_UNPLUG_ALL;
+		else if (!strcmp(p, "ide-disks"))
+			xen_emul_unplug |= XEN_UNPLUG_ALL_IDE_DISKS;
+		else if (!strcmp(p, "aux-ide-disks"))
+			xen_emul_unplug |= XEN_UNPLUG_AUX_IDE_DISKS;
+		else if (!strcmp(p, "nics"))
+			xen_emul_unplug |= XEN_UNPLUG_ALL_NICS;
+		else if (!strcmp(p, "ignore"))
+			xen_emul_unplug |= XEN_UNPLUG_IGNORE;
+		else
+			printk(KERN_WARNING "unrecognised option '%s' "
+				 "in module parameter 'dev_unplug'\n", p);
+	}
+	return 0;
+}
+early_param("xen_emul_unplug", parse_xen_emul_unplug);
diff --git a/arch/x86/xen/xen-ops.h b/arch/x86/xen/xen-ops.h
index 01c9dd3..615e037 100644
--- a/arch/x86/xen/xen-ops.h
+++ b/arch/x86/xen/xen-ops.h
@@ -40,6 +40,7 @@ void xen_vcpu_restore(void);
 
 void xen_callback_vector(void);
 void xen_hvm_init_shared_info(void);
+void __init xen_unplug_emulated_devices(void);
 
 void __init xen_build_dynamic_phys_to_machine(void);
 
diff --git a/drivers/xen/platform-pci.c b/drivers/xen/platform-pci.c
index 6285bd9..32ec950 100644
--- a/drivers/xen/platform-pci.c
+++ b/drivers/xen/platform-pci.c
@@ -27,6 +27,7 @@
 #include <linux/module.h>
 #include <linux/pci.h>
 
+#include <xen/platform_pci.h>
 #include <xen/grant_table.h>
 #include <xen/xenbus.h>
 #include <xen/events.h>
@@ -198,6 +199,9 @@ static int __init platform_pci_module_init(void)
 {
 	int rc;
 
+	if (!xen_platform_pci_enabled)
+		return -ENODEV;
+
 	rc = pci_register_driver(&platform_driver);
 	if (rc) {
 		printk(KERN_INFO DRV_NAME
diff --git a/drivers/xen/xenbus/xenbus_probe.c b/drivers/xen/xenbus/xenbus_probe.c
index cf971da..b9427da 100644
--- a/drivers/xen/xenbus/xenbus_probe.c
+++ b/drivers/xen/xenbus/xenbus_probe.c
@@ -56,6 +56,7 @@
 #include <xen/events.h>
 #include <xen/page.h>
 
+#include <xen/platform_pci.h>
 #include <xen/hvm.h>
 
 #include "xenbus_comms.h"
@@ -974,6 +975,9 @@ static void wait_for_devices(struct xenbus_driver *xendrv)
 #ifndef MODULE
 static int __init boot_wait_for_devices(void)
 {
+	if (xen_hvm_domain() && !xen_platform_pci_enabled)
+		return -ENODEV;
+
 	ready_to_wait_for_devices = 1;
 	wait_for_devices(NULL);
 	return 0;
diff --git a/include/xen/platform_pci.h b/include/xen/platform_pci.h
new file mode 100644
index 0000000..22f5a13
--- /dev/null
+++ b/include/xen/platform_pci.h
@@ -0,0 +1,27 @@
+#ifndef _XEN_PLATFORM_PCI_H
+#define _XEN_PLATFORM_PCI_H
+
+#define XEN_IOPORT_MAGIC_VAL 0x49d2
+#define XEN_IOPORT_LINUX_PRODNUM 0x0003
+#define XEN_IOPORT_LINUX_DRVVER  0x0001
+
+#define XEN_IOPORT_BASE 0x10
+
+#define XEN_IOPORT_PLATFLAGS	(XEN_IOPORT_BASE + 0) /* 1 byte access (R/W) */
+#define XEN_IOPORT_MAGIC	(XEN_IOPORT_BASE + 0) /* 2 byte access (R) */
+#define XEN_IOPORT_UNPLUG	(XEN_IOPORT_BASE + 0) /* 2 byte access (W) */
+#define XEN_IOPORT_DRVVER	(XEN_IOPORT_BASE + 0) /* 4 byte access (W) */
+
+#define XEN_IOPORT_SYSLOG	(XEN_IOPORT_BASE + 2) /* 1 byte access (W) */
+#define XEN_IOPORT_PROTOVER	(XEN_IOPORT_BASE + 2) /* 1 byte access (R) */
+#define XEN_IOPORT_PRODNUM	(XEN_IOPORT_BASE + 2) /* 2 byte access (W) */
+
+#define XEN_UNPLUG_ALL_IDE_DISKS 1
+#define XEN_UNPLUG_ALL_NICS 2
+#define XEN_UNPLUG_AUX_IDE_DISKS 4
+#define XEN_UNPLUG_ALL 7
+#define XEN_UNPLUG_IGNORE 8
+
+extern bool xen_platform_pci_enabled;
+
+#endif /* _XEN_PLATFORM_PCI_H */
-- 
1.7.0.4

Re: [Xen-devel] [PATCH 12/12] Unplug emulated disks and nics

[Konrad Rzeszutek Wilk]

> +#if (defined(CONFIG_XEN_NETDEV_FRONTEND) || \
> +		defined(CONFIG_XEN_NETDEV_FRONTEND_MODULE)) && \
> +		(defined(CONFIG_XEN_PLATFORM_PCI) || \
> +		 defined(CONFIG_XEN_PLATFORM_PCI_MODULE))
> +		printk(KERN_INFO "Netfront and the Xen platform PCI driver have "
> +				"been compiled for this kernel: unplug emulated NICs.\n");
> +		xen_emul_unplug |= XEN_UNPLUG_ALL_NICS;
> +#endif
> +#if (defined(CONFIG_XEN_BLKDEV_FRONTEND) || \
> +		defined(CONFIG_XEN_BLKDEV_FRONTEND_MODULE)) && \
> +		(defined(CONFIG_XEN_PLATFORM_PCI) || \
> +		 defined(CONFIG_XEN_PLATFORM_PCI_MODULE))
> +		printk(KERN_INFO "Blkfront and the Xen platform PCI driver have "
> +				"been compiled for this kernel: unplug emulated disks.\n"
> +				"You might have to change the root device\n"
> +				"from /dev/hd[a-d] to /dev/xvd[a-d]\n"
> +				"in your root= kernel command line option\n");
> +		xen_emul_unplug |= XEN_UNPLUG_ALL_IDE_DISKS;
> +#endif

Wow. Can you move those checks to the header file and make it deal with
the #ifdef and setting of xen_emul_unplug?

> +	}
> +	/* Now unplug the emulated devices */
> +	if (xen_platform_pci_enabled && !(xen_emul_unplug & XEN_UNPLUG_IGNORE))
> +		outw(xen_emul_unplug, XEN_IOPORT_UNPLUG);
> +}
> +
> +static int __init parse_xen_emul_unplug(char *arg)
> +{
> +	char *p, *q;
> +
> +	for (p = arg; p; p = q) {
> +		q = strchr(arg, ',');
> +		if (q)
> +			*q++ = '\0';
> +		if (!strcmp(p, "all"))
> +			xen_emul_unplug |= XEN_UNPLUG_ALL;

strncmp..

Re: [Xen-devel] [PATCH 12/12] Unplug emulated disks and nics

[Stefano Stabellini]

On Mon, 14 Jun 2010, Konrad Rzeszutek Wilk wrote:
> > +#if (defined(CONFIG_XEN_NETDEV_FRONTEND) || \
> > +		defined(CONFIG_XEN_NETDEV_FRONTEND_MODULE)) && \
> > +		(defined(CONFIG_XEN_PLATFORM_PCI) || \
> > +		 defined(CONFIG_XEN_PLATFORM_PCI_MODULE))
> > +		printk(KERN_INFO "Netfront and the Xen platform PCI driver have "
> > +				"been compiled for this kernel: unplug emulated NICs.\n");
> > +		xen_emul_unplug |= XEN_UNPLUG_ALL_NICS;
> > +#endif
> > +#if (defined(CONFIG_XEN_BLKDEV_FRONTEND) || \
> > +		defined(CONFIG_XEN_BLKDEV_FRONTEND_MODULE)) && \
> > +		(defined(CONFIG_XEN_PLATFORM_PCI) || \
> > +		 defined(CONFIG_XEN_PLATFORM_PCI_MODULE))
> > +		printk(KERN_INFO "Blkfront and the Xen platform PCI driver have "
> > +				"been compiled for this kernel: unplug emulated disks.\n"
> > +				"You might have to change the root device\n"
> > +				"from /dev/hd[a-d] to /dev/xvd[a-d]\n"
> > +				"in your root= kernel command line option\n");
> > +		xen_emul_unplug |= XEN_UNPLUG_ALL_IDE_DISKS;
> > +#endif
> 
> Wow. Can you move those checks to the header file and make it deal with
> the #ifdef and setting of xen_emul_unplug?
> 

I tried, but it didn't improve the elegance of the code, mainly because I
want to keep the printk in place, so the code would look very much like
this, but instead of being in platform-pci-unplug.c would be in
platform_pci.h.


> > +	}
> > +	/* Now unplug the emulated devices */
> > +	if (xen_platform_pci_enabled && !(xen_emul_unplug & XEN_UNPLUG_IGNORE))
> > +		outw(xen_emul_unplug, XEN_IOPORT_UNPLUG);
> > +}
> > +
> > +static int __init parse_xen_emul_unplug(char *arg)
> > +{
> > +	char *p, *q;
> > +
> > +	for (p = arg; p; p = q) {
> > +		q = strchr(arg, ',');
> > +		if (q)
> > +			*q++ = '\0';
> > +		if (!strcmp(p, "all"))
> > +			xen_emul_unplug |= XEN_UNPLUG_ALL;
> 
> strncmp..
> 

is it really needed considering that we know that both strings are NULL
terminated and one of them is a constant?

Re: [PATCH 12/12] Unplug emulated disks and nics

[Konrad Rzeszutek Wilk]

On Thu, Jun 17, 2010 at 04:42:14PM +0100, Stefano Stabellini wrote:
> On Mon, 14 Jun 2010, Konrad Rzeszutek Wilk wrote:
> > > +#if (defined(CONFIG_XEN_NETDEV_FRONTEND) || \
> > > +		defined(CONFIG_XEN_NETDEV_FRONTEND_MODULE)) && \
> > > +		(defined(CONFIG_XEN_PLATFORM_PCI) || \
> > > +		 defined(CONFIG_XEN_PLATFORM_PCI_MODULE))
> > > +		printk(KERN_INFO "Netfront and the Xen platform PCI driver have "
> > > +				"been compiled for this kernel: unplug emulated NICs.\n");
> > > +		xen_emul_unplug |= XEN_UNPLUG_ALL_NICS;
> > > +#endif
> > > +#if (defined(CONFIG_XEN_BLKDEV_FRONTEND) || \
> > > +		defined(CONFIG_XEN_BLKDEV_FRONTEND_MODULE)) && \
> > > +		(defined(CONFIG_XEN_PLATFORM_PCI) || \
> > > +		 defined(CONFIG_XEN_PLATFORM_PCI_MODULE))
> > > +		printk(KERN_INFO "Blkfront and the Xen platform PCI driver have "
> > > +				"been compiled for this kernel: unplug emulated disks.\n"
> > > +				"You might have to change the root device\n"
> > > +				"from /dev/hd[a-d] to /dev/xvd[a-d]\n"
> > > +				"in your root= kernel command line option\n");
> > > +		xen_emul_unplug |= XEN_UNPLUG_ALL_IDE_DISKS;
> > > +#endif
> > 
> > Wow. Can you move those checks to the header file and make it deal with
> > the #ifdef and setting of xen_emul_unplug?
> > 
> 
> I tried, but it didn't improve the elegance of the code, mainly because I
> want to keep the printk in place, so the code would look very much like
> this, but instead of being in platform-pci-unplug.c would be in
> platform_pci.h.

If was thinking of something like this in the header file:


int xen_must_unplug_nics() {
#if (defined(CONFIG_XEN_NETDEV_FRONTEND) || \
		defined(CONFIG_XEN_NETDEV_FRONTEND_MODULE)) && \
		(defined(CONFIG_XEN_PLATFORM_PCI) || \
		 defined(CONFIG_XEN_PLATFORM_PCI_MODULE))
	return 1;
#else
	return 0;
}

and then your code would be:

	if (xen_must_unplug_nics()) {
		printk(".. blah blah ");
		xen_emul_unplug |- XEN_unPLIG_ALL_NICS;
	}

and similar for the IDE disks.

> 
> 
> > > +	}
> > > +	/* Now unplug the emulated devices */
> > > +	if (xen_platform_pci_enabled && !(xen_emul_unplug & XEN_UNPLUG_IGNORE))
> > > +		outw(xen_emul_unplug, XEN_IOPORT_UNPLUG);
> > > +}
> > > +
> > > +static int __init parse_xen_emul_unplug(char *arg)
> > > +{
> > > +	char *p, *q;
> > > +
> > > +	for (p = arg; p; p = q) {
> > > +		q = strchr(arg, ',');
> > > +		if (q)
> > > +			*q++ = '\0';
> > > +		if (!strcmp(p, "all"))
> > > +			xen_emul_unplug |= XEN_UNPLUG_ALL;
> > 
> > strncmp..
> > 
> 
> is it really needed considering that we know that both strings are NULL
> terminated and one of them is a constant?

Please do.

Re: [Xen-devel] [PATCH 12/12] Unplug emulated disks and nics

[Stefano Stabellini]

On Thu, 17 Jun 2010, Konrad Rzeszutek Wilk wrote:
> If was thinking of something like this in the header file:
> 
> 
> int xen_must_unplug_nics() {
> #if (defined(CONFIG_XEN_NETDEV_FRONTEND) || \
> 		defined(CONFIG_XEN_NETDEV_FRONTEND_MODULE)) && \
> 		(defined(CONFIG_XEN_PLATFORM_PCI) || \
> 		 defined(CONFIG_XEN_PLATFORM_PCI_MODULE))
> 	return 1;
> #else
> 	return 0;
> }
> 
> and then your code would be:
> 
> 	if (xen_must_unplug_nics()) {
> 		printk(".. blah blah ");
> 		xen_emul_unplug |- XEN_unPLIG_ALL_NICS;
> 	}
> 
> and similar for the IDE disks.
> 

this seems actually better, I'll do that

> > 
> > 
> > > > +	}
> > > > +	/* Now unplug the emulated devices */
> > > > +	if (xen_platform_pci_enabled && !(xen_emul_unplug & XEN_UNPLUG_IGNORE))
> > > > +		outw(xen_emul_unplug, XEN_IOPORT_UNPLUG);
> > > > +}
> > > > +
> > > > +static int __init parse_xen_emul_unplug(char *arg)
> > > > +{
> > > > +	char *p, *q;
> > > > +
> > > > +	for (p = arg; p; p = q) {
> > > > +		q = strchr(arg, ',');
> > > > +		if (q)
> > > > +			*q++ = '\0';
> > > > +		if (!strcmp(p, "all"))
> > > > +			xen_emul_unplug |= XEN_UNPLUG_ALL;
> > > 
> > > strncmp..
> > > 
> > 
> > is it really needed considering that we know that both strings are NULL
> > terminated and one of them is a constant?
> 
> Please do.
> 

all right

Re: [Xen-devel] [PATCH 12/12] Unplug emulated disks and nics

[Jeremy Fitzhardinge]

On 06/17/2010 04:42 PM, Stefano Stabellini wrote:
> On Mon, 14 Jun 2010, Konrad Rzeszutek Wilk wrote:
>   
>>> +#if (defined(CONFIG_XEN_NETDEV_FRONTEND) || \
>>> +		defined(CONFIG_XEN_NETDEV_FRONTEND_MODULE)) && \
>>> +		(defined(CONFIG_XEN_PLATFORM_PCI) || \
>>> +		 defined(CONFIG_XEN_PLATFORM_PCI_MODULE))
>>> +		printk(KERN_INFO "Netfront and the Xen platform PCI driver have "
>>> +				"been compiled for this kernel: unplug emulated NICs.\n");
>>> +		xen_emul_unplug |= XEN_UNPLUG_ALL_NICS;
>>> +#endif
>>> +#if (defined(CONFIG_XEN_BLKDEV_FRONTEND) || \
>>> +		defined(CONFIG_XEN_BLKDEV_FRONTEND_MODULE)) && \
>>> +		(defined(CONFIG_XEN_PLATFORM_PCI) || \
>>> +		 defined(CONFIG_XEN_PLATFORM_PCI_MODULE))
>>> +		printk(KERN_INFO "Blkfront and the Xen platform PCI driver have "
>>> +				"been compiled for this kernel: unplug emulated disks.\n"
>>> +				"You might have to change the root device\n"
>>> +				"from /dev/hd[a-d] to /dev/xvd[a-d]\n"
>>> +				"in your root= kernel command line option\n");
>>> +		xen_emul_unplug |= XEN_UNPLUG_ALL_IDE_DISKS;
>>> +#endif
>>>       
>> Wow. Can you move those checks to the header file and make it deal with
>> the #ifdef and setting of xen_emul_unplug?
>>
>>     
> I tried, but it didn't improve the elegance of the code, mainly because I
> want to keep the printk in place, so the code would look very much like
> this, but instead of being in platform-pci-unplug.c would be in
> platform_pci.h.
>   

What about using Kconfig to define an appropriate symbol and just #ifdef
on that?

>>> +	}
>>> +	/* Now unplug the emulated devices */
>>> +	if (xen_platform_pci_enabled && !(xen_emul_unplug & XEN_UNPLUG_IGNORE))
>>> +		outw(xen_emul_unplug, XEN_IOPORT_UNPLUG);
>>> +}
>>> +
>>> +static int __init parse_xen_emul_unplug(char *arg)
>>> +{
>>> +	char *p, *q;
>>> +
>>> +	for (p = arg; p; p = q) {
>>> +		q = strchr(arg, ',');
>>> +		if (q)
>>> +			*q++ = '\0';
>>> +		if (!strcmp(p, "all"))
>>> +			xen_emul_unplug |= XEN_UNPLUG_ALL;
>>>       
>> strncmp..
>>
>>     
> is it really needed considering that we know that both strings are NULL
> terminated and one of them is a constant?
>   

strncmp would avoid having to modify the string in place.

    J

[PATCH 0 of 12] PV on HVM Xen

[Stefano Stabellini]

Hi all,
this is the fixed, updated and rebased version of the PV on HVM series:
the series is based on 2.6.34 now and supports Xen PV frontends running
in a HVM domain, including netfront, blkfront and the VIRQ_TIMER.

The list of bugs fixed in this update includes: xenbus drivers crashes
when xenbus is not properly initialized, a memory corruption bug in
suspend/resume and testing for the xen platform pci version and protocol
has been moved to enlighten.c (before unplugging emulated devices).

In order to be able to use VIRQ_TIMER and to improve performances you
need a patch to Xen to implement the vector callback mechanism
for event channel delivery.

A git tree is also available here:

git://xenbits.xen.org/people/sstabellini/linux-pvhvm.git

branch name 2.6.34-pvhvm.

Cheers,

Stefano

[PATCH 03/12] evtchn delivery on HVM

[Stefano Stabellini]

From: Sheng Yang <sheng@linux.intel.com>

Set the callback to receive evtchns from Xen, using the
callback vector delivery mechanism.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Signed-off-by: Sheng Yang <sheng@linux.intel.com>
---
 arch/x86/xen/enlighten.c         |   35 +++++++++++++++++++++++++++++++++++
 drivers/xen/events.c             |   31 ++++++++++++++++++++++++-------
 include/xen/events.h             |    3 +++
 include/xen/hvm.h                |    9 +++++++++
 include/xen/interface/features.h |    3 +++
 5 files changed, 74 insertions(+), 7 deletions(-)

diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 87a3b10..502c4f8 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -37,8 +37,11 @@
 #include <xen/interface/vcpu.h>
 #include <xen/interface/memory.h>
 #include <xen/interface/hvm/hvm_op.h>
+#include <xen/interface/hvm/params.h>
 #include <xen/features.h>
 #include <xen/page.h>
+#include <xen/hvm.h>
+#include <xen/events.h>
 #include <xen/hvc-console.h>
 
 #include <asm/paravirt.h>
@@ -79,6 +82,8 @@ struct shared_info xen_dummy_shared_info;
 
 void *xen_initial_gdt;
 
+int xen_have_vector_callback;
+
 /*
  * Point at some empty memory to start with. We map the real shared_info
  * page as soon as fixmap is up and running.
@@ -1279,6 +1284,31 @@ static void __init init_shared_info(void)
 	per_cpu(xen_vcpu, 0) = &HYPERVISOR_shared_info->vcpu_info[0];
 }
 
+int xen_set_callback_via(uint64_t via)
+{
+	struct xen_hvm_param a;
+	a.domid = DOMID_SELF;
+	a.index = HVM_PARAM_CALLBACK_IRQ;
+	a.value = via;
+	return HYPERVISOR_hvm_op(HVMOP_set_param, &a);
+}
+
+void do_hvm_pv_evtchn_intr(void)
+{
+	xen_hvm_evtchn_do_upcall(get_irq_regs());
+}
+
+static void xen_callback_vector(void)
+{
+	uint64_t callback_via;
+	if (xen_feature(XENFEAT_hvm_callback_vector)) {
+		callback_via = HVM_CALLBACK_VECTOR(X86_PLATFORM_IPI_VECTOR);
+		xen_set_callback_via(callback_via);
+		x86_platform_ipi_callback = do_hvm_pv_evtchn_intr;
+		xen_have_vector_callback = 1;
+	}
+}
+
 void __init xen_guest_init(void)
 {
 	int r;
@@ -1292,4 +1322,9 @@ void __init xen_guest_init(void)
 		return;
 
 	init_shared_info();
+
+	xen_callback_vector();
+
+	have_vcpu_info_placement = 0;
+	x86_init.irqs.intr_init = xen_init_IRQ;
 }
diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index db8f506..3523dbb 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -36,6 +36,8 @@
 #include <asm/xen/hypercall.h>
 #include <asm/xen/hypervisor.h>
 
+#include <xen/xen.h>
+#include <xen/hvm.h>
 #include <xen/xen-ops.h>
 #include <xen/events.h>
 #include <xen/interface/xen.h>
@@ -617,17 +619,13 @@ static DEFINE_PER_CPU(unsigned, xed_nesting_count);
  * a bitset of words which contain pending event bits.  The second
  * level is a bitset of pending events themselves.
  */
-void xen_evtchn_do_upcall(struct pt_regs *regs)
+void __xen_evtchn_do_upcall(struct pt_regs *regs)
 {
 	int cpu = get_cpu();
-	struct pt_regs *old_regs = set_irq_regs(regs);
 	struct shared_info *s = HYPERVISOR_shared_info;
 	struct vcpu_info *vcpu_info = __get_cpu_var(xen_vcpu);
  	unsigned count;
 
-	exit_idle();
-	irq_enter();
-
 	do {
 		unsigned long pending_words;
 
@@ -667,10 +665,26 @@ void xen_evtchn_do_upcall(struct pt_regs *regs)
 	} while(count != 1);
 
 out:
+
+	put_cpu();
+}
+
+void xen_evtchn_do_upcall(struct pt_regs *regs)
+{
+	struct pt_regs *old_regs = set_irq_regs(regs);
+
+	exit_idle();
+	irq_enter();
+
+	__xen_evtchn_do_upcall(regs);
+
 	irq_exit();
 	set_irq_regs(old_regs);
+}
 
-	put_cpu();
+void xen_hvm_evtchn_do_upcall(struct pt_regs *regs)
+{
+	__xen_evtchn_do_upcall(regs);
 }
 
 /* Rebind a new event channel to an existing irq. */
@@ -947,5 +961,8 @@ void __init xen_init_IRQ(void)
 	for (i = 0; i < NR_EVENT_CHANNELS; i++)
 		mask_evtchn(i);
 
-	irq_ctx_init(smp_processor_id());
+	if (xen_hvm_domain())
+		native_init_IRQ();
+	else
+		irq_ctx_init(smp_processor_id());
 }
diff --git a/include/xen/events.h b/include/xen/events.h
index e68d59a..868e5d6 100644
--- a/include/xen/events.h
+++ b/include/xen/events.h
@@ -56,4 +56,7 @@ void xen_poll_irq(int irq);
 /* Determine the IRQ which is bound to an event channel */
 unsigned irq_from_evtchn(unsigned int evtchn);
 
+void xen_evtchn_do_upcall(struct pt_regs *regs);
+void xen_hvm_evtchn_do_upcall(struct pt_regs *regs);
+
 #endif	/* _XEN_EVENTS_H */
diff --git a/include/xen/hvm.h b/include/xen/hvm.h
index 6b0d418..5940ee5 100644
--- a/include/xen/hvm.h
+++ b/include/xen/hvm.h
@@ -3,6 +3,7 @@
 #define XEN_HVM_H__
 
 #include <xen/interface/hvm/params.h>
+#include <asm/xen/hypercall.h>
 
 static inline int hvm_get_parameter(int idx, uint64_t *value)
 {
@@ -21,4 +22,12 @@ static inline int hvm_get_parameter(int idx, uint64_t *value)
        return r;
 }
 
+int xen_set_callback_via(uint64_t via);
+extern int xen_have_vector_callback;
+
+#define HVM_CALLBACK_VIA_TYPE_VECTOR 0x2
+#define HVM_CALLBACK_VIA_TYPE_SHIFT 56
+#define HVM_CALLBACK_VECTOR(x) (((uint64_t)HVM_CALLBACK_VIA_TYPE_VECTOR)<<\
+                               HVM_CALLBACK_VIA_TYPE_SHIFT | (x))
+
 #endif /* XEN_HVM_H__ */
diff --git a/include/xen/interface/features.h b/include/xen/interface/features.h
index f51b641..8ab08b9 100644
--- a/include/xen/interface/features.h
+++ b/include/xen/interface/features.h
@@ -41,6 +41,9 @@
 /* x86: Does this Xen host support the MMU_PT_UPDATE_PRESERVE_AD hypercall? */
 #define XENFEAT_mmu_pt_update_preserve_ad  5
 
+/* x86: Does this Xen host support the HVM callback vector type? */
+#define XENFEAT_hvm_callback_vector        8
+
 #define XENFEAT_NR_SUBMAPS 1
 
 #endif /* __XEN_PUBLIC_FEATURES_H__ */
-- 
1.5.4.3

Re: [PATCH 03/12] evtchn delivery on HVM

[Jeremy Fitzhardinge]

On 05/18/2010 03:22 AM, Stefano Stabellini wrote:
> From: Sheng Yang <sheng@linux.intel.com>
>
> Set the callback to receive evtchns from Xen, using the
> callback vector delivery mechanism.
>   

Could you expand on this a little?  Like, why is this desireable?  What
functional difference does it make?  Is this patch useful in its own
right, or is it just laying the groundwork for something else?

Thanks,
    J

> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Signed-off-by: Sheng Yang <sheng@linux.intel.com>
> ---
>  arch/x86/xen/enlighten.c         |   35 +++++++++++++++++++++++++++++++++++
>  drivers/xen/events.c             |   31 ++++++++++++++++++++++++-------
>  include/xen/events.h             |    3 +++
>  include/xen/hvm.h                |    9 +++++++++
>  include/xen/interface/features.h |    3 +++
>  5 files changed, 74 insertions(+), 7 deletions(-)
>
> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> index 87a3b10..502c4f8 100644
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
> @@ -37,8 +37,11 @@
>  #include <xen/interface/vcpu.h>
>  #include <xen/interface/memory.h>
>  #include <xen/interface/hvm/hvm_op.h>
> +#include <xen/interface/hvm/params.h>
>  #include <xen/features.h>
>  #include <xen/page.h>
> +#include <xen/hvm.h>
> +#include <xen/events.h>
>  #include <xen/hvc-console.h>
>  
>  #include <asm/paravirt.h>
> @@ -79,6 +82,8 @@ struct shared_info xen_dummy_shared_info;
>  
>  void *xen_initial_gdt;
>  
> +int xen_have_vector_callback;
> +
>  /*
>   * Point at some empty memory to start with. We map the real shared_info
>   * page as soon as fixmap is up and running.
> @@ -1279,6 +1284,31 @@ static void __init init_shared_info(void)
>  	per_cpu(xen_vcpu, 0) = &HYPERVISOR_shared_info->vcpu_info[0];
>  }
>  
> +int xen_set_callback_via(uint64_t via)
> +{
> +	struct xen_hvm_param a;
> +	a.domid = DOMID_SELF;
> +	a.index = HVM_PARAM_CALLBACK_IRQ;
> +	a.value = via;
> +	return HYPERVISOR_hvm_op(HVMOP_set_param, &a);
> +}
> +
> +void do_hvm_pv_evtchn_intr(void)
> +{
> +	xen_hvm_evtchn_do_upcall(get_irq_regs());
> +}
> +
> +static void xen_callback_vector(void)
> +{
> +	uint64_t callback_via;
> +	if (xen_feature(XENFEAT_hvm_callback_vector)) {
> +		callback_via = HVM_CALLBACK_VECTOR(X86_PLATFORM_IPI_VECTOR);
> +		xen_set_callback_via(callback_via);
> +		x86_platform_ipi_callback = do_hvm_pv_evtchn_intr;
> +		xen_have_vector_callback = 1;
> +	}
> +}
> +
>  void __init xen_guest_init(void)
>  {
>  	int r;
> @@ -1292,4 +1322,9 @@ void __init xen_guest_init(void)
>  		return;
>  
>  	init_shared_info();
> +
> +	xen_callback_vector();
> +
> +	have_vcpu_info_placement = 0;
> +	x86_init.irqs.intr_init = xen_init_IRQ;
>  }
> diff --git a/drivers/xen/events.c b/drivers/xen/events.c
> index db8f506..3523dbb 100644
> --- a/drivers/xen/events.c
> +++ b/drivers/xen/events.c
> @@ -36,6 +36,8 @@
>  #include <asm/xen/hypercall.h>
>  #include <asm/xen/hypervisor.h>
>  
> +#include <xen/xen.h>
> +#include <xen/hvm.h>
>  #include <xen/xen-ops.h>
>  #include <xen/events.h>
>  #include <xen/interface/xen.h>
> @@ -617,17 +619,13 @@ static DEFINE_PER_CPU(unsigned, xed_nesting_count);
>   * a bitset of words which contain pending event bits.  The second
>   * level is a bitset of pending events themselves.
>   */
> -void xen_evtchn_do_upcall(struct pt_regs *regs)
> +void __xen_evtchn_do_upcall(struct pt_regs *regs)
>  {
>  	int cpu = get_cpu();
> -	struct pt_regs *old_regs = set_irq_regs(regs);
>  	struct shared_info *s = HYPERVISOR_shared_info;
>  	struct vcpu_info *vcpu_info = __get_cpu_var(xen_vcpu);
>   	unsigned count;
>  
> -	exit_idle();
> -	irq_enter();
> -
>  	do {
>  		unsigned long pending_words;
>  
> @@ -667,10 +665,26 @@ void xen_evtchn_do_upcall(struct pt_regs *regs)
>  	} while(count != 1);
>  
>  out:
> +
> +	put_cpu();
> +}
> +
> +void xen_evtchn_do_upcall(struct pt_regs *regs)
> +{
> +	struct pt_regs *old_regs = set_irq_regs(regs);
> +
> +	exit_idle();
> +	irq_enter();
> +
> +	__xen_evtchn_do_upcall(regs);
> +
>  	irq_exit();
>  	set_irq_regs(old_regs);
> +}
>  
> -	put_cpu();
> +void xen_hvm_evtchn_do_upcall(struct pt_regs *regs)
> +{
> +	__xen_evtchn_do_upcall(regs);
>  }
>  
>  /* Rebind a new event channel to an existing irq. */
> @@ -947,5 +961,8 @@ void __init xen_init_IRQ(void)
>  	for (i = 0; i < NR_EVENT_CHANNELS; i++)
>  		mask_evtchn(i);
>  
> -	irq_ctx_init(smp_processor_id());
> +	if (xen_hvm_domain())
> +		native_init_IRQ();
> +	else
> +		irq_ctx_init(smp_processor_id());
>  }
> diff --git a/include/xen/events.h b/include/xen/events.h
> index e68d59a..868e5d6 100644
> --- a/include/xen/events.h
> +++ b/include/xen/events.h
> @@ -56,4 +56,7 @@ void xen_poll_irq(int irq);
>  /* Determine the IRQ which is bound to an event channel */
>  unsigned irq_from_evtchn(unsigned int evtchn);
>  
> +void xen_evtchn_do_upcall(struct pt_regs *regs);
> +void xen_hvm_evtchn_do_upcall(struct pt_regs *regs);
> +
>  #endif	/* _XEN_EVENTS_H */
> diff --git a/include/xen/hvm.h b/include/xen/hvm.h
> index 6b0d418..5940ee5 100644
> --- a/include/xen/hvm.h
> +++ b/include/xen/hvm.h
> @@ -3,6 +3,7 @@
>  #define XEN_HVM_H__
>  
>  #include <xen/interface/hvm/params.h>
> +#include <asm/xen/hypercall.h>
>  
>  static inline int hvm_get_parameter(int idx, uint64_t *value)
>  {
> @@ -21,4 +22,12 @@ static inline int hvm_get_parameter(int idx, uint64_t *value)
>         return r;
>  }
>  
> +int xen_set_callback_via(uint64_t via);
> +extern int xen_have_vector_callback;
> +
> +#define HVM_CALLBACK_VIA_TYPE_VECTOR 0x2
> +#define HVM_CALLBACK_VIA_TYPE_SHIFT 56
> +#define HVM_CALLBACK_VECTOR(x) (((uint64_t)HVM_CALLBACK_VIA_TYPE_VECTOR)<<\
> +                               HVM_CALLBACK_VIA_TYPE_SHIFT | (x))
> +
>  #endif /* XEN_HVM_H__ */
> diff --git a/include/xen/interface/features.h b/include/xen/interface/features.h
> index f51b641..8ab08b9 100644
> --- a/include/xen/interface/features.h
> +++ b/include/xen/interface/features.h
> @@ -41,6 +41,9 @@
>  /* x86: Does this Xen host support the MMU_PT_UPDATE_PRESERVE_AD hypercall? */
>  #define XENFEAT_mmu_pt_update_preserve_ad  5
>  
> +/* x86: Does this Xen host support the HVM callback vector type? */
> +#define XENFEAT_hvm_callback_vector        8
> +
>  #define XENFEAT_NR_SUBMAPS 1
>  
>  #endif /* __XEN_PUBLIC_FEATURES_H__ */
>   

Re: [PATCH 03/12] evtchn delivery on HVM

[Stefano Stabellini]

On Tue, 18 May 2010, Jeremy Fitzhardinge wrote:
> On 05/18/2010 03:22 AM, Stefano Stabellini wrote:
> > From: Sheng Yang <sheng@linux.intel.com>
> >
> > Set the callback to receive evtchns from Xen, using the
> > callback vector delivery mechanism.
> >   
> 
> Could you expand on this a little?  Like, why is this desireable?  What
> functional difference does it make?  Is this patch useful in its own
> right, or is it just laying the groundwork for something else?
> 

In order to use PV frontends on HVM we need to receive notifications on
event channel deliveries somehow.
Using the callback vector is the preferred way, because it is available
independently from any (emulated) PCI device, all the vcpus can receive
these callbacks and theoretically there is no need to interact with the
emulated lapic (even though at the moment we are doing it anyway because
we are using the IPI vector).
The other way is to receive interrupts from the xen platform pci device,
but in that case interaction with the emulated lapic is unavoidable and
we are limited to receive interrupts on vcpu 0.

Re: [PATCH 03/12] evtchn delivery on HVM

[Jeremy Fitzhardinge]

On 05/19/2010 05:24 AM, Stefano Stabellini wrote:
> On Tue, 18 May 2010, Jeremy Fitzhardinge wrote:
>   
>> On 05/18/2010 03:22 AM, Stefano Stabellini wrote:
>>     
>>> From: Sheng Yang <sheng@linux.intel.com>
>>>
>>> Set the callback to receive evtchns from Xen, using the
>>> callback vector delivery mechanism.
>>>   
>>>       
>> Could you expand on this a little?  Like, why is this desireable?  What
>> functional difference does it make?  Is this patch useful in its own
>> right, or is it just laying the groundwork for something else?
>>
>>     
> In order to use PV frontends on HVM we need to receive notifications on
> event channel deliveries somehow.
>   
(OK, but I just meant update the commit comment on the patch itself.)

> Using the callback vector is the preferred way, because it is available
> independently from any (emulated) PCI device, all the vcpus can receive
> these callbacks and theoretically there is no need to interact with the
> emulated lapic (even though at the moment we are doing it anyway because
> we are using the IPI vector).
>   
> The other way is to receive interrupts from the xen platform pci device,
> but in that case interaction with the emulated lapic is unavoidable and
> we are limited to receive interrupts on vcpu 0.
>   

Perhaps you should mention this first, since it is the historical way of
doing it, and then talk about its limitations, and then talk about the
replacement to address those limitations.

    J

Re: [PATCH 03/12] evtchn delivery on HVM

[Jeremy Fitzhardinge]

On 05/18/2010 03:22 AM, Stefano Stabellini wrote:
> From: Sheng Yang <sheng@linux.intel.com>
>
> Set the callback to receive evtchns from Xen, using the
> callback vector delivery mechanism.
>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Signed-off-by: Sheng Yang <sheng@linux.intel.com>
> ---
>  arch/x86/xen/enlighten.c         |   35 +++++++++++++++++++++++++++++++++++
>  drivers/xen/events.c             |   31 ++++++++++++++++++++++++-------
>  include/xen/events.h             |    3 +++
>  include/xen/hvm.h                |    9 +++++++++
>  include/xen/interface/features.h |    3 +++
>  5 files changed, 74 insertions(+), 7 deletions(-)
>
> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> index 87a3b10..502c4f8 100644
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
> @@ -37,8 +37,11 @@
>  #include <xen/interface/vcpu.h>
>  #include <xen/interface/memory.h>
>  #include <xen/interface/hvm/hvm_op.h>
> +#include <xen/interface/hvm/params.h>
>  #include <xen/features.h>
>  #include <xen/page.h>
> +#include <xen/hvm.h>
> +#include <xen/events.h>
>  #include <xen/hvc-console.h>
>  
>  #include <asm/paravirt.h>
> @@ -79,6 +82,8 @@ struct shared_info xen_dummy_shared_info;
>  
>  void *xen_initial_gdt;
>  
> +int xen_have_vector_callback;
>   

BTW, this can be a __read_mostly.

    J

Re: [PATCH 03/12] evtchn delivery on HVM

[Stefano Stabellini]

On Tue, 18 May 2010, Jeremy Fitzhardinge wrote:
oid *xen_initial_gdt;
> >  
> > +int xen_have_vector_callback;
> >   
> 
> BTW, this can be a __read_mostly.
> 
 
I'll do that, thanks.

Re: [PATCH 03/12] evtchn delivery on HVM

[Jeremy Fitzhardinge]

On 05/18/2010 03:22 AM, Stefano Stabellini wrote:
> From: Sheng Yang <sheng@linux.intel.com>
>
> Set the callback to receive evtchns from Xen, using the
> callback vector delivery mechanism.
>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Signed-off-by: Sheng Yang <sheng@linux.intel.com>
> ---
>  arch/x86/xen/enlighten.c         |   35 +++++++++++++++++++++++++++++++++++
>  drivers/xen/events.c             |   31 ++++++++++++++++++++++++-------
>  include/xen/events.h             |    3 +++
>  include/xen/hvm.h                |    9 +++++++++
>  include/xen/interface/features.h |    3 +++
>  5 files changed, 74 insertions(+), 7 deletions(-)
>
> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> index 87a3b10..502c4f8 100644
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
> @@ -37,8 +37,11 @@
>  #include <xen/interface/vcpu.h>
>  #include <xen/interface/memory.h>
>  #include <xen/interface/hvm/hvm_op.h>
> +#include <xen/interface/hvm/params.h>
>  #include <xen/features.h>
>  #include <xen/page.h>
> +#include <xen/hvm.h>
> +#include <xen/events.h>
>  #include <xen/hvc-console.h>
>  
>  #include <asm/paravirt.h>
> @@ -79,6 +82,8 @@ struct shared_info xen_dummy_shared_info;
>  
>  void *xen_initial_gdt;
>  
> +int xen_have_vector_callback;
> +
>  /*
>   * Point at some empty memory to start with. We map the real shared_info
>   * page as soon as fixmap is up and running.
> @@ -1279,6 +1284,31 @@ static void __init init_shared_info(void)
>  	per_cpu(xen_vcpu, 0) = &HYPERVISOR_shared_info->vcpu_info[0];
>  }
>  
> +int xen_set_callback_via(uint64_t via)
> +{
> +	struct xen_hvm_param a;
> +	a.domid = DOMID_SELF;
> +	a.index = HVM_PARAM_CALLBACK_IRQ;
> +	a.value = via;
> +	return HYPERVISOR_hvm_op(HVMOP_set_param, &a);
>   

Does this implicitly set the vector delivery on all vcpus, current and
future?

> +}
> +
> +void do_hvm_pv_evtchn_intr(void)
> +{
> +	xen_hvm_evtchn_do_upcall(get_irq_regs());
> +}
> +
> +static void xen_callback_vector(void)
>   

All this callback vector stuff should be in drivers/xen/events.c.  It
would also be good to give it a more descriptive name
("xen_set_callback_vector"?), and make it an init function.

> +{
> +	uint64_t callback_via;
> +	if (xen_feature(XENFEAT_hvm_callback_vector)) {
> +		callback_via = HVM_CALLBACK_VECTOR(X86_PLATFORM_IPI_VECTOR);
> +		xen_set_callback_via(callback_via);
>   

Do you need to check the return value here?  Can it possibly fail?

> +		x86_platform_ipi_callback = do_hvm_pv_evtchn_intr;
> +		xen_have_vector_callback = 1;
> +	}
> +}
> +
>  void __init xen_guest_init(void)
>  {
>  	int r;
> @@ -1292,4 +1322,9 @@ void __init xen_guest_init(void)
>  		return;
>  
>  	init_shared_info();
> +
> +	xen_callback_vector();
> +
> +	have_vcpu_info_placement = 0;
> +	x86_init.irqs.intr_init = xen_init_IRQ;
>  }
> diff --git a/drivers/xen/events.c b/drivers/xen/events.c
> index db8f506..3523dbb 100644
> --- a/drivers/xen/events.c
> +++ b/drivers/xen/events.c
> @@ -36,6 +36,8 @@
>  #include <asm/xen/hypercall.h>
>  #include <asm/xen/hypervisor.h>
>  
> +#include <xen/xen.h>
> +#include <xen/hvm.h>
>  #include <xen/xen-ops.h>
>  #include <xen/events.h>
>  #include <xen/interface/xen.h>
> @@ -617,17 +619,13 @@ static DEFINE_PER_CPU(unsigned, xed_nesting_count);
>   * a bitset of words which contain pending event bits.  The second
>   * level is a bitset of pending events themselves.
>   */
> -void xen_evtchn_do_upcall(struct pt_regs *regs)
> +void __xen_evtchn_do_upcall(struct pt_regs *regs)
>   

Given that the regs arg is completely unused, you should drop it.

>  {
>  	int cpu = get_cpu();
> -	struct pt_regs *old_regs = set_irq_regs(regs);
>  	struct shared_info *s = HYPERVISOR_shared_info;
>  	struct vcpu_info *vcpu_info = __get_cpu_var(xen_vcpu);
>   	unsigned count;
>  
> -	exit_idle();
> -	irq_enter();
> -
>  	do {
>  		unsigned long pending_words;
>  
> @@ -667,10 +665,26 @@ void xen_evtchn_do_upcall(struct pt_regs *regs)
>  	} while(count != 1);
>  
>  out:
> +
> +	put_cpu();
> +}
> +
> +void xen_evtchn_do_upcall(struct pt_regs *regs)
> +{
> +	struct pt_regs *old_regs = set_irq_regs(regs);
> +
> +	exit_idle();
> +	irq_enter();
> +
> +	__xen_evtchn_do_upcall(regs);
> +
>  	irq_exit();
>  	set_irq_regs(old_regs);
> +}
>  
> -	put_cpu();
> +void xen_hvm_evtchn_do_upcall(struct pt_regs *regs)
> +{
> +	__xen_evtchn_do_upcall(regs);
>   

Don't you need to set_irq_regs here?

>  }
>  
>  /* Rebind a new event channel to an existing irq. */
> @@ -947,5 +961,8 @@ void __init xen_init_IRQ(void)
>  	for (i = 0; i < NR_EVENT_CHANNELS; i++)
>  		mask_evtchn(i);
>  
> -	irq_ctx_init(smp_processor_id());
> +	if (xen_hvm_domain())
> +		native_init_IRQ();
> +	else
> +		irq_ctx_init(smp_processor_id());
>  }
> diff --git a/include/xen/events.h b/include/xen/events.h
> index e68d59a..868e5d6 100644
> --- a/include/xen/events.h
> +++ b/include/xen/events.h
> @@ -56,4 +56,7 @@ void xen_poll_irq(int irq);
>  /* Determine the IRQ which is bound to an event channel */
>  unsigned irq_from_evtchn(unsigned int evtchn);
>  
> +void xen_evtchn_do_upcall(struct pt_regs *regs);
> +void xen_hvm_evtchn_do_upcall(struct pt_regs *regs);
> +
>  #endif	/* _XEN_EVENTS_H */
> diff --git a/include/xen/hvm.h b/include/xen/hvm.h
> index 6b0d418..5940ee5 100644
> --- a/include/xen/hvm.h
> +++ b/include/xen/hvm.h
> @@ -3,6 +3,7 @@
>  #define XEN_HVM_H__
>  
>  #include <xen/interface/hvm/params.h>
> +#include <asm/xen/hypercall.h>
>  
>  static inline int hvm_get_parameter(int idx, uint64_t *value)
>  {
> @@ -21,4 +22,12 @@ static inline int hvm_get_parameter(int idx, uint64_t *value)
>         return r;
>  }
>  
> +int xen_set_callback_via(uint64_t via);
> +extern int xen_have_vector_callback;
> +
> +#define HVM_CALLBACK_VIA_TYPE_VECTOR 0x2
> +#define HVM_CALLBACK_VIA_TYPE_SHIFT 56
> +#define HVM_CALLBACK_VECTOR(x) (((uint64_t)HVM_CALLBACK_VIA_TYPE_VECTOR)<<\
> +                               HVM_CALLBACK_VIA_TYPE_SHIFT | (x))
> +
>  #endif /* XEN_HVM_H__ */
> diff --git a/include/xen/interface/features.h b/include/xen/interface/features.h
> index f51b641..8ab08b9 100644
> --- a/include/xen/interface/features.h
> +++ b/include/xen/interface/features.h
> @@ -41,6 +41,9 @@
>  /* x86: Does this Xen host support the MMU_PT_UPDATE_PRESERVE_AD hypercall? */
>  #define XENFEAT_mmu_pt_update_preserve_ad  5
>  
> +/* x86: Does this Xen host support the HVM callback vector type? */
> +#define XENFEAT_hvm_callback_vector        8
> +
>  #define XENFEAT_NR_SUBMAPS 1
>  
>  #endif /* __XEN_PUBLIC_FEATURES_H__ */
>   

Re: [PATCH 03/12] evtchn delivery on HVM

[Stefano Stabellini]

On Tue, 18 May 2010, Jeremy Fitzhardinge wrote:
> On 05/18/2010 03:22 AM, Stefano Stabellini wrote:
> > From: Sheng Yang <sheng@linux.intel.com>
> >
> > Set the callback to receive evtchns from Xen, using the
> > callback vector delivery mechanism.
> >
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > Signed-off-by: Sheng Yang <sheng@linux.intel.com>
> > ---
> >  arch/x86/xen/enlighten.c         |   35 +++++++++++++++++++++++++++++++++++
> >  drivers/xen/events.c             |   31 ++++++++++++++++++++++++-------
> >  include/xen/events.h             |    3 +++
> >  include/xen/hvm.h                |    9 +++++++++
> >  include/xen/interface/features.h |    3 +++
> >  5 files changed, 74 insertions(+), 7 deletions(-)
> >
> > diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> > index 87a3b10..502c4f8 100644
> > --- a/arch/x86/xen/enlighten.c
> > +++ b/arch/x86/xen/enlighten.c
> > @@ -37,8 +37,11 @@
> >  #include <xen/interface/vcpu.h>
> >  #include <xen/interface/memory.h>
> >  #include <xen/interface/hvm/hvm_op.h>
> > +#include <xen/interface/hvm/params.h>
> >  #include <xen/features.h>
> >  #include <xen/page.h>
> > +#include <xen/hvm.h>
> > +#include <xen/events.h>
> >  #include <xen/hvc-console.h>
> >  
> >  #include <asm/paravirt.h>
> > @@ -79,6 +82,8 @@ struct shared_info xen_dummy_shared_info;
> >  
> >  void *xen_initial_gdt;
> >  
> > +int xen_have_vector_callback;
> > +
> >  /*
> >   * Point at some empty memory to start with. We map the real shared_info
> >   * page as soon as fixmap is up and running.
> > @@ -1279,6 +1284,31 @@ static void __init init_shared_info(void)
> >  	per_cpu(xen_vcpu, 0) = &HYPERVISOR_shared_info->vcpu_info[0];
> >  }
> >  
> > +int xen_set_callback_via(uint64_t via)
> > +{
> > +	struct xen_hvm_param a;
> > +	a.domid = DOMID_SELF;
> > +	a.index = HVM_PARAM_CALLBACK_IRQ;
> > +	a.value = via;
> > +	return HYPERVISOR_hvm_op(HVMOP_set_param, &a);
> >   
> 
> Does this implicitly set the vector delivery on all vcpus, current and
> future?
> 

Yes.

> > +}
> > +
> > +void do_hvm_pv_evtchn_intr(void)
> > +{
> > +	xen_hvm_evtchn_do_upcall(get_irq_regs());
> > +}
> > +
> > +static void xen_callback_vector(void)
> >   
> 
> All this callback vector stuff should be in drivers/xen/events.c.  It
> would also be good to give it a more descriptive name
> ("xen_set_callback_vector"?), and make it an init function.
> 

I could move it events.c and call it at the beginning of xen_init_IRQ,
is that OK?


> > +{
> > +	uint64_t callback_via;
> > +	if (xen_feature(XENFEAT_hvm_callback_vector)) {
> > +		callback_via = HVM_CALLBACK_VECTOR(X86_PLATFORM_IPI_VECTOR);
> > +		xen_set_callback_via(callback_via);
> >   
> 
> Do you need to check the return value here?  Can it possibly fail?
> 

Yes, it can fail. The vector delivery mechanism hasn't been checked in
Xen yet (I sent the patch right after this patch series).


> > +		x86_platform_ipi_callback = do_hvm_pv_evtchn_intr;
> > +		xen_have_vector_callback = 1;
> > +	}
> > +}
> > +
> >  void __init xen_guest_init(void)
> >  {
> >  	int r;
> > @@ -1292,4 +1322,9 @@ void __init xen_guest_init(void)
> >  		return;
> >  
> >  	init_shared_info();
> > +
> > +	xen_callback_vector();
> > +
> > +	have_vcpu_info_placement = 0;
> > +	x86_init.irqs.intr_init = xen_init_IRQ;
> >  }
> > diff --git a/drivers/xen/events.c b/drivers/xen/events.c
> > index db8f506..3523dbb 100644
> > --- a/drivers/xen/events.c
> > +++ b/drivers/xen/events.c
> > @@ -36,6 +36,8 @@
> >  #include <asm/xen/hypercall.h>
> >  #include <asm/xen/hypervisor.h>
> >  
> > +#include <xen/xen.h>
> > +#include <xen/hvm.h>
> >  #include <xen/xen-ops.h>
> >  #include <xen/events.h>
> >  #include <xen/interface/xen.h>
> > @@ -617,17 +619,13 @@ static DEFINE_PER_CPU(unsigned, xed_nesting_count);
> >   * a bitset of words which contain pending event bits.  The second
> >   * level is a bitset of pending events themselves.
> >   */
> > -void xen_evtchn_do_upcall(struct pt_regs *regs)
> > +void __xen_evtchn_do_upcall(struct pt_regs *regs)
> >   
> 
> Given that the regs arg is completely unused, you should drop it.
> 

OK

> >  {
> >  	int cpu = get_cpu();
> > -	struct pt_regs *old_regs = set_irq_regs(regs);
> >  	struct shared_info *s = HYPERVISOR_shared_info;
> >  	struct vcpu_info *vcpu_info = __get_cpu_var(xen_vcpu);
> >   	unsigned count;
> >  
> > -	exit_idle();
> > -	irq_enter();
> > -
> >  	do {
> >  		unsigned long pending_words;
> >  
> > @@ -667,10 +665,26 @@ void xen_evtchn_do_upcall(struct pt_regs *regs)
> >  	} while(count != 1);
> >  
> >  out:
> > +
> > +	put_cpu();
> > +}
> > +
> > +void xen_evtchn_do_upcall(struct pt_regs *regs)
> > +{
> > +	struct pt_regs *old_regs = set_irq_regs(regs);
> > +
> > +	exit_idle();
> > +	irq_enter();
> > +
> > +	__xen_evtchn_do_upcall(regs);
> > +
> >  	irq_exit();
> >  	set_irq_regs(old_regs);
> > +}
> >  
> > -	put_cpu();
> > +void xen_hvm_evtchn_do_upcall(struct pt_regs *regs)
> > +{
> > +	__xen_evtchn_do_upcall(regs);
> >   
> 
> Don't you need to set_irq_regs here?

No, that was done by smp_x86_platform_ipi or do_IRQ.