[PATCH v4 0/7] xen/arm: move to mach-virt and support SMP

[PATCH v4 0/7] xen/arm: move to mach-virt and support SMP

[Stefano Stabellini]

Hi all,
this patch series, based on 3.9-rc3, moves xenvm to mach-virt and
implements SMP support in Xen on ARM.

Each patch comes with a detailed changelog.


Changes in v4:
- XEN should select ARM_PSCI;
- update arch/arm/boot/dts/Makefile to build the xenvm dts
  if CONFIG_ARCH_VIRT;
- add a patch to introduce Xen shutdown hypercall support to mach-virt.


Stefano Stabellini (7):
      xen/arm: actually pass a non-NULL percpu pointer to request_percpu_irq
      xen/arm: SMP support
      xen: move the xenvm machine to mach-virt
      xen/arm: implement HYPERVISOR_vcpu_op
      xen/arm: XEN selects ARM_PSCI
      xenvm: add a simple PSCI node and a second cpu
      mach-virt: support Xen hypercalls for shutdown and reboot

 arch/arm/Kconfig                     |    1 +
 arch/arm/boot/dts/Makefile           |    4 +-
 arch/arm/boot/dts/xenvm-4.2.dts      |   13 ++++++++++
 arch/arm/include/asm/xen/hypercall.h |    1 +
 arch/arm/mach-vexpress/v2m.c         |    1 -
 arch/arm/mach-virt/virt.c            |   33 +++++++++++++++++++++++++++
 arch/arm/xen/enlighten.c             |   41 ++++++++++++++++++++++++++++++++-
 arch/arm/xen/hypercall.S             |    1 +
 8 files changed, 90 insertions(+), 5 deletions(-)

git://git.kernel.org/pub/scm/linux/kernel/git/sstabellini/xen.git 3.9-rc3-smp-4-tag

Cheers,

Stefano

[PATCH v4 1/7] xen/arm: actually pass a non-NULL percpu pointer to request_percpu_irq

[Stefano Stabellini]

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
CC: stable@vger.kernel.org
---
 arch/arm/xen/enlighten.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index 8dc0605..99ce189 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -239,7 +239,7 @@ static int __init xen_init_events(void)
 	xen_init_IRQ();
 
 	if (request_percpu_irq(xen_events_irq, xen_arm_callback,
-			"events", xen_vcpu)) {
+			"events", &xen_vcpu)) {
 		pr_err("Error requesting IRQ %d\n", xen_events_irq);
 		return -EINVAL;
 	}
-- 
1.7.2.5

[PATCH v4 2/7] xen/arm: SMP support

[Stefano Stabellini]

Map vcpu_info using VCPUOP_register_vcpu_info on secondary cpus.

Call enable_percpu_irq on every cpu.

Changed in v2:
- move the percpu variable argument fix to a separate patch;
- remove unused variable.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 arch/arm/xen/enlighten.c |   38 +++++++++++++++++++++++++++++++++++++-
 1 files changed, 37 insertions(+), 1 deletions(-)

diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index 99ce189..94bbf3b 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -2,6 +2,7 @@
 #include <xen/events.h>
 #include <xen/grant_table.h>
 #include <xen/hvm.h>
+#include <xen/interface/vcpu.h>
 #include <xen/interface/xen.h>
 #include <xen/interface/memory.h>
 #include <xen/interface/hvm/params.h>
@@ -32,6 +33,7 @@ struct shared_info xen_dummy_shared_info;
 struct shared_info *HYPERVISOR_shared_info = (void *)&xen_dummy_shared_info;
 
 DEFINE_PER_CPU(struct vcpu_info *, xen_vcpu);
+DEFINE_PER_CPU(struct vcpu_info, xen_vcpu_info);
 
 /* These are unused until we support booting "pre-ballooned" */
 unsigned long xen_released_pages;
@@ -148,6 +150,32 @@ int xen_unmap_domain_mfn_range(struct vm_area_struct *vma,
 }
 EXPORT_SYMBOL_GPL(xen_unmap_domain_mfn_range);
 
+static int __init xen_secondary_init(unsigned int cpu)
+{
+	struct vcpu_register_vcpu_info info;
+	struct vcpu_info *vcpup;
+	int err;
+
+	if (cpu == 0)
+		return 0;
+
+	pr_info("Xen: initializing cpu%d\n", cpu);
+	vcpup = &per_cpu(xen_vcpu_info, cpu);
+
+	info.mfn = __pa(vcpup) >> PAGE_SHIFT;
+	info.offset = offset_in_page(vcpup);
+
+	err = HYPERVISOR_vcpu_op(VCPUOP_register_vcpu_info, cpu, &info);
+	if (err) {
+		pr_debug("register_vcpu_info failed: err=%d\n", err);
+	} else {
+		/* This cpu is using the registered vcpu info, even if
+		   later ones fail to. */
+		per_cpu(xen_vcpu, cpu) = vcpup;
+	}
+	return 0;
+}
+
 /*
  * see Documentation/devicetree/bindings/arm/xen.txt for the
  * documentation of the Xen Device Tree format.
@@ -163,6 +191,7 @@ static int __init xen_guest_init(void)
 	const char *version = NULL;
 	const char *xen_prefix = "xen,xen-";
 	struct resource res;
+	int i;
 
 	node = of_find_compatible_node(NULL, NULL, "xen,xen");
 	if (!node) {
@@ -216,6 +245,8 @@ static int __init xen_guest_init(void)
 	 * is required to use VCPUOP_register_vcpu_info to place vcpu info
 	 * for secondary CPUs as they are brought up. */
 	per_cpu(xen_vcpu, 0) = &HYPERVISOR_shared_info->vcpu_info[0];
