2013-04-24 19:25:47

by Stefano Stabellini

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

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


2013-04-24 19:44:31

by Stefano Stabellini

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

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

Changes in v4:
- update the dts Makefile too.

Signed-off-by: Stefano Stabellini <[email protected]>
CC: Marc Zyngier <[email protected]>
CC: [email protected]
CC: [email protected]
CC: [email protected]
---
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

2013-04-24 19:44:43

by Stefano Stabellini

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

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 <[email protected]>
---
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

2013-04-24 19:45:00

by Stefano Stabellini

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

Signed-off-by: Stefano Stabellini <[email protected]>
---
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

2013-04-24 19:44:59

by Stefano Stabellini

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

Signed-off-by: Stefano Stabellini <[email protected]>
CC: [email protected]
---
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

2013-04-24 19:44:29

by Stefano Stabellini

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

Signed-off-by: Stefano Stabellini <[email protected]>
CC: Marc Zyngier <[email protected]>
CC: [email protected]
CC: [email protected]
CC: [email protected]
---
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

2013-04-24 19:46:27

by Stefano Stabellini

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

Signed-off-by: Stefano Stabellini <[email protected]>
---
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

2013-04-24 19:46:40

by Stefano Stabellini

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

Signed-off-by: Stefano Stabellini <[email protected]>
Acked-by: Marc Zyngier <[email protected]>
CC: [email protected]
CC: [email protected]
CC: [email protected]
---
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

2013-04-24 22:14:34

by Stefano Stabellini

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

On Wed, 24 Apr 2013, Stefano Stabellini wrote:
> Signed-off-by: Stefano Stabellini <[email protected]>
> CC: Marc Zyngier <[email protected]>
> CC: [email protected]
> CC: [email protected]
> CC: [email protected]

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
>

2013-04-25 08:55:17

by Will Deacon

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

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 <[email protected]>
> > CC: Marc Zyngier <[email protected]>
> > CC: [email protected]
> > CC: [email protected]
> > CC: [email protected]
>
> 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

2013-04-25 10:14:49

by Stefano Stabellini

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

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 <[email protected]>
> > > CC: Marc Zyngier <[email protected]>
> > > CC: [email protected]
> > > CC: [email protected]
> > > CC: [email protected]
> >
> > 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

2013-04-25 13:40:25

by Ian Campbell

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

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 <[email protected]>
> ---
> 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;
> }

2013-04-25 13:41:22

by Ian Campbell

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

On Wed, 2013-04-24 at 20:28 +0100, Stefano Stabellini wrote:
> Signed-off-by: Stefano Stabellini <[email protected]>

Reviewed-by: Ian Campbell <[email protected]>

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

2013-04-25 13:48:44

by Ian Campbell

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

On Wed, 2013-04-24 at 20:28 +0100, Stefano Stabellini wrote:
> Signed-off-by: Stefano Stabellini <[email protected]>

Reviewed-by: Ian Campbell <[email protected]>

> CC: [email protected]
> ---
> 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;
> }

2013-04-25 16:49:28

by Stefano Stabellini

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

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 <[email protected]>
> > ---
> > 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.

2013-04-25 16:59:48

by Ian Campbell

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


> > > @@ -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?

2013-04-25 18:46:01

by Stefano Stabellini

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

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

2013-04-26 08:51:49

by Ian Campbell

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

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.

2013-04-26 10:27:08

by Stefano Stabellini

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

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.

2013-04-26 10:30:59

by Ian Campbell

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

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.

2013-04-26 10:33:57

by Stefano Stabellini

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

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.