+	for_each_online_cpu(i)
+		xen_secondary_init(i);
 
 	gnttab_init();
 	if (!xen_initial_domain())
@@ -231,6 +262,11 @@ static irqreturn_t xen_arm_callback(int irq, void *arg)
 	return IRQ_HANDLED;
 }
 
+static __init void xen_percpu_enable_events(void *unused)
+{
+	enable_percpu_irq(xen_events_irq, 0);
+}
+
 static int __init xen_init_events(void)
 {
 	if (!xen_domain() || xen_events_irq < 0)
@@ -244,7 +280,7 @@ static int __init xen_init_events(void)
 		return -EINVAL;
 	}
 
-	enable_percpu_irq(xen_events_irq, 0);
+	on_each_cpu(xen_percpu_enable_events, NULL, 0);
 
 	return 0;
 }
-- 
1.7.2.5

[PATCH v4 3/7] xen: move the xenvm machine to mach-virt

[Stefano Stabellini]

xenvm is based on mach-vexpress, move it to mach-virt.

Changes in v4:
- update the dts Makefile too.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
CC: Marc Zyngier <marc.zyngier@arm.com>
CC: will.deacon@arm.com
CC: arnd@arndb.de
CC: rob.herring@calxeda.com
---
 arch/arm/boot/dts/Makefile   |    4 ++--
 arch/arm/mach-vexpress/v2m.c |    1 -
 arch/arm/mach-virt/virt.c    |    1 +
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 9c62558..b6289b7 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -168,8 +168,8 @@ dtb-$(CONFIG_ARCH_TEGRA) += tegra20-harmony.dtb \
 dtb-$(CONFIG_ARCH_VEXPRESS) += vexpress-v2p-ca5s.dtb \
 	vexpress-v2p-ca9.dtb \
 	vexpress-v2p-ca15-tc1.dtb \
-	vexpress-v2p-ca15_a7.dtb \
-	xenvm-4.2.dtb
+	vexpress-v2p-ca15_a7.dtb
+dtb-$(CONFIG_ARCH_VIRT) += xenvm-4.2.dtb
 dtb-$(CONFIG_ARCH_VT8500) += vt8500-bv07.dtb \
 	wm8505-ref.dtb \
 	wm8650-mid.dtb \
diff --git a/arch/arm/mach-vexpress/v2m.c b/arch/arm/mach-vexpress/v2m.c
index 915683c..c43ec78 100644
--- a/arch/arm/mach-vexpress/v2m.c
+++ b/arch/arm/mach-vexpress/v2m.c
@@ -469,7 +469,6 @@ static void __init v2m_dt_init(void)
 
 static const char * const v2m_dt_match[] __initconst = {
 	"arm,vexpress",
-	"xen,xenvm",
 	NULL,
 };
 
diff --git a/arch/arm/mach-virt/virt.c b/arch/arm/mach-virt/virt.c
index 31666f6..528c05e 100644
--- a/arch/arm/mach-virt/virt.c
+++ b/arch/arm/mach-virt/virt.c
@@ -40,6 +40,7 @@ static void __init virt_timer_init(void)
 
 static const char *virt_dt_match[] = {
 	"linux,dummy-virt",
+	"xen,xenvm",
 	NULL
 };
 
-- 
1.7.2.5

[PATCH v4 4/7] xen/arm: implement HYPERVISOR_vcpu_op

[Stefano Stabellini]

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 arch/arm/include/asm/xen/hypercall.h |    1 +
 arch/arm/xen/enlighten.c             |    1 +
 arch/arm/xen/hypercall.S             |    1 +
 3 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/arch/arm/include/asm/xen/hypercall.h b/arch/arm/include/asm/xen/hypercall.h
index 8a82325..799f42e 100644
--- a/arch/arm/include/asm/xen/hypercall.h
+++ b/arch/arm/include/asm/xen/hypercall.h
@@ -46,6 +46,7 @@ int HYPERVISOR_event_channel_op(int cmd, void *arg);
 unsigned long HYPERVISOR_hvm_op(int op, void *arg);
 int HYPERVISOR_memory_op(unsigned int cmd, void *arg);
 int HYPERVISOR_physdev_op(int cmd, void *arg);
+int HYPERVISOR_vcpu_op(int cmd, int vcpuid, void *extra_args);
 
 static inline void
 MULTI_update_va_mapping(struct multicall_entry *mcl, unsigned long va,
diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index 94bbf3b..b002822 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -295,4 +295,5 @@ EXPORT_SYMBOL_GPL(HYPERVISOR_sched_op);
 EXPORT_SYMBOL_GPL(HYPERVISOR_hvm_op);
 EXPORT_SYMBOL_GPL(HYPERVISOR_memory_op);
 EXPORT_SYMBOL_GPL(HYPERVISOR_physdev_op);
+EXPORT_SYMBOL_GPL(HYPERVISOR_vcpu_op);
 EXPORT_SYMBOL_GPL(privcmd_call);
diff --git a/arch/arm/xen/hypercall.S b/arch/arm/xen/hypercall.S
index 71f7239..199cb2d 100644
--- a/arch/arm/xen/hypercall.S
+++ b/arch/arm/xen/hypercall.S
@@ -87,6 +87,7 @@ HYPERCALL2(event_channel_op);
 HYPERCALL2(hvm_op);
 HYPERCALL2(memory_op);
 HYPERCALL2(physdev_op);
+HYPERCALL3(vcpu_op);
 
 ENTRY(privcmd_call)
 	stmdb sp!, {r4}
-- 
1.7.2.5

[PATCH v4 5/7] xen/arm: XEN selects ARM_PSCI

[Stefano Stabellini]

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 arch/arm/Kconfig |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 2c3bdce..344e299 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1892,6 +1892,7 @@ config XEN
 	depends on ARM && AEABI && OF
 	depends on CPU_V7 && !CPU_V6
 	depends on !GENERIC_ATOMIC64
+	select ARM_PSCI
 	help
 	  Say Y if you want to run Linux in a Virtual Machine on Xen on ARM.
 
-- 
1.7.2.5

[PATCH v4 6/7] xenvm: add a simple PSCI node and a second cpu

[Stefano Stabellini]

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
CC: rob.herring@calxeda.com
CC: will.deacon@arm.com
CC: arnd@arndb.de
---
 arch/arm/boot/dts/xenvm-4.2.dts |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/xenvm-4.2.dts b/arch/arm/boot/dts/xenvm-4.2.dts
index ec3f952..3369151 100644
--- a/arch/arm/boot/dts/xenvm-4.2.dts
+++ b/arch/arm/boot/dts/xenvm-4.2.dts
@@ -29,6 +29,19 @@
 			compatible = "arm,cortex-a15";
 			reg = <0>;
 		};
+
+		cpu@1 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a15";
+			reg = <1>;
+		};
+	};
+
+	psci {
+		compatible      = "arm,psci";
+		method          = "hvc";
+		cpu_off         = <1>;
+		cpu_on          = <2>;
 	};
 
 	memory@80000000 {
-- 
1.7.2.5

[PATCH v4 7/7] mach-virt: support Xen hypercalls for shutdown and reboot

[Stefano Stabellini]

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
CC: Marc Zyngier <marc.zyngier@arm.com>
CC: will.deacon@arm.com
CC: arnd@arndb.de
CC: rob.herring@calxeda.com
---
 arch/arm/mach-virt/virt.c |   32 ++++++++++++++++++++++++++++++++
 1 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-virt/virt.c b/arch/arm/mach-virt/virt.c
index 528c05e..ba14321 100644
--- a/arch/arm/mach-virt/virt.c
+++ b/arch/arm/mach-virt/virt.c
@@ -23,13 +23,44 @@
 #include <linux/of_platform.h>
 #include <linux/smp.h>
 
+#include <xen/xen.h>
+#include <xen/interface/sched.h>
+
 #include <asm/arch_timer.h>
 #include <asm/mach/arch.h>
 #include <asm/mach/time.h>
+#include <asm/xen/hypercall.h>
+
+static void virt_restart(char str, const char *cmd)
+{
+#ifdef CONFIG_XEN
+	if (xen_domain()) {
+		struct sched_shutdown r = { .reason = SHUTDOWN_reboot };
+		int rc;
+		rc = HYPERVISOR_sched_op(SCHEDOP_shutdown, &r);
+		if (rc)
+			BUG();
+	}
+#endif
+}
+
+static void virt_power_off(void)
+{
+#ifdef CONFIG_XEN
+	if (xen_domain()) {
+		struct sched_shutdown r = { .reason = SHUTDOWN_poweroff };
+		int rc;
+		rc = HYPERVISOR_sched_op(SCHEDOP_shutdown, &r);
+		if (rc)
+			BUG();
+	}
+#endif
+}
 
 static void __init virt_init(void)
 {
 	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
+	pm_power_off = virt_power_off;
 }
 
 static void __init virt_timer_init(void)
@@ -52,4 +83,5 @@ DT_MACHINE_START(VIRT, "Dummy Virtual Machine")
 	.init_machine	= virt_init,
 	.smp		= smp_ops(virt_smp_ops),
 	.dt_compat	= virt_dt_match,
+	.restart	= virt_restart,
 MACHINE_END
-- 
1.7.2.5

Re: [PATCH v4 7/7] mach-virt: support Xen hypercalls for shutdown and reboot

[Stefano Stabellini]

On Wed, 24 Apr 2013, Stefano Stabellini wrote:
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> CC: Marc Zyngier <marc.zyngier@arm.com>
> CC: will.deacon@arm.com
> CC: arnd@arndb.de
> CC: rob.herring@calxeda.com

Thinking twice about this patch, a less intrusive alternative would be
to set arm_pm_restart and pm_power_off to two xen specific functions
later on from xen_guest_init.

>  arch/arm/mach-virt/virt.c |   32 ++++++++++++++++++++++++++++++++
>  1 files changed, 32 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-virt/virt.c b/arch/arm/mach-virt/virt.c
> index 528c05e..ba14321 100644
> --- a/arch/arm/mach-virt/virt.c
> +++ b/arch/arm/mach-virt/virt.c
> @@ -23,13 +23,44 @@
>  #include <linux/of_platform.h>
>  #include <linux/smp.h>
>  
> +#include <xen/xen.h>
> +#include <xen/interface/sched.h>
> +
>  #include <asm/arch_timer.h>
>  #include <asm/mach/arch.h>
>  #include <asm/mach/time.h>
> +#include <asm/xen/hypercall.h>
> +
> +static void virt_restart(char str, const char *cmd)
> +{
> +#ifdef CONFIG_XEN
> +	if (xen_domain()) {
> +		struct sched_shutdown r = { .reason = SHUTDOWN_reboot };
> +		int rc;
> +		rc = HYPERVISOR_sched_op(SCHEDOP_shutdown, &r);
> +		if (rc)
> +			BUG();
> +	}
> +#endif
> +}
> +
> +static void virt_power_off(void)
> +{
> +#ifdef CONFIG_XEN
> +	if (xen_domain()) {
> +		struct sched_shutdown r = { .reason = SHUTDOWN_poweroff };
> +		int rc;
> +		rc = HYPERVISOR_sched_op(SCHEDOP_shutdown, &r);
> +		if (rc)
> +			BUG();
> +	}
> +#endif
> +}
>  
>  static void __init virt_init(void)
>  {
>  	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
> +	pm_power_off = virt_power_off;
>  }
>  
>  static void __init virt_timer_init(void)
> @@ -52,4 +83,5 @@ DT_MACHINE_START(VIRT, "Dummy Virtual Machine")
>  	.init_machine	= virt_init,
>  	.smp		= smp_ops(virt_smp_ops),
>  	.dt_compat	= virt_dt_match,
> +	.restart	= virt_restart,
>  MACHINE_END
> -- 
> 1.7.2.5
> 

Re: [PATCH v4 7/7] mach-virt: support Xen hypercalls for shutdown and reboot

[Will Deacon]

On Wed, Apr 24, 2013 at 11:14:28PM +0100, Stefano Stabellini wrote:
> On Wed, 24 Apr 2013, Stefano Stabellini wrote:
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > CC: Marc Zyngier <marc.zyngier@arm.com>
> > CC: will.deacon@arm.com
> > CC: arnd@arndb.de
> > CC: rob.herring@calxeda.com
> 
> Thinking twice about this patch, a less intrusive alternative would be
> to set arm_pm_restart and pm_power_off to two xen specific functions
> later on from xen_guest_init.

Yes please. We'd like to remove mach-virt/* some day.

Will

Re: [PATCH v4 7/7] mach-virt: support Xen hypercalls for shutdown and reboot

[Stefano Stabellini]

On Thu, 25 Apr 2013, Will Deacon wrote:
> On Wed, Apr 24, 2013 at 11:14:28PM +0100, Stefano Stabellini wrote:
> > On Wed, 24 Apr 2013, Stefano Stabellini wrote:
> > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > > CC: Marc Zyngier <marc.zyngier@arm.com>
> > > CC: will.deacon@arm.com
> > > CC: arnd@arndb.de
> > > CC: rob.herring@calxeda.com
> > 
> > Thinking twice about this patch, a less intrusive alternative would be
> > to set arm_pm_restart and pm_power_off to two xen specific functions
> > later on from xen_guest_init.
> 
> Yes please. We'd like to remove mach-virt/* some day.

I'll do

Re: [PATCH v4 2/7] xen/arm: SMP support

[Ian Campbell]

On Wed, 2013-04-24 at 20:28 +0100, Stefano Stabellini wrote:
> Map vcpu_info using VCPUOP_register_vcpu_info on secondary cpus.
> 
> Call enable_percpu_irq on every cpu.
> 
> Changed in v2:
> - move the percpu variable argument fix to a separate patch;
> - remove unused variable.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> ---
>  arch/arm/xen/enlighten.c |   38 +++++++++++++++++++++++++++++++++++++-
>  1 files changed, 37 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> index 99ce189..94bbf3b 100644
> --- a/arch/arm/xen/enlighten.c
> +++ b/arch/arm/xen/enlighten.c
> @@ -2,6 +2,7 @@
>  #include <xen/events.h>
>  #include <xen/grant_table.h>
>  #include <xen/hvm.h>
> +#include <xen/interface/vcpu.h>
>  #include <xen/interface/xen.h>
>  #include <xen/interface/memory.h>
>  #include <xen/interface/hvm/params.h>
> @@ -32,6 +33,7 @@ struct shared_info xen_dummy_shared_info;
>  struct shared_info *HYPERVISOR_shared_info = (void *)&xen_dummy_shared_info;
>  
>  DEFINE_PER_CPU(struct vcpu_info *, xen_vcpu);
> +DEFINE_PER_CPU(struct vcpu_info, xen_vcpu_info);

>  /* These are unused until we support booting "pre-ballooned" */
>  unsigned long xen_released_pages;
> @@ -148,6 +150,32 @@ int xen_unmap_domain_mfn_range(struct vm_area_struct *vma,
>  }
>  EXPORT_SYMBOL_GPL(xen_unmap_domain_mfn_range);
>  
> +static int __init xen_secondary_init(unsigned int cpu)
> +{
> +	struct vcpu_register_vcpu_info info;
> +	struct vcpu_info *vcpup;
> +	int err;
> +
> +	if (cpu == 0)
> +		return 0;
> +
> +	pr_info("Xen: initializing cpu%d\n", cpu);
> +	vcpup = &per_cpu(xen_vcpu_info, cpu);
> +
> +	info.mfn = __pa(vcpup) >> PAGE_SHIFT;
> +	info.offset = offset_in_page(vcpup);

Do you need to somehow guarantee it's not going to cross a page? Maybe
standard C alignment rules make that the case?

> @@ -216,6 +245,8 @@ static int __init xen_guest_init(void)
>  	 * is required to use VCPUOP_register_vcpu_info to place vcpu info
>  	 * for secondary CPUs as they are brought up. */
>  	per_cpu(xen_vcpu, 0) = &HYPERVISOR_shared_info->vcpu_info[0];
> +	for_each_online_cpu(i)
> +		xen_secondary_init(i);
>  
>  	gnttab_init();
>  	if (!xen_initial_domain())
[...]
> @@ -244,7 +280,7 @@ static int __init xen_init_events(void)
>  		return -EINVAL;
>  	}
>  
> -	enable_percpu_irq(xen_events_irq, 0);
> +	on_each_cpu(xen_percpu_enable_events, NULL, 0);

It feels like there ought to be some sort of per-cpu bringup callback
which takes care of these dynamically. Maybe that doesn't matter until
we get vcpu hotplug going?

>  
>  	return 0;
>  }

Re: [PATCH v4 4/7] xen/arm: implement HYPERVISOR_vcpu_op

[Ian Campbell]

On Wed, 2013-04-24 at 20:28 +0100, Stefano Stabellini wrote:
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

Reviewed-by: Ian Campbell <ian.campbell@citrix.com>

> ---
>  arch/arm/include/asm/xen/hypercall.h |    1 +
>  arch/arm/xen/enlighten.c             |    1 +
>  arch/arm/xen/hypercall.S             |    1 +
>  3 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/include/asm/xen/hypercall.h b/arch/arm/include/asm/xen/hypercall.h
> index 8a82325..799f42e 100644
> --- a/arch/arm/include/asm/xen/hypercall.h
> +++ b/arch/arm/include/asm/xen/hypercall.h
> @@ -46,6 +46,7 @@ int HYPERVISOR_event_channel_op(int cmd, void *arg);
>  unsigned long HYPERVISOR_hvm_op(int op, void *arg);
>  int HYPERVISOR_memory_op(unsigned int cmd, void *arg);
>  int HYPERVISOR_physdev_op(int cmd, void *arg);
> +int HYPERVISOR_vcpu_op(int cmd, int vcpuid, void *extra_args);
>  
>  static inline void
>  MULTI_update_va_mapping(struct multicall_entry *mcl, unsigned long va,
> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> index 94bbf3b..b002822 100644
> --- a/arch/arm/xen/enlighten.c
> +++ b/arch/arm/xen/enlighten.c
> @@ -295,4 +295,5 @@ EXPORT_SYMBOL_GPL(HYPERVISOR_sched_op);
>  EXPORT_SYMBOL_GPL(HYPERVISOR_hvm_op);
>  EXPORT_SYMBOL_GPL(HYPERVISOR_memory_op);
>  EXPORT_SYMBOL_GPL(HYPERVISOR_physdev_op);
> +EXPORT_SYMBOL_GPL(HYPERVISOR_vcpu_op);
>  EXPORT_SYMBOL_GPL(privcmd_call);
> diff --git a/arch/arm/xen/hypercall.S b/arch/arm/xen/hypercall.S
> index 71f7239..199cb2d 100644
> --- a/arch/arm/xen/hypercall.S
> +++ b/arch/arm/xen/hypercall.S
> @@ -87,6 +87,7 @@ HYPERCALL2(event_channel_op);
>  HYPERCALL2(hvm_op);
>  HYPERCALL2(memory_op);
>  HYPERCALL2(physdev_op);
> +HYPERCALL3(vcpu_op);
>  
>  ENTRY(privcmd_call)
>  	stmdb sp!, {r4}

Re: [PATCH v4 1/7] xen/arm: actually pass a non-NULL percpu pointer to request_percpu_irq

[Ian Campbell]

On Wed, 2013-04-24 at 20:28 +0100, Stefano Stabellini wrote:
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

Reviewed-by: Ian Campbell <ian.camjpbell@citrix.com>

> CC: stable@vger.kernel.org
> ---
>  arch/arm/xen/enlighten.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> index 8dc0605..99ce189 100644
> --- a/arch/arm/xen/enlighten.c
> +++ b/arch/arm/xen/enlighten.c
> @@ -239,7 +239,7 @@ static int __init xen_init_events(void)
>  	xen_init_IRQ();
>  
>  	if (request_percpu_irq(xen_events_irq, xen_arm_callback,
> -			"events", xen_vcpu)) {
> +			"events", &xen_vcpu)) {
>  		pr_err("Error requesting IRQ %d\n", xen_events_irq);
>  		return -EINVAL;
>  	}

Re: [PATCH v4 2/7] xen/arm: SMP support

[Stefano Stabellini]

On Thu, 25 Apr 2013, Ian Campbell wrote:
> On Wed, 2013-04-24 at 20:28 +0100, Stefano Stabellini wrote:
> > Map vcpu_info using VCPUOP_register_vcpu_info on secondary cpus.
> > 
> > Call enable_percpu_irq on every cpu.
> > 
> > Changed in v2:
> > - move the percpu variable argument fix to a separate patch;
> > - remove unused variable.
> > 
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > ---
> >  arch/arm/xen/enlighten.c |   38 +++++++++++++++++++++++++++++++++++++-
> >  1 files changed, 37 insertions(+), 1 deletions(-)
> > 
> > diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> > index 99ce189..94bbf3b 100644
> > --- a/arch/arm/xen/enlighten.c
> > +++ b/arch/arm/xen/enlighten.c
> > @@ -2,6 +2,7 @@
> >  #include <xen/events.h>
> >  #include <xen/grant_table.h>
> >  #include <xen/hvm.h>
> > +#include <xen/interface/vcpu.h>
> >  #include <xen/interface/xen.h>
> >  #include <xen/interface/memory.h>
> >  #include <xen/interface/hvm/params.h>
> > @@ -32,6 +33,7 @@ struct shared_info xen_dummy_shared_info;
> >  struct shared_info *HYPERVISOR_shared_info = (void *)&xen_dummy_shared_info;
> >  
> >  DEFINE_PER_CPU(struct vcpu_info *, xen_vcpu);
> > +DEFINE_PER_CPU(struct vcpu_info, xen_vcpu_info);
> 
> >  /* These are unused until we support booting "pre-ballooned" */
> >  unsigned long xen_released_pages;
> > @@ -148,6 +150,32 @@ int xen_unmap_domain_mfn_range(struct vm_area_struct *vma,
> >  }
> >  EXPORT_SYMBOL_GPL(xen_unmap_domain_mfn_range);
> >  
> > +static int __init xen_secondary_init(unsigned int cpu)
> > +{
> > +	struct vcpu_register_vcpu_info info;
> > +	struct vcpu_info *vcpup;
> > +	int err;
> > +
> > +	if (cpu == 0)
> > +		return 0;
> > +
> > +	pr_info("Xen: initializing cpu%d\n", cpu);
> > +	vcpup = &per_cpu(xen_vcpu_info, cpu);
> > +
> > +	info.mfn = __pa(vcpup) >> PAGE_SHIFT;
> > +	info.offset = offset_in_page(vcpup);
> 
> Do you need to somehow guarantee it's not going to cross a page? Maybe
> standard C alignment rules make that the case?

That is a good question, I don't think that DEFINE_PER_CPU makes any
alignment guarantees (standard C alignment aside).
I'll switch to __alloc_percpu that allows me to specify an alignment and
has the advantage of being dynamically allocated.


> > @@ -216,6 +245,8 @@ static int __init xen_guest_init(void)
> >  	 * is required to use VCPUOP_register_vcpu_info to place vcpu info
> >  	 * for secondary CPUs as they are brought up. */
> >  	per_cpu(xen_vcpu, 0) = &HYPERVISOR_shared_info->vcpu_info[0];
> > +	for_each_online_cpu(i)
> > +		xen_secondary_init(i);
> >  
> >  	gnttab_init();
> >  	if (!xen_initial_domain())
> [...]
> > @@ -244,7 +280,7 @@ static int __init xen_init_events(void)
> >  		return -EINVAL;
> >  	}
> >  
> > -	enable_percpu_irq(xen_events_irq, 0);
> > +	on_each_cpu(xen_percpu_enable_events, NULL, 0);
> 
> It feels like there ought to be some sort of per-cpu bringup callback
> which takes care of these dynamically. Maybe that doesn't matter until
> we get vcpu hotplug going?

I suspect there isn't one, considering that on_each_cpu is also used by 
kvm_vgic_hyp_init, kvm_timer_hyp_init and others.

Re: [PATCH v4 2/7] xen/arm: SMP support

[Ian Campbell]

> > > @@ -216,6 +245,8 @@ static int __init xen_guest_init(void)
> > >  	 * is required to use VCPUOP_register_vcpu_info to place vcpu info
> > >  	 * for secondary CPUs as they are brought up. */
> > >  	per_cpu(xen_vcpu, 0) = &HYPERVISOR_shared_info->vcpu_info[0];
> > > +	for_each_online_cpu(i)
> > > +		xen_secondary_init(i);
> > >  
> > >  	gnttab_init();
> > >  	if (!xen_initial_domain())
> > [...]
> > > @@ -244,7 +280,7 @@ static int __init xen_init_events(void)
> > >  		return -EINVAL;
> > >  	}
> > >  
> > > -	enable_percpu_irq(xen_events_irq, 0);
> > > +	on_each_cpu(xen_percpu_enable_events, NULL, 0);
> > 
> > It feels like there ought to be some sort of per-cpu bringup callback
> > which takes care of these dynamically. Maybe that doesn't matter until
> > we get vcpu hotplug going?
> 
> I suspect there isn't one, considering that on_each_cpu is also used by 
> kvm_vgic_hyp_init, kvm_timer_hyp_init and others.

Could we use cpu_notifiers for this?

Re: [PATCH v4 2/7] xen/arm: SMP support

[Stefano Stabellini]

On Thu, 25 Apr 2013, Ian Campbell wrote:
> > > > @@ -216,6 +245,8 @@ static int __init xen_guest_init(void)
> > > >  	 * is required to use VCPUOP_register_vcpu_info to place vcpu info
> > > >  	 * for secondary CPUs as they are brought up. */
> > > >  	per_cpu(xen_vcpu, 0) = &HYPERVISOR_shared_info->vcpu_info[0];
> > > > +	for_each_online_cpu(i)
> > > > +		xen_secondary_init(i);
> > > >  
> > > >  	gnttab_init();
> > > >  	if (!xen_initial_domain())
> > > [...]
> > > > @@ -244,7 +280,7 @@ static int __init xen_init_events(void)
> > > >  		return -EINVAL;
> > > >  	}
> > > >  
> > > > -	enable_percpu_irq(xen_events_irq, 0);
> > > > +	on_each_cpu(xen_percpu_enable_events, NULL, 0);
> > > 
> > > It feels like there ought to be some sort of per-cpu bringup callback
> > > which takes care of these dynamically. Maybe that doesn't matter until
> > > we get vcpu hotplug going?
> > 
> > I suspect there isn't one, considering that on_each_cpu is also used by 
> > kvm_vgic_hyp_init, kvm_timer_hyp_init and others.
> 
> Could we use cpu_notifiers for this?

cpu_notifiers are for cpu hotplug, not for secondary cpu bringup

Re: [PATCH v4 2/7] xen/arm: SMP support

[Ian Campbell]

On Thu, 2013-04-25 at 19:45 +0100, Stefano Stabellini wrote:
> On Thu, 25 Apr 2013, Ian Campbell wrote:
> > > > > @@ -216,6 +245,8 @@ static int __init xen_guest_init(void)
> > > > >  	 * is required to use VCPUOP_register_vcpu_info to place vcpu info
> > > > >  	 * for secondary CPUs as they are brought up. */
> > > > >  	per_cpu(xen_vcpu, 0) = &HYPERVISOR_shared_info->vcpu_info[0];
> > > > > +	for_each_online_cpu(i)
> > > > > +		xen_secondary_init(i);
> > > > >  
> > > > >  	gnttab_init();
> > > > >  	if (!xen_initial_domain())
> > > > [...]
> > > > > @@ -244,7 +280,7 @@ static int __init xen_init_events(void)
> > > > >  		return -EINVAL;
> > > > >  	}
> > > > >  
> > > > > -	enable_percpu_irq(xen_events_irq, 0);
> > > > > +	on_each_cpu(xen_percpu_enable_events, NULL, 0);
> > > > 
> > > > It feels like there ought to be some sort of per-cpu bringup callback
> > > > which takes care of these dynamically. Maybe that doesn't matter until
> > > > we get vcpu hotplug going?
> > > 
> > > I suspect there isn't one, considering that on_each_cpu is also used by 
> > > kvm_vgic_hyp_init, kvm_timer_hyp_init and others.
> > 
> > Could we use cpu_notifiers for this?
> 
> cpu_notifiers are for cpu hotplug, not for secondary cpu bringup

Are you sure they don't also trigger during bringup, because the
distinction is a little bit academic...

Ian.

Re: [PATCH v4 2/7] xen/arm: SMP support

[Stefano Stabellini]

On Fri, 26 Apr 2013, Ian Campbell wrote:
> On Thu, 2013-04-25 at 19:45 +0100, Stefano Stabellini wrote:
> > On Thu, 25 Apr 2013, Ian Campbell wrote:
> > > > > > @@ -216,6 +245,8 @@ static int __init xen_guest_init(void)
> > > > > >  	 * is required to use VCPUOP_register_vcpu_info to place vcpu info
> > > > > >  	 * for secondary CPUs as they are brought up. */
> > > > > >  	per_cpu(xen_vcpu, 0) = &HYPERVISOR_shared_info->vcpu_info[0];
> > > > > > +	for_each_online_cpu(i)
> > > > > > +		xen_secondary_init(i);
> > > > > >  
> > > > > >  	gnttab_init();
> > > > > >  	if (!xen_initial_domain())
> > > > > [...]
> > > > > > @@ -244,7 +280,7 @@ static int __init xen_init_events(void)
> > > > > >  		return -EINVAL;
> > > > > >  	}
> > > > > >  
> > > > > > -	enable_percpu_irq(xen_events_irq, 0);
> > > > > > +	on_each_cpu(xen_percpu_enable_events, NULL, 0);
> > > > > 
> > > > > It feels like there ought to be some sort of per-cpu bringup callback
> > > > > which takes care of these dynamically. Maybe that doesn't matter until
> > > > > we get vcpu hotplug going?
> > > > 
> > > > I suspect there isn't one, considering that on_each_cpu is also used by 
> > > > kvm_vgic_hyp_init, kvm_timer_hyp_init and others.
> > > 
> > > Could we use cpu_notifiers for this?
> > 
> > cpu_notifiers are for cpu hotplug, not for secondary cpu bringup
> 
> Are you sure they don't also trigger during bringup, because the
> distinction is a little bit academic...

My mistake, they do run on secondary cpus, but not in our case because
xen_guest_init is called *after* cpu_notifiers are called.
So, they are too early for Xen.

Re: [PATCH v4 2/7] xen/arm: SMP support

[Ian Campbell]

On Fri, 2013-04-26 at 11:27 +0100, Stefano Stabellini wrote:
> On Fri, 26 Apr 2013, Ian Campbell wrote:
> > On Thu, 2013-04-25 at 19:45 +0100, Stefano Stabellini wrote:
> > > On Thu, 25 Apr 2013, Ian Campbell wrote:
> > > > > > > @@ -216,6 +245,8 @@ static int __init xen_guest_init(void)
> > > > > > >  	 * is required to use VCPUOP_register_vcpu_info to place vcpu info
> > > > > > >  	 * for secondary CPUs as they are brought up. */
> > > > > > >  	per_cpu(xen_vcpu, 0) = &HYPERVISOR_shared_info->vcpu_info[0];
> > > > > > > +	for_each_online_cpu(i)
> > > > > > > +		xen_secondary_init(i);
> > > > > > >  
> > > > > > >  	gnttab_init();
> > > > > > >  	if (!xen_initial_domain())
> > > > > > [...]
> > > > > > > @@ -244,7 +280,7 @@ static int __init xen_init_events(void)
> > > > > > >  		return -EINVAL;
> > > > > > >  	}
> > > > > > >  
> > > > > > > -	enable_percpu_irq(xen_events_irq, 0);
> > > > > > > +	on_each_cpu(xen_percpu_enable_events, NULL, 0);
> > > > > > 
> > > > > > It feels like there ought to be some sort of per-cpu bringup callback
> > > > > > which takes care of these dynamically. Maybe that doesn't matter until
> > > > > > we get vcpu hotplug going?
> > > > > 
> > > > > I suspect there isn't one, considering that on_each_cpu is also used by 
> > > > > kvm_vgic_hyp_init, kvm_timer_hyp_init and others.
> > > > 
> > > > Could we use cpu_notifiers for this?
> > > 
> > > cpu_notifiers are for cpu hotplug, not for secondary cpu bringup
> > 
> > Are you sure they don't also trigger during bringup, because the
> > distinction is a little bit academic...
> 
> My mistake, they do run on secondary cpus, but not in our case because
> xen_guest_init is called *after* cpu_notifiers are called.
> So, they are too early for Xen.

Another reason to consider calling xen_guest_init much earlier then
IMHO. although we can live with the solution you have now I suppose.

Ian.

Re: [PATCH v4 2/7] xen/arm: SMP support

[Stefano Stabellini]

On Fri, 26 Apr 2013, Ian Campbell wrote:
> On Fri, 2013-04-26 at 11:27 +0100, Stefano Stabellini wrote:
> > On Fri, 26 Apr 2013, Ian Campbell wrote:
> > > On Thu, 2013-04-25 at 19:45 +0100, Stefano Stabellini wrote:
> > > > On Thu, 25 Apr 2013, Ian Campbell wrote:
> > > > > > > > @@ -216,6 +245,8 @@ static int __init xen_guest_init(void)
> > > > > > > >  	 * is required to use VCPUOP_register_vcpu_info to place vcpu info
> > > > > > > >  	 * for secondary CPUs as they are brought up. */
> > > > > > > >  	per_cpu(xen_vcpu, 0) = &HYPERVISOR_shared_info->vcpu_info[0];
> > > > > > > > +	for_each_online_cpu(i)
> > > > > > > > +		xen_secondary_init(i);
> > > > > > > >  
> > > > > > > >  	gnttab_init();
> > > > > > > >  	if (!xen_initial_domain())
> > > > > > > [...]
> > > > > > > > @@ -244,7 +280,7 @@ static int __init xen_init_events(void)
> > > > > > > >  		return -EINVAL;
> > > > > > > >  	}
> > > > > > > >  
> > > > > > > > -	enable_percpu_irq(xen_events_irq, 0);
> > > > > > > > +	on_each_cpu(xen_percpu_enable_events, NULL, 0);
> > > > > > > 
> > > > > > > It feels like there ought to be some sort of per-cpu bringup callback
> > > > > > > which takes care of these dynamically. Maybe that doesn't matter until
> > > > > > > we get vcpu hotplug going?
> > > > > > 
> > > > > > I suspect there isn't one, considering that on_each_cpu is also used by 
> > > > > > kvm_vgic_hyp_init, kvm_timer_hyp_init and others.
> > > > > 
> > > > > Could we use cpu_notifiers for this?
> > > > 
> > > > cpu_notifiers are for cpu hotplug, not for secondary cpu bringup
> > > 
> > > Are you sure they don't also trigger during bringup, because the
> > > distinction is a little bit academic...
> > 
> > My mistake, they do run on secondary cpus, but not in our case because
> > xen_guest_init is called *after* cpu_notifiers are called.
> > So, they are too early for Xen.
> 
> Another reason to consider calling xen_guest_init much earlier then
> IMHO. although we can live with the solution you have now I suppose.

Yeah, moving the call to xen_guest_init earlier might be a good idea,
but I wouldn't want to do it in this patch series, at this point of the
Linux release cycle